Thread (22 messages) 22 messages, 4 authors, 2021-02-12

Re: [PATCH v6 09/10] usb: dwc3: qcom: Detect DWC3 DT-nodes with "usb"-prefixed names

From: Bjorn Andersson <hidden>
Date: 2021-02-12 18:02:19
Also in: linux-arm-msm, lkml

On Fri 12 Feb 11:33 CST 2021, Serge Semin wrote:
On Wed, Feb 10, 2021 at 10:33:26PM +0300, Serge Semin wrote:
quoted
On Wed, Feb 10, 2021 at 12:56:59PM -0600, Bjorn Andersson wrote:
quoted
On Wed 10 Feb 12:40 CST 2021, Serge Semin wrote:
quoted
On Wed, Feb 10, 2021 at 12:17:27PM -0600, Rob Herring wrote:
quoted
On Wed, Feb 10, 2021 at 11:29 AM Serge Semin
[off-list ref] wrote:
quoted
In accordance with the USB HCD/DRD schema all the USB controllers are
supposed to have DT-nodes named with prefix "^usb(@.*)?".  Since the
existing DT-nodes will be renamed in a subsequent patch let's first make
sure the DWC3 Qualcomm driver supports them and second falls back to the
deprecated naming so not to fail on the legacy DTS-files passed to the
newer kernels.

Signed-off-by: Serge Semin <redacted>
Reviewed-by: Bjorn Andersson <redacted>
---
 drivers/usb/dwc3/dwc3-qcom.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index c703d552bbcf..49ad8d507d37 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -630,7 +630,8 @@ static int dwc3_qcom_of_register_core(struct platform_device *pdev)
        struct device           *dev = &pdev->dev;
        int                     ret;

-       dwc3_np = of_get_child_by_name(np, "dwc3");
+       dwc3_np = of_get_child_by_name(np, "usb") ?:
+                 of_get_child_by_name(np, "dwc3");
quoted
Is there some reason using compatible instead wouldn't work here?
I don't know for sure. The fix has been requested in the framework of
this discussion:
https://lore.kernel.org/linux-usb/20201020115959.2658-30-Sergey.Semin@baikalelectronics.ru/#t (local)
by the driver maintainer Bjorn. To get a firm answer it's better to
have him asked.
My feedback was simply that it has to catch both cases, I didn't
consider the fact that we have a compatible to match against.
quoted
As I see it having of_get_compatible_child() utilized
here would also work. At least for the available in kernel dt-files.
See the affected dts-es in:
https://lore.kernel.org/linux-usb/20210210172850.20849-11-Sergey.Semin@baikalelectronics.ru/ (local)

A problem may happen if some older versions of DTS-es had another
compatible string in the dwc3 sub-node...
Afaict all Qualcomm dts files has "snps,dwc3", so you can match against
that instead.
Ok then. I'll replace of_get_child_by_name() here with
of_get_compatible_child() matching just against "snps,dwc3" in v7. Can you
confirm that noone ever had a Qcom-based hardware described with dts having
the "synopsys,dwc3" compatible used as the DWC USB3 sub-node here? That
string has been marked as deprecated recently because the vendor-prefix
was changed sometime ago, but the original driver still accept it.

Alternatively to be on a safe side we could match against both
compatibles here as Rob suggests. What do you think?
Bjorn, any comment on the question above? So I could respin the series
with this patch updated.

Also note, since the patch's gonna be changed I'll have to remove your
Reviewed-by tag unless u explicitly say I shouldn't.
Sounds good, I'd be happy to review the new patch.

PS. As this only affect the Qualcomm part of the series I would suggest
that you send these patches off separate and don't send all 10 patches
together.

Thanks,
Bjorn
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help