| Message ID | ad96fff723675c2d65a5e3328da9b09f2781cbcd.1713036964.git.andrea.porta@suse.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for BCM2712 SD card controller | expand |
On Sun, 14 Apr 2024 00:14:24 +0200, Andrea della Porta wrote: > Signed-off-by: Andrea della Porta <andrea.porta@suse.com> > --- > .../bindings/mmc/brcm,sdhci-brcmstb.yaml | 51 ++++++++++++++++++- > 1 file changed, 50 insertions(+), 1 deletion(-) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: ./Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml:91:10: [warning] wrong indentation: expected 10 but found 9 (indentation) ./Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml:91:17: [error] string value is redundantly quoted with any quotes (quoted-strings) dtschema/dtc warnings/errors: doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/ad96fff723675c2d65a5e3328da9b09f2781cbcd.1713036964.git.andrea.porta@suse.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On 14/04/2024 00:14, Andrea della Porta wrote: > Signed-off-by: Andrea della Porta <andrea.porta@suse.com> ? And what is being done here and why? > --- > .../bindings/mmc/brcm,sdhci-brcmstb.yaml | 51 ++++++++++++++++++- > 1 file changed, 50 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml > index cbd3d6c6c77f..6aa137d78e4f 100644 > --- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml > +++ b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml > @@ -13,6 +13,7 @@ maintainers: > properties: > compatible: > oneOf: > + - const: brcm,bcm2712-sdhci > - items: > - enum: > - brcm,bcm7216-sdhci > @@ -26,12 +27,16 @@ properties: > - const: brcm,sdhci-brcmstb > > reg: > - maxItems: 2 > + minItems: 2 > + maxItems: 4 > > reg-names: > + minItems: 2 > items: > - const: host > - const: cfg > + - const: busisol > + - const: lcpll > > interrupts: > maxItems: 1 > @@ -60,6 +65,7 @@ properties: > description: Specifies that controller should use auto CMD12 > > allOf: > + - $ref: sdhci-common.yaml > - $ref: mmc-controller.yaml# Why? Anyway, this replaces mmc-controller, doesn't it? > - if: > properties: > @@ -71,6 +77,28 @@ allOf: > required: > - clock-frequency > > + - if: > + properties: > + compatible: > + contains: > + const: brcm,bcm2712-sdhci > + > + then: > + properties: > + reg: > + maxItems: 4 > + clock-names: > + const: "sw_sdio" Not tested. > + > + else: > + properties: > + reg: > + minItems: 2 > + maxItems: 2 > + reg-names: > + minItems: 2 > + maxItems: 2 > + > required: > - compatible > - reg > @@ -114,3 +142,24 @@ examples: > clocks = <&scmi_clk 245>; > clock-names = "sw_sdio"; > }; > + > + - | > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + > + mmc@fff000 { > + compatible = "brcm,bcm2712-sdhci"; > + reg = <0x10 0x00fff000 0x0 0x260>, > + <0x10 0x00fff400 0x0 0x200>, > + <0x10 0x015040b0 0x0 0x4>, // Bus isolation control > + <0x10 0x015200f0 0x0 0x24>; // LCPLL control misc0-8 > + reg-names = "host", "cfg", "busisol", "lcpll"; > + interrupts = <0x0 0x111 0x4>; Use proper defines. > + clocks = <&clk_emmc2>; > + sdhci-caps-mask = <0x0000C000 0x0>; > + sdhci-caps = <0x0 0x0>; > + mmc-ddr-3_3v; > + clock-names = "sw_sdio"; names *always* follow property. In every DTS. Please fix youro DTS. Best regards, Krzysztof
On 4/13/2024 3:14 PM, Andrea della Porta wrote: > Signed-off-by: Andrea della Porta <andrea.porta@suse.com> > --- > .../bindings/mmc/brcm,sdhci-brcmstb.yaml | 51 ++++++++++++++++++- > 1 file changed, 50 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml > index cbd3d6c6c77f..6aa137d78e4f 100644 > --- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml > +++ b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml > @@ -13,6 +13,7 @@ maintainers: > properties: > compatible: > oneOf: > + - const: brcm,bcm2712-sdhci > - items: > - enum: > - brcm,bcm7216-sdhci > @@ -26,12 +27,16 @@ properties: > - const: brcm,sdhci-brcmstb > > reg: > - maxItems: 2 > + minItems: 2 > + maxItems: 4 > > reg-names: > + minItems: 2 > items: > - const: host > - const: cfg > + - const: busisol > + - const: lcpll > > interrupts: > maxItems: 1 > @@ -60,6 +65,7 @@ properties: > description: Specifies that controller should use auto CMD12 > > allOf: > + - $ref: sdhci-common.yaml > - $ref: mmc-controller.yaml# > - if: > properties: > @@ -71,6 +77,28 @@ allOf: > required: > - clock-frequency > > + - if: > + properties: > + compatible: > + contains: > + const: brcm,bcm2712-sdhci > + > + then: > + properties: > + reg: > + maxItems: 4 > + clock-names: > + const: "sw_sdio" > + > + else: > + properties: > + reg: > + minItems: 2 > + maxItems: 2 > + reg-names: > + minItems: 2 > + maxItems: 2 > + > required: > - compatible > - reg > @@ -114,3 +142,24 @@ examples: > clocks = <&scmi_clk 245>; > clock-names = "sw_sdio"; > }; > + > + - | > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + > + mmc@fff000 { > + compatible = "brcm,bcm2712-sdhci"; > + reg = <0x10 0x00fff000 0x0 0x260>, > + <0x10 0x00fff400 0x0 0x200>, > + <0x10 0x015040b0 0x0 0x4>, // Bus isolation control That should be a syscon, not under direct management of the driver because there are other bits in that register that are relevant here. > + <0x10 0x015200f0 0x0 0x24>; // LCPLL control misc0-8 And likewise, this should be a clock provider, or the firmware could add support for configuring the LCPLL since there are other things going on there.
diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml index cbd3d6c6c77f..6aa137d78e4f 100644 --- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml +++ b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml @@ -13,6 +13,7 @@ maintainers: properties: compatible: oneOf: + - const: brcm,bcm2712-sdhci - items: - enum: - brcm,bcm7216-sdhci @@ -26,12 +27,16 @@ properties: - const: brcm,sdhci-brcmstb reg: - maxItems: 2 + minItems: 2 + maxItems: 4 reg-names: + minItems: 2 items: - const: host - const: cfg + - const: busisol + - const: lcpll interrupts: maxItems: 1 @@ -60,6 +65,7 @@ properties: description: Specifies that controller should use auto CMD12 allOf: + - $ref: sdhci-common.yaml - $ref: mmc-controller.yaml# - if: properties: @@ -71,6 +77,28 @@ allOf: required: - clock-frequency + - if: + properties: + compatible: + contains: + const: brcm,bcm2712-sdhci + + then: + properties: + reg: + maxItems: 4 + clock-names: + const: "sw_sdio" + + else: + properties: + reg: + minItems: 2 + maxItems: 2 + reg-names: + minItems: 2 + maxItems: 2 + required: - compatible - reg @@ -114,3 +142,24 @@ examples: clocks = <&scmi_clk 245>; clock-names = "sw_sdio"; }; + + - | + soc { + #address-cells = <2>; + #size-cells = <2>; + + mmc@fff000 { + compatible = "brcm,bcm2712-sdhci"; + reg = <0x10 0x00fff000 0x0 0x260>, + <0x10 0x00fff400 0x0 0x200>, + <0x10 0x015040b0 0x0 0x4>, // Bus isolation control + <0x10 0x015200f0 0x0 0x24>; // LCPLL control misc0-8 + reg-names = "host", "cfg", "busisol", "lcpll"; + interrupts = <0x0 0x111 0x4>; + clocks = <&clk_emmc2>; + sdhci-caps-mask = <0x0000C000 0x0>; + sdhci-caps = <0x0 0x0>; + mmc-ddr-3_3v; + clock-names = "sw_sdio"; + }; + };
Signed-off-by: Andrea della Porta <andrea.porta@suse.com> --- .../bindings/mmc/brcm,sdhci-brcmstb.yaml | 51 ++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-)