[PATCH RFC 0/3] dt-bindings: net: Add network-class.yaml schema

[PATCH RFC 0/3] dt-bindings: net: Add network-class.yaml schema

[Janne Grunau]

The Devicetree Specification, Release v0.3 specifies in section 4.3.1
a "Network Class Binding". This covers MAC address and maximal frame
size properties. "local-mac-address" and "mac-address" with a fixed
address-size of 48 bits is already in the ethernet-controller.yaml
schema so move those over.
I think the only commonly used values for address-size are 48 and 64
bits (EUI-48 and EUI-64). Unfortunately I was not able to restrict the
mac-address size based on the address-size. This seems to be an side
effect of the array definition and I was not able to restrict "minItems"
or "maxItems" based on the address-size value in an "if"-"then"-"else"
block.
An easy way out would be to restrict address-size to 48-bits for now.

I've ignored "max-frame-size" since the description in
ethernet-controller.yaml claims there is a contradiction in the
Devicetree specification. I suppose it is describing the property
"max-frame-size" with "Specifies maximum packet length ...".

My understanding from the dt-schema README is that network-class.yaml
should live in the dt-schema repository since it describes properties
from the Devicetree specification. How is the synchronization handled in
this case? The motivation for this series is to fix dtbs_check failures
for Apple silicon devices both in the tree and upcoming ones.

Signed-off-by: Janne Grunau <j@jannau.net>
---
Janne Grunau (3):
      dt-bindings: net: Add network-class schema for mac-address properties
      dt-bindings: wireless: bcm4329-fmac: Use network-class.yaml schema
      dt-bindings: wireless: silabs,wfx: Use network-class.yaml

 .../bindings/net/ethernet-controller.yaml          | 18 +---------
 .../devicetree/bindings/net/network-class.yaml     | 40 ++++++++++++++++++++++
 .../bindings/net/wireless/brcm,bcm4329-fmac.yaml   |  5 ++-
 .../bindings/net/wireless/silabs,wfx.yaml          |  5 +--
 4 files changed, 46 insertions(+), 22 deletions(-)
---
base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2
change-id: 20230203-dt-bindings-network-class-8367edd679d2

Best regards,
-- 
Janne Grunau <j@jannau.net>

[PATCH RFC 1/3] dt-bindings: net: Add network-class schema for mac-address properties

[Janne Grunau]

The ethernet-controller schema specifies "mac-address" and
"local-mac-address" but other network devices such as wireless network
adapters use mac addresses as well.
The Devicetree Specification, Release v0.3 specifies in section 4.3.1
a generic "Network Class Binding" with "address-bits", "mac-address",
"local-mac-address" and "max-frame-size". This schema specifies the
"address-bits" property and moves "local-mac-address" and "mac-address"
over from ethernet-controller.yaml.
The schema currently does not restrict MAC address size based on
address-bits.

Signed-off-by: Janne Grunau <j@jannau.net>
---
 .../bindings/net/ethernet-controller.yaml          | 18 +---------
 .../devicetree/bindings/net/network-class.yaml     | 40 ++++++++++++++++++++++
 2 files changed, 41 insertions(+), 17 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
index 00be387984ac..a5f6a09dfdea 100644
--- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
@@ -17,23 +17,6 @@ properties:
     $ref: /schemas/types.yaml#/definitions/string
     description: Human readable label on a port of a box.
 
-  local-mac-address:
-    description:
-      Specifies the MAC address that was assigned to the network device.
-    $ref: /schemas/types.yaml#/definitions/uint8-array
-    minItems: 6
-    maxItems: 6
-
-  mac-address:
-    description:
-      Specifies the MAC address that was last used by the boot
-      program; should be used in cases where the MAC address assigned
-      to the device by the boot program is different from the
-      local-mac-address property.
-    $ref: /schemas/types.yaml#/definitions/uint8-array
-    minItems: 6
-    maxItems: 6
-
   max-frame-size:
     $ref: /schemas/types.yaml#/definitions/uint32
     description:
@@ -226,6 +209,7 @@ dependencies:
   pcs-handle-names: [pcs-handle]
 
 allOf:
+  - $ref: /schemas/net/network-class.yaml#
   - if:
       properties:
         phy-mode:
diff --git a/Documentation/devicetree/bindings/net/network-class.yaml b/Documentation/devicetree/bindings/net/network-class.yaml
new file mode 100644
index 000000000000..676aec1c458e
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/network-class.yaml
@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/network-class.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Network Class Bindings
+
+maintainers:
+  - Devicetree Specification Mailing List <devicetree-spec-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
+
+properties:
+  address-bits:
+    description:
+      Specifies number of address bits required to address the device described
+      by this node. This property specifies number of bits in MAC address.
+      If unspecified, the default value is 48.
+    default: 48
+    enum: [48, 64]
+
+  local-mac-address:
+    description:
+      Specifies MAC address that was assigned to the network device described by
+      the node containing this property.
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    minItems: 6
+    maxItems: 8
+
+  mac-address:
+    description:
+      Specifies the MAC address that was last used by the boot program. This
+      property should be used in cases where the MAC address assigned to the
+      device by the boot program is different from the
+      local-mac-address property. This property shall be used only if the value
+      differs from local-mac-address property value.
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    minItems: 6
+    maxItems: 8
+
+additionalProperties: true

-- 
2.39.1

Re: [PATCH RFC 1/3] dt-bindings: net: Add network-class schema for mac-address properties

[Rob Herring]

On Fri, Feb 3, 2023 at 7:56 AM Janne Grunau <j@jannau.net> wrote:
>
> The ethernet-controller schema specifies "mac-address" and
> "local-mac-address" but other network devices such as wireless network
> adapters use mac addresses as well.
> The Devicetree Specification, Release v0.3 specifies in section 4.3.1
> a generic "Network Class Binding" with "address-bits", "mac-address",
> "local-mac-address" and "max-frame-size". This schema specifies the
> "address-bits" property and moves "local-mac-address" and "mac-address"
> over from ethernet-controller.yaml.
> The schema currently does not restrict MAC address size based on
> address-bits.
>
> Signed-off-by: Janne Grunau <j@jannau.net>
> ---
>  .../bindings/net/ethernet-controller.yaml          | 18 +---------
>  .../devicetree/bindings/net/network-class.yaml     | 40 ++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> index 00be387984ac..a5f6a09dfdea 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> @@ -17,23 +17,6 @@ properties:
>      $ref: /schemas/types.yaml#/definitions/string
>      description: Human readable label on a port of a box.
>
> -  local-mac-address:
> -    description:
> -      Specifies the MAC address that was assigned to the network device.
> -    $ref: /schemas/types.yaml#/definitions/uint8-array
> -    minItems: 6
> -    maxItems: 6
> -
> -  mac-address:
> -    description:
> -      Specifies the MAC address that was last used by the boot
> -      program; should be used in cases where the MAC address assigned
> -      to the device by the boot program is different from the
> -      local-mac-address property.
> -    $ref: /schemas/types.yaml#/definitions/uint8-array
> -    minItems: 6
> -    maxItems: 6
> -
>    max-frame-size:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      description:
> @@ -226,6 +209,7 @@ dependencies:
>    pcs-handle-names: [pcs-handle]
>
>  allOf:
> +  - $ref: /schemas/net/network-class.yaml#
>    - if:
>        properties:
>          phy-mode:
> diff --git a/Documentation/devicetree/bindings/net/network-class.yaml b/Documentation/devicetree/bindings/net/network-class.yaml
> new file mode 100644
> index 000000000000..676aec1c458e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/network-class.yaml
> @@ -0,0 +1,40 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/network-class.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Network Class Bindings
> +
> +maintainers:
> +  - Devicetree Specification Mailing List <devicetree-spec-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> +
> +properties:
> +  address-bits:
> +    description:
> +      Specifies number of address bits required to address the device described
> +      by this node. This property specifies number of bits in MAC address.
> +      If unspecified, the default value is 48.

You can drop the last sentence.

> +    default: 48
> +    enum: [48, 64]

I wonder if we should just deprecate this property. I see 1 occurrence
and no consumer in the kernel tree at least. I guess one could set the
length, but not have mac addresses in the DT. Otherwise you could just
infer the length from the property length. Anyways, a conversation for
another time I guess.

Rob

Re: [PATCH RFC 1/3] dt-bindings: net: Add network-class schema for mac-address properties

[Krzysztof Kozlowski]

On 03/02/2023 14:56, Janne Grunau wrote:
> The ethernet-controller schema specifies "mac-address" and
> "local-mac-address" but other network devices such as wireless network
> adapters use mac addresses as well.
> The Devicetree Specification, Release v0.3 specifies in section 4.3.1
> a generic "Network Class Binding" with "address-bits", "mac-address",
> "local-mac-address" and "max-frame-size". This schema specifies the
> "address-bits" property and moves "local-mac-address" and "mac-address"
> over from ethernet-controller.yaml.
> The schema currently does not restrict MAC address size based on
> address-bits.

A nit below

>          phy-mode:
> diff --git a/Documentation/devicetree/bindings/net/network-class.yaml b/Documentation/devicetree/bindings/net/network-class.yaml
> new file mode 100644
> index 000000000000..676aec1c458e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/network-class.yaml
> @@ -0,0 +1,40 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/network-class.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Network Class Bindings

s/Bindings/Common Properties/
Best regards,
Krzysztof

Re: [PATCH RFC 0/3] dt-bindings: net: Add network-class.yaml schema

[Rob Herring]

On Fri, Feb 3, 2023 at 7:56 AM Janne Grunau <j@jannau.net> wrote:
>
> The Devicetree Specification, Release v0.3 specifies in section 4.3.1
> a "Network Class Binding". This covers MAC address and maximal frame
> size properties. "local-mac-address" and "mac-address" with a fixed
> address-size of 48 bits is already in the ethernet-controller.yaml
> schema so move those over.
> I think the only commonly used values for address-size are 48 and 64
> bits (EUI-48 and EUI-64). Unfortunately I was not able to restrict the
> mac-address size based on the address-size. This seems to be an side
> effect of the array definition and I was not able to restrict "minItems"
> or "maxItems" based on the address-size value in an "if"-"then"-"else"
> block.
> An easy way out would be to restrict address-size to 48-bits for now.

I've never seen 64-bits used...

> I've ignored "max-frame-size" since the description in
> ethernet-controller.yaml claims there is a contradiction in the
> Devicetree specification. I suppose it is describing the property
> "max-frame-size" with "Specifies maximum packet length ...".

Please include it and we'll fix the spec. It is clearly wrong. 2 nios
boards use 1518 and the consumer for them says it is MTU. Everything
else clearly uses mtu with 1500 or 9000.

> My understanding from the dt-schema README is that network-class.yaml
> should live in the dt-schema repository since it describes properties
> from the Devicetree specification. How is the synchronization handled in
> this case? The motivation for this series is to fix dtbs_check failures
> for Apple silicon devices both in the tree and upcoming ones.

Let's add it to the kernel, then later we can copy it to dtschema,
bump the minimum version the kernel requires, and delete the kernel
copy.

Rob

Re: [PATCH RFC 0/3] dt-bindings: net: Add network-class.yaml schema

[Janne Grunau]

On 2023-02-03 08:41:28 -0600, Rob Herring wrote:
> On Fri, Feb 3, 2023 at 7:56 AM Janne Grunau <j@jannau.net> wrote:
> >
> > The Devicetree Specification, Release v0.3 specifies in section 4.3.1
> > a "Network Class Binding". This covers MAC address and maximal frame
> > size properties. "local-mac-address" and "mac-address" with a fixed
> > address-size of 48 bits is already in the ethernet-controller.yaml
> > schema so move those over.
> > I think the only commonly used values for address-size are 48 and 64
> > bits (EUI-48 and EUI-64). Unfortunately I was not able to restrict the
> > mac-address size based on the address-size. This seems to be an side
> > effect of the array definition and I was not able to restrict "minItems"
> > or "maxItems" based on the address-size value in an "if"-"then"-"else"
> > block.
> > An easy way out would be to restrict address-size to 48-bits for now.
> 
> I've never seen 64-bits used...

ZigBee and 6LoWPAN use 64-bits for example. Let's hardcode 48 bits for 
now as that's what all in-tree devicetrees implicitly use. If needed it 
can be changed later.

> > I've ignored "max-frame-size" since the description in
> > ethernet-controller.yaml claims there is a contradiction in the
> > Devicetree specification. I suppose it is describing the property
> > "max-frame-size" with "Specifies maximum packet length ...".
> 
> Please include it and we'll fix the spec. It is clearly wrong. 2 nios
> boards use 1518 and the consumer for them says it is MTU. Everything
> else clearly uses mtu with 1500 or 9000.

Ok, the example in the pdf is 'max-frame-size = <1518>;'. I'll include 
it with the description of ethernet-controller.yaml which specifies it 
as MTU.

Janne

Re: [PATCH RFC 0/3] dt-bindings: net: Add network-class.yaml schema

[Andrew Lunn]

> > > I've ignored "max-frame-size" since the description in
> > > ethernet-controller.yaml claims there is a contradiction in the
> > > Devicetree specification. I suppose it is describing the property
> > > "max-frame-size" with "Specifies maximum packet length ...".
> > 
> > Please include it and we'll fix the spec. It is clearly wrong. 2 nios
> > boards use 1518 and the consumer for them says it is MTU. Everything
> > else clearly uses mtu with 1500 or 9000.
> 
> Ok, the example in the pdf is 'max-frame-size = <1518>;'. I'll include 
> it with the description of ethernet-controller.yaml which specifies it 
> as MTU.

You need to be careful here. Frame and MTU are different things.

The IEEE 802.3 standard says nothing about MTU. I believe MTU is an IP
concept. It is the size of the SDU an Ethernet PDU can carry. This is
typically 1500.

Historically, the max Ethernet frame size was 1518. But with 802.1Q
which added the VLAN header, all modern hardware actual uses 1522 to
accommodate the extra 4 bytes VLAN header. So i would not actually put
max-frame-size = <1518> anywhere, because it will get copy/pasted and
break VLAN setups.

It looks like the ibm,emac.txt makes this error, max-frame-size =
<5dc>; 0x5dc is 1500. And there are a few powerpc .dtc using
1500/0x5dc, which are probably broken.

      Andrew

Re: [PATCH RFC 0/3] dt-bindings: net: Add network-class.yaml schema

[Janne Grunau]

On 2023-02-07 02:34:41 +0100, Andrew Lunn wrote:
> > > > I've ignored "max-frame-size" since the description in
> > > > ethernet-controller.yaml claims there is a contradiction in the
> > > > Devicetree specification. I suppose it is describing the property
> > > > "max-frame-size" with "Specifies maximum packet length ...".
> > > 
> > > Please include it and we'll fix the spec. It is clearly wrong. 2 nios
> > > boards use 1518 and the consumer for them says it is MTU. Everything
> > > else clearly uses mtu with 1500 or 9000.
> > 
> > Ok, the example in the pdf is 'max-frame-size = <1518>;'. I'll include 
> > it with the description of ethernet-controller.yaml which specifies it 
> > as MTU.
> 
> You need to be careful here. Frame and MTU are different things.

yes, we are aware. The description in of the property in
Documentation/devicetree/bindings/net/ethernet-controller.yaml is:

| Maximum transfer unit (IEEE defined MTU), rather than the
| maximum frame size (there\'s contradiction in the Devicetree
| Specification).

The description for the property in the Devicetree is:

| Specifies maximum packet length in bytes that the physical interface
| can send and receive.

While the "packet length" in the description is a little confusing this 
seems to refer to the ethernet frame size.

> The IEEE 802.3 standard says nothing about MTU. I believe MTU is an IP
> concept. It is the size of the SDU an Ethernet PDU can carry. This is
> typically 1500.
> 
> Historically, the max Ethernet frame size was 1518. But with 802.1Q
> which added the VLAN header, all modern hardware actual uses 1522 to
> accommodate the extra 4 bytes VLAN header. So i would not actually put
> max-frame-size = <1518> anywhere, because it will get copy/pasted and
> break VLAN setups.
> 
> It looks like the ibm,emac.txt makes this error, max-frame-size =
> <5dc>; 0x5dc is 1500. And there are a few powerpc .dtc using
> 1500/0x5dc, which are probably broken.

I would not say it is an error. The specification/name and use of 
"max-frame-size" has clearly diverged. All 4 in-tree users of this 
property interpret it as MTU. With the exception of the 2 nios2 boards 
Rob found all device trees use either 1500, 3800 or 9000 as 
'max-frame-size'.

I think Rob's plan to deal with this conflict between specification and 
actual use is to accept the use and update the description in the 
specification. This results in a "max-frame-size" property which 
describes the maximal payload / MTU. The upside of this is that we can 
leave all devicetrees and drivers unchanged and avoid breaking 
out-of-tree users.

I'll fix the 2 nios2 boards since those currently end up with a MTU of 
1518 in altera_tse_main.c.

Janne

[PATCH RFC 2/3] dt-bindings: wireless: bcm4329-fmac: Use network-class.yaml schema

[Janne Grunau]

The network-class schema specifies local-mac-address as used in the
bcm4329-fmac device nodes of Apple silicon devices
(arch/arm64/boot/dts/apple).
Fixes `make dtbs_check` for those devices.

Signed-off-by: Janne Grunau <j@jannau.net>
---
 .../devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml          | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
index fec1cc9b9a08..55b0a21acb96 100644
--- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
+++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
@@ -116,11 +116,14 @@ properties:
       NVRAM. This would normally be filled in by the bootloader from platform
       configuration data.
 
+allOf:
+  - $ref: /schemas/net/network-class.yaml#
+
 required:
   - compatible
   - reg
 
-additionalProperties: false
+unevaluatedProperties: false
 
 examples:
   - |

-- 
2.39.1

[PATCH RFC 3/3] dt-bindings: wireless: silabs,wfx: Use network-class.yaml

[Janne Grunau]

Instead listing local-mac-address and mac-address properties, reference
network-class.yaml schema. The schema brings in constraints for the
property checked during `make dtbs_check`.

Signed-off-by: Janne Grunau <j@jannau.net>
---
 Documentation/devicetree/bindings/net/wireless/silabs,wfx.yaml | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/wireless/silabs,wfx.yaml b/Documentation/devicetree/bindings/net/wireless/silabs,wfx.yaml
index 583db5d42226..2ce50b57c096 100644
--- a/Documentation/devicetree/bindings/net/wireless/silabs,wfx.yaml
+++ b/Documentation/devicetree/bindings/net/wireless/silabs,wfx.yaml
@@ -72,15 +72,12 @@ properties:
       "Platform Data Set" in Silabs jargon). Default depends of "compatible"
       string. For "silabs,wf200", the default is 'wf200.pds'.
 
-  local-mac-address: true
-
-  mac-address: true
-
 required:
   - compatible
   - reg
 
 allOf:
+  - $ref: /schemas/net/network-class.yaml#
   - $ref: /schemas/spi/spi-peripheral-props.yaml#
 
 unevaluatedProperties: false

-- 
2.39.1