[PATCH 4/5] ARM: dts: microchip: Rename the usb node

[Krzysztof Kozlowski]

On 05/09/2024 15:51, Andrei.Simion at microchip.com wrote:
> Hi,
> 
> On 14.08.2024 16:22, Krzysztof Kozlowski wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 14/08/2024 14:26, Andrei Simion wrote:
>>> Rename the usb node according to devicetree
>>> specification and update the label according
>>> with the node-specific standard as: ohci, ehci
>>> or gadget.
>>
>> Please wrap commit message according to Linux coding style / submission
>> process (neither too early nor over the limit):
>> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
>>
>> ...
>>
> 
> In V2 I will wrap the commit message according to Linux coding style / submission process
> 
>>
>>>
>>> diff --git a/arch/arm/boot/dts/microchip/aks-cdu.dts b/arch/arm/boot/dts/microchip/aks-cdu.dts
>>> index 742fcf525e1b..52e166c8a365 100644
>>> --- a/arch/arm/boot/dts/microchip/aks-cdu.dts
>>> +++ b/arch/arm/boot/dts/microchip/aks-cdu.dts
>>> @@ -50,13 +50,13 @@ macb0: ethernet at fffc4000 {
>>>                               status = "okay";
>>>                       };
>>>
>>> -                     usb1: gadget at fffa4000 {
>>> +                     gadget: usb at fffa4000 {
>>>                               atmel,vbus-gpio = <&pioC 15 GPIO_ACTIVE_HIGH>;
>>>                               status = "okay";
>>>                       };
>>>               };
>>>
>>> -             usb0: ohci at 500000 {
>>> +             ohci: usb at 500000 {
>>
>> I don't think that these label renames are correct.
>>
> 
> I checked in other dts dtsi (other silicon vendors) the USB node 
> and it uses ohci, ehci, gadget, etc as a label.  Also, I verified the
> address in the datasheet and they are correctly labeled (meant that 
> they were: usb gadget, usb ohci, usb ehci)
> 
> From what point of view is it not correct?
> I would like to know so I can do it right

Because:
1. You drop existing information about interface number. That's USB0.
2. You did not provide answer why.


> 
> 
>>>                       num-ports = <2>;
>>>                       status = "okay";
>>>               };
>>> diff --git a/arch/arm/boot/dts/microchip/animeo_ip.dts b/arch/arm/boot/dts/microchip/animeo_ip.dts
>>> index 29936bfbeeb7..911c8d9ee013 100644
>>> --- a/arch/arm/boot/dts/microchip/animeo_ip.dts
>>> +++ b/arch/arm/boot/dts/microchip/animeo_ip.dts
>>> @@ -136,7 +136,7 @@ ubi at 60000 {
>>>                       };
>>>               };
>>>
>>> -             usb0: ohci at 500000 {
>>> +             ohci: usb at 500000 {
>>>                       num-ports = <2>;
>>>                       atmel,vbus-gpio = <&pioB 15 GPIO_ACTIVE_LOW>;
>>>                       status = "okay";
>>> diff --git a/arch/arm/boot/dts/microchip/at91-ariag25.dts b/arch/arm/boot/dts/microchip/at91-ariag25.dts
>>> index 713d18f80356..fedcf30a924e 100644
>>> --- a/arch/arm/boot/dts/microchip/at91-ariag25.dts
>>> +++ b/arch/arm/boot/dts/microchip/at91-ariag25.dts
>>> @@ -173,11 +173,11 @@ &usart3 {
>>>       status = "okay";
>>>  };
>>>
>>> -&usb0 {
>>> +&ohci {
>>>       status = "okay";
>>>       num-ports = <3>;
>>>  };
>>>
>>> -&usb1 {
>>> +&ehci {
>>>       status = "okay";
>>>  };
>>
>> And how now the sorting works? I don't get the point of it. What is
>> exactly wrong in the label that justifies the code reshuffling.
>>
> 
> The point is to be easy to determine what kind of standard 
> represents/implements the usb node.

For one, number of interface is important, e.g. because it matches
schematics or datasheet. For other, maybe the standard. You did not
provide any rationale for this.

Plus you ignored comment about sorting.


Best regards,
Krzysztof