[RFC PATCH 1/3] ep93xx i2s driver

[Ryan Mallon]

H Hartley Sweeten wrote:
> On Monday, May 17, 2010 9:53 PM, Ryan Mallon wrote:
>> Add ep93xx i2s audio driver
>>
>> Signed-off-by: Ryan Mallon <ryan at bluewatersys.com>
>> ---
>>

>> +config SND_EP93XX_SOC_SNAPPERCL15
>> +        tristate "SoC Audio support for Bluewater Systems Snapper CL15 module"
>> +        depends on SND_EP93XX_SOC && MACH_SNAPPER_CL15
>> +        select SND_EP93XX_SOC_I2S
>> +        select SND_SOC_TLV320AIC23
> 
> Missing help text?

Thanks. Fixed.

>> +     struct ep93xx_pcm_dma_params    *dma_params[2];
>> +     struct resource                 *mem;
>> +     void __iomem                    *regs;
>> +};
> 
> Also, I saw Chase Douglas' comment on this struct and the defines.
> 
> If they are "only" used in this source file please leave them here.  If there
> is not a reason to expose the information globally, keep it private to the
> driver.

Yes, I'm also inclined to leave this all here since it keeps the file
self contained.

>> +static inline void ep93xx_i2s_enable_fifos(struct ep93xx_i2s_info *info,
>> +                                        int enable)
> 
> Not sure if this should be 'inline'

Removed.

>> +
>> +     ep93xx_syscon_swlocked_write(val, EP93XX_SYSCON_I2SCLKDIV);
>> +     return 0;
>> +}
> 
> As you noted, this is a bit ugly.
> 
> If you create two pseudo clocks in clock.c for the sclk and lrclk this can
> all be changed to:

I'm going to try and rework this a little. I think the pseudo clock
approach is the way to go. As Mark pointed out, the clkdiv function can
probably be removed and the sdiv/lrdiv calculation done as part of
ep93xx_i2s_hw_params.

>> diff --git a/sound/soc/ep93xx/ep93xx-pcm.c b/sound/soc/ep93xx/ep93xx-pcm.c
>> new file mode 100644
>> index 0000000..c071b5c
>> --- /dev/null
>> +++ b/sound/soc/ep93xx/ep93xx-pcm.c
> 
> Is this pcm driver generic enough to work with an AC97 driver?  Looking at
> some of the other soc drivers it appears there are different pcm drivers for
> different codec interfaces.  If it's not completely generic please rename this
> file as appropriate.

I don't actually know, I think it is. The PCM driver is originally
written by Lennert and the only changes I have really made are to update
it for the alsa soc interface. IIRC, Lennert was using it with an ac97
driver.

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com         	PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com	New Zealand
Phone: +64 3 3779127		Freecall: Australia 1800 148 751
Fax:   +64 3 3779135			  USA 1800 261 2934