[U-Boot] [PATCH 0/3 V2] EXYNOS5: Add FDT support for USB

[U-Boot] [PATCH 0/3 V2] EXYNOS5: Add FDT support for USB

[Rajeshwari Shinde]

This patch set adds FDT support for USB device.

This patch set is based on
"EXYNOS5: MAX77686 add FDT support"

Changes in V2:
	- Removed checkpatch errors.

Rajeshwari Shinde (3):
  EHCI: Exynos: Add fdt support
  EXYNOS5: Add devine node for USB.
  EXYNOS5: FDT: Add compatible strings for USB

 arch/arm/dts/exynos5250.dtsi   |    7 +++++
 drivers/usb/host/ehci-exynos.c |   59 ++++++++++++++++++++++++++++++++++++---
 include/fdtdec.h               |    1 +
 lib/fdtdec.c                   |    1 +
 4 files changed, 63 insertions(+), 5 deletions(-)

-- 
1.7.4.4

[U-Boot] [PATCH 1/3 V2] EHCI: Exynos: Add fdt support

[Rajeshwari Shinde]

Adding fdt support to ehci-exynos in order to parse
register base addresses from the device node.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com>
---
Chnages in V2:
	- Removed checkpatch errors.
 drivers/usb/host/ehci-exynos.c |   59 ++++++++++++++++++++++++++++++++++++---
 1 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index 9f0ed06..f9189a5 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -21,6 +21,8 @@
  */
 
 #include <common.h>
+#include <fdtdec.h>
+#include <libfdt.h>
 #include <usb.h>
 #include <asm/arch/cpu.h>
 #include <asm/arch/ehci.h>
@@ -28,6 +30,9 @@
 #include <asm/arch/power.h>
 #include "ehci.h"
 
+/* Declare global data pointer */
+DECLARE_GLOBAL_DATA_PTR;
+
 /* Setup the EHCI host controller. */
 static void setup_usb_phy(struct exynos_usb_phy *usb)
 {
@@ -86,12 +91,39 @@ static void reset_usb_phy(struct exynos_usb_phy *usb)
  */
 int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor)
 {
-	struct exynos_usb_phy *usb;
+	struct exynos_usb_phy *usb = NULL;
+	unsigned int *hcd = NULL;
+	unsigned int node;
+
+	node = fdtdec_next_compatible(gd->fdt_blob, 0,
+					COMPAT_SAMSUNG_EXYNOS_EHCI);
+	if (node <= 0) {
+		debug("EHCI: Can't get device tree node for ehci\n");
+		return -1;
+	}
+
+	/*
+	 * Get the base address for usbphy from the device node
+	 */
+	usb = (struct exynos_usb_phy *)fdtdec_get_addr(gd->fdt_blob, node,
+								"phyreg");
+	if (usb == NULL) {
+		debug("Can't get the usbphy register address\n");
+		return -1;
+	}
 
-	usb = (struct exynos_usb_phy *)samsung_get_base_usb_phy();
 	setup_usb_phy(usb);
 
-	*hccr = (struct ehci_hccr *)samsung_get_base_usb_ehci();
+	/*
+	 * Get the base address for XHCI controller from the device node
+	 */
+	hcd = (unsigned int *)fdtdec_get_addr(gd->fdt_blob, node, "reg");
+	if (hcd == NULL) {
+		debug("Can't get the XHCI registere address\n");
+		return -1;
+	}
+
+	*hccr = (struct ehci_hccr *)hcd;
 	*hcor = (struct ehci_hcor *)((uint32_t) *hccr
 				+ HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
 
@@ -108,9 +140,26 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor)
  */
 int ehci_hcd_stop(int index)
 {
-	struct exynos_usb_phy *usb;
+	struct exynos_usb_phy *usb = NULL;
+	unsigned int node;
+
+	node = fdtdec_next_compatible(gd->fdt_blob, 0,
+					COMPAT_SAMSUNG_EXYNOS_EHCI);
+	if (node <= 0) {
+		debug("EHCI: Can't get device tree node for ehci\n");
+		return -1;
+	}
+
+	/*
+	 * Get the base address for usbphy from the device node
+	 */
+	usb = (struct exynos_usb_phy *)fdtdec_get_addr(gd->fdt_blob, node,
+								"phyreg");
+	if (usb == NULL) {
+		debug("Can't get the usbphy register address\n");
+		return -1;
+	}
 
-	usb = (struct exynos_usb_phy *)samsung_get_base_usb_phy();
 	reset_usb_phy(usb);
 
 	return 0;
-- 
1.7.4.4

[U-Boot] [PATCH 2/3 V2] EXYNOS5: Add devine node for USB.

[Rajeshwari Shinde]

This patch adds the device node required for USB

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
Chnages in V2:
	- None
 arch/arm/dts/exynos5250.dtsi |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/dts/exynos5250.dtsi b/arch/arm/dts/exynos5250.dtsi
index 2073ef2..dc727fc 100644
--- a/arch/arm/dts/exynos5250.dtsi
+++ b/arch/arm/dts/exynos5250.dtsi
@@ -143,4 +143,11 @@
 		reg = <0x131b0000 0x30>;
 		interrupts = <0 130 0>;
 	};
+
+	ehci at 12110000 {
+		compatible = "samsung,exynos-ehci";
+		reg = <0x12110000 0x100>;
+		phyreg = <0x12130000>;
+	};
+
 };
-- 
1.7.4.4

[U-Boot] [PATCH 3/3 V2] EXYNOS5: FDT: Add compatible strings for USB

[Rajeshwari Shinde]

Add required compatible information for USB

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
Acked-by: Simon Glass <sjg@chromium.org>
---
Changes in V2:
	- None.
 include/fdtdec.h |    1 +
 lib/fdtdec.c     |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/fdtdec.h b/include/fdtdec.h
index 5934af1..c464d58 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -74,6 +74,7 @@ enum fdt_compat_id {
 	COMPAT_WOLFSON_WM8994_CODEC,	/* Wolfson WM8994 Sound Codec */
 	COMPAT_SAMSUNG_EXYNOS_SPI,	/* Exynos SPI */
 	COMPAT_MAXIM_MAX77686_PMIC,	/* MAX77686 PMIC */
+	COMPAT_SAMSUNG_EXYNOS_EHCI,	/* Exynos EHCI controller */
 
 	COMPAT_COUNT,
 };
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 6bc57c5..eb2ff6c 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -51,6 +51,7 @@ static const char * const compat_names[COMPAT_COUNT] = {
 	COMPAT(WOLFSON_WM8994_CODEC, "wolfson,wm8994-codec"),
 	COMPAT(SAMSUNG_EXYNOS_SPI, "samsung,exynos-spi"),
 	COMPAT(MAXIM_MAX77686_PMIC, "maxim,MAX77686_PMIC"),
+	COMPAT(SAMSUNG_EXYNOS_EHCI, "samsung,exynos-ehci"),
 };
 
 const char *fdtdec_get_compatible(enum fdt_compat_id id)
-- 
1.7.4.4

[U-Boot] [PATCH 1/3 V2] EHCI: Exynos: Add fdt support

[Marek Vasut]

Dear Rajeshwari Shinde,

> Adding fdt support to ehci-exynos in order to parse
> register base addresses from the device node.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com>
> ---
> Chnages in V2:
> 	- Removed checkpatch errors.
>  drivers/usb/host/ehci-exynos.c |   59
> ++++++++++++++++++++++++++++++++++++--- 1 files changed, 54 insertions(+),
> 5 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-exynos.c
> b/drivers/usb/host/ehci-exynos.c index 9f0ed06..f9189a5 100644
> --- a/drivers/usb/host/ehci-exynos.c
> +++ b/drivers/usb/host/ehci-exynos.c
> @@ -21,6 +21,8 @@
>   */
> 
>  #include <common.h>
> +#include <fdtdec.h>
> +#include <libfdt.h>
>  #include <usb.h>
>  #include <asm/arch/cpu.h>
>  #include <asm/arch/ehci.h>
> @@ -28,6 +30,9 @@
>  #include <asm/arch/power.h>
>  #include "ehci.h"
> 
> +/* Declare global data pointer */
> +DECLARE_GLOBAL_DATA_PTR;
> +
>  /* Setup the EHCI host controller. */
>  static void setup_usb_phy(struct exynos_usb_phy *usb)
>  {
> @@ -86,12 +91,39 @@ static void reset_usb_phy(struct exynos_usb_phy *usb)
>   */
>  int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor
> **hcor) {
> -	struct exynos_usb_phy *usb;
> +	struct exynos_usb_phy *usb = NULL;
> +	unsigned int *hcd = NULL;
> +	unsigned int node;
> +
> +	node = fdtdec_next_compatible(gd->fdt_blob, 0,
> +					COMPAT_SAMSUNG_EXYNOS_EHCI);
> +	if (node <= 0) {
> +		debug("EHCI: Can't get device tree node for ehci\n");
[...]

error output should be really puts() or printf() ...

You can also use errno.h instead of -1.

Rest is good

[U-Boot] [PATCH 2/3 V2] EXYNOS5: Add devine node for USB.

[Marek Vasut]

Dear Rajeshwari Shinde,

Subject ... add "device" node ...

> This patch adds the device node required for USB
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> ---
> Chnages in V2:
> 	- None
>  arch/arm/dts/exynos5250.dtsi |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/dts/exynos5250.dtsi b/arch/arm/dts/exynos5250.dtsi
> index 2073ef2..dc727fc 100644
> --- a/arch/arm/dts/exynos5250.dtsi
> +++ b/arch/arm/dts/exynos5250.dtsi
> @@ -143,4 +143,11 @@
>  		reg = <0x131b0000 0x30>;
>  		interrupts = <0 130 0>;
>  	};
> +
> +	ehci at 12110000 {
> +		compatible = "samsung,exynos-ehci";
> +		reg = <0x12110000 0x100>;
> +		phyreg = <0x12130000>;
> +	};
> +
>  };

Best regards,
Marek Vasut

[U-Boot] [PATCH 1/3 V2] EHCI: Exynos: Add fdt support

[Simon Glass]

Hi Marek,

On Thu, Dec 6, 2012 at 9:39 AM, Marek Vasut <marex@denx.de> wrote:
> Dear Rajeshwari Shinde,
>
>> Adding fdt support to ehci-exynos in order to parse
>> register base addresses from the device node.
>>
>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>> Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com>
>> ---
>> Chnages in V2:
>>       - Removed checkpatch errors.
>>  drivers/usb/host/ehci-exynos.c |   59
>> ++++++++++++++++++++++++++++++++++++--- 1 files changed, 54 insertions(+),
>> 5 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-exynos.c
>> b/drivers/usb/host/ehci-exynos.c index 9f0ed06..f9189a5 100644
>> --- a/drivers/usb/host/ehci-exynos.c
>> +++ b/drivers/usb/host/ehci-exynos.c
>> @@ -21,6 +21,8 @@
>>   */
>>
>>  #include <common.h>
>> +#include <fdtdec.h>
>> +#include <libfdt.h>
>>  #include <usb.h>
>>  #include <asm/arch/cpu.h>
>>  #include <asm/arch/ehci.h>
>> @@ -28,6 +30,9 @@
>>  #include <asm/arch/power.h>
>>  #include "ehci.h"
>>
>> +/* Declare global data pointer */
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>>  /* Setup the EHCI host controller. */
>>  static void setup_usb_phy(struct exynos_usb_phy *usb)
>>  {
>> @@ -86,12 +91,39 @@ static void reset_usb_phy(struct exynos_usb_phy *usb)
>>   */
>>  int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor
>> **hcor) {
>> -     struct exynos_usb_phy *usb;
>> +     struct exynos_usb_phy *usb = NULL;
>> +     unsigned int *hcd = NULL;
>> +     unsigned int node;
>> +
>> +     node = fdtdec_next_compatible(gd->fdt_blob, 0,
>> +                                     COMPAT_SAMSUNG_EXYNOS_EHCI);
>> +     if (node <= 0) {
>> +             debug("EHCI: Can't get device tree node for ehci\n");
> [...]
>
> error output should be really puts() or printf() ...

Ick that bloats the code badly for an uncommon case. Would really
prefer to avoid this.

>
> You can also use errno.h instead of -1.

True, it might help debugging, although many times it is hard to map
the error onto a suitable number designed for Linux. This 'return -1'
is pretty common in U-Boot (generic error).

>
> Rest is good

Regards,
Simon

[U-Boot] [PATCH 1/3 V2] EHCI: Exynos: Add fdt support

[Marek Vasut]

Dear Simon Glass,

[...]

> > [...]
> > 
> > error output should be really puts() or printf() ...
> 
> Ick that bloats the code badly for an uncommon case. Would really
> prefer to avoid this.

What do you mean? Are you saying this debug() is correct and this is triggered 
often? How come?

> > You can also use errno.h instead of -1.
> 
> True, it might help debugging, although many times it is hard to map
> the error onto a suitable number designed for Linux. This 'return -1'
> is pretty common in U-Boot (generic error).

I know, but it'd be nice if this changed. Just a suggestion.

Best regards,
Marek Vasut

[U-Boot] [PATCH 1/3 V2] EHCI: Exynos: Add fdt support

[Simon Glass]

Hi Marek,

On Thu, Dec 6, 2012 at 10:02 AM, Marek Vasut <marex@denx.de> wrote:
> Dear Simon Glass,
>
> [...]
>
>> > [...]
>> >
>> > error output should be really puts() or printf() ...
>>
>> Ick that bloats the code badly for an uncommon case. Would really
>> prefer to avoid this.
>
> What do you mean? Are you saying this debug() is correct and this is triggered
> often? How come?

I mean that debug() in a driver does not generate any code unless
DEBUG is defined in that module. The way I do it is when I have a
problem in a module I define DEBUG there, which makes all the messages
work. But then in the normal case (when not debugging) the code size
is not bloated by messages.

So I much prefer debug() to printf() for uncommon messages in drivers, etc.

>
>> > You can also use errno.h instead of -1.
>>
>> True, it might help debugging, although many times it is hard to map
>> the error onto a suitable number designed for Linux. This 'return -1'
>> is pretty common in U-Boot (generic error).
>
> I know, but it'd be nice if this changed. Just a suggestion.

Yes agreed. I recently had a driver which could fail in about 12
different places (different stages of hardware init), so I just
created an enum for 12 errors. It wouldn't have been sensible in that
case to try and fail to map those onto the errno errors, but in many
cases (with fewer error conditions) it would be useful.

Regards,
Simon

>
> Best regards,
> Marek Vasut

[U-Boot] [PATCH 1/3 V2] EHCI: Exynos: Add fdt support

[Marek Vasut]

Dear Simon Glass,

> Hi Marek,
> 
> On Thu, Dec 6, 2012 at 10:02 AM, Marek Vasut <marex@denx.de> wrote:
> > Dear Simon Glass,
> > 
> > [...]
> > 
> >> > [...]
> >> > 
> >> > error output should be really puts() or printf() ...
> >> 
> >> Ick that bloats the code badly for an uncommon case. Would really
> >> prefer to avoid this.
> > 
> > What do you mean? Are you saying this debug() is correct and this is
> > triggered often? How come?
> 
> I mean that debug() in a driver does not generate any code unless
> DEBUG is defined in that module. The way I do it is when I have a
> problem in a module I define DEBUG there, which makes all the messages
> work. But then in the normal case (when not debugging) the code size
> is not bloated by messages.
> 
> So I much prefer debug() to printf() for uncommon messages in drivers, etc.

That's true ... and you're right if the FDT is correct, this will all work well. 
Ok, whichever way works for me.

> >> > You can also use errno.h instead of -1.
> >> 
> >> True, it might help debugging, although many times it is hard to map
> >> the error onto a suitable number designed for Linux. This 'return -1'
> >> is pretty common in U-Boot (generic error).
> > 
> > I know, but it'd be nice if this changed. Just a suggestion.
> 
> Yes agreed. I recently had a driver which could fail in about 12
> different places (different stages of hardware init), so I just
> created an enum for 12 errors. It wouldn't have been sensible in that
> case to try and fail to map those onto the errno errors, but in many
> cases (with fewer error conditions) it would be useful.

Yup

Best regards,
Marek Vasut