* [PATCH 3/3] phy: sun4i-usb: Use power efficient workqueue for debounce and poll
@ 2020-11-09 12:12 ` Frank Lee
0 siblings, 0 replies; 14+ messages in thread
From: Frank Lee @ 2020-11-09 12:12 UTC (permalink / raw)
To: vkoul, mripard, wens, krzk, colin.king, tiny.windzz
Cc: Yangtao Li, linux-kernel, linux-arm-kernel
From: Yangtao Li <frank@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 <frank@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. */
if (phy->index == 0 && sun4i_usb_phy0_poll(data))
- mod_delayed_work(system_wq, &data->detect, DEBOUNCE_TIME);
+ mod_delayed_work(system_power_efficient_wq, &data->detect, DEBOUNCE_TIME);
return 0;
}
@@ -465,7 +465,7 @@ static int sun4i_usb_phy_power_off(struct phy *_phy)
* Vbus gpio to not trigger an edge irq on Vbus off, so force a rescan.
*/
if (phy->index == 0 && !sun4i_usb_phy0_poll(data))
- mod_delayed_work(system_wq, &data->detect, POLL_TIME);
+ mod_delayed_work(system_power_efficient_wq, &data->detect, POLL_TIME);
return 0;
}
@@ -504,7 +504,7 @@ static int sun4i_usb_phy_set_mode(struct phy *_phy,
data->id_det = -1; /* Force reprocessing of id */
data->force_session_end = true;
- queue_delayed_work(system_wq, &data->detect, 0);
+ queue_delayed_work(system_power_efficient_wq, &data->detect, 0);
return 0;
}
@@ -616,7 +616,7 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
extcon_set_state_sync(data->extcon, EXTCON_USB, vbus_det);
if (sun4i_usb_phy0_poll(data))
- queue_delayed_work(system_wq, &data->detect, POLL_TIME);
+ queue_delayed_work(system_power_efficient_wq, &data->detect, POLL_TIME);
}
static irqreturn_t sun4i_usb_phy0_id_vbus_det_irq(int irq, void *dev_id)
@@ -624,7 +624,7 @@ static irqreturn_t sun4i_usb_phy0_id_vbus_det_irq(int irq, void *dev_id)
struct sun4i_usb_phy_data *data = dev_id;
/* vbus or id changed, let the pins settle and then scan them */
- mod_delayed_work(system_wq, &data->detect, DEBOUNCE_TIME);
+ mod_delayed_work(system_power_efficient_wq, &data->detect, DEBOUNCE_TIME);
return IRQ_HANDLED;
}
@@ -638,7 +638,7 @@ static int sun4i_usb_phy0_vbus_notify(struct notifier_block *nb,
/* Properties on the vbus_power_supply changed, scan vbus_det */
if (val == PSY_EVENT_PROP_CHANGED && psy == data->vbus_power_supply)
- mod_delayed_work(system_wq, &data->detect, DEBOUNCE_TIME);
+ mod_delayed_work(system_power_efficient_wq, &data->detect, DEBOUNCE_TIME);
return NOTIFY_OK;
}
--
2.28.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] phy: sun4i-usb: Use power efficient workqueue for debounce and poll
2020-11-09 12:12 ` Frank Lee
@ 2020-11-10 10:02 ` Maxime Ripard
-1 siblings, 0 replies; 14+ messages in thread
From: Maxime Ripard @ 2020-11-10 10:02 UTC (permalink / raw)
To: Frank Lee
Cc: vkoul, wens, krzk, colin.king, tiny.windzz, linux-kernel,
linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 427 bytes --]
On Mon, Nov 09, 2020 at 08:12:14PM +0800, Frank Lee wrote:
> From: Yangtao Li <frank@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 <frank@allwinnertech.com>
Acked-by: Maxime Ripard <mripard@kernel.org>
Thanks!
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] phy: sun4i-usb: Use power efficient workqueue for debounce and poll
@ 2020-11-10 10:02 ` Maxime Ripard
0 siblings, 0 replies; 14+ messages in thread
From: Maxime Ripard @ 2020-11-10 10:02 UTC (permalink / raw)
To: Frank Lee
Cc: tiny.windzz, wens, krzk, linux-kernel, vkoul, colin.king,
linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 427 bytes --]
On Mon, Nov 09, 2020 at 08:12:14PM +0800, Frank Lee wrote:
> From: Yangtao Li <frank@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 <frank@allwinnertech.com>
Acked-by: Maxime Ripard <mripard@kernel.org>
Thanks!
Maxime
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] phy: sun4i-usb: Use power efficient workqueue for debounce and poll
2020-11-09 12:12 ` Frank Lee
@ 2020-11-11 3:44 ` Samuel Holland
-1 siblings, 0 replies; 14+ messages in thread
From: Samuel Holland @ 2020-11-11 3:44 UTC (permalink / raw)
To: Frank Lee, vkoul, mripard, wens, krzk, colin.king, tiny.windzz
Cc: linux-kernel, linux-arm-kernel
On 11/9/20 6:12 AM, Frank Lee wrote:
> From: Yangtao Li <frank@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 <frank@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?
> if (phy->index == 0 && sun4i_usb_phy0_poll(data))
> - mod_delayed_work(system_wq, &data->detect, DEBOUNCE_TIME);
> + mod_delayed_work(system_power_efficient_wq, &data->detect, DEBOUNCE_TIME);
>
> return 0;
> }
> @@ -465,7 +465,7 @@ static int sun4i_usb_phy_power_off(struct phy *_phy)
> * Vbus gpio to not trigger an edge irq on Vbus off, so force a rescan.
> */
> if (phy->index == 0 && !sun4i_usb_phy0_poll(data))
> - mod_delayed_work(system_wq, &data->detect, POLL_TIME);
> + mod_delayed_work(system_power_efficient_wq, &data->detect, POLL_TIME);
>
> return 0;
> }
> @@ -504,7 +504,7 @@ static int sun4i_usb_phy_set_mode(struct phy *_phy,
>
> data->id_det = -1; /* Force reprocessing of id */
> data->force_session_end = true;
> - queue_delayed_work(system_wq, &data->detect, 0);
> + queue_delayed_work(system_power_efficient_wq, &data->detect, 0);
>
> return 0;
> }
> @@ -616,7 +616,7 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
> extcon_set_state_sync(data->extcon, EXTCON_USB, vbus_det);
>
> if (sun4i_usb_phy0_poll(data))
> - queue_delayed_work(system_wq, &data->detect, POLL_TIME);
> + queue_delayed_work(system_power_efficient_wq, &data->detect, POLL_TIME);
> }
>
> static irqreturn_t sun4i_usb_phy0_id_vbus_det_irq(int irq, void *dev_id)
> @@ -624,7 +624,7 @@ static irqreturn_t sun4i_usb_phy0_id_vbus_det_irq(int irq, void *dev_id)
> struct sun4i_usb_phy_data *data = dev_id;
>
> /* vbus or id changed, let the pins settle and then scan them */
> - mod_delayed_work(system_wq, &data->detect, DEBOUNCE_TIME);
> + mod_delayed_work(system_power_efficient_wq, &data->detect, DEBOUNCE_TIME);
>
> return IRQ_HANDLED;
> }
> @@ -638,7 +638,7 @@ static int sun4i_usb_phy0_vbus_notify(struct notifier_block *nb,
>
> /* Properties on the vbus_power_supply changed, scan vbus_det */
> if (val == PSY_EVENT_PROP_CHANGED && psy == data->vbus_power_supply)
> - mod_delayed_work(system_wq, &data->detect, DEBOUNCE_TIME);
> + mod_delayed_work(system_power_efficient_wq, &data->detect, DEBOUNCE_TIME);
>
> return NOTIFY_OK;
> }
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] phy: sun4i-usb: Use power efficient workqueue for debounce and poll
@ 2020-11-11 3:44 ` Samuel Holland
0 siblings, 0 replies; 14+ messages in thread
From: Samuel Holland @ 2020-11-11 3:44 UTC (permalink / raw)
To: Frank Lee, vkoul, mripard, wens, krzk, colin.king, tiny.windzz
Cc: linux-kernel, linux-arm-kernel
On 11/9/20 6:12 AM, Frank Lee wrote:
> From: Yangtao Li <frank@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 <frank@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?
> if (phy->index == 0 && sun4i_usb_phy0_poll(data))
> - mod_delayed_work(system_wq, &data->detect, DEBOUNCE_TIME);
> + mod_delayed_work(system_power_efficient_wq, &data->detect, DEBOUNCE_TIME);
>
> return 0;
> }
> @@ -465,7 +465,7 @@ static int sun4i_usb_phy_power_off(struct phy *_phy)
> * Vbus gpio to not trigger an edge irq on Vbus off, so force a rescan.
> */
> if (phy->index == 0 && !sun4i_usb_phy0_poll(data))
> - mod_delayed_work(system_wq, &data->detect, POLL_TIME);
> + mod_delayed_work(system_power_efficient_wq, &data->detect, POLL_TIME);
>
> return 0;
> }
> @@ -504,7 +504,7 @@ static int sun4i_usb_phy_set_mode(struct phy *_phy,
>
> data->id_det = -1; /* Force reprocessing of id */
> data->force_session_end = true;
> - queue_delayed_work(system_wq, &data->detect, 0);
> + queue_delayed_work(system_power_efficient_wq, &data->detect, 0);
>
> return 0;
> }
> @@ -616,7 +616,7 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
> extcon_set_state_sync(data->extcon, EXTCON_USB, vbus_det);
>
> if (sun4i_usb_phy0_poll(data))
> - queue_delayed_work(system_wq, &data->detect, POLL_TIME);
> + queue_delayed_work(system_power_efficient_wq, &data->detect, POLL_TIME);
> }
>
> static irqreturn_t sun4i_usb_phy0_id_vbus_det_irq(int irq, void *dev_id)
> @@ -624,7 +624,7 @@ static irqreturn_t sun4i_usb_phy0_id_vbus_det_irq(int irq, void *dev_id)
> struct sun4i_usb_phy_data *data = dev_id;
>
> /* vbus or id changed, let the pins settle and then scan them */
> - mod_delayed_work(system_wq, &data->detect, DEBOUNCE_TIME);
> + mod_delayed_work(system_power_efficient_wq, &data->detect, DEBOUNCE_TIME);
>
> return IRQ_HANDLED;
> }
> @@ -638,7 +638,7 @@ static int sun4i_usb_phy0_vbus_notify(struct notifier_block *nb,
>
> /* Properties on the vbus_power_supply changed, scan vbus_det */
> if (val == PSY_EVENT_PROP_CHANGED && psy == data->vbus_power_supply)
> - mod_delayed_work(system_wq, &data->detect, DEBOUNCE_TIME);
> + mod_delayed_work(system_power_efficient_wq, &data->detect, DEBOUNCE_TIME);
>
> return NOTIFY_OK;
> }
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] phy: sun4i-usb: Use power efficient workqueue for debounce and poll
2020-11-11 3:44 ` Samuel Holland
@ 2020-11-11 5:54 ` Frank Lee
-1 siblings, 0 replies; 14+ messages in thread
From: Frank Lee @ 2020-11-11 5:54 UTC (permalink / raw)
To: Samuel Holland
Cc: Frank Lee, vkoul, Maxime Ripard, Chen-Yu Tsai,
Krzysztof Kozlowski, Colin King, Linux Kernel Mailing List,
Linux ARM
HI,
On Wed, Nov 11, 2020 at 11:44 AM Samuel Holland <samuel@sholland.org> wrote:
>
> On 11/9/20 6:12 AM, Frank Lee wrote:
> > From: Yangtao Li <frank@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 <frank@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?
According to my understanding, the more meaning of performance here
comes from cache locality.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] phy: sun4i-usb: Use power efficient workqueue for debounce and poll
@ 2020-11-11 5:54 ` Frank Lee
0 siblings, 0 replies; 14+ messages in thread
From: Frank Lee @ 2020-11-11 5:54 UTC (permalink / raw)
To: Samuel Holland
Cc: Chen-Yu Tsai, Krzysztof Kozlowski, Linux Kernel Mailing List,
vkoul, Maxime Ripard, Frank Lee, Colin King, Linux ARM
HI,
On Wed, Nov 11, 2020 at 11:44 AM Samuel Holland <samuel@sholland.org> wrote:
>
> On 11/9/20 6:12 AM, Frank Lee wrote:
> > From: Yangtao Li <frank@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 <frank@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?
According to my understanding, the more meaning of performance here
comes from cache locality.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] phy: sun4i-usb: Use power efficient workqueue for debounce and poll
2020-11-11 3:44 ` Samuel Holland
@ 2020-11-12 9:53 ` Maxime Ripard
-1 siblings, 0 replies; 14+ messages in thread
From: Maxime Ripard @ 2020-11-12 9:53 UTC (permalink / raw)
To: Samuel Holland
Cc: Frank Lee, vkoul, wens, krzk, colin.king, tiny.windzz,
linux-kernel, linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 2293 bytes --]
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@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 <frank@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
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] phy: sun4i-usb: Use power efficient workqueue for debounce and poll
@ 2020-11-12 9:53 ` Maxime Ripard
0 siblings, 0 replies; 14+ messages in thread
From: Maxime Ripard @ 2020-11-12 9:53 UTC (permalink / raw)
To: Samuel Holland
Cc: tiny.windzz, wens, krzk, linux-kernel, vkoul, Frank Lee,
colin.king, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 2293 bytes --]
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@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 <frank@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
Maxime
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] phy: sun4i-usb: Use power efficient workqueue for debounce and poll
2020-11-12 9:53 ` Maxime Ripard
@ 2020-11-17 4:48 ` Samuel Holland
-1 siblings, 0 replies; 14+ messages in thread
From: Samuel Holland @ 2020-11-17 4:48 UTC (permalink / raw)
To: Maxime Ripard
Cc: Frank Lee, vkoul, wens, krzk, colin.king, tiny.windzz,
linux-kernel, linux-arm-kernel
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 <frank@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 <frank@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 <samuel@sholland.org>
Cheers,
Samuel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] phy: sun4i-usb: Use power efficient workqueue for debounce and poll
@ 2020-11-17 4:48 ` Samuel Holland
0 siblings, 0 replies; 14+ messages in thread
From: Samuel Holland @ 2020-11-17 4:48 UTC (permalink / raw)
To: Maxime Ripard
Cc: tiny.windzz, wens, krzk, linux-kernel, vkoul, Frank Lee,
colin.king, linux-arm-kernel
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 <frank@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 <frank@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 <samuel@sholland.org>
Cheers,
Samuel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] phy: sun4i-usb: Use power efficient workqueue for debounce and poll
2020-11-09 12:12 ` Frank Lee
@ 2020-11-20 10:08 ` Vinod Koul
-1 siblings, 0 replies; 14+ messages in thread
From: Vinod Koul @ 2020-11-20 10:08 UTC (permalink / raw)
To: Frank Lee
Cc: mripard, wens, krzk, colin.king, tiny.windzz, linux-kernel,
linux-arm-kernel
On 09-11-20, 20:12, Frank Lee wrote:
> From: Yangtao Li <frank@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.
Applied, thanks
--
~Vinod
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] phy: sun4i-usb: Use power efficient workqueue for debounce and poll
@ 2020-11-20 10:08 ` Vinod Koul
0 siblings, 0 replies; 14+ messages in thread
From: Vinod Koul @ 2020-11-20 10:08 UTC (permalink / raw)
To: Frank Lee
Cc: tiny.windzz, linux-kernel, krzk, wens, mripard, colin.king,
linux-arm-kernel
On 09-11-20, 20:12, Frank Lee wrote:
> From: Yangtao Li <frank@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.
Applied, thanks
--
~Vinod
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread