Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 3/3] phy: sun4i-usb: Use power efficient workqueue for
 debounce and poll
To:     Frank Lee <frank@allwinnertech.com>, vkoul@kernel.org,
        mripard@kernel.org, wens@csie.org, krzk@kernel.org,
        colin.king@canonical.com, tiny.windzz@gmail.com
Cc:     linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org
References: <20201109121214.19012-1-frank@allwinnertech.com>
From:   Samuel Holland <samuel@sholland.org>
Message-ID: <fba49549-9a78-32ee-a55c-97499f24fe62@sholland.org>
Date:   Tue, 10 Nov 2020 21:44:37 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.6.0
MIME-Version: 1.0
In-Reply-To: <20201109121214.19012-1-frank@allwinnertech.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

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;
>  }
>