svn commit: r307195 - head/sys/dev/iicbus

Andriy Gapon avg at FreeBSD.org
Thu Oct 13 07:50:01 UTC 2016


On 13/10/2016 10:25, Andriy Gapon wrote:
> Author: avg
> Date: Thu Oct 13 07:25:18 2016
> New Revision: 307195
> URL: https://svnweb.freebsd.org/changeset/base/307195
> 
> Log:
>   convert iicsmb to use iicbus_transfer for all operations
>   
>   Previously the driver used more low level operations like iicbus_start
>   and iicbus_write.  The problem is that those operations are not
>   implemented by iicbus(4) and the calls were effectively routed to
>   a driver to which the bus is attached.
>   But not all of the controllers implement such low level operations
>   while all of the drivers are expected to have iicbus_transfer.
>   
>   While there fix incorrect implementation of iicsmb_bwrite and iicsmb_bread.
>   The former should send a byte count before the actual bytes, while the
>   latter should first receive the byte count and then receive the bytes.

Just a thought.  It would be much easier to implement iicsmb_bread() if we had a
flag like I2C_M_RECV_LEN in Linux.  The flag signals that the first byte
received from slave is a count of how many more bytes we should read from that
slave.  But adding support for a new flag to all controller drivers is not fun.

>   I have tested only these commands:
>   - quick (r/w)
>   - send byte
>   - receive byte
>   - read byte
>   - write byte
>   
>   MFC after:	1 month
>   Differential Revision: https://reviews.freebsd.org/D8170
> 
> Modified:
>   head/sys/dev/iicbus/iicsmb.c
> 
> Modified: head/sys/dev/iicbus/iicsmb.c
> ==============================================================================
> --- head/sys/dev/iicbus/iicsmb.c	Thu Oct 13 07:22:13 2016	(r307194)
> +++ head/sys/dev/iicbus/iicsmb.c	Thu Oct 13 07:25:18 2016	(r307195)
> @@ -131,8 +131,6 @@ static driver_t iicsmb_driver = {
>  	sizeof(struct iicsmb_softc),
>  };
>  
> -#define IICBUS_TIMEOUT	100	/* us */
> -
>  static void
>  iicsmb_identify(driver_t *driver, device_t parent)
>  {
> @@ -276,237 +274,213 @@ iicsmb_callback(device_t dev, int index,
>  }
>  
>  static int
> +iic2smb_error(int error)
> +{
> +	switch (error) {
> +	case IIC_NOERR:
> +		return (SMB_ENOERR);
> +	case IIC_EBUSERR:
> +		return (SMB_EBUSERR);
> +	case IIC_ENOACK:
> +		return (SMB_ENOACK);
> +	case IIC_ETIMEOUT:
> +		return (SMB_ETIMEOUT);
> +	case IIC_EBUSBSY:
> +		return (SMB_EBUSY);
> +	case IIC_ESTATUS:
> +		return (SMB_EBUSERR);
> +	case IIC_EUNDERFLOW:
> +		return (SMB_EBUSERR);
> +	case IIC_EOVERFLOW:
> +		return (SMB_EBUSERR);
> +	case IIC_ENOTSUPP:
> +		return (SMB_ENOTSUPP);
> +	case IIC_ENOADDR:
> +		return (SMB_EBUSERR);
> +	case IIC_ERESOURCE:
> +		return (SMB_EBUSERR);
> +	default:
> +		return (SMB_EBUSERR);
> +	}
> +}
> +
> +#define	TRANSFER_MSGS(dev, msgs)	iicbus_transfer(dev, msgs, nitems(msgs))
> +
> +static int
>  iicsmb_quick(device_t dev, u_char slave, int how)
>  {
> -	device_t parent = device_get_parent(dev);
> +	struct iic_msg msgs[] = {
> +	     { slave, how == SMB_QWRITE ? IIC_M_WR : IIC_M_RD, 0, NULL },
> +	};
>  	int error;
>  
>  	switch (how) {
>  	case SMB_QWRITE:
> -		error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT);
> -		break;
> -
>  	case SMB_QREAD:
> -		error = iicbus_start(parent, slave | LSB, IICBUS_TIMEOUT);
>  		break;
> -
>  	default:
> -		error = EINVAL;
> -		break;
> +		return (SMB_EINVAL);
>  	}
>  
> -	if (!error)
> -		error = iicbus_stop(parent);
> -		
> -	return (error);
> +	error = TRANSFER_MSGS(dev, msgs);
> +	return (iic2smb_error(error));
>  }
>  
>  static int
>  iicsmb_sendb(device_t dev, u_char slave, char byte)
>  {
> -	device_t parent = device_get_parent(dev);
> -	int error, sent;
> -
> -	error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT);
> -
> -	if (!error) {
> -		error = iicbus_write(parent, &byte, 1, &sent, IICBUS_TIMEOUT);
> -
> -		iicbus_stop(parent);
> -	}
> +	struct iic_msg msgs[] = {
> +	     { slave, IIC_M_WR, 1, &byte },
> +	};
> +	int error;
>  
> -	return (error);
> +	error = TRANSFER_MSGS(dev, msgs);
> +	return (iic2smb_error(error));
>  }
>  
>  static int
>  iicsmb_recvb(device_t dev, u_char slave, char *byte)
>  {
> -	device_t parent = device_get_parent(dev);
> -	int error, read;
> -
> -	error = iicbus_start(parent, slave | LSB, 0);
> -
> -	if (!error) {
> -		error = iicbus_read(parent, byte, 1, &read, IIC_LAST_READ, IICBUS_TIMEOUT);
> -
> -		iicbus_stop(parent);
> -	}
> +	struct iic_msg msgs[] = {
> +	     { slave, IIC_M_RD, 1, byte },
> +	};
> +	int error;
>  
> -	return (error);
> +	error = TRANSFER_MSGS(dev, msgs);
> +	return (iic2smb_error(error));
>  }
>  
>  static int
>  iicsmb_writeb(device_t dev, u_char slave, char cmd, char byte)
>  {
> -	device_t parent = device_get_parent(dev);
> -	int error, sent;
> -
> -	error = iicbus_start(parent, slave & ~LSB, 0);
> -
> -	if (!error) {
> -		if (!(error = iicbus_write(parent, &cmd, 1, &sent, IICBUS_TIMEOUT)))
> -			error = iicbus_write(parent, &byte, 1, &sent, IICBUS_TIMEOUT);
> -
> -		iicbus_stop(parent);
> -	}
> +	uint8_t bytes[] = { cmd, byte };
> +	struct iic_msg msgs[] = {
> +	     { slave, IIC_M_WR, nitems(bytes), bytes },
> +	};
> +	int error;
>  
> -	return (error);
> +	error = TRANSFER_MSGS(dev, msgs);
> +	return (iic2smb_error(error));
>  }
>  
>  static int
>  iicsmb_writew(device_t dev, u_char slave, char cmd, short word)
>  {
> -	device_t parent = device_get_parent(dev);
> -	int error, sent;
> -
> -	char low = (char)(word & 0xff);
> -	char high = (char)((word & 0xff00) >> 8);
> -
> -	error = iicbus_start(parent, slave & ~LSB, 0);
> -
> -	if (!error) {
> -		if (!(error = iicbus_write(parent, &cmd, 1, &sent, IICBUS_TIMEOUT)))
> -		  if (!(error = iicbus_write(parent, &low, 1, &sent, IICBUS_TIMEOUT)))
> -		    error = iicbus_write(parent, &high, 1, &sent, IICBUS_TIMEOUT);
> -
> -		iicbus_stop(parent);
> -	}
> +	uint8_t bytes[] = { cmd, word & 0xff, word >> 8 };
> +	struct iic_msg msgs[] = {
> +	     { slave, IIC_M_WR, nitems(bytes), bytes },
> +	};
> +	int error;
>  
> -	return (error);
> +	error = TRANSFER_MSGS(dev, msgs);
> +	return (iic2smb_error(error));
>  }
>  
>  static int
>  iicsmb_readb(device_t dev, u_char slave, char cmd, char *byte)
>  {
> -	device_t parent = device_get_parent(dev);
> -	int error, sent, read;
> -
> -	if ((error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT)))
> -		return (error);
> -
> -	if ((error = iicbus_write(parent, &cmd, 1, &sent, IICBUS_TIMEOUT)))
> -		goto error;
> -
> -	if ((error = iicbus_repeated_start(parent, slave | LSB, IICBUS_TIMEOUT)))
> -		goto error;
> -
> -	if ((error = iicbus_read(parent, byte, 1, &read, IIC_LAST_READ, IICBUS_TIMEOUT)))
> -		goto error;
> +	struct iic_msg msgs[] = {
> +	     { slave, IIC_M_WR | IIC_M_NOSTOP, 1, &cmd },
> +	     { slave, IIC_M_RD, 1, byte },
> +	};
> +	int error;
>  
> -error:
> -	iicbus_stop(parent);
> -	return (error);
> +	error = TRANSFER_MSGS(dev, msgs);
> +	return (iic2smb_error(error));
>  }
>  
> -#define BUF2SHORT(low,high) \
> -	((short)(((high) & 0xff) << 8) | (short)((low) & 0xff))
> -
>  static int
>  iicsmb_readw(device_t dev, u_char slave, char cmd, short *word)
>  {
> -	device_t parent = device_get_parent(dev);
> -	int error, sent, read;
> -	char buf[2];
> -
> -	if ((error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT)))
> -		return (error);
> -
> -	if ((error = iicbus_write(parent, &cmd, 1, &sent, IICBUS_TIMEOUT)))
> -		goto error;
> -
> -	if ((error = iicbus_repeated_start(parent, slave | LSB, IICBUS_TIMEOUT)))
> -		goto error;
> -
> -	if ((error = iicbus_read(parent, buf, 2, &read, IIC_LAST_READ, IICBUS_TIMEOUT)))
> -		goto error;
> -
> -	/* first, receive low, then high byte */
> -	*word = BUF2SHORT(buf[0], buf[1]);
> +	uint8_t buf[2];
> +	struct iic_msg msgs[] = {
> +	     { slave, IIC_M_WR | IIC_M_NOSTOP, 1, &cmd },
> +	     { slave, IIC_M_RD, nitems(buf), buf },
> +	};
> +	int error;
>  
> -error:
> -	iicbus_stop(parent);
> -	return (error);
> +	error = TRANSFER_MSGS(dev, msgs);
> +	if (error == 0)
> +		*word = ((uint16_t)buf[1] << 8) | buf[0];
> +	return (iic2smb_error(error));
>  }
>  
>  static int
>  iicsmb_pcall(device_t dev, u_char slave, char cmd, short sdata, short *rdata)
>  {
> -	device_t parent = device_get_parent(dev);
> -	int error, sent, read;
> -	char buf[2];
> -
> -	if ((error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT)))
> -		return (error);
> -
> -	if ((error = iicbus_write(parent, &cmd, 1, &sent, IICBUS_TIMEOUT)))
> -		goto error;
> -
> -	/* first, send low, then high byte */
> -	buf[0] = (char)(sdata & 0xff);
> -	buf[1] = (char)((sdata & 0xff00) >> 8);
> -
> -	if ((error = iicbus_write(parent, buf, 2, &sent, IICBUS_TIMEOUT)))
> -		goto error;
> -
> -	if ((error = iicbus_repeated_start(parent, slave | LSB, IICBUS_TIMEOUT)))
> -		goto error;
> -
> -	if ((error = iicbus_read(parent, buf, 2, &read, IIC_LAST_READ, IICBUS_TIMEOUT)))
> -		goto error;
> -
> -	/* first, receive low, then high byte */
> -	*rdata = BUF2SHORT(buf[0], buf[1]);
> +	uint8_t in[3] = { cmd, sdata & 0xff, sdata >> 8 };
> +	uint8_t out[2];
> +	struct iic_msg msgs[] = {
> +	     { slave, IIC_M_WR | IIC_M_NOSTOP, nitems(in), in },
> +	     { slave, IIC_M_RD, nitems(out), out },
> +	};
> +	int error;
>  
> -error:
> -	iicbus_stop(parent);
> -	return (error);
> +	error = TRANSFER_MSGS(dev, msgs);
> +	if (error == 0)
> +		*rdata = ((uint16_t)out[1] << 8) | out[0];
> +	return (iic2smb_error(error));
>  }
>  
>  static int
>  iicsmb_bwrite(device_t dev, u_char slave, char cmd, u_char count, char *buf)
>  {
> -	device_t parent = device_get_parent(dev);
> -	int error, sent;
> -
> -	if ((error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT)))
> -		goto error;
> -
> -	if ((error = iicbus_write(parent, &cmd, 1, &sent, IICBUS_TIMEOUT)))
> -		goto error;
> -
> -	if ((error = iicbus_write(parent, buf, (int)count, &sent, IICBUS_TIMEOUT)))
> -		goto error;
> -
> -	if ((error = iicbus_stop(parent)))
> -		goto error;
> +	uint8_t bytes[2] = { cmd, count };
> +	struct iic_msg msgs[] = {
> +	     { slave, IIC_M_WR | IIC_M_NOSTOP, nitems(bytes), bytes },
> +	     { slave, IIC_M_WR | IIC_M_NOSTART, count, buf },
> +	};
> +	int error;
>  
> -error:
> -	return (error);
> +	if (count > 32 || count == 0)
> +		return (SMB_EINVAL);
> +	error = TRANSFER_MSGS(dev, msgs);
> +	return (iic2smb_error(error));
>  }
>  
>  static int
>  iicsmb_bread(device_t dev, u_char slave, char cmd, u_char *count, char *buf)
>  {
> +	struct iic_msg msgs[] = {
> +	     { slave, IIC_M_WR | IIC_M_NOSTOP, 1, &cmd },
> +	     { slave, IIC_M_RD | IIC_M_NOSTOP, 1, count },
> +	};
> +	struct iic_msg block_msg[] = {
> +	     { slave, IIC_M_RD | IIC_M_NOSTART, 0, buf },
> +	};
>  	device_t parent = device_get_parent(dev);
> -	int error, sent, read;
> -
> -	if ((error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT)))
> -		return (error);
> -
> -	if ((error = iicbus_write(parent, &cmd, 1, &sent, IICBUS_TIMEOUT)))
> -		goto error;
> -
> -	if ((error = iicbus_repeated_start(parent, slave | LSB, IICBUS_TIMEOUT)))
> -		goto error;
> -
> -	if ((error = iicbus_read(parent, buf, (int)*count, &read,
> -						IIC_LAST_READ, IICBUS_TIMEOUT)))
> -		goto error;
> -	*count = read;
> +	int error;
> +	u_char bufsz;
>  
> -error:
> -	iicbus_stop(parent);
> -	return (error);
> +	/* Stash output buffer size before overwriting it. */
> +	bufsz = *count;
> +	if (bufsz == 0)
> +		return (SMB_EINVAL);
> +
> +	/* Have to do this because the command is split in two transfers. */
> +	error = iicbus_request_bus(parent, dev, IIC_WAIT);
> +	if (error == 0)
> +		error = TRANSFER_MSGS(dev, msgs);
> +	if (error == 0) {
> +		/*
> +		 * If the slave offers an empty or a too long reply,
> +		 * read one byte to generate the stop or abort.
> +		 * XXX 32 is hardcoded until SMB_MAXBLOCKSIZE is restored
> +		 * to sanity.
> +		 */
> +		if (*count > 32 || *count == 0)
> +			block_msg[0].len = 1;
> +		/* If longer than the buffer, then clamp at the buffer size. */
> +		if (*count > bufsz)
> +			block_msg[0].len = bufsz;
> +		else
> +			block_msg[0].len = *count;
> +		error = TRANSFER_MSGS(dev, block_msg);
> +		if (*count > 32 || *count == 0)
> +			error = SMB_EINVAL;
> +	}
> +	(void)iicbus_release_bus(parent, dev);
> +	return (iic2smb_error(error));
>  }
>  
>  DRIVER_MODULE(iicsmb, iicbus, iicsmb_driver, iicsmb_devclass, 0, 0);
> 


-- 
Andriy Gapon


More information about the svn-src-all mailing list