Re: [PATCH v4] spi: dt-bindings: microchip,pic32mzda-spi: Convert to DT schema
| Newsgroups | org.kernel.vger.linux-devicetree |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The schema incorrectly restricts `cs-gpios` to `maxItems: 1` and makes it strictly required, breaking multi-device and grounded-CS SPI bus configurations. -- --- Patch [1]: [PATCH v4] spi: dt-bindings: microchip,pic32mzda-spi: Convert to DT schema --- commit fca958be633f7ae75e390f0bb61a2c6c29a85193 Author: Udaya Kiran Challa <[email protected]> spi: dt-bindings: microchip,pic32mzda-spi: Convert to DT schema Convert Microchip PIC32 SPI controller devicetree binding from legacy text format to DT schema. Link to v3: https://lore.kernel.org/all/[email protected]/ Link to v2: https://lore.kernel.org/all/[email protected]/ Link to v1: https://lore.kernel.org/all/[email protected]/ > diff --git a/Documentation/devicetree/bindings/spi/microchip,pic32mzda-spi.yaml b/Documentation/devicetree/bindings/spi/microchip,pic32mzda-spi.yaml > new file mode 100644 > index 0000000000000..2ef3480fa63e4 > --- /dev/null > +++ b/Documentation/devicetree/bindings/spi/microchip,pic32mzda-spi.yaml [ ... ] > + clocks: > + maxItems: 1 > + > + clock-names: > + items: > + - const: mck0 > + > + cs-gpios: > + maxItems: 1 [Severity: Medium] Does this limit the controller to a single device? The legacy text binding explicitly allowed multiple pins to be used for chip selects. The SPI core in drivers/spi/spi.c supports multiple devices by dynamically expanding the number of chip selects if multiple GPIOs are provided. Setting this to a maximum of one breaks valid multiplexed SPI bus configurations. > + > + dmas: > + items: > + - description: RX DMA channel > + - description: TX DMA channel [ ... ] > +required: > + - compatible > + - reg > + - interrupts > + - interrupt-names > + - clocks > + - clock-names > + - cs-gpios [Severity: Medium] Should cs-gpios be strictly required? Making it mandatory breaks device tree backward compatibility for valid hardware configurations where a single SPI device's chip select is permanently tied to its active state (e.g., tied to ground). In such tied-low configurations, no software chip select toggling is needed, so the property is legitimately omitted from the hardware description. > + > +unevaluatedProperties: false > + > +examples: -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1