There can be configurations in which DWC3 is hoooked up only to USB2 PHY.
In such cases we should not return -EPROBE_DEFER, rather continue probe
even if there is no USB3 PHY.
Signed-off-by: George Cherian <[email protected]>
---
drivers/usb/dwc3/core.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index c35d49d..d5e6f3e 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -100,7 +100,9 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc)
dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
usb_phy_init(dwc->usb2_phy);
- usb_phy_init(dwc->usb3_phy);
+
+ if (dwc->usb3_phy)
+ usb_phy_init(dwc->usb3_phy);
mdelay(100);
/* Clear USB3 PHY reset */
@@ -360,7 +362,9 @@ err0:
static void dwc3_core_exit(struct dwc3 *dwc)
{
usb_phy_shutdown(dwc->usb2_phy);
- usb_phy_shutdown(dwc->usb3_phy);
+
+ if (dwc->usb3_phy)
+ usb_phy_shutdown(dwc->usb3_phy);
}
#define DWC3_ALIGN_MASK (16 - 1)
@@ -460,12 +464,19 @@ static int dwc3_probe(struct platform_device *pdev)
if (ret == -ENXIO)
return ret;
+ /*
+ * In some cases we wont have a USB3 PHY, in such cases
+ * if we return EPROBE_DEFER we would never complete the
+ * probe. Just print error and continue.
+ */
dev_err(dev, "no usb3 phy configured\n");
- return -EPROBE_DEFER;
+ dwc->usb3_phy = NULL;
}
usb_phy_set_suspend(dwc->usb2_phy, 0);
- usb_phy_set_suspend(dwc->usb3_phy, 0);
+
+ if (dwc->usb3_phy)
+ usb_phy_set_suspend(dwc->usb3_phy, 0);
spin_lock_init(&dwc->lock);
platform_set_drvdata(pdev, dwc);
@@ -604,7 +615,9 @@ static int dwc3_remove(struct platform_device *pdev)
struct dwc3 *dwc = platform_get_drvdata(pdev);
usb_phy_set_suspend(dwc->usb2_phy, 1);
- usb_phy_set_suspend(dwc->usb3_phy, 1);
+
+ if (dwc->usb3_phy)
+ usb_phy_set_suspend(dwc->usb3_phy, 1);
pm_runtime_put(&pdev->dev);
pm_runtime_disable(&pdev->dev);
@@ -700,7 +713,9 @@ static int dwc3_suspend(struct device *dev)
dwc->gctl = dwc3_readl(dwc->regs, DWC3_GCTL);
spin_unlock_irqrestore(&dwc->lock, flags);
- usb_phy_shutdown(dwc->usb3_phy);
+ if (dwc->usb3_phy)
+ usb_phy_shutdown(dwc->usb3_phy);
+
usb_phy_shutdown(dwc->usb2_phy);
return 0;
@@ -711,7 +726,9 @@ static int dwc3_resume(struct device *dev)
struct dwc3 *dwc = dev_get_drvdata(dev);
unsigned long flags;
- usb_phy_init(dwc->usb3_phy);
+ if (dwc->usb3_phy)
+ usb_phy_init(dwc->usb3_phy);
+
usb_phy_init(dwc->usb2_phy);
msleep(100);
--
1.8.1.4
Hi,
On Wed, Jun 26, 2013 at 02:59:14PM +0530, George Cherian wrote:
> There can be configurations in which DWC3 is hoooked up only to USB2 PHY.
> In such cases we should not return -EPROBE_DEFER, rather continue probe
> even if there is no USB3 PHY.
>
> Signed-off-by: George Cherian <[email protected]>
> ---
> drivers/usb/dwc3/core.c | 31 ++++++++++++++++++++++++-------
> 1 file changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index c35d49d..d5e6f3e 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -100,7 +100,9 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc)
> dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>
> usb_phy_init(dwc->usb2_phy);
> - usb_phy_init(dwc->usb3_phy);
> +
> + if (dwc->usb3_phy)
> + usb_phy_init(dwc->usb3_phy);
I would feel more comfortable if you would move our maximum_speed module
parameter to DT with a property such as:
snps,maximum_speed = "highspeed";
then on driver you could:
ret = of_property_read_string(np, "snps,maximum_speed", &maximum_speed);
if (ret < 0)
bailout();
if (strncmp(maximum_speed, "superspeed", 10) == 0) {
/* grab USB3 PHY, return EPROBE_DEFER if not found */
grab_usb3_phy();
}
if ((strncmp(maximum_speed, "highspeed", 9) == 0) ||
(strncmp(maximum_speed, "fullspeed", 9) == 0) ||
(strncmp(maximum_speed, "lowspeed", 8) == 0)) {
/* grab USB2 PHY, return EPROBE_DEFER if not found */
grab_usb2_phy();
}
this way, we depend solely on setting maximum_speed to highspeed for
AM437x :-)
ps: don't forget to default to superspeed in case no property is passed.
--
balbi
On 6/26/2013 3:46 PM, Felipe Balbi wrote:
> Hi,
>
> On Wed, Jun 26, 2013 at 02:59:14PM +0530, George Cherian wrote:
>> There can be configurations in which DWC3 is hoooked up only to USB2 PHY.
>> In such cases we should not return -EPROBE_DEFER, rather continue probe
>> even if there is no USB3 PHY.
>>
>> Signed-off-by: George Cherian <[email protected]>
>> ---
>> drivers/usb/dwc3/core.c | 31 ++++++++++++++++++++++++-------
>> 1 file changed, 24 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index c35d49d..d5e6f3e 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -100,7 +100,9 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc)
>> dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>>
>> usb_phy_init(dwc->usb2_phy);
>> - usb_phy_init(dwc->usb3_phy);
>> +
>> + if (dwc->usb3_phy)
>> + usb_phy_init(dwc->usb3_phy);
> I would feel more comfortable if you would move our maximum_speed module
> parameter to DT with a property such as:
>
> snps,maximum_speed = "highspeed";
>
> then on driver you could:
okay
> ret = of_property_read_string(np, "snps,maximum_speed", &maximum_speed);
> if (ret < 0)
> bailout();
>
> if (strncmp(maximum_speed, "superspeed", 10) == 0) {
> /* grab USB3 PHY, return EPROBE_DEFER if not found */
> grab_usb3_phy();
> }
>
> if ((strncmp(maximum_speed, "highspeed", 9) == 0) ||
> (strncmp(maximum_speed, "fullspeed", 9) == 0) ||
> (strncmp(maximum_speed, "lowspeed", 8) == 0)) {
> /* grab USB2 PHY, return EPROBE_DEFER if not found */
> grab_usb2_phy();
> }
>
> this way, we depend solely on setting maximum_speed to highspeed for
> AM437x :-)
In dra7xx one instance is superspeed and one instance highspeed.
>
> ps: don't forget to default to superspeed in case no property is passed.
okay
Regards
-George
On Wed, Jun 26, 2013 at 05:37:19PM +0530, George Cherian wrote:
> On 6/26/2013 3:46 PM, Felipe Balbi wrote:
> >Hi,
> >
> >On Wed, Jun 26, 2013 at 02:59:14PM +0530, George Cherian wrote:
> >>There can be configurations in which DWC3 is hoooked up only to USB2 PHY.
> >>In such cases we should not return -EPROBE_DEFER, rather continue probe
> >>even if there is no USB3 PHY.
> >>
> >>Signed-off-by: George Cherian <[email protected]>
> >>---
> >> drivers/usb/dwc3/core.c | 31 ++++++++++++++++++++++++-------
> >> 1 file changed, 24 insertions(+), 7 deletions(-)
> >>
> >>diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> >>index c35d49d..d5e6f3e 100644
> >>--- a/drivers/usb/dwc3/core.c
> >>+++ b/drivers/usb/dwc3/core.c
> >>@@ -100,7 +100,9 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc)
> >> dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> >> usb_phy_init(dwc->usb2_phy);
> >>- usb_phy_init(dwc->usb3_phy);
> >>+
> >>+ if (dwc->usb3_phy)
> >>+ usb_phy_init(dwc->usb3_phy);
> >I would feel more comfortable if you would move our maximum_speed module
> >parameter to DT with a property such as:
> >
> >snps,maximum_speed = "highspeed";
> >
> >then on driver you could:
> okay
> >ret = of_property_read_string(np, "snps,maximum_speed", &maximum_speed);
> >if (ret < 0)
> > bailout();
> >
> >if (strncmp(maximum_speed, "superspeed", 10) == 0) {
> > /* grab USB3 PHY, return EPROBE_DEFER if not found */
> > grab_usb3_phy();
> >}
> >
> >if ((strncmp(maximum_speed, "highspeed", 9) == 0) ||
> > (strncmp(maximum_speed, "fullspeed", 9) == 0) ||
> > (strncmp(maximum_speed, "lowspeed", 8) == 0)) {
> > /* grab USB2 PHY, return EPROBE_DEFER if not found */
> > grab_usb2_phy();
> >}
> >
> >this way, we depend solely on setting maximum_speed to highspeed for
> >AM437x :-)
> In dra7xx one instance is superspeed and one instance highspeed.
right, but in DT you will define both instances and each instance will
have a seaparate snps,maximum_speed attribute :-)
I'm now considering if we should make maximum_speed a generic attribute,
Kishon ? Alex ? Alan ?
anyone else needs such thing ?
--
balbi
Felipe Balbi <[email protected]> writes:
> On Wed, Jun 26, 2013 at 05:37:19PM +0530, George Cherian wrote:
>> On 6/26/2013 3:46 PM, Felipe Balbi wrote:
>> >Hi,
>> >
>> >On Wed, Jun 26, 2013 at 02:59:14PM +0530, George Cherian wrote:
>> >>There can be configurations in which DWC3 is hoooked up only to USB2 PHY.
>> >>In such cases we should not return -EPROBE_DEFER, rather continue probe
>> >>even if there is no USB3 PHY.
>> >>
>> >>Signed-off-by: George Cherian <[email protected]>
>> >>---
>> >> drivers/usb/dwc3/core.c | 31 ++++++++++++++++++++++++-------
>> >> 1 file changed, 24 insertions(+), 7 deletions(-)
>> >>
>> >>diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> >>index c35d49d..d5e6f3e 100644
>> >>--- a/drivers/usb/dwc3/core.c
>> >>+++ b/drivers/usb/dwc3/core.c
>> >>@@ -100,7 +100,9 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc)
>> >> dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>> >> usb_phy_init(dwc->usb2_phy);
>> >>- usb_phy_init(dwc->usb3_phy);
>> >>+
>> >>+ if (dwc->usb3_phy)
>> >>+ usb_phy_init(dwc->usb3_phy);
>> >I would feel more comfortable if you would move our maximum_speed module
>> >parameter to DT with a property such as:
>> >
>> >snps,maximum_speed = "highspeed";
>> >
>> >then on driver you could:
>> okay
>> >ret = of_property_read_string(np, "snps,maximum_speed", &maximum_speed);
>> >if (ret < 0)
>> > bailout();
>> >
>> >if (strncmp(maximum_speed, "superspeed", 10) == 0) {
>> > /* grab USB3 PHY, return EPROBE_DEFER if not found */
>> > grab_usb3_phy();
>> >}
>> >
>> >if ((strncmp(maximum_speed, "highspeed", 9) == 0) ||
>> > (strncmp(maximum_speed, "fullspeed", 9) == 0) ||
>> > (strncmp(maximum_speed, "lowspeed", 8) == 0)) {
>> > /* grab USB2 PHY, return EPROBE_DEFER if not found */
>> > grab_usb2_phy();
>> >}
>> >
>> >this way, we depend solely on setting maximum_speed to highspeed for
>> >AM437x :-)
>> In dra7xx one instance is superspeed and one instance highspeed.
>
> right, but in DT you will define both instances and each instance will
> have a seaparate snps,maximum_speed attribute :-)
>
> I'm now considering if we should make maximum_speed a generic attribute,
> Kishon ? Alex ? Alan ?
>
> anyone else needs such thing ?
We have a force-full-speed attibute for chipidea on the way. This
maximum_speed looks like a more generic alternative. Michael, what say
you?
Regards,
--
Alex
On Wed, Jun 26, 2013 at 03:46:45PM +0300, Alexander Shishkin wrote:
> Felipe Balbi <[email protected]> writes:
>
> > On Wed, Jun 26, 2013 at 05:37:19PM +0530, George Cherian wrote:
> >> On 6/26/2013 3:46 PM, Felipe Balbi wrote:
> >> >Hi,
> >> >
> >> >On Wed, Jun 26, 2013 at 02:59:14PM +0530, George Cherian wrote:
> >> >>There can be configurations in which DWC3 is hoooked up only to USB2 PHY.
> >> >>In such cases we should not return -EPROBE_DEFER, rather continue probe
> >> >>even if there is no USB3 PHY.
> >> >>
> >> >>Signed-off-by: George Cherian <[email protected]>
> >> >>---
> >> >> drivers/usb/dwc3/core.c | 31 ++++++++++++++++++++++++-------
> >> >> 1 file changed, 24 insertions(+), 7 deletions(-)
> >> >>
> >> >>diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> >> >>index c35d49d..d5e6f3e 100644
> >> >>--- a/drivers/usb/dwc3/core.c
> >> >>+++ b/drivers/usb/dwc3/core.c
> >> >>@@ -100,7 +100,9 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc)
> >> >> dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> >> >> usb_phy_init(dwc->usb2_phy);
> >> >>- usb_phy_init(dwc->usb3_phy);
> >> >>+
> >> >>+ if (dwc->usb3_phy)
> >> >>+ usb_phy_init(dwc->usb3_phy);
> >> >I would feel more comfortable if you would move our maximum_speed module
> >> >parameter to DT with a property such as:
> >> >
> >> >snps,maximum_speed = "highspeed";
> >> >
> >> >then on driver you could:
> >> okay
> >> >ret = of_property_read_string(np, "snps,maximum_speed", &maximum_speed);
> >> >if (ret < 0)
> >> > bailout();
> >> >
> >> >if (strncmp(maximum_speed, "superspeed", 10) == 0) {
> >> > /* grab USB3 PHY, return EPROBE_DEFER if not found */
> >> > grab_usb3_phy();
> >> >}
> >> >
> >> >if ((strncmp(maximum_speed, "highspeed", 9) == 0) ||
> >> > (strncmp(maximum_speed, "fullspeed", 9) == 0) ||
> >> > (strncmp(maximum_speed, "lowspeed", 8) == 0)) {
> >> > /* grab USB2 PHY, return EPROBE_DEFER if not found */
> >> > grab_usb2_phy();
> >> >}
> >> >
> >> >this way, we depend solely on setting maximum_speed to highspeed for
> >> >AM437x :-)
> >> In dra7xx one instance is superspeed and one instance highspeed.
> >
> > right, but in DT you will define both instances and each instance will
> > have a seaparate snps,maximum_speed attribute :-)
> >
> > I'm now considering if we should make maximum_speed a generic attribute,
> > Kishon ? Alex ? Alan ?
> >
> > anyone else needs such thing ?
>
> We have a force-full-speed attibute for chipidea on the way. This
> maximum_speed looks like a more generic alternative. Michael, what say
> you?
The maximum_speed attribute sounds more reasonable. I will change my
patch to it.
Regards,
Michael
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi,
On Thu, Jun 27, 2013 at 08:14:16AM +0200, Michael Grzeschik wrote:
> > > right, but in DT you will define both instances and each instance will
> > > have a seaparate snps,maximum_speed attribute :-)
> > >
> > > I'm now considering if we should make maximum_speed a generic attribute,
> > > Kishon ? Alex ? Alan ?
> > >
> > > anyone else needs such thing ?
> >
> > We have a force-full-speed attibute for chipidea on the way. This
> > maximum_speed looks like a more generic alternative. Michael, what say
> > you?
>
> The maximum_speed attribute sounds more reasonable. I will change my
> patch to it.
thanks, perhaps we can add a generic helper in udc-core or usb-common ?
--
balbi
Hi,
On Thu, Jun 27, 2013 at 09:35:26AM +0300, Felipe Balbi wrote:
> Hi,
>
> On Thu, Jun 27, 2013 at 08:14:16AM +0200, Michael Grzeschik wrote:
> > > > right, but in DT you will define both instances and each instance will
> > > > have a seaparate snps,maximum_speed attribute :-)
> > > >
> > > > I'm now considering if we should make maximum_speed a generic attribute,
> > > > Kishon ? Alex ? Alan ?
> > > >
> > > > anyone else needs such thing ?
> > >
> > > We have a force-full-speed attibute for chipidea on the way. This
> > > maximum_speed looks like a more generic alternative. Michael, what say
> > > you?
> >
> > The maximum_speed attribute sounds more reasonable. I will change my
> > patch to it.
>
> thanks, perhaps we can add a generic helper in udc-core or usb-common ?
As this function is not only udc specific, this should go into usb-common.
But right, we should go with a helper here. I will write one in addition
to my full-speed patch as first user of it.
Thanks,
Michael
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi,
On Thu, Jun 27, 2013 at 09:24:08AM +0200, Michael Grzeschik wrote:
> > > > > right, but in DT you will define both instances and each instance will
> > > > > have a seaparate snps,maximum_speed attribute :-)
> > > > >
> > > > > I'm now considering if we should make maximum_speed a generic attribute,
> > > > > Kishon ? Alex ? Alan ?
> > > > >
> > > > > anyone else needs such thing ?
> > > >
> > > > We have a force-full-speed attibute for chipidea on the way. This
> > > > maximum_speed looks like a more generic alternative. Michael, what say
> > > > you?
> > >
> > > The maximum_speed attribute sounds more reasonable. I will change my
> > > patch to it.
> >
> > thanks, perhaps we can add a generic helper in udc-core or usb-common ?
>
> As this function is not only udc specific, this should go into usb-common.
> But right, we should go with a helper here. I will write one in addition
> to my full-speed patch as first user of it.
alright, thanks :-)
--
balbi
Hi,
On Thu, Jun 27, 2013 at 10:30:33AM +0300, Felipe Balbi wrote:
> Hi,
>
> On Thu, Jun 27, 2013 at 09:24:08AM +0200, Michael Grzeschik wrote:
> > > > > > right, but in DT you will define both instances and each instance will
> > > > > > have a seaparate snps,maximum_speed attribute :-)
> > > > > >
> > > > > > I'm now considering if we should make maximum_speed a generic attribute,
> > > > > > Kishon ? Alex ? Alan ?
> > > > > >
> > > > > > anyone else needs such thing ?
> > > > >
> > > > > We have a force-full-speed attibute for chipidea on the way. This
> > > > > maximum_speed looks like a more generic alternative. Michael, what say
> > > > > you?
> > > >
> > > > The maximum_speed attribute sounds more reasonable. I will change my
> > > > patch to it.
> > >
> > > thanks, perhaps we can add a generic helper in udc-core or usb-common ?
> >
> > As this function is not only udc specific, this should go into usb-common.
> > But right, we should go with a helper here. I will write one in addition
> > to my full-speed patch as first user of it.
>
> alright, thanks :-)
Michael, if you need any help with patching in maximum_speed attribute,
let me know as I have some time to work on that part.
cheers
--
balbi
On Fri, Jun 28, 2013 at 09:00:00PM +0300, Felipe Balbi wrote:
> Hi,
>
> On Thu, Jun 27, 2013 at 10:30:33AM +0300, Felipe Balbi wrote:
> > Hi,
> >
> > On Thu, Jun 27, 2013 at 09:24:08AM +0200, Michael Grzeschik wrote:
> > > > > > > right, but in DT you will define both instances and each instance will
> > > > > > > have a seaparate snps,maximum_speed attribute :-)
> > > > > > >
> > > > > > > I'm now considering if we should make maximum_speed a generic attribute,
> > > > > > > Kishon ? Alex ? Alan ?
> > > > > > >
> > > > > > > anyone else needs such thing ?
> > > > > >
> > > > > > We have a force-full-speed attibute for chipidea on the way. This
> > > > > > maximum_speed looks like a more generic alternative. Michael, what say
> > > > > > you?
> > > > >
> > > > > The maximum_speed attribute sounds more reasonable. I will change my
> > > > > patch to it.
> > > >
> > > > thanks, perhaps we can add a generic helper in udc-core or usb-common ?
> > >
> > > As this function is not only udc specific, this should go into usb-common.
> > > But right, we should go with a helper here. I will write one in addition
> > > to my full-speed patch as first user of it.
> >
> > alright, thanks :-)
>
> Michael, if you need any help with patching in maximum_speed attribute,
> let me know as I have some time to work on that part.
Feel free to begin with anything. I am currently short on time.
Do you need some pointers?
Thanks,
Michael
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi,
On Fri, Jun 28, 2013 at 10:06:01PM +0200, Michael Grzeschik wrote:
> > > > > > > > right, but in DT you will define both instances and each instance will
> > > > > > > > have a seaparate snps,maximum_speed attribute :-)
> > > > > > > >
> > > > > > > > I'm now considering if we should make maximum_speed a generic attribute,
> > > > > > > > Kishon ? Alex ? Alan ?
> > > > > > > >
> > > > > > > > anyone else needs such thing ?
> > > > > > >
> > > > > > > We have a force-full-speed attibute for chipidea on the way. This
> > > > > > > maximum_speed looks like a more generic alternative. Michael, what say
> > > > > > > you?
> > > > > >
> > > > > > The maximum_speed attribute sounds more reasonable. I will change my
> > > > > > patch to it.
> > > > >
> > > > > thanks, perhaps we can add a generic helper in udc-core or usb-common ?
> > > >
> > > > As this function is not only udc specific, this should go into usb-common.
> > > > But right, we should go with a helper here. I will write one in addition
> > > > to my full-speed patch as first user of it.
> > >
> > > alright, thanks :-)
> >
> > Michael, if you need any help with patching in maximum_speed attribute,
> > let me know as I have some time to work on that part.
>
> Feel free to begin with anything. I am currently short on time.
> Do you need some pointers?
not, really v1 will come on monday.
--
balbi