Re: [PATCH 1/8] dt-bindings: iio: adc: stm32-adc: add support for stm32mp23 and stm32mp25

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] Documentation for the deprecated `st,adc-channels` property explicitly lists `stm32mp23` and `stm32mp25` as supporting up to 20 channels, but the property is explicitly forbidden in the schema for these compatibles.
- [Low] The `st,adc-trigger-sel` property specifies `default: 0` but is also strictly required for the `stm32mp23` and `stm32mp25` variants.
- [Low] The schema attempts to restrict channel 19 using `channel@19: false`, which fails to prevent the usage of channel 19 in `reg` or `diff-channels` within other channel nodes.
--

commit 928337c3337781d9cd13fb9cf442caeafdde7db6
Author: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

dt-bindings: iio: adc: stm32-adc: add support for stm32mp23 and stm32mp25

Add stm32mp23 and stm32mp25 compatibles. Three ADCs can be found on
these SoCs, split into two blocks:
- ADC12 are tightly coupled (common registers).
- ADC3 is managed independently.
Each ADC is a 12-bits successive approximation analog-to-digital converter,
with up to 20 multiplexed channels that can be configured as single ended
or differential.

> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.yaml b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.yaml
> index c4c4575d3fa96..0e0ceaa3d8d6f 100644
> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.yaml
[ ... ]
> @@ -294,8 +338,8 @@ patternProperties:
>            List of single-ended channels muxed for this ADC. It can have up to:
>              - 16 channels, numbered from 0 to 15 (for in0..in15) on stm32f4
>              - 19 channels, numbered from 0 to 18 (for in0..in18) on stm32mp13.
> -            - 20 channels, numbered from 0 to 19 (for in0..in19) on stm32h7 and
> -              stm32mp1.
> +            - 20 channels, numbered from 0 to 19 (for in0..in19) on stm32h7,
> +              stm32mp1, stm32mp23 and stm32mp25.
>          $ref: /schemas/types.yaml#/definitions/uint32-array
>          deprecated: true

[Severity: Low]
The description explicitly adds stm32mp23 and stm32mp25 to the list of SoCs
supporting this deprecated property, but the if-then block below enforces
st,adc-channels: false for these variants. Does the documentation need to be
updated to clarify this property is not supported on these new SoCs?

[ ... ]
> @@ -344,6 +388,12 @@ patternProperties:
>          items:
>            - const: vrefint
>  
> +      st,adc-trigger-sel:
> +        description: Selects the HW trigger configuration available on this ADC
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        enum: [0, 1]
> +        default: 0
> +

[Severity: Low]
Does declaring a default value here make sense since this property is
strictly required by the schema block below? A required property must be
explicitly present in the device tree for validation to pass, which renders
this default declaration useless.

[ ... ]
> @@ -486,6 +536,38 @@ patternProperties:
>                maxItems: 19
>                items:
>                  minimum: 40
> +
> +      - if:
> +          properties:
> +            compatible:
> +              contains:
> +                enum:
> +                  - st,stm32mp23-adc
> +                  - st,stm32mp25-adc
> +
> +        then:
> +          properties:
> +            reg:
> +              enum:
> +                - 0x0
> +                - 0x100
> +
> +            interrupts:
> +              minimum: 0
> +              maximum: 1
> +
> +            assigned-resolution-bits:
> +              enum: [6, 8, 10, 12]
> +              default: 12
> +
> +            channel@19: false

[Severity: Low]
The commit message mentions restricting the last channel (19) as it is wired
directly to ADC Vref+. However, setting channel@19: false only prevents a
node explicitly named channel@19. Could a user still bypass this restriction
by using diff-channels = <X 19>; or reg = <19> inside another channel node
name?

> +            st,adc-channels: false
> +            st,adc-diff-channels: false
> +            st,min-sample-time-nsecs: false
> +
> +          required:
> +            - st,adc-trigger-sel

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-adc-stm32mp25-v1-v1-0-cef26eecdef2@foss.st.com?part=1