Re: [PATCH 1/3] dt-bindings: iio: adc: add bindings for stm32 mdf filter

[Olivier MOYSAN <olivier.moysan@foss.st.com>]

Hi Rob,

On 10/7/26 15:47, Rob Herring wrote:
> On Wed, Oct 07, 2026 at 11:01:28AM +0200, Olivier MOYSAN wrote:
>> Hi Conor,
>>
>> Thanks for the review
>>
>> On 10/1/26 20:32, Conor Dooley wrote:
>>> On Thu, Oct 01, 2026 at 04:56:45PM +0200, Olivier Moysan wrote:
>>>> Add bindings that describes STM32 MDF settings to support
>>>> digital filtering for Pulse Density Modulation (PDM) microphones
>>>> and analog sigma delta modulators.
>>>>
>>>> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
>>>> ---
>>>>    .../bindings/iio/adc/st,stm32-mdf-adc.yaml    | 383 ++++++++++++++++++
>>>>    1 file changed, 383 insertions(+)
>>>>    create mode 100644 Documentation/devicetree/bindings/iio/adc/st,stm32-mdf-adc.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-mdf-adc.yaml b/Documentation/devicetree/bindings/iio/adc/st,stm32-mdf-adc.yaml
>>>> new file mode 100644
>>>> index 000000000000..f2fbc3e150e8
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-mdf-adc.yaml
>>>> @@ -0,0 +1,383 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/iio/adc/st,stm32-mdf-adc.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: STMicroelectronics STM32 Multi-function Digital Filter (MDF) ADC
>>>> +
>>>> +maintainers:
>>>> +  - Olivier Moysan <olivier.moysan@foss.st.com>
>>>> +
>>>> +description: |
>>>> +  STM32 MDF ADC is a sigma delta analog-to-digital converter dedicated to
>>>> +  interface external sigma delta modulators to STM32 micro controllers.
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    enum:
>>>> +      - st,stm32mp25-mdf
>>>> +      - st,stm32mp23-mdf
>>>> +
>>>> +  reg:
>>>> +    minItems: 1
>>>> +    maxItems: 2
>>>
>>> This needs an items list here. The size of the regions seems like crap
>>> to begin with...
>>>
>>>> +
>>>> +  clocks:
>>>> +    maxItems: 1
>>>> +
>>>> +  clock-names:
>>>> +    description: Internal clock used for MDF digital processing.
>>>> +    items:
>>>> +      - const: ker_ck
>>>
>>> This is pointless when you only have one.
>>>
>>
>> I agree that it could be dropped. However, it is useful for using
>> devm_regmap_init_mmio_clk.
> 
> I don't know what that function is/does, but that's not justification
> for bindings. Maybe you need a helper that handles a single clock. Or
> that function could take a NULL string for single clock?
> 

Ok. I will drop this clock, and use a helper to replace 
devm_regmap_init_mmio_clk api calls.

>>
>>>> +
>>>> +  "#clock-cells":
>>>> +    enum: [0, 1]
>>>
>>> Why is this not fixed? Also why are parts of your own device consuming
>>> the clocks?
>>>
>>>> +
>>>> +  clock-output-names:
>>>> +    description: |
>>>> +      CCK0 and CCK1 are optional output clocks, which share the same clock frequency,
>>>> +      but can be gated independently to save power.
>>>> +    minItems: 1
>>>> +    maxItems: 2
>>>> +    oneOf:
>>>> +      - items:
>>>> +          - const: cck0
>>>> +      - items:
>>>> +          - const: cck1
>>>> +      - items:
>>>> +          - const: cck0
>>>> +          - const: cck1
>>>> +
>>>> +  clock-frequency:
>>>> +    description: |
>>>> +      Common clock frequency (Hz) for CCK0 and CCK1 output clocks.
>>>> +      The frequency must be a multiple of the "ker_ck" clock frequency.
>>>> +    maximum: 25000000
>>>
>>> Should not be needed, the consumers request what they need.
>>>
>>
>> The CCKx clock frequency depends on the maximum rate supported by the sigma
>> delta converters (for instance a digital mic) and the expected decimation
>> ratio on the bitstream. Typically this determines the frequency on the SPI
>> bus.
>> This rate is defined statically and shared by the CCKx clocks. So IMHO, as
>> this rate is unique, it can look strange to let the consumer define it.
>> Moreover, it seems to me that clock-frequency is already used to configure
>> the frequency of a provider in some other bindings.
>> For instance: Documentation/devicetree/bindings/clock/silabs,si570.yaml
>> So, it's not clear for me, what is the restriction on clock-frequency
>> property.

Would you have any feedback regarding clock-frequency property usage ?

>>>> +
>>>> +  ranges: true
>>>> +
>>>> +  clock-ranges: true
>>>> +
>>>> +  resets:
>>>> +    maxItems: 1
>>>> +
>>>> +  reset-names:
>>>> +    items:
>>>> +      - const: mdf
>>>> +
>>>> +  access-controllers:
>>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>>> +    description: |
>>>> +      Phandle to the rifsc device to check access right.
>>>> +
>>>> +  power-domains:
>>>> +    maxItems: 1
>>>> +
>>>> +  st,interleave:
>>>> +    description: |
>>>> +      List of phandles of interleaved filters. The indexes of interleaved filters must be
>>>> +      consecutives starting from 0 (i.e in range [0..N]). The samples from interleaved filters
>>>> +      are muxed in a single channel and retrieved through the device associated to the filter 0.
>>>> +      The filters 1..N have to be enabled, but inherit their configuration from filter 0.
>>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>>
>>> No idea what these even are, but this is probably not the right way to
>>> represent the relationship between devices. They're apparently ADCs, but
>>> this is also an ADC so I'm not sure what's going on here at all.
> 
> I don't under it either, but regardless phandle-array needs constraints
> on the items. It's really a matrix with array of phandle+args arrays.
> 

What is expected for this property is a list of phandles (from 2 to 8)
For instance: st,interleave = <&filter0 &filter1>;

So, if I just consider the missing constraints, I need to add
     minItems: 2
     maxItems: 8
     items:
       maxItems: 1

Is this correct ?

> Rob

Best regards
Olivier