[U-Boot] [PATCH] usb: ehci-mx6: allow board_ehci_hcd_init to fail

[U-Boot] [PATCH] usb: ehci-mx6: allow board_ehci_hcd_init to fail

[Stefan Agner]

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;
 
 	usb_power_config(index);
 	usb_oc_config(index);
-- 
2.8.2

[U-Boot] [PATCH] usb: ehci-mx6: allow board_ehci_hcd_init to fail

[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>
> ---
>  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);
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH] usb: ehci-mx6: allow board_ehci_hcd_init to fail

[Stefan Agner]

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);
>>

[U-Boot] [PATCH] usb: ehci-mx6: allow board_ehci_hcd_init to fail

[Marek Vasut]

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

[U-Boot] [PATCH] usb: ehci-mx6: allow board_ehci_hcd_init to fail

[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

[U-Boot] [PATCH] usb: ehci-mx6: allow board_ehci_hcd_init to fail

[Stefan Agner]

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

[U-Boot] [PATCH] usb: ehci-mx6: allow board_ehci_hcd_init to fail

[Marek Vasut]

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
> 


-- 
Best regards,
Marek Vasut