Sashiko discussions
 help / color / mirror / Atom feed
* Re: [PATCH net-next 08/12] dt-bindings: net: toshiba,tc965x-dwmac: add TC956x Ethernet bridge
       [not found] ` <20260502155615.AF378C19425@smtp.kernel.org>
@ 2026-05-07 19:02   ` Rob Herring
  2026-05-08 14:36     ` Alex Elder
  0 siblings, 1 reply; 2+ messages in thread
From: Rob Herring @ 2026-05-07 19:02 UTC (permalink / raw)
  To: sashiko; +Cc: Alex Elder, bpf

On Sat, May 02, 2026 at 03:56:15PM +0000, sashiko-bot@kernel.org 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 <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.

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.


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH net-next 08/12] dt-bindings: net: toshiba,tc965x-dwmac: add TC956x Ethernet bridge
  2026-05-07 19:02   ` [PATCH net-next 08/12] dt-bindings: net: toshiba,tc965x-dwmac: add TC956x Ethernet bridge Rob Herring
@ 2026-05-08 14:36     ` Alex Elder
  0 siblings, 0 replies; 2+ messages in thread
From: Alex Elder @ 2026-05-08 14:36 UTC (permalink / raw)
  To: Rob Herring, sashiko; +Cc: bpf

On 5/7/26 2:02 PM, Rob Herring wrote:
> 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.

I plan to.  I've been responding to maintainer/reviewer things first,
but I fully intend to treat sashiko as a "proper reviewer" and will
respond to those as well.

					-Alex

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-05-08 14:36 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260501155421.3329862-9-elder@riscstar.com>
     [not found] ` <20260502155615.AF378C19425@smtp.kernel.org>
2026-05-07 19:02   ` [PATCH net-next 08/12] dt-bindings: net: toshiba,tc965x-dwmac: add TC956x Ethernet bridge Rob Herring
2026-05-08 14:36     ` Alex Elder

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox