[{"id":1761757,"web_url":"http://patchwork.ozlabs.org/comment/1761757/","msgid":"<20170901152128.GA17058@bill-the-cat>","list_archive_url":null,"date":"2017-09-01T15:21:28","subject":"Re: [U-Boot] [PATCH] arm: am33xx: Make pin multiplexing functions\n\toptional","submitter":{"id":65875,"url":"http://patchwork.ozlabs.org/api/people/65875/","name":"Tom Rini","email":"trini@konsulko.com"},"content":"On Thu, Aug 31, 2017 at 03:16:17PM +0200, Felix Brack wrote:\n\n> Boards using the single-register-pin-controller can configure all\n> pins by means of the device tree. This renders the implementation of\n> the two functions set_uart_mux_conf and set_mux_conf_regs obsolete.\n> Using the weak attribute for these two function declarations allows\n> the omission of the respective definitions.\n> \n> Signed-off-by: Felix Brack \n> ---\n> \n> arch/arm/include/asm/arch-am33xx/sys_proto.h | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/arch/arm/include/asm/arch-am33xx/sys_proto.h b/arch/arm/include/asm/arch-am33xx/sys_proto.h\n> index 4e78aaf..e31c25c 100644\n> --- a/arch/arm/include/asm/arch-am33xx/sys_proto.h\n> +++ b/arch/arm/include/asm/arch-am33xx/sys_proto.h\n> @@ -31,8 +31,8 @@ void enable_gpmc_cs_config(const u32 *gpmc_config, const struct gpmc_cs *cs, u32\n> \t\t\tu32 size);\n> int omap_nand_switch_ecc(uint32_t, uint32_t);\n> \n> -void set_uart_mux_conf(void);\n> -void set_mux_conf_regs(void);\n> +__weak void set_uart_mux_conf(void);\n> +__weak void set_mux_conf_regs(void);\n> void sdram_init(void);\n> u32 wait_on_value(u32, u32, void *, u32);\n> #ifdef CONFIG_NOR_BOOT\n\nThis seems wrong in a few ways. First, this (and the matching ones for\nomap3, etc) should be full of externs instead and in that case __weak\nmakes no sense. Then, on the technical side, what you're describing\nisn't quite true in that you're likely relying on having the ROM to have\nsetup the 'normal' UART still for you, as it so often does, I believe.\nOr in the case I'm wrong, then yes, you do get UART to work once we have\nparsed the DT, but the \"usual\" case is that we want UART and thus\nprintf/etc to be available asap in case of errors, so it's still not a\nrecommended way to go. Thanks!","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=konsulko.com header.i=@konsulko.com\n\theader.b=\"eWA6ZzUb\"; dkim-atps=neutral"],"Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xkNHf580Rz9t1t\n\tfor ;\n\tSat, 2 Sep 2017 01:21:18 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid 6A33FC21EDC; Fri, 1 Sep 2017 15:21:11 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 43440C21D93;\n\tFri, 1 Sep 2017 15:21:09 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 48107C21D93; Fri, 1 Sep 2017 15:21:08 +0000 (UTC)","from mail-yw0-f174.google.com (mail-yw0-f174.google.com\n\t[209.85.161.174])\n\tby lists.denx.de (Postfix) with ESMTPS id B25A9C21C3C\n\tfor ; Fri, 1 Sep 2017 15:21:07 +0000 (UTC)","by mail-yw0-f174.google.com with SMTP id s187so2575871ywf.2\n\tfor ; Fri, 01 Sep 2017 08:21:07 -0700 (PDT)","from bill-the-cat ([2606:a000:140b:dc:8d41:cbff:603f:b0f2])\n\tby smtp.gmail.com with ESMTPSA id\n\tu2sm143621ywi.72.2017.09.01.08.21.05\n\t(version=TLS1_2 cipher=AES128-SHA bits=128/128);\n\tFri, 01 Sep 2017 08:21:05 -0700 (PDT)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-0.0 required=5.0 tests=RCVD_IN_DNSWL_NONE,\n\tRCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,\n\tT_DKIM_INVALID autolearn=unavailable\n\tautolearn_force=no version=3.4.0","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com;\n\ts=google; \n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=eiUwb240L/FFpZ5/5/a74I0jnFeB4DtO5UxpwOjw56g=;\n\tb=eWA6ZzUbO3vk7I+7OChVavYlTgMV6q6XZdOQwUqgsMlGNsndWPXKY+b1VBJGXrNxb8\n\t0TbJ5oVLoAJ14Vz8XjcXjMeCI1vKEkiNAjDu9QwvOKx9kCVwj8/VyTlrNRj8gGHHdpzc\n\t4mkz9dC+Sg6KyoItNbrPgJvxSczejZWeDW754=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=eiUwb240L/FFpZ5/5/a74I0jnFeB4DtO5UxpwOjw56g=;\n\tb=BJSF6tzslcpu+5X1I/N85Qq2WCmLXPSjsqO5OHLifMQpvYe6waXybQVHYAw61MrRCU\n\tggdJh6eTCcN+KwlJSk11pngyYtN5bUhbBydygpq+DIgvKXzTR2vr0VParD+cIWuJVbJu\n\tPXS8jNhR6xPhG/99LdxA7GLtiJ4z/ByV5zJ+o028qckkv4hjZLcY3CAJcyY+Df6y9Mox\n\tk6bfSMF2+ymylNB06wUF3QGM9XOuSpdc3v2SCFi1s2fKrm4t/1DZ0KVUY5njy39kfJO+\n\tfPvssTl8Gn7tDiEgWW6TEVG/xvxvqAF+FmgBLspKTr67VXaD180Fa1VhRpzgmHN95Tbc\n\ttirg==","X-Gm-Message-State":"AHPjjUhUAK1NPuO21qkyRHfig1HkqTSQ16WXSiD330D083cNVezD95SR\n\tMiKQ6QR+8KVAaU5c","X-Google-Smtp-Source":"ADKCNb7u9XXirz+WyNiavQp/+WxqeS1UTzM1Tv5ItgtWKsRP4lmmgPXsBO6/yVypnu2njGw2u1xwwA==","X-Received":"by 10.37.86.86 with SMTP id k83mr2003919ybb.69.1504279266154;\n\tFri, 01 Sep 2017 08:21:06 -0700 (PDT)","Date":"Fri, 1 Sep 2017 11:21:28 -0400","From":"Tom Rini ","To":"Felix Brack ","Message-ID":"<20170901152128.GA17058@bill-the-cat>","References":"<1504185377-461-1-git-send-email-fb@ltec.ch>","MIME-Version":"1.0","In-Reply-To":"<1504185377-461-1-git-send-email-fb@ltec.ch>","User-Agent":"Mutt/1.5.21 (2010-09-15)","Cc":"u-boot@lists.denx.de, Albert Aribaud ","Subject":"Re: [U-Boot] [PATCH] arm: am33xx: Make pin multiplexing functions\n\toptional","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"multipart/mixed;\n\tboundary=\"===============3364915045783750533==\"","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}},{"id":1764173,"web_url":"http://patchwork.ozlabs.org/comment/1764173/","msgid":"","list_archive_url":null,"date":"2017-09-06T14:57:52","subject":"Re: [U-Boot] [PATCH] arm: am33xx: Make pin multiplexing functions\n\toptional","submitter":{"id":70195,"url":"http://patchwork.ozlabs.org/api/people/70195/","name":"Felix Brack","email":"fb@ltec.ch"},"content":"On 01.09.2017 17:21, Tom Rini wrote:\n> On Thu, Aug 31, 2017 at 03:16:17PM +0200, Felix Brack wrote:\n> \n>> Boards using the single-register-pin-controller can configure all\n>> pins by means of the device tree. This renders the implementation of\n>> the two functions set_uart_mux_conf and set_mux_conf_regs obsolete.\n>> Using the weak attribute for these two function declarations allows\n>> the omission of the respective definitions.\n>>\n>> Signed-off-by: Felix Brack \n>> ---\n>>\n>> arch/arm/include/asm/arch-am33xx/sys_proto.h | 4 ++--\n>> 1 file changed, 2 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/arch/arm/include/asm/arch-am33xx/sys_proto.h b/arch/arm/include/asm/arch-am33xx/sys_proto.h\n>> index 4e78aaf..e31c25c 100644\n>> --- a/arch/arm/include/asm/arch-am33xx/sys_proto.h\n>> +++ b/arch/arm/include/asm/arch-am33xx/sys_proto.h\n>> @@ -31,8 +31,8 @@ void enable_gpmc_cs_config(const u32 *gpmc_config, const struct gpmc_cs *cs, u32\n>> \t\t\tu32 size);\n>> int omap_nand_switch_ecc(uint32_t, uint32_t);\n>> \n>> -void set_uart_mux_conf(void);\n>> -void set_mux_conf_regs(void);\n>> +__weak void set_uart_mux_conf(void);\n>> +__weak void set_mux_conf_regs(void);\n>> void sdram_init(void);\n>> u32 wait_on_value(u32, u32, void *, u32);\n>> #ifdef CONFIG_NOR_BOOT\n> \n> This seems wrong in a few ways. First, this (and the matching ones for\n> omap3, etc) should be full of externs instead and in that case __weak\n> makes no sense. Then, on the technical side, what you're describing\n> isn't quite true in that you're likely relying on having the ROM to have\n> setup the 'normal' UART still for you, as it so often does, I believe.\n> Or in the case I'm wrong, then yes, you do get UART to work once we have\n> parsed the DT, but the \"usual\" case is that we want UART and thus\n> printf/etc to be available asap in case of errors, so it's still not a\n> recommended way to go. Thanks!\n> \n\nHi Tom,\n\nCould you please explain what you mean by \"... should be full of externs\ninstead and in that case __weak makes no sense\"?\nThe pins of the UART I use are not configured by the ROM, it is the pin\ncontroller driver configuring them.\nYou are of course right in that UART output is not available before the\npin controller driver has executed correctly. In my case the UART is\navailable in 'spl_board_init()'. I know that many things can go wrong\nbefore this and therefore configuring UART pins in 'set_uart_mux_conf()'\nwhile using 'CONFIG_DEBUG_UART' is fine. In this context the word\n'obsolete' may be wrong. The idea behind the patch is to not force the\nimplementation of those two (potentially empty) functions.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xnRXT18R2z9s7C\n\tfor ;\n\tThu, 7 Sep 2017 00:58:00 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid 8267FC21EC3; Wed, 6 Sep 2017 14:57:57 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 15296C21E31;\n\tWed, 6 Sep 2017 14:57:55 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid EF1C7C21EB9; Wed, 6 Sep 2017 14:57:53 +0000 (UTC)","from mail.ltec.ch (mail.ltec.ch [95.143.48.181])\n\tby lists.denx.de (Postfix) with ESMTPS id 9107FC21DC4\n\tfor ; Wed, 6 Sep 2017 14:57:53 +0000 (UTC)","from nebula.ltec ([172.27.11.2] helo=[172.27.11.32])\n\tby mail.ltec.ch with esmtpsa\n\t(TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128)\n\t(Exim 4.84_2) (envelope-from )\n\tid 1dpbmh-0003Ix-GE; Wed, 06 Sep 2017 16:58:31 +0200"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-2.3 required=5.0 tests=RCVD_IN_DNSWL_MED\n\tautolearn=unavailable autolearn_force=no version=3.4.0","To":"Tom Rini ","References":"<1504185377-461-1-git-send-email-fb@ltec.ch>\n\t<20170901152128.GA17058@bill-the-cat>","From":"Felix Brack ","Message-ID":"","Date":"Wed, 6 Sep 2017 16:57:52 +0200","User-Agent":"Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170901152128.GA17058@bill-the-cat>","Content-Language":"en-US","Cc":"u-boot@lists.denx.de","Subject":"Re: [U-Boot] [PATCH] arm: am33xx: Make pin multiplexing functions\n\toptional","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}},{"id":1764780,"web_url":"http://patchwork.ozlabs.org/comment/1764780/","msgid":"<20170907151428.GS17058@bill-the-cat>","list_archive_url":null,"date":"2017-09-07T15:14:28","subject":"Re: [U-Boot] [PATCH] arm: am33xx: Make pin multiplexing functions\n\toptional","submitter":{"id":65875,"url":"http://patchwork.ozlabs.org/api/people/65875/","name":"Tom Rini","email":"trini@konsulko.com"},"content":"On Wed, Sep 06, 2017 at 04:57:52PM +0200, Felix Brack wrote:\n> On 01.09.2017 17:21, Tom Rini wrote:\n> > On Thu, Aug 31, 2017 at 03:16:17PM +0200, Felix Brack wrote:\n> > \n> >> Boards using the single-register-pin-controller can configure all\n> >> pins by means of the device tree. This renders the implementation of\n> >> the two functions set_uart_mux_conf and set_mux_conf_regs obsolete.\n> >> Using the weak attribute for these two function declarations allows\n> >> the omission of the respective definitions.\n> >>\n> >> Signed-off-by: Felix Brack \n> >> ---\n> >>\n> >> arch/arm/include/asm/arch-am33xx/sys_proto.h | 4 ++--\n> >> 1 file changed, 2 insertions(+), 2 deletions(-)\n> >>\n> >> diff --git a/arch/arm/include/asm/arch-am33xx/sys_proto.h b/arch/arm/include/asm/arch-am33xx/sys_proto.h\n> >> index 4e78aaf..e31c25c 100644\n> >> --- a/arch/arm/include/asm/arch-am33xx/sys_proto.h\n> >> +++ b/arch/arm/include/asm/arch-am33xx/sys_proto.h\n> >> @@ -31,8 +31,8 @@ void enable_gpmc_cs_config(const u32 *gpmc_config, const struct gpmc_cs *cs, u32\n> >> \t\t\tu32 size);\n> >> int omap_nand_switch_ecc(uint32_t, uint32_t);\n> >> \n> >> -void set_uart_mux_conf(void);\n> >> -void set_mux_conf_regs(void);\n> >> +__weak void set_uart_mux_conf(void);\n> >> +__weak void set_mux_conf_regs(void);\n> >> void sdram_init(void);\n> >> u32 wait_on_value(u32, u32, void *, u32);\n> >> #ifdef CONFIG_NOR_BOOT\n> > \n> > This seems wrong in a few ways. First, this (and the matching ones for\n> > omap3, etc) should be full of externs instead and in that case __weak\n> > makes no sense. Then, on the technical side, what you're describing\n> > isn't quite true in that you're likely relying on having the ROM to have\n> > setup the 'normal' UART still for you, as it so often does, I believe.\n> > Or in the case I'm wrong, then yes, you do get UART to work once we have\n> > parsed the DT, but the \"usual\" case is that we want UART and thus\n> > printf/etc to be available asap in case of errors, so it's still not a\n> > recommended way to go. Thanks!\n> > \n> \n> Hi Tom,\n> \n> Could you please explain what you mean by \"... should be full of externs\n> instead and in that case __weak makes no sense\"?\n\nI mean it's a header file. We should only be declaring function\nprototypes, and thus using 'extern' here. It's not the right place to\nhave an inline weak function.\n\n> The pins of the UART I use are not configured by the ROM, it is the pin\n> controller driver configuring them.\n> You are of course right in that UART output is not available before the\n> pin controller driver has executed correctly. In my case the UART is\n> available in 'spl_board_init()'. I know that many things can go wrong\n> before this and therefore configuring UART pins in 'set_uart_mux_conf()'\n> while using 'CONFIG_DEBUG_UART' is fine. In this context the word\n> 'obsolete' may be wrong. The idea behind the patch is to not force the\n> implementation of those two (potentially empty) functions.\n\nI'm open to discussing making these functions be not required but then\nthe weak empty function should be under arch/arm/mach-omap2/ somewhere.\nThanks!","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=konsulko.com header.i=@konsulko.com\n\theader.b=\"b0ybSPhT\"; dkim-atps=neutral"],"Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xp3rh531fz9s2G\n\tfor ;\n\tFri, 8 Sep 2017 01:14:11 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid 2D631C21EA5; Thu, 7 Sep 2017 15:14:04 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id B8A10C21D70;\n\tThu, 7 Sep 2017 15:14:01 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid B85C1C21D88; Thu, 7 Sep 2017 15:13:59 +0000 (UTC)","from mail-yw0-f177.google.com (mail-yw0-f177.google.com\n\t[209.85.161.177])\n\tby lists.denx.de (Postfix) with ESMTPS id 2F2E7C21C73\n\tfor ; Thu, 7 Sep 2017 15:13:59 +0000 (UTC)","by mail-yw0-f177.google.com with SMTP id r85so98839ywg.1\n\tfor ; Thu, 07 Sep 2017 08:13:59 -0700 (PDT)","from bill-the-cat ([2606:a000:140b:dc:58d5:266b:224e:9609])\n\tby smtp.gmail.com with ESMTPSA id\n\tm76sm955268ywd.10.2017.09.07.08.13.56\n\t(version=TLS1_2 cipher=AES128-SHA bits=128/128);\n\tThu, 07 Sep 2017 08:13:56 -0700 (PDT)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-0.0 required=5.0 tests=RCVD_IN_DNSWL_NONE,\n\tRCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,\n\tT_DKIM_INVALID autolearn=unavailable\n\tautolearn_force=no version=3.4.0","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com;\n\ts=google; \n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=UDL6XnIYSoU3+7xRdg6XXg+4M22Zs/MIFdgftKkRVQI=;\n\tb=b0ybSPhTJQsftrYis+QX5nRjOYe3J2IVi3qbrOGmSp+nny2kg7kLdkxVW3zm2fj3QZ\n\tv/DYhx6kHhwq17PaghjSnvZ2vDiJS9PrQr9asOPkcO/adVx67m3wHPKIImfy2A/y3lo2\n\tNABGxSDp5dz5u0GAQO7YSWbtVQaPTz6i5KodI=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=UDL6XnIYSoU3+7xRdg6XXg+4M22Zs/MIFdgftKkRVQI=;\n\tb=e8TusuDnc1yODERE0G0xwZQpF8ED3ai4g1x9ilFFI8c/byzotFcrf4Ajsz8GEzgkOI\n\tpCRJ/fYz77zF41GaYYEXusF2dOrjTBQK98ec3ihkuL89PyV1lxLDc2wccpejKXOdpRWb\n\tm6+tnrpyB/ryYEguPE7P6vM9VceNvK8NOO95O9pJK0mOQkY/ltp2kj+yfg0eRzcP/Zu5\n\tIQX5mJWfa9RPgcaKt3u8JKsAhmPsytNP5wVvUof5Ph6pmcH7DJON+DAQYLx1sO8arYA4\n\txB2DTwIP0jxC8+zMQepcaTm7jpzygt02+JRJcknnG0xDrSjKbkkF9SFqQfV1fyH00MKj\n\teiEw==","X-Gm-Message-State":"AHPjjUjWx9Ap8XU4Ogpj0SWR54rfMj7Gc+A+fyl2as6XTwV8tm7/6XK8\n\tgi9LUw570FsU69fx","X-Google-Smtp-Source":"ADKCNb49vsD7XtH/lTpSeFE8qxNgTCaijWqzi2FHj8zG3KB3nhb0SSK4SK9ZEZrhNvIlLOSrcaN25A==","X-Received":"by 10.13.192.130 with SMTP id b124mr2438275ywd.370.1504797237779;\n\tThu, 07 Sep 2017 08:13:57 -0700 (PDT)","Date":"Thu, 7 Sep 2017 11:14:28 -0400","From":"Tom Rini ","To":"Felix Brack ","Message-ID":"<20170907151428.GS17058@bill-the-cat>","References":"<1504185377-461-1-git-send-email-fb@ltec.ch>\n\t<20170901152128.GA17058@bill-the-cat>\n\t","MIME-Version":"1.0","In-Reply-To":"","User-Agent":"Mutt/1.5.21 (2010-09-15)","Cc":"u-boot@lists.denx.de","Subject":"Re: [U-Boot] [PATCH] arm: am33xx: Make pin multiplexing functions\n\toptional","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"multipart/mixed;\n\tboundary=\"===============2072019364441927017==\"","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}},{"id":1766933,"web_url":"http://patchwork.ozlabs.org/comment/1766933/","msgid":"<90e7d653-c994-bc87-cf24-324f1ddb1b72@ltec.ch>","list_archive_url":null,"date":"2017-09-12T12:05:01","subject":"Re: [U-Boot] [PATCH] arm: am33xx: Make pin multiplexing functions\n\toptional","submitter":{"id":70195,"url":"http://patchwork.ozlabs.org/api/people/70195/","name":"Felix Brack","email":"fb@ltec.ch"},"content":"On 07.09.2017 17:14, Tom Rini wrote:\n> On Wed, Sep 06, 2017 at 04:57:52PM +0200, Felix Brack wrote:\n>> On 01.09.2017 17:21, Tom Rini wrote:\n>>> On Thu, Aug 31, 2017 at 03:16:17PM +0200, Felix Brack wrote:\n>>>\n>>>> Boards using the single-register-pin-controller can configure all\n>>>> pins by means of the device tree. This renders the implementation of\n>>>> the two functions set_uart_mux_conf and set_mux_conf_regs obsolete.\n>>>> Using the weak attribute for these two function declarations allows\n>>>> the omission of the respective definitions.\n>>>>\n>>>> Signed-off-by: Felix Brack \n>>>> ---\n>>>>\n>>>> arch/arm/include/asm/arch-am33xx/sys_proto.h | 4 ++--\n>>>> 1 file changed, 2 insertions(+), 2 deletions(-)\n>>>>\n>>>> diff --git a/arch/arm/include/asm/arch-am33xx/sys_proto.h b/arch/arm/include/asm/arch-am33xx/sys_proto.h\n>>>> index 4e78aaf..e31c25c 100644\n>>>> --- a/arch/arm/include/asm/arch-am33xx/sys_proto.h\n>>>> +++ b/arch/arm/include/asm/arch-am33xx/sys_proto.h\n>>>> @@ -31,8 +31,8 @@ void enable_gpmc_cs_config(const u32 *gpmc_config, const struct gpmc_cs *cs, u32\n>>>> \t\t\tu32 size);\n>>>> int omap_nand_switch_ecc(uint32_t, uint32_t);\n>>>> \n>>>> -void set_uart_mux_conf(void);\n>>>> -void set_mux_conf_regs(void);\n>>>> +__weak void set_uart_mux_conf(void);\n>>>> +__weak void set_mux_conf_regs(void);\n>>>> void sdram_init(void);\n>>>> u32 wait_on_value(u32, u32, void *, u32);\n>>>> #ifdef CONFIG_NOR_BOOT\n>>>\n>>> This seems wrong in a few ways. First, this (and the matching ones for\n>>> omap3, etc) should be full of externs instead and in that case __weak\n>>> makes no sense. Then, on the technical side, what you're describing\n>>> isn't quite true in that you're likely relying on having the ROM to have\n>>> setup the 'normal' UART still for you, as it so often does, I believe.\n>>> Or in the case I'm wrong, then yes, you do get UART to work once we have\n>>> parsed the DT, but the \"usual\" case is that we want UART and thus\n>>> printf/etc to be available asap in case of errors, so it's still not a\n>>> recommended way to go. Thanks!\n>>>\n>>\n>> Hi Tom,\n>>\n>> Could you please explain what you mean by \"... should be full of externs\n>> instead and in that case __weak makes no sense\"?\n> \n> I mean it's a header file. We should only be declaring function\n> prototypes, and thus using 'extern' here. It's not the right place to\n> have an inline weak function.\n> \n\nReflecting once more on my patch and after some tests I must admit that\nthe patch is based on a fundamental misunderstanding of the attribute\n'weak' that could even lead to segmentation faults :$.\nThe patch, as such, should therefore be discarded. However I will retain\nthe idea behind the patch (see below).\n\n>> The pins of the UART I use are not configured by the ROM, it is the pin\n>> controller driver configuring them.\n>> You are of course right in that UART output is not available before the\n>> pin controller driver has executed correctly. In my case the UART is\n>> available in 'spl_board_init()'. I know that many things can go wrong\n>> before this and therefore configuring UART pins in 'set_uart_mux_conf()'\n>> while using 'CONFIG_DEBUG_UART' is fine. In this context the word\n>> 'obsolete' may be wrong. The idea behind the patch is to not force the\n>> implementation of those two (potentially empty) functions.\n> \n> I'm open to discussing making these functions be not required but then\n> the weak empty function should be under arch/arm/mach-omap2/ somewhere.\n\nHow about adding a 'weak', default, empty implementation of these two\nfunctions to arch/arm/mach-omap2/am33xx/mux.c?\n\n> Thanks!\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xs3R400F2z9t34\n\tfor ;\n\tTue, 12 Sep 2017 22:05:14 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid D59E1C21FA0; Tue, 12 Sep 2017 12:05:06 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 8C783C21F02;\n\tTue, 12 Sep 2017 12:05:04 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 35A60C21C40; Tue, 12 Sep 2017 12:05:03 +0000 (UTC)","from mail.ltec.ch (mail.ltec.ch [95.143.48.181])\n\tby lists.denx.de (Postfix) with ESMTPS id D27B6C21E65\n\tfor ; Tue, 12 Sep 2017 12:05:02 +0000 (UTC)","from nebula.ltec ([172.27.11.2] helo=[172.27.11.32])\n\tby mail.ltec.ch with esmtpsa\n\t(TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128)\n\t(Exim 4.84_2) (envelope-from )\n\tid 1drjwp-0001lU-9a; Tue, 12 Sep 2017 14:05:47 +0200"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-2.3 required=5.0 tests=RCVD_IN_DNSWL_MED\n\tautolearn=unavailable autolearn_force=no version=3.4.0","To":"Tom Rini ","References":"<1504185377-461-1-git-send-email-fb@ltec.ch>\n\t<20170901152128.GA17058@bill-the-cat>\n\t\n\t<20170907151428.GS17058@bill-the-cat>","From":"Felix Brack ","Message-ID":"<90e7d653-c994-bc87-cf24-324f1ddb1b72@ltec.ch>","Date":"Tue, 12 Sep 2017 14:05:01 +0200","User-Agent":"Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170907151428.GS17058@bill-the-cat>","Content-Language":"en-US","Cc":"u-boot@lists.denx.de","Subject":"Re: [U-Boot] [PATCH] arm: am33xx: Make pin multiplexing functions\n\toptional","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}},{"id":1767519,"web_url":"http://patchwork.ozlabs.org/comment/1767519/","msgid":"<20170913025645.GY4474@bill-the-cat>","list_archive_url":null,"date":"2017-09-13T02:56:45","subject":"Re: [U-Boot] [PATCH] arm: am33xx: Make pin multiplexing functions\n\toptional","submitter":{"id":65875,"url":"http://patchwork.ozlabs.org/api/people/65875/","name":"Tom Rini","email":"trini@konsulko.com"},"content":"On Tue, Sep 12, 2017 at 02:05:01PM +0200, Felix Brack wrote:\n> \n> \n> On 07.09.2017 17:14, Tom Rini wrote:\n> > On Wed, Sep 06, 2017 at 04:57:52PM +0200, Felix Brack wrote:\n> >> On 01.09.2017 17:21, Tom Rini wrote:\n> >>> On Thu, Aug 31, 2017 at 03:16:17PM +0200, Felix Brack wrote:\n> >>>\n> >>>> Boards using the single-register-pin-controller can configure all\n> >>>> pins by means of the device tree. This renders the implementation of\n> >>>> the two functions set_uart_mux_conf and set_mux_conf_regs obsolete.\n> >>>> Using the weak attribute for these two function declarations allows\n> >>>> the omission of the respective definitions.\n> >>>>\n> >>>> Signed-off-by: Felix Brack \n> >>>> ---\n> >>>>\n> >>>> arch/arm/include/asm/arch-am33xx/sys_proto.h | 4 ++--\n> >>>> 1 file changed, 2 insertions(+), 2 deletions(-)\n> >>>>\n> >>>> diff --git a/arch/arm/include/asm/arch-am33xx/sys_proto.h b/arch/arm/include/asm/arch-am33xx/sys_proto.h\n> >>>> index 4e78aaf..e31c25c 100644\n> >>>> --- a/arch/arm/include/asm/arch-am33xx/sys_proto.h\n> >>>> +++ b/arch/arm/include/asm/arch-am33xx/sys_proto.h\n> >>>> @@ -31,8 +31,8 @@ void enable_gpmc_cs_config(const u32 *gpmc_config, const struct gpmc_cs *cs, u32\n> >>>> \t\t\tu32 size);\n> >>>> int omap_nand_switch_ecc(uint32_t, uint32_t);\n> >>>> \n> >>>> -void set_uart_mux_conf(void);\n> >>>> -void set_mux_conf_regs(void);\n> >>>> +__weak void set_uart_mux_conf(void);\n> >>>> +__weak void set_mux_conf_regs(void);\n> >>>> void sdram_init(void);\n> >>>> u32 wait_on_value(u32, u32, void *, u32);\n> >>>> #ifdef CONFIG_NOR_BOOT\n> >>>\n> >>> This seems wrong in a few ways. First, this (and the matching ones for\n> >>> omap3, etc) should be full of externs instead and in that case __weak\n> >>> makes no sense. Then, on the technical side, what you're describing\n> >>> isn't quite true in that you're likely relying on having the ROM to have\n> >>> setup the 'normal' UART still for you, as it so often does, I believe.\n> >>> Or in the case I'm wrong, then yes, you do get UART to work once we have\n> >>> parsed the DT, but the \"usual\" case is that we want UART and thus\n> >>> printf/etc to be available asap in case of errors, so it's still not a\n> >>> recommended way to go. Thanks!\n> >>>\n> >>\n> >> Hi Tom,\n> >>\n> >> Could you please explain what you mean by \"... should be full of externs\n> >> instead and in that case __weak makes no sense\"?\n> > \n> > I mean it's a header file. We should only be declaring function\n> > prototypes, and thus using 'extern' here. It's not the right place to\n> > have an inline weak function.\n> > \n> \n> Reflecting once more on my patch and after some tests I must admit that\n> the patch is based on a fundamental misunderstanding of the attribute\n> 'weak' that could even lead to segmentation faults :$.\n> The patch, as such, should therefore be discarded. However I will retain\n> the idea behind the patch (see below).\n> \n> >> The pins of the UART I use are not configured by the ROM, it is the pin\n> >> controller driver configuring them.\n> >> You are of course right in that UART output is not available before the\n> >> pin controller driver has executed correctly. In my case the UART is\n> >> available in 'spl_board_init()'. I know that many things can go wrong\n> >> before this and therefore configuring UART pins in 'set_uart_mux_conf()'\n> >> while using 'CONFIG_DEBUG_UART' is fine. In this context the word\n> >> 'obsolete' may be wrong. The idea behind the patch is to not force the\n> >> implementation of those two (potentially empty) functions.\n> > \n> > I'm open to discussing making these functions be not required but then\n> > the weak empty function should be under arch/arm/mach-omap2/ somewhere.\n> \n> How about adding a 'weak', default, empty implementation of these two\n> functions to arch/arm/mach-omap2/am33xx/mux.c?\n\nSounds good, thanks!","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=konsulko.com header.i=@konsulko.com\n\theader.b=\"CBNZMus3\"; dkim-atps=neutral"],"Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsRKT5c4Jz9t39\n\tfor ;\n\tWed, 13 Sep 2017 13:02:21 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid 8C73CC22363; Wed, 13 Sep 2017 03:01:37 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id E417DC22473;\n\tWed, 13 Sep 2017 03:01:34 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 131C0C2228B; Wed, 13 Sep 2017 02:56:48 +0000 (UTC)","from mail-yw0-f182.google.com (mail-yw0-f182.google.com\n\t[209.85.161.182])\n\tby lists.denx.de (Postfix) with ESMTPS id 3DD7DC2249A\n\tfor ; Wed, 13 Sep 2017 02:56:48 +0000 (UTC)","by mail-yw0-f182.google.com with SMTP id s62so32215966ywg.0\n\tfor ; Tue, 12 Sep 2017 19:56:48 -0700 (PDT)","from bill-the-cat ([2606:a000:140b:dc:202f:e2cf:a300:56c])\n\tby smtp.gmail.com with ESMTPSA id\n\tx4sm4585396ywj.97.2017.09.12.19.56.46\n\t(version=TLS1_2 cipher=AES128-SHA bits=128/128);\n\tTue, 12 Sep 2017 19:56:46 -0700 (PDT)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-0.0 required=5.0 tests=RCVD_IN_DNSWL_NONE,\n\tRCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,\n\tT_DKIM_INVALID autolearn=unavailable\n\tautolearn_force=no version=3.4.0","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com;\n\ts=google; \n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=Ybp5OxaezOYXvyBr2TjVGPysy/yG5LYjCOyt4i3A4gg=;\n\tb=CBNZMus3ws79HjmLm6ISYdS2eRZ9pG72jzMvl0aqzA+xlNnqUVofwhZUYdcr8jvKXH\n\tSlKsxiogRMJn2lDbswM4SZC5JSwt6/YVRmJZBIHSfjyXqNBzCUGJldsqS6X8f+iQM3bT\n\t3eT+dHgNe7iKtNc119c9jUUZLJ9b04beohPwQ=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=Ybp5OxaezOYXvyBr2TjVGPysy/yG5LYjCOyt4i3A4gg=;\n\tb=uCrqABhFS8xbokpZ3zx+vh4t1R9qB9Mk0U+uLc2ynAFY+f+3PP/qxuakWDC/i+GSnK\n\tp+v6FdlUC5ZGd0b1KIZGrJqYFJoCRWYRxGKBZ4e1xEyxJfQkVoTxHkQx057Q2nRQQ33W\n\tB2GBc40vE9l9oxP7QvyabXfIhvK2ydHSWnQP3iMwHu4mcIzVRafLYe1GxzihitqZ2mLg\n\teDG+NNkmAGYZslcZBe6iesCaJRcu7q731yNEs5xesjpTA+5DKJkFOjzXFWuU4o/lNcBG\n\tKtNw5lGIszlD7q9CEKPQjTC/A3Ee8hb/2FimV6VNluCL/ajOHZRsX5VrSbgxwRV9AJ89\n\tOtpQ==","X-Gm-Message-State":"AHPjjUgFwoJVVL4a/f204u4H2P0x2cJhwqrQAwicngYDFs6nerr4KBK/\n\taH6gB/mxM/lQXo9zEYgiNQ==","X-Google-Smtp-Source":"ADKCNb4haB0ItCG4rpy54I7szurxt4oF/Ho1woLgEvxLGnZf6PJR2pPd4W56luVRhyZ+O5le3o77Rg==","X-Received":"by 10.129.129.70 with SMTP id r67mr13903167ywf.421.1505271407100;\n\tTue, 12 Sep 2017 19:56:47 -0700 (PDT)","Date":"Tue, 12 Sep 2017 22:56:45 -0400","From":"Tom Rini ","To":"Felix Brack ","Message-ID":"<20170913025645.GY4474@bill-the-cat>","References":"<1504185377-461-1-git-send-email-fb@ltec.ch>\n\t<20170901152128.GA17058@bill-the-cat>\n\t\n\t<20170907151428.GS17058@bill-the-cat>\n\t<90e7d653-c994-bc87-cf24-324f1ddb1b72@ltec.ch>","MIME-Version":"1.0","In-Reply-To":"<90e7d653-c994-bc87-cf24-324f1ddb1b72@ltec.ch>","User-Agent":"Mutt/1.5.21 (2010-09-15)","Cc":"u-boot@lists.denx.de","Subject":"Re: [U-Boot] [PATCH] arm: am33xx: Make pin multiplexing functions\n\toptional","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"multipart/mixed;\n\tboundary=\"===============3785513142437119794==\"","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}}]