Re: [PATCH] regulator: bd718x7, bd71828, Fix dvs voltage levels

[Vaittinen, Matti]

On Fri, 2021-01-15 at 14:41 +0000, Lee Jones wrote:
> On Fri, 15 Jan 2021, Matti Vaittinen wrote:
> 
> > The ROHM BD718x7 and BD71828 drivers support setting HW state
> > specific voltages from device-tree. This is used also by various
> > in-tree DTS files.
> > 
> > These drivers do incorrectly try to compose bit-map using enum
> > values. By a chance this works for first two valid levels having
> > values 1 and 2 - but setting values for the rest of the levels
> > do indicate capbility of setting values for first levels as
> > well. Luckily the regulators which support settin values for
> > SUSPEND/LPSR do usually also support setting values for RUN
> > and IDLE too - thus this has not been such a fatal issue.
> > 
> > Fix this by defining the old enum values as bits and using
> > new enum in parsing code. This allows keeping existing IC
> > specific drivers intact and only adding the defines and
> > slightly changing the rohm-regulator.c
> > 
> > Fixes: 21b72156ede8b ("regulator: bd718x7: Split driver to common
> > and bd718x7 specific parts")
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx>
> > ---
> > 
> > One more attempt today. I did test the driver is not causing a
> > crash
> > when load but no further tests concluded as I don't have
> > BD71837/47/50
> > at home. This looks now trivial though so I decided to give it a
> > go.
> > Sorry for all the trouble this far - and also for the mistakes to
> > come.
> 
> Why don't you wait until next week when you can run this on real h/w
> with some pretty debug to ensure it does the right thing?

I first thought I would. Then I didn't wait because - as I said - this
looks pretty trivial to me now - and because I thought it might get
merged quickly. If you see it risky, then please don't merge. I will
test this next week and can resend after that if necessary.

Sorry for the trouble again.

Best Regards
--Matti