Re: [PATCH] usb: dwc3: qcom: Remove ACPI support from glue driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 3/5/2024 12:39 PM, Johan Hovold wrote:
On Tue, Mar 05, 2024 at 09:51:43AM +0530, Krishna Kurapati wrote:
Minimal ACPI support was added to the Qualcomm DWC3 glue driver in order to
enable USB on SDM850 and SC8180X compute platforms. The support is still
functional, but unnoticed regressions in other drivers indicates that no
one actually booting any of platforms dependent on this implementation.

The functionality provides is the bare minimum and is not expected to aid
in the effort of bringing full ACPI support to the driver in the future.

Remove the ACPI code from the Qualcomm DWC3 glue driver to aid in the
implementation of improvements that are actually used like multiport and
flattening device tree.

With a simple lookup function that returns the ACPI index based on name
this shouldn't be required to add multiport support even if it may
simplify it slightly. But IIRC it would help more with the devicetree
binding rework.

I agree to both your comments.
Actually both series are equally important to me. Adding a lookup function must ACPI index must help multiport code without this patch as well. But removing this is helping in multiport to write better code and definitely helps in flattening series.

Commit message by Bjorn Andersson.

Signed-off-by: Krishna Kurapati <quic_kriskura@xxxxxxxxxxx>
---
  drivers/usb/dwc3/dwc3-qcom.c | 273 ++---------------------------------
  1 file changed, 11 insertions(+), 262 deletions(-)

You should update the Kconfig entry for USB_DWC3_QCOM as well and drop
the ACPI dependency.

Missed it. Thanks for the catch. The following is the one right ?

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 5fc27b20df63..31078f3d41b8 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -131,7 +131,7 @@ config USB_DWC3_QCOM
        tristate "Qualcomm Platform"
        depends on ARCH_QCOM || COMPILE_TEST
        depends on EXTCON || !EXTCON
-       depends on (OF || ACPI)
+       depends on OF



  static int dwc3_qcom_probe(struct platform_device *pdev)
  {
  	struct device_node	*np = pdev->dev.of_node;
  	struct device		*dev = &pdev->dev;
  	struct dwc3_qcom	*qcom;
  	struct resource		*res, *parent_res = NULL;

You should drop parent_res as well.

Ahh, thanks for the catch.


-	struct resource		local_res;
  	int			ret, i;
  	bool			ignore_pipe_clk;
  	bool			wakeup_source;
@@ -825,14 +659,6 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
  	platform_set_drvdata(pdev, qcom);
  	qcom->dev = &pdev->dev;
- if (has_acpi_companion(dev)) {
-		qcom->acpi_pdata = acpi_device_get_match_data(dev);
-		if (!qcom->acpi_pdata) {
-			dev_err(&pdev->dev, "no supporting ACPI device data\n");
-			return -EINVAL;
-		}
-	}
-
  	qcom->resets = devm_reset_control_array_get_optional_exclusive(dev);
  	if (IS_ERR(qcom->resets)) {
  		return dev_err_probe(&pdev->dev, PTR_ERR(qcom->resets),
@@ -860,41 +686,18 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
  	}
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-
-	if (np) {
-		parent_res = res;
-	} else {
-		memcpy(&local_res, res, sizeof(struct resource));
-		parent_res = &local_res;
-
-		parent_res->start = res->start +
-			qcom->acpi_pdata->qscratch_base_offset;
-		parent_res->end = parent_res->start +
-			qcom->acpi_pdata->qscratch_base_size;
-
-		if (qcom->acpi_pdata->is_urs) {
-			qcom->urs_usb = dwc3_qcom_create_urs_usb_platdev(dev);
-			if (IS_ERR_OR_NULL(qcom->urs_usb)) {
-				dev_err(dev, "failed to create URS USB platdev\n");
-				if (!qcom->urs_usb)
-					ret = -ENODEV;
-				else
-					ret = PTR_ERR(qcom->urs_usb);
-				goto clk_disable;
-			}
-		}
-	}
+	parent_res = res;
qcom->qscratch_base = devm_ioremap_resource(dev, parent_res);

And just use res here.

ACK.


  	if (IS_ERR(qcom->qscratch_base)) {
  		ret = PTR_ERR(qcom->qscratch_base);
-		goto free_urs;
+		goto clk_disable;
}

Looks good to me otherwise.

Thanks for the review. Wanted to reply to your comments on multiport series depending on how this patch goes in upstream.

Can I push out v2 or wait for a couple of days (as a standard practice before putting a new version) ?

Regards,
Krishna,




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux