On 11/12/20 3:53 AM, Maxime Ripard wrote: > 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 <fr...@allwinnertech.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 <fr...@allwinnertech.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
Thanks for the explanation. Then the patch looks fine to me. Reviewed-by: Samuel Holland <sam...@sholland.org> Cheers, Samuel
