Messages in this thread | | | Date | Mon, 23 Mar 2015 13:52:13 +0000 | From | Lee Jones <> | Subject | Re: [PATCH v2 1/4] mfd: arizona: Factor out SYSCLK enable from wm5102 hardware patch |
| |
On Tue, 17 Mar 2015, Charles Keepax wrote:
> wm5102 applies a custom hardware boot sequence, for this the SYSCLK > needs to be enabled. This patch factors out the code that enables > SYSCLK for this sequence such that it can be used for other boot time > operations that require SYSCLK. > > Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com> > --- > > Changes since v1: > - Added some comments in patch 4 to explain the exact regulator voltage > ranges > - Removed the reset patch, whilst I work on the suggestions by Mark, the > rest of the series is useful in its own right and doesn't need to be > blocked by that > > Thanks, > Charles > > drivers/mfd/arizona-core.c | 48 ++++++++++++++++++++++++++++--------------- > 1 files changed, 31 insertions(+), 17 deletions(-) > > diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c > index 6ca6dfa..ef1f8aa 100644 > --- a/drivers/mfd/arizona-core.c > +++ b/drivers/mfd/arizona-core.c > @@ -250,7 +250,8 @@ static int arizona_wait_for_boot(struct arizona *arizona) > return ret; > } > > -static int arizona_apply_hardware_patch(struct arizona* arizona) > +static int arizona_exec_with_sysclk(struct arizona *arizona, > + int (*exec)(struct arizona *)) > { > unsigned int fll, sysclk; > int ret, err; > @@ -292,23 +293,8 @@ static int arizona_apply_hardware_patch(struct arizona* arizona) > goto err_fll; > } > > - /* Start the write sequencer and wait for it to finish */ > - ret = regmap_write(arizona->regmap, ARIZONA_WRITE_SEQUENCER_CTRL_0, > - ARIZONA_WSEQ_ENA | ARIZONA_WSEQ_START | 160); > - if (ret != 0) { > - dev_err(arizona->dev, "Failed to start write sequencer: %d\n", > - ret); > - goto err_sysclk; > - } > - ret = arizona_poll_reg(arizona, 5, ARIZONA_WRITE_SEQUENCER_CTRL_1, > - ARIZONA_WSEQ_BUSY, 0); > - if (ret != 0) { > - regmap_write(arizona->regmap, ARIZONA_WRITE_SEQUENCER_CTRL_0, > - ARIZONA_WSEQ_ABORT); > - ret = -ETIMEDOUT; > - } > + ret = exec(arizona); > > -err_sysclk: > err = regmap_write(arizona->regmap, ARIZONA_SYSTEM_CLOCK_1, sysclk); > if (err != 0) { > dev_err(arizona->dev, > @@ -330,6 +316,34 @@ err_fll: > return err; > } > > +static int arizona_hardware_patch_wseq(struct arizona *arizona) > +{ > + int ret; > + > + /* Start the write sequencer and wait for it to finish */ > + ret = regmap_write(arizona->regmap, ARIZONA_WRITE_SEQUENCER_CTRL_0, > + ARIZONA_WSEQ_ENA | ARIZONA_WSEQ_START | 160); > + if (ret != 0) { > + dev_err(arizona->dev, "Failed to start write sequencer: %d\n", > + ret); > + return ret; > + } > + ret = arizona_poll_reg(arizona, 5, ARIZONA_WRITE_SEQUENCER_CTRL_1, > + ARIZONA_WSEQ_BUSY, 0); > + if (ret != 0) { > + regmap_write(arizona->regmap, ARIZONA_WRITE_SEQUENCER_CTRL_0, > + ARIZONA_WSEQ_ABORT); > + ret = -ETIMEDOUT; > + } > + > + return ret; > +} > + > +static inline int arizona_apply_hardware_patch(struct arizona *arizona) > +{ > + return arizona_exec_with_sysclk(arizona, arizona_hardware_patch_wseq);
Not sure I like this much.
What's the reason for over-complicating this? Can you just:
arizona_exec_with_sysclk(arizona); arizona_hardware_patch_wseq(arizona);
... or if you need that call to be in the middle, split the calls up further.
> +} > + > #ifdef CONFIG_PM > static int arizona_runtime_resume(struct device *dev) > {
-- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
| |