linux-kernel - Re: [PATCH] regulator: fan53555: add back tcs4526

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: <20210527122911.GA1640@7698f5da3a10>
Date:   Thu, 27 May 2021 12:29:15 +0000
From:   Rudi Heitbaum <rudi@...tbaum.com>
To:     Ezequiel Garcia <ezequiel@...labora.com>
Cc:     Peter Geis <pgwipeout@...il.com>, devicetree@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        Mark Brown <broonie@...nel.org>, chenjh@...k-chips.com
Subject: Re: [PATCH] regulator: fan53555: add back tcs4526

Hi Ezequiel and Peter,

Thanks for the feedback.

On Thu, May 27, 2021 at 08:51:27AM -0300, Ezequiel Garcia wrote:
> Hi Rudi,
> 
> Thanks for the patch.
> 
> On Thu, 2021-05-27 at 10:59 +0000, Rudi Heitbaum wrote:
> > On Wed, May 26, 2021 at 02:41:00PM -0400, Peter Geis wrote:
> > > On Wed, May 26, 2021 at 12:23 PM Rudi Heitbaum <rudi@...tbaum.com> wrote:

...

> > I chose to follow the example of silergy,syr827 and silergy,syr828 for
> > tcs4526 (given I made the mistake in assuming that support for tcs4525
> > meant support for tcs4525.) This would maintain consistency of naming
> > of
> > tcs4526 throughout the source. Is that ok?
> 
> It's fine to have both compatibles (and avoids confusion in
> device-trees), just remember to update the dt-bindings as well.
> It's funny to see drivers with both schemes, so we really have to
> decide which path we want to go down.
> Considering the syr827/syr828 as convention, we should probably just
> go down that route for consistency within the driver.

Thanks Peter - I will resubmit the tcs4526 patch along these lines.

> > Removal of the operating points kind of makes the gpu regulator
> > moot, don't you think?

> As Peter rightly said, this will prevent gpu devfreq from working.

This is the draft that I have been working on within LibreELEC10, still
a ways to go I'm afraid. Having decided to getting the SBC to run
mainline kernel and u-boot. The regulator and subsequent regulator fix
will hopefully address https://bugzilla.kernel.org/show_bug.cgi?id=212917 
too. As you have identified - it is not ready for upstreaming :-)

Thank you both for the direction and pointers on the dts. I will get the
v2 patch going first.

> Out of curiosity, why disabling the little VOP?

Only disabled within my WIP dts so as to focus my attention on successful 
running of LE10 on the rk3399pro, before I move to attempting to enable 
the NPU and the other nodes.

> I can be wrong, but I think regulator-compatible is deprecated.

I will look it to this. 

Now with the addition of the regulator and Peter's fix I will start
qualifing each of the dts nodes and the correct options against the
schematic.

> > +??????????????????????????????vsel-gpios = <&gpio1 RK_PC1 GPIO_ACTIVE_HIGH>;
> 
> Is vsel-gpios ever used in the mainline driver?
>
> > +??????????????????????????????regulator-compatible = "fan53555-reg";
> 
> Ditto.
>
> > +??????????????????????????????vsel-gpios = <&gpio1 RK_PB6 GPIO_ACTIVE_HIGH>;
> 
> Ditto.
>
> > +??????????????????????????????regulator-boot-on;
> 
> Just out of curiosity, is regulator-boot-on really needed for the GPU?

Will check.