diff mbox series

arm64: dts: imx8mn-var-som-symphony: fix USB OTG

Message ID 20230704150240.2022020-1-hugo@hugovil.com
State New
Headers
Series arm64: dts: imx8mn-var-som-symphony: fix USB OTG |

Commit Message

Hugo Villeneuve July 4, 2023, 3:02 p.m. UTC 
  From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

USB OTG is currently broken on the Variscite Symphony EVK and imx8mn
nano SOM.

Import changes from linux-5.15 branch of varigit repos to fix it.

Link: https://github.com/varigit/linux-imx.git
Fixes: 7358e05bddca ("arm64: dts: imx8mn-var-som-symphony: Add Variscite Symphony board with VAR-SOM-MX8MN")
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 .../dts/freescale/imx8mn-var-som-symphony.dts | 37 ++++++++++++++++++-
 1 file changed, 35 insertions(+), 2 deletions(-)

Comments

Krzysztof Kozlowski July 4, 2023, 3:08 p.m. UTC | #1 
On 04/07/2023 17:02, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> USB OTG is currently broken on the Variscite Symphony EVK and imx8mn
> nano SOM.
> 
> Import changes from linux-5.15 branch of varigit repos to fix it.
> 
> Link: https://github.com/varigit/linux-imx.git
> Fixes: 7358e05bddca ("arm64: dts: imx8mn-var-som-symphony: Add Variscite Symphony board with VAR-SOM-MX8MN")
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> ---
>  .../dts/freescale/imx8mn-var-som-symphony.dts | 37 ++++++++++++++++++-
>  1 file changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts b/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
> index 406a711486da..aef89198f24c 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
> @@ -6,6 +6,7 @@
>  
>  /dts-v1/;
>  
> +#include <dt-bindings/usb/pd.h>
>  #include "imx8mn-var-som.dtsi"
>  
>  / {
> @@ -104,10 +105,29 @@ extcon_usbotg1: typec@3d {
>  		compatible = "nxp,ptn5150";
>  		reg = <0x3d>;
>  		interrupt-parent = <&gpio1>;
> -		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
> +		interrupts = <11 IRQ_TYPE_NONE>;

That's surprising, why?


Best regards,
Krzysztof
Hugo Villeneuve July 4, 2023, 3:31 p.m. UTC | #2 
On Tue, 4 Jul 2023 17:08:12 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 04/07/2023 17:02, Hugo Villeneuve wrote:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > 
> > USB OTG is currently broken on the Variscite Symphony EVK and imx8mn
> > nano SOM.
> > 
> > Import changes from linux-5.15 branch of  doen't giveto fix it.
> > 
> > Link: https://github.com/varigit/linux-imx.git
> > Fixes: 7358e05bddca ("arm64: dts: imx8mn-var-som-symphony: Add Variscite Symphony board with VAR-SOM-MX8MN")
> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > ---
> >  .../dts/freescale/imx8mn-var-som-symphony.dts | 37 ++++++++++++++++++-
> >  1 file changed, 35 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts b/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
> > index 406a711486da..aef89198f24c 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
> > @@ -6,6 +6,7 @@
> >  
> >  /dts-v1/;
> >  
> > +#include <dt-bindings/usb/pd.h>
> >  #include "imx8mn-var-som.dtsi"
> >  
> >  / {
> > @@ -104,10 +105,29 @@ extcon_usbotg1: typec@3d {
> >  		compatible = "nxp,ptn5150";
> >  		reg = <0x3d>;
> >  		interrupt-parent = <&gpio1>;
> > -		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
> > +		interrupts = <11 IRQ_TYPE_NONE>;
> 
> That's surprising, why?

Hi,
the varigit repos log or source code has no information about this
particular configuration.

In the schematics, the interrupt output pin of the PTN5150 is connected
to two different resistors, one of these being connected to GPIO1 pin
11. But these two resistors are not assembled on any versions of the
board, so the interrupt pin is currently not used.

Hugo.
Krzysztof Kozlowski July 4, 2023, 4:20 p.m. UTC | #3 
On 04/07/2023 17:31, Hugo Villeneuve wrote:
> On Tue, 4 Jul 2023 17:08:12 +0200
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
>> On 04/07/2023 17:02, Hugo Villeneuve wrote:
>>> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
>>>
>>> USB OTG is currently broken on the Variscite Symphony EVK and imx8mn
>>> nano SOM.
>>>
>>> Import changes from linux-5.15 branch of  doen't giveto fix it.
>>>
>>> Link: https://github.com/varigit/linux-imx.git
>>> Fixes: 7358e05bddca ("arm64: dts: imx8mn-var-som-symphony: Add Variscite Symphony board with VAR-SOM-MX8MN")
>>> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
>>> ---
>>>  .../dts/freescale/imx8mn-var-som-symphony.dts | 37 ++++++++++++++++++-
>>>  1 file changed, 35 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts b/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
>>> index 406a711486da..aef89198f24c 100644
>>> --- a/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
>>> +++ b/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
>>> @@ -6,6 +6,7 @@
>>>  
>>>  /dts-v1/;
>>>  
>>> +#include <dt-bindings/usb/pd.h>
>>>  #include "imx8mn-var-som.dtsi"
>>>  
>>>  / {
>>> @@ -104,10 +105,29 @@ extcon_usbotg1: typec@3d {
>>>  		compatible = "nxp,ptn5150";
>>>  		reg = <0x3d>;
>>>  		interrupt-parent = <&gpio1>;
>>> -		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
>>> +		interrupts = <11 IRQ_TYPE_NONE>;
>>
>> That's surprising, why?
> 
> Hi,
> the varigit repos log or source code has no information about this
> particular configuration.
> 
> In the schematics, the interrupt output pin of the PTN5150 is connected
> to two different resistors, one of these being connected to GPIO1 pin
> 11. But these two resistors are not assembled on any versions of the
> board, so the interrupt pin is currently not used.

OK, so there is no interrupt, but not interrupt of type none. Just drop
the property and make it optional in the bindings. The driver however
requires the interrupt, so I wonder how the device is going to work
without it?

Are you sure that interrupt line is not shorted instead of missing resistor?

Best regards,
Krzysztof
Fabio Estevam July 4, 2023, 4:33 p.m. UTC | #4 
Adding some Variscite folks in case they can help to clarify.

On Tue, Jul 4, 2023 at 1:20 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 04/07/2023 17:31, Hugo Villeneuve wrote:
> > On Tue, 4 Jul 2023 17:08:12 +0200
> > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> >
> >> On 04/07/2023 17:02, Hugo Villeneuve wrote:
> >>> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> >>>
> >>> USB OTG is currently broken on the Variscite Symphony EVK and imx8mn
> >>> nano SOM.
> >>>
> >>> Import changes from linux-5.15 branch of  doen't giveto fix it.
> >>>
> >>> Link: https://github.com/varigit/linux-imx.git
> >>> Fixes: 7358e05bddca ("arm64: dts: imx8mn-var-som-symphony: Add Variscite Symphony board with VAR-SOM-MX8MN")
> >>> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> >>> ---
> >>>  .../dts/freescale/imx8mn-var-som-symphony.dts | 37 ++++++++++++++++++-
> >>>  1 file changed, 35 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts b/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
> >>> index 406a711486da..aef89198f24c 100644
> >>> --- a/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
> >>> +++ b/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
> >>> @@ -6,6 +6,7 @@
> >>>
> >>>  /dts-v1/;
> >>>
> >>> +#include <dt-bindings/usb/pd.h>
> >>>  #include "imx8mn-var-som.dtsi"
> >>>
> >>>  / {
> >>> @@ -104,10 +105,29 @@ extcon_usbotg1: typec@3d {
> >>>             compatible = "nxp,ptn5150";
> >>>             reg = <0x3d>;
> >>>             interrupt-parent = <&gpio1>;
> >>> -           interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
> >>> +           interrupts = <11 IRQ_TYPE_NONE>;
> >>
> >> That's surprising, why?
> >
> > Hi,
> > the varigit repos log or source code has no information about this
> > particular configuration.
> >
> > In the schematics, the interrupt output pin of the PTN5150 is connected
> > to two different resistors, one of these being connected to GPIO1 pin
> > 11. But these two resistors are not assembled on any versions of the
> > board, so the interrupt pin is currently not used.
>
> OK, so there is no interrupt, but not interrupt of type none. Just drop
> the property and make it optional in the bindings. The driver however
> requires the interrupt, so I wonder how the device is going to work
> without it?
>
> Are you sure that interrupt line is not shorted instead of missing resistor?
>
> Best regards,
> Krzysztof
>
Hugo Villeneuve July 4, 2023, 4:55 p.m. UTC | #5 
On Tue, 4 Jul 2023 13:33:10 -0300
Fabio Estevam <festevam@gmail.com> wrote:

> Adding some Variscite folks in case they can help to clarify.
> 
> On Tue, Jul 4, 2023 at 1:20 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 04/07/2023 17:31, Hugo Villeneuve wrote:
> > > On Tue, 4 Jul 2023 17:08:12 +0200
> > > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> > >
> > >> On 04/07/2023 17:02, Hugo Villeneuve wrote:
> > >>> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > >>>
> > >>> USB OTG is currently broken on the Variscite Symphony EVK and imx8mn
> > >>> nano SOM.
> > >>>
> > >>> Import changes from linux-5.15 branch of  doen't giveto fix it.
> > >>>
> > >>> Link: https://github.com/varigit/linux-imx.git
> > >>> Fixes: 7358e05bddca ("arm64: dts: imx8mn-var-som-symphony: Add Variscite Symphony board with VAR-SOM-MX8MN")
> > >>> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > >>> ---
> > >>>  .../dts/freescale/imx8mn-var-som-symphony.dts | 37 ++++++++++++++++++-
> > >>>  1 file changed, 35 insertions(+), 2 deletions(-)
> > >>>
> > >>> diff --git a/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts b/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
> > >>> index 406a711486da..aef89198f24c 100644
> > >>> --- a/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
> > >>> +++ b/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
> > >>> @@ -6,6 +6,7 @@
> > >>>
> > >>>  /dts-v1/;
> > >>>
> > >>> +#include <dt-bindings/usb/pd.h>
> > >>>  #include "imx8mn-var-som.dtsi"
> > >>>
> > >>>  / {
> > >>> @@ -104,10 +105,29 @@ extcon_usbotg1: typec@3d {
> > >>>             compatible = "nxp,ptn5150";
> > >>>             reg = <0x3d>;
> > >>>             interrupt-parent = <&gpio1>;
> > >>> -           interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
> > >>> +           interrupts = <11 IRQ_TYPE_NONE>;
> > >>
> > >> That's surprising, why?
> > >
> > > Hi,
> > > the varigit repos log or source code has no information about this
> > > particular configuration.
> > >
> > > In the schematics, the interrupt output pin of the PTN5150 is connected
> > > to two different resistors, one of these being connected to GPIO1 pin
> > > 11. But these two resistors are not assembled on any versions of the
> > > board, so the interrupt pin is currently not used.
> >
> > OK, so there is no interrupt, but not interrupt of type none. Just drop
> > the property and make it optional in the bindings. The driver however
> > requires the interrupt, so I wonder how the device is going to work
> > without it?
> >
> > Are you sure that interrupt line is not shorted instead of missing resistor?

Hi,
Link for schematics:
https://www.variscite.com/wp-content/uploads/2019/07/Symphony-Board-Schematics.zip

In the schematics, both resistors R106 (connected to PTN5150 on one
side and GPIO1 pin 11 on the other size) and R131 have the text "NC"
near their reference designator. And I visually confirm that R106
and R131 are not soldered on the board.

However, GPIO1 pin 11 (the interrupt pin configured in the DTS) is
also connected to PTN5150 pin 9 (ID), which has a simple pull-up to
3.3V. So from what I can see/deduce, the DTS interrupt pin
will always be 3.3V, and never pulled to GND.

Hugo.
Hugo Villeneuve July 4, 2023, 5:02 p.m. UTC | #6 
On Tue, 4 Jul 2023 12:55:41 -0400
Hugo Villeneuve <hugo@hugovil.com> wrote:

> On Tue, 4 Jul 2023 13:33:10 -0300
> Fabio Estevam <festevam@gmail.com> wrote:
> 
> > Adding some Variscite folks in case they can help to clarify.
> > 
> > On Tue, Jul 4, 2023 at 1:20 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> > >
> > > On 04/07/2023 17:31, Hugo Villeneuve wrote:
> > > > On Tue, 4 Jul 2023 17:08:12 +0200
> > > > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> > > >
> > > >> On 04/07/2023 17:02, Hugo Villeneuve wrote:
> > > >>> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > >>>
> > > >>> USB OTG is currently broken on the Variscite Symphony EVK and imx8mn
> > > >>> nano SOM.
> > > >>>
> > > >>> Import changes from linux-5.15 branch of  doen't giveto fix it.
> > > >>>
> > > >>> Link: https://github.com/varigit/linux-imx.git
> > > >>> Fixes: 7358e05bddca ("arm64: dts: imx8mn-var-som-symphony: Add Variscite Symphony board with VAR-SOM-MX8MN")
> > > >>> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > >>> ---
> > > >>>  .../dts/freescale/imx8mn-var-som-symphony.dts | 37 ++++++++++++++++++-
> > > >>>  1 file changed, 35 insertions(+), 2 deletions(-)
> > > >>>
> > > >>> diff --git a/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts b/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
> > > >>> index 406a711486da..aef89198f24c 100644
> > > >>> --- a/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
> > > >>> +++ b/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
> > > >>> @@ -6,6 +6,7 @@
> > > >>>
> > > >>>  /dts-v1/;
> > > >>>
> > > >>> +#include <dt-bindings/usb/pd.h>
> > > >>>  #include "imx8mn-var-som.dtsi"
> > > >>>
> > > >>>  / {
> > > >>> @@ -104,10 +105,29 @@ extcon_usbotg1: typec@3d {
> > > >>>             compatible = "nxp,ptn5150";
> > > >>>             reg = <0x3d>;
> > > >>>             interrupt-parent = <&gpio1>;
> > > >>> -           interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
> > > >>> +           interrupts = <11 IRQ_TYPE_NONE>;
> > > >>
> > > >> That's surprising, why?
> > > >
> > > > Hi,
> > > > the varigit repos log or source code has no information about this
> > > > particular configuration.
> > > >
> > > > In the schematics, the interrupt output pin of the PTN5150 is connected
> > > > to two different resistors, one of these being connected to GPIO1 pin
> > > > 11. But these two resistors are not assembled on any versions of the
> > > > board, so the interrupt pin is currently not used.
> > >
> > > OK, so there is no interrupt, but not interrupt of type none. Just drop
> > > the property and make it optional in the bindings. The driver however
> > > requires the interrupt, so I wonder how the device is going to work
> > > without it?
> > >
> > > Are you sure that interrupt line is not shorted instead of missing resistor?
> 
> Hi,
> Link for schematics:
> https://www.variscite.com/wp-content/uploads/2019/07/Symphony-Board-Schematics.zip
> 
> In the schematics, both resistors R106 (connected to PTN5150 on one
> side and GPIO1 pin 11 on the other size) and R131 have the text "NC"
> near their reference designator. And I visually confirm that R106
> and R131 are not soldered on the board.
> 
> However, GPIO1 pin 11 (the interrupt pin configured in the DTS) is
> also connected to PTN5150 pin 9 (ID), which has a simple pull-up to
> 3.3V. So from what I can see/deduce, the DTS interrupt pin
> will always be 3.3V, and never pulled to GND.

Ok, I tought pin 9 (ID) was an input, but it is in fact an output
driven by the PTN5150 so the last sentence is incorrect.

Hugo.
Fabio Estevam July 4, 2023, 6:04 p.m. UTC | #7 
Hi Hugo,

On Tue, Jul 4, 2023 at 2:02 PM Hugo Villeneuve <hugo@hugovil.com> wrote:

> Ok, I tought pin 9 (ID) was an input, but it is in fact an output
> driven by the PTN5150 so the last sentence is incorrect.

Does USB OTG work if you keep interrupts = <11 IRQ_TYPE_LEVEL_LOW> ?
Hugo Villeneuve July 4, 2023, 8:41 p.m. UTC | #8 
On Tue, 4 Jul 2023 15:04:37 -0300
Fabio Estevam <festevam@gmail.com> wrote:

> Hi Hugo,
> 
> On Tue, Jul 4, 2023 at 2:02 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> 
> > Ok, I tought pin 9 (ID) was an input, but it is in fact an output
> > driven by the PTN5150 so the last sentence is incorrect.
> 
> Does USB OTG work if you keep interrupts = <11 IRQ_TYPE_LEVEL_LOW> ?

Hi Fabio,
with interrupts = <11 IRQ_TYPE_LEVEL_LOW>, USB OTG doesn't work.

I have added additional debug messages to help diagnose
(prefixed by DEBUG_IRQ), and here is what I have:

With IRQ_TYPE_LEVEL_LOW:

$ dmesg | grep 5150
[    4.729134] ptn5150 1-003d: No VBUS GPIO, ignoring VBUS control
[    4.735257] ptn5150 1-003d: DEBUG_IRQ: i2c->irq: 42
[    4.749563] ptn5150 1-003d: DEBUG_IRQ: error in usb_role_switch_get()
[    4.792956] ptn5150 1-003d: No VBUS GPIO, ignoring VBUS control
[    4.799022] ptn5150 1-003d: DEBUG_IRQ: i2c->irq: 0
[    4.803889] ptn5150 1-003d: error -ENOENT: failed to get INT GPIO
[    4.810085] ptn5150: probe of 1-003d failed with error -2


With IRQ_TYPE_NONE:

$ dmesg | grep 5150
[    4.726860] ptn5150 1-003d: No VBUS GPIO, ignoring VBUS control
[    4.736050] ptn5150 1-003d: DEBUG_IRQ: i2c->irq: 42
[    4.768351] ptn5150 1-003d: DEBUG_IRQ: error in usb_role_switch_get()
[    4.803091] ptn5150 1-003d: No VBUS GPIO, ignoring VBUS control
[    4.809172] ptn5150 1-003d: DEBUG_IRQ: i2c->irq: 42
[    4.816030] ptn5150 1-003d: DEBUG_IRQ: error in usb_role_switch_get()
[    4.825915] ptn5150 1-003d: No VBUS GPIO, ignoring VBUS control
[    4.832057] ptn5150 1-003d: DEBUG_IRQ: i2c->irq: 42
[    4.840722] ptn5150 1-003d: DEBUG_IRQ: error in usb_role_switch_get()
[    4.889220] ptn5150 1-003d: No VBUS GPIO, ignoring VBUS control
[    4.895271] ptn5150 1-003d: DEBUG_IRQ: i2c->irq: 42
[    4.901636] ptn5150 1-003d: DEBUG_IRQ: error in usb_role_switch_get()
[    4.934740] ptn5150 1-003d: No VBUS GPIO, ignoring VBUS control
[    4.940840] ptn5150 1-003d: DEBUG_IRQ: i2c->irq: 42
[    4.947223] ptn5150 1-003d: DEBUG_IRQ: error in usb_role_switch_get()
[    4.993208] ptn5150 1-003d: No VBUS GPIO, ignoring VBUS control
[    4.999347] ptn5150 1-003d: DEBUG_IRQ: i2c->irq: 42
[    5.014494] ptn5150 1-003d: DEBUG_IRQ: error in usb_role_switch_get()
[    5.071615] ptn5150 1-003d: No VBUS GPIO, ignoring VBUS control
[    5.077771] ptn5150 1-003d: DEBUG_IRQ: i2c->irq: 42
[    5.101222] ptn5150 1-003d: DEBUG_IRQ: probe done

Hugo.
Fabio Estevam July 4, 2023, 9:02 p.m. UTC | #9 
Hi Hugo,

On Tue, Jul 4, 2023 at 5:41 PM Hugo Villeneuve <hugo@hugovil.com> wrote:

> Hi Fabio,
> with interrupts = <11 IRQ_TYPE_LEVEL_LOW>, USB OTG doesn't work.

PTN5150 datasheet says:

"Any changes in the attach/detach events or Rp current source changes
will trigger INTB pin to go LOW."

What about:  interrupts = <11 IRQ_TYPE_EDGE_FALLING>; ?

Also, please add a pullup to the GPIO1_11 pad.

Does it work?
Hugo Villeneuve July 4, 2023, 9:28 p.m. UTC | #10 
On Tue, 4 Jul 2023 18:02:53 -0300
Fabio Estevam <festevam@gmail.com> wrote:

> Hi Hugo,
> 
> On Tue, Jul 4, 2023 at 5:41 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> 
> > Hi Fabio,
> > with interrupts = <11 IRQ_TYPE_LEVEL_LOW>, USB OTG doesn't work.
> 
> PTN5150 datasheet says:
> 
> "Any changes in the attach/detach events or Rp current source changes
> will trigger INTB pin to go LOW."

Hi Fabio,
it is important to remember that on this board, like I explained
before, the INTB pin is not connected to anything.

It is only the ID pin (9) that is connected to the GPIO1_11 pin.

> What about:  interrupts = <11 IRQ_TYPE_EDGE_FALLING>; ?

With this setting, USB OTG works:

$ dmesg | grep 5150
[    4.833529] ptn5150 1-003d: No VBUS GPIO, ignoring VBUS control
[    4.839972] ptn5150 1-003d: DEBUG_IRQ: i2c->irq: 42
[    4.874173] ptn5150 1-003d: DEBUG_IRQ: error in usb_role_switch_get()
[    4.896822] ptn5150 1-003d: No VBUS GPIO, ignoring VBUS control
[    4.902905] ptn5150 1-003d: DEBUG_IRQ: i2c->irq: 42
[    4.911190] ptn5150 1-003d: DEBUG_IRQ: error in usb_role_switch_get()
[    4.918462] ptn5150 1-003d: No VBUS GPIO, ignoring VBUS control
[    4.926197] ptn5150 1-003d: DEBUG_IRQ: i2c->irq: 42
[    4.935210] ptn5150 1-003d: DEBUG_IRQ: error in usb_role_switch_get()
[    4.947673] ptn5150 1-003d: No VBUS GPIO, ignoring VBUS control
[    4.953771] ptn5150 1-003d: DEBUG_IRQ: i2c->irq: 42
[    4.961104] ptn5150 1-003d: DEBUG_IRQ: error in usb_role_switch_get()
[    5.052165] ptn5150 1-003d: No VBUS GPIO, ignoring VBUS control
[    5.058234] ptn5150 1-003d: DEBUG_IRQ: i2c->irq: 42
[    5.064632] ptn5150 1-003d: DEBUG_IRQ: error in usb_role_switch_get()
[    5.096452] ptn5150 1-003d: No VBUS GPIO, ignoring VBUS control
[    5.102578] ptn5150 1-003d: DEBUG_IRQ: i2c->irq: 42
[    5.114959] ptn5150 1-003d: DEBUG_IRQ: error in usb_role_switch_get()
[    5.187060] ptn5150 1-003d: No VBUS GPIO, ignoring VBUS control
[    5.193243] ptn5150 1-003d: DEBUG_IRQ: i2c->irq: 42
[    5.206235] ptn5150 1-003d: DEBUG_IRQ: probe done


> Also, please add a pullup to the GPIO1_11 pad.

There is already an external 10K pull-up resistor (R88) to +3.3V 
on GPIO1_11 pin...

Hugo.
Fabio Estevam July 4, 2023, 9:50 p.m. UTC | #11 
On Tue, Jul 4, 2023 at 6:28 PM Hugo Villeneuve <hugo@hugovil.com> wrote:

> Hi Fabio,
> it is important to remember that on this board, like I explained
> before, the INTB pin is not connected to anything.
>
> It is only the ID pin (9) that is connected to the GPIO1_11 pin.

Now I looked at the schematics and you are right.

In this case, GPIO1_11 should not be represented as irq then.
Fabio Estevam July 4, 2023, 11 p.m. UTC | #12 
On Tue, Jul 4, 2023 at 6:50 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> On Tue, Jul 4, 2023 at 6:28 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
>
> > Hi Fabio,
> > it is important to remember that on this board, like I explained
> > before, the INTB pin is not connected to anything.
> >
> > It is only the ID pin (9) that is connected to the GPIO1_11 pin.
>
> Now I looked at the schematics and you are right.
>
> In this case, GPIO1_11 should not be represented as irq then.

Variscite added an "irq-is-id-quirk"  property on their tree to handle this:

https://github.com/varigit/linux-imx/commit/fbe6aa2a9c014fdb10b29a715a1be695dac60828
Hugo Villeneuve July 5, 2023, 1:35 p.m. UTC | #13 
On Tue, 4 Jul 2023 20:00:13 -0300
Fabio Estevam <festevam@gmail.com> wrote:

> On Tue, Jul 4, 2023 at 6:50 PM Fabio Estevam <festevam@gmail.com> wrote:
> >
> > On Tue, Jul 4, 2023 at 6:28 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> >
> > > Hi Fabio,
> > > it is important to remember that on this board, like I explained
> > > before, the INTB pin is not connected to anything.
> > >
> > > It is only the ID pin (9) that is connected to the GPIO1_11 pin.
> >
> > Now I looked at the schematics and you are right.
> >
> > In this case, GPIO1_11 should not be represented as irq then.
> 
> Variscite added an "irq-is-id-quirk"  property on their tree to handle this:
> 
> https://github.com/varigit/linux-imx/commit/fbe6aa2a9c014fdb10b29a715a1be695dac60828

Hi Fabio,
what do you think of Variscite's patch, is it something
worth doing?

At least the comment is interesting about the different EVK board
versions: mine is v1.6, which confirms the connection of the ID pin to
the GPIO1_11 pin. This also means that old boards have a connection from
the IRQ pin to GPIO1_11, and for these older boards the DTS is probably
Ok as it is?

How can we support both configurations?

Hugo.
Nate Drude July 5, 2023, 1:42 p.m. UTC | #14 
On 7/5/23 8:35 AM, Hugo Villeneuve wrote:
> [Some people who received this message don't often get email from hugo@hugovil.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Tue, 4 Jul 2023 20:00:13 -0300
> Fabio Estevam <festevam@gmail.com> wrote:
> 
>> On Tue, Jul 4, 2023 at 6:50 PM Fabio Estevam <festevam@gmail.com> wrote:
>>>
>>> On Tue, Jul 4, 2023 at 6:28 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
>>>
>>>> Hi Fabio,
>>>> it is important to remember that on this board, like I explained
>>>> before, the INTB pin is not connected to anything.
>>>>
>>>> It is only the ID pin (9) that is connected to the GPIO1_11 pin.
>>>
>>> Now I looked at the schematics and you are right.
>>>
>>> In this case, GPIO1_11 should not be represented as irq then.
>>
>> Variscite added an "irq-is-id-quirk"  property on their tree to handle this:
>>
>> https://github.com/varigit/linux-imx/commit/fbe6aa2a9c014fdb10b29a715a1be695dac60828
> 
> Hi Fabio,
> what do you think of Variscite's patch, is it something
> worth doing?
> 
> At least the comment is interesting about the different EVK board
> versions: mine is v1.6, which confirms the connection of the ID pin to
> the GPIO1_11 pin. This also means that old boards have a connection from
> the IRQ pin to GPIO1_11, and for these older boards the DTS is probably
> Ok as it is?
> 
> How can we support both configurations?
> 
> Hugo.
> 
> 
The patch 'drivers: extcon: ptn5150: Add irq-is-id-quirk' referred to by Fabio is required for OTG to work correctly on all versions of the Symphony board.

I can submit this patch mainline, do you think it will be accepted as is?

Thanks,
Nate
Fabio Estevam July 5, 2023, 1:49 p.m. UTC | #15 
Hi Nate,

On Wed, Jul 5, 2023 at 10:42 AM Nate Drude <Nate.D@variscite.com> wrote:

> The patch 'drivers: extcon: ptn5150: Add irq-is-id-quirk' referred to by Fabio is required for OTG to work correctly on all versions of the Symphony board.
>
> I can submit this patch mainline, do you think it will be accepted as is?

I think it is worth submitting it to get some feedback from the
ptn5150 and DT maintainers.

Thanks
Hugo Villeneuve July 5, 2023, 2:25 p.m. UTC | #16 
On Wed, 5 Jul 2023 10:49:01 -0300
Fabio Estevam <festevam@gmail.com> wrote:

> Hi Nate,
> 
> On Wed, Jul 5, 2023 at 10:42 AM Nate Drude <Nate.D@variscite.com> wrote:
> 
> > The patch 'drivers: extcon: ptn5150: Add irq-is-id-quirk' referred to by Fabio is required for OTG to work correctly on all versions of the Symphony board.
> >
> > I can submit this patch mainline, do you think it will be accepted as is?
> 
> I think it is worth submitting it to get some feedback from the
> ptn5150 and DT maintainers.
> 
> Thanks

Hi,
if I understand correctly, the irq-is-id-quirk device tree property
would be required for newer EVK boards, but not for older boards.

How can we support both configurations with the current device tree?

Hugo.
Nate Drude July 5, 2023, 2:31 p.m. UTC | #17 
Hi Hugo,

On 7/5/23 9:25 AM, Hugo Villeneuve wrote:
> [Some people who received this message don't often get email from hugo@hugovil.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Wed, 5 Jul 2023 10:49:01 -0300
> Fabio Estevam <festevam@gmail.com> wrote:
> 
>> Hi Nate,
>>
>> On Wed, Jul 5, 2023 at 10:42 AM Nate Drude <Nate.D@variscite.com> wrote:
>>
>>> The patch 'drivers: extcon: ptn5150: Add irq-is-id-quirk' referred to by Fabio is required for OTG to work correctly on all versions of the Symphony board.
>>>
>>> I can submit this patch mainline, do you think it will be accepted as is?
>>
>> I think it is worth submitting it to get some feedback from the
>> ptn5150 and DT maintainers.
>>
>> Thanks
> 
> Hi,
> if I understand correctly, the irq-is-id-quirk device tree property
> would be required for newer EVK boards, but not for older boards.
> 
> How can we support both configurations with the current device tree?
> 
> Hugo.
For some time, we maintained a legacy device tree file: https://github.com/varigit/linux-imx/blob/5.4-2.1.x-imx_var01/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony-legacy.dts

However, it is no longer being maintained in our newest kernel branches.

Regards,
Nate
Hugo Villeneuve July 5, 2023, 2:48 p.m. UTC | #18 
On Wed, 5 Jul 2023 14:31:40 +0000
Nate Drude <Nate.D@variscite.com> wrote:

> Hi Hugo,
> 
> On 7/5/23 9:25 AM, Hugo Villeneuve wrote:
> > [Some people who received this message don't often get email from hugo@hugovil.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> > 
> > On Wed, 5 Jul 2023 10:49:01 -0300
> > Fabio Estevam <festevam@gmail.com> wrote:
> > 
> >> Hi Nate,
> >>
> >> On Wed, Jul 5, 2023 at 10:42 AM Nate Drude <Nate.D@variscite.com> wrote:
> >>
> >>> The patch 'drivers: extcon: ptn5150: Add irq-is-id-quirk' referred to by Fabio is required for OTG to work correctly on all versions of the Symphony board.
> >>>
> >>> I can submit this patch mainline, do you think it will be accepted as is?
> >>
> >> I think it is worth submitting it to get some feedback from the
> >> ptn5150 and DT maintainers.
> >>
> >> Thanks
> > 
> > Hi,
> > if I understand correctly, the irq-is-id-quirk device tree property
> > would be required for newer EVK boards, but not for older boards.
> > 
> > How can we support both configurations with the current device tree?
> > 
> > Hugo.
> For some time, we maintained a legacy device tree file: https://github.com/varigit/linux-imx/blob/5.4-2.1.x-imx_var01/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony-legacy.dts
> 
> However, it is no longer being maintained in our newest kernel branches.
> 
> Regards,
> Nate

Fabio: do we need to support both configurations in the Linux kernel
tree, and if yes how do you propose we do it?

Thank you,
Hugo.
Fabio Estevam July 5, 2023, 3:22 p.m. UTC | #19 
Hi Hugo,

On Wed, Jul 5, 2023 at 11:48 AM Hugo Villeneuve <hugo@hugovil.com> wrote:

> Fabio: do we need to support both configurations in the Linux kernel
> tree, and if yes how do you propose we do it?

I would suggest supporting the new revision only.

Thanks
Hugo Villeneuve July 5, 2023, 3:34 p.m. UTC | #20 
On Wed, 5 Jul 2023 12:22:55 -0300
Fabio Estevam <festevam@gmail.com> wrote:

> Hi Hugo,
> 
> On Wed, Jul 5, 2023 at 11:48 AM Hugo Villeneuve <hugo@hugovil.com> wrote:
> 
> > Fabio: do we need to support both configurations in the Linux kernel
> > tree, and if yes how do you propose we do it?
> 
> I would suggest supporting the new revision only.

Ok, no problem.

If we go back to my original patch, the changes in it, apart from the
interrupt, are still required to make USB OTG work (at least in host
mode, so that we can plug a USB key for example). Also looking at the
latest varigit changes, I have removed the "typec1_con:
connector" node (tested ok in host mode). I also added comments in the
DTS about the particular PTN5150 interrupt pin configurations.

Let me know if I can resubmit it, and if so can I leave the interrupt
property type to IRQ_TYPE_NONE?

Thank you,
Hugo.
Krzysztof Kozlowski July 5, 2023, 3:44 p.m. UTC | #21 
On 05/07/2023 17:34, Hugo Villeneuve wrote:
> On Wed, 5 Jul 2023 12:22:55 -0300
> Fabio Estevam <festevam@gmail.com> wrote:
> 
>> Hi Hugo,
>>
>> On Wed, Jul 5, 2023 at 11:48 AM Hugo Villeneuve <hugo@hugovil.com> wrote:
>>
>>> Fabio: do we need to support both configurations in the Linux kernel
>>> tree, and if yes how do you propose we do it?
>>
>> I would suggest supporting the new revision only.
> 
> Ok, no problem.
> 
> If we go back to my original patch, the changes in it, apart from the
> interrupt, are still required to make USB OTG work (at least in host
> mode, so that we can plug a USB key for example). Also looking at the
> latest varigit changes, I have removed the "typec1_con:
> connector" node (tested ok in host mode). I also added comments in the
> DTS about the particular PTN5150 interrupt pin configurations.
> 
> Let me know if I can resubmit it, and if so can I leave the interrupt
> property type to IRQ_TYPE_NONE?

As I wrote, interrupt type cannot be none. What does it even mean "none"
for your case?

Best regards,
Krzysztof
Hugo Villeneuve July 5, 2023, 3:51 p.m. UTC | #22 
On Wed, 5 Jul 2023 17:44:05 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 05/07/2023 17:34, Hugo Villeneuve wrote:
> > On Wed, 5 Jul 2023 12:22:55 -0300
> > Fabio Estevam <festevam@gmail.com> wrote:
> > 
> >> Hi Hugo,
> >>
> >> On Wed, Jul 5, 2023 at 11:48 AM Hugo Villeneuve <hugo@hugovil.com> wrote:
> >>
> >>> Fabio: do we need to support both configurations in the Linux kernel
> >>> tree, and if yes how do you propose we do it?
> >>
> >> I would suggest supporting the new revision only.
> > 
> > Ok, no problem.
> > 
> > If we go back to my original patch, the changes in it, apart from the
> > interrupt, are still required to make USB OTG work (at least in host
> > mode, so that we can plug a USB key for example). Also looking at the
> > latest varigit changes, I have removed the "typec1_con:
> > connector" node (tested ok in host mode). I also added comments in the
> > DTS about the particular PTN5150 interrupt pin configurations.
> > 
> > Let me know if I can resubmit it, and if so can I leave the interrupt
> > property type to IRQ_TYPE_NONE?
> 
> As I wrote, interrupt type cannot be none. What does it even mean "none"
> for your case?

Hi,
I have no idea why Variscite are using this IRQ type of NONE.

I can put IRQ_TYPE_EDGE_FALLING since I tested it and it works.

Hugo.
Krzysztof Kozlowski July 5, 2023, 3:54 p.m. UTC | #23 
On 05/07/2023 17:51, Hugo Villeneuve wrote:
>> As I wrote, interrupt type cannot be none. What does it even mean "none"
>> for your case?
> 
> Hi,
> I have no idea why Variscite are using this IRQ type of NONE.

Because it worked :)

> 
> I can put IRQ_TYPE_EDGE_FALLING since I tested it and it works.

Seems reasonable because on schematics this looked pulled up.

Best regards,
Krzysztof
Hugo Villeneuve July 5, 2023, 3:57 p.m. UTC | #24 
On Wed, 5 Jul 2023 17:54:05 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 05/07/2023 17:51, Hugo Villeneuve wrote:
> >> As I wrote, interrupt type cannot be none. What does it even mean "none"
> >> for your case?
> > 
> > Hi,
> > I have no idea why Variscite are using this IRQ type of NONE.
> 
> Because it worked :)

lol

> > 
> > I can put IRQ_TYPE_EDGE_FALLING since I tested it and it works.
> 
> Seems reasonable because on schematics this looked pulled up.

Ok, I will resubmit a V2 of the patch then with this.

Thank you,
Hugo.
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts b/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
index 406a711486da..aef89198f24c 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
@@ -6,6 +6,7 @@ 
 
 /dts-v1/;
 
+#include <dt-bindings/usb/pd.h>
 #include "imx8mn-var-som.dtsi"
 
 / {
@@ -104,10 +105,29 @@  extcon_usbotg1: typec@3d {
 		compatible = "nxp,ptn5150";
 		reg = <0x3d>;
 		interrupt-parent = <&gpio1>;
-		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
+		interrupts = <11 IRQ_TYPE_NONE>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinctrl_ptn5150>;
 		status = "okay";
+
+		port {
+			typec1_dr_sw: endpoint {
+				remote-endpoint = <&usb1_drd_sw>;
+			};
+		};
+
+		typec1_con: connector {
+			compatible = "usb-c-connector";
+			label = "USB-C";
+			power-role = "dual";
+			data-role = "dual";
+			try-power-role = "sink";
+			source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
+			sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
+				     PDO_VAR(5000, 20000, 3000)>;
+			op-sink-microwatt = <15000000>;
+			self-powered;
+		};
 	};
 };
 
@@ -148,8 +168,21 @@  &uart3 {
 };
 
 &usbotg1 {
+	dr_mode = "otg";
+	hnp-disable;
+	srp-disable;
+	adp-disable;
+	usb-role-switch;
 	disable-over-current;
-	extcon = <&extcon_usbotg1>, <&extcon_usbotg1>;
+	samsung,picophy-pre-emp-curr-control = <3>;
+	samsung,picophy-dc-vol-level-adjust = <7>;
+	status = "okay";
+
+	port {
+		usb1_drd_sw: endpoint {
+			remote-endpoint = <&typec1_dr_sw>;
+		};
+	};
 };
 
 &iomuxc {