[alsa-devel] [PATCH v2 7/9] ASoC: Codec: Add sti platform codec

[Mark Brown]

On Mon, Jun 01, 2015 at 07:41:54PM +0200, Arnaud Pouliquen wrote:
> On 05/25/2015 03:01 PM, Mark Brown wrote:

> >These event functions all look very similar and I can't help but think
> >that they look awfully like the sort of register write sequences that
> >DAPM normally generates anyway.  Is there any great reason for not doing
> >these by registering multiple widgets with routes between them rather
> >than with custom code?

> I need to respect the sequence for power up /power down. for sure I could
> manage it using 3 DAPMs with routes, but how to ensure sequence?

There's a defined ordering for handling widgets of different types wich
probably suffices.

> Moreover, i will need to implement a cpu dai DAPM to manage uniperipheral
> clock. This clock need to be enabled/disabled before/after DAC to avoid
> plop. So would prefer to manage the DAC sequence in a single function...

We already handle clock and supply widgets very early and late in the
sequence for precisely this reason.

> >>+	div = sti_sas_dai_clk_div[dai->id];
> >>+	if (cpu_dai->driver->ops->set_clkdiv)
> >>+		return cpu_dai->driver->ops->set_clkdiv(cpu_dai,
> >>+							SND_SOC_CLOCK_OUT, div);
> >>+	dev_warn(codec->dev, "WARN: CPU DAI not support sysclk div");

> >This is worrying, we shouldn't be peering inside the CPU DAI like this.
> >I'd expect this to either be done autonomously by the CPU DAI or handled
> >in a machine driver.

> I think i misunderstand your remark in V1...but i still not understand how
> you want that i implement it, if i can't neither use snd_soc_dai_set_clkdiv
> (except implement it in simple_card).
> Please could you precise how you would like that i implement the feature?

I don't know exactly what "this feature" is so it's hard to be sure what
the above is supposed to do but the fact that you're having one driver
for your platform set a clock divider in a completely different driver
for that platform should be an enormous red flag - at a bare minimum
whatever you're doing here doesn't seem like it should be done in this
other driver.

> I'm kind surprise that nobody else have this kind of setup. I know at least
> one codec that should need this kind of service: AK4628.
> For this codec, depending on runtime frequency, a division value should be
> apply between MCLK and SCLK...

We already have a _set_bclk_ratio() API if that's what you're looking
for?  It's not something that your CPU drivers should ever be using,
though - it's something for machine drivers.  Your CPU drivers can't
know what configuration a particular machine is going to require.  I
don't know what SCLK is here, do you mean LRCLK?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20150602/b079c6b5/attachment.sig>