diff mbox series

serial: parisc: GSC: fix build when PCI_LBA is not set

Message ID 20220213193903.8815-1-rdunlap@infradead.org (mailing list archive)
State Accepted, archived
Headers show
Series serial: parisc: GSC: fix build when PCI_LBA is not set | expand

Commit Message

Randy Dunlap Feb. 13, 2022, 7:39 p.m. UTC
There is a build error when using a kernel .config file from
'kernel test robot' for a different build problem:

hppa64-linux-ld: drivers/tty/serial/8250/8250_gsc.o: in function `.LC3':
(.data.rel.ro+0x18): undefined reference to `iosapic_serial_irq'

when:
  CONFIG_GSC=y
  CONFIG_SERIO_GSCPS2=y
  CONFIG_SERIAL_8250_GSC=y
  CONFIG_PCI is not set
    and hence PCI_LBA is not set.
  IOSAPIC depends on PCI_LBA, so IOSAPIC is not set/enabled.

Making SERIAL_8250_GSC depend on PCI_LBA prevents the build error.

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Reported-by: kernel test robot <lkp@intel.com>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
Cc: linux-parisc@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-serial@vger.kernel.org
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
---
 drivers/tty/serial/8250/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Helge Deller Feb. 13, 2022, 8:35 p.m. UTC | #1
Hi Randy,

On 2/13/22 20:39, Randy Dunlap wrote:
> There is a build error when using a kernel .config file from
> 'kernel test robot' for a different build problem:
>
> hppa64-linux-ld: drivers/tty/serial/8250/8250_gsc.o: in function `.LC3':
> (.data.rel.ro+0x18): undefined reference to `iosapic_serial_irq'
>
> when:
>   CONFIG_GSC=y
>   CONFIG_SERIO_GSCPS2=y
>   CONFIG_SERIAL_8250_GSC=y
>   CONFIG_PCI is not set
>     and hence PCI_LBA is not set.
>   IOSAPIC depends on PCI_LBA, so IOSAPIC is not set/enabled.
>
> Making SERIAL_8250_GSC depend on PCI_LBA prevents the build error.

It maybe makes the build error go away, but ...

> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Reported-by: kernel test robot <lkp@intel.com>
> Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
> Cc: Helge Deller <deller@gmx.de>
> Cc: linux-parisc@vger.kernel.org
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-serial@vger.kernel.org
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: Johan Hovold <johan@kernel.org>
> ---
>  drivers/tty/serial/8250/Kconfig |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- linux-next-20220211.orig/drivers/tty/serial/8250/Kconfig
> +++ linux-next-20220211/drivers/tty/serial/8250/Kconfig
> @@ -118,7 +118,7 @@ config SERIAL_8250_CONSOLE
>
>  config SERIAL_8250_GSC
>  	tristate
> -	depends on SERIAL_8250 && GSC
> +	depends on SERIAL_8250 && GSC && PCI_LBA
>  	default SERIAL_8250

The serial device is on the GSC bus, so if you make it
dependend on the PCI bus it will not be useable on machines
which only have a GSC bus...

We need another patch.
Do you have a link to the build error?

Helge
Randy Dunlap Feb. 13, 2022, 9:07 p.m. UTC | #2
On 2/13/22 12:35, Helge Deller wrote:
> Hi Randy,
> 
> On 2/13/22 20:39, Randy Dunlap wrote:
>> There is a build error when using a kernel .config file from
>> 'kernel test robot' for a different build problem:
>>
>> hppa64-linux-ld: drivers/tty/serial/8250/8250_gsc.o: in function `.LC3':
>> (.data.rel.ro+0x18): undefined reference to `iosapic_serial_irq'
>>
>> when:
>>   CONFIG_GSC=y
>>   CONFIG_SERIO_GSCPS2=y
>>   CONFIG_SERIAL_8250_GSC=y
>>   CONFIG_PCI is not set
>>     and hence PCI_LBA is not set.
>>   IOSAPIC depends on PCI_LBA, so IOSAPIC is not set/enabled.
>>
>> Making SERIAL_8250_GSC depend on PCI_LBA prevents the build error.
> 
> It maybe makes the build error go away, but ...
> 
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
>> Cc: Helge Deller <deller@gmx.de>
>> Cc: linux-parisc@vger.kernel.org
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: linux-serial@vger.kernel.org
>> Cc: Jiri Slaby <jirislaby@kernel.org>
>> Cc: Johan Hovold <johan@kernel.org>
>> ---
>>  drivers/tty/serial/8250/Kconfig |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> --- linux-next-20220211.orig/drivers/tty/serial/8250/Kconfig
>> +++ linux-next-20220211/drivers/tty/serial/8250/Kconfig
>> @@ -118,7 +118,7 @@ config SERIAL_8250_CONSOLE
>>
>>  config SERIAL_8250_GSC
>>  	tristate
>> -	depends on SERIAL_8250 && GSC
>> +	depends on SERIAL_8250 && GSC && PCI_LBA
>>  	default SERIAL_8250
> 
> The serial device is on the GSC bus, so if you make it
> dependend on the PCI bus it will not be useable on machines
> which only have a GSC bus...
> 
> We need another patch.
> Do you have a link to the build error?


No, it's from the other build error that you just replied to,
where the incorrect compiler was used.

I'll recheck it and reconsider what to do, if anything.

thanks.
Helge Deller Feb. 13, 2022, 10:15 p.m. UTC | #3
On 2/13/22 22:07, Randy Dunlap wrote:
>
>
> On 2/13/22 12:35, Helge Deller wrote:
>> Hi Randy,
>>
>> On 2/13/22 20:39, Randy Dunlap wrote:
>>> There is a build error when using a kernel .config file from
>>> 'kernel test robot' for a different build problem:
>>>
>>> hppa64-linux-ld: drivers/tty/serial/8250/8250_gsc.o: in function `.LC3':
>>> (.data.rel.ro+0x18): undefined reference to `iosapic_serial_irq'
>>>
>>> when:
>>>   CONFIG_GSC=y
>>>   CONFIG_SERIO_GSCPS2=y
>>>   CONFIG_SERIAL_8250_GSC=y
>>>   CONFIG_PCI is not set
>>>     and hence PCI_LBA is not set.
>>>   IOSAPIC depends on PCI_LBA, so IOSAPIC is not set/enabled.
>>>
>>> Making SERIAL_8250_GSC depend on PCI_LBA prevents the build error.
>>
>> It maybe makes the build error go away, but ...
>>
>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
>>> Cc: Helge Deller <deller@gmx.de>
>>> Cc: linux-parisc@vger.kernel.org
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: linux-serial@vger.kernel.org
>>> Cc: Jiri Slaby <jirislaby@kernel.org>
>>> Cc: Johan Hovold <johan@kernel.org>
>>> ---
>>>  drivers/tty/serial/8250/Kconfig |    2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> --- linux-next-20220211.orig/drivers/tty/serial/8250/Kconfig
>>> +++ linux-next-20220211/drivers/tty/serial/8250/Kconfig
>>> @@ -118,7 +118,7 @@ config SERIAL_8250_CONSOLE
>>>
>>>  config SERIAL_8250_GSC
>>>  	tristate
>>> -	depends on SERIAL_8250 && GSC
>>> +	depends on SERIAL_8250 && GSC && PCI_LBA
>>>  	default SERIAL_8250
>>
>> The serial device is on the GSC bus, so if you make it
>> dependend on the PCI bus it will not be useable on machines
>> which only have a GSC bus...
>>
>> We need another patch.
>> Do you have a link to the build error?
>
>
> No, it's from the other build error that you just replied to,
> where the incorrect compiler was used.
>
> I'll recheck it and reconsider what to do, if anything.

Ok, thank you!

By the way, I just sent another patch (and added it to the parisc for-next tree)
which should at least give a better error message if someone uses the wrong compiler:
https://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux.git/commit/?h=for-next&id=b160628e9ebcdc85d0db9d7f423c26b3c7c179d0

Helge
Randy Dunlap Feb. 14, 2022, 12:15 a.m. UTC | #4
Hi,

On 2/13/22 14:15, Helge Deller wrote:
> On 2/13/22 22:07, Randy Dunlap wrote:
>>
>>
>> On 2/13/22 12:35, Helge Deller wrote:
>>> Hi Randy,
>>>
>>> On 2/13/22 20:39, Randy Dunlap wrote:
>>>> There is a build error when using a kernel .config file from
>>>> 'kernel test robot' for a different build problem:
>>>>
>>>> hppa64-linux-ld: drivers/tty/serial/8250/8250_gsc.o: in function `.LC3':
>>>> (.data.rel.ro+0x18): undefined reference to `iosapic_serial_irq'
>>>>
>>>> when:
>>>>   CONFIG_GSC=y
>>>>   CONFIG_SERIO_GSCPS2=y
>>>>   CONFIG_SERIAL_8250_GSC=y
>>>>   CONFIG_PCI is not set
>>>>     and hence PCI_LBA is not set.
>>>>   IOSAPIC depends on PCI_LBA, so IOSAPIC is not set/enabled.
>>>>
>>>> Making SERIAL_8250_GSC depend on PCI_LBA prevents the build error.
>>>
>>> It maybe makes the build error go away, but ...
>>>
>>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>> Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
>>>> Cc: Helge Deller <deller@gmx.de>
>>>> Cc: linux-parisc@vger.kernel.org
>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>> Cc: linux-serial@vger.kernel.org
>>>> Cc: Jiri Slaby <jirislaby@kernel.org>
>>>> Cc: Johan Hovold <johan@kernel.org>
>>>> ---
>>>>  drivers/tty/serial/8250/Kconfig |    2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> --- linux-next-20220211.orig/drivers/tty/serial/8250/Kconfig
>>>> +++ linux-next-20220211/drivers/tty/serial/8250/Kconfig
>>>> @@ -118,7 +118,7 @@ config SERIAL_8250_CONSOLE
>>>>
>>>>  config SERIAL_8250_GSC
>>>>  	tristate
>>>> -	depends on SERIAL_8250 && GSC
>>>> +	depends on SERIAL_8250 && GSC && PCI_LBA
>>>>  	default SERIAL_8250
>>>
>>> The serial device is on the GSC bus, so if you make it
>>> dependend on the PCI bus it will not be useable on machines
>>> which only have a GSC bus...
>>>
>>> We need another patch.
>>> Do you have a link to the build error?
>>
>>
>> No, it's from the other build error that you just replied to,
>> where the incorrect compiler was used.
>>
>> I'll recheck it and reconsider what to do, if anything.
> 
> Ok, thank you!

I dunno what to do. This:

#ifdef CONFIG_64BIT
	if (!dev->irq && (dev->id.sversion == 0xad))
		dev->irq = iosapic_serial_irq(dev);
#endif

makes it look like 64BIT requires IOSAPIC (hence PCI_LBA).


> By the way, I just sent another patch (and added it to the parisc for-next tree)
> which should at least give a better error message if someone uses the wrong compiler:
> https://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux.git/commit/?h=for-next&id=b160628e9ebcdc85d0db9d7f423c26b3c7c179d0

Yes, I applied that, but that's not the problem here.
It is a 64BIT config and parisc64 compiler.
Helge Deller Feb. 14, 2022, 12:05 p.m. UTC | #5
On 2/14/22 01:15, Randy Dunlap wrote:
> Hi,
>
> On 2/13/22 14:15, Helge Deller wrote:
>> On 2/13/22 22:07, Randy Dunlap wrote:
>>>
>>>
>>> On 2/13/22 12:35, Helge Deller wrote:
>>>> Hi Randy,
>>>>
>>>> On 2/13/22 20:39, Randy Dunlap wrote:
>>>>> There is a build error when using a kernel .config file from
>>>>> 'kernel test robot' for a different build problem:
>>>>>
>>>>> hppa64-linux-ld: drivers/tty/serial/8250/8250_gsc.o: in function `.LC3':
>>>>> (.data.rel.ro+0x18): undefined reference to `iosapic_serial_irq'
>>>>>
>>>>> when:
>>>>>   CONFIG_GSC=y
>>>>>   CONFIG_SERIO_GSCPS2=y
>>>>>   CONFIG_SERIAL_8250_GSC=y
>>>>>   CONFIG_PCI is not set
>>>>>     and hence PCI_LBA is not set.
>>>>>   IOSAPIC depends on PCI_LBA, so IOSAPIC is not set/enabled.
>>>>>
>>>>> Making SERIAL_8250_GSC depend on PCI_LBA prevents the build error.
>>>>
>>>> It maybe makes the build error go away, but ...
>>>>
>>>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>>> Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
>>>>> Cc: Helge Deller <deller@gmx.de>
>>>>> Cc: linux-parisc@vger.kernel.org
>>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>>> Cc: linux-serial@vger.kernel.org
>>>>> Cc: Jiri Slaby <jirislaby@kernel.org>
>>>>> Cc: Johan Hovold <johan@kernel.org>
>>>>> ---
>>>>>  drivers/tty/serial/8250/Kconfig |    2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> --- linux-next-20220211.orig/drivers/tty/serial/8250/Kconfig
>>>>> +++ linux-next-20220211/drivers/tty/serial/8250/Kconfig
>>>>> @@ -118,7 +118,7 @@ config SERIAL_8250_CONSOLE
>>>>>
>>>>>  config SERIAL_8250_GSC
>>>>>  	tristate
>>>>> -	depends on SERIAL_8250 && GSC
>>>>> +	depends on SERIAL_8250 && GSC && PCI_LBA
>>>>>  	default SERIAL_8250
>>>>
>>>> The serial device is on the GSC bus, so if you make it
>>>> dependend on the PCI bus it will not be useable on machines
>>>> which only have a GSC bus...
>>>>
>>>> We need another patch.
>>>> Do you have a link to the build error?
>>>
>>>
>>> No, it's from the other build error that you just replied to,
>>> where the incorrect compiler was used.
>>>
>>> I'll recheck it and reconsider what to do, if anything.
>>
>> Ok, thank you!
>
> I dunno what to do. This:
>
> #ifdef CONFIG_64BIT
> 	if (!dev->irq && (dev->id.sversion == 0xad))
> 		dev->irq = iosapic_serial_irq(dev);
> #endif
>
> makes it look like 64BIT requires IOSAPIC (hence PCI_LBA).

Although I think all 64bit machines have a PCI bus, the better
fix is that the driver should only call iosapic_serial_irq(dev)
if CONFIG_IOSAPIC is set. This patch fixes the build:

-#ifdef CONFIG_64BIT
+#ifdef CONFIG_IOSAPIC
        if (!dev->irq && (dev->id.sversion == 0xad))
                dev->irq = iosapic_serial_irq(dev);
 #endif

Will you send an updated patch?

Thanks!
Helge
Helge Deller Feb. 14, 2022, 12:24 p.m. UTC | #6
On 2/14/22 13:05, Helge Deller wrote:
> On 2/14/22 01:15, Randy Dunlap wrote:
>> Hi,
>>
>> On 2/13/22 14:15, Helge Deller wrote:
>>> On 2/13/22 22:07, Randy Dunlap wrote:
>>>>
>>>>
>>>> On 2/13/22 12:35, Helge Deller wrote:
>>>>> Hi Randy,
>>>>>
>>>>> On 2/13/22 20:39, Randy Dunlap wrote:
>>>>>> There is a build error when using a kernel .config file from
>>>>>> 'kernel test robot' for a different build problem:
>>>>>>
>>>>>> hppa64-linux-ld: drivers/tty/serial/8250/8250_gsc.o: in function `.LC3':
>>>>>> (.data.rel.ro+0x18): undefined reference to `iosapic_serial_irq'
>>>>>>
>>>>>> when:
>>>>>>   CONFIG_GSC=y
>>>>>>   CONFIG_SERIO_GSCPS2=y
>>>>>>   CONFIG_SERIAL_8250_GSC=y
>>>>>>   CONFIG_PCI is not set
>>>>>>     and hence PCI_LBA is not set.
>>>>>>   IOSAPIC depends on PCI_LBA, so IOSAPIC is not set/enabled.
>>>>>>
>>>>>> Making SERIAL_8250_GSC depend on PCI_LBA prevents the build error.
>>>>>
>>>>> It maybe makes the build error go away, but ...
>>>>>
>>>>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>>>> Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
>>>>>> Cc: Helge Deller <deller@gmx.de>
>>>>>> Cc: linux-parisc@vger.kernel.org
>>>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>>>> Cc: linux-serial@vger.kernel.org
>>>>>> Cc: Jiri Slaby <jirislaby@kernel.org>
>>>>>> Cc: Johan Hovold <johan@kernel.org>
>>>>>> ---
>>>>>>  drivers/tty/serial/8250/Kconfig |    2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> --- linux-next-20220211.orig/drivers/tty/serial/8250/Kconfig
>>>>>> +++ linux-next-20220211/drivers/tty/serial/8250/Kconfig
>>>>>> @@ -118,7 +118,7 @@ config SERIAL_8250_CONSOLE
>>>>>>
>>>>>>  config SERIAL_8250_GSC
>>>>>>  	tristate
>>>>>> -	depends on SERIAL_8250 && GSC
>>>>>> +	depends on SERIAL_8250 && GSC && PCI_LBA
>>>>>>  	default SERIAL_8250
>>>>>
>>>>> The serial device is on the GSC bus, so if you make it
>>>>> dependend on the PCI bus it will not be useable on machines
>>>>> which only have a GSC bus...
>>>>>
>>>>> We need another patch.
>>>>> Do you have a link to the build error?
>>>>
>>>>
>>>> No, it's from the other build error that you just replied to,
>>>> where the incorrect compiler was used.
>>>>
>>>> I'll recheck it and reconsider what to do, if anything.
>>>
>>> Ok, thank you!
>>
>> I dunno what to do. This:
>>
>> #ifdef CONFIG_64BIT
>> 	if (!dev->irq && (dev->id.sversion == 0xad))
>> 		dev->irq = iosapic_serial_irq(dev);
>> #endif
>>
>> makes it look like 64BIT requires IOSAPIC (hence PCI_LBA).
>
> Although I think all 64bit machines have a PCI bus, the better
> fix is that the driver should only call iosapic_serial_irq(dev)
> if CONFIG_IOSAPIC is set. This patch fixes the build:
>
> -#ifdef CONFIG_64BIT
> +#ifdef CONFIG_IOSAPIC
>         if (!dev->irq && (dev->id.sversion == 0xad))
>                 dev->irq = iosapic_serial_irq(dev);
>  #endif

That was not fully correct.
It needs to be:

#if defined(CONFIG_64BIT) && defined(CONFIG_IOSAPIC)

Otherwise you'll get an undefined reference in the 32-bit build.

Helge
Randy Dunlap Feb. 14, 2022, 5:47 p.m. UTC | #7
Hi Helge,

On 2/14/22 04:24, Helge Deller wrote:
> On 2/14/22 13:05, Helge Deller wrote:
>> On 2/14/22 01:15, Randy Dunlap wrote:
>>> Hi,
>>>
>>> On 2/13/22 14:15, Helge Deller wrote:
>>>> On 2/13/22 22:07, Randy Dunlap wrote:
>>>>>
>>>>>
>>>>> On 2/13/22 12:35, Helge Deller wrote:
>>>>>> Hi Randy,
>>>>>>
>>>>>> On 2/13/22 20:39, Randy Dunlap wrote:
>>>>>>> There is a build error when using a kernel .config file from
>>>>>>> 'kernel test robot' for a different build problem:
>>>>>>>
>>>>>>> hppa64-linux-ld: drivers/tty/serial/8250/8250_gsc.o: in function `.LC3':
>>>>>>> (.data.rel.ro+0x18): undefined reference to `iosapic_serial_irq'
>>>>>>>
>>>>>>> when:
>>>>>>>   CONFIG_GSC=y
>>>>>>>   CONFIG_SERIO_GSCPS2=y
>>>>>>>   CONFIG_SERIAL_8250_GSC=y
>>>>>>>   CONFIG_PCI is not set
>>>>>>>     and hence PCI_LBA is not set.
>>>>>>>   IOSAPIC depends on PCI_LBA, so IOSAPIC is not set/enabled.
>>>>>>>
>>>>>>> Making SERIAL_8250_GSC depend on PCI_LBA prevents the build error.
>>>>>>
>>>>>> It maybe makes the build error go away, but ...
>>>>>>
>>>>>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>>>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>>>>> Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
>>>>>>> Cc: Helge Deller <deller@gmx.de>
>>>>>>> Cc: linux-parisc@vger.kernel.org
>>>>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>>>>> Cc: linux-serial@vger.kernel.org
>>>>>>> Cc: Jiri Slaby <jirislaby@kernel.org>
>>>>>>> Cc: Johan Hovold <johan@kernel.org>
>>>>>>> ---
>>>>>>>  drivers/tty/serial/8250/Kconfig |    2 +-
>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> --- linux-next-20220211.orig/drivers/tty/serial/8250/Kconfig
>>>>>>> +++ linux-next-20220211/drivers/tty/serial/8250/Kconfig
>>>>>>> @@ -118,7 +118,7 @@ config SERIAL_8250_CONSOLE
>>>>>>>
>>>>>>>  config SERIAL_8250_GSC
>>>>>>>  	tristate
>>>>>>> -	depends on SERIAL_8250 && GSC
>>>>>>> +	depends on SERIAL_8250 && GSC && PCI_LBA
>>>>>>>  	default SERIAL_8250
>>>>>>
>>>>>> The serial device is on the GSC bus, so if you make it
>>>>>> dependend on the PCI bus it will not be useable on machines
>>>>>> which only have a GSC bus...
>>>>>>
>>>>>> We need another patch.
>>>>>> Do you have a link to the build error?
>>>>>
>>>>>
>>>>> No, it's from the other build error that you just replied to,
>>>>> where the incorrect compiler was used.
>>>>>
>>>>> I'll recheck it and reconsider what to do, if anything.
>>>>
>>>> Ok, thank you!
>>>
>>> I dunno what to do. This:
>>>
>>> #ifdef CONFIG_64BIT
>>> 	if (!dev->irq && (dev->id.sversion == 0xad))
>>> 		dev->irq = iosapic_serial_irq(dev);
>>> #endif
>>>
>>> makes it look like 64BIT requires IOSAPIC (hence PCI_LBA).
>>
>> Although I think all 64bit machines have a PCI bus, the better
>> fix is that the driver should only call iosapic_serial_irq(dev)
>> if CONFIG_IOSAPIC is set. This patch fixes the build:
>>
>> -#ifdef CONFIG_64BIT
>> +#ifdef CONFIG_IOSAPIC
>>         if (!dev->irq && (dev->id.sversion == 0xad))
>>                 dev->irq = iosapic_serial_irq(dev);
>>  #endif
> 
> That was not fully correct.
> It needs to be:
> 
> #if defined(CONFIG_64BIT) && defined(CONFIG_IOSAPIC)
> 
> Otherwise you'll get an undefined reference in the 32-bit build.

Sure, I can send such a patch.
I would have used a bigger hammer and done something like

	depends on IOSAPIC if 64BIT


Just for info, how would dev->irq be set for CONFIG_64BIT
when CONFIG_IOSAPIC is not set?


thanks.
Helge Deller Feb. 14, 2022, 6:29 p.m. UTC | #8
On 2/14/22 18:47, Randy Dunlap wrote:
> Hi Helge,
>
> On 2/14/22 04:24, Helge Deller wrote:
>> On 2/14/22 13:05, Helge Deller wrote:
>>> On 2/14/22 01:15, Randy Dunlap wrote:
>>>> Hi,
>>>>
>>>> On 2/13/22 14:15, Helge Deller wrote:
>>>>> On 2/13/22 22:07, Randy Dunlap wrote:
>>>>>>
>>>>>>
>>>>>> On 2/13/22 12:35, Helge Deller wrote:
>>>>>>> Hi Randy,
>>>>>>>
>>>>>>> On 2/13/22 20:39, Randy Dunlap wrote:
>>>>>>>> There is a build error when using a kernel .config file from
>>>>>>>> 'kernel test robot' for a different build problem:
>>>>>>>>
>>>>>>>> hppa64-linux-ld: drivers/tty/serial/8250/8250_gsc.o: in function `.LC3':
>>>>>>>> (.data.rel.ro+0x18): undefined reference to `iosapic_serial_irq'
>>>>>>>>
>>>>>>>> when:
>>>>>>>>   CONFIG_GSC=y
>>>>>>>>   CONFIG_SERIO_GSCPS2=y
>>>>>>>>   CONFIG_SERIAL_8250_GSC=y
>>>>>>>>   CONFIG_PCI is not set
>>>>>>>>     and hence PCI_LBA is not set.
>>>>>>>>   IOSAPIC depends on PCI_LBA, so IOSAPIC is not set/enabled.
>>>>>>>>
>>>>>>>> Making SERIAL_8250_GSC depend on PCI_LBA prevents the build error.
>>>>>>>
>>>>>>> It maybe makes the build error go away, but ...
>>>>>>>
>>>>>>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>>>>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>>>>>> Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
>>>>>>>> Cc: Helge Deller <deller@gmx.de>
>>>>>>>> Cc: linux-parisc@vger.kernel.org
>>>>>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>>>>>> Cc: linux-serial@vger.kernel.org
>>>>>>>> Cc: Jiri Slaby <jirislaby@kernel.org>
>>>>>>>> Cc: Johan Hovold <johan@kernel.org>
>>>>>>>> ---
>>>>>>>>  drivers/tty/serial/8250/Kconfig |    2 +-
>>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> --- linux-next-20220211.orig/drivers/tty/serial/8250/Kconfig
>>>>>>>> +++ linux-next-20220211/drivers/tty/serial/8250/Kconfig
>>>>>>>> @@ -118,7 +118,7 @@ config SERIAL_8250_CONSOLE
>>>>>>>>
>>>>>>>>  config SERIAL_8250_GSC
>>>>>>>>  	tristate
>>>>>>>> -	depends on SERIAL_8250 && GSC
>>>>>>>> +	depends on SERIAL_8250 && GSC && PCI_LBA
>>>>>>>>  	default SERIAL_8250
>>>>>>>
>>>>>>> The serial device is on the GSC bus, so if you make it
>>>>>>> dependend on the PCI bus it will not be useable on machines
>>>>>>> which only have a GSC bus...
>>>>>>>
>>>>>>> We need another patch.
>>>>>>> Do you have a link to the build error?
>>>>>>
>>>>>>
>>>>>> No, it's from the other build error that you just replied to,
>>>>>> where the incorrect compiler was used.
>>>>>>
>>>>>> I'll recheck it and reconsider what to do, if anything.
>>>>>
>>>>> Ok, thank you!
>>>>
>>>> I dunno what to do. This:
>>>>
>>>> #ifdef CONFIG_64BIT
>>>> 	if (!dev->irq && (dev->id.sversion == 0xad))
>>>> 		dev->irq = iosapic_serial_irq(dev);
>>>> #endif
>>>>
>>>> makes it look like 64BIT requires IOSAPIC (hence PCI_LBA).
>>>
>>> Although I think all 64bit machines have a PCI bus, the better
>>> fix is that the driver should only call iosapic_serial_irq(dev)
>>> if CONFIG_IOSAPIC is set. This patch fixes the build:
>>>
>>> -#ifdef CONFIG_64BIT
>>> +#ifdef CONFIG_IOSAPIC
>>>         if (!dev->irq && (dev->id.sversion == 0xad))
>>>                 dev->irq = iosapic_serial_irq(dev);
>>>  #endif
>>
>> That was not fully correct.
>> It needs to be:
>>
>> #if defined(CONFIG_64BIT) && defined(CONFIG_IOSAPIC)
>>
>> Otherwise you'll get an undefined reference in the 32-bit build.
>
> Sure, I can send such a patch.

Thanks!
I see you sent it in a seperate mail.
I can take it through the parisc tree.

> I would have used a bigger hammer and done something like
>
> 	depends on IOSAPIC if 64BIT
>
> Just for info, how would dev->irq be set for CONFIG_64BIT
> when CONFIG_IOSAPIC is not set?

All found devices in the parisc system are in a device table.
The serial driver scans those against the ones listed in
serial_tbl[] and lasi_tbl[], and hand over the parisc_device
pointer which usually has the irq set.
In case the device is connected via iosapic and hasn't an irq
already set, iosapic_serial_irq() is called to find the irq.
The
 	depends on IOSAPIC if 64BIT
would have dropped *all* serial ports on 64bit kernels, even
those which are on the GSC bus.

Helge
diff mbox series

Patch

--- linux-next-20220211.orig/drivers/tty/serial/8250/Kconfig
+++ linux-next-20220211/drivers/tty/serial/8250/Kconfig
@@ -118,7 +118,7 @@  config SERIAL_8250_CONSOLE
 
 config SERIAL_8250_GSC
 	tristate
-	depends on SERIAL_8250 && GSC
+	depends on SERIAL_8250 && GSC && PCI_LBA
 	default SERIAL_8250
 
 config SERIAL_8250_DMA