lkml.org 
[lkml]   [2021]   [Nov]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v9 3/5] usb: dwc3: qcom: Add helper functions to enable,disable wake irqs
On Mon 01 Nov 00:53 PDT 2021, Sandeep Maheswaram wrote:

> Adding helper functions to enable,disable wake irqs to make
> the code simple and readable.
>
> Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> drivers/usb/dwc3/dwc3-qcom.c | 58 ++++++++++++++++++++------------------------
> 1 file changed, 26 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 9abbd01..54461f1 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -296,50 +296,44 @@ static void dwc3_qcom_interconnect_exit(struct dwc3_qcom *qcom)
> icc_put(qcom->icc_path_apps);
> }
>
> +static void dwc3_qcom_enable_wakeup_irq(int irq)
> +{
> + if (!irq)
> + return;
> +
> + enable_irq(irq);
> + enable_irq_wake(irq);
> +}
> +
> +static void dwc3_qcom_disable_wakeup_irq(int irq)
> +{
> + if (!irq)
> + return;
> +
> + disable_irq_wake(irq);

Now that you touch this code path.

I presume keeping these interrupts enabled during runtime would cause
the interrupt to fire during normal operation, but why do we need to
toggle the irq_wake? Can't we just leave that flag set always?

> + disable_irq_nosync(irq);
> +}
> +
> static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)
> {
> - if (qcom->hs_phy_irq) {
> - disable_irq_wake(qcom->hs_phy_irq);
> - disable_irq_nosync(qcom->hs_phy_irq);
> - }
> + dwc3_qcom_disable_wakeup_irq(qcom->hs_phy_irq);

Why don't we just replace *_phy_irq with an array and turn these two
function into two loops? That seems to be quite suitable for the
multi-port dwc found in e.g. sc8180x as well...

Regards,
Bjorn

>
> - if (qcom->dp_hs_phy_irq) {
> - disable_irq_wake(qcom->dp_hs_phy_irq);
> - disable_irq_nosync(qcom->dp_hs_phy_irq);
> - }
> + dwc3_qcom_disable_wakeup_irq(qcom->dp_hs_phy_irq);
>
> - if (qcom->dm_hs_phy_irq) {
> - disable_irq_wake(qcom->dm_hs_phy_irq);
> - disable_irq_nosync(qcom->dm_hs_phy_irq);
> - }
> + dwc3_qcom_disable_wakeup_irq(qcom->dm_hs_phy_irq);
>
> - if (qcom->ss_phy_irq) {
> - disable_irq_wake(qcom->ss_phy_irq);
> - disable_irq_nosync(qcom->ss_phy_irq);
> - }
> + dwc3_qcom_disable_wakeup_irq(qcom->ss_phy_irq);
> }
>
> static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
> {
> - if (qcom->hs_phy_irq) {
> - enable_irq(qcom->hs_phy_irq);
> - enable_irq_wake(qcom->hs_phy_irq);
> - }
> + dwc3_qcom_enable_wakeup_irq(qcom->hs_phy_irq);
>
> - if (qcom->dp_hs_phy_irq) {
> - enable_irq(qcom->dp_hs_phy_irq);
> - enable_irq_wake(qcom->dp_hs_phy_irq);
> - }
> + dwc3_qcom_enable_wakeup_irq(qcom->dp_hs_phy_irq);
>
> - if (qcom->dm_hs_phy_irq) {
> - enable_irq(qcom->dm_hs_phy_irq);
> - enable_irq_wake(qcom->dm_hs_phy_irq);
> - }
> + dwc3_qcom_enable_wakeup_irq(qcom->dm_hs_phy_irq);
>
> - if (qcom->ss_phy_irq) {
> - enable_irq(qcom->ss_phy_irq);
> - enable_irq_wake(qcom->ss_phy_irq);
> - }
> + dwc3_qcom_enable_wakeup_irq(qcom->ss_phy_irq);
> }
>
> static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
> --
> 2.7.4
>

\
 
 \ /
  Last update: 2021-11-01 17:41    [W:0.226 / U:0.696 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site