Add binding for Nuvoton MA35D1 SDHCI controller.
Signed-off-by: schung <schung@nuvoton.com>
---
.../bindings/mmc/nuvoton,ma35d1-sdhci.yaml | 106 ++++++++++++++++++
1 file changed, 106 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.yaml
diff --git a/Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.yaml b/Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.yaml
new file mode 100644
index 000000000000..173449360dea
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.yaml
@@ -0,0 +1,106 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/nuvoton,ma35d1-sdhci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton MA35D1 SD/SDIO/MMC Controller
+
+maintainers:
+ - Shan-Chun Hung <shanchun1218@gmail.com>
+
+description: |
+ This controller on Nuvoton MA35 family SoCs provides an interface for MMC, SD, and
+ SDIO types of memory cards.
+
+properties:
+ compatible:
+ oneOf:
+ - enum:
+ - nuvoton,ma35d1-sdhci
+ reg:
+ maxItems: 1
+ description: The SDHCI registers
+
+ interrupts:
+ maxItems: 1
+
+ pinctrl-names:
+ description:
+ Should at least contain default and state_uhs.
+ minItems: 1
+ items:
+ - const: default
+ - const: state_uhs
+
+ pinctrl-0:
+ description:
+ Should contain default/high speed pin ctrl.
+ maxItems: 1
+
+ pinctrl-1:
+ description:
+ Should contain uhs mode pin ctrl.
+ maxItems: 1
+
+ clocks:
+ minItems: 1
+ description: The SDHCI bus clock
+
+ resets:
+ maxItems: 1
+ description:
+ Phandle and reset specifier pair to softreset line of sdhci.
+
+ nuvoton,sys:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ Phandle to the syscon that configure sdhci votage stable
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - pinctrl-names
+ - pinctrl-0
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
+ #include <dt-bindings/reset/nuvoton,ma35d1-reset.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ sdhci0: sdhci@40180000 {
+ compatible = "nuvoton,ma35d1-sdhci";
+ reg = <0x0 0x40180000 0x0 0x2000>;
+ interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk SDH0_GATE>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_sdhci0>;
+ bus-width = <4>;
+ max-frequency = <100000000>;
+ no-1-8-v;
+ status = "disabled";
+ };
+
+ sdhci1: sdhci@40190000 {
+ compatible = "nuvoton,ma35d1-sdhci";
+ reg = <0x0 0x40190000 0x0 0x2000>;
+ interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk SDH1_GATE>;
+ pinctrl-names = "default", "state_uhs";
+ pinctrl-0 = <&pinctrl_sdhci1>;
+ pinctrl-1 = <&pinctrl_sdhci1_uhs>;
+ resets = <&sys MA35D1_RESET_SDH1>;
+ nuvoton,sys = <&sys>;
+ bus-width = <8>;
+ max-frequency = <200000000>;
+ status = "disabled";
+ };
+ };
--
2.25.1
On 19/06/2024 07:46, Shan-Chun Hung wrote:
> Add binding for Nuvoton MA35D1 SDHCI controller.
>
> Signed-off-by: schung <schung@nuvoton.com>
Since this was not tested, only limited review follows. Please test your
future patches.
> ---
> .../bindings/mmc/nuvoton,ma35d1-sdhci.yaml | 106 ++++++++++++++++++
> 1 file changed, 106 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.yaml
>
> diff --git a/Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.yaml b/Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.yaml
> new file mode 100644
> index 000000000000..173449360dea
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.yaml
> @@ -0,0 +1,106 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mmc/nuvoton,ma35d1-sdhci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nuvoton MA35D1 SD/SDIO/MMC Controller
> +
> +maintainers:
> + - Shan-Chun Hung <shanchun1218@gmail.com>
> +
> +description: |
Do not need '|' unless you need to preserve formatting.
> + This controller on Nuvoton MA35 family SoCs provides an interface for MMC, SD, and
> + SDIO types of memory cards.
> +
> +properties:
> + compatible:
> + oneOf:
Drop
> + - enum:
> + - nuvoton,ma35d1-sdhci
Blank line
> + reg:
> + maxItems: 1
> + description: The SDHCI registers
Drop
> +
> + interrupts:
> + maxItems: 1
> +
> + pinctrl-names:
> + description:
> + Should at least contain default and state_uhs.
? Contradicts constraints.
> + minItems: 1
> + items:
> + - const: default
> + - const: state_uhs
> +
> + pinctrl-0:
> + description:
> + Should contain default/high speed pin ctrl.
> + maxItems: 1
> +
> + pinctrl-1:
> + description:
> + Should contain uhs mode pin ctrl.
> + maxItems: 1
> +
> + clocks:
> + minItems: 1
No, maxItems instead.
> + description: The SDHCI bus clock
Drop
> +
> + resets:
> + maxItems: 1
> + description:
> + Phandle and reset specifier pair to softreset line of sdhci.
Drop
> +
> + nuvoton,sys:
1. too generic, what is sys?
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to the syscon that configure sdhci votage stable
2. typo: voltage
3. Which syscon?
4. Why you are not implementing regulators?
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - pinctrl-names
> + - pinctrl-0
> +
> +unevaluatedProperties: false
Hm? And where is ref to MMC schema?
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
> + #include <dt-bindings/reset/nuvoton,ma35d1-reset.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
Fix your indentation.
Use 4 spaces for example indentation.
> + sdhci0: sdhci@40180000 {
Drop label
> + compatible = "nuvoton,ma35d1-sdhci";
> + reg = <0x0 0x40180000 0x0 0x2000>;
> + interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk SDH0_GATE>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_sdhci0>;
> + bus-width = <4>;
> + max-frequency = <100000000>;
> + no-1-8-v;
> + status = "disabled";
Drop
> + };
> +
> + sdhci1: sdhci@40190000 {
Drop this example. One is enough.
Best regards,
Krzysztof
Dear Krzysztof,
Thanks for you review
On 2024/6/19 下午 03:29, Krzysztof Kozlowski wrote:
> On 19/06/2024 07:46, Shan-Chun Hung wrote:
>> Add binding for Nuvoton MA35D1 SDHCI controller.
>>
>> Signed-off-by: schung<schung@nuvoton.com>
> Since this was not tested, only limited review follows. Please test your
> future patches.
>
>> ---
>> .../bindings/mmc/nuvoton,ma35d1-sdhci.yaml | 106 ++++++++++++++++++
>> 1 file changed, 106 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.yaml b/Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.yaml
>> new file mode 100644
>> index 000000000000..173449360dea
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.yaml
>> @@ -0,0 +1,106 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id:http://devicetree.org/schemas/mmc/nuvoton,ma35d1-sdhci.yaml#
>> +$schema:http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Nuvoton MA35D1 SD/SDIO/MMC Controller
>> +
>> +maintainers:
>> + - Shan-Chun Hung<shanchun1218@gmail.com>
>> +
>> +description: |
> Do not need '|' unless you need to preserve formatting.
I will remove '|'
>> + This controller on Nuvoton MA35 family SoCs provides an interface for MMC, SD, and
>> + SDIO types of memory cards.
>> +
>> +properties:
>> + compatible:
>> + oneOf:
> Drop
I will remove oneof.
>> + - enum:
>> + - nuvoton,ma35d1-sdhci
> Blank line
I will fix it.
>> + reg:
>> + maxItems: 1
>> + description: The SDHCI registers
> Drop
I will remove description.
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + pinctrl-names:
>> + description:
>> + Should at least contain default and state_uhs.
> ? Contradicts constraints.
I will modify the description to "Should at least contain default.
state_uhs is mandatory in this scenario."
>> + minItems: 1
>> + items:
>> + - const: default
>> + - const: state_uhs
>> +
>> + pinctrl-0:
>> + description:
>> + Should contain default/high speed pin ctrl.
>> + maxItems: 1
>> +
>> + pinctrl-1:
>> + description:
>> + Should contain uhs mode pin ctrl.
>> + maxItems: 1
>> +
>> + clocks:
>> + minItems: 1
> No, maxItems instead.
I will fix it.
>> + description: The SDHCI bus clock
> Drop
I will remove it.
>> +
>> + resets:
>> + maxItems: 1
>> + description:
>> + Phandle and reset specifier pair to softreset line of sdhci.
> Drop
I will remove it.
>> +
>> + nuvoton,sys:
> 1. too generic, what is sys?
>
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> + description:
>> + Phandle to the syscon that configure sdhci votage stable
I will modify description as follows:
nuvoton,sys:
$ref: /schemas/types.yaml#/definitions/phandle
description: phandle to access GCR (Global Control Register) registers.
>> 2. typo: voltage
I will fix it.
>> 3. Which syscon?
same as 1.
>> 4. Why you are not implementing regulators?
>>
I will add "vqmmc-supply = <&sdhci1_vqmmc_regulator>;" in the examples.
This requlator is used by regulator-gpio driver.
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> + - clocks
>> + - pinctrl-names
>> + - pinctrl-0
>> +
>> +unevaluatedProperties: false
> Hm? And where is ref to MMC schema?
I will add as follows:
allOf:
- $ref: sdhci-common.yaml#
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
>> + #include <dt-bindings/reset/nuvoton,ma35d1-reset.h>
>> +
>> + soc {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
> Fix your indentation.
>
> Use 4 spaces for example indentation.
I will fix it.
>> + sdhci0: sdhci@40180000 {
> Drop label
I will fix it.
>> + compatible = "nuvoton,ma35d1-sdhci";
>> + reg = <0x0 0x40180000 0x0 0x2000>;
>> + interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
>> + clocks = <&clk SDH0_GATE>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_sdhci0>;
>> + bus-width = <4>;
>> + max-frequency = <100000000>;
>> + no-1-8-v;
>> + status = "disabled";
> Drop
I will fix it.
>> + };
>> +
>> + sdhci1: sdhci@40190000 {
> Drop this example. One is enough.
>
>
>
> Best regards,
> Krzysztof
OK , I will remove one.
Best Regards
Shan-Chun
On 21/06/2024 01:53, Shan-Chun Hung wrote: >>> + >>> + interrupts: >>> + maxItems: 1 >>> + >>> + pinctrl-names: >>> + description: >>> + Should at least contain default and state_uhs. >> ? Contradicts constraints. > I will modify the description to "Should at least contain default. > state_uhs is mandatory in this scenario." Then just drop it. Don't repeat constraints in free form text. Best regards, Krzysztof
Dear Krzysztof, Thanks for your review. On 2024/6/21 下午 02:04, Krzysztof Kozlowski wrote: > On 21/06/2024 01:53, Shan-Chun Hung wrote: >>>> + >>>> + interrupts: >>>> + maxItems: 1 >>>> + >>>> + pinctrl-names: >>>> + description: >>>> + Should at least contain default and state_uhs. >>> ? Contradicts constraints. >> I will modify the description to "Should at least contain default. >> state_uhs is mandatory in this scenario." > Then just drop it. Don't repeat constraints in free form text. > > > Best regards, > Krzysztof OK , I will drop the description. Best Regards, Shan-Chun
On Wed, 19 Jun 2024 13:46:40 +0800, Shan-Chun Hung wrote: > Add binding for Nuvoton MA35D1 SDHCI controller. > > Signed-off-by: schung <schung@nuvoton.com> > --- > .../bindings/mmc/nuvoton,ma35d1-sdhci.yaml | 106 ++++++++++++++++++ > 1 file changed, 106 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: ./Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.yaml:77:1: [error] syntax error: found character '\t' that cannot start any token (syntax) dtschema/dtc warnings/errors: make[2]: *** Deleting file 'Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.example.dts' Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.yaml:77:1: found a tab character where an indentation space is expected make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.example.dts] Error 1 make[2]: *** Waiting for unfinished jobs.... ./Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.yaml:77:1: found a tab character where an indentation space is expected /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.yaml: ignoring, error parsing file make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1430: dt_binding_check] Error 2 make: *** [Makefile:240: __sub-make] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240619054641.277062-2-shanchun1218@gmail.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.
© 2016 - 2026 Red Hat, Inc.