linux-kernel - Re: [PATCH] mmc: dw_mmc: Disable SDIO interrupts while suspended to fix suspend/resume

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]

Message-ID: <CAD=FV=XuCrymazJDKBYqLezkYkikzGNiv6X0FdzNG0SaR8PuRw@mail.gmail.com>
Date:   Tue, 23 Apr 2019 18:09:19 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Shawn Lin <shawn.lin@...k-chips.com>
Cc:     Jaehoon Chung <jh80.chung@...sung.com>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Kalle Valo <kvalo@...eaurora.org>,
        Heiko Stübner <heiko@...ech.de>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        Brian Norris <briannorris@...omium.org>,
        linux-wireless <linux-wireless@...r.kernel.org>,
        Matthias Kaehlcke <mka@...omium.org>,
        Ryan Case <ryandcase@...omium.org>, stable@...r.kernel.org,
        Linux MMC List <linux-mmc@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mmc: dw_mmc: Disable SDIO interrupts while suspended to
 fix suspend/resume

Hi,

On Tue, Apr 23, 2019 at 5:57 PM Shawn Lin <shawn.lin@...k-chips.com> wrote:
>
> The intention seems reasonable to me, but just wonder if we need
> mask/unmask SDIO interrupt when it's never used?

I don't think we do.

Specifically "client_sdio_enb" starts out as false.  If nobody ever
calls dw_mci_enable_sdio_irq() then "client_sdio_enb" will always
continue to be false.

Now at suspend time we'll call "__dw_mci_enable_sdio_irq".  Because
"client_sdio_enb" is false then the local variable "enb" will always
be false.  Sure we'll clear the "SDMMC_INT_SDIO" from the "INTMASK"
register, but it should already have been cleared so this is a no-op.

...at resume time we'll have a similar situation where
"client_sdio_enb" is false and thus we'll (again) just clear the
"SDMMC_INT_SDIO" from the "INTMASK".

I could potentially optimize away the "mci_writel()" if we're not
changing anything if you're worried about that?

> It's the same
> situation for SDMMC_CLKEN_LOW_PWR that we couldn't stop providing
> clock for SDIO cards, so I guess we need to check MMC_CAP_SDIO_IRQ
> as well.

I think it might be a slightly different situation though.  In this
case I believe it's not just a problem with clock stoppage.  I believe
the problem is that the interrupt will be passed to the SDIO device
driver right away and that'll call back into dw_mmc.  dw_mmc is just
not in a state to handle it until we've more fully resumed things.

In any case with my patch the only way we'd ever end up unmasking the
SDIO IRQ here would be if dw_mci_enable_sdio_irq() was called.  That
will only happen if there's an SDIO card plugged in.

-Doug