| Message ID | 20220329032913.8750-2-axe.yang@mediatek.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v9,1/3] dt-bindings: mmc: mtk-sd: extend interrupts and pinctrls properties | expand |
On Tue, 29 Mar 2022 11:29:11 +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 | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > Reviewed-by: Rob Herring <robh@kernel.org>
On Tue, 29 Mar 2022 at 05:29, Axe Yang <axe.yang@mediatek.com> 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 | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > index 297ada03e3de..3872a6ce2867 100644 > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > @@ -69,12 +69,22 @@ 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. If I understand correctly, the extended interrupt (a GPIO irq) may not necessarily share the same interrupt parent as the primary device interrupt. Perhaps it's then better to extend this with "interrupts-extended" instead. See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt. > + 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 > > pinctrl-0: > description: > @@ -86,6 +96,11 @@ properties: > should contain uhs mode pin ctrl. > maxItems: 1 > > + pinctrl-2: > + description: > + should switch dat1 pin to GPIO mode. > + maxItems: 1 > + > assigned-clocks: > description: > PLL of the source clock. > -- > 2.25.1 > Kind regards Uffe
On Fri, 1 Apr 2022 at 11:22, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > On Tue, 29 Mar 2022 at 05:29, Axe Yang <axe.yang@mediatek.com> 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 | 17 ++++++++++++++++- > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > index 297ada03e3de..3872a6ce2867 100644 > > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > @@ -69,12 +69,22 @@ 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. > > If I understand correctly, the extended interrupt (a GPIO irq) may not > necessarily share the same interrupt parent as the primary device > interrupt. > > Perhaps it's then better to extend this with "interrupts-extended" > instead. See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt. One more thing, looks like using the "interrupt-names" property would be good to use too. To easier distinguish the different irqs. [...] Kind regards Uffe
On Fri, Apr 01, 2022 at 11:22:13AM +0200, Ulf Hansson wrote: > On Tue, 29 Mar 2022 at 05:29, Axe Yang <axe.yang@mediatek.com> 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 | 17 ++++++++++++++++- > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > index 297ada03e3de..3872a6ce2867 100644 > > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > @@ -69,12 +69,22 @@ 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. > > If I understand correctly, the extended interrupt (a GPIO irq) may not > necessarily share the same interrupt parent as the primary device > interrupt. > > Perhaps it's then better to extend this with "interrupts-extended" > instead. See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt. 'interrupts-extended' is interchangeable with 'interrupts'. For schemas, use 'interrupts' and the tools take care of supporting both forms. Rob
On Fri, 2022-04-01 at 12:43 -0500, Rob Herring wrote: > On Fri, Apr 01, 2022 at 11:22:13AM +0200, Ulf Hansson wrote: > > On Tue, 29 Mar 2022 at 05:29, Axe Yang <axe.yang@mediatek.com> > > 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 | 17 > > > ++++++++++++++++- > > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > > > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > > b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > > index 297ada03e3de..3872a6ce2867 100644 > > > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > > @@ -69,12 +69,22 @@ 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. > > > > If I understand correctly, the extended interrupt (a GPIO irq) may > > not > > necessarily share the same interrupt parent as the primary device > > interrupt. > > > > Perhaps it's then better to extend this with "interrupts-extended" > > instead. See Documentation/devicetree/bindings/interrupt- > > controller/interrupts.txt. > > 'interrupts-extended' is interchangeable with 'interrupts'. For > schemas, > use 'interrupts' and the tools take care of supporting both forms. > hello Ulf, you are right, the wakeup interrupt(parent is &pio) do not share same parent as primary interrupt(parent is &gic). And as you said, I am using "interrupts-extended" to declare the wakeup irq, see commit message in patch 3/3: &mmcX { ... interrupts-extended = <...>, <&pio xxx IRQ_TYPE_LEVEL_LOW>; ... pinctrl-names = "default", "state_uhs", "state_eint"; ... pinctrl-2 = <&mmc2_pins_eint>; ... cap-sdio-irq; keep-power-in-suspend; wakeup-source; ... }; But the wakup interrupt is for SDIO only, in most instances, MSDC is been used as eMMC/SD card host, they do not need this interrupt. So as Rob suggested, I think we'd better keep using 'interrupts'. And I will update the description for 'interrupts', suggest to use 'interrupts- extended' to declare SDIO wakeup interrupt. And 'interrupt-names' is a good idea, I will add this property to document too. Thank you for the advice. Regards, Axe
diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml index 297ada03e3de..3872a6ce2867 100644 --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml @@ -69,12 +69,22 @@ 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 pinctrl-0: description: @@ -86,6 +96,11 @@ properties: should contain uhs mode pin ctrl. maxItems: 1 + pinctrl-2: + description: + should switch dat1 pin to GPIO 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 | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)