Re: [U-Boot] [RFC] usb: host: xhci-omap: Remove redundant board_usb_init and board_usb_cleanup functions

2018-02-14 Thread Uri Mashiach

Hi Faiz,

On 02/14/2018 05:47 PM, Faiz Abbas wrote:

Hi Uri,

On Wednesday 14 February 2018 08:56 PM, Uri Mashiach wrote:

Hi,
Sorry for the late response.

On 02/14/2018 04:19 PM, Marek Vasut wrote:

On 02/14/2018 03:14 PM, Faiz Abbas wrote:

Hi,

On Wednesday 14 February 2018 06:53 PM, Marek Vasut wrote:

On 02/14/2018 12:20 PM, Faiz Abbas wrote:

Hi,

On Wednesday 14 February 2018 03:46 PM, Marek Vasut wrote:

On 02/14/2018 11:10 AM, Faiz Abbas wrote:

board_usb_init()/_cleanup() should be in board files and don't have
a place in the xhci-omap driver. Weak versions for
board_usb_init()/_cleanup() already exist in common/usb.c
(for host mode) and drivers/usb/gadget/g_dnl.c (for gadget mode).

Signed-off-by: Faiz Abbas 


Reviewed-by: Marek Vasut 

I'd like some TBs from the people using those boards.


I have tested this for dra7xx, am43xx and am57xx.


So why is it marked RFC ?



Because I was unclear why Uri Mashiach did not do this in 1a9a5f7 ("fix
double weak board_usb_init functions").


OK, submit it as a normal patch with my RB.



As mentioned in 1a9a5f7, the functions omap_xhci_board_usb_init() and
omap_xhci_board_usb_cleanup() are called for the boards with the
CONFIG_USB_XHCI_OMAP definition.

The weak implementation of the functions omap_xhci_board_usb_init()
executed enable_usb_clocks().
The weak implementation of the function omap_xhci_board_usb_cleanup()
executed the function disable_usb_clocks().

For the boards compulab/cl-som-am57x and compulab/cm_t43:
* CONFIG_USB_XHCI_OMAP is defined
* omap_xhci_board_usb_init is not implemented, relying on the weak
implementation.
* omap_xhci_board_usb_cleanup is not defined, relying on the weak
implementation.

The fix is missing the implementation of board_usb_init and
board_usb_cleanup in the compulab/cl-som-am57x and compulab/cm_t43.
The implementation should include the content of the deleted weak
functions omap_xhci_board_usb_init() and omap_xhci_board_usb_cleanup().



Thanks for clarifying. Shouldn't that be implemented in the board files?


Yes


I can add board_usb_init and board_usb_cleanup for those platforms in
v2. Can you help me test this on those platforms?


Yes



Thanks,
Faiz



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


Re: [U-Boot] [RFC] usb: host: xhci-omap: Remove redundant board_usb_init and board_usb_cleanup functions

2018-02-14 Thread Faiz Abbas
Hi Uri,

On Wednesday 14 February 2018 08:56 PM, Uri Mashiach wrote:
> Hi,
> Sorry for the late response.
> 
> On 02/14/2018 04:19 PM, Marek Vasut wrote:
>> On 02/14/2018 03:14 PM, Faiz Abbas wrote:
>>> Hi,
>>>
>>> On Wednesday 14 February 2018 06:53 PM, Marek Vasut wrote:
 On 02/14/2018 12:20 PM, Faiz Abbas wrote:
> Hi,
>
> On Wednesday 14 February 2018 03:46 PM, Marek Vasut wrote:
>> On 02/14/2018 11:10 AM, Faiz Abbas wrote:
>>> board_usb_init()/_cleanup() should be in board files and don't have
>>> a place in the xhci-omap driver. Weak versions for
>>> board_usb_init()/_cleanup() already exist in common/usb.c
>>> (for host mode) and drivers/usb/gadget/g_dnl.c (for gadget mode).
>>>
>>> Signed-off-by: Faiz Abbas 
>>
>> Reviewed-by: Marek Vasut 
>>
>> I'd like some TBs from the people using those boards.
>
> I have tested this for dra7xx, am43xx and am57xx.

 So why is it marked RFC ?

>>>
>>> Because I was unclear why Uri Mashiach did not do this in 1a9a5f7 ("fix
>>> double weak board_usb_init functions").
>>
>> OK, submit it as a normal patch with my RB.
>>
> 
> As mentioned in 1a9a5f7, the functions omap_xhci_board_usb_init() and
> omap_xhci_board_usb_cleanup() are called for the boards with the
> CONFIG_USB_XHCI_OMAP definition.
> 
> The weak implementation of the functions omap_xhci_board_usb_init()
> executed enable_usb_clocks().
> The weak implementation of the function omap_xhci_board_usb_cleanup()
> executed the function disable_usb_clocks().
> 
> For the boards compulab/cl-som-am57x and compulab/cm_t43:
> * CONFIG_USB_XHCI_OMAP is defined
> * omap_xhci_board_usb_init is not implemented, relying on the weak
> implementation.
> * omap_xhci_board_usb_cleanup is not defined, relying on the weak
> implementation.
> 
> The fix is missing the implementation of board_usb_init and
> board_usb_cleanup in the compulab/cl-som-am57x and compulab/cm_t43.
> The implementation should include the content of the deleted weak
> functions omap_xhci_board_usb_init() and omap_xhci_board_usb_cleanup().
> 

Thanks for clarifying. Shouldn't that be implemented in the board files?
I can add board_usb_init and board_usb_cleanup for those platforms in
v2. Can you help me test this on those platforms?

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


Re: [U-Boot] [RFC] usb: host: xhci-omap: Remove redundant board_usb_init and board_usb_cleanup functions

2018-02-14 Thread Uri Mashiach

Hi,
Sorry for the late response.

On 02/14/2018 04:19 PM, Marek Vasut wrote:

On 02/14/2018 03:14 PM, Faiz Abbas wrote:

Hi,

On Wednesday 14 February 2018 06:53 PM, Marek Vasut wrote:

On 02/14/2018 12:20 PM, Faiz Abbas wrote:

Hi,

On Wednesday 14 February 2018 03:46 PM, Marek Vasut wrote:

On 02/14/2018 11:10 AM, Faiz Abbas wrote:

board_usb_init()/_cleanup() should be in board files and don't have
a place in the xhci-omap driver. Weak versions for
board_usb_init()/_cleanup() already exist in common/usb.c
(for host mode) and drivers/usb/gadget/g_dnl.c (for gadget mode).

Signed-off-by: Faiz Abbas 


Reviewed-by: Marek Vasut 

I'd like some TBs from the people using those boards.


I have tested this for dra7xx, am43xx and am57xx.


So why is it marked RFC ?



Because I was unclear why Uri Mashiach did not do this in 1a9a5f7 ("fix
double weak board_usb_init functions").


OK, submit it as a normal patch with my RB.



As mentioned in 1a9a5f7, the functions omap_xhci_board_usb_init() and 
omap_xhci_board_usb_cleanup() are called for the boards with the 
CONFIG_USB_XHCI_OMAP definition.


The weak implementation of the functions omap_xhci_board_usb_init()
executed enable_usb_clocks().
The weak implementation of the function omap_xhci_board_usb_cleanup() 
executed the function disable_usb_clocks().


For the boards compulab/cl-som-am57x and compulab/cm_t43:
* CONFIG_USB_XHCI_OMAP is defined
* omap_xhci_board_usb_init is not implemented, relying on the weak 
implementation.
* omap_xhci_board_usb_cleanup is not defined, relying on the weak 
implementation.


The fix is missing the implementation of board_usb_init and 
board_usb_cleanup in the compulab/cl-som-am57x and compulab/cm_t43.
The implementation should include the content of the deleted weak 
functions omap_xhci_board_usb_init() and omap_xhci_board_usb_cleanup().


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


Re: [U-Boot] [RFC] usb: host: xhci-omap: Remove redundant board_usb_init and board_usb_cleanup functions

2018-02-14 Thread Marek Vasut
On 02/14/2018 03:14 PM, Faiz Abbas wrote:
> Hi,
> 
> On Wednesday 14 February 2018 06:53 PM, Marek Vasut wrote:
>> On 02/14/2018 12:20 PM, Faiz Abbas wrote:
>>> Hi,
>>>
>>> On Wednesday 14 February 2018 03:46 PM, Marek Vasut wrote:
 On 02/14/2018 11:10 AM, Faiz Abbas wrote:
> board_usb_init()/_cleanup() should be in board files and don't have
> a place in the xhci-omap driver. Weak versions for
> board_usb_init()/_cleanup() already exist in common/usb.c
> (for host mode) and drivers/usb/gadget/g_dnl.c (for gadget mode).
>
> Signed-off-by: Faiz Abbas 

 Reviewed-by: Marek Vasut 

 I'd like some TBs from the people using those boards.
>>>
>>> I have tested this for dra7xx, am43xx and am57xx.
>>
>> So why is it marked RFC ?
>>
> 
> Because I was unclear why Uri Mashiach did not do this in 1a9a5f7 ("fix
> double weak board_usb_init functions").

OK, submit it as a normal patch with my RB.

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [RFC] usb: host: xhci-omap: Remove redundant board_usb_init and board_usb_cleanup functions

2018-02-14 Thread Faiz Abbas
Hi,

On Wednesday 14 February 2018 06:53 PM, Marek Vasut wrote:
> On 02/14/2018 12:20 PM, Faiz Abbas wrote:
>> Hi,
>>
>> On Wednesday 14 February 2018 03:46 PM, Marek Vasut wrote:
>>> On 02/14/2018 11:10 AM, Faiz Abbas wrote:
 board_usb_init()/_cleanup() should be in board files and don't have
 a place in the xhci-omap driver. Weak versions for
 board_usb_init()/_cleanup() already exist in common/usb.c
 (for host mode) and drivers/usb/gadget/g_dnl.c (for gadget mode).

 Signed-off-by: Faiz Abbas 
>>>
>>> Reviewed-by: Marek Vasut 
>>>
>>> I'd like some TBs from the people using those boards.
>>
>> I have tested this for dra7xx, am43xx and am57xx.
> 
> So why is it marked RFC ?
> 

Because I was unclear why Uri Mashiach did not do this in 1a9a5f7 ("fix
double weak board_usb_init functions").

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


Re: [U-Boot] [RFC] usb: host: xhci-omap: Remove redundant board_usb_init and board_usb_cleanup functions

2018-02-14 Thread Marek Vasut
On 02/14/2018 12:20 PM, Faiz Abbas wrote:
> Hi,
> 
> On Wednesday 14 February 2018 03:46 PM, Marek Vasut wrote:
>> On 02/14/2018 11:10 AM, Faiz Abbas wrote:
>>> board_usb_init()/_cleanup() should be in board files and don't have
>>> a place in the xhci-omap driver. Weak versions for
>>> board_usb_init()/_cleanup() already exist in common/usb.c
>>> (for host mode) and drivers/usb/gadget/g_dnl.c (for gadget mode).
>>>
>>> Signed-off-by: Faiz Abbas 
>>
>> Reviewed-by: Marek Vasut 
>>
>> I'd like some TBs from the people using those boards.
> 
> I have tested this for dra7xx, am43xx and am57xx.

So why is it marked RFC ?

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [RFC] usb: host: xhci-omap: Remove redundant board_usb_init and board_usb_cleanup functions

2018-02-14 Thread Bin Meng
On Wed, Feb 14, 2018 at 6:10 PM, Faiz Abbas  wrote:
> board_usb_init()/_cleanup() should be in board files and don't have
> a place in the xhci-omap driver. Weak versions for
> board_usb_init()/_cleanup() already exist in common/usb.c
> (for host mode) and drivers/usb/gadget/g_dnl.c (for gadget mode).
>
> Signed-off-by: Faiz Abbas 
> ---
> Since 1a9a5f7 ("fix double weak board_usb_init functions") the
> board_usb_init()/_cleanup() a prefix omap_xhci_* was added to the board
> files and in omap-xhci.c because there was a duplicate implementation
> in common/usb.c. However, this broke the gadget mode path which uses the
> same board_usb_init/cleanup() in the board specific files.
>
> I think a better way would be to just remove the functions from xhci-omap
> like in this patch. Any issues with this?
>
>  board/ti/am43xx/board.c  |  4 ++--
>  board/ti/am57xx/board.c  |  4 ++--
>  board/ti/dra7xx/evm.c|  4 ++--
>  drivers/usb/host/xhci-omap.c | 22 --
>  4 files changed, 6 insertions(+), 28 deletions(-)
>

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


Re: [U-Boot] [RFC] usb: host: xhci-omap: Remove redundant board_usb_init and board_usb_cleanup functions

2018-02-14 Thread Faiz Abbas
Hi,

On Wednesday 14 February 2018 03:46 PM, Marek Vasut wrote:
> On 02/14/2018 11:10 AM, Faiz Abbas wrote:
>> board_usb_init()/_cleanup() should be in board files and don't have
>> a place in the xhci-omap driver. Weak versions for
>> board_usb_init()/_cleanup() already exist in common/usb.c
>> (for host mode) and drivers/usb/gadget/g_dnl.c (for gadget mode).
>>
>> Signed-off-by: Faiz Abbas 
> 
> Reviewed-by: Marek Vasut 
> 
> I'd like some TBs from the people using those boards.

I have tested this for dra7xx, am43xx and am57xx.

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


Re: [U-Boot] [RFC] usb: host: xhci-omap: Remove redundant board_usb_init and board_usb_cleanup functions

2018-02-14 Thread Marek Vasut
On 02/14/2018 11:10 AM, Faiz Abbas wrote:
> board_usb_init()/_cleanup() should be in board files and don't have
> a place in the xhci-omap driver. Weak versions for
> board_usb_init()/_cleanup() already exist in common/usb.c
> (for host mode) and drivers/usb/gadget/g_dnl.c (for gadget mode).
> 
> Signed-off-by: Faiz Abbas 

Reviewed-by: Marek Vasut 

I'd like some TBs from the people using those boards.

> ---
> Since 1a9a5f7 ("fix double weak board_usb_init functions") the
> board_usb_init()/_cleanup() a prefix omap_xhci_* was added to the board
> files and in omap-xhci.c because there was a duplicate implementation
> in common/usb.c. However, this broke the gadget mode path which uses the
> same board_usb_init/cleanup() in the board specific files.
> 
> I think a better way would be to just remove the functions from xhci-omap
> like in this patch. Any issues with this?
> 
>  board/ti/am43xx/board.c  |  4 ++--
>  board/ti/am57xx/board.c  |  4 ++--
>  board/ti/dra7xx/evm.c|  4 ++--
>  drivers/usb/host/xhci-omap.c | 22 --
>  4 files changed, 6 insertions(+), 28 deletions(-)
> 
> diff --git a/board/ti/am43xx/board.c b/board/ti/am43xx/board.c
> index 06082f1..6286af4 100644
> --- a/board/ti/am43xx/board.c
> +++ b/board/ti/am43xx/board.c
> @@ -744,7 +744,7 @@ int usb_gadget_handle_interrupts(int index)
>  #endif /* CONFIG_USB_DWC3 */
>  
>  #if defined(CONFIG_USB_DWC3) || defined(CONFIG_USB_XHCI_OMAP)
> -int omap_xhci_board_usb_init(int index, enum usb_init_type init)
> +int board_usb_init(int index, enum usb_init_type init)
>  {
>   enable_usb_clocks(index);
>  #ifdef CONFIG_USB_DWC3
> @@ -775,7 +775,7 @@ int omap_xhci_board_usb_init(int index, enum 
> usb_init_type init)
>   return 0;
>  }
>  
> -int omap_xhci_board_usb_cleanup(int index, enum usb_init_type init)
> +int board_usb_cleanup(int index, enum usb_init_type init)
>  {
>  #ifdef CONFIG_USB_DWC3
>   switch (index) {
> diff --git a/board/ti/am57xx/board.c b/board/ti/am57xx/board.c
> index 1128784..c3f60f6 100644
> --- a/board/ti/am57xx/board.c
> +++ b/board/ti/am57xx/board.c
> @@ -867,7 +867,7 @@ int usb_gadget_handle_interrupts(int index)
>  #endif /* CONFIG_USB_DWC3 */
>  
>  #if defined(CONFIG_USB_DWC3) || defined(CONFIG_USB_XHCI_OMAP)
> -int omap_xhci_board_usb_init(int index, enum usb_init_type init)
> +int board_usb_init(int index, enum usb_init_type init)
>  {
>   enable_usb_clocks(index);
>   switch (index) {
> @@ -901,7 +901,7 @@ int omap_xhci_board_usb_init(int index, enum 
> usb_init_type init)
>   return 0;
>  }
>  
> -int omap_xhci_board_usb_cleanup(int index, enum usb_init_type init)
> +int board_usb_cleanup(int index, enum usb_init_type init)
>  {
>  #ifdef CONFIG_USB_DWC3
>   switch (index) {
> diff --git a/board/ti/dra7xx/evm.c b/board/ti/dra7xx/evm.c
> index 6ecf971..519475e 100644
> --- a/board/ti/dra7xx/evm.c
> +++ b/board/ti/dra7xx/evm.c
> @@ -907,7 +907,7 @@ static struct ti_usb_phy_device usb_phy2_device = {
>   .index = 1,
>  };
>  
> -int omap_xhci_board_usb_init(int index, enum usb_init_type init)
> +int board_usb_init(int index, enum usb_init_type init)
>  {
>   enable_usb_clocks(index);
>   switch (index) {
> @@ -944,7 +944,7 @@ int omap_xhci_board_usb_init(int index, enum 
> usb_init_type init)
>   return 0;
>  }
>  
> -int omap_xhci_board_usb_cleanup(int index, enum usb_init_type init)
> +int board_usb_cleanup(int index, enum usb_init_type init)
>  {
>   switch (index) {
>   case 0:
> diff --git a/drivers/usb/host/xhci-omap.c b/drivers/usb/host/xhci-omap.c
> index d6c5744..b814500 100644
> --- a/drivers/usb/host/xhci-omap.c
> +++ b/drivers/usb/host/xhci-omap.c
> @@ -27,28 +27,6 @@ DECLARE_GLOBAL_DATA_PTR;
>  
>  static struct omap_xhci omap;
>  
> -__weak int omap_xhci_board_usb_init(int index, enum usb_init_type init)
> -{
> - enable_usb_clocks(index);
> - return 0;
> -}
> -
> -int board_usb_init(int index, enum usb_init_type init)
> -{
> - return omap_xhci_board_usb_init(index, init);
> -}
> -
> -__weak int omap_xhci_board_usb_cleanup(int index, enum usb_init_type init)
> -{
> - disable_usb_clocks(index);
> - return 0;
> -}
> -
> -int board_usb_cleanup(int index, enum usb_init_type init)
> -{
> - return omap_xhci_board_usb_cleanup(index, init);
> -}
> -
>  static int omap_xhci_core_init(struct omap_xhci *omap)
>  {
>   int ret = 0;
> 


-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot