| Message ID | 1418241578-17049-1-git-send-email-alexanders83@web.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/10/2014 08:59 PM, Alexander Stein wrote: > This adds device tree support for the AC97 controller. It uses the > soc-ac97link bindings, but actually only ac97-reset is used. > > Signed-off-by: Alexander Stein <alexanders83@web.de> > --- > Changes in v2: > * It now uses the soc-ac97link bindings (partly) instead of defining its own. > This should ease the transition to ASoC once a new driver has been written. Partly? How do they differ? And why do they differ? The devicetree should describe the hardware, which I guess is the same in both cases. - Lars
On Wednesday 10 December 2014, 21:04:54 wrote Lars-Peter Clausen: > On 12/10/2014 08:59 PM, Alexander Stein wrote: > > This adds device tree support for the AC97 controller. It uses the > > soc-ac97link bindings, but actually only ac97-reset is used. > > > > Signed-off-by: Alexander Stein <alexanders83@web.de> > > --- > > Changes in v2: > > * It now uses the soc-ac97link bindings (partly) instead of defining its own. > > This should ease the transition to ASoC once a new driver has been written. > > Partly? How do they differ? And why do they differ? The devicetree should > describe the hardware, which I guess is the same in both cases. The devicetree is defined the same way, but not all set properties are used. The soc-ac97link binding defines 3 GPIOs where 2 are used in conjunction with different pinctl modes for different reset modes. The rather old atmel_ac97c driver only supports reset per Reset-GPIO which is the 3rd one given in devicetree. Because this driver only uses the 3rd GPIO I've written 'partly'. Best regards, Alexander
Hi, On 10/12/2014 at 20:59:36 +0100, Alexander Stein wrote : > +#ifdef CONFIG_OF > +static const struct of_device_id atmel_ac97c_dt_ids[] = { > + { .compatible = "atmel,atmel_ac97c", }, I would use atmel,at91sam9263-ac97c, we never now, maybe they will have another ac97 IP at some point in time ;) > + { } > +}; > +MODULE_DEVICE_TABLE(of, atmel_ac97c_dt_ids); > +
On 19/12/2014 at 21:42:54 +0100, Alexandre Belloni wrote : > Hi, > > On 10/12/2014 at 20:59:36 +0100, Alexander Stein wrote : > > +#ifdef CONFIG_OF > > +static const struct of_device_id atmel_ac97c_dt_ids[] = { > > + { .compatible = "atmel,atmel_ac97c", }, > > I would use atmel,at91sam9263-ac97c, we never now, maybe they will have > another ac97 IP at some point in time ;) > This applies for the other patches, else I'm fine with the series. Maybe you could add CONFIG_SND_ATMEL_AC97C=y in at91_dt_defconfig as all the other defconfigs have disappeared.
On Friday 19 December 2014, 21:47:24 wrote Alexandre Belloni: > On 19/12/2014 at 21:42:54 +0100, Alexandre Belloni wrote : > > Hi, > > > > On 10/12/2014 at 20:59:36 +0100, Alexander Stein wrote : > > > +#ifdef CONFIG_OF > > > +static const struct of_device_id atmel_ac97c_dt_ids[] = { > > > + { .compatible = "atmel,atmel_ac97c", }, > > > > I would use atmel,at91sam9263-ac97c, we never now, maybe they will have > > another ac97 IP at some point in time ;) > > > > This applies for the other patches, else I'm fine with the series. Is this an > Acked-By Alexandre Belloni <alexandre.belloni@free-electrons.com> ? > Maybe you could add CONFIG_SND_ATMEL_AC97C=y in at91_dt_defconfig as all > the other defconfigs have disappeared. Sure, I'll add both suggestions and respin nother round next week. Best regards, Alexander
On Friday 19 December 2014, 21:47:24 wrote Alexandre Belloni: > On 19/12/2014 at 21:42:54 +0100, Alexandre Belloni wrote : > > Hi, > > > > On 10/12/2014 at 20:59:36 +0100, Alexander Stein wrote : > > > +#ifdef CONFIG_OF > > > +static const struct of_device_id atmel_ac97c_dt_ids[] = { > > > + { .compatible = "atmel,atmel_ac97c", }, > > > > I would use atmel,at91sam9263-ac97c, we never now, maybe they will have > > another ac97 IP at some point in time ;) > > > > This applies for the other patches, else I'm fine with the series. > > Maybe you could add CONFIG_SND_ATMEL_AC97C=y in at91_dt_defconfig as all > the other defconfigs have disappeared. BTW: Shouldn't this be rather CONFIG_SND_ATMEL_AC97C=m since it is only applicable on a single microcontroller (AT91SAM9263). There is no need to compile this driver into the kernel on all at91 variants. Best regards, Alexander
On 19/12/2014 at 21:51:01 +0100, Alexander Stein wrote : > On Friday 19 December 2014, 21:47:24 wrote Alexandre Belloni: > > On 19/12/2014 at 21:42:54 +0100, Alexandre Belloni wrote : > > > Hi, > > > > > > On 10/12/2014 at 20:59:36 +0100, Alexander Stein wrote : > > > > +#ifdef CONFIG_OF > > > > +static const struct of_device_id atmel_ac97c_dt_ids[] = { > > > > + { .compatible = "atmel,atmel_ac97c", }, > > > > > > I would use atmel,at91sam9263-ac97c, we never now, maybe they will have > > > another ac97 IP at some point in time ;) > > > > > > > This applies for the other patches, else I'm fine with the series. > > Is this an > > Acked-By Alexandre Belloni <alexandre.belloni@free-electrons.com> > ? Yes :) > > > Maybe you could add CONFIG_SND_ATMEL_AC97C=y in at91_dt_defconfig as all > > the other defconfigs have disappeared. > > Sure, I'll add both suggestions and respin nother round next week. > > Best regards, > Alexander >
On 19/12/2014 at 22:05:31 +0100, Alexander Stein wrote : > On Friday 19 December 2014, 21:47:24 wrote Alexandre Belloni: > > On 19/12/2014 at 21:42:54 +0100, Alexandre Belloni wrote : > > > Hi, > > > > > > On 10/12/2014 at 20:59:36 +0100, Alexander Stein wrote : > > > > +#ifdef CONFIG_OF > > > > +static const struct of_device_id atmel_ac97c_dt_ids[] = { > > > > + { .compatible = "atmel,atmel_ac97c", }, > > > > > > I would use atmel,at91sam9263-ac97c, we never now, maybe they will have > > > another ac97 IP at some point in time ;) > > > > > > > This applies for the other patches, else I'm fine with the series. > > > > Maybe you could add CONFIG_SND_ATMEL_AC97C=y in at91_dt_defconfig as all > > the other defconfigs have disappeared. > > BTW: Shouldn't this be rather CONFIG_SND_ATMEL_AC97C=m since it is only applicable on a single microcontroller (AT91SAM9263). There is no need to compile this driver into the kernel on all at91 variants. > I think compiling statically is preferred by Nicolas (added in Cc). It is actually also applicable to sam9g45 and sam9rl.
diff --git a/sound/atmel/ac97c.c b/sound/atmel/ac97c.c index cb44c74..73b61b2 100644 --- a/sound/atmel/ac97c.c +++ b/sound/atmel/ac97c.c @@ -22,6 +22,9 @@ #include <linux/gpio.h> #include <linux/types.h> #include <linux/io.h> +#include <linux/of.h> +#include <linux/of_gpio.h> +#include <linux/of_device.h> #include <sound/core.h> #include <sound/initval.h> @@ -902,6 +905,40 @@ static void atmel_ac97c_reset(struct atmel_ac97c *chip) } } +#ifdef CONFIG_OF +static const struct of_device_id atmel_ac97c_dt_ids[] = { + { .compatible = "atmel,atmel_ac97c", }, + { } +}; +MODULE_DEVICE_TABLE(of, atmel_ac97c_dt_ids); + +static struct ac97c_platform_data *atmel_ac97c_probe_dt(struct device *dev) +{ + struct ac97c_platform_data *pdata; + struct device_node *node = dev->of_node; + const struct of_device_id *match; + + if (!node) { + dev_err(dev, "Device does not have associated DT data\n"); + return ERR_PTR(-EINVAL); + } + + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return ERR_PTR(-ENOMEM); + + pdata->reset_pin = of_get_named_gpio(dev->of_node, "ac97-gpios", 2); + + return pdata; +} +#else +static struct ac97c_platform_data *atmel_ac97c_probe_dt(struct device *dev) +{ + dev_err(dev, "no platform data defined\n"); + return ERR_PTR(-ENXIO); +} +#endif + static int atmel_ac97c_probe(struct platform_device *pdev) { struct snd_card *card; @@ -922,10 +959,11 @@ static int atmel_ac97c_probe(struct platform_device *pdev) return -ENXIO; } - pdata = pdev->dev.platform_data; + pdata = dev_get_platdata(&pdev->dev); if (!pdata) { - dev_dbg(&pdev->dev, "no platform data\n"); - return -ENXIO; + pdata = atmel_ac97c_probe_dt(&pdev->dev); + if (IS_ERR(pdata)) + return PTR_ERR(pdata); } irq = platform_get_irq(pdev, 0); @@ -1205,6 +1243,7 @@ static struct platform_driver atmel_ac97c_driver = { .name = "atmel_ac97c", .owner = THIS_MODULE, .pm = ATMEL_AC97C_PM_OPS, + .of_match_table = of_match_ptr(atmel_ac97c_dt_ids), }, }; module_platform_driver(atmel_ac97c_driver);
This adds device tree support for the AC97 controller. It uses the soc-ac97link bindings, but actually only ac97-reset is used. Signed-off-by: Alexander Stein <alexanders83@web.de> --- Changes in v2: * It now uses the soc-ac97link bindings (partly) instead of defining its own. This should ease the transition to ASoC once a new driver has been written. sound/atmel/ac97c.c | 45 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-)