2002-08-22 00:51:21

by Hanna Linder

[permalink] [raw]
Subject: Re: PCI Cleanup


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

-----

diff -Nru a/arch/sh/kernel/pci-dc.c b/arch/sh/kernel/pci-dc.c
--- a/arch/sh/kernel/pci-dc.c Wed Aug 21 17:55:02 2002
+++ b/arch/sh/kernel/pci-dc.c Wed Aug 21 17:55:02 2002
@@ -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;


2002-08-22 03:12:01

by Greg KH

[permalink] [raw]
Subject: Re: PCI Cleanup

Minor comments:

On Wed, Aug 21, 2002 at 05:59:31PM -0700, Hanna Linder wrote:
> +static int gapspci_write(struct pci_bus *bus, unsigned int devfn,
> + int where, u32 val)

You forgot the size parameter.

> {
> - 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;

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

> }
>
> 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;

2002-08-22 08:23:23

by Gérard Roudier

[permalink] [raw]
Subject: Re: PCI Cleanup



On Wed, 21 Aug 2002, Hanna Linder wrote:

> 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.

> 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
> --- a/arch/sh/kernel/pci-dc.c Wed Aug 21 17:55:02 2002
> +++ b/arch/sh/kernel/pci-dc.c Wed Aug 21 17:55:02 2002
> @@ -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/
>

2002-08-22 15:38:16

by Greg KH

[permalink] [raw]
Subject: Re: PCI Cleanup

On Thu, Aug 22, 2002 at 12:11:41PM +0200, G?rard Roudier wrote:
>
>
> On Wed, 21 Aug 2002, Hanna Linder wrote:
>
> > 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.

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