Re: [PATCH v3 03/17] spi: dt-bindings: cdns,qspi-nor: Add examples for testing the specific cases

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Rob,

On 21/01/2026 at 17:07:59 -06, Rob Herring <robh@kernel.org> wrote:

> On Wed, Jan 21, 2026 at 06:04:59PM +0100, Miquel Raynal (Schneider Electric) wrote:
>> It is very painful to modify this file because the core IP described is
>> so common, it has been implemented in many SoCs from different
>> architectures. Both `dtbs_check` and `dt_binding_check` are rather long
>> commands, even when restricted to a single schema files, and letting
>> this file evolve without risking to break other DTSs is painful, because
>> there are arm, arm64 and riscv platforms impacted and no way to check
>> all of them at the same time.
>
> OTOH, examples aren't meant to be exhaustive test cases of all 
> possibilities. If it was me, I'd actually just get rid of all the 
> examples. They are generally just a copy from some .dts we already
> have.

I will align with this idea the day `make dtbs_check` (or something
similarly simple) is exhaustive and cross platform :-)

Maybe cdns,qspi-nor is an exception, but it impacts different
architectures, which means the output of `make dtbs_check` is
meaningless because it only covers a subset of the possible cases. Hence
my attempt to gather all specific cases in the bindings, so I could run
all the meaningful checks I wanted more easily.

I think this patch has its usefulness, but I don't mind dropping it.

>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/reset/starfive,jh7110-crg.h>
>> +    #include <dt-bindings/clock/starfive,jh7110-crg.h>
>> +    spi@13010000 {
>> +        compatible = "starfive,jh7110-qspi", "cdns,qspi-nor";
>> +        reg = <0x13010000 0x10000>, <0x21000000 0x400000>;
>> +        interrupts = <25>;
>> +        clocks = <&syscrg JH7110_SYSCLK_QSPI_REF>, <&syscrg JH7110_SYSCLK_QSPI_AHB>,
>> +                 <&syscrg JH7110_SYSCLK_QSPI_APB>;
>> +        clock-names = "ref", "ahb", "apb";
>> +        resets = <&syscrg JH7110_SYSRST_QSPI_APB>, <&syscrg JH7110_SYSRST_QSPI_AHB>,
>> +                 <&syscrg JH7110_SYSRST_QSPI_REF>;
>> +        reset-names = "qspi", "qspi-ocp", "rstc_ref";
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +        cdns,fifo-depth = <256>;
>> +        cdns,fifo-width = <4>;
>> +        cdns,trigger-address = <0x0>;
>> +    };
>> +
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    spi@2400 {
>> +        compatible = "amd,pensando-elba-qspi", "cdns,qspi-nor";
>> +        reg = <0x2400 0x400>, <0x7fff0000 0x1000>;
>> +        interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
>> +        clocks = <&flash_clk>;
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +        cdns,fifo-depth = <1024>;
>> +        cdns,fifo-width = <4>;
>> +        cdns,trigger-address = <0x7fff0000>;
>
> This one really just looks like a subset of the others.

The fifo-depth possibilities are extended just for this
compatible. Basically I captured in the examples every specific case
covered with an 'if' schema.

Miquèl