Re: [PATCH v2 4/5] dt-bindings: PCI: spacemit: Introduce Spacemit K3 PCIe host controller

[Inochi Amaoto <inochiama@gmail.com>]

On Sun, May 17, 2026 at 02:48:47AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] The binding completely omits the `interrupts` and `interrupt-names` properties for the host controller.
> - [Medium] The schema omits standard legacy INTx interrupt routing properties (`#interrupt-cells`, `interrupt-map`, `interrupt-map-mask`).
> - [Low] The `phy-names` property lacks an `items` definition to restrict valid string values.
> --
> 
> commit 7b8ba34456e71d6a9cf1028caa5504d64e8f58ba
> Author: Inochi Amaoto <inochiama@gmail.com>
> 
> dt-bindings: PCI: spacemit: Introduce Spacemit K3 PCIe host controller
> 
> This commit adds device tree binding support for the PCIe controller on the
> SpacemiT K3 SoC. The controller is based on the Synopsys DesignWare PCIe IP
> with some extra link and reset state control.
> 
> > diff --git a/Documentation/devicetree/bindings/pci/spacemit,k3-pcie-host.yaml b/Documentation/devicetree/bindings/pci/spacemit,k3-pcie-host.yaml
> > new file mode 100644
> > index 0000000000000..46147a37a9ce6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/spacemit,k3-pcie-host.yaml
> [ ... ]
> > +  reset-names:
> > +    items:
> > +      - const: dbi
> > +      - const: mstr
> > +      - const: slv
> > +
> > +  msi-parent: true
> > +
> 
> The driver implementation reads and clears interrupt status registers for
> host-level events, and references registers for receiving INTx events from
> downstream devices.
> 
> Should this binding include interrupts and interrupt-names properties to
> document the host's interrupt routing?
> 
> Additionally, should standard legacy INTx interrupt routing properties like
> #interrupt-cells, interrupt-map, and interrupt-map-mask be included in the
> schema and example to support downstream devices that do not use MSI?
> 

The driver just does the interrupt cleanup as it requires, and no
additional interrupt is needed

> > +  phys:
> > +    description:
> > +      PHY phandle from the Combo PHY, the lane number does not depends
> > +      on this, since the number of lanes provided by Combo PHY can be
> > +      1 or 2.
> > +    minItems: 1
> > +    maxItems: 6
> > +
> > +  phy-names:
> > +    minItems: 1
> > +    maxItems: 6
> 
> This isn't a bug, but the phy-names property lacks an items definition to
> restrict valid string values. The schema allows any arbitrary string here.
> 
> Since the driver retrieves PHYs by index rather than name, is this property
> necessary, or should the expected string items be defined?
> 

The driver just use index, and the name follow generic DesignWare
binding.

> [ ... ]
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260517014841.254085-1-inochiama@gmail.com?part=4

Regards,
Inochi