[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1478527862.5295.69.camel@linux.intel.com>
Date: Mon, 07 Nov 2016 16:11:02 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: tnhuynh@....com, Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
Mika Westerberg <mika.westerberg@...ux.intel.com>,
Wolfram Sang <wsa@...-dreams.de>, linux-i2c@...r.kernel.org,
linux-kernel@...r.kernel.org
Cc: Loc Ho <lho@....com>, Thang Nguyen <tqnguyen@....com>,
Phong Vo <pvo@....com>, patches@....com
Subject: Re: [PATCH v3] i2c: designware: Implement support for SMBus block
read and write
On Mon, 2016-10-31 at 13:51 +0700, tnhuynh@....com wrote:
> From: Tin Huynh <tnhuynh@....com>
>
> 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 <andriy.shevchenko@...ux.intel.com>
Intel Finland Oy
Powered by blists - more mailing lists