Re: [PATCH v4 3/3] ASoC: fsl: add imx-es8328 machine driver

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1666 bytes --]

On Wed, Jun 18, 2014 at 06:22:52PM +0800, Sean Cross wrote:
> On 06/18/14 18:02, Mark Brown wrote:

> > This should be handled by the clock bindings not open coded in the
> > driver - leaving this here most likely won't play nicely when the clock
> > API can configure the defaults for the tree.  There is supposed to be
> > support for setting default clock trees going in (or perhaps already in)
> > the clock bindings.

> Can you give me more information on it?  Currently, it looks like most
> boards use a 24 MHz clock, judging from this comment in
> mach-imx/clk-imx6q.c:

Look at the clock API, this stuff was introduced in the last merge
window if it's there yet at all.

> This codec requires the more unusual 22.5792 MHz clock.  What is the
> appropriate method of obtaining this particular frequency?

clk_set_rate() on the directly connected clock, the problem is fiddling
about with the parenting rather than setting the rate.

> > No, this is broken.  The CODEC should request its own supplies which
> > need to correspond to the supplies the physical device has and failing
> > to get the supplies should be a fatal error unless the device works
> > without power (in which case why bother enablin them at all?).

> Not all codecs have power supplies.  Most don't, in fact, it's just this

The manufactuers of those that don't are being awfully quiet about what
sounds like a rather impressive feature...

> Additionally, since the regulator is external to the codec (as it
> physically cuts 3.3V from the power supply), it doesn't make sense to
> put it in the codec driver.

I'm not sure you've quite understood what the regulator API is there
for.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]