Message ID | 20240223-sam-fix-sparc32-all-builds-v1-5-5c60fd5c9250@ravnborg.org |
---|---|
State | New |
Headers | show |
Series | sparc32: build fixes for all{yes,mod}config builds | expand |
On 2/23/24 11:36, Sam Ravnborg via B4 Relay wrote: > From: Sam Ravnborg <sam@ravnborg.org> > > sparc32 do not support generic isa dma, so do not select the symbol. > Without this fix, the following patch would break the build with a > missing prototype. > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Cc: Andreas Larsson <andreas@gaisler.com> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Randy Dunlap <rdunlap@infradead.org> > Cc: Maciej W. Rozycki <macro@orcam.me.uk> > Fixes: 66bcd06099bb ("parport_pc: Also enable driver for PCI systems") Acked-by: Randy Dunlap <rdunlap@infradead.org> Tested-by: Randy Dunlap <rdunlap@infradead.org> # build-tested > --- > arch/sparc/Kconfig | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig > index 734f23daecca..d08a5662ea60 100644 > --- a/arch/sparc/Kconfig > +++ b/arch/sparc/Kconfig > @@ -138,10 +138,6 @@ config HIGHMEM > default y if SPARC32 > select KMAP_LOCAL > > -config GENERIC_ISA_DMA > - bool > - default y if SPARC32 > - > config PGTABLE_LEVELS > default 4 if 64BIT > default 3 >
On Fri, 23 Feb 2024, Sam Ravnborg via B4 Relay wrote: > sparc32 do not support generic isa dma, so do not select the symbol. > Without this fix, the following patch would break the build with a > missing prototype. Not according to my observations, kernel/dma.c is always built for GENERIC_ISA_DMA configurations, so: kernel/dma.c:70:5: error: no previous prototype for 'request_dma' [-Werror=missing-prototypes] 70 | int request_dma(unsigned int dmanr, const char * device_id) | ^~~~~~~~~~~ kernel/dma.c:88:6: error: no previous prototype for 'free_dma' [-Werror=missing-prototypes] 88 | void free_dma(unsigned int dmanr) | ^~~~~~~~ are issued regardless (and FAOD with PARPORT_PC unset). I can't speak for SPARC support for ISA DMA, but it seems to me like the second sentence would best be removed, as would the Fixes: tag (in favour to: Fixes: 0fcb70851fbf ("Makefile.extrawarn: turn on missing-prototypes globally") I presume), and possibly the messages quoted above included instead. Maciej
On Sat, 24 Feb 2024, Maciej W. Rozycki wrote: > > sparc32 do not support generic isa dma, so do not select the symbol. > > Without this fix, the following patch would break the build with a > > missing prototype. > > Not according to my observations, kernel/dma.c is always built for > GENERIC_ISA_DMA configurations, so: > > kernel/dma.c:70:5: error: no previous prototype for 'request_dma' [-Werror=missing-prototypes] > 70 | int request_dma(unsigned int dmanr, const char * device_id) > | ^~~~~~~~~~~ > kernel/dma.c:88:6: error: no previous prototype for 'free_dma' [-Werror=missing-prototypes] > 88 | void free_dma(unsigned int dmanr) > | ^~~~~~~~ > > are issued regardless (and FAOD with PARPORT_PC unset). > > I can't speak for SPARC support for ISA DMA, but it seems to me like the > second sentence would best be removed, as would the Fixes: tag (in favour > to: > > Fixes: 0fcb70851fbf ("Makefile.extrawarn: turn on missing-prototypes globally") > > I presume), and possibly the messages quoted above included instead. Actually I think ZONE_DMA should go too (it's linked to GENERIC_ISA_DMA, isn't it? -- cf. commit 5ac6da669e24 ("[PATCH] Set CONFIG_ZONE_DMA for arches with GENERIC_ISA_DMA")), and the whole thing use: Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") The GENERIC_ISA_DMA option itself was added to arch/sparc/config.in with 2.5.31 as: define_bool CONFIG_GENERIC_ISA_DMA y despite of: define_bool CONFIG_ISA n for a reason not clear to me (BLK_DEV_FD? -- but on SPARC that uses some hacks to work in the absence of ISA DMA anyway). Am I missing anything here? Maciej
Hi Marciej, On Sat, Feb 24, 2024 at 05:29:43AM +0000, Maciej W. Rozycki wrote: > On Sat, 24 Feb 2024, Maciej W. Rozycki wrote: > > > > sparc32 do not support generic isa dma, so do not select the symbol. > > > Without this fix, the following patch would break the build with a > > > missing prototype. > > > > Not according to my observations, kernel/dma.c is always built for > > GENERIC_ISA_DMA configurations, so: > > > > kernel/dma.c:70:5: error: no previous prototype for 'request_dma' [-Werror=missing-prototypes] > > 70 | int request_dma(unsigned int dmanr, const char * device_id) > > | ^~~~~~~~~~~ > > kernel/dma.c:88:6: error: no previous prototype for 'free_dma' [-Werror=missing-prototypes] > > 88 | void free_dma(unsigned int dmanr) > > | ^~~~~~~~ > > > > are issued regardless (and FAOD with PARPORT_PC unset). > > > > I can't speak for SPARC support for ISA DMA, but it seems to me like the > > second sentence would best be removed, as would the Fixes: tag (in favour > > to: > > > > Fixes: 0fcb70851fbf ("Makefile.extrawarn: turn on missing-prototypes globally") > > > > I presume), and possibly the messages quoted above included instead. Thanks, I will update in v2. > > Actually I think ZONE_DMA should go too (it's linked to GENERIC_ISA_DMA, > isn't it? -- cf. commit 5ac6da669e24 ("[PATCH] Set CONFIG_ZONE_DMA for > arches with GENERIC_ISA_DMA")), and the whole thing use: > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > > The GENERIC_ISA_DMA option itself was added to arch/sparc/config.in with > 2.5.31 as: > > define_bool CONFIG_GENERIC_ISA_DMA y > > despite of: > > define_bool CONFIG_ISA n > > for a reason not clear to me (BLK_DEV_FD? -- but on SPARC that uses some > hacks to work in the absence of ISA DMA anyway). > > Am I missing anything here? Nice find - the code below conforms you are right: max_zone_pfn[ZONE_DMA] = max_low_pfn; max_zone_pfn[ZONE_NORMAL] = max_low_pfn; As they are set to the same value there is no smaller ZONE_DMA area. I will add an extra patch for this in v2. Sam
On Sat, Feb 24, 2024, at 06:29, Maciej W. Rozycki wrote: > On Sat, 24 Feb 2024, Maciej W. Rozycki wrote: > > The GENERIC_ISA_DMA option itself was added to arch/sparc/config.in with > 2.5.31 as: > > define_bool CONFIG_GENERIC_ISA_DMA y > > despite of: > > define_bool CONFIG_ISA n I think I've seen any combination of CONFIG_ISA (the 62/98 pin slots), CONFIG_GENERIC_ISA_DMA (the request_dma() interface) and CONFIG_ISA_DMA_API (the set_dma_addr()/enable_dma() type interface), but I agree that sparc should have none of the three as both floppy and parport use some other interface. > for a reason not clear to me (BLK_DEV_FD? -- but on SPARC that uses some > hacks to work in the absence of ISA DMA anyway). > > Am I missing anything here? I think it was part of the ISA DMA lookalike that got removed in 334ae614772b ("sparc: Kill SBUS DVMA layer.") and should have been changed back then. Arnd
Hi Arnd, On Sat, Feb 24, 2024 at 12:24:01PM +0100, Arnd Bergmann wrote: > On Sat, Feb 24, 2024, at 06:29, Maciej W. Rozycki wrote: > > On Sat, 24 Feb 2024, Maciej W. Rozycki wrote: > > > > The GENERIC_ISA_DMA option itself was added to arch/sparc/config.in with > > 2.5.31 as: > > > > define_bool CONFIG_GENERIC_ISA_DMA y > > > > despite of: > > > > define_bool CONFIG_ISA n > > I think I've seen any combination of CONFIG_ISA (the 62/98 pin slots), CONFIG_GENERIC_ISA_DMA (the request_dma() interface) and > CONFIG_ISA_DMA_API (the set_dma_addr()/enable_dma() type interface), > but I agree that sparc should have none of the three as both > floppy and parport use some other interface. > > > for a reason not clear to me (BLK_DEV_FD? -- but on SPARC that uses some > > hacks to work in the absence of ISA DMA anyway). > > > > Am I missing anything here? > > I think it was part of the ISA DMA lookalike that got removed > in 334ae614772b ("sparc: Kill SBUS DVMA layer.") and should > have been changed back then. Hmm, that may well be the case. I checked and sparc32 do not set any ISA symbols anymore so we should be OK now. I decided to just drop the Fixes: tag to not confuse anyone. Sam
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig index 734f23daecca..d08a5662ea60 100644 --- a/arch/sparc/Kconfig +++ b/arch/sparc/Kconfig @@ -138,10 +138,6 @@ config HIGHMEM default y if SPARC32 select KMAP_LOCAL -config GENERIC_ISA_DMA - bool - default y if SPARC32 - config PGTABLE_LEVELS default 4 if 64BIT default 3