Messages in this thread | | | Date | Thu, 27 May 2021 12:29:15 +0000 | From | Rudi Heitbaum <> | 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@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.
| |