| Message ID | 20230406160548.25721-1-rdunlap@infradead.org |
|---|---|
| State | New |
| Headers | show |
| Series | parport_pc: don't allow driver for SPARC32 | expand |
On Thu, Apr 06, 2023 at 09:05:48AM -0700, Randy Dunlap wrote: > arch/sparc/kernel/ebus.o is only built for SPARC64. > ns87303_lock is only built for SPARC64. > arch/sparc/include/asm/parport.h says that it is for sparc64. > Various documentation on the internet says that ebus is for UltraSPARC > systems (64-bit). > > Therefore don't allow PARPORT_PC to be built for SPARC32. > > Fixes these build errors on SPARC32: > > ERROR: modpost: "ebus_dma_irq_enable" [drivers/parport/parport_pc.ko] undefined! > ERROR: modpost: "ebus_dma_unregister" [drivers/parport/parport_pc.ko] undefined! > ERROR: modpost: "ebus_dma_register" [drivers/parport/parport_pc.ko] undefined! > ERROR: modpost: "ns87303_lock" [drivers/parport/parport_pc.ko] undefined! > ERROR: modpost: "ebus_dma_enable" [drivers/parport/parport_pc.ko] undefined! > ERROR: modpost: "ebus_dma_prepare" [drivers/parport/parport_pc.ko] undefined! > ERROR: modpost: "ebus_dma_request" [drivers/parport/parport_pc.ko] undefined! > ERROR: modpost: "ebus_dma_residue" [drivers/parport/parport_pc.ko] undefined! > > Fixes: 66bcd06099bb ("parport_pc: Also enable driver for PCI systems") > Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com> > Cc: Maciej W. Rozycki <macro@orcam.me.uk> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: sparclinux@vger.kernel.org > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: linux-parport@lists.infradead.org Acked-by: Sam Ravnborg <sam@ravnborg.org> > --- > drivers/parport/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff -- a/drivers/parport/Kconfig b/drivers/parport/Kconfig > --- a/drivers/parport/Kconfig > +++ b/drivers/parport/Kconfig > @@ -42,7 +42,7 @@ if PARPORT > > config PARPORT_PC > tristate "PC-style hardware" > - depends on ARCH_MIGHT_HAVE_PC_PARPORT || (PCI && !S390) > + depends on ARCH_MIGHT_HAVE_PC_PARPORT || (PCI && !S390 && !SPARC32) > help > You should say Y here if you have a PC-style parallel port. All > IBM PC compatible computers and some Alphas have PC-style
On Thu, 6 Apr 2023, Randy Dunlap wrote: > arch/sparc/kernel/ebus.o is only built for SPARC64. > ns87303_lock is only built for SPARC64. > arch/sparc/include/asm/parport.h says that it is for sparc64. > Various documentation on the internet says that ebus is for UltraSPARC > systems (64-bit). > > Therefore don't allow PARPORT_PC to be built for SPARC32. This looks completely wrong to me, any ordinary PCI parallel port card ought just to work as long as you have PCI (S390 is special I'm told). What needs to be done is AFAICT just making `parport_pc_find_nonpci_ports' in arch/sparc/include/asm/parport.h SPARC64-specific, i.e.: static int parport_pc_find_nonpci_ports(int autoirq, int autodma) { return (IS_ENABLED(CONFIG_SPARC64) && platform_driver_register(&ecpp_driver)); } or suchlike and let the optimiser get rid of all the unwanted unsupported stuff. Maciej
On 4/6/23 12:49, Maciej W. Rozycki wrote: > On Thu, 6 Apr 2023, Randy Dunlap wrote: > >> arch/sparc/kernel/ebus.o is only built for SPARC64. >> ns87303_lock is only built for SPARC64. >> arch/sparc/include/asm/parport.h says that it is for sparc64. >> Various documentation on the internet says that ebus is for UltraSPARC >> systems (64-bit). >> >> Therefore don't allow PARPORT_PC to be built for SPARC32. > > This looks completely wrong to me, any ordinary PCI parallel port card > ought just to work as long as you have PCI (S390 is special I'm told). > What needs to be done is AFAICT just making `parport_pc_find_nonpci_ports' > in arch/sparc/include/asm/parport.h SPARC64-specific, i.e.: > > static int parport_pc_find_nonpci_ports(int autoirq, int autodma) > { > return (IS_ENABLED(CONFIG_SPARC64) && > platform_driver_register(&ecpp_driver)); > } > > or suchlike and let the optimiser get rid of all the unwanted unsupported > stuff. Fine, please go ahead with that solution.
Hi Maciej, On Thu, Apr 06, 2023 at 08:49:50PM +0100, Maciej W. Rozycki wrote: > On Thu, 6 Apr 2023, Randy Dunlap wrote: > > > arch/sparc/kernel/ebus.o is only built for SPARC64. > > ns87303_lock is only built for SPARC64. > > arch/sparc/include/asm/parport.h says that it is for sparc64. > > Various documentation on the internet says that ebus is for UltraSPARC > > systems (64-bit). > > > > Therefore don't allow PARPORT_PC to be built for SPARC32. > > This looks completely wrong to me, any ordinary PCI parallel port card > ought just to work as long as you have PCI (S390 is special I'm told). > What needs to be done is AFAICT just making `parport_pc_find_nonpci_ports' > in arch/sparc/include/asm/parport.h SPARC64-specific, i.e.: > > static int parport_pc_find_nonpci_ports(int autoirq, int autodma) > { > return (IS_ENABLED(CONFIG_SPARC64) && > platform_driver_register(&ecpp_driver)); > } > > or suchlike and let the optimiser get rid of all the unwanted unsupported > stuff. arch/sparc/include/asm/parport.h is sparc64 specific - and it will result in the wrong result if it is pulled in for sparc32 builds. This is what we see today. Randy's suggestion is fine, as we avoid building parport support for sparc32. If someone shows up and need parport support for sparc32 then we could look into how to enable it. Until then, we are better helped avoiding building the driver. Hence, my ack on the patch from Randy. Sam
Hi Sam, > > This looks completely wrong to me, any ordinary PCI parallel port card > > ought just to work as long as you have PCI (S390 is special I'm told). > > What needs to be done is AFAICT just making `parport_pc_find_nonpci_ports' > > in arch/sparc/include/asm/parport.h SPARC64-specific, i.e.: > > > > static int parport_pc_find_nonpci_ports(int autoirq, int autodma) > > { > > return (IS_ENABLED(CONFIG_SPARC64) && > > platform_driver_register(&ecpp_driver)); > > } > > > > or suchlike and let the optimiser get rid of all the unwanted unsupported > > stuff. > > arch/sparc/include/asm/parport.h is sparc64 specific - and it will > result in the wrong result if it is pulled in for sparc32 builds. > This is what we see today. > > Randy's suggestion is fine, as we avoid building parport support > for sparc32. If someone shows up and need parport support > for sparc32 then we could look into how to enable it. > Until then, we are better helped avoiding building the driver. I disagree. Why artificially prevent perfectly good hardware from working with a perfectly good driver especially as the fix is just a trivial exercise? And I offered a solution. I don't have a SPARC toolchain handy or I could even try and build it (but I'm sure there are many people around who can do it without bending backwards). NB conversely we have plenty of useless irrelevant stuff presented in configration even if it genuinely makes no sense and won't ever be used for the given platform (e.g. some Intel CPU management stuff shown for RISC-V or even DEC Alpha systems). Maciej
On 4/6/23 14:01, Maciej W. Rozycki wrote: > Hi Sam, > >>> This looks completely wrong to me, any ordinary PCI parallel port card >>> ought just to work as long as you have PCI (S390 is special I'm told). >>> What needs to be done is AFAICT just making `parport_pc_find_nonpci_ports' >>> in arch/sparc/include/asm/parport.h SPARC64-specific, i.e.: >>> >>> static int parport_pc_find_nonpci_ports(int autoirq, int autodma) >>> { >>> return (IS_ENABLED(CONFIG_SPARC64) && >>> platform_driver_register(&ecpp_driver)); >>> } >>> >>> or suchlike and let the optimiser get rid of all the unwanted unsupported >>> stuff. >> >> arch/sparc/include/asm/parport.h is sparc64 specific - and it will >> result in the wrong result if it is pulled in for sparc32 builds. >> This is what we see today. >> >> Randy's suggestion is fine, as we avoid building parport support >> for sparc32. If someone shows up and need parport support >> for sparc32 then we could look into how to enable it. >> Until then, we are better helped avoiding building the driver. > > I disagree. Why artificially prevent perfectly good hardware from > working with a perfectly good driver especially as the fix is just a > trivial exercise? And I offered a solution. > > I don't have a SPARC toolchain handy or I could even try and build it > (but I'm sure there are many people around who can do it without bending > backwards). https://mirrors.edge.kernel.org/pub/tools/crosstool/
Hi Maciej, On Thu, Apr 06, 2023 at 10:01:16PM +0100, Maciej W. Rozycki wrote: > Hi Sam, > > > > This looks completely wrong to me, any ordinary PCI parallel port card > > > ought just to work as long as you have PCI (S390 is special I'm told). > > > What needs to be done is AFAICT just making `parport_pc_find_nonpci_ports' > > > in arch/sparc/include/asm/parport.h SPARC64-specific, i.e.: > > > > > > static int parport_pc_find_nonpci_ports(int autoirq, int autodma) > > > { > > > return (IS_ENABLED(CONFIG_SPARC64) && > > > platform_driver_register(&ecpp_driver)); > > > } > > > > > > or suchlike and let the optimiser get rid of all the unwanted unsupported > > > stuff. > > > > arch/sparc/include/asm/parport.h is sparc64 specific - and it will > > result in the wrong result if it is pulled in for sparc32 builds. > > This is what we see today. > > > > Randy's suggestion is fine, as we avoid building parport support > > for sparc32. If someone shows up and need parport support > > for sparc32 then we could look into how to enable it. > > Until then, we are better helped avoiding building the driver. > > I disagree. Why artificially prevent perfectly good hardware from > working with a perfectly good driver especially as the fix is just a > trivial exercise? And I offered a solution. There is no sparc32 with a PC style parallel port, so the parport_pc have no value for a sparc32 machine. Some sparc Ultra have PC style parallel ports - but this is sparc64 machines and they are covered. The sparc32 machines have the parport_sunbpp driver for their parallel port. An alternative fix, and better I think, would be to audit all archs and let the relevant ones select ARCH_MIGHT_HAVE_PC_PARPORT, so we avoided the ugly "|| (PCI && !S390 && !SPARC32)" case for PARPORT_PC. Sam
On Fri, 7 Apr 2023, Sam Ravnborg wrote: > > > Randy's suggestion is fine, as we avoid building parport support > > > for sparc32. If someone shows up and need parport support > > > for sparc32 then we could look into how to enable it. > > > Until then, we are better helped avoiding building the driver. > > > > I disagree. Why artificially prevent perfectly good hardware from > > working with a perfectly good driver especially as the fix is just a > > trivial exercise? And I offered a solution. > > There is no sparc32 with a PC style parallel port, so the parport_pc > have no value for a sparc32 machine. There are PC-style PCI (and PCIe) parallel ports in the form of option cards being sold; I have one in my RISC-V machine (and I had to go through the hassle of figuring out why the heck I am not able to select the driver in configuration; a situation analogous to what Randy's change wants to arrange). You can plug one into any machine that has PCI slots and my understanding from Linux Kconfig files is there are such 32-bit SPARC machines in existence or the dependency on PCI wouldn't offer the driver. Otherwise just don't enable CONFIG_PCI for 32-bit SPARC. Apologies if I wasn't clear enough with my reasoning, although I think the lone presence of the PCI dependency in Kconfig ought have to make it clear. > The sparc32 machines have the parport_sunbpp driver for their parallel > port. That's an onboard device or an SBus option card though, right? > An alternative fix, and better I think, would be to audit all archs > and let the relevant ones select ARCH_MIGHT_HAVE_PC_PARPORT, so we > avoided the ugly "|| (PCI && !S390 && !SPARC32)" case for PARPORT_PC. It's only S390 that is special in that it has a limited set of specially crafted PCI options it can ever support (or so I am told; something about the firmware or suchlike). Any other platform that has PCI slots will handle PC-style PCI parallel port option cards just fine, as long as it supports PCI I/O read/write commands (some systems such as POWER9 machines don't; Niklas Schnelle has been recently working on a generic way to exclude drivers for devices that require PCI port I/O from being offered with systems that have no support for PCI port I/O). Let me know if you find anything here unclear or have any other questions or comments. Maciej
On 4/7/23 14:01, Maciej W. Rozycki wrote: > On Fri, 7 Apr 2023, Sam Ravnborg wrote: > >>>> Randy's suggestion is fine, as we avoid building parport support >>>> for sparc32. If someone shows up and need parport support >>>> for sparc32 then we could look into how to enable it. >>>> Until then, we are better helped avoiding building the driver. >>> >>> I disagree. Why artificially prevent perfectly good hardware from >>> working with a perfectly good driver especially as the fix is just a >>> trivial exercise? And I offered a solution. >> >> There is no sparc32 with a PC style parallel port, so the parport_pc >> have no value for a sparc32 machine. > > There are PC-style PCI (and PCIe) parallel ports in the form of option > cards being sold; I have one in my RISC-V machine (and I had to go through > the hassle of figuring out why the heck I am not able to select the driver > in configuration; a situation analogous to what Randy's change wants to > arrange). You can plug one into any machine that has PCI slots and my > understanding from Linux Kconfig files is there are such 32-bit SPARC > machines in existence or the dependency on PCI wouldn't offer the driver. > Otherwise just don't enable CONFIG_PCI for 32-bit SPARC. > If there are 32-bit Sparc machines with PCI slots, we must not have any users with parallel ports or we should have heard about it IMO. > Apologies if I wasn't clear enough with my reasoning, although I think > the lone presence of the PCI dependency in Kconfig ought have to make it > clear. > >> The sparc32 machines have the parport_sunbpp driver for their parallel >> port. > > That's an onboard device or an SBus option card though, right? > >> An alternative fix, and better I think, would be to audit all archs >> and let the relevant ones select ARCH_MIGHT_HAVE_PC_PARPORT, so we >> avoided the ugly "|| (PCI && !S390 && !SPARC32)" case for PARPORT_PC. > > It's only S390 that is special in that it has a limited set of specially > crafted PCI options it can ever support (or so I am told; something about > the firmware or suchlike). From my reading, if a Sparc32 machine has a PCI port, it might be able to have a parallel port. However, even with Maciej's suggested code change instead of my patch, the ebus code is not being compiled for Sparc32 -- only for Sparc64, so more changes are needed beyond Maciej's suggestion. But the documentation that I found refers to Ebus on Sparc4 machines. > Any other platform that has PCI slots will handle PC-style PCI parallel > port option cards just fine, as long as it supports PCI I/O read/write > commands (some systems such as POWER9 machines don't; Niklas Schnelle has > been recently working on a generic way to exclude drivers for devices that > require PCI port I/O from being offered with systems that have no support > for PCI port I/O). > > Let me know if you find anything here unclear or have any other questions > or comments. /me wishes that we had a Sparc maintainer.
On Fri, 7 Apr 2023, Randy Dunlap wrote: > > There are PC-style PCI (and PCIe) parallel ports in the form of option > > cards being sold; I have one in my RISC-V machine (and I had to go through > > the hassle of figuring out why the heck I am not able to select the driver > > in configuration; a situation analogous to what Randy's change wants to > > arrange). You can plug one into any machine that has PCI slots and my > > understanding from Linux Kconfig files is there are such 32-bit SPARC > > machines in existence or the dependency on PCI wouldn't offer the driver. > > Otherwise just don't enable CONFIG_PCI for 32-bit SPARC. > > > > If there are 32-bit Sparc machines with PCI slots, we must not have any > users with parallel ports or we should have heard about it IMO. I wouldn't be surprised, parallel ports are not that common nowadays, let alone used. Myself I haven't used a parallel printer for ages now, though I still own a couple of odd other parallel devices, such as a ROM emulator or the firmware download port of an old MIPS development board (actually a regular Super I/O parallel port of said device hijacked by an onboard FPGA for this second purpose if enabled with an onboard rocker switch). That's not the usual stuff people tend to use I suppose. > >> An alternative fix, and better I think, would be to audit all archs > >> and let the relevant ones select ARCH_MIGHT_HAVE_PC_PARPORT, so we > >> avoided the ugly "|| (PCI && !S390 && !SPARC32)" case for PARPORT_PC. I should have noted this before: ARCH_MIGHT_HAVE_PC_PARPORT is for true ISA or Super I/O parallel ports only. We handle true PCI implementations in a generic platform-agnostic way, as long as the platform implements PCI I/O commands in the host bridge. The latter requirement only excludes a bunch of platforms, most notably S390 and recent 64-bit POWER systems. With Niklas's HAS_IOPORT patches the S390 special case will soon be gone. > > It's only S390 that is special in that it has a limited set of specially > > crafted PCI options it can ever support (or so I am told; something about > > the firmware or suchlike). > > >From my reading, if a Sparc32 machine has a PCI port, it might be able to > have a parallel port. However, even with Maciej's suggested code change > instead of my patch, the ebus code is not being compiled for Sparc32 -- only > for Sparc64, so more changes are needed beyond Maciej's suggestion. > > But the documentation that I found refers to Ebus on Sparc4 machines. Well, even though I came across a bunch of SPARC machines in the past I'm not familiar enough with the platform to have an idea what SPARC4 refers to. You can enable CONFIG_PCI for a SPARC32 kernel however, which I infer from there are 32-bit SPARC machines in existence with PCI connectivity. That I find enough for a potential PC-style parallel port configuration with such a system, for as many ports as the availability of slots allows. > > Any other platform that has PCI slots will handle PC-style PCI parallel > > port option cards just fine, as long as it supports PCI I/O read/write > > commands (some systems such as POWER9 machines don't; Niklas Schnelle has > > been recently working on a generic way to exclude drivers for devices that > > require PCI port I/O from being offered with systems that have no support > > for PCI port I/O). > > > > Let me know if you find anything here unclear or have any other questions > > or comments. > > /me wishes that we had a Sparc maintainer. What happened to DaveM? In any case after a couple of iterations I have made a succesful build of a 32-bit SPARC toolchain now, which I was able to verify a fix with I have outlined earlier in this thread. Posted as archived at: <https://lore.kernel.org/r/alpine.DEB.2.21.2306182347101.14084@angie.orcam.me.uk/>. Maciej
Hi, On 6/18/23 16:42, Maciej W. Rozycki wrote: > On Fri, 7 Apr 2023, Randy Dunlap wrote: > >> /me wishes that we had a Sparc maintainer. > > What happened to DaveM? I haven't seen him merge any arch/sparc/ patches lately. I have a couple that are still pending. > In any case after a couple of iterations I have made a succesful build of > a 32-bit SPARC toolchain now, which I was able to verify a fix with I have Is your newly built toolchain for riscv hosting? > outlined earlier in this thread. Posted as archived at: > <https://lore.kernel.org/r/alpine.DEB.2.21.2306182347101.14084@angie.orcam.me.uk/>. thanks.
Hi Randy, > > What happened to DaveM? > > I haven't seen him merge any arch/sparc/ patches lately. > I have a couple that are still pending. Oh, I hope he's been doing good then, and it's just a change of life priorities or suchlike. Patch reviews can take a lot of mental effort, and I can't claim I've been as effective as I wished to with stuff that lands on my plate either. > > In any case after a couple of iterations I have made a succesful build of > > a 32-bit SPARC toolchain now, which I was able to verify a fix with I have > > Is your newly built toolchain for riscv hosting? Are you asking whether the SPARC toolchain has been built/installed on a RISC-V system? If so, then no, it hasn't. It runs on POWER9. Maciej
On 6/18/23 18:29, Maciej W. Rozycki wrote: > Hi Randy, > >>> What happened to DaveM? >> >> I haven't seen him merge any arch/sparc/ patches lately. >> I have a couple that are still pending. > > Oh, I hope he's been doing good then, and it's just a change of life > priorities or suchlike. Patch reviews can take a lot of mental effort, > and I can't claim I've been as effective as I wished to with stuff that > lands on my plate either. > >>> In any case after a couple of iterations I have made a succesful build of >>> a 32-bit SPARC toolchain now, which I was able to verify a fix with I have >> >> Is your newly built toolchain for riscv hosting? > > Are you asking whether the SPARC toolchain has been built/installed on a > RISC-V system? If so, then no, it hasn't. It runs on POWER9. Yes, that's what I was asking. So you could have used the compilers that Arnd builds: https://mirrors.edge.kernel.org/pub/tools/crosstool/ Thanks.
From: "Maciej W. Rozycki" <macro@orcam.me.uk> Date: Mon, 19 Jun 2023 02:29:57 +0100 (BST) > Hi Randy, > >> > What happened to DaveM? >> >> I haven't seen him merge any arch/sparc/ patches lately. >> I have a couple that are still pending. > > Oh, I hope he's been doing good then, and it's just a change of life > priorities or suchlike. Patch reviews can take a lot of mental effort, > and I can't claim I've been as effective as I wished to with stuff that > lands on my plate either. I'm good just too busy with networking and real life. Thanks.
Hi Davem, > > > >> > What happened to DaveM? > >> > >> I haven't seen him merge any arch/sparc/ patches lately. > >> I have a couple that are still pending. > > > > Oh, I hope he's been doing good then, and it's just a change of life > > priorities or suchlike. Patch reviews can take a lot of mental effort, > > and I can't claim I've been as effective as I wished to with stuff that > > lands on my plate either. > > I'm good just too busy with networking and real life. Enjoy the real life! Sam
On 6/23/23 10:22, Sam Ravnborg wrote: > Hi Davem, > >>> >>>>> What happened to DaveM? >>>> >>>> I haven't seen him merge any arch/sparc/ patches lately. >>>> I have a couple that are still pending. >>> >>> Oh, I hope he's been doing good then, and it's just a change of life >>> priorities or suchlike. Patch reviews can take a lot of mental effort, >>> and I can't claim I've been as effective as I wished to with stuff that >>> lands on my plate either. >> >> I'm good just too busy with networking and real life. > > > Enjoy the real life! > > Sam I agree with that. :) But is there an alternate route for sparc patches? thanks.
diff -- a/drivers/parport/Kconfig b/drivers/parport/Kconfig --- a/drivers/parport/Kconfig +++ b/drivers/parport/Kconfig @@ -42,7 +42,7 @@ if PARPORT config PARPORT_PC tristate "PC-style hardware" - depends on ARCH_MIGHT_HAVE_PC_PARPORT || (PCI && !S390) + depends on ARCH_MIGHT_HAVE_PC_PARPORT || (PCI && !S390 && !SPARC32) help You should say Y here if you have a PC-style parallel port. All IBM PC compatible computers and some Alphas have PC-style
arch/sparc/kernel/ebus.o is only built for SPARC64. ns87303_lock is only built for SPARC64. arch/sparc/include/asm/parport.h says that it is for sparc64. Various documentation on the internet says that ebus is for UltraSPARC systems (64-bit). Therefore don't allow PARPORT_PC to be built for SPARC32. Fixes these build errors on SPARC32: ERROR: modpost: "ebus_dma_irq_enable" [drivers/parport/parport_pc.ko] undefined! ERROR: modpost: "ebus_dma_unregister" [drivers/parport/parport_pc.ko] undefined! ERROR: modpost: "ebus_dma_register" [drivers/parport/parport_pc.ko] undefined! ERROR: modpost: "ns87303_lock" [drivers/parport/parport_pc.ko] undefined! ERROR: modpost: "ebus_dma_enable" [drivers/parport/parport_pc.ko] undefined! ERROR: modpost: "ebus_dma_prepare" [drivers/parport/parport_pc.ko] undefined! ERROR: modpost: "ebus_dma_request" [drivers/parport/parport_pc.ko] undefined! ERROR: modpost: "ebus_dma_residue" [drivers/parport/parport_pc.ko] undefined! Fixes: 66bcd06099bb ("parport_pc: Also enable driver for PCI systems") Signed-off-by: Randy Dunlap <rdunlap@infradead.org> Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com> Cc: Maciej W. Rozycki <macro@orcam.me.uk> Cc: "David S. Miller" <davem@davemloft.net> Cc: sparclinux@vger.kernel.org Cc: Sam Ravnborg <sam@ravnborg.org> Cc: linux-parport@lists.infradead.org --- drivers/parport/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)