Re: [PATCH v20 6/9] usb: dwc3: qcom: Add helper function to request wakeup interrupts

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

On Mon, Apr 08, 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 DT.
> 
> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
> Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  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