Re: [PATCH] dt-bindings: mmc: ti,da830-mmc: convert text based binding to json schema

[Charan Pedumuru <charan.pedumuru@gmail.com>]

On 28-05-2025 13:32, Krzysztof Kozlowski wrote:
> On 23/05/2025 15:34, Charan Pedumuru wrote:
>> Convert TI Highspeed MMC host controller binding to YAML format. Add
>> 'clocks' property to resolve errors identified by 'dtb_check'.
>>
>> Signed-off-by: Charan Pedumuru <charan.pedumuru@gmail.com>
>> ---
>>  .../devicetree/bindings/mmc/davinci_mmc.txt        | 32 -----------
>>  .../devicetree/bindings/mmc/ti,da830-mmc.yaml      | 62 ++++++++++++++++++++++
>>  2 files changed, 62 insertions(+), 32 deletions(-)
>>
> 
> 
> A nit, subject: drop second/last, redundant "binding". The "dt-bindings"
> prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
> 
> "convert to DT Schema"

Sure, will change that.

> 
> 
> 
> ...
> 
>> +
>> +allOf:
>> +  - $ref: mmc-controller.yaml
>> +
>> +maintainers:
>> +  - Rob Herring <robh@kernel.org>
> 
> No, I really doubt Rob cares about this hardware.

I will remove Rob from maintainers and add Ulf under the maintainers.

> 
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - ti,da830-mmc
>> +      - ti,dm355-mmc
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 2
>> +
> 
> This wasn't in original binding. You need to document this in the commit
> msg. Also, list the items.

Sure, but what list of items for interrupts?

> 
>> +  dmas:
>> +    maxItems: 2
>> +
>> +  dma-names:
>> +    items:
>> +      - const: rx
>> +      - const: tx
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
> 
> why interrupts are required?

It is not necessary, will remove it from required.

> 
> 
> Best regards,
> Krzysztof

-- 
Best Regards,
Charan.