[PATCH 5/5] ARM: dts: imx6qdl: Rename USB hub node name

Markus Niebel posted 5 patches 1 year, 6 months ago
There is a newer version of this series
[PATCH 5/5] ARM: dts: imx6qdl: Rename USB hub node name
Posted by Markus Niebel 1 year, 6 months ago
From: Alexander Stein <alexander.stein@ew.tq-group.com>

According to microchip,usb2514.yaml the node name shall be usb-hub.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 arch/arm/boot/dts/nxp/imx/imx6qdl-mba6.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/nxp/imx/imx6qdl-mba6.dtsi b/arch/arm/boot/dts/nxp/imx/imx6qdl-mba6.dtsi
index 60aa1e947f62..d03f7065ddfd 100644
--- a/arch/arm/boot/dts/nxp/imx/imx6qdl-mba6.dtsi
+++ b/arch/arm/boot/dts/nxp/imx/imx6qdl-mba6.dtsi
@@ -293,7 +293,7 @@ &usbh1 {
 	#address-cells = <1>;
 	#size-cells = <0>;
 
-	hub@1 {
+	usb-hub@1 {
 		compatible = "usb424,2517";
 		reg = <1>;
 		#address-cells = <1>;
-- 
2.34.1
Re: [PATCH 5/5] ARM: dts: imx6qdl: Rename USB hub node name
Posted by Krzysztof Kozlowski 1 year, 6 months ago
On 12/08/2024 16:34, Markus Niebel wrote:
> From: Alexander Stein <alexander.stein@ew.tq-group.com>
> 
> According to microchip,usb2514.yaml the node name shall be usb-hub.

That's not true. The schema does not say anything like this. Old name is
correct. NAK.


Best regards,
Krzysztof
Re: [PATCH 5/5] ARM: dts: imx6qdl: Rename USB hub node name
Posted by Alexander Stein 1 year, 6 months ago
Am Dienstag, 13. August 2024, 11:20:08 CEST schrieb Krzysztof Kozlowski:
> On 12/08/2024 16:34, Markus Niebel wrote:
> > From: Alexander Stein <alexander.stein@ew.tq-group.com>
> > 
> > According to microchip,usb2514.yaml the node name shall be usb-hub.
> 
> That's not true. The schema does not say anything like this. Old name is
> correct. NAK.

So, is the schema incorrect? There is the dtbs_check warning:
arch/arm/boot/dts/nxp/imx/imx6q-mba6b.dtb: hub@1: $nodename:0: 'hub@1' does not match '^usb(@.*)?'
        from schema $id: http://devicetree.org/schemas/usb/microchip,usb2514.yaml#

Best regards,
Alexander
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
Re: [PATCH 5/5] ARM: dts: imx6qdl: Rename USB hub node name
Posted by Krzysztof Kozlowski 1 year, 6 months ago
On 13/08/2024 11:27, Alexander Stein wrote:
> Am Dienstag, 13. August 2024, 11:20:08 CEST schrieb Krzysztof Kozlowski:
>> On 12/08/2024 16:34, Markus Niebel wrote:
>>> From: Alexander Stein <alexander.stein@ew.tq-group.com>
>>>
>>> According to microchip,usb2514.yaml the node name shall be usb-hub.
>>
>> That's not true. The schema does not say anything like this. Old name is
>> correct. NAK.
> 
> So, is the schema incorrect? There is the dtbs_check warning:
> arch/arm/boot/dts/nxp/imx/imx6q-mba6b.dtb: hub@1: $nodename:0: 'hub@1' does not match '^usb(@.*)?'
>         from schema $id: http://devicetree.org/schemas/usb/microchip,usb2514.yaml#

If you have a warning, shorten it and paste it so this will be obvious.
If you look at several bindings, the hub is widely used name. I think
the schema is not correct here - I do not see any properties from
usb.yaml being used here (for usb2514). What's more, if you compare
usb2514 with any other on-board HUB representations (because that's the
only point why we have it in bindings, right?), none of them reference
usb(-hcd)?.yaml.

These are not USB controllers, IMO.

Best regards,
Krzysztof
Re: [PATCH 5/5] ARM: dts: imx6qdl: Rename USB hub node name
Posted by Alexander Stein 1 year, 6 months ago
Am Dienstag, 13. August 2024, 11:44:28 CEST schrieb Krzysztof Kozlowski:
> On 13/08/2024 11:27, Alexander Stein wrote:
> > Am Dienstag, 13. August 2024, 11:20:08 CEST schrieb Krzysztof Kozlowski:
> >> On 12/08/2024 16:34, Markus Niebel wrote:
> >>> From: Alexander Stein <alexander.stein@ew.tq-group.com>
> >>>
> >>> According to microchip,usb2514.yaml the node name shall be usb-hub.
> >>
> >> That's not true. The schema does not say anything like this. Old name is
> >> correct. NAK.
> > 
> > So, is the schema incorrect? There is the dtbs_check warning:
> > arch/arm/boot/dts/nxp/imx/imx6q-mba6b.dtb: hub@1: $nodename:0: 'hub@1' does not match '^usb(@.*)?'
> >         from schema $id: http://devicetree.org/schemas/usb/microchip,usb2514.yaml#
> 
> If you have a warning, shorten it and paste it so this will be obvious.
> If you look at several bindings, the hub is widely used name. I think
> the schema is not correct here - I do not see any properties from
> usb.yaml being used here (for usb2514). What's more, if you compare
> usb2514 with any other on-board HUB representations (because that's the
> only point why we have it in bindings, right?), none of them reference
> usb(-hcd)?.yaml.
> 
> These are not USB controllers, IMO.

I raised that concern in [1] already, but nobody commented.

Best regards,
Alexander

[1] https://lore.kernel.org/all/3633627.mvXUDI8C0e@steina-w/
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
Re: [PATCH 5/5] ARM: dts: imx6qdl: Rename USB hub node name
Posted by Krzysztof Kozlowski 1 year, 6 months ago
On 13/08/2024 13:02, Alexander Stein wrote:
> Am Dienstag, 13. August 2024, 11:44:28 CEST schrieb Krzysztof Kozlowski:
>> On 13/08/2024 11:27, Alexander Stein wrote:
>>> Am Dienstag, 13. August 2024, 11:20:08 CEST schrieb Krzysztof Kozlowski:
>>>> On 12/08/2024 16:34, Markus Niebel wrote:
>>>>> From: Alexander Stein <alexander.stein@ew.tq-group.com>
>>>>>
>>>>> According to microchip,usb2514.yaml the node name shall be usb-hub.
>>>>
>>>> That's not true. The schema does not say anything like this. Old name is
>>>> correct. NAK.
>>>
>>> So, is the schema incorrect? There is the dtbs_check warning:
>>> arch/arm/boot/dts/nxp/imx/imx6q-mba6b.dtb: hub@1: $nodename:0: 'hub@1' does not match '^usb(@.*)?'
>>>         from schema $id: http://devicetree.org/schemas/usb/microchip,usb2514.yaml#
>>
>> If you have a warning, shorten it and paste it so this will be obvious.
>> If you look at several bindings, the hub is widely used name. I think
>> the schema is not correct here - I do not see any properties from
>> usb.yaml being used here (for usb2514). What's more, if you compare
>> usb2514 with any other on-board HUB representations (because that's the
>> only point why we have it in bindings, right?), none of them reference
>> usb(-hcd)?.yaml.
>>
>> These are not USB controllers, IMO.
> 
> I raised that concern in [1] already, but nobody commented.

With 200 new emails in my inbox every day, I bet I miss many threads...

The nodes could be named usb-hub, makes sense, but:
1. Several bindings examples have hub, so we would need to correct all
of them for consistency?

2. For me hub is generic and obvious enough. Just like "phy", even
though DT spec has "usb-phy".

3. Renaming nodes to fix issues is very good. Renaming just to make
things more readable is also good or because coding style asks for it.
Renaming just because one person likes hub, other likes usb-hub, so just
personal preference, is for me churn. Considering it sometimes breaks
some users (it is not ABI... till it is), personal preference is poor
reason.

I think the schema should be fixed, because this is not an USB
controller, in terms how we call things which are "USB controllers". On
the bus it is USB device and USB hub. For USB device we have schema. For
USB hub we don't, so it does not matter.

Best regards,
Krzysztof
Re: [PATCH 5/5] ARM: dts: imx6qdl: Rename USB hub node name
Posted by Niebel, Markus 1 year, 6 months ago
On Tue, 2024-08-13 at 13:02 +0200, Alexander Stein wrote:
> Am Dienstag, 13. August 2024, 11:44:28 CEST schrieb Krzysztof Kozlowski:
> > On 13/08/2024 11:27, Alexander Stein wrote:
> > > Am Dienstag, 13. August 2024, 11:20:08 CEST schrieb Krzysztof Kozlowski:
> > > > On 12/08/2024 16:34, Markus Niebel wrote:
> > > > > From: Alexander Stein <alexander.stein@ew.tq-group.com>
> > > > > 
> > > > > According to microchip,usb2514.yaml the node name shall be usb-hub.
> > > > 
> > > > That's not true. The schema does not say anything like this. Old name is
> > > > correct. NAK.
> > > 
> > > So, is the schema incorrect? There is the dtbs_check warning:
> > > arch/arm/boot/dts/nxp/imx/imx6q-mba6b.dtb: hub@1: $nodename:0: 'hub@1' does not match '^usb(@.*)?'
> > >         from schema $id: http://devicetree.org/schemas/usb/microchip,usb2514.yaml#
> > 
> > If you have a warning, shorten it and paste it so this will be obvious.
> > If you look at several bindings, the hub is widely used name. I think
> > the schema is not correct here - I do not see any properties from
> > usb.yaml being used here (for usb2514). What's more, if you compare
> > usb2514 with any other on-board HUB representations (because that's the
> > only point why we have it in bindings, right?), none of them reference
> > usb(-hcd)?.yaml.
> > 
> > These are not USB controllers, IMO.
> 
> I raised that concern in [1] already, but nobody commented.
> 

Besides that, current version of device tree specification [1], section
'2.2.2 Generic Names Recommendation' exlicitely suggests 'usb-hub' but
not 'hub'

See
https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.4

> Best regards,
> Alexander
> 
> [1] https://lore.kernel.org/all/3633627.mvXUDI8C0e@steina-w/

Best regards,
Markus

-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
Re: [PATCH 5/5] ARM: dts: imx6qdl: Rename USB hub node name
Posted by Niebel, Markus 1 year, 6 months ago
On Mon, 2024-08-12 at 16:34 +0200, Markus Niebel wrote:
> From: Alexander Stein <alexander.stein@ew.tq-group.com>
> 
> According to microchip,usb2514.yaml the node name shall be usb-hub.
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> 

Sorry, i forgot my Signed-off-by. Will resend if needed

> ---
Best regards,
Markus