On Tue, 7 Jun 2022 at 10:24, Axe Yang <axe.yang@xxxxxxxxxxxx> wrote: > > On Fri, 2022-06-03 at 09:28 +0200, Ulf Hansson wrote: > > On Wed, 25 May 2022 at 03:51, Axe Yang <axe.yang@xxxxxxxxxxxx> 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. > > > > > > Reviewed-by: AngeloGioacchino Del Regno < > > > angelogioacchino.delregno@xxxxxxxxxxxxx> > > > Signed-off-by: Axe Yang <axe.yang@xxxxxxxxxxxx> > > > --- > > > .../devicetree/bindings/mmc/mtk-sd.yaml | 50 > > > ++++++++++++++++++- > > > 1 file changed, 49 insertions(+), 1 deletion(-) > > > > > > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > > b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > > index 2a2e9fa8c188..e83bf10281d6 100644 > > > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml > > > @@ -72,12 +72,27 @@ 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 > > > + > > > + interrupt-names: > > > + items: > > > + - const: msdc > > > + - const: sdio_wakeup > > > > > > 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 > > > > Don't you need something along the lines of the below instead? I mean > > the "state_eint" isn't always needed, right? > > > > oneOf: > > - items: > > - const: default > > - const: state_uhs > > - items: > > - const: default > > - const: state_uhs > > - const: state_eint > > > No, it is not always needed. > As Rob said, the 'minItems: 2' makes the 3rd item optional. > Combine 'minItems' and 'description' fields, it is easy for others to > understand how to set pinctrl related properities. Yes, I agree. > > Anyway, If you insist 'oneOf' is the better way, I can update that in > next version. What do you think? I am fine with it as is, sorry for the noise. > > And thanks to your comment, I found a mistake in 'description', I > should remove descriptions related to 'state_dat1', and I will update > that in next version. Okay. > > And do you have any comment on patch 2/3 and 3/3? Sorry for the delay, I will have a look asap. > > > Regards, > Axe > > Kind regards Uffe
