Message ID | b4127646-565b-ab16-0a8f-1fd6c263389e@ADLINKtech.com |
---|---|
State | Changes Requested, archived |
Headers | show |
On Fri, Apr 21, 2017 at 09:22:02PM +0200, Jens Rottmann wrote: > The iMX-TLV320AIC23 driver isn't from Freescale, but from a company named > Eukrea Electromatique, originally for their own boards. From the code I get > the impression it is a bit older, its DT options use a differing naming > scheme. Patch it up a bit: Needs a subject following the format of the subsystem. > > - Remove Eukrea naming, i.MX is from Freescale, TLV320AIC23 is from TI, > driver was written by Eukrea, but it's DT capable, so it's not exclusive: > - Kconfig option title > - 'model' option > - driver 'compatible' string > - Other options just have changed over time, this driver remaining (one of) > the last with the old semantics: > - 'audio-codec' option (also moved from ssi node) > - 'mux-int/ext-port' options > - All options stay backwards compatible, the DT binding documents new and > old names. > > CONFIG variable and files have not been renamed, though, so no need to > change old defconfigs. > > Signed-off-by: Jens Rottmann <Jens.Rottmann@ADLINKtech.com> > --- > > --- a/Documentation/devicetree/bindings/sound/eukrea-tlv320.txt > +++ b/Documentation/devicetree/bindings/sound/eukrea-tlv320.txt Perhaps change the filename. The compatible string is a good choice. > @@ -1,16 +1,23 @@ > -Audio complex for Eukrea boards with tlv320aic23 codec. > +Audio complex for Freescale i.MX boards with TI TLV320AIC23 I2S codecs, > +like those from Eukrea Electromatique. > > Required properties: > > - - compatible : "eukrea,asoc-tlv320" > + - compatible : "fsl,imx-audio-tlv320aic23" or > + "eukrea,asoc-tlv320" (deprecated) > > - - eukrea,model : The user-visible name of this sound complex. > + - model : The user-visible name of this sound complex. > + - eukrea,model : Dito, deprecated. > > - ssi-controller : The phandle of the SSI controller. > > - - fsl,mux-int-port : The internal port of the i.MX audio muxer (AUDMUX). > + - mux-int-port : The internal port of the i.MX audio muxer (AUDMUX). > + - fsl,mux-int-port : Dito, deprecated. > > - - fsl,mux-ext-port : The external port of the i.MX audio muxer. > + - mux-ext-port : The external port of the i.MX audio muxer. > + - fsl,mux-ext-port : Dito, deprecated. Is this used elsewhere? This is FSL specific, so you should keep the prefix. > + > + - audio-codec : The phandle of the audio codec. > > Note: The AUDMUX port numbering should start at 1, which is consistent with > hardware manual. > @@ -18,9 +25,10 @@ hardware manual. > Example: > > sound { > - compatible = "eukrea,asoc-tlv320"; > - eukrea,model = "imx51-eukrea-tlv320aic23"; > + compatible = "fsl,imx-audio-tlv320aic23"; > + model = "imx51-eukrea-tlv320aic23"; > ssi-controller = <&ssi2>; > - fsl,mux-int-port = <2>; > - fsl,mux-ext-port = <3>; > + mux-int-port = <2>; > + mux-ext-port = <3>; > + audio-codec = <&codec>; > }; -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rob, repeatedly got no response. Assuming all my mails got blocked, so moved to yet another account. Hope I'll get through this time. Regards, Jens -------- Original Message -------- Subject: Re: [PATCH] streamline TLV320AIC23 drivers Date: Sat, 6 May 2017 00:39:20 +0200 From: Jens Rottmann <JRottm@yahoo.com> To: Rob Herring CC: Mark Rutland, Jaroslav Kysela, Takashi Iwai, alsa-devel@alsa-project.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Liam Girdwood, Mark Brown Hi Rob, sorry for the delay. > On Fri, Apr 21, 2017 at 09:22:02PM +0200, Jens Rottmann wrote: >> - Remove Eukrea naming, i.MX is from Freescale, TLV320AIC23 is from TI, >> driver was written by Eukrea, but it's DT capable, so it's not exclusive: >> - Kconfig option title >> - 'model' option >> - driver 'compatible' string >> [...] >> CONFIG variable and files have not been renamed, though, so no need to >> change old defconfigs. On 04/28/2017 19:11, Rob Herring answered: > Perhaps change the filename. [...] I wanted to avoid the churn, but ok. In that case: Do you want me to change all indentifiers inside driver, too? eukrea_tlv320_probe() --> imx_tlv320aic23_probe() Do you want me to also rename the CONFIG var? CONFIG_SND_SOC_EUKREA_TLV320 --> CONFIG_SND_SOC_IMX_TLV320 I'd adapt in-tree defconfigs, but would unavoidably break out-of-tree defconfigs... >> - - fsl,mux-int-port : The internal port of the i.MX audio muxer (AUDMUX). >> + - mux-int-port : The internal port of the i.MX audio muxer (AUDMUX). >> + - fsl,mux-int-port : Dito, deprecated. >> - - fsl,mux-ext-port : The external port of the i.MX audio muxer. >> + - mux-ext-port : The external port of the i.MX audio muxer. >> + - fsl,mux-ext-port : Dito, deprecated. > Is this used elsewhere? This is FSL specific, so you should keep the > prefix. .../sound/soc/fsl$ egrep 'property.*mux-(int|ext)-port' * eukrea-tlv320.c: ret = of_property_read_u32(np, "fsl,mux-int-port", &int_port); eukrea-tlv320.c: ret = of_property_read_u32(np, "fsl,mux-ext-port", &ext_port); fsl-asoc-card.c: ret = of_property_read_u32(np, "mux-int-port", &int_port); fsl-asoc-card.c: ret = of_property_read_u32(np, "mux-ext-port", &ext_port); imx-es8328.c: ret = of_property_read_u32(np, "mux-int-port", &int_port); imx-es8328.c: ret = of_property_read_u32(np, "mux-ext-port", &ext_port); imx-sgtl5000.c: ret = of_property_read_u32(np, "mux-int-port", &int_port); imx-sgtl5000.c: ret = of_property_read_u32(np, "mux-ext-port", &ext_port); imx-wm8962.c: ret = of_property_read_u32(np, "mux-int-port", &int_port); imx-wm8962.c: ret = of_property_read_u32(np, "mux-ext-port", &ext_port); All drivers use mux-int|ext-port without prefix, tlv320 is the only odd driver out, that's why I did this streamlining patch in the 1st place. I would rather not have to change all other drivers to match the tlv320 one. Thanks, Jens -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rob, repeatedly got no response. Hope I'll get through this time. Regards, Jens -------- Original Message -------- Subject: Re: [PATCH] streamline TLV320AIC23 drivers Date: Sat, 6 May 2017 00:39:20 +0200 From: Jens Rottmann <JRottm@yahoo.com> To: Rob Herring CC: Mark Rutland, Jaroslav Kysela, Takashi Iwai, alsa-devel@alsa-project.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Liam Girdwood, Mark Brown Hi Rob, sorry for the delay. > On Fri, Apr 21, 2017 at 09:22:02PM +0200, Jens Rottmann wrote: >> - Remove Eukrea naming, i.MX is from Freescale, TLV320AIC23 is from TI, >> driver was written by Eukrea, but it's DT capable, so it's not exclusive: >> - Kconfig option title >> - 'model' option >> - driver 'compatible' string >> [...] >> CONFIG variable and files have not been renamed, though, so no need to >> change old defconfigs. On 04/28/2017 19:11, Rob Herring answered: > Perhaps change the filename. [...] I wanted to avoid the churn, but ok. In that case: Do you want me to change all indentifiers inside driver, too? eukrea_tlv320_probe() --> imx_tlv320aic23_probe() Do you want me to also rename the CONFIG var? CONFIG_SND_SOC_EUKREA_TLV320 --> CONFIG_SND_SOC_IMX_TLV320 I'd adapt in-tree defconfigs, but would unavoidably break out-of-tree defconfigs... >> - - fsl,mux-int-port : The internal port of the i.MX audio muxer (AUDMUX). >> + - mux-int-port : The internal port of the i.MX audio muxer (AUDMUX). >> + - fsl,mux-int-port : Dito, deprecated. >> - - fsl,mux-ext-port : The external port of the i.MX audio muxer. >> + - mux-ext-port : The external port of the i.MX audio muxer. >> + - fsl,mux-ext-port : Dito, deprecated. > Is this used elsewhere? This is FSL specific, so you should keep the > prefix. .../sound/soc/fsl$ egrep 'property.*mux-(int|ext)-port' * eukrea-tlv320.c: ret = of_property_read_u32(np, "fsl,mux-int-port", &int_port); eukrea-tlv320.c: ret = of_property_read_u32(np, "fsl,mux-ext-port", &ext_port); fsl-asoc-card.c: ret = of_property_read_u32(np, "mux-int-port", &int_port); fsl-asoc-card.c: ret = of_property_read_u32(np, "mux-ext-port", &ext_port); imx-es8328.c: ret = of_property_read_u32(np, "mux-int-port", &int_port); imx-es8328.c: ret = of_property_read_u32(np, "mux-ext-port", &ext_port); imx-sgtl5000.c: ret = of_property_read_u32(np, "mux-int-port", &int_port); imx-sgtl5000.c: ret = of_property_read_u32(np, "mux-ext-port", &ext_port); imx-wm8962.c: ret = of_property_read_u32(np, "mux-int-port", &int_port); imx-wm8962.c: ret = of_property_read_u32(np, "mux-ext-port", &ext_port); All drivers use mux-int|ext-port without prefix, tlv320 is the only odd driver out, that's why I did this streamlining patch in the 1st place. I would rather not have to change all other drivers to match the tlv320 one. Thanks, Jens -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/Documentation/devicetree/bindings/sound/eukrea-tlv320.txt +++ b/Documentation/devicetree/bindings/sound/eukrea-tlv320.txt @@ -1,16 +1,23 @@ -Audio complex for Eukrea boards with tlv320aic23 codec. +Audio complex for Freescale i.MX boards with TI TLV320AIC23 I2S codecs, +like those from Eukrea Electromatique. Required properties: - - compatible : "eukrea,asoc-tlv320" + - compatible : "fsl,imx-audio-tlv320aic23" or + "eukrea,asoc-tlv320" (deprecated) - - eukrea,model : The user-visible name of this sound complex. + - model : The user-visible name of this sound complex. + - eukrea,model : Dito, deprecated. - ssi-controller : The phandle of the SSI controller. - - fsl,mux-int-port : The internal port of the i.MX audio muxer (AUDMUX). + - mux-int-port : The internal port of the i.MX audio muxer (AUDMUX). + - fsl,mux-int-port : Dito, deprecated. - - fsl,mux-ext-port : The external port of the i.MX audio muxer. + - mux-ext-port : The external port of the i.MX audio muxer. + - fsl,mux-ext-port : Dito, deprecated. + + - audio-codec : The phandle of the audio codec. Note: The AUDMUX port numbering should start at 1, which is consistent with hardware manual. @@ -18,9 +25,10 @@ hardware manual. Example: sound { - compatible = "eukrea,asoc-tlv320"; - eukrea,model = "imx51-eukrea-tlv320aic23"; + compatible = "fsl,imx-audio-tlv320aic23"; + model = "imx51-eukrea-tlv320aic23"; ssi-controller = <&ssi2>; - fsl,mux-int-port = <2>; - fsl,mux-ext-port = <3>; + mux-int-port = <2>; + mux-ext-port = <3>; + audio-codec = <&codec>; }; --- a/sound/soc/fsl/Kconfig +++ b/sound/soc/fsl/Kconfig @@ -220,7 +220,7 @@ config SND_SOC_PHYCORE_AC97 and phyCARD boards in AC97 mode config SND_SOC_EUKREA_TLV320 - tristate "Eukrea TLV320" + tristate "SoC Audio support for i.MX boards with TLV320AIC23" depends on ARCH_MXC && I2C select SND_SOC_TLV320AIC23_I2C select SND_SOC_IMX_AUDMUX --- a/sound/soc/fsl/eukrea-tlv320.c +++ b/sound/soc/fsl/eukrea-tlv320.c @@ -92,11 +92,13 @@ static int eukrea_tlv320_probe(struct platform_device *pdev) eukrea_tlv320.dev = &pdev->dev; if (np) { - ret = snd_soc_of_parse_card_name(&eukrea_tlv320, - "eukrea,model"); + ret = snd_soc_of_parse_card_name(&eukrea_tlv320, "model"); + if (ret) /* backwards compatible */ + ret = snd_soc_of_parse_card_name(&eukrea_tlv320, + "eukrea,model"); if (ret) { dev_err(&pdev->dev, - "eukrea,model node missing or invalid.\n"); + "model node missing or invalid.\n"); goto err; } @@ -109,22 +111,28 @@ static int eukrea_tlv320_probe(struct platform_device *pdev) goto err; } - codec_np = of_parse_phandle(ssi_np, "codec-handle", 0); + codec_np = of_parse_phandle(pdev->dev.of_node, "audio-codec", 0); + if (!codec_np) /* backwards compatible */ + codec_np = of_parse_phandle(ssi_np, "codec-handle", 0); if (codec_np) eukrea_tlv320_dai.codec_of_node = codec_np; else - dev_err(&pdev->dev, "codec-handle node missing or invalid.\n"); + dev_err(&pdev->dev, "audio-codec node missing or invalid.\n"); - ret = of_property_read_u32(np, "fsl,mux-int-port", &int_port); + ret = of_property_read_u32(np, "mux-int-port", &int_port); + if (ret) /* backwards compatible */ + ret = of_property_read_u32(np, "fsl,mux-int-port", &int_port); if (ret) { dev_err(&pdev->dev, - "fsl,mux-int-port node missing or invalid.\n"); + "mux-int-port node missing or invalid.\n"); return ret; } - ret = of_property_read_u32(np, "fsl,mux-ext-port", &ext_port); + ret = of_property_read_u32(np, "mux-ext-port", &ext_port); + if (ret) /* backwards compatible */ + ret = of_property_read_u32(np, "fsl,mux-ext-port", &ext_port); if (ret) { dev_err(&pdev->dev, - "fsl,mux-ext-port node missing or invalid.\n"); + "mux-ext-port node missing or invalid.\n"); return ret; } @@ -213,7 +221,8 @@ static int eukrea_tlv320_remove(struct platform_device *pdev) } static const struct of_device_id imx_tlv320_dt_ids[] = { - { .compatible = "eukrea,asoc-tlv320"}, + { .compatible = "eukrea,asoc-tlv320"}, /* backwards compatible */ + { .compatible = "fsl,imx-audio-tlv320aic23"}, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, imx_tlv320_dt_ids);
The iMX-TLV320AIC23 driver isn't from Freescale, but from a company named Eukrea Electromatique, originally for their own boards. From the code I get the impression it is a bit older, its DT options use a differing naming scheme. Patch it up a bit: - Remove Eukrea naming, i.MX is from Freescale, TLV320AIC23 is from TI, driver was written by Eukrea, but it's DT capable, so it's not exclusive: - Kconfig option title - 'model' option - driver 'compatible' string - Other options just have changed over time, this driver remaining (one of) the last with the old semantics: - 'audio-codec' option (also moved from ssi node) - 'mux-int/ext-port' options - All options stay backwards compatible, the DT binding documents new and old names. CONFIG variable and files have not been renamed, though, so no need to change old defconfigs. Signed-off-by: Jens Rottmann <Jens.Rottmann@ADLINKtech.com> ---