| Message ID | 1462492752-27815-1-git-send-email-stefan@agner.ch |
|---|---|
| State | Accepted |
| Commit | 79d867c2e683f7080a8724a54a4a12ac0ce1f837 |
| Delegated to: | Marek Vasut |
| Headers | show |
On 05/06/2016 01:59 AM, Stefan Agner wrote: > There could be runtime determined board specific reason why a EHCI > initialization fails (e.g. ENODEV if a Port is not available). In > this case, properly return the error code. > While at it, that function (board_ehci_hcd_init) has actually two > documentation blocks... Use the correct function name for the > documentation block of board_usb_phy_mode. > > Signed-off-by: Stefan Agner <stefan@agner.ch> > --- > drivers/usb/host/ehci-mx6.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c > index a981b50..bb48d0d 100644 > --- a/drivers/usb/host/ehci-mx6.c > +++ b/drivers/usb/host/ehci-mx6.c > @@ -254,7 +254,7 @@ static void usb_oc_config(int index) > } > > /** > - * board_ehci_hcd_init - override usb phy mode > + * board_usb_phy_mode - override usb phy mode > * @port: usb host/otg port > * > * Target board specific, override usb_phy_mode. > @@ -310,6 +310,7 @@ int ehci_hcd_init(int index, enum usb_init_type init, > #endif > struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR + > (controller_spacing * index)); > + int ret; > > if (index > 3) > return -EINVAL; > @@ -317,7 +318,9 @@ int ehci_hcd_init(int index, enum usb_init_type init, > mdelay(1); > > /* Do board specific initialization */ > - board_ehci_hcd_init(index); > + ret = board_ehci_hcd_init(index); > + if (ret) > + return ret; Shouldn't this disable the OH3 clock , which were enabled a few lines above? Otherwise this is nice, thanks. > usb_power_config(index); > usb_oc_config(index); >
On 2016-05-05 17:39, Marek Vasut wrote: > On 05/06/2016 01:59 AM, Stefan Agner wrote: >> There could be runtime determined board specific reason why a EHCI >> initialization fails (e.g. ENODEV if a Port is not available). In >> this case, properly return the error code. >> While at it, that function (board_ehci_hcd_init) has actually two >> documentation blocks... Use the correct function name for the >> documentation block of board_usb_phy_mode. >> >> Signed-off-by: Stefan Agner <stefan@agner.ch> >> --- >> drivers/usb/host/ehci-mx6.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c >> index a981b50..bb48d0d 100644 >> --- a/drivers/usb/host/ehci-mx6.c >> +++ b/drivers/usb/host/ehci-mx6.c >> @@ -254,7 +254,7 @@ static void usb_oc_config(int index) >> } >> >> /** >> - * board_ehci_hcd_init - override usb phy mode >> + * board_usb_phy_mode - override usb phy mode >> * @port: usb host/otg port >> * >> * Target board specific, override usb_phy_mode. >> @@ -310,6 +310,7 @@ int ehci_hcd_init(int index, enum usb_init_type init, >> #endif >> struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR + >> (controller_spacing * index)); >> + int ret; >> >> if (index > 3) >> return -EINVAL; >> @@ -317,7 +318,9 @@ int ehci_hcd_init(int index, enum usb_init_type init, >> mdelay(1); >> >> /* Do board specific initialization */ >> - board_ehci_hcd_init(index); >> + ret = board_ehci_hcd_init(index); >> + if (ret) >> + return ret; > > Shouldn't this disable the OH3 clock , which were enabled a few lines above? Hm, not sure, the clock does not take a controller index... I guess if the second instance is returning an error, that would disable the clock also for the first controller... -- Stefan > > Otherwise this is nice, thanks. > >> usb_power_config(index); >> usb_oc_config(index); >>
On 05/06/2016 06:12 PM, Stefan Agner wrote: > On 2016-05-05 17:39, Marek Vasut wrote: >> On 05/06/2016 01:59 AM, Stefan Agner wrote: >>> There could be runtime determined board specific reason why a EHCI >>> initialization fails (e.g. ENODEV if a Port is not available). In >>> this case, properly return the error code. >>> While at it, that function (board_ehci_hcd_init) has actually two >>> documentation blocks... Use the correct function name for the >>> documentation block of board_usb_phy_mode. >>> >>> Signed-off-by: Stefan Agner <stefan@agner.ch> >>> --- >>> drivers/usb/host/ehci-mx6.c | 7 +++++-- >>> 1 file changed, 5 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c >>> index a981b50..bb48d0d 100644 >>> --- a/drivers/usb/host/ehci-mx6.c >>> +++ b/drivers/usb/host/ehci-mx6.c >>> @@ -254,7 +254,7 @@ static void usb_oc_config(int index) >>> } >>> >>> /** >>> - * board_ehci_hcd_init - override usb phy mode >>> + * board_usb_phy_mode - override usb phy mode >>> * @port: usb host/otg port >>> * >>> * Target board specific, override usb_phy_mode. >>> @@ -310,6 +310,7 @@ int ehci_hcd_init(int index, enum usb_init_type init, >>> #endif >>> struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR + >>> (controller_spacing * index)); >>> + int ret; >>> >>> if (index > 3) >>> return -EINVAL; >>> @@ -317,7 +318,9 @@ int ehci_hcd_init(int index, enum usb_init_type init, >>> mdelay(1); >>> >>> /* Do board specific initialization */ >>> - board_ehci_hcd_init(index); >>> + ret = board_ehci_hcd_init(index); >>> + if (ret) >>> + return ret; >> >> Shouldn't this disable the OH3 clock , which were enabled a few lines above? > > Hm, not sure, the clock does not take a controller index... I guess if > the second instance is returning an error, that would disable the clock > also for the first controller... Siiiiiiigh! All right, that sucks and we absolutely have no time for reworking the whole clock framework on imx for this release. You want this stuff in for this release, right ? Best regards, Marek Vasut
On 05/06/2016 01:59 AM, Stefan Agner wrote: > There could be runtime determined board specific reason why a EHCI > initialization fails (e.g. ENODEV if a Port is not available). In > this case, properly return the error code. > While at it, that function (board_ehci_hcd_init) has actually two > documentation blocks... Use the correct function name for the > documentation block of board_usb_phy_mode. > > Signed-off-by: Stefan Agner <stefan@agner.ch> Applied, thanks. Best regards, Marek Vasut
On 2016-05-06 09:30, Marek Vasut wrote: > On 05/06/2016 06:12 PM, Stefan Agner wrote: >> On 2016-05-05 17:39, Marek Vasut wrote: >>> On 05/06/2016 01:59 AM, Stefan Agner wrote: >>>> There could be runtime determined board specific reason why a EHCI >>>> initialization fails (e.g. ENODEV if a Port is not available). In >>>> this case, properly return the error code. >>>> While at it, that function (board_ehci_hcd_init) has actually two >>>> documentation blocks... Use the correct function name for the >>>> documentation block of board_usb_phy_mode. >>>> >>>> Signed-off-by: Stefan Agner <stefan@agner.ch> >>>> --- >>>> drivers/usb/host/ehci-mx6.c | 7 +++++-- >>>> 1 file changed, 5 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c >>>> index a981b50..bb48d0d 100644 >>>> --- a/drivers/usb/host/ehci-mx6.c >>>> +++ b/drivers/usb/host/ehci-mx6.c >>>> @@ -254,7 +254,7 @@ static void usb_oc_config(int index) >>>> } >>>> >>>> /** >>>> - * board_ehci_hcd_init - override usb phy mode >>>> + * board_usb_phy_mode - override usb phy mode >>>> * @port: usb host/otg port >>>> * >>>> * Target board specific, override usb_phy_mode. >>>> @@ -310,6 +310,7 @@ int ehci_hcd_init(int index, enum usb_init_type init, >>>> #endif >>>> struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR + >>>> (controller_spacing * index)); >>>> + int ret; >>>> >>>> if (index > 3) >>>> return -EINVAL; >>>> @@ -317,7 +318,9 @@ int ehci_hcd_init(int index, enum usb_init_type init, >>>> mdelay(1); >>>> >>>> /* Do board specific initialization */ >>>> - board_ehci_hcd_init(index); >>>> + ret = board_ehci_hcd_init(index); >>>> + if (ret) >>>> + return ret; >>> >>> Shouldn't this disable the OH3 clock , which were enabled a few lines above? >> >> Hm, not sure, the clock does not take a controller index... I guess if >> the second instance is returning an error, that would disable the clock >> also for the first controller... > > Siiiiiiigh! All right, that sucks and we absolutely have no time for > reworking the whole clock framework on imx for this release. You want > this stuff in for this release, right ? Wasn't that urgent as the board which will use this is not upstream yet anyway... But still, nice to have it in. Btw, I really like the new 2 month release cycle. -- Stefan
On 05/06/2016 07:11 PM, Stefan Agner wrote: > On 2016-05-06 09:30, Marek Vasut wrote: >> On 05/06/2016 06:12 PM, Stefan Agner wrote: >>> On 2016-05-05 17:39, Marek Vasut wrote: >>>> On 05/06/2016 01:59 AM, Stefan Agner wrote: >>>>> There could be runtime determined board specific reason why a EHCI >>>>> initialization fails (e.g. ENODEV if a Port is not available). In >>>>> this case, properly return the error code. >>>>> While at it, that function (board_ehci_hcd_init) has actually two >>>>> documentation blocks... Use the correct function name for the >>>>> documentation block of board_usb_phy_mode. >>>>> >>>>> Signed-off-by: Stefan Agner <stefan@agner.ch> >>>>> --- >>>>> drivers/usb/host/ehci-mx6.c | 7 +++++-- >>>>> 1 file changed, 5 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c >>>>> index a981b50..bb48d0d 100644 >>>>> --- a/drivers/usb/host/ehci-mx6.c >>>>> +++ b/drivers/usb/host/ehci-mx6.c >>>>> @@ -254,7 +254,7 @@ static void usb_oc_config(int index) >>>>> } >>>>> >>>>> /** >>>>> - * board_ehci_hcd_init - override usb phy mode >>>>> + * board_usb_phy_mode - override usb phy mode >>>>> * @port: usb host/otg port >>>>> * >>>>> * Target board specific, override usb_phy_mode. >>>>> @@ -310,6 +310,7 @@ int ehci_hcd_init(int index, enum usb_init_type init, >>>>> #endif >>>>> struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR + >>>>> (controller_spacing * index)); >>>>> + int ret; >>>>> >>>>> if (index > 3) >>>>> return -EINVAL; >>>>> @@ -317,7 +318,9 @@ int ehci_hcd_init(int index, enum usb_init_type init, >>>>> mdelay(1); >>>>> >>>>> /* Do board specific initialization */ >>>>> - board_ehci_hcd_init(index); >>>>> + ret = board_ehci_hcd_init(index); >>>>> + if (ret) >>>>> + return ret; >>>> >>>> Shouldn't this disable the OH3 clock , which were enabled a few lines above? >>> >>> Hm, not sure, the clock does not take a controller index... I guess if >>> the second instance is returning an error, that would disable the clock >>> also for the first controller... >> >> Siiiiiiigh! All right, that sucks and we absolutely have no time for >> reworking the whole clock framework on imx for this release. You want >> this stuff in for this release, right ? > > Wasn't that urgent as the board which will use this is not upstream yet > anyway... But still, nice to have it in. > > Btw, I really like the new 2 month release cycle. Heh, tell that to Tom :) > -- > Stefan >
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index a981b50..bb48d0d 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -254,7 +254,7 @@ static void usb_oc_config(int index) } /** - * board_ehci_hcd_init - override usb phy mode + * board_usb_phy_mode - override usb phy mode * @port: usb host/otg port * * Target board specific, override usb_phy_mode. @@ -310,6 +310,7 @@ int ehci_hcd_init(int index, enum usb_init_type init, #endif struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR + (controller_spacing * index)); + int ret; if (index > 3) return -EINVAL; @@ -317,7 +318,9 @@ int ehci_hcd_init(int index, enum usb_init_type init, mdelay(1); /* Do board specific initialization */ - board_ehci_hcd_init(index); + ret = board_ehci_hcd_init(index); + if (ret) + return ret; usb_power_config(index); usb_oc_config(index);
There could be runtime determined board specific reason why a EHCI initialization fails (e.g. ENODEV if a Port is not available). In this case, properly return the error code. While at it, that function (board_ehci_hcd_init) has actually two documentation blocks... Use the correct function name for the documentation block of board_usb_phy_mode. Signed-off-by: Stefan Agner <stefan@agner.ch> --- drivers/usb/host/ehci-mx6.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)