Re: svn commit: r307195 - head/sys/dev/iicbus
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 = TRAN
svn commit: r307195 - head/sys/dev/iicbus
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. 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.cThu Oct 13 07:22:13 2016 (r307194) +++ head/sys/dev/iicbus/iicsmb.cThu 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); + } +} + +#defineTRANSFER_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); -