Re: [PATCH v2 2/2] devicetree bindings mmc Add bindings doc for Sunplus SP7021

[Rob Herring]

On Sun, Nov 28, 2021 at 11:30 PM Lh Kuo 郭力豪 <lh.Kuo@xxxxxxxxxxx> wrote:
>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - sunplus,sp7021-card1
> > > +      - sunplus,sp7021-sdio
> >
> > What's the difference between these 2 blocks?
> >
>
> One for SD card One for SDIO

If the programming model is the same, then it should be the same
compatible string. We have various properties to handle differences
like bus width, card detect or not, etc.

> > > +  reg:
> > > +    items:
> > > +      - description: Base address and length of the SD/SDIO registers
> >
> > Just 'maxItems: 1' is sufficient.
> >
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    maxItems: 1
> > > +
> > > +  resets:
> > > +    maxItems: 1
> > > +
> > > +  pinctrl-names:
> > > +    description:
> > > +      A pinctrl state named "default" must be defined.
> > > +    const: default
> > > +
> > > +  pinctrl-0:
> > > +    description:
> > > +      A phandle to the default pinctrl state.
> > > +
> > > +  max-frequency: true
> > > +
> > > +allOf:
> > > +  - $ref: "mmc-controller.yaml"
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - interrupts
> > > +  - clocks
> > > +  - resets
> > > +  - pinctrl-names
> > > +  - pinctrl-0
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/clock/sp-sp7021.h>
> > > +    #include <dt-bindings/reset/sp-sp7021.h>
> > > +    #include <dt-bindings/interrupt-controller/irq.h>
> > > +    mmc1: mmc@9C003e80 {
> >
> > Use lower case hex.
>
> Do you mean as follows? ?
>
> mmc1: mmc@3e80 {

No, 'mmc@9c003e80 {'

You also don't need 'mmc1'.

> > > +       compatible = "sunplus,sp7021-card1";
> > > +       reg = <0x9c003e80 0x280>;
> > > +       interrupts = <21 IRQ_TYPE_LEVEL_HIGH>;
> > > +       clocks = <&clkc CARD_CTL1>;
> > > +       resets = <&rstc RST_CARD_CTL1>;
> > > +       pinctrl-names = "default";
> > > +       pinctrl-0 = <&mmc1_mux &mmc1_mux_cd>;
> > > +       max-frequency = <52000000>;
> > > +    };
> > > +    sdio: mmc@9C008400 {
> >
>
> Do you mean as follows? ?
>
> mmc1: mmc@8400 {
>
>
> > Use lower case hex.
> >
> > > +       compatible = "sunplus,sp7021-sdio";
>