linux-kernel - Re: [PATCH] regulator: core: resolve supply voltage deferral silently

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210903111023.GE4932@sirena.org.uk>
Date:   Fri, 3 Sep 2021 12:10:23 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Brian Norris <briannorris@...omium.org>
Cc:     Chen-Yu Tsai <wenst@...omium.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] regulator: core: resolve supply voltage deferral silently

On Thu, Sep 02, 2021 at 03:41:02PM -0700, Brian Norris wrote:
> On Thu, Sep 2, 2021 at 10:06 AM Mark Brown <broonie@...nel.org> wrote:
> > On Wed, Sep 01, 2021 at 01:06:28PM -0700, Brian Norris wrote:

> > > Take a look at the commit this "Fixes":
> >
> > > 21e39809fd7c ("regulator: vctrl: Avoid lockdep warning in enable/disable ops")

> > That driver change is at most tangentially related to the code that's
> > being updated,

> It introduced another case where we hit a spurious error log. And
> below, you admit that you didn't understand what this is fixing
> without that pointer. I guess we disagree.

The point is the "another" bit - by just picking a random commit you
will cause people to think that an issue was introduced by that commit
which in turn means that people will for example use the presence of
that commit as a guide to backporting.  They may not backport things far
enough since the random commit isn't there, or backport things too far
if the actual issue was introduced later which can be even worse as it
can introduce breakage where it wasn't before.

In terms of not understanding the issue here is that the patch didn't
pass the smell test, it was your explanation that helped here not the
pointing at a driver change that lacks obvious relevance.  I really
don't know what the reader is supposed to infer about the change from
the commit, 

> > That's definitely an abuse of the API, the hardware design is pretty
> > much a gross hack anywhere as far as I remember.  As Chen-Yu says I'd
> > only expect this to be possible in the case where the supply is in
> > bypass mode and hasn't got its own parent.  In any case I can see why
> > it's happening now...

> Well the hardware exists, the driver exists, and it all worked OK
> until somewhat recently (and now it works again, thanks to Chen-Yu).
> What should we do here, then? Just leave the "abuse" in place?

I don't think anyone came up with anything more tasteful to do with that
hardware, like I say the hardware is itself very hacky.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)