Re: [PATCH 3/3] dt-bindings: mmc: dw-mshc-hi3798cv200: rename to dw-mshc-histb

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 16/02/2024 09:29, Yang Xiwen wrote:
>>>     reg:
>>>       maxItems: 1
>>> @@ -48,6 +46,12 @@ properties:
>>>         control the clock phases, "ciu-sample" is required for tuning
>>>         high speed modes.
>>>   
>>> +  hisilicon,sap-dll-reg:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>> +    description:
>>> +      A phandle points to the sample delay-locked-loop(DLL)
>>> +      syscon node, used for tuning.
>> Does hi3798cv200 have it?
> No it does not. Currently only hi3798mv200 has it (it's called himci 
> v300 in downstream, while cv200 is using himci v200).

then in your if:
else:
  properties:
    hisilicon,sap-dll-reg: false

>>
>>> +
>>>   required:
>>>     - compatible
>>>     - reg
>>> @@ -55,13 +59,25 @@ required:
>>>     - clocks
>>>     - clock-names
>>>   
>>> +allOf:
>>> +  - $ref: synopsys-dw-mshc-common.yaml#
>>> +
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: hisilicon,hi3798mv200-dw-mshc
>>> +    then:
>>> +      required:
>>> +        - hisilicon,sap-dll-reg
>>> +
>>>   unevaluatedProperties: false
>>>   
>>>   examples:
>>>     - |
>>>       #include <dt-bindings/clock/histb-clock.h>
>>>       #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> -    emmc: mmc@9830000 {
>>> +    mmc@9830000 {
>> ???
> It's complaining about duplicated label when i added emmc label to both 
> nodes. I'll remove it in previous patch in v2.
>>>         compatible = "hisilicon,hi3798cv200-dw-mshc";
>>>         reg = <0x9830000 0x10000>;
>>>         interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
>>> @@ -84,3 +100,31 @@ examples:
>>>         bus-width = <8>;
>>>         status = "okay";
>>>       };
>>> +  - |
>>> +    #include <dt-bindings/clock/histb-clock.h>
>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +    mmc@9830000 {
>>> +      compatible = "hisilicon,hi3798mv200-dw-mshc";
>> No need for new example.
>>
>>> +      reg = <0x9830000 0x10000>;
>>> +      interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
>>> +      clocks = <&crg HISTB_MMC_CIU_CLK>,
>>> +               <&crg HISTB_MMC_BIU_CLK>,
>>> +               <&crg HISTB_MMC_SAMPLE_CLK>,
>>> +               <&crg HISTB_MMC_DRV_CLK>;
>>> +      clock-names = "ciu", "biu", "ciu-sample", "ciu-drive";
>>> +      resets = <&crg 0xa0 4>;
>>> +      reset-names = "reset";
>>> +      pinctrl-names = "default";
>>> +      pinctrl-0 = <&emmc_pins>;
>>> +      fifo-depth = <256>;
>>> +      clock-frequency = <50000000>;
>>> +      max-frequency = <150000000>;
>>> +      cap-mmc-highspeed;
>>> +      mmc-ddr-1_8v;
>>> +      mmc-hs200-1_8v;
>>> +      mmc-hs400-1_8v;
>>> +      non-removable;
>>> +      bus-width = <8>;
>>> +      hisilicon,sap-dll-reg = <&emmc_sap_dll_reg>;
>>> +      status = "okay";
>> No, really...
> The property "hisilicon,sap-dll-reg" is introduced in this patch, i want 
> to add an example for it here since the common dtsi will use this 
> binding and will be submitted when it gets ready.

One new property does not justify new example.

Best regards,
Krzysztof