linux-kernel - Re: [PATCH 3/3] phy: sun4i-usb: Use power efficient workqueue for debounce and poll

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: <20201112095307.pblcmrn6smyecvqi@gilmour.lan>
Date:   Thu, 12 Nov 2020 10:53:07 +0100
From:   Maxime Ripard <maxime@...no.tech>
To:     Samuel Holland <samuel@...lland.org>
Cc:     Frank Lee <frank@...winnertech.com>, vkoul@...nel.org,
        wens@...e.org, krzk@...nel.org, colin.king@...onical.com,
        tiny.windzz@...il.com, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 3/3] phy: sun4i-usb: Use power efficient workqueue for
 debounce and poll

On Tue, Nov 10, 2020 at 09:44:37PM -0600, Samuel Holland wrote:
> On 11/9/20 6:12 AM, Frank Lee wrote:
> > From: Yangtao Li <frank@...winnertech.com>
> > 
> > The debounce and poll time is generally quite long and the work not
> > performance critical so allow the scheduler to run the work anywhere
> > rather than in the normal per-CPU workqueue.
> > 
> > Signed-off-by: Yangtao Li <frank@...winnertech.com>
> > ---
> >  drivers/phy/allwinner/phy-sun4i-usb.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
> > index 651d5e2a25ce..4787ad13b255 100644
> > --- a/drivers/phy/allwinner/phy-sun4i-usb.c
> > +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
> > @@ -326,7 +326,7 @@ static int sun4i_usb_phy_init(struct phy *_phy)
> >  		/* Force ISCR and cable state updates */
> >  		data->id_det = -1;
> >  		data->vbus_det = -1;
> > -		queue_delayed_work(system_wq, &data->detect, 0);
> > +		queue_delayed_work(system_power_efficient_wq, &data->detect, 0);
> >  	}
> >  
> >  	return 0;
> > @@ -444,7 +444,7 @@ static int sun4i_usb_phy_power_on(struct phy *_phy)
> >  
> >  	/* We must report Vbus high within OTG_TIME_A_WAIT_VRISE msec. */
> 
> This doesn't sound like "not performance critical" to me. My understanding is
> the debouncing has a deadline from the USB spec. Maybe this is more flexible
> than the comment makes it sound?

It's not really clear to me what the power_efficient workqueue brings to
the table exactly from the comments on WQ_POWER_EFFICIENT (and the
associated gmane link is long dead).

It's only effect seems to be that it sets WQ_UNBOUND when the proper
command line option is set, and WQ_UNBOUND allows for the scheduled work
to run on any CPU instead of the local one.

Given that we don't have any constraint on the CPU here, and the CPU
locality shouldn't really make any difference, I'm not sure we should
expect any meaningful difference.

This is also what the rest of the similar drivers seem to be using:
https://elixir.bootlin.com/linux/v5.10-rc3/source/drivers/usb/common/usb-conn-gpio.c#L119
https://elixir.bootlin.com/linux/v5.10-rc3/source/drivers/usb/core/hub.c#L1254

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)