Messages in this thread | | | Date | Thu, 11 Nov 2010 12:51:27 +0100 (CET) | From | Thomas Gleixner <> | Subject | Re: [PATCH 2/6] ce4100: add PCI register emulation for CE4100 |
| |
On Tue, 9 Nov 2010, dirk.brandewie@gmail.com wrote: > + > +static int ce4100_conf_read(unsigned int seg, unsigned int bus, > + unsigned int devfn, int reg, int len, u32 *value) > +{ > + int retval = 1; > + int i; > + > + if (bus == 0 && (PCI_DEVFN(1, 0) == devfn)) > + retval = bridge_read(devfn, reg, len, value); > + > + if (bus == 1) { > + for (i = 0; i < num_bus1_fixups; i++) { > + if (bus1_fixups[i].dev_func == devfn && > + bus1_fixups[i].reg == (reg & ~3) && > + bus1_fixups[i].read) { > + bus1_fixups[i].read(&(bus1_fixups[i]), > + value); > + extract_bytes(value, reg, len); > + retval = 0;
Shouldn't we return 0 here and be done ?
> + } > + } > + } > + if (retval) > + retval = pci_direct_conf1.read(seg, bus, devfn, reg, > + len, value); > + return retval; > +} > + > + > +static int ce4100_conf_write(unsigned int seg, unsigned int bus, > + unsigned int devfn, int reg, int len, u32 value) > +{ > + int retval = 1; > + int i; > + > + if (bus == 1) { > + for (i = 0; i < num_bus1_fixups; i++) { > + if (bus1_fixups[i].dev_func == devfn && > + bus1_fixups[i].reg == (reg & ~3) && > + bus1_fixups[i].write) { > + bus1_fixups[i].write(&(bus1_fixups[i]), > + value); > + retval = 0;
Ditto
> + } > + } > + } > + > + /* Discard writes to A/V bridge BAR. */ > + if (bus == 0 && PCI_DEVFN(1, 0) == devfn && > + ((reg & ~3) == PCI_BASE_ADDRESS_0)) > + retval = 0;
Ditto
> + > + if (retval) > + retval = pci_direct_conf1.write(seg, bus, devfn, reg, > + len, value); > + return retval; > +}
It took me some time to grok the above retval magic. I'd rather write it this way:
+static int ce4100_conf_read(unsigned int seg, unsigned int bus, + unsigned int devfn, int reg, int len, u32 *value) +{ + int i; + + if (bus == 1) { + for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) { + if (bus1_fixups[i].dev_func == devfn && + bus1_fixups[i].reg == (reg & ~3) && + bus1_fixups[i].read) { + bus1_fixups[i].read(&(bus1_fixups[i]), + value); + extract_bytes(value, reg, len); + return 0; + } + } + } + + if (bus == 0 && (PCI_DEVFN(1, 0) == devfn) && + !bridge_read(devfn, reg, len, value)) + return 0; + + return pci_direct_conf1.read(seg, bus, devfn, reg, len, value); +} + +static int ce4100_conf_write(unsigned int seg, unsigned int bus, + unsigned int devfn, int reg, int len, u32 value) +{ + int i; + + if (bus == 1) { + for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) { + if (bus1_fixups[i].dev_func == devfn && + bus1_fixups[i].reg == (reg & ~3) && + bus1_fixups[i].write) { + bus1_fixups[i].write(&(bus1_fixups[i]), + value); + return 0; + } + } + } + + /* Discard writes to A/V bridge BAR. */ + if (bus == 0 && PCI_DEVFN(1, 0) == devfn && + ((reg & ~3) == PCI_BASE_ADDRESS_0)) + return 0; + + return pci_direct_conf1.write(seg, bus, devfn, reg, len, value); +}
Thanks,
tglx
| |