Re: [PATCH] dt-bindings: mmc: xenon: Convert to JSON schema

[Krzysztof Kozlowski <krzk@kernel.org>]

On 20/03/2022 20:51, Chris Packham wrote:
> 

(...)

>>> +
>>> +patternProperties:
>>> +  "^sdhci@[0-9a-f]+$":
>>> +    type: object
>>> +    $ref: mmc-controller.yaml
>> This is unusual schema... What are you matching here? Are these children
>> of this device?
> I was going for compatibility with existing uses. The 
> mmc-controller.yaml schema expects these nodes to be mmc@... . But all 
> of the existing usages of these bindings use sdhci@... as the primary 
> node. I could make my example use mmc@ to squash the warning but I was 
> hoping to be able to do something that didn't make the existing usages 
> invalid.

Please do not create inconsistent bindings because some DTS are
inconsistent. Change the DTS and align them with generic MMC schema.
Node name should not be considered an ABI, so it can be changed in DTS.
Some systems unfortunately break (usually Android and Chrome like to
encode node names), so then it would have to be individually discussed.

>> Looks like you wanted allOf. See some existing examples, like:
>> Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>>
>>> +
>>> +    properties:
>>> +      compatible:
>>> +        oneOf:
>>> +          - const: marvell,armada-3700-sdhci
>>> +            description: |
>>> +              Must provide a second register area and marvell,pad-type
>>> +          - const: marvell,armada-ap806-sdhci
>>> +          - const: marvell,armada-ap807-sdhci
>> This looks wrong. Either these can be standalone properties or in a list
>> like in your last items below.
> I was trying to allow 'compatible = "marvell,armada-ap806-sdhci";' or 
> 'compatible = "marvell,armada-ap807-sdhci", "marvell,armada-ap806-sdhci";'

But you have here 807! Both 806 and 807.  So is 807 compatible with 806
or not?

Best regards,
Krzysztof