Re: [PATCH v13 3/3] mmc: mediatek: add support for SDIO eint wakup IRQ

[Linus Walleij]

On Mon, Jul 25, 2022 at 11:13 AM Axe Yang <axe.yang@xxxxxxxxxxxx> wrote:
> On Fri, 2022-07-22 at 13:21 +0200, Linus Walleij wrote:
> > On Thu, Jun 23, 2022 at 11:10 AM Axe Yang <axe.yang@xxxxxxxxxxxx>
> > wrote:

> SDIO DAT1 pin mode is changed to GPIO mode in
> dev_pm_set_dedicated_wake_irq_reverse():
>
> https://elixir.bootlin.com/linux/latest/source/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c#L339
>
> dev_pm_set_dedicated_wake_irq_reverse() -> ... ->request_threaded_irq()
> -> __setup_irq() -> irq_request_resources() ->
> mtk_eint_irq_request_resources()-> mtk_xt_set_gpio_as_eint()

This doesn't seem to have much to do with pin control?
No pin control functions are called on this execution path,
no pin control state is changed, right?

If what you mean is that
it happens to poke into the same hardware registers that is
mainly a matter of concurrency in the driver, sometimes two
abstractions happen to have to poke into the same hardware
registers and then it is up to the driver to maintain state for
the hardware, this is not a question for the framework.

How is Mediatek developers thinking about this thing in general?
You are a few people who developed the driver so certainly
you must have some design idea to why irq_request_resources()
poke around in these registers? Does it even perform pin
control behind the back of the pin control framework?

> To restore SDIO DAT1 pin to uhs mode. I have to call
> pinctrl_select_state() twice(change pinctrl to another state, then
> change back to uhs mode). Ulf worried we might be doing something at
> the mmc driver level, which should really be managed at the pinctrl
> layer.
>
> Do you have any comment or suggestion on this?

The pin control state transitions are really just finite automata.

Your pin control needs to be different when using wakeup from
when being used for SDIO and this is perfectly fine, it's no
different from the fact that the regulator and clock might need
to be in different states, so I don't quite understand the
question?

Yours,
Linus Walleij