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

[Mehta, Piyush]

> -----Original Message-----
> From: Simek, Michal <michal.simek@xxxxxxx>
> Sent: Thursday, September 22, 2022 5:05 PM
> To: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>; Mehta, Piyush
> <piyush.mehta@xxxxxxx>; gregkh@xxxxxxxxxxxxxxxxxxx;
> robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; balbi@xxxxxxxxxx
> Cc: linux-usb@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; Paladugu, Siva Durga Prasad
> <siva.durga.prasad.paladugu@xxxxxxx>; Manish Narani
> <manish.narani@xxxxxxxxxx>
> Subject: Re: [PATCH] dt-bindings: usb: dwc3: Add interrupt-names to include
> hibernation interrupt
> 
> 
> 
> 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

Enabling wakeup in zynqMp we need to put the core into hibernation, as versal don't have hibernation concept, but we require interrupt for wakeup.
We have a versal platform where we are not using hibernation, but system wake up we need the interrupt. For this interrupt-name enum would be:
- enum [host, peripheral, otg, usb-wakeup]

zynqMp :
- enum [host, peripheral, otg, hiber]

Versal:
- enum [host, peripheral, otg, usb-wakeup]

Regards,
Piyush Mehta