[alsa-devel] [PATCH 1/5] ASoC: mc13783: Use module_platform_driver_probe()
mc13783-codec is probed only by MC13XXX MFD core driver so use module_platform_driver_probe().
Signed-off-by: Alexander Shiyan shc_work@mail.ru --- sound/soc/codecs/mc13783.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/sound/soc/codecs/mc13783.c b/sound/soc/codecs/mc13783.c index bae6016..8ab9668 100644 --- a/sound/soc/codecs/mc13783.c +++ b/sound/soc/codecs/mc13783.c @@ -750,7 +750,7 @@ static struct snd_soc_codec_driver soc_codec_dev_mc13783 = { .num_dapm_routes = ARRAY_SIZE(mc13783_routes), };
-static int mc13783_codec_probe(struct platform_device *pdev) +static int __init mc13783_codec_probe(struct platform_device *pdev) { struct mc13xxx *mc13xxx; struct mc13783_priv *priv; @@ -804,11 +804,9 @@ static struct platform_driver mc13783_codec_driver = { .name = "mc13783-codec", .owner = THIS_MODULE, }, - .probe = mc13783_codec_probe, .remove = mc13783_codec_remove, }; - -module_platform_driver(mc13783_codec_driver); +module_platform_driver_probe(mc13783_codec_driver, mc13783_codec_probe);
MODULE_DESCRIPTION("ASoC MC13783 driver"); MODULE_AUTHOR("Sascha Hauer, Pengutronix s.hauer@pengutronix.de");
Signed-off-by: Alexander Shiyan shc_work@mail.ru --- sound/soc/codecs/mc13783.c | 8 -------- 1 file changed, 8 deletions(-)
diff --git a/sound/soc/codecs/mc13783.c b/sound/soc/codecs/mc13783.c index 8ab9668..c2def5d 100644 --- a/sound/soc/codecs/mc13783.c +++ b/sound/soc/codecs/mc13783.c @@ -781,14 +781,6 @@ static int __init mc13783_codec_probe(struct platform_device *pdev) ret = snd_soc_register_codec(&pdev->dev, &soc_codec_dev_mc13783, mc13783_dai_async, ARRAY_SIZE(mc13783_dai_async));
- if (ret) - goto err_register_codec; - - return 0; - -err_register_codec: - dev_err(&pdev->dev, "register codec failed with %d\n", ret); - return ret; }
On Sun, Jan 05, 2014 at 11:38:32AM +0400, Alexander Shiyan wrote:
Signed-off-by: Alexander Shiyan shc_work@mail.ru
Applied, thanks.
There are no users of this driver without pdata, so stop using constant assignment of ADC and DAC ports.
Signed-off-by: Alexander Shiyan shc_work@mail.ru --- sound/soc/codecs/mc13783.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/codecs/mc13783.c b/sound/soc/codecs/mc13783.c index c2def5d..997f708 100644 --- a/sound/soc/codecs/mc13783.c +++ b/sound/soc/codecs/mc13783.c @@ -770,8 +770,7 @@ static int __init mc13783_codec_probe(struct platform_device *pdev) priv->adc_ssi_port = pdata->adc_ssi_port; priv->dac_ssi_port = pdata->dac_ssi_port; } else { - priv->adc_ssi_port = MC13783_SSI1_PORT; - priv->dac_ssi_port = MC13783_SSI2_PORT; + return -ENOSYS; }
if (priv->adc_ssi_port == priv->dac_ssi_port)
On Sun, Jan 05, 2014 at 11:38:33AM +0400, Alexander Shiyan wrote:
There are no users of this driver without pdata, so stop using constant assignment of ADC and DAC ports.
Applied, thanks.
This is a trivial cleanup: remove useless variable mc13xxx and extra spaces. No functional changes.
Signed-off-by: Alexander Shiyan shc_work@mail.ru --- sound/soc/codecs/mc13783.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/sound/soc/codecs/mc13783.c b/sound/soc/codecs/mc13783.c index 997f708..582c2bb 100644 --- a/sound/soc/codecs/mc13783.c +++ b/sound/soc/codecs/mc13783.c @@ -752,20 +752,14 @@ static struct snd_soc_codec_driver soc_codec_dev_mc13783 = {
static int __init mc13783_codec_probe(struct platform_device *pdev) { - struct mc13xxx *mc13xxx; struct mc13783_priv *priv; struct mc13xxx_codec_platform_data *pdata = pdev->dev.platform_data; int ret;
- mc13xxx = dev_get_drvdata(pdev->dev.parent); - - priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); - if (priv == NULL) + if (!priv) return -ENOMEM;
- dev_set_drvdata(&pdev->dev, priv); - priv->mc13xxx = mc13xxx; if (pdata) { priv->adc_ssi_port = pdata->adc_ssi_port; priv->dac_ssi_port = pdata->dac_ssi_port; @@ -773,6 +767,9 @@ static int __init mc13783_codec_probe(struct platform_device *pdev) return -ENOSYS; }
+ dev_set_drvdata(&pdev->dev, priv); + priv->mc13xxx = dev_get_drvdata(pdev->dev.parent); + if (priv->adc_ssi_port == priv->dac_ssi_port) ret = snd_soc_register_codec(&pdev->dev, &soc_codec_dev_mc13783, mc13783_dai_sync, ARRAY_SIZE(mc13783_dai_sync)); @@ -792,9 +789,9 @@ static int mc13783_codec_remove(struct platform_device *pdev)
static struct platform_driver mc13783_codec_driver = { .driver = { - .name = "mc13783-codec", - .owner = THIS_MODULE, - }, + .name = "mc13783-codec", + .owner = THIS_MODULE, + }, .remove = mc13783_codec_remove, }; module_platform_driver_probe(mc13783_codec_driver, mc13783_codec_probe);
This patch adds devicetree support for mc13783-codec. Since we have not compatible string for this codec, just override of_node for allow using phandle of node in the DT.
Signed-off-by: Alexander Shiyan shc_work@mail.ru --- Documentation/devicetree/bindings/mfd/mc13xxx.txt | 4 +++- drivers/mfd/mc13xxx-core.c | 12 +++++++----- sound/soc/codecs/mc13783.c | 20 ++++++++++++++++++++ 3 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/mfd/mc13xxx.txt b/Documentation/devicetree/bindings/mfd/mc13xxx.txt index abd9e3c..f3047c9 100644 --- a/Documentation/devicetree/bindings/mfd/mc13xxx.txt +++ b/Documentation/devicetree/bindings/mfd/mc13xxx.txt @@ -5,11 +5,13 @@ Required properties:
Optional properties: - fsl,mc13xxx-uses-adc : Indicate the ADC is being used -- fsl,mc13xxx-uses-codec : Indicate the Audio Codec is being used - fsl,mc13xxx-uses-rtc : Indicate the RTC is being used - fsl,mc13xxx-uses-touch : Indicate the touchscreen controller is being used
Sub-nodes: +- codec: Contain the Audio Codec node. + - adc-port: Contain PMIC SSI port used for ADC. + - dac-port: Contain PMIC SSI port used for DAC. - regulators : Contain the regulator nodes. The regulators are bound using their names as listed below with their registers and bits for enabling.
diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c index 06e64b6..7d144aa 100644 --- a/drivers/mfd/mc13xxx-core.c +++ b/drivers/mfd/mc13xxx-core.c @@ -618,7 +618,7 @@ static int mc13xxx_probe_flags_dt(struct mc13xxx *mc13xxx) if (of_get_property(np, "fsl,mc13xxx-uses-adc", NULL)) mc13xxx->flags |= MC13XXX_USE_ADC;
- if (of_get_property(np, "fsl,mc13xxx-uses-codec", NULL)) + if (of_find_node_by_name(np, "codec")) mc13xxx->flags |= MC13XXX_USE_CODEC;
if (of_get_property(np, "fsl,mc13xxx-uses-rtc", NULL)) @@ -673,10 +673,6 @@ int mc13xxx_common_init(struct device *dev) if (mc13xxx->flags & MC13XXX_USE_ADC) mc13xxx_add_subdevice(mc13xxx, "%s-adc");
- if (mc13xxx->flags & MC13XXX_USE_CODEC) - mc13xxx_add_subdevice_pdata(mc13xxx, "%s-codec", - pdata->codec, sizeof(*pdata->codec)); - if (mc13xxx->flags & MC13XXX_USE_RTC) mc13xxx_add_subdevice(mc13xxx, "%s-rtc");
@@ -691,10 +687,16 @@ int mc13xxx_common_init(struct device *dev) pdata->leds, sizeof(*pdata->leds)); mc13xxx_add_subdevice_pdata(mc13xxx, "%s-pwrbutton", pdata->buttons, sizeof(*pdata->buttons)); + if (mc13xxx->flags & MC13XXX_USE_CODEC) + mc13xxx_add_subdevice_pdata(mc13xxx, "%s-codec", + pdata->codec, + sizeof(*pdata->codec)); } else { mc13xxx_add_subdevice(mc13xxx, "%s-regulator"); mc13xxx_add_subdevice(mc13xxx, "%s-led"); mc13xxx_add_subdevice(mc13xxx, "%s-pwrbutton"); + if (mc13xxx->flags & MC13XXX_USE_CODEC) + mc13xxx_add_subdevice(mc13xxx, "%s-codec"); }
return 0; diff --git a/sound/soc/codecs/mc13783.c b/sound/soc/codecs/mc13783.c index 582c2bb..cd0181f 100644 --- a/sound/soc/codecs/mc13783.c +++ b/sound/soc/codecs/mc13783.c @@ -23,6 +23,7 @@ #include <linux/module.h> #include <linux/device.h> #include <linux/mfd/mc13xxx.h> +#include <linux/of.h> #include <linux/slab.h> #include <sound/core.h> #include <sound/control.h> @@ -760,13 +761,32 @@ static int __init mc13783_codec_probe(struct platform_device *pdev) if (!priv) return -ENOMEM;
+ pdev->dev.of_node = of_find_node_by_name(pdev->dev.parent->of_node, + "codec"); + if (pdata) { priv->adc_ssi_port = pdata->adc_ssi_port; priv->dac_ssi_port = pdata->dac_ssi_port; + } else if (pdev->dev.of_node) { + ret = of_property_read_u32(pdev->dev.of_node, "adc-port", + &priv->adc_ssi_port); + if (ret) + return ret; + ret = of_property_read_u32(pdev->dev.of_node, "dac-port", + &priv->dac_ssi_port); + if (ret) + return ret; } else { return -ENOSYS; }
+ if (priv->adc_ssi_port != MC13783_SSI1_PORT && + priv->adc_ssi_port != MC13783_SSI2_PORT) + return -EINVAL; + if (priv->dac_ssi_port != MC13783_SSI1_PORT && + priv->dac_ssi_port != MC13783_SSI2_PORT) + return -EINVAL; + dev_set_drvdata(&pdev->dev, priv); priv->mc13xxx = dev_get_drvdata(pdev->dev.parent);
On Sun, Jan 05, 2014 at 11:38:35AM +0400, Alexander Shiyan wrote:
This patch adds devicetree support for mc13783-codec. Since we have not compatible string for this codec, just override of_node for allow using phandle of node in the DT.
All DT patches should be sent to the DT maintainers for review.
Documentation/devicetree/bindings/mfd/mc13xxx.txt | 4 +++- drivers/mfd/mc13xxx-core.c | 12 +++++++-----
Since this has MFD updates you also need to send it to the MFD maintainers.
Optional properties:
- fsl,mc13xxx-uses-adc : Indicate the ADC is being used
-- fsl,mc13xxx-uses-codec : Indicate the Audio Codec is being used
This is an incompatible binding change; deprecating is fine but it should still be supported. Is there not a default set of port assignments that can be used?
- pdev->dev.of_node = of_find_node_by_name(pdev->dev.parent->of_node,
"codec");
You shouldn't be messing with the of_node in the struct device, this is managed by the driver model and setting one that wasn't added in sync with everything else can introduce breakage.
- if (priv->adc_ssi_port != MC13783_SSI1_PORT &&
priv->adc_ssi_port != MC13783_SSI2_PORT)return -EINVAL;- if (priv->dac_ssi_port != MC13783_SSI1_PORT &&
priv->dac_ssi_port != MC13783_SSI2_PORT)return -EINVAL;
What about DAC or ADC only systems?
Hello.
Понедельник, 6 января 2014, 13:28 UTC от Mark Brown broonie@kernel.org:
On Sun, Jan 05, 2014 at 11:38:35AM +0400, Alexander Shiyan wrote:
This patch adds devicetree support for mc13783-codec. Since we have not compatible string for this codec, just override of_node for allow using phandle of node in the DT.
All DT patches should be sent to the DT maintainers for review.
Documentation/devicetree/bindings/mfd/mc13xxx.txt | 4 +++- drivers/mfd/mc13xxx-core.c | 12 +++++++-----
Since this has MFD updates you also need to send it to the MFD maintainers.
Optional properties:
- fsl,mc13xxx-uses-adc : Indicate the ADC is being used
-- fsl,mc13xxx-uses-codec : Indicate the Audio Codec is being used
This is an incompatible binding change; deprecating is fine but it should still be supported. Is there not a default set of port assignments that can be used?
"fsl,mc13xxx-uses-codec" is not used anywhere and even if it was used, it would not usable, because we can not get to the codec phandle. So I think this change is safe.
- pdev->dev.of_node = of_find_node_by_name(pdev->dev.parent->of_node,
"codec");You shouldn't be messing with the of_node in the struct device, this is managed by the driver model and setting one that wasn't added in sync with everything else can introduce breakage.
Hmm, probably you are right, but I cannot imagine other way to set of_node... Have you other ideas?
- if (priv->adc_ssi_port != MC13783_SSI1_PORT &&
priv->adc_ssi_port != MC13783_SSI2_PORT)return -EINVAL;- if (priv->dac_ssi_port != MC13783_SSI1_PORT &&
priv->dac_ssi_port != MC13783_SSI2_PORT)return -EINVAL;What about DAC or ADC only systems?
In this case we should rewrite this codec driver completely from scratch. ...
Thanks.
---
On Mon, Jan 06, 2014 at 05:46:21PM +0400, Alexander Shiyan wrote:
Понедельник, 6 января 2014, 13:28 UTC от Mark Brown broonie@kernel.org:
This is an incompatible binding change; deprecating is fine but it should still be supported. Is there not a default set of port assignments that can be used?
"fsl,mc13xxx-uses-codec" is not used anywhere and even if it was used, it would not usable, because we can not get to the codec phandle. So I think this change is safe.
Why would it prevent anything getting the CODEC phandle and what do you mean by that anyway? A lack of in tree users shouldn't mean anything, we need to be compatible with DTs shipped with boards.
- pdev->dev.of_node = of_find_node_by_name(pdev->dev.parent->of_node,
"codec");
You shouldn't be messing with the of_node in the struct device, this is managed by the driver model and setting one that wasn't added in sync with everything else can introduce breakage.
Hmm, probably you are right, but I cannot imagine other way to set of_node... Have you other ideas?
The whole problem is that you're trying to set of_node. Don't do that.
- if (priv->adc_ssi_port != MC13783_SSI1_PORT &&
priv->adc_ssi_port != MC13783_SSI2_PORT)return -EINVAL;- if (priv->dac_ssi_port != MC13783_SSI1_PORT &&
priv->dac_ssi_port != MC13783_SSI2_PORT)return -EINVAL;
What about DAC or ADC only systems?
In this case we should rewrite this codec driver completely from scratch.
What makes you say that?
Hello.
On Mon, 6 Jan 2014 15:12:27 +0000 Mark Brown broonie@kernel.org wrote:
This is an incompatible binding change; deprecating is fine but it should still be supported. Is there not a default set of port assignments that can be used?
"fsl,mc13xxx-uses-codec" is not used anywhere and even if it was used, it would not usable, because we can not get to the codec phandle. So I think this change is safe.
Why would it prevent anything getting the CODEC phandle and what do you mean by that anyway? A lack of in tree users shouldn't mean anything, we need to be compatible with DTs shipped with boards.
- pdev->dev.of_node = of_find_node_by_name(pdev->dev.parent->of_node,
"codec");You shouldn't be messing with the of_node in the struct device, this is managed by the driver model and setting one that wasn't added in sync with everything else can introduce breakage.
Hmm, probably you are right, but I cannot imagine other way to set of_node... Have you other ideas?
The whole problem is that you're trying to set of_node. Don't do that.
I try to explain my vision of this:
With DT and simple audio codec we should have description like this: sound { compatible = "simple-audio-card"; ... simple-audio-card,cpu { sound-dai = <&ssi1>; ... }; simple-audio-card,codec { sound-dai = <&mc13783codec>; ... }; }; So, we need codec phandle. The codec is probed by MC13XXX MFD core, and codec pdev not contain compatible string, but we should associate our codec with DT. if ASoC core wants to use codec it will try to find it by of_node. Maybe my opinion is wrong, fixme. Thanks.
On Tue, Jan 07, 2014 at 11:00:53AM +0400, Alexander Shiyan wrote:
So, we need codec phandle. The codec is probed by MC13XXX MFD core, and codec pdev not contain compatible string, but we should associate our codec with DT. if ASoC core wants to use codec it will try to find it by of_node. Maybe my opinion is wrong, fixme.
The fact that we happen to use platform devices inside Linux to split the MFD between subsystems isn't something that should leak out into the device tree representation, this is a Linux implementation detail. The card should be referencing the root node for the device in its binding.
You're also confusing the idea of using an of_node for something with putting it into the struct device; you should never need to modify what's in the struct device to use the device tree data.
participants (2)
-
Alexander Shiyan -
Mark Brown