Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 27 May 2021 12:29:15 +0000
From:   Rudi Heitbaum <rudi@heitbaum.com>
To:     Ezequiel Garcia <ezequiel@collabora.com>
Cc:     Peter Geis <pgwipeout@gmail.com>, devicetree@vger.kernel.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@lists.infradead.org>,
        Mark Brown <broonie@kernel.org>, chenjh@rock-chips.com
Subject: Re: [PATCH] regulator: fan53555: add back tcs4526
Message-ID: <20210527122911.GA1640@7698f5da3a10>
References: <20210526162342.GA20@8bbba9ba63a4>
 <CAMdYzYpZoKs3P62j02RW-+5BEpqC9JL3apjucTWLWmvNFrOrCg@mail.gmail.com>
 <20210527105943.GA441@7698f5da3a10>
 <462b8d80447efb6c00e93704914169bceb5adc4d.camel@collabora.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <462b8d80447efb6c00e93704914169bceb5adc4d.camel@collabora.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

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@heitbaum.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.