Re: [PATCH] arch: consolidate pm_power_off callback
Hi "Enrico, I love your patch! Yet something to improve: [auto build test ERROR on powerpc/next] [also build test ERROR on linux/master arm/for-next arm64/for-next/core m68k/for-next linus/master v5.10 next-20201223] [cannot apply to arc/for-next uclinux-h8/h8300-next ia64/next nios2/for-linus] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Enrico-Weigelt-metux-IT-consult/arch-consolidate-pm_power_off-callback/20201223-025109 base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next config: powerpc64-randconfig-s031-20201222 (attached as .config) compiler: powerpc64-linux-gcc (GCC) 9.3.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # apt-get install sparse # sparse version: v0.6.3-184-g1b896707-dirty # https://github.com/0day-ci/linux/commit/8c71c92c0cdeb84fc023e18d7a59542a24801d3e git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Enrico-Weigelt-metux-IT-consult/arch-consolidate-pm_power_off-callback/20201223-025109 git checkout 8c71c92c0cdeb84fc023e18d7a59542a24801d3e # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=powerpc64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): >> powerpc64-linux-ld: kernel/reboot.o:(.bss.pm_power_off+0x0): multiple >> definition of `pm_power_off'; >> arch/powerpc/kernel/setup-common.o:(.bss.pm_power_off+0x0): first defined >> here --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH] arch: consolidate pm_power_off callback
Hi "Enrico, I love your patch! Yet something to improve: [auto build test ERROR on powerpc/next] [also build test ERROR on linux/master arm/for-next arm64/for-next/core m68k/for-next linus/master v5.10 next-20201223] [cannot apply to arc/for-next uclinux-h8/h8300-next ia64/next nios2/for-linus] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Enrico-Weigelt-metux-IT-consult/arch-consolidate-pm_power_off-callback/20201223-025109 base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next config: powerpc-allyesconfig (attached as .config) compiler: powerpc64-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/8c71c92c0cdeb84fc023e18d7a59542a24801d3e git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Enrico-Weigelt-metux-IT-consult/arch-consolidate-pm_power_off-callback/20201223-025109 git checkout 8c71c92c0cdeb84fc023e18d7a59542a24801d3e # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): powerpc64-linux-ld: kernel/reboot.o:(.bss.pm_power_off+0x0): multiple definition of `pm_power_off'; arch/powerpc/kernel/setup-common.o:(.bss.pm_power_off+0x0): first defined here powerpc64-linux-ld: kernel/reboot.o: in function `__crc_pm_power_off': >> (.rodata+0x8): multiple definition of `__crc_pm_power_off'; >> arch/powerpc/kernel/setup-common.o:(.rodata+0x18): first defined here --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH] arch: consolidate pm_power_off callback
Hi Enrico,
On Tue, Dec 22, 2020 at 9:15 PM Enrico Weigelt, metux IT consult
wrote:
> On 22.12.20 19:54, Geert Uytterhoeven wrote:
> > On Tue, Dec 22, 2020 at 7:46 PM Enrico Weigelt, metux IT consult
> > wrote:
> >> Move the pm_power_off callback into one global place and also add an
> >> function for conditionally calling it (when not NULL), in order to remove
> >> code duplication in all individual archs.
> >>
> >> Signed-off-by: Enrico Weigelt, metux IT consult
> >
> > Thanks for your patch!
> >
> >> --- a/arch/alpha/kernel/process.c
> >> +++ b/arch/alpha/kernel/process.c
> >> @@ -43,12 +43,6 @@
> >> #include "proto.h"
> >> #include "pci_impl.h"
> >>
> >> -/*
> >> - * Power off function, if any
> >> - */
> >> -void (*pm_power_off)(void) = machine_power_off;
> >
> > Assignments like these are lost in the conversion.
>
> Yes, but this doesn't seem to be ever called anyways. (in arch/alpha)
> And, BTW, letting it point to machine_power_off() doesn't make much
> sense, since it's the arch's machine_power_off() function, who're
> calling pm_power_off().
>
> Actually, we could remove pm_power_off completely from here, assuming
> nobody would *build* any drivers that register themselves into
> pm_power_off.
>
> If you feel better with it, I could post a patch that just removes
> pm_power_off from arch/alpha.
This is not limited to alpha, there are similar initializations on
m68k, openrisc,
and s390.
If none of these are called, they can be removed, but you should mention
that in the patch description.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH] arch: consolidate pm_power_off callback
On 22.12.20 19:54, Geert Uytterhoeven wrote: Hi, > On Tue, Dec 22, 2020 at 7:46 PM Enrico Weigelt, metux IT consult > wrote: >> Move the pm_power_off callback into one global place and also add an >> function for conditionally calling it (when not NULL), in order to remove >> code duplication in all individual archs. >> >> Signed-off-by: Enrico Weigelt, metux IT consult > > Thanks for your patch! > >> --- a/arch/alpha/kernel/process.c >> +++ b/arch/alpha/kernel/process.c >> @@ -43,12 +43,6 @@ >> #include "proto.h" >> #include "pci_impl.h" >> >> -/* >> - * Power off function, if any >> - */ >> -void (*pm_power_off)(void) = machine_power_off; > > Assignments like these are lost in the conversion. Yes, but this doesn't seem to be ever called anyways. (in arch/alpha) And, BTW, letting it point to machine_power_off() doesn't make much sense, since it's the arch's machine_power_off() function, who're calling pm_power_off(). Actually, we could remove pm_power_off completely from here, assuming nobody would *build* any drivers that register themselves into pm_power_off. If you feel better with it, I could post a patch that just removes pm_power_off from arch/alpha. --mtx -- --- Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren GPG/PGP-Schlüssel zu. --- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering i...@metux.net -- +49-151-27565287
Re: [PATCH] arch: consolidate pm_power_off callback
Hi Enrico,
On Tue, Dec 22, 2020 at 7:46 PM Enrico Weigelt, metux IT consult
wrote:
> Move the pm_power_off callback into one global place and also add an
> function for conditionally calling it (when not NULL), in order to remove
> code duplication in all individual archs.
>
> Signed-off-by: Enrico Weigelt, metux IT consult
Thanks for your patch!
> --- a/arch/alpha/kernel/process.c
> +++ b/arch/alpha/kernel/process.c
> @@ -43,12 +43,6 @@
> #include "proto.h"
> #include "pci_impl.h"
>
> -/*
> - * Power off function, if any
> - */
> -void (*pm_power_off)(void) = machine_power_off;
Assignments like these are lost in the conversion.
> -EXPORT_SYMBOL(pm_power_off);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
