Re: [PATCH v2 2/2] ARM: dts: rockchip: consolidate veyron panel and backlight settings

Am Mittwoch, 24. Juli 2019, 23:46:30 CEST schrieb Doug Anderson:

Hi,

On Thu, Jul 11, 2019 at 3:35 PM Matthias Kaehlcke [off-list ref] wrote:

quoted

diff --git a/arch/arm/boot/dts/rk3288-veyron-minnie.dts b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
index 4cc7d3659484..2b0801a539c9 100644
--- a/arch/arm/boot/dts/rk3288-veyron-minnie.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-minnie.dts

@@ -15,40 +15,6 @@
                     "google,veyron-minnie-rev0", "google,veyron-minnie",
                     "google,veyron", "rockchip,rk3288";

-       backlight_regulator: backlight-regulator {
-               compatible = "regulator-fixed";
-               enable-active-high;
-               gpio = <&gpio2 RK_PB4 GPIO_ACTIVE_HIGH>;
-               pinctrl-names = "default";
-               pinctrl-0 = <&bl_pwr_en>;
-               regulator-name = "backlight_regulator";
-               vin-supply = <&vcc33_sys>;
-               startup-delay-us = <15000>;
-       };
-
-       panel_regulator: panel-regulator {
-               compatible = "regulator-fixed";
-               enable-active-high;
-               gpio = <&gpio7 RK_PB6 GPIO_ACTIVE_HIGH>;
-               pinctrl-names = "default";
-               pinctrl-0 = <&lcd_enable_h>;
-               regulator-name = "panel_regulator";
-               startup-delay-us = <100000>;
-               vin-supply = <&vcc33_sys>;
-       };
-
-       vcc18_lcd: vcc18-lcd {
-               compatible = "regulator-fixed";
-               enable-active-high;
-               gpio = <&gpio2 RK_PB5 GPIO_ACTIVE_HIGH>;
-               pinctrl-names = "default";
-               pinctrl-0 = <&avdd_1v8_disp_en>;
-               regulator-name = "vcc18_lcd";
-               regulator-always-on;
-               regulator-boot-on;
-               vin-supply = <&vcc18_wl>;
-       };
-
        volume_buttons: volume-buttons {
                compatible = "gpio-keys";
                pinctrl-names = "default";

You forgot to remove the line:

power-supply = <&backlight_regulator>;

...from minnie.

quoted

diff --git a/arch/arm/boot/dts/rk3288-veyron-pinky.dts b/arch/arm/boot/dts/rk3288-veyron-pinky.dts
index 9b6f4d9b03b6..06af58e37a4b 100644
--- a/arch/arm/boot/dts/rk3288-veyron-pinky.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-pinky.dts

@@ -14,7 +14,14 @@
        compatible = "google,veyron-pinky-rev2", "google,veyron-pinky",
                     "google,veyron", "rockchip,rk3288";

+       /delete-node/backlight-regulator;
+       /delete-node/panel-regulator;
        /delete-node/emmc-pwrseq;
+       /delete-node/vcc18-lcd;
+};
+
+&backlight {
+       /delete-property/power-supply;
 };

 &emmc {

@@ -52,7 +59,17 @@
        i2c-scl-rising-time-ns = <300>;
 };

+&panel {
+       power-supply= <&vcc33_lcd>;

Might as well put a space before the "="?

quoted

 &pinctrl {
+       /delete-node/ lcd;
+
+       backlight {
+               /delete-node/ bl_pwr_en;
+       };

I general as the defender of "pinky", I'll let Heiko confirm he's OK
with the color of this bikeshed.  Sometimes a bit of repetition is
preferred over a bunch of confusing /delete-node/ statements since
those tend to make things harder to reason about in general.  In this
case I think things are cleaner after your patch but I won't say it's
100% clear cut.

going this way with the delete-nodes looks good to me :-) ... pinky is
the "odd" one in that, so I think it can carry the burden ... especially
as you said, this really only affects me and my boardfarm at all ;-) .


Heiko

Other than nits I have double-checked this patch, so feel free to add
my Reviewed-by after nits are fixed.

-Doug

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel