Re: [PATCH net-next 08/12] dt-bindings: net: toshiba,tc965x-dwmac: add TC956x Ethernet bridge

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The binding adds GPIO provider capabilities but omits `gpio-controller` and `#gpio-cells` from the `required` list.
- [Medium] The Ethernet endpoint is incorrectly modeled as a PCI bridge, applying bus properties to an endpoint.
--

commit c5a5d0d7a89f5ad66c878c805902efc5e0bd117a
Author: Daniel Thompson <daniel@riscstar.com>

dt-bindings: net: toshiba,tc965x-dwmac: add TC956x Ethernet bridge

This commit adds devicetree bindings for the Toshiba TC956x family of
Ethernet-AVB/TSN bridges.

> diff --git a/Documentation/devicetree/bindings/net/toshiba,tc956x-dwmac.yaml b/Documentation/devicetree/bindings/net/toshiba,tc956x-dwmac.yaml
> new file mode 100644
> index 000000000000..d95d22a3761d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/toshiba,tc956x-dwmac.yaml
> @@ -0,0 +1,111 @@

[ ... ]

> +allOf:
> +  - $ref: /schemas/pci/pci-bus-common.yaml#

Is it correct to reference pci-bus-common.yaml# here? Since the tc956x_emac0
node is a PCI endpoint (an Ethernet controller attached to an internal PCIe
switch) rather than a PCI bus or bridge, applying bridge-specific properties
could be semantically invalid.

> +  - $ref: /schemas/pci/pci-device.yaml#
> +
> +unevaluatedProperties: false
> +
> +properties:
> +  compatible:
> +    enum:
> +      - pci1179,0220 # Toshiba TC9564 (a.k.a. Qualcomm QPS615)
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  gpio-controller: true

[ ... ]

> +required:
> +  - compatible
> +  - reg

Should gpio-controller and #gpio-cells be explicitly included in this
required list?

The binding defines gpio-controller and #gpio-cells above, indicating the
hardware acts as a GPIO provider. Omitting them from the required list could
allow incomplete device tree nodes to silently pass schema validation.

> +examples:
> +  - |
> +    pcie {
> +      #address-cells = <3>;
> +      #size-cells = <2>;
> +
> +      tc956x_emac0: pci@0,0 {
> +        compatible = "pci1179,0220";
> +        reg = <0x50000 0x0 0x0 0x0 0x0>;
> +        #address-cells = <3>;
> +        #size-cells = <2>;
> +        device_type = "pci";
> +        ranges;

Does this endpoint node need bridge properties like device_type = "pci",
#address-cells = <3>, #size-cells = <2>, and ranges?

The child of this node is an mdio bus, which does not use PCI addresses and
does not require PCI bridge translation. Using device_type = "pci" on an
endpoint might confuse OS PCI enumeration logic, which expects nodes with
this device_type to be actual PCI-to-PCI bridges.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260501155421.3329862-1-elder@riscstar.com?part=8