netdev - Re: [PATCH] dt-bindings: net: davinci_emac: Convert to yaml version from txt

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: Mon, 24 Jun 2024 07:07:07 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Adam Ford <aford173@...il.com>, devicetree@...r.kernel.org
Cc: woods.technical@...il.com, aford@...conembedded.com,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Adam Ford <aford@...il.com>,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] dt-bindings: net: davinci_emac: Convert to yaml version
 from txt

On 23/06/2024 19:09, Adam Ford wrote:
> The davinci_emac is used by several devices which are still maintained,
> but to make some improvements, it's necessary to convert from txt to yaml.
> 
> Signed-off-by: Adam Ford <aford173@...il.com>
> 
> diff --git a/Documentation/devicetree/bindings/net/davinci_emac.txt b/Documentation/devicetree/bindings/net/davinci_emac.txt
> deleted file mode 100644
> index 5e3579e72e2d..000000000000
> --- a/Documentation/devicetree/bindings/net/davinci_emac.txt
> +++ /dev/null
> @@ -1,44 +0,0 @@
> -* Texas Instruments Davinci EMAC
> -
> -This file provides information, what the device node
> -for the davinci_emac interface contains.
> -
> -Required properties:
> -- compatible: "ti,davinci-dm6467-emac", "ti,am3517-emac" or
> -  "ti,dm816-emac"
> -- reg: Offset and length of the register set for the device
> -- ti,davinci-ctrl-reg-offset: offset to control register
> -- ti,davinci-ctrl-mod-reg-offset: offset to control module register
> -- ti,davinci-ctrl-ram-offset: offset to control module ram
> -- ti,davinci-ctrl-ram-size: size of control module ram
> -- interrupts: interrupt mapping for the davinci emac interrupts sources:
> -              4 sources: <Receive Threshold Interrupt
> -			  Receive Interrupt
> -			  Transmit Interrupt
> -			  Miscellaneous Interrupt>
> -
> -Optional properties:
> -- phy-handle: See ethernet.txt file in the same directory.
> -              If absent, davinci_emac driver defaults to 100/FULL.
> -- ti,davinci-rmii-en: 1 byte, 1 means use RMII
> -- ti,davinci-no-bd-ram: boolean, does EMAC have BD RAM?
> -
> -The MAC address will be determined using the optional properties
> -defined in ethernet.txt.
> -
> -Example (enbw_cmc board):
> -	eth0: emac@...0000 {
> -		compatible = "ti,davinci-dm6467-emac";
> -		reg = <0x220000 0x4000>;
> -		ti,davinci-ctrl-reg-offset = <0x3000>;
> -		ti,davinci-ctrl-mod-reg-offset = <0x2000>;
> -		ti,davinci-ctrl-ram-offset = <0>;
> -		ti,davinci-ctrl-ram-size = <0x2000>;
> -		local-mac-address = [ 00 00 00 00 00 00 ];
> -		interrupts = <33
> -				34
> -				35
> -				36
> -				>;
> -		interrupt-parent = <&intc>;
> -	};
> diff --git a/Documentation/devicetree/bindings/net/davinci_emac.yaml b/Documentation/devicetree/bindings/net/davinci_emac.yaml
> new file mode 100644
> index 000000000000..4c2640aef8a1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/davinci_emac.yaml

Filename matching compatible format. Missing vendor prefix. Underscores
are not used in names or compatibles.


> @@ -0,0 +1,111 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/davinci_emac.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments Davici EMAC
> +
> +maintainers:
> +  - Adam Ford <aford@...il.com>
> +
> +description:
> +  Ethernet based on the Programmable Real-Time Unit and Industrial
> +  Communication Subsystem.
> +
> +allOf:
> +  - $ref: ethernet-controller.yaml#
> +
> +properties:
> +  compatible:
> +    items:


That's just enum, no need for items here.

> +      - enum:
> +          - ti,davinci-dm6467-emac # da850
> +          - ti,dm816-emac
> +          - ti,am3517-emac
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    minItems: 4

You need to list and describe the items.

> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: ick
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 1
> +
> +  local-mac-address: true

Drop

> +  mac-address: true

Drop

You miss top-level $ref to appropriate schema.

> +
> +  syscon:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: a phandle to the global system controller on
> +      to enable/disable interrupts

Drop entire property. There was no such property in old binding and
nothing explains why it was added.

> +
> +  ti,davinci-ctrl-reg-offset:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Offset to control register
> +
> +  ti,davinci-ctrl-mod-reg-offset:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Offset to control module register
> +
> +  ti,davinci-ctrl-ram-offset:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Offset to control module ram
> +
> +  ti,davinci-ctrl-ram-size:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Size of control module ram
> +
> +  ti,davinci-rmii-en:
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    description:
> +      RMII enable means use RMII
> +
> +  ti,davinci-no-bd-ram:
> +    type: boolean
> +    description:
> +      Enable if EMAC have BD RAM
> +
> +additionalProperties: false

Look at example-schema. This goes after required, although anyway should
be unevaluatedProperties after adding proper $ref.

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - ti,davinci-ctrl-reg-offset
> +  - ti,davinci-ctrl-mod-reg-offset
> +  - ti,davinci-ctrl-ram-offset
> +  - ti,davinci-ctrl-ram-size
> +
> +examples:
> +  - |
> +    eth0: ethernet@...000 {

Drop label.

> +      compatible = "ti,davinci-dm6467-emac";
> +      reg = <0x220000 0x4000>;
> +      ti,davinci-ctrl-reg-offset = <0x3000>;
> +      ti,davinci-ctrl-mod-reg-offset = <0x2000>;
> +      ti,davinci-ctrl-ram-offset = <0>;
> +      ti,davinci-ctrl-ram-size = <0x2000>;
> +      local-mac-address = [ 00 00 00 00 00 00 ];
> +      interrupts = <33>, <34>, <35>,<36>;
> +      clocks = <&psc1 5>;
> +      power-domains = <&psc1 5>;
> +      status = "disabled";

Drop. It cannot be disabled, otherwise what would be the point of this
example?


Best regards,
Krzysztof