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, , 1, , IICBUS_TIMEOUT);
> -
> - iicbus_stop(parent);
> - }
> + struct iic_msg msgs[] = {
> + { slave, IIC_M_WR, 1, },
> + };
> + 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, , 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,
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, , 1, , IICBUS_TIMEOUT);
-
- iicbus_stop(parent);
- }
+ struct iic_msg msgs[] = {
+{ slave, IIC_M_WR, 1, },
+ };
+ 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, , 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, , 1, ,
IICBUS_TIMEOUT)))
- error = iicbus_write(parent, , 1, ,
IICBUS_TIMEOUT);
-
- iicbus_stop(parent);
- }
+ uint8_t bytes[] = {
