linux-kernel - Re: [PATCH] misc: rtsx: rts5249 support runtime PM

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: <CAJZ5v0ggtd3+YEo2nERhuTeurRx5OQrvLkAz3aEMPRmEfo_Rnw@mail.gmail.com>
Date:   Thu, 26 Nov 2020 15:19:26 +0100
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     吳昊澄 Ricky <ricky_wu@...ltek.com>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        Bjorn Helgaas <helgaas@...nel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        "vaibhavgupta40@...il.com" <vaibhavgupta40@...il.com>,
        "kdlnx@...h.eu" <kdlnx@...h.eu>,
        Doug Anderson <dianders@...omium.org>,
        "rmfrfs@...il.com" <rmfrfs@...il.com>,
        Lee Jones <lee.jones@...aro.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-mmc <linux-mmc@...r.kernel.org>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Linux PM <linux-pm@...r.kernel.org>
Subject: Re: [PATCH] misc: rtsx: rts5249 support runtime PM

On Thu, Nov 26, 2020 at 4:07 AM 吳昊澄 Ricky <ricky_wu@...ltek.com> wrote:
>
> > -----Original Message-----
> > From: Rafael J. Wysocki [mailto:rafael@...nel.org]
> > Sent: Wednesday, November 25, 2020 10:04 PM
> > To: Bjorn Helgaas; 吳昊澄 Ricky

[cut]

> > > > +static void rtsx_pci_rtd3_work(struct work_struct *work)
> > > > +{
> > > > +     struct delayed_work *dwork = to_delayed_work(work);
> > > > +     struct rtsx_pcr *pcr = container_of(dwork, struct rtsx_pcr,
> > rtd3_work);
> > > > +
> > > > +     pcr_dbg(pcr, "--> %s\n", __func__);
> > > > +
> > > > +     while (pcr->pci->dev.power.usage_count.counter > 0) {
> > > > +             if (pm_runtime_active(&(pcr->pci->dev)))
> > > > +                     pm_runtime_put(&(pcr->pci->dev));
> > >
> > > I'm not a runtime PM expert, but this looks fishy.  AFAICT this is the
> > > only driver in the tree that uses usage_count.counter this way, which
> > > is a pretty big hint that this needs a closer look.  Cc'd Rafael.
> >
> > You are right, this is not correct from the PM-runtime POV.
> >
> > It looks like this attempts to force the PM-runtime usage counter down
> > to 0 and it's kind of hard to say why this is done (and it shouldn't
> > be done in the first place, because it destroys the usage counter
> > balance).
> >
> > Ricky, is this an attempt to work around an issue of some sort?
> >
>
> Thanks Bjorn and Rafael
> I found when we boot up, our dev pcr->pci->dev.power.usage_count.counter always is 2,
> Don’t know how to make it to 0 because we need to support D3 and run runtime_suspended callback function
> Is there something wrong with us to enable runtime PM?

That is possible.

If you want it to be enabled by default, you need to call
pm_runtime_allow() from the driver at probe time, in addition to
pm_runtime_enable(), in the first place, but that only drops one
reference, so question is where the other one comes from.

Are the pm_runtime_get*() and pm_runtime_put*() calls balanced?