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, 1 Jul 2024 12:21:51 -0600
From: Rob Herring <robh@...nel.org>
To: Lorenzo Bianconi <lorenzo@...nel.org>
Cc: netdev@...r.kernel.org, nbd@....name, lorenzo.bianconi83@...il.com,
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, conor@...nel.org,
	linux-arm-kernel@...ts.infradead.org,
	krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
	devicetree@...r.kernel.org, catalin.marinas@....com,
	will@...nel.org, upstream@...oha.com,
	angelogioacchino.delregno@...labora.com,
	benjamin.larsson@...exis.eu, rkannoth@...vell.com,
	sgoutham@...vell.com, andrew@...n.ch, arnd@...db.de,
	horms@...nel.org
Subject: Re: [PATCH v4 1/2] dt-bindings: net: airoha: Add EN7581 ethernet
 controller

On Sat, Jun 29, 2024 at 05:01:37PM +0200, Lorenzo Bianconi wrote:
> Introduce device-tree binding documentation for Airoha EN7581 ethernet
> mac controller.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@...nel.org>
> ---
>  .../bindings/net/airoha,en7581-eth.yaml       | 171 ++++++++++++++++++
>  1 file changed, 171 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml b/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
> new file mode 100644
> index 000000000000..e2c0da02ccf2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
> @@ -0,0 +1,171 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/airoha,en7581-eth.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Airoha EN7581 Frame Engine Ethernet controller
> +
> +allOf:
> +  - $ref: ethernet-controller.yaml#
> +
> +maintainers:
> +  - Lorenzo Bianconi <lorenzo@...nel.org>
> +
> +description:
> +  The frame engine ethernet controller can be found on Airoha SoCs.
> +  These SoCs have dual GMAC ports.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - airoha,en7581-eth
> +
> +  reg:
> +    items:
> +      - description: Frame engine base address
> +      - description: QDMA0 base address
> +      - description: QDMA1 base address
> +
> +  reg-names:
> +    items:
> +      - const: fe
> +      - const: qdma0
> +      - const: qdma1
> +
> +  interrupts:
> +    items:
> +      - description: QDMA lan irq0
> +      - description: QDMA lan irq1
> +      - description: QDMA lan irq2
> +      - description: QDMA lan irq3
> +      - description: QDMA wan irq0
> +      - description: QDMA wan irq1
> +      - description: QDMA wan irq2
> +      - description: QDMA wan irq3
> +      - description: FE error irq
> +      - description: PDMA irq
> +
> +  resets:
> +    maxItems: 8
> +
> +  reset-names:
> +    items:
> +      - const: fe
> +      - const: pdma
> +      - const: qdma
> +      - const: xsi-mac
> +      - const: hsi0-mac
> +      - const: hsi1-mac
> +      - const: hsi-mac
> +      - const: xfp-mac
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +patternProperties:
> +  "^mac@[1-4]$":

ethernet@

> +    type: object
> +    unevaluatedProperties: false
> +    allOf:

Can drop 'allOf'

> +      - $ref: ethernet-controller.yaml#

Which node represents an ethernet controller? This one or the parent? 
Most likely it is not both.

> +    description:
> +      Ethernet MAC node
> +    properties:
> +      compatible:
> +        const: airoha,eth-mac
> +
> +      reg:
> +        maxItems: 1

Based on the unit-address, you need instead:

minimum: 1
maximum: 4

But what does 1-4 represent? There are no MMIO registers associated with 
the MACs? Please describe.

> +
> +    required:
> +      - reg
> +      - compatible
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - resets
> +  - reset-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/clock/en7523-clk.h>
> +
> +    soc {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      eth0: ethernet@...50000 {
> +        compatible = "airoha,en7581-eth";
> +        reg = <0 0x1fb50000 0 0x2600>,
> +              <0 0x1fb54000 0 0x2000>,
> +              <0 0x1fb56000 0 0x2000>;
> +        reg-names = "fe", "qdma0", "qdma1";
> +
> +        resets = <&scuclk 44>,
> +                 <&scuclk 30>,
> +                 <&scuclk 31>,
> +                 <&scuclk 6>,
> +                 <&scuclk 15>,
> +                 <&scuclk 16>,
> +                 <&scuclk 17>,
> +                 <&scuclk 26>;
> +        reset-names = "fe", "pdma", "qdma", "xsi-mac",
> +                      "hsi0-mac", "hsi1-mac", "hsi-mac",
> +                      "xfp-mac";
> +
> +        interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
> +
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        mac1: mac@1 {
> +          compatible = "airoha,eth-mac";
> +          reg = <1>;
> +          phy-mode = "2500base-x";
> +          phy-handle = <&phy0>;
> +        };
> +
> +        mac2: mac@2 {
> +          compatible = "airoha,eth-mac";
> +          reg = <2>;
> +          phy-mode = "2500base-x";
> +          phy-handle = <&phy1>;
> +        };
> +      };
> +
> +      mdio: mdio-bus {

mdio {

But really, drop this if it is not part of this device (binding). What 
is the control interface for this MDIO bus? If it is part of this 
device, then the mdio node needs to be within the device's node.

> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        phy0: ethernet-phy@0 {
> +            compatible = "ethernet-phy-id67c9.de0a";
> +            reg = <0>;
> +            phy-mode = "2500base-x";
> +        };
> +
> +        phy1: ethernet-phy@1 {
> +            compatible = "ethernet-phy-id67c9.de0a";
> +            reg = <1>;
> +            phy-mode = "2500base-x";
> +        };
> +      };
> +    };
> -- 
> 2.45.2
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ