| Message ID | 20220321115133.32121-2-axe.yang@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: mediatek: add support for SDIO async IRQ | expand |
On Mon, Mar 21, 2022 at 07:51:32PM +0800, Axe Yang wrote: > Extend interrupts and pinctrls for SDIO wakeup interrupt feature. > This feature allow SDIO devices alarm asynchronous interrupt to host > even when host stop providing clock to SDIO card. An extra wakeup > interrupt and pinctrl states for SDIO DAT1 pin state switching are > required in this scenario. > > Signed-off-by: Axe Yang <axe.yang@mediatek.com> > --- > .../devicetree/bindings/mmc/mtk-sd.yaml | 23 ++++++++++++++++++- > 1 file changed, 22 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > index 297ada03e3de..f57774535a1d 100644 > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > @@ -69,12 +69,23 @@ properties: > - const: ahb_cg > > interrupts: > - maxItems: 1 > + description: > + Should at least contain MSDC GIC interrupt. To support SDIO in-band wakeup, an extended > + interrupt is required and be configured as wakeup source irq. > + minItems: 1 > + maxItems: 2 > > pinctrl-names: > + description: > + Should at least contain default and state_uhs. To support SDIO in-band wakeup, dat1 pin > + will be switched between GPIO mode and SDIO DAT1 mode, state_eint and state_dat1 are > + mandatory in this scenarios. > + minItems: 2 > items: > - const: default > - const: state_uhs > + - const: state_eint > + - const: state_dat1 > > pinctrl-0: > description: > @@ -86,6 +97,16 @@ properties: > should contain uhs mode pin ctrl. > maxItems: 1 > > + pinctrl-2: > + description: > + should switch dat1 pin to GPIO mode. > + maxItems: 1 > + > + pinctrl-3: > + description: > + should switch SDIO dat1 pin from GPIO mode back to SDIO mode. How is this different than pinctrl-0? > + maxItems: 1 > + > assigned-clocks: > description: > PLL of the source clock. > -- > 2.25.1 > >
On Mon, 2022-03-21 at 18:29 -0500, Rob Herring wrote: > On Mon, Mar 21, 2022 at 07:51:32PM +0800, Axe Yang wrote: > > Extend interrupts and pinctrls for SDIO wakeup interrupt feature. > > This feature allow SDIO devices alarm asynchronous interrupt to > > host > > even when host stop providing clock to SDIO card. An extra wakeup > > interrupt and pinctrl states for SDIO DAT1 pin state switching are > > required in this scenario. > > > > Signed-off-by: Axe Yang <axe.yang@mediatek.com> > > --- > > .../devicetree/bindings/mmc/mtk-sd.yaml | 23 > > ++++++++++++++++++- > > 1 file changed, 22 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > index 297ada03e3de..f57774535a1d 100644 > > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > @@ -69,12 +69,23 @@ properties: > > - const: ahb_cg > > > > interrupts: > > - maxItems: 1 > > + description: > > + Should at least contain MSDC GIC interrupt. To support SDIO > > in-band wakeup, an extended > > + interrupt is required and be configured as wakeup source > > irq. > > + minItems: 1 > > + maxItems: 2 > > > > pinctrl-names: > > + description: > > + Should at least contain default and state_uhs. To support > > SDIO in-band wakeup, dat1 pin > > + will be switched between GPIO mode and SDIO DAT1 mode, > > state_eint and state_dat1 are > > + mandatory in this scenarios. > > + minItems: 2 > > items: > > - const: default > > - const: state_uhs > > + - const: state_eint > > + - const: state_dat1 > > > > pinctrl-0: > > description: > > @@ -86,6 +97,16 @@ properties: > > should contain uhs mode pin ctrl. > > maxItems: 1 > > > > + pinctrl-2: > > + description: > > + should switch dat1 pin to GPIO mode. > > + maxItems: 1 > > + > > + pinctrl-3: > > + description: > > + should switch SDIO dat1 pin from GPIO mode back to SDIO > > mode. > > How is this different than pinctrl-0? pinctrl-0 contains default settings for all IO pins(CLK/CMD/DAT). pinctrl-1 contains settings for all IO pins(CLK/CMD/DAT) in UHS mode. pinctrl-3 is lightweight pinctrl-1, only keep SDIO DAT1 pin function switch part. ... Regards, Axe
Il 22/03/22 02:35, Axe Yang ha scritto: > On Mon, 2022-03-21 at 18:29 -0500, Rob Herring wrote: >> On Mon, Mar 21, 2022 at 07:51:32PM +0800, Axe Yang wrote: >>> Extend interrupts and pinctrls for SDIO wakeup interrupt feature. >>> This feature allow SDIO devices alarm asynchronous interrupt to >>> host >>> even when host stop providing clock to SDIO card. An extra wakeup >>> interrupt and pinctrl states for SDIO DAT1 pin state switching are >>> required in this scenario. >>> >>> Signed-off-by: Axe Yang <axe.yang@mediatek.com> >>> --- >>> .../devicetree/bindings/mmc/mtk-sd.yaml | 23 >>> ++++++++++++++++++- >>> 1 file changed, 22 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml >>> b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml >>> index 297ada03e3de..f57774535a1d 100644 >>> --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml >>> +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml >>> @@ -69,12 +69,23 @@ properties: >>> - const: ahb_cg >>> >>> interrupts: >>> - maxItems: 1 >>> + description: >>> + Should at least contain MSDC GIC interrupt. To support SDIO >>> in-band wakeup, an extended >>> + interrupt is required and be configured as wakeup source >>> irq. >>> + minItems: 1 >>> + maxItems: 2 >>> >>> pinctrl-names: >>> + description: >>> + Should at least contain default and state_uhs. To support >>> SDIO in-band wakeup, dat1 pin >>> + will be switched between GPIO mode and SDIO DAT1 mode, >>> state_eint and state_dat1 are >>> + mandatory in this scenarios. >>> + minItems: 2 >>> items: >>> - const: default >>> - const: state_uhs >>> + - const: state_eint >>> + - const: state_dat1 >>> >>> pinctrl-0: >>> description: >>> @@ -86,6 +97,16 @@ properties: >>> should contain uhs mode pin ctrl. >>> maxItems: 1 >>> >>> + pinctrl-2: >>> + description: >>> + should switch dat1 pin to GPIO mode. >>> + maxItems: 1 >>> + >>> + pinctrl-3: >>> + description: >>> + should switch SDIO dat1 pin from GPIO mode back to SDIO >>> mode. >> >> How is this different than pinctrl-0? > > pinctrl-0 contains default settings for all IO pins(CLK/CMD/DAT). > pinctrl-1 contains settings for all IO pins(CLK/CMD/DAT) in UHS mode. > pinctrl-3 is lightweight pinctrl-1, only keep SDIO DAT1 pin function > switch part. > Is there any particular reason why we cannot simply select pinctrl-1 again instead of pinctrl-3, apart from the virtually not existent overhead of one more mmio write? > ... > > Regards, > Axe >
Hello AngeloGioacchino, On Tue, 2022-03-22 at 09:42 +0100, AngeloGioacchino Del Regno wrote: > Il 22/03/22 02:35, Axe Yang ha scritto: > > On Mon, 2022-03-21 at 18:29 -0500, Rob Herring wrote: > > > On Mon, Mar 21, 2022 at 07:51:32PM +0800, Axe Yang wrote: > > > > Extend interrupts and pinctrls for SDIO wakeup interrupt > > > > feature. > > > > This feature allow SDIO devices alarm asynchronous interrupt to > > > > host > > > > even when host stop providing clock to SDIO card. An extra > > > > wakeup > > > > interrupt and pinctrl states for SDIO DAT1 pin state switching > > > > are > > > > required in this scenario. > > > > > > > > Signed-off-by: Axe Yang <axe.yang@mediatek.com> > > > > --- > > > > .../devicetree/bindings/mmc/mtk-sd.yaml | 23 > > > > ++++++++++++++++++- > > > > 1 file changed, 22 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > > > b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > > > index 297ada03e3de..f57774535a1d 100644 > > > > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > > > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > > > @@ -69,12 +69,23 @@ properties: > > > > - const: ahb_cg > > > > > > > > interrupts: > > > > - maxItems: 1 > > > > + description: > > > > + Should at least contain MSDC GIC interrupt. To support > > > > SDIO > > > > in-band wakeup, an extended > > > > + interrupt is required and be configured as wakeup source > > > > irq. > > > > + minItems: 1 > > > > + maxItems: 2 > > > > > > > > pinctrl-names: > > > > + description: > > > > + Should at least contain default and state_uhs. To > > > > support > > > > SDIO in-band wakeup, dat1 pin > > > > + will be switched between GPIO mode and SDIO DAT1 mode, > > > > state_eint and state_dat1 are > > > > + mandatory in this scenarios. > > > > + minItems: 2 > > > > items: > > > > - const: default > > > > - const: state_uhs > > > > + - const: state_eint > > > > + - const: state_dat1 > > > > > > > > pinctrl-0: > > > > description: > > > > @@ -86,6 +97,16 @@ properties: > > > > should contain uhs mode pin ctrl. > > > > maxItems: 1 > > > > > > > > + pinctrl-2: > > > > + description: > > > > + should switch dat1 pin to GPIO mode. > > > > + maxItems: 1 > > > > + > > > > + pinctrl-3: > > > > + description: > > > > + should switch SDIO dat1 pin from GPIO mode back to SDIO > > > > mode. > > > > > > How is this different than pinctrl-0? > > > > pinctrl-0 contains default settings for all IO pins(CLK/CMD/DAT). > > pinctrl-1 contains settings for all IO pins(CLK/CMD/DAT) in UHS > > mode. > > pinctrl-3 is lightweight pinctrl-1, only keep SDIO DAT1 pin > > function > > switch part. > > > > Is there any particular reason why we cannot simply select pinctrl-1 > again > instead of pinctrl-3, apart from the virtually not existent overhead > of one more mmio write? No, there is no particular reason. I just want to do the pin function switch quick and clean. The intention of pinctrl-1 is to set the most initial state of IO pins in UHS mode. If I don't need to adjust IO settings any longer, it is okay to select pinctrl-1 state instead of pinctrl-3. But think about this scenarios: after initial SDIO IO pins to UHS mode, I want to adjust some IO related properties, such as driving strength. And I want to keep these settings because with new driving strength, the signal is better. I'd rather to choose pinctrl-3 but not pinctrl-1, because I do not want the change be restored after next runtime resume. Regards, Axe
On Tue, Mar 22, 2022 at 05:33:55PM +0800, Axe Yang wrote: > Hello AngeloGioacchino, > > On Tue, 2022-03-22 at 09:42 +0100, AngeloGioacchino Del Regno wrote: > > Il 22/03/22 02:35, Axe Yang ha scritto: > > > On Mon, 2022-03-21 at 18:29 -0500, Rob Herring wrote: > > > > On Mon, Mar 21, 2022 at 07:51:32PM +0800, Axe Yang wrote: > > > > > Extend interrupts and pinctrls for SDIO wakeup interrupt > > > > > feature. > > > > > This feature allow SDIO devices alarm asynchronous interrupt to > > > > > host > > > > > even when host stop providing clock to SDIO card. An extra > > > > > wakeup > > > > > interrupt and pinctrl states for SDIO DAT1 pin state switching > > > > > are > > > > > required in this scenario. > > > > > > > > > > Signed-off-by: Axe Yang <axe.yang@mediatek.com> > > > > > --- > > > > > .../devicetree/bindings/mmc/mtk-sd.yaml | 23 > > > > > ++++++++++++++++++- > > > > > 1 file changed, 22 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > > > > b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > > > > index 297ada03e3de..f57774535a1d 100644 > > > > > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > > > > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > > > > @@ -69,12 +69,23 @@ properties: > > > > > - const: ahb_cg > > > > > > > > > > interrupts: > > > > > - maxItems: 1 > > > > > + description: > > > > > + Should at least contain MSDC GIC interrupt. To support > > > > > SDIO > > > > > in-band wakeup, an extended > > > > > + interrupt is required and be configured as wakeup source > > > > > irq. > > > > > + minItems: 1 > > > > > + maxItems: 2 > > > > > > > > > > pinctrl-names: > > > > > + description: > > > > > + Should at least contain default and state_uhs. To > > > > > support > > > > > SDIO in-band wakeup, dat1 pin > > > > > + will be switched between GPIO mode and SDIO DAT1 mode, > > > > > state_eint and state_dat1 are > > > > > + mandatory in this scenarios. > > > > > + minItems: 2 > > > > > items: > > > > > - const: default > > > > > - const: state_uhs > > > > > + - const: state_eint > > > > > + - const: state_dat1 > > > > > > > > > > pinctrl-0: > > > > > description: > > > > > @@ -86,6 +97,16 @@ properties: > > > > > should contain uhs mode pin ctrl. > > > > > maxItems: 1 > > > > > > > > > > + pinctrl-2: > > > > > + description: > > > > > + should switch dat1 pin to GPIO mode. > > > > > + maxItems: 1 > > > > > + > > > > > + pinctrl-3: > > > > > + description: > > > > > + should switch SDIO dat1 pin from GPIO mode back to SDIO > > > > > mode. > > > > > > > > How is this different than pinctrl-0? > > > > > > pinctrl-0 contains default settings for all IO pins(CLK/CMD/DAT). > > > pinctrl-1 contains settings for all IO pins(CLK/CMD/DAT) in UHS > > > mode. > > > pinctrl-3 is lightweight pinctrl-1, only keep SDIO DAT1 pin > > > function > > > switch part. > > > > > > > Is there any particular reason why we cannot simply select pinctrl-1 > > again > > instead of pinctrl-3, apart from the virtually not existent overhead > > of one more mmio write? > > No, there is no particular reason. > I just want to do the pin function switch quick and clean. > > The intention of pinctrl-1 is to set the most initial state of IO pins > in UHS mode. If I don't need to adjust IO settings any longer, it is > okay to select pinctrl-1 state instead of pinctrl-3. > But think about this scenarios: after initial SDIO IO pins to UHS mode, > I want to adjust some IO related properties, such as driving strength. > And I want to keep these settings because with new driving strength, > the signal is better. I'd rather to choose pinctrl-3 but not pinctrl-1, > because I do not want the change be restored after next runtime resume. The pinctrl-X properties set modes, they aren't supposed to be a state machine. Rob
Hi Rob, Sorry, my last mail may mislead your understanding. Let me explain more about this. On Tue, 2022-03-22 at 11:42 -0500, Rob Herring wrote: > On Tue, Mar 22, 2022 at 05:33:55PM +0800, Axe Yang wrote: > > Hello AngeloGioacchino, > > > > On Tue, 2022-03-22 at 09:42 +0100, AngeloGioacchino Del Regno > > wrote: > > > Il 22/03/22 02:35, Axe Yang ha scritto: > > > > On Mon, 2022-03-21 at 18:29 -0500, Rob Herring wrote: > > > > > On Mon, Mar 21, 2022 at 07:51:32PM +0800, Axe Yang wrote: > > > > > > Extend interrupts and pinctrls for SDIO wakeup interrupt > > > > > > feature. > > > > > > This feature allow SDIO devices alarm asynchronous > > > > > > interrupt to > > > > > > host > > > > > > even when host stop providing clock to SDIO card. An extra > > > > > > wakeup > > > > > > interrupt and pinctrl states for SDIO DAT1 pin state > > > > > > switching > > > > > > are > > > > > > required in this scenario. > > > > > > > > > > > > Signed-off-by: Axe Yang <axe.yang@mediatek.com> > > > > > > --- > > > > > > .../devicetree/bindings/mmc/mtk-sd.yaml | 23 > > > > > > ++++++++++++++++++- > > > > > > 1 file changed, 22 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/mmc/mtk- > > > > > > sd.yaml > > > > > > b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > > > > > index 297ada03e3de..f57774535a1d 100644 > > > > > > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > > > > > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > > > > > @@ -69,12 +69,23 @@ properties: > > > > > > - const: ahb_cg > > > > > > > > > > > > interrupts: > > > > > > - maxItems: 1 > > > > > > + description: > > > > > > + Should at least contain MSDC GIC interrupt. To > > > > > > support > > > > > > SDIO > > > > > > in-band wakeup, an extended > > > > > > + interrupt is required and be configured as wakeup > > > > > > source > > > > > > irq. > > > > > > + minItems: 1 > > > > > > + maxItems: 2 > > > > > > > > > > > > pinctrl-names: > > > > > > + description: > > > > > > + Should at least contain default and state_uhs. To > > > > > > support > > > > > > SDIO in-band wakeup, dat1 pin > > > > > > + will be switched between GPIO mode and SDIO DAT1 > > > > > > mode, > > > > > > state_eint and state_dat1 are > > > > > > + mandatory in this scenarios. > > > > > > + minItems: 2 > > > > > > items: > > > > > > - const: default > > > > > > - const: state_uhs > > > > > > + - const: state_eint > > > > > > + - const: state_dat1 > > > > > > > > > > > > pinctrl-0: > > > > > > description: > > > > > > @@ -86,6 +97,16 @@ properties: > > > > > > should contain uhs mode pin ctrl. > > > > > > maxItems: 1 > > > > > > > > > > > > + pinctrl-2: > > > > > > + description: > > > > > > + should switch dat1 pin to GPIO mode. > > > > > > + maxItems: 1 > > > > > > + > > > > > > + pinctrl-3: > > > > > > + description: > > > > > > + should switch SDIO dat1 pin from GPIO mode back to > > > > > > SDIO > > > > > > mode. > > > > > > > > > > How is this different than pinctrl-0? > > > > > > > > pinctrl-0 contains default settings for all IO > > > > pins(CLK/CMD/DAT). > > > > pinctrl-1 contains settings for all IO pins(CLK/CMD/DAT) in UHS > > > > mode. > > > > pinctrl-3 is lightweight pinctrl-1, only keep SDIO DAT1 pin > > > > function > > > > switch part. > > > > > > > > > > Is there any particular reason why we cannot simply select > > > pinctrl-1 > > > again > > > instead of pinctrl-3, apart from the virtually not existent > > > overhead > > > of one more mmio write? > > > > No, there is no particular reason. > > I just want to do the pin function switch quick and clean. > > > > The intention of pinctrl-1 is to set the most initial state of IO > > pins > > in UHS mode. If I don't need to adjust IO settings any longer, it > > is > > okay to select pinctrl-1 state instead of pinctrl-3. > > But think about this scenarios: after initial SDIO IO pins to UHS > > mode, > > I want to adjust some IO related properties, such as driving > > strength. > > And I want to keep these settings because with new driving > > strength, > > the signal is better. I'd rather to choose pinctrl-3 but not > > pinctrl-1, > > because I do not want the change be restored after next runtime > > resume. > > The pinctrl-X properties set modes, they aren't supposed to be a > state > machine I do use the pinctl-x properties to set modes, but not judge state from pin state. I need to time-multiplex SDIO DAT1 pin, shift it between GPIO mode and SDIO IO mode, for example: mmc2_pins_default { ... }; mmc2_pins_uhs { pins_clk { pinmux = <PINMUX_GPIO170__FUNC_B1_MSDC2_CLK>; drive-strength = <MTK_DRIVE_4mA>; bias-pull-down = <MTK_PUPD_SET_R1R0_10>; }; pins_cmd_dat { pinmux = <PINMUX_GPIO169__FUNC_B1_MSDC2_CMD>, <PINMUX_GPIO171__FUNC_B1_MSDC2_DAT0>, <PINMUX_GPIO172__FUNC_B1_MSDC2_DAT1>, <PINMUX_GPIO173__FUNC_B1_MSDC2_DAT2>, <PINMUX_GPIO174__FUNC_B1_MSDC2_DAT3>; input-enable; drive-strength = <MTK_DRIVE_6mA>; bias-pull-up = <MTK_PUPD_SET_R1R0_01>; }; }; mmc2_pins_eint { pins_dat1 { pinmux = <PINMUX_GPIO172__FUNC_B_GPIO172>; input-enable; bias-pull-up = <MTK_PUPD_SET_R1R0_01>; }; }; mmc2_pins_dat1 { pins_dat1 { pinmux = <PINMUX_GPIO172__FUNC_B1_MSDC2_DAT1>; input-enable; bias-pull-up = <MTK_PUPD_SET_R1R0_01>; }; }; The pinctrl-1(mmc2_pin_uhs) here contain all SDIO IO pins, for the most early initialize. I SELECT pinctrl-1 when I change SDIO to SDR104 mode, before calibration. And when SDIO bus is idle, I SELECT pinctrl-2(mmc2_pins_eint), use DAT1 as a interrupt line when steping into runtime suspend. After resume, I SELECT pinctrl-3(mmc2_pins_dat1) to set DAT1 pin back to SDIO IO mode. I need all the pinctrl-x properties, and take the initiative to set pin mode according to needs. Best Regard, Axe
diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml index 297ada03e3de..f57774535a1d 100644 --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml @@ -69,12 +69,23 @@ properties: - const: ahb_cg interrupts: - maxItems: 1 + description: + Should at least contain MSDC GIC interrupt. To support SDIO in-band wakeup, an extended + interrupt is required and be configured as wakeup source irq. + minItems: 1 + maxItems: 2 pinctrl-names: + description: + Should at least contain default and state_uhs. To support SDIO in-band wakeup, dat1 pin + will be switched between GPIO mode and SDIO DAT1 mode, state_eint and state_dat1 are + mandatory in this scenarios. + minItems: 2 items: - const: default - const: state_uhs + - const: state_eint + - const: state_dat1 pinctrl-0: description: @@ -86,6 +97,16 @@ properties: should contain uhs mode pin ctrl. maxItems: 1 + pinctrl-2: + description: + should switch dat1 pin to GPIO mode. + maxItems: 1 + + pinctrl-3: + description: + should switch SDIO dat1 pin from GPIO mode back to SDIO mode. + maxItems: 1 + assigned-clocks: description: PLL of the source clock.
Extend interrupts and pinctrls for SDIO wakeup interrupt feature. This feature allow SDIO devices alarm asynchronous interrupt to host even when host stop providing clock to SDIO card. An extra wakeup interrupt and pinctrl states for SDIO DAT1 pin state switching are required in this scenario. Signed-off-by: Axe Yang <axe.yang@mediatek.com> --- .../devicetree/bindings/mmc/mtk-sd.yaml | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)