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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 27 Sep 2020 17:01:04 +0200
From:   Luka Kovacic <luka.kovacic@...tura.hr>
To:     Marek Behun <marek.behun@....cz>, Andrew Lunn <andrew@...n.ch>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-hwmon@...r.kernel.org,
        linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>,
        Linux LED Subsystem <linux-leds@...r.kernel.org>,
        Lee Jones <lee.jones@...aro.org>, Pavel Machek <pavel@....cz>,
        Dan Murphy <dmurphy@...com>, Rob Herring <robh+dt@...nel.org>,
        Jean Delvare <jdelvare@...e.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Jason Cooper <jason@...edaemon.net>,
        Gregory Clement <gregory.clement@...tlin.com>,
        Luka Perkov <luka.perkov@...tura.hr>,
        Robert Marko <robert.marko@...tura.hr>
Subject: 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 <marek.behun@....cz> wrote:
>
> On Sat, 26 Sep 2020 15:55:14 +0200
> Luka Kovacic <luka.kovacic@...tura.hr> 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