Skip to contentSkip to conversation

PCI Cleanup

DISCUSSION Hanna Linder 4 participants 4 messages Aug 22, 2002 SuperH

lore
Share & tools
lore
#0 lore raw

Here is the first part of the sh port of the pci_ops changes. If anyone can compile this for Sega let me know if there are any problems.

Thanks.

Hanna Linder
[email protected]

ps -this patches against bk://linuxusb.bkbits.net/pci_hp-2.5

-----

arch/sh/kernel/pci-dc.c +44 -62
diff -Nru a/arch/sh/kernel/pci-dc.c b/arch/sh/kernel/pci-dc.c
@@ -31,76 +31,58 @@
 	{0, 0, 0, NULL}
 };
 
-#define BBA_SELECTED(dev) (dev->bus->number==0 && dev->devfn==0)
+#define BBA_SELECTED(bus,devfn) (bus->number==0 && devfn==0)
 
-static int gapspci_read_config_byte(struct pci_dev *dev, int where,
-                                    u8 * val)
+static int gapspci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 * val)
 {
-	if (BBA_SELECTED(dev))
-		*val = inb(GAPSPCI_BBA_CONFIG+where);
-	else
-                *val = 0xff;
-
+	switch (size) {
+	case 1:
+		if (BBA_SELECTED(bus, devfn))
+			*val = (u8)inb(GAPSPCI_BBA_CONFIG+where);
+		else
+			*val = (u8)0xff;
+		break;
+	case 2:
+		if (BBA_SELECTED(bus, devfn))
+			*val = (u16)inw(GAPSPCI_BBA_CONFIG+where);
+		else
+			*val = (u16)0xffff;
+		break;
+	case 4:
+		if (BBA_SELECTED(bus, devfn))
+			*val = inl(GAPSPCI_BBA_CONFIG+where);
+		else
+			*val = 0xffffffff;
+		break;
+	}	
 	return PCIBIOS_SUCCESSFUL;
 }
 
-static int gapspci_read_config_word(struct pci_dev *dev, int where,
-                                    u16 * val)
-{
-        if (BBA_SELECTED(dev))
-		*val = inw(GAPSPCI_BBA_CONFIG+where);
-	else
-                *val = 0xffff;
-
-        return PCIBIOS_SUCCESSFUL;
-}
-
-static int gapspci_read_config_dword(struct pci_dev *dev, int where,
-                                     u32 * val)
-{
-        if (BBA_SELECTED(dev))
-		*val = inl(GAPSPCI_BBA_CONFIG+where);
-	else
-                *val = 0xffffffff;
-
-        return PCIBIOS_SUCCESSFUL;
-}
-
-static int gapspci_write_config_byte(struct pci_dev *dev, int where,
-                                     u8 val)
-{
-        if (BBA_SELECTED(dev))
-		outb(val, GAPSPCI_BBA_CONFIG+where);
-
-        return PCIBIOS_SUCCESSFUL;
-}
-
-
-static int gapspci_write_config_word(struct pci_dev *dev, int where,
-                                     u16 val)
-{
-        if (BBA_SELECTED(dev))
-		outw(val, GAPSPCI_BBA_CONFIG+where);
-
-        return PCIBIOS_SUCCESSFUL;
-}
-
-static int gapspci_write_config_dword(struct pci_dev *dev, int where,
-                                      u32 val)
+static int gapspci_write(struct pci_bus *bus, unsigned int devfn,
+				    int where, u32 val)
 {
-        if (BBA_SELECTED(dev))
-		outl(val, GAPSPCI_BBA_CONFIG+where);
-
-        return PCIBIOS_SUCCESSFUL;
+	if (BBA_SELECTED(bus, devfn)) {
+		switch (size) {
+	case 1:
+		if (BBA_SELECTED(bus, devfn))
+			outb((u8)val, GAPSPCI_BBA_CONFIG+where);
+		break;
+	case 2:
+		if (BBA_SELECTED(bus, devfn))
+			outw((u16)val, GAPSPCI_BBA_CONFIG+where);
+		break;
+	case 4:
+		if (BBA_SELECTED(bus, devfn))
+			outl(val, GAPSPCI_BBA_CONFIG+where);
+		break;
+		}
+	}
+	return PCIBIOS_SUCCESSFUL;
 }
 
 static struct pci_ops pci_config_ops = {
-        gapspci_read_config_byte,
-        gapspci_read_config_word,
-        gapspci_read_config_dword,
-        gapspci_write_config_byte,
-        gapspci_write_config_word,
-        gapspci_write_config_dword
+	.read = 	gapspci_read,
+	.write = 	gapspci_write,
 };
 
 
@@ -143,7 +125,7 @@
 
 	for (ln=bus->devices.next; ln != &bus->devices; ln=ln->next) {
 		dev = pci_dev_b(ln);
-		if (!BBA_SELECTED(dev)) continue;
+		if (!BBA_SELECTED(bus, dev->devfn)) continue;
 
 		printk("PCI: MMIO fixup to %s\n", dev->name);
 		dev->resource[1].start=0x01001700;
#1 lore raw

Minor comments:

arch/sh/kernel/pci-dc.c line 62

Quoted 2 lines · Hanna Linder
arch/sh/kernel/pci-dc.c +2 -0
+static int gapspci_write(struct pci_bus *bus, unsigned int devfn,
+				    int where, u32 val)

You forgot the size parameter.

Quoted 12 lines

{

arch/sh/kernel/pci-dc.c +7 -4
-        if (BBA_SELECTED(dev))
-		outl(val, GAPSPCI_BBA_CONFIG+where);
-
-        return PCIBIOS_SUCCESSFUL;
+	if (BBA_SELECTED(bus, devfn)) {
+		switch (size) {
+	case 1:
+		if (BBA_SELECTED(bus, devfn))
+			outb((u8)val, GAPSPCI_BBA_CONFIG+where);
+		break;
+	case 2:

arch/sh/kernel/pci-dc.c line 80

Quoted 10 lines
arch/sh/kernel/pci-dc.c +10 -0
+		if (BBA_SELECTED(bus, devfn))
+			outw((u16)val, GAPSPCI_BBA_CONFIG+where);
+		break;
+	case 4:
+		if (BBA_SELECTED(bus, devfn))
+			outl(val, GAPSPCI_BBA_CONFIG+where);
+		break;
+		}
+	}
+	return PCIBIOS_SUCCESSFUL;

Your formatting is a bit off here (the case statements should be one level to the right.)

Other than that, looks good.

thanks,

greg k-h

Quoted 23 lines

}

static struct pci_ops pci_config_ops = {

arch/sh/kernel/pci-dc.c +3 -7
-        gapspci_read_config_byte,
-        gapspci_read_config_word,
-        gapspci_read_config_dword,
-        gapspci_write_config_byte,
-        gapspci_write_config_word,
-        gapspci_write_config_dword
+	.read = 	gapspci_read,
+	.write = 	gapspci_write,
 };
@@ -143,7 +125,7 @@
 
 	for (ln=bus->devices.next; ln != &bus->devices; ln=ln->next) {
 		dev = pci_dev_b(ln);
-		if (!BBA_SELECTED(dev)) continue;
+		if (!BBA_SELECTED(bus, dev->devfn)) continue;
 
 		printk("PCI: MMIO fixup to %s\n", dev->name);
 		dev->resource[1].start=0x01001700;
#2 lore raw
In reply to Hanna Linder
Quoted 3 lines · Hanna Linder

Here is the first part of the sh port of the pci_ops changes. If anyone can compile this for Sega let me know if there are any problems.

The 'val' pointer is declared 'u32 *', then casted 'u8 *' or 'u16 *' if needed. The compiler will not warn you. But user that wants to operate on u8 or u16 has to cast the 'val' argument to 'u32 *' and should get a warning from any decent C compiler. The normal C-way for such 'sorry-typed' argument is 'void *val', IMO.

Gérard.

Quoted 148 lines

Thanks.

Hanna Linder
[email protected]

ps -this patches against bk://linuxusb.bkbits.net/pci_hp-2.5

-----

diff -Nru a/arch/sh/kernel/pci-dc.c b/arch/sh/kernel/pci-dc.c

arch/sh/kernel/pci-dc.c +44 -63
@@ -31,76 +31,58 @@
 	{0, 0, 0, NULL}
 };
 
-#define BBA_SELECTED(dev) (dev->bus->number==0 && dev->devfn==0)
+#define BBA_SELECTED(bus,devfn) (bus->number==0 && devfn==0)
 
-static int gapspci_read_config_byte(struct pci_dev *dev, int where,
-                                    u8 * val)
+static int gapspci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 * val)
 {
-	if (BBA_SELECTED(dev))
-		*val = inb(GAPSPCI_BBA_CONFIG+where);
-	else
-                *val = 0xff;
-
+	switch (size) {
+	case 1:
+		if (BBA_SELECTED(bus, devfn))
+			*val = (u8)inb(GAPSPCI_BBA_CONFIG+where);
+		else
+			*val = (u8)0xff;
+		break;
+	case 2:
+		if (BBA_SELECTED(bus, devfn))
+			*val = (u16)inw(GAPSPCI_BBA_CONFIG+where);
+		else
+			*val = (u16)0xffff;
+		break;
+	case 4:
+		if (BBA_SELECTED(bus, devfn))
+			*val = inl(GAPSPCI_BBA_CONFIG+where);
+		else
+			*val = 0xffffffff;
+		break;
+	}
 	return PCIBIOS_SUCCESSFUL;
 }
 
-static int gapspci_read_config_word(struct pci_dev *dev, int where,
-                                    u16 * val)
-{
-        if (BBA_SELECTED(dev))
-		*val = inw(GAPSPCI_BBA_CONFIG+where);
-	else
-                *val = 0xffff;
-
-        return PCIBIOS_SUCCESSFUL;
-}
-
-static int gapspci_read_config_dword(struct pci_dev *dev, int where,
-                                     u32 * val)
-{
-        if (BBA_SELECTED(dev))
-		*val = inl(GAPSPCI_BBA_CONFIG+where);
-	else
-                *val = 0xffffffff;
-
-        return PCIBIOS_SUCCESSFUL;
-}
-
-static int gapspci_write_config_byte(struct pci_dev *dev, int where,
-                                     u8 val)
-{
-        if (BBA_SELECTED(dev))
-		outb(val, GAPSPCI_BBA_CONFIG+where);
-
-        return PCIBIOS_SUCCESSFUL;
-}
-
-
-static int gapspci_write_config_word(struct pci_dev *dev, int where,
-                                     u16 val)
-{
-        if (BBA_SELECTED(dev))
-		outw(val, GAPSPCI_BBA_CONFIG+where);
-
-        return PCIBIOS_SUCCESSFUL;
-}
-
-static int gapspci_write_config_dword(struct pci_dev *dev, int where,
-                                      u32 val)
+static int gapspci_write(struct pci_bus *bus, unsigned int devfn,
+				    int where, u32 val)
 {
-        if (BBA_SELECTED(dev))
-		outl(val, GAPSPCI_BBA_CONFIG+where);
-
-        return PCIBIOS_SUCCESSFUL;
+	if (BBA_SELECTED(bus, devfn)) {
+		switch (size) {
+	case 1:
+		if (BBA_SELECTED(bus, devfn))
+			outb((u8)val, GAPSPCI_BBA_CONFIG+where);
+		break;
+	case 2:
+		if (BBA_SELECTED(bus, devfn))
+			outw((u16)val, GAPSPCI_BBA_CONFIG+where);
+		break;
+	case 4:
+		if (BBA_SELECTED(bus, devfn))
+			outl(val, GAPSPCI_BBA_CONFIG+where);
+		break;
+		}
+	}
+	return PCIBIOS_SUCCESSFUL;
 }
 
 static struct pci_ops pci_config_ops = {
-        gapspci_read_config_byte,
-        gapspci_read_config_word,
-        gapspci_read_config_dword,
-        gapspci_write_config_byte,
-        gapspci_write_config_word,
-        gapspci_write_config_dword
+	.read = 	gapspci_read,
+	.write = 	gapspci_write,
 };
 
 
@@ -143,7 +125,7 @@
 
 	for (ln=bus->devices.next; ln != &bus->devices; ln=ln->next) {
 		dev = pci_dev_b(ln);
-		if (!BBA_SELECTED(dev)) continue;
+		if (!BBA_SELECTED(bus, dev->devfn)) continue;
 
 		printk("PCI: MMIO fixup to %s\n", dev->name);
 		dev->resource[1].start=0x01001700;
 
-

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

#3 lore raw
Quoted 8 lines · Gérard Roudier
Quoted 3 lines · Hanna Linder

Here is the first part of the sh port of the pci_ops changes. If anyone can compile this for Sega let me know if there are any problems.

Quoted 5 lines

The 'val' pointer is declared 'u32 *', then casted 'u8 *' or 'u16 *' if needed. The compiler will not warn you. But user that wants to operate on u8 or u16 has to cast the 'val' argument to 'u32 *' and should get a warning from any decent C compiler. The normal C-way for such 'sorry-typed' argument is 'void *val', IMO.

We are filling up a u32 here (see the previous patches), so leaving this as a u32 * and casting for the other sizes makes sense in this situation.

thanks,

greg k-h