[U-Boot] [PATCH] arm: am33xx: Make pin multiplexing functions optional

[Tom Rini]

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 <fb at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170907/58fe9e7f/attachment.sig>