diff mbox series

[1/5] ASoC: atmel: enable SSC_PCM_DMA in Kconfig

Message ID ee65cc7b889b2a8d1139d1d25977842c956d1cf4.1563819483.git.mirq-linux@rere.qmqm.pl (mailing list archive)
State New, archived
Headers show
Series ASoC: atmel: extend SSC support | expand

Commit Message

Michał Mirosław July 22, 2019, 6:27 p.m. UTC
Allow SSC to be used on platforms described using audio-graph-card
in Device Tree.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 sound/soc/atmel/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Codrin Ciubotariu July 23, 2019, 1:36 p.m. UTC | #1
On 22.07.2019 21:27, Michał Mirosław wrote:
> Allow SSC to be used on platforms described using audio-graph-card
> in Device Tree.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
>   sound/soc/atmel/Kconfig | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/atmel/Kconfig b/sound/soc/atmel/Kconfig
> index 06c1d5ce642c..9ef9d25bb517 100644
> --- a/sound/soc/atmel/Kconfig
> +++ b/sound/soc/atmel/Kconfig
> @@ -25,7 +25,7 @@ config SND_ATMEL_SOC_DMA
>   	default y if SND_ATMEL_SOC_SSC_DMA=y || (SND_ATMEL_SOC_SSC_DMA=m && SND_ATMEL_SOC_SSC=y)
>   
>   config SND_ATMEL_SOC_SSC_DMA
> -	tristate
> +	tristate "SoC PCM DAI support for AT91 SSC controller using DMA"

Could you please make something similar for SND_ATMEL_SOC_SSC_PDC? Also, 
I think that it should select ATMEL_SSC, to be able to use 
simple/graph-card with SSC.

Thanks and best regards,
Codrin

>   
>   config SND_ATMEL_SOC_SSC
>   	tristate
>
Codrin Ciubotariu July 23, 2019, 2:26 p.m. UTC | #2
On 23.07.2019 16:36, Codrin.Ciubotariu@microchip.com wrote:
> On 22.07.2019 21:27, Michał Mirosław wrote:
>> Allow SSC to be used on platforms described using audio-graph-card
>> in Device Tree.
>>
>> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>> ---
>>    sound/soc/atmel/Kconfig | 2 +-
>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/atmel/Kconfig b/sound/soc/atmel/Kconfig
>> index 06c1d5ce642c..9ef9d25bb517 100644
>> --- a/sound/soc/atmel/Kconfig
>> +++ b/sound/soc/atmel/Kconfig
>> @@ -25,7 +25,7 @@ config SND_ATMEL_SOC_DMA
>>    	default y if SND_ATMEL_SOC_SSC_DMA=y || (SND_ATMEL_SOC_SSC_DMA=m && SND_ATMEL_SOC_SSC=y)
>>    
>>    config SND_ATMEL_SOC_SSC_DMA
>> -	tristate
>> +	tristate "SoC PCM DAI support for AT91 SSC controller using DMA"
> 
> Could you please make something similar for SND_ATMEL_SOC_SSC_PDC? Also,
> I think that it should select ATMEL_SSC, to be able to use
> simple/graph-card with SSC.

It looks like 'select' creates a recursive dependency with audio machine 
drivers, so try 'depends on' instead.

> 
> Thanks and best regards,
> Codrin
> 
>>    
>>    config SND_ATMEL_SOC_SSC
>>    	tristate
>>
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Michał Mirosław July 23, 2019, 4:43 p.m. UTC | #3
On Tue, Jul 23, 2019 at 01:36:37PM +0000, Codrin.Ciubotariu@microchip.com wrote:
> On 22.07.2019 21:27, Michał Mirosław wrote:
> > Allow SSC to be used on platforms described using audio-graph-card
> > in Device Tree.
> > 
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > ---
> >   sound/soc/atmel/Kconfig | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/sound/soc/atmel/Kconfig b/sound/soc/atmel/Kconfig
> > index 06c1d5ce642c..9ef9d25bb517 100644
> > --- a/sound/soc/atmel/Kconfig
> > +++ b/sound/soc/atmel/Kconfig
> > @@ -25,7 +25,7 @@ config SND_ATMEL_SOC_DMA
> >   	default y if SND_ATMEL_SOC_SSC_DMA=y || (SND_ATMEL_SOC_SSC_DMA=m && SND_ATMEL_SOC_SSC=y)
> >   
> >   config SND_ATMEL_SOC_SSC_DMA
> > -	tristate
> > +	tristate "SoC PCM DAI support for AT91 SSC controller using DMA"
> 
> Could you please make something similar for SND_ATMEL_SOC_SSC_PDC? Also, 
> I think that it should select ATMEL_SSC, to be able to use 
> simple/graph-card with SSC.

Hmm. The Kconfig dependencies seems overly complicated, do you mind if I
get rid of most of the entries in the process?

Best Regards,
Michał Mirosław
Mark Brown July 23, 2019, 5:18 p.m. UTC | #4
On Mon, Jul 22, 2019 at 08:27:20PM +0200, Michał Mirosław wrote:

>  config SND_ATMEL_SOC_SSC_DMA
> -	tristate
> +	tristate "SoC PCM DAI support for AT91 SSC controller using DMA"

This breaks the build for me:

ld: sound/soc/atmel/atmel_ssc_dai.o: in function `atmel_ssc_set_audio':
atmel_ssc_dai.c:(.text+0xbd9): undefined reference to `ssc_request'
ld: sound/soc/atmel/atmel_ssc_dai.o: in function `atmel_ssc_put_audio':
atmel_ssc_dai.c:(.text+0xc74): undefined reference to `ssc_free'

It's not selecting the SSC comon code.  It also looks like it'd be
sensible to add a dependency on the platform (or at least architecture),
with a || COMPILE_TEST to help with build coverage.
Codrin Ciubotariu July 23, 2019, 5:27 p.m. UTC | #5
On 23.07.2019 19:43, mirq-linux@rere.qmqm.pl wrote:
> External E-Mail
> 
> 
> On Tue, Jul 23, 2019 at 01:36:37PM +0000, Codrin.Ciubotariu@microchip.com wrote:
>> On 22.07.2019 21:27, Michał Mirosław wrote:
>>> Allow SSC to be used on platforms described using audio-graph-card
>>> in Device Tree.
>>>
>>> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>>> ---
>>>    sound/soc/atmel/Kconfig | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/sound/soc/atmel/Kconfig b/sound/soc/atmel/Kconfig
>>> index 06c1d5ce642c..9ef9d25bb517 100644
>>> --- a/sound/soc/atmel/Kconfig
>>> +++ b/sound/soc/atmel/Kconfig
>>> @@ -25,7 +25,7 @@ config SND_ATMEL_SOC_DMA
>>>    	default y if SND_ATMEL_SOC_SSC_DMA=y || (SND_ATMEL_SOC_SSC_DMA=m && SND_ATMEL_SOC_SSC=y)
>>>    
>>>    config SND_ATMEL_SOC_SSC_DMA
>>> -	tristate
>>> +	tristate "SoC PCM DAI support for AT91 SSC controller using DMA"
>>
>> Could you please make something similar for SND_ATMEL_SOC_SSC_PDC? Also,
>> I think that it should select ATMEL_SSC, to be able to use
>> simple/graph-card with SSC.
> 
> Hmm. The Kconfig dependencies seems overly complicated, do you mind if I
> get rid of most of the entries in the process?
> 
> Best Regards,
> Michał Mirosław
> 

'select' creates recursive dependencies with our machine
drivers, but 'depends on' should work. The complication comes from the 
fact that PDC and DMA support for SSC can be both enabled at the same 
time, so I think we need all of them...

I am reviewing the rest of your patches, so bear with me please.

Best regards,
Codrin
Alexandre Belloni July 23, 2019, 6:39 p.m. UTC | #6
On 23/07/2019 18:43:12+0200, mirq-linux@rere.qmqm.pl wrote:
> On Tue, Jul 23, 2019 at 01:36:37PM +0000, Codrin.Ciubotariu@microchip.com wrote:
> > On 22.07.2019 21:27, Michał Mirosław wrote:
> > > Allow SSC to be used on platforms described using audio-graph-card
> > > in Device Tree.
> > > 
> > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > > ---
> > >   sound/soc/atmel/Kconfig | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/sound/soc/atmel/Kconfig b/sound/soc/atmel/Kconfig
> > > index 06c1d5ce642c..9ef9d25bb517 100644
> > > --- a/sound/soc/atmel/Kconfig
> > > +++ b/sound/soc/atmel/Kconfig
> > > @@ -25,7 +25,7 @@ config SND_ATMEL_SOC_DMA
> > >   	default y if SND_ATMEL_SOC_SSC_DMA=y || (SND_ATMEL_SOC_SSC_DMA=m && SND_ATMEL_SOC_SSC=y)
> > >   
> > >   config SND_ATMEL_SOC_SSC_DMA
> > > -	tristate
> > > +	tristate "SoC PCM DAI support for AT91 SSC controller using DMA"
> > 
> > Could you please make something similar for SND_ATMEL_SOC_SSC_PDC? Also, 
> > I think that it should select ATMEL_SSC, to be able to use 
> > simple/graph-card with SSC.
> 
> Hmm. The Kconfig dependencies seems overly complicated, do you mind if I
> get rid of most of the entries in the process?
> 

Unfortunately, it is just complicated enough. This is done to support
all the possible configurations. Removing them will lead to linking
errors.

After having that discussion back in March, I had a very quick look and
didn't send a patch because I still had linking issues. It is not
impossible but it required more time than I had.
Michał Mirosław July 23, 2019, 11:25 p.m. UTC | #7
On Tue, Jul 23, 2019 at 08:39:15PM +0200, Alexandre Belloni wrote:
> On 23/07/2019 18:43:12+0200, mirq-linux@rere.qmqm.pl wrote:
> > On Tue, Jul 23, 2019 at 01:36:37PM +0000, Codrin.Ciubotariu@microchip.com wrote:
> > > On 22.07.2019 21:27, Michał Mirosław wrote:
> > > > Allow SSC to be used on platforms described using audio-graph-card
> > > > in Device Tree.
> > > > 
> > > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > > > ---
> > > >   sound/soc/atmel/Kconfig | 2 +-
> > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/sound/soc/atmel/Kconfig b/sound/soc/atmel/Kconfig
> > > > index 06c1d5ce642c..9ef9d25bb517 100644
> > > > --- a/sound/soc/atmel/Kconfig
> > > > +++ b/sound/soc/atmel/Kconfig
> > > > @@ -25,7 +25,7 @@ config SND_ATMEL_SOC_DMA
> > > >   	default y if SND_ATMEL_SOC_SSC_DMA=y || (SND_ATMEL_SOC_SSC_DMA=m && SND_ATMEL_SOC_SSC=y)
> > > >   
> > > >   config SND_ATMEL_SOC_SSC_DMA
> > > > -	tristate
> > > > +	tristate "SoC PCM DAI support for AT91 SSC controller using DMA"
> > > 
> > > Could you please make something similar for SND_ATMEL_SOC_SSC_PDC? Also, 
> > > I think that it should select ATMEL_SSC, to be able to use 
> > > simple/graph-card with SSC.
> > 
> > Hmm. The Kconfig dependencies seems overly complicated, do you mind if I
> > get rid of most of the entries in the process?
> > 
> 
> Unfortunately, it is just complicated enough. This is done to support
> all the possible configurations. Removing them will lead to linking
> errors.
> 
> After having that discussion back in March, I had a very quick look and
> didn't send a patch because I still had linking issues. It is not
> impossible but it required more time than I had.

Can you try patch below if it covers the configurations you mention?
This uses Kconfig's m/y resolution instead of open-coded defaults.

Best Regards,
Michał Mirosław


diff --git a/sound/soc/atmel/Kconfig b/sound/soc/atmel/Kconfig
index 06c1d5ce642c..f118c229ed82 100644
--- a/sound/soc/atmel/Kconfig
+++ b/sound/soc/atmel/Kconfig
@@ -12,25 +12,31 @@ if SND_ATMEL_SOC
 config SND_ATMEL_SOC_PDC
 	tristate
 	depends on HAS_DMA
-	default m if SND_ATMEL_SOC_SSC_PDC=m && SND_ATMEL_SOC_SSC=m
-	default y if SND_ATMEL_SOC_SSC_PDC=y || (SND_ATMEL_SOC_SSC_PDC=m && SND_ATMEL_SOC_SSC=y)
-
-config SND_ATMEL_SOC_SSC_PDC
-	tristate
 
 config SND_ATMEL_SOC_DMA
 	tristate
 	select SND_SOC_GENERIC_DMAENGINE_PCM
-	default m if SND_ATMEL_SOC_SSC_DMA=m && SND_ATMEL_SOC_SSC=m
-	default y if SND_ATMEL_SOC_SSC_DMA=y || (SND_ATMEL_SOC_SSC_DMA=m && SND_ATMEL_SOC_SSC=y)
-
-config SND_ATMEL_SOC_SSC_DMA
-	tristate
 
 config SND_ATMEL_SOC_SSC
 	tristate
-	default y if SND_ATMEL_SOC_SSC_DMA=y || SND_ATMEL_SOC_SSC_PDC=y
-	default m if SND_ATMEL_SOC_SSC_DMA=m || SND_ATMEL_SOC_SSC_PDC=m
+
+config SND_ATMEL_SOC_SSC_PDC
+	tristate "SoC PCM DAI support for AT91 SSC controller using PDC"
+	depends on ATMEL_SSC
+	select SND_ATMEL_SOC_PDC
+	select SND_ATMEL_SOC_SSC
+	help
+	  Say Y or M if you want to add support for Atmel SSC interface
+	  in PDC mode configured using audio-graph-card in device-tree.
+
+config SND_ATMEL_SOC_SSC_DMA
+	tristate "SoC PCM DAI support for AT91 SSC controller using DMA"
+	depends on ATMEL_SSC
+	select SND_ATMEL_SOC_DMA
+	select SND_ATMEL_SOC_SSC
+	help
+	  Say Y or M if you want to add support for Atmel SSC interface
+	  in DMA mode configured using audio-graph-card in device-tree.
 
 config SND_AT91_SOC_SAM9G20_WM8731
 	tristate "SoC Audio support for WM8731-based At91sam9g20 evaluation board"
Codrin Ciubotariu July 25, 2019, 3:25 p.m. UTC | #8
On 24.07.2019 02:25, mirq-linux@rere.qmqm.pl wrote:
> External E-Mail
> 
> 
> On Tue, Jul 23, 2019 at 08:39:15PM +0200, Alexandre Belloni wrote:
>> On 23/07/2019 18:43:12+0200, mirq-linux@rere.qmqm.pl wrote:
>>> On Tue, Jul 23, 2019 at 01:36:37PM +0000, Codrin.Ciubotariu@microchip.com wrote:
>>>> On 22.07.2019 21:27, Michał Mirosław wrote:
>>>>> Allow SSC to be used on platforms described using audio-graph-card
>>>>> in Device Tree.
>>>>>
>>>>> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>>>>> ---
>>>>>    sound/soc/atmel/Kconfig | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/sound/soc/atmel/Kconfig b/sound/soc/atmel/Kconfig
>>>>> index 06c1d5ce642c..9ef9d25bb517 100644
>>>>> --- a/sound/soc/atmel/Kconfig
>>>>> +++ b/sound/soc/atmel/Kconfig
>>>>> @@ -25,7 +25,7 @@ config SND_ATMEL_SOC_DMA
>>>>>    	default y if SND_ATMEL_SOC_SSC_DMA=y || (SND_ATMEL_SOC_SSC_DMA=m && SND_ATMEL_SOC_SSC=y)
>>>>>    
>>>>>    config SND_ATMEL_SOC_SSC_DMA
>>>>> -	tristate
>>>>> +	tristate "SoC PCM DAI support for AT91 SSC controller using DMA"
>>>>
>>>> Could you please make something similar for SND_ATMEL_SOC_SSC_PDC? Also,
>>>> I think that it should select ATMEL_SSC, to be able to use
>>>> simple/graph-card with SSC.
>>>
>>> Hmm. The Kconfig dependencies seems overly complicated, do you mind if I
>>> get rid of most of the entries in the process?
>>>
>>
>> Unfortunately, it is just complicated enough. This is done to support
>> all the possible configurations. Removing them will lead to linking
>> errors.
>>
>> After having that discussion back in March, I had a very quick look and
>> didn't send a patch because I still had linking issues. It is not
>> impossible but it required more time than I had.
> 
> Can you try patch below if it covers the configurations you mention?
> This uses Kconfig's m/y resolution instead of open-coded defaults.
> 
> Best Regards,
> Michał Mirosław
> 
> 
> diff --git a/sound/soc/atmel/Kconfig b/sound/soc/atmel/Kconfig
> index 06c1d5ce642c..f118c229ed82 100644
> --- a/sound/soc/atmel/Kconfig
> +++ b/sound/soc/atmel/Kconfig
> @@ -12,25 +12,31 @@ if SND_ATMEL_SOC
>   config SND_ATMEL_SOC_PDC
>   	tristate
>   	depends on HAS_DMA
> -	default m if SND_ATMEL_SOC_SSC_PDC=m && SND_ATMEL_SOC_SSC=m
> -	default y if SND_ATMEL_SOC_SSC_PDC=y || (SND_ATMEL_SOC_SSC_PDC=m && SND_ATMEL_SOC_SSC=y)
> -
> -config SND_ATMEL_SOC_SSC_PDC
> -	tristate
>   
>   config SND_ATMEL_SOC_DMA
>   	tristate
>   	select SND_SOC_GENERIC_DMAENGINE_PCM
> -	default m if SND_ATMEL_SOC_SSC_DMA=m && SND_ATMEL_SOC_SSC=m
> -	default y if SND_ATMEL_SOC_SSC_DMA=y || (SND_ATMEL_SOC_SSC_DMA=m && SND_ATMEL_SOC_SSC=y)
> -
> -config SND_ATMEL_SOC_SSC_DMA
> -	tristate
>   
>   config SND_ATMEL_SOC_SSC
>   	tristate
> -	default y if SND_ATMEL_SOC_SSC_DMA=y || SND_ATMEL_SOC_SSC_PDC=y
> -	default m if SND_ATMEL_SOC_SSC_DMA=m || SND_ATMEL_SOC_SSC_PDC=m
> +
> +config SND_ATMEL_SOC_SSC_PDC
> +	tristate "SoC PCM DAI support for AT91 SSC controller using PDC"
> +	depends on ATMEL_SSC
> +	select SND_ATMEL_SOC_PDC
> +	select SND_ATMEL_SOC_SSC
> +	help
> +	  Say Y or M if you want to add support for Atmel SSC interface
> +	  in PDC mode configured using audio-graph-card in device-tree.
> +
> +config SND_ATMEL_SOC_SSC_DMA
> +	tristate "SoC PCM DAI support for AT91 SSC controller using DMA"
> +	depends on ATMEL_SSC
> +	select SND_ATMEL_SOC_DMA
> +	select SND_ATMEL_SOC_SSC
> +	help
> +	  Say Y or M if you want to add support for Atmel SSC interface
> +	  in DMA mode configured using audio-graph-card in device-tree.
>   
>   config SND_AT91_SOC_SAM9G20_WM8731
>   	tristate "SoC Audio support for WM8731-based At91sam9g20 evaluation board"
>

Looks fine to me.

Best regards,
Codrin
Alexandre Belloni Aug. 23, 2019, 3:09 p.m. UTC | #9
On 24/07/2019 01:25:05+0200, mirq-linux@rere.qmqm.pl wrote:
> On Tue, Jul 23, 2019 at 08:39:15PM +0200, Alexandre Belloni wrote:
> > On 23/07/2019 18:43:12+0200, mirq-linux@rere.qmqm.pl wrote:
> > > On Tue, Jul 23, 2019 at 01:36:37PM +0000, Codrin.Ciubotariu@microchip.com wrote:
> > > > On 22.07.2019 21:27, Michał Mirosław wrote:
> > > > > Allow SSC to be used on platforms described using audio-graph-card
> > > > > in Device Tree.
> > > > > 
> > > > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > > > > ---
> > > > >   sound/soc/atmel/Kconfig | 2 +-
> > > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/sound/soc/atmel/Kconfig b/sound/soc/atmel/Kconfig
> > > > > index 06c1d5ce642c..9ef9d25bb517 100644
> > > > > --- a/sound/soc/atmel/Kconfig
> > > > > +++ b/sound/soc/atmel/Kconfig
> > > > > @@ -25,7 +25,7 @@ config SND_ATMEL_SOC_DMA
> > > > >   	default y if SND_ATMEL_SOC_SSC_DMA=y || (SND_ATMEL_SOC_SSC_DMA=m && SND_ATMEL_SOC_SSC=y)
> > > > >   
> > > > >   config SND_ATMEL_SOC_SSC_DMA
> > > > > -	tristate
> > > > > +	tristate "SoC PCM DAI support for AT91 SSC controller using DMA"
> > > > 
> > > > Could you please make something similar for SND_ATMEL_SOC_SSC_PDC? Also, 
> > > > I think that it should select ATMEL_SSC, to be able to use 
> > > > simple/graph-card with SSC.
> > > 
> > > Hmm. The Kconfig dependencies seems overly complicated, do you mind if I
> > > get rid of most of the entries in the process?
> > > 
> > 
> > Unfortunately, it is just complicated enough. This is done to support
> > all the possible configurations. Removing them will lead to linking
> > errors.
> > 
> > After having that discussion back in March, I had a very quick look and
> > didn't send a patch because I still had linking issues. It is not
> > impossible but it required more time than I had.
> 
> Can you try patch below if it covers the configurations you mention?
> This uses Kconfig's m/y resolution instead of open-coded defaults.
> 

Seems good to me, thanks.
diff mbox series

Patch

diff --git a/sound/soc/atmel/Kconfig b/sound/soc/atmel/Kconfig
index 06c1d5ce642c..9ef9d25bb517 100644
--- a/sound/soc/atmel/Kconfig
+++ b/sound/soc/atmel/Kconfig
@@ -25,7 +25,7 @@  config SND_ATMEL_SOC_DMA
 	default y if SND_ATMEL_SOC_SSC_DMA=y || (SND_ATMEL_SOC_SSC_DMA=m && SND_ATMEL_SOC_SSC=y)
 
 config SND_ATMEL_SOC_SSC_DMA
-	tristate
+	tristate "SoC PCM DAI support for AT91 SSC controller using DMA"
 
 config SND_ATMEL_SOC_SSC
 	tristate