[U-Boot] [PATCH 0/2] Add usb eth support

[U-Boot] [PATCH 0/2] Add usb eth support

[Inderpal Singh]

From: Chander Kashyap <chander.kashyap@linaro.org>

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 patchset add support for this usb ethernet controllor.

Chander Kashyap (1):
  exynos5: arndale: Add network support

Inderpal Singh (1):
  usb: ehci: exynos: set/reset hsic phys

 arch/arm/include/asm/arch-exynos/ehci.h |   14 +++++++++
 board/samsung/arndale/arndale.c         |   23 +++++++++++++++
 drivers/usb/host/ehci-exynos.c          |   48 +++++++++++++++++++++++++++++++
 include/configs/arndale.h               |    6 ++++
 4 files changed, 91 insertions(+)

-- 
1.7.9.5

[U-Boot] [PATCH 1/2] usb: ehci: exynos: set/reset hsic phys

[Inderpal Singh]

The controller has 3 ports. The port0 is for USB 2.0 Phy, port1 and port2
are for HSIC phys. The usb 2.0 phy is already being setup. This patch
sets up the hsic phys.

Signed-off-by: Inderpal Singh <inderpal.singh@linaro.org>
---
 arch/arm/include/asm/arch-exynos/ehci.h |   14 +++++++++++
 drivers/usb/host/ehci-exynos.c          |   39 +++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/arch/arm/include/asm/arch-exynos/ehci.h b/arch/arm/include/asm/arch-exynos/ehci.h
index d79f25c..d2d70bd 100644
--- a/arch/arm/include/asm/arch-exynos/ehci.h
+++ b/arch/arm/include/asm/arch-exynos/ehci.h
@@ -29,6 +29,20 @@
 #define EHCICTRL_ENAINCR8			(1 << 27)
 #define EHCICTRL_ENAINCR16			(1 << 26)
 
+#define HSIC_CTRL_REFCLKSEL                     (0x2)
+#define HSIC_CTRL_REFCLKSEL_MASK                (0x3)
+#define HSIC_CTRL_REFCLKSEL_SHIFT               (23)
+
+#define HSIC_CTRL_REFCLKDIV_12                  (0x24)
+#define HSIC_CTRL_REFCLKDIV_MASK                (0x7f)
+#define HSIC_CTRL_REFCLKDIV_SHIFT               (16)
+
+#define HSIC_CTRL_SIDDQ                         (0x1 << 6)
+#define HSIC_CTRL_FORCESLEEP                    (0x1 << 5)
+#define HSIC_CTRL_FORCESUSPEND                  (0x1 << 4)
+#define HSIC_CTRL_UTMISWRST                     (0x1 << 2)
+#define HSIC_CTRL_PHYSWRST                      (0x1 << 0)
+
 /* Register map for PHY control */
 struct exynos_usb_phy {
 	unsigned int usbphyctrl0;
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index 155677e..50fbf36 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -83,6 +83,8 @@ static int exynos_usb_parse_dt(const void *blob, struct exynos_ehci *exynos)
 /* Setup the EHCI host controller. */
 static void setup_usb_phy(struct exynos_usb_phy *usb)
 {
+	u32 hsic_ctrl;
+
 	set_usbhost_mode(USB20_PHY_CFG_HOST_LINK_EN);
 
 	set_usbhost_phy_ctrl(POWER_USB_HOST_PHY_CTRL_EN);
@@ -107,6 +109,32 @@ static void setup_usb_phy(struct exynos_usb_phy *usb)
 	clrbits_le32(&usb->usbphyctrl0,
 			HOST_CTRL0_LINKSWRST |
 			HOST_CTRL0_UTMISWRST);
+
+	/* HSIC Phy Setting */
+	hsic_ctrl = (HSIC_CTRL_FORCESUSPEND |
+			HSIC_CTRL_FORCESLEEP |
+			HSIC_CTRL_SIDDQ);
+
+	clrbits_le32(&usb->hsicphyctrl1, hsic_ctrl);
+	clrbits_le32(&usb->hsicphyctrl2, hsic_ctrl);
+
+	hsic_ctrl = (((HSIC_CTRL_REFCLKDIV_12 & HSIC_CTRL_REFCLKDIV_MASK)
+				<< HSIC_CTRL_REFCLKDIV_SHIFT)
+			| ((HSIC_CTRL_REFCLKSEL & HSIC_CTRL_REFCLKSEL_MASK)
+				<< HSIC_CTRL_REFCLKSEL_SHIFT)
+			| HSIC_CTRL_UTMISWRST);
+
+	setbits_le32(&usb->hsicphyctrl1, hsic_ctrl);
+	setbits_le32(&usb->hsicphyctrl2, hsic_ctrl);
+
+	udelay(10);
+
+	clrbits_le32(&usb->hsicphyctrl1, HSIC_CTRL_PHYSWRST |
+					HSIC_CTRL_UTMISWRST);
+
+	clrbits_le32(&usb->hsicphyctrl2, HSIC_CTRL_PHYSWRST |
+					HSIC_CTRL_UTMISWRST);
+
 	udelay(20);
 
 	/* EHCI Ctrl setting */
@@ -120,6 +148,8 @@ static void setup_usb_phy(struct exynos_usb_phy *usb)
 /* Reset the EHCI host controller. */
 static void reset_usb_phy(struct exynos_usb_phy *usb)
 {
+	u32 hsic_ctrl;
+
 	/* HOST_PHY reset */
 	setbits_le32(&usb->usbphyctrl0,
 			HOST_CTRL0_PHYSWRST |
@@ -128,6 +158,15 @@ static void reset_usb_phy(struct exynos_usb_phy *usb)
 			HOST_CTRL0_FORCESUSPEND |
 			HOST_CTRL0_FORCESLEEP);
 
+	/* HSIC Phy reset */
+	hsic_ctrl = (HSIC_CTRL_FORCESUSPEND |
+			HSIC_CTRL_FORCESLEEP |
+			HSIC_CTRL_SIDDQ |
+			HSIC_CTRL_PHYSWRST);
+
+	setbits_le32(&usb->hsicphyctrl1, hsic_ctrl);
+	setbits_le32(&usb->hsicphyctrl2, hsic_ctrl);
+
 	set_usbhost_phy_ctrl(POWER_USB_HOST_PHY_CTRL_DISABLE);
 }
 
-- 
1.7.9.5

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

[Inderpal Singh]

From: Chander Kashyap <chander.kashyap@linaro.org>

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@linaro.org>
Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
---
 board/samsung/arndale/arndale.c |   23 +++++++++++++++++++++++
 drivers/usb/host/ehci-exynos.c  |    9 +++++++++
 include/configs/arndale.h       |    6 ++++++
 3 files changed, 38 insertions(+)

diff --git a/board/samsung/arndale/arndale.c b/board/samsung/arndale/arndale.c
index 052fecd..44f20b9 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 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);
@@ -91,6 +104,16 @@ int board_early_init_f(void)
 }
 #endif
 
+#ifdef CONFIG_BOARD_LATE_INIT
+int board_late_init(void)
+{
+#ifdef CONFIG_PREBOOT
+	setenv("preboot", CONFIG_PREBOOT);
+	setenv("usbethaddr", "00:40:5c:26:0a:5b");
+#endif
+}
+#endif
+
 #ifdef CONFIG_DISPLAY_BOARDINFO
 int checkboard(void)
 {
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index 50fbf36..cba450e 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -170,6 +170,11 @@ static void reset_usb_phy(struct exynos_usb_phy *usb)
 	set_usbhost_phy_ctrl(POWER_USB_HOST_PHY_CTRL_DISABLE);
 }
 
+inline void __board_usb_init(int value)
+{
+}
+void board_usb_init(int) __attribute__((weak, alias("__board_usb_init")));
+
 /*
  * EHCI-initialization
  * Create the appropriate control structures to manage
@@ -189,8 +194,12 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor)
 	ctx->hcd = (struct ehci_hccr *)samsung_get_base_usb_ehci();
 #endif
 
+	board_usb_init(0);
+
 	setup_usb_phy(ctx->usb);
 
+	board_usb_init(1);
+
 	*hccr = ctx->hcd;
 	*hcor = (struct ehci_hcor *)((uint32_t) *hccr
 				+ HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
diff --git a/include/configs/arndale.h b/include/configs/arndale.h
index ed44a04..ea24661 100644
--- a/include/configs/arndale.h
+++ b/include/configs/arndale.h
@@ -116,6 +116,10 @@
 #define CONFIG_USB_EHCI_EXYNOS
 #define CONFIG_USB_STORAGE
 
+#define CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS	3
+#define CONFIG_USB_HOST_ETHER
+#define CONFIG_USB_ETHER_ASIX
+#define CONFIG_PREBOOT                 "usb start"
 /* MMC SPL */
 #define CONFIG_SPL
 #define COPY_BL2_FNPTR_ADDR	0x02020030
@@ -179,6 +183,8 @@
 #define CONFIG_ENV_IS_IN_MMC
 #define CONFIG_SECURE_BL1_ONLY
 
+#define CONFIG_BOARD_LATE_INIT
+
 /* Secure FW size configuration */
 #ifdef	CONFIG_SECURE_BL1_ONLY
 #define	CONFIG_SEC_FW_SIZE		(8 << 10)	/* 8KB */
-- 
1.7.9.5

[U-Boot] [PATCH 0/2] Add usb eth support

[Chander Kashyap]

Ping..

On 19 October 2013 12:19, Inderpal Singh <chander.kashyap@linaro.org> wrote:
> From: Chander Kashyap <chander.kashyap@linaro.org>
>
> 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 patchset add support for this usb ethernet controllor.
>
> Chander Kashyap (1):
>   exynos5: arndale: Add network support
>
> Inderpal Singh (1):
>   usb: ehci: exynos: set/reset hsic phys
>
>  arch/arm/include/asm/arch-exynos/ehci.h |   14 +++++++++
>  board/samsung/arndale/arndale.c         |   23 +++++++++++++++
>  drivers/usb/host/ehci-exynos.c          |   48 +++++++++++++++++++++++++++++++
>  include/configs/arndale.h               |    6 ++++
>  4 files changed, 91 insertions(+)
>
> --
> 1.7.9.5
>



-- 
with warm regards,
Chander Kashyap

[U-Boot] [PATCH 0/2] Add usb eth support

[Chander Kashyap]

Ping

On 19 November 2013 11:09, Chander Kashyap <chander.kashyap@linaro.org> wrote:
> Ping..
>
> On 19 October 2013 12:19, Inderpal Singh <chander.kashyap@linaro.org> wrote:
>> From: Chander Kashyap <chander.kashyap@linaro.org>
>>
>> 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 patchset add support for this usb ethernet controllor.
>>
>> Chander Kashyap (1):
>>   exynos5: arndale: Add network support
>>
>> Inderpal Singh (1):
>>   usb: ehci: exynos: set/reset hsic phys
>>
>>  arch/arm/include/asm/arch-exynos/ehci.h |   14 +++++++++
>>  board/samsung/arndale/arndale.c         |   23 +++++++++++++++
>>  drivers/usb/host/ehci-exynos.c          |   48 +++++++++++++++++++++++++++++++
>>  include/configs/arndale.h               |    6 ++++
>>  4 files changed, 91 insertions(+)
>>
>> --
>> 1.7.9.5
>>
>
>
>
> --
> with warm regards,
> Chander Kashyap



-- 
with warm regards,
Chander Kashyap

[U-Boot] [PATCH 0/2] Add usb eth support

[Minkyu Kang]

Dear Marek Vasut,

please check this patchset.
This patchset is delegated to you.

http://patchwork.ozlabs.org/patch/284845/
http://patchwork.ozlabs.org/patch/284839/

On 02/12/13 19:21, Chander Kashyap wrote:
> Ping
> 
> On 19 November 2013 11:09, Chander Kashyap <chander.kashyap@linaro.org> wrote:
>> Ping..
>>
>> On 19 October 2013 12:19, Inderpal Singh <chander.kashyap@linaro.org> wrote:
>>> From: Chander Kashyap <chander.kashyap@linaro.org>
>>>
>>> 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 patchset add support for this usb ethernet controllor.
>>>
>>> Chander Kashyap (1):
>>>   exynos5: arndale: Add network support
>>>
>>> Inderpal Singh (1):
>>>   usb: ehci: exynos: set/reset hsic phys
>>>
>>>  arch/arm/include/asm/arch-exynos/ehci.h |   14 +++++++++
>>>  board/samsung/arndale/arndale.c         |   23 +++++++++++++++
>>>  drivers/usb/host/ehci-exynos.c          |   48 +++++++++++++++++++++++++++++++
>>>  include/configs/arndale.h               |    6 ++++
>>>  4 files changed, 91 insertions(+)
>>>
>>> --
>>> 1.7.9.5
>>>
>>
>>
>>
>> --
>> with warm regards,
>> Chander Kashyap
> 
> 
> 

Thanks,
Minkyu Kang.

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

[Marek Vasut]

On Saturday, October 19, 2013 at 08:49:27 AM, Inderpal Singh wrote:
> From: Chander Kashyap <chander.kashyap@linaro.org>
> 
> 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@linaro.org>
> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
> ---
>  board/samsung/arndale/arndale.c |   23 +++++++++++++++++++++++
>  drivers/usb/host/ehci-exynos.c  |    9 +++++++++
>  include/configs/arndale.h       |    6 ++++++
>  3 files changed, 38 insertions(+)
> 
> diff --git a/board/samsung/arndale/arndale.c
> b/board/samsung/arndale/arndale.c index 052fecd..44f20b9 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 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);
> @@ -91,6 +104,16 @@ int board_early_init_f(void)
>  }
>  #endif
> 
> +#ifdef CONFIG_BOARD_LATE_INIT
> +int board_late_init(void)
> +{
> +#ifdef CONFIG_PREBOOT
> +	setenv("preboot", CONFIG_PREBOOT);
> +	setenv("usbethaddr", "00:40:5c:26:0a:5b");

Why do you need these 'setenv' calls ? This logic seems completely broken. The 
'preboot' env variable is set already by defining the CONFIG_PREBOOT and the 
usbethaddr shall be set by user.
[...]

[U-Boot] [PATCH 0/2] Add usb eth support

[Marek Vasut]

On Thursday, December 12, 2013 at 09:24:48 AM, Minkyu Kang wrote:
> Dear Marek Vasut,
> 
> please check this patchset.
> This patchset is delegated to you.
> 
> http://patchwork.ozlabs.org/patch/284845/
> http://patchwork.ozlabs.org/patch/284839/

I did check them now. The patches didn't have 'usb:' tag on them nor was I on 
CC, so I missed them. Thanks for bringing them to my attention.

Best regards,
Marek Vasut

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

[Inderpal Singh]

Hi Marek,

Thanks for the review.


On 12 December 2013 19:03, Marek Vasut <marex@denx.de> wrote:

> On Saturday, October 19, 2013 at 08:49:27 AM, Inderpal Singh wrote:
> > From: Chander Kashyap <chander.kashyap@linaro.org>
> >
> > 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@linaro.org>
> > Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
> > ---
> >  board/samsung/arndale/arndale.c |   23 +++++++++++++++++++++++
> >  drivers/usb/host/ehci-exynos.c  |    9 +++++++++
> >  include/configs/arndale.h       |    6 ++++++
> >  3 files changed, 38 insertions(+)
> >
> > diff --git a/board/samsung/arndale/arndale.c
> > b/board/samsung/arndale/arndale.c index 052fecd..44f20b9 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 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);
> > @@ -91,6 +104,16 @@ int board_early_init_f(void)
> >  }
> >  #endif
> >
> > +#ifdef CONFIG_BOARD_LATE_INIT
> > +int board_late_init(void)
> > +{
> > +#ifdef CONFIG_PREBOOT
> > +     setenv("preboot", CONFIG_PREBOOT);
> > +     setenv("usbethaddr", "00:40:5c:26:0a:5b");
>
> Why do you need these 'setenv' calls ? This logic seems completely broken.
> The
> 'preboot' env variable is set already by defining the CONFIG_PREBOOT and
> the
> usbethaddr shall be set by user.
>

Since arndale is a development board so this was done so that user
(developer) does not has to set usbethaddr on every boot. If its redundant
then I will remove this in the next version of the patchset.

Thanks,
Inder



> [...]
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

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

[Marek Vasut]

On Saturday, December 14, 2013 at 03:00:39 PM, Inderpal Singh wrote:
> Hi Marek,
> 
> Thanks for the review.
> 
> On 12 December 2013 19:03, Marek Vasut <marex@denx.de> wrote:
> > On Saturday, October 19, 2013 at 08:49:27 AM, Inderpal Singh wrote:
> > > From: Chander Kashyap <chander.kashyap@linaro.org>
> > > 
> > > 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@linaro.org>
> > > Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
> > > ---
> > > 
> > >  board/samsung/arndale/arndale.c |   23 +++++++++++++++++++++++
> > >  drivers/usb/host/ehci-exynos.c  |    9 +++++++++
> > >  include/configs/arndale.h       |    6 ++++++
> > >  3 files changed, 38 insertions(+)
> > > 
> > > diff --git a/board/samsung/arndale/arndale.c
> > > b/board/samsung/arndale/arndale.c index 052fecd..44f20b9 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 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);
> > > 
> > > @@ -91,6 +104,16 @@ int board_early_init_f(void)
> > > 
> > >  }
> > >  #endif
> > > 
> > > +#ifdef CONFIG_BOARD_LATE_INIT
> > > +int board_late_init(void)
> > > +{
> > > +#ifdef CONFIG_PREBOOT
> > > +     setenv("preboot", CONFIG_PREBOOT);
> > > +     setenv("usbethaddr", "00:40:5c:26:0a:5b");
> > 
> > Why do you need these 'setenv' calls ? This logic seems completely
> > broken. The
> > 'preboot' env variable is set already by defining the CONFIG_PREBOOT and
> > the
> > usbethaddr shall be set by user.
> 
> Since arndale is a development board so this was done so that user
> (developer) does not has to set usbethaddr on every boot. If its redundant
> then I will remove this in the next version of the patchset.

Can the user/developer/ ... just run:

=> setenv usbethaddr 00:11:22:33:44:fg
=> saveenv

on the U-Boot command line?

setenv("preboot", CONFIG_PREBOOT); is broken, the user might want to override 
this variable, yet you don't allow him to as you always reset it back to default 
value. If the board not loaded with a valid environment, a default env will be 
used and if CONFIG_PREBOOT is defined in your include/configs/<board>.h , then 
the 'preboot' variable will also be set to the default value so no need to set 
it like so explicitly. You can reset a variable using "env default -f preboot'.

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

[Inderpal Singh]

On 14 December 2013 23:20, Marek Vasut <marex@denx.de> wrote:

> On Saturday, December 14, 2013 at 03:00:39 PM, Inderpal Singh wrote:
> > Hi Marek,
> >
> > Thanks for the review.
> >
> > On 12 December 2013 19:03, Marek Vasut <marex@denx.de> wrote:
> > > On Saturday, October 19, 2013 at 08:49:27 AM, Inderpal Singh wrote:
> > > > From: Chander Kashyap <chander.kashyap@linaro.org>
> > > >
> > > > 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@linaro.org>
> > > > Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
> > > > ---
> > > >
> > > >  board/samsung/arndale/arndale.c |   23 +++++++++++++++++++++++
> > > >  drivers/usb/host/ehci-exynos.c  |    9 +++++++++
> > > >  include/configs/arndale.h       |    6 ++++++
> > > >  3 files changed, 38 insertions(+)
> > > >
> > > > diff --git a/board/samsung/arndale/arndale.c
> > > > b/board/samsung/arndale/arndale.c index 052fecd..44f20b9 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 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);
> > > >
> > > > @@ -91,6 +104,16 @@ int board_early_init_f(void)
> > > >
> > > >  }
> > > >  #endif
> > > >
> > > > +#ifdef CONFIG_BOARD_LATE_INIT
> > > > +int board_late_init(void)
> > > > +{
> > > > +#ifdef CONFIG_PREBOOT
> > > > +     setenv("preboot", CONFIG_PREBOOT);
> > > > +     setenv("usbethaddr", "00:40:5c:26:0a:5b");
> > >
> > > Why do you need these 'setenv' calls ? This logic seems completely
> > > broken. The
> > > 'preboot' env variable is set already by defining the CONFIG_PREBOOT
> and
> > > the
> > > usbethaddr shall be set by user.
> >
> > Since arndale is a development board so this was done so that user
> > (developer) does not has to set usbethaddr on every boot. If its
> redundant
> > then I will remove this in the next version of the patchset.
>
> Can the user/developer/ ... just run:
>
> => setenv usbethaddr 00:11:22:33:44:fg
> => saveenv
>
> on the U-Boot command line?
>
> setenv("preboot", CONFIG_PREBOOT); is broken, the user might want to
> override
> this variable, yet you don't allow him to as you always reset it back to
> default
> value. If the board not loaded with a valid environment, a default env
> will be
> used and if CONFIG_PREBOOT is defined in your include/configs/<board>.h ,
> then
> the 'preboot' variable will also be set to the default value so no need to
> set
> it like so explicitly. You can reset a variable using "env default -f
> preboot'.
>

Thanks a lot Marek for the explanation.
Will remove this in the next patchset.


Thanks,
Inder

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

[Marek Vasut]

On Saturday, December 14, 2013 at 03:28:57 PM, Inderpal Singh wrote:
> On 14 December 2013 23:20, Marek Vasut <marex@denx.de> wrote:
> > On Saturday, December 14, 2013 at 03:00:39 PM, Inderpal Singh wrote:
> > > Hi Marek,
> > > 
> > > Thanks for the review.
> > > 
> > > On 12 December 2013 19:03, Marek Vasut <marex@denx.de> wrote:
> > > > On Saturday, October 19, 2013 at 08:49:27 AM, Inderpal Singh wrote:
> > > > > From: Chander Kashyap <chander.kashyap@linaro.org>
> > > > > 
> > > > > 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@linaro.org>
> > > > > Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
> > > > > ---
> > > > > 
> > > > >  board/samsung/arndale/arndale.c |   23 +++++++++++++++++++++++
> > > > >  drivers/usb/host/ehci-exynos.c  |    9 +++++++++
> > > > >  include/configs/arndale.h       |    6 ++++++
> > > > >  3 files changed, 38 insertions(+)
> > > > > 
> > > > > diff --git a/board/samsung/arndale/arndale.c
> > > > > b/board/samsung/arndale/arndale.c index 052fecd..44f20b9 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 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);
> > > > > 
> > > > > @@ -91,6 +104,16 @@ int board_early_init_f(void)
> > > > > 
> > > > >  }
> > > > >  #endif
> > > > > 
> > > > > +#ifdef CONFIG_BOARD_LATE_INIT
> > > > > +int board_late_init(void)
> > > > > +{
> > > > > +#ifdef CONFIG_PREBOOT
> > > > > +     setenv("preboot", CONFIG_PREBOOT);
> > > > > +     setenv("usbethaddr", "00:40:5c:26:0a:5b");
> > > > 
> > > > Why do you need these 'setenv' calls ? This logic seems completely
> > > > broken. The
> > > > 'preboot' env variable is set already by defining the CONFIG_PREBOOT
> > 
> > and
> > 
> > > > the
> > > > usbethaddr shall be set by user.
> > > 
> > > Since arndale is a development board so this was done so that user
> > > (developer) does not has to set usbethaddr on every boot. If its
> > 
> > redundant
> > 
> > > then I will remove this in the next version of the patchset.
> > 
> > Can the user/developer/ ... just run:
> > 
> > => setenv usbethaddr 00:11:22:33:44:fg
> > => saveenv
> > 
> > on the U-Boot command line?
> > 
> > setenv("preboot", CONFIG_PREBOOT); is broken, the user might want to
> > override
> > this variable, yet you don't allow him to as you always reset it back to
> > default
> > value. If the board not loaded with a valid environment, a default env
> > will be
> > used and if CONFIG_PREBOOT is defined in your include/configs/<board>.h ,
> > then
> > the 'preboot' variable will also be set to the default value so no need
> > to set
> > it like so explicitly. You can reset a variable using "env default -f
> > preboot'.
> 
> Thanks a lot Marek for the explanation.
> Will remove this in the next patchset.

Sure, HTH. Sorry I didn't review your patch, next time please at least add me to 
the CC or add 'usb:' tag to the patch.