On Mon, 2016-10-31 at 13:51 +0700, [email protected] wrote:
> From: Tin Huynh <[email protected]>
> 
> Free and Open IPMI use SMBUS BLOCK Read/Write to support SSIF
> protocol.
> However, I2C Designware Core Driver doesn't handle the case at the
> moment.
> The below patch supports this feature.

My comments below.


> @@ -543,6 +543,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev
> *dev)
>       intr_mask = DW_IC_INTR_DEFAULT_MASK;
>  
>       for (; dev->msg_write_idx < dev->msgs_num; dev-
> >msg_write_idx++) {
> +             u32 flags = msgs[dev->msg_write_idx].flags;

+ empty line.

>               /*
>                * if target address has changed, we need to
>                * reprogram the target address in the i2c

> @@ -588,8 +589,15 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev
> *dev)
>                        * detected from the registers so we set it
> always
>                        * when writing/reading the last byte.
>                        */
> +
> +                     /*
> +                      * i2c-core.c always set the buffer length of

set -> sets

> +                      * I2C_FUNC_SMBUS_BLOCK_DATA to 1. The length
> will
> +                      * be adjusted when receiving the first byte.
> +                      * Thus we can't stop the transaction here.
> +                      */
> 

> @@ -635,6 +648,25 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev
> *dev)
>       dw_writel(dev, intr_mask,  DW_IC_INTR_MASK);
>  }
>  
> +static u8
> +i2c_dw_recv_len(struct dw_i2c_dev *dev, u8 len)
> +{
> +     struct i2c_msg *msgs = dev->msgs;
> +     u32 flags = msgs[dev->msg_read_idx].flags;
> +
> +     /*
> +      * Adjust the buffer length and mask the flag
> +      * after receiving the first byte

Add dot to the end, please.

> +      */
> +     len = (flags & I2C_CLIENT_PEC) ? len + 2 : len + 1;

len += flags & I2C_CLIENT_PEC ? 2 : 1;

> +     dev->tx_buf_len = len > dev->rx_outstanding ?
> +             len - dev->rx_outstanding : 0;

Can be len more than twice longer as rx_outstanding?

Would it be better to write as
tx_buf_len = len - min(len, rx_outstanding);
?

> +     msgs[dev->msg_read_idx].len = len;
> +     msgs[dev->msg_read_idx].flags &= ~I2C_M_RECV_LEN;
> +
> +     return len;
> +}
> +
>  static void
>  i2c_dw_read(struct dw_i2c_dev *dev)
>  {
> @@ -659,7 +691,14 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev
> *dev)
>               rx_valid = dw_readl(dev, DW_IC_RXFLR);
>  
>               for (; len > 0 && rx_valid > 0; len--, rx_valid--) {
> -                     *buf++ = dw_readl(dev, DW_IC_DATA_CMD);
> +                     u32 flags = msgs[dev->msg_read_idx].flags;

+ empty line.

> +                     *buf = dw_readl(dev, DW_IC_DATA_CMD);
> +                     /* Ensure length byte is a valid value */
> +                     if (flags & I2C_M_RECV_LEN &&
> +                             *buf <= I2C_SMBUS_BLOCK_MAX && *buf >
> 0) {

Is it my mail client or indentation is wrong?

> +                             len = i2c_dw_recv_len(dev, *buf);
> +                     }
> +                     buf++;
>                       dev->rx_outstanding--;
>               }

-- 
Andy Shevchenko <[email protected]>
Intel Finland Oy

Reply via email to