* [PATCH v1] I2C Designware Core Supports SMBUS BLOCK
@ 2016-10-25 4:35 tnhuynh
2016-10-25 7:58 ` Jarkko Nikula
2016-10-25 11:50 ` Mika Westerberg
0 siblings, 2 replies; 6+ messages in thread
From: tnhuynh @ 2016-10-25 4:35 UTC (permalink / raw)
To: Jarkko Nikula, Andy Shevchenko, Mika Westerberg, Wolfram Sang,
linux-i2c, linux-kernel
Cc: Loc Ho, Thang Nguyen, Phong Vo, patches, Tin Huynh
From: Tin Huynh <tnhuynh@apm.com>
Free and Open IPMI use SMBUS BLOCK Read/Write to support SSIF protocol.
However, I2C Designwave Core Driver doesn't handle the case at the moment.
The below patch supports this feature.
Signed-off-by: Tin Huynh <tnhuynh@apm.com>
---
drivers/i2c/busses/i2c-designware-core.c | 42 +++++++++++++++++++++++++--
drivers/i2c/busses/i2c-designware-platdrv.c | 1 +
2 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 1fe93c4..3abf0e5 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -588,8 +588,17 @@ 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
+ * 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.
+ */
+
if (dev->msg_write_idx == dev->msgs_num - 1 &&
- buf_len == 1)
+ buf_len == 1 &&
+ !(msgs[dev->msg_write_idx].flags & I2C_M_RECV_LEN))
cmd |= BIT(9);
if (need_restart) {
@@ -614,7 +623,14 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
dev->tx_buf = buf;
dev->tx_buf_len = buf_len;
- if (buf_len > 0) {
+ /*
+ * Because we don't know the buffer length in the
+ * I2C_FUNC_SMBUS_BLOCK_DATA case, we can't stop
+ * the transcation here.
+ */
+
+ if (buf_len > 0 ||
+ msgs[dev->msg_write_idx].flags & I2C_M_RECV_LEN) {
/* more bytes to be written */
dev->status |= STATUS_WRITE_IN_PROGRESS;
break;
@@ -659,7 +675,27 @@ 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);
+ *buf = dw_readl(dev, DW_IC_DATA_CMD);
+ /* ensure length byte is a valid value */
+ if (msgs[dev->msg_read_idx].flags & I2C_M_RECV_LEN
+ && *buf <= I2C_SMBUS_BLOCK_MAX && *buf > 0) {
+ /*
+ * Adjust the buffer length and mask the flag
+ * after receiving the first byte
+ */
+ msgs[dev->msg_read_idx].flags &=
+ ~I2C_M_RECV_LEN;
+ len = *buf + 1;
+ /* Increase one with PEC flag */
+ if (msgs[dev->msg_read_idx].flags &
+ I2C_CLIENT_PEC)
+ len++;
+
+ dev->tx_buf_len = len > dev->rx_outstanding ?
+ len - dev->rx_outstanding : 0;
+ msgs[dev->msg_read_idx].len = len;
+ }
+ buf++;
dev->rx_outstanding--;
}
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 0b42a12..886fb62 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -220,6 +220,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
I2C_FUNC_SMBUS_BYTE |
I2C_FUNC_SMBUS_BYTE_DATA |
I2C_FUNC_SMBUS_WORD_DATA |
+ I2C_FUNC_SMBUS_BLOCK_DATA |
I2C_FUNC_SMBUS_I2C_BLOCK;
dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
--
1.7.1
--
CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is
for the sole use of the intended recipient(s) and contains information that
is confidential and proprietary to Applied Micro Circuits Corporation or
its subsidiaries. It is to be used solely for the purpose of furthering the
parties' business relationship. All unauthorized review, use, disclosure or
distribution is prohibited. If you are not the intended recipient, please
contact the sender by reply e-mail and destroy all copies of the original
message.
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v1] I2C Designware Core Supports SMBUS BLOCK
2016-10-25 4:35 [PATCH v1] I2C Designware Core Supports SMBUS BLOCK tnhuynh
@ 2016-10-25 7:58 ` Jarkko Nikula
2016-10-25 11:50 ` Mika Westerberg
1 sibling, 0 replies; 6+ messages in thread
From: Jarkko Nikula @ 2016-10-25 7:58 UTC (permalink / raw)
To: tnhuynh, Andy Shevchenko, Mika Westerberg, Wolfram Sang,
linux-i2c, linux-kernel
Cc: Loc Ho, Thang Nguyen, Phong Vo, patches
Hi
On 10/25/2016 07:35 AM, tnhuynh@apm.com wrote:
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -220,6 +220,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
> I2C_FUNC_SMBUS_BYTE |
> I2C_FUNC_SMBUS_BYTE_DATA |
> I2C_FUNC_SMBUS_WORD_DATA |
> + I2C_FUNC_SMBUS_BLOCK_DATA |
> I2C_FUNC_SMBUS_I2C_BLOCK;
>
I guess you need to add the same to DW_DEFAULT_FUNCTIONALITY in
drivers/i2c/busses/i2c-designware-pcidrv.c too.
--
Jarkko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] I2C Designware Core Supports SMBUS BLOCK
2016-10-25 4:35 [PATCH v1] I2C Designware Core Supports SMBUS BLOCK tnhuynh
2016-10-25 7:58 ` Jarkko Nikula
@ 2016-10-25 11:50 ` Mika Westerberg
2016-10-25 12:34 ` Andy Shevchenko
[not found] ` <CANQ9TuB5BbR40TEndGqQ6ABDSUsCOGjEhXSvmD78jtuqGRpXvg@mail.gmail.com>
1 sibling, 2 replies; 6+ messages in thread
From: Mika Westerberg @ 2016-10-25 11:50 UTC (permalink / raw)
To: tnhuynh
Cc: Jarkko Nikula, Andy Shevchenko, Wolfram Sang, linux-i2c,
linux-kernel, Loc Ho, Thang Nguyen, Phong Vo, patches
On Tue, Oct 25, 2016 at 11:35:13AM +0700, tnhuynh@apm.com wrote:
> From: Tin Huynh <tnhuynh@apm.com>
>
> Free and Open IPMI use SMBUS BLOCK Read/Write to support SSIF protocol.
> However, I2C Designwave Core Driver doesn't handle the case at the moment.
> The below patch supports this feature.
>
> Signed-off-by: Tin Huynh <tnhuynh@apm.com>
> ---
> drivers/i2c/busses/i2c-designware-core.c | 42 +++++++++++++++++++++++++--
> drivers/i2c/busses/i2c-designware-platdrv.c | 1 +
> 2 files changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
> index 1fe93c4..3abf0e5 100644
> --- a/drivers/i2c/busses/i2c-designware-core.c
> +++ b/drivers/i2c/busses/i2c-designware-core.c
> @@ -588,8 +588,17 @@ 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
> + * 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.
> + */
> +
No empty line here.
> if (dev->msg_write_idx == dev->msgs_num - 1 &&
> - buf_len == 1)
> + buf_len == 1 &&
> + !(msgs[dev->msg_write_idx].flags & I2C_M_RECV_LEN))
Why not store flags somewhere in local variable. Then you do not need
the ugly line splitting above.
So instead
if (dev->msg_write_idx == dev->msgs_num -1 &&
buf_len == 1 && !(flags & I2C_M_RECV_LEN))
but even that starts to require small helper function, I think.
> cmd |= BIT(9);
>
> if (need_restart) {
> @@ -614,7 +623,14 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
> dev->tx_buf = buf;
> dev->tx_buf_len = buf_len;
>
> - if (buf_len > 0) {
> + /*
> + * Because we don't know the buffer length in the
> + * I2C_FUNC_SMBUS_BLOCK_DATA case, we can't stop
> + * the transcation here.
^^^^^^^^^^^
transaction
> + */
> +
No empty line here.
> + if (buf_len > 0 ||
> + msgs[dev->msg_write_idx].flags & I2C_M_RECV_LEN) {
Here also same comments about "flags".
> /* more bytes to be written */
> dev->status |= STATUS_WRITE_IN_PROGRESS;
> break;
> @@ -659,7 +675,27 @@ 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);
> + *buf = dw_readl(dev, DW_IC_DATA_CMD);
> + /* ensure length byte is a valid value */
> + if (msgs[dev->msg_read_idx].flags & I2C_M_RECV_LEN
> + && *buf <= I2C_SMBUS_BLOCK_MAX && *buf > 0) {
And here. Also you have " " between I2C_SMBUS_BLOCK_MAX and &&.
> + /*
> + * Adjust the buffer length and mask the flag
> + * after receiving the first byte
> + */
> + msgs[dev->msg_read_idx].flags &=
> + ~I2C_M_RECV_LEN;
And here.
Actually you should move the whole block into a helper function.
> + len = *buf + 1;
> + /* Increase one with PEC flag */
> + if (msgs[dev->msg_read_idx].flags &
> + I2C_CLIENT_PEC)
> + len++;
> +
> + dev->tx_buf_len = len > dev->rx_outstanding ?
> + len - dev->rx_outstanding : 0;
> + msgs[dev->msg_read_idx].len = len;
> + }
> + buf++;
> dev->rx_outstanding--;
> }
>
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 0b42a12..886fb62 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -220,6 +220,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
> I2C_FUNC_SMBUS_BYTE |
> I2C_FUNC_SMBUS_BYTE_DATA |
> I2C_FUNC_SMBUS_WORD_DATA |
> + I2C_FUNC_SMBUS_BLOCK_DATA |
> I2C_FUNC_SMBUS_I2C_BLOCK;
>
> dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> --
> 1.7.1
>
>
> --
> CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is
> for the sole use of the intended recipient(s) and contains information that
> is confidential and proprietary to Applied Micro Circuits Corporation or
> its subsidiaries. It is to be used solely for the purpose of furthering the
> parties' business relationship. All unauthorized review, use, disclosure or
> distribution is prohibited. If you are not the intended recipient, please
> contact the sender by reply e-mail and destroy all copies of the original
> message.
Yeah, right.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1] I2C Designware Core Supports SMBUS BLOCK
2016-10-25 11:50 ` Mika Westerberg
@ 2016-10-25 12:34 ` Andy Shevchenko
[not found] ` <CANQ9TuB5BbR40TEndGqQ6ABDSUsCOGjEhXSvmD78jtuqGRpXvg@mail.gmail.com>
1 sibling, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2016-10-25 12:34 UTC (permalink / raw)
To: Mika Westerberg, tnhuynh
Cc: Jarkko Nikula, Wolfram Sang, linux-i2c, linux-kernel, Loc Ho,
Thang Nguyen, Phong Vo, patches
On Tue, 2016-10-25 at 14:50 +0300, Mika Westerberg wrote:
> On Tue, Oct 25, 2016 at 11:35:13AM +0700, tnhuynh@apm.com wrote:
> > --
> > CONFIDENTIALITY NOTICE: This e-mail message, including any
> > attachments, is
> > for the sole use of the intended recipient(s) and contains
> > information that
> > is confidential and proprietary to Applied Micro Circuits
> > Corporation or
> > its subsidiaries. It is to be used solely for the purpose of
> > furthering the
> > parties' business relationship. All unauthorized review, use,
> > disclosure or
> > distribution is prohibited. If you are not the intended recipient,
> > please
> > contact the sender by reply e-mail and destroy all copies of the
> > original
> > message.
Tin Huynh, if you want to continue, you *have to* get rid of this
footer, otherwise no one is going to commit even *the best feature
ever*.
Perhaps you need to talk to your lawyers first.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 6+ messages in thread[parent not found: <CANQ9TuB5BbR40TEndGqQ6ABDSUsCOGjEhXSvmD78jtuqGRpXvg@mail.gmail.com>]
* Re: [PATCH v1] I2C Designware Core Supports SMBUS BLOCK
[not found] ` <CANQ9TuB5BbR40TEndGqQ6ABDSUsCOGjEhXSvmD78jtuqGRpXvg@mail.gmail.com>
@ 2016-10-26 8:16 ` Mika Westerberg
[not found] ` <CANQ9TuAgW7ubyKq1RAiw+s=ooF4G99_MMWnG1a0bV3DxKa+UPw@mail.gmail.com>
0 siblings, 1 reply; 6+ messages in thread
From: Mika Westerberg @ 2016-10-26 8:16 UTC (permalink / raw)
To: Tin Huynh
Cc: Jarkko Nikula, Andy Shevchenko, Wolfram Sang, linux-i2c,
linux-kernel, Loc Ho, Thang Nguyen, Phong Vo, patches
On Wed, Oct 26, 2016 at 02:26:05PM +0700, Tin Huynh wrote:
> But can you explain more about 'but even that starts to require small
> helper function'?
So you could add a small function i2c_dw_xfer_need_stop() that returns
true if you need to set the stop bit.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-10-26 9:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-25 4:35 [PATCH v1] I2C Designware Core Supports SMBUS BLOCK tnhuynh
2016-10-25 7:58 ` Jarkko Nikula
2016-10-25 11:50 ` Mika Westerberg
2016-10-25 12:34 ` Andy Shevchenko
[not found] ` <CANQ9TuB5BbR40TEndGqQ6ABDSUsCOGjEhXSvmD78jtuqGRpXvg@mail.gmail.com>
2016-10-26 8:16 ` Mika Westerberg
[not found] ` <CANQ9TuAgW7ubyKq1RAiw+s=ooF4G99_MMWnG1a0bV3DxKa+UPw@mail.gmail.com>
2016-10-26 9:30 ` Mika Westerberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).