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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 25 Jul 2022 14:46:30 +0200
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Axe Yang <axe.yang@...iatek.com>
Cc:     Ulf Hansson <ulf.hansson@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Chaotian Jing <chaotian.jing@...iatek.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
        Satya Tangirala <satyat@...gle.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Wolfram Sang <wsa+renesas@...g-engineering.com>,
        Lucas Stach <dev@...xeye.de>,
        Eric Biggers <ebiggers@...gle.com>,
        Andrew Jeffery <andrew@...id.au>,
        Stephen Boyd <swboyd@...omium.org>,
        Kiwoong Kim <kwmad.kim@...sung.com>,
        Yue Hu <huyue2@...ong.com>, Tian Tao <tiantao6@...ilicon.com>,
        angelogioacchino.delregno@...labora.com, linux-mmc@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org,
        Project_Global_Chrome_Upstream_Group@...iatek.com,
        Yong Mao <yong.mao@...iatek.com>
Subject: Re: [PATCH v13 3/3] mmc: mediatek: add support for SDIO eint wakup IRQ

On Mon, Jul 25, 2022 at 11:13 AM Axe Yang <axe.yang@...iatek.com> 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@...iatek.com>
> > 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