[alsa-devel] [PATCH 2/2] ASoC: sgtl5000: clean up sgtl5000_enable_regulators()

[Li.Xiubo at freescale.com]

> On Fri, Dec 13, 2013 at 02:43:03PM +0800, Shawn Guo wrote:
> > Function sgtl5000_enable_regulators() is somehow odd in handling the
> > optional external VDDD supply.  The driver can only enable this supply
> > on SGTL5000 chip before revision 0x11, and of course when this
> > external VDDD is present.  It currently does something like below.
> >
> > 1. Check if regulator_bulk_get() on VDDA, VDDIO and VDDD will fail.  If
> >    it fails, VDDD must be absent and it falls on internal LDO by calling
> >    sgtl5000_replace_vddd_with_ldo().  Otherwise, VDDD is used.  And in
> >    either case, regulator_bulk_enable() will be called to enable
> >    3 supplies.
> >
> > 2. In case that SGTL5000 revision is later than 0x11, even if external
> >    VDDD is present, it has to roll back the 'enable' and 'get' calls
> >    with regulator_bulk_disable() and regulator_bulk_free(), and starts
> >    over again by calling sgtl5000_replace_vddd_with_ldo() and
> >    regulator_bulk_enable().
> >
> > Such back and forth calls sequence is complicated and unnecessary.
> > Also, since commit 4ddfebd (regulator: core: Provide a dummy regulator
> > with full constraints), regulator_bulk_get() will always succeeds
> > because of the dummy regulator.  Thus the VDDD detection is broken.
> >
> > The patch changes the flow to something like the following, which
> > should be more reasonable and clear, and also fix the VDDD detection
> breakage.
> >
> > 1. Check if we're running a chip before revision 0x11, on which an
> >    external VDDD can possibly be an option.
> >
> > 2. If it is an early revision, call regulator_get_optional() to detect
> >    whether an external VDDD supply is available.
> >
> > 3. If external VDDD is present, call sgtl5000_replace_vddd_with_ldo() to
> >    update sgtl5000->supplies info.
> >
> > 4. Drop regulator_bulk_get() call in sgtl5000_replace_vddd_with_ldo(),
> >    and call it in sgtl5000_enable_regulators() no matter it's an
> >    external VDDD or internal LDO.
> >
> > 5. Call regulator_bulk_enable() to enable these 3 regulators.
> >
> > Signed-off-by: Shawn Guo <shawn.guo at linaro.org>
> 
> Apart from that separate issue we're discussing, do you have any comment on
> this patch itself?

Hi Shawn,

Sorry for late.

It's looks good to me.

--
Xiubo