[PATCH net-next + leds v2 1/7] dt-bindings: leds: document binding for HW controlled LEDs

[PATCH net-next + leds v2 1/7] dt-bindings: leds: document binding for HW controlled LEDs

[Marek Behún]

Document binding for LEDs connected to and controlled by various chips
(such as ethernet PHY chips).

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
---
 .../leds/linux,hw-controlled-leds.yaml        | 99 +++++++++++++++++++
 1 file changed, 99 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml

diff --git a/Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml b/Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
new file mode 100644
index 0000000000000..eaf6e5d80c5f5
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
@@ -0,0 +1,99 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/linux,hw-controlled-leds.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LEDs that can be controlled by hardware (eg. by an ethernet PHY chip)
+
+maintainers:
+  - Marek Behún <marek.behun@nic.cz>
+
+description:
+  Many an ethernet PHY (and other chips) supports various HW control modes
+  for LEDs connected directly to them. With this binding such LEDs can be
+  described.
+
+properties:
+  compatible:
+    const: linux,hw-controlled-leds
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+patternProperties:
+  "^led@[0-9a-f]+$":
+    type: object
+    allOf:
+      - $ref: common.yaml#
+    description:
+      This node represents a LED device connected to a chip that can control
+      the LED in various HW controlled modes.
+
+    properties:
+      reg:
+        maxItems: 1
+        description:
+          This property identifies the LED to the chip the LED is connected to
+          (eg. an ethernet PHY chip can have multiple LEDs connected to it).
+
+      enable-active-high:
+        description:
+          Polarity of LED is active high. If missing, assumed default is active
+          low.
+        type: boolean
+
+      led-tristate:
+        description:
+          LED pin is tristate type. If missing, assumed false.
+        type: boolean
+
+      linux,default-hw-mode:
+        description:
+          This parameter, if present, specifies the default HW triggering mode
+          of the LED when LED trigger is set to `dev-hw-mode`.
+          Available values are specific per device the LED is connected to and
+          per LED itself.
+        $ref: /schemas/types.yaml#definitions/string
+
+    required:
+      - reg
+
+additionalProperties: false
+
+examples:
+  - |
+
+    #include <dt-bindings/leds/common.h>
+
+    ethernet-phy@0 {
+        compatible = "ethernet-phy-ieee802.3-c45";
+        reg = <0>;
+
+        leds {
+            compatible = "linux,hw-controlled-leds";
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            led@0 {
+                reg = <0>;
+                color = <LED_COLOR_ID_GREEN>;
+                function = <LED_FUNCTION_STATUS>;
+                linux,default-trigger = "dev-hw-mode";
+                linux,default-hw-mode = "1Gbps";
+            };
+
+            led@1 {
+                reg = <1>;
+                color = <LED_COLOR_ID_YELLOW>;
+                function = <LED_FUNCTION_ACTIVITY>;
+                linux,default-trigger = "dev-hw-mode";
+                linux,default-hw-mode = "activity";
+            };
+        };
+    };
+
+...
-- 
2.26.2

[PATCH net-next + leds v2 4/7] dt-bindings: net: ethernet-phy: add description for PHY LEDs

[Marek Behún]

Document binding for LEDs connected to an ethernet PHY chip.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
---
 Documentation/devicetree/bindings/net/ethernet-phy.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index a9e547ac79051..f593e8709dd0d 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -174,6 +174,14 @@ properties:
       PHY's that have configurable TX internal delays. If this property is
       present then the PHY applies the TX delay.
 
+  leds:
+    type: object
+    description: |
+      This is used to described LEDs that are connected to the PHY chip and
+      their blinking can be controlled by the PHY.
+    allOf:
+      - $ref: /schemas/leds/linux,hw-controlled-leds.yaml#
+
 required:
   - reg
 
-- 
2.26.2

Re: [PATCH net-next + leds v2 1/7] dt-bindings: leds: document binding for HW controlled LEDs

[Andrew Lunn]

On Wed, Sep 09, 2020 at 06:25:46PM +0200, Marek Behún wrote:
> Document binding for LEDs connected to and controlled by various chips
> (such as ethernet PHY chips).
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> ---
>  .../leds/linux,hw-controlled-leds.yaml        | 99 +++++++++++++++++++
>  1 file changed, 99 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
> 
> diff --git a/Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml b/Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
> new file mode 100644
> index 0000000000000..eaf6e5d80c5f5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
> @@ -0,0 +1,99 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/linux,hw-controlled-leds.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LEDs that can be controlled by hardware (eg. by an ethernet PHY chip)
> +
> +maintainers:
> +  - Marek Behún <marek.behun@nic.cz>
> +
> +description:
> +  Many an ethernet PHY (and other chips) supports various HW control modes
> +  for LEDs connected directly to them. With this binding such LEDs can be
> +  described.
> +
> +properties:
> +  compatible:
> +    const: linux,hw-controlled-leds
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +patternProperties:
> +  "^led@[0-9a-f]+$":
> +    type: object
> +    allOf:
> +      - $ref: common.yaml#
> +    description:
> +      This node represents a LED device connected to a chip that can control
> +      the LED in various HW controlled modes.
> +
> +    properties:
> +      reg:
> +        maxItems: 1
> +        description:
> +          This property identifies the LED to the chip the LED is connected to
> +          (eg. an ethernet PHY chip can have multiple LEDs connected to it).
> +
> +      enable-active-high:
> +        description:
> +          Polarity of LED is active high. If missing, assumed default is active
> +          low.
> +        type: boolean
> +
> +      led-tristate:
> +        description:
> +          LED pin is tristate type. If missing, assumed false.
> +        type: boolean
> +
> +      linux,default-hw-mode:
> +        description:
> +          This parameter, if present, specifies the default HW triggering mode
> +          of the LED when LED trigger is set to `dev-hw-mode`.
> +          Available values are specific per device the LED is connected to and
> +          per LED itself.
> +        $ref: /schemas/types.yaml#definitions/string
> +
> +    required:
> +      - reg

My Yaml foo is not very good. Do you need to list colour, function and
linux,default-trigger, or do they automagically get included from the
generic LED binding?

	Andrew

Re: [PATCH net-next + leds v2 1/7] dt-bindings: leds: document binding for HW controlled LEDs

[Marek Behún]

On Wed, 9 Sep 2020 20:27:30 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> On Wed, Sep 09, 2020 at 06:25:46PM +0200, Marek Behún wrote:
> > Document binding for LEDs connected to and controlled by various
> > chips (such as ethernet PHY chips).
> > 
> > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: devicetree@vger.kernel.org
> > ---
> >  .../leds/linux,hw-controlled-leds.yaml        | 99
> > +++++++++++++++++++ 1 file changed, 99 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
> > b/Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
> > new file mode 100644 index 0000000000000..eaf6e5d80c5f5 ---
> > /dev/null +++
> > b/Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
> > @@ -0,0 +1,99 @@ +# SPDX-License-Identifier: GPL-2.0-only OR
> > BSD-2-Clause +%YAML 1.2
> > +---
> > +$id:
> > http://devicetree.org/schemas/leds/linux,hw-controlled-leds.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml# +
> > +title: LEDs that can be controlled by hardware (eg. by an ethernet
> > PHY chip) +
> > +maintainers:
> > +  - Marek Behún <marek.behun@nic.cz>
> > +
> > +description:
> > +  Many an ethernet PHY (and other chips) supports various HW
> > control modes
> > +  for LEDs connected directly to them. With this binding such LEDs
> > can be
> > +  described.
> > +
> > +properties:
> > +  compatible:
> > +    const: linux,hw-controlled-leds
> > +
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 0
> > +
> > +patternProperties:
> > +  "^led@[0-9a-f]+$":
> > +    type: object
> > +    allOf:
> > +      - $ref: common.yaml#
> > +    description:
> > +      This node represents a LED device connected to a chip that
> > can control
> > +      the LED in various HW controlled modes.
> > +
> > +    properties:
> > +      reg:
> > +        maxItems: 1
> > +        description:
> > +          This property identifies the LED to the chip the LED is
> > connected to
> > +          (eg. an ethernet PHY chip can have multiple LEDs
> > connected to it). +
> > +      enable-active-high:
> > +        description:
> > +          Polarity of LED is active high. If missing, assumed
> > default is active
> > +          low.
> > +        type: boolean
> > +
> > +      led-tristate:
> > +        description:
> > +          LED pin is tristate type. If missing, assumed false.
> > +        type: boolean
> > +
> > +      linux,default-hw-mode:
> > +        description:
> > +          This parameter, if present, specifies the default HW
> > triggering mode
> > +          of the LED when LED trigger is set to `dev-hw-mode`.
> > +          Available values are specific per device the LED is
> > connected to and
> > +          per LED itself.
> > +        $ref: /schemas/types.yaml#definitions/string
> > +
> > +    required:
> > +      - reg  
> 
> My Yaml foo is not very good. Do you need to list colour, function and
> linux,default-trigger, or do they automagically get included from the
> generic LED binding?
> 
> 	Andrew

I don't know :) I copied this from other drivers, I once tried setting
up environment for doing checking of device trees with YAML schemas,
and it was a little painful :)

Re: [PATCH net-next + leds v2 1/7] dt-bindings: leds: document binding for HW controlled LEDs

[Rob Herring]

On Wed, 09 Sep 2020 18:25:46 +0200, Marek Behún wrote:
> Document binding for LEDs connected to and controlled by various chips
> (such as ethernet PHY chips).
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> ---
>  .../leds/linux,hw-controlled-leds.yaml        | 99 +++++++++++++++++++
>  1 file changed, 99 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
> 


My bot found errors running 'make dt_binding_check' on your patch:

Error: Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.example.dts:34.33-41 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:342: Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1366: dt_binding_check] Error 2


See https://patchwork.ozlabs.org/patch/1360778

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.

Re: [PATCH net-next + leds v2 1/7] dt-bindings: leds: document binding for HW controlled LEDs

[Rob Herring]

On Wed, Sep 09, 2020 at 08:33:10PM +0200, Marek Behún wrote:
> On Wed, 9 Sep 2020 20:27:30 +0200
> Andrew Lunn <andrew@lunn.ch> wrote:
> 
> > On Wed, Sep 09, 2020 at 06:25:46PM +0200, Marek Behún wrote:
> > > Document binding for LEDs connected to and controlled by various
> > > chips (such as ethernet PHY chips).
> > > 
> > > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: devicetree@vger.kernel.org
> > > ---
> > >  .../leds/linux,hw-controlled-leds.yaml        | 99
> > > +++++++++++++++++++ 1 file changed, 99 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
> > > b/Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
> > > new file mode 100644 index 0000000000000..eaf6e5d80c5f5 ---
> > > /dev/null +++
> > > b/Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
> > > @@ -0,0 +1,99 @@ +# SPDX-License-Identifier: GPL-2.0-only OR
> > > BSD-2-Clause +%YAML 1.2
> > > +---
> > > +$id:
> > > http://devicetree.org/schemas/leds/linux,hw-controlled-leds.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml# +
> > > +title: LEDs that can be controlled by hardware (eg. by an ethernet
> > > PHY chip) +
> > > +maintainers:
> > > +  - Marek Behún <marek.behun@nic.cz>
> > > +
> > > +description:
> > > +  Many an ethernet PHY (and other chips) supports various HW
> > > control modes
> > > +  for LEDs connected directly to them. With this binding such LEDs
> > > can be
> > > +  described.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: linux,hw-controlled-leds
> > > +
> > > +  "#address-cells":
> > > +    const: 1
> > > +
> > > +  "#size-cells":
> > > +    const: 0
> > > +
> > > +patternProperties:
> > > +  "^led@[0-9a-f]+$":
> > > +    type: object
> > > +    allOf:
> > > +      - $ref: common.yaml#
> > > +    description:
> > > +      This node represents a LED device connected to a chip that
> > > can control
> > > +      the LED in various HW controlled modes.
> > > +
> > > +    properties:
> > > +      reg:
> > > +        maxItems: 1
> > > +        description:
> > > +          This property identifies the LED to the chip the LED is
> > > connected to
> > > +          (eg. an ethernet PHY chip can have multiple LEDs
> > > connected to it). +
> > > +      enable-active-high:
> > > +        description:
> > > +          Polarity of LED is active high. If missing, assumed
> > > default is active
> > > +          low.
> > > +        type: boolean
> > > +
> > > +      led-tristate:
> > > +        description:
> > > +          LED pin is tristate type. If missing, assumed false.
> > > +        type: boolean
> > > +
> > > +      linux,default-hw-mode:
> > > +        description:
> > > +          This parameter, if present, specifies the default HW
> > > triggering mode
> > > +          of the LED when LED trigger is set to `dev-hw-mode`.
> > > +          Available values are specific per device the LED is
> > > connected to and
> > > +          per LED itself.
> > > +        $ref: /schemas/types.yaml#definitions/string
> > > +
> > > +    required:
> > > +      - reg  
> > 
> > My Yaml foo is not very good. Do you need to list colour, function and
> > linux,default-trigger, or do they automagically get included from the
> > generic LED binding?

They do with the '$ref: common.yaml#'. You need to list them if you want 
to say which properties of a common schema you are using (IMO, you 
should, but that's a secondary concern) and/or you have additional 
constraints on them.

> > 
> > 	Andrew
> 
> I don't know :) I copied this from other drivers, I once tried setting
> up environment for doing checking of device trees with YAML schemas,
> and it was a little painful :)

pip3 install dtschema ?

Can you elaborate on the issue.

Rob

Re: [PATCH net-next + leds v2 1/7] dt-bindings: leds: document binding for HW controlled LEDs

[Marek Behun]

On Wed, 9 Sep 2020 14:59:23 -0600
Rob Herring <robh@kernel.org> wrote:

> > 
> > I don't know :) I copied this from other drivers, I once tried setting
> > up environment for doing checking of device trees with YAML schemas,
> > and it was a little painful :)  
> 
> pip3 install dtschema ?
> 
> Can you elaborate on the issue.
> 
> Rob
> 

I am using Gentoo and didn't want to bloat system with non-portage
packages, nor try to start a virtual environment. In the end I did it
in a chroot Ubuntu :)

The other thing is that the make dt_binding_check executed for
quite a long time, and I didn't find a way to just do the binding check
some of the schemas.

But I am not criticizing anything, I think that it is a good thing to
have this system.

Marek

Re: [PATCH net-next + leds v2 1/7] dt-bindings: leds: document binding for HW controlled LEDs

[Rob Herring]

On Wed, Sep 09, 2020 at 06:25:46PM +0200, Marek Behún wrote:
> Document binding for LEDs connected to and controlled by various chips
> (such as ethernet PHY chips).

If they are h/w controlled, then why are they in DT?

> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> ---
>  .../leds/linux,hw-controlled-leds.yaml        | 99 +++++++++++++++++++
>  1 file changed, 99 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
> 
> diff --git a/Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml b/Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
> new file mode 100644
> index 0000000000000..eaf6e5d80c5f5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
> @@ -0,0 +1,99 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/linux,hw-controlled-leds.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LEDs that can be controlled by hardware (eg. by an ethernet PHY chip)
> +
> +maintainers:
> +  - Marek Behún <marek.behun@nic.cz>
> +
> +description:
> +  Many an ethernet PHY (and other chips) supports various HW control modes
> +  for LEDs connected directly to them. With this binding such LEDs can be
> +  described.
> +
> +properties:
> +  compatible:
> +    const: linux,hw-controlled-leds

What makes this linux specific?

Unless you're going to make this h/w specific, then it probably should 
just be dropped. 

The phy schema will need:

leds:
  type: object
  $ref: /schemas/leds/hw-controlled-leds.yaml#

> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +patternProperties:
> +  "^led@[0-9a-f]+$":
> +    type: object
> +    allOf:
> +      - $ref: common.yaml#
> +    description:
> +      This node represents a LED device connected to a chip that can control
> +      the LED in various HW controlled modes.
> +
> +    properties:
> +      reg:
> +        maxItems: 1
> +        description:
> +          This property identifies the LED to the chip the LED is connected to
> +          (eg. an ethernet PHY chip can have multiple LEDs connected to it).
> +
> +      enable-active-high:
> +        description:
> +          Polarity of LED is active high. If missing, assumed default is active
> +          low.
> +        type: boolean
> +
> +      led-tristate:
> +        description:
> +          LED pin is tristate type. If missing, assumed false.
> +        type: boolean
> +
> +      linux,default-hw-mode:

How is this linux specific? It sounds device specific. Your choice is 
make this a device specific property with device specific values or come 
up with generic modes.

Perhaps 'function' should be expanded.

> +        description:
> +          This parameter, if present, specifies the default HW triggering mode
> +          of the LED when LED trigger is set to `dev-hw-mode`.
> +          Available values are specific per device the LED is connected to and
> +          per LED itself.
> +        $ref: /schemas/types.yaml#definitions/string
> +
> +    required:
> +      - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +
> +    #include <dt-bindings/leds/common.h>
> +
> +    ethernet-phy@0 {
> +        compatible = "ethernet-phy-ieee802.3-c45";
> +        reg = <0>;
> +
> +        leds {
> +            compatible = "linux,hw-controlled-leds";
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            led@0 {
> +                reg = <0>;
> +                color = <LED_COLOR_ID_GREEN>;
> +                function = <LED_FUNCTION_STATUS>;

Reading the description of LED_FUNCTION_STATUS doesn't align with how 
you are using it. Think of it as user alert/notification.

> +                linux,default-trigger = "dev-hw-mode";

This is deprecated in favor of 'function'.

> +                linux,default-hw-mode = "1Gbps";
> +            };
> +
> +            led@1 {
> +                reg = <1>;
> +                color = <LED_COLOR_ID_YELLOW>;
> +                function = <LED_FUNCTION_ACTIVITY>;
> +                linux,default-trigger = "dev-hw-mode";
> +                linux,default-hw-mode = "activity";
> +            };
> +        };
> +    };
> +
> +...
> -- 
> 2.26.2
> 

Re: [PATCH net-next + leds v2 1/7] dt-bindings: leds: document binding for HW controlled LEDs

[Rob Herring]

On Wed, Sep 09, 2020 at 11:07:26PM +0200, Marek Behun wrote:
> On Wed, 9 Sep 2020 14:59:23 -0600
> Rob Herring <robh@kernel.org> wrote:
> 
> > > 
> > > I don't know :) I copied this from other drivers, I once tried setting
> > > up environment for doing checking of device trees with YAML schemas,
> > > and it was a little painful :)  
> > 
> > pip3 install dtschema ?
> > 
> > Can you elaborate on the issue.
> > 
> > Rob
> > 
> 
> I am using Gentoo and didn't want to bloat system with non-portage
> packages, nor try to start a virtual environment. In the end I did it
> in a chroot Ubuntu :)

A user install doesn't work?

I don't really care for virtual env either.

> The other thing is that the make dt_binding_check executed for
> quite a long time, and I didn't find a way to just do the binding check
> some of the schemas.

It's a bit faster now with what's queued for 5.10. The schema 
validation is under 10sec now on my laptop. For the examples, any 
new schema could apply to any example, so we have to check them all. 
It's faster too, but still minutes to run.

> But I am not criticizing anything, I think that it is a good thing to
> have this system.

Good to hear. Just want to improve any pain points if possible.

Rob

Re: [PATCH net-next + leds v2 1/7] dt-bindings: leds: document binding for HW controlled LEDs

[Marek Behun]

On Wed, 9 Sep 2020 15:31:22 -0600
Rob Herring <robh@kernel.org> wrote:

> On Wed, Sep 09, 2020 at 11:07:26PM +0200, Marek Behun wrote:
> > On Wed, 9 Sep 2020 14:59:23 -0600
> > Rob Herring <robh@kernel.org> wrote:
> >   
> > > > 
> > > > I don't know :) I copied this from other drivers, I once tried setting
> > > > up environment for doing checking of device trees with YAML schemas,
> > > > and it was a little painful :)    
> > > 
> > > pip3 install dtschema ?
> > > 
> > > Can you elaborate on the issue.
> > > 
> > > Rob
> > >   
> > 
> > I am using Gentoo and didn't want to bloat system with non-portage
> > packages, nor try to start a virtual environment. In the end I did it
> > in a chroot Ubuntu :)  
> 
> A user install doesn't work?
> 
> I don't really care for virtual env either.
> 
> > The other thing is that the make dt_binding_check executed for
> > quite a long time, and I didn't find a way to just do the binding check
> > some of the schemas.  
> 
> It's a bit faster now with what's queued for 5.10. The schema 
> validation is under 10sec now on my laptop. For the examples, any 
> new schema could apply to any example, so we have to check them all. 
> It's faster too, but still minutes to run.
> 
> > But I am not criticizing anything, I think that it is a good thing to
> > have this system.  
> 
> Good to hear. Just want to improve any pain points if possible.
> 
> Rob
> 

OK I am running it now in my chroot environment :)

Re: [PATCH net-next + leds v2 1/7] dt-bindings: leds: document binding for HW controlled LEDs

[Marek Behun]

On Wed, 9 Sep 2020 15:15:52 -0600
Rob Herring <robh@kernel.org> wrote:

> On Wed, Sep 09, 2020 at 06:25:46PM +0200, Marek Behún wrote:
> > Document binding for LEDs connected to and controlled by various chips
> > (such as ethernet PHY chips).  
> 
> If they are h/w controlled, then why are they in DT?

The idea is that by default these LEDs are in some specific HW control
mode (the chip default), but the chip supports various HW control
modes, and also supports SW control.
For example on Marvell PHYs there is a 4-bit register for each LED, so
16 values, and some of them are:
  0000: On - Receive
        Off - No receive
  0001: On - Link
        Blink - Activity
        Off - No Link
  ...
  0101: On - 100Mbps or Fiber Link
        Off - Else
  ...
  1000: Force Off
  1001: Force On
  ...
  1011: Force Blink

So writing 0x8 disables the LED, 0x9 enabled it (SW control via
/sys/class/leds/<LED>/brightness), other values change the HW control
mode (in this proposal /sys/class/leds/<LED>/hw_mode when trigger is
set to dev-hw-mode).

> > 
> > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: devicetree@vger.kernel.org
> > ---
> >  .../leds/linux,hw-controlled-leds.yaml        | 99 +++++++++++++++++++
> >  1 file changed, 99 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml b/Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
> > new file mode 100644
> > index 0000000000000..eaf6e5d80c5f5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/linux,hw-controlled-leds.yaml
> > @@ -0,0 +1,99 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/leds/linux,hw-controlled-leds.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: LEDs that can be controlled by hardware (eg. by an ethernet PHY chip)
> > +
> > +maintainers:
> > +  - Marek Behún <marek.behun@nic.cz>
> > +
> > +description:
> > +  Many an ethernet PHY (and other chips) supports various HW control modes
> > +  for LEDs connected directly to them. With this binding such LEDs can be
> > +  described.
> > +
> > +properties:
> > +  compatible:
> > +    const: linux,hw-controlled-leds  
> 
> What makes this linux specific?
> 
> Unless you're going to make this h/w specific, then it probably should 
> just be dropped. 
> 

Will do, thanks.

> The phy schema will need:
> 
> leds:
>   type: object
>   $ref: /schemas/leds/hw-controlled-leds.yaml#
> 
> > +
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 0
> > +
> > +patternProperties:
> > +  "^led@[0-9a-f]+$":
> > +    type: object
> > +    allOf:
> > +      - $ref: common.yaml#
> > +    description:
> > +      This node represents a LED device connected to a chip that can control
> > +      the LED in various HW controlled modes.
> > +
> > +    properties:
> > +      reg:
> > +        maxItems: 1
> > +        description:
> > +          This property identifies the LED to the chip the LED is connected to
> > +          (eg. an ethernet PHY chip can have multiple LEDs connected to it).
> > +
> > +      enable-active-high:
> > +        description:
> > +          Polarity of LED is active high. If missing, assumed default is active
> > +          low.
> > +        type: boolean
> > +
> > +      led-tristate:
> > +        description:
> > +          LED pin is tristate type. If missing, assumed false.
> > +        type: boolean
> > +
> > +      linux,default-hw-mode:  
> 
> How is this linux specific? It sounds device specific. Your choice is 
> make this a device specific property with device specific values or come 
> up with generic modes.

I was inspired by `linux,default-trigger` and `linux,keycode`
properties...
 
> Perhaps 'function' should be expanded.

The thing is that `function` is now used for creating LED device name.
I was not aware that `linux,default-trigger` is deprecated
Perhaps this should be discussed with Pavel as well.
But of course from the perspective that DT should be independent from
Linux, you are right.
I fear this will be quite a pain to resolve...

> > +        description:
> > +          This parameter, if present, specifies the default HW triggering mode
> > +          of the LED when LED trigger is set to `dev-hw-mode`.
> > +          Available values are specific per device the LED is connected to and
> > +          per LED itself.
> > +        $ref: /schemas/types.yaml#definitions/string
> > +
> > +    required:
> > +      - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +
> > +    #include <dt-bindings/leds/common.h>
> > +
> > +    ethernet-phy@0 {
> > +        compatible = "ethernet-phy-ieee802.3-c45";
> > +        reg = <0>;
> > +
> > +        leds {
> > +            compatible = "linux,hw-controlled-leds";
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            led@0 {
> > +                reg = <0>;
> > +                color = <LED_COLOR_ID_GREEN>;
> > +                function = <LED_FUNCTION_STATUS>;  
> 
> Reading the description of LED_FUNCTION_STATUS doesn't align with how 
> you are using it. Think of it as user alert/notification.
> 
> > +                linux,default-trigger = "dev-hw-mode";  
> 
> This is deprecated in favor of 'function'.

As written above, this is not how the LED subsystem currently works.
Nor is the deprecation documented.

Currently `function` is used to create LED device name in the form
`device:color:function` or `device:color:function-N` if
`function-enumerator` is also set.

> 
> > +                linux,default-hw-mode = "1Gbps";
> > +            };
> > +
> > +            led@1 {
> > +                reg = <1>;
> > +                color = <LED_COLOR_ID_YELLOW>;
> > +                function = <LED_FUNCTION_ACTIVITY>;
> > +                linux,default-trigger = "dev-hw-mode";
> > +                linux,default-hw-mode = "activity";
> > +            };
> > +        };
> > +    };
> > +
> > +...
> > -- 
> > 2.26.2
> >