linux-kernel - Re: [PATCH 01/11] mmc: core: Add helper function to indicate if SDIO IRQs is enabled

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:   Thu, 5 Sep 2019 09:28:58 +0200
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     Matthias Kaehlcke <mka@...omium.org>
Cc:     "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Douglas Anderson <dianders@...omium.org>,
        Shawn Lin <shawn.lin@...k-chips.com>,
        Jaehoon Chung <jh80.chung@...sung.com>,
        Yong Mao <yong.mao@...iatek.com>,
        Chaotian Jing <chaotian.jing@...iatek.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 01/11] mmc: core: Add helper function to indicate if SDIO
 IRQs is enabled

On Thu, 5 Sep 2019 at 01:58, Matthias Kaehlcke <mka@...omium.org> wrote:
>
> Hi Ulf,
>
> On Tue, Sep 03, 2019 at 04:21:57PM +0200, Ulf Hansson wrote:
> > To avoid each host driver supporting SDIO IRQs, from keeping track
> > internally about if SDIO IRQs has been enabled, let's introduce a common
> > helper function, sdio_irq_enabled().
> >
> > The function returns true if SDIO IRQs are enabled, via using the
> > information about the number of claimed irqs. This is safe, even without
> > any locks, as long as the helper function is called only from
> > runtime/system suspend callbacks of the host driver.
> >
> > Signed-off-by: Ulf Hansson <ulf.hansson@...aro.org>
> > ---
> >  include/linux/mmc/host.h | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> > index 4a351cb7f20f..0c0a565c7ff1 100644
> > --- a/include/linux/mmc/host.h
> > +++ b/include/linux/mmc/host.h
> > @@ -493,6 +493,15 @@ void mmc_command_done(struct mmc_host *host, struct mmc_request *mrq);
> >
> >  void mmc_cqe_request_done(struct mmc_host *host, struct mmc_request *mrq);
> >
> > +/*
> > + * May be called from host driver's system/runtime suspend/resume callbacks,
> > + * to know if SDIO IRQs has been enabled.
> > +*/
> > +static inline bool sdio_irq_enabled(struct mmc_host *host)
> > +{
> > +     return host->sdio_irqs > 0;
> > +}
> > +
>
> The name of the function is a bit misleadling, since it indicates
> if SDIO IRQs should be enabled, not whether they are actually enabled
> by the host. The resulting code can look a bit confusing to the
> uninstructed reader:
>
>   if (sdio_irq_enabled(host->slot->mmc))
>     __dw_mci_enable_sdio_irq(host->slot, 1);
>
> aka 'if SDIO IRQ is enabled, enable SDIO IRQ'.
>
> sdio_irqs_claimed() could be a possible alternative.
>
> No biggie though, just something I noticed.

Thanks for the suggestions. It makes perfect sense to me, let me rename it.

Kind regards
Uffe