From: srinivas.kandagatla@linaro.org (Srinivas Kandagatla)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: davinci: only use NVMEM when available
Date: Thu, 17 Mar 2016 10:26:28 +0000 [thread overview]
Message-ID: <56EA8654.5070100@linaro.org> (raw)
In-Reply-To: <1458077705-408437-1-git-send-email-arnd@arndb.de>
Thanks for the patch,
On 15/03/16 21:34, Arnd Bergmann wrote:
> The davinci platform contains code that calls into the nvmem
> subsystem, but that might be a loadable module, causing a
> link error:
>
> arch/arm/mach-davinci/built-in.o: In function `davinci_get_mac_addr':
> :(.text+0x1088): undefined reference to `nvmem_device_read'
> arch/arm/mach-davinci/built-in.o: In function `read_factory_config':
> :(.text+0x214c): undefined reference to `nvmem_device_read'
>
This fix does not look correct to me.
IMO, This is a issue with incorrect dependencies in the Kconfig, we
should not attempt to fix it in the code, rather fix the kconfig
dependencies in the mach-davnici or the EEPROM_AT24 kconfig itself. one
of them should select the NVMEM.
thanks,
srini
> Also, when NVMEM is completely disabled, the functions fail with
> nonobvious error messages.
>
> This ensures we only call the API functions when the code is actually
> reachable from the board file, and otherwise prints a unique log
> message.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: bec3c11bad0e ("misc: at24: replace memory_accessor with nvmem_device_read")
> ---
>
> Hi Greg,
>
> The commit that introduced this is currently in the char-misc tree,
> please apply this fixup on top if you haven't already sent it to Linus.
>
> arch/arm/mach-davinci/board-mityomapl138.c | 5 +++++
> arch/arm/mach-davinci/common.c | 5 +++++
> 2 files changed, 10 insertions(+)
>
> diff --git a/arch/arm/mach-davinci/board-mityomapl138.c b/arch/arm/mach-davinci/board-mityomapl138.c
> index d97c588550ad..bc4e63fa9808 100644
> --- a/arch/arm/mach-davinci/board-mityomapl138.c
> +++ b/arch/arm/mach-davinci/board-mityomapl138.c
> @@ -121,6 +121,11 @@ static void read_factory_config(struct nvmem_device *nvmem, void *context)
> const char *partnum = NULL;
> struct davinci_soc_info *soc_info = &davinci_soc_info;
>
> + if (!IS_BUILTIN(CONFIG_NVMEM)) {
> + pr_warn("Factory Config not available without CONFIG_NVMEM\n");
> + goto bad_config;
> + }
> +
> ret = nvmem_device_read(nvmem, 0, sizeof(factory_config),
> &factory_config);
> if (ret != sizeof(struct factory_config)) {
> diff --git a/arch/arm/mach-davinci/common.c b/arch/arm/mach-davinci/common.c
> index f55ef2ef2f92..742133b7266a 100644
> --- a/arch/arm/mach-davinci/common.c
> +++ b/arch/arm/mach-davinci/common.c
> @@ -33,6 +33,11 @@ void davinci_get_mac_addr(struct nvmem_device *nvmem, void *context)
> char *mac_addr = davinci_soc_info.emac_pdata->mac_addr;
> off_t offset = (off_t)context;
>
> + if (!IS_BUILTIN(CONFIG_NVMEM)) {
> + pr_warn("Cannot read MAC addr from EEPROM without CONFIG_NVMEM\n");
> + return;
> + }
> +
> /* Read MAC addr from EEPROM */
> if (nvmem_device_read(nvmem, offset, ETH_ALEN, mac_addr) == ETH_ALEN)
> pr_info("Read MAC addr from EEPROM: %pM\n", mac_addr);
>
WARNING: multiple messages have this Message-ID (diff)
From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
To: Arnd Bergmann <arnd@arndb.de>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Sekhar Nori <nsekhar@ti.com>, Kevin Hilman <khilman@kernel.org>,
Andrew Lunn <andrew@lunn.ch>, Wolfram Sang <wsa@the-dreams.de>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ARM: davinci: only use NVMEM when available
Date: Thu, 17 Mar 2016 10:26:28 +0000 [thread overview]
Message-ID: <56EA8654.5070100@linaro.org> (raw)
In-Reply-To: <1458077705-408437-1-git-send-email-arnd@arndb.de>
Thanks for the patch,
On 15/03/16 21:34, Arnd Bergmann wrote:
> The davinci platform contains code that calls into the nvmem
> subsystem, but that might be a loadable module, causing a
> link error:
>
> arch/arm/mach-davinci/built-in.o: In function `davinci_get_mac_addr':
> :(.text+0x1088): undefined reference to `nvmem_device_read'
> arch/arm/mach-davinci/built-in.o: In function `read_factory_config':
> :(.text+0x214c): undefined reference to `nvmem_device_read'
>
This fix does not look correct to me.
IMO, This is a issue with incorrect dependencies in the Kconfig, we
should not attempt to fix it in the code, rather fix the kconfig
dependencies in the mach-davnici or the EEPROM_AT24 kconfig itself. one
of them should select the NVMEM.
thanks,
srini
> Also, when NVMEM is completely disabled, the functions fail with
> nonobvious error messages.
>
> This ensures we only call the API functions when the code is actually
> reachable from the board file, and otherwise prints a unique log
> message.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: bec3c11bad0e ("misc: at24: replace memory_accessor with nvmem_device_read")
> ---
>
> Hi Greg,
>
> The commit that introduced this is currently in the char-misc tree,
> please apply this fixup on top if you haven't already sent it to Linus.
>
> arch/arm/mach-davinci/board-mityomapl138.c | 5 +++++
> arch/arm/mach-davinci/common.c | 5 +++++
> 2 files changed, 10 insertions(+)
>
> diff --git a/arch/arm/mach-davinci/board-mityomapl138.c b/arch/arm/mach-davinci/board-mityomapl138.c
> index d97c588550ad..bc4e63fa9808 100644
> --- a/arch/arm/mach-davinci/board-mityomapl138.c
> +++ b/arch/arm/mach-davinci/board-mityomapl138.c
> @@ -121,6 +121,11 @@ static void read_factory_config(struct nvmem_device *nvmem, void *context)
> const char *partnum = NULL;
> struct davinci_soc_info *soc_info = &davinci_soc_info;
>
> + if (!IS_BUILTIN(CONFIG_NVMEM)) {
> + pr_warn("Factory Config not available without CONFIG_NVMEM\n");
> + goto bad_config;
> + }
> +
> ret = nvmem_device_read(nvmem, 0, sizeof(factory_config),
> &factory_config);
> if (ret != sizeof(struct factory_config)) {
> diff --git a/arch/arm/mach-davinci/common.c b/arch/arm/mach-davinci/common.c
> index f55ef2ef2f92..742133b7266a 100644
> --- a/arch/arm/mach-davinci/common.c
> +++ b/arch/arm/mach-davinci/common.c
> @@ -33,6 +33,11 @@ void davinci_get_mac_addr(struct nvmem_device *nvmem, void *context)
> char *mac_addr = davinci_soc_info.emac_pdata->mac_addr;
> off_t offset = (off_t)context;
>
> + if (!IS_BUILTIN(CONFIG_NVMEM)) {
> + pr_warn("Cannot read MAC addr from EEPROM without CONFIG_NVMEM\n");
> + return;
> + }
> +
> /* Read MAC addr from EEPROM */
> if (nvmem_device_read(nvmem, offset, ETH_ALEN, mac_addr) == ETH_ALEN)
> pr_info("Read MAC addr from EEPROM: %pM\n", mac_addr);
>
next prev parent reply other threads:[~2016-03-17 10:26 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-15 21:34 [PATCH] ARM: davinci: only use NVMEM when available Arnd Bergmann
2016-03-15 21:34 ` Arnd Bergmann
2016-03-17 10:26 ` Srinivas Kandagatla [this message]
2016-03-17 10:26 ` Srinivas Kandagatla
2016-03-17 10:30 ` Arnd Bergmann
2016-03-17 10:30 ` Arnd Bergmann
2016-04-12 6:59 ` Sekhar Nori
2016-04-12 6:59 ` Sekhar Nori
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56EA8654.5070100@linaro.org \
--to=srinivas.kandagatla@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.