All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dt-bindings: usb: dwc3: Add interrupt-names to include hibernation interrupt
@ 2022-09-12  8:57 Piyush Mehta
  2022-09-13  9:21 ` Krzysztof Kozlowski
  2022-09-23 12:43 ` Felipe Balbi
  0 siblings, 2 replies; 13+ messages in thread
From: Piyush Mehta @ 2022-09-12  8:57 UTC (permalink / raw)
  To: gregkh, robh+dt, krzysztof.kozlowski+dt, balbi
  Cc: linux-usb, devicetree, linux-kernel, michal.simek,
	siva.durga.prasad.paladugu, Manish Narani, Piyush Mehta

From: Manish Narani <manish.narani@xilinx.com>

The hibernation feature enabled for Xilinx ZynqMP SoC in DWC3 IP.
Added the below interrupt-names in the binding schema for the same.

dwc_usb3: dwc3 core interrupt-names
otg: otg interrupt-names
hiber: hibernation interrupt-names

Signed-off-by: Manish Narani <manish.narani@xilinx.com>
Signed-off-by: Piyush Mehta <piyush.mehta@amd.com>
---
 Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index 1779d08ba1c0..618fa7bd32be 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -53,6 +53,8 @@ properties:
       - const: dwc_usb3
       - items:
           enum: [host, peripheral, otg]
+      - items:
+          enum: [dwc_usb3, otg, hiber]
 
   clocks:
     description:
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] dt-bindings: usb: dwc3: Add interrupt-names to include hibernation interrupt
  2022-09-12  8:57 [PATCH] dt-bindings: usb: dwc3: Add interrupt-names to include hibernation interrupt Piyush Mehta
@ 2022-09-13  9:21 ` Krzysztof Kozlowski
  2022-09-14 13:15   ` Mehta, Piyush
  2022-09-23 12:43 ` Felipe Balbi
  1 sibling, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-13  9:21 UTC (permalink / raw)
  To: Piyush Mehta, gregkh, robh+dt, krzysztof.kozlowski+dt, balbi
  Cc: linux-usb, devicetree, linux-kernel, michal.simek,
	siva.durga.prasad.paladugu, Manish Narani

On 12/09/2022 10:57, Piyush Mehta wrote:
> From: Manish Narani <manish.narani@xilinx.com>
> 
> The hibernation feature enabled for Xilinx ZynqMP SoC in DWC3 IP.
> Added the below interrupt-names in the binding schema for the same.
> 
> dwc_usb3: dwc3 core interrupt-names
> otg: otg interrupt-names
> hiber: hibernation interrupt-names

This does not make sense in commit msg. Don't duplicate patch in commit msg.

Where is the user (DTS) and implementation of this change? If this is
specific to Xilinx, why you do not have device specific compatible?

> 
> Signed-off-by: Manish Narani <manish.narani@xilinx.com>
> Signed-off-by: Piyush Mehta <piyush.mehta@amd.com>
> ---
>  Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> index 1779d08ba1c0..618fa7bd32be 100644
> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> @@ -53,6 +53,8 @@ properties:
>        - const: dwc_usb3
>        - items:
>            enum: [host, peripheral, otg]
> +      - items:
> +          enum: [dwc_usb3, otg, hiber]



Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH] dt-bindings: usb: dwc3: Add interrupt-names to include hibernation interrupt
  2022-09-13  9:21 ` Krzysztof Kozlowski
@ 2022-09-14 13:15   ` Mehta, Piyush
  2022-09-15  8:44     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 13+ messages in thread
From: Mehta, Piyush @ 2022-09-14 13:15 UTC (permalink / raw)
  To: Krzysztof Kozlowski, gregkh, robh+dt, krzysztof.kozlowski+dt, balbi
  Cc: linux-usb, devicetree, linux-kernel, Simek, Michal, Paladugu,
	Siva Durga Prasad, Manish Narani

Hello Krzysztof,

Thanks for the review comments.

Please find my inline comments with tag [Piyush].

Regards,
Piyush Mehta

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Tuesday, September 13, 2022 2:52 PM
> To: Mehta, Piyush <piyush.mehta@amd.com>; gregkh@linuxfoundation.org;
> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; balbi@kernel.org
> Cc: linux-usb@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; Simek, Michal <michal.simek@amd.com>; Paladugu,
> Siva Durga Prasad <siva.durga.prasad.paladugu@amd.com>; Manish Narani
> <manish.narani@xilinx.com>
> Subject: Re: [PATCH] dt-bindings: usb: dwc3: Add interrupt-names to include
> hibernation interrupt
> 
> On 12/09/2022 10:57, Piyush Mehta wrote:
> > From: Manish Narani <manish.narani@xilinx.com>
> >
> > The hibernation feature enabled for Xilinx ZynqMP SoC in DWC3 IP.
> > Added the below interrupt-names in the binding schema for the same.
> >
> > dwc_usb3: dwc3 core interrupt-names
> > otg: otg interrupt-names
> > hiber: hibernation interrupt-names
> 
> This does not make sense in commit msg. Don't duplicate patch in commit
> msg.
[Piyush]:
Will rephrase the commit message and send V2.
 
> 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.

> 
> >
> > Signed-off-by: Manish Narani <manish.narani@xilinx.com>
> > Signed-off-by: Piyush Mehta <piyush.mehta@amd.com>
> > ---
> >  Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > index 1779d08ba1c0..618fa7bd32be 100644
> > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > @@ -53,6 +53,8 @@ properties:
> >        - const: dwc_usb3
> >        - items:
> >            enum: [host, peripheral, otg]
> > +      - items:
> > +          enum: [dwc_usb3, otg, hiber]
> 
> 
> 
> Best regards,
> Krzysztof

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dt-bindings: usb: dwc3: Add interrupt-names to include hibernation interrupt
  2022-09-14 13:15   ` Mehta, Piyush
@ 2022-09-15  8:44     ` Krzysztof Kozlowski
  2022-09-15  9:04       ` Michal Simek
  0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-15  8:44 UTC (permalink / raw)
  To: Mehta, Piyush, gregkh, robh+dt, krzysztof.kozlowski+dt, balbi
  Cc: linux-usb, devicetree, linux-kernel, Simek, Michal, Paladugu,
	Siva Durga Prasad, Manish Narani

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.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dt-bindings: usb: dwc3: Add interrupt-names to include hibernation interrupt
  2022-09-15  8:44     ` Krzysztof Kozlowski
@ 2022-09-15  9:04       ` Michal Simek
  2022-09-16 10:10         ` Krzysztof Kozlowski
  0 siblings, 1 reply; 13+ messages in thread
From: Michal Simek @ 2022-09-15  9:04 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Mehta, Piyush, gregkh, robh+dt,
	krzysztof.kozlowski+dt, balbi
  Cc: linux-usb, devicetree, linux-kernel, Paladugu, Siva Durga Prasad,
	Manish Narani



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

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).

Thanks,
Michal

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dt-bindings: usb: dwc3: Add interrupt-names to include hibernation interrupt
  2022-09-15  9:04       ` Michal Simek
@ 2022-09-16 10:10         ` Krzysztof Kozlowski
  2022-09-22 11:34           ` Michal Simek
  0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-16 10:10 UTC (permalink / raw)
  To: Michal Simek, Mehta, Piyush, gregkh, robh+dt,
	krzysztof.kozlowski+dt, balbi
  Cc: linux-usb, devicetree, linux-kernel, Paladugu, Siva Durga Prasad,
	Manish Narani

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...

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dt-bindings: usb: dwc3: Add interrupt-names to include hibernation interrupt
  2022-09-16 10:10         ` Krzysztof Kozlowski
@ 2022-09-22 11:34           ` Michal Simek
  2022-09-23  4:28             ` Mehta, Piyush
  2022-09-23  4:38             ` Mehta, Piyush
  0 siblings, 2 replies; 13+ messages in thread
From: Michal Simek @ 2022-09-22 11:34 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Mehta, Piyush, gregkh, robh+dt,
	krzysztof.kozlowski+dt, balbi
  Cc: linux-usb, devicetree, linux-kernel, Paladugu, Siva Durga Prasad,
	Manish Narani



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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH] dt-bindings: usb: dwc3: Add interrupt-names to include hibernation interrupt
  2022-09-22 11:34           ` Michal Simek
@ 2022-09-23  4:28             ` Mehta, Piyush
  2022-09-23  4:38             ` Mehta, Piyush
  1 sibling, 0 replies; 13+ messages in thread
From: Mehta, Piyush @ 2022-09-23  4:28 UTC (permalink / raw)
  To: Simek, Michal, Krzysztof Kozlowski, gregkh, robh+dt,
	krzysztof.kozlowski+dt, balbi
  Cc: linux-usb, devicetree, linux-kernel, Paladugu, Siva Durga Prasad,
	Manish Narani



> -----Original Message-----
> From: Simek, Michal <michal.simek@amd.com>
> Sent: Thursday, September 22, 2022 5:05 PM
> To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>; Mehta, Piyush
> <piyush.mehta@amd.com>; gregkh@linuxfoundation.org;
> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; balbi@kernel.org
> Cc: linux-usb@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; Paladugu, Siva Durga Prasad
> <siva.durga.prasad.paladugu@amd.com>; Manish Narani
> <manish.narani@xilinx.com>
> 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]

Regards,
Piyush Mehta

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH] dt-bindings: usb: dwc3: Add interrupt-names to include hibernation interrupt
  2022-09-22 11:34           ` Michal Simek
  2022-09-23  4:28             ` Mehta, Piyush
@ 2022-09-23  4:38             ` Mehta, Piyush
  2022-09-23  9:22               ` Krzysztof Kozlowski
  1 sibling, 1 reply; 13+ messages in thread
From: Mehta, Piyush @ 2022-09-23  4:38 UTC (permalink / raw)
  To: Simek, Michal, Krzysztof Kozlowski, gregkh, robh+dt,
	krzysztof.kozlowski+dt, balbi
  Cc: linux-usb, devicetree, linux-kernel, Paladugu, Siva Durga Prasad,
	Manish Narani



> -----Original Message-----
> From: Simek, Michal <michal.simek@amd.com>
> Sent: Thursday, September 22, 2022 5:05 PM
> To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>; Mehta, Piyush
> <piyush.mehta@amd.com>; gregkh@linuxfoundation.org;
> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; balbi@kernel.org
> Cc: linux-usb@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; Paladugu, Siva Durga Prasad
> <siva.durga.prasad.paladugu@amd.com>; Manish Narani
> <manish.narani@xilinx.com>
> 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dt-bindings: usb: dwc3: Add interrupt-names to include hibernation interrupt
  2022-09-23  4:38             ` Mehta, Piyush
@ 2022-09-23  9:22               ` Krzysztof Kozlowski
  2022-09-23  9:34                 ` Michal Simek
  0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-23  9:22 UTC (permalink / raw)
  To: Mehta, Piyush, Simek, Michal, gregkh, robh+dt,
	krzysztof.kozlowski+dt, balbi
  Cc: linux-usb, devicetree, linux-kernel, Paladugu, Siva Durga Prasad,
	Manish Narani

On 23/09/2022 06:38, Mehta, Piyush wrote:
>> 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]

That's a different name you use now...

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dt-bindings: usb: dwc3: Add interrupt-names to include hibernation interrupt
  2022-09-23  9:22               ` Krzysztof Kozlowski
@ 2022-09-23  9:34                 ` Michal Simek
  0 siblings, 0 replies; 13+ messages in thread
From: Michal Simek @ 2022-09-23  9:34 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Mehta, Piyush, gregkh, robh+dt,
	krzysztof.kozlowski+dt, balbi
  Cc: linux-usb, devicetree, linux-kernel, Paladugu, Siva Durga Prasad,
	Manish Narani

Hi Krzysztof,

On 9/23/22 11:22, Krzysztof Kozlowski wrote:
> On 23/09/2022 06:38, Mehta, Piyush wrote:
>>> 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]
> 
> That's a different name you use now...

It is small confusion.

We have dwc3 in 3 SOCs. ZynqMP/Versal and Versal NET.

In ZynqMP currently we are define it as
interrupt-names = "dwc_usb3", "otg", "hiber";
(https://github.com/Xilinx/u-boot-xlnx/blob/master/arch/arm/dts/zynqmp.dtsi#L996)
where I think we should be able to convert it
host, peripheral, otg, hiber

And Versal (Versal NET is the same as Versal) is now using
interrupt-names = "dwc_usb3","otg","usb-wakeup";
https://github.com/Xilinx/u-boot-xlnx/blob/master/arch/arm/dts/versal.dtsi#L597

where it can be converted to
host, peripheral, otg, usb-wakeup

but the last usb-wakeup entry is the problematic one.
You mentioned before if this is SOC specific maybe we should consider to create 
new compatible string to match this.

Thanks,
Michal


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dt-bindings: usb: dwc3: Add interrupt-names to include hibernation interrupt
  2022-09-12  8:57 [PATCH] dt-bindings: usb: dwc3: Add interrupt-names to include hibernation interrupt Piyush Mehta
  2022-09-13  9:21 ` Krzysztof Kozlowski
@ 2022-09-23 12:43 ` Felipe Balbi
  2022-09-23 13:25   ` Michal Simek
  1 sibling, 1 reply; 13+ messages in thread
From: Felipe Balbi @ 2022-09-23 12:43 UTC (permalink / raw)
  To: Piyush Mehta
  Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, linux-usb, devicetree,
	linux-kernel, michal.simek, siva.durga.prasad.paladugu,
	Manish Narani


Hi,

Piyush Mehta <piyush.mehta@amd.com> writes:
> From: Manish Narani <manish.narani@xilinx.com>
>
> The hibernation feature enabled for Xilinx ZynqMP SoC in DWC3 IP.
> Added the below interrupt-names in the binding schema for the same.
>
> dwc_usb3: dwc3 core interrupt-names
> otg: otg interrupt-names
> hiber: hibernation interrupt-names
>
> Signed-off-by: Manish Narani <manish.narani@xilinx.com>
> Signed-off-by: Piyush Mehta <piyush.mehta@amd.com>
> ---
>  Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> index 1779d08ba1c0..618fa7bd32be 100644
> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> @@ -53,6 +53,8 @@ properties:
>        - const: dwc_usb3
>        - items:
>            enum: [host, peripheral, otg]
> +      - items:
> +          enum: [dwc_usb3, otg, hiber]

I would spell it out; i.e. `hibernation' instead of `hiber'.

-- 
balbi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dt-bindings: usb: dwc3: Add interrupt-names to include hibernation interrupt
  2022-09-23 12:43 ` Felipe Balbi
@ 2022-09-23 13:25   ` Michal Simek
  0 siblings, 0 replies; 13+ messages in thread
From: Michal Simek @ 2022-09-23 13:25 UTC (permalink / raw)
  To: Felipe Balbi, Piyush Mehta
  Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, linux-usb, devicetree,
	linux-kernel, siva.durga.prasad.paladugu, Manish Narani

Hi Felipe,

On 9/23/22 14:43, Felipe Balbi wrote:
> 
> Hi,
> 
> Piyush Mehta <piyush.mehta@amd.com> writes:
>> From: Manish Narani <manish.narani@xilinx.com>
>>
>> The hibernation feature enabled for Xilinx ZynqMP SoC in DWC3 IP.
>> Added the below interrupt-names in the binding schema for the same.
>>
>> dwc_usb3: dwc3 core interrupt-names
>> otg: otg interrupt-names
>> hiber: hibernation interrupt-names
>>
>> Signed-off-by: Manish Narani <manish.narani@xilinx.com>
>> Signed-off-by: Piyush Mehta <piyush.mehta@amd.com>
>> ---
>>   Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> index 1779d08ba1c0..618fa7bd32be 100644
>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> @@ -53,6 +53,8 @@ properties:
>>         - const: dwc_usb3
>>         - items:
>>             enum: [host, peripheral, otg]
>> +      - items:
>> +          enum: [dwc_usb3, otg, hiber]
> 
> I would spell it out; i.e. `hibernation' instead of `hiber'.

that wouldn't be an issue. What about that wake-up line?

Thanks,
Michal


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-09-23 13:25 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-12  8:57 [PATCH] dt-bindings: usb: dwc3: Add interrupt-names to include hibernation interrupt Piyush Mehta
2022-09-13  9:21 ` Krzysztof Kozlowski
2022-09-14 13:15   ` Mehta, Piyush
2022-09-15  8:44     ` Krzysztof Kozlowski
2022-09-15  9:04       ` Michal Simek
2022-09-16 10:10         ` Krzysztof Kozlowski
2022-09-22 11:34           ` Michal Simek
2022-09-23  4:28             ` Mehta, Piyush
2022-09-23  4:38             ` Mehta, Piyush
2022-09-23  9:22               ` Krzysztof Kozlowski
2022-09-23  9:34                 ` Michal Simek
2022-09-23 12:43 ` Felipe Balbi
2022-09-23 13:25   ` Michal Simek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.