[PATCH 5/6] ASoC: fsl_ssi: instantiate AC'97 CODEC

[PATCH 5/6] ASoC: fsl_ssi: instantiate AC'97 CODEC

[Maciej S. Szmigiero]

Instantiate AC'97 CODEC in fsl_ssi driver AC'97 mode.

Signed-off-by: Maciej Szmigiero <mail@maciej.szmigiero.name>
---
 sound/soc/fsl/fsl_ssi.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 0b4fcd9..e79dc16 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -1460,6 +1460,27 @@ done:
 		_fsl_ssi_set_dai_fmt(&pdev->dev, ssi_private,
 				     ssi_private->dai_fmt);
 
+	if (fsl_ssi_is_ac97(ssi_private)) {
+		u32 ssi_idx;
+
+		ret = of_property_read_u32(np, "cell-index", &ssi_idx);
+		if (ret) {
+			dev_err(&pdev->dev, "cannot get SSI index property\n");
+			goto error_sound_card;
+		}
+
+		ssi_private->pdev =
+			platform_device_register_data(NULL,
+					"ac97-codec", ssi_idx, NULL, 0);
+		if (IS_ERR(ssi_private->pdev)) {
+			ret = PTR_ERR(ssi_private->pdev);
+			dev_err(&pdev->dev,
+				"failed to register AC97 codec platform: %d\n",
+				ret);
+			goto error_sound_card;
+		}
+	}
+
 	return 0;
 
 error_sound_card:

[PATCH 5/6] ASoC: fsl_ssi: instantiate AC'97 CODEC

[Maciej S. Szmigiero]

Instantiate AC'97 CODEC in fsl_ssi driver AC'97 mode.

Signed-off-by: Maciej Szmigiero <mail@maciej.szmigiero.name>
---
 sound/soc/fsl/fsl_ssi.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 154bcf6..8e5ff5e 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -1460,6 +1460,27 @@ done:
 		_fsl_ssi_set_dai_fmt(&pdev->dev, ssi_private,
 				     ssi_private->dai_fmt);
 
+	if (fsl_ssi_is_ac97(ssi_private)) {
+		u32 ssi_idx;
+
+		ret = of_property_read_u32(np, "cell-index", &ssi_idx);
+		if (ret) {
+			dev_err(&pdev->dev, "cannot get SSI index property\n");
+			goto error_sound_card;
+		}
+
+		ssi_private->pdev =
+			platform_device_register_data(NULL,
+					"ac97-codec", ssi_idx, NULL, 0);
+		if (IS_ERR(ssi_private->pdev)) {
+			ret = PTR_ERR(ssi_private->pdev);
+			dev_err(&pdev->dev,
+				"failed to register AC97 codec platform: %d\n",
+				ret);
+			goto error_sound_card;
+		}
+	}
+
 	return 0;
 
 error_sound_card:

Re: [PATCH 5/6] ASoC: fsl_ssi: instantiate AC'97 CODEC

[Markus Pargmann]

[-- Attachment #1: Type: text/plain, Size: 2088 bytes --]

On Thu, Jul 30, 2015 at 04:35:23PM +0200, Maciej S. Szmigiero wrote:
> Instantiate AC'97 CODEC in fsl_ssi driver AC'97 mode.
> 
> Signed-off-by: Maciej Szmigiero <mail@maciej.szmigiero.name>
> ---
>  sound/soc/fsl/fsl_ssi.c |   21 +++++++++++++++++++++
>  1 files changed, 21 insertions(+), 0 deletions(-)
> 
> diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
> index 154bcf6..8e5ff5e 100644
> --- a/sound/soc/fsl/fsl_ssi.c
> +++ b/sound/soc/fsl/fsl_ssi.c
> @@ -1460,6 +1460,27 @@ done:
>  		_fsl_ssi_set_dai_fmt(&pdev->dev, ssi_private,
>  				     ssi_private->dai_fmt);
>  
> +	if (fsl_ssi_is_ac97(ssi_private)) {
> +		u32 ssi_idx;
> +
> +		ret = of_property_read_u32(np, "cell-index", &ssi_idx);

This property is not set anywhere in the imx* DTs.

> +		if (ret) {
> +			dev_err(&pdev->dev, "cannot get SSI index property\n");
> +			goto error_sound_card;
> +		}
> +
> +		ssi_private->pdev =
> +			platform_device_register_data(NULL,
> +					"ac97-codec", ssi_idx, NULL, 0);

If you really want to create a device at this point you should use
PLATFORM_DEVID_AUTO. I would prefer something where this is created in
DT. On the other side it is a discoverable bus..

Regards,

Markus

> +		if (IS_ERR(ssi_private->pdev)) {
> +			ret = PTR_ERR(ssi_private->pdev);
> +			dev_err(&pdev->dev,
> +				"failed to register AC97 codec platform: %d\n",
> +				ret);
> +			goto error_sound_card;
> +		}
> +	}
> +
>  	return 0;
>  
>  error_sound_card:
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 5/6] ASoC: fsl_ssi: instantiate AC'97 CODEC

[Maciej S. Szmigiero]

On 31.07.2015 07:46, Markus Pargmann wrote:
> On Thu, Jul 30, 2015 at 04:35:23PM +0200, Maciej S. Szmigiero wrote:
>> Instantiate AC'97 CODEC in fsl_ssi driver AC'97 mode.
>>
>> Signed-off-by: Maciej Szmigiero <mail@maciej.szmigiero.name>
>> ---
>>  sound/soc/fsl/fsl_ssi.c |   21 +++++++++++++++++++++
>>  1 files changed, 21 insertions(+), 0 deletions(-)
>>
>> diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
>> index 154bcf6..8e5ff5e 100644
>> --- a/sound/soc/fsl/fsl_ssi.c
>> +++ b/sound/soc/fsl/fsl_ssi.c
>> @@ -1460,6 +1460,27 @@ done:
>>  		_fsl_ssi_set_dai_fmt(&pdev->dev, ssi_private,
>>  				     ssi_private->dai_fmt);
>>  
>> +	if (fsl_ssi_is_ac97(ssi_private)) {
>> +		u32 ssi_idx;
>> +
>> +		ret = of_property_read_u32(np, "cell-index", &ssi_idx);
> 
> This property is not set anywhere in the imx* DTs.

That's right, but it is documented as required property in sound/fsl,ssi.txt:
> Required properties:
> (..)
> - cell-index:       The SSI, <0> = SSI1, <1> = SSI2, and so on.

> 
>> +		if (ret) {
>> +			dev_err(&pdev->dev, "cannot get SSI index property\n");
>> +			goto error_sound_card;
>> +		}
>> +
>> +		ssi_private->pdev =
>> +			platform_device_register_data(NULL,
>> +					"ac97-codec", ssi_idx, NULL, 0);
> 
> If you really want to create a device at this point you should use
> PLATFORM_DEVID_AUTO. I would prefer something where this is created in
> DT. On the other side it is a discoverable bus..

In the original implementation the codec was instantiated in DT but
the feedback was that this is wrong since devices on discoverable
buses shouldn't be in DT.

The platform device index is based on SSI index since the sound
card driver (fsl-asoc-card) has to know CODEC device name to identify
it in DAI links - as the only identification options seem to be either
DT node of CODEC or its device name.

That's why the (platform) device name has to be deterministic
if there is no mechanism to pass it from controller driver
to sound card driver.

> Regards,
> 
> Markus

Best regards,
Maciej Szmigiero

Re: [PATCH 5/6] ASoC: fsl_ssi: instantiate AC'97 CODEC

[Markus Pargmann]

[-- Attachment #1: Type: text/plain, Size: 3150 bytes --]

On Fri, Jul 31, 2015 at 04:39:20PM +0200, Maciej S. Szmigiero wrote:
> On 31.07.2015 07:46, Markus Pargmann wrote:
> > On Thu, Jul 30, 2015 at 04:35:23PM +0200, Maciej S. Szmigiero wrote:
> >> Instantiate AC'97 CODEC in fsl_ssi driver AC'97 mode.
> >>
> >> Signed-off-by: Maciej Szmigiero <mail@maciej.szmigiero.name>
> >> ---
> >>  sound/soc/fsl/fsl_ssi.c |   21 +++++++++++++++++++++
> >>  1 files changed, 21 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
> >> index 154bcf6..8e5ff5e 100644
> >> --- a/sound/soc/fsl/fsl_ssi.c
> >> +++ b/sound/soc/fsl/fsl_ssi.c
> >> @@ -1460,6 +1460,27 @@ done:
> >>  		_fsl_ssi_set_dai_fmt(&pdev->dev, ssi_private,
> >>  				     ssi_private->dai_fmt);
> >>  
> >> +	if (fsl_ssi_is_ac97(ssi_private)) {
> >> +		u32 ssi_idx;
> >> +
> >> +		ret = of_property_read_u32(np, "cell-index", &ssi_idx);
> > 
> > This property is not set anywhere in the imx* DTs.
> 
> That's right, but it is documented as required property in sound/fsl,ssi.txt:
> > Required properties:
> > (..)
> > - cell-index:       The SSI, <0> = SSI1, <1> = SSI2, and so on.
> 
> > 
> >> +		if (ret) {
> >> +			dev_err(&pdev->dev, "cannot get SSI index property\n");
> >> +			goto error_sound_card;
> >> +		}
> >> +
> >> +		ssi_private->pdev =
> >> +			platform_device_register_data(NULL,
> >> +					"ac97-codec", ssi_idx, NULL, 0);
> > 
> > If you really want to create a device at this point you should use
> > PLATFORM_DEVID_AUTO. I would prefer something where this is created in
> > DT. On the other side it is a discoverable bus..
> 
> In the original implementation the codec was instantiated in DT but
> the feedback was that this is wrong since devices on discoverable
> buses shouldn't be in DT.
> 
> The platform device index is based on SSI index since the sound
> card driver (fsl-asoc-card) has to know CODEC device name to identify
> it in DAI links - as the only identification options seem to be either
> DT node of CODEC or its device name.
> 
> That's why the (platform) device name has to be deterministic
> if there is no mechanism to pass it from controller driver
> to sound card driver.

Thanks for clarification. I am not really happy with using this
cell-index as the whole information that should be necessary is already
contained in the memory address range given. The SSI units are otherwise
identical in hardware, so cell-index feels like some arbitrary
description that is used in the Reference Manual and for software.

However I don't have a better idea how to solve this at the moment and
as it is listed as required property and not a new DT binding it can be
used.

Best regards,

Markus

> 
> > Regards,
> > 
> > Markus
> 
> Best regards,
> Maciej Szmigiero
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]