* [PATCH v1 0/7] i2c: npcm: Bug fixes read/write operation, checkpatch
@ 2024-08-07 10:02 warp5tw
2024-08-07 10:02 ` [PATCH v1 1/7] i2c: npcm: correct the read/write operation procedure warp5tw
` (6 more replies)
0 siblings, 7 replies; 12+ messages in thread
From: warp5tw @ 2024-08-07 10:02 UTC (permalink / raw)
To: tali.perry, Avi.Fishman, tomer.maimon, avifishman70, tmaimon77,
tali.perry1, venture, yuenn, benjaminfair, andi.shyti,
wsa+renesas, rand.sec96, kwliu, jjliu0, kfting, warp5tw
Cc: linux-i2c, linux-kernel, openbmc
From: Tyrone Ting <kfting@nuvoton.com>
This patchset includes the following fixes:
- Enable the target functionality in the interrupt handling routine when the
i2c transfer is about to finish.
- Correct the read/write operation procedure.
- Introduce a software flag to handle the bus error (BER) condition
which is not caused by the i2c transfer.
- Modify timeout calculation.
- Assign the client address earlier logically.
- Use an i2c frequency table for the frequency parameters assignment.
- Coding style fix.
The NPCM I2C driver is tested on NPCM750 and NPCM845 evaluation boards.
Charles Boyer (1):
i2c-npcm7xx.c: Enable slave in eob interrupt
Tyrone Ting (6):
i2c: npcm: correct the read/write operation procedure
i2c: npcm: use a software flag to indicate a BER condition
i2c: npcm: Modify timeout evaluation mechanism
i2c: npcm: Modify the client address assignment
drivers: i2c: use i2c frequency table
i2c: npcm: fix checkpatch
drivers/i2c/busses/i2c-npcm7xx.c | 266 +++++++++++++++++++------------
1 file changed, 167 insertions(+), 99 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v1 1/7] i2c: npcm: correct the read/write operation procedure 2024-08-07 10:02 [PATCH v1 0/7] i2c: npcm: Bug fixes read/write operation, checkpatch warp5tw @ 2024-08-07 10:02 ` warp5tw 2024-08-07 10:02 ` [PATCH v1 2/7] i2c: npcm: use a software flag to indicate a BER condition warp5tw ` (5 subsequent siblings) 6 siblings, 0 replies; 12+ messages in thread From: warp5tw @ 2024-08-07 10:02 UTC (permalink / raw) To: tali.perry, Avi.Fishman, tomer.maimon, avifishman70, tmaimon77, tali.perry1, venture, yuenn, benjaminfair, andi.shyti, wsa+renesas, rand.sec96, kwliu, jjliu0, kfting, warp5tw Cc: linux-i2c, linux-kernel, openbmc From: Tyrone Ting <kfting@nuvoton.com> Originally the driver uses the XMIT bit in SMBnST register to decide the upcoming i2c transaction. If XMIT bit is 1, then it will be an i2c write operation. If it's 0, then a read operation will be executed. After checking the datasheet, the XMIT bit is valid when the i2c module is acting in a slave role. Use the software status to control the i2c transaction flow instead when the i2c module is acting in a master role. Fixes: 48acf8292280 ("i2c: Remove redundant comparison in npcm_i2c_reg_slave") Signed-off-by: Tyrone Ting <kfting@nuvoton.com> --- drivers/i2c/busses/i2c-npcm7xx.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/i2c/busses/i2c-npcm7xx.c b/drivers/i2c/busses/i2c-npcm7xx.c index 2fe68615942e..c8503acdaff8 100644 --- a/drivers/i2c/busses/i2c-npcm7xx.c +++ b/drivers/i2c/busses/i2c-npcm7xx.c @@ -1626,13 +1626,10 @@ static void npcm_i2c_irq_handle_sda(struct npcm_i2c *bus, u8 i2cst) npcm_i2c_wr_byte(bus, bus->dest_addr | BIT(0)); /* SDA interrupt, after start\restart */ } else { - if (NPCM_I2CST_XMIT & i2cst) { - bus->operation = I2C_WRITE_OPER; + if (bus->operation == I2C_WRITE_OPER) npcm_i2c_irq_master_handler_write(bus); - } else { - bus->operation = I2C_READ_OPER; + else if (bus->operation == I2C_READ_OPER) npcm_i2c_irq_master_handler_read(bus); - } } } -- 2.34.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v1 2/7] i2c: npcm: use a software flag to indicate a BER condition 2024-08-07 10:02 [PATCH v1 0/7] i2c: npcm: Bug fixes read/write operation, checkpatch warp5tw 2024-08-07 10:02 ` [PATCH v1 1/7] i2c: npcm: correct the read/write operation procedure warp5tw @ 2024-08-07 10:02 ` warp5tw 2024-08-07 10:02 ` [PATCH v1 3/7] i2c: npcm: Modify timeout evaluation mechanism warp5tw ` (4 subsequent siblings) 6 siblings, 0 replies; 12+ messages in thread From: warp5tw @ 2024-08-07 10:02 UTC (permalink / raw) To: tali.perry, Avi.Fishman, tomer.maimon, avifishman70, tmaimon77, tali.perry1, venture, yuenn, benjaminfair, andi.shyti, wsa+renesas, rand.sec96, kwliu, jjliu0, kfting, warp5tw Cc: linux-i2c, linux-kernel, openbmc From: Tyrone Ting <kfting@nuvoton.com> If not clearing the BB (bus busy) condition in the BER (bus error) interrupt, the driver causes a timeout and hence the i2c core doesn't do the i2c transfer retry but returns the driver's return value to the upper layer instead. Clear the BB condition in the BER interrupt and a software flag is used. The driver does an i2c recovery without causing the timeout if the flag is set. Fixes: 48acf8292280 ("i2c: Remove redundant comparison in npcm_i2c_reg_slave") Signed-off-by: Tyrone Ting <kfting@nuvoton.com> --- drivers/i2c/busses/i2c-npcm7xx.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-npcm7xx.c b/drivers/i2c/busses/i2c-npcm7xx.c index c8503acdaff8..bd444ff83a8c 100644 --- a/drivers/i2c/busses/i2c-npcm7xx.c +++ b/drivers/i2c/busses/i2c-npcm7xx.c @@ -332,6 +332,7 @@ struct npcm_i2c { u64 nack_cnt; u64 timeout_cnt; u64 tx_complete_cnt; + bool ber_state; }; static inline void npcm_i2c_select_bank(struct npcm_i2c *bus, @@ -1519,6 +1520,7 @@ static void npcm_i2c_irq_handle_ber(struct npcm_i2c *bus) if (npcm_i2c_is_master(bus)) { npcm_i2c_master_abort(bus); } else { + bus->ber_state = true; npcm_i2c_clear_master_status(bus); /* Clear BB (BUS BUSY) bit */ @@ -1697,6 +1699,7 @@ static int npcm_i2c_recovery_tgclk(struct i2c_adapter *_adap) dev_dbg(bus->dev, "bus%d-0x%x recovery skipped, bus not stuck", bus->num, bus->dest_addr); npcm_i2c_reset(bus); + bus->ber_state = false; return 0; } @@ -1761,6 +1764,7 @@ static int npcm_i2c_recovery_tgclk(struct i2c_adapter *_adap) if (bus->rec_succ_cnt < ULLONG_MAX) bus->rec_succ_cnt++; } + bus->ber_state = false; return status; } @@ -2156,7 +2160,7 @@ static int npcm_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, } while (time_is_after_jiffies(time_left) && bus_busy); - if (bus_busy) { + if (bus_busy || bus->ber_state) { iowrite8(NPCM_I2CCST_BB, bus->reg + NPCM_I2CCST); npcm_i2c_reset(bus); i2c_recover_bus(adap); -- 2.34.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v1 3/7] i2c: npcm: Modify timeout evaluation mechanism 2024-08-07 10:02 [PATCH v1 0/7] i2c: npcm: Bug fixes read/write operation, checkpatch warp5tw 2024-08-07 10:02 ` [PATCH v1 1/7] i2c: npcm: correct the read/write operation procedure warp5tw 2024-08-07 10:02 ` [PATCH v1 2/7] i2c: npcm: use a software flag to indicate a BER condition warp5tw @ 2024-08-07 10:02 ` warp5tw 2024-08-07 10:02 ` [PATCH v1 4/7] i2c: npcm: Modify the client address assignment warp5tw ` (3 subsequent siblings) 6 siblings, 0 replies; 12+ messages in thread From: warp5tw @ 2024-08-07 10:02 UTC (permalink / raw) To: tali.perry, Avi.Fishman, tomer.maimon, avifishman70, tmaimon77, tali.perry1, venture, yuenn, benjaminfair, andi.shyti, wsa+renesas, rand.sec96, kwliu, jjliu0, kfting, warp5tw Cc: linux-i2c, linux-kernel, openbmc From: Tyrone Ting <kfting@nuvoton.com> Increase the timeout and treat it as the total timeout, including retries. The total timeout is 2 seconds now. The i2c core layer will have chances to retry to call the i2c driver transfer function if the i2c driver reports that the bus is busy and returns EAGAIN. Fixes: 48acf8292280 ("i2c: Remove redundant comparison in npcm_i2c_reg_slave") Signed-off-by: Tyrone Ting <kfting@nuvoton.com> --- drivers/i2c/busses/i2c-npcm7xx.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/drivers/i2c/busses/i2c-npcm7xx.c b/drivers/i2c/busses/i2c-npcm7xx.c index bd444ff83a8c..d115ac659900 100644 --- a/drivers/i2c/busses/i2c-npcm7xx.c +++ b/drivers/i2c/busses/i2c-npcm7xx.c @@ -2130,19 +2130,12 @@ static int npcm_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, } } - /* - * Adaptive TimeOut: estimated time in usec + 100% margin: - * 2: double the timeout for clock stretching case - * 9: bits per transaction (including the ack/nack) - */ - timeout_usec = (2 * 9 * USEC_PER_SEC / bus->bus_freq) * (2 + nread + nwrite); - timeout = max_t(unsigned long, bus->adap.timeout, usecs_to_jiffies(timeout_usec)); if (nwrite >= 32 * 1024 || nread >= 32 * 1024) { dev_err(bus->dev, "i2c%d buffer too big\n", bus->num); return -EINVAL; } - time_left = jiffies + timeout + 1; + time_left = jiffies + bus->adap.timeout / bus->adap.retries + 1; do { /* * we must clear slave address immediately when the bus is not @@ -2181,6 +2174,14 @@ static int npcm_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, if (npcm_i2c_master_start_xmit(bus, slave_addr, nwrite, nread, write_data, read_data, read_PEC, read_block)) { + /* + * Adaptive TimeOut: estimated time in usec + 100% margin: + * 2: double the timeout for clock stretching case + * 9: bits per transaction (including the ack/nack) + */ + timeout_usec = (2 * 9 * USEC_PER_SEC / bus->bus_freq) * (2 + nread + nwrite); + timeout = max_t(unsigned long, bus->adap.timeout / bus->adap.retries, + usecs_to_jiffies(timeout_usec)); time_left = wait_for_completion_timeout(&bus->cmd_complete, timeout); @@ -2306,7 +2307,7 @@ static int npcm_i2c_probe_bus(struct platform_device *pdev) adap = &bus->adap; adap->owner = THIS_MODULE; adap->retries = 3; - adap->timeout = msecs_to_jiffies(35); + adap->timeout = 2 * HZ; adap->algo = &npcm_i2c_algo; adap->quirks = &npcm_i2c_quirks; adap->algo_data = bus; -- 2.34.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v1 4/7] i2c: npcm: Modify the client address assignment 2024-08-07 10:02 [PATCH v1 0/7] i2c: npcm: Bug fixes read/write operation, checkpatch warp5tw ` (2 preceding siblings ...) 2024-08-07 10:02 ` [PATCH v1 3/7] i2c: npcm: Modify timeout evaluation mechanism warp5tw @ 2024-08-07 10:02 ` warp5tw 2024-08-07 10:02 ` [PATCH v1 5/7] drivers: i2c: use i2c frequency table warp5tw ` (2 subsequent siblings) 6 siblings, 0 replies; 12+ messages in thread From: warp5tw @ 2024-08-07 10:02 UTC (permalink / raw) To: tali.perry, Avi.Fishman, tomer.maimon, avifishman70, tmaimon77, tali.perry1, venture, yuenn, benjaminfair, andi.shyti, wsa+renesas, rand.sec96, kwliu, jjliu0, kfting, warp5tw Cc: linux-i2c, linux-kernel, openbmc From: Tyrone Ting <kfting@nuvoton.com> Store the client address earlier since it's used in the i2c_recover_bus logic flow. Fixes: 48acf8292280 ("i2c: Remove redundant comparison in npcm_i2c_reg_slave") Signed-off-by: Tyrone Ting <kfting@nuvoton.com> --- drivers/i2c/busses/i2c-npcm7xx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-npcm7xx.c b/drivers/i2c/busses/i2c-npcm7xx.c index d115ac659900..c6f72c0e3464 100644 --- a/drivers/i2c/busses/i2c-npcm7xx.c +++ b/drivers/i2c/busses/i2c-npcm7xx.c @@ -2153,6 +2153,7 @@ static int npcm_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, } while (time_is_after_jiffies(time_left) && bus_busy); + bus->dest_addr = slave_addr << 1; if (bus_busy || bus->ber_state) { iowrite8(NPCM_I2CCST_BB, bus->reg + NPCM_I2CCST); npcm_i2c_reset(bus); @@ -2161,7 +2162,6 @@ static int npcm_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, } npcm_i2c_init_params(bus); - bus->dest_addr = slave_addr; bus->msgs = msgs; bus->msgs_num = num; bus->cmd_err = 0; -- 2.34.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v1 5/7] drivers: i2c: use i2c frequency table 2024-08-07 10:02 [PATCH v1 0/7] i2c: npcm: Bug fixes read/write operation, checkpatch warp5tw ` (3 preceding siblings ...) 2024-08-07 10:02 ` [PATCH v1 4/7] i2c: npcm: Modify the client address assignment warp5tw @ 2024-08-07 10:02 ` warp5tw 2024-08-07 19:55 ` kernel test robot 2024-08-13 7:58 ` Dan Carpenter 2024-08-07 10:02 ` [PATCH v1 6/7] i2c-npcm7xx.c: Enable slave in eob interrupt warp5tw 2024-08-07 10:02 ` [PATCH v1 7/7] i2c: npcm: fix checkpatch warp5tw 6 siblings, 2 replies; 12+ messages in thread From: warp5tw @ 2024-08-07 10:02 UTC (permalink / raw) To: tali.perry, Avi.Fishman, tomer.maimon, avifishman70, tmaimon77, tali.perry1, venture, yuenn, benjaminfair, andi.shyti, wsa+renesas, rand.sec96, kwliu, jjliu0, kfting, warp5tw Cc: linux-i2c, linux-kernel, openbmc From: Tyrone Ting <kfting@nuvoton.com> Modify i2c frequency from table parameters for NPCM i2c modules. Supported frequencies are: 1. 100KHz 2. 400KHz 3. 1MHz Signed-off-by: Tyrone Ting <kfting@nuvoton.com> --- drivers/i2c/busses/i2c-npcm7xx.c | 226 +++++++++++++++++++------------ 1 file changed, 143 insertions(+), 83 deletions(-) diff --git a/drivers/i2c/busses/i2c-npcm7xx.c b/drivers/i2c/busses/i2c-npcm7xx.c index c6f72c0e3464..1537fb7baa8c 100644 --- a/drivers/i2c/busses/i2c-npcm7xx.c +++ b/drivers/i2c/busses/i2c-npcm7xx.c @@ -261,6 +261,121 @@ static const int npcm_i2caddr[I2C_NUM_OWN_ADDR] = { #define I2C_FREQ_MIN_HZ 10000 #define I2C_FREQ_MAX_HZ I2C_MAX_FAST_MODE_PLUS_FREQ +struct SMB_TIMING_T { + u32 core_clk; + u8 hldt; + u8 dbcnt; + u16 sclfrq; + u8 scllt; + u8 sclht; + bool fast_mode; +}; + +static struct SMB_TIMING_T SMB_TIMING_100KHZ[] = { + {.core_clk = 100000000, .hldt = 0x2A, .dbcnt = 0x4, .sclfrq = 0xFB, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 62500000, .hldt = 0x2A, .dbcnt = 0x1, .sclfrq = 0x9D, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 50000000, .hldt = 0x2A, .dbcnt = 0x1, .sclfrq = 0x7E, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 48000000, .hldt = 0x2A, .dbcnt = 0x1, .sclfrq = 0x79, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 40000000, .hldt = 0x2A, .dbcnt = 0x1, .sclfrq = 0x65, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 30000000, .hldt = 0x2A, .dbcnt = 0x1, .sclfrq = 0x4C, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 29000000, .hldt = 0x2A, .dbcnt = 0x1, .sclfrq = 0x49, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 26000000, .hldt = 0x2A, .dbcnt = 0x1, .sclfrq = 0x42, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 25000000, .hldt = 0x2A, .dbcnt = 0x1, .sclfrq = 0x3F, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 24000000, .hldt = 0x2A, .dbcnt = 0x1, .sclfrq = 0x3D, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 20000000, .hldt = 0x2A, .dbcnt = 0x1, .sclfrq = 0x33, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 16180000, .hldt = 0x2A, .dbcnt = 0x1, .sclfrq = 0x29, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 15000000, .hldt = 0x23, .dbcnt = 0x1, .sclfrq = 0x26, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 13000000, .hldt = 0x1D, .dbcnt = 0x1, .sclfrq = 0x21, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 12000000, .hldt = 0x1B, .dbcnt = 0x1, .sclfrq = 0x1F, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 10000000, .hldt = 0x18, .dbcnt = 0x1, .sclfrq = 0x1A, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 9000000, .hldt = 0x16, .dbcnt = 0x1, .sclfrq = 0x17, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 8090000, .hldt = 0x14, .dbcnt = 0x1, .sclfrq = 0x15, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 7500000, .hldt = 0x7, .dbcnt = 0x1, .sclfrq = 0x13, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 6500000, .hldt = 0xE, .dbcnt = 0x1, .sclfrq = 0x11, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false }, + {.core_clk = 4000000, .hldt = 0x9, .dbcnt = 0x1, .sclfrq = 0xB, .scllt = 0x0, + .sclht = 0x0, .fast_mode = false } +}; + +static struct SMB_TIMING_T SMB_TIMING_400KHZ[] = { + {.core_clk = 100000000, .hldt = 0x2A, .dbcnt = 0x3, .sclfrq = 0x0, .scllt = 0x47, + .sclht = 0x35, .fast_mode = true }, + {.core_clk = 62500000, .hldt = 0x2A, .dbcnt = 0x2, .sclfrq = 0x0, .scllt = 0x2C, + .sclht = 0x22, .fast_mode = true }, + {.core_clk = 50000000, .hldt = 0x21, .dbcnt = 0x1, .sclfrq = 0x0, .scllt = 0x24, + .sclht = 0x1B, .fast_mode = true }, + {.core_clk = 48000000, .hldt = 0x1E, .dbcnt = 0x1, .sclfrq = 0x0, .scllt = 0x24, + .sclht = 0x19, .fast_mode = true }, + {.core_clk = 40000000, .hldt = 0x1B, .dbcnt = 0x1, .sclfrq = 0x0, .scllt = 0x1E, + .sclht = 0x14, .fast_mode = true }, + {.core_clk = 33000000, .hldt = 0x15, .dbcnt = 0x1, .sclfrq = 0x0, .scllt = 0x19, + .sclht = 0x11, .fast_mode = true }, + {.core_clk = 30000000, .hldt = 0x15, .dbcnt = 0x1, .sclfrq = 0x0, .scllt = 0x19, + .sclht = 0xD, .fast_mode = true }, + {.core_clk = 29000000, .hldt = 0x11, .dbcnt = 0x1, .sclfrq = 0x0, .scllt = 0x15, + .sclht = 0x10, .fast_mode = true }, + {.core_clk = 26000000, .hldt = 0x10, .dbcnt = 0x1, .sclfrq = 0x0, .scllt = 0x13, + .sclht = 0xE, .fast_mode = true }, + {.core_clk = 25000000, .hldt = 0xF, .dbcnt = 0x1, .sclfrq = 0x0, .scllt = 0x13, + .sclht = 0xD, .fast_mode = true }, + {.core_clk = 24000000, .hldt = 0xD, .dbcnt = 0x1, .sclfrq = 0x0, .scllt = 0x12, + .sclht = 0xD, .fast_mode = true }, + {.core_clk = 20000000, .hldt = 0xB, .dbcnt = 0x1, .sclfrq = 0x0, .scllt = 0xF, + .sclht = 0xA, .fast_mode = true }, + {.core_clk = 16180000, .hldt = 0xA, .dbcnt = 0x1, .sclfrq = 0x0, .scllt = 0xC, + .sclht = 0x9, .fast_mode = true }, + {.core_clk = 15000000, .hldt = 0x9, .dbcnt = 0x1, .sclfrq = 0x0, .scllt = 0xB, + .sclht = 0x8, .fast_mode = true }, + {.core_clk = 13000000, .hldt = 0x7, .dbcnt = 0x1, .sclfrq = 0x0, .scllt = 0xA, + .sclht = 0x7, .fast_mode = true }, + {.core_clk = 12000000, .hldt = 0x7, .dbcnt = 0x1, .sclfrq = 0x0, .scllt = 0xA, + .sclht = 0x6, .fast_mode = true }, + {.core_clk = 10000000, .hldt = 0x6, .dbcnt = 0x1, .sclfrq = 0x0, .scllt = 0x8, + .sclht = 0x5, .fast_mode = true }, +}; + +static struct SMB_TIMING_T SMB_TIMING_1000KHZ[] = { + {.core_clk = 100000000, .hldt = 0x15, .dbcnt = 0x4, .sclfrq = 0x0, .scllt = 0x1C, + .sclht = 0x15, .fast_mode = true }, + {.core_clk = 62500000, .hldt = 0xF, .dbcnt = 0x3, .sclfrq = 0x0, .scllt = 0x11, + .sclht = 0xE, .fast_mode = true }, + {.core_clk = 50000000, .hldt = 0xA, .dbcnt = 0x2, .sclfrq = 0x0, .scllt = 0xE, + .sclht = 0xB, .fast_mode = true }, + {.core_clk = 48000000, .hldt = 0x9, .dbcnt = 0x2, .sclfrq = 0x0, .scllt = 0xD, + .sclht = 0xB, .fast_mode = true }, + {.core_clk = 41000000, .hldt = 0x9, .dbcnt = 0x2, .sclfrq = 0x0, .scllt = 0xC, + .sclht = 0x9, .fast_mode = true }, + {.core_clk = 40000000, .hldt = 0x8, .dbcnt = 0x2, .sclfrq = 0x0, .scllt = 0xB, + .sclht = 0x9, .fast_mode = true }, + {.core_clk = 33000000, .hldt = 0x7, .dbcnt = 0x1, .sclfrq = 0x0, .scllt = 0xA, + .sclht = 0x7, .fast_mode = true }, + {.core_clk = 25000000, .hldt = 0x4, .dbcnt = 0x1, .sclfrq = 0x0, .scllt = 0x7, + .sclht = 0x6, .fast_mode = true }, + {.core_clk = 24000000, .hldt = 0x7, .dbcnt = 0x1, .sclfrq = 0x0, .scllt = 0x8, + .sclht = 0x5, .fast_mode = true }, + {.core_clk = 20000000, .hldt = 0x4, .dbcnt = 0x1, .sclfrq = 0x0, .scllt = 0x6, + .sclht = 0x4, .fast_mode = true }, +}; + struct npcm_i2c_data { u8 fifo_size; u32 segctl_init_val; @@ -1803,11 +1918,9 @@ static void npcm_i2c_recovery_init(struct i2c_adapter *_adap) */ static int npcm_i2c_init_clk(struct npcm_i2c *bus, u32 bus_freq_hz) { - u32 k1 = 0; - u32 k2 = 0; - u8 dbnct = 0; - u32 sclfrq = 0; - u8 hldt = 7; + struct SMB_TIMING_T *smb_timing; + u8 scl_table_cnt = 0, table_size = 0; + u8 fast_mode = 0; u32 src_clk_khz; u32 bus_freq_khz; @@ -1816,89 +1929,36 @@ static int npcm_i2c_init_clk(struct npcm_i2c *bus, u32 bus_freq_hz) bus_freq_khz = bus_freq_hz / 1000; bus->bus_freq = bus_freq_hz; - /* 100KHz and below: */ - if (bus_freq_hz <= I2C_MAX_STANDARD_MODE_FREQ) { - sclfrq = src_clk_khz / (bus_freq_khz * 4); - - if (sclfrq < SCLFRQ_MIN || sclfrq > SCLFRQ_MAX) - return -EDOM; - - if (src_clk_khz >= 40000) - hldt = 17; - else if (src_clk_khz >= 12500) - hldt = 15; - else - hldt = 7; - } - - /* 400KHz: */ - else if (bus_freq_hz <= I2C_MAX_FAST_MODE_FREQ) { - sclfrq = 0; + switch (bus_freq_hz) { + case I2C_MAX_STANDARD_MODE_FREQ: + smb_timing = SMB_TIMING_100KHZ; + table_size = ARRAY_SIZE(SMB_TIMING_100KHZ); + break; + case I2C_MAX_FAST_MODE_FREQ: + smb_timing = SMB_TIMING_400KHZ; + table_size = ARRAY_SIZE(SMB_TIMING_400KHZ); fast_mode = I2CCTL3_400K_MODE; - - if (src_clk_khz < 7500) - /* 400KHZ cannot be supported for core clock < 7.5MHz */ - return -EDOM; - - else if (src_clk_khz >= 50000) { - k1 = 80; - k2 = 48; - hldt = 12; - dbnct = 7; - } - - /* Master or Slave with frequency > 25MHz */ - else if (src_clk_khz > 25000) { - hldt = clk_coef(src_clk_khz, 300) + 7; - k1 = clk_coef(src_clk_khz, 1600); - k2 = clk_coef(src_clk_khz, 900); - } - } - - /* 1MHz: */ - else if (bus_freq_hz <= I2C_MAX_FAST_MODE_PLUS_FREQ) { - sclfrq = 0; + break; + case I2C_MAX_FAST_MODE_PLUS_FREQ: + smb_timing = SMB_TIMING_1000KHZ; + table_size = ARRAY_SIZE(SMB_TIMING_1000KHZ); fast_mode = I2CCTL3_400K_MODE; - - /* 1MHZ cannot be supported for core clock < 24 MHz */ - if (src_clk_khz < 24000) - return -EDOM; - - k1 = clk_coef(src_clk_khz, 620); - k2 = clk_coef(src_clk_khz, 380); - - /* Core clk > 40 MHz */ - if (src_clk_khz > 40000) { - /* - * Set HLDT: - * SDA hold time: (HLDT-7) * T(CLK) >= 120 - * HLDT = 120/T(CLK) + 7 = 120 * FREQ(CLK) + 7 - */ - hldt = clk_coef(src_clk_khz, 120) + 7; - } else { - hldt = 7; - dbnct = 2; - } - } - - /* Frequency larger than 1 MHz is not supported */ - else + break; + default: return -EINVAL; - - if (bus_freq_hz >= I2C_MAX_FAST_MODE_FREQ) { - k1 = round_up(k1, 2); - k2 = round_up(k2 + 1, 2); - if (k1 < SCLFRQ_MIN || k1 > SCLFRQ_MAX || - k2 < SCLFRQ_MIN || k2 > SCLFRQ_MAX) - return -EDOM; } + for (scl_table_cnt = 0 ; scl_table_cnt < table_size ; scl_table_cnt++) + if (bus->apb_clk >= smb_timing[scl_table_cnt].core_clk) + break; + /* write sclfrq value. bits [6:0] are in I2CCTL2 reg */ - iowrite8(FIELD_PREP(I2CCTL2_SCLFRQ6_0, sclfrq & 0x7F), + iowrite8(FIELD_PREP(I2CCTL2_SCLFRQ6_0, smb_timing[scl_table_cnt].sclfrq & 0x7F), bus->reg + NPCM_I2CCTL2); /* bits [8:7] are in I2CCTL3 reg */ - iowrite8(fast_mode | FIELD_PREP(I2CCTL3_SCLFRQ8_7, (sclfrq >> 7) & 0x3), + iowrite8(fast_mode | FIELD_PREP(I2CCTL3_SCLFRQ8_7, (smb_timing[scl_table_cnt].sclfrq >> 7) + & 0x3), bus->reg + NPCM_I2CCTL3); /* Select Bank 0 to access NPCM_I2CCTL4/NPCM_I2CCTL5 */ @@ -1910,13 +1970,13 @@ static int npcm_i2c_init_clk(struct npcm_i2c *bus, u32 bus_freq_hz) * k1 = 2 * SCLLT7-0 -> Low Time = k1 / 2 * k2 = 2 * SCLLT7-0 -> High Time = k2 / 2 */ - iowrite8(k1 / 2, bus->reg + NPCM_I2CSCLLT); - iowrite8(k2 / 2, bus->reg + NPCM_I2CSCLHT); + iowrite8(smb_timing[scl_table_cnt].scllt, bus->reg + NPCM_I2CSCLLT); + iowrite8(smb_timing[scl_table_cnt].sclht, bus->reg + NPCM_I2CSCLHT); - iowrite8(dbnct, bus->reg + NPCM_I2CCTL5); + iowrite8(smb_timing[scl_table_cnt].dbcnt, bus->reg + NPCM_I2CCTL5); } - iowrite8(hldt, bus->reg + NPCM_I2CCTL4); + iowrite8(smb_timing[scl_table_cnt].hldt, bus->reg + NPCM_I2CCTL4); /* Return to Bank 1, and stay there by default: */ npcm_i2c_select_bank(bus, I2C_BANK_1); -- 2.34.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v1 5/7] drivers: i2c: use i2c frequency table 2024-08-07 10:02 ` [PATCH v1 5/7] drivers: i2c: use i2c frequency table warp5tw @ 2024-08-07 19:55 ` kernel test robot 2024-08-13 7:58 ` Dan Carpenter 1 sibling, 0 replies; 12+ messages in thread From: kernel test robot @ 2024-08-07 19:55 UTC (permalink / raw) To: warp5tw, tali.perry, Avi.Fishman, tomer.maimon, avifishman70, tmaimon77, tali.perry1, venture, yuenn, benjaminfair, andi.shyti, wsa+renesas, rand.sec96, kwliu, jjliu0, kfting Cc: oe-kbuild-all, linux-i2c, linux-kernel, openbmc Hi, kernel test robot noticed the following build warnings: [auto build test WARNING on andi-shyti/i2c/i2c-host] [also build test WARNING on linus/master v6.11-rc2 next-20240807] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/warp5tw-gmail-com/i2c-npcm-correct-the-read-write-operation-procedure/20240807-182210 base: https://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git i2c/i2c-host patch link: https://lore.kernel.org/r/20240807100244.16872-6-kfting%40nuvoton.com patch subject: [PATCH v1 5/7] drivers: i2c: use i2c frequency table config: arm-randconfig-001-20240808 (https://download.01.org/0day-ci/archive/20240808/202408080319.de2B6PgU-lkp@intel.com/config) compiler: arm-linux-gnueabi-gcc (GCC) 14.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240808/202408080319.de2B6PgU-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202408080319.de2B6PgU-lkp@intel.com/ All warnings (new ones prefixed by >>): drivers/i2c/busses/i2c-npcm7xx.c: In function 'npcm_i2c_init_clk': >> drivers/i2c/busses/i2c-npcm7xx.c:1926:14: warning: variable 'bus_freq_khz' set but not used [-Wunused-but-set-variable] 1926 | u32 bus_freq_khz; | ^~~~~~~~~~~~ >> drivers/i2c/busses/i2c-npcm7xx.c:1925:14: warning: variable 'src_clk_khz' set but not used [-Wunused-but-set-variable] 1925 | u32 src_clk_khz; | ^~~~~~~~~~~ vim +/bus_freq_khz +1926 drivers/i2c/busses/i2c-npcm7xx.c 56a1485b102ed1 Tali Perry 2020-05-27 1909 56a1485b102ed1 Tali Perry 2020-05-27 1910 /* 56a1485b102ed1 Tali Perry 2020-05-27 1911 * npcm_i2c_init_clk: init HW timing parameters. 0c47dd7d09bb5d Jonathan Neuschäfer 2022-01-29 1912 * NPCM7XX i2c module timing parameters are dependent on module core clk (APB) 56a1485b102ed1 Tali Perry 2020-05-27 1913 * and bus frequency. 0c47dd7d09bb5d Jonathan Neuschäfer 2022-01-29 1914 * 100kHz bus requires tSCL = 4 * SCLFRQ * tCLK. LT and HT are symmetric. 0c47dd7d09bb5d Jonathan Neuschäfer 2022-01-29 1915 * 400kHz bus requires asymmetric HT and LT. A different equation is recommended 56a1485b102ed1 Tali Perry 2020-05-27 1916 * by the HW designer, given core clock range (equations in comments below). 56a1485b102ed1 Tali Perry 2020-05-27 1917 * 56a1485b102ed1 Tali Perry 2020-05-27 1918 */ 56a1485b102ed1 Tali Perry 2020-05-27 1919 static int npcm_i2c_init_clk(struct npcm_i2c *bus, u32 bus_freq_hz) 56a1485b102ed1 Tali Perry 2020-05-27 1920 { a946fe9698f261 Tyrone Ting 2024-08-07 1921 struct SMB_TIMING_T *smb_timing; a946fe9698f261 Tyrone Ting 2024-08-07 1922 u8 scl_table_cnt = 0, table_size = 0; a946fe9698f261 Tyrone Ting 2024-08-07 1923 56a1485b102ed1 Tali Perry 2020-05-27 1924 u8 fast_mode = 0; 56a1485b102ed1 Tali Perry 2020-05-27 @1925 u32 src_clk_khz; 56a1485b102ed1 Tali Perry 2020-05-27 @1926 u32 bus_freq_khz; 56a1485b102ed1 Tali Perry 2020-05-27 1927 56a1485b102ed1 Tali Perry 2020-05-27 1928 src_clk_khz = bus->apb_clk / 1000; 56a1485b102ed1 Tali Perry 2020-05-27 1929 bus_freq_khz = bus_freq_hz / 1000; 56a1485b102ed1 Tali Perry 2020-05-27 1930 bus->bus_freq = bus_freq_hz; 56a1485b102ed1 Tali Perry 2020-05-27 1931 a946fe9698f261 Tyrone Ting 2024-08-07 1932 switch (bus_freq_hz) { a946fe9698f261 Tyrone Ting 2024-08-07 1933 case I2C_MAX_STANDARD_MODE_FREQ: a946fe9698f261 Tyrone Ting 2024-08-07 1934 smb_timing = SMB_TIMING_100KHZ; a946fe9698f261 Tyrone Ting 2024-08-07 1935 table_size = ARRAY_SIZE(SMB_TIMING_100KHZ); a946fe9698f261 Tyrone Ting 2024-08-07 1936 break; a946fe9698f261 Tyrone Ting 2024-08-07 1937 case I2C_MAX_FAST_MODE_FREQ: a946fe9698f261 Tyrone Ting 2024-08-07 1938 smb_timing = SMB_TIMING_400KHZ; a946fe9698f261 Tyrone Ting 2024-08-07 1939 table_size = ARRAY_SIZE(SMB_TIMING_400KHZ); 56a1485b102ed1 Tali Perry 2020-05-27 1940 fast_mode = I2CCTL3_400K_MODE; a946fe9698f261 Tyrone Ting 2024-08-07 1941 break; a946fe9698f261 Tyrone Ting 2024-08-07 1942 case I2C_MAX_FAST_MODE_PLUS_FREQ: a946fe9698f261 Tyrone Ting 2024-08-07 1943 smb_timing = SMB_TIMING_1000KHZ; a946fe9698f261 Tyrone Ting 2024-08-07 1944 table_size = ARRAY_SIZE(SMB_TIMING_1000KHZ); 56a1485b102ed1 Tali Perry 2020-05-27 1945 fast_mode = I2CCTL3_400K_MODE; a946fe9698f261 Tyrone Ting 2024-08-07 1946 break; a946fe9698f261 Tyrone Ting 2024-08-07 1947 default: 56a1485b102ed1 Tali Perry 2020-05-27 1948 return -EINVAL; 56a1485b102ed1 Tali Perry 2020-05-27 1949 } 56a1485b102ed1 Tali Perry 2020-05-27 1950 a946fe9698f261 Tyrone Ting 2024-08-07 1951 for (scl_table_cnt = 0 ; scl_table_cnt < table_size ; scl_table_cnt++) a946fe9698f261 Tyrone Ting 2024-08-07 1952 if (bus->apb_clk >= smb_timing[scl_table_cnt].core_clk) a946fe9698f261 Tyrone Ting 2024-08-07 1953 break; a946fe9698f261 Tyrone Ting 2024-08-07 1954 56a1485b102ed1 Tali Perry 2020-05-27 1955 /* write sclfrq value. bits [6:0] are in I2CCTL2 reg */ a946fe9698f261 Tyrone Ting 2024-08-07 1956 iowrite8(FIELD_PREP(I2CCTL2_SCLFRQ6_0, smb_timing[scl_table_cnt].sclfrq & 0x7F), 56a1485b102ed1 Tali Perry 2020-05-27 1957 bus->reg + NPCM_I2CCTL2); 56a1485b102ed1 Tali Perry 2020-05-27 1958 56a1485b102ed1 Tali Perry 2020-05-27 1959 /* bits [8:7] are in I2CCTL3 reg */ a946fe9698f261 Tyrone Ting 2024-08-07 1960 iowrite8(fast_mode | FIELD_PREP(I2CCTL3_SCLFRQ8_7, (smb_timing[scl_table_cnt].sclfrq >> 7) a946fe9698f261 Tyrone Ting 2024-08-07 1961 & 0x3), 56a1485b102ed1 Tali Perry 2020-05-27 1962 bus->reg + NPCM_I2CCTL3); 56a1485b102ed1 Tali Perry 2020-05-27 1963 56a1485b102ed1 Tali Perry 2020-05-27 1964 /* Select Bank 0 to access NPCM_I2CCTL4/NPCM_I2CCTL5 */ 56a1485b102ed1 Tali Perry 2020-05-27 1965 npcm_i2c_select_bank(bus, I2C_BANK_0); 56a1485b102ed1 Tali Perry 2020-05-27 1966 56a1485b102ed1 Tali Perry 2020-05-27 1967 if (bus_freq_hz >= I2C_MAX_FAST_MODE_FREQ) { 56a1485b102ed1 Tali Perry 2020-05-27 1968 /* 56a1485b102ed1 Tali Perry 2020-05-27 1969 * Set SCL Low/High Time: 56a1485b102ed1 Tali Perry 2020-05-27 1970 * k1 = 2 * SCLLT7-0 -> Low Time = k1 / 2 56a1485b102ed1 Tali Perry 2020-05-27 1971 * k2 = 2 * SCLLT7-0 -> High Time = k2 / 2 56a1485b102ed1 Tali Perry 2020-05-27 1972 */ a946fe9698f261 Tyrone Ting 2024-08-07 1973 iowrite8(smb_timing[scl_table_cnt].scllt, bus->reg + NPCM_I2CSCLLT); a946fe9698f261 Tyrone Ting 2024-08-07 1974 iowrite8(smb_timing[scl_table_cnt].sclht, bus->reg + NPCM_I2CSCLHT); 56a1485b102ed1 Tali Perry 2020-05-27 1975 a946fe9698f261 Tyrone Ting 2024-08-07 1976 iowrite8(smb_timing[scl_table_cnt].dbcnt, bus->reg + NPCM_I2CCTL5); 56a1485b102ed1 Tali Perry 2020-05-27 1977 } 56a1485b102ed1 Tali Perry 2020-05-27 1978 a946fe9698f261 Tyrone Ting 2024-08-07 1979 iowrite8(smb_timing[scl_table_cnt].hldt, bus->reg + NPCM_I2CCTL4); 56a1485b102ed1 Tali Perry 2020-05-27 1980 56a1485b102ed1 Tali Perry 2020-05-27 1981 /* Return to Bank 1, and stay there by default: */ 56a1485b102ed1 Tali Perry 2020-05-27 1982 npcm_i2c_select_bank(bus, I2C_BANK_1); 56a1485b102ed1 Tali Perry 2020-05-27 1983 56a1485b102ed1 Tali Perry 2020-05-27 1984 return 0; 56a1485b102ed1 Tali Perry 2020-05-27 1985 } 56a1485b102ed1 Tali Perry 2020-05-27 1986 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 5/7] drivers: i2c: use i2c frequency table 2024-08-07 10:02 ` [PATCH v1 5/7] drivers: i2c: use i2c frequency table warp5tw 2024-08-07 19:55 ` kernel test robot @ 2024-08-13 7:58 ` Dan Carpenter 1 sibling, 0 replies; 12+ messages in thread From: Dan Carpenter @ 2024-08-13 7:58 UTC (permalink / raw) To: oe-kbuild, warp5tw, tali.perry, Avi.Fishman, tomer.maimon, avifishman70, tmaimon77, tali.perry1, venture, yuenn, benjaminfair, andi.shyti, wsa+renesas, rand.sec96, kwliu, jjliu0, kfting Cc: lkp, oe-kbuild-all, linux-i2c, linux-kernel, openbmc Hi, kernel test robot noticed the following build warnings: https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/warp5tw-gmail-com/i2c-npcm-correct-the-read-write-operation-procedure/20240807-182210 base: https://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git i2c/i2c-host patch link: https://lore.kernel.org/r/20240807100244.16872-6-kfting%40nuvoton.com patch subject: [PATCH v1 5/7] drivers: i2c: use i2c frequency table config: arm-randconfig-r073-20240812 (https://download.01.org/0day-ci/archive/20240813/202408130818.FgDP5uNm-lkp@intel.com/config) compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project f86594788ce93b696675c94f54016d27a6c21d18) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org> | Closes: https://lore.kernel.org/r/202408130818.FgDP5uNm-lkp@intel.com/ New smatch warnings: drivers/i2c/busses/i2c-npcm7xx.c:1956 npcm_i2c_init_clk() error: buffer overflow 'smb_timing' 21 <= 21 (assuming for loop doesn't break) drivers/i2c/busses/i2c-npcm7xx.c:1973 npcm_i2c_init_clk() error: buffer overflow 'smb_timing' 17 <= 21 Old smatch warnings: drivers/i2c/busses/i2c-npcm7xx.c:752 npcm_i2c_slave_enable() error: buffer overflow 'npcm_i2caddr' 2 <= 9 drivers/i2c/busses/i2c-npcm7xx.c:1960 npcm_i2c_init_clk() error: buffer overflow 'smb_timing' 21 <= 21 (assuming for loop doesn't break) drivers/i2c/busses/i2c-npcm7xx.c:1974 npcm_i2c_init_clk() error: buffer overflow 'smb_timing' 17 <= 21 drivers/i2c/busses/i2c-npcm7xx.c:1976 npcm_i2c_init_clk() error: buffer overflow 'smb_timing' 17 <= 21 drivers/i2c/busses/i2c-npcm7xx.c:1979 npcm_i2c_init_clk() error: buffer overflow 'smb_timing' 21 <= 21 (assuming for loop doesn't break) vim +1956 drivers/i2c/busses/i2c-npcm7xx.c 56a1485b102ed1 Tali Perry 2020-05-27 1919 static int npcm_i2c_init_clk(struct npcm_i2c *bus, u32 bus_freq_hz) 56a1485b102ed1 Tali Perry 2020-05-27 1920 { a946fe9698f261 Tyrone Ting 2024-08-07 1921 struct SMB_TIMING_T *smb_timing; a946fe9698f261 Tyrone Ting 2024-08-07 1922 u8 scl_table_cnt = 0, table_size = 0; a946fe9698f261 Tyrone Ting 2024-08-07 1923 56a1485b102ed1 Tali Perry 2020-05-27 1924 u8 fast_mode = 0; 56a1485b102ed1 Tali Perry 2020-05-27 1925 u32 src_clk_khz; 56a1485b102ed1 Tali Perry 2020-05-27 1926 u32 bus_freq_khz; 56a1485b102ed1 Tali Perry 2020-05-27 1927 56a1485b102ed1 Tali Perry 2020-05-27 1928 src_clk_khz = bus->apb_clk / 1000; 56a1485b102ed1 Tali Perry 2020-05-27 1929 bus_freq_khz = bus_freq_hz / 1000; 56a1485b102ed1 Tali Perry 2020-05-27 1930 bus->bus_freq = bus_freq_hz; 56a1485b102ed1 Tali Perry 2020-05-27 1931 a946fe9698f261 Tyrone Ting 2024-08-07 1932 switch (bus_freq_hz) { a946fe9698f261 Tyrone Ting 2024-08-07 1933 case I2C_MAX_STANDARD_MODE_FREQ: a946fe9698f261 Tyrone Ting 2024-08-07 1934 smb_timing = SMB_TIMING_100KHZ; a946fe9698f261 Tyrone Ting 2024-08-07 1935 table_size = ARRAY_SIZE(SMB_TIMING_100KHZ); a946fe9698f261 Tyrone Ting 2024-08-07 1936 break; a946fe9698f261 Tyrone Ting 2024-08-07 1937 case I2C_MAX_FAST_MODE_FREQ: a946fe9698f261 Tyrone Ting 2024-08-07 1938 smb_timing = SMB_TIMING_400KHZ; a946fe9698f261 Tyrone Ting 2024-08-07 1939 table_size = ARRAY_SIZE(SMB_TIMING_400KHZ); 56a1485b102ed1 Tali Perry 2020-05-27 1940 fast_mode = I2CCTL3_400K_MODE; a946fe9698f261 Tyrone Ting 2024-08-07 1941 break; a946fe9698f261 Tyrone Ting 2024-08-07 1942 case I2C_MAX_FAST_MODE_PLUS_FREQ: a946fe9698f261 Tyrone Ting 2024-08-07 1943 smb_timing = SMB_TIMING_1000KHZ; a946fe9698f261 Tyrone Ting 2024-08-07 1944 table_size = ARRAY_SIZE(SMB_TIMING_1000KHZ); 56a1485b102ed1 Tali Perry 2020-05-27 1945 fast_mode = I2CCTL3_400K_MODE; a946fe9698f261 Tyrone Ting 2024-08-07 1946 break; a946fe9698f261 Tyrone Ting 2024-08-07 1947 default: 56a1485b102ed1 Tali Perry 2020-05-27 1948 return -EINVAL; 56a1485b102ed1 Tali Perry 2020-05-27 1949 } 56a1485b102ed1 Tali Perry 2020-05-27 1950 a946fe9698f261 Tyrone Ting 2024-08-07 1951 for (scl_table_cnt = 0 ; scl_table_cnt < table_size ; scl_table_cnt++) a946fe9698f261 Tyrone Ting 2024-08-07 1952 if (bus->apb_clk >= smb_timing[scl_table_cnt].core_clk) a946fe9698f261 Tyrone Ting 2024-08-07 1953 break; The minimum smb_timing[scl_table_cnt].core_clk value is 10000000 or 20000000 so I can't tell just from the context that we are always going to hit this break statement. a946fe9698f261 Tyrone Ting 2024-08-07 1954 56a1485b102ed1 Tali Perry 2020-05-27 1955 /* write sclfrq value. bits [6:0] are in I2CCTL2 reg */ a946fe9698f261 Tyrone Ting 2024-08-07 @1956 iowrite8(FIELD_PREP(I2CCTL2_SCLFRQ6_0, smb_timing[scl_table_cnt].sclfrq & 0x7F), ^^^^^^^^^^^^^^^^^^^^^^^^^ 56a1485b102ed1 Tali Perry 2020-05-27 1957 bus->reg + NPCM_I2CCTL2); 56a1485b102ed1 Tali Perry 2020-05-27 1958 56a1485b102ed1 Tali Perry 2020-05-27 1959 /* bits [8:7] are in I2CCTL3 reg */ a946fe9698f261 Tyrone Ting 2024-08-07 1960 iowrite8(fast_mode | FIELD_PREP(I2CCTL3_SCLFRQ8_7, (smb_timing[scl_table_cnt].sclfrq >> 7) a946fe9698f261 Tyrone Ting 2024-08-07 1961 & 0x3), 56a1485b102ed1 Tali Perry 2020-05-27 1962 bus->reg + NPCM_I2CCTL3); 56a1485b102ed1 Tali Perry 2020-05-27 1963 56a1485b102ed1 Tali Perry 2020-05-27 1964 /* Select Bank 0 to access NPCM_I2CCTL4/NPCM_I2CCTL5 */ 56a1485b102ed1 Tali Perry 2020-05-27 1965 npcm_i2c_select_bank(bus, I2C_BANK_0); 56a1485b102ed1 Tali Perry 2020-05-27 1966 56a1485b102ed1 Tali Perry 2020-05-27 1967 if (bus_freq_hz >= I2C_MAX_FAST_MODE_FREQ) { 56a1485b102ed1 Tali Perry 2020-05-27 1968 /* 56a1485b102ed1 Tali Perry 2020-05-27 1969 * Set SCL Low/High Time: 56a1485b102ed1 Tali Perry 2020-05-27 1970 * k1 = 2 * SCLLT7-0 -> Low Time = k1 / 2 56a1485b102ed1 Tali Perry 2020-05-27 1971 * k2 = 2 * SCLLT7-0 -> High Time = k2 / 2 56a1485b102ed1 Tali Perry 2020-05-27 1972 */ a946fe9698f261 Tyrone Ting 2024-08-07 @1973 iowrite8(smb_timing[scl_table_cnt].scllt, bus->reg + NPCM_I2CSCLLT); a946fe9698f261 Tyrone Ting 2024-08-07 1974 iowrite8(smb_timing[scl_table_cnt].sclht, bus->reg + NPCM_I2CSCLHT); 56a1485b102ed1 Tali Perry 2020-05-27 1975 a946fe9698f261 Tyrone Ting 2024-08-07 1976 iowrite8(smb_timing[scl_table_cnt].dbcnt, bus->reg + NPCM_I2CCTL5); 56a1485b102ed1 Tali Perry 2020-05-27 1977 } 56a1485b102ed1 Tali Perry 2020-05-27 1978 a946fe9698f261 Tyrone Ting 2024-08-07 1979 iowrite8(smb_timing[scl_table_cnt].hldt, bus->reg + NPCM_I2CCTL4); 56a1485b102ed1 Tali Perry 2020-05-27 1980 56a1485b102ed1 Tali Perry 2020-05-27 1981 /* Return to Bank 1, and stay there by default: */ 56a1485b102ed1 Tali Perry 2020-05-27 1982 npcm_i2c_select_bank(bus, I2C_BANK_1); 56a1485b102ed1 Tali Perry 2020-05-27 1983 56a1485b102ed1 Tali Perry 2020-05-27 1984 return 0; 56a1485b102ed1 Tali Perry 2020-05-27 1985 } -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v1 6/7] i2c-npcm7xx.c: Enable slave in eob interrupt 2024-08-07 10:02 [PATCH v1 0/7] i2c: npcm: Bug fixes read/write operation, checkpatch warp5tw ` (4 preceding siblings ...) 2024-08-07 10:02 ` [PATCH v1 5/7] drivers: i2c: use i2c frequency table warp5tw @ 2024-08-07 10:02 ` warp5tw 2024-08-07 10:02 ` [PATCH v1 7/7] i2c: npcm: fix checkpatch warp5tw 6 siblings, 0 replies; 12+ messages in thread From: warp5tw @ 2024-08-07 10:02 UTC (permalink / raw) To: tali.perry, Avi.Fishman, tomer.maimon, avifishman70, tmaimon77, tali.perry1, venture, yuenn, benjaminfair, andi.shyti, wsa+renesas, rand.sec96, kwliu, jjliu0, kfting, warp5tw Cc: linux-i2c, linux-kernel, openbmc, Charles Boyer, Vivekanand Veeracholan From: Charles Boyer <Charles.Boyer@fii-usa.com> Nuvoton slave enable was in user space API call master_xfer, so it is subject to delays from the OS scheduler. If the BMC is not enabled for slave mode in time for master to send response, then it will NAK the address match. Then the PLDM request timeout occurs. If the slave enable is moved to the EOB interrupt service routine, then the BMC can be ready in slave mode by the time it needs to receive a response. Signed-off-by: Charles Boyer <Charles.Boyer@fii-usa.com> Signed-off-by: Vivekanand Veeracholan <vveerach@google.com> Signed-off-by: Tyrone Ting <kfting@nuvoton.com> --- drivers/i2c/busses/i2c-npcm7xx.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/i2c/busses/i2c-npcm7xx.c b/drivers/i2c/busses/i2c-npcm7xx.c index 1537fb7baa8c..1af6a927b9c1 100644 --- a/drivers/i2c/busses/i2c-npcm7xx.c +++ b/drivers/i2c/busses/i2c-npcm7xx.c @@ -1779,6 +1779,12 @@ static int npcm_i2c_int_master_handler(struct npcm_i2c *bus) (FIELD_GET(NPCM_I2CCST3_EO_BUSY, ioread8(bus->reg + NPCM_I2CCST3)))) { npcm_i2c_irq_handle_eob(bus); +#if IS_ENABLED(CONFIG_I2C_SLAVE) + /* reenable slave if it was enabled */ + if (bus->slave) + iowrite8((bus->slave->addr & 0x7F) | NPCM_I2CADDR_SAEN, + bus->reg + NPCM_I2CADDR1); +#endif return 0; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v1 7/7] i2c: npcm: fix checkpatch 2024-08-07 10:02 [PATCH v1 0/7] i2c: npcm: Bug fixes read/write operation, checkpatch warp5tw ` (5 preceding siblings ...) 2024-08-07 10:02 ` [PATCH v1 6/7] i2c-npcm7xx.c: Enable slave in eob interrupt warp5tw @ 2024-08-07 10:02 ` warp5tw 2024-08-09 6:50 ` Andrew Jeffery 6 siblings, 1 reply; 12+ messages in thread From: warp5tw @ 2024-08-07 10:02 UTC (permalink / raw) To: tali.perry, Avi.Fishman, tomer.maimon, avifishman70, tmaimon77, tali.perry1, venture, yuenn, benjaminfair, andi.shyti, wsa+renesas, rand.sec96, kwliu, jjliu0, kfting, warp5tw Cc: linux-i2c, linux-kernel, openbmc From: Tyrone Ting <kfting@nuvoton.com> Fix checkpatch warning. Fixes: 48acf8292280 ("i2c: Remove redundant comparison in npcm_i2c_reg_slave") Signed-off-by: Tyrone Ting <kfting@nuvoton.com> --- drivers/i2c/busses/i2c-npcm7xx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-npcm7xx.c b/drivers/i2c/busses/i2c-npcm7xx.c index 1af6a927b9c1..dbe652d628ee 100644 --- a/drivers/i2c/busses/i2c-npcm7xx.c +++ b/drivers/i2c/busses/i2c-npcm7xx.c @@ -1783,7 +1783,7 @@ static int npcm_i2c_int_master_handler(struct npcm_i2c *bus) /* reenable slave if it was enabled */ if (bus->slave) iowrite8((bus->slave->addr & 0x7F) | NPCM_I2CADDR_SAEN, - bus->reg + NPCM_I2CADDR1); + bus->reg + NPCM_I2CADDR1); #endif return 0; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v1 7/7] i2c: npcm: fix checkpatch 2024-08-07 10:02 ` [PATCH v1 7/7] i2c: npcm: fix checkpatch warp5tw @ 2024-08-09 6:50 ` Andrew Jeffery 2024-08-09 7:17 ` Tyrone Ting 0 siblings, 1 reply; 12+ messages in thread From: Andrew Jeffery @ 2024-08-09 6:50 UTC (permalink / raw) To: warp5tw, tali.perry, Avi.Fishman, tomer.maimon, avifishman70, tmaimon77, tali.perry1, venture, yuenn, benjaminfair, andi.shyti, wsa+renesas, rand.sec96, kwliu, jjliu0, kfting Cc: openbmc, linux-i2c, linux-kernel Hello, On Wed, 2024-08-07 at 18:02 +0800, warp5tw@gmail.com wrote: > From: Tyrone Ting <kfting@nuvoton.com> > > Fix checkpatch warning. > > Fixes: 48acf8292280 ("i2c: Remove redundant comparison in npcm_i2c_reg_slave") > Signed-off-by: Tyrone Ting <kfting@nuvoton.com> > --- > drivers/i2c/busses/i2c-npcm7xx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/i2c/busses/i2c-npcm7xx.c b/drivers/i2c/busses/i2c-npcm7xx.c > index 1af6a927b9c1..dbe652d628ee 100644 > --- a/drivers/i2c/busses/i2c-npcm7xx.c > +++ b/drivers/i2c/busses/i2c-npcm7xx.c > @@ -1783,7 +1783,7 @@ static int npcm_i2c_int_master_handler(struct npcm_i2c *bus) > /* reenable slave if it was enabled */ > if (bus->slave) > iowrite8((bus->slave->addr & 0x7F) | NPCM_I2CADDR_SAEN, > - bus->reg + NPCM_I2CADDR1); > + bus->reg + NPCM_I2CADDR1); > #endif > return 0; > } Fixing checkpatch warnings means you need to modify the commit that checkpatch identified as having problems, not just add a fix-up patch on top. It looks like this change should be squashed into the patch before it. Andrew ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 7/7] i2c: npcm: fix checkpatch 2024-08-09 6:50 ` Andrew Jeffery @ 2024-08-09 7:17 ` Tyrone Ting 0 siblings, 0 replies; 12+ messages in thread From: Tyrone Ting @ 2024-08-09 7:17 UTC (permalink / raw) To: Andrew Jeffery Cc: tali.perry, Avi.Fishman, tomer.maimon, avifishman70, tmaimon77, tali.perry1, venture, yuenn, benjaminfair, andi.shyti, wsa+renesas, rand.sec96, kwliu, jjliu0, kfting, openbmc, linux-i2c, linux-kernel Hi Andrew: Andrew Jeffery <andrew@codeconstruct.com.au> 於 2024年8月9日 週五 下午2:50寫道： > > Hello, > > On Wed, 2024-08-07 at 18:02 +0800, warp5tw@gmail.com wrote: > > From: Tyrone Ting <kfting@nuvoton.com> > > > > Fix checkpatch warning. > > > > Fixes: 48acf8292280 ("i2c: Remove redundant comparison in npcm_i2c_reg_slave") > > Signed-off-by: Tyrone Ting <kfting@nuvoton.com> > > --- > > drivers/i2c/busses/i2c-npcm7xx.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/i2c/busses/i2c-npcm7xx.c b/drivers/i2c/busses/i2c-npcm7xx.c > > index 1af6a927b9c1..dbe652d628ee 100644 > > --- a/drivers/i2c/busses/i2c-npcm7xx.c > > +++ b/drivers/i2c/busses/i2c-npcm7xx.c > > @@ -1783,7 +1783,7 @@ static int npcm_i2c_int_master_handler(struct npcm_i2c *bus) > > /* reenable slave if it was enabled */ > > if (bus->slave) > > iowrite8((bus->slave->addr & 0x7F) | NPCM_I2CADDR_SAEN, > > - bus->reg + NPCM_I2CADDR1); > > + bus->reg + NPCM_I2CADDR1); > > #endif > > return 0; > > } > > Fixing checkpatch warnings means you need to modify the commit that > checkpatch identified as having problems, not just add a fix-up patch > on top. > > It looks like this change should be squashed into the patch before it. Got it, thank you for your comments. > > Andrew > Regards, Tyrone ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2024-08-13 7:58 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-07 10:02 [PATCH v1 0/7] i2c: npcm: Bug fixes read/write operation, checkpatch warp5tw 2024-08-07 10:02 ` [PATCH v1 1/7] i2c: npcm: correct the read/write operation procedure warp5tw 2024-08-07 10:02 ` [PATCH v1 2/7] i2c: npcm: use a software flag to indicate a BER condition warp5tw 2024-08-07 10:02 ` [PATCH v1 3/7] i2c: npcm: Modify timeout evaluation mechanism warp5tw 2024-08-07 10:02 ` [PATCH v1 4/7] i2c: npcm: Modify the client address assignment warp5tw 2024-08-07 10:02 ` [PATCH v1 5/7] drivers: i2c: use i2c frequency table warp5tw 2024-08-07 19:55 ` kernel test robot 2024-08-13 7:58 ` Dan Carpenter 2024-08-07 10:02 ` [PATCH v1 6/7] i2c-npcm7xx.c: Enable slave in eob interrupt warp5tw 2024-08-07 10:02 ` [PATCH v1 7/7] i2c: npcm: fix checkpatch warp5tw 2024-08-09 6:50 ` Andrew Jeffery 2024-08-09 7:17 ` Tyrone Ting
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox