83xx/fdt_support: let user specifiy FSL USB Dual-Role controller role

PATCH New Anton Vorontsov 3 participants 11 messages Mar 14, 2008 – Mar 26, 2008

Share & tools

Anton Vorontsov

author

2008-03-14 20:20 UTC #0 lore raw

PATCH 6/7 83xx/fdt_support: let user specifiy FSL USB Dual-Role controller role

Linux understands "host" (default), "peripheral" and "otg" (broken). Though, U-Boot doesn't restrict dr_mode variable to these values (think of renames in future).

Signed-off-by Anton Vorontsov <[email protected]>

---
 board/freescale/mpc837xerdb/mpc837xerdb.c |    1 +
 common/fdt_support.c                      |   25 +++++++++++++++++++++++++
 include/configs/MPC837XERDB.h             |    1 +
 include/fdt_support.h                     |    6 ++++++
 4 files changed, 33 insertions(+), 0 deletions(-)

board/freescale/mpc837xerdb/mpc837xerdb.c +1 -0

index 992c9cf..0ccac7b 100644
@@ -148,5 +148,6 @@ void ft_board_setup(void *blob, bd_t *bd)
 	ft_pci_setup(blob, bd);
 #endif
 	ft_cpu_setup(blob, bd);
+	fdt_fixup_dr_usb(blob, bd);
 }
 #endif /* CONFIG_OF_BOARD_SETUP */

common/fdt_support.c +25 -0

index 69eb667..10b66fc 100644
@@ -615,3 +615,28 @@ void fdt_fixup_ethernet(void *fdt, bd_t *bd)
 	}
 }
 #endif
+
+#ifdef CONFIG_HAS_FSL_DR_USB
+void fdt_fixup_dr_usb(void *blob, bd_t *bd)
+{
+	char *mode;
+	const char *compat = "fsl-usb2-dr";
+	const char *prop = "dr_mode";
+	int node_offset;
+	int err;
+
+	mode = getenv("usb_dr_mode");
+	if (!mode)
+		return;
+
+	node_offset = fdt_node_offset_by_compatible(blob, 0, compat);
+	if (node_offset < 0)
+		printf("WARNING: could not find compatible node %s: %s.\n",
+			compat, fdt_strerror(node_offset));
+
+	err = fdt_setprop(blob, node_offset, prop, mode, strlen(mode) + 1);
+	if (err < 0)
+		printf("WARNING: could not set %s for %s: %s.\n",
+		       prop, compat, fdt_strerror(err));
+}
+#endif /* CONFIG_HAS_FSL_DR_USB */

include/configs/MPC837XERDB.h +1 -0

index 033a5fb..2287fbc 100644
@@ -576,6 +576,7 @@
  */
 #define CONFIG_ENV_OVERWRITE
 
+#define CONFIG_HAS_FSL_DR_USB
 #define CONFIG_HAS_ETH0				/* add support for "ethaddr" */
 #define CONFIG_ETHADDR	00:04:9f:ef:04:01
 #define CONFIG_HAS_ETH1				/* add support for "eth1addr" */

include/fdt_support.h +6 -0

index 7836f28..c10de8a 100644
@@ -50,6 +50,12 @@ int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
 			 const void *val, int len, int create);
 void fdt_fixup_qe_firmware(void *fdt);
 
+#ifdef CONFIG_HAS_FSL_DR_USB
+void fdt_fixup_dr_usb(void *blob, bd_t *bd);
+#else
+static inline void fdt_fixup_dr_usb(void *blob, bd_t *bd) {}
+#endif /* CONFIG_HAS_FSL_DR_USB */
+
 #ifdef CONFIG_OF_HAS_UBOOT_ENV
 int fdt_env(void *fdt);
 #endif

Signature · 1 line

1.5.2.2

Kim Phillips

2008-03-20 01:35 UTC #1 lore raw

PATCH 6/7 83xx/fdt_support: let user specifiy FSL USB Dual-Role controller role

Quoted 5 lines · On Fri, 14 Mar 2008 23:20:18 +0300 Anton Vorontsov <[email protected]>…

include/fdt_support.h +0 -0
index 7836f28..c10de8a 100644

include/fdt_support.h int fdt_find_and_setprop(void *fdt, const char *node, const char *prop, · line 58

Quoted 9 lines

include/fdt_support.h +6 -0

@@ -50,6 +50,12 @@ int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
 			 const void *val, int len, int create);
 void fdt_fixup_qe_firmware(void *fdt);
 
+#ifdef CONFIG_HAS_FSL_DR_USB
+void fdt_fixup_dr_usb(void *blob, bd_t *bd);
+#else
+static inline void fdt_fixup_dr_usb(void *blob, bd_t *bd) {}
+#endif /* CONFIG_HAS_FSL_DR_USB */
+

this looks like a prime candidate for a weak function (which would also eliminate the need for the new CONFIG_HAS_FSL_DR_USB introduced here).

Kim

Anton Vorontsov

author

2008-03-24 14:44 UTC #2 lore raw

PATCH 6/7 83xx/fdt_support: let user specifiy FSL USB Dual-Role controller role

Quoted 12 lines · Kim Phillips

Quoted 8 lines · On Fri, 14 Mar 2008 23:20:18 +0300 Anton Vorontsov <[email protected]>…
include/fdt_support.h +0 -0
index 7836f28..c10de8a 100644
@@ -50,6 +50,12 @@ int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
 			 const void *val, int len, int create);
 void fdt_fixup_qe_firmware(void *fdt);
 

Quoted 9 lines

include/fdt_support.h int fdt_find_and_setprop(void *fdt, const char *node, const char *prop, · line 58
Quoted 6 lines
include/fdt_support.h +6 -0
+#ifdef CONFIG_HAS_FSL_DR_USB
+void fdt_fixup_dr_usb(void *blob, bd_t *bd);
+#else
+static inline void fdt_fixup_dr_usb(void *blob, bd_t *bd) {}
+#endif /* CONFIG_HAS_FSL_DR_USB */
+
this looks like a prime candidate for a weak function (which would also eliminate the need for the new CONFIG_HAS_FSL_DR_USB introduced here).

Are you sure it's supposed to work like this?.. I don't see the linker dropping unused weak symbols with the patch below.

board/freescale/mpc837xerdb/mpc837xerdb.c +2 -0

index d091538..70f52da 100644
@@ -13,6 +13,7 @@
  */
 
 #include <common.h>
+#include <fdt_support.h>
 #include <i2c.h>
 #include <asm/io.h>
 #include <asm/fsl_serdes.h>

@@ -197,5 +198,6 @@ void ft_board_setup(void *blob, bd_t *bd)
 	ft_pci_setup(blob, bd);
 #endif
 	ft_cpu_setup(blob, bd);
+	/* fdt_fixup_dr_usb(blob); */
 }
 #endif /* CONFIG_OF_BOARD_SETUP */

common/fdt_support.c +23 -0

index 69eb667..22a2d99 100644
@@ -615,3 +615,26 @@ void fdt_fixup_ethernet(void *fdt, bd_t *bd)
 	}
 }
 #endif
+
+void __attribute__((weak)) fdt_fixup_dr_usb(void *fdt)
+{
+	char *mode;
+	const char *compat = "fsl-usb2-dr";
+	const char *prop = "dr_mode";
+	int node_offset;
+	int err;
+
+	mode = getenv("usb_dr_mode");
+	if (!mode)
+		return;
+
+	node_offset = fdt_node_offset_by_compatible(fdt, 0, compat);
+	if (node_offset < 0)
+		printf("WARNING: could not find compatible node %s: %s.\n",
+			compat, fdt_strerror(node_offset));
+
+	err = fdt_setprop(fdt, node_offset, prop, mode, strlen(mode) + 1);
+	if (err < 0)
+		printf("WARNING: could not set %s for %s: %s.\n",
+		       prop, compat, fdt_strerror(err));
+}

include/fdt_support.h +1 -0

index 7836f28..9e980b8 100644
@@ -49,6 +49,7 @@ void fdt_fixup_ethernet(void *fdt, bd_t *bd);
 int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
 			 const void *val, int len, int create);
 void fdt_fixup_qe_firmware(void *fdt);
+void __attribute__((weak)) fdt_fixup_dr_usb(void *fdt);
 
 #ifdef CONFIG_OF_HAS_UBOOT_ENV
 int fdt_env(void *fdt);

Signature · 1 line

1.5.2.2

Kim Phillips

2008-03-24 23:00 UTC #3 lore raw

PATCH 6/7 83xx/fdt_support: let user specifiy FSL USB Dual-Role controller role

Quoted 23 lines · On Mon, 24 Mar 2008 17:44:19 +0300 Anton Vorontsov <[email protected]>…

Quoted 12 lines · Kim Phillips
Quoted 8 lines · On Fri, 14 Mar 2008 23:20:18 +0300 Anton Vorontsov <[email protected]>…
include/fdt_support.h +0 -0
index 7836f28..c10de8a 100644
@@ -50,6 +50,12 @@ int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
 			 const void *val, int len, int create);
 void fdt_fixup_qe_firmware(void *fdt);
 
Quoted 9 lines
include/fdt_support.h int fdt_find_and_setprop(void *fdt, const char *node, const char *prop, · line 58
Quoted 6 lines
include/fdt_support.h +6 -0
+#ifdef CONFIG_HAS_FSL_DR_USB
+void fdt_fixup_dr_usb(void *blob, bd_t *bd);
+#else
+static inline void fdt_fixup_dr_usb(void *blob, bd_t *bd) {}
+#endif /* CONFIG_HAS_FSL_DR_USB */
+
this looks like a prime candidate for a weak function (which would also eliminate the need for the new CONFIG_HAS_FSL_DR_USB introduced here).

Quoted 2 lines

Are you sure it's supposed to work like this?.. I don't see the linker dropping unused weak symbols with the patch below.

with something like the following I get 24 bytes more text in fdt_support.o, and 200 total bytes added 83xx-wide:

board/freescale/mpc837xerdb/mpc837xerdb.c +3 -0

index a3b20b8..e8d130c 100644
@@ -13,6 +13,7 @@
  */
 
 #include <common.h>
+#include <fdt_support.h>
 #include <i2c.h>
 #include <asm/io.h>
 #include <spd_sdram.h>

@@ -153,6 +154,7 @@ int misc_init_r(void)
 }
 
 #if defined(CONFIG_OF_BOARD_SETUP)
+extern void fdt_fixup_dr_usb(void *);
 
 void ft_board_setup(void *blob, bd_t *bd)
 {

@@ -160,5 +162,6 @@ void ft_board_setup(void *blob, bd_t *bd)
 	ft_pci_setup(blob, bd);
 #endif
 	ft_cpu_setup(blob, bd);
+	fdt_fixup_dr_usb(blob);
 }
 #endif /* CONFIG_OF_BOARD_SETUP */

common/fdt_support.c +7 -0

index 69eb667..355b741 100644
@@ -615,3 +615,10 @@ void fdt_fixup_ethernet(void *fdt, bd_t *bd)
 	}
 }
 #endif
+
+void __fdt_fixup_dr_usb(void *fdt)
+{
+	/* add your own */
+}
+void fdt_fixup_dr_usb(void *fdt) __attribute__((weak,
+	alias("__fdt_fixup_dr_usb")));

cpu/mpc83xx/fdt.c +23 -0

index 6f55932..3224286 100644
@@ -68,4 +68,27 @@ void ft_cpu_setup(void *blob, bd_t *bd)
 
 	fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
 }
+
+void fdt_fixup_dr_usb(void *fdt)
+{
+	char *mode;
+	const char *compat = "fsl-usb2-dr";
+	const char *prop = "dr_mode";
+	int node_offset;
+	int err;
+
+	mode = getenv("usb_dr_mode");
+	if (!mode)
+		return;
+
+	node_offset = fdt_node_offset_by_compatible(fdt, 0, compat);
+	if (node_offset < 0)
+		printf("WARNING: could not find compatible node %s: %s.\n",
+			compat, fdt_strerror(node_offset));
+
+	err = fdt_setprop(fdt, node_offset, prop, mode, strlen(mode) + 1);
+	if (err < 0)
+		printf("WARNING: could not set %s for %s: %s.\n",
+		       prop, compat, fdt_strerror(err));
+}
 #endif /* CONFIG_OF_LIBFDT */

but the QE based 83xx SoCs don't use this fixup. So while CONFIG_HAS_FSL_DR_USB gives you more control, I doubt saving 200 bytes justifies the new config either. Either way really, but I tend to be against any new CONFIG_s.

Kim

Anton Vorontsov

author

2008-03-25 13:48 UTC #4 lore raw

PATCH 6/7 83xx/fdt_support: let user specifiy FSL USB Dual-Role controller role

Quoted 32 lines · Kim Phillips

Quoted 23 lines · On Mon, 24 Mar 2008 17:44:19 +0300 Anton Vorontsov <[email protected]>…
Quoted 12 lines · Kim Phillips
Quoted 8 lines · On Fri, 14 Mar 2008 23:20:18 +0300 Anton Vorontsov <[email protected]>…
include/fdt_support.h +0 -0
index 7836f28..c10de8a 100644
@@ -50,6 +50,12 @@ int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
 			 const void *val, int len, int create);
 void fdt_fixup_qe_firmware(void *fdt);
 
Quoted 9 lines
include/fdt_support.h int fdt_find_and_setprop(void *fdt, const char *node, const char *prop, · line 58
Quoted 6 lines
include/fdt_support.h +6 -0
+#ifdef CONFIG_HAS_FSL_DR_USB
+void fdt_fixup_dr_usb(void *blob, bd_t *bd);
+#else
+static inline void fdt_fixup_dr_usb(void *blob, bd_t *bd) {}
+#endif /* CONFIG_HAS_FSL_DR_USB */
+
this looks like a prime candidate for a weak function (which would also eliminate the need for the new CONFIG_HAS_FSL_DR_USB introduced here).
Quoted 2 lines
Are you sure it's supposed to work like this?.. I don't see the linker dropping unused weak symbols with the patch below.
with something like the following I get 24 bytes more text in fdt_support.o, and 200 total bytes added 83xx-wide:

[...]

Quoted 4 lines

but the QE based 83xx SoCs don't use this fixup. So while CONFIG_HAS_FSL_DR_USB gives you more control, I doubt saving 200 bytes justifies the new config either. Either way really, but I tend to be against any new CONFIG_s.

Hm... maybe then it's better to place fdt_fixup_usb_dr function inside the fdt_support.h, marking it with static inline -- it's used just once per board anyway..? Then neither space wasted nor the new configs introduced.

Signature · 3 lines

Anton Vorontsov
email: cboumailru at gmail.com
irc://irc.freenode.net/bd2

Kim Phillips

2008-03-25 15:42 UTC #5 lore raw

PATCH 6/7 83xx/fdt_support: let user specifiy FSL USB Dual-Role controller role

Quoted 39 lines · On Tue, 25 Mar 2008 16:48:54 +0300 Anton Vorontsov <[email protected]>…

Quoted 32 lines · Kim Phillips
Quoted 23 lines · On Mon, 24 Mar 2008 17:44:19 +0300 Anton Vorontsov <[email protected]>…
Quoted 12 lines · Kim Phillips
Quoted 8 lines · On Fri, 14 Mar 2008 23:20:18 +0300 Anton Vorontsov <[email protected]>…
include/fdt_support.h +0 -0
index 7836f28..c10de8a 100644
@@ -50,6 +50,12 @@ int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
 			 const void *val, int len, int create);
 void fdt_fixup_qe_firmware(void *fdt);
 
Quoted 9 lines
include/fdt_support.h int fdt_find_and_setprop(void *fdt, const char *node, const char *prop, · line 58
Quoted 6 lines
include/fdt_support.h +6 -0
+#ifdef CONFIG_HAS_FSL_DR_USB
+void fdt_fixup_dr_usb(void *blob, bd_t *bd);
+#else
+static inline void fdt_fixup_dr_usb(void *blob, bd_t *bd) {}
+#endif /* CONFIG_HAS_FSL_DR_USB */
+
this looks like a prime candidate for a weak function (which would also eliminate the need for the new CONFIG_HAS_FSL_DR_USB introduced here).
Quoted 2 lines
Are you sure it's supposed to work like this?.. I don't see the linker dropping unused weak symbols with the patch below.
with something like the following I get 24 bytes more text in fdt_support.o, and 200 total bytes added 83xx-wide:
[...]
Quoted 4 lines
but the QE based 83xx SoCs don't use this fixup. So while CONFIG_HAS_FSL_DR_USB gives you more control, I doubt saving 200 bytes justifies the new config either. Either way really, but I tend to be against any new CONFIG_s.

Quoted 4 lines

Hm... maybe then it's better to place fdt_fixup_usb_dr function inside the fdt_support.h, marking it with static inline -- it's used just once per board anyway..? Then neither space wasted nor the new configs introduced.

I'd like to see that. Do you want to send a patch?

Kim

Anton Vorontsov

author

2008-03-25 16:06 UTC #6 lore raw

PATCH 6/7 83xx/fdt_support: let user specifiy FSL USB Dual-Role controller role

On Tue, Mar 25, 2008 at 10:42:11AM -0500, Kim Phillips wrote:
[...]

Quoted 11 lines

Quoted 9 lines
Quoted 4 lines
but the QE based 83xx SoCs don't use this fixup. So while CONFIG_HAS_FSL_DR_USB gives you more control, I doubt saving 200 bytes justifies the new config either. Either way really, but I tend to be against any new CONFIG_s.
Hm... maybe then it's better to place fdt_fixup_usb_dr function inside the fdt_support.h, marking it with static inline -- it's used just once per board anyway..? Then neither space wasted nor the new configs introduced.
I'd like to see that. Do you want to send a patch?

Sure, here it is.

- - - -
From: Anton Vorontsov <[email protected]>
Subject: mpc83xx/fdt_support: let user specifiy FSL USB Dual-Role controller role

Linux understands "host" (default), "peripheral" and "otg" (broken). Though, U-Boot doesn't restrict dr_mode variable to these values (think of renames in future).

Signed-off-by Anton Vorontsov <[email protected]>

---
 board/freescale/mpc837xerdb/mpc837xerdb.c |    2 ++
 include/fdt_support.h                     |   24 ++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 0 deletions(-)

board/freescale/mpc837xerdb/mpc837xerdb.c +2 -0

index d091538..70f52da 100644
@@ -13,6 +13,7 @@
  */
 
 #include <common.h>
+#include <fdt_support.h>
 #include <i2c.h>
 #include <asm/io.h>
 #include <asm/fsl_serdes.h>

@@ -197,5 +198,6 @@ void ft_board_setup(void *blob, bd_t *bd)
 	ft_pci_setup(blob, bd);
 #endif
 	ft_cpu_setup(blob, bd);
+	fdt_fixup_dr_usb(blob);
 }
 #endif /* CONFIG_OF_BOARD_SETUP */

include/fdt_support.h +24 -0

index 7836f28..933ef41 100644
@@ -27,6 +27,7 @@
 #ifdef CONFIG_OF_LIBFDT
 
 #include <fdt.h>
+#include <libfdt.h>
 
 int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force);
 void do_fixup_by_path(void *fdt, const char *path, const char *prop,

@@ -64,5 +65,28 @@ void ft_cpu_setup(void *blob, bd_t *bd);
 void ft_pci_setup(void *blob, bd_t *bd);
 #endif
 
+static inline void fdt_fixup_dr_usb(void *fdt)
+{
+	char *mode;
+	const char *compat = "fsl-usb2-dr";
+	const char *prop = "dr_mode";
+	int node_offset;
+	int err;
+
+	mode = getenv("usb_dr_mode");
+	if (!mode)
+		return;
+
+	node_offset = fdt_node_offset_by_compatible(fdt, 0, compat);
+	if (node_offset < 0)
+		printf("WARNING: could not find compatible node %s: %s.\n",
+			compat, fdt_strerror(node_offset));
+
+	err = fdt_setprop(fdt, node_offset, prop, mode, strlen(mode) + 1);
+	if (err < 0)
+		printf("WARNING: could not set %s for %s: %s.\n",
+		       prop, compat, fdt_strerror(err));
+}
+
 #endif /* ifdef CONFIG_OF_LIBFDT */
 #endif /* ifndef __FDT_SUPPORT_H */

Signature · 1 line

1.5.2.2

Jerry Van Baren

2008-03-25 16:32 UTC #7 lore raw

PATCH 6/7 83xx/fdt_support: let user specifiy FSL USB Dual-Role controller role

Quoted 78 lines · Anton Vorontsov

On Tue, Mar 25, 2008 at 10:42:11AM -0500, Kim Phillips wrote:
[...]
Quoted 9 lines
Quoted 8 lines
Quoted 4 lines
but the QE based 83xx SoCs don't use this fixup. So while CONFIG_HAS_FSL_DR_USB gives you more control, I doubt saving 200 bytes justifies the new config either. Either way really, but I tend to be against any new CONFIG_s.
Hm... maybe then it's better to place fdt_fixup_usb_dr function inside the fdt_support.h, marking it with static inline -- it's used just once per board anyway..? Then neither space wasted nor the new configs introduced.
I'd like to see that. Do you want to send a patch?
Sure, here it is.
- - - -
From: Anton Vorontsov <[email protected]>
Subject: mpc83xx/fdt_support: let user specifiy FSL USB Dual-Role controller role
Linux understands "host" (default), "peripheral" and "otg" (broken). Though, U-Boot doesn't restrict dr_mode variable to these values (think of renames in future).
Signed-off-by Anton Vorontsov <[email protected]>
---
 board/freescale/mpc837xerdb/mpc837xerdb.c |    2 ++
 include/fdt_support.h                     |   24 ++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 0 deletions(-)
board/freescale/mpc837xerdb/mpc837xerdb.c +2 -0
index d091538..70f52da 100644
@@ -13,6 +13,7 @@
  */
 
 #include <common.h>
+#include <fdt_support.h>
 #include <i2c.h>
 #include <asm/io.h>
 #include <asm/fsl_serdes.h>
@@ -197,5 +198,6 @@ void ft_board_setup(void *blob, bd_t *bd)
 	ft_pci_setup(blob, bd);
 #endif
 	ft_cpu_setup(blob, bd);
+	fdt_fixup_dr_usb(blob);
 }
 #endif /* CONFIG_OF_BOARD_SETUP */
include/fdt_support.h +16 -0
index 7836f28..933ef41 100644
@@ -27,6 +27,7 @@
 #ifdef CONFIG_OF_LIBFDT
 
 #include <fdt.h>
+#include <libfdt.h>
 
 int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force);
 void do_fixup_by_path(void *fdt, const char *path, const char *prop,
@@ -64,5 +65,28 @@ void ft_cpu_setup(void *blob, bd_t *bd);
 void ft_pci_setup(void *blob, bd_t *bd);
 #endif
 
+static inline void fdt_fixup_dr_usb(void *fdt)
+{
+	char *mode;
+	const char *compat = "fsl-usb2-dr";
+	const char *prop = "dr_mode";
+	int node_offset;
+	int err;
+
+	mode = getenv("usb_dr_mode");
+	if (!mode)
+		return;
+
+	node_offset = fdt_node_offset_by_compatible(fdt, 0, compat);
+	if (node_offset < 0)
+		printf("WARNING: could not find compatible node %s: %s.\n",

include/fdt_support.h void ft_cpu_setup(void *blob, bd_t *bd); · line 92

Quoted 10 lines

include/fdt_support.h +8 -0

+			compat, fdt_strerror(node_offset));
+
+	err = fdt_setprop(fdt, node_offset, prop, mode, strlen(mode) + 1);
+	if (err < 0)
+		printf("WARNING: could not set %s for %s: %s.\n",
+		       prop, compat, fdt_strerror(err));
+}
+
 #endif /* ifdef CONFIG_OF_LIBFDT */
 #endif /* ifndef __FDT_SUPPORT_H */

I have reservations on this:
1) We are adding code to a header file (arguably unavoidable, saving grace is that it is static inline so it presumably doesn't bloat boards that don't use it).

2) This is CPU-specific in a general purpose fdt_support.h header file.

Is anybody else's tail tingling[1]?

Best regards,
gvb

[1] http://en.wikipedia.org/wiki/Verne_(Over_the_Hedge)

Kim Phillips

2008-03-25 16:33 UTC #8 lore raw

PATCH 6/7 83xx/fdt_support: let user specifiy FSL USB Dual-Role controller role

In reply to Anton Vorontsov

Quoted 80 lines · On Tue, 25 Mar 2008 19:06:34 +0300 Anton Vorontsov <[email protected]>…

On Tue, Mar 25, 2008 at 10:42:11AM -0500, Kim Phillips wrote:
[...]
Quoted 11 lines
Quoted 9 lines
Quoted 4 lines
but the QE based 83xx SoCs don't use this fixup. So while CONFIG_HAS_FSL_DR_USB gives you more control, I doubt saving 200 bytes justifies the new config either. Either way really, but I tend to be against any new CONFIG_s.
Hm... maybe then it's better to place fdt_fixup_usb_dr function inside the fdt_support.h, marking it with static inline -- it's used just once per board anyway..? Then neither space wasted nor the new configs introduced.
I'd like to see that. Do you want to send a patch?
Sure, here it is.
- - - -
From: Anton Vorontsov <[email protected]>
Subject: mpc83xx/fdt_support: let user specifiy FSL USB Dual-Role controller role
Linux understands "host" (default), "peripheral" and "otg" (broken). Though, U-Boot doesn't restrict dr_mode variable to these values (think of renames in future).
Signed-off-by Anton Vorontsov <[email protected]>
---
 board/freescale/mpc837xerdb/mpc837xerdb.c |    2 ++
 include/fdt_support.h                     |   24 ++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 0 deletions(-)
board/freescale/mpc837xerdb/mpc837xerdb.c +2 -0
index d091538..70f52da 100644
@@ -13,6 +13,7 @@
  */
 
 #include <common.h>
+#include <fdt_support.h>
 #include <i2c.h>
 #include <asm/io.h>
 #include <asm/fsl_serdes.h>
@@ -197,5 +198,6 @@ void ft_board_setup(void *blob, bd_t *bd)
 	ft_pci_setup(blob, bd);
 #endif
 	ft_cpu_setup(blob, bd);
+	fdt_fixup_dr_usb(blob);
 }
 #endif /* CONFIG_OF_BOARD_SETUP */
include/fdt_support.h +16 -0
index 7836f28..933ef41 100644
@@ -27,6 +27,7 @@
 #ifdef CONFIG_OF_LIBFDT
 
 #include <fdt.h>
+#include <libfdt.h>
 
 int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force);
 void do_fixup_by_path(void *fdt, const char *path, const char *prop,
@@ -64,5 +65,28 @@ void ft_cpu_setup(void *blob, bd_t *bd);
 void ft_pci_setup(void *blob, bd_t *bd);
 #endif
 
+static inline void fdt_fixup_dr_usb(void *fdt)
+{
+	char *mode;
+	const char *compat = "fsl-usb2-dr";
+	const char *prop = "dr_mode";
+	int node_offset;
+	int err;
+
+	mode = getenv("usb_dr_mode");
+	if (!mode)
+		return;
+
+	node_offset = fdt_node_offset_by_compatible(fdt, 0, compat);
+	if (node_offset < 0)
+		printf("WARNING: could not find compatible node %s: %s.\n",

include/fdt_support.h void ft_cpu_setup(void *blob, bd_t *bd); · line 92

Quoted 10 lines

include/fdt_support.h +8 -0

+			compat, fdt_strerror(node_offset));
+
+	err = fdt_setprop(fdt, node_offset, prop, mode, strlen(mode) + 1);
+	if (err < 0)
+		printf("WARNING: could not set %s for %s: %s.\n",
+		       prop, compat, fdt_strerror(err));
+}
+
 #endif /* ifdef CONFIG_OF_LIBFDT */
 #endif /* ifndef __FDT_SUPPORT_H */

<sigh> yeah, that's too much code for a header file though.

I think your original patch was the more prudent approach, it follows existing practice and it fits with the assumption that there'll be a usb driver one day :). I'll put it in my queue and apply it unless anyone has further comments.

Kim

Jerry Van Baren

2008-03-25 16:35 UTC #9 lore raw

PATCH 6/7 83xx/fdt_support: let user specifiy FSL USB Dual-Role controller role

Kim Phillips wrote:

[snip]

Quoted 8 lines

<sigh> yeah, that's too much code for a header file though.
I think your original patch was the more prudent approach, it follows existing practice and it fits with the assumption that there'll be a usb driver one day :). I'll put it in my queue and apply it unless anyone has further comments.
Kim

Yup, Kim's tail was tingling too. :-D

Best regards,
gvb

Kim Phillips

Applied

2008-03-26 00:27 UTC #10 lore raw

PATCH 6/7 83xx/fdt_support: let user specifiy FSL USB Dual-Role controller role

In reply to Anton Vorontsov

Quoted 4 lines · On Fri, 14 Mar 2008 23:20:18 +0300 Anton Vorontsov <[email protected]>…

Linux understands "host" (default), "peripheral" and "otg" (broken). Though, U-Boot doesn't restrict dr_mode variable to these values (think of renames in future).

Quoted 8 lines

Signed-off-by Anton Vorontsov <[email protected]>

---
 board/freescale/mpc837xerdb/mpc837xerdb.c |    1 +
 common/fdt_support.c                      |   25 +++++++++++++++++++++++++
 include/configs/MPC837XERDB.h             |    1 +
 include/fdt_support.h                     |    6 ++++++
 4 files changed, 33 insertions(+), 0 deletions(-)

applied, thanks.

Kim