[Linaro-mm-sig] Re: [PATCH net-next 1/6] dt-bindings: net: Brcm ASP 2.0 Ethernet controller

On 4/24/23 11:14, Justin Chen wrote:

On Fri, Apr 21, 2023 at 12:29 AM Krzysztof Kozlowski krzysztof.kozlowski@linaro.org wrote:

...

On 19/04/2023 02:10, Justin Chen wrote:

...

From: Florian Fainelli f.fainelli@gmail.com

Add a binding document for the Broadcom ASP 2.0 Ethernet controller.

Signed-off-by: Florian Fainelli f.fainelli@gmail.com Signed-off-by: Justin Chen justinpopo6@gmail.com

---

.../devicetree/bindings/net/brcm,asp-v2.0.yaml | 146 +++++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/brcm,asp-v2.0.yaml

diff --git a/Documentation/devicetree/bindings/net/brcm,asp-v2.0.yaml b/Documentation/devicetree/bindings/net/brcm,asp-v2.0.yaml new file mode 100644 index 000000000000..3817d722244f --- /dev/null +++ b/Documentation/devicetree/bindings/net/brcm,asp-v2.0.yaml @@ -0,0 +1,146 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/net/brcm,asp-v2.0.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#"

Drop quotes.

...

+title: Broadcom ASP 2.0 Ethernet controller

+maintainers:

• • Justin Chen justinpopo6@gmail.com
• • Florian Fainelli f.fainelli@gmail.com

+description: Broadcom Ethernet controller first introduced with 72165

+properties:

• '#address-cells':
• const: 1
• '#size-cells':
• const: 1
• compatible:
• enum:

•  - brcm,bcm72165-asp-v2.0
  

•  - brcm,asp-v2.0
  

•  - brcm,asp-v2.1
  

Is this part of SoC? If so, then SoC compatibles are preferred, not IP block versions.

We have the same IP on different chips. So no, it isn't tied to a specific SoC.

...

...

• reg:
• maxItems: 1
• description: ASP registers

Drop description.

...

• ranges: true
• interrupts:
• minItems: 1
• items:

•  - description: RX/TX interrupt
  

•  - description: Port 0 Wake-on-LAN
  

•  - description: Port 1 Wake-on-LAN
  

• clocks:
• $ref: /schemas/types.yaml#/definitions/phandle-array

Drop.

...

• description: Phandle to clock controller

Drop.

Instead maxItems.

...

• clock-names:
• const: sw_asp

Drop entire property.

...

• ethernet-ports:
• type: object
• properties:

•  '#address-cells':
  

•    const: 1
  

•  '#size-cells':
  

•    const: 0
  

Missing additionalProperties:false. Look at existing bindings how it is done.

...

• patternProperties:

•  "^port@[0-9]+$":
  

•    type: object
  

•    $ref: ethernet-controller.yaml#
  

•    properties:
  

•      reg:
  

•        maxItems: 1
  

•        description: Port number
  

•      channel:
  

•        maxItems: 1
  

•        description: ASP channel number
  

•    required:
  

•      - reg
  

•      - channel
  

+patternProperties:

• "^mdio@[0-9a-f]+$":
• type: object
• $ref: "brcm,unimac-mdio.yaml"
• description:

•  ASP internal UniMAC MDIO bus
  

+required:

• • compatible
• • reg
• • interrupts
• • clocks
• • clock-names
• • ranges

+additionalProperties: false

+examples:

• • |
• asp@9c00000 {

Node names should be generic. https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetre...

...

•    compatible = "brcm,asp-v2.0";
  

•    reg = <0x9c00000 0x1fff14>;
  

•    interrupts = <0x0 0x33 0x4>;
  

Use proper defines for flags.

Not understanding this comment. Can you elaborate?

I believe Krzysztof would prefer that you use:

interrupts = <GIC_SPI 0x33 IRQ_TYPE_LEVEL_HIGH>

here, which would require using defined from include/dt-bindings/interrupt-controller/{arm-gic.h,irq.h}