| Message ID | 20210503014336.20256-2-steven_lee@aspeedtech.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/3] dt-bindings: mmc: sdhci-of-aspeed: Add description for AST2600 EVB. | expand |
Hi Steven, On Mon, 3 May 2021, at 11:13, Steven Lee wrote: > Add the description for describing the AST 2600 EVB reference design of > GPIO regulators and provide the example in the document. > > AST2600-A2 EVB has the reference design for enabling SD bus > power and toggling SD bus signal voltage by GPIO pins. > > In the reference design, GPIOV0 of AST2600-A2 EVB is connected to > power load switch that providing 3.3v to SD1 bus vdd. GPIOV1 is > connected to a 1.8v and a 3.3v power load switch that providing > signal voltage to > SD1 bus. > > If GPIOV0 is active high, SD1 bus is enabled. Otherwise, SD1 bus is > disabled. > If GPIOV1 is active high, 3.3v power load switch is enabled, SD1 > signal voltage is 3.3v. Otherwise, 1.8v power load switch will be > enabled, SD1 signal voltage becomes 1.8v. > > AST2600-A2 EVB also support toggling signal voltage for SD2 bus. > The design is the same as SD1 bus. It uses GPIOV2 as power-gpio and > GPIOV3 as power-switch-gpio. > > Signed-off-by: Steven Lee <steven_lee@aspeedtech.com> > --- > .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 99 +++++++++++++++++++ > 1 file changed, 99 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > index 987b287f3bff..dd894aba0bb7 100644 > --- a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > @@ -20,6 +20,19 @@ description: |+ > the slots are dependent on the common configuration area, they are > described > as child nodes. > > + The signal voltage of SDHCIs on AST2600-A2 EVB is able to be toggled > by GPIO > + pins. In the reference design, GPIOV0 of AST2600-A2 EVB is connected > to the > + power load switch that providing 3.3v to SD1 bus vdd, GPIOV1 is > connected to > + a 1.8v and a 3.3v power load switch that providing signal voltage to > + SD1 bus. > + If GPIOV0 is active high, SD1 bus is enabled. Otherwise, SD1 bus is > + disabled. If GPIOV1 is active high, 3.3v power load switch is > enabled, SD1 > + signal voltage is 3.3v. Otherwise, 1.8v power load switch will be > enabled, SD1 > + signal voltage becomes 1.8v. > + AST2600-A2 EVB also support toggling signal voltage for SD2 bus. > + The design is the same as SD1 bus. It uses GPIOV2 as power-gpio and > GPIOV3 > + as power-switch-gpio. I don't think we should be describing design-specific details in the binding document. However, I think this would be a great comment in the AST2600 EVB devicetree. Can you please move it there? > + > properties: > compatible: > enum: > @@ -78,6 +91,7 @@ required: > - clocks > > examples: > + //Example 1 > - | > #include <dt-bindings/clock/aspeed-clock.h> > sdc@1e740000 { > @@ -104,3 +118,88 @@ examples: > clocks = <&syscon ASPEED_CLK_SDIO>; > }; > }; > + > + //Example 2 (AST2600EVB with GPIO regulator) I feel you didn't test this with `make dt_binding_check` as `//` isn't a valid YAML comment token. You need to use `#` for comments ( https://yaml.org/spec/1.2/spec.html#id2780069 ). > + - | > + #include <dt-bindings/clock/aspeed-clock.h> > + #include <dt-bindings/gpio/aspeed-gpio.h> > + vcc_sdhci0: regulator-vcc-sdhci0 { > + compatible = "regulator-fixed"; > + > + regulator-name = "SDHCI0 Vcc"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + gpios = <&gpio0 ASPEED_GPIO(V, 0) > + GPIO_ACTIVE_HIGH>; > + enable-active-high; > + }; > + > + vccq_sdhci0: regulator-vccq-sdhci0 { > + compatible = "regulator-gpio"; > + > + regulator-name = "SDHCI0 VccQ"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + gpios = <&gpio0 ASPEED_GPIO(V, 1) > + GPIO_ACTIVE_HIGH>; > + gpios-states = <1>; > + states = <3300000 1 > + 1800000 0>; > + }; > + > + vcc_sdhci1: regulator-vcc-sdhci1 { > + compatible = "regulator-fixed"; > + > + regulator-name = "SDHCI1 Vcc"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + gpios = <&gpio0 ASPEED_GPIO(V, 2) > + GPIO_ACTIVE_HIGH>; > + enable-active-high; > + }; > + > + vccq_sdhci1: regulator-vccq-sdhci1 { > + compatible = "regulator-gpio"; > + > + regulator-name = "SDHCI1 VccQ"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + gpios = <&gpio0 ASPEED_GPIO(V, 3) > + GPIO_ACTIVE_HIGH>; > + gpios-states = <1>; > + states = <3300000 1 > + 1800000 0>; > + }; > + > + sdc@1e740000 { > + compatible = "aspeed,ast2600-sd-controller"; > + reg = <0x1e740000 0x100>; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0 0x1e740000 0x20000>; > + clocks = <&syscon ASPEED_CLK_GATE_SDCLK>; > + > + sdhci0: sdhci@100 { > + compatible = "aspeed,ast2600-sdhci", "sdhci"; > + reg = <0x100 0x100>; > + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>; > + sdhci,auto-cmd12; > + clocks = <&syscon ASPEED_CLK_SDIO>; > + vmmc-supply = <&vcc_sdhci0>; > + vqmmc-supply = <&vccq_sdhci0>; > + sd-uhs-sdr104; > + clk-phase-uhs-sdr104 = <180>, <180>; > + }; > + > + sdhci1: sdhci@200 { > + compatible = "aspeed,ast2600-sdhci", "sdhci"; > + reg = <0x200 0x100>; > + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>; > + sdhci,auto-cmd12; > + clocks = <&syscon ASPEED_CLK_SDIO>; > + vmmc-supply = <&vcc_sdhci1>; > + vqmmc-supply = <&vccq_sdhci1>; > + sd-uhs-sdr104; > + clk-phase-uhs-sdr104 = <0>, <0>; > + }; > + }; This is a good example, so can we keep this and just drop the comment from the binding document? Cheers, Andrew
The 05/03/2021 12:19, Andrew Jeffery wrote: > Hi Steven, > > On Mon, 3 May 2021, at 11:13, Steven Lee wrote: > > Add the description for describing the AST 2600 EVB reference design of > > GPIO regulators and provide the example in the document. > > > > AST2600-A2 EVB has the reference design for enabling SD bus > > power and toggling SD bus signal voltage by GPIO pins. > > > > In the reference design, GPIOV0 of AST2600-A2 EVB is connected to > > power load switch that providing 3.3v to SD1 bus vdd. GPIOV1 is > > connected to a 1.8v and a 3.3v power load switch that providing > > signal voltage to > > SD1 bus. > > > > If GPIOV0 is active high, SD1 bus is enabled. Otherwise, SD1 bus is > > disabled. > > If GPIOV1 is active high, 3.3v power load switch is enabled, SD1 > > signal voltage is 3.3v. Otherwise, 1.8v power load switch will be > > enabled, SD1 signal voltage becomes 1.8v. > > > > AST2600-A2 EVB also support toggling signal voltage for SD2 bus. > > The design is the same as SD1 bus. It uses GPIOV2 as power-gpio and > > GPIOV3 as power-switch-gpio. > > > > Signed-off-by: Steven Lee <steven_lee@aspeedtech.com> > > --- > > .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 99 +++++++++++++++++++ > > 1 file changed, 99 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > index 987b287f3bff..dd894aba0bb7 100644 > > --- a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > @@ -20,6 +20,19 @@ description: |+ > > the slots are dependent on the common configuration area, they are > > described > > as child nodes. > > > > + The signal voltage of SDHCIs on AST2600-A2 EVB is able to be toggled > > by GPIO > > + pins. In the reference design, GPIOV0 of AST2600-A2 EVB is connected > > to the > > + power load switch that providing 3.3v to SD1 bus vdd, GPIOV1 is > > connected to > > + a 1.8v and a 3.3v power load switch that providing signal voltage to > > + SD1 bus. > > + If GPIOV0 is active high, SD1 bus is enabled. Otherwise, SD1 bus is > > + disabled. If GPIOV1 is active high, 3.3v power load switch is > > enabled, SD1 > > + signal voltage is 3.3v. Otherwise, 1.8v power load switch will be > > enabled, SD1 > > + signal voltage becomes 1.8v. > > + AST2600-A2 EVB also support toggling signal voltage for SD2 bus. > > + The design is the same as SD1 bus. It uses GPIOV2 as power-gpio and > > GPIOV3 > > + as power-switch-gpio. > > I don't think we should be describing design-specific details in the > binding document. However, I think this would be a great comment in the > AST2600 EVB devicetree. Can you please move it there? > Ok, I will move it to the device tree. I was wondering if the following place is a good place to put the comment at line 534 of aspeed-g6.dtsi sdc: sdc@1e740000 { // Comment here... compatible = "aspeed,ast2600-sd-controller"; reg = <0x1e740000 0x100>; sdhci0: sdhci@1e740100 { compatible = "aspeed,ast2600-sdhci", "sdhci"; reg = <0x100 0x100>; interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>; ... } > > + > > properties: > > compatible: > > enum: > > @@ -78,6 +91,7 @@ required: > > - clocks > > > > examples: > > + //Example 1 > > - | > > #include <dt-bindings/clock/aspeed-clock.h> > > sdc@1e740000 { > > @@ -104,3 +118,88 @@ examples: > > clocks = <&syscon ASPEED_CLK_SDIO>; > > }; > > }; > > + > > + //Example 2 (AST2600EVB with GPIO regulator) > > I feel you didn't test this with `make dt_binding_check` as `//` isn't > a valid YAML comment token. You need to use `#` for comments ( > https://yaml.org/spec/1.2/spec.html#id2780069 ). > Sorry, I don't know that there is a binding check command for valiating YAML document. Regardless, thanks for the reference link. I will test with dt_binding_check. > > + - | > > + #include <dt-bindings/clock/aspeed-clock.h> > > + #include <dt-bindings/gpio/aspeed-gpio.h> > > + vcc_sdhci0: regulator-vcc-sdhci0 { > > + compatible = "regulator-fixed"; > > + > > + regulator-name = "SDHCI0 Vcc"; > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + gpios = <&gpio0 ASPEED_GPIO(V, 0) > > + GPIO_ACTIVE_HIGH>; > > + enable-active-high; > > + }; > > + > > + vccq_sdhci0: regulator-vccq-sdhci0 { > > + compatible = "regulator-gpio"; > > + > > + regulator-name = "SDHCI0 VccQ"; > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <3300000>; > > + gpios = <&gpio0 ASPEED_GPIO(V, 1) > > + GPIO_ACTIVE_HIGH>; > > + gpios-states = <1>; > > + states = <3300000 1 > > + 1800000 0>; > > + }; > > + > > + vcc_sdhci1: regulator-vcc-sdhci1 { > > + compatible = "regulator-fixed"; > > + > > + regulator-name = "SDHCI1 Vcc"; > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + gpios = <&gpio0 ASPEED_GPIO(V, 2) > > + GPIO_ACTIVE_HIGH>; > > + enable-active-high; > > + }; > > + > > + vccq_sdhci1: regulator-vccq-sdhci1 { > > + compatible = "regulator-gpio"; > > + > > + regulator-name = "SDHCI1 VccQ"; > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <3300000>; > > + gpios = <&gpio0 ASPEED_GPIO(V, 3) > > + GPIO_ACTIVE_HIGH>; > > + gpios-states = <1>; > > + states = <3300000 1 > > + 1800000 0>; > > + }; > > + > > + sdc@1e740000 { > > + compatible = "aspeed,ast2600-sd-controller"; > > + reg = <0x1e740000 0x100>; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + ranges = <0 0x1e740000 0x20000>; > > + clocks = <&syscon ASPEED_CLK_GATE_SDCLK>; > > + > > + sdhci0: sdhci@100 { > > + compatible = "aspeed,ast2600-sdhci", "sdhci"; > > + reg = <0x100 0x100>; > > + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>; > > + sdhci,auto-cmd12; > > + clocks = <&syscon ASPEED_CLK_SDIO>; > > + vmmc-supply = <&vcc_sdhci0>; > > + vqmmc-supply = <&vccq_sdhci0>; > > + sd-uhs-sdr104; > > + clk-phase-uhs-sdr104 = <180>, <180>; > > + }; > > + > > + sdhci1: sdhci@200 { > > + compatible = "aspeed,ast2600-sdhci", "sdhci"; > > + reg = <0x200 0x100>; > > + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>; > > + sdhci,auto-cmd12; > > + clocks = <&syscon ASPEED_CLK_SDIO>; > > + vmmc-supply = <&vcc_sdhci1>; > > + vqmmc-supply = <&vccq_sdhci1>; > > + sd-uhs-sdr104; > > + clk-phase-uhs-sdr104 = <0>, <0>; > > + }; > > + }; > > This is a good example, so can we keep this and just drop the comment > from the binding document? Ok, I will remove the comment. > > Cheers, > > Andrew
On Mon, 3 May 2021, at 19:10, Steven Lee wrote: > The 05/03/2021 12:19, Andrew Jeffery wrote: > > Hi Steven, > > > > On Mon, 3 May 2021, at 11:13, Steven Lee wrote: > > > Add the description for describing the AST 2600 EVB reference design of > > > GPIO regulators and provide the example in the document. > > > > > > AST2600-A2 EVB has the reference design for enabling SD bus > > > power and toggling SD bus signal voltage by GPIO pins. > > > > > > In the reference design, GPIOV0 of AST2600-A2 EVB is connected to > > > power load switch that providing 3.3v to SD1 bus vdd. GPIOV1 is > > > connected to a 1.8v and a 3.3v power load switch that providing > > > signal voltage to > > > SD1 bus. > > > > > > If GPIOV0 is active high, SD1 bus is enabled. Otherwise, SD1 bus is > > > disabled. > > > If GPIOV1 is active high, 3.3v power load switch is enabled, SD1 > > > signal voltage is 3.3v. Otherwise, 1.8v power load switch will be > > > enabled, SD1 signal voltage becomes 1.8v. > > > > > > AST2600-A2 EVB also support toggling signal voltage for SD2 bus. > > > The design is the same as SD1 bus. It uses GPIOV2 as power-gpio and > > > GPIOV3 as power-switch-gpio. > > > > > > Signed-off-by: Steven Lee <steven_lee@aspeedtech.com> > > > --- > > > .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 99 +++++++++++++++++++ > > > 1 file changed, 99 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > index 987b287f3bff..dd894aba0bb7 100644 > > > --- a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > > > @@ -20,6 +20,19 @@ description: |+ > > > the slots are dependent on the common configuration area, they are > > > described > > > as child nodes. > > > > > > + The signal voltage of SDHCIs on AST2600-A2 EVB is able to be toggled > > > by GPIO > > > + pins. In the reference design, GPIOV0 of AST2600-A2 EVB is connected > > > to the > > > + power load switch that providing 3.3v to SD1 bus vdd, GPIOV1 is > > > connected to > > > + a 1.8v and a 3.3v power load switch that providing signal voltage to > > > + SD1 bus. > > > + If GPIOV0 is active high, SD1 bus is enabled. Otherwise, SD1 bus is > > > + disabled. If GPIOV1 is active high, 3.3v power load switch is > > > enabled, SD1 > > > + signal voltage is 3.3v. Otherwise, 1.8v power load switch will be > > > enabled, SD1 > > > + signal voltage becomes 1.8v. > > > + AST2600-A2 EVB also support toggling signal voltage for SD2 bus. > > > + The design is the same as SD1 bus. It uses GPIOV2 as power-gpio and > > > GPIOV3 > > > + as power-switch-gpio. > > > > I don't think we should be describing design-specific details in the > > binding document. However, I think this would be a great comment in the > > AST2600 EVB devicetree. Can you please move it there? > > > > Ok, I will move it to the device tree. > > I was wondering if the following place is a good place to put the > comment > > at line 534 of aspeed-g6.dtsi What you're describing is specific to the AST2600 EVB, so I suggest you put it in the EVB dts, e.g. at: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/aspeed-ast2600-evb.dts#n103 > sdc: sdc@1e740000 { > // Comment here... > > compatible = "aspeed,ast2600-sd-controller"; > reg = <0x1e740000 0x100>; > > sdhci0: sdhci@1e740100 { > compatible = "aspeed,ast2600-sdhci", "sdhci"; > reg = <0x100 0x100>; > interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>; > ... > } > > > > + > > > properties: > > > compatible: > > > enum: > > > @@ -78,6 +91,7 @@ required: > > > - clocks > > > > > > examples: > > > + //Example 1 > > > - | > > > #include <dt-bindings/clock/aspeed-clock.h> > > > sdc@1e740000 { > > > @@ -104,3 +118,88 @@ examples: > > > clocks = <&syscon ASPEED_CLK_SDIO>; > > > }; > > > }; > > > + > > > + //Example 2 (AST2600EVB with GPIO regulator) > > > > I feel you didn't test this with `make dt_binding_check` as `//` isn't > > a valid YAML comment token. You need to use `#` for comments ( > > https://yaml.org/spec/1.2/spec.html#id2780069 ). > > > > Sorry, I don't know that there is a binding check command for valiating > YAML document. No worries! There's also `make dtbs_check` to validate the devicetree files against the bindings. It's useful to run both, as usually when you're adding to the binding you're modifying a devicetree as well. Unfortunately we need to do a bit of a cleanup of the Aspeed dts files, they generate a number of warnings right now. > Regardless, thanks for the reference link. > I will test with dt_binding_check. > > > > + - | > > > + #include <dt-bindings/clock/aspeed-clock.h> > > > + #include <dt-bindings/gpio/aspeed-gpio.h> > > > + vcc_sdhci0: regulator-vcc-sdhci0 { > > > + compatible = "regulator-fixed"; > > > + > > > + regulator-name = "SDHCI0 Vcc"; > > > + regulator-min-microvolt = <3300000>; > > > + regulator-max-microvolt = <3300000>; > > > + gpios = <&gpio0 ASPEED_GPIO(V, 0) > > > + GPIO_ACTIVE_HIGH>; > > > + enable-active-high; > > > + }; > > > + > > > + vccq_sdhci0: regulator-vccq-sdhci0 { > > > + compatible = "regulator-gpio"; > > > + > > > + regulator-name = "SDHCI0 VccQ"; > > > + regulator-min-microvolt = <1800000>; > > > + regulator-max-microvolt = <3300000>; > > > + gpios = <&gpio0 ASPEED_GPIO(V, 1) > > > + GPIO_ACTIVE_HIGH>; > > > + gpios-states = <1>; > > > + states = <3300000 1 > > > + 1800000 0>; > > > + }; > > > + > > > + vcc_sdhci1: regulator-vcc-sdhci1 { > > > + compatible = "regulator-fixed"; > > > + > > > + regulator-name = "SDHCI1 Vcc"; > > > + regulator-min-microvolt = <3300000>; > > > + regulator-max-microvolt = <3300000>; > > > + gpios = <&gpio0 ASPEED_GPIO(V, 2) > > > + GPIO_ACTIVE_HIGH>; > > > + enable-active-high; > > > + }; > > > + > > > + vccq_sdhci1: regulator-vccq-sdhci1 { > > > + compatible = "regulator-gpio"; > > > + > > > + regulator-name = "SDHCI1 VccQ"; > > > + regulator-min-microvolt = <1800000>; > > > + regulator-max-microvolt = <3300000>; > > > + gpios = <&gpio0 ASPEED_GPIO(V, 3) > > > + GPIO_ACTIVE_HIGH>; > > > + gpios-states = <1>; > > > + states = <3300000 1 > > > + 1800000 0>; > > > + }; > > > + > > > + sdc@1e740000 { > > > + compatible = "aspeed,ast2600-sd-controller"; > > > + reg = <0x1e740000 0x100>; > > > + #address-cells = <1>; > > > + #size-cells = <1>; > > > + ranges = <0 0x1e740000 0x20000>; > > > + clocks = <&syscon ASPEED_CLK_GATE_SDCLK>; > > > + > > > + sdhci0: sdhci@100 { > > > + compatible = "aspeed,ast2600-sdhci", "sdhci"; > > > + reg = <0x100 0x100>; > > > + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>; > > > + sdhci,auto-cmd12; > > > + clocks = <&syscon ASPEED_CLK_SDIO>; > > > + vmmc-supply = <&vcc_sdhci0>; > > > + vqmmc-supply = <&vccq_sdhci0>; > > > + sd-uhs-sdr104; > > > + clk-phase-uhs-sdr104 = <180>, <180>; > > > + }; > > > + > > > + sdhci1: sdhci@200 { > > > + compatible = "aspeed,ast2600-sdhci", "sdhci"; > > > + reg = <0x200 0x100>; > > > + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>; > > > + sdhci,auto-cmd12; > > > + clocks = <&syscon ASPEED_CLK_SDIO>; > > > + vmmc-supply = <&vcc_sdhci1>; > > > + vqmmc-supply = <&vccq_sdhci1>; > > > + sd-uhs-sdr104; > > > + clk-phase-uhs-sdr104 = <0>, <0>; > > > + }; > > > + }; > > > > This is a good example, so can we keep this and just drop the comment > > from the binding document? > > Ok, I will remove the comment. Thanks. Andrew
On Mon, 03 May 2021 09:43:34 +0800, Steven Lee wrote: > Add the description for describing the AST 2600 EVB reference design of > GPIO regulators and provide the example in the document. > > AST2600-A2 EVB has the reference design for enabling SD bus > power and toggling SD bus signal voltage by GPIO pins. > > In the reference design, GPIOV0 of AST2600-A2 EVB is connected to > power load switch that providing 3.3v to SD1 bus vdd. GPIOV1 is > connected to a 1.8v and a 3.3v power load switch that providing > signal voltage to > SD1 bus. > > If GPIOV0 is active high, SD1 bus is enabled. Otherwise, SD1 bus is > disabled. > If GPIOV1 is active high, 3.3v power load switch is enabled, SD1 > signal voltage is 3.3v. Otherwise, 1.8v power load switch will be > enabled, SD1 signal voltage becomes 1.8v. > > AST2600-A2 EVB also support toggling signal voltage for SD2 bus. > The design is the same as SD1 bus. It uses GPIOV2 as power-gpio and > GPIOV3 as power-switch-gpio. > > Signed-off-by: Steven Lee <steven_lee@aspeedtech.com> > --- > .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 99 +++++++++++++++++++ > 1 file changed, 99 insertions(+) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: ./Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml:97:5: [error] syntax error: expected <block end>, but found '<scalar>' (syntax) dtschema/dtc warnings/errors: make[1]: *** Deleting file 'Documentation/devicetree/bindings/mmc/aspeed,sdhci.example.dts' Traceback (most recent call last): File "/usr/local/bin/dt-extract-example", line 45, in <module> binding = yaml.load(open(args.yamlfile, encoding='utf-8').read()) File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 421, in load return constructor.get_single_data() File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 109, in get_single_data node = self.composer.get_single_node() File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node File "_ruamel_yaml.pyx", line 891, in _ruamel_yaml.CParser._compose_mapping_node File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event ruamel.yaml.parser.ParserError: while parsing a block mapping in "<unicode string>", line 5, column 1 did not find expected key in "<unicode string>", line 97, column 5 make[1]: *** [Documentation/devicetree/bindings/Makefile:20: Documentation/devicetree/bindings/mmc/aspeed,sdhci.example.dts] Error 1 make[1]: *** Waiting for unfinished jobs.... ./Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml: while parsing a block mapping in "<unicode string>", line 5, column 1 did not find expected key in "<unicode string>", line 97, column 5 /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml: ignoring, error parsing file warning: no schema found in file: ./Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml make: *** [Makefile:1414: dt_binding_check] Error 2 See https://patchwork.ozlabs.org/patch/1472993 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. 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.
The 05/03/2021 23:20, Rob Herring wrote: > On Mon, 03 May 2021 09:43:34 +0800, Steven Lee wrote: > > Add the description for describing the AST 2600 EVB reference design of > > GPIO regulators and provide the example in the document. > > > > AST2600-A2 EVB has the reference design for enabling SD bus > > power and toggling SD bus signal voltage by GPIO pins. > > > > In the reference design, GPIOV0 of AST2600-A2 EVB is connected to > > power load switch that providing 3.3v to SD1 bus vdd. GPIOV1 is > > connected to a 1.8v and a 3.3v power load switch that providing > > signal voltage to > > SD1 bus. > > > > If GPIOV0 is active high, SD1 bus is enabled. Otherwise, SD1 bus is > > disabled. > > If GPIOV1 is active high, 3.3v power load switch is enabled, SD1 > > signal voltage is 3.3v. Otherwise, 1.8v power load switch will be > > enabled, SD1 signal voltage becomes 1.8v. > > > > AST2600-A2 EVB also support toggling signal voltage for SD2 bus. > > The design is the same as SD1 bus. It uses GPIOV2 as power-gpio and > > GPIOV3 as power-switch-gpio. > > > > Signed-off-by: Steven Lee <steven_lee@aspeedtech.com> > > --- > > .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 99 +++++++++++++++++++ > > 1 file changed, 99 insertions(+) > > > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > yamllint warnings/errors: > ./Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml:97:5: [error] syntax error: expected <block end>, but found '<scalar>' (syntax) > > dtschema/dtc warnings/errors: > make[1]: *** Deleting file 'Documentation/devicetree/bindings/mmc/aspeed,sdhci.example.dts' > Traceback (most recent call last): > File "/usr/local/bin/dt-extract-example", line 45, in <module> > binding = yaml.load(open(args.yamlfile, encoding='utf-8').read()) > File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 421, in load > return constructor.get_single_data() > File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 109, in get_single_data > node = self.composer.get_single_node() > File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node > File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document > File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node > File "_ruamel_yaml.pyx", line 891, in _ruamel_yaml.CParser._compose_mapping_node > File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event > ruamel.yaml.parser.ParserError: while parsing a block mapping > in "<unicode string>", line 5, column 1 > did not find expected key > in "<unicode string>", line 97, column 5 > make[1]: *** [Documentation/devicetree/bindings/Makefile:20: Documentation/devicetree/bindings/mmc/aspeed,sdhci.example.dts] Error 1 > make[1]: *** Waiting for unfinished jobs.... > ./Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml: while parsing a block mapping > in "<unicode string>", line 5, column 1 > did not find expected key > in "<unicode string>", line 97, column 5 > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml: ignoring, error parsing file > warning: no schema found in file: ./Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml > make: *** [Makefile:1414: dt_binding_check] Error 2 > > See https://patchwork.ozlabs.org/patch/1472993 > > This check can fail if there are any dependencies. The base for a patch > series is generally the most recent rc1. > > 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. > Thanks for the log and the information, I will install the package and do the check before re-submiting the patch.
diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml index 987b287f3bff..dd894aba0bb7 100644 --- a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml @@ -20,6 +20,19 @@ description: |+ the slots are dependent on the common configuration area, they are described as child nodes. + The signal voltage of SDHCIs on AST2600-A2 EVB is able to be toggled by GPIO + pins. In the reference design, GPIOV0 of AST2600-A2 EVB is connected to the + power load switch that providing 3.3v to SD1 bus vdd, GPIOV1 is connected to + a 1.8v and a 3.3v power load switch that providing signal voltage to + SD1 bus. + If GPIOV0 is active high, SD1 bus is enabled. Otherwise, SD1 bus is + disabled. If GPIOV1 is active high, 3.3v power load switch is enabled, SD1 + signal voltage is 3.3v. Otherwise, 1.8v power load switch will be enabled, SD1 + signal voltage becomes 1.8v. + AST2600-A2 EVB also support toggling signal voltage for SD2 bus. + The design is the same as SD1 bus. It uses GPIOV2 as power-gpio and GPIOV3 + as power-switch-gpio. + properties: compatible: enum: @@ -78,6 +91,7 @@ required: - clocks examples: + //Example 1 - | #include <dt-bindings/clock/aspeed-clock.h> sdc@1e740000 { @@ -104,3 +118,88 @@ examples: clocks = <&syscon ASPEED_CLK_SDIO>; }; }; + + //Example 2 (AST2600EVB with GPIO regulator) + - | + #include <dt-bindings/clock/aspeed-clock.h> + #include <dt-bindings/gpio/aspeed-gpio.h> + vcc_sdhci0: regulator-vcc-sdhci0 { + compatible = "regulator-fixed"; + + regulator-name = "SDHCI0 Vcc"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + gpios = <&gpio0 ASPEED_GPIO(V, 0) + GPIO_ACTIVE_HIGH>; + enable-active-high; + }; + + vccq_sdhci0: regulator-vccq-sdhci0 { + compatible = "regulator-gpio"; + + regulator-name = "SDHCI0 VccQ"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + gpios = <&gpio0 ASPEED_GPIO(V, 1) + GPIO_ACTIVE_HIGH>; + gpios-states = <1>; + states = <3300000 1 + 1800000 0>; + }; + + vcc_sdhci1: regulator-vcc-sdhci1 { + compatible = "regulator-fixed"; + + regulator-name = "SDHCI1 Vcc"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + gpios = <&gpio0 ASPEED_GPIO(V, 2) + GPIO_ACTIVE_HIGH>; + enable-active-high; + }; + + vccq_sdhci1: regulator-vccq-sdhci1 { + compatible = "regulator-gpio"; + + regulator-name = "SDHCI1 VccQ"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + gpios = <&gpio0 ASPEED_GPIO(V, 3) + GPIO_ACTIVE_HIGH>; + gpios-states = <1>; + states = <3300000 1 + 1800000 0>; + }; + + sdc@1e740000 { + compatible = "aspeed,ast2600-sd-controller"; + reg = <0x1e740000 0x100>; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0 0x1e740000 0x20000>; + clocks = <&syscon ASPEED_CLK_GATE_SDCLK>; + + sdhci0: sdhci@100 { + compatible = "aspeed,ast2600-sdhci", "sdhci"; + reg = <0x100 0x100>; + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>; + sdhci,auto-cmd12; + clocks = <&syscon ASPEED_CLK_SDIO>; + vmmc-supply = <&vcc_sdhci0>; + vqmmc-supply = <&vccq_sdhci0>; + sd-uhs-sdr104; + clk-phase-uhs-sdr104 = <180>, <180>; + }; + + sdhci1: sdhci@200 { + compatible = "aspeed,ast2600-sdhci", "sdhci"; + reg = <0x200 0x100>; + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>; + sdhci,auto-cmd12; + clocks = <&syscon ASPEED_CLK_SDIO>; + vmmc-supply = <&vcc_sdhci1>; + vqmmc-supply = <&vccq_sdhci1>; + sd-uhs-sdr104; + clk-phase-uhs-sdr104 = <0>, <0>; + }; + };
Add the description for describing the AST 2600 EVB reference design of GPIO regulators and provide the example in the document. AST2600-A2 EVB has the reference design for enabling SD bus power and toggling SD bus signal voltage by GPIO pins. In the reference design, GPIOV0 of AST2600-A2 EVB is connected to power load switch that providing 3.3v to SD1 bus vdd. GPIOV1 is connected to a 1.8v and a 3.3v power load switch that providing signal voltage to SD1 bus. If GPIOV0 is active high, SD1 bus is enabled. Otherwise, SD1 bus is disabled. If GPIOV1 is active high, 3.3v power load switch is enabled, SD1 signal voltage is 3.3v. Otherwise, 1.8v power load switch will be enabled, SD1 signal voltage becomes 1.8v. AST2600-A2 EVB also support toggling signal voltage for SD2 bus. The design is the same as SD1 bus. It uses GPIOV2 as power-gpio and GPIOV3 as power-switch-gpio. Signed-off-by: Steven Lee <steven_lee@aspeedtech.com> --- .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+)