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

[Charles Keepax <ckeepax-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>]

On Thu, Jun 19, 2014 at 09:34:19AM +0800, Sean Cross wrote:
> On 06/18/14 18:31, Mark Brown wrote:
> > On Wed, Jun 18, 2014 at 06:22:52PM +0800, Sean Cross wrote:
> >> On 06/18/14 18:02, Mark Brown wrote:
> >>> 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.
> I'm having trouble understanding where the separating line is between
> machine drivers and codec drivers.  A random sampling of codec drivers
> doesn't yield any drivers grabbing their own power switches.  Is that an
> oversight?  Should every codec driver include at least one regulator to
> describe its power source?
> 
> If you like, I can move the regulator from the machine driver to the
> codec driver, and make it non-optional.  But we've done designs in the
> past where it just hangs off the 3.3V rail where it's non-switchable,
> and the concept of describing a regulator seemed overkill.
> 
> 
> Sean
> 

I think for your power switch you should be able to use the
existing gpio regulator binding, so you shouldn't need to
actually write any code.

Take for example wm2200.c, this registers two supplies for the
CODEC through DAPM:

SND_SOC_DAPM_REGULATOR_SUPPLY("CPVDD", 20, 0),
SND_SOC_DAPM_REGULATOR_SUPPLY("AVDD", 20, 0),

These are then hooked up to whatever parts of the CODEC require
power from them. CPVDD powers the charge pumps so is only hooked
up to those, AVDD is the main analogue power for the chip so is
needed whenever the CODEC is up, for such supplies one approach
is to just to hook them up to all the CODEC inputs and outputs,
which is what we do here.

You mention that your power gate might only exist on some
hardware. This hardware differences are what device tree is there
to describe. So lets say your CODEC has a main power supply called
AVDD as in my example above:

codec_pwr: regulator@0 {
	compatble = "regulator-fixed";

	... fixed regulator stuff ...
};

codec_pwr_switch: regulator@0 {
	compatible = "regulator-gpio";

	... regulator gpio stuff ...
};

/* Board with switch */
codec: es8328@11 {
       compatible = "es8328";
       reg = <0x11>;
	AVDD-supply = <&codec_pwr_switch>;
};

/* Board without switch */
codec: es8328@11 {
       compatible = "es8328";
       reg = <0x11>;
	AVDD-supply = <&codec_pwr>;
};

Thanks,
Charles

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html