Anton Vorontsov wrote:
On Tue, Mar 25, 2008 at 10:42:11AM -0500, Kim Phillips wrote: [...]
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 avorontsov@ru.mvista.com 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 avorontsov@ru.mvista.com
board/freescale/mpc837xerdb/mpc837xerdb.c | 2 ++ include/fdt_support.h | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/board/freescale/mpc837xerdb/mpc837xerdb.c b/board/freescale/mpc837xerdb/mpc837xerdb.c index d091538..70f52da 100644 --- a/board/freescale/mpc837xerdb/mpc837xerdb.c +++ b/board/freescale/mpc837xerdb/mpc837xerdb.c @@ -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 */ diff --git a/include/fdt_support.h b/include/fdt_support.h index 7836f28..933ef41 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -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 */
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