Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-02-13 Thread Felipe Balbi
On Fri, Feb 13, 2015 at 02:03:45PM -0800, David Cohen wrote:
> Hi Heikki,
> 
> On Fri, Feb 13, 2015 at 03:16:40PM +0200, Heikki Krogerus wrote:
> > On Thu, Feb 12, 2015 at 05:41:30PM -0800, David Cohen wrote:
> > > On Thu, Feb 12, 2015 at 02:12:14PM +0200, Heikki Krogerus wrote:
> > > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > > > index a8c9062..66cbf38 100644
> > > > > > --- a/drivers/usb/dwc3/core.c
> > > > > > +++ b/drivers/usb/dwc3/core.c
> > > > > > @@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device 
> > > > > > *pdev)
> > > > > > platform_set_drvdata(pdev, dwc);
> > > > > > dwc3_cache_hwparams(dwc);
> > > > > >  
> > > > > > +   ret = dwc3_ulpi_init(dwc);
> > > > > 
> > > > > If I understood correctly, this call will result in enumerating the 
> > > > > phy
> > > > > via ULPI bus, then registering the correct ULPI device.
> > > > > Can you guarantee ULPI will be always accessible at this point if we
> > > > > remove dwc3 module and load it again?
> > > > 
> > > > OK, got it. So yes, I can guarantee that ULPI will be acessible at
> > > > this point. If we are in a state where we could soft reset dwc3, we
> > > > know we can access ULPI. The fact that dwc3 itself expects to be able
> > > > to write to the ULPI registers at that point guarantees it for us.
> > > 
> > > I just double checked DWC3 TRM.
> > > You are correct, by the time we're executing dwc3_core_soft_reset() ULPI
> > > bus is already accessible. But the TRM also specifies an ULPI phy would
> > > be reset by DCTL's core soft reset, which is executed by dwc3_core_init()
> > > before calling dwc3_core_soft_reset(). It does mention DWC3 writes data
> > > to an ULPI phy register during reset, but it also mentions the clock
> > > sync happens during that time.
> > > 
> > > That makes no even OK, but more correct IMO to power on phy before
> > > core's soft reset (i.e. by ACPI methods). But it lets us in a grey area
> > > whether ULPI is reliably accessible before core's soft reset.
> > > 
> > > I believe if you move the dwc3_ulpi_init() to dwc3_core_init(), after
> > > core's soft reset we've got no more grey area.
> > 
> > Well, we have already requested the phys in dwc3_probe before that so
> > some refactoring will be needed. It's probable no a problem.
> 
> Sounds good :)
> 
> > 
> > Btw, I'm sorry about telling this so late, but I'm going to be on
> > vacation for the next two weeks.
> 
> Thanks for the notice.

you'll be back right around merge window closing time :-) Not to worry,
we'll have time to get this into the next merge window.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-02-13 Thread David Cohen
Hi Heikki,

On Fri, Feb 13, 2015 at 03:16:40PM +0200, Heikki Krogerus wrote:
> On Thu, Feb 12, 2015 at 05:41:30PM -0800, David Cohen wrote:
> > On Thu, Feb 12, 2015 at 02:12:14PM +0200, Heikki Krogerus wrote:
> > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > > index a8c9062..66cbf38 100644
> > > > > --- a/drivers/usb/dwc3/core.c
> > > > > +++ b/drivers/usb/dwc3/core.c
> > > > > @@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device 
> > > > > *pdev)
> > > > >   platform_set_drvdata(pdev, dwc);
> > > > >   dwc3_cache_hwparams(dwc);
> > > > >  
> > > > > + ret = dwc3_ulpi_init(dwc);
> > > > 
> > > > If I understood correctly, this call will result in enumerating the phy
> > > > via ULPI bus, then registering the correct ULPI device.
> > > > Can you guarantee ULPI will be always accessible at this point if we
> > > > remove dwc3 module and load it again?
> > > 
> > > OK, got it. So yes, I can guarantee that ULPI will be acessible at
> > > this point. If we are in a state where we could soft reset dwc3, we
> > > know we can access ULPI. The fact that dwc3 itself expects to be able
> > > to write to the ULPI registers at that point guarantees it for us.
> > 
> > I just double checked DWC3 TRM.
> > You are correct, by the time we're executing dwc3_core_soft_reset() ULPI
> > bus is already accessible. But the TRM also specifies an ULPI phy would
> > be reset by DCTL's core soft reset, which is executed by dwc3_core_init()
> > before calling dwc3_core_soft_reset(). It does mention DWC3 writes data
> > to an ULPI phy register during reset, but it also mentions the clock
> > sync happens during that time.
> > 
> > That makes no even OK, but more correct IMO to power on phy before
> > core's soft reset (i.e. by ACPI methods). But it lets us in a grey area
> > whether ULPI is reliably accessible before core's soft reset.
> > 
> > I believe if you move the dwc3_ulpi_init() to dwc3_core_init(), after
> > core's soft reset we've got no more grey area.
> 
> Well, we have already requested the phys in dwc3_probe before that so
> some refactoring will be needed. It's probable no a problem.

Sounds good :)

> 
> Btw, I'm sorry about telling this so late, but I'm going to be on
> vacation for the next two weeks.

Thanks for the notice.

Br, David

> 
> 
> Cheers,
> 
> -- 
> heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-02-13 Thread Heikki Krogerus
On Thu, Feb 12, 2015 at 05:41:30PM -0800, David Cohen wrote:
> On Thu, Feb 12, 2015 at 02:12:14PM +0200, Heikki Krogerus wrote:
> > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > index a8c9062..66cbf38 100644
> > > > --- a/drivers/usb/dwc3/core.c
> > > > +++ b/drivers/usb/dwc3/core.c
> > > > @@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device *pdev)
> > > > platform_set_drvdata(pdev, dwc);
> > > > dwc3_cache_hwparams(dwc);
> > > >  
> > > > +   ret = dwc3_ulpi_init(dwc);
> > > 
> > > If I understood correctly, this call will result in enumerating the phy
> > > via ULPI bus, then registering the correct ULPI device.
> > > Can you guarantee ULPI will be always accessible at this point if we
> > > remove dwc3 module and load it again?
> > 
> > OK, got it. So yes, I can guarantee that ULPI will be acessible at
> > this point. If we are in a state where we could soft reset dwc3, we
> > know we can access ULPI. The fact that dwc3 itself expects to be able
> > to write to the ULPI registers at that point guarantees it for us.
> 
> I just double checked DWC3 TRM.
> You are correct, by the time we're executing dwc3_core_soft_reset() ULPI
> bus is already accessible. But the TRM also specifies an ULPI phy would
> be reset by DCTL's core soft reset, which is executed by dwc3_core_init()
> before calling dwc3_core_soft_reset(). It does mention DWC3 writes data
> to an ULPI phy register during reset, but it also mentions the clock
> sync happens during that time.
> 
> That makes no even OK, but more correct IMO to power on phy before
> core's soft reset (i.e. by ACPI methods). But it lets us in a grey area
> whether ULPI is reliably accessible before core's soft reset.
> 
> I believe if you move the dwc3_ulpi_init() to dwc3_core_init(), after
> core's soft reset we've got no more grey area.

Well, we have already requested the phys in dwc3_probe before that so
some refactoring will be needed. It's probable no a problem.

Btw, I'm sorry about telling this so late, but I'm going to be on
vacation for the next two weeks.


Cheers,

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-02-13 Thread Heikki Krogerus
On Thu, Feb 12, 2015 at 05:41:30PM -0800, David Cohen wrote:
 On Thu, Feb 12, 2015 at 02:12:14PM +0200, Heikki Krogerus wrote:
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index a8c9062..66cbf38 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, dwc);
dwc3_cache_hwparams(dwc);
 
+   ret = dwc3_ulpi_init(dwc);
   
   If I understood correctly, this call will result in enumerating the phy
   via ULPI bus, then registering the correct ULPI device.
   Can you guarantee ULPI will be always accessible at this point if we
   remove dwc3 module and load it again?
  
  OK, got it. So yes, I can guarantee that ULPI will be acessible at
  this point. If we are in a state where we could soft reset dwc3, we
  know we can access ULPI. The fact that dwc3 itself expects to be able
  to write to the ULPI registers at that point guarantees it for us.
 
 I just double checked DWC3 TRM.
 You are correct, by the time we're executing dwc3_core_soft_reset() ULPI
 bus is already accessible. But the TRM also specifies an ULPI phy would
 be reset by DCTL's core soft reset, which is executed by dwc3_core_init()
 before calling dwc3_core_soft_reset(). It does mention DWC3 writes data
 to an ULPI phy register during reset, but it also mentions the clock
 sync happens during that time.
 
 That makes no even OK, but more correct IMO to power on phy before
 core's soft reset (i.e. by ACPI methods). But it lets us in a grey area
 whether ULPI is reliably accessible before core's soft reset.
 
 I believe if you move the dwc3_ulpi_init() to dwc3_core_init(), after
 core's soft reset we've got no more grey area.

Well, we have already requested the phys in dwc3_probe before that so
some refactoring will be needed. It's probable no a problem.

Btw, I'm sorry about telling this so late, but I'm going to be on
vacation for the next two weeks.


Cheers,

-- 
heikki
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-02-13 Thread Felipe Balbi
On Fri, Feb 13, 2015 at 02:03:45PM -0800, David Cohen wrote:
 Hi Heikki,
 
 On Fri, Feb 13, 2015 at 03:16:40PM +0200, Heikki Krogerus wrote:
  On Thu, Feb 12, 2015 at 05:41:30PM -0800, David Cohen wrote:
   On Thu, Feb 12, 2015 at 02:12:14PM +0200, Heikki Krogerus wrote:
  diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
  index a8c9062..66cbf38 100644
  --- a/drivers/usb/dwc3/core.c
  +++ b/drivers/usb/dwc3/core.c
  @@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device 
  *pdev)
  platform_set_drvdata(pdev, dwc);
  dwc3_cache_hwparams(dwc);
   
  +   ret = dwc3_ulpi_init(dwc);
 
 If I understood correctly, this call will result in enumerating the 
 phy
 via ULPI bus, then registering the correct ULPI device.
 Can you guarantee ULPI will be always accessible at this point if we
 remove dwc3 module and load it again?

OK, got it. So yes, I can guarantee that ULPI will be acessible at
this point. If we are in a state where we could soft reset dwc3, we
know we can access ULPI. The fact that dwc3 itself expects to be able
to write to the ULPI registers at that point guarantees it for us.
   
   I just double checked DWC3 TRM.
   You are correct, by the time we're executing dwc3_core_soft_reset() ULPI
   bus is already accessible. But the TRM also specifies an ULPI phy would
   be reset by DCTL's core soft reset, which is executed by dwc3_core_init()
   before calling dwc3_core_soft_reset(). It does mention DWC3 writes data
   to an ULPI phy register during reset, but it also mentions the clock
   sync happens during that time.
   
   That makes no even OK, but more correct IMO to power on phy before
   core's soft reset (i.e. by ACPI methods). But it lets us in a grey area
   whether ULPI is reliably accessible before core's soft reset.
   
   I believe if you move the dwc3_ulpi_init() to dwc3_core_init(), after
   core's soft reset we've got no more grey area.
  
  Well, we have already requested the phys in dwc3_probe before that so
  some refactoring will be needed. It's probable no a problem.
 
 Sounds good :)
 
  
  Btw, I'm sorry about telling this so late, but I'm going to be on
  vacation for the next two weeks.
 
 Thanks for the notice.

you'll be back right around merge window closing time :-) Not to worry,
we'll have time to get this into the next merge window.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-02-13 Thread David Cohen
Hi Heikki,

On Fri, Feb 13, 2015 at 03:16:40PM +0200, Heikki Krogerus wrote:
 On Thu, Feb 12, 2015 at 05:41:30PM -0800, David Cohen wrote:
  On Thu, Feb 12, 2015 at 02:12:14PM +0200, Heikki Krogerus wrote:
 diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
 index a8c9062..66cbf38 100644
 --- a/drivers/usb/dwc3/core.c
 +++ b/drivers/usb/dwc3/core.c
 @@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device 
 *pdev)
   platform_set_drvdata(pdev, dwc);
   dwc3_cache_hwparams(dwc);
  
 + ret = dwc3_ulpi_init(dwc);

If I understood correctly, this call will result in enumerating the phy
via ULPI bus, then registering the correct ULPI device.
Can you guarantee ULPI will be always accessible at this point if we
remove dwc3 module and load it again?
   
   OK, got it. So yes, I can guarantee that ULPI will be acessible at
   this point. If we are in a state where we could soft reset dwc3, we
   know we can access ULPI. The fact that dwc3 itself expects to be able
   to write to the ULPI registers at that point guarantees it for us.
  
  I just double checked DWC3 TRM.
  You are correct, by the time we're executing dwc3_core_soft_reset() ULPI
  bus is already accessible. But the TRM also specifies an ULPI phy would
  be reset by DCTL's core soft reset, which is executed by dwc3_core_init()
  before calling dwc3_core_soft_reset(). It does mention DWC3 writes data
  to an ULPI phy register during reset, but it also mentions the clock
  sync happens during that time.
  
  That makes no even OK, but more correct IMO to power on phy before
  core's soft reset (i.e. by ACPI methods). But it lets us in a grey area
  whether ULPI is reliably accessible before core's soft reset.
  
  I believe if you move the dwc3_ulpi_init() to dwc3_core_init(), after
  core's soft reset we've got no more grey area.
 
 Well, we have already requested the phys in dwc3_probe before that so
 some refactoring will be needed. It's probable no a problem.

Sounds good :)

 
 Btw, I'm sorry about telling this so late, but I'm going to be on
 vacation for the next two weeks.

Thanks for the notice.

Br, David

 
 
 Cheers,
 
 -- 
 heikki
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-02-12 Thread David Cohen
On Thu, Feb 12, 2015 at 05:41:30PM -0800, David Cohen wrote:
> On Thu, Feb 12, 2015 at 02:12:14PM +0200, Heikki Krogerus wrote:
> > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > index a8c9062..66cbf38 100644
> > > > --- a/drivers/usb/dwc3/core.c
> > > > +++ b/drivers/usb/dwc3/core.c
> > > > @@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device *pdev)
> > > > platform_set_drvdata(pdev, dwc);
> > > > dwc3_cache_hwparams(dwc);
> > > >  
> > > > +   ret = dwc3_ulpi_init(dwc);
> > > 
> > > If I understood correctly, this call will result in enumerating the phy
> > > via ULPI bus, then registering the correct ULPI device.
> > > Can you guarantee ULPI will be always accessible at this point if we
> > > remove dwc3 module and load it again?
> > 
> > OK, got it. So yes, I can guarantee that ULPI will be acessible at
> > this point. If we are in a state where we could soft reset dwc3, we
> > know we can access ULPI. The fact that dwc3 itself expects to be able
> > to write to the ULPI registers at that point guarantees it for us.
> 
> I just double checked DWC3 TRM.
> You are correct, by the time we're executing dwc3_core_soft_reset() ULPI
> bus is already accessible. But the TRM also specifies an ULPI phy would
> be reset by DCTL's core soft reset, which is executed by dwc3_core_init()
> before calling dwc3_core_soft_reset(). It does mention DWC3 writes data
> to an ULPI phy register during reset, but it also mentions the clock
> sync happens during that time.
> 
> That makes no even OK, but more correct IMO to power on phy before

Sorry for the typo. I meant:
That makes not only OK, but more correct...

> core's soft reset (i.e. by ACPI methods). But it lets us in a grey area
> whether ULPI is reliably accessible before core's soft reset.
> 
> I believe if you move the dwc3_ulpi_init() to dwc3_core_init(), after
> core's soft reset we've got no more grey area.
> 
> Br, David
> 
> > 
> > So in practice when ever dwc3 is powered we will be able to access
> > ULPI for as long as the USB2 PHY interface is not suspended separately
> > with GUSB2PHYCFG SusPHY bit. And even then we would only need to
> > resume it with the same bit and ULPI is accessible again.
> > 
> > 
> > Cheers,
> > 
> > -- 
> > heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-02-12 Thread David Cohen
On Thu, Feb 12, 2015 at 02:12:14PM +0200, Heikki Krogerus wrote:
> > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > index a8c9062..66cbf38 100644
> > > --- a/drivers/usb/dwc3/core.c
> > > +++ b/drivers/usb/dwc3/core.c
> > > @@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device *pdev)
> > >   platform_set_drvdata(pdev, dwc);
> > >   dwc3_cache_hwparams(dwc);
> > >  
> > > + ret = dwc3_ulpi_init(dwc);
> > 
> > If I understood correctly, this call will result in enumerating the phy
> > via ULPI bus, then registering the correct ULPI device.
> > Can you guarantee ULPI will be always accessible at this point if we
> > remove dwc3 module and load it again?
> 
> OK, got it. So yes, I can guarantee that ULPI will be acessible at
> this point. If we are in a state where we could soft reset dwc3, we
> know we can access ULPI. The fact that dwc3 itself expects to be able
> to write to the ULPI registers at that point guarantees it for us.

I just double checked DWC3 TRM.
You are correct, by the time we're executing dwc3_core_soft_reset() ULPI
bus is already accessible. But the TRM also specifies an ULPI phy would
be reset by DCTL's core soft reset, which is executed by dwc3_core_init()
before calling dwc3_core_soft_reset(). It does mention DWC3 writes data
to an ULPI phy register during reset, but it also mentions the clock
sync happens during that time.

That makes no even OK, but more correct IMO to power on phy before
core's soft reset (i.e. by ACPI methods). But it lets us in a grey area
whether ULPI is reliably accessible before core's soft reset.

I believe if you move the dwc3_ulpi_init() to dwc3_core_init(), after
core's soft reset we've got no more grey area.

Br, David

> 
> So in practice when ever dwc3 is powered we will be able to access
> ULPI for as long as the USB2 PHY interface is not suspended separately
> with GUSB2PHYCFG SusPHY bit. And even then we would only need to
> resume it with the same bit and ULPI is accessible again.
> 
> 
> Cheers,
> 
> -- 
> heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-02-12 Thread Heikki Krogerus
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index a8c9062..66cbf38 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device *pdev)
> > platform_set_drvdata(pdev, dwc);
> > dwc3_cache_hwparams(dwc);
> >  
> > +   ret = dwc3_ulpi_init(dwc);
> 
> If I understood correctly, this call will result in enumerating the phy
> via ULPI bus, then registering the correct ULPI device.
> Can you guarantee ULPI will be always accessible at this point if we
> remove dwc3 module and load it again?

OK, got it. So yes, I can guarantee that ULPI will be acessible at
this point. If we are in a state where we could soft reset dwc3, we
know we can access ULPI. The fact that dwc3 itself expects to be able
to write to the ULPI registers at that point guarantees it for us.

So in practice when ever dwc3 is powered we will be able to access
ULPI for as long as the USB2 PHY interface is not suspended separately
with GUSB2PHYCFG SusPHY bit. And even then we would only need to
resume it with the same bit and ULPI is accessible again.


Cheers,

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-02-12 Thread Heikki Krogerus
  diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
  index a8c9062..66cbf38 100644
  --- a/drivers/usb/dwc3/core.c
  +++ b/drivers/usb/dwc3/core.c
  @@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device *pdev)
  platform_set_drvdata(pdev, dwc);
  dwc3_cache_hwparams(dwc);
   
  +   ret = dwc3_ulpi_init(dwc);
 
 If I understood correctly, this call will result in enumerating the phy
 via ULPI bus, then registering the correct ULPI device.
 Can you guarantee ULPI will be always accessible at this point if we
 remove dwc3 module and load it again?

OK, got it. So yes, I can guarantee that ULPI will be acessible at
this point. If we are in a state where we could soft reset dwc3, we
know we can access ULPI. The fact that dwc3 itself expects to be able
to write to the ULPI registers at that point guarantees it for us.

So in practice when ever dwc3 is powered we will be able to access
ULPI for as long as the USB2 PHY interface is not suspended separately
with GUSB2PHYCFG SusPHY bit. And even then we would only need to
resume it with the same bit and ULPI is accessible again.


Cheers,

-- 
heikki
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-02-12 Thread David Cohen
On Thu, Feb 12, 2015 at 02:12:14PM +0200, Heikki Krogerus wrote:
   diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
   index a8c9062..66cbf38 100644
   --- a/drivers/usb/dwc3/core.c
   +++ b/drivers/usb/dwc3/core.c
   @@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device *pdev)
 platform_set_drvdata(pdev, dwc);
 dwc3_cache_hwparams(dwc);

   + ret = dwc3_ulpi_init(dwc);
  
  If I understood correctly, this call will result in enumerating the phy
  via ULPI bus, then registering the correct ULPI device.
  Can you guarantee ULPI will be always accessible at this point if we
  remove dwc3 module and load it again?
 
 OK, got it. So yes, I can guarantee that ULPI will be acessible at
 this point. If we are in a state where we could soft reset dwc3, we
 know we can access ULPI. The fact that dwc3 itself expects to be able
 to write to the ULPI registers at that point guarantees it for us.

I just double checked DWC3 TRM.
You are correct, by the time we're executing dwc3_core_soft_reset() ULPI
bus is already accessible. But the TRM also specifies an ULPI phy would
be reset by DCTL's core soft reset, which is executed by dwc3_core_init()
before calling dwc3_core_soft_reset(). It does mention DWC3 writes data
to an ULPI phy register during reset, but it also mentions the clock
sync happens during that time.

That makes no even OK, but more correct IMO to power on phy before
core's soft reset (i.e. by ACPI methods). But it lets us in a grey area
whether ULPI is reliably accessible before core's soft reset.

I believe if you move the dwc3_ulpi_init() to dwc3_core_init(), after
core's soft reset we've got no more grey area.

Br, David

 
 So in practice when ever dwc3 is powered we will be able to access
 ULPI for as long as the USB2 PHY interface is not suspended separately
 with GUSB2PHYCFG SusPHY bit. And even then we would only need to
 resume it with the same bit and ULPI is accessible again.
 
 
 Cheers,
 
 -- 
 heikki
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-02-12 Thread David Cohen
On Thu, Feb 12, 2015 at 05:41:30PM -0800, David Cohen wrote:
 On Thu, Feb 12, 2015 at 02:12:14PM +0200, Heikki Krogerus wrote:
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index a8c9062..66cbf38 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, dwc);
dwc3_cache_hwparams(dwc);
 
+   ret = dwc3_ulpi_init(dwc);
   
   If I understood correctly, this call will result in enumerating the phy
   via ULPI bus, then registering the correct ULPI device.
   Can you guarantee ULPI will be always accessible at this point if we
   remove dwc3 module and load it again?
  
  OK, got it. So yes, I can guarantee that ULPI will be acessible at
  this point. If we are in a state where we could soft reset dwc3, we
  know we can access ULPI. The fact that dwc3 itself expects to be able
  to write to the ULPI registers at that point guarantees it for us.
 
 I just double checked DWC3 TRM.
 You are correct, by the time we're executing dwc3_core_soft_reset() ULPI
 bus is already accessible. But the TRM also specifies an ULPI phy would
 be reset by DCTL's core soft reset, which is executed by dwc3_core_init()
 before calling dwc3_core_soft_reset(). It does mention DWC3 writes data
 to an ULPI phy register during reset, but it also mentions the clock
 sync happens during that time.
 
 That makes no even OK, but more correct IMO to power on phy before

Sorry for the typo. I meant:
That makes not only OK, but more correct...

 core's soft reset (i.e. by ACPI methods). But it lets us in a grey area
 whether ULPI is reliably accessible before core's soft reset.
 
 I believe if you move the dwc3_ulpi_init() to dwc3_core_init(), after
 core's soft reset we've got no more grey area.
 
 Br, David
 
  
  So in practice when ever dwc3 is powered we will be able to access
  ULPI for as long as the USB2 PHY interface is not suspended separately
  with GUSB2PHYCFG SusPHY bit. And even then we would only need to
  resume it with the same bit and ULPI is accessible again.
  
  
  Cheers,
  
  -- 
  heikki
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-02-11 Thread David Cohen
Hi Heikki,

On Fri, Jan 23, 2015 at 05:12:56PM +0200, Heikki Krogerus wrote:
> Registers DWC3's ULPI interface with the ULPI bus when it's
> available.
> 
> Signed-off-by: Heikki Krogerus 
> ---
>  drivers/usb/dwc3/Kconfig  |   7 +++
>  drivers/usb/dwc3/Makefile |   4 ++
>  drivers/usb/dwc3/core.c   |   6 +++
>  drivers/usb/dwc3/core.h   |  13 ++
>  drivers/usb/dwc3/ulpi.c   | 112 
> ++
>  5 files changed, 142 insertions(+)
>  create mode 100644 drivers/usb/dwc3/ulpi.c
> 
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 58b5b2c..cda82ad 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -11,6 +11,13 @@ config USB_DWC3
>  
>  if USB_DWC3
>  
> +config USB_DWC3_ULPI
> + bool "Register ULPI PHY Interface"
> + depends on USB_ULPI_BUS=y
> + help
> +   Select this if you have ULPI type PHY attached to your DWC3
> +   controller.
> +
>  choice
>   bool "DWC3 Mode Selection"
>   default USB_DWC3_DUAL_ROLE if (USB && USB_GADGET)
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index bb34fbc..2fc44e0 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -16,6 +16,10 @@ ifneq ($(filter y,$(CONFIG_USB_DWC3_GADGET) 
> $(CONFIG_USB_DWC3_DUAL_ROLE)),)
>   dwc3-y  += gadget.o ep0.o
>  endif
>  
> +ifneq ($(CONFIG_USB_DWC3_ULPI),)
> + dwc3-y  += ulpi.o
> +endif
> +
>  ifneq ($(CONFIG_DEBUG_FS),)
>   dwc3-y  += debugfs.o
>  endif
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index a8c9062..66cbf38 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device *pdev)
>   platform_set_drvdata(pdev, dwc);
>   dwc3_cache_hwparams(dwc);
>  
> + ret = dwc3_ulpi_init(dwc);

If I understood correctly, this call will result in enumerating the phy
via ULPI bus, then registering the correct ULPI device.
Can you guarantee ULPI will be always accessible at this point if we
remove dwc3 module and load it again?

Br, David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-02-11 Thread David Cohen
Hi Heikki,

On Fri, Jan 23, 2015 at 05:12:56PM +0200, Heikki Krogerus wrote:
 Registers DWC3's ULPI interface with the ULPI bus when it's
 available.
 
 Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
 ---
  drivers/usb/dwc3/Kconfig  |   7 +++
  drivers/usb/dwc3/Makefile |   4 ++
  drivers/usb/dwc3/core.c   |   6 +++
  drivers/usb/dwc3/core.h   |  13 ++
  drivers/usb/dwc3/ulpi.c   | 112 
 ++
  5 files changed, 142 insertions(+)
  create mode 100644 drivers/usb/dwc3/ulpi.c
 
 diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
 index 58b5b2c..cda82ad 100644
 --- a/drivers/usb/dwc3/Kconfig
 +++ b/drivers/usb/dwc3/Kconfig
 @@ -11,6 +11,13 @@ config USB_DWC3
  
  if USB_DWC3
  
 +config USB_DWC3_ULPI
 + bool Register ULPI PHY Interface
 + depends on USB_ULPI_BUS=y
 + help
 +   Select this if you have ULPI type PHY attached to your DWC3
 +   controller.
 +
  choice
   bool DWC3 Mode Selection
   default USB_DWC3_DUAL_ROLE if (USB  USB_GADGET)
 diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
 index bb34fbc..2fc44e0 100644
 --- a/drivers/usb/dwc3/Makefile
 +++ b/drivers/usb/dwc3/Makefile
 @@ -16,6 +16,10 @@ ifneq ($(filter y,$(CONFIG_USB_DWC3_GADGET) 
 $(CONFIG_USB_DWC3_DUAL_ROLE)),)
   dwc3-y  += gadget.o ep0.o
  endif
  
 +ifneq ($(CONFIG_USB_DWC3_ULPI),)
 + dwc3-y  += ulpi.o
 +endif
 +
  ifneq ($(CONFIG_DEBUG_FS),)
   dwc3-y  += debugfs.o
  endif
 diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
 index a8c9062..66cbf38 100644
 --- a/drivers/usb/dwc3/core.c
 +++ b/drivers/usb/dwc3/core.c
 @@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device *pdev)
   platform_set_drvdata(pdev, dwc);
   dwc3_cache_hwparams(dwc);
  
 + ret = dwc3_ulpi_init(dwc);

If I understood correctly, this call will result in enumerating the phy
via ULPI bus, then registering the correct ULPI device.
Can you guarantee ULPI will be always accessible at this point if we
remove dwc3 module and load it again?

Br, David
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-01-27 Thread Felipe Balbi
On Tue, Jan 27, 2015 at 01:09:25PM +0200, Heikki Krogerus wrote:
> > look at your patch again. In case DWC3_ULPI isn't enabled, this file
> > won't be linked at all. You're using stubs, so taht's fine.
> > 
> > In case it _is_ enabled, you're breaking out early if you can't register
> > ulpi interface, meaning you're exitting probe() (which, in fact, seems
> > wrong as I want to be able to run dwc3 with ULPI enabled on a platform
> > that was configured with ULPI+UTMI, but uses UTMI PHY).
> > 
> > I still think this patch won't work in all cases.
> 
> OK. So in case of ULPI+UTMI we'll try the ulpi interface, and if it
> fails, fall back to UTMI. Or can we use some other method to determine
> to which interface the PHY is really attached to in that case?

I think we would need a phy_interface_sel pdata/DT binding for those
cases. It should be optional and probably only needed for a few odd
devices.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-01-27 Thread Heikki Krogerus
> look at your patch again. In case DWC3_ULPI isn't enabled, this file
> won't be linked at all. You're using stubs, so taht's fine.
> 
> In case it _is_ enabled, you're breaking out early if you can't register
> ulpi interface, meaning you're exitting probe() (which, in fact, seems
> wrong as I want to be able to run dwc3 with ULPI enabled on a platform
> that was configured with ULPI+UTMI, but uses UTMI PHY).
> 
> I still think this patch won't work in all cases.

OK. So in case of ULPI+UTMI we'll try the ulpi interface, and if it
fails, fall back to UTMI. Or can we use some other method to determine
to which interface the PHY is really attached to in that case?


Thanks,

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-01-27 Thread Heikki Krogerus
 look at your patch again. In case DWC3_ULPI isn't enabled, this file
 won't be linked at all. You're using stubs, so taht's fine.
 
 In case it _is_ enabled, you're breaking out early if you can't register
 ulpi interface, meaning you're exitting probe() (which, in fact, seems
 wrong as I want to be able to run dwc3 with ULPI enabled on a platform
 that was configured with ULPI+UTMI, but uses UTMI PHY).
 
 I still think this patch won't work in all cases.

OK. So in case of ULPI+UTMI we'll try the ulpi interface, and if it
fails, fall back to UTMI. Or can we use some other method to determine
to which interface the PHY is really attached to in that case?


Thanks,

-- 
heikki
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-01-27 Thread Felipe Balbi
On Tue, Jan 27, 2015 at 01:09:25PM +0200, Heikki Krogerus wrote:
  look at your patch again. In case DWC3_ULPI isn't enabled, this file
  won't be linked at all. You're using stubs, so taht's fine.
  
  In case it _is_ enabled, you're breaking out early if you can't register
  ulpi interface, meaning you're exitting probe() (which, in fact, seems
  wrong as I want to be able to run dwc3 with ULPI enabled on a platform
  that was configured with ULPI+UTMI, but uses UTMI PHY).
  
  I still think this patch won't work in all cases.
 
 OK. So in case of ULPI+UTMI we'll try the ulpi interface, and if it
 fails, fall back to UTMI. Or can we use some other method to determine
 to which interface the PHY is really attached to in that case?

I think we would need a phy_interface_sel pdata/DT binding for those
cases. It should be optional and probably only needed for a few odd
devices.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-01-26 Thread Felipe Balbi
On Mon, Jan 26, 2015 at 01:46:10PM +0200, Heikki Krogerus wrote:
> On Fri, Jan 23, 2015 at 10:24:43AM -0600, Felipe Balbi wrote:
> > On Fri, Jan 23, 2015 at 05:12:56PM +0200, Heikki Krogerus wrote:
> > > +int dwc3_ulpi_init(struct dwc3 *dwc)
> > > +{
> > > + u32 reg;
> > > +
> > > + /* First check USB2 PHY interface type */
> > > + switch (DWC3_GHWPARAMS3_HSPHY_IFC(dwc->hwparams.hwparams3)) {
> > > + case DWC3_GHWPARAMS3_HSPHY_IFC_UTMI_ULPI:
> > > + /* Select ULPI Interface */
> > > + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> > > + reg |= DWC3_GUSB2PHYCFG_ULPI_UTMI;
> > > + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> > > + /* FALLTHROUGH */
> > > + case DWC3_GHWPARAMS3_HSPHY_IFC_ULPI:
> > > + break;
> > > + default:
> > > + return 0;
> > > + }
> > > +
> > > + /* Register the interface */
> > > + dwc->ulpi = ulpi_register_interface(dwc->dev, _ulpi);
> > > + if (IS_ERR(dwc->ulpi)) {
> > 
> > so, this will only build and link if DWC3_ULPI is enabled, in case of
> > error you return early...
> > 
> > > + dev_err(dwc->dev, "failed to register ULPI interface");
> > > + return PTR_ERR(dwc->ulpi);
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +void dwc3_ulpi_exit(struct dwc3 *dwc)
> > > +{
> > > + if (dwc->ulpi) {
> > 
> > ... looks like this branch is unnecessary.
> 
> We can't do that, or distros that select DWC3_ULPI option can only use
> dwc3 with hardware that really has ULPI PHY. So I guess we'll drop the
> DWC3_ULPI option and build the dwc3 ulpi support always if ULPI bus is
> enabled. OK?

look at your patch again. In case DWC3_ULPI isn't enabled, this file
won't be linked at all. You're using stubs, so taht's fine.

In case it _is_ enabled, you're breaking out early if you can't register
ulpi interface, meaning you're exitting probe() (which, in fact, seems
wrong as I want to be able to run dwc3 with ULPI enabled on a platform
that was configured with ULPI+UTMI, but uses UTMI PHY).

I still think this patch won't work in all cases.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-01-26 Thread Heikki Krogerus
On Fri, Jan 23, 2015 at 10:24:43AM -0600, Felipe Balbi wrote:
> On Fri, Jan 23, 2015 at 05:12:56PM +0200, Heikki Krogerus wrote:
> > +int dwc3_ulpi_init(struct dwc3 *dwc)
> > +{
> > +   u32 reg;
> > +
> > +   /* First check USB2 PHY interface type */
> > +   switch (DWC3_GHWPARAMS3_HSPHY_IFC(dwc->hwparams.hwparams3)) {
> > +   case DWC3_GHWPARAMS3_HSPHY_IFC_UTMI_ULPI:
> > +   /* Select ULPI Interface */
> > +   reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> > +   reg |= DWC3_GUSB2PHYCFG_ULPI_UTMI;
> > +   dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> > +   /* FALLTHROUGH */
> > +   case DWC3_GHWPARAMS3_HSPHY_IFC_ULPI:
> > +   break;
> > +   default:
> > +   return 0;
> > +   }
> > +
> > +   /* Register the interface */
> > +   dwc->ulpi = ulpi_register_interface(dwc->dev, _ulpi);
> > +   if (IS_ERR(dwc->ulpi)) {
> 
> so, this will only build and link if DWC3_ULPI is enabled, in case of
> error you return early...
> 
> > +   dev_err(dwc->dev, "failed to register ULPI interface");
> > +   return PTR_ERR(dwc->ulpi);
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> > +void dwc3_ulpi_exit(struct dwc3 *dwc)
> > +{
> > +   if (dwc->ulpi) {
> 
> ... looks like this branch is unnecessary.

We can't do that, or distros that select DWC3_ULPI option can only use
dwc3 with hardware that really has ULPI PHY. So I guess we'll drop the
DWC3_ULPI option and build the dwc3 ulpi support always if ULPI bus is
enabled. OK?


Thanks,

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-01-26 Thread Felipe Balbi
On Mon, Jan 26, 2015 at 01:46:10PM +0200, Heikki Krogerus wrote:
 On Fri, Jan 23, 2015 at 10:24:43AM -0600, Felipe Balbi wrote:
  On Fri, Jan 23, 2015 at 05:12:56PM +0200, Heikki Krogerus wrote:
   +int dwc3_ulpi_init(struct dwc3 *dwc)
   +{
   + u32 reg;
   +
   + /* First check USB2 PHY interface type */
   + switch (DWC3_GHWPARAMS3_HSPHY_IFC(dwc-hwparams.hwparams3)) {
   + case DWC3_GHWPARAMS3_HSPHY_IFC_UTMI_ULPI:
   + /* Select ULPI Interface */
   + reg = dwc3_readl(dwc-regs, DWC3_GUSB2PHYCFG(0));
   + reg |= DWC3_GUSB2PHYCFG_ULPI_UTMI;
   + dwc3_writel(dwc-regs, DWC3_GUSB2PHYCFG(0), reg);
   + /* FALLTHROUGH */
   + case DWC3_GHWPARAMS3_HSPHY_IFC_ULPI:
   + break;
   + default:
   + return 0;
   + }
   +
   + /* Register the interface */
   + dwc-ulpi = ulpi_register_interface(dwc-dev, dwc3_ulpi);
   + if (IS_ERR(dwc-ulpi)) {
  
  so, this will only build and link if DWC3_ULPI is enabled, in case of
  error you return early...
  
   + dev_err(dwc-dev, failed to register ULPI interface);
   + return PTR_ERR(dwc-ulpi);
   + }
   +
   + return 0;
   +}
   +
   +void dwc3_ulpi_exit(struct dwc3 *dwc)
   +{
   + if (dwc-ulpi) {
  
  ... looks like this branch is unnecessary.
 
 We can't do that, or distros that select DWC3_ULPI option can only use
 dwc3 with hardware that really has ULPI PHY. So I guess we'll drop the
 DWC3_ULPI option and build the dwc3 ulpi support always if ULPI bus is
 enabled. OK?

look at your patch again. In case DWC3_ULPI isn't enabled, this file
won't be linked at all. You're using stubs, so taht's fine.

In case it _is_ enabled, you're breaking out early if you can't register
ulpi interface, meaning you're exitting probe() (which, in fact, seems
wrong as I want to be able to run dwc3 with ULPI enabled on a platform
that was configured with ULPI+UTMI, but uses UTMI PHY).

I still think this patch won't work in all cases.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-01-26 Thread Heikki Krogerus
On Fri, Jan 23, 2015 at 10:24:43AM -0600, Felipe Balbi wrote:
 On Fri, Jan 23, 2015 at 05:12:56PM +0200, Heikki Krogerus wrote:
  +int dwc3_ulpi_init(struct dwc3 *dwc)
  +{
  +   u32 reg;
  +
  +   /* First check USB2 PHY interface type */
  +   switch (DWC3_GHWPARAMS3_HSPHY_IFC(dwc-hwparams.hwparams3)) {
  +   case DWC3_GHWPARAMS3_HSPHY_IFC_UTMI_ULPI:
  +   /* Select ULPI Interface */
  +   reg = dwc3_readl(dwc-regs, DWC3_GUSB2PHYCFG(0));
  +   reg |= DWC3_GUSB2PHYCFG_ULPI_UTMI;
  +   dwc3_writel(dwc-regs, DWC3_GUSB2PHYCFG(0), reg);
  +   /* FALLTHROUGH */
  +   case DWC3_GHWPARAMS3_HSPHY_IFC_ULPI:
  +   break;
  +   default:
  +   return 0;
  +   }
  +
  +   /* Register the interface */
  +   dwc-ulpi = ulpi_register_interface(dwc-dev, dwc3_ulpi);
  +   if (IS_ERR(dwc-ulpi)) {
 
 so, this will only build and link if DWC3_ULPI is enabled, in case of
 error you return early...
 
  +   dev_err(dwc-dev, failed to register ULPI interface);
  +   return PTR_ERR(dwc-ulpi);
  +   }
  +
  +   return 0;
  +}
  +
  +void dwc3_ulpi_exit(struct dwc3 *dwc)
  +{
  +   if (dwc-ulpi) {
 
 ... looks like this branch is unnecessary.

We can't do that, or distros that select DWC3_ULPI option can only use
dwc3 with hardware that really has ULPI PHY. So I guess we'll drop the
DWC3_ULPI option and build the dwc3 ulpi support always if ULPI bus is
enabled. OK?


Thanks,

-- 
heikki
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-01-23 Thread Felipe Balbi
Hi,

On Fri, Jan 23, 2015 at 05:12:56PM +0200, Heikki Krogerus wrote:
> +int dwc3_ulpi_init(struct dwc3 *dwc)
> +{
> + u32 reg;
> +
> + /* First check USB2 PHY interface type */
> + switch (DWC3_GHWPARAMS3_HSPHY_IFC(dwc->hwparams.hwparams3)) {
> + case DWC3_GHWPARAMS3_HSPHY_IFC_UTMI_ULPI:
> + /* Select ULPI Interface */
> + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> + reg |= DWC3_GUSB2PHYCFG_ULPI_UTMI;
> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> + /* FALLTHROUGH */
> + case DWC3_GHWPARAMS3_HSPHY_IFC_ULPI:
> + break;
> + default:
> + return 0;
> + }
> +
> + /* Register the interface */
> + dwc->ulpi = ulpi_register_interface(dwc->dev, _ulpi);
> + if (IS_ERR(dwc->ulpi)) {

so, this will only build and link if DWC3_ULPI is enabled, in case of
error you return early...

> + dev_err(dwc->dev, "failed to register ULPI interface");
> + return PTR_ERR(dwc->ulpi);
> + }
> +
> + return 0;
> +}
> +
> +void dwc3_ulpi_exit(struct dwc3 *dwc)
> +{
> + if (dwc->ulpi) {

... looks like this branch is unnecessary.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

2015-01-23 Thread Felipe Balbi
Hi,

On Fri, Jan 23, 2015 at 05:12:56PM +0200, Heikki Krogerus wrote:
 +int dwc3_ulpi_init(struct dwc3 *dwc)
 +{
 + u32 reg;
 +
 + /* First check USB2 PHY interface type */
 + switch (DWC3_GHWPARAMS3_HSPHY_IFC(dwc-hwparams.hwparams3)) {
 + case DWC3_GHWPARAMS3_HSPHY_IFC_UTMI_ULPI:
 + /* Select ULPI Interface */
 + reg = dwc3_readl(dwc-regs, DWC3_GUSB2PHYCFG(0));
 + reg |= DWC3_GUSB2PHYCFG_ULPI_UTMI;
 + dwc3_writel(dwc-regs, DWC3_GUSB2PHYCFG(0), reg);
 + /* FALLTHROUGH */
 + case DWC3_GHWPARAMS3_HSPHY_IFC_ULPI:
 + break;
 + default:
 + return 0;
 + }
 +
 + /* Register the interface */
 + dwc-ulpi = ulpi_register_interface(dwc-dev, dwc3_ulpi);
 + if (IS_ERR(dwc-ulpi)) {

so, this will only build and link if DWC3_ULPI is enabled, in case of
error you return early...

 + dev_err(dwc-dev, failed to register ULPI interface);
 + return PTR_ERR(dwc-ulpi);
 + }
 +
 + return 0;
 +}
 +
 +void dwc3_ulpi_exit(struct dwc3 *dwc)
 +{
 + if (dwc-ulpi) {

... looks like this branch is unnecessary.

-- 
balbi


signature.asc
Description: Digital signature