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/+/48363
to review the following change.
Change subject: sb/intel/x/smbus.c: Add block read/write support ......................................................................
sb/intel/x/smbus.c: Add block read/write support
Copy and paste the i82801gx code onto all newer southbridges. This will be factored out into common code in a follow-up.
Change-Id: Ic4b7d657865f61703e4310423c565786badf6f40 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/bd82x6x/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 5 files changed, 107 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/48363/1
diff --git a/src/southbridge/intel/bd82x6x/smbus.c b/src/southbridge/intel/bd82x6x/smbus.c index 4255a47..0da3b76 100644 --- a/src/southbridge/intel/bd82x6x/smbus.c +++ b/src/southbridge/intel/bd82x6x/smbus.c @@ -49,13 +49,38 @@ 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) diff --git a/src/southbridge/intel/i82801ix/smbus.c b/src/southbridge/intel/i82801ix/smbus.c index 4f46217..8daaf2e 100644 --- a/src/southbridge/intel/i82801ix/smbus.c +++ b/src/southbridge/intel/i82801ix/smbus.c @@ -38,13 +38,38 @@ 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) diff --git a/src/southbridge/intel/i82801jx/smbus.c b/src/southbridge/intel/i82801jx/smbus.c index c3a798a..2e2491a 100644 --- a/src/southbridge/intel/i82801jx/smbus.c +++ b/src/southbridge/intel/i82801jx/smbus.c @@ -38,12 +38,10 @@ 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) +static int lsmbus_block_write(struct device *dev, u8 cmd, u8 bytes, const u8 *buf) { u16 device; struct resource *res; @@ -70,8 +68,8 @@ 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, + .block_read = lsmbus_block_read, + .block_write = lsmbus_block_write, };
static void smbus_read_resources(struct device *dev) diff --git a/src/southbridge/intel/ibexpeak/smbus.c b/src/southbridge/intel/ibexpeak/smbus.c index 331021f..7e976e0 100644 --- a/src/southbridge/intel/ibexpeak/smbus.c +++ b/src/southbridge/intel/ibexpeak/smbus.c @@ -48,13 +48,38 @@ 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) diff --git a/src/southbridge/intel/lynxpoint/smbus.c b/src/southbridge/intel/lynxpoint/smbus.c index d0a621e..a173b43 100644 --- a/src/southbridge/intel/lynxpoint/smbus.c +++ b/src/southbridge/intel/lynxpoint/smbus.c @@ -51,9 +51,35 @@ 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)
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48363 )
Change subject: sb/intel/x/smbus.c: Add block read/write support ......................................................................
Patch Set 1: Code-Review+2
Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48363 )
Change subject: sb/intel/x/smbus.c: Add block read/write support ......................................................................
sb/intel/x/smbus.c: Add block read/write support
Copy and paste the i82801gx code onto all newer southbridges. This will be factored out into common code in a follow-up.
Change-Id: Ic4b7d657865f61703e4310423c565786badf6f40 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/48363 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M src/southbridge/intel/bd82x6x/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 5 files changed, 107 insertions(+), 8 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/src/southbridge/intel/bd82x6x/smbus.c b/src/southbridge/intel/bd82x6x/smbus.c index 4255a47..0da3b76 100644 --- a/src/southbridge/intel/bd82x6x/smbus.c +++ b/src/southbridge/intel/bd82x6x/smbus.c @@ -49,13 +49,38 @@ 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) diff --git a/src/southbridge/intel/i82801ix/smbus.c b/src/southbridge/intel/i82801ix/smbus.c index 4f46217..8daaf2e 100644 --- a/src/southbridge/intel/i82801ix/smbus.c +++ b/src/southbridge/intel/i82801ix/smbus.c @@ -38,13 +38,38 @@ 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) diff --git a/src/southbridge/intel/i82801jx/smbus.c b/src/southbridge/intel/i82801jx/smbus.c index c3a798a..2e2491a 100644 --- a/src/southbridge/intel/i82801jx/smbus.c +++ b/src/southbridge/intel/i82801jx/smbus.c @@ -38,12 +38,10 @@ 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) +static int lsmbus_block_write(struct device *dev, u8 cmd, u8 bytes, const u8 *buf) { u16 device; struct resource *res; @@ -70,8 +68,8 @@ 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, + .block_read = lsmbus_block_read, + .block_write = lsmbus_block_write, };
static void smbus_read_resources(struct device *dev) diff --git a/src/southbridge/intel/ibexpeak/smbus.c b/src/southbridge/intel/ibexpeak/smbus.c index 331021f..7e976e0 100644 --- a/src/southbridge/intel/ibexpeak/smbus.c +++ b/src/southbridge/intel/ibexpeak/smbus.c @@ -48,13 +48,38 @@ 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) diff --git a/src/southbridge/intel/lynxpoint/smbus.c b/src/southbridge/intel/lynxpoint/smbus.c index d0a621e..a173b43 100644 --- a/src/southbridge/intel/lynxpoint/smbus.c +++ b/src/southbridge/intel/lynxpoint/smbus.c @@ -51,9 +51,35 @@ 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)