Re: [RFC 2/2] ARM: dts: imx6dl-yapp4: Fix lp5562 driver probe

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 23. 06. 21 22:13, Pavel Machek wrote:
On Fri 2021-05-14 16:44:37, Michal Vokáč wrote:
Since the LED multicolor framework support was added in commit
92a81562e695 ("leds: lp55xx: Add multicolor framework support to lp55xx")
LEDs on this platform stopped working.

Author of the framework attempted to accommodate this DT to the
framework in commit b86d3d21cd4c ("ARM: dts: imx6dl-yapp4: Add reg property
to the lp5562 channel node") but that is not sufficient. A color property
is now required even if the multicolor framework is not used, otherwise
the driver probe fails:

   lp5562: probe of 1-0030 failed with error -22

Add the color property to fix this and remove the actually unused white
channel.

Fixes: b86d3d21cd4c ("ARM: dts: imx6dl-yapp4: Add reg property to the lp5562 channel node")

I believe this is for arm maintainers to take...

Hi Pavel,

Thank you for your reply.
As described in the cover letter, my primary intention was to bring
attention to the problem. Addition of the multicolor framework broke
devicetree forward compatibility. The old devicetrees does not work
with newer kernels. Addition of the multicolor framework caused the
color property to become a required one even if the framework is
not enabled in kernel config nor used in the dts. So the reality and
the dt-bindings documentation do not match.

IMO this could be fixed in two ways. First is adapt the dt-binding
documentation to match the reality. State that the color property is
always required. With this we need to fix all the examples and dts
files by adding the color property. This is quite tricky because we
do not always know the color and it also becomes required for the
led-controller node. See the error reported by Rob's bot for patch 1/2:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/leds/leds-lp55xx.example.dt.yaml: led-controller@32: 'color' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml

Second option is to fix this in the LED driver. The driver should not
require the color property if the multicolor framework is not enabled.

I would really like to know Rob's opinion here.
diff --git a/arch/arm/boot/dts/imx6dl-yapp4-common.dtsi b/arch/arm/boot/dts/imx6dl-yapp4-common.dtsi
index 7d2c72562c73..3107bf7fbce5 100644
--- a/arch/arm/boot/dts/imx6dl-yapp4-common.dtsi
+++ b/arch/arm/boot/dts/imx6dl-yapp4-common.dtsi
@@ -5,6 +5,7 @@
  #include <dt-bindings/gpio/gpio.h>
  #include <dt-bindings/interrupt-controller/irq.h>
  #include <dt-bindings/input/input.h>
+#include <dt-bindings/leds/common.h>
  #include <dt-bindings/pwm/pwm.h>
/ {
@@ -271,6 +272,7 @@
  			led-cur = /bits/ 8 <0x20>;
  			max-cur = /bits/ 8 <0x60>;
  			reg = <0>;
+			color = <LED_COLOR_ID_RED>;
  		};
chan@1 {
@@ -278,6 +280,7 @@
  			led-cur = /bits/ 8 <0x20>;
  			max-cur = /bits/ 8 <0x60>;
  			reg = <1>;
+			color = <LED_COLOR_ID_GREEN>;
  		};
chan@2 {
@@ -285,13 +288,7 @@
  			led-cur = /bits/ 8 <0x20>;
  			max-cur = /bits/ 8 <0x60>;
  			reg = <2>;
-		};
-
-		chan@3 {
-			chan-name = "W";
-			led-cur = /bits/ 8 <0x0>;
-			max-cur = /bits/ 8 <0x0>;
-			reg = <3>;
+			color = <LED_COLOR_ID_BLUE>;
  		};
  	};

What is going on here? "White" channel seems to have disappeared?

Yes, it is described in the commit message. I know this is not optimal.
The white channel is actually not used on this platform. So the right
approach would be to add the white color property in this fix commit
and remove the whole chan@3 node in next commit. I can do it that way.

Michal



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux