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 _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
