diff mbox series

[RESEND,v9,5/5] arm64: dts: apm: Harmonize DWC USB3 DT nodes name

Message ID 20220624141622.7149-6-Sergey.Semin@baikalelectronics.ru (mailing list archive)
State Accepted
Commit 87ccc38e2f8e55853ddfe633d9934bc7ca74b21c
Headers show
Series dt-bindings: usb: Harmonize xHCI/EHCI/OHCI/DWC3 nodes name | expand

Commit Message

Serge Semin June 24, 2022, 2:16 p.m. UTC
In accordance with the DWC USB3 bindings the corresponding node
name is suppose to comply with the Generic USB HCD DT schema, which
requires the USB nodes to have the name acceptable by the regexp:
"^usb(@.*)?" . Make sure the "snps,dwc3"-compatible nodes are correctly
named despite of the warning comment about possible backward
compatibility issues.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 arch/arm64/boot/dts/apm/apm-shadowcat.dtsi | 4 ++--
 arch/arm64/boot/dts/apm/apm-storm.dtsi     | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

Comments

Krzysztof Kozlowski June 24, 2022, 5:17 p.m. UTC | #1
On 24/06/2022 16:16, Serge Semin wrote:
> In accordance with the DWC USB3 bindings the corresponding node
> name is suppose to comply with the Generic USB HCD DT schema, which
> requires the USB nodes to have the name acceptable by the regexp:
> "^usb(@.*)?" . Make sure the "snps,dwc3"-compatible nodes are correctly
> named despite of the warning comment about possible backward
> compatibility issues.

Sometimes node name is exposed to user-space which depends on it. How
did you check there is no issue here?

> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  arch/arm64/boot/dts/apm/apm-shadowcat.dtsi | 4 ++--
>  arch/arm64/boot/dts/apm/apm-storm.dtsi     | 6 +++---
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
> index a83c82c50e29..832dd85b00bd 100644
> --- a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
> +++ b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
> @@ -597,8 +597,8 @@ serial0: serial@10600000 {
>  			interrupts = <0x0 0x4c 0x4>;
>  		};
>  
> -		/* Do not change dwusb name, coded for backward compatibility */
> -		usb0: dwusb@19000000 {
> +		/* Node-name might need to be coded as dwusb for backward compatibility */
> +		usb0: usb@19000000 {
>  			status = "disabled";
>  			compatible = "snps,dwc3";
>  			reg =  <0x0 0x19000000 0x0 0x100000>;
> diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi b/arch/arm64/boot/dts/apm/apm-storm.dtsi
> index 0f37e77f5459..1520a945b7f9 100644
> --- a/arch/arm64/boot/dts/apm/apm-storm.dtsi
> +++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi
> @@ -923,8 +923,8 @@ sata3: sata@1a800000 {
>  			phy-names = "sata-phy";
>  		};
>  
> -		/* Do not change dwusb name, coded for backward compatibility */
> -		usb0: dwusb@19000000 {
> +		/* Node-name might need to be coded as dwusb for backward compatibility */
> +		usb0: usb@19000000 {
>  			status = "disabled";
>  			compatible = "snps,dwc3";
>  			reg =  <0x0 0x19000000 0x0 0x100000>;
> @@ -933,7 +933,7 @@ usb0: dwusb@19000000 {
>  			dr_mode = "host";
>  		};
>  
> -		usb1: dwusb@19800000 {
> +		usb1: usb@19800000 {
>  			status = "disabled";
>  			compatible = "snps,dwc3";
>  			reg =  <0x0 0x19800000 0x0 0x100000>;


Best regards,
Krzysztof
Serge Semin June 24, 2022, 8:59 p.m. UTC | #2
On Fri, Jun 24, 2022 at 07:17:53PM +0200, Krzysztof Kozlowski wrote:
> On 24/06/2022 16:16, Serge Semin wrote:
> > In accordance with the DWC USB3 bindings the corresponding node
> > name is suppose to comply with the Generic USB HCD DT schema, which
> > requires the USB nodes to have the name acceptable by the regexp:
> > "^usb(@.*)?" . Make sure the "snps,dwc3"-compatible nodes are correctly
> > named despite of the warning comment about possible backward
> > compatibility issues.
> 

> Sometimes node name is exposed to user-space which depends on it. How
> did you check there is no issue here?

I well remember the Qcom problem caused by one of my patch:
https://lore.kernel.org/lkml/CALAqxLX_FNvFndEDWtGbFPjSzuAbfqxQE07diBJFZtftwEJX5A@mail.gmail.com/

The next patch caused the same problem, but hasn't been reverted.
https://lore.kernel.org/lkml/CALAqxLWGujgR7p8Vb5S_RimRVYxwm5XF-c4NkKgMH-43wEBaWg@mail.gmail.com/

As before I am more inclined to thinking that the problem was mainly caused
by the improper node-name utilization. Anyway John later noted that the
problem was fixed in the user-space. That why afterwards you were able
to provide the commit b77a1c4d6b05 ("arm64: dts: qcom: correct DWC3
node names and unit addresses").

Anyway I am not able to track the way the node-name is used on the
affected platform and can't make sure that the dts would be still
working well on that devices. But seeing nobody responded/commented on
this patch for more than a year we can at least try to merge this in
and see whether it causes any problem should the denoted platform is
still in use. If it does we can revert the update back and forget
about it.

-Sergey

> 
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
> > ---
> >  arch/arm64/boot/dts/apm/apm-shadowcat.dtsi | 4 ++--
> >  arch/arm64/boot/dts/apm/apm-storm.dtsi     | 6 +++---
> >  2 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
> > index a83c82c50e29..832dd85b00bd 100644
> > --- a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
> > +++ b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
> > @@ -597,8 +597,8 @@ serial0: serial@10600000 {
> >  			interrupts = <0x0 0x4c 0x4>;
> >  		};
> >  
> > -		/* Do not change dwusb name, coded for backward compatibility */
> > -		usb0: dwusb@19000000 {
> > +		/* Node-name might need to be coded as dwusb for backward compatibility */
> > +		usb0: usb@19000000 {
> >  			status = "disabled";
> >  			compatible = "snps,dwc3";
> >  			reg =  <0x0 0x19000000 0x0 0x100000>;
> > diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi b/arch/arm64/boot/dts/apm/apm-storm.dtsi
> > index 0f37e77f5459..1520a945b7f9 100644
> > --- a/arch/arm64/boot/dts/apm/apm-storm.dtsi
> > +++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi
> > @@ -923,8 +923,8 @@ sata3: sata@1a800000 {
> >  			phy-names = "sata-phy";
> >  		};
> >  
> > -		/* Do not change dwusb name, coded for backward compatibility */
> > -		usb0: dwusb@19000000 {
> > +		/* Node-name might need to be coded as dwusb for backward compatibility */
> > +		usb0: usb@19000000 {
> >  			status = "disabled";
> >  			compatible = "snps,dwc3";
> >  			reg =  <0x0 0x19000000 0x0 0x100000>;
> > @@ -933,7 +933,7 @@ usb0: dwusb@19000000 {
> >  			dr_mode = "host";
> >  		};
> >  
> > -		usb1: dwusb@19800000 {
> > +		usb1: usb@19800000 {
> >  			status = "disabled";
> >  			compatible = "snps,dwc3";
> >  			reg =  <0x0 0x19800000 0x0 0x100000>;
> 
> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski June 26, 2022, 10:49 a.m. UTC | #3
On 24/06/2022 22:59, Serge Semin wrote:
> On Fri, Jun 24, 2022 at 07:17:53PM +0200, Krzysztof Kozlowski wrote:
>> On 24/06/2022 16:16, Serge Semin wrote:
>>> In accordance with the DWC USB3 bindings the corresponding node
>>> name is suppose to comply with the Generic USB HCD DT schema, which
>>> requires the USB nodes to have the name acceptable by the regexp:
>>> "^usb(@.*)?" . Make sure the "snps,dwc3"-compatible nodes are correctly
>>> named despite of the warning comment about possible backward
>>> compatibility issues.
>>
> 
>> Sometimes node name is exposed to user-space which depends on it. How
>> did you check there is no issue here?
> 
> I well remember the Qcom problem caused by one of my patch:
> https://lore.kernel.org/lkml/CALAqxLX_FNvFndEDWtGbFPjSzuAbfqxQE07diBJFZtftwEJX5A@mail.gmail.com/
> 
> The next patch caused the same problem, but hasn't been reverted.
> https://lore.kernel.org/lkml/CALAqxLWGujgR7p8Vb5S_RimRVYxwm5XF-c4NkKgMH-43wEBaWg@mail.gmail.com/
> 
> As before I am more inclined to thinking that the problem was mainly caused
> by the improper node-name utilization. Anyway John later noted that the
> problem was fixed in the user-space. 

Yes, I remember. The node names are not considered ABI, therefore any
reliance on them is not correct.

I wonder however what was the reasoning for this comment in APM DTS.

> That why afterwards you were able
> to provide the commit b77a1c4d6b05 ("arm64: dts: qcom: correct DWC3
> node names and unit addresses").
> 
> Anyway I am not able to track the way the node-name is used on the
> affected platform and can't make sure that the dts would be still
> working well on that devices. But seeing nobody responded/commented on
> this patch for more than a year we can at least try to merge this in
> and see whether it causes any problem should the denoted platform is
> still in use. If it does we can revert the update back and forget
> about it.

The APM is kind of abandoned, so indeed we might never get a reply.

I'll take it.

Best regards,
Krzysztof
Krzysztof Kozlowski June 26, 2022, 10:50 a.m. UTC | #4
On Fri, 24 Jun 2022 17:16:21 +0300, Serge Semin wrote:
> In accordance with the DWC USB3 bindings the corresponding node
> name is suppose to comply with the Generic USB HCD DT schema, which
> requires the USB nodes to have the name acceptable by the regexp:
> "^usb(@.*)?" . Make sure the "snps,dwc3"-compatible nodes are correctly
> named despite of the warning comment about possible backward
> compatibility issues.
> 
> [...]

Applied, thanks!

[5/5] arm64: dts: apm: Harmonize DWC USB3 DT nodes name
      https://git.kernel.org/krzk/linux/c/fcf036a017b251d362559cf7eb0bb6e614ccf842

Best regards,
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
index a83c82c50e29..832dd85b00bd 100644
--- a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
+++ b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
@@ -597,8 +597,8 @@  serial0: serial@10600000 {
 			interrupts = <0x0 0x4c 0x4>;
 		};
 
-		/* Do not change dwusb name, coded for backward compatibility */
-		usb0: dwusb@19000000 {
+		/* Node-name might need to be coded as dwusb for backward compatibility */
+		usb0: usb@19000000 {
 			status = "disabled";
 			compatible = "snps,dwc3";
 			reg =  <0x0 0x19000000 0x0 0x100000>;
diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi b/arch/arm64/boot/dts/apm/apm-storm.dtsi
index 0f37e77f5459..1520a945b7f9 100644
--- a/arch/arm64/boot/dts/apm/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi
@@ -923,8 +923,8 @@  sata3: sata@1a800000 {
 			phy-names = "sata-phy";
 		};
 
-		/* Do not change dwusb name, coded for backward compatibility */
-		usb0: dwusb@19000000 {
+		/* Node-name might need to be coded as dwusb for backward compatibility */
+		usb0: usb@19000000 {
 			status = "disabled";
 			compatible = "snps,dwc3";
 			reg =  <0x0 0x19000000 0x0 0x100000>;
@@ -933,7 +933,7 @@  usb0: dwusb@19000000 {
 			dr_mode = "host";
 		};
 
-		usb1: dwusb@19800000 {
+		usb1: usb@19800000 {
 			status = "disabled";
 			compatible = "snps,dwc3";
 			reg =  <0x0 0x19800000 0x0 0x100000>;