Re: [U-Boot] [PATCH] usb: dwc3: Handle case where setup_phy is not needed

2019-06-06 Thread Siva Durga Prasad Paladugu
Hi,

> -Original Message-
> From: Siva Durga Prasad Paladugu
> Sent: Tuesday, May 28, 2019 3:36 PM
> To: Michal Simek ; Lukasz Majewski
> 
> Cc: u-boot@lists.denx.de; ma...@denx.de; jjhib...@ti.com;
> s...@chromium.org; patrick.delau...@st.com
> Subject: RE: [PATCH] usb: dwc3: Handle case where setup_phy is not needed
> 
> Hi,
> 
> > -Original Message-
> > From: Michal Simek [mailto:michal.si...@xilinx.com]
> > Sent: Monday, April 1, 2019 4:10 PM
> > To: Siva Durga Prasad Paladugu ; Lukasz Majewski
> > 
> > Cc: u-boot@lists.denx.de; ma...@denx.de; jjhib...@ti.com;
> > s...@chromium.org; patrick.delau...@st.com
> > Subject: Re: [PATCH] usb: dwc3: Handle case where setup_phy is not
> > needed
> >
> > On 01. 04. 19 12:38, Siva Durga Prasad Paladugu wrote:
> > > Hi Lukasz,
> > >
> > >> -Original Message-
> > >> From: Lukasz Majewski [mailto:lu...@denx.de]
> > >> Sent: Monday, April 01, 2019 4:04 PM
> > >> To: Siva Durga Prasad Paladugu 
> > >> Cc: u-boot@lists.denx.de; ma...@denx.de; jjhib...@ti.com;
> > >> s...@chromium.org; patrick.delau...@st.com; Michal Simek
> > >> 
> > >> Subject: Re: [PATCH] usb: dwc3: Handle case where setup_phy is not
> > >> needed
> > >>
> > >> On Mon, 1 Apr 2019 10:23:42 +
> > >> Siva Durga Prasad Paladugu  wrote:
> > >>
> > >>> Hi Lukasz,
> > >>>
> >  -Original Message-
> >  From: Lukasz Majewski [mailto:lu...@denx.de]
> >  Sent: Monday, April 01, 2019 1:03 PM
> >  To: Siva Durga Prasad Paladugu 
> >  Cc: u-boot@lists.denx.de; ma...@denx.de; jjhib...@ti.com;
> >  s...@chromium.org; patrick.delau...@st.com; Michal Simek
> >  
> >  Subject: Re: [PATCH] usb: dwc3: Handle case where setup_phy is
> >  not needed
> > 
> >  Hi Siva Durga Prasad Paladugu,
> > 
> > > If CONFIG_PHY is not enabled then the dwc3_setup_phy() returns
> > > ENOTSUPP which can be still valid and intentional
> >  ^^^ -
> >  could you elaborate on this a bit more?
> > 
> >  What is the use case when -ENOTSUPP is valid and intentional ?
> > >>>
> > >>> In the cases where phy will be setup by First stage boot loader
> > >>> itself and don't want to do it again at u-boot.
> > >>
> > >> I think that I saw some time ago similar patch for uart - on IMX
> > >> IIRC (it was also setup by BootROM on SoC).
> > >>
> > >> Maybe we would need some kind of switch in Kconfig to indicate this
> > >> use case?
> > >>
> > >> Another option would be to mark this in DTS as TI specific property?
> > >>
> > >> My point is that we shall not proceed with the flow when we do
> > >> receive - ENOTSUPP
> > >
> > > TBH, I feel we should not call dwc3_setup_phy() if CONFIG_PHY is not
> > > enabled. If we look at dwc3_setup_phy() definition, it is under
> > CONFIG_PHY.
> >
> > +1 on this.
> 
> Any update on this patch?

This is how its already being handled in drivers/usb/host/xhci-dwc3.c at line 
127 in routine xhci_dwc3_probe() (snippet below)

ret = dwc3_setup_phy(dev, >usb_phys, >num_phys);
if (ret && (ret != -ENOTSUPP))
return ret;

We have two options either treat -ENOTSUPP as valid as above patch which I sent 
or don’t invoke dwc3_setup_phy() if CONFIG_PHY is not enabled.

Thanks,
Siva
> 
> Thanks,
> Siva
> 
> >
> > M

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] usb: dwc3: Handle case where setup_phy is not needed

2019-05-28 Thread Siva Durga Prasad Paladugu
Hi,

> -Original Message-
> From: Michal Simek [mailto:michal.si...@xilinx.com]
> Sent: Monday, April 1, 2019 4:10 PM
> To: Siva Durga Prasad Paladugu ; Lukasz Majewski
> 
> Cc: u-boot@lists.denx.de; ma...@denx.de; jjhib...@ti.com;
> s...@chromium.org; patrick.delau...@st.com
> Subject: Re: [PATCH] usb: dwc3: Handle case where setup_phy is not needed
> 
> On 01. 04. 19 12:38, Siva Durga Prasad Paladugu wrote:
> > Hi Lukasz,
> >
> >> -Original Message-
> >> From: Lukasz Majewski [mailto:lu...@denx.de]
> >> Sent: Monday, April 01, 2019 4:04 PM
> >> To: Siva Durga Prasad Paladugu 
> >> Cc: u-boot@lists.denx.de; ma...@denx.de; jjhib...@ti.com;
> >> s...@chromium.org; patrick.delau...@st.com; Michal Simek
> >> 
> >> Subject: Re: [PATCH] usb: dwc3: Handle case where setup_phy is not
> >> needed
> >>
> >> On Mon, 1 Apr 2019 10:23:42 +
> >> Siva Durga Prasad Paladugu  wrote:
> >>
> >>> Hi Lukasz,
> >>>
>  -Original Message-
>  From: Lukasz Majewski [mailto:lu...@denx.de]
>  Sent: Monday, April 01, 2019 1:03 PM
>  To: Siva Durga Prasad Paladugu 
>  Cc: u-boot@lists.denx.de; ma...@denx.de; jjhib...@ti.com;
>  s...@chromium.org; patrick.delau...@st.com; Michal Simek
>  
>  Subject: Re: [PATCH] usb: dwc3: Handle case where setup_phy is not
>  needed
> 
>  Hi Siva Durga Prasad Paladugu,
> 
> > If CONFIG_PHY is not enabled then the dwc3_setup_phy() returns
> > ENOTSUPP which can be still valid and intentional
>  ^^^ - could
>  you elaborate on this a bit more?
> 
>  What is the use case when -ENOTSUPP is valid and intentional ?
> >>>
> >>> In the cases where phy will be setup by First stage boot loader
> >>> itself and don't want to do it again at u-boot.
> >>
> >> I think that I saw some time ago similar patch for uart - on IMX IIRC
> >> (it was also setup by BootROM on SoC).
> >>
> >> Maybe we would need some kind of switch in Kconfig to indicate this
> >> use case?
> >>
> >> Another option would be to mark this in DTS as TI specific property?
> >>
> >> My point is that we shall not proceed with the flow when we do
> >> receive - ENOTSUPP
> >
> > TBH, I feel we should not call dwc3_setup_phy() if CONFIG_PHY is not
> > enabled. If we look at dwc3_setup_phy() definition, it is under
> CONFIG_PHY.
> 
> +1 on this.

Any update on this patch?

Thanks,
Siva

> 
> M

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] usb: dwc3: Handle case where setup_phy is not needed

2019-04-01 Thread Michal Simek
On 01. 04. 19 12:38, Siva Durga Prasad Paladugu wrote:
> Hi Lukasz,
> 
>> -Original Message-
>> From: Lukasz Majewski [mailto:lu...@denx.de]
>> Sent: Monday, April 01, 2019 4:04 PM
>> To: Siva Durga Prasad Paladugu 
>> Cc: u-boot@lists.denx.de; ma...@denx.de; jjhib...@ti.com;
>> s...@chromium.org; patrick.delau...@st.com; Michal Simek
>> 
>> Subject: Re: [PATCH] usb: dwc3: Handle case where setup_phy is not
>> needed
>>
>> On Mon, 1 Apr 2019 10:23:42 +
>> Siva Durga Prasad Paladugu  wrote:
>>
>>> Hi Lukasz,
>>>
 -Original Message-
 From: Lukasz Majewski [mailto:lu...@denx.de]
 Sent: Monday, April 01, 2019 1:03 PM
 To: Siva Durga Prasad Paladugu 
 Cc: u-boot@lists.denx.de; ma...@denx.de; jjhib...@ti.com;
 s...@chromium.org; patrick.delau...@st.com; Michal Simek
 
 Subject: Re: [PATCH] usb: dwc3: Handle case where setup_phy is not
 needed

 Hi Siva Durga Prasad Paladugu,

> If CONFIG_PHY is not enabled then the dwc3_setup_phy() returns
> ENOTSUPP which can be still valid and intentional
 ^^^ - could
 you elaborate on this a bit more?

 What is the use case when -ENOTSUPP is valid and intentional ?
>>>
>>> In the cases where phy will be setup by First stage boot loader itself
>>> and don't want to do it again at u-boot.
>>
>> I think that I saw some time ago similar patch for uart - on IMX IIRC (it was
>> also setup by BootROM on SoC).
>>
>> Maybe we would need some kind of switch in Kconfig to indicate this use
>> case?
>>
>> Another option would be to mark this in DTS as TI specific property?
>>
>> My point is that we shall not proceed with the flow when we do receive -
>> ENOTSUPP
> 
> TBH, I feel we should not call dwc3_setup_phy() if CONFIG_PHY is not enabled. 
> If we look at dwc3_setup_phy() definition, it is under 
CONFIG_PHY.

+1 on this.

M

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] usb: dwc3: Handle case where setup_phy is not needed

2019-04-01 Thread Siva Durga Prasad Paladugu
Hi Lukasz,

> -Original Message-
> From: Lukasz Majewski [mailto:lu...@denx.de]
> Sent: Monday, April 01, 2019 4:04 PM
> To: Siva Durga Prasad Paladugu 
> Cc: u-boot@lists.denx.de; ma...@denx.de; jjhib...@ti.com;
> s...@chromium.org; patrick.delau...@st.com; Michal Simek
> 
> Subject: Re: [PATCH] usb: dwc3: Handle case where setup_phy is not
> needed
> 
> On Mon, 1 Apr 2019 10:23:42 +
> Siva Durga Prasad Paladugu  wrote:
> 
> > Hi Lukasz,
> >
> > > -Original Message-
> > > From: Lukasz Majewski [mailto:lu...@denx.de]
> > > Sent: Monday, April 01, 2019 1:03 PM
> > > To: Siva Durga Prasad Paladugu 
> > > Cc: u-boot@lists.denx.de; ma...@denx.de; jjhib...@ti.com;
> > > s...@chromium.org; patrick.delau...@st.com; Michal Simek
> > > 
> > > Subject: Re: [PATCH] usb: dwc3: Handle case where setup_phy is not
> > > needed
> > >
> > > Hi Siva Durga Prasad Paladugu,
> > >
> > > > If CONFIG_PHY is not enabled then the dwc3_setup_phy() returns
> > > > ENOTSUPP which can be still valid and intentional
> > > ^^^ - could
> > > you elaborate on this a bit more?
> > >
> > > What is the use case when -ENOTSUPP is valid and intentional ?
> >
> > In the cases where phy will be setup by First stage boot loader itself
> > and don't want to do it again at u-boot.
> 
> I think that I saw some time ago similar patch for uart - on IMX IIRC (it was
> also setup by BootROM on SoC).
> 
> Maybe we would need some kind of switch in Kconfig to indicate this use
> case?
> 
> Another option would be to mark this in DTS as TI specific property?
> 
> My point is that we shall not proceed with the flow when we do receive -
> ENOTSUPP

TBH, I feel we should not call dwc3_setup_phy() if CONFIG_PHY is not enabled. 
If we look at dwc3_setup_phy() definition, it is under CONFIG_PHY.

Thanks,
Siva

> 
> >
> > Thanks,
> > Siva
> >
> > >
> > > > so modify error check to handle this -ENOTSUPP.
> > > >
> > > > Signed-off-by: Siva Durga Prasad Paladugu
> > > >  ---
> > > > drivers/usb/dwc3/dwc3-generic.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/usb/dwc3/dwc3-generic.c
> > > > b/drivers/usb/dwc3/dwc3-generic.c index 3e6c494..a261d8d 100644
> > > > --- a/drivers/usb/dwc3/dwc3-generic.c
> > > > +++ b/drivers/usb/dwc3/dwc3-generic.c
> > > > @@ -47,7 +47,7 @@ static int dwc3_generic_peripheral_probe(struct
> > > > udevice *dev) struct dwc3 *dwc3 = >dwc3;
> > > >
> > > > rc = dwc3_setup_phy(dev, >phys, >num_phys);
> > > > -   if (rc)
> > > > +   if (rc && rc != -ENOTSUPP)
> > > > return rc;
> > > >
> > > > dwc3->regs = map_physmem(priv->base, DWC3_OTG_REGS_END,
> > > > MAP_NOCACHE);
> > >
> > >
> > >
> > >
> > > Best regards,
> > >
> > > Lukasz Majewski
> > >
> > > --
> > >
> > > DENX Software Engineering GmbH,  Managing Director: Wolfgang
> > > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80
> Email:
> > > lu...@denx.de
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> lu...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] usb: dwc3: Handle case where setup_phy is not needed

2019-04-01 Thread Lukasz Majewski
On Mon, 1 Apr 2019 10:23:42 +
Siva Durga Prasad Paladugu  wrote:

> Hi Lukasz,
> 
> > -Original Message-
> > From: Lukasz Majewski [mailto:lu...@denx.de]
> > Sent: Monday, April 01, 2019 1:03 PM
> > To: Siva Durga Prasad Paladugu 
> > Cc: u-boot@lists.denx.de; ma...@denx.de; jjhib...@ti.com;
> > s...@chromium.org; patrick.delau...@st.com; Michal Simek
> > 
> > Subject: Re: [PATCH] usb: dwc3: Handle case where setup_phy is not
> > needed
> > 
> > Hi Siva Durga Prasad Paladugu,
> >   
> > > If CONFIG_PHY is not enabled then the dwc3_setup_phy() returns
> > > ENOTSUPP which can be still valid and intentional  
> > ^^^ - could
> > you elaborate on this a bit more?
> > 
> > What is the use case when -ENOTSUPP is valid and intentional ?  
> 
> In the cases where phy will be setup by First stage boot loader
> itself and don't want to do it again at u-boot.

I think that I saw some time ago similar patch for uart - on IMX IIRC
(it was also setup by BootROM on SoC).

Maybe we would need some kind of switch in Kconfig to indicate this use
case?

Another option would be to mark this in DTS as TI specific property?

My point is that we shall not proceed with the flow when we do receive
-ENOTSUPP 

> 
> Thanks,
> Siva
> 
> >   
> > > so modify error check to handle this -ENOTSUPP.
> > >
> > > Signed-off-by: Siva Durga Prasad Paladugu
> > >  ---
> > > drivers/usb/dwc3/dwc3-generic.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/dwc3/dwc3-generic.c
> > > b/drivers/usb/dwc3/dwc3-generic.c index 3e6c494..a261d8d 100644
> > > --- a/drivers/usb/dwc3/dwc3-generic.c
> > > +++ b/drivers/usb/dwc3/dwc3-generic.c
> > > @@ -47,7 +47,7 @@ static int dwc3_generic_peripheral_probe(struct
> > > udevice *dev) struct dwc3 *dwc3 = >dwc3;
> > >
> > >   rc = dwc3_setup_phy(dev, >phys, >num_phys);
> > > - if (rc)
> > > + if (rc && rc != -ENOTSUPP)
> > >   return rc;
> > >
> > >   dwc3->regs = map_physmem(priv->base, DWC3_OTG_REGS_END,
> > > MAP_NOCACHE);  
> > 
> > 
> > 
> > 
> > Best regards,
> > 
> > Lukasz Majewski
> > 
> > --
> > 
> > DENX Software Engineering GmbH,  Managing Director: Wolfgang
> > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > lu...@denx.de  




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lu...@denx.de


pgp2YLYxcx045.pgp
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] usb: dwc3: Handle case where setup_phy is not needed

2019-04-01 Thread Siva Durga Prasad Paladugu
Hi Lukasz,

> -Original Message-
> From: Lukasz Majewski [mailto:lu...@denx.de]
> Sent: Monday, April 01, 2019 1:03 PM
> To: Siva Durga Prasad Paladugu 
> Cc: u-boot@lists.denx.de; ma...@denx.de; jjhib...@ti.com;
> s...@chromium.org; patrick.delau...@st.com; Michal Simek
> 
> Subject: Re: [PATCH] usb: dwc3: Handle case where setup_phy is not
> needed
> 
> Hi Siva Durga Prasad Paladugu,
> 
> > If CONFIG_PHY is not enabled then the dwc3_setup_phy() returns
> > ENOTSUPP which can be still valid and intentional
> ^^^ - could you
> elaborate on this a bit more?
> 
> What is the use case when -ENOTSUPP is valid and intentional ?

In the cases where phy will be setup by First stage boot loader itself and 
don't want to do it again at u-boot.

Thanks,
Siva

> 
> > so modify error check to handle this -ENOTSUPP.
> >
> > Signed-off-by: Siva Durga Prasad Paladugu
> >  ---  drivers/usb/dwc3/dwc3-generic.c
> > | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/dwc3/dwc3-generic.c
> > b/drivers/usb/dwc3/dwc3-generic.c index 3e6c494..a261d8d 100644
> > --- a/drivers/usb/dwc3/dwc3-generic.c
> > +++ b/drivers/usb/dwc3/dwc3-generic.c
> > @@ -47,7 +47,7 @@ static int dwc3_generic_peripheral_probe(struct
> > udevice *dev) struct dwc3 *dwc3 = >dwc3;
> >
> > rc = dwc3_setup_phy(dev, >phys, >num_phys);
> > -   if (rc)
> > +   if (rc && rc != -ENOTSUPP)
> > return rc;
> >
> > dwc3->regs = map_physmem(priv->base, DWC3_OTG_REGS_END,
> > MAP_NOCACHE);
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> lu...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] usb: dwc3: Handle case where setup_phy is not needed

2019-04-01 Thread Lukasz Majewski
Hi Siva Durga Prasad Paladugu,

> If CONFIG_PHY is not enabled then the dwc3_setup_phy()
> returns ENOTSUPP which can be still valid and intentional
^^^ - could you
elaborate on this a bit more?

What is the use case when -ENOTSUPP is valid and intentional ?

> so modify error check to handle this -ENOTSUPP.
> 
> Signed-off-by: Siva Durga Prasad Paladugu
>  ---
>  drivers/usb/dwc3/dwc3-generic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-generic.c
> b/drivers/usb/dwc3/dwc3-generic.c index 3e6c494..a261d8d 100644
> --- a/drivers/usb/dwc3/dwc3-generic.c
> +++ b/drivers/usb/dwc3/dwc3-generic.c
> @@ -47,7 +47,7 @@ static int dwc3_generic_peripheral_probe(struct
> udevice *dev) struct dwc3 *dwc3 = >dwc3;
>  
>   rc = dwc3_setup_phy(dev, >phys, >num_phys);
> - if (rc)
> + if (rc && rc != -ENOTSUPP)
>   return rc;
>  
>   dwc3->regs = map_physmem(priv->base, DWC3_OTG_REGS_END,
> MAP_NOCACHE);




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lu...@denx.de


pgpb9yYEx1MvF.pgp
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH] usb: dwc3: Handle case where setup_phy is not needed

2019-04-01 Thread Siva Durga Prasad Paladugu
If CONFIG_PHY is not enabled then the dwc3_setup_phy()
returns ENOTSUPP which can be still valid and intentional
so modify error check to handle this -ENOTSUPP.

Signed-off-by: Siva Durga Prasad Paladugu 
---
 drivers/usb/dwc3/dwc3-generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
index 3e6c494..a261d8d 100644
--- a/drivers/usb/dwc3/dwc3-generic.c
+++ b/drivers/usb/dwc3/dwc3-generic.c
@@ -47,7 +47,7 @@ static int dwc3_generic_peripheral_probe(struct udevice *dev)
struct dwc3 *dwc3 = >dwc3;
 
rc = dwc3_setup_phy(dev, >phys, >num_phys);
-   if (rc)
+   if (rc && rc != -ENOTSUPP)
return rc;
 
dwc3->regs = map_physmem(priv->base, DWC3_OTG_REGS_END, MAP_NOCACHE);
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot