ARM: omap1: set ARCH_OMAP1_ANY for ARCH_OMAP1

Message ID 20221106062536.26369-1-rdunlap@infradead.org
State New
Headers
Series ARM: omap1: set ARCH_OMAP1_ANY for ARCH_OMAP1 |

Commit Message

Randy Dunlap Nov. 6, 2022, 6:25 a.m. UTC
  Fix a build error by setting ARCH_OMAP1_ANY Kconfig symbol.
Fixes this build error:

arm-linux-gnueabi-ld: drivers/video/backlight/omap1_bl.o: in function `omapbl_probe':
omap1_bl.c:(.text+0x1b4): undefined reference to `omap_cfg_reg'

Fixes: 7036440eab3e ("ARM: omap1: enable multiplatform")
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Reported-by: kernel test robot <lkp@intel.com>
Cc: Aaro Koskinen <aaro.koskinen@iki.fi>
Cc: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-omap@vger.kernel.org
---
 arch/arm/mach-omap1/Kconfig |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

Comments

Arnd Bergmann Nov. 6, 2022, 7:43 a.m. UTC | #1
On Sun, Nov 6, 2022, at 07:25, Randy Dunlap wrote:
> Fix a build error by setting ARCH_OMAP1_ANY Kconfig symbol.
> Fixes this build error:
>
> arm-linux-gnueabi-ld: drivers/video/backlight/omap1_bl.o: in function 
> `omapbl_probe':
> omap1_bl.c:(.text+0x1b4): undefined reference to `omap_cfg_reg'
>
> Fixes: 7036440eab3e ("ARM: omap1: enable multiplatform")
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Reported-by: kernel test robot <lkp@intel.com>
> Cc: Aaro Koskinen <aaro.koskinen@iki.fi>
> Cc: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-omap@vger.kernel.org
> ---
>  arch/arm/mach-omap1/Kconfig |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff -- a/arch/arm/mach-omap1/Kconfig b/arch/arm/mach-omap1/Kconfig
> --- a/arch/arm/mach-omap1/Kconfig
> +++ b/arch/arm/mach-omap1/Kconfig
> @@ -47,7 +47,8 @@ config ARCH_OMAP16XX
> 
>  config ARCH_OMAP1_ANY
>  	select ARCH_OMAP
> -	def_bool ARCH_OMAP730 || ARCH_OMAP850 || ARCH_OMAP15XX || ARCH_OMAP16XX
> +	def_bool ARCH_OMAP730 || ARCH_OMAP850 || ARCH_OMAP15XX || \
> +		ARCH_OMAP16XX || ARCH_OMAP1

I think this would introduce other build failures, because it
makes ARCH_OMAP1_ANY the same as ARCH_OMAP1, bringing back the
problems I solved with 615dce5bf736 ("ARM: omap1: fix build with
no SoC selected").

What you probably see here is a preexisting bug that bisects
to 7036440eab3e because of the contents of your .config file
that no longer enable ARCH_OMAP1 without 7036440eab3e.

I have not tested it, but I suspect what we want instead is
the change below, limiting OMAP_MUX and related symbols to
configs that enable at least one of the OMAP1 variants.

     Arnd

--- a/arch/arm/mach-omap1/Kconfig
+++ b/arch/arm/mach-omap1/Kconfig
@@ -49,7 +49,7 @@ config ARCH_OMAP1_ANY
        select ARCH_OMAP
        def_bool ARCH_OMAP730 || ARCH_OMAP850 || ARCH_OMAP15XX || ARCH_OMAP16XX
 
-config ARCH_OMAP
+config ARCH_OMAP1_ANY
        bool
 
 comment "OMAP Feature Selections"
  
kernel test robot Nov. 6, 2022, 8:41 a.m. UTC | #2
Hi Randy,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tmlind-omap/for-next]
[also build test WARNING on linus/master v6.1-rc3 next-20221104]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Randy-Dunlap/ARM-omap1-set-ARCH_OMAP1_ANY-for-ARCH_OMAP1/20221106-142818
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git for-next
patch link:    https://lore.kernel.org/r/20221106062536.26369-1-rdunlap%40infradead.org
patch subject: [PATCH] ARM: omap1: set ARCH_OMAP1_ANY for ARCH_OMAP1
config: arm-randconfig-r032-20221106
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.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/intel-lab-lkp/linux/commit/4e4d77cfb79b150e9f99202283604a52d9d238d4
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Randy-Dunlap/ARM-omap1-set-ARCH_OMAP1_ANY-for-ARCH_OMAP1/20221106-142818
        git checkout 4e4d77cfb79b150e9f99202283604a52d9d238d4
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from arch/arm/mach-omap1/serial.c:24:
>> arch/arm/mach-omap1/pm.h:113:2: warning: #warning "Power management for this processor not implemented yet" [-Wcpp]
     113 | #warning "Power management for this processor not implemented yet"
         |  ^~~~~~~


vim +113 arch/arm/mach-omap1/pm.h

9ad5897c2659b3 include/asm-arm/arch-omap/pm.h       Tony Lindgren   2005-11-10  108  
9ad5897c2659b3 include/asm-arm/arch-omap/pm.h       Tony Lindgren   2005-11-10  109  #if     !defined(CONFIG_ARCH_OMAP730) && \
1f73b4de93e06d arch/arm/mach-omap1/pm.h             Alistair Buxton 2009-09-18  110  	!defined(CONFIG_ARCH_OMAP850) && \
9ad5897c2659b3 include/asm-arm/arch-omap/pm.h       Tony Lindgren   2005-11-10  111  	!defined(CONFIG_ARCH_OMAP15XX) && \
c912f7e1eae169 arch/arm/mach-omap1/pm.h             Kevin Hilman    2009-05-15  112  	!defined(CONFIG_ARCH_OMAP16XX)
87d99d6f7ee3ec arch/arm/plat-omap/include/mach/pm.h David Brownell  2009-03-04 @113  #warning "Power management for this processor not implemented yet"
^1da177e4c3f41 include/asm-arm/arch-omap/pm.h       Linus Torvalds  2005-04-16  114  #endif
^1da177e4c3f41 include/asm-arm/arch-omap/pm.h       Linus Torvalds  2005-04-16  115
  
Russell King (Oracle) Nov. 6, 2022, 10:26 a.m. UTC | #3
On Sun, Nov 06, 2022 at 08:43:50AM +0100, Arnd Bergmann wrote:
> --- a/arch/arm/mach-omap1/Kconfig
> +++ b/arch/arm/mach-omap1/Kconfig
> @@ -49,7 +49,7 @@ config ARCH_OMAP1_ANY
>         select ARCH_OMAP
>         def_bool ARCH_OMAP730 || ARCH_OMAP850 || ARCH_OMAP15XX || ARCH_OMAP16XX
>  
> -config ARCH_OMAP
> +config ARCH_OMAP1_ANY

This patch can't be right - look at the first line of context above, you
have symbols that select ARCH_OMAP and you've just removed the
definition of ARCH_OMAP.
  
Arnd Bergmann Nov. 6, 2022, 10:37 a.m. UTC | #4
On Sun, Nov 6, 2022, at 11:26, Russell King (Oracle) wrote:
> On Sun, Nov 06, 2022 at 08:43:50AM +0100, Arnd Bergmann wrote:
>> --- a/arch/arm/mach-omap1/Kconfig
>> +++ b/arch/arm/mach-omap1/Kconfig
>> @@ -49,7 +49,7 @@ config ARCH_OMAP1_ANY
>>         select ARCH_OMAP
>>         def_bool ARCH_OMAP730 || ARCH_OMAP850 || ARCH_OMAP15XX || ARCH_OMAP16XX
>>  
>> -config ARCH_OMAP
>> +config ARCH_OMAP1_ANY
>
> This patch can't be right - look at the first line of context above, you
> have symbols that select ARCH_OMAP and you've just removed the
> definition of ARCH_OMAP.

Right, I misread this, I thought this was an 'if ARCH_OMAP' block
instead of the Kconfig symbol. Part of the problem is now is
possibly that there are two symbols with that name now, I'll have
to try to reproduce the problem first and then see what caused it
originally.

      Arnd
  
Arnd Bergmann Nov. 6, 2022, 11:32 a.m. UTC | #5
On Sun, Nov 6, 2022, at 11:37, Arnd Bergmann wrote:
> On Sun, Nov 6, 2022, at 11:26, Russell King (Oracle) wrote:
>> On Sun, Nov 06, 2022 at 08:43:50AM +0100, Arnd Bergmann wrote:
>>> --- a/arch/arm/mach-omap1/Kconfig
>>> +++ b/arch/arm/mach-omap1/Kconfig
>>> @@ -49,7 +49,7 @@ config ARCH_OMAP1_ANY
>>>         select ARCH_OMAP
>>>         def_bool ARCH_OMAP730 || ARCH_OMAP850 || ARCH_OMAP15XX || ARCH_OMAP16XX
>>>  
>>> -config ARCH_OMAP
>>> +config ARCH_OMAP1_ANY
>>
>> This patch can't be right - look at the first line of context above, you
>> have symbols that select ARCH_OMAP and you've just removed the
>> definition of ARCH_OMAP.
>
> Right, I misread this, I thought this was an 'if ARCH_OMAP' block
> instead of the Kconfig symbol. Part of the problem is now is
> possibly that there are two symbols with that name now, I'll have
> to try to reproduce the problem first and then see what caused it
> originally.

It seems the root cause was actually 804f7f19c2e2 ("fbdev: omap:
avoid using mach/*.h files"), where I started allowing building with
CONFIG_COMPILE_TEST, but this fails when ARCH_OMAP1_ANY is disabled,
as Randy correctly found.

We could tighten this gain by adding something like

     depends on ARCH_OMAP1_ANY || !OMAP_MUX

to still allow compile-testing but specifically avoiding the
broken case. I think we have the same problem in the pcmcia and
spi-uwire drivers, so maybe something like this would be
a better approach:

--- a/include/linux/soc/ti/omap1-mux.h
+++ b/include/linux/soc/ti/omap1-mux.h
@@ -302,7 +302,7 @@ enum omap1xxx_index {
 
 };
 
-#ifdef CONFIG_OMAP_MUX
+#if defined(CONFIG_OMAP_MUX) && defined(CONFIG_ARCH_OMAP1_ANY)
 extern int omap_cfg_reg(unsigned long reg_cfg);
 #else
 static inline int omap_cfg_reg(unsigned long reg_cfg) { return 0; }


      Arnd
  
Randy Dunlap Nov. 7, 2022, 12:55 a.m. UTC | #6
On 11/6/22 03:32, Arnd Bergmann wrote:
> On Sun, Nov 6, 2022, at 11:37, Arnd Bergmann wrote:
>> On Sun, Nov 6, 2022, at 11:26, Russell King (Oracle) wrote:
>>> On Sun, Nov 06, 2022 at 08:43:50AM +0100, Arnd Bergmann wrote:
>>>> --- a/arch/arm/mach-omap1/Kconfig
>>>> +++ b/arch/arm/mach-omap1/Kconfig
>>>> @@ -49,7 +49,7 @@ config ARCH_OMAP1_ANY
>>>>         select ARCH_OMAP
>>>>         def_bool ARCH_OMAP730 || ARCH_OMAP850 || ARCH_OMAP15XX || ARCH_OMAP16XX
>>>>  
>>>> -config ARCH_OMAP
>>>> +config ARCH_OMAP1_ANY
>>>
>>> This patch can't be right - look at the first line of context above, you
>>> have symbols that select ARCH_OMAP and you've just removed the
>>> definition of ARCH_OMAP.
>>
>> Right, I misread this, I thought this was an 'if ARCH_OMAP' block
>> instead of the Kconfig symbol. Part of the problem is now is
>> possibly that there are two symbols with that name now, I'll have
>> to try to reproduce the problem first and then see what caused it
>> originally.
> 
> It seems the root cause was actually 804f7f19c2e2 ("fbdev: omap:
> avoid using mach/*.h files"), where I started allowing building with
> CONFIG_COMPILE_TEST, but this fails when ARCH_OMAP1_ANY is disabled,
> as Randy correctly found.
> 
> We could tighten this gain by adding something like
> 
>      depends on ARCH_OMAP1_ANY || !OMAP_MUX
> 
> to still allow compile-testing but specifically avoiding the
> broken case. I think we have the same problem in the pcmcia and
> spi-uwire drivers, so maybe something like this would be
> a better approach:
> 
> --- a/include/linux/soc/ti/omap1-mux.h
> +++ b/include/linux/soc/ti/omap1-mux.h
> @@ -302,7 +302,7 @@ enum omap1xxx_index {
>  
>  };
>  
> -#ifdef CONFIG_OMAP_MUX
> +#if defined(CONFIG_OMAP_MUX) && defined(CONFIG_ARCH_OMAP1_ANY)
>  extern int omap_cfg_reg(unsigned long reg_cfg);
>  #else
>  static inline int omap_cfg_reg(unsigned long reg_cfg) { return 0; }

That seems to work. Push it into -next and see what happens. :)
  

Patch

diff -- a/arch/arm/mach-omap1/Kconfig b/arch/arm/mach-omap1/Kconfig
--- a/arch/arm/mach-omap1/Kconfig
+++ b/arch/arm/mach-omap1/Kconfig
@@ -47,7 +47,8 @@  config ARCH_OMAP16XX
 
 config ARCH_OMAP1_ANY
 	select ARCH_OMAP
-	def_bool ARCH_OMAP730 || ARCH_OMAP850 || ARCH_OMAP15XX || ARCH_OMAP16XX
+	def_bool ARCH_OMAP730 || ARCH_OMAP850 || ARCH_OMAP15XX || \
+		ARCH_OMAP16XX || ARCH_OMAP1
 
 config ARCH_OMAP
 	bool