Re: [PATCH] dt-bindings: usb: dwc3: Add interrupt-names to include hibernation interrupt

[Michal Simek <michal.simek@amd.com>]

On 9/16/22 12:10, Krzysztof Kozlowski wrote:
> On 15/09/2022 10:04, Michal Simek wrote:
>>
>>
>> On 9/15/22 10:44, Krzysztof Kozlowski wrote:
>>> On 14/09/2022 14:15, Mehta, Piyush wrote:
>>>>    
>>>>> Where is the user (DTS) and implementation of this change? If this is specific
>>>>> to Xilinx, why you do not have device specific compatible?
>>>> [Piyush]:
>>>> We have dedicated irq line for hibernation feature,  "hiber" irq line triggers hibernation interrupt.
>>>> DWC3 core supports the hibernation feature, we have a dedicated code which is yet to be upstreamed.
>>>> As the hibernation feature provided by dwc3-core, so this will be supported by other SOC/vendors.
>>>
>>> But is hiber irq line present in other vendors? What confuses me is
>>> adding not only "hiber" irq but also otg in completely new enum.
>>
>> I will let Piyush to comment hiber IRQ. But I expect we don't have visibility
>> what others are doing but this is line is not Xilinx invention that's why I
>> expect IP from Synopsys have it by default but it is up to soc vendor if
>> hibernation feature is enabled or not.
>>
>> otg is already listed in
>> Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>>
>> It is only about order.
>> Driver is already using
>> platform_get_irq_byname..() functions
> 
> Linux driver yes, but other platforms (bootloaders, operating systems)
> might be doing things differently. Therefore the order and items are
> usually strict. If they cannot be strict, it is nice to know why or it
> is nice to restrict it to some specific variant (if it is applicable).
> 
> This is why I asked whether the line is specific to Xilinx or to others.
> 
>>
>> I think any combination should be fine. Do we need to record used order or there
>> is way in yaml to support any combination with dwc_usb3, host, peripheral, otg
>> should be working (ignoring that hiber which should be likely there too).
> 
> What confuses me here more, is having otg. I understand that dwc_usb3 is
> the single interrupt for all the modes, so my naive approach would be:
> oneOf:
>   - dwc_usb3
>   - enum [dwc_usb3, hiber]
>   - enum [host, peripheral, otg]
>   - enum [host, peripheral, otg, hiber]
> 
> However here Piyush adds not only hiber but also otg...

I was looking at code and I think we should be able to use this order
- enum [host, peripheral, otg, hiber]
which should ensure compatibility in other SW projects.

We can completely ignore dwc_usb3. It means above dwc_usb3, hiber shouldn't be 
also listed to make sure that the second entry is all the time irq for peripheral.

Thanks,
Michal