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