Re: [PATCH v4] spi: dt-bindings: microchip,pic32mzda-spi: Convert to DT schema

[email protected]
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.