Messages in this thread |  | | | Date | Thu, 2 Nov 2006 13:14:43 -0800 | | From | Andrew Morton <> | | Subject | Re: [patch] video: support for VGA hoses on alpha TITAN, TSUNAMI systems (ES45, DS25) |
| |
On Thu, 2 Nov 2006 00:37:20 -0800 Steve Langasek <vorlon@debian.org> wrote:
> From: Jay Estabrook <jay.estabrook@hp.com> > > Add working support for VGA hoses on Alpha, which is required in order to > use a VGA console on TITAN- and TSUNAMI-class Alpha systems. This also > generalizes support for non-standard VGA offsets on Alpha to cover the > Marvel class of alphas without special-casing. Includes recognizing an > additional model of TITAN-class Alpha. > > Tested successfully on an ES45 and a DS25 with a vanilla 2.6.18 kernel, > also tested on an LX164 with the Debian 2.6.18 kernel with no regressions. > Included in Alpha Core distribution for some time. >
It would be appropriate for both yourself and Jay to provide signoffs for this work, as per section 11 of Documentation/SubmittingPatches, please.
What is a "hose"?
Some trivia:
> + if (!hose || (conswitchp == &vga_con && pci_vga_hose == hose)) return;
This is missing a newline.
> +static int titan_pchip1_present = 0;
Unneeded (and undesirable) initialisation.
> +++ source-es45/arch/alpha/kernel/proto.h 2006-09-30 03:14:44.000000000 -0700 > @@ -106,6 +106,11 @@ > extern unsigned long wildfire_node_mem_start(int); > extern unsigned long wildfire_node_mem_size(int); > > +/* console.c */ > +#ifdef CONFIG_VGA_HOSE > + extern void locate_and_init_vga(void *(*)(void *, void *)); > +#endif
The ifdef isn't really needed - we usually leave it out.
> diff -uNr source/arch/alpha/kernel/sys_dp264.c source-es45/arch/alpha/kernel/sys_dp264.c > --- source/arch/alpha/kernel/sys_dp264.c 2006-09-19 20:42:06.000000000 -0700 > +++ source-es45/arch/alpha/kernel/sys_dp264.c 2006-09-30 03:14:44.000000000 -0700 > @@ -543,6 +543,9 @@ > { > common_init_pci(); > SMC669_Init(0); > +#ifdef CONFIG_VGA_HOSE > + locate_and_init_vga(NULL); > +#endif > } > > static void __init > @@ -551,6 +554,18 @@ > common_init_pci(); > SMC669_Init(1); > es1888_init(); > +#ifdef CONFIG_VGA_HOSE > + locate_and_init_vga(NULL); > +#endif > +} > + > +static void __init > +clipper_init_pci(void) > +{ > + common_init_pci(); > +#ifdef CONFIG_VGA_HOSE > + locate_and_init_vga(NULL); > +#endif > }
We normally do:
#ifdef CONFIG_VGA_HOSE extern void locate_and_init_vga(void *(*handler)(void *, void *)); #else static inline void locate_and_init_vga(void *(*handler)(void *, void *)) { } #endif so that all those ifdefs go away and so that the new code gets compile-checked regardless of config settings.
> > +/* wait until after includes to test for this, to allow arch-specific mod. */ > +#ifndef vga_request_resource > +# define vga_request_resource request_resource > +#endif > +
erk.
> +#define vga_request_resource alpha_vga_request_resource > + > +static int __inline__ > +alpha_vga_request_resource(struct resource *root, struct resource *new) > +{ > + /* First, fixup the VGA resource bounds WRT the hose it is on. */ > + if (pci_vga_hose) { > + new->start += pci_vga_hose->io_space->start; > + new->end += pci_vga_hose->io_space->start; > + } > + > + /* Finally, do a normal request_resource(). */ > + return request_resource(root, new); > +} >
So if the resource is being requested by vga we adjust the caller's resource before actually requesting it.
Isn't this a bit hacky? How come vgacon ended up requesting the wrong addresses in the first place?
(And please don't use __inline__ or __inline - plain old `inline' works fine)
> static DEFINE_SPINLOCK(vga_lock); > static int cursor_size_lastfrom; > static int cursor_size_lastto; > @@ -393,7 +398,7 @@ > vga_video_type = VIDEO_TYPE_EGAM; > vga_vram_size = 0x8000; > display_desc = "EGA+"; > - request_resource(&ioport_resource, > + vga_request_resource(&ioport_resource, > &ega_console_resource); > > .. > > diff -uNr source/include/asm-alpha/core_titan.h source-es45/include/asm-alpha/core_titan.h > --- source/include/asm-alpha/core_titan.h 2006-09-19 20:42:06.000000000 -0700 > +++ source-es45/include/asm-alpha/core_titan.h 2006-09-30 03:14:44.000000000 -0700 > @@ -3,6 +3,7 @@ > > #include <linux/types.h> > #include <linux/pci.h> > +#include <asm/pci.h> > #include <asm/compiler.h> > > ... > > diff -uNr source/include/asm-alpha/core_tsunami.h source-es45/include/asm-alpha/core_tsunami.h > --- source/include/asm-alpha/core_tsunami.h 2006-09-19 20:42:06.000000000 -0700 > +++ source-es45/include/asm-alpha/core_tsunami.h 2006-09-30 03:14:44.000000000 -0700 > @@ -2,6 +2,8 @@ > #define __ALPHA_TSUNAMI__H__ > > #include <linux/types.h> > +#include <linux/pci.h> > +#include <asm/pci.h> > #include <asm/compiler.h>
So we actually need to include asm/pci.h in these files? linux/pci.h already did that?
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |