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,
