diff mbox series

[v2,5/7] usb: dwc3: qcom: Don't reply on drvdata during probe

Message ID 20240811-dwc3-refactor-v2-5-91f370d61ad2@quicinc.com (mailing list archive)
State New
Headers show
Series usb: dwc3: qcom: Flatten dwc3 structure | expand

Commit Message

Bjorn Andersson Aug. 12, 2024, 3:12 a.m. UTC 
From: Bjorn Andersson <quic_bjorande@quicinc.com>

With the upcoming transition to a model where DWC3 core and glue operate
on a single struct device the drvdata datatype will change to be owned
by the core.

The drvdata is however used by the Qualcomm DWC3 glue to pass the qcom
glue context around before the core is allocated.

Remove this problem, and clean up the code, by passing the dwc3_qcom
struct around during probe, instead of acquiring it from the drvdata.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 drivers/usb/dwc3/dwc3-qcom.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

Comments

Frank Li Aug. 13, 2024, 6:18 p.m. UTC | #1 
On Sun, Aug 11, 2024 at 08:12:02PM -0700, Bjorn Andersson wrote:
> From: Bjorn Andersson <quic_bjorande@quicinc.com>
>
> With the upcoming transition to a model where DWC3 core and glue operate
> on a single struct device the drvdata datatype will change to be owned
> by the core.
>
> The drvdata is however used by the Qualcomm DWC3 glue to pass the qcom
> glue context around before the core is allocated.
>
> Remove this problem, and clean up the code, by passing the dwc3_qcom
> struct around during probe, instead of acquiring it from the drvdata.
>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 88fb6706a18d..33de03f2d782 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -546,9 +546,10 @@ static int dwc3_qcom_request_irq(struct dwc3_qcom *qcom, int irq,
>  	return ret;
>  }
>
> -static int dwc3_qcom_setup_port_irq(struct platform_device *pdev, int port_index, bool is_multiport)
> +static int dwc3_qcom_setup_port_irq(struct dwc3_qcom *qcom,

If pass "qcom", do you need "pdev"? generaly, qcom should have pdev information.

Frank
> +				    struct platform_device *pdev,
> +				    int port_index, bool is_multiport)
>  {
> -	struct dwc3_qcom *qcom = platform_get_drvdata(pdev);
>  	const char *irq_name;
>  	int irq;
>  	int ret;
> @@ -633,9 +634,8 @@ static int dwc3_qcom_find_num_ports(struct platform_device *pdev)
>  	return DWC3_QCOM_MAX_PORTS;
>  }
>
> -static int dwc3_qcom_setup_irq(struct platform_device *pdev)
> +static int dwc3_qcom_setup_irq(struct dwc3_qcom *qcom, struct platform_device *pdev)
>  {
> -	struct dwc3_qcom *qcom = platform_get_drvdata(pdev);
>  	bool is_multiport;
>  	int ret;
>  	int i;
> @@ -644,7 +644,7 @@ static int dwc3_qcom_setup_irq(struct platform_device *pdev)
>  	is_multiport = (qcom->num_ports > 1);
>
>  	for (i = 0; i < qcom->num_ports; i++) {
> -		ret = dwc3_qcom_setup_port_irq(pdev, i, is_multiport);
> +		ret = dwc3_qcom_setup_port_irq(qcom, pdev, i, is_multiport);
>  		if (ret)
>  			return ret;
>  	}
> @@ -699,9 +699,8 @@ static int dwc3_qcom_clk_init(struct dwc3_qcom *qcom, int count)
>  	return 0;
>  }
>
> -static int dwc3_qcom_of_register_core(struct platform_device *pdev)
> +static int dwc3_qcom_of_register_core(struct dwc3_qcom *qcom, struct platform_device *pdev)
>  {
> -	struct dwc3_qcom	*qcom = platform_get_drvdata(pdev);
>  	struct device_node	*np = pdev->dev.of_node, *dwc3_np;
>  	struct device		*dev = &pdev->dev;
>  	int			ret;
> @@ -782,7 +781,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  		goto clk_disable;
>  	}
>
> -	ret = dwc3_qcom_setup_irq(pdev);
> +	ret = dwc3_qcom_setup_irq(qcom, pdev);
>  	if (ret) {
>  		dev_err(dev, "failed to setup IRQs, err=%d\n", ret);
>  		goto clk_disable;
> @@ -797,7 +796,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  	if (ignore_pipe_clk)
>  		dwc3_qcom_select_utmi_clk(qcom);
>
> -	ret = dwc3_qcom_of_register_core(pdev);
> +	ret = dwc3_qcom_of_register_core(qcom, pdev);
>  	if (ret) {
>  		dev_err(dev, "failed to register DWC3 Core, err=%d\n", ret);
>  		goto clk_disable;
>
> --
> 2.45.2
>
Bjorn Andersson Aug. 19, 2024, 9:17 p.m. UTC | #2 
On Tue, Aug 13, 2024 at 02:18:44PM -0400, Frank Li wrote:
> On Sun, Aug 11, 2024 at 08:12:02PM -0700, Bjorn Andersson wrote:
> > From: Bjorn Andersson <quic_bjorande@quicinc.com>
> >
> > With the upcoming transition to a model where DWC3 core and glue operate
> > on a single struct device the drvdata datatype will change to be owned
> > by the core.
> >
> > The drvdata is however used by the Qualcomm DWC3 glue to pass the qcom
> > glue context around before the core is allocated.
> >
> > Remove this problem, and clean up the code, by passing the dwc3_qcom
> > struct around during probe, instead of acquiring it from the drvdata.
> >
> > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> > ---
> >  drivers/usb/dwc3/dwc3-qcom.c | 17 ++++++++---------
> >  1 file changed, 8 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> > index 88fb6706a18d..33de03f2d782 100644
> > --- a/drivers/usb/dwc3/dwc3-qcom.c
> > +++ b/drivers/usb/dwc3/dwc3-qcom.c
> > @@ -546,9 +546,10 @@ static int dwc3_qcom_request_irq(struct dwc3_qcom *qcom, int irq,
> >  	return ret;
> >  }
> >
> > -static int dwc3_qcom_setup_port_irq(struct platform_device *pdev, int port_index, bool is_multiport)
> > +static int dwc3_qcom_setup_port_irq(struct dwc3_qcom *qcom,
> 
> If pass "qcom", do you need "pdev"? generaly, qcom should have pdev information.
> 

We're only carrying the struct device reference in the dwc3_qcom struct,
as we don't have a use for the platform_device reference beyond probe.

Regards,
Bjorn
Masahiro Yamada Sept. 12, 2024, 10:21 p.m. UTC | #3 
Regarding the patch subject,
do you mean "Don't rely on drvdata" instead of
"Don't reply on drvdata"?







On Mon, Aug 12, 2024 at 12:07 PM Bjorn Andersson <andersson@kernel.org> wrote:
>
> From: Bjorn Andersson <quic_bjorande@quicinc.com>
>
> With the upcoming transition to a model where DWC3 core and glue operate
> on a single struct device the drvdata datatype will change to be owned
> by the core.
>
> The drvdata is however used by the Qualcomm DWC3 glue to pass the qcom
> glue context around before the core is allocated.
>
> Remove this problem, and clean up the code, by passing the dwc3_qcom
> struct around during probe, instead of acquiring it from the drvdata.
>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 88fb6706a18d..33de03f2d782 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -546,9 +546,10 @@ static int dwc3_qcom_request_irq(struct dwc3_qcom *qcom, int irq,
>         return ret;
>  }
>
> -static int dwc3_qcom_setup_port_irq(struct platform_device *pdev, int port_index, bool is_multiport)
> +static int dwc3_qcom_setup_port_irq(struct dwc3_qcom *qcom,
> +                                   struct platform_device *pdev,
> +                                   int port_index, bool is_multiport)
>  {
> -       struct dwc3_qcom *qcom = platform_get_drvdata(pdev);
>         const char *irq_name;
>         int irq;
>         int ret;
> @@ -633,9 +634,8 @@ static int dwc3_qcom_find_num_ports(struct platform_device *pdev)
>         return DWC3_QCOM_MAX_PORTS;
>  }
>
> -static int dwc3_qcom_setup_irq(struct platform_device *pdev)
> +static int dwc3_qcom_setup_irq(struct dwc3_qcom *qcom, struct platform_device *pdev)
>  {
> -       struct dwc3_qcom *qcom = platform_get_drvdata(pdev);
>         bool is_multiport;
>         int ret;
>         int i;
> @@ -644,7 +644,7 @@ static int dwc3_qcom_setup_irq(struct platform_device *pdev)
>         is_multiport = (qcom->num_ports > 1);
>
>         for (i = 0; i < qcom->num_ports; i++) {
> -               ret = dwc3_qcom_setup_port_irq(pdev, i, is_multiport);
> +               ret = dwc3_qcom_setup_port_irq(qcom, pdev, i, is_multiport);
>                 if (ret)
>                         return ret;
>         }
> @@ -699,9 +699,8 @@ static int dwc3_qcom_clk_init(struct dwc3_qcom *qcom, int count)
>         return 0;
>  }
>
> -static int dwc3_qcom_of_register_core(struct platform_device *pdev)
> +static int dwc3_qcom_of_register_core(struct dwc3_qcom *qcom, struct platform_device *pdev)
>  {
> -       struct dwc3_qcom        *qcom = platform_get_drvdata(pdev);
>         struct device_node      *np = pdev->dev.of_node, *dwc3_np;
>         struct device           *dev = &pdev->dev;
>         int                     ret;
> @@ -782,7 +781,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>                 goto clk_disable;
>         }
>
> -       ret = dwc3_qcom_setup_irq(pdev);
> +       ret = dwc3_qcom_setup_irq(qcom, pdev);
>         if (ret) {
>                 dev_err(dev, "failed to setup IRQs, err=%d\n", ret);
>                 goto clk_disable;
> @@ -797,7 +796,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>         if (ignore_pipe_clk)
>                 dwc3_qcom_select_utmi_clk(qcom);
>
> -       ret = dwc3_qcom_of_register_core(pdev);
> +       ret = dwc3_qcom_of_register_core(qcom, pdev);
>         if (ret) {
>                 dev_err(dev, "failed to register DWC3 Core, err=%d\n", ret);
>                 goto clk_disable;
>
> --
> 2.45.2
>
>


--
Best Regards
Masahiro Yamada
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 88fb6706a18d..33de03f2d782 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -546,9 +546,10 @@  static int dwc3_qcom_request_irq(struct dwc3_qcom *qcom, int irq,
 	return ret;
 }
 
-static int dwc3_qcom_setup_port_irq(struct platform_device *pdev, int port_index, bool is_multiport)
+static int dwc3_qcom_setup_port_irq(struct dwc3_qcom *qcom,
+				    struct platform_device *pdev,
+				    int port_index, bool is_multiport)
 {
-	struct dwc3_qcom *qcom = platform_get_drvdata(pdev);
 	const char *irq_name;
 	int irq;
 	int ret;
@@ -633,9 +634,8 @@  static int dwc3_qcom_find_num_ports(struct platform_device *pdev)
 	return DWC3_QCOM_MAX_PORTS;
 }
 
-static int dwc3_qcom_setup_irq(struct platform_device *pdev)
+static int dwc3_qcom_setup_irq(struct dwc3_qcom *qcom, struct platform_device *pdev)
 {
-	struct dwc3_qcom *qcom = platform_get_drvdata(pdev);
 	bool is_multiport;
 	int ret;
 	int i;
@@ -644,7 +644,7 @@  static int dwc3_qcom_setup_irq(struct platform_device *pdev)
 	is_multiport = (qcom->num_ports > 1);
 
 	for (i = 0; i < qcom->num_ports; i++) {
-		ret = dwc3_qcom_setup_port_irq(pdev, i, is_multiport);
+		ret = dwc3_qcom_setup_port_irq(qcom, pdev, i, is_multiport);
 		if (ret)
 			return ret;
 	}
@@ -699,9 +699,8 @@  static int dwc3_qcom_clk_init(struct dwc3_qcom *qcom, int count)
 	return 0;
 }
 
-static int dwc3_qcom_of_register_core(struct platform_device *pdev)
+static int dwc3_qcom_of_register_core(struct dwc3_qcom *qcom, struct platform_device *pdev)
 {
-	struct dwc3_qcom	*qcom = platform_get_drvdata(pdev);
 	struct device_node	*np = pdev->dev.of_node, *dwc3_np;
 	struct device		*dev = &pdev->dev;
 	int			ret;
@@ -782,7 +781,7 @@  static int dwc3_qcom_probe(struct platform_device *pdev)
 		goto clk_disable;
 	}
 
-	ret = dwc3_qcom_setup_irq(pdev);
+	ret = dwc3_qcom_setup_irq(qcom, pdev);
 	if (ret) {
 		dev_err(dev, "failed to setup IRQs, err=%d\n", ret);
 		goto clk_disable;
@@ -797,7 +796,7 @@  static int dwc3_qcom_probe(struct platform_device *pdev)
 	if (ignore_pipe_clk)
 		dwc3_qcom_select_utmi_clk(qcom);
 
-	ret = dwc3_qcom_of_register_core(pdev);
+	ret = dwc3_qcom_of_register_core(qcom, pdev);
 	if (ret) {
 		dev_err(dev, "failed to register DWC3 Core, err=%d\n", ret);
 		goto clk_disable;