Re: [PATCH 2/3] sound/soc/lapis: add machine driver

From: Mark Brown
Date: Mon Nov 21 2011 - 06:34:36 EST


On Mon, Nov 21, 2011 at 01:08:51PM +0900, Tomoya MORINAGA wrote:
> Add machine driver for LAPIS Semiconductor ML7213 IOH.

This needs to come after you add the base platform support in the series
- clearly this won't work without the CPU side drivers it depends on.

As with the CODEC driver you really should be looking at the standards
for modern drivers and following them.

> @@ -60,6 +60,7 @@ source "sound/soc/s6000/Kconfig"
> source "sound/soc/sh/Kconfig"
> source "sound/soc/tegra/Kconfig"
> source "sound/soc/txx9/Kconfig"
> +source "sound/soc/lapis/Kconfig"

Keep the Makefile and Kconfig entries sorted.

> +static struct snd_soc_dai_link ioh_i2s_dai = {
> + .name = "I2S",
> + .stream_name = "I2S HiFi",
> + .cpu_dai_name = "ml7213ioh-i2s",
> + .codec_dai_name = "ml7213ioh-hifi",

I'm very surprised this works... If nothing else I'd expec the CODEC
DAI to have a different name to the CPU.

> +static int __init ioh_i2s_probe(struct platform_device *pdev)
> +{
> + int ret;
> +
> + soc_pdev = platform_device_alloc("soc-audio", -1);
> + if (!soc_pdev)
> + return -ENOMEM;

You should register the card as a standard platform device for new
drivers and use snd_soc_register_card().

> +static struct platform_driver ioh_i2s_driver = {
> + .remove = ioh_i2s_remove,
> + .driver = {
> + .name = "ml7213ioh-machine",
> + .owner = THIS_MODULE,
> + },
> +};

You're already doing the platform device bit so it should be be a case
of changing the body of probe() and remove().

> +static int __init ioh_i2s_init(void)
> +{
> + return platform_driver_probe(&ioh_i2s_driver,
> + ioh_i2s_probe);
> +}

Just make this a standard platform_driver_register().
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/