[Pdbg] [PATCH 2/3] libpdbg: Add i2c get and put functions for i2c master on CFAM
Alistair Popple
alistair at popple.id.au
Mon Apr 15 15:09:50 AEST 2019
> > + //TODO: if size > 64kB then use I2C_CMD_READ_CONTINUE and do
> > multiple commands
> > + if (size > 64*1024 -1)
> > + size = 64*1024 -1;
>
> Do we want to log something here? It's good to let user know if we are
> doing something different from what user asked.
If the size is too big we should log an error and abort with an error code
rather than attempt to continue. The use can always rerun with a smaller size.
> > +
> > + /* Tell i2c master to read from slave device's fifo */
> > + fsidata = I2C_CMD_WITH_START | I2C_CMD_WITH_STOP |
> > I2C_CMD_WITH_ADDR | I2C_CMD_READ_NOT_WRITE;
> > + fsidata = SETFIELD(I2C_CMD_DEV_ADDR, fsidata, addr);
> > + fsidata = SETFIELD(I2C_CMD_LENGTH, fsidata, size);
> > + _i2cm_reg_write(i2cm, I2C_CMD_REG, fsidata);
> > +
> > + bytes_read = i2c_fifo_read(i2cm, (uint32_t*)d, size);
> > +
> > + rc = i2c_poll_status(i2cm, &data);
> > + if (rc) {
> > + PR_ERROR("ERROR while reading FIFO\n");
> > + return rc;
> > + }
> > +
> > + if (bytes_read < size) {
> > + PR_ERROR("ERROR didn't read all bytes %i\n",
> > bytes_read);
> > + debug_print_reg(i2cm);
> > + return rc;
> > +
> > + }
> > +
> > + if (data & I2C_STAT_CMD_COMP)
> > + PR_INFO(" cmd completed!\n");
> > +
> > + return rc;
> > +}
> > +
> > +int i2cm_target_probe(struct pdbg_target *target)
> > +{
> > + return 0;
> > +}
> > +
> > +static struct i2cm i2c_fsi = {
> > + .target = {
> > + .name = "CFAM I2C Master",
> > + .compatible = "ibm,fsi-i2c-master",
> > + .class = "i2cm",
> > + .probe = i2cm_target_probe,
> > + },
> > + .read = i2c_get,
> > + .write = i2c_put,
> > +};
> > +DECLARE_HW_UNIT(i2c_fsi);
> > +
> > +////////////////////////////////////////////////////////////////////
> > /////////
> > +
> >
> > #ifndef DISABLE_I2CLIB
> > #include <i2c/smbus.h>
> >
> > diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h
> > index 9baab0e..1c60a21 100644
> > --- a/libpdbg/libpdbg.h
> > +++ b/libpdbg/libpdbg.h
> > @@ -125,6 +125,8 @@ int fsi_write(struct pdbg_target *target,
> > uint32_t addr, uint32_t val);
> >
> > int i2c_read(struct pdbg_target *target, uint16_t port, uint32_t
> >
> > addr,
> >
> > uint16_t size, uint8_t *data);
> >
> > +int i2c_write(struct pdbg_target *target, uint16_t port, uint32_t
> > addr,
> > + uint16_t size, uint8_t *data);
> >
> > int pib_read(struct pdbg_target *target, uint64_t addr, uint64_t
> >
> > *val);
> >
> > int pib_write(struct pdbg_target *target, uint64_t addr, uint64_t
> >
> > val);
> > diff --git a/libpdbg/target.c b/libpdbg/target.c
> > index f577bb3..ed7bc18 100644
> > --- a/libpdbg/target.c
> > +++ b/libpdbg/target.c
> > @@ -202,10 +202,23 @@ int i2c_read(struct pdbg_target *i2cm_dt,
> > uint16_t port, uint32_t addr, uint16_t
> >
> > i2cm_dt = get_class_target_addr(i2cm_dt, "i2cm", &addr64);
> > i2cm = target_to_i2cm(i2cm_dt);
> >
> > + i2cm->port = port;
> >
> > return i2cm->read(i2cm, addr, size, data);
> >
> > }
> >
> > +int i2c_write(struct pdbg_target *i2cm_dt, uint16_t port, uint32_t
> > addr, uint16_t size, uint8_t *data)
> > +{
> > + struct i2cm *i2cm;
> > + uint64_t addr64 = addr;
> > +
> > + i2cm_dt = get_class_target_addr(i2cm_dt, "i2cm", &addr64);
> > + i2cm = target_to_i2cm(i2cm_dt);
> > + i2cm->port = port;
> > +
> > + return i2cm->write(i2cm, addr, size, data);
> > +}
> > +
> >
> > int fsi_read(struct pdbg_target *fsi_dt, uint32_t addr, uint32_t
> >
> > *data)
> >
> > {
> >
> > struct fsi *fsi;
> >
> > diff --git a/libpdbg/target.h b/libpdbg/target.h
> > index 14f2b79..8b53a0e 100644
> > --- a/libpdbg/target.h
> > +++ b/libpdbg/target.h
> > @@ -141,7 +141,8 @@ struct fsi {
> >
> > struct i2cm {
> >
> > struct pdbg_target target;
> > int (*read)(struct i2cm *, uint32_t, uint16_t, uint8_t *);
> >
> > - int (*write)(struct i2cm *, uint32_t, uint16_t, uint8_t*);
> > + int (*write)(struct i2cm *, uint32_t, uint16_t, uint8_t *);
> > + uint8_t port;
> >
> > int i2c_fd;
> >
> > };
> > #define target_to_i2cm(x) container_of(x, struct i2cm, target)
> >
> > diff --git a/p9-fsi.dtsi.m4 b/p9-fsi.dtsi.m4
> > index afa7d39..89ba387 100644
> > --- a/p9-fsi.dtsi.m4
> > +++ b/p9-fsi.dtsi.m4
> > @@ -21,6 +21,13 @@
> >
> > include(p9-pib.dts.m4)dnl
> >
> > };
> >
> > + i2cm at 1800 {
> > + #address-cells = <0x2>;
> > + #size-cells = <0x1>;
> > + reg = <0x0 0x1800 0x400>;
> > + compatible = "ibm,fsi-i2c-master";
> > + };
> > +
> >
> > hmfsi at 100000 {
> >
> > #address-cells = <0x2>;
> > #size-cells = <0x1>;
> >
> > diff --git a/p9-kernel.dts.m4 b/p9-kernel.dts.m4
> > index 474beca..dacea83 100644
> > --- a/p9-kernel.dts.m4
> > +++ b/p9-kernel.dts.m4
> > @@ -26,7 +26,7 @@
> >
> > #address-cells = <0x2>;
> > #size-cells = <0x1>;
> > reg = <0x0 0x1800 0x400>;
> >
> > - compatible = "ibm,kernel-i2c-master";
> > + compatible = "ibm,fsi-i2c-master";
> >
> > include(p9-i2c.dts.m4)dnl
> >
> > };
> >
> > diff --git a/src/i2c.c b/src/i2c.c
> > index 9ac3d53..641355c 100644
> > --- a/src/i2c.c
> > +++ b/src/i2c.c
> > @@ -17,11 +17,14 @@
> >
> > #include <libpdbg.h>
> > #include <inttypes.h>
> > #include <stdlib.h>
> >
> > +#include <stdlib.h>
> > +#include <unistd.h>
> >
> > #include "main.h"
> > #include "optcmd.h"
> > #include "path.h"
> > #include "target.h"
> >
> > +#include "util.h"
> >
> > static int geti2c(uint16_t port, uint32_t addr, uint16_t size)
> > {
> >
> > @@ -30,8 +33,9 @@ static int geti2c(uint16_t port, uint32_t addr,
> > uint16_t size)
> >
> > data = malloc(size);
> > assert(data);
> >
> > + assert(!(size % 4));
> >
> > - for_each_path_target_class("i2cbus", target) {
> > + for_each_path_target_class("i2cm", target) {
> >
> > if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
> >
> > continue;
> >
> > selected = target;
> >
> > @@ -42,7 +46,37 @@ static int geti2c(uint16_t port, uint32_t addr,
> > uint16_t size)
> >
> > if (selected == NULL)
> >
> > return -1;
> >
> > - printf("data read: 0x%016" PRIx64 "\n", (uint64_t)*data);
> > +
> > + hexdump(size, data, size, 1);
>
> Do you mean hexdump(addr, ...) ?
>
> This adds a dependency on hexdump patches.
Yep. It was my suggestion, I should go review them :-)
> > return 0;
> >
> > }
> > OPTCMD_DEFINE_CMD_WITH_ARGS(geti2c, geti2c, (DATA16, ADDRESS32,
> >
> > DATA16));
> > +
> > +static int puti2c(uint16_t port, uint32_t addr, uint8_t offset,
> > uint16_t size, uint64_t data)
> > +{
> > + uint8_t *d = NULL;
> > + struct pdbg_target *target, *selected = NULL;
> > +
> > + size += sizeof(offset);
> > + assert(!(size % 4));
> > + d = malloc(size);
> > + assert(d);
> > +
> > + memcpy(d, &offset, sizeof(offset));
> > + memcpy(d + sizeof(offset), &data, sizeof(data));
> > +
> > + for_each_path_target_class("i2cm", target) {
> > + if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
> > + continue;
> > + selected = target;
> > + if (i2c_write(target, port, addr, size, d) == 0)
> > + break;
> > + break;
> > + }
> > +
> > + if (selected == NULL)
> > + return -1;
> > + printf("wrote %i bytes \n", size);
> > + return 0;
> > +}
> > +OPTCMD_DEFINE_CMD_WITH_ARGS(puti2c, puti2c, (DATA16, ADDRESS32,
> > DATA8, DATA16, DATA));
> > diff --git a/src/main.c b/src/main.c
> > index 1702efa..2e16b67 100644
> > --- a/src/main.c
> > +++ b/src/main.c
> > @@ -93,7 +93,7 @@ extern struct optcmd_cmd
> >
> > optcmd_threadstatus, optcmd_sreset, optcmd_regs, optcmd_probe,
> > optcmd_getmem, optcmd_putmem, optcmd_getmemio, optcmd_putmemio,
> > optcmd_getxer, optcmd_putxer, optcmd_getcr, optcmd_putcr,
> >
> > - optcmd_gdbserver, optcmd_geti2c;
> > + optcmd_gdbserver, optcmd_geti2c, optcmd_puti2c;
> >
> > static struct optcmd_cmd *cmds[] = {
> >
> > &optcmd_getscom, &optcmd_putscom, &optcmd_getcfam,
> >
> > &optcmd_putcfam,
> > @@ -103,7 +103,7 @@ static struct optcmd_cmd *cmds[] = {
> >
> > &optcmd_threadstatus, &optcmd_sreset, &optcmd_regs,
> >
> > &optcmd_probe,
> >
> > &optcmd_getmem, &optcmd_putmem, &optcmd_getmemio,
> >
> > &optcmd_putmemio,
> >
> > &optcmd_getxer, &optcmd_putxer, &optcmd_getcr, &optcmd_putcr,
> >
> > - &optcmd_gdbserver, &optcmd_geti2c,
> > + &optcmd_gdbserver, &optcmd_geti2c, &optcmd_puti2c,
> >
> > };
> >
> > /* Purely for printing usage text. We could integrate printing
> >
> > argument and flag
> > @@ -146,6 +146,7 @@ static struct action actions[] = {
> >
> > { "regs", "[--backtrace]", "State (optionally display
> >
> > backtrace)" },
> >
> > { "gdbserver", "", "Start a gdb server" },
> > { "geti2c", "<port> <device> <offset> <size>", "Read size bytes
> >
> > from the offset of specified device" },
> > + { "puti2c", "<port> <device> <offset> <size>", "Read size bytes
> > from the offset of specified device" },
> >
> > };
> >
> > static void print_usage(void)
>
> Amitay.
More information about the Pdbg
mailing list