[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 11 Nov 2010 12:51:27 +0100 (CET)
From: Thomas Gleixner <tglx@...utronix.de>
To: Dirk Brandewie <dirk.brandewie@...il.com>
cc: linux-kernel@...r.kernel.org,
Dirk Brandewie <dirk.j.brandewie@...el.com>, x86@...nel.org
Subject: Re: [PATCH 2/6] ce4100: add PCI register emulation for CE4100
On Tue, 9 Nov 2010, dirk.brandewie@...il.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists