[2/3] libpdbg: Add i2c get and put functions for i2c master on CFAM

This enables the two basic i2c functions from the BMC.

Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
---
 libpdbg/i2cm.c    | 411 +++++++++++++++++++++++++++++++++++++++++++++-
 libpdbg/libpdbg.h |   2 +
 libpdbg/target.c  |  13 ++
 libpdbg/target.h  |   3 +-
 p9-fsi.dtsi.m4    |   7 +
 p9-kernel.dts.m4  |   2 +-
 src/i2c.c         |  38 ++++-
 src/main.c        |   5 +-
 8 files changed, 473 insertions(+), 8 deletions(-)

On Mon, 2019-04-15 at 11:14 +1000, Rashmica Gupta wrote:
> This enables the two basic i2c functions from the BMC.
> 
> Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
> ---
>  libpdbg/i2cm.c    | 411
> +++++++++++++++++++++++++++++++++++++++++++++-
>  libpdbg/libpdbg.h |   2 +
>  libpdbg/target.c  |  13 ++
>  libpdbg/target.h  |   3 +-
>  p9-fsi.dtsi.m4    |   7 +
>  p9-kernel.dts.m4  |   2 +-
>  src/i2c.c         |  38 ++++-
>  src/main.c        |   5 +-
>  8 files changed, 473 insertions(+), 8 deletions(-)
> 
> diff --git a/libpdbg/i2cm.c b/libpdbg/i2cm.c
> index 0c8129b..2022122 100644
> --- a/libpdbg/i2cm.c
> +++ b/libpdbg/i2cm.c
> @@ -24,15 +24,422 @@
>  #include <sys/ioctl.h>
>  #include <linux/i2c-dev.h>
>  
> -#include "bitutils.h"
>  #include "operations.h"
>  #include "debug.h"
> -
> +#include "bitutils.h"
>  
>  #include <errno.h>
>  #include <sys/param.h>
>  #include <dirent.h>
>  
> +
> +/* I2C common registers */
> +#define I2C_FIFO_REG 		0x0
> +#define I2C_CMD_REG 		0x1
> +#define I2C_MODE_REG 		0x2
> +#define I2C_WATERMARK_REG	0x3
> +#define I2C_INT_MASK_REG	0x4
> +#define I2C_INT_COND_REG	0x5
> +#define I2C_STATUS_REG		0x7
> +#define I2C_IMD_RESET_REG	0x7
> +#define I2C_IMD_RESET_ERR_REG	0x8
> +#define I2C_ESTAT_REG		0x8
> +#define I2C_RESIDUAL_REG	0x9
> +#define I2C_PORT_BUSY_REG	0xA
> +
> +#define I2C_PIB_OFFSET 		0x4
> +#define I2C_PIB_ENGINE_0 	0x0000
> +#define I2C_PIB_ENGINE_1 	0x1000
> +#define I2C_PIB_ENGINE_2 	0x2000
> +#define I2C_PIB_ENGINE_3 	0x3000
> +
> +/* I2C command register bits */
> +#define I2C_CMD_WITH_START		PPC_BIT32(0)
> +#define I2C_CMD_WITH_ADDR		PPC_BIT32(1)
> +#define I2C_CMD_READ_CONTINUE	PPC_BIT32(2)
> +#define I2C_CMD_WITH_STOP		PPC_BIT32(3)
> +#define I2C_CMD_INT_STEER		PPC_BITMASK32(6, 7)
> +#define I2C_CMD_DEV_ADDR		PPC_BITMASK32(8, 14)
> +#define I2C_CMD_READ_NOT_WRITE	PPC_BIT32(15)
> +#define I2C_CMD_LENGTH			PPC_BITMASK32(16, 31)
> +
> +/* I2C mode register bits */
> +#define I2C_MODE_BIT_RATE_DIV	PPC_BITMASK32(0, 15)
> +#define I2C_MODE_PORT_NUM		PPC_BITMASK32(16, 21)
> +#define I2C_ENHANCED_MODE		PPC_BIT32(28)
> +#define I2C_MODE_PACING			PPC_BIT32(30)
> +
> +/* watermark */
> +#define I2C_WATERMARK_HIGH		PPC_BITMASK32(16,19)
> +#define I2C_WATERMARK_LOW		PPC_BITMASK32(24,27)
> +
> +/* I2C status register */
> +#define I2C_STAT_INV_CMD		PPC_BIT32(0)
> +#define I2C_STAT_PARITY			PPC_BIT32(1)
> +#define I2C_STAT_BE_OVERRUN		PPC_BIT32(2)
> +#define I2C_STAT_BE_ACCESS		PPC_BIT32(3)
> +#define I2C_STAT_LOST_ARB		PPC_BIT32(4)
> +#define I2C_STAT_NACK			PPC_BIT32(5)
> +#define I2C_STAT_DAT_REQ		PPC_BIT32(6)
> +#define I2C_STAT_CMD_COMP		PPC_BIT32(7)
> +#define I2C_STAT_STOP_ERR		PPC_BIT32(8)
> +#define I2C_STAT_MAX_PORT		PPC_BITMASK32(9, 15)
> +#define I2C_STAT_ANY_INT		PPC_BIT32(16)
> +#define I2C_STAT_WAIT_BUSY		PPC_BIT32(17)
> +#define I2C_STAT_ERR_IN			PPC_BIT32(18)
> +#define I2C_STAT_PORT_HIST_BUSY	PPC_BIT32(19)
> +#define I2C_STAT_SCL_IN			PPC_BIT32(20)
> +#define I2C_STAT_SDA_IN			PPC_BIT32(21)
> +#define I2C_STAT_PORT_BUSY		PPC_BIT32(22)
> +#define I2C_STAT_SELF_BUSY		PPC_BIT32(23)
> +#define I2C_STAT_FIFO_COUNT		PPC_BITMASK32(24, 31)
> +
> +#define I2C_STAT_ERR		(I2C_STAT_INV_CMD |			
> \
> +						 	 I2C_STAT_PARITY |	
> 		\
> +			 				 I2C_STAT_BE_OVERRUN
> |		\
> +			 				 I2C_STAT_BE_ACCESS |
> 		\
> +			 				 I2C_STAT_LOST_ARB |	
> 	\
> +			 				 I2C_STAT_NACK |	
> 		\
> +			 				 I2C_STAT_STOP_ERR)
> +
> +#define I2C_STAT_ANY_RESP	(I2C_STAT_ERR |				
> \
> +						 	I2C_STAT_DAT_REQ |	
> 		\
> +							I2C_STAT_CMD_CO
> MP)
> +
> +/* I2C extended status register */
> +#define I2C_ESTAT_FIFO_SIZE PPC_BITMASK32(0,7)
> +#define I2C_ESTAT_MSM_STATE PPC_BITMASK32(11,15)
> +#define I2C_ESTAT_HIGH_WATER 	PPC_BIT32(22)
> +#define I2C_ESTAT_LOW_WATER 	PPC_BIT32(23)
> +
> +/* I2C interrupt mask register */
> +#define I2C_INT_INV_CMD		PPC_BIT32(16)
> +#define I2C_INT_PARITY		PPC_BIT32(17)
> +#define I2C_INT_BE_OVERRUN	PPC_BIT32(18)
> +#define I2C_INT_BE_ACCESS	PPC_BIT32(19)
> +#define I2C_INT_LOST_ARB	PPC_BIT32(20)
> +#define I2C_INT_NACK		PPC_BIT32(21)
> +#define I2C_INT_DAT_REQ		PPC_BIT32(22)
> +#define I2C_INT_CMD_COMP	PPC_BIT32(23)
> +#define I2C_INT_STOP_ERR	PPC_BIT32(24)
> +#define I2C_INT_BUSY		PPC_BIT32(25)
> +#define I2C_INT_IDLE		PPC_BIT32(26)
> +
> +/* I2C residual  register */
> +#define I2C_RESID_FRONT		PPC_BITMASK32(0,15)
> +#define I2C_RESID_BACK		PPC_BITMASK32(16,31)
> +
> +static int _i2cm_reg_write(struct i2cm *i2cm, uint32_t addr,
> uint32_t data)
> +{
> +	CHECK_ERR(fsi_write(&i2cm->target, addr, data));
> +	return 0;
> +}
> +
> +static int _i2cm_reg_read(struct i2cm *i2cm, uint32_t addr, uint32_t
> *data)
> +{
> +	CHECK_ERR(fsi_read(&i2cm->target, addr, data));
> +	return 0;
> +}
> +
> +static void debug_print_reg(struct i2cm *i2cm)
> +{
> +	uint32_t fsidata = 0;
> +
> +	PR_INFO("\t --------\n");
> +	_i2cm_reg_read(i2cm,  I2C_STATUS_REG, &fsidata);
> +	PR_INFO("\t status reg \t has value 0x%x \n", fsidata);
> +	if (fsidata & I2C_STAT_INV_CMD)
> +		 PR_INFO("\t\tinvalid cmd\n");
> +	if (fsidata & I2C_STAT_PARITY)
> +		 PR_INFO("\t\tparity\n");
> +	if (fsidata & I2C_STAT_BE_OVERRUN)
> +		 PR_INFO("\t\tback endoverrun\n");
> +	if (fsidata & I2C_STAT_BE_ACCESS)
> +		 PR_INFO("\t\tback end access error\n");
> +	if (fsidata & I2C_STAT_LOST_ARB)
> +		 PR_INFO("\t\tarbitration lost\n");
> +	if (fsidata & I2C_STAT_NACK)
> +		 PR_INFO("\t\tnack\n");
> +	if (fsidata & I2C_STAT_DAT_REQ)
> +		 PR_INFO("\t\tdata request\n");
> +	if (fsidata & I2C_STAT_STOP_ERR)
> +		 PR_INFO("\t\tstop error\n");
> +	if (fsidata & I2C_STAT_PORT_BUSY)
> +		 PR_INFO("\t\ti2c busy\n");
> +	PR_INFO("\t\tfifo entry count: %li
> \n",fsidata&I2C_STAT_FIFO_COUNT);
> +
> +	_i2cm_reg_read(i2cm,  I2C_ESTAT_REG, &fsidata);
> +	PR_INFO("\t extended status reg has value 0x%x \n", fsidata);
> +	if (fsidata & I2C_ESTAT_HIGH_WATER)
> +		PR_INFO("\t\thigh water mark reached\n");
> +	if (fsidata & I2C_ESTAT_LOW_WATER)
> +		PR_INFO("\t\tlow water mark reached\n");
> +
> +
> +	_i2cm_reg_read(i2cm,  I2C_WATERMARK_REG, &fsidata);
> +	PR_INFO("\t watermark reg  has value 0x%x \n", fsidata);
> +	PR_INFO("\t\twatermark high: %li
> \n",fsidata&I2C_WATERMARK_HIGH);
> +	PR_INFO("\t\twatermark low: %li \n",fsidata&I2C_WATERMARK_LOW);
> +
> +	_i2cm_reg_read(i2cm,  I2C_RESIDUAL_REG, &fsidata);
> +	PR_INFO("\t residual reg  has value 0x%x \n", fsidata);
> +	PR_INFO("\t\tfrontend_len: %li \n",fsidata&I2C_RESID_FRONT);
> +	PR_INFO("\t\tbackend_len: %li \n",fsidata&I2C_RESID_BACK);
> +
> +	_i2cm_reg_read(i2cm,  I2C_PORT_BUSY_REG, &fsidata);
> +	PR_INFO("\t port busy reg  has value 0x%x \n", fsidata);
> +	PR_INFO("\t -------\n");
> +
> +}
> +
> +static void i2c_mode_write(struct i2cm *i2cm)
> +{
> +	uint32_t data = I2C_MODE_PACING;
> +
> +	// hardcoding bit rate divisor because not too important
> +	data = SETFIELD(I2C_MODE_BIT_RATE_DIV, data, 28);
> +	data = SETFIELD(I2C_MODE_PORT_NUM, data, i2cm->port);
> +	PR_INFO("setting mode to %x\n", data);
> +	_i2cm_reg_write(i2cm, I2C_MODE_REG, data);
> +}
> +
> +static void i2c_watermark_write(struct i2cm *i2cm)
> +{
> +	uint32_t data = 0;
> +
> +	data = SETFIELD(I2C_WATERMARK_HIGH, data, 4);
> +	data = SETFIELD(I2C_WATERMARK_LOW, data, 4);
> +	PR_INFO("setting watermark (0x%x) to: %x\n", I2C_WATERMARK_REG,
> data);
> +	_i2cm_reg_write(i2cm, I2C_WATERMARK_REG, data);
> +}
> +
> +static int i2c_reset(struct i2cm *i2cm)
> +{
> +	uint32_t fsidata = 0;
> +	debug_print_reg(i2cm);
> +	PR_INFO("### RESETING \n");
> +
> +	fsidata = 0xB;
> +	_i2cm_reg_write(i2cm, I2C_IMD_RESET_REG, fsidata);
> +	_i2cm_reg_write(i2cm, I2C_IMD_RESET_ERR_REG, fsidata);
> +
> +	usleep(10000);
> +	debug_print_reg(i2cm);
> +	return 0;
> +}
> +
> +/*	
> + *	If there are errors in the status register, redo the whole
> + *	operation after restting the i2cm.
> +*/
> +static int i2c_poll_status(struct i2cm *i2cm, uint32_t *data)
> +{
> +	int loop;
> +
> +	for (loop = 0; loop < 10; loop++)
> +	{
> +		usleep(10000);
> +		_i2cm_reg_read(i2cm, I2C_STATUS_REG, data);
> +
> +		if (((*data) & I2C_STAT_CMD_COMP))
> +			break;
> +	}
> +	if ((*data) & I2C_STAT_PORT_BUSY)
> +		PR_INFO("portbusy\n");
> +	if ((*data) & I2C_STAT_ERR) {
> +		PR_INFO("ERROR IN STATUS\n");
> +		debug_print_reg(i2cm);
> +		return 1;
> +	}
> +	return 0;
> +}
> +
> +static int i2c_fifo_write(struct i2cm *i2cm, uint32_t *data,
> uint16_t size)
> +{
> +	int bytes_in_fifo = 1;
> +	int rc = 0, bytes_written = 0;
> +	uint32_t status;
> +
> +	while (bytes_written < size) {
> +
> +		if (bytes_written == size)
> +			return 0;
> +
> +		/* Poll status register's FIFO_ENTRY_COUNT to check
> that FIFO isn't full */
> +		rc = i2c_poll_status(i2cm, &status);
> +		bytes_in_fifo = status & I2C_STAT_FIFO_COUNT;
> +		PR_INFO("%x bytes in fifo \n", bytes_in_fifo);
> +
> +		if (rc)
> +			return 0;
> +
> +		if (bytes_in_fifo == 8)
> +			continue;
> +
> +		PR_INFO("\twriting: %x  to FIFO\n", data[bytes_written
> / 4]);
> +		rc = _i2cm_reg_write(i2cm, I2C_FIFO_REG,
> data[bytes_written / 4]);
> +		if (rc)
> +			return bytes_written;
> +		bytes_written += 4;
> +	}
> +	return bytes_written;
> +}
> +
> +static int i2c_fifo_read(struct i2cm *i2cm, uint32_t *data, uint16_t
> size)
> +{
> +	int bytes_to_read = 1;
> +	int rc = 0, bytes_read = 0;
> +	uint32_t tmp;
> +	uint32_t status;
> +
> +	while (bytes_to_read) {	
> +
> +		if (bytes_read > size)
> +			return 0;
> +
> +		/* Poll status register's FIFO_ENTRY_COUNT to check
> that there is data to consume */
> +		rc = i2c_poll_status(i2cm, &status);
> +		bytes_to_read = status & I2C_STAT_FIFO_COUNT;
> +		PR_INFO("%x bytes in fifo \n", bytes_to_read);
> +
> +		if (rc)
> +			return 0;
> +
> +		if (!bytes_to_read)
> +			continue;
> +
> +		rc = _i2cm_reg_read(i2cm, I2C_FIFO_REG, &tmp);
> +		if (rc)
> +			return bytes_read;
> +		memcpy(data + (bytes_read / 4), &tmp, 4);
> +		PR_INFO(" %x \n", data[bytes_read / 4]);
> +		bytes_read += 4;
> +	}
> +	return bytes_read;
> +}
> +
> +static int i2cm_ok_to_use(struct i2cm *i2cm)
> +{
> +	uint32_t data;
> +
> +	_i2cm_reg_read(i2cm, I2C_STATUS_REG, &data);
> +
> +	if (!(data & I2C_STAT_CMD_COMP) || (data & I2C_STAT_ERR))
> +	{
> +		PR_ERROR("ERROR IN STATUS, attempting to reset %x \n",
> data);
> +		i2c_reset(i2cm);
> +		_i2cm_reg_read(i2cm, I2C_STATUS_REG, &data);
> +	}
> +	if (data & I2C_STAT_ERR) {
> +		PR_ERROR("ERROR IN STATUS %x\n", data);
> +		return 0;
> +	}
> +	i2c_watermark_write(i2cm);
> +	i2c_mode_write(i2cm);
> +	return 1;
> +}
> +
> +static int i2c_put(struct i2cm *i2cm, uint32_t addr, uint16_t size,
> uint8_t *d)
> +{
> +	uint32_t fsidata;
> +	uint32_t data = 0;
> +	int rc = 0;
> +
> +	if(!i2cm_ok_to_use(i2cm)) {
> +		rc = 1;
> +		return rc;
> +	}
> +	//TODO: if size > 64kB then use I2C_CMD_READ_CONTINUE and do
> multiple commands
> +	if (size > 64*1024 -1)
> +		size = 64*1024 -1;
> +
> +
> +	/* Set slave device */
> +	fsidata = I2C_CMD_WITH_START | I2C_CMD_WITH_ADDR;
> +	fsidata = SETFIELD(I2C_CMD_DEV_ADDR, fsidata, addr);
> +	fsidata = SETFIELD(I2C_CMD_LENGTH, fsidata, size);
> +	_i2cm_reg_write(i2cm, I2C_CMD_REG, fsidata);
> +
> +	rc = i2c_poll_status(i2cm, &data);
> +	if (rc) {
> +		PR_ERROR("FAILED to set i2c device\n");
> +		return rc;
> +	}
> +
> +	printf("%i\n", size);
> +	/* Write data into the FIFO of the slave device */
> +	i2c_fifo_write(i2cm, (uint32_t *)d, size);
> +
> +	rc = i2c_poll_status(i2cm, &data);
> +	if (rc) {
> +		PR_ERROR("FAILED to write all data\n");
> +		return rc;
> +	}
> +	return rc;
> +}
> +
> +static int i2c_get(struct i2cm *i2cm, uint32_t addr, uint16_t size,
> uint8_t *d)
> +{
> +	uint32_t fsidata;
> +	uint32_t data = 0;
> +	int rc = 0;
> +	int bytes_read;
> +
> +	if(!i2cm_ok_to_use(i2cm)) {
> +		rc = 1;
> +		return rc;
> +	}
> +
> +	//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.

> +
> +	/* 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@1800 {
> +			#address-cells = <0x2>;
> +			#size-cells = <0x1>;
> +			reg = <0x0 0x1800 0x400>;
> +			compatible = "ibm,fsi-i2c-master";
> +		};
> +
>  		hmfsi@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.

>  	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)
> -- 
> 2.17.2
> 

Amitay.

> > +	//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@1800 {
> > +			#address-cells = <0x2>;
> > +			#size-cells = <0x1>;
> > +			reg = <0x0 0x1800 0x400>;
> > +			compatible = "ibm,fsi-i2c-master";
> > +		};
> > +
> > 
> >  		hmfsi@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.

On Monday, 15 April 2019 11:14:13 AM AEST Rashmica Gupta wrote:
> This enables the two basic i2c functions from the BMC.
> 
> Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
> ---
>  libpdbg/i2cm.c    | 411 +++++++++++++++++++++++++++++++++++++++++++++-
>  libpdbg/libpdbg.h |   2 +
>  libpdbg/target.c  |  13 ++
>  libpdbg/target.h  |   3 +-
>  p9-fsi.dtsi.m4    |   7 +
>  p9-kernel.dts.m4  |   2 +-
>  src/i2c.c         |  38 ++++-
>  src/main.c        |   5 +-
>  8 files changed, 473 insertions(+), 8 deletions(-)
> 
> diff --git a/libpdbg/i2cm.c b/libpdbg/i2cm.c
> index 0c8129b..2022122 100644
> --- a/libpdbg/i2cm.c
> +++ b/libpdbg/i2cm.c
> @@ -24,15 +24,422 @@
>  #include <sys/ioctl.h>
>  #include <linux/i2c-dev.h>
> 
> -#include "bitutils.h"
>  #include "operations.h"
>  #include "debug.h"
> -
> +#include "bitutils.h"
> 
>  #include <errno.h>
>  #include <sys/param.h>
>  #include <dirent.h>
> 
> +
> +/* I2C common registers */
> +#define I2C_FIFO_REG 		0x0
> +#define I2C_CMD_REG 		0x1
> +#define I2C_MODE_REG 		0x2
> +#define I2C_WATERMARK_REG	0x3
> +#define I2C_INT_MASK_REG	0x4
> +#define I2C_INT_COND_REG	0x5
> +#define I2C_STATUS_REG		0x7
> +#define I2C_IMD_RESET_REG	0x7
> +#define I2C_IMD_RESET_ERR_REG	0x8
> +#define I2C_ESTAT_REG		0x8
> +#define I2C_RESIDUAL_REG	0x9
> +#define I2C_PORT_BUSY_REG	0xA
> +
> +#define I2C_PIB_OFFSET 		0x4
> +#define I2C_PIB_ENGINE_0 	0x0000
> +#define I2C_PIB_ENGINE_1 	0x1000
> +#define I2C_PIB_ENGINE_2 	0x2000
> +#define I2C_PIB_ENGINE_3 	0x3000
> +
> +/* I2C command register bits */
> +#define I2C_CMD_WITH_START		PPC_BIT32(0)
> +#define I2C_CMD_WITH_ADDR		PPC_BIT32(1)
> +#define I2C_CMD_READ_CONTINUE	PPC_BIT32(2)
> +#define I2C_CMD_WITH_STOP		PPC_BIT32(3)
> +#define I2C_CMD_INT_STEER		PPC_BITMASK32(6, 7)
> +#define I2C_CMD_DEV_ADDR		PPC_BITMASK32(8, 14)
> +#define I2C_CMD_READ_NOT_WRITE	PPC_BIT32(15)
> +#define I2C_CMD_LENGTH			PPC_BITMASK32(16, 31)
> +
> +/* I2C mode register bits */
> +#define I2C_MODE_BIT_RATE_DIV	PPC_BITMASK32(0, 15)
> +#define I2C_MODE_PORT_NUM		PPC_BITMASK32(16, 21)
> +#define I2C_ENHANCED_MODE		PPC_BIT32(28)
> +#define I2C_MODE_PACING			PPC_BIT32(30)
> +
> +/* watermark */
> +#define I2C_WATERMARK_HIGH		PPC_BITMASK32(16,19)
> +#define I2C_WATERMARK_LOW		PPC_BITMASK32(24,27)
> +
> +/* I2C status register */
> +#define I2C_STAT_INV_CMD		PPC_BIT32(0)
> +#define I2C_STAT_PARITY			PPC_BIT32(1)
> +#define I2C_STAT_BE_OVERRUN		PPC_BIT32(2)
> +#define I2C_STAT_BE_ACCESS		PPC_BIT32(3)
> +#define I2C_STAT_LOST_ARB		PPC_BIT32(4)
> +#define I2C_STAT_NACK			PPC_BIT32(5)
> +#define I2C_STAT_DAT_REQ		PPC_BIT32(6)
> +#define I2C_STAT_CMD_COMP		PPC_BIT32(7)
> +#define I2C_STAT_STOP_ERR		PPC_BIT32(8)
> +#define I2C_STAT_MAX_PORT		PPC_BITMASK32(9, 15)
> +#define I2C_STAT_ANY_INT		PPC_BIT32(16)
> +#define I2C_STAT_WAIT_BUSY		PPC_BIT32(17)
> +#define I2C_STAT_ERR_IN			PPC_BIT32(18)
> +#define I2C_STAT_PORT_HIST_BUSY	PPC_BIT32(19)
> +#define I2C_STAT_SCL_IN			PPC_BIT32(20)
> +#define I2C_STAT_SDA_IN			PPC_BIT32(21)
> +#define I2C_STAT_PORT_BUSY		PPC_BIT32(22)
> +#define I2C_STAT_SELF_BUSY		PPC_BIT32(23)
> +#define I2C_STAT_FIFO_COUNT		PPC_BITMASK32(24, 31)
> +
> +#define I2C_STAT_ERR		(I2C_STAT_INV_CMD |			\
> +						 	 I2C_STAT_PARITY |			\
> +			 				 I2C_STAT_BE_OVERRUN |		\
> +			 				 I2C_STAT_BE_ACCESS |		\
> +			 				 I2C_STAT_LOST_ARB |		\
> +			 				 I2C_STAT_NACK |			\
> +			 				 I2C_STAT_STOP_ERR)
> +
> +#define I2C_STAT_ANY_RESP	(I2C_STAT_ERR |				\
> +						 	I2C_STAT_DAT_REQ |			\
> +							I2C_STAT_CMD_COMP)
> +
> +/* I2C extended status register */
> +#define I2C_ESTAT_FIFO_SIZE PPC_BITMASK32(0,7)
> +#define I2C_ESTAT_MSM_STATE PPC_BITMASK32(11,15)
> +#define I2C_ESTAT_HIGH_WATER 	PPC_BIT32(22)
> +#define I2C_ESTAT_LOW_WATER 	PPC_BIT32(23)
> +
> +/* I2C interrupt mask register */
> +#define I2C_INT_INV_CMD		PPC_BIT32(16)
> +#define I2C_INT_PARITY		PPC_BIT32(17)
> +#define I2C_INT_BE_OVERRUN	PPC_BIT32(18)
> +#define I2C_INT_BE_ACCESS	PPC_BIT32(19)
> +#define I2C_INT_LOST_ARB	PPC_BIT32(20)
> +#define I2C_INT_NACK		PPC_BIT32(21)
> +#define I2C_INT_DAT_REQ		PPC_BIT32(22)
> +#define I2C_INT_CMD_COMP	PPC_BIT32(23)
> +#define I2C_INT_STOP_ERR	PPC_BIT32(24)
> +#define I2C_INT_BUSY		PPC_BIT32(25)
> +#define I2C_INT_IDLE		PPC_BIT32(26)
> +
> +/* I2C residual  register */
> +#define I2C_RESID_FRONT		PPC_BITMASK32(0,15)
> +#define I2C_RESID_BACK		PPC_BITMASK32(16,31)
> +
> +static int _i2cm_reg_write(struct i2cm *i2cm, uint32_t addr, uint32_t data)
> +{
> +	CHECK_ERR(fsi_write(&i2cm->target, addr, data));
> +	return 0;
> +}
> +
> +static int _i2cm_reg_read(struct i2cm *i2cm, uint32_t addr, uint32_t *data)
> +{
> +	CHECK_ERR(fsi_read(&i2cm->target, addr, data));
> +	return 0;
> +}
> +
> +static void debug_print_reg(struct i2cm *i2cm)
> +{
> +	uint32_t fsidata = 0;
> +
> +	PR_INFO("\t --------\n");
> +	_i2cm_reg_read(i2cm,  I2C_STATUS_REG, &fsidata);
> +	PR_INFO("\t status reg \t has value 0x%x \n", fsidata);
> +	if (fsidata & I2C_STAT_INV_CMD)
> +		 PR_INFO("\t\tinvalid cmd\n");
> +	if (fsidata & I2C_STAT_PARITY)
> +		 PR_INFO("\t\tparity\n");
> +	if (fsidata & I2C_STAT_BE_OVERRUN)
> +		 PR_INFO("\t\tback endoverrun\n");
> +	if (fsidata & I2C_STAT_BE_ACCESS)
> +		 PR_INFO("\t\tback end access error\n");
> +	if (fsidata & I2C_STAT_LOST_ARB)
> +		 PR_INFO("\t\tarbitration lost\n");
> +	if (fsidata & I2C_STAT_NACK)
> +		 PR_INFO("\t\tnack\n");
> +	if (fsidata & I2C_STAT_DAT_REQ)
> +		 PR_INFO("\t\tdata request\n");
> +	if (fsidata & I2C_STAT_STOP_ERR)
> +		 PR_INFO("\t\tstop error\n");
> +	if (fsidata & I2C_STAT_PORT_BUSY)
> +		 PR_INFO("\t\ti2c busy\n");
> +	PR_INFO("\t\tfifo entry count: %li \n",fsidata&I2C_STAT_FIFO_COUNT);
> +
> +	_i2cm_reg_read(i2cm,  I2C_ESTAT_REG, &fsidata);
> +	PR_INFO("\t extended status reg has value 0x%x \n", fsidata);
> +	if (fsidata & I2C_ESTAT_HIGH_WATER)
> +		PR_INFO("\t\thigh water mark reached\n");
> +	if (fsidata & I2C_ESTAT_LOW_WATER)
> +		PR_INFO("\t\tlow water mark reached\n");
> +
> +
> +	_i2cm_reg_read(i2cm,  I2C_WATERMARK_REG, &fsidata);
> +	PR_INFO("\t watermark reg  has value 0x%x \n", fsidata);
> +	PR_INFO("\t\twatermark high: %li \n",fsidata&I2C_WATERMARK_HIGH);
> +	PR_INFO("\t\twatermark low: %li \n",fsidata&I2C_WATERMARK_LOW);
> +
> +	_i2cm_reg_read(i2cm,  I2C_RESIDUAL_REG, &fsidata);
> +	PR_INFO("\t residual reg  has value 0x%x \n", fsidata);
> +	PR_INFO("\t\tfrontend_len: %li \n",fsidata&I2C_RESID_FRONT);
> +	PR_INFO("\t\tbackend_len: %li \n",fsidata&I2C_RESID_BACK);
> +
> +	_i2cm_reg_read(i2cm,  I2C_PORT_BUSY_REG, &fsidata);
> +	PR_INFO("\t port busy reg  has value 0x%x \n", fsidata);
> +	PR_INFO("\t -------\n");
> +
> +}
> +
> +static void i2c_mode_write(struct i2cm *i2cm)
> +{
> +	uint32_t data = I2C_MODE_PACING;
> +
> +	// hardcoding bit rate divisor because not too important
> +	data = SETFIELD(I2C_MODE_BIT_RATE_DIV, data, 28);
> +	data = SETFIELD(I2C_MODE_PORT_NUM, data, i2cm->port);
> +	PR_INFO("setting mode to %x\n", data);
> +	_i2cm_reg_write(i2cm, I2C_MODE_REG, data);
> +}
> +
> +static void i2c_watermark_write(struct i2cm *i2cm)
> +{
> +	uint32_t data = 0;
> +
> +	data = SETFIELD(I2C_WATERMARK_HIGH, data, 4);
> +	data = SETFIELD(I2C_WATERMARK_LOW, data, 4);
> +	PR_INFO("setting watermark (0x%x) to: %x\n", I2C_WATERMARK_REG, data);
> +	_i2cm_reg_write(i2cm, I2C_WATERMARK_REG, data);
> +}
> +
> +static int i2c_reset(struct i2cm *i2cm)
> +{
> +	uint32_t fsidata = 0;
> +	debug_print_reg(i2cm);
> +	PR_INFO("### RESETING \n");
> +
> +	fsidata = 0xB;
> +	_i2cm_reg_write(i2cm, I2C_IMD_RESET_REG, fsidata);
> +	_i2cm_reg_write(i2cm, I2C_IMD_RESET_ERR_REG, fsidata);
> +
> +	usleep(10000);
> +	debug_print_reg(i2cm);
> +	return 0;
> +}
> +
> +/*
> + *	If there are errors in the status register, redo the whole
> + *	operation after restting the i2cm.
> +*/
> +static int i2c_poll_status(struct i2cm *i2cm, uint32_t *data)
> +{
> +	int loop;
> +
> +	for (loop = 0; loop < 10; loop++)
> +	{
> +		usleep(10000);
> +		_i2cm_reg_read(i2cm, I2C_STATUS_REG, data);
> +
> +		if (((*data) & I2C_STAT_CMD_COMP))
> +			break;
> +	}
> +	if ((*data) & I2C_STAT_PORT_BUSY)
> +		PR_INFO("portbusy\n");
> +	if ((*data) & I2C_STAT_ERR) {
> +		PR_INFO("ERROR IN STATUS\n");
> +		debug_print_reg(i2cm);
> +		return 1;
> +	}
> +	return 0;
> +}
> +
> +static int i2c_fifo_write(struct i2cm *i2cm, uint32_t *data, uint16_t size)
> +{
> +	int bytes_in_fifo = 1;
> +	int rc = 0, bytes_written = 0;
> +	uint32_t status;
> +
> +	while (bytes_written < size) {
> +
> +		if (bytes_written == size)
> +			return 0;
> +
> +		/* Poll status register's FIFO_ENTRY_COUNT to check that FIFO isn't 
full
> */ +		rc = i2c_poll_status(i2cm, &status);
> +		bytes_in_fifo = status & I2C_STAT_FIFO_COUNT;
> +		PR_INFO("%x bytes in fifo \n", bytes_in_fifo);
> +
> +		if (rc)
> +			return 0;
> +
> +		if (bytes_in_fifo == 8)
> +			continue;
> +
> +		PR_INFO("\twriting: %x  to FIFO\n", data[bytes_written / 4]);
> +		rc = _i2cm_reg_write(i2cm, I2C_FIFO_REG, data[bytes_written / 4]);
> +		if (rc)
> +			return bytes_written;
> +		bytes_written += 4;
> +	}
> +	return bytes_written;
> +}
> +
> +static int i2c_fifo_read(struct i2cm *i2cm, uint32_t *data, uint16_t size)
> +{
> +	int bytes_to_read = 1;
> +	int rc = 0, bytes_read = 0;
> +	uint32_t tmp;
> +	uint32_t status;
> +
> +	while (bytes_to_read) {
> +
> +		if (bytes_read > size)
> +			return 0;
> +
> +		/* Poll status register's FIFO_ENTRY_COUNT to check that there is data 
to
> consume */ +		rc = i2c_poll_status(i2cm, &status);
> +		bytes_to_read = status & I2C_STAT_FIFO_COUNT;
> +		PR_INFO("%x bytes in fifo \n", bytes_to_read);
> +
> +		if (rc)
> +			return 0;
> +
> +		if (!bytes_to_read)
> +			continue;
> +
> +		rc = _i2cm_reg_read(i2cm, I2C_FIFO_REG, &tmp);
> +		if (rc)
> +			return bytes_read;
> +		memcpy(data + (bytes_read / 4), &tmp, 4);
> +		PR_INFO(" %x \n", data[bytes_read / 4]);
> +		bytes_read += 4;
> +	}
> +	return bytes_read;
> +}
> +
> +static int i2cm_ok_to_use(struct i2cm *i2cm)
> +{
> +	uint32_t data;
> +
> +	_i2cm_reg_read(i2cm, I2C_STATUS_REG, &data);
> +
> +	if (!(data & I2C_STAT_CMD_COMP) || (data & I2C_STAT_ERR))
> +	{
> +		PR_ERROR("ERROR IN STATUS, attempting to reset %x \n", data);
> +		i2c_reset(i2cm);
> +		_i2cm_reg_read(i2cm, I2C_STATUS_REG, &data);
> +	}
> +	if (data & I2C_STAT_ERR) {
> +		PR_ERROR("ERROR IN STATUS %x\n", data);
> +		return 0;
> +	}
> +	i2c_watermark_write(i2cm);
> +	i2c_mode_write(i2cm);
> +	return 1;
> +}
> +
> +static int i2c_put(struct i2cm *i2cm, uint32_t addr, uint16_t size, uint8_t
> *d) +{
> +	uint32_t fsidata;
> +	uint32_t data = 0;
> +	int rc = 0;
> +
> +	if(!i2cm_ok_to_use(i2cm)) {
> +		rc = 1;
> +		return rc;
> +	}
> +	//TODO: if size > 64kB then use I2C_CMD_READ_CONTINUE and do multiple
> commands +	if (size > 64*1024 -1)
> +		size = 64*1024 -1;
> +
> +
> +	/* Set slave device */
> +	fsidata = I2C_CMD_WITH_START | I2C_CMD_WITH_ADDR;
> +	fsidata = SETFIELD(I2C_CMD_DEV_ADDR, fsidata, addr);
> +	fsidata = SETFIELD(I2C_CMD_LENGTH, fsidata, size);
> +	_i2cm_reg_write(i2cm, I2C_CMD_REG, fsidata);
> +
> +	rc = i2c_poll_status(i2cm, &data);
> +	if (rc) {
> +		PR_ERROR("FAILED to set i2c device\n");
> +		return rc;
> +	}
> +
> +	printf("%i\n", size);
> +	/* Write data into the FIFO of the slave device */
> +	i2c_fifo_write(i2cm, (uint32_t *)d, size);
> +
> +	rc = i2c_poll_status(i2cm, &data);
> +	if (rc) {
> +		PR_ERROR("FAILED to write all data\n");
> +		return rc;
> +	}
> +	return rc;
> +}
> +
> +static int i2c_get(struct i2cm *i2cm, uint32_t addr, uint16_t size, uint8_t
> *d) +{
> +	uint32_t fsidata;
> +	uint32_t data = 0;
> +	int rc = 0;
> +	int bytes_read;
> +
> +	if(!i2cm_ok_to_use(i2cm)) {
> +		rc = 1;
> +		return rc;
> +	}
> +
> +	//TODO: if size > 64kB then use I2C_CMD_READ_CONTINUE and do multiple
> commands +	if (size > 64*1024 -1)
> +		size = 64*1024 -1;
> +
> +	/* 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;

This should be an inherent property of the target rather than something 
specified by the user. I can understand how this is convenient given we have a 
single master with multiple ports but having each port as a target means you 
can use all the existing target selection code, etc.

So instead I would create a device-tree structure like so (we can bikeshed 
names as required):

i2c_perv@1800 {
	compatible = "ibm,cfam-i2c-master";
	#address-cells = <0x1>;
	#size-cells = <0x0>;

	i2cm_port@0 {
		reg = <0x0>;
		compatible = "ibm,i2c-master-port";
	};

	i2cm_port@1 {
		reg = <0x1>;
		compatible = "ibm,i2c-master-port";
	};
};

Your i2c-master-port hw unit will then get it's `->port` propterty filled in 
from the device-tree and it will become the target that gets passed to 
i2c_read() which can then call some internal function with the appropriate 
port number.

> 
>  	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@1800 {
> +			#address-cells = <0x2>;
> +			#size-cells = <0x1>;
> +			reg = <0x0 0x1800 0x400>;
> +			compatible = "ibm,fsi-i2c-master";
> +		};
> +
>  		hmfsi@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);
>  	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,

Argh, this is where we use DATA8 :-)

- Alistair

> 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)

On Mon, 2019-04-15 at 15:24 +1000, Alistair Popple wrote:
> On Monday, 15 April 2019 11:14:13 AM AEST Rashmica Gupta wrote:
> > This enables the two basic i2c functions from the BMC.
> > 
> > Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
> > ---
> >  libpdbg/i2cm.c    | 411
> > +++++++++++++++++++++++++++++++++++++++++++++-
> >  libpdbg/libpdbg.h |   2 +
> >  libpdbg/target.c  |  13 ++
> >  libpdbg/target.h  |   3 +-
> >  p9-fsi.dtsi.m4    |   7 +
> >  p9-kernel.dts.m4  |   2 +-
> >  src/i2c.c         |  38 ++++-
> >  src/main.c        |   5 +-
> >  8 files changed, 473 insertions(+), 8 deletions(-)
> > 
> > diff --git a/libpdbg/i2cm.c b/libpdbg/i2cm.c
> > index 0c8129b..2022122 100644
> > --- a/libpdbg/i2cm.c
> > +++ b/libpdbg/i2cm.c
> > @@ -24,15 +24,422 @@
> >  #include <sys/ioctl.h>
> >  #include <linux/i2c-dev.h>
> > 
> > -#include "bitutils.h"
> >  #include "operations.h"
> >  #include "debug.h"
> > -
> > +#include "bitutils.h"
> > 
> >  #include <errno.h>
> >  #include <sys/param.h>
> >  #include <dirent.h>
> > 
> > +
> > +/* I2C common registers */
> > +#define I2C_FIFO_REG 		0x0
> > +#define I2C_CMD_REG 		0x1
> > +#define I2C_MODE_REG 		0x2
> > +#define I2C_WATERMARK_REG	0x3
> > +#define I2C_INT_MASK_REG	0x4
> > +#define I2C_INT_COND_REG	0x5
> > +#define I2C_STATUS_REG		0x7
> > +#define I2C_IMD_RESET_REG	0x7
> > +#define I2C_IMD_RESET_ERR_REG	0x8
> > +#define I2C_ESTAT_REG		0x8
> > +#define I2C_RESIDUAL_REG	0x9
> > +#define I2C_PORT_BUSY_REG	0xA
> > +
> > +#define I2C_PIB_OFFSET 		0x4
> > +#define I2C_PIB_ENGINE_0 	0x0000
> > +#define I2C_PIB_ENGINE_1 	0x1000
> > +#define I2C_PIB_ENGINE_2 	0x2000
> > +#define I2C_PIB_ENGINE_3 	0x3000
> > +
> > +/* I2C command register bits */
> > +#define I2C_CMD_WITH_START		PPC_BIT32(0)
> > +#define I2C_CMD_WITH_ADDR		PPC_BIT32(1)
> > +#define I2C_CMD_READ_CONTINUE	PPC_BIT32(2)
> > +#define I2C_CMD_WITH_STOP		PPC_BIT32(3)
> > +#define I2C_CMD_INT_STEER		PPC_BITMASK32(6, 7)
> > +#define I2C_CMD_DEV_ADDR		PPC_BITMASK32(8, 14)
> > +#define I2C_CMD_READ_NOT_WRITE	PPC_BIT32(15)
> > +#define I2C_CMD_LENGTH			PPC_BITMASK32(16, 31)
> > +
> > +/* I2C mode register bits */
> > +#define I2C_MODE_BIT_RATE_DIV	PPC_BITMASK32(0, 15)
> > +#define I2C_MODE_PORT_NUM		PPC_BITMASK32(16, 21)
> > +#define I2C_ENHANCED_MODE		PPC_BIT32(28)
> > +#define I2C_MODE_PACING			PPC_BIT32(30)
> > +
> > +/* watermark */
> > +#define I2C_WATERMARK_HIGH		PPC_BITMASK32(16,19)
> > +#define I2C_WATERMARK_LOW		PPC_BITMASK32(24,27)
> > +
> > +/* I2C status register */
> > +#define I2C_STAT_INV_CMD		PPC_BIT32(0)
> > +#define I2C_STAT_PARITY			PPC_BIT32(1)
> > +#define I2C_STAT_BE_OVERRUN		PPC_BIT32(2)
> > +#define I2C_STAT_BE_ACCESS		PPC_BIT32(3)
> > +#define I2C_STAT_LOST_ARB		PPC_BIT32(4)
> > +#define I2C_STAT_NACK			PPC_BIT32(5)
> > +#define I2C_STAT_DAT_REQ		PPC_BIT32(6)
> > +#define I2C_STAT_CMD_COMP		PPC_BIT32(7)
> > +#define I2C_STAT_STOP_ERR		PPC_BIT32(8)
> > +#define I2C_STAT_MAX_PORT		PPC_BITMASK32(9, 15)
> > +#define I2C_STAT_ANY_INT		PPC_BIT32(16)
> > +#define I2C_STAT_WAIT_BUSY		PPC_BIT32(17)
> > +#define I2C_STAT_ERR_IN			PPC_BIT32(18)
> > +#define I2C_STAT_PORT_HIST_BUSY	PPC_BIT32(19)
> > +#define I2C_STAT_SCL_IN			PPC_BIT32(20)
> > +#define I2C_STAT_SDA_IN			PPC_BIT32(21)
> > +#define I2C_STAT_PORT_BUSY		PPC_BIT32(22)
> > +#define I2C_STAT_SELF_BUSY		PPC_BIT32(23)
> > +#define I2C_STAT_FIFO_COUNT		PPC_BITMASK32(24, 31)
> > +
> > +#define I2C_STAT_ERR		(I2C_STAT_INV_CMD |			
> > \
> > +						 	 I2C_STAT_PARITY |	
> > 		\
> > +			 				 I2C_STAT_BE_OVERRU
> > N |		\
> > +			 				 I2C_STAT_BE_ACCESS
> > |		\
> > +			 				 I2C_STAT_LOST_ARB
> > |		\
> > +			 				 I2C_STAT_NACK |	
> > 		\
> > +			 				 I2C_STAT_STOP_ERR)
> > +
> > +#define I2C_STAT_ANY_RESP	(I2C_STAT_ERR |				
> > \
> > +						 	I2C_STAT_DAT_REQ |	
> > 		\
> > +							I2C_STAT_CMD_CO
> > MP)
> > +
> > +/* I2C extended status register */
> > +#define I2C_ESTAT_FIFO_SIZE PPC_BITMASK32(0,7)
> > +#define I2C_ESTAT_MSM_STATE PPC_BITMASK32(11,15)
> > +#define I2C_ESTAT_HIGH_WATER 	PPC_BIT32(22)
> > +#define I2C_ESTAT_LOW_WATER 	PPC_BIT32(23)
> > +
> > +/* I2C interrupt mask register */
> > +#define I2C_INT_INV_CMD		PPC_BIT32(16)
> > +#define I2C_INT_PARITY		PPC_BIT32(17)
> > +#define I2C_INT_BE_OVERRUN	PPC_BIT32(18)
> > +#define I2C_INT_BE_ACCESS	PPC_BIT32(19)
> > +#define I2C_INT_LOST_ARB	PPC_BIT32(20)
> > +#define I2C_INT_NACK		PPC_BIT32(21)
> > +#define I2C_INT_DAT_REQ		PPC_BIT32(22)
> > +#define I2C_INT_CMD_COMP	PPC_BIT32(23)
> > +#define I2C_INT_STOP_ERR	PPC_BIT32(24)
> > +#define I2C_INT_BUSY		PPC_BIT32(25)
> > +#define I2C_INT_IDLE		PPC_BIT32(26)
> > +
> > +/* I2C residual  register */
> > +#define I2C_RESID_FRONT		PPC_BITMASK32(0,15)
> > +#define I2C_RESID_BACK		PPC_BITMASK32(16,31)
> > +
> > +static int _i2cm_reg_write(struct i2cm *i2cm, uint32_t addr,
> > uint32_t data)
> > +{
> > +	CHECK_ERR(fsi_write(&i2cm->target, addr, data));
> > +	return 0;
> > +}
> > +
> > +static int _i2cm_reg_read(struct i2cm *i2cm, uint32_t addr,
> > uint32_t *data)
> > +{
> > +	CHECK_ERR(fsi_read(&i2cm->target, addr, data));
> > +	return 0;
> > +}
> > +
> > +static void debug_print_reg(struct i2cm *i2cm)
> > +{
> > +	uint32_t fsidata = 0;
> > +
> > +	PR_INFO("\t --------\n");
> > +	_i2cm_reg_read(i2cm,  I2C_STATUS_REG, &fsidata);
> > +	PR_INFO("\t status reg \t has value 0x%x \n", fsidata);
> > +	if (fsidata & I2C_STAT_INV_CMD)
> > +		 PR_INFO("\t\tinvalid cmd\n");
> > +	if (fsidata & I2C_STAT_PARITY)
> > +		 PR_INFO("\t\tparity\n");
> > +	if (fsidata & I2C_STAT_BE_OVERRUN)
> > +		 PR_INFO("\t\tback endoverrun\n");
> > +	if (fsidata & I2C_STAT_BE_ACCESS)
> > +		 PR_INFO("\t\tback end access error\n");
> > +	if (fsidata & I2C_STAT_LOST_ARB)
> > +		 PR_INFO("\t\tarbitration lost\n");
> > +	if (fsidata & I2C_STAT_NACK)
> > +		 PR_INFO("\t\tnack\n");
> > +	if (fsidata & I2C_STAT_DAT_REQ)
> > +		 PR_INFO("\t\tdata request\n");
> > +	if (fsidata & I2C_STAT_STOP_ERR)
> > +		 PR_INFO("\t\tstop error\n");
> > +	if (fsidata & I2C_STAT_PORT_BUSY)
> > +		 PR_INFO("\t\ti2c busy\n");
> > +	PR_INFO("\t\tfifo entry count: %li
> > \n",fsidata&I2C_STAT_FIFO_COUNT);
> > +
> > +	_i2cm_reg_read(i2cm,  I2C_ESTAT_REG, &fsidata);
> > +	PR_INFO("\t extended status reg has value 0x%x \n", fsidata);
> > +	if (fsidata & I2C_ESTAT_HIGH_WATER)
> > +		PR_INFO("\t\thigh water mark reached\n");
> > +	if (fsidata & I2C_ESTAT_LOW_WATER)
> > +		PR_INFO("\t\tlow water mark reached\n");
> > +
> > +
> > +	_i2cm_reg_read(i2cm,  I2C_WATERMARK_REG, &fsidata);
> > +	PR_INFO("\t watermark reg  has value 0x%x \n", fsidata);
> > +	PR_INFO("\t\twatermark high: %li
> > \n",fsidata&I2C_WATERMARK_HIGH);
> > +	PR_INFO("\t\twatermark low: %li \n",fsidata&I2C_WATERMARK_LOW);
> > +
> > +	_i2cm_reg_read(i2cm,  I2C_RESIDUAL_REG, &fsidata);
> > +	PR_INFO("\t residual reg  has value 0x%x \n", fsidata);
> > +	PR_INFO("\t\tfrontend_len: %li \n",fsidata&I2C_RESID_FRONT);
> > +	PR_INFO("\t\tbackend_len: %li \n",fsidata&I2C_RESID_BACK);
> > +
> > +	_i2cm_reg_read(i2cm,  I2C_PORT_BUSY_REG, &fsidata);
> > +	PR_INFO("\t port busy reg  has value 0x%x \n", fsidata);
> > +	PR_INFO("\t -------\n");
> > +
> > +}
> > +
> > +static void i2c_mode_write(struct i2cm *i2cm)
> > +{
> > +	uint32_t data = I2C_MODE_PACING;
> > +
> > +	// hardcoding bit rate divisor because not too important
> > +	data = SETFIELD(I2C_MODE_BIT_RATE_DIV, data, 28);
> > +	data = SETFIELD(I2C_MODE_PORT_NUM, data, i2cm->port);
> > +	PR_INFO("setting mode to %x\n", data);
> > +	_i2cm_reg_write(i2cm, I2C_MODE_REG, data);
> > +}
> > +
> > +static void i2c_watermark_write(struct i2cm *i2cm)
> > +{
> > +	uint32_t data = 0;
> > +
> > +	data = SETFIELD(I2C_WATERMARK_HIGH, data, 4);
> > +	data = SETFIELD(I2C_WATERMARK_LOW, data, 4);
> > +	PR_INFO("setting watermark (0x%x) to: %x\n", I2C_WATERMARK_REG,
> > data);
> > +	_i2cm_reg_write(i2cm, I2C_WATERMARK_REG, data);
> > +}
> > +
> > +static int i2c_reset(struct i2cm *i2cm)
> > +{
> > +	uint32_t fsidata = 0;
> > +	debug_print_reg(i2cm);
> > +	PR_INFO("### RESETING \n");
> > +
> > +	fsidata = 0xB;
> > +	_i2cm_reg_write(i2cm, I2C_IMD_RESET_REG, fsidata);
> > +	_i2cm_reg_write(i2cm, I2C_IMD_RESET_ERR_REG, fsidata);
> > +
> > +	usleep(10000);
> > +	debug_print_reg(i2cm);
> > +	return 0;
> > +}
> > +
> > +/*
> > + *	If there are errors in the status register, redo the whole
> > + *	operation after restting the i2cm.
> > +*/
> > +static int i2c_poll_status(struct i2cm *i2cm, uint32_t *data)
> > +{
> > +	int loop;
> > +
> > +	for (loop = 0; loop < 10; loop++)
> > +	{
> > +		usleep(10000);
> > +		_i2cm_reg_read(i2cm, I2C_STATUS_REG, data);
> > +
> > +		if (((*data) & I2C_STAT_CMD_COMP))
> > +			break;
> > +	}
> > +	if ((*data) & I2C_STAT_PORT_BUSY)
> > +		PR_INFO("portbusy\n");
> > +	if ((*data) & I2C_STAT_ERR) {
> > +		PR_INFO("ERROR IN STATUS\n");
> > +		debug_print_reg(i2cm);
> > +		return 1;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int i2c_fifo_write(struct i2cm *i2cm, uint32_t *data,
> > uint16_t size)
> > +{
> > +	int bytes_in_fifo = 1;
> > +	int rc = 0, bytes_written = 0;
> > +	uint32_t status;
> > +
> > +	while (bytes_written < size) {
> > +
> > +		if (bytes_written == size)
> > +			return 0;
> > +
> > +		/* Poll status register's FIFO_ENTRY_COUNT to check
> > that FIFO isn't 
> 
> full
> > */ +		rc = i2c_poll_status(i2cm, &status);
> > +		bytes_in_fifo = status & I2C_STAT_FIFO_COUNT;
> > +		PR_INFO("%x bytes in fifo \n", bytes_in_fifo);
> > +
> > +		if (rc)
> > +			return 0;
> > +
> > +		if (bytes_in_fifo == 8)
> > +			continue;
> > +
> > +		PR_INFO("\twriting: %x  to FIFO\n", data[bytes_written
> > / 4]);
> > +		rc = _i2cm_reg_write(i2cm, I2C_FIFO_REG,
> > data[bytes_written / 4]);
> > +		if (rc)
> > +			return bytes_written;
> > +		bytes_written += 4;
> > +	}
> > +	return bytes_written;
> > +}
> > +
> > +static int i2c_fifo_read(struct i2cm *i2cm, uint32_t *data,
> > uint16_t size)
> > +{
> > +	int bytes_to_read = 1;
> > +	int rc = 0, bytes_read = 0;
> > +	uint32_t tmp;
> > +	uint32_t status;
> > +
> > +	while (bytes_to_read) {
> > +
> > +		if (bytes_read > size)
> > +			return 0;
> > +
> > +		/* Poll status register's FIFO_ENTRY_COUNT to check
> > that there is data 
> 
> to
> > consume */ +		rc = i2c_poll_status(i2cm, &status);
> > +		bytes_to_read = status & I2C_STAT_FIFO_COUNT;
> > +		PR_INFO("%x bytes in fifo \n", bytes_to_read);
> > +
> > +		if (rc)
> > +			return 0;
> > +
> > +		if (!bytes_to_read)
> > +			continue;
> > +
> > +		rc = _i2cm_reg_read(i2cm, I2C_FIFO_REG, &tmp);
> > +		if (rc)
> > +			return bytes_read;
> > +		memcpy(data + (bytes_read / 4), &tmp, 4);
> > +		PR_INFO(" %x \n", data[bytes_read / 4]);
> > +		bytes_read += 4;
> > +	}
> > +	return bytes_read;
> > +}
> > +
> > +static int i2cm_ok_to_use(struct i2cm *i2cm)
> > +{
> > +	uint32_t data;
> > +
> > +	_i2cm_reg_read(i2cm, I2C_STATUS_REG, &data);
> > +
> > +	if (!(data & I2C_STAT_CMD_COMP) || (data & I2C_STAT_ERR))
> > +	{
> > +		PR_ERROR("ERROR IN STATUS, attempting to reset %x \n",
> > data);
> > +		i2c_reset(i2cm);
> > +		_i2cm_reg_read(i2cm, I2C_STATUS_REG, &data);
> > +	}
> > +	if (data & I2C_STAT_ERR) {
> > +		PR_ERROR("ERROR IN STATUS %x\n", data);
> > +		return 0;
> > +	}
> > +	i2c_watermark_write(i2cm);
> > +	i2c_mode_write(i2cm);
> > +	return 1;
> > +}
> > +
> > +static int i2c_put(struct i2cm *i2cm, uint32_t addr, uint16_t
> > size, uint8_t
> > *d) +{
> > +	uint32_t fsidata;
> > +	uint32_t data = 0;
> > +	int rc = 0;
> > +
> > +	if(!i2cm_ok_to_use(i2cm)) {
> > +		rc = 1;
> > +		return rc;
> > +	}
> > +	//TODO: if size > 64kB then use I2C_CMD_READ_CONTINUE and do
> > multiple
> > commands +	if (size > 64*1024 -1)
> > +		size = 64*1024 -1;
> > +
> > +
> > +	/* Set slave device */
> > +	fsidata = I2C_CMD_WITH_START | I2C_CMD_WITH_ADDR;
> > +	fsidata = SETFIELD(I2C_CMD_DEV_ADDR, fsidata, addr);
> > +	fsidata = SETFIELD(I2C_CMD_LENGTH, fsidata, size);
> > +	_i2cm_reg_write(i2cm, I2C_CMD_REG, fsidata);
> > +
> > +	rc = i2c_poll_status(i2cm, &data);
> > +	if (rc) {
> > +		PR_ERROR("FAILED to set i2c device\n");
> > +		return rc;
> > +	}
> > +
> > +	printf("%i\n", size);
> > +	/* Write data into the FIFO of the slave device */
> > +	i2c_fifo_write(i2cm, (uint32_t *)d, size);
> > +
> > +	rc = i2c_poll_status(i2cm, &data);
> > +	if (rc) {
> > +		PR_ERROR("FAILED to write all data\n");
> > +		return rc;
> > +	}
> > +	return rc;
> > +}
> > +
> > +static int i2c_get(struct i2cm *i2cm, uint32_t addr, uint16_t
> > size, uint8_t
> > *d) +{
> > +	uint32_t fsidata;
> > +	uint32_t data = 0;
> > +	int rc = 0;
> > +	int bytes_read;
> > +
> > +	if(!i2cm_ok_to_use(i2cm)) {
> > +		rc = 1;
> > +		return rc;
> > +	}
> > +
> > +	//TODO: if size > 64kB then use I2C_CMD_READ_CONTINUE and do
> > multiple
> > commands +	if (size > 64*1024 -1)
> > +		size = 64*1024 -1;
> > +
> > +	/* 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;
> 
> This should be an inherent property of the target rather than
> something 
> specified by the user. I can understand how this is convenient given
> we have a 
> single master with multiple ports but having each port as a target
> means you 
> can use all the existing target selection code, etc.
> 
> So instead I would create a device-tree structure like so (we can
> bikeshed 
> names as required):
> 
> i2c_perv@1800 {
> 	compatible = "ibm,cfam-i2c-master";
> 	#address-cells = <0x1>;
> 	#size-cells = <0x0>;
> 
> 	i2cm_port@0 {
> 		reg = <0x0>;
> 		compatible = "ibm,i2c-master-port";
> 	};
> 
> 	i2cm_port@1 {
> 		reg = <0x1>;
> 		compatible = "ibm,i2c-master-port";
> 	};
> };
> 
> Your i2c-master-port hw unit will then get it's `->port` propterty
> filled in 
> from the device-tree and it will become the target that gets passed
> to 
> i2c_read() which can then call some internal function with the
> appropriate 
> port number.
> 

OK!

> > 
> >  	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@1800 {
> > +			#address-cells = <0x2>;
> > +			#size-cells = <0x1>;
> > +			reg = <0x0 0x1800 0x400>;
> > +			compatible = "ibm,fsi-i2c-master";
> > +		};
> > +
> >  		hmfsi@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);
> >  	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,
> 
> Argh, this is where we use DATA8 :-)
> 

Just checking that you were reading the patches :-P 



> - Alistair
> 
> > 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)
> 
>