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:

quoted

On Wed, May 26, 2021 at 02:41:00PM -0400, Peter Geis wrote:

quoted

On Wed, May 26, 2021 at 12:23 PM Rudi Heitbaum [off-list ref] wrote:

...

quoted

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.

quoted

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.

quoted

+??????????????????????????????vsel-gpios = <&gpio1 RK_PC1 GPIO_ACTIVE_HIGH>;

Is vsel-gpios ever used in the mainline driver?

quoted

+??????????????????????????????regulator-compatible = "fan53555-reg";

Ditto.

quoted

+??????????????????????????????vsel-gpios = <&gpio1 RK_PB6 GPIO_ACTIVE_HIGH>;

Ditto.

quoted

+??????????????????????????????regulator-boot-on;

Just out of curiosity, is regulator-boot-on really needed for the GPU?

Will check.