| Message ID | 20220213193903.8815-1-rdunlap@infradead.org |
|---|---|
| State | New |
| Headers | show |
| Series | serial: parisc: GSC: fix build when PCI_LBA is not set | expand |
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
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.
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
--- 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
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(-)