| Message ID | 20240619054641.277062-1-shanchun1218@gmail.com |
|---|---|
| Headers | show |
| Series | Add support for Nuvovon MA35D1 SDHCI | expand |
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.
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
From: schung <schung@nuvoton.com> This patch series adds the SDHCI driver for the Nuvoton MA35D1 series platform. It includes DT binding documentation, the MA35D1 SDHCI driver. This MA35D1 SDHCI driver has been tested on the MA35D1 SOM board with Linux 6.10 Shan-Chun Hung (2): dt-bindings: mmc: nuvoton,ma35d1-sdhci: Document MA35D1 SDHCI controller mmc: sdhci-of-ma35d1: Add Novoton MA35D1 SDHCI driver .../bindings/mmc/nuvoton,ma35d1-sdhci.yaml | 106 +++++++ drivers/mmc/host/Kconfig | 13 + drivers/mmc/host/Makefile | 1 + drivers/mmc/host/sdhci-of-ma35d1.c | 297 ++++++++++++++++++ 4 files changed, 417 insertions(+) create mode 100644 Documentation/devicetree/bindings/mmc/nuvoton,ma35d1-sdhci.yaml create mode 100644 drivers/mmc/host/sdhci-of-ma35d1.c -- 2.25.1