Re: [PATCH v9 01/10] dt-bindings: usb: qcom,dwc3: Add bindings for SC8280 Multiport

[Krishna Kurapati PSSNV <quic_kriskura@quicinc.com>]

On 6/27/2023 9:08 PM, Johan Hovold wrote:
> On Tue, Jun 27, 2023 at 01:20:59PM +0200, Johan Hovold wrote:
>> On Wed, Jun 21, 2023 at 10:06:19AM +0530, Krishna Kurapati wrote:
> 
>>> +          items:
>>> +            - const: dp1_hs_phy_irq
>>> +            - const: dm1_hs_phy_irq
>>> +            - const: dp2_hs_phy_irq
>>> +            - const: dm2_hs_phy_irq
>>> +            - const: dp3_hs_phy_irq
>>> +            - const: dm4_hs_phy_irq
>>> +            - const: dp4_hs_phy_irq
>>> +            - const: dm4_hs_phy_irq
>>> +            - const: ss1_phy_irq
>>> +            - const: ss2_phy_irq
>>> +            - const: pwr_event_1
>>> +            - const: pwr_event_2
>>> +            - const: pwr_event_3
>>> +            - const: pwr_event_4
>>
>> The naming here is inconsistent and interrupts should not have "_irq"
>> suffixes (even if some of the current ones do for historical reasons).
>>
>> I believe these should be named
>>
>> 	pwr_event_1
>> 	dp_hs_phy_1
>> 	dm_hs_phy_1
>> 	ss_phy_1
>>
>> 	pwr_event_2
>> 	dp_hs_phy_2
>> 	dm_hs_phy_2
>> 	ss_phy_2
>>
>> 	pwr_event_3
>> 	dp_hs_phy_3
>> 	dm_hs_phy_3
>>
>> 	pwr_event_4
>> 	dp_hs_phy_4
>> 	dm_hs_phy_4
>>
>> or similar and be grouped by port while using the the
>> qcom,sc8280xp-dwc ordering for the individual lines.
> 
> Perhaps the ordering you suggested is fine too, but I'd probably move
> the pwr_event ones first to match qcom,sc8280xp-dwc then, that is:
> 
>   	pwr_event_1
>   	pwr_event_2
>   	pwr_event_3
>   	pwr_event_4
>   	dp_hs_phy_1
>   	dm_hs_phy_1
>   	dp_hs_phy_2
>   	dm_hs_phy_2
>   	dp_hs_phy_3
>   	dm_hs_phy_3
>   	dp_hs_phy_4
>   	dm_hs_phy_4
>   	ss_phy_1
>   	ss_phy_2
> 
> so we have them grouped as pwr_event followed by HS and with SS last.
> 
>> Side note: Please note how the above interrupt properties can also be
>> used to infer the number of HS and SS ports.
> 
> Johan

Can't we just cleanup all at once later ? Might not be a good idea for 
some properties in the file to have _irq and for some to not have it. I 
will modify the order though.

Regards,
Krishna,