| Message ID | 20190513104638.9326-1-igor.opaniuk@toradex.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | [U-Boot,1/1] imx8: cpu: fix warning for cpu_imx_get_temp | expand |
Hi Igor, On 13.05.2019 12:46, Igor Opaniuk wrote: > cpu_imx_get_temp() definition is wrapped with a ifdef macro, > therefore all function references should be also wrapped the same way > instead IS_ENABLED() usage. > > Fix warning: > arch/arm/mach-imx/imx8/cpu.c: In function ‘cpu_imx_get_desc’: > arch/arm/mach-imx/imx8/cpu.c:612:40: warning: implicit declaration of > function ‘cpu_imx_get_temp’; did you mean ‘cpu_imx_get_desc’? > [-Wimplicit-function-declaration] > ret = snprintf(buf, size, " at %dC", cpu_imx_get_temp()); > ^~~~~~~~~~~~~~~~ > cpu_imx_get_desc > cpu_imx_get_desc > Using IS_ENABLED is typically preferred over ifdef since it assures that the code is compileable even if CONFIG_IMX_SCU_THERMAL is not enabled. I'd rather prefer we drop the the ifdef around cpu_imx_get_temp(). The linker will remove the function in case CONFIG_IMX_SCU_THERMAL is not enabled. -- Stefan > Fixes: 82467cb217 ("imx8: cpu: get temperature when print cpu desc") > Signed-off-by: Igor Opaniuk <igor.opaniuk@toradex.com> > --- > arch/arm/mach-imx/imx8/cpu.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/mach-imx/imx8/cpu.c b/arch/arm/mach-imx/imx8/cpu.c > index 12596c6387..616baed7cc 100644 > --- a/arch/arm/mach-imx/imx8/cpu.c > +++ b/arch/arm/mach-imx/imx8/cpu.c > @@ -606,11 +606,11 @@ int cpu_imx_get_desc(struct udevice *dev, char > *buf, int size) > ret = snprintf(buf, size, "NXP i.MX8%s Rev%s %s at %u MHz", > plat->type, plat->rev, plat->name, plat->freq_mhz); > > - if (IS_ENABLED(CONFIG_IMX_SCU_THERMAL)) { > - buf = buf + ret; > - size = size - ret; > - ret = snprintf(buf, size, " at %dC", cpu_imx_get_temp()); > - } > +#if defined(CONFIG_IMX_SCU_THERMAL) > + buf = buf + ret; > + size = size - ret; > + ret = snprintf(buf, size, " at %dC", cpu_imx_get_temp()); > +#endif > > snprintf(buf + ret, size - ret, "\n");
Hi Igor, Not found your original mail, so reply Stefan's mail here. > -----Original Message----- > From: Stefan Agner [mailto:stefan@agner.ch] > Sent: 2019年5月13日 21:56 > To: Igor Opaniuk <igor.opaniuk@toradex.com> > Cc: u-boot@lists.denx.de; festevam@gmail.com; dl-uboot-imx > <uboot-imx@nxp.com>; albert.u.boot@aribaud.net; Peng Fan > <peng.fan@nxp.com>; agust@denx.de; Marcel Ziswiler > <marcel.ziswiler@toradex.com>; marcel@ziswiler.com; Max Krummenacher > <max.krummenacher@toradex.com>; sbabic@denx.de > Subject: Re: [PATCH 1/1] imx8: cpu: fix warning for cpu_imx_get_temp > > Hi Igor, > > On 13.05.2019 12:46, Igor Opaniuk wrote: > > cpu_imx_get_temp() definition is wrapped with a ifdef macro, therefore > > all function references should be also wrapped the same way instead > > IS_ENABLED() usage. Are you trying to fix https://patchwork.ozlabs.org/patch/1095430/ ? Thanks, Peng > > > > Fix warning: > > arch/arm/mach-imx/imx8/cpu.c: In function ‘cpu_imx_get_desc’: > > arch/arm/mach-imx/imx8/cpu.c:612:40: warning: implicit declaration of > > function ‘cpu_imx_get_temp’; did you mean ‘cpu_imx_get_desc’? > > [-Wimplicit-function-declaration] > > ret = snprintf(buf, size, " at %dC", cpu_imx_get_temp()); > > ^~~~~~~~~~~~~~~~ > > cpu_imx_get_desc > > cpu_imx_get_desc > > > > Using IS_ENABLED is typically preferred over ifdef since it assures that the > code is compileable even if CONFIG_IMX_SCU_THERMAL is not enabled. > > I'd rather prefer we drop the the ifdef around cpu_imx_get_temp(). The linker > will remove the function in case CONFIG_IMX_SCU_THERMAL is not enabled. > > -- > Stefan > > > Fixes: 82467cb217 ("imx8: cpu: get temperature when print cpu desc") > > Signed-off-by: Igor Opaniuk <igor.opaniuk@toradex.com> > > --- > > arch/arm/mach-imx/imx8/cpu.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/arch/arm/mach-imx/imx8/cpu.c > > b/arch/arm/mach-imx/imx8/cpu.c index 12596c6387..616baed7cc 100644 > > --- a/arch/arm/mach-imx/imx8/cpu.c > > +++ b/arch/arm/mach-imx/imx8/cpu.c > > @@ -606,11 +606,11 @@ int cpu_imx_get_desc(struct udevice *dev, char > > *buf, int size) > > ret = snprintf(buf, size, "NXP i.MX8%s Rev%s %s at %u MHz", > > plat->type, plat->rev, plat->name, plat->freq_mhz); > > > > - if (IS_ENABLED(CONFIG_IMX_SCU_THERMAL)) { > > - buf = buf + ret; > > - size = size - ret; > > - ret = snprintf(buf, size, " at %dC", cpu_imx_get_temp()); > > - } > > +#if defined(CONFIG_IMX_SCU_THERMAL) > > + buf = buf + ret; > > + size = size - ret; > > + ret = snprintf(buf, size, " at %dC", cpu_imx_get_temp()); #endif > > > > snprintf(buf + ret, size - ret, "\n");
Hi Peng, Seems this is a glitch, I was working with the local forked tree (a copy of the latest U-boot mainline master) + manually applied patch series for Toradex Colibri iMX8QXP [1]/Apalis iMX8QM [2] support on top of it (that are still pending in the U-boot ML), but seems that some of your patches were also applied and I wasn't aware about this (I thought that they were already in `next`). So just ignore this patch. Thanks [1]: https://patchwork.ozlabs.org/project/uboot/list/?series=105770 [2]: https://patchwork.ozlabs.org/project/uboot/list/?series=105774 On Tue, May 14, 2019 at 4:09 AM Peng Fan <peng.fan@nxp.com> wrote: > > Hi Igor, > > Not found your original mail, so reply Stefan's mail here. > > > -----Original Message----- > > From: Stefan Agner [mailto:stefan@agner.ch] > > Sent: 2019年5月13日 21:56 > > To: Igor Opaniuk <igor.opaniuk@toradex.com> > > Cc: u-boot@lists.denx.de; festevam@gmail.com; dl-uboot-imx > > <uboot-imx@nxp.com>; albert.u.boot@aribaud.net; Peng Fan > > <peng.fan@nxp.com>; agust@denx.de; Marcel Ziswiler > > <marcel.ziswiler@toradex.com>; marcel@ziswiler.com; Max Krummenacher > > <max.krummenacher@toradex.com>; sbabic@denx.de > > Subject: Re: [PATCH 1/1] imx8: cpu: fix warning for cpu_imx_get_temp > > > > Hi Igor, > > > > On 13.05.2019 12:46, Igor Opaniuk wrote: > > > cpu_imx_get_temp() definition is wrapped with a ifdef macro, therefore > > > all function references should be also wrapped the same way instead > > > IS_ENABLED() usage. > > Are you trying to fix https://patchwork.ozlabs.org/patch/1095430/ ? > > Thanks, > Peng > > > > > > > Fix warning: > > > arch/arm/mach-imx/imx8/cpu.c: In function ‘cpu_imx_get_desc’: > > > arch/arm/mach-imx/imx8/cpu.c:612:40: warning: implicit declaration of > > > function ‘cpu_imx_get_temp’; did you mean ‘cpu_imx_get_desc’? > > > [-Wimplicit-function-declaration] > > > ret = snprintf(buf, size, " at %dC", cpu_imx_get_temp()); > > > ^~~~~~~~~~~~~~~~ > > > cpu_imx_get_desc > > > cpu_imx_get_desc > > > > > > > Using IS_ENABLED is typically preferred over ifdef since it assures that the > > code is compileable even if CONFIG_IMX_SCU_THERMAL is not enabled. > > > > I'd rather prefer we drop the the ifdef around cpu_imx_get_temp(). The linker > > will remove the function in case CONFIG_IMX_SCU_THERMAL is not enabled. > > > > -- > > Stefan > > > > > Fixes: 82467cb217 ("imx8: cpu: get temperature when print cpu desc") > > > Signed-off-by: Igor Opaniuk <igor.opaniuk@toradex.com> > > > --- > > > arch/arm/mach-imx/imx8/cpu.c | 10 +++++----- > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > diff --git a/arch/arm/mach-imx/imx8/cpu.c > > > b/arch/arm/mach-imx/imx8/cpu.c index 12596c6387..616baed7cc 100644 > > > --- a/arch/arm/mach-imx/imx8/cpu.c > > > +++ b/arch/arm/mach-imx/imx8/cpu.c > > > @@ -606,11 +606,11 @@ int cpu_imx_get_desc(struct udevice *dev, char > > > *buf, int size) > > > ret = snprintf(buf, size, "NXP i.MX8%s Rev%s %s at %u MHz", > > > plat->type, plat->rev, plat->name, plat->freq_mhz); > > > > > > - if (IS_ENABLED(CONFIG_IMX_SCU_THERMAL)) { > > > - buf = buf + ret; > > > - size = size - ret; > > > - ret = snprintf(buf, size, " at %dC", cpu_imx_get_temp()); > > > - } > > > +#if defined(CONFIG_IMX_SCU_THERMAL) > > > + buf = buf + ret; > > > + size = size - ret; > > > + ret = snprintf(buf, size, " at %dC", cpu_imx_get_temp()); #endif > > > > > > snprintf(buf + ret, size - ret, "\n"); > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot
diff --git a/arch/arm/mach-imx/imx8/cpu.c b/arch/arm/mach-imx/imx8/cpu.c index 12596c6387..616baed7cc 100644 --- a/arch/arm/mach-imx/imx8/cpu.c +++ b/arch/arm/mach-imx/imx8/cpu.c @@ -606,11 +606,11 @@ int cpu_imx_get_desc(struct udevice *dev, char *buf, int size) ret = snprintf(buf, size, "NXP i.MX8%s Rev%s %s at %u MHz", plat->type, plat->rev, plat->name, plat->freq_mhz); - if (IS_ENABLED(CONFIG_IMX_SCU_THERMAL)) { - buf = buf + ret; - size = size - ret; - ret = snprintf(buf, size, " at %dC", cpu_imx_get_temp()); - } +#if defined(CONFIG_IMX_SCU_THERMAL) + buf = buf + ret; + size = size - ret; + ret = snprintf(buf, size, " at %dC", cpu_imx_get_temp()); +#endif snprintf(buf + ret, size - ret, "\n");
cpu_imx_get_temp() definition is wrapped with a ifdef macro, therefore all function references should be also wrapped the same way instead IS_ENABLED() usage. Fix warning: arch/arm/mach-imx/imx8/cpu.c: In function ‘cpu_imx_get_desc’: arch/arm/mach-imx/imx8/cpu.c:612:40: warning: implicit declaration of function ‘cpu_imx_get_temp’; did you mean ‘cpu_imx_get_desc’? [-Wimplicit-function-declaration] ret = snprintf(buf, size, " at %dC", cpu_imx_get_temp()); ^~~~~~~~~~~~~~~~ cpu_imx_get_desc cpu_imx_get_desc Fixes: 82467cb217 ("imx8: cpu: get temperature when print cpu desc") Signed-off-by: Igor Opaniuk <igor.opaniuk@toradex.com> --- arch/arm/mach-imx/imx8/cpu.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)