Skip to content

Commit 518edab

Browse files
Artem ShimkoAndi Shyti
authored andcommitted
i2c: designware: Replace magic numbers with named constants
Replace various magic numbers with properly named constants to improve code readability and maintainability. This includes constants for register access, timing adjustments, timeouts, FIFO parameters, and default values. This makes the code more self-documenting without altering any functionality. Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com> Signed-off-by: Andi Shyti <andi.shyti@kernel.org> Link: https://lore.kernel.org/r/20251211122947.1469666-1-a.shimko.dev@gmail.com
1 parent f6551f7 commit 518edab

2 files changed

Lines changed: 31 additions & 11 deletions

File tree

drivers/i2c/busses/i2c-designware-common.c

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#define DEFAULT_SYMBOL_NAMESPACE "I2C_DW_COMMON"
1313

1414
#include <linux/acpi.h>
15+
#include <linux/bitfield.h>
1516
#include <linux/clk.h>
1617
#include <linux/delay.h>
1718
#include <linux/device.h>
@@ -34,6 +35,10 @@
3435

3536
#include "i2c-designware-core.h"
3637

38+
#define DW_IC_DEFAULT_BUS_CAPACITANCE_pF 100
39+
#define DW_IC_ABORT_TIMEOUT_US 10
40+
#define DW_IC_BUSY_POLL_TIMEOUT_US (1 * USEC_PER_MSEC)
41+
3742
static const char *const abort_sources[] = {
3843
[ABRT_7B_ADDR_NOACK] =
3944
"slave address not acknowledged (7bit mode)",
@@ -106,7 +111,7 @@ static int dw_reg_read_word(void *context, unsigned int reg, unsigned int *val)
106111
struct dw_i2c_dev *dev = context;
107112

108113
*val = readw(dev->base + reg) |
109-
(readw(dev->base + reg + 2) << 16);
114+
(readw(dev->base + reg + DW_IC_REG_STEP_BYTES) << DW_IC_REG_WORD_SHIFT);
110115

111116
return 0;
112117
}
@@ -116,7 +121,7 @@ static int dw_reg_write_word(void *context, unsigned int reg, unsigned int val)
116121
struct dw_i2c_dev *dev = context;
117122

118123
writew(val, dev->base + reg);
119-
writew(val >> 16, dev->base + reg + 2);
124+
writew(val >> DW_IC_REG_WORD_SHIFT, dev->base + reg + DW_IC_REG_STEP_BYTES);
120125

121126
return 0;
122127
}
@@ -165,7 +170,7 @@ int i2c_dw_init_regmap(struct dw_i2c_dev *dev)
165170
if (reg == swab32(DW_IC_COMP_TYPE_VALUE)) {
166171
map_cfg.reg_read = dw_reg_read_swab;
167172
map_cfg.reg_write = dw_reg_write_swab;
168-
} else if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) {
173+
} else if (reg == lower_16_bits(DW_IC_COMP_TYPE_VALUE)) {
169174
map_cfg.reg_read = dw_reg_read_word;
170175
map_cfg.reg_write = dw_reg_write_word;
171176
} else if (reg != DW_IC_COMP_TYPE_VALUE) {
@@ -384,7 +389,7 @@ int i2c_dw_fw_parse_and_configure(struct dw_i2c_dev *dev)
384389
i2c_parse_fw_timings(device, t, false);
385390

386391
if (device_property_read_u32(device, "snps,bus-capacitance-pf", &dev->bus_capacitance_pF))
387-
dev->bus_capacitance_pF = 100;
392+
dev->bus_capacitance_pF = DW_IC_DEFAULT_BUS_CAPACITANCE_pF;
388393

389394
dev->clk_freq_optimized = device_property_read_bool(device, "snps,clk-freq-optimized");
390395

@@ -539,8 +544,9 @@ void __i2c_dw_disable(struct dw_i2c_dev *dev)
539544

540545
regmap_write(dev->map, DW_IC_ENABLE, enable | DW_IC_ENABLE_ABORT);
541546
ret = regmap_read_poll_timeout(dev->map, DW_IC_ENABLE, enable,
542-
!(enable & DW_IC_ENABLE_ABORT), 10,
543-
100);
547+
!(enable & DW_IC_ENABLE_ABORT),
548+
DW_IC_ABORT_TIMEOUT_US,
549+
10 * DW_IC_ABORT_TIMEOUT_US);
544550
if (ret)
545551
dev_err(dev->dev, "timeout while trying to abort current transfer\n");
546552
}
@@ -552,7 +558,7 @@ void __i2c_dw_disable(struct dw_i2c_dev *dev)
552558
* in that case this test reads zero and exits the loop.
553559
*/
554560
regmap_read(dev->map, DW_IC_ENABLE_STATUS, &status);
555-
if ((status & 1) == 0)
561+
if (!(status & 1))
556562
return;
557563

558564
/*
@@ -635,7 +641,8 @@ int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev)
635641

636642
ret = regmap_read_poll_timeout(dev->map, DW_IC_STATUS, status,
637643
!(status & DW_IC_STATUS_ACTIVITY),
638-
1100, 20000);
644+
DW_IC_BUSY_POLL_TIMEOUT_US,
645+
20 * DW_IC_BUSY_POLL_TIMEOUT_US);
639646
if (ret) {
640647
dev_warn(dev->dev, "timeout waiting for bus ready\n");
641648

@@ -699,12 +706,12 @@ int i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
699706
if (ret)
700707
return ret;
701708

702-
tx_fifo_depth = ((param >> 16) & 0xff) + 1;
703-
rx_fifo_depth = ((param >> 8) & 0xff) + 1;
709+
tx_fifo_depth = FIELD_GET(DW_IC_FIFO_TX_FIELD, param) + 1;
710+
rx_fifo_depth = FIELD_GET(DW_IC_FIFO_RX_FIELD, param) + 1;
704711
if (!dev->tx_fifo_depth) {
705712
dev->tx_fifo_depth = tx_fifo_depth;
706713
dev->rx_fifo_depth = rx_fifo_depth;
707-
} else if (tx_fifo_depth >= 2) {
714+
} else if (tx_fifo_depth >= DW_IC_FIFO_MIN_DEPTH) {
708715
dev->tx_fifo_depth = min_t(u32, dev->tx_fifo_depth,
709716
tx_fifo_depth);
710717
dev->rx_fifo_depth = min_t(u32, dev->rx_fifo_depth,

drivers/i2c/busses/i2c-designware-core.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,19 @@
4141
#define DW_IC_DATA_CMD_DAT GENMASK(7, 0)
4242
#define DW_IC_DATA_CMD_FIRST_DATA_BYTE BIT(11)
4343

44+
/*
45+
* Register access parameters
46+
*/
47+
#define DW_IC_REG_STEP_BYTES 2
48+
#define DW_IC_REG_WORD_SHIFT 16
49+
50+
/*
51+
* FIFO depth configuration
52+
*/
53+
#define DW_IC_FIFO_TX_FIELD GENMASK(23, 16)
54+
#define DW_IC_FIFO_RX_FIELD GENMASK(15, 8)
55+
#define DW_IC_FIFO_MIN_DEPTH 2
56+
4457
/*
4558
* Registers offset
4659
*/

0 commit comments

Comments
 (0)