Re: [PATCH] quiet non-x86 option ROM warnings

[Jesse Barnes <jbarnes@sgi.com>]

[-- Attachment #1: Type: text/plain, Size: 951 bytes --]

On Thursday, February 17, 2005 9:32 am, Jon Smirl wrote:
> On Thu, 17 Feb 2005 09:29:53 -0800, Jesse Barnes <jbarnes@sgi.com> wrote:
> > On Thursday, February 17, 2005 8:33 am, Jon Smirl wrote:
> > > > No, pci_map_rom shouldn't test the signature IMHO. While PCI ROMs
> > > > should have the signature to be recognized as containing valid
> > > > firmware images on x86 BIOSes an OF, it's just a convention on these
> > > > platforms, and I would rather let people put whatever they want in
> > > > those ROMs and still let them map it...
> > >
> > > pci_map_rom will return a pointer to any ROM it finds. It the
> > > signature is invalid the size returned will be zero. Is this ok or do
> > > we want it to do something different?
> >
> > Shouldn't it return NULL if the signature is invalid?
>
> But then you couldn't get to your non-standard ROMs

Ok, how does this one look to you guys?  The r128 driver would need similar 
fixes.

Thanks,
Jesse

[-- Attachment #2: radeonfb-pci-map-rom-fix.patch --]
[-- Type: text/x-diff, Size: 2467 bytes --]

===== drivers/video/aty/radeon_base.c 1.39 vs edited =====
--- 1.39/drivers/video/aty/radeon_base.c	2005-02-10 22:57:44 -08:00
+++ edited/drivers/video/aty/radeon_base.c	2005-02-17 09:43:13 -08:00
@@ -318,54 +318,20 @@
 	temp |= 0x04 << 24;
 	OUTREG(MPP_TB_CONFIG, temp);
 	temp = INREG(MPP_TB_CONFIG);
-                                                                                                          
+
 	rom = pci_map_rom(dev, &rom_size);
 	if (!rom) {
-		printk(KERN_ERR "radeonfb (%s): ROM failed to map\n",
+		printk(KERN_INFO "radeonfb (%s): ROM failed to map\n",
 		       pci_name(rinfo->pdev));
 		return -ENOMEM;
 	}
-	
-	rinfo->bios_seg = rom;
 
-	/* Very simple test to make sure it appeared */
-	if (BIOS_IN16(0) != 0xaa55) {
-		printk(KERN_ERR "radeonfb (%s): Invalid ROM signature %x should be"
-		       "0xaa55\n", pci_name(rinfo->pdev), BIOS_IN16(0));
+	if (!rom_size) /* we mapped something, but it wasn't valid */
 		goto failed;
-	}
-	/* Look for the PCI data to check the ROM type */
+	
+	rinfo->bios_seg = rom;
 	dptr = BIOS_IN16(0x18);
 
-	/* Check the PCI data signature. If it's wrong, we still assume a normal x86 ROM
-	 * for now, until I've verified this works everywhere. The goal here is more
-	 * to phase out Open Firmware images.
-	 *
-	 * Currently, we only look at the first PCI data, we could iteratre and deal with
-	 * them all, and we should use fb_bios_start relative to start of image and not
-	 * relative start of ROM, but so far, I never found a dual-image ATI card
-	 *
-	 * typedef struct {
-	 * 	u32	signature;	+ 0x00
-	 * 	u16	vendor;		+ 0x04
-	 * 	u16	device;		+ 0x06
-	 * 	u16	reserved_1;	+ 0x08
-	 * 	u16	dlen;		+ 0x0a
-	 * 	u8	drevision;	+ 0x0c
-	 * 	u8	class_hi;	+ 0x0d
-	 * 	u16	class_lo;	+ 0x0e
-	 * 	u16	ilen;		+ 0x10
-	 * 	u16	irevision;	+ 0x12
-	 * 	u8	type;		+ 0x14
-	 * 	u8	indicator;	+ 0x15
-	 * 	u16	reserved_2;	+ 0x16
-	 * } pci_data_t;
-	 */
-	if (BIOS_IN32(dptr) !=  (('R' << 24) | ('I' << 16) | ('C' << 8) | 'P')) {
-		printk(KERN_WARNING "radeonfb (%s): PCI DATA signature in ROM"
-		       "incorrect: %08x\n", pci_name(rinfo->pdev), BIOS_IN32(dptr));
-		goto anyway;
-	}
 	rom_type = BIOS_IN8(dptr + 0x14);
 	switch(rom_type) {
 	case 0:
@@ -381,7 +347,7 @@
 		printk(KERN_INFO "radeonfb: Found unknown type %d ROM Image\n", rom_type);
 		goto failed;
 	}
- anyway:
+	
 	/* Locate the flat panel infos, do some sanity checking !!! */
 	rinfo->fp_bios_start = BIOS_IN16(0x48);
 	return 0;