* [PATCH v3 01/12] i2c: designware: Fix slave state machine for sequential reads
2022-11-07 13:42 [PATCH v3 00/12] i2c: designware: Slave fixes and generic cleanups Jarkko Nikula
@ 2022-11-07 13:42 ` Jarkko Nikula
2022-11-07 13:42 ` [PATCH v3 02/12] i2c: designware: Empty receive FIFO in slave interrupt handler Jarkko Nikula
` (11 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Jarkko Nikula @ 2022-11-07 13:42 UTC (permalink / raw)
To: linux-i2c
Cc: Wolfram Sang, Andy Shevchenko, Mika Westerberg, Jan Dabros,
Michael Wu, Tian Ye, Jarkko Nikula
Some read types from I2C bus don't work correctly when testing the
i2c-designware-slave.c with the slave-eeprom backend. The same reads
work correctly when testing with a real 24c02 EEPROM chip.
In the following tests an i2c-designware-slave.c instance with the
slave-eeprom backend is configured to act as a simulated 24c02 at
address 0x65 on an I2C host bus 6:
1. i2cdump -y 6 0x65 b (OK)
Random read. Each byte are read using a byte address write with a
current address read in a same message.
2. i2cdump -y 6 0x65 c (OK, was NOK before commit 3b5f7f10ff6e when it
was repeating the 1st byte)
Repeated current address read. One byte address write message
followed by repeated current address read messages.
3. i2cdump -y 6 0x65 i (NOK, each 32 byte block repeats the 1st byte of
block)
Sequential read using SMBus Block Read. For each 32 byte block a byte
address write followed by 32 sequental reads in a same message.
These findings are explained because the implementation has had a
mismatch between hardware interrupts and what I2C slave events should be
sent after those interrupts. Despite that the case 1 happened to have
always the I2C slave events sent to a right order with a right data
between backend and the I2C bus.
Hardware generates the DW_IC_INTR_RD_REQ interrupt when another host is
attempting to read and for sequential reads after. DW_IC_INTR_RX_DONE
occurs when host does not acknowledge a transmitted byte which is an
indication the end of transmission.
Those interrupts do not match directly with I2C_SLAVE_READ_REQUESTED and
I2C_SLAVE_READ_PROCESSED events which is how the code was and is
practically using them. The slave-eeprom backend increases the buffer
index with the I2C_SLAVE_READ_PROCESSED event and returns the data from
current index when receiving only the I2C_SLAVE_READ_REQUESTED event.
That explains the repeated bytes in case 3 and also case 2 before
commit 3b5f7f10ff6e ("i2c: designware: slave should do WRITE_REQUESTED
before WRITE_RECEIVED").
Patch fixes the case 3 while keep cases 1 and 2 working with following
changes:
- First DW_IC_INTR_RD_REQ interrupt will change the state machine to
read in progress state, send I2C_SLAVE_READ_REQUESTED event and
transmit the first byte from backend
- Subsequent DW_IC_INTR_RD_REQ interrupts will send
I2C_SLAVE_READ_PROCESSED events and transmit next bytes from backend
- STOP won't change the state machine. Otherwise case 2 won't work since
we cannot distinguish current address read from sequentiel read
- DW_IC_INTR_RX_DONE interrupt is needless since there is no mechanism
to inform it to a backend. It cannot be used to change state machine
at the end of read either due the same reason than above
- Next host write to us will change the state machine from read to write
in progress state
- STATUS_WRITE_IN_PROGRESS and STATUS_READ_IN_PROGRESS are considered
now to be status flags not the state of the driver. This is how we
treat them in i2c-designware-master.c
While at it do not test the return code from i2c_slave_event() for
I2C_SLAVE_READ_REQUESTED and I2C_SLAVE_READ_PROCESSED since it returns
always 0.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/i2c/busses/i2c-designware-core.h | 1 -
drivers/i2c/busses/i2c-designware-slave.c | 32 +++++++++++------------
2 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 4d3a3b464ecd..dbf6bdc5f01b 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -103,7 +103,6 @@
#define DW_IC_INTR_MASTER_MASK (DW_IC_INTR_DEFAULT_MASK | \
DW_IC_INTR_TX_EMPTY)
#define DW_IC_INTR_SLAVE_MASK (DW_IC_INTR_DEFAULT_MASK | \
- DW_IC_INTR_RX_DONE | \
DW_IC_INTR_RX_UNDER | \
DW_IC_INTR_RD_REQ)
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index 0d15f4c1e9f7..1eac4f4d5573 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -173,8 +173,9 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
enabled, slave_activity, raw_stat, stat);
if (stat & DW_IC_INTR_RX_FULL) {
- if (dev->status != STATUS_WRITE_IN_PROGRESS) {
- dev->status = STATUS_WRITE_IN_PROGRESS;
+ if (!(dev->status & STATUS_WRITE_IN_PROGRESS)) {
+ dev->status |= STATUS_WRITE_IN_PROGRESS;
+ dev->status &= ~STATUS_READ_IN_PROGRESS;
i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED,
&val);
}
@@ -190,24 +191,23 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
if (slave_activity) {
regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
- dev->status = STATUS_READ_IN_PROGRESS;
- if (!i2c_slave_event(dev->slave,
- I2C_SLAVE_READ_REQUESTED,
- &val))
- regmap_write(dev->map, DW_IC_DATA_CMD, val);
+ if (!(dev->status & STATUS_READ_IN_PROGRESS)) {
+ i2c_slave_event(dev->slave,
+ I2C_SLAVE_READ_REQUESTED,
+ &val);
+ dev->status |= STATUS_READ_IN_PROGRESS;
+ dev->status &= ~STATUS_WRITE_IN_PROGRESS;
+ } else {
+ i2c_slave_event(dev->slave,
+ I2C_SLAVE_READ_PROCESSED,
+ &val);
+ }
+ regmap_write(dev->map, DW_IC_DATA_CMD, val);
}
}
- if (stat & DW_IC_INTR_RX_DONE) {
- if (!i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED,
- &val))
- regmap_read(dev->map, DW_IC_CLR_RX_DONE, &tmp);
- }
-
- if (stat & DW_IC_INTR_STOP_DET) {
- dev->status = STATUS_IDLE;
+ if (stat & DW_IC_INTR_STOP_DET)
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
- }
return 1;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 02/12] i2c: designware: Empty receive FIFO in slave interrupt handler
2022-11-07 13:42 [PATCH v3 00/12] i2c: designware: Slave fixes and generic cleanups Jarkko Nikula
2022-11-07 13:42 ` [PATCH v3 01/12] i2c: designware: Fix slave state machine for sequential reads Jarkko Nikula
@ 2022-11-07 13:42 ` Jarkko Nikula
2022-11-12 6:57 ` Wolfram Sang
2022-11-07 13:42 ` [PATCH v3 03/12] i2c: designware: Define software status flags with BIT() Jarkko Nikula
` (10 subsequent siblings)
12 siblings, 1 reply; 16+ messages in thread
From: Jarkko Nikula @ 2022-11-07 13:42 UTC (permalink / raw)
To: linux-i2c
Cc: Wolfram Sang, Andy Shevchenko, Mika Westerberg, Jan Dabros,
Michael Wu, Tian Ye, Jarkko Nikula
Writes from I2C bus often fail when testing the i2c-designware-slave.c
with the slave-eeprom backend. The same writes work correctly when
testing with a real 24c02 EEPROM chip.
In the tests below an i2c-designware-slave.c instance with the
slave-eeprom backend is configured to act as a simulated 24c02 at
address 0x65 on an I2C host bus 6.
1. i2cset -y 6 0x65 0x00 0x55
Single byte 0x55 write into address 0x00. No data goes into simulated
EEPROM. Debug prints from the i2c_dw_irq_handler_slave():
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x714 : INTR_STAT=0x204
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
2. i2ctransfer -y 6 w9@0x65 0x00 0xff-
Write 8 bytes with decrementing value starting from 0xff at address 0x00
and forward. Only some of the data goes into arbitrary addresses.
Content is something like below but varies:
00000000 f9 f8 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00000050 00 00 00 00 00 00 ff fe 00 00 00 00 00 00 00 00 |................|
000000f0 00 00 00 00 00 00 00 00 00 00 00 00 00 fc fb fa |................|
In this case debug prints were:
0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x714 : INTR_STAT=0x204
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x510 : INTR_STAT=0x0
Both cases show there is more data coming from the receive FIFO still
after detecting the STOP condition. This can be seen from interrupt
status bits DW_IC_INTR_STOP_DET (0x200) and DW_IC_INTR_RX_FULL (0x4).
Perhaps due interrupt latencies the receive FIFO is not read fast
enough, STOP detection happens synchronously when it occurs on the I2C
bus and the DW_IC_INTR_RX_FULL keeps coming as long as there are more
bytes in the receive FIFO.
Fix this by reading the receive FIFO completely empty whenever
DW_IC_INTR_RX_FULL occurs. Use RFNE, Receive FIFO Not Empty bit in the
DW_IC_STATUS register to loop through bytes in the FIFO.
While at it do not test the return code from i2c_slave_event() for the
I2C_SLAVE_WRITE_RECEIVED since to my understanding this hardware cannot
generate NACK to incoming bytes and debug print itself does not have
much value.
Reported-by: Tian Ye <tianye@sugon.com>
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Hi Tian Ye. I've been testing the i2c-designware-slave.c recently and
discovered these write issues. Your recent patch gave an idea what might
cause them. In my solution I went testing DW_IC_STATUS_RFNE since
according to datasheet it's equivalent to RX_FULL interrupt in case
interrupts are masked. Seems to work here too.
Does this fix the issue you were seeing?
---
drivers/i2c/busses/i2c-designware-core.h | 1 +
drivers/i2c/busses/i2c-designware-slave.c | 12 +++++++-----
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index dbf6bdc5f01b..6d1df28dd93b 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -108,6 +108,7 @@
#define DW_IC_STATUS_ACTIVITY BIT(0)
#define DW_IC_STATUS_TFE BIT(2)
+#define DW_IC_STATUS_RFNE BIT(3)
#define DW_IC_STATUS_MASTER_ACTIVITY BIT(5)
#define DW_IC_STATUS_SLAVE_ACTIVITY BIT(6)
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index 1eac4f4d5573..295774a69b67 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -180,11 +180,13 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
&val);
}
- regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
- val = tmp;
- if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
- &val))
- dev_vdbg(dev->dev, "Byte %X acked!", val);
+ do {
+ regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
+ val = tmp;
+ i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
+ &val);
+ regmap_read(dev->map, DW_IC_STATUS, &tmp);
+ } while (tmp & DW_IC_STATUS_RFNE);
}
if (stat & DW_IC_INTR_RD_REQ) {
--
2.35.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 02/12] i2c: designware: Empty receive FIFO in slave interrupt handler
2022-11-07 13:42 ` [PATCH v3 02/12] i2c: designware: Empty receive FIFO in slave interrupt handler Jarkko Nikula
@ 2022-11-12 6:57 ` Wolfram Sang
2022-11-14 9:53 ` Jarkko Nikula
0 siblings, 1 reply; 16+ messages in thread
From: Wolfram Sang @ 2022-11-12 6:57 UTC (permalink / raw)
To: Jarkko Nikula
Cc: linux-i2c, Andy Shevchenko, Mika Westerberg, Jan Dabros,
Michael Wu, Tian Ye
[-- Attachment #1: Type: text/plain, Size: 417 bytes --]
> 2. i2ctransfer -y 6 w9@0x65 0x00 0xff-
Always nice to see 'i2ctransfer' in action!
> While at it do not test the return code from i2c_slave_event() for the
> I2C_SLAVE_WRITE_RECEIVED since to my understanding this hardware cannot
> generate NACK to incoming bytes
Not even on the last byte? That would be bad. If a backend encounters a
problem, there is no way then to communicate that back to the
controller.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 02/12] i2c: designware: Empty receive FIFO in slave interrupt handler
2022-11-12 6:57 ` Wolfram Sang
@ 2022-11-14 9:53 ` Jarkko Nikula
0 siblings, 0 replies; 16+ messages in thread
From: Jarkko Nikula @ 2022-11-14 9:53 UTC (permalink / raw)
To: Wolfram Sang, linux-i2c, Andy Shevchenko, Mika Westerberg,
Jan Dabros, Michael Wu, Tian Ye
On 11/12/22 08:57, Wolfram Sang wrote:
>
>> 2. i2ctransfer -y 6 w9@0x65 0x00 0xff-
>
> Always nice to see 'i2ctransfer' in action!
>
>> While at it do not test the return code from i2c_slave_event() for the
>> I2C_SLAVE_WRITE_RECEIVED since to my understanding this hardware cannot
>> generate NACK to incoming bytes
>
> Not even on the last byte? That would be bad. If a backend encounters a
> problem, there is no way then to communicate that back to the
> controller.
>
Yeah, that's how I understood it. HW has an optional register (I didn't
check is it enabled on our HW) that enable generation of NACK for the
incoming data byte but it can be enabled only when the controller is
disabled and slave is inactive so it doesn't look very useful.
Jarkko
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 03/12] i2c: designware: Define software status flags with BIT()
2022-11-07 13:42 [PATCH v3 00/12] i2c: designware: Slave fixes and generic cleanups Jarkko Nikula
2022-11-07 13:42 ` [PATCH v3 01/12] i2c: designware: Fix slave state machine for sequential reads Jarkko Nikula
2022-11-07 13:42 ` [PATCH v3 02/12] i2c: designware: Empty receive FIFO in slave interrupt handler Jarkko Nikula
@ 2022-11-07 13:42 ` Jarkko Nikula
2022-11-07 13:42 ` [PATCH v3 04/12] i2c: designware: Remove needless initializations from i2c_dw_reg_slave() Jarkko Nikula
` (9 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Jarkko Nikula @ 2022-11-07 13:42 UTC (permalink / raw)
To: linux-i2c
Cc: Wolfram Sang, Andy Shevchenko, Mika Westerberg, Jan Dabros,
Michael Wu, Tian Ye, Jarkko Nikula
Define software status flags with a BIT() macro. While at it remove
STATUS_IDLE and replace its use with zero initialization and status
flags clearing with a mask.
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/i2c/busses/i2c-designware-core.h | 10 +++++-----
drivers/i2c/busses/i2c-designware-master.c | 4 ++--
drivers/i2c/busses/i2c-designware-slave.c | 2 +-
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 6d1df28dd93b..457e6966f85e 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -123,12 +123,12 @@
#define DW_IC_COMP_PARAM_1_SPEED_MODE_MASK GENMASK(3, 2)
/*
- * status codes
+ * Sofware status flags
*/
-#define STATUS_IDLE 0x0
-#define STATUS_ACTIVE 0x1
-#define STATUS_WRITE_IN_PROGRESS 0x2
-#define STATUS_READ_IN_PROGRESS 0x4
+#define STATUS_ACTIVE BIT(0)
+#define STATUS_WRITE_IN_PROGRESS BIT(1)
+#define STATUS_READ_IN_PROGRESS BIT(2)
+#define STATUS_MASK GENMASK(2, 0)
/*
* operation modes
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index dc3c5a15a95b..1b7db2b58f31 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -574,7 +574,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
dev->msg_write_idx = 0;
dev->msg_read_idx = 0;
dev->msg_err = 0;
- dev->status = STATUS_IDLE;
+ dev->status = 0;
dev->abort_source = 0;
dev->rx_outstanding = 0;
@@ -731,7 +731,7 @@ static int i2c_dw_irq_handler_master(struct dw_i2c_dev *dev)
if (stat & DW_IC_INTR_TX_ABRT) {
dev->cmd_err |= DW_IC_ERR_TX_ABRT;
- dev->status = STATUS_IDLE;
+ dev->status &= ~STATUS_MASK;
dev->rx_outstanding = 0;
/*
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index 295774a69b67..84eb0bec70fa 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -82,7 +82,7 @@ static int i2c_dw_reg_slave(struct i2c_client *slave)
dev->msg_write_idx = 0;
dev->msg_read_idx = 0;
dev->msg_err = 0;
- dev->status = STATUS_IDLE;
+ dev->status = 0;
dev->abort_source = 0;
dev->rx_outstanding = 0;
--
2.35.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 04/12] i2c: designware: Remove needless initializations from i2c_dw_reg_slave()
2022-11-07 13:42 [PATCH v3 00/12] i2c: designware: Slave fixes and generic cleanups Jarkko Nikula
` (2 preceding siblings ...)
2022-11-07 13:42 ` [PATCH v3 03/12] i2c: designware: Define software status flags with BIT() Jarkko Nikula
@ 2022-11-07 13:42 ` Jarkko Nikula
2022-11-07 13:42 ` [PATCH v3 05/12] i2c: designware: Remove unused completion code from i2c-designware-slave Jarkko Nikula
` (8 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Jarkko Nikula @ 2022-11-07 13:42 UTC (permalink / raw)
To: linux-i2c
Cc: Wolfram Sang, Andy Shevchenko, Mika Westerberg, Jan Dabros,
Michael Wu, Tian Ye, Jarkko Nikula
These struct dw_i2c_dev members are not used in i2c-designware-slave.c
so remove re-initialization of them from i2c_dw_reg_slave().
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/i2c/busses/i2c-designware-slave.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index 84eb0bec70fa..421a604bf68f 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -78,13 +78,7 @@ static int i2c_dw_reg_slave(struct i2c_client *slave)
__i2c_dw_enable(dev);
- dev->cmd_err = 0;
- dev->msg_write_idx = 0;
- dev->msg_read_idx = 0;
- dev->msg_err = 0;
dev->status = 0;
- dev->abort_source = 0;
- dev->rx_outstanding = 0;
return 0;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 05/12] i2c: designware: Remove unused completion code from i2c-designware-slave
2022-11-07 13:42 [PATCH v3 00/12] i2c: designware: Slave fixes and generic cleanups Jarkko Nikula
` (3 preceding siblings ...)
2022-11-07 13:42 ` [PATCH v3 04/12] i2c: designware: Remove needless initializations from i2c_dw_reg_slave() Jarkko Nikula
@ 2022-11-07 13:42 ` Jarkko Nikula
2022-11-07 13:42 ` [PATCH v3 06/12] i2c: designware: Simplify slave interrupt handler nesting Jarkko Nikula
` (7 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Jarkko Nikula @ 2022-11-07 13:42 UTC (permalink / raw)
To: linux-i2c
Cc: Wolfram Sang, Andy Shevchenko, Mika Westerberg, Jan Dabros,
Michael Wu, Tian Ye, Jarkko Nikula
Remove unused completion code from i2c-designware-slave.c. Used only in
i2c-designware-master.c.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/i2c/busses/i2c-designware-slave.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index 421a604bf68f..12f0417aa0ae 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -214,8 +214,6 @@ static irqreturn_t i2c_dw_isr_slave(int this_irq, void *dev_id)
int ret;
ret = i2c_dw_irq_handler_slave(dev);
- if (ret > 0)
- complete(&dev->cmd_complete);
return IRQ_RETVAL(ret);
}
@@ -242,8 +240,6 @@ int i2c_dw_probe_slave(struct dw_i2c_dev *dev)
struct i2c_adapter *adap = &dev->adapter;
int ret;
- init_completion(&dev->cmd_complete);
-
dev->init = i2c_dw_init_slave;
dev->disable = i2c_dw_disable;
dev->disable_int = i2c_dw_disable_int;
--
2.35.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 06/12] i2c: designware: Simplify slave interrupt handler nesting
2022-11-07 13:42 [PATCH v3 00/12] i2c: designware: Slave fixes and generic cleanups Jarkko Nikula
` (4 preceding siblings ...)
2022-11-07 13:42 ` [PATCH v3 05/12] i2c: designware: Remove unused completion code from i2c-designware-slave Jarkko Nikula
@ 2022-11-07 13:42 ` Jarkko Nikula
2022-11-07 13:42 ` [PATCH v3 07/12] i2c: designware: Do not process interrupt when device is suspended Jarkko Nikula
` (6 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Jarkko Nikula @ 2022-11-07 13:42 UTC (permalink / raw)
To: linux-i2c
Cc: Wolfram Sang, Andy Shevchenko, Mika Westerberg, Jan Dabros,
Michael Wu, Tian Ye, Jarkko Nikula
Interrupt processing code in i2c-designware-slave.c is bit more readable
if not divided into another subroutine. Also explicit IRQ_NONE and
IRQ_HANDLED return values are more obvious.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/i2c/busses/i2c-designware-slave.c | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index 12f0417aa0ae..3c855cd45c34 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -147,9 +147,9 @@ static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
* Interrupt service routine. This gets called whenever an I2C slave interrupt
* occurs.
*/
-
-static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
+static irqreturn_t i2c_dw_isr_slave(int this_irq, void *dev_id)
{
+ struct dw_i2c_dev *dev = dev_id;
u32 raw_stat, stat, enabled, tmp;
u8 val = 0, slave_activity;
@@ -159,7 +159,7 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
slave_activity = ((tmp & DW_IC_STATUS_SLAVE_ACTIVITY) >> 6);
if (!enabled || !(raw_stat & ~DW_IC_INTR_ACTIVITY) || !dev->slave)
- return 0;
+ return IRQ_NONE;
stat = i2c_dw_read_clear_intrbits_slave(dev);
dev_dbg(dev->dev,
@@ -205,17 +205,7 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
if (stat & DW_IC_INTR_STOP_DET)
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
- return 1;
-}
-
-static irqreturn_t i2c_dw_isr_slave(int this_irq, void *dev_id)
-{
- struct dw_i2c_dev *dev = dev_id;
- int ret;
-
- ret = i2c_dw_irq_handler_slave(dev);
-
- return IRQ_RETVAL(ret);
+ return IRQ_HANDLED;
}
static const struct i2c_algorithm i2c_dw_algo = {
--
2.35.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 07/12] i2c: designware: Do not process interrupt when device is suspended
2022-11-07 13:42 [PATCH v3 00/12] i2c: designware: Slave fixes and generic cleanups Jarkko Nikula
` (5 preceding siblings ...)
2022-11-07 13:42 ` [PATCH v3 06/12] i2c: designware: Simplify slave interrupt handler nesting Jarkko Nikula
@ 2022-11-07 13:42 ` Jarkko Nikula
2022-11-07 13:42 ` [PATCH v3 08/12] i2c: designware: Move debug print in i2c_dw_isr() Jarkko Nikula
` (5 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Jarkko Nikula @ 2022-11-07 13:42 UTC (permalink / raw)
To: linux-i2c
Cc: Wolfram Sang, Andy Shevchenko, Mika Westerberg, Jan Dabros,
Michael Wu, Tian Ye, Jarkko Nikula
Do not return with interrupt handled if host controller is off and thus
interrupt is originating from other device or is spurious.
Add a check to detect when controller is runtime suspended or
transitioning/reset. In latter case all raw interrupt status register
bits may read one. In both cases return IRQ_NONE to indicate interrupt
was not from this device.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: Use GENMASK(31, 0) instead of ~0.
---
drivers/i2c/busses/i2c-designware-master.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 1b7db2b58f31..d6fcf955dfc0 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -778,6 +778,8 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
dev_dbg(dev->dev, "enabled=%#x stat=%#x\n", enabled, stat);
if (!enabled || !(stat & ~DW_IC_INTR_ACTIVITY))
return IRQ_NONE;
+ if (pm_runtime_suspended(dev->dev) || stat == GENMASK(31, 0))
+ return IRQ_NONE;
i2c_dw_irq_handler_master(dev);
--
2.35.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 08/12] i2c: designware: Move debug print in i2c_dw_isr()
2022-11-07 13:42 [PATCH v3 00/12] i2c: designware: Slave fixes and generic cleanups Jarkko Nikula
` (6 preceding siblings ...)
2022-11-07 13:42 ` [PATCH v3 07/12] i2c: designware: Do not process interrupt when device is suspended Jarkko Nikula
@ 2022-11-07 13:42 ` Jarkko Nikula
2022-11-07 13:42 ` [PATCH v3 09/12] i2c: designware: Simplify master interrupt handler nesting Jarkko Nikula
` (4 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Jarkko Nikula @ 2022-11-07 13:42 UTC (permalink / raw)
To: linux-i2c
Cc: Wolfram Sang, Andy Shevchenko, Mika Westerberg, Jan Dabros,
Michael Wu, Tian Ye, Jarkko Nikula
It is kind of needless to print interrupt status when code immediately
after that finds interrupt was not originating from this device.
Therefore move it after spurious interrupt detection.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/i2c/busses/i2c-designware-master.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index d6fcf955dfc0..9c2c9d002dc3 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -775,11 +775,11 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
regmap_read(dev->map, DW_IC_ENABLE, &enabled);
regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &stat);
- dev_dbg(dev->dev, "enabled=%#x stat=%#x\n", enabled, stat);
if (!enabled || !(stat & ~DW_IC_INTR_ACTIVITY))
return IRQ_NONE;
if (pm_runtime_suspended(dev->dev) || stat == GENMASK(31, 0))
return IRQ_NONE;
+ dev_dbg(dev->dev, "enabled=%#x stat=%#x\n", enabled, stat);
i2c_dw_irq_handler_master(dev);
--
2.35.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 09/12] i2c: designware: Simplify master interrupt handler nesting
2022-11-07 13:42 [PATCH v3 00/12] i2c: designware: Slave fixes and generic cleanups Jarkko Nikula
` (7 preceding siblings ...)
2022-11-07 13:42 ` [PATCH v3 08/12] i2c: designware: Move debug print in i2c_dw_isr() Jarkko Nikula
@ 2022-11-07 13:42 ` Jarkko Nikula
2022-11-07 13:42 ` [PATCH v3 10/12] i2c: designware: Remove common i2c_dw_disable_int() Jarkko Nikula
` (3 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Jarkko Nikula @ 2022-11-07 13:42 UTC (permalink / raw)
To: linux-i2c
Cc: Wolfram Sang, Andy Shevchenko, Mika Westerberg, Jan Dabros,
Michael Wu, Tian Ye, Jarkko Nikula
In my opinion a few lines of spurious interrupt detection code can be
moved to the actual master interrupt handling function i2c_dw_isr()
without hurting readability.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: s/opinnion/opinion/. Thanks Andy for noticing.
---
drivers/i2c/busses/i2c-designware-master.c | 33 ++++++++--------------
1 file changed, 12 insertions(+), 21 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 9c2c9d002dc3..dfb499e54c05 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -711,9 +711,18 @@ static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
* Interrupt service routine. This gets called whenever an I2C master interrupt
* occurs.
*/
-static int i2c_dw_irq_handler_master(struct dw_i2c_dev *dev)
+static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
{
- u32 stat;
+ struct dw_i2c_dev *dev = dev_id;
+ u32 stat, enabled;
+
+ regmap_read(dev->map, DW_IC_ENABLE, &enabled);
+ regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &stat);
+ if (!enabled || !(stat & ~DW_IC_INTR_ACTIVITY))
+ return IRQ_NONE;
+ if (pm_runtime_suspended(dev->dev) || stat == GENMASK(31, 0))
+ return IRQ_NONE;
+ dev_dbg(dev->dev, "enabled=%#x stat=%#x\n", enabled, stat);
stat = i2c_dw_read_clear_intrbits(dev);
@@ -726,7 +735,7 @@ static int i2c_dw_irq_handler_master(struct dw_i2c_dev *dev)
* the HW active).
*/
regmap_write(dev->map, DW_IC_INTR_MASK, 0);
- return 0;
+ return IRQ_HANDLED;
}
if (stat & DW_IC_INTR_TX_ABRT) {
@@ -765,24 +774,6 @@ static int i2c_dw_irq_handler_master(struct dw_i2c_dev *dev)
regmap_write(dev->map, DW_IC_INTR_MASK, stat);
}
- return 0;
-}
-
-static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
-{
- struct dw_i2c_dev *dev = dev_id;
- u32 stat, enabled;
-
- regmap_read(dev->map, DW_IC_ENABLE, &enabled);
- regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &stat);
- if (!enabled || !(stat & ~DW_IC_INTR_ACTIVITY))
- return IRQ_NONE;
- if (pm_runtime_suspended(dev->dev) || stat == GENMASK(31, 0))
- return IRQ_NONE;
- dev_dbg(dev->dev, "enabled=%#x stat=%#x\n", enabled, stat);
-
- i2c_dw_irq_handler_master(dev);
-
return IRQ_HANDLED;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 10/12] i2c: designware: Remove common i2c_dw_disable_int()
2022-11-07 13:42 [PATCH v3 00/12] i2c: designware: Slave fixes and generic cleanups Jarkko Nikula
` (8 preceding siblings ...)
2022-11-07 13:42 ` [PATCH v3 09/12] i2c: designware: Simplify master interrupt handler nesting Jarkko Nikula
@ 2022-11-07 13:42 ` Jarkko Nikula
2022-11-07 13:42 ` [PATCH v3 11/12] i2c: designware: Align defines in i2c-designware-core.h Jarkko Nikula
` (2 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Jarkko Nikula @ 2022-11-07 13:42 UTC (permalink / raw)
To: linux-i2c
Cc: Wolfram Sang, Andy Shevchenko, Mika Westerberg, Jan Dabros,
Michael Wu, Tian Ye, Jarkko Nikula
Commit 90312351fd1e ("i2c: designware: MASTER mode as separated driver")
introduced disable_int pointer but there is no real use for it. Both
i2c-designware-master.c and i2c-designware-slave.c set it to the same
i2c_dw_disable_int() and scope is inside the same kernel module.
Since i2c_dw_disable_int() is just masking interrupts and the direct
DW_IC_INTR_MASK register write looks more clear in the code use that and
remove it from common code.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: Remove disable_int kerneldoc comment, i2c_dw_disable_int() forward
declaration and update commit log.
---
drivers/i2c/busses/i2c-designware-common.c | 5 -----
drivers/i2c/busses/i2c-designware-core.h | 3 ---
drivers/i2c/busses/i2c-designware-master.c | 9 ++++-----
drivers/i2c/busses/i2c-designware-slave.c | 3 +--
4 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index c023b691441e..a3240ece55b2 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -625,10 +625,5 @@ void i2c_dw_disable(struct dw_i2c_dev *dev)
i2c_dw_release_lock(dev);
}
-void i2c_dw_disable_int(struct dw_i2c_dev *dev)
-{
- regmap_write(dev->map, DW_IC_INTR_MASK, 0);
-}
-
MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter core");
MODULE_LICENSE("GPL");
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 457e6966f85e..49e5860b1665 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -232,7 +232,6 @@ struct reset_control;
* -1 if there is no semaphore.
* @shared_with_punit: true if this bus is shared with the SoCs PUNIT
* @disable: function to disable the controller
- * @disable_int: function to disable all interrupts
* @init: function to initialize the I2C hardware
* @set_sda_hold_time: callback to retrieve IP specific SDA hold timing
* @mode: operation mode - DW_IC_MASTER or DW_IC_SLAVE
@@ -290,7 +289,6 @@ struct dw_i2c_dev {
int semaphore_idx;
bool shared_with_punit;
void (*disable)(struct dw_i2c_dev *dev);
- void (*disable_int)(struct dw_i2c_dev *dev);
int (*init)(struct dw_i2c_dev *dev);
int (*set_sda_hold_time)(struct dw_i2c_dev *dev);
int mode;
@@ -331,7 +329,6 @@ int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev);
int i2c_dw_set_fifo_size(struct dw_i2c_dev *dev);
u32 i2c_dw_func(struct i2c_adapter *adap);
void i2c_dw_disable(struct dw_i2c_dev *dev);
-void i2c_dw_disable_int(struct dw_i2c_dev *dev);
static inline void __i2c_dw_enable(struct dw_i2c_dev *dev)
{
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index dfb499e54c05..45f569155bfe 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -239,7 +239,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
msgs[dev->msg_write_idx].addr | ic_tar);
/* Enforce disabled interrupts (due to HW issues) */
- i2c_dw_disable_int(dev);
+ regmap_write(dev->map, DW_IC_INTR_MASK, 0);
/* Enable the adapter */
__i2c_dw_enable(dev);
@@ -299,7 +299,7 @@ static int amd_i2c_dw_xfer_quirk(struct i2c_adapter *adap, struct i2c_msg *msgs,
dev->msgs = msgs;
dev->msgs_num = num_msgs;
i2c_dw_xfer_init(dev);
- i2c_dw_disable_int(dev);
+ regmap_write(dev->map, DW_IC_INTR_MASK, 0);
/* Initiate messages read/write transaction */
for (msg_wrt_idx = 0; msg_wrt_idx < num_msgs; msg_wrt_idx++) {
@@ -770,7 +770,7 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
else if (unlikely(dev->flags & ACCESS_INTR_MASK)) {
/* Workaround to trigger pending interrupt */
regmap_read(dev->map, DW_IC_INTR_MASK, &stat);
- i2c_dw_disable_int(dev);
+ regmap_write(dev->map, DW_IC_INTR_MASK, 0);
regmap_write(dev->map, DW_IC_INTR_MASK, stat);
}
@@ -871,7 +871,6 @@ int i2c_dw_probe_master(struct dw_i2c_dev *dev)
dev->init = i2c_dw_init_master;
dev->disable = i2c_dw_disable;
- dev->disable_int = i2c_dw_disable_int;
ret = i2c_dw_init_regmap(dev);
if (ret)
@@ -910,7 +909,7 @@ int i2c_dw_probe_master(struct dw_i2c_dev *dev)
if (ret)
return ret;
- i2c_dw_disable_int(dev);
+ regmap_write(dev->map, DW_IC_INTR_MASK, 0);
i2c_dw_release_lock(dev);
ret = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr, irq_flags,
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index 3c855cd45c34..c6d2e4c2ac23 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -87,7 +87,7 @@ static int i2c_dw_unreg_slave(struct i2c_client *slave)
{
struct dw_i2c_dev *dev = i2c_get_adapdata(slave->adapter);
- dev->disable_int(dev);
+ regmap_write(dev->map, DW_IC_INTR_MASK, 0);
dev->disable(dev);
synchronize_irq(dev->irq);
dev->slave = NULL;
@@ -232,7 +232,6 @@ int i2c_dw_probe_slave(struct dw_i2c_dev *dev)
dev->init = i2c_dw_init_slave;
dev->disable = i2c_dw_disable;
- dev->disable_int = i2c_dw_disable_int;
ret = i2c_dw_init_regmap(dev);
if (ret)
--
2.35.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 11/12] i2c: designware: Align defines in i2c-designware-core.h
2022-11-07 13:42 [PATCH v3 00/12] i2c: designware: Slave fixes and generic cleanups Jarkko Nikula
` (9 preceding siblings ...)
2022-11-07 13:42 ` [PATCH v3 10/12] i2c: designware: Remove common i2c_dw_disable_int() Jarkko Nikula
@ 2022-11-07 13:42 ` Jarkko Nikula
2022-11-07 13:42 ` [PATCH v3 12/12] i2c: designware: Add comment to custom register value constants Jarkko Nikula
2022-11-12 6:57 ` [PATCH v3 00/12] i2c: designware: Slave fixes and generic cleanups Wolfram Sang
12 siblings, 0 replies; 16+ messages in thread
From: Jarkko Nikula @ 2022-11-07 13:42 UTC (permalink / raw)
To: linux-i2c
Cc: Wolfram Sang, Andy Shevchenko, Mika Westerberg, Jan Dabros,
Michael Wu, Tian Ye, Jarkko Nikula
Align all defines to the same column.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: Remove accidental "struct i2c_bus_recovery_info rinfo" align that
was done even wrongly.
---
drivers/i2c/busses/i2c-designware-core.h | 230 +++++++++++------------
1 file changed, 115 insertions(+), 115 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 49e5860b1665..0668888d557d 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -18,12 +18,12 @@
#include <linux/regmap.h>
#include <linux/types.h>
-#define DW_IC_DEFAULT_FUNCTIONALITY (I2C_FUNC_I2C | \
- I2C_FUNC_SMBUS_BYTE | \
- I2C_FUNC_SMBUS_BYTE_DATA | \
- I2C_FUNC_SMBUS_WORD_DATA | \
- I2C_FUNC_SMBUS_BLOCK_DATA | \
- I2C_FUNC_SMBUS_I2C_BLOCK)
+#define DW_IC_DEFAULT_FUNCTIONALITY (I2C_FUNC_I2C | \
+ I2C_FUNC_SMBUS_BYTE | \
+ I2C_FUNC_SMBUS_BYTE_DATA | \
+ I2C_FUNC_SMBUS_WORD_DATA | \
+ I2C_FUNC_SMBUS_BLOCK_DATA | \
+ I2C_FUNC_SMBUS_I2C_BLOCK)
#define DW_IC_CON_MASTER BIT(0)
#define DW_IC_CON_SPEED_STD (1 << 1)
@@ -43,81 +43,81 @@
/*
* Registers offset
*/
-#define DW_IC_CON 0x00
-#define DW_IC_TAR 0x04
-#define DW_IC_SAR 0x08
-#define DW_IC_DATA_CMD 0x10
-#define DW_IC_SS_SCL_HCNT 0x14
-#define DW_IC_SS_SCL_LCNT 0x18
-#define DW_IC_FS_SCL_HCNT 0x1c
-#define DW_IC_FS_SCL_LCNT 0x20
-#define DW_IC_HS_SCL_HCNT 0x24
-#define DW_IC_HS_SCL_LCNT 0x28
-#define DW_IC_INTR_STAT 0x2c
-#define DW_IC_INTR_MASK 0x30
-#define DW_IC_RAW_INTR_STAT 0x34
-#define DW_IC_RX_TL 0x38
-#define DW_IC_TX_TL 0x3c
-#define DW_IC_CLR_INTR 0x40
-#define DW_IC_CLR_RX_UNDER 0x44
-#define DW_IC_CLR_RX_OVER 0x48
-#define DW_IC_CLR_TX_OVER 0x4c
-#define DW_IC_CLR_RD_REQ 0x50
-#define DW_IC_CLR_TX_ABRT 0x54
-#define DW_IC_CLR_RX_DONE 0x58
-#define DW_IC_CLR_ACTIVITY 0x5c
-#define DW_IC_CLR_STOP_DET 0x60
-#define DW_IC_CLR_START_DET 0x64
-#define DW_IC_CLR_GEN_CALL 0x68
-#define DW_IC_ENABLE 0x6c
-#define DW_IC_STATUS 0x70
-#define DW_IC_TXFLR 0x74
-#define DW_IC_RXFLR 0x78
-#define DW_IC_SDA_HOLD 0x7c
-#define DW_IC_TX_ABRT_SOURCE 0x80
-#define DW_IC_ENABLE_STATUS 0x9c
-#define DW_IC_CLR_RESTART_DET 0xa8
-#define DW_IC_COMP_PARAM_1 0xf4
-#define DW_IC_COMP_VERSION 0xf8
-#define DW_IC_SDA_HOLD_MIN_VERS 0x3131312A
-#define DW_IC_COMP_TYPE 0xfc
-#define DW_IC_COMP_TYPE_VALUE 0x44570140
-
-#define DW_IC_INTR_RX_UNDER BIT(0)
-#define DW_IC_INTR_RX_OVER BIT(1)
-#define DW_IC_INTR_RX_FULL BIT(2)
-#define DW_IC_INTR_TX_OVER BIT(3)
-#define DW_IC_INTR_TX_EMPTY BIT(4)
-#define DW_IC_INTR_RD_REQ BIT(5)
-#define DW_IC_INTR_TX_ABRT BIT(6)
-#define DW_IC_INTR_RX_DONE BIT(7)
-#define DW_IC_INTR_ACTIVITY BIT(8)
-#define DW_IC_INTR_STOP_DET BIT(9)
-#define DW_IC_INTR_START_DET BIT(10)
-#define DW_IC_INTR_GEN_CALL BIT(11)
-#define DW_IC_INTR_RESTART_DET BIT(12)
-
-#define DW_IC_INTR_DEFAULT_MASK (DW_IC_INTR_RX_FULL | \
- DW_IC_INTR_TX_ABRT | \
- DW_IC_INTR_STOP_DET)
-#define DW_IC_INTR_MASTER_MASK (DW_IC_INTR_DEFAULT_MASK | \
- DW_IC_INTR_TX_EMPTY)
-#define DW_IC_INTR_SLAVE_MASK (DW_IC_INTR_DEFAULT_MASK | \
- DW_IC_INTR_RX_UNDER | \
- DW_IC_INTR_RD_REQ)
-
-#define DW_IC_STATUS_ACTIVITY BIT(0)
-#define DW_IC_STATUS_TFE BIT(2)
-#define DW_IC_STATUS_RFNE BIT(3)
-#define DW_IC_STATUS_MASTER_ACTIVITY BIT(5)
-#define DW_IC_STATUS_SLAVE_ACTIVITY BIT(6)
-
-#define DW_IC_SDA_HOLD_RX_SHIFT 16
-#define DW_IC_SDA_HOLD_RX_MASK GENMASK(23, 16)
-
-#define DW_IC_ERR_TX_ABRT 0x1
-
-#define DW_IC_TAR_10BITADDR_MASTER BIT(12)
+#define DW_IC_CON 0x00
+#define DW_IC_TAR 0x04
+#define DW_IC_SAR 0x08
+#define DW_IC_DATA_CMD 0x10
+#define DW_IC_SS_SCL_HCNT 0x14
+#define DW_IC_SS_SCL_LCNT 0x18
+#define DW_IC_FS_SCL_HCNT 0x1c
+#define DW_IC_FS_SCL_LCNT 0x20
+#define DW_IC_HS_SCL_HCNT 0x24
+#define DW_IC_HS_SCL_LCNT 0x28
+#define DW_IC_INTR_STAT 0x2c
+#define DW_IC_INTR_MASK 0x30
+#define DW_IC_RAW_INTR_STAT 0x34
+#define DW_IC_RX_TL 0x38
+#define DW_IC_TX_TL 0x3c
+#define DW_IC_CLR_INTR 0x40
+#define DW_IC_CLR_RX_UNDER 0x44
+#define DW_IC_CLR_RX_OVER 0x48
+#define DW_IC_CLR_TX_OVER 0x4c
+#define DW_IC_CLR_RD_REQ 0x50
+#define DW_IC_CLR_TX_ABRT 0x54
+#define DW_IC_CLR_RX_DONE 0x58
+#define DW_IC_CLR_ACTIVITY 0x5c
+#define DW_IC_CLR_STOP_DET 0x60
+#define DW_IC_CLR_START_DET 0x64
+#define DW_IC_CLR_GEN_CALL 0x68
+#define DW_IC_ENABLE 0x6c
+#define DW_IC_STATUS 0x70
+#define DW_IC_TXFLR 0x74
+#define DW_IC_RXFLR 0x78
+#define DW_IC_SDA_HOLD 0x7c
+#define DW_IC_TX_ABRT_SOURCE 0x80
+#define DW_IC_ENABLE_STATUS 0x9c
+#define DW_IC_CLR_RESTART_DET 0xa8
+#define DW_IC_COMP_PARAM_1 0xf4
+#define DW_IC_COMP_VERSION 0xf8
+#define DW_IC_SDA_HOLD_MIN_VERS 0x3131312A
+#define DW_IC_COMP_TYPE 0xfc
+#define DW_IC_COMP_TYPE_VALUE 0x44570140
+
+#define DW_IC_INTR_RX_UNDER BIT(0)
+#define DW_IC_INTR_RX_OVER BIT(1)
+#define DW_IC_INTR_RX_FULL BIT(2)
+#define DW_IC_INTR_TX_OVER BIT(3)
+#define DW_IC_INTR_TX_EMPTY BIT(4)
+#define DW_IC_INTR_RD_REQ BIT(5)
+#define DW_IC_INTR_TX_ABRT BIT(6)
+#define DW_IC_INTR_RX_DONE BIT(7)
+#define DW_IC_INTR_ACTIVITY BIT(8)
+#define DW_IC_INTR_STOP_DET BIT(9)
+#define DW_IC_INTR_START_DET BIT(10)
+#define DW_IC_INTR_GEN_CALL BIT(11)
+#define DW_IC_INTR_RESTART_DET BIT(12)
+
+#define DW_IC_INTR_DEFAULT_MASK (DW_IC_INTR_RX_FULL | \
+ DW_IC_INTR_TX_ABRT | \
+ DW_IC_INTR_STOP_DET)
+#define DW_IC_INTR_MASTER_MASK (DW_IC_INTR_DEFAULT_MASK | \
+ DW_IC_INTR_TX_EMPTY)
+#define DW_IC_INTR_SLAVE_MASK (DW_IC_INTR_DEFAULT_MASK | \
+ DW_IC_INTR_RX_UNDER | \
+ DW_IC_INTR_RD_REQ)
+
+#define DW_IC_STATUS_ACTIVITY BIT(0)
+#define DW_IC_STATUS_TFE BIT(2)
+#define DW_IC_STATUS_RFNE BIT(3)
+#define DW_IC_STATUS_MASTER_ACTIVITY BIT(5)
+#define DW_IC_STATUS_SLAVE_ACTIVITY BIT(6)
+
+#define DW_IC_SDA_HOLD_RX_SHIFT 16
+#define DW_IC_SDA_HOLD_RX_MASK GENMASK(23, 16)
+
+#define DW_IC_ERR_TX_ABRT 0x1
+
+#define DW_IC_TAR_10BITADDR_MASTER BIT(12)
#define DW_IC_COMP_PARAM_1_SPEED_MODE_HIGH (BIT(2) | BIT(3))
#define DW_IC_COMP_PARAM_1_SPEED_MODE_MASK GENMASK(3, 2)
@@ -125,16 +125,16 @@
/*
* Sofware status flags
*/
-#define STATUS_ACTIVE BIT(0)
-#define STATUS_WRITE_IN_PROGRESS BIT(1)
-#define STATUS_READ_IN_PROGRESS BIT(2)
-#define STATUS_MASK GENMASK(2, 0)
+#define STATUS_ACTIVE BIT(0)
+#define STATUS_WRITE_IN_PROGRESS BIT(1)
+#define STATUS_READ_IN_PROGRESS BIT(2)
+#define STATUS_MASK GENMASK(2, 0)
/*
* operation modes
*/
-#define DW_IC_MASTER 0
-#define DW_IC_SLAVE 1
+#define DW_IC_MASTER 0
+#define DW_IC_SLAVE 1
/*
* Hardware abort codes from the DW_IC_TX_ABRT_SOURCE register
@@ -142,20 +142,20 @@
* Only expected abort codes are listed here
* refer to the datasheet for the full list
*/
-#define ABRT_7B_ADDR_NOACK 0
-#define ABRT_10ADDR1_NOACK 1
-#define ABRT_10ADDR2_NOACK 2
-#define ABRT_TXDATA_NOACK 3
-#define ABRT_GCALL_NOACK 4
-#define ABRT_GCALL_READ 5
-#define ABRT_SBYTE_ACKDET 7
-#define ABRT_SBYTE_NORSTRT 9
-#define ABRT_10B_RD_NORSTRT 10
-#define ABRT_MASTER_DIS 11
-#define ARB_LOST 12
-#define ABRT_SLAVE_FLUSH_TXFIFO 13
-#define ABRT_SLAVE_ARBLOST 14
-#define ABRT_SLAVE_RD_INTX 15
+#define ABRT_7B_ADDR_NOACK 0
+#define ABRT_10ADDR1_NOACK 1
+#define ABRT_10ADDR2_NOACK 2
+#define ABRT_TXDATA_NOACK 3
+#define ABRT_GCALL_NOACK 4
+#define ABRT_GCALL_READ 5
+#define ABRT_SBYTE_ACKDET 7
+#define ABRT_SBYTE_NORSTRT 9
+#define ABRT_10B_RD_NORSTRT 10
+#define ABRT_MASTER_DIS 11
+#define ARB_LOST 12
+#define ABRT_SLAVE_FLUSH_TXFIFO 13
+#define ABRT_SLAVE_ARBLOST 14
+#define ABRT_SLAVE_RD_INTX 15
#define DW_IC_TX_ABRT_7B_ADDR_NOACK BIT(ABRT_7B_ADDR_NOACK)
#define DW_IC_TX_ABRT_10ADDR1_NOACK BIT(ABRT_10ADDR1_NOACK)
@@ -172,11 +172,11 @@
#define DW_IC_RX_ABRT_SLAVE_ARBLOST BIT(ABRT_SLAVE_ARBLOST)
#define DW_IC_RX_ABRT_SLAVE_FLUSH_TXFIFO BIT(ABRT_SLAVE_FLUSH_TXFIFO)
-#define DW_IC_TX_ABRT_NOACK (DW_IC_TX_ABRT_7B_ADDR_NOACK | \
- DW_IC_TX_ABRT_10ADDR1_NOACK | \
- DW_IC_TX_ABRT_10ADDR2_NOACK | \
- DW_IC_TX_ABRT_TXDATA_NOACK | \
- DW_IC_TX_ABRT_GCALL_NOACK)
+#define DW_IC_TX_ABRT_NOACK (DW_IC_TX_ABRT_7B_ADDR_NOACK | \
+ DW_IC_TX_ABRT_10ADDR1_NOACK | \
+ DW_IC_TX_ABRT_10ADDR2_NOACK | \
+ DW_IC_TX_ABRT_TXDATA_NOACK | \
+ DW_IC_TX_ABRT_GCALL_NOACK)
struct clk;
struct device;
@@ -295,21 +295,21 @@ struct dw_i2c_dev {
struct i2c_bus_recovery_info rinfo;
};
-#define ACCESS_INTR_MASK BIT(0)
-#define ACCESS_NO_IRQ_SUSPEND BIT(1)
-#define ARBITRATION_SEMAPHORE BIT(2)
+#define ACCESS_INTR_MASK BIT(0)
+#define ACCESS_NO_IRQ_SUSPEND BIT(1)
+#define ARBITRATION_SEMAPHORE BIT(2)
-#define MODEL_MSCC_OCELOT BIT(8)
-#define MODEL_BAIKAL_BT1 BIT(9)
-#define MODEL_AMD_NAVI_GPU BIT(10)
-#define MODEL_MASK GENMASK(11, 8)
+#define MODEL_MSCC_OCELOT BIT(8)
+#define MODEL_BAIKAL_BT1 BIT(9)
+#define MODEL_AMD_NAVI_GPU BIT(10)
+#define MODEL_MASK GENMASK(11, 8)
/*
* Enable UCSI interrupt by writing 0xd at register
* offset 0x474 specified in hardware specification.
*/
-#define AMD_UCSI_INTR_REG 0x474
-#define AMD_UCSI_INTR_EN 0xd
+#define AMD_UCSI_INTR_REG 0x474
+#define AMD_UCSI_INTR_EN 0xd
struct i2c_dw_semaphore_callbacks {
int (*probe)(struct dw_i2c_dev *dev);
--
2.35.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 12/12] i2c: designware: Add comment to custom register value constants
2022-11-07 13:42 [PATCH v3 00/12] i2c: designware: Slave fixes and generic cleanups Jarkko Nikula
` (10 preceding siblings ...)
2022-11-07 13:42 ` [PATCH v3 11/12] i2c: designware: Align defines in i2c-designware-core.h Jarkko Nikula
@ 2022-11-07 13:42 ` Jarkko Nikula
2022-11-12 6:57 ` [PATCH v3 00/12] i2c: designware: Slave fixes and generic cleanups Wolfram Sang
12 siblings, 0 replies; 16+ messages in thread
From: Jarkko Nikula @ 2022-11-07 13:42 UTC (permalink / raw)
To: linux-i2c
Cc: Wolfram Sang, Andy Shevchenko, Mika Westerberg, Jan Dabros,
Michael Wu, Tian Ye, Jarkko Nikula
DW_IC_COMP_VERSION register contains the ASCII representation of the
Synopsys component version. Here 0x3131312A == "111*" means version
1.11* required for DW_IC_SDA_HOLD register availability where '*' means
any letter starting from 'a'.
DW_IC_COMP_TYPE is constant and is derived from two ASCII letters "DW"
followed by a 16-bit unsigned number.
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: Updated commit log and comment about DW_IC_COMP_VERSION and
DW_IC_SDA_HOLD_MIN_VERS define.
---
drivers/i2c/busses/i2c-designware-core.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 0668888d557d..95ebc5eaa5d1 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -79,9 +79,9 @@
#define DW_IC_CLR_RESTART_DET 0xa8
#define DW_IC_COMP_PARAM_1 0xf4
#define DW_IC_COMP_VERSION 0xf8
-#define DW_IC_SDA_HOLD_MIN_VERS 0x3131312A
+#define DW_IC_SDA_HOLD_MIN_VERS 0x3131312A /* "111*" == v1.11* */
#define DW_IC_COMP_TYPE 0xfc
-#define DW_IC_COMP_TYPE_VALUE 0x44570140
+#define DW_IC_COMP_TYPE_VALUE 0x44570140 /* "DW" + 0x0140 */
#define DW_IC_INTR_RX_UNDER BIT(0)
#define DW_IC_INTR_RX_OVER BIT(1)
--
2.35.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 00/12] i2c: designware: Slave fixes and generic cleanups
2022-11-07 13:42 [PATCH v3 00/12] i2c: designware: Slave fixes and generic cleanups Jarkko Nikula
` (11 preceding siblings ...)
2022-11-07 13:42 ` [PATCH v3 12/12] i2c: designware: Add comment to custom register value constants Jarkko Nikula
@ 2022-11-12 6:57 ` Wolfram Sang
12 siblings, 0 replies; 16+ messages in thread
From: Wolfram Sang @ 2022-11-12 6:57 UTC (permalink / raw)
To: Jarkko Nikula
Cc: linux-i2c, Andy Shevchenko, Mika Westerberg, Jan Dabros,
Michael Wu, Tian Ye
[-- Attachment #1: Type: text/plain, Size: 134 bytes --]
> Version 3 of the patchset I sent last week:
> https://www.spinics.net/lists/linux-i2c/msg59046.html
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread