| Message ID | 20231102081514.22945-5-jyanchou@realtek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add DesignWare Mobile mmc driver | expand |
On 02/11/2023 09:15, Jyan Chou wrote: > Document the device-tree bindings for Realtek SoCs mmc driver. > > Signed-off-by: Jyan Chou <jyanchou@realtek.com> > > --- > v4 -> v5: > - Fix compatible to match filename. That's not what I said. Filename must match compatible, not the other way around. > - Remove unused property, e.g.,cqe, resets, clock-freq-min-max. > - Fix indentation. > > v3 -> v4: > - Describe the items to make properties and item easy to understand. > - Fix examples' indentation and compiling error. > - Drop useless properties. > > v2 -> v3: > - Modify dt-bindings' content and description. > - Fix coding style. > - Update the list of maintainers. > > v1 -> v2: > - Add dt-bindings. > --- > .../bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml | 157 ++++++++++++++++++ > 1 file changed, 157 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml > > diff --git a/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml b/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml > new file mode 100644 > index 000000000000..f422a216ff93 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml > @@ -0,0 +1,157 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mmc/realtek,rtd-dw-cqe-emmc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Realtek DesignWare mobile storage host controller > + > +description: > + Realtek uses the Synopsys DesignWare mobile storage host controller > + to interface a SoC with storage medium. This file documents the Realtek > + specific extensions. > + > +maintainers: > + - Jyan Chou <jyanchou@realtek.com> > + > +allOf: > + - $ref: mmc-controller.yaml# > + > +properties: > + compatible: > + enum: > + - realtek,rtd-dw-cqe-emmc I don't understand what happened here. I asked you to drop the incorrect, generic compatible. Instead you dropped specific compatibles and left generic. Nope, this does not work like it. You *must* use specific compatibles. > + > + reg: > + items: > + - description: emmc base address > + - description: cqhci base address > + > + reg-names: > + items: > + - const: emmc > + - const: cqhci > + > + interrupts: > + maxItems: 1 > + > + clocks: > + description: Handles to input clocks Instead: maxItems: 4 > + > + clock-names: > + items: > + - const: biu > + - const: ciu > + - const: vp0 > + - const: vp1 > + > + clock-frequency: > + description: > + Operating frequency of realtek emmc controller clock Drop entire property. There is already max-frequency. > + minimum: 300000 > + maximum: 400000000 > + > + vmmc-supply: > + description: > + Handle to fixed-voltage supply for the card power. Drop entire property. Not needed. > + > + pinctrl-0: > + description: > + should contain default/high speed pin ctrl. > + maxItems: 1 > + > + pinctrl-1: > + description: > + should contain sdr50 mode pin ctrl. > + maxItems: 1 > + > + pinctrl-2: > + description: > + should contain ddr50 mode pin ctrl. > + maxItems: 1 > + > + pinctrl-3: > + description: > + should contain hs200 speed pin ctrl. > + maxItems: 1 > + > + pinctrl-4: > + description: > + should contain hs400 speed pin ctrl. > + maxItems: 1 > + > + pinctrl-5: > + description: > + should contain tune0 pin ctrl. > + maxItems: 1 > + > + pinctrl-6: > + description: > + should contain tune1 pin ctrl. > + maxItems: 1 > + > + pinctrl-7: > + description: > + should contain tune2 pin ctrl. > + maxItems: 1 > + > + pinctrl-8: > + description: > + should contain tune3 pin ctrl. > + maxItems: 1 > + > + pinctrl-9: > + description: > + should contain tune4 pin ctrl. > + maxItems: 1 > + > + pinctrl-names: > + maxItems: 10 > + > +required: > + - compatible > + - reg > + - reg-names > + - interrupts > + - clocks > + - clock-names > + - clock-frequency > + - vmmc-supply > + - pinctrl-names > + - pinctrl-0 > + - pinctrl-1 > + - pinctrl-3 > + - pinctrl-4 > + - pinctrl-5 > + - pinctrl-6 > + - pinctrl-7 > + - pinctrl-8 > + - pinctrl-9 > + > +additionalProperties: false unevaluatedProperties: false > + > +examples: > + - | > + emmc: mmc@12000 { > + compatible = "realtek,rtd-dw-cqe-emmc"; > + reg = <0x00012000 0x00600>, > + <0x00012180 0x00060>; > + reg-names = "emmc", "cqhci"; > + interrupts = <0 42 4>; > + clocks = <&cc 22>, <&cc 26>, <&cc 121>, <&cc 122>; > + clock-names = "biu", "ciu", "vp0", "vp1"; I asked you to test the bindings. This also means that you must test your DTS against bindings. Your bindings, DTS and driver do not match, therefore let's be a bit more clear: NAK, till you upstream your DTS. Best regards, Krzysztof
>> v4 -> v5: >> - Fix compatible to match filename. > That's not what I said. Filename must match compatible, not the other way around. Sorry for misunderstand your advice. We name our filename realtek,rtd-dw-cqe-emmc.yaml and correct our compatible to compatible: enum: - realtek,rtd1325-dw-cqe-emmc - realtek,rtd1319-dw-cqe-emmc - realtek,rtd1315e-dw-cqe-emmc - realtek,rtd1619b-dw-cqe-emmc in new version. >> - Remove unused property, e.g.,cqe, resets, clock-freq-min-max. >> - Fix indentation. >> >> v3 -> v4: >> - Describe the items to make properties and item easy to understand. >> - Fix examples' indentation and compiling error. >> - Drop useless properties. >> >> v2 -> v3: >> - Modify dt-bindings' content and description. >> - Fix coding style. >> - Update the list of maintainers. >> >> v1 -> v2: >> - Add dt-bindings. >> --- >> .../bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml | 157 >> ++++++++++++++++++ >> 1 file changed, 157 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml >> >> diff --git >> a/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml >> b/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml >> new file mode 100644 >> index 000000000000..f422a216ff93 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.ya >> +++ ml >> @@ -0,0 +1,157 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/mmc/realtek,rtd-dw-cqe-emmc.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Realtek DesignWare mobile storage host controller >> + >> +description: >> + Realtek uses the Synopsys DesignWare mobile storage host controller >> + to interface a SoC with storage medium. This file documents the >> +Realtek >> + specific extensions. >> + >> +maintainers: >> + - Jyan Chou <jyanchou@realtek.com> >> + >> +allOf: >> + - $ref: mmc-controller.yaml# >> + >> +properties: >> + compatible: >> + enum: >> + - realtek,rtd-dw-cqe-emmc > I don't understand what happened here. I asked you to drop the incorrect, generic compatible. Instead you dropped specific compatibles and left generic. Nope, this does not work like it. > You *must* use specific compatibles. We change our compatible to be like realtek,rtd1325-dw-cqe-emmc. >> + >> + reg: >> + items: >> + - description: emmc base address >> + - description: cqhci base address >> + >> + reg-names: >> + items: >> + - const: emmc >> + - const: cqhci >> + >> + interrupts: >> + maxItems: 1 >> + >> + clocks: >> + description: Handles to input clocks > Instead: maxItems: 4 Okay. I will add maxItems: 4. >> + >> + clock-names: >> + items: >> + - const: biu >> + - const: ciu >> + - const: vp0 >> + - const: vp1 >> + >> + clock-frequency: >> + description: >> + Operating frequency of realtek emmc controller clock > Drop entire property. There is already max-frequency. We will drop it in our new version. >> + minimum: 300000 >> + maximum: 400000000 >> + >> + vmmc-supply: >> + description: >> + Handle to fixed-voltage supply for the card power. > Drop entire property. Not needed. We will drop it. >> + >> + pinctrl-0: >> + description: >> + should contain default/high speed pin ctrl. >> + maxItems: 1 >> + >> + pinctrl-1: >> + description: >> + should contain sdr50 mode pin ctrl. >> + maxItems: 1 >> + >> + pinctrl-2: >> + description: >> + should contain ddr50 mode pin ctrl. >> + maxItems: 1 >> + >> + pinctrl-3: >> + description: >> + should contain hs200 speed pin ctrl. >> + maxItems: 1 >> + >> + pinctrl-4: >> + description: >> + should contain hs400 speed pin ctrl. >> + maxItems: 1 >> + >> + pinctrl-5: >> + description: >> + should contain tune0 pin ctrl. >> + maxItems: 1 >> + >> + pinctrl-6: >> + description: >> + should contain tune1 pin ctrl. >> + maxItems: 1 >> + >> + pinctrl-7: >> + description: >> + should contain tune2 pin ctrl. >> + maxItems: 1 >> + >> + pinctrl-8: >> + description: >> + should contain tune3 pin ctrl. >> + maxItems: 1 >> + >> + pinctrl-9: >> + description: >> + should contain tune4 pin ctrl. >> + maxItems: 1 >> + >> + pinctrl-names: >> + maxItems: 10 >> + >> +required: >> + - compatible >> + - reg >> + - reg-names >> + - interrupts >> + - clocks >> + - clock-names >> + - clock-frequency >> + - vmmc-supply >> + - pinctrl-names >> + - pinctrl-0 >> + - pinctrl-1 >> + - pinctrl-3 >> + - pinctrl-4 >> + - pinctrl-5 >> + - pinctrl-6 >> + - pinctrl-7 >> + - pinctrl-8 >> + - pinctrl-9 >> + >> +additionalProperties: false > unevaluatedProperties: false I will replace additionalProperties: false with unevaluatedProperties: false. >> + >> +examples: >> + - | >> + emmc: mmc@12000 { >> + compatible = "realtek,rtd-dw-cqe-emmc"; >> + reg = <0x00012000 0x00600>, >> + <0x00012180 0x00060>; >> + reg-names = "emmc", "cqhci"; >> + interrupts = <0 42 4>; >> + clocks = <&cc 22>, <&cc 26>, <&cc 121>, <&cc 122>; >> + clock-names = "biu", "ciu", "vp0", "vp1"; > I asked you to test the bindings. This also means that you must test your DTS against bindings. Your bindings, DTS and driver do not match, therefore let's be a bit more clear: > NAK, till you upstream your DTS. Okay, I will test it and push a correct one again. Thanks. -----Original Message----- From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Sent: Thursday, November 2, 2023 4:54 PM To: Jyan Chou [周芷安] <jyanchou@realtek.com>; ulf.hansson@linaro.org; adrian.hunter@intel.com; jh80.chung@samsung.com; riteshh@codeaurora.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; asutoshd@codeaurora.org Cc: p.zabel@pengutronix.de; linux-mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; arnd@arndb.de; briannorris@chromium.org; doug@schmorgal.com; tonyhuang.sunplus@gmail.com; abel.vesa@linaro.org; william.qiu@starfivetech.com Subject: Re: [PATCH V5][4/4] dt-bindings: mmc: Add dt-bindings for realtek mmc driver External mail. On 02/11/2023 09:15, Jyan Chou wrote: > Document the device-tree bindings for Realtek SoCs mmc driver. > > Signed-off-by: Jyan Chou <jyanchou@realtek.com> > > --- > v4 -> v5: > - Fix compatible to match filename. That's not what I said. Filename must match compatible, not the other way around. > - Remove unused property, e.g.,cqe, resets, clock-freq-min-max. > - Fix indentation. > > v3 -> v4: > - Describe the items to make properties and item easy to understand. > - Fix examples' indentation and compiling error. > - Drop useless properties. > > v2 -> v3: > - Modify dt-bindings' content and description. > - Fix coding style. > - Update the list of maintainers. > > v1 -> v2: > - Add dt-bindings. > --- > .../bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml | 157 > ++++++++++++++++++ > 1 file changed, 157 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml > > diff --git > a/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml > b/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml > new file mode 100644 > index 000000000000..f422a216ff93 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.ya > +++ ml > @@ -0,0 +1,157 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mmc/realtek,rtd-dw-cqe-emmc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Realtek DesignWare mobile storage host controller > + > +description: > + Realtek uses the Synopsys DesignWare mobile storage host controller > + to interface a SoC with storage medium. This file documents the > +Realtek > + specific extensions. > + > +maintainers: > + - Jyan Chou <jyanchou@realtek.com> > + > +allOf: > + - $ref: mmc-controller.yaml# > + > +properties: > + compatible: > + enum: > + - realtek,rtd-dw-cqe-emmc I don't understand what happened here. I asked you to drop the incorrect, generic compatible. Instead you dropped specific compatibles and left generic. Nope, this does not work like it. You *must* use specific compatibles. > + > + reg: > + items: > + - description: emmc base address > + - description: cqhci base address > + > + reg-names: > + items: > + - const: emmc > + - const: cqhci > + > + interrupts: > + maxItems: 1 > + > + clocks: > + description: Handles to input clocks Instead: maxItems: 4 > + > + clock-names: > + items: > + - const: biu > + - const: ciu > + - const: vp0 > + - const: vp1 > + > + clock-frequency: > + description: > + Operating frequency of realtek emmc controller clock Drop entire property. There is already max-frequency. > + minimum: 300000 > + maximum: 400000000 > + > + vmmc-supply: > + description: > + Handle to fixed-voltage supply for the card power. Drop entire property. Not needed. > + > + pinctrl-0: > + description: > + should contain default/high speed pin ctrl. > + maxItems: 1 > + > + pinctrl-1: > + description: > + should contain sdr50 mode pin ctrl. > + maxItems: 1 > + > + pinctrl-2: > + description: > + should contain ddr50 mode pin ctrl. > + maxItems: 1 > + > + pinctrl-3: > + description: > + should contain hs200 speed pin ctrl. > + maxItems: 1 > + > + pinctrl-4: > + description: > + should contain hs400 speed pin ctrl. > + maxItems: 1 > + > + pinctrl-5: > + description: > + should contain tune0 pin ctrl. > + maxItems: 1 > + > + pinctrl-6: > + description: > + should contain tune1 pin ctrl. > + maxItems: 1 > + > + pinctrl-7: > + description: > + should contain tune2 pin ctrl. > + maxItems: 1 > + > + pinctrl-8: > + description: > + should contain tune3 pin ctrl. > + maxItems: 1 > + > + pinctrl-9: > + description: > + should contain tune4 pin ctrl. > + maxItems: 1 > + > + pinctrl-names: > + maxItems: 10 > + > +required: > + - compatible > + - reg > + - reg-names > + - interrupts > + - clocks > + - clock-names > + - clock-frequency > + - vmmc-supply > + - pinctrl-names > + - pinctrl-0 > + - pinctrl-1 > + - pinctrl-3 > + - pinctrl-4 > + - pinctrl-5 > + - pinctrl-6 > + - pinctrl-7 > + - pinctrl-8 > + - pinctrl-9 > + > +additionalProperties: false unevaluatedProperties: false > + > +examples: > + - | > + emmc: mmc@12000 { > + compatible = "realtek,rtd-dw-cqe-emmc"; > + reg = <0x00012000 0x00600>, > + <0x00012180 0x00060>; > + reg-names = "emmc", "cqhci"; > + interrupts = <0 42 4>; > + clocks = <&cc 22>, <&cc 26>, <&cc 121>, <&cc 122>; > + clock-names = "biu", "ciu", "vp0", "vp1"; I asked you to test the bindings. This also means that you must test your DTS against bindings. Your bindings, DTS and driver do not match, therefore let's be a bit more clear: NAK, till you upstream your DTS. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml b/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml new file mode 100644 index 000000000000..f422a216ff93 --- /dev/null +++ b/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml @@ -0,0 +1,157 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mmc/realtek,rtd-dw-cqe-emmc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Realtek DesignWare mobile storage host controller + +description: + Realtek uses the Synopsys DesignWare mobile storage host controller + to interface a SoC with storage medium. This file documents the Realtek + specific extensions. + +maintainers: + - Jyan Chou <jyanchou@realtek.com> + +allOf: + - $ref: mmc-controller.yaml# + +properties: + compatible: + enum: + - realtek,rtd-dw-cqe-emmc + + reg: + items: + - description: emmc base address + - description: cqhci base address + + reg-names: + items: + - const: emmc + - const: cqhci + + interrupts: + maxItems: 1 + + clocks: + description: Handles to input clocks + + clock-names: + items: + - const: biu + - const: ciu + - const: vp0 + - const: vp1 + + clock-frequency: + description: + Operating frequency of realtek emmc controller clock + minimum: 300000 + maximum: 400000000 + + vmmc-supply: + description: + Handle to fixed-voltage supply for the card power. + + pinctrl-0: + description: + should contain default/high speed pin ctrl. + maxItems: 1 + + pinctrl-1: + description: + should contain sdr50 mode pin ctrl. + maxItems: 1 + + pinctrl-2: + description: + should contain ddr50 mode pin ctrl. + maxItems: 1 + + pinctrl-3: + description: + should contain hs200 speed pin ctrl. + maxItems: 1 + + pinctrl-4: + description: + should contain hs400 speed pin ctrl. + maxItems: 1 + + pinctrl-5: + description: + should contain tune0 pin ctrl. + maxItems: 1 + + pinctrl-6: + description: + should contain tune1 pin ctrl. + maxItems: 1 + + pinctrl-7: + description: + should contain tune2 pin ctrl. + maxItems: 1 + + pinctrl-8: + description: + should contain tune3 pin ctrl. + maxItems: 1 + + pinctrl-9: + description: + should contain tune4 pin ctrl. + maxItems: 1 + + pinctrl-names: + maxItems: 10 + +required: + - compatible + - reg + - reg-names + - interrupts + - clocks + - clock-names + - clock-frequency + - vmmc-supply + - pinctrl-names + - pinctrl-0 + - pinctrl-1 + - pinctrl-3 + - pinctrl-4 + - pinctrl-5 + - pinctrl-6 + - pinctrl-7 + - pinctrl-8 + - pinctrl-9 + +additionalProperties: false + +examples: + - | + emmc: mmc@12000 { + compatible = "realtek,rtd-dw-cqe-emmc"; + reg = <0x00012000 0x00600>, + <0x00012180 0x00060>; + reg-names = "emmc", "cqhci"; + interrupts = <0 42 4>; + clocks = <&cc 22>, <&cc 26>, <&cc 121>, <&cc 122>; + clock-names = "biu", "ciu", "vp0", "vp1"; + clock-frequency = <400000>; + vmmc-supply = <®_vcc1v8>; + pinctrl-names = "default", "sdr50", "ddr50", "hs200", "hs400", + "tune0","tune1", "tune2","tune3", "tune4"; + pinctrl-0 = <&emmc_pins_sdr50>; + pinctrl-1 = <&emmc_pins_sdr50>; + pinctrl-2 = <&emmc_pins_ddr50>; + pinctrl-3 = <&emmc_pins_hs200>; + pinctrl-4 = <&emmc_pins_hs400>; + pinctrl-5 = <&emmc_pins_tune0>; + pinctrl-6 = <&emmc_pins_tune1>; + pinctrl-7 = <&emmc_pins_tune2>; + pinctrl-8 = <&emmc_pins_tune3>; + pinctrl-9 = <&emmc_pins_tune4>; + };
Document the device-tree bindings for Realtek SoCs mmc driver. Signed-off-by: Jyan Chou <jyanchou@realtek.com> --- v4 -> v5: - Fix compatible to match filename. - Remove unused property, e.g.,cqe, resets, clock-freq-min-max. - Fix indentation. v3 -> v4: - Describe the items to make properties and item easy to understand. - Fix examples' indentation and compiling error. - Drop useless properties. v2 -> v3: - Modify dt-bindings' content and description. - Fix coding style. - Update the list of maintainers. v1 -> v2: - Add dt-bindings. --- .../bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml | 157 ++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100644 Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml