From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerry Van Baren Date: Tue, 25 Mar 2008 12:32:06 -0400 Subject: [U-Boot-Users] [PATCH 6/7] 83xx/fdt_support: let user specifiy FSL USB Dual-Role controller role In-Reply-To: <20080325160634.GA13440@localhost.localdomain> References: <20080314202018.GF22581@localhost.localdomain> <20080319203558.cdd4be6f.kim.phillips@freescale.com> <20080324144419.GA30190@localhost.localdomain> <20080324180049.73f2f60b.kim.phillips@freescale.com> <20080325134854.GA25951@localhost.localdomain> <20080325104211.dbc40a03.kim.phillips@freescale.com> <20080325160634.GA13440@localhost.localdomain> Message-ID: <47E92906.9080407@ge.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > 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 > --- > 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 > +#include > #include > #include > #include > @@ -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 > +#include > > 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 [1] http://en.wikipedia.org/wiki/Verne_(Over_the_Hedge)