Re: [PATCH 1/2] dt-bindings: mfd: syscon: Add img,boston-platform-regs

[Krzysztof Kozlowski <krzk@kernel.org>]

On 17/05/2024 12:15, Jiaxun Yang wrote:
>>>
>>> Do you mind telling dumb as me how to test bindings properly?
>>>
>>> I tried to run make check_dtbs after applying this patch and that's all
>>> the warnings I got:
>>> ```
>>> arch/mips/boot/dts/img/boston.dtb: Warning (interrupt_map): Failed prerequisite 'interrupt_provider'
>>> /home/flygoat/linux-next/arch/mips/boot/dts/img/boston.dtb: /: 'model' is a required property
>>>         from schema $id: http://devicetree.org/schemas/root-node.yaml#
>>> arch/mips/boot/dts/img/boston.dtb: /: failed to match any schema with compatible: ['img,boston']
>>> arch/mips/boot/dts/img/boston.dtb: /cpus/cpu@0: failed to match any schema with compatible: ['img,mips']
>>> arch/mips/boot/dts/img/boston.dtb: /system-controller@17ffd000/clock: failed to match any schema with compatible: ['img,boston-clock']
>>> /home/flygoat/linux-next/arch/mips/boot/dts/img/boston.dtb: uart@17ffe000: $nodename:0: 'uart@17ffe000' does not match '^serial(@.*)?$'
>>>         from schema $id: http://devicetree.org/schemas/serial/8250.yaml#
>>> ```
>>>
>>> I think there is no new warning introduced by this patch.
>>> Did I miss anything here?
>>
>> Indeed there might be no warning for this, because syscon allows a lot,
>> including children. You need custom binding anyway.
> 
> Hi Krzysztof,
> 
> I really don't understand the reason behind having a custom binding for
> such device. It even doesn't have a driver and purely rely on generic
> syscon regmap to work.
> 
> I can see devices like mediatek,mt8365-syscfg being described by simple
> binding have a similar nature, they all describe a register bank and with
> serval sub nodes to describe devices being instanced under that register
> block.
> 
> What makes img,boston-platform-regs special here?

Nothing is special here, I am in writing a patchset which will fix this
up and all will be treated similarly.

Bindings for devices should be specific, which then allows to see that
you do not have dependency of children on the parent, and your current
patch allows basically anything as a child. It allows also an
dependency, even if you did not code it.

Best regards,
Krzysztof