[U-Boot] [PATCH v2 2/2] usb: exynos5: arndale: Add network support

[Marek Vasut]

On Friday, January 03, 2014 at 06:14:38 AM, Inderpal Singh wrote:
> Hi Marek,
> 
> Thanks for review.
> 
> On 3 January 2014 06:26, Marek Vasut <marex at denx.de> wrote:
> > On Thursday, January 02, 2014 at 10:41:59 AM, Inderpal Singh wrote:
> > > Arndale board has AX88760, which is USB 2.0 Hub & USB 2.0 Ethernet
> > > Combo controller, connected to HSIC Phy of USB host controller via
> > > USB3503 hub.
> > > 
> > > This patch implements a board specific board_usb_init function in ehci
> > > driver to perform reset sequence for USB3503 hub and enables the
> > > relevant config options for network to work.
> > > 
> > > Signed-off-by: Inderpal Singh <inderpal.singh at linaro.org>
> > > Signed-off-by: Chander Kashyap <chander.kashyap at linaro.org>
> > > ---
> > > 
> > >  board/samsung/arndale/arndale.c |   13 +++++++++++++
> > >  drivers/usb/host/ehci-exynos.c  |   10 ++++++++++
> > >  include/configs/arndale.h       |    4 ++++
> > >  3 files changed, 27 insertions(+)
> > > 
> > > diff --git a/board/samsung/arndale/arndale.c
> > > b/board/samsung/arndale/arndale.c index 052fecd..deca348 100644
> > > --- a/board/samsung/arndale/arndale.c
> > > +++ b/board/samsung/arndale/arndale.c
> > > @@ -7,10 +7,23 @@
> > > 
> > >  #include <common.h>
> > >  #include <asm/arch/pinmux.h>
> > >  #include <asm/arch/dwmmc.h>
> > > 
> > > +#include <asm/arch/gpio.h>
> > > 
> > >  #include <asm/arch/power.h>
> > >  
> > >  DECLARE_GLOBAL_DATA_PTR;
> > > 
> > > +#ifdef CONFIG_USB_EHCI_EXYNOS
> > > +void exynos_board_usb_init(int value)
> > > +{
> > > +     struct exynos5_gpio_part1 *gpio = (struct exynos5_gpio_part1 *)
> > > +
> > 
> > samsung_get_base_gpio_part1();
> > 
> > > +
> > > +     /* Configure gpios for usb 3503 hub's reset and connect */
> > > +     s5p_gpio_direction_output(&gpio->x3, 5, value);
> > > +     s5p_gpio_direction_output(&gpio->d1, 7, value);
> > > +}
> > > +#endif
> > > +
> > > 
> > >  int board_init(void)
> > >  {
> > >  
> > >       gd->bd->bi_boot_params = (PHYS_SDRAM_1 + 0x100UL);
> > > 
> > > diff --git a/drivers/usb/host/ehci-exynos.c
> > > b/drivers/usb/host/ehci-exynos.c index 88e6466..4be6a60 100644
> > > --- a/drivers/usb/host/ehci-exynos.c
> > > +++ b/drivers/usb/host/ehci-exynos.c
> > > @@ -175,6 +175,12 @@ static void reset_usb_phy(struct exynos_usb_phy
> > 
> > *usb)
> > 
> > >       set_usbhost_phy_ctrl(POWER_USB_HOST_PHY_CTRL_DISABLE);
> > >  
> > >  }
> > > 
> > > +inline void __exynos_board_usb_init(int value)
> > > +{
> > > +}
> > > +void exynos_board_usb_init(int)
> > > +      __attribute__((weak, alias("__exynos_board_usb_init")));
> > > +
> > 
> > Sorry, this is not happening. Why can you not use the existing
> > board_usb_init()
> > and instead have to invent new stuff ?
> 
> I did not use board_usb_init because it has 2 parameters which may not be
> used as described in usb.h for my use case, which is to only configure
> gpios for reset and connect of usb 3503 hub.
> 
> Let me know if you still feel that board_usb_init should be used. In that
> case I will have to utilize index parameter in my own way.

Shall I read that as "I will abuse API" ? You know that this will not happen ;-)

Why can you not reset the USB HUB in regular board_usb_init() simply by toggling 
the switch's reset gpio as done on the rest of the boards ?