Hello Geert, On Fri, Mar 06, 2026 at 08:57:27AM +0100, Geert Uytterhoeven wrote: > On Thu, 5 Mar 2026 at 09:58, Helge Deller wrote: > > On 3/5/26 09:07, Uwe Kleine-König wrote: > > > On Wed, Mar 04, 2026 at 07:23:30PM +0100, Helge Deller wrote: > > >> -#if defined(CONFIG_COMPILE_TEST) && !defined(CONFIG_MIPS) > > >> +#if defined(CONFIG_COMPILE_TEST) && !(defined(CONFIG_MIPS) && !defined(CONFIG_64BIT)) > > > > > > The condition is equivalent to > > > > > > defined(CONFIG_COMPILE_TEST) && (!defined(CONFIG_MIPS) || defined(CONFIG_64BIT)) > > > > > > which is logically a bit easier, but I'm unsure if it's easier to > > > understand (IMHO both are bad). > > > > Yes, both are bad. I changed it to your proposal. > > Yes, I had to read it twice, but that applies to both :-( > > What about "#ifndef KSEG1ADDR" instead, or would that be considered > too dangerous? is included by MIPS' > so it should always be included if the I/O accessors are available. Just to state the (maybe?) obvious: With the more complicated expression it is asserted that the build breaks if on mips32 the definition disappears e.g. because arch/mips/include/asm/addrspace.h isn't included implicitly any more or the macro is renamed to unify with mips64's CKSEG1ADDR. With just `#ifndef KSEG1ADDR` the driver would break without the compiler noticing and put it's fbdata somewhere in the address space. I guess it's subjective, but my preference would be to keep the more complicated expression (despite being complicated). An alternative would be: diff --git a/arch/mips/alchemy/devboards/db1000.c b/arch/mips/alchemy/devboards/db1000.c index 6984cd5169b5..1958ad0e02a8 100644 --- a/arch/mips/alchemy/devboards/db1000.c +++ b/arch/mips/alchemy/devboards/db1000.c @@ -98,8 +98,8 @@ int __init db1500_pci_setup(void) static struct resource au1100_lcd_resources[] = { [0] = { - .start = AU1100_LCD_PHYS_ADDR, - .end = AU1100_LCD_PHYS_ADDR + 0x800 - 1, + .start = KSEG1ADDR(AU1100_LCD_PHYS_ADDR), + .end = KSEG1ADDR(AU1100_LCD_PHYS_ADDR + 0x800 - 1), .flags = IORESOURCE_MEM, }, [1] = { diff --git a/drivers/video/fbdev/au1100fb.c b/drivers/video/fbdev/au1100fb.c index 1a04154bc535..70b96e9ea454 100644 --- a/drivers/video/fbdev/au1100fb.c +++ b/drivers/video/fbdev/au1100fb.c @@ -380,11 +380,6 @@ static struct au1100fb_panel known_lcd_panels[] = #define panel_is_color(panel) (panel->control_base & LCD_CONTROL_PC) #define panel_swap_rgb(panel) (panel->control_base & LCD_CONTROL_CCO) -#if defined(CONFIG_COMPILE_TEST) && !defined(CONFIG_MIPS) -/* This is only defined to be able to compile this driver on non-mips platforms */ -#define KSEG1ADDR(x) (x) -#endif - #define DRIVER_NAME "au1100fb" #define DRIVER_DESC "LCD controller driver for AU1100 processors" @@ -764,7 +759,7 @@ static int au1100fb_drv_probe(struct platform_device *dev) return -EBUSY; } - fbdev->regs = (struct au1100fb_regs*)KSEG1ADDR(fbdev->info.fix.mmio_start); + fbdev->regs = (struct au1100fb_regs*)fbdev->info.fix.mmio_start; pr_devel("Register memory map at %p", fbdev->regs); pr_devel("phys=0x%08x, size=%zu", fbdev->regs_phys, fbdev->regs_len); Best regards Uwe