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

Reply via email to