[alsa-devel] [PATCH V5 3/3] ASoC: AMD: add AMD ASoC ACP-I2S driver

[maruthi srinivas]

On Tue, Aug 25, 2015 at 11:36 AM, Mark Brown <broonie at kernel.org> wrote:
> On Mon, Aug 24, 2015 at 04:08:31PM -0400, Alex Deucher wrote:
>> On Fri, Aug 21, 2015 at 12:17 PM, Mark Brown <broonie at kernel.org> wrote:
>
>> > What I'm looking for is actual code sharing where we use the same code
>> > for the I2S controller block or a clear and documented understanding of
>> > why it is not possible to share things.
>
>> Maruthi can clarify further, but it's not possible to use the
>> designware driver directly because:
>> 1. The i2s registers are within the same MMIO aperture as our other
>> GPU registers.  Our GPU driver is designed in such a way that the
>> specific IP modules don't have direct access to the MMIO aperture.
>> They use functions provided by the core driver to access registers.
>> Thus the ACP IP module within the GPU driver does not have direct
>> access to the mmio base pointer in order to pass it on.
>
> Please explain this in more detail, shared register ranges are very
> common and are the sort of things MFDs are supposed to help with.
>

In our case, ACP I2S driver need not do a  'devm_ioremap_resource' to
get mmio base.
ACP audio IP (DMA + I2S+ Others) registers can be accessed, using
GPU's MMIO base.
During GPU driver design, it was decided that all the register access
for entire GPU MMIO
aperture (includes ACP and others) to be done in GPU module only.
This is implemented in another patch in this patch series using a
abstraction layer.
A set of functions were defined for audio DMA and I2S functionality
within GPU driver
module and those function pointers were passed as platform data to ALSA PCM
driver to handle both DMA and I2S.

>> 2. The designware driver depends on the CLKDEV framework which we
>> don't currently support.
>
> You need to support the clock API, it's very easy to do so so there is
> no excuse for doing something custom here.
>

Codec acts as master in our case to provide clock to i2s controller and
there wasn't a need to use clock APIs unlike in existing designware i2s driver.
There is no custom implementation.

>> 3. Our hardware does not support S16_LE
>
> If you have modified the designware IP to remove this support (why would
> anyone do that?) it's a trivial quirk, if the restriction comes from
> some other part of the system like the DMA driver then the constraint
> will come from that part of the system.

There is a bug in ACP SoC implementation (which combines internal DMA,
designware I2S
and other blocks) for 16bit and lower resolution. I felt , it would be
better to limit functionality
in I2S DAI capabilities. I will put  this limitation in DMA driver
capabilities, to represent overall
sound card capabilities, if you suggest.