Re: [PATCH v2 2/4] mfd: wm5110: Add register patch required for low power sleep

Charles Keepax
2015-03-23T14:22:25Z

On Mon, Mar 23, 2015 at 02:13:07PM +0000, Lee Jones wrote:
> On Tue, 17 Mar 2015, Charles Keepax wrote:
>
> > Some register settings must be applied before the first time low power
> > sleep mode is entered on the wm5110 to ensure optimium performance.
> > These settings require SYSCLK to be enabled whilst they are being
> > applied. This patch applies the settings using the recently factored out
> > boot time SYSCLK functionality.
> >
> > Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> > ---
> > drivers/mfd/arizona-core.c | 29 +++++++++++++++++++++++++++++
> > 1 files changed, 29 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
> > index ef1f8aa..8f61ccf 100644
> > --- a/drivers/mfd/arizona-core.c
> > +++ b/drivers/mfd/arizona-core.c
> > @@ -344,6 +344,25 @@ static inline int arizona_apply_hardware_patch(struct arizona *arizona)
> > return arizona_exec_with_sysclk(arizona, arizona_hardware_patch_wseq);
> > }
> >
> > +static const struct reg_default wm5110_sleep_patch[] = {
> > + { 0x337A, 0xC100 },
> > + { 0x337B, 0x0041 },
> > + { 0x3300, 0xA210 },
> > + { 0x3301, 0x050C },
> > +};
>
> Really not keen on all these magic numbers.
>
> Would you be kind enough to define them please?

This is basically just a register patch, its only not included in
the normal patch due to the constraint on SYSCLK being enabled.
Its not very normal to add defines for register patch entries and
often not really practical in terms of IP. I would prefer to
leave this as is if possible, I am not sure the sort of naming I
am going to put on it in terms of defines is likely to add much
more clarity to the situation.

>
> > +static inline int wm5110_sleep_patch_wseq(struct arizona *arizona)
>
> What is 'wseq'?
>
> I suggest if I'm having to ask that it's not a very good name.

Yeah good point I will update the name here.

Thanks,
Charles