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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <0f2b2a84818b7c83ce50ff20833c7c77150690ad.camel@fi.rohmeurope.com>
Date:   Fri, 15 Jan 2021 14:48:06 +0000
From:   "Vaittinen, Matti" <Matti.Vaittinen@...rohmeurope.com>
To:     "lee.jones@...aro.org" <lee.jones@...aro.org>
CC:     "lgirdwood@...il.com" <lgirdwood@...il.com>,
        linux-power <linux-power@...rohmeurope.com>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] regulator: bd718x7, bd71828, Fix dvs voltage levels


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@...rohmeurope.com>
> > ---
> > 
> > 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