Re: [PATCH] dt-bindings: scsi: Convert hisilicon,hip05-sas-v1 to DT schema

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] dt-bindings: hisi_sas: incorrect minItems and truncated example for interrupts
- [Low] dt-bindings: hisi_sas: incorrect description for syscon offset registers
--

commit df864f7524bed6f8dd806c6c738b770b5097c904
Author: Rob Herring (Arm) <robh@kernel.org>

dt-bindings: scsi: Convert hisilicon,hip05-sas-v1 to DT schema

Convert the HiSilicon SAS controller binding to DT schema. It's a
straight forward conversion.

> diff --git a/Documentation/devicetree/bindings/scsi/hisilicon,hip05-sas-v1.yaml b/Documentation/devicetree/bindings/scsi/hisilicon,hip05-sas-v1.yaml
> new file mode 100644
> index 0000000000000..847d90018243f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/scsi/hisilicon,hip05-sas-v1.yaml

[ ... ]

> +  interrupts:
> +    description: >
> +      Interrupt specifiers for PHY, SATA, and completion queues; ordering and
> +      number depend on controller version.
> +
> +      For v1 hw: Interrupts for phys, completion queues, and fatal
> +      sources; the interrupts are ordered in 3 groups, as follows:

[ ... ]

> +    minItems: 53
> +    maxItems: 128

[Severity: Medium]
Does this enforce an artificially low minimum bound based on a corrupted
example? 

The YAML conversion drops 5 interrupts from the example (3 CQ interrupts and 2
fatal interrupts), artificially reducing the array size to 53, and then
erroneously sets minItems: 53 to match. 

If a developer uses this example as a template, the resulting device tree will
pass schema validation but fail during driver probe. The hisi_sas_v1_hw.c
driver's interrupt_init_v1_hw() function sequentially requests exactly 58
interrupts (phy-count * 3 + queue-count + 2) via platform_get_irq(). For the
example configuration (8 PHYs, 32 queues), it will fail with an error when it
attempts to fetch index 53, aborting the probe sequence.

> +
> +  ctrl-reset-reg:
> +    description: Offset to controller reset register in SAS register region.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  ctrl-reset-sts-reg:
> +    description: Offset to controller reset status register in SAS register region.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  ctrl-clock-ena-reg:
> +    description: Offset to controller clock enable register in SAS register region.
> +    $ref: /schemas/types.yaml#/definitions/uint32

[Severity: Low]
Are these descriptions accurate? 

They incorrectly state these are offsets in the "SAS register region". This
misinforms hardware integrators about which memory space these registers reside
in.

The kernel driver (hisi_sas_main.c and hisi_sas_v1_hw.c) correctly retrieves a
syscon regmap using the hisilicon,sas-syscon phandle and applies these offsets
to the syscon region, not the SAS register region defined by the reg property.
This is a documentation error introduced during the text-to-YAML conversion
(the original text binding correctly said "ctrl reg").

[ ... ]

> +examples:
> +  - |
> +    sas@c1000000 {
> +        compatible = "hisilicon,hip05-sas-v1";
> +        sas-addr = [50 01 88 20 16 00 00 0a];
> +        reg = <0x0 0xc1000000 0x0 0x10000>;
> +        hisilicon,sas-syscon = <&pcie_sas>;
> +        ctrl-reset-reg = <0xa60>;
> +        ctrl-reset-sts-reg = <0x5a30>;
> +        ctrl-clock-ena-reg = <0x338>;
> +        queue-count = <32>;
> +        phy-count = <8>;
> +        dma-coherent;
> +        interrupts = <259 4>, <263 4>, <264 4>,
> +                     <269 4>, <273 4>, <274 4>,
> +                     <279 4>, <283 4>, <284 4>,
> +                     <289 4>, <293 4>, <294 4>,
> +                     <299 4>, <303 4>, <304 4>,
> +                     <309 4>, <313 4>, <314 4>,
> +                     <319 4>, <323 4>, <324 4>,
> +                     <329 4>, <333 4>, <334 4>,
> +                     <339 1>, <340 1>, <341 1>,

[Severity: Medium]
Is this example missing entries?

It appears to be missing CQ 0-2 and the two fatal interrupts, yielding only 53
interrupts despite specifying a queue-count of 32.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922190128.376072-1-robh@kernel.org?part=1