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

On Sat, May 02, 2026 at 03:56:15PM +0000, [email protected] wrote:
> 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.
> --

Please answer sashiko reports because if maintainers are, it's just more 
work for us. While agreeing with maintainer comments don't need a reply, 
I think we're still at the point any AI review does. Though sashiko is 
2/2 here.

> 
> commit c5a5d0d7a89f5ad66c878c805902efc5e0bd117a
> Author: Daniel Thompson <[email protected]>
> 
> 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.

Correct, should be dropped.

> 
> > +  - $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?

No, it doesn't.

> 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.