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

[Luka Kovacic]

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 <marek.behun@xxxxxx> wrote:
>
> On Sat, 26 Sep 2020 15:55:14 +0200
> Luka Kovacic <luka.kovacic@xxxxxxxxxx> wrote:
>
> > +     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?
>
>
>
> > +             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.
>
> > +             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