On Wed, Sep 06, 2017 at 04:57:52PM +0200, Felix Brack wrote: > On 01.09.2017 17:21, Tom Rini wrote: > > On Thu, Aug 31, 2017 at 03:16:17PM +0200, Felix Brack wrote: > > > >> Boards using the single-register-pin-controller can configure all > >> pins by means of the device tree. This renders the implementation of > >> the two functions set_uart_mux_conf and set_mux_conf_regs obsolete. > >> Using the weak attribute for these two function declarations allows > >> the omission of the respective definitions. > >> > >> Signed-off-by: Felix Brack <f...@ltec.ch> > >> --- > >> > >> arch/arm/include/asm/arch-am33xx/sys_proto.h | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/arch/arm/include/asm/arch-am33xx/sys_proto.h > >> b/arch/arm/include/asm/arch-am33xx/sys_proto.h > >> index 4e78aaf..e31c25c 100644 > >> --- a/arch/arm/include/asm/arch-am33xx/sys_proto.h > >> +++ b/arch/arm/include/asm/arch-am33xx/sys_proto.h > >> @@ -31,8 +31,8 @@ void enable_gpmc_cs_config(const u32 *gpmc_config, const > >> struct gpmc_cs *cs, u32 > >> u32 size); > >> int omap_nand_switch_ecc(uint32_t, uint32_t); > >> > >> -void set_uart_mux_conf(void); > >> -void set_mux_conf_regs(void); > >> +__weak void set_uart_mux_conf(void); > >> +__weak void set_mux_conf_regs(void); > >> void sdram_init(void); > >> u32 wait_on_value(u32, u32, void *, u32); > >> #ifdef CONFIG_NOR_BOOT > > > > This seems wrong in a few ways. First, this (and the matching ones for > > omap3, etc) should be full of externs instead and in that case __weak > > makes no sense. Then, on the technical side, what you're describing > > isn't quite true in that you're likely relying on having the ROM to have > > setup the 'normal' UART still for you, as it so often does, I believe. > > Or in the case I'm wrong, then yes, you do get UART to work once we have > > parsed the DT, but the "usual" case is that we want UART and thus > > printf/etc to be available asap in case of errors, so it's still not a > > recommended way to go. Thanks! > > > > Hi Tom, > > Could you please explain what you mean by "... should be full of externs > instead and in that case __weak makes no sense"?
I mean it's a header file. We should only be declaring function prototypes, and thus using 'extern' here. It's not the right place to have an inline weak function. > The pins of the UART I use are not configured by the ROM, it is the pin > controller driver configuring them. > You are of course right in that UART output is not available before the > pin controller driver has executed correctly. In my case the UART is > available in 'spl_board_init()'. I know that many things can go wrong > before this and therefore configuring UART pins in 'set_uart_mux_conf()' > while using 'CONFIG_DEBUG_UART' is fine. In this context the word > 'obsolete' may be wrong. The idea behind the patch is to not force the > implementation of those two (potentially empty) functions. I'm open to discussing making these functions be not required but then the weak empty function should be under arch/arm/mach-omap2/ somewhere. Thanks! -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
