Re: [PATCH v2 7/7] arm64: dts: marvell: Add a device tree for the iEi Puzzle-M801 board

Hello Andrew and Marek,

I will break the new patchset up and also add Gregory to the DT conversation.
Should I exclude this patch from this patchset or can I just add him to Cc?

First six LEDs are used to indicate port status and activity on the SFP+ ports.
Certainly, I will change this once the API is solved. There are currently many
similar boards with no real solution for the network LED triggers.

I'll add the color and correct the function properties for the LEDs.

Kind regards,
Luka

On Sat, Sep 26, 2020 at 7:50 PM Marek Behun [off-list ref] wrote:

On Sat, 26 Sep 2020 15:55:14 +0200
Luka Kovacic [off-list ref] wrote:

quoted

+     leds {
+             compatible = "gpio-leds";
+             status = "okay";
+             pinctrl-0 = <&cp0_sfpplus_led_pins &cp1_sfpplus_led_pins>;
+             pinctrl-names = "default";
+
+             led0 {
+                     function = LED_FUNCTION_STATUS;
+                     label = "p2_act";
+                     gpios = <&cp1_gpio1 6 GPIO_ACTIVE_LOW>;
+             };

There should be a dash in LED node name, please pass this dts via
dt_binding_check
  led-0 {
    ...
  };

Also why not add the `color` property to the LED? This is DTS for a
specific device, right?
`label` is obsolete. The LED subsystem creates a name in form
  [device:]color:function
If this LED should blink for activity on port 2 (is this an ethernet
port?), the function should be LED_FUNCTION_LAN and function-enumerator
should be <2> (or function should be LED_FUNCTION_ACTIVITY, depending
on how the LED subsystem goes forward with this, but certainly not
LED_FUNCTION_STATUS), and trigger-sources should be set to point to the
ethernet port.

Luka, are you willing to change this once we solve this API properly
in LED subsystem?

quoted

+             led6 {
+                     function = LED_FUNCTION_STATUS;
+                     linux,default-trigger = "disk-activity";
+                     label = "front-hdd-led";
+                     gpios = <&cp0_gpio2 22 GPIO_ACTIVE_HIGH>;
+             };

led-6. LED_FUNCTION_DISK. `label` deprecated.

quoted

+             leds {
+                     compatible = "iei,wt61p803-puzzle-leds";
+                     #address-cells = <1>;
+                     #size-cells = <0>;
+
+                     led@0 {
+                             reg = <0>;
+                             color = <LED_COLOR_ID_BLUE>;
+                             label = "front-power-led";
+                     };

Again, `label` is deprecated. Rather use function =
<LED_FUNCTION_POWER>;

Marek

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