| Message ID | 20240420044901.884098-7-quic_kriskura@quicinc.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6410c8033ba74bf6be19515be5164161abd5ef63 |
| Headers | show |
| Series | Add multiport support for DWC3 controllers | expand |
On Sat, Apr 20, 2024 at 10:18:58AM +0530, Krishna Kurapati wrote: > The logic for requesting interrupts is duplicated for each interrupt. In > the upcoming patches that introduces support for multiport, it would be > better to clean up the duplication before reading mulitport related > interrupts. > > Refactor interrupt setup call by adding a new helper function for > requesting the wakeup interrupts. To simplify implementation, make > the display name same as the interrupt name expected in Device tree. > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com> As far I can see, you only replaced "DT" with "Device tree" in the commit message. For changes like that you could have kept my Reviewed-by tag (but I appreciate that you dropped it from some of the others): Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
On Sat, Apr 20, 2024, Krishna Kurapati wrote: > The logic for requesting interrupts is duplicated for each interrupt. In > the upcoming patches that introduces support for multiport, it would be > better to clean up the duplication before reading mulitport related > interrupts. > > Refactor interrupt setup call by adding a new helper function for > requesting the wakeup interrupts. To simplify implementation, make > the display name same as the interrupt name expected in Device tree. > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com> > --- > drivers/usb/dwc3/dwc3-qcom.c | 53 ++++++++++++++++-------------------- > 1 file changed, 24 insertions(+), 29 deletions(-) > > diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c > index f6b2fab49d5e..cae5dab8fcfc 100644 > --- a/drivers/usb/dwc3/dwc3-qcom.c > +++ b/drivers/usb/dwc3/dwc3-qcom.c > @@ -501,6 +501,22 @@ static void dwc3_qcom_select_utmi_clk(struct dwc3_qcom *qcom) > PIPE_UTMI_CLK_DIS); > } > > +static int dwc3_qcom_request_irq(struct dwc3_qcom *qcom, int irq, > + const char *name) > +{ > + int ret; > + > + /* Keep wakeup interrupts disabled until suspend */ > + ret = devm_request_threaded_irq(qcom->dev, irq, NULL, > + qcom_dwc3_resume_irq, > + IRQF_ONESHOT | IRQF_NO_AUTOEN, > + name, qcom); > + if (ret) > + dev_err(qcom->dev, "failed to request irq %s: %d\n", name, ret); > + > + return ret; > +} > + > static int dwc3_qcom_setup_irq(struct platform_device *pdev) > { > struct dwc3_qcom *qcom = platform_get_drvdata(pdev); > @@ -509,54 +525,33 @@ static int dwc3_qcom_setup_irq(struct platform_device *pdev) > > irq = platform_get_irq_byname_optional(pdev, "qusb2_phy"); > if (irq > 0) { > - /* Keep wakeup interrupts disabled until suspend */ > - ret = devm_request_threaded_irq(qcom->dev, irq, NULL, > - qcom_dwc3_resume_irq, > - IRQF_ONESHOT | IRQF_NO_AUTOEN, > - "qcom_dwc3 QUSB2", qcom); > - if (ret) { > - dev_err(qcom->dev, "qusb2_phy_irq failed: %d\n", ret); > + ret = dwc3_qcom_request_irq(qcom, irq, "qusb2_phy"); > + if (ret) > return ret; > - } > qcom->qusb2_phy_irq = irq; > } > > irq = platform_get_irq_byname_optional(pdev, "dp_hs_phy_irq"); > if (irq > 0) { > - ret = devm_request_threaded_irq(qcom->dev, irq, NULL, > - qcom_dwc3_resume_irq, > - IRQF_ONESHOT | IRQF_NO_AUTOEN, > - "qcom_dwc3 DP_HS", qcom); > - if (ret) { > - dev_err(qcom->dev, "dp_hs_phy_irq failed: %d\n", ret); > + ret = dwc3_qcom_request_irq(qcom, irq, "dp_hs_phy_irq"); > + if (ret) > return ret; > - } > qcom->dp_hs_phy_irq = irq; > } > > irq = platform_get_irq_byname_optional(pdev, "dm_hs_phy_irq"); > if (irq > 0) { > - ret = devm_request_threaded_irq(qcom->dev, irq, NULL, > - qcom_dwc3_resume_irq, > - IRQF_ONESHOT | IRQF_NO_AUTOEN, > - "qcom_dwc3 DM_HS", qcom); > - if (ret) { > - dev_err(qcom->dev, "dm_hs_phy_irq failed: %d\n", ret); > + ret = dwc3_qcom_request_irq(qcom, irq, "dm_hs_phy_irq"); > + if (ret) > return ret; > - } > qcom->dm_hs_phy_irq = irq; > } > > irq = platform_get_irq_byname_optional(pdev, "ss_phy_irq"); > if (irq > 0) { > - ret = devm_request_threaded_irq(qcom->dev, irq, NULL, > - qcom_dwc3_resume_irq, > - IRQF_ONESHOT | IRQF_NO_AUTOEN, > - "qcom_dwc3 SS", qcom); > - if (ret) { > - dev_err(qcom->dev, "ss_phy_irq failed: %d\n", ret); > + ret = dwc3_qcom_request_irq(qcom, irq, "ss_phy_irq"); > + if (ret) > return ret; > - } > qcom->ss_phy_irq = irq; > } > > -- > 2.34.1 > Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> Thanks, Thinh
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c index f6b2fab49d5e..cae5dab8fcfc 100644 --- a/drivers/usb/dwc3/dwc3-qcom.c +++ b/drivers/usb/dwc3/dwc3-qcom.c @@ -501,6 +501,22 @@ static void dwc3_qcom_select_utmi_clk(struct dwc3_qcom *qcom) PIPE_UTMI_CLK_DIS); } +static int dwc3_qcom_request_irq(struct dwc3_qcom *qcom, int irq, + const char *name) +{ + int ret; + + /* Keep wakeup interrupts disabled until suspend */ + ret = devm_request_threaded_irq(qcom->dev, irq, NULL, + qcom_dwc3_resume_irq, + IRQF_ONESHOT | IRQF_NO_AUTOEN, + name, qcom); + if (ret) + dev_err(qcom->dev, "failed to request irq %s: %d\n", name, ret); + + return ret; +} + static int dwc3_qcom_setup_irq(struct platform_device *pdev) { struct dwc3_qcom *qcom = platform_get_drvdata(pdev); @@ -509,54 +525,33 @@ static int dwc3_qcom_setup_irq(struct platform_device *pdev) irq = platform_get_irq_byname_optional(pdev, "qusb2_phy"); if (irq > 0) { - /* Keep wakeup interrupts disabled until suspend */ - ret = devm_request_threaded_irq(qcom->dev, irq, NULL, - qcom_dwc3_resume_irq, - IRQF_ONESHOT | IRQF_NO_AUTOEN, - "qcom_dwc3 QUSB2", qcom); - if (ret) { - dev_err(qcom->dev, "qusb2_phy_irq failed: %d\n", ret); + ret = dwc3_qcom_request_irq(qcom, irq, "qusb2_phy"); + if (ret) return ret; - } qcom->qusb2_phy_irq = irq; } irq = platform_get_irq_byname_optional(pdev, "dp_hs_phy_irq"); if (irq > 0) { - ret = devm_request_threaded_irq(qcom->dev, irq, NULL, - qcom_dwc3_resume_irq, - IRQF_ONESHOT | IRQF_NO_AUTOEN, - "qcom_dwc3 DP_HS", qcom); - if (ret) { - dev_err(qcom->dev, "dp_hs_phy_irq failed: %d\n", ret); + ret = dwc3_qcom_request_irq(qcom, irq, "dp_hs_phy_irq"); + if (ret) return ret; - } qcom->dp_hs_phy_irq = irq; } irq = platform_get_irq_byname_optional(pdev, "dm_hs_phy_irq"); if (irq > 0) { - ret = devm_request_threaded_irq(qcom->dev, irq, NULL, - qcom_dwc3_resume_irq, - IRQF_ONESHOT | IRQF_NO_AUTOEN, - "qcom_dwc3 DM_HS", qcom); - if (ret) { - dev_err(qcom->dev, "dm_hs_phy_irq failed: %d\n", ret); + ret = dwc3_qcom_request_irq(qcom, irq, "dm_hs_phy_irq"); + if (ret) return ret; - } qcom->dm_hs_phy_irq = irq; } irq = platform_get_irq_byname_optional(pdev, "ss_phy_irq"); if (irq > 0) { - ret = devm_request_threaded_irq(qcom->dev, irq, NULL, - qcom_dwc3_resume_irq, - IRQF_ONESHOT | IRQF_NO_AUTOEN, - "qcom_dwc3 SS", qcom); - if (ret) { - dev_err(qcom->dev, "ss_phy_irq failed: %d\n", ret); + ret = dwc3_qcom_request_irq(qcom, irq, "ss_phy_irq"); + if (ret) return ret; - } qcom->ss_phy_irq = irq; }