Hello Felix Singer, Nico Huber, Arthur Heymans, Michael Niewöhner, Patrick Rudolph,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/48364
to review the following change.
Change subject: sb/intel/x/smbus.c: Factor out common code ......................................................................
sb/intel/x/smbus.c: Factor out common code
Since common smbus.c gets built for romstage as well, create a new file to hold this common code. Account for ICH7 not having a memory BAR, too.
Change-Id: I4ab46750c6fb7f71cbd55848e79ecc3e44cbbd04 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/bd82x6x/smbus.c M src/southbridge/intel/common/Makefile.inc A src/southbridge/intel/common/smbus_ops.c A src/southbridge/intel/common/smbus_ops.h M src/southbridge/intel/i82801gx/smbus.c M src/southbridge/intel/i82801ix/smbus.c M src/southbridge/intel/i82801jx/smbus.c M src/southbridge/intel/ibexpeak/smbus.c M src/southbridge/intel/lynxpoint/smbus.c 9 files changed, 99 insertions(+), 411 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/48364/1
diff --git a/src/southbridge/intel/bd82x6x/smbus.c b/src/southbridge/intel/bd82x6x/smbus.c index 0da3b76..9ba9e12 100644 --- a/src/southbridge/intel/bd82x6x/smbus.c +++ b/src/southbridge/intel/bd82x6x/smbus.c @@ -8,6 +8,7 @@ #include <device/pci_ids.h> #include <device/pci_ops.h> #include <device/smbus_host.h> +#include <southbridge/intel/common/smbus_ops.h> #include "pch.h"
static void pch_smbus_init(struct device *dev) @@ -27,75 +28,6 @@ smbus_set_slave_addr(res->base, SMBUS_SLAVE_ADDR); }
-static int lsmbus_read_byte(struct device *dev, u8 address) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - - return do_smbus_read_byte(res->base, device, address); -} - -static int lsmbus_write_byte(struct device *dev, u8 address, u8 data) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - return do_smbus_write_byte(res->base, device, address, data); -} - -static int lsmbus_block_write(struct device *dev, u8 cmd, u8 bytes, const u8 *buf) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - return do_smbus_block_write(res->base, device, cmd, bytes, buf); -} - -static int lsmbus_block_read(struct device *dev, u8 cmd, u8 bytes, u8 *buf) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - return do_smbus_block_read(res->base, device, cmd, bytes, buf); -} - -static struct smbus_bus_operations lops_smbus_bus = { - .read_byte = lsmbus_read_byte, - .write_byte = lsmbus_write_byte, - .block_read = lsmbus_block_read, - .block_write = lsmbus_block_write, -}; - -static void smbus_read_resources(struct device *dev) -{ - struct resource *res = new_resource(dev, PCI_BASE_ADDRESS_4); - res->base = CONFIG_FIXED_SMBUS_IO_BASE; - res->size = 32; - res->limit = res->base + res->size - 1; - res->flags = IORESOURCE_IO | IORESOURCE_FIXED | IORESOURCE_RESERVE | - IORESOURCE_STORED | IORESOURCE_ASSIGNED; - - /* Also add MMIO resource */ - res = pci_get_resource(dev, PCI_BASE_ADDRESS_0); -} - static const char *smbus_acpi_name(const struct device *dev) { return "SBUS"; diff --git a/src/southbridge/intel/common/Makefile.inc b/src/southbridge/intel/common/Makefile.inc index 1ededd2..6c57481 100644 --- a/src/southbridge/intel/common/Makefile.inc +++ b/src/southbridge/intel/common/Makefile.inc @@ -9,6 +9,7 @@
romstage-$(CONFIG_SOUTHBRIDGE_INTEL_COMMON_SMBUS) += smbus.c ramstage-$(CONFIG_SOUTHBRIDGE_INTEL_COMMON_SMBUS) += smbus.c +ramstage-$(CONFIG_SOUTHBRIDGE_INTEL_COMMON_SMBUS) += smbus_ops.c
romstage-$(CONFIG_SOUTHBRIDGE_INTEL_COMMON_PMCLIB) += pmclib.c
diff --git a/src/southbridge/intel/common/smbus_ops.c b/src/southbridge/intel/common/smbus_ops.c new file mode 100644 index 0000000..b0ecc1a --- /dev/null +++ b/src/southbridge/intel/common/smbus_ops.c @@ -0,0 +1,83 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <device/device.h> +#include <device/path.h> +#include <device/smbus.h> +#include <device/pci.h> +#include <device/pci_def.h> +#include <device/pci_ids.h> +#include <device/smbus_host.h> +#include <southbridge/intel/common/smbus_ops.h> + +static int lsmbus_read_byte(struct device *dev, u8 address) +{ + u16 device; + struct resource *res; + struct bus *pbus; + + device = dev->path.i2c.device; + pbus = get_pbus_smbus(dev); + res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); + + return do_smbus_read_byte(res->base, device, address); +} + +static int lsmbus_write_byte(struct device *dev, u8 address, u8 data) +{ + u16 device; + struct resource *res; + struct bus *pbus; + + device = dev->path.i2c.device; + pbus = get_pbus_smbus(dev); + res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); + return do_smbus_write_byte(res->base, device, address, data); +} + +static int lsmbus_block_write(struct device *dev, u8 cmd, u8 bytes, const u8 *buf) +{ + u16 device; + struct resource *res; + struct bus *pbus; + + device = dev->path.i2c.device; + pbus = get_pbus_smbus(dev); + res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); + return do_smbus_block_write(res->base, device, cmd, bytes, buf); +} + +static int lsmbus_block_read(struct device *dev, u8 cmd, u8 bytes, u8 *buf) +{ + u16 device; + struct resource *res; + struct bus *pbus; + + device = dev->path.i2c.device; + pbus = get_pbus_smbus(dev); + res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); + return do_smbus_block_read(res->base, device, cmd, bytes, buf); +} + +struct smbus_bus_operations lops_smbus_bus = { + .read_byte = lsmbus_read_byte, + .write_byte = lsmbus_write_byte, + .block_read = lsmbus_block_read, + .block_write = lsmbus_block_write, +}; + +void smbus_read_resources(struct device *dev) +{ + struct resource *res = new_resource(dev, PCI_BASE_ADDRESS_4); + res->base = CONFIG_FIXED_SMBUS_IO_BASE; + res->size = 32; + res->limit = res->base + res->size - 1; + res->flags = IORESOURCE_IO | IORESOURCE_FIXED | IORESOURCE_RESERVE | + IORESOURCE_STORED | IORESOURCE_ASSIGNED; + + /* The memory BAR does not exist for ICH7 and earlier */ + if (CONFIG(SOUTHBRIDGE_INTEL_I82801GX)) + return; + + /* Also add MMIO resource */ + res = pci_get_resource(dev, PCI_BASE_ADDRESS_0); +} diff --git a/src/southbridge/intel/common/smbus_ops.h b/src/southbridge/intel/common/smbus_ops.h new file mode 100644 index 0000000..fbdbb99 --- /dev/null +++ b/src/southbridge/intel/common/smbus_ops.h @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <device/device.h> +#include <device/smbus.h> +#include <device/smbus_host.h> + +extern struct smbus_bus_operations lops_smbus_bus; + +void smbus_read_resources(struct device *dev); diff --git a/src/southbridge/intel/i82801gx/smbus.c b/src/southbridge/intel/i82801gx/smbus.c index 5915f3d..739e552 100644 --- a/src/southbridge/intel/i82801gx/smbus.c +++ b/src/southbridge/intel/i82801gx/smbus.c @@ -7,74 +7,9 @@ #include <device/pci_def.h> #include <device/pci_ids.h> #include <device/smbus_host.h> +#include <southbridge/intel/common/smbus_ops.h> #include "i82801gx.h"
-static int lsmbus_read_byte(struct device *dev, u8 address) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - - return do_smbus_read_byte(res->base, device, address); -} - -static int lsmbus_write_byte(struct device *dev, u8 address, u8 data) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - return do_smbus_write_byte(res->base, device, address, data); -} - -static int lsmbus_block_write(struct device *dev, u8 cmd, u8 bytes, const u8 *buf) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - return do_smbus_block_write(res->base, device, cmd, bytes, buf); -} - -static int lsmbus_block_read(struct device *dev, u8 cmd, u8 bytes, u8 *buf) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - return do_smbus_block_read(res->base, device, cmd, bytes, buf); -} - -static struct smbus_bus_operations lops_smbus_bus = { - .read_byte = lsmbus_read_byte, - .write_byte = lsmbus_write_byte, - .block_read = lsmbus_block_read, - .block_write = lsmbus_block_write, -}; - -static void smbus_read_resources(struct device *dev) -{ - struct resource *res = new_resource(dev, PCI_BASE_ADDRESS_4); - res->base = CONFIG_FIXED_SMBUS_IO_BASE; - res->size = 32; - res->limit = res->base + res->size - 1; - res->flags = IORESOURCE_IO | IORESOURCE_FIXED | IORESOURCE_RESERVE | - IORESOURCE_STORED | IORESOURCE_ASSIGNED; -} - static struct device_operations smbus_ops = { .read_resources = smbus_read_resources, .set_resources = pci_dev_set_resources, diff --git a/src/southbridge/intel/i82801ix/smbus.c b/src/southbridge/intel/i82801ix/smbus.c index 8daaf2e..1ccd9f5 100644 --- a/src/southbridge/intel/i82801ix/smbus.c +++ b/src/southbridge/intel/i82801ix/smbus.c @@ -8,6 +8,7 @@ #include <device/pci_ids.h> #include <device/pci_ops.h> #include <device/smbus_host.h> +#include <southbridge/intel/common/smbus_ops.h> #include "i82801ix.h"
static void pch_smbus_init(struct device *dev) @@ -16,75 +17,6 @@ pci_and_config16(dev, 0x80, ~((1 << 8) | (1 << 10) | (1 << 12) | (1 << 14))); }
-static int lsmbus_read_byte(struct device *dev, u8 address) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - - return do_smbus_read_byte(res->base, device, address); -} - -static int lsmbus_write_byte(struct device *dev, u8 address, u8 data) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - return do_smbus_write_byte(res->base, device, address, data); -} - -static int lsmbus_block_write(struct device *dev, u8 cmd, u8 bytes, const u8 *buf) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - return do_smbus_block_write(res->base, device, cmd, bytes, buf); -} - -static int lsmbus_block_read(struct device *dev, u8 cmd, u8 bytes, u8 *buf) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - return do_smbus_block_read(res->base, device, cmd, bytes, buf); -} - -static struct smbus_bus_operations lops_smbus_bus = { - .read_byte = lsmbus_read_byte, - .write_byte = lsmbus_write_byte, - .block_read = lsmbus_block_read, - .block_write = lsmbus_block_write, -}; - -static void smbus_read_resources(struct device *dev) -{ - struct resource *res = new_resource(dev, PCI_BASE_ADDRESS_4); - res->base = CONFIG_FIXED_SMBUS_IO_BASE; - res->size = 32; - res->limit = res->base + res->size - 1; - res->flags = IORESOURCE_IO | IORESOURCE_FIXED | IORESOURCE_RESERVE | - IORESOURCE_STORED | IORESOURCE_ASSIGNED; - - /* Also add MMIO resource */ - res = pci_get_resource(dev, PCI_BASE_ADDRESS_0); -} - static struct device_operations smbus_ops = { .read_resources = smbus_read_resources, .set_resources = pci_dev_set_resources, diff --git a/src/southbridge/intel/i82801jx/smbus.c b/src/southbridge/intel/i82801jx/smbus.c index 2e2491a..11a3daa 100644 --- a/src/southbridge/intel/i82801jx/smbus.c +++ b/src/southbridge/intel/i82801jx/smbus.c @@ -8,6 +8,7 @@ #include <device/pci_ids.h> #include <device/pci_ops.h> #include <device/smbus_host.h> +#include <southbridge/intel/common/smbus_ops.h> #include "i82801jx.h"
static void pch_smbus_init(struct device *dev) @@ -16,75 +17,6 @@ pci_and_config16(dev, 0x80, ~((1 << 8) | (1 << 10) | (1 << 12) | (1 << 14))); }
-static int lsmbus_read_byte(struct device *dev, u8 address) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - - return do_smbus_read_byte(res->base, device, address); -} - -static int lsmbus_write_byte(struct device *dev, u8 address, u8 data) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - return do_smbus_write_byte(res->base, device, address, data); -} - -static int lsmbus_block_write(struct device *dev, u8 cmd, u8 bytes, const u8 *buf) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - return do_smbus_block_write(res->base, device, cmd, bytes, buf); -} - -static int lsmbus_block_read(struct device *dev, u8 cmd, u8 bytes, u8 *buf) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - return do_smbus_block_read(res->base, device, cmd, bytes, buf); -} - -static struct smbus_bus_operations lops_smbus_bus = { - .read_byte = lsmbus_read_byte, - .write_byte = lsmbus_write_byte, - .block_read = lsmbus_block_read, - .block_write = lsmbus_block_write, -}; - -static void smbus_read_resources(struct device *dev) -{ - struct resource *res = new_resource(dev, PCI_BASE_ADDRESS_4); - res->base = CONFIG_FIXED_SMBUS_IO_BASE; - res->size = 32; - res->limit = res->base + res->size - 1; - res->flags = IORESOURCE_IO | IORESOURCE_FIXED | IORESOURCE_RESERVE | - IORESOURCE_STORED | IORESOURCE_ASSIGNED; - - /* Also add MMIO resource */ - res = pci_get_resource(dev, PCI_BASE_ADDRESS_0); -} - static struct device_operations smbus_ops = { .read_resources = smbus_read_resources, .set_resources = pci_dev_set_resources, diff --git a/src/southbridge/intel/ibexpeak/smbus.c b/src/southbridge/intel/ibexpeak/smbus.c index 7e976e0..352b589 100644 --- a/src/southbridge/intel/ibexpeak/smbus.c +++ b/src/southbridge/intel/ibexpeak/smbus.c @@ -8,6 +8,7 @@ #include <device/pci_ids.h> #include <device/pci_ops.h> #include <device/smbus_host.h> +#include <southbridge/intel/common/smbus_ops.h> #include "pch.h"
static void pch_smbus_init(struct device *dev) @@ -26,75 +27,6 @@ smbus_set_slave_addr(res->base, SMBUS_SLAVE_ADDR); }
-static int lsmbus_read_byte(struct device *dev, u8 address) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - - return do_smbus_read_byte(res->base, device, address); -} - -static int lsmbus_write_byte(struct device *dev, u8 address, u8 data) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - return do_smbus_write_byte(res->base, device, address, data); -} - -static int lsmbus_block_write(struct device *dev, u8 cmd, u8 bytes, const u8 *buf) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - return do_smbus_block_write(res->base, device, cmd, bytes, buf); -} - -static int lsmbus_block_read(struct device *dev, u8 cmd, u8 bytes, u8 *buf) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - return do_smbus_block_read(res->base, device, cmd, bytes, buf); -} - -static struct smbus_bus_operations lops_smbus_bus = { - .read_byte = lsmbus_read_byte, - .write_byte = lsmbus_write_byte, - .block_read = lsmbus_block_read, - .block_write = lsmbus_block_write, -}; - -static void smbus_read_resources(struct device *dev) -{ - struct resource *res = new_resource(dev, PCI_BASE_ADDRESS_4); - res->base = CONFIG_FIXED_SMBUS_IO_BASE; - res->size = 32; - res->limit = res->base + res->size - 1; - res->flags = IORESOURCE_IO | IORESOURCE_FIXED | IORESOURCE_RESERVE | - IORESOURCE_STORED | IORESOURCE_ASSIGNED; - - /* Also add MMIO resource */ - res = pci_get_resource(dev, PCI_BASE_ADDRESS_0); -} - static struct device_operations smbus_ops = { .read_resources = smbus_read_resources, .set_resources = pci_dev_set_resources, diff --git a/src/southbridge/intel/lynxpoint/smbus.c b/src/southbridge/intel/lynxpoint/smbus.c index a173b43..e2d42ac 100644 --- a/src/southbridge/intel/lynxpoint/smbus.c +++ b/src/southbridge/intel/lynxpoint/smbus.c @@ -7,6 +7,7 @@ #include <device/pci_ids.h> #include <device/pci_ops.h> #include <device/smbus_host.h> +#include <southbridge/intel/common/smbus_ops.h> #include "pch.h"
static void pch_smbus_init(struct device *dev) @@ -26,75 +27,6 @@ smbus_set_slave_addr(res->base, SMBUS_SLAVE_ADDR); }
-static int lsmbus_read_byte(struct device *dev, u8 address) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - - return do_smbus_read_byte(res->base, device, address); -} - -static int lsmbus_write_byte(struct device *dev, u8 address, u8 data) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - return do_smbus_write_byte(res->base, device, address, data); -} - -static int lsmbus_block_write(struct device *dev, u8 cmd, u8 bytes, const u8 *buf) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - return do_smbus_block_write(res->base, device, cmd, bytes, buf); -} - -static int lsmbus_block_read(struct device *dev, u8 cmd, u8 bytes, u8 *buf) -{ - u16 device; - struct resource *res; - struct bus *pbus; - - device = dev->path.i2c.device; - pbus = get_pbus_smbus(dev); - res = find_resource(pbus->dev, PCI_BASE_ADDRESS_4); - return do_smbus_block_read(res->base, device, cmd, bytes, buf); -} - -static struct smbus_bus_operations lops_smbus_bus = { - .read_byte = lsmbus_read_byte, - .write_byte = lsmbus_write_byte, - .block_read = lsmbus_block_read, - .block_write = lsmbus_block_write, -}; - -static void smbus_read_resources(struct device *dev) -{ - struct resource *res = new_resource(dev, PCI_BASE_ADDRESS_4); - res->base = CONFIG_FIXED_SMBUS_IO_BASE; - res->size = 32; - res->limit = res->base + res->size - 1; - res->flags = IORESOURCE_IO | IORESOURCE_FIXED | IORESOURCE_RESERVE | - IORESOURCE_STORED | IORESOURCE_ASSIGNED; - - /* Also add MMIO resource */ - res = pci_get_resource(dev, PCI_BASE_ADDRESS_0); -} - static struct device_operations smbus_ops = { .read_resources = smbus_read_resources, .set_resources = pci_dev_set_resources,