Re: [PATCH v2 1/5] sound: sam9x5_wm8731: machine driver for at91sam9x5 wm8731 boards

2013-07-09 Thread Richard Genoud
2013/7/8 Mark Brown broo...@kernel.org:
 On Mon, Jul 08, 2013 at 03:29:49PM +0200, Richard Genoud wrote:

 + * Nicolas Ferre nicolas.fe...@atmel.com
 + *
 + * Based on sam9g20_wm8731.c by:
 + * Sedji Gaouaou sedji.gaou...@atmel.com

 The obvious question here is of course if we can use the same driver for
 both of them.
I haven't got a g20 to test that, but that's the goal.
For now, g20 is still non-DT, so I think it's best to have a DT-only
driver like this one for the 9x5 family (9g15, 9g25, 9x25, 9g35,
9x25).
When the g20 will move to DT completely, we can drop sam9g20_wm8731.c
and adjust sam9x5_wm8731.c (mainly master clock and widgets it seems)
By the way, maybe g45 could use that also (and SAMA5 ?)


 + codec_dai-driver-playback.rates = SNDRV_PCM_RATE_8000 |
 + SNDRV_PCM_RATE_32000 |
 + SNDRV_PCM_RATE_48000 |
 + SNDRV_PCM_RATE_96000;
 + codec_dai-driver-capture.rates = SNDRV_PCM_RATE_8000 |
 + SNDRV_PCM_RATE_32000 |
 + SNDRV_PCM_RATE_48000 |
 + SNDRV_PCM_RATE_96000;

 You definitely shouldn't be fiddling with a driver's constant static
 data.  You want to be using snd_pcm_hw_constraint() APIs to set
 additional constraints intead.
Ok, I'll change that.


Thanks !

-- 
for me, ck means con kolivas and not calvin klein... does it mean I'm a geek ?
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v2 1/5] sound: sam9x5_wm8731: machine driver for at91sam9x5 wm8731 boards

2013-07-09 Thread Bo Shen

Hi Richard,

On 7/9/2013 16:19, Richard Genoud wrote:

2013/7/8 Mark Brown broo...@kernel.org:

On Mon, Jul 08, 2013 at 03:29:49PM +0200, Richard Genoud wrote:


+ * Nicolas Ferre nicolas.fe...@atmel.com
+ *
+ * Based on sam9g20_wm8731.c by:
+ * Sedji Gaouaou sedji.gaou...@atmel.com


The obvious question here is of course if we can use the same driver for
both of them.

I haven't got a g20 to test that, but that's the goal.
For now, g20 is still non-DT, so I think it's best to have a DT-only
driver like this one for the 9x5 family (9g15, 9g25, 9x25, 9g35,
9x25).
When the g20 will move to DT completely, we can drop sam9g20_wm8731.c
and adjust sam9x5_wm8731.c (mainly master clock and widgets it seems)


The at91sam9g20ek board can work in DT mode, and the sound has support 
in DT mode already.


Sure, the mainly thing we need deal with is the master clock and 
widgets. If put them together will make code ugly or need many many 
#ifdef, I suggest to keep them separately.



By the way, maybe g45 could use that also (and SAMA5 ?)


For g45ek board, it use AC97 interface, not the case.




+ codec_dai-driver-playback.rates = SNDRV_PCM_RATE_8000 |
+ SNDRV_PCM_RATE_32000 |
+ SNDRV_PCM_RATE_48000 |
+ SNDRV_PCM_RATE_96000;
+ codec_dai-driver-capture.rates = SNDRV_PCM_RATE_8000 |
+ SNDRV_PCM_RATE_32000 |
+ SNDRV_PCM_RATE_48000 |
+ SNDRV_PCM_RATE_96000;


You definitely shouldn't be fiddling with a driver's constant static
data.  You want to be using snd_pcm_hw_constraint() APIs to set
additional constraints intead.

Ok, I'll change that.


Thanks !



Best Regards,
Bo Shen
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v2 1/5] sound: sam9x5_wm8731: machine driver for at91sam9x5 wm8731 boards

2013-07-09 Thread Mark Brown
On Tue, Jul 09, 2013 at 10:19:45AM +0200, Richard Genoud wrote:
 2013/7/8 Mark Brown broo...@kernel.org:

  The obvious question here is of course if we can use the same driver for
  both of them.

 I haven't got a g20 to test that, but that's the goal.

I think I do somewhere.

 For now, g20 is still non-DT, so I think it's best to have a DT-only
 driver like this one for the 9x5 family (9g15, 9g25, 9x25, 9g35,
 9x25).
 When the g20 will move to DT completely, we can drop sam9g20_wm8731.c
 and adjust sam9x5_wm8731.c (mainly master clock and widgets it seems)
 By the way, maybe g45 could use that also (and SAMA5 ?)

If this is the goal then this driver needs a more generic name.


signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v2 1/5] sound: sam9x5_wm8731: machine driver for at91sam9x5 wm8731 boards

2013-07-09 Thread Richard Genoud
2013/7/9 Mark Brown broo...@kernel.org:
 On Tue, Jul 09, 2013 at 10:19:45AM +0200, Richard Genoud wrote:
 2013/7/8 Mark Brown broo...@kernel.org:

  The obvious question here is of course if we can use the same driver for
  both of them.

 I haven't got a g20 to test that, but that's the goal.

 I think I do somewhere.

 For now, g20 is still non-DT, so I think it's best to have a DT-only
 driver like this one for the 9x5 family (9g15, 9g25, 9x25, 9g35,
 9x25).
 When the g20 will move to DT completely, we can drop sam9g20_wm8731.c
 and adjust sam9x5_wm8731.c (mainly master clock and widgets it seems)
 By the way, maybe g45 could use that also (and SAMA5 ?)

 If this is the goal then this driver needs a more generic name.
Hum. I guess we could call it atmel-ssc-sound.c, since the ultimate
goal would be to make it codec agnostic, but there's quite some work
to achieve that (handle the widgets, mclk and rates by DT).
But I really don't know if it's better to keep this name as long as
g20 is not supported, and change it after or have a name that is not
reflecting what the machine driver is doing right now.
Maybe Bo or Nicolas can give their feeling on that (since I'm not
Atmel, I don't know every SoC or what their Big Plan is for sound
machine drivers)

Richard.

-- 
for me, ck means con kolivas and not calvin klein... does it mean I'm a geek ?
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss