* [PATCH 0/3] i2c: designware: Add DWC_i2c support
@ 2026-09-19 9:06 Aniket Limaye
2026-09-19 9:06 ` [PATCH 1/3] dt-bindings: i2c: dw: Add DWC_i2c compatible Aniket Limaye
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Aniket Limaye @ 2026-09-19 9:06 UTC (permalink / raw)
To: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mika Westerberg, Nirujogi Pratap, Bin Du, Andy Shevchenko
Cc: linux-i2c, devicetree, linux-kernel, vigneshr, nm, u-kumar1,
lianfeng.ouyang, Aniket Limaye, Ritwick.Sharma
Add new compatible and update driver to support Synopsys Advanced I2C
Controller (DWC_i2c [0]). This is needed since this controller differs
from the existing designware i2c (DW_apb_i2c [1]) in its register
offsets and some register definitions.
[0]: DWC_i2c_reference.pdf
[1]: DW_apb_i2c_databook.pdf
The new register offsets are handled by first refactoring the driver to
use a map of register IDs to their offsets, and then match the
compatible to the respective offset-map. Similarly, also update the
driver to use an updated CON-register definition.
The new compatible also updates driver logic due to changes in register
definitions:
- Interrupts are acknowledged by writing a bitmask to a single CLR_INTR
register instead of reading N dedicated CLR_* registers;
i2c_dw_ack_intr() picks the right method based on dev->flags.
- One HCNT/LCNT register pair is shared between standard and fast speed
instead of having one pair each; i2c_dw_write_timings() writes
whichever value set matches the configured speed.
- No COMP_PARAM_1 register, so FIFO depth and high-speed-mode support
can't be autodetected: FIFO depth now comes from the required
snps,tx-fifo-depth/snps,rx-fifo-depth DT properties, and the
high-speed capability check is skipped.
- No defined CON.RESTART_EN bit; treat it as always set.
The register offsets used for the new DWC_i2c are those present on TI
TDA54 SoC.
Signed-off-by: Aniket Limaye <a-limaye@ti.com>
---
Aniket Limaye (3):
dt-bindings: i2c: dw: Add DWC_i2c compatible
i2c: designware: Introduce per-variant register offset and bit-layout tables
i2c: designware: Add snps,dwc-i2c support and new compatible
.../bindings/i2c/snps,designware-i2c.yaml | 33 +++
drivers/i2c/busses/i2c-designware-amdisp.c | 1 +
drivers/i2c/busses/i2c-designware-common.c | 281 ++++++++++++++++++---
drivers/i2c/busses/i2c-designware-core.h | 130 +++++++++-
drivers/i2c/busses/i2c-designware-master.c | 147 ++++++-----
drivers/i2c/busses/i2c-designware-pcidrv.c | 2 +
drivers/i2c/busses/i2c-designware-platdrv.c | 3 +
drivers/i2c/busses/i2c-designware-slave.c | 42 +--
8 files changed, 507 insertions(+), 132 deletions(-)
---
base-commit: 587858367581b9c55c3690f4e63382ad622719d4
change-id: 20260919-tda54-upstream-i2c-d0c67f16b4fc
Best regards,
--
Aniket Limaye <a-limaye@ti.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] dt-bindings: i2c: dw: Add DWC_i2c compatible
2026-09-19 9:06 [PATCH 0/3] i2c: designware: Add DWC_i2c support Aniket Limaye
@ 2026-09-19 9:06 ` Aniket Limaye
2026-09-20 18:24 ` Krzysztof Kozlowski
2026-09-19 9:06 ` [PATCH 2/3] i2c: designware: Introduce per-variant register offset and bit-layout tables Aniket Limaye
2026-09-19 9:06 ` [PATCH 3/3] i2c: designware: Add snps,dwc-i2c support and new compatible Aniket Limaye
2 siblings, 1 reply; 16+ messages in thread
From: Aniket Limaye @ 2026-09-19 9:06 UTC (permalink / raw)
To: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mika Westerberg, Nirujogi Pratap, Bin Du, Andy Shevchenko
Cc: linux-i2c, devicetree, linux-kernel, vigneshr, nm, u-kumar1,
lianfeng.ouyang, Aniket Limaye, Ritwick.Sharma
Add the "snps,dwc-i2c" compatible for Synopsys Advanced I2C Controller
referred as DWC_i2c. Compared to the existing "snps,designware-i2c"
(DW_apb_i2c), it broadly differs in its register offsets and some bit
fields, requiring changes to interrupt handling, timing handling, etc.
Unlike DW_apb_i2c, this IP has no COMP_PARAM_1 register to autodetect
the FIFO depth from, so add snps,tx-fifo-depth and
snps,rx-fifo-depth as required properties.
Signed-off-by: Aniket Limaye <a-limaye@ti.com>
---
.../bindings/i2c/snps,designware-i2c.yaml | 33 ++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
index 467bdcbb8538..f56272a45096 100644
--- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
@@ -21,12 +21,23 @@ allOf:
properties:
reg:
maxItems: 1
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: snps,dwc-i2c
+ then:
+ required:
+ - snps,tx-fifo-depth
+ - snps,rx-fifo-depth
properties:
compatible:
oneOf:
- description: Generic Synopsys DesignWare I2C controller
const: snps,designware-i2c
+ - description: Synopsys Advanced I2C/SMBus/PMBus Controller
+ const: snps,dwc-i2c
- description: Renesas RZ/N1D I2C controller
items:
- const: renesas,r9a06g032-i2c # RZ/N1D
@@ -121,6 +132,18 @@ properties:
low period of SCL line.
type: boolean
+ snps,tx-fifo-depth:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ The depth of the hardware TX FIFO. Required on the snps,dwc-i2c IP
+ variant, where fifo depth cannot be autodetected.
+
+ snps,rx-fifo-depth:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ The depth of the hardware RX FIFO. Required on the snps,dwc-i2c IP
+ variant, where fifo depth cannot be autodetected.
+
unevaluatedProperties: false
required:
@@ -172,4 +195,14 @@ examples:
interrupts = <8>;
clocks = <&ahb_clk>;
};
+ - |
+ i2c@53b00000 {
+ compatible = "snps,dwc-i2c";
+ reg = <0x53b00000 0x1000>;
+ interrupts = <166>;
+ clocks = <&sysclk>;
+ clock-frequency = <100000>;
+ snps,tx-fifo-depth = <32>;
+ snps,rx-fifo-depth = <32>;
+ };
...
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] i2c: designware: Introduce per-variant register offset and bit-layout tables
2026-09-19 9:06 [PATCH 0/3] i2c: designware: Add DWC_i2c support Aniket Limaye
2026-09-19 9:06 ` [PATCH 1/3] dt-bindings: i2c: dw: Add DWC_i2c compatible Aniket Limaye
@ 2026-09-19 9:06 ` Aniket Limaye
2026-09-21 11:11 ` Mika Westerberg
2026-09-19 9:06 ` [PATCH 3/3] i2c: designware: Add snps,dwc-i2c support and new compatible Aniket Limaye
2 siblings, 1 reply; 16+ messages in thread
From: Aniket Limaye @ 2026-09-19 9:06 UTC (permalink / raw)
To: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mika Westerberg, Nirujogi Pratap, Bin Du, Andy Shevchenko
Cc: linux-i2c, devicetree, linux-kernel, vigneshr, nm, u-kumar1,
lianfeng.ouyang, Aniket Limaye, Ritwick.Sharma
Every DW_IC_* register offset and CON-register bit position is currently
baked in as a compile-time constant, which only works while there is a
single register layout. Introduce a logical register-ID enum (enum
dw_i2c_reg_idx) plus a per-variant offset table (dev->regs[]) and a
per-variant CON-register bit-layout descriptor (dev->con_bits), selected
at probe time via the new i2c_dw_select_variant().
Replace every direct DW_IC_* offset/bit-position reference with a lookup
through dev->regs[]/dev->con_bits. Also fold the read-to-clear
interrupt-acknowledgment pattern into a new i2c_dw_ack_intr() helper,
driven by a per-variant dev->intr_clr[] table.
Only one variant exists at this point (DW_apb_i2c), so this is a
mechanical, behavior-preserving change: the values in
dw_i2c_reg_offsets[] and dw_i2c_con_bits match the DW_IC_* macros
exactly. It lays the groundwork for adding a second register layout
(DWC_i2c) without duplicating the whole driver.
Signed-off-by: Aniket Limaye <a-limaye@ti.com>
---
drivers/i2c/busses/i2c-designware-amdisp.c | 1 +
drivers/i2c/busses/i2c-designware-common.c | 158 ++++++++++++++++++++++------
drivers/i2c/busses/i2c-designware-core.h | 108 ++++++++++++++++++-
drivers/i2c/busses/i2c-designware-master.c | 125 +++++++++++-----------
drivers/i2c/busses/i2c-designware-pcidrv.c | 2 +
drivers/i2c/busses/i2c-designware-platdrv.c | 2 +
drivers/i2c/busses/i2c-designware-slave.c | 42 ++++----
7 files changed, 318 insertions(+), 120 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-amdisp.c b/drivers/i2c/busses/i2c-designware-amdisp.c
index 9f0ec0fae6f2..f7aa075c9977 100644
--- a/drivers/i2c/busses/i2c-designware-amdisp.c
+++ b/drivers/i2c/busses/i2c-designware-amdisp.c
@@ -46,6 +46,7 @@ static int amd_isp_dw_i2c_plat_probe(struct platform_device *pdev)
isp_i2c_dev->flags |= ACCESS_POLLING;
platform_set_drvdata(pdev, isp_i2c_dev);
+ i2c_dw_select_variant(isp_i2c_dev);
isp_i2c_dev->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(isp_i2c_dev->base))
return dev_err_probe(&pdev->dev, PTR_ERR(isp_i2c_dev->base),
diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index a1eca6cd4b75..a21aeb7f415a 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -72,6 +72,95 @@ static const char *const abort_sources[] = {
"incorrect slave-transmitter mode configuration",
};
+/* "snps,designware-i2c" flat register layout */
+static const u32 dw_i2c_reg_offsets[DW_REG_IDX_MAX] = {
+ [DW_REG_IDX_CON] = DW_IC_CON,
+ [DW_REG_IDX_TAR] = DW_IC_TAR,
+ [DW_REG_IDX_SAR] = DW_IC_SAR,
+ [DW_REG_IDX_DATA_CMD] = DW_IC_DATA_CMD,
+ [DW_REG_IDX_SS_SCL_HCNT] = DW_IC_SS_SCL_HCNT,
+ [DW_REG_IDX_SS_SCL_LCNT] = DW_IC_SS_SCL_LCNT,
+ [DW_REG_IDX_FS_SCL_HCNT] = DW_IC_FS_SCL_HCNT,
+ [DW_REG_IDX_FS_SCL_LCNT] = DW_IC_FS_SCL_LCNT,
+ [DW_REG_IDX_HS_SCL_HCNT] = DW_IC_HS_SCL_HCNT,
+ [DW_REG_IDX_HS_SCL_LCNT] = DW_IC_HS_SCL_LCNT,
+ [DW_REG_IDX_INTR_STAT] = DW_IC_INTR_STAT,
+ [DW_REG_IDX_INTR_MASK] = DW_IC_INTR_MASK,
+ [DW_REG_IDX_RAW_INTR_STAT] = DW_IC_RAW_INTR_STAT,
+ [DW_REG_IDX_RX_TL] = DW_IC_RX_TL,
+ [DW_REG_IDX_TX_TL] = DW_IC_TX_TL,
+ [DW_REG_IDX_CLR_INTR] = DW_IC_CLR_INTR,
+ [DW_REG_IDX_CLR_RX_UNDER] = DW_IC_CLR_RX_UNDER,
+ [DW_REG_IDX_CLR_RX_OVER] = DW_IC_CLR_RX_OVER,
+ [DW_REG_IDX_CLR_TX_OVER] = DW_IC_CLR_TX_OVER,
+ [DW_REG_IDX_CLR_RD_REQ] = DW_IC_CLR_RD_REQ,
+ [DW_REG_IDX_CLR_TX_ABRT] = DW_IC_CLR_TX_ABRT,
+ [DW_REG_IDX_CLR_RX_DONE] = DW_IC_CLR_RX_DONE,
+ [DW_REG_IDX_CLR_ACTIVITY] = DW_IC_CLR_ACTIVITY,
+ [DW_REG_IDX_CLR_STOP_DET] = DW_IC_CLR_STOP_DET,
+ [DW_REG_IDX_CLR_START_DET] = DW_IC_CLR_START_DET,
+ [DW_REG_IDX_CLR_GEN_CALL] = DW_IC_CLR_GEN_CALL,
+ [DW_REG_IDX_ENABLE] = DW_IC_ENABLE,
+ [DW_REG_IDX_STATUS] = DW_IC_STATUS,
+ [DW_REG_IDX_TXFLR] = DW_IC_TXFLR,
+ [DW_REG_IDX_RXFLR] = DW_IC_RXFLR,
+ [DW_REG_IDX_SDA_HOLD] = DW_IC_SDA_HOLD,
+ [DW_REG_IDX_TX_ABRT_SOURCE] = DW_IC_TX_ABRT_SOURCE,
+ [DW_REG_IDX_ENABLE_STATUS] = DW_IC_ENABLE_STATUS,
+ [DW_REG_IDX_SMBUS_INTR_MASK] = DW_IC_SMBUS_INTR_MASK,
+ [DW_REG_IDX_COMP_PARAM_1] = DW_IC_COMP_PARAM_1,
+ [DW_REG_IDX_COMP_VERSION] = DW_IC_COMP_VERSION,
+ [DW_REG_IDX_COMP_TYPE] = DW_IC_COMP_TYPE,
+};
+
+static const struct dw_i2c_con_bits dw_i2c_con_bits = {
+ .master = DW_IC_CON_MASTER,
+ .speed_std = DW_IC_CON_SPEED_STD,
+ .speed_fast = DW_IC_CON_SPEED_FAST,
+ .speed_high = DW_IC_CON_SPEED_HIGH,
+ .speed_mask = DW_IC_CON_SPEED_MASK,
+ .bit10_slave = DW_IC_CON_10BITADDR_SLAVE,
+ .bit10_master = DW_IC_CON_10BITADDR_MASTER,
+ .restart_en = DW_IC_CON_RESTART_EN,
+ .slave_disable = DW_IC_CON_SLAVE_DISABLE,
+ .stop_det_ifaddressed = DW_IC_CON_STOP_DET_IFADDRESSED,
+ .tx_empty_ctrl = DW_IC_CON_TX_EMPTY_CTRL,
+ .rx_fifo_full_hld_ctrl = DW_IC_CON_RX_FIFO_FULL_HLD_CTRL,
+ .bus_clear_ctrl = DW_IC_CON_BUS_CLEAR_CTRL,
+};
+
+/* "snps,designware-i2c": dedicated read-to-clear register ID per logical interrupt */
+static const u32 dw_i2c_intr_clr[DW_INTR_IDX_MAX] = {
+ [DW_INTR_IDX_ALL] = DW_REG_IDX_CLR_INTR,
+ [DW_INTR_IDX_RX_UNDER] = DW_REG_IDX_CLR_RX_UNDER,
+ [DW_INTR_IDX_RX_OVER] = DW_REG_IDX_CLR_RX_OVER,
+ [DW_INTR_IDX_TX_OVER] = DW_REG_IDX_CLR_TX_OVER,
+ [DW_INTR_IDX_RD_REQ] = DW_REG_IDX_CLR_RD_REQ,
+ [DW_INTR_IDX_TX_ABRT] = DW_REG_IDX_CLR_TX_ABRT,
+ [DW_INTR_IDX_RX_DONE] = DW_REG_IDX_CLR_RX_DONE,
+ [DW_INTR_IDX_ACTIVITY] = DW_REG_IDX_CLR_ACTIVITY,
+ [DW_INTR_IDX_STOP_DET] = DW_REG_IDX_CLR_STOP_DET,
+ [DW_INTR_IDX_START_DET] = DW_REG_IDX_CLR_START_DET,
+ [DW_INTR_IDX_GEN_CALL] = DW_REG_IDX_CLR_GEN_CALL,
+};
+
+/**
+ * i2c_dw_select_variant() - Pick the register offset table, CON-register bit
+ * layout and interrupt-ack mapping matching this device's IP variant
+ * @dev: device private data
+ *
+ * Must be called after dev->flags has been populated from
+ * device_get_match_data()/ACPI id data, and before any register access
+ * (including i2c_dw_init_regmap()).
+ */
+void i2c_dw_select_variant(struct dw_i2c_dev *dev)
+{
+ dev->regs = dw_i2c_reg_offsets;
+ dev->con_bits = &dw_i2c_con_bits;
+ dev->intr_clr = dw_i2c_intr_clr;
+}
+EXPORT_SYMBOL_GPL(i2c_dw_select_variant);
+
static int dw_reg_read(void *context, unsigned int reg, unsigned int *val)
{
struct dw_i2c_dev *dev = context;
@@ -147,7 +236,7 @@ static int i2c_dw_init_regmap(struct dw_i2c_dev *dev)
.disable_locking = true,
.reg_read = dw_reg_read,
.reg_write = dw_reg_write,
- .max_register = DW_IC_COMP_TYPE,
+ .max_register = dev->regs[DW_REG_IDX_COMP_TYPE],
};
u32 reg;
int ret;
@@ -163,7 +252,7 @@ static int i2c_dw_init_regmap(struct dw_i2c_dev *dev)
if (ret)
return ret;
- reg = readl(dev->base + DW_IC_COMP_TYPE);
+ reg = readl(dev->base + dev->regs[DW_REG_IDX_COMP_TYPE]);
i2c_dw_release_lock(dev);
if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU)
@@ -365,17 +454,17 @@ static void i2c_dw_configure_mode(struct dw_i2c_dev *dev, int mode)
{
switch (mode) {
case DW_IC_MASTER:
- regmap_write(dev->map, DW_IC_TX_TL, dev->tx_fifo_depth / 2);
- regmap_write(dev->map, DW_IC_RX_TL, 0);
- regmap_write(dev->map, DW_IC_CON, dev->master_cfg);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_TX_TL], dev->tx_fifo_depth / 2);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_RX_TL], 0);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_CON], dev->master_cfg);
break;
case DW_IC_SLAVE:
dev->status = 0;
- regmap_write(dev->map, DW_IC_TX_TL, 0);
- regmap_write(dev->map, DW_IC_RX_TL, 0);
- regmap_write(dev->map, DW_IC_CON, dev->slave_cfg);
- regmap_write(dev->map, DW_IC_SAR, dev->slave->addr);
- regmap_write(dev->map, DW_IC_INTR_MASK, DW_IC_INTR_SLAVE_MASK);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_TX_TL], 0);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_RX_TL], 0);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_CON], dev->slave_cfg);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_SAR], dev->slave->addr);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_INTR_MASK], DW_IC_INTR_SLAVE_MASK);
__i2c_dw_enable(dev);
break;
default:
@@ -387,16 +476,16 @@ static void i2c_dw_configure_mode(struct dw_i2c_dev *dev, int mode)
static void i2c_dw_write_timings(struct dw_i2c_dev *dev)
{
/* Write standard speed timing parameters */
- regmap_write(dev->map, DW_IC_SS_SCL_HCNT, dev->ss_hcnt);
- regmap_write(dev->map, DW_IC_SS_SCL_LCNT, dev->ss_lcnt);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_SS_SCL_HCNT], dev->ss_hcnt);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_SS_SCL_LCNT], dev->ss_lcnt);
/* Write fast mode/fast mode plus timing parameters */
- regmap_write(dev->map, DW_IC_FS_SCL_HCNT, dev->fs_hcnt);
- regmap_write(dev->map, DW_IC_FS_SCL_LCNT, dev->fs_lcnt);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_FS_SCL_HCNT], dev->fs_hcnt);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_FS_SCL_LCNT], dev->fs_lcnt);
/* Write high speed timing parameters */
- regmap_write(dev->map, DW_IC_HS_SCL_HCNT, dev->hs_hcnt);
- regmap_write(dev->map, DW_IC_HS_SCL_LCNT, dev->hs_lcnt);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_HS_SCL_HCNT], dev->hs_hcnt);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_HS_SCL_LCNT], dev->hs_lcnt);
}
/**
@@ -448,13 +537,13 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
* firmware that leaves IC_SMBUS=1; the handler never
* services them.
*/
- regmap_write(dev->map, DW_IC_SMBUS_INTR_MASK, 0);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_SMBUS_INTR_MASK], 0);
i2c_dw_write_timings(dev);
/* Write SDA hold time if supported */
if (dev->sda_hold_time)
- regmap_write(dev->map, DW_IC_SDA_HOLD, dev->sda_hold_time);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_SDA_HOLD], dev->sda_hold_time);
i2c_dw_configure_mode(dev, dev->mode);
@@ -579,14 +668,14 @@ static int i2c_dw_set_sda_hold(struct dw_i2c_dev *dev)
return ret;
/* Configure SDA Hold Time if required */
- ret = regmap_read(dev->map, DW_IC_COMP_VERSION, ®);
+ ret = regmap_read(dev->map, dev->regs[DW_REG_IDX_COMP_VERSION], ®);
if (ret)
goto err_release_lock;
if (reg >= DW_IC_SDA_HOLD_MIN_VERS) {
if (!dev->sda_hold_time) {
/* Keep previous hold time setting if no one set it */
- ret = regmap_read(dev->map, DW_IC_SDA_HOLD,
+ ret = regmap_read(dev->map, dev->regs[DW_REG_IDX_SDA_HOLD],
&dev->sda_hold_time);
if (ret)
goto err_release_lock;
@@ -629,9 +718,9 @@ void __i2c_dw_disable(struct dw_i2c_dev *dev)
unsigned int status;
int ret;
- regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &raw_intr_stats);
- regmap_read(dev->map, DW_IC_STATUS, &ic_stats);
- regmap_read(dev->map, DW_IC_ENABLE, &enable);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_RAW_INTR_STAT], &raw_intr_stats);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_STATUS], &ic_stats);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_ENABLE], &enable);
abort_needed = (raw_intr_stats & DW_IC_INTR_MST_ON_HOLD) ||
(ic_stats & DW_IC_STATUS_MASTER_HOLD_TX_FIFO_EMPTY);
@@ -645,7 +734,7 @@ void __i2c_dw_disable(struct dw_i2c_dev *dev)
if (abort_needed) {
if (!(enable & DW_IC_ENABLE_ENABLE)) {
- regmap_write(dev->map, DW_IC_ENABLE, DW_IC_ENABLE_ENABLE);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_ENABLE], DW_IC_ENABLE_ENABLE);
/*
* Wait 10 times the signaling period of the highest I2C
* transfer supported by the driver (for 400KHz this is
@@ -657,8 +746,8 @@ void __i2c_dw_disable(struct dw_i2c_dev *dev)
enable |= DW_IC_ENABLE_ENABLE;
}
- regmap_write(dev->map, DW_IC_ENABLE, enable | DW_IC_ENABLE_ABORT);
- ret = regmap_read_poll_timeout(dev->map, DW_IC_ENABLE, enable,
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_ENABLE], enable | DW_IC_ENABLE_ABORT);
+ ret = regmap_read_poll_timeout(dev->map, dev->regs[DW_REG_IDX_ENABLE], enable,
!(enable & DW_IC_ENABLE_ABORT),
DW_IC_ABORT_TIMEOUT_US,
10 * DW_IC_ABORT_TIMEOUT_US);
@@ -672,7 +761,7 @@ void __i2c_dw_disable(struct dw_i2c_dev *dev)
* The enable status register may be unimplemented, but
* in that case this test reads zero and exits the loop.
*/
- regmap_read(dev->map, DW_IC_ENABLE_STATUS, &status);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_ENABLE_STATUS], &status);
if (!(status & 1))
return;
@@ -754,7 +843,7 @@ int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev)
unsigned int status;
int ret;
- ret = regmap_read_poll_timeout(dev->map, DW_IC_STATUS, status,
+ ret = regmap_read_poll_timeout(dev->map, dev->regs[DW_REG_IDX_STATUS], status,
!(status & DW_IC_STATUS_ACTIVITY),
DW_IC_BUSY_POLL_TIMEOUT_US,
20 * DW_IC_BUSY_POLL_TIMEOUT_US);
@@ -763,7 +852,7 @@ int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev)
i2c_recover_bus(&dev->adapter);
- regmap_read(dev->map, DW_IC_STATUS, &status);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_STATUS], &status);
if (!(status & DW_IC_STATUS_ACTIVITY))
ret = 0;
}
@@ -816,7 +905,7 @@ static int i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
if (ret)
return ret;
- ret = regmap_read(dev->map, DW_IC_COMP_PARAM_1, ¶m);
+ ret = regmap_read(dev->map, dev->regs[DW_REG_IDX_COMP_PARAM_1], ¶m);
i2c_dw_release_lock(dev);
if (ret)
return ret;
@@ -845,7 +934,6 @@ u32 i2c_dw_func(struct i2c_adapter *adap)
void i2c_dw_disable(struct dw_i2c_dev *dev)
{
- unsigned int dummy;
int ret;
ret = i2c_dw_acquire_lock(dev);
@@ -857,7 +945,7 @@ void i2c_dw_disable(struct dw_i2c_dev *dev)
/* Disable all interrupts */
__i2c_dw_write_intr_mask(dev, 0);
- regmap_read(dev->map, DW_IC_CLR_INTR, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_ALL);
i2c_dw_release_lock(dev);
}
@@ -1054,9 +1142,9 @@ void i2c_dw_shutdown(struct dw_i2c_dev *dev)
* To quickly NACK the controller during shutdown, we set the target
* disable bit while the controller is still enabled.
*/
- regmap_read(dev->map, DW_IC_CON, &con);
- con |= DW_IC_CON_SLAVE_DISABLE;
- regmap_write(dev->map, DW_IC_CON, con);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_CON], &con);
+ con |= dev->con_bits->slave_disable;
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_CON], con);
i2c_dw_disable(dev);
}
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 2c929a6e8da2..2fad18582db7 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -43,6 +43,91 @@
#define DW_IC_SDA_HOLD_MIN_VERS 0x3131312A /* "111*" == v1.11* */
#define DW_IC_COMP_TYPE_VALUE 0x44570140 /* "DW" + 0x0140 */
+/*
+ * Logical register IDs. The physical offset backing each ID depends on the
+ * variant of the IP (selected via i2c_dw_select_variant() based on the
+ * device's compatible string) and is looked up at runtime through
+ * dev->regs[]. This lets a single set of driver code paths serve IP
+ * variants with different register layouts.
+ */
+enum dw_i2c_reg_idx {
+ DW_REG_IDX_CON,
+ DW_REG_IDX_TAR,
+ DW_REG_IDX_SAR,
+ DW_REG_IDX_DATA_CMD,
+ DW_REG_IDX_SS_SCL_HCNT,
+ DW_REG_IDX_SS_SCL_LCNT,
+ DW_REG_IDX_FS_SCL_HCNT,
+ DW_REG_IDX_FS_SCL_LCNT,
+ DW_REG_IDX_HS_SCL_HCNT,
+ DW_REG_IDX_HS_SCL_LCNT,
+ DW_REG_IDX_INTR_STAT,
+ DW_REG_IDX_INTR_MASK,
+ DW_REG_IDX_RAW_INTR_STAT,
+ DW_REG_IDX_RX_TL,
+ DW_REG_IDX_TX_TL,
+ DW_REG_IDX_CLR_INTR,
+ DW_REG_IDX_CLR_RX_UNDER,
+ DW_REG_IDX_CLR_RX_OVER,
+ DW_REG_IDX_CLR_TX_OVER,
+ DW_REG_IDX_CLR_RD_REQ,
+ DW_REG_IDX_CLR_TX_ABRT,
+ DW_REG_IDX_CLR_RX_DONE,
+ DW_REG_IDX_CLR_ACTIVITY,
+ DW_REG_IDX_CLR_STOP_DET,
+ DW_REG_IDX_CLR_START_DET,
+ DW_REG_IDX_CLR_GEN_CALL,
+ DW_REG_IDX_ENABLE,
+ DW_REG_IDX_STATUS,
+ DW_REG_IDX_TXFLR,
+ DW_REG_IDX_RXFLR,
+ DW_REG_IDX_SDA_HOLD,
+ DW_REG_IDX_TX_ABRT_SOURCE,
+ DW_REG_IDX_ENABLE_STATUS,
+ DW_REG_IDX_SMBUS_INTR_MASK,
+ DW_REG_IDX_COMP_PARAM_1,
+ DW_REG_IDX_COMP_VERSION,
+ DW_REG_IDX_COMP_TYPE,
+ DW_REG_IDX_MAX,
+};
+
+/*
+ * Bit positions within the CON register that could differ between IP
+ * variants. Values here match the DW_IC_CON_* macros in
+ * <linux/designware_i2c.h>.
+ */
+struct dw_i2c_con_bits {
+ u32 master;
+ u32 speed_std;
+ u32 speed_fast;
+ u32 speed_high;
+ u32 speed_mask;
+ u32 bit10_slave;
+ u32 bit10_master;
+ u32 restart_en;
+ u32 slave_disable;
+ u32 stop_det_ifaddressed;
+ u32 tx_empty_ctrl;
+ u32 rx_fifo_full_hld_ctrl;
+ u32 bus_clear_ctrl;
+};
+
+/* Logical interrupt IDs for i2c_dw_ack_intr(); DW_INTR_IDX_ALL = "current pending interrupt" */
+enum dw_i2c_intr_idx {
+ DW_INTR_IDX_ALL,
+ DW_INTR_IDX_RX_UNDER,
+ DW_INTR_IDX_RX_OVER,
+ DW_INTR_IDX_TX_OVER,
+ DW_INTR_IDX_RD_REQ,
+ DW_INTR_IDX_TX_ABRT,
+ DW_INTR_IDX_RX_DONE,
+ DW_INTR_IDX_ACTIVITY,
+ DW_INTR_IDX_STOP_DET,
+ DW_INTR_IDX_START_DET,
+ DW_INTR_IDX_GEN_CALL,
+ DW_INTR_IDX_MAX,
+};
+
#define DW_IC_INTR_DEFAULT_MASK (DW_IC_INTR_RX_FULL | \
DW_IC_INTR_TX_ABRT | \
DW_IC_INTR_STOP_DET)
@@ -126,6 +211,9 @@ struct reset_control;
* struct dw_i2c_dev - private i2c-designware data
* @dev: driver model device node
* @map: IO registers map
+ * @regs: logical-to-physical register offset table for the active IP variant
+ * @con_bits: CON register bit-layout for the active IP variant
+ * @intr_clr: logical intr number to reg table for the active IP variant
* @sysmap: System controller registers map
* @base: IO registers pointer
* @ext: Extended IO registers pointer
@@ -189,6 +277,9 @@ struct reset_control;
struct dw_i2c_dev {
struct device *dev;
struct regmap *map;
+ const u32 *regs;
+ const struct dw_i2c_con_bits *con_bits;
+ const u32 *intr_clr;
struct regmap *sysmap;
void __iomem *base;
void __iomem *ext;
@@ -265,6 +356,7 @@ struct i2c_dw_semaphore_callbacks {
int (*probe)(struct dw_i2c_dev *dev);
};
+void i2c_dw_select_variant(struct dw_i2c_dev *dev);
u32 i2c_dw_scl_hcnt(struct dw_i2c_dev *dev, unsigned int reg, u32 ic_clk,
u32 tSYMBOL, u32 tf, int offset);
u32 i2c_dw_scl_lcnt(struct dw_i2c_dev *dev, unsigned int reg, u32 ic_clk,
@@ -283,12 +375,12 @@ extern const struct dev_pm_ops i2c_dw_dev_pm_ops;
static inline void __i2c_dw_enable(struct dw_i2c_dev *dev)
{
dev->status |= STATUS_ACTIVE;
- regmap_write(dev->map, DW_IC_ENABLE, 1);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_ENABLE], 1);
}
static inline void __i2c_dw_disable_nowait(struct dw_i2c_dev *dev)
{
- regmap_write(dev->map, DW_IC_ENABLE, 0);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_ENABLE], 0);
dev->status &= ~STATUS_ACTIVE;
}
@@ -297,7 +389,7 @@ static inline void __i2c_dw_write_intr_mask(struct dw_i2c_dev *dev,
{
unsigned int val = dev->flags & ACCESS_POLLING ? 0 : intr_mask;
- regmap_write(dev->map, DW_IC_INTR_MASK, val);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_INTR_MASK], val);
dev->sw_mask = intr_mask;
}
@@ -305,11 +397,19 @@ static inline void __i2c_dw_read_intr_mask(struct dw_i2c_dev *dev,
unsigned int *intr_mask)
{
if (!(dev->flags & ACCESS_POLLING))
- regmap_read(dev->map, DW_IC_INTR_MASK, intr_mask);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_INTR_MASK], intr_mask);
else
*intr_mask = dev->sw_mask;
}
+/* Acknowledge a logical interrupt via dev->intr_clr[]: reg ID */
+static inline void i2c_dw_ack_intr(struct dw_i2c_dev *dev, enum dw_i2c_intr_idx intr)
+{
+ unsigned int dummy;
+
+ regmap_read(dev->map, dev->regs[dev->intr_clr[intr]], &dummy);
+}
+
void __i2c_dw_disable(struct dw_i2c_dev *dev);
void i2c_dw_disable(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index a1bcc3797e4f..f3b952f730bc 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -46,7 +46,7 @@ static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
if (ret)
return ret;
- ret = regmap_read(dev->map, DW_IC_COMP_PARAM_1, &comp_param1);
+ ret = regmap_read(dev->map, dev->regs[DW_REG_IDX_COMP_PARAM_1], &comp_param1);
i2c_dw_release_lock(dev);
if (ret)
return ret;
@@ -60,14 +60,14 @@ static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
ic_clk = i2c_dw_clk_rate(dev);
dev->ss_hcnt =
i2c_dw_scl_hcnt(dev,
- DW_IC_SS_SCL_HCNT,
+ dev->regs[DW_REG_IDX_SS_SCL_HCNT],
ic_clk,
4000, /* tHD;STA = tHIGH = 4.0 us */
sda_falling_time,
0); /* No offset */
dev->ss_lcnt =
i2c_dw_scl_lcnt(dev,
- DW_IC_SS_SCL_LCNT,
+ dev->regs[DW_REG_IDX_SS_SCL_LCNT],
ic_clk,
4700, /* tLOW = 4.7 us */
scl_falling_time,
@@ -93,14 +93,14 @@ static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
ic_clk = i2c_dw_clk_rate(dev);
dev->fs_hcnt =
i2c_dw_scl_hcnt(dev,
- DW_IC_FS_SCL_HCNT,
+ dev->regs[DW_REG_IDX_FS_SCL_HCNT],
ic_clk,
260, /* tHIGH = 260 ns */
sda_falling_time,
0); /* No offset */
dev->fs_lcnt =
i2c_dw_scl_lcnt(dev,
- DW_IC_FS_SCL_LCNT,
+ dev->regs[DW_REG_IDX_FS_SCL_LCNT],
ic_clk,
500, /* tLOW = 500 ns */
scl_falling_time,
@@ -116,14 +116,14 @@ static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
ic_clk = i2c_dw_clk_rate(dev);
dev->fs_hcnt =
i2c_dw_scl_hcnt(dev,
- DW_IC_FS_SCL_HCNT,
+ dev->regs[DW_REG_IDX_FS_SCL_HCNT],
ic_clk,
600, /* tHD;STA = tHIGH = 0.6 us */
sda_falling_time,
0); /* No offset */
dev->fs_lcnt =
i2c_dw_scl_lcnt(dev,
- DW_IC_FS_SCL_LCNT,
+ dev->regs[DW_REG_IDX_FS_SCL_LCNT],
ic_clk,
1300, /* tLOW = 1.3 us */
scl_falling_time,
@@ -133,14 +133,14 @@ static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
fp_str, dev->fs_hcnt, dev->fs_lcnt);
/* Check is high speed possible and fall back to fast mode if not */
- if ((dev->master_cfg & DW_IC_CON_SPEED_MASK) ==
- DW_IC_CON_SPEED_HIGH) {
+ if ((dev->master_cfg & dev->con_bits->speed_mask) ==
+ dev->con_bits->speed_high) {
if ((comp_param1 & DW_IC_COMP_PARAM_1_SPEED_MODE_MASK)
!= DW_IC_COMP_PARAM_1_SPEED_MODE_HIGH) {
dev_err(dev->dev, "High Speed not supported!\n");
t->bus_freq_hz = I2C_MAX_FAST_MODE_FREQ;
- dev->master_cfg &= ~DW_IC_CON_SPEED_MASK;
- dev->master_cfg |= DW_IC_CON_SPEED_FAST;
+ dev->master_cfg &= ~dev->con_bits->speed_mask;
+ dev->master_cfg |= dev->con_bits->speed_fast;
dev->hs_hcnt = 0;
dev->hs_lcnt = 0;
} else if (!dev->hs_hcnt || !dev->hs_lcnt) {
@@ -166,14 +166,14 @@ static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
ic_clk = i2c_dw_clk_rate(dev);
dev->hs_hcnt =
i2c_dw_scl_hcnt(dev,
- DW_IC_HS_SCL_HCNT,
+ dev->regs[DW_REG_IDX_HS_SCL_HCNT],
ic_clk,
t_high,
sda_falling_time,
0); /* No offset */
dev->hs_lcnt =
i2c_dw_scl_lcnt(dev,
- DW_IC_HS_SCL_LCNT,
+ dev->regs[DW_REG_IDX_HS_SCL_LCNT],
ic_clk,
t_low,
scl_falling_time,
@@ -200,7 +200,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
/* If the slave address is ten bit address, enable 10BITADDR */
if (msgs[dev->msg_write_idx].flags & I2C_M_TEN) {
- ic_con = DW_IC_CON_10BITADDR_MASTER;
+ ic_con = dev->con_bits->bit10_master;
/*
* If I2C_DYNAMIC_TAR_UPDATE is set, the 10-bit addressing
* mode has to be enabled via bit 12 of IC_TAR register.
@@ -210,14 +210,14 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
ic_tar = DW_IC_TAR_10BITADDR_MASTER;
}
- regmap_update_bits(dev->map, DW_IC_CON, DW_IC_CON_10BITADDR_MASTER,
+ regmap_update_bits(dev->map, dev->regs[DW_REG_IDX_CON], dev->con_bits->bit10_master,
ic_con);
/*
* Set the slave (target) address and enable 10-bit addressing mode
* if applicable.
*/
- regmap_write(dev->map, DW_IC_TAR,
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_TAR],
msgs[dev->msg_write_idx].addr | ic_tar);
/* Enforce disabled interrupts (due to HW issues) */
@@ -227,10 +227,10 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
__i2c_dw_enable(dev);
/* Dummy read to avoid the register getting stuck on Bay Trail */
- regmap_read(dev->map, DW_IC_ENABLE_STATUS, &dummy);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_ENABLE_STATUS], &dummy);
/* Clear and enable interrupts */
- regmap_read(dev->map, DW_IC_CLR_INTR, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_ALL);
__i2c_dw_write_intr_mask(dev, DW_IC_INTR_MASTER_MASK);
}
@@ -253,11 +253,11 @@ static bool i2c_dw_is_controller_active(struct dw_i2c_dev *dev)
{
u32 status;
- regmap_read(dev->map, DW_IC_STATUS, &status);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_STATUS], &status);
if (!(status & DW_IC_STATUS_MASTER_ACTIVITY))
return false;
- return regmap_read_poll_timeout(dev->map, DW_IC_STATUS, status,
+ return regmap_read_poll_timeout(dev->map, dev->regs[DW_REG_IDX_STATUS], status,
!(status & DW_IC_STATUS_MASTER_ACTIVITY),
1100, 20000) != 0;
}
@@ -267,7 +267,7 @@ static int i2c_dw_check_stopbit(struct dw_i2c_dev *dev)
u32 val;
int ret;
- ret = regmap_read_poll_timeout(dev->map, DW_IC_INTR_STAT, val,
+ ret = regmap_read_poll_timeout(dev->map, dev->regs[DW_REG_IDX_INTR_STAT], val,
!(val & DW_IC_INTR_STOP_DET),
1100, 20000);
if (ret)
@@ -320,7 +320,7 @@ static int amd_i2c_dw_xfer_quirk(struct dw_i2c_dev *dev, struct i2c_msg *msgs, i
buf_len = msgs[msg_wrt_idx].len;
if (!(msgs[msg_wrt_idx].flags & I2C_M_RD))
- regmap_write(dev->map, DW_IC_TX_TL, buf_len - 1);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_TX_TL], buf_len - 1);
/*
* Initiate the i2c read/write transaction of buffer length,
* and poll for bus busy status. For the last message transfer,
@@ -332,11 +332,13 @@ static int amd_i2c_dw_xfer_quirk(struct dw_i2c_dev *dev, struct i2c_msg *msgs, i
if (msgs[msg_wrt_idx].flags & I2C_M_RD) {
/* Due to hardware bug, need to write the same command twice. */
- regmap_write(dev->map, DW_IC_DATA_CMD, 0x100);
- regmap_write(dev->map, DW_IC_DATA_CMD, 0x100 | cmd);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_DATA_CMD], 0x100);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_DATA_CMD], 0x100 | cmd);
if (cmd) {
- regmap_write(dev->map, DW_IC_TX_TL, 2 * (buf_len - 1));
- regmap_write(dev->map, DW_IC_RX_TL, 2 * (buf_len - 1));
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_TX_TL],
+ 2 * (buf_len - 1));
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_RX_TL],
+ 2 * (buf_len - 1));
/*
* Need to check the stop bit. However, it cannot be
* detected from the registers so we check it always
@@ -347,7 +349,9 @@ static int amd_i2c_dw_xfer_quirk(struct dw_i2c_dev *dev, struct i2c_msg *msgs, i
return status;
for (data_idx = 0; data_idx < buf_len; data_idx++) {
- regmap_read(dev->map, DW_IC_DATA_CMD, &val);
+ regmap_read(dev->map,
+ dev->regs[DW_REG_IDX_DATA_CMD],
+ &val);
tx_buf[data_idx] = val;
}
status = i2c_dw_check_stopbit(dev);
@@ -355,7 +359,8 @@ static int amd_i2c_dw_xfer_quirk(struct dw_i2c_dev *dev, struct i2c_msg *msgs, i
return status;
}
} else {
- regmap_write(dev->map, DW_IC_DATA_CMD, *tx_buf++ | cmd);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_DATA_CMD],
+ *tx_buf++ | cmd);
usleep_range(AMD_TIMEOUT_MIN_US, AMD_TIMEOUT_MAX_US);
}
}
@@ -399,15 +404,15 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
* IC_RESTART_EN are set, we must manually
* set restart bit between messages.
*/
- if ((dev->master_cfg & DW_IC_CON_RESTART_EN) &&
- (dev->msg_write_idx > 0))
+ if (dev->master_cfg & dev->con_bits->restart_en &&
+ dev->msg_write_idx > 0)
need_restart = true;
}
- regmap_read(dev->map, DW_IC_TXFLR, &flr);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_TXFLR], &flr);
tx_limit = dev->tx_fifo_depth - flr;
- regmap_read(dev->map, DW_IC_RXFLR, &flr);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_RXFLR], &flr);
rx_limit = dev->rx_fifo_depth - flr;
while (buf_len > 0 && tx_limit > 0 && rx_limit > 0) {
@@ -441,12 +446,12 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
if (dev->rx_outstanding >= dev->rx_fifo_depth)
break;
- regmap_write(dev->map, DW_IC_DATA_CMD,
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_DATA_CMD],
cmd | 0x100);
rx_limit--;
dev->rx_outstanding++;
} else {
- regmap_write(dev->map, DW_IC_DATA_CMD,
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_DATA_CMD],
cmd | *buf++);
}
tx_limit--; buf_len--;
@@ -537,10 +542,10 @@ i2c_dw_read(struct dw_i2c_dev *dev)
buf = dev->rx_buf;
}
- regmap_read(dev->map, DW_IC_RXFLR, &rx_valid);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_RXFLR], &rx_valid);
for (; len > 0 && rx_valid > 0; len--, rx_valid--) {
- regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_DATA_CMD], &tmp);
tmp &= DW_IC_DATA_CMD_DAT;
/* Ensure length byte is a valid value */
if (flags & I2C_M_RECV_LEN) {
@@ -574,7 +579,7 @@ i2c_dw_read(struct dw_i2c_dev *dev)
static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
{
- unsigned int stat, dummy;
+ unsigned int stat;
/*
* The IC_INTR_STAT register just indicates "enabled" interrupts.
@@ -589,9 +594,9 @@ static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
* The raw version might be useful for debugging purposes.
*/
if (!(dev->flags & ACCESS_POLLING)) {
- regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_INTR_STAT], &stat);
} else {
- regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &stat);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_RAW_INTR_STAT], &stat);
stat &= dev->sw_mask;
}
@@ -603,32 +608,32 @@ static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
* Instead, use the separately-prepared IC_CLR_* registers.
*/
if (stat & DW_IC_INTR_RX_UNDER)
- regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_RX_UNDER);
if (stat & DW_IC_INTR_RX_OVER)
- regmap_read(dev->map, DW_IC_CLR_RX_OVER, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_RX_OVER);
if (stat & DW_IC_INTR_TX_OVER)
- regmap_read(dev->map, DW_IC_CLR_TX_OVER, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_TX_OVER);
if (stat & DW_IC_INTR_RD_REQ)
- regmap_read(dev->map, DW_IC_CLR_RD_REQ, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_RD_REQ);
if (stat & DW_IC_INTR_TX_ABRT) {
/*
* The IC_TX_ABRT_SOURCE register is cleared whenever
* the IC_CLR_TX_ABRT is read. Preserve it beforehand.
*/
- regmap_read(dev->map, DW_IC_TX_ABRT_SOURCE, &dev->abort_source);
- regmap_read(dev->map, DW_IC_CLR_TX_ABRT, &dummy);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_TX_ABRT_SOURCE], &dev->abort_source);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_TX_ABRT);
}
if (stat & DW_IC_INTR_RX_DONE)
- regmap_read(dev->map, DW_IC_CLR_RX_DONE, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_RX_DONE);
if (stat & DW_IC_INTR_ACTIVITY)
- regmap_read(dev->map, DW_IC_CLR_ACTIVITY, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_ACTIVITY);
if ((stat & DW_IC_INTR_STOP_DET) &&
((dev->rx_outstanding == 0) || (stat & DW_IC_INTR_RX_FULL)))
- regmap_read(dev->map, DW_IC_CLR_STOP_DET, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_STOP_DET);
if (stat & DW_IC_INTR_START_DET)
- regmap_read(dev->map, DW_IC_CLR_START_DET, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_START_DET);
if (stat & DW_IC_INTR_GEN_CALL)
- regmap_read(dev->map, DW_IC_CLR_GEN_CALL, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_GEN_CALL);
return stat;
}
@@ -688,8 +693,8 @@ irqreturn_t i2c_dw_isr_master(struct dw_i2c_dev *dev)
{
unsigned int stat, enabled;
- regmap_read(dev->map, DW_IC_ENABLE, &enabled);
- regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &stat);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_ENABLE], &enabled);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_RAW_INTR_STAT], &stat);
if (!enabled || !(stat & ~DW_IC_INTR_ACTIVITY))
return IRQ_NONE;
if (pm_runtime_suspended(dev->dev) || stat == GENMASK(31, 0))
@@ -940,20 +945,20 @@ void i2c_dw_configure_master(struct dw_i2c_dev *dev)
if ((dev->flags & MODEL_MASK) != MODEL_AMD_NAVI_GPU)
dev->functionality |= I2C_FUNC_PROTOCOL_MANGLING;
- dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
- DW_IC_CON_RESTART_EN;
+ dev->master_cfg = dev->con_bits->master | dev->con_bits->slave_disable |
+ dev->con_bits->restart_en;
dev->mode = DW_IC_MASTER;
switch (t->bus_freq_hz) {
case I2C_MAX_STANDARD_MODE_FREQ:
- dev->master_cfg |= DW_IC_CON_SPEED_STD;
+ dev->master_cfg |= dev->con_bits->speed_std;
break;
case I2C_MAX_HIGH_SPEED_MODE_FREQ:
- dev->master_cfg |= DW_IC_CON_SPEED_HIGH;
+ dev->master_cfg |= dev->con_bits->speed_high;
break;
default:
- dev->master_cfg |= DW_IC_CON_SPEED_FAST;
+ dev->master_cfg |= dev->con_bits->speed_fast;
}
}
EXPORT_SYMBOL_GPL(i2c_dw_configure_master);
@@ -1037,13 +1042,13 @@ int i2c_dw_probe_master(struct dw_i2c_dev *dev)
* bus recovery process. Driver should not ignore this BIOS
* advertisement of bus clear feature.
*/
- ret = regmap_read(dev->map, DW_IC_CON, &ic_con);
+ ret = regmap_read(dev->map, dev->regs[DW_REG_IDX_CON], &ic_con);
i2c_dw_release_lock(dev);
if (ret)
return ret;
- if (ic_con & DW_IC_CON_BUS_CLEAR_CTRL)
- dev->master_cfg |= DW_IC_CON_BUS_CLEAR_CTRL;
+ if (ic_con & dev->con_bits->bus_clear_ctrl)
+ dev->master_cfg |= dev->con_bits->bus_clear_ctrl;
return i2c_dw_init_recovery_info(dev);
}
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 468287922363..fc73106fb0f9 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -246,6 +246,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
pci_set_drvdata(pdev, dev);
+ i2c_dw_select_variant(dev);
+
if (controller->setup) {
r = controller->setup(pdev, controller);
if (r)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 447af5523c2e..42b34c678146 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -156,6 +156,8 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
dev->flags = flags;
platform_set_drvdata(pdev, dev);
+ i2c_dw_select_variant(dev);
+
ret = dw_i2c_plat_request_regs(dev);
if (ret)
return ret;
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index 0abcc7757b23..7f20124d1181 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -53,7 +53,7 @@ int i2c_dw_unreg_slave(struct i2c_client *slave)
{
struct dw_i2c_dev *dev = i2c_get_adapdata(slave->adapter);
- regmap_write(dev->map, DW_IC_INTR_MASK, 0);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_INTR_MASK], 0);
i2c_dw_disable(dev);
synchronize_irq(dev->irq);
dev->slave = NULL;
@@ -65,7 +65,7 @@ int i2c_dw_unreg_slave(struct i2c_client *slave)
static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
{
- unsigned int stat, dummy;
+ unsigned int stat;
/*
* The IC_INTR_STAT register just indicates "enabled" interrupts.
@@ -79,7 +79,7 @@ static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
*
* The raw version might be useful for debugging purposes.
*/
- regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_INTR_STAT], &stat);
/*
* Do not use the IC_CLR_INTR register to clear interrupts, or
@@ -89,23 +89,23 @@ static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
* Instead, use the separately-prepared IC_CLR_* registers.
*/
if (stat & DW_IC_INTR_TX_ABRT)
- regmap_read(dev->map, DW_IC_CLR_TX_ABRT, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_TX_ABRT);
if (stat & DW_IC_INTR_RX_UNDER)
- regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_RX_UNDER);
if (stat & DW_IC_INTR_RX_OVER)
- regmap_read(dev->map, DW_IC_CLR_RX_OVER, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_RX_OVER);
if (stat & DW_IC_INTR_TX_OVER)
- regmap_read(dev->map, DW_IC_CLR_TX_OVER, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_TX_OVER);
if (stat & DW_IC_INTR_RX_DONE)
- regmap_read(dev->map, DW_IC_CLR_RX_DONE, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_RX_DONE);
if (stat & DW_IC_INTR_ACTIVITY)
- regmap_read(dev->map, DW_IC_CLR_ACTIVITY, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_ACTIVITY);
if (stat & DW_IC_INTR_STOP_DET)
- regmap_read(dev->map, DW_IC_CLR_STOP_DET, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_STOP_DET);
if (stat & DW_IC_INTR_START_DET)
- regmap_read(dev->map, DW_IC_CLR_START_DET, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_START_DET);
if (stat & DW_IC_INTR_GEN_CALL)
- regmap_read(dev->map, DW_IC_CLR_GEN_CALL, &dummy);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_GEN_CALL);
return stat;
}
@@ -119,9 +119,9 @@ irqreturn_t i2c_dw_isr_slave(struct dw_i2c_dev *dev)
unsigned int raw_stat, stat, enabled, tmp;
u8 val = 0, slave_activity;
- regmap_read(dev->map, DW_IC_ENABLE, &enabled);
- regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &raw_stat);
- regmap_read(dev->map, DW_IC_STATUS, &tmp);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_ENABLE], &enabled);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_RAW_INTR_STAT], &raw_stat);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_STATUS], &tmp);
slave_activity = ((tmp & DW_IC_STATUS_SLAVE_ACTIVITY) >> 6);
if (!enabled || !(raw_stat & ~DW_IC_INTR_ACTIVITY) || !dev->slave)
@@ -141,7 +141,7 @@ irqreturn_t i2c_dw_isr_slave(struct dw_i2c_dev *dev)
}
do {
- regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_DATA_CMD], &tmp);
if (tmp & DW_IC_DATA_CMD_FIRST_DATA_BYTE)
i2c_slave_event(dev->slave,
I2C_SLAVE_WRITE_REQUESTED,
@@ -149,13 +149,13 @@ irqreturn_t i2c_dw_isr_slave(struct dw_i2c_dev *dev)
val = tmp;
i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
&val);
- regmap_read(dev->map, DW_IC_STATUS, &tmp);
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_STATUS], &tmp);
} while (tmp & DW_IC_STATUS_RFNE);
}
if (stat & DW_IC_INTR_RD_REQ) {
if (slave_activity) {
- regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
+ i2c_dw_ack_intr(dev, DW_INTR_IDX_RD_REQ);
if (!(dev->status & STATUS_READ_IN_PROGRESS)) {
i2c_slave_event(dev->slave,
@@ -168,7 +168,7 @@ irqreturn_t i2c_dw_isr_slave(struct dw_i2c_dev *dev)
I2C_SLAVE_READ_PROCESSED,
&val);
}
- regmap_write(dev->map, DW_IC_DATA_CMD, val);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_DATA_CMD], val);
}
}
@@ -185,8 +185,8 @@ void i2c_dw_configure_slave(struct dw_i2c_dev *dev)
dev->functionality |= I2C_FUNC_SLAVE;
- dev->slave_cfg = DW_IC_CON_RX_FIFO_FULL_HLD_CTRL |
- DW_IC_CON_RESTART_EN | DW_IC_CON_STOP_DET_IFADDRESSED;
+ dev->slave_cfg = dev->con_bits->rx_fifo_full_hld_ctrl |
+ dev->con_bits->restart_en | dev->con_bits->stop_det_ifaddressed;
}
EXPORT_SYMBOL_GPL(i2c_dw_configure_slave);
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] i2c: designware: Add snps,dwc-i2c support and new compatible
2026-09-19 9:06 [PATCH 0/3] i2c: designware: Add DWC_i2c support Aniket Limaye
2026-09-19 9:06 ` [PATCH 1/3] dt-bindings: i2c: dw: Add DWC_i2c compatible Aniket Limaye
2026-09-19 9:06 ` [PATCH 2/3] i2c: designware: Introduce per-variant register offset and bit-layout tables Aniket Limaye
@ 2026-09-19 9:06 ` Aniket Limaye
2026-09-19 9:15 ` sashiko-bot
2 siblings, 1 reply; 16+ messages in thread
From: Aniket Limaye @ 2026-09-19 9:06 UTC (permalink / raw)
To: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mika Westerberg, Nirujogi Pratap, Bin Du, Andy Shevchenko
Cc: linux-i2c, devicetree, linux-kernel, vigneshr, nm, u-kumar1,
lianfeng.ouyang, Aniket Limaye, Ritwick.Sharma
Add the register offset table and CON-register bit layout for the
DWC_i2c IP ("snps,dwc-i2c"), and register it in dw_i2c_of_match[] with
the new MODEL_DWC_I2C flag.
Compared to the existing DW_apb_i2c layout [1], DWC_i2c [0] differs
enough to need MODEL_DWC_I2C-gated special-casing in a few places,
besides the offset/bit-layout tables themselves:
- Interrupts are acknowledged by writing a bitmask to a single CLR_INTR
register instead of reading N dedicated CLR_* registers;
i2c_dw_ack_intr() picks the right method based on dev->flags.
- One HCNT/LCNT register pair is shared between standard and fast speed
instead of having one pair each; i2c_dw_write_timings() writes
whichever value set matches the configured speed.
- No COMP_PARAM_1 register, so FIFO depth and high-speed-mode support
can't be autodetected: FIFO depth now comes from the required
snps,tx-fifo-depth/snps,rx-fifo-depth DT properties, and the
high-speed capability check is skipped.
- No defined CON.RESTART_EN bit; treat it as always set.
The register offsets used for DWC_i2c are those present in TI TDA54 SoC.
[0]: DWC_i2c_reference.pdf
[1]: DW_apb_i2c_databook.pdf
Signed-off-by: Aniket Limaye <a-limaye@ti.com>
---
SMBus is not supported on TI TDA54 SoC, and hence this patch does not
add SMBus register offsets.
---
drivers/i2c/busses/i2c-designware-common.c | 139 +++++++++++++++++++++++++---
drivers/i2c/busses/i2c-designware-core.h | 26 +++++-
drivers/i2c/busses/i2c-designware-master.c | 26 ++++--
drivers/i2c/busses/i2c-designware-platdrv.c | 1 +
4 files changed, 168 insertions(+), 24 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index a21aeb7f415a..cc05c822aa56 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -113,6 +113,41 @@ static const u32 dw_i2c_reg_offsets[DW_REG_IDX_MAX] = {
[DW_REG_IDX_COMP_TYPE] = DW_IC_COMP_TYPE,
};
+/* "snps,dwc-i2c" block-based register layout */
+#define DWC_OPERATION_BLK_OFFSET 0x00
+#define DWC_I2C_BLK_OFFSET 0x28
+#define DWC_FIFO_INTR_BLK_OFFSET 0xbc
+#define DWC_DEBUG_BLK_OFFSET 0xf8
+
+static const u32 dwc_i2c_reg_offsets[DW_REG_IDX_MAX] = {
+ [DW_REG_IDX_ENABLE] = DWC_OPERATION_BLK_OFFSET + 0x04,
+ [DW_REG_IDX_CAPABILITIES] = DWC_OPERATION_BLK_OFFSET + 0x0c,
+ [DW_REG_IDX_CON] = DWC_I2C_BLK_OFFSET + 0x04,
+ [DW_REG_IDX_TAR] = DWC_I2C_BLK_OFFSET + 0x08,
+ [DW_REG_IDX_SAR] = DWC_I2C_BLK_OFFSET + 0x0c, /* DAR on this IP */
+ [DW_REG_IDX_DATA_CMD] = DWC_I2C_BLK_OFFSET + 0x58,
+ [DW_REG_IDX_SS_SCL_HCNT] = DWC_I2C_BLK_OFFSET + 0x24, /* shared SS/FS pair */
+ [DW_REG_IDX_SS_SCL_LCNT] = DWC_I2C_BLK_OFFSET + 0x28,
+ [DW_REG_IDX_FS_SCL_HCNT] = DWC_I2C_BLK_OFFSET + 0x24,
+ [DW_REG_IDX_FS_SCL_LCNT] = DWC_I2C_BLK_OFFSET + 0x28,
+ [DW_REG_IDX_HS_SCL_HCNT] = DWC_I2C_BLK_OFFSET + 0x2c,
+ [DW_REG_IDX_HS_SCL_LCNT] = DWC_I2C_BLK_OFFSET + 0x30,
+ [DW_REG_IDX_SDA_HOLD] = DWC_I2C_BLK_OFFSET + 0x34,
+ [DW_REG_IDX_RX_TL] = DWC_I2C_BLK_OFFSET + 0x5c,
+ [DW_REG_IDX_TX_TL] = DWC_I2C_BLK_OFFSET + 0x60,
+ [DW_REG_IDX_INTR_STAT] = DWC_FIFO_INTR_BLK_OFFSET + 0x04,
+ [DW_REG_IDX_INTR_MASK] = DWC_FIFO_INTR_BLK_OFFSET + 0x08,
+ [DW_REG_IDX_RAW_INTR_STAT] = DWC_FIFO_INTR_BLK_OFFSET + 0x0c,
+ [DW_REG_IDX_CLR_INTR] = DWC_FIFO_INTR_BLK_OFFSET + 0x10,
+ [DW_REG_IDX_STATUS] = DWC_FIFO_INTR_BLK_OFFSET + 0x1c,
+ [DW_REG_IDX_TXFLR] = DWC_FIFO_INTR_BLK_OFFSET + 0x20,
+ [DW_REG_IDX_RXFLR] = DWC_FIFO_INTR_BLK_OFFSET + 0x24,
+ [DW_REG_IDX_TX_ABRT_SOURCE] = DWC_FIFO_INTR_BLK_OFFSET + 0x18,
+ [DW_REG_IDX_ENABLE_STATUS] = DWC_FIFO_INTR_BLK_OFFSET + 0x14,
+ [DW_REG_IDX_COMP_VERSION] = DWC_DEBUG_BLK_OFFSET + 0x08,
+ [DW_REG_IDX_COMP_TYPE] = DWC_DEBUG_BLK_OFFSET + 0x0c,
+};
+
static const struct dw_i2c_con_bits dw_i2c_con_bits = {
.master = DW_IC_CON_MASTER,
.speed_std = DW_IC_CON_SPEED_STD,
@@ -129,6 +164,26 @@ static const struct dw_i2c_con_bits dw_i2c_con_bits = {
.bus_clear_ctrl = DW_IC_CON_BUS_CLEAR_CTRL,
};
+/*
+ * DWC_IC_CTRL bit layout for "snps,dwc-i2c".
+ * There is no defined bit for RESTART_EN or SLAVE_DISABLE on this IP.
+ */
+static const struct dw_i2c_con_bits dwc_i2c_con_bits = {
+ .master = BIT(0),
+ .speed_std = (1 << 4),
+ .speed_fast = (2 << 4),
+ .speed_high = (3 << 4),
+ .speed_mask = GENMASK(5, 4),
+ .bit10_slave = BIT(8),
+ .bit10_master = BIT(9),
+ .restart_en = 0,
+ .slave_disable = 0,
+ .stop_det_ifaddressed = BIT(10),
+ .tx_empty_ctrl = BIT(11),
+ .rx_fifo_full_hld_ctrl = BIT(12),
+ .bus_clear_ctrl = 0,
+};
+
/* "snps,designware-i2c": dedicated read-to-clear register ID per logical interrupt */
static const u32 dw_i2c_intr_clr[DW_INTR_IDX_MAX] = {
[DW_INTR_IDX_ALL] = DW_REG_IDX_CLR_INTR,
@@ -144,6 +199,21 @@ static const u32 dw_i2c_intr_clr[DW_INTR_IDX_MAX] = {
[DW_INTR_IDX_GEN_CALL] = DW_REG_IDX_CLR_GEN_CALL,
};
+/* "snps,dwc-i2c" DW_REG_IDX_CLR_INTR bit to write per logical interrupt */
+static const u32 dwc_i2c_intr_clr[DW_INTR_IDX_MAX] = {
+ [DW_INTR_IDX_ALL] = DWC_IC_INTR_CLR_INTR,
+ [DW_INTR_IDX_RX_UNDER] = DWC_IC_INTR_CLR_RX_UNDER,
+ [DW_INTR_IDX_RX_OVER] = DWC_IC_INTR_CLR_RX_OVER,
+ [DW_INTR_IDX_TX_OVER] = DWC_IC_INTR_CLR_TX_OVER,
+ [DW_INTR_IDX_RD_REQ] = DWC_IC_INTR_CLR_RD_REQ,
+ [DW_INTR_IDX_TX_ABRT] = DWC_IC_INTR_CLR_TX_ABRT,
+ [DW_INTR_IDX_RX_DONE] = DWC_IC_INTR_CLR_RX_DONE,
+ [DW_INTR_IDX_ACTIVITY] = DWC_IC_INTR_CLR_ACTIVITY,
+ [DW_INTR_IDX_STOP_DET] = DWC_IC_INTR_CLR_STOP_DET,
+ [DW_INTR_IDX_START_DET] = DWC_IC_INTR_CLR_START_DET,
+ [DW_INTR_IDX_GEN_CALL] = DWC_IC_INTR_CLR_GEN_CALL,
+};
+
/**
* i2c_dw_select_variant() - Pick the register offset table, CON-register bit
* layout and interrupt-ack mapping matching this device's IP variant
@@ -155,9 +225,15 @@ static const u32 dw_i2c_intr_clr[DW_INTR_IDX_MAX] = {
*/
void i2c_dw_select_variant(struct dw_i2c_dev *dev)
{
- dev->regs = dw_i2c_reg_offsets;
- dev->con_bits = &dw_i2c_con_bits;
- dev->intr_clr = dw_i2c_intr_clr;
+ if (dev->flags & MODEL_DWC_I2C) {
+ dev->regs = dwc_i2c_reg_offsets;
+ dev->con_bits = &dwc_i2c_con_bits;
+ dev->intr_clr = dwc_i2c_intr_clr;
+ } else {
+ dev->regs = dw_i2c_reg_offsets;
+ dev->con_bits = &dw_i2c_con_bits;
+ dev->intr_clr = dw_i2c_intr_clr;
+ }
}
EXPORT_SYMBOL_GPL(i2c_dw_select_variant);
@@ -475,13 +551,27 @@ static void i2c_dw_configure_mode(struct dw_i2c_dev *dev, int mode)
static void i2c_dw_write_timings(struct dw_i2c_dev *dev)
{
- /* Write standard speed timing parameters */
- regmap_write(dev->map, dev->regs[DW_REG_IDX_SS_SCL_HCNT], dev->ss_hcnt);
- regmap_write(dev->map, dev->regs[DW_REG_IDX_SS_SCL_LCNT], dev->ss_lcnt);
-
- /* Write fast mode/fast mode plus timing parameters */
- regmap_write(dev->map, dev->regs[DW_REG_IDX_FS_SCL_HCNT], dev->fs_hcnt);
- regmap_write(dev->map, dev->regs[DW_REG_IDX_FS_SCL_LCNT], dev->fs_lcnt);
+ if (dev->flags & MODEL_DWC_I2C) {
+ /*
+ * Only one HCNT/LCNT register pair backs both speeds on
+ * this IP -- write whichever value set matches master_cfg.
+ */
+ if ((dev->master_cfg & dev->con_bits->speed_mask) == dev->con_bits->speed_std) {
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_SS_SCL_HCNT], dev->ss_hcnt);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_SS_SCL_LCNT], dev->ss_lcnt);
+ } else {
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_FS_SCL_HCNT], dev->fs_hcnt);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_FS_SCL_LCNT], dev->fs_lcnt);
+ }
+ } else {
+ /* Write standard speed timing parameters */
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_SS_SCL_HCNT], dev->ss_hcnt);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_SS_SCL_LCNT], dev->ss_lcnt);
+
+ /* Write fast mode/fast mode plus timing parameters */
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_FS_SCL_HCNT], dev->fs_hcnt);
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_FS_SCL_LCNT], dev->fs_lcnt);
+ }
/* Write high speed timing parameters */
regmap_write(dev->map, dev->regs[DW_REG_IDX_HS_SCL_HCNT], dev->hs_hcnt);
@@ -513,6 +603,16 @@ void i2c_dw_set_mode(struct dw_i2c_dev *dev, int mode)
dev->mode = mode;
}
+/* Not every snps,dwc-i2c instance has the SMBus block; check IC_CAPABILITIES */
+static bool i2c_dwc_has_smbus(struct dw_i2c_dev *dev)
+{
+ u32 caps = 0;
+
+ regmap_read(dev->map, dev->regs[DW_REG_IDX_CAPABILITIES], &caps);
+
+ return caps & DWC_IC_CAPABILITIES_SMBUS;
+}
+
/**
* i2c_dw_init() - Initialize the DesignWare I2C hardware
* @dev: device private data
@@ -536,8 +636,10 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
* Mask SMBus interrupts to block storms from broken
* firmware that leaves IC_SMBUS=1; the handler never
* services them.
+ * For DWC-i2c, need to first check if SMBus is supported
*/
- regmap_write(dev->map, dev->regs[DW_REG_IDX_SMBUS_INTR_MASK], 0);
+ if (!(dev->flags & MODEL_DWC_I2C) || i2c_dwc_has_smbus(dev))
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_SMBUS_INTR_MASK], 0);
i2c_dw_write_timings(dev);
@@ -581,6 +683,14 @@ int i2c_dw_fw_parse_and_configure(struct dw_i2c_dev *dev)
if (device_property_read_u32(device, "snps,bus-capacitance-pf", &dev->bus_capacitance_pF))
dev->bus_capacitance_pF = DW_IC_DEFAULT_BUS_CAPACITANCE_pF;
+ if (dev->flags & MODEL_DWC_I2C) {
+ device_property_read_u32(device, "snps,tx-fifo-depth", &dev->tx_fifo_depth);
+ device_property_read_u32(device, "snps,rx-fifo-depth", &dev->rx_fifo_depth);
+
+ if (!dev->tx_fifo_depth || !dev->rx_fifo_depth)
+ return -EINVAL;
+ }
+
dev->clk_freq_optimized = device_property_read_bool(device, "snps,clk-freq-optimized");
/* Mobileye controllers do not hold the clock on empty FIFO */
@@ -897,6 +1007,13 @@ static int i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
return 0;
}
+ /*
+ * DW_IC_COMP_PARAM_1 not implemented on this IP;
+ * fifo depth set in i2c_dw_fw_parse_and_configure().
+ */
+ if (dev->flags & MODEL_DWC_I2C)
+ return 0;
+
/*
* Try to detect the FIFO depth if not set by interface driver,
* the depth could be from 2 to 256 from HW spec.
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 2fad18582db7..08a29f70c5dd 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -88,6 +88,7 @@ enum dw_i2c_reg_idx {
DW_REG_IDX_COMP_PARAM_1,
DW_REG_IDX_COMP_VERSION,
DW_REG_IDX_COMP_TYPE,
+ DW_REG_IDX_CAPABILITIES,
DW_REG_IDX_MAX,
};
@@ -128,6 +129,21 @@ enum dw_i2c_intr_idx {
DW_INTR_IDX_MAX,
};
+#define DWC_IC_INTR_CLR_INTR BIT(0)
+#define DWC_IC_INTR_CLR_RX_UNDER BIT(1)
+#define DWC_IC_INTR_CLR_RX_OVER BIT(2)
+#define DWC_IC_INTR_CLR_TX_OVER BIT(3)
+#define DWC_IC_INTR_CLR_RD_REQ BIT(4)
+#define DWC_IC_INTR_CLR_TX_ABRT BIT(5)
+#define DWC_IC_INTR_CLR_RX_DONE BIT(6)
+#define DWC_IC_INTR_CLR_ACTIVITY BIT(7)
+#define DWC_IC_INTR_CLR_STOP_DET BIT(8)
+#define DWC_IC_INTR_CLR_START_DET BIT(9)
+#define DWC_IC_INTR_CLR_GEN_CALL BIT(10)
+
+/* IC_CAPABILITIES bit reporting whether SMBus is present on this instance */
+#define DWC_IC_CAPABILITIES_SMBUS BIT(10)
+
#define DW_IC_INTR_DEFAULT_MASK (DW_IC_INTR_RX_FULL | \
DW_IC_INTR_TX_ABRT | \
DW_IC_INTR_STOP_DET)
@@ -340,7 +356,8 @@ struct dw_i2c_dev {
#define MODEL_AMD_NAVI_GPU BIT(10)
#define MODEL_WANGXUN_SP BIT(11)
-#define MODEL_MASK GENMASK(11, 8)
+#define MODEL_DWC_I2C BIT(12)
+#define MODEL_MASK GENMASK(12, 8)
/*
* Enable UCSI interrupt by writing 0xd at register
@@ -402,12 +419,15 @@ static inline void __i2c_dw_read_intr_mask(struct dw_i2c_dev *dev,
*intr_mask = dev->sw_mask;
}
-/* Acknowledge a logical interrupt via dev->intr_clr[]: reg ID */
+/* Acknowledge a logical interrupt via dev->intr_clr[]: reg ID or bit offset */
static inline void i2c_dw_ack_intr(struct dw_i2c_dev *dev, enum dw_i2c_intr_idx intr)
{
unsigned int dummy;
- regmap_read(dev->map, dev->regs[dev->intr_clr[intr]], &dummy);
+ if (dev->flags & MODEL_DWC_I2C)
+ regmap_write(dev->map, dev->regs[DW_REG_IDX_CLR_INTR], dev->intr_clr[intr]);
+ else
+ regmap_read(dev->map, dev->regs[dev->intr_clr[intr]], &dummy);
}
void __i2c_dw_disable(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index f3b952f730bc..8ed4d89e631c 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -42,14 +42,16 @@ static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
u32 ic_clk;
int ret;
- ret = i2c_dw_acquire_lock(dev);
- if (ret)
- return ret;
-
- ret = regmap_read(dev->map, dev->regs[DW_REG_IDX_COMP_PARAM_1], &comp_param1);
- i2c_dw_release_lock(dev);
- if (ret)
- return ret;
+ if (!(dev->flags & MODEL_DWC_I2C)) {
+ ret = i2c_dw_acquire_lock(dev);
+ if (ret)
+ return ret;
+
+ ret = regmap_read(dev->map, dev->regs[DW_REG_IDX_COMP_PARAM_1], &comp_param1);
+ i2c_dw_release_lock(dev);
+ if (ret)
+ return ret;
+ }
/* Set standard and fast speed dividers for high/low periods */
sda_falling_time = t->sda_fall_ns ?: 300; /* ns */
@@ -135,7 +137,8 @@ static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
/* Check is high speed possible and fall back to fast mode if not */
if ((dev->master_cfg & dev->con_bits->speed_mask) ==
dev->con_bits->speed_high) {
- if ((comp_param1 & DW_IC_COMP_PARAM_1_SPEED_MODE_MASK)
+ if (!(dev->flags & MODEL_DWC_I2C) &&
+ (comp_param1 & DW_IC_COMP_PARAM_1_SPEED_MODE_MASK)
!= DW_IC_COMP_PARAM_1_SPEED_MODE_HIGH) {
dev_err(dev->dev, "High Speed not supported!\n");
t->bus_freq_hz = I2C_MAX_FAST_MODE_FREQ;
@@ -403,8 +406,11 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
* If both IC_EMPTYFIFO_HOLD_MASTER_EN and
* IC_RESTART_EN are set, we must manually
* set restart bit between messages.
+ * snps,dwc-i2c does not define a CON.RESTART_EN
+ * bit, and behaves like it is set to 1.
*/
- if (dev->master_cfg & dev->con_bits->restart_en &&
+ if (((dev->master_cfg & dev->con_bits->restart_en) ||
+ (dev->flags & MODEL_DWC_I2C)) &&
dev->msg_write_idx > 0)
need_restart = true;
}
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 42b34c678146..a3cd81cc2c0b 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -257,6 +257,7 @@ static const struct of_device_id dw_i2c_of_match[] = {
{ .compatible = "mobileye,eyeq6lplus-i2c" },
{ .compatible = "mscc,ocelot-i2c" },
{ .compatible = "snps,designware-i2c" },
+ { .compatible = "snps,dwc-i2c", .data = (void *)MODEL_DWC_I2C },
{}
};
MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] i2c: designware: Add snps,dwc-i2c support and new compatible
2026-09-19 9:06 ` [PATCH 3/3] i2c: designware: Add snps,dwc-i2c support and new compatible Aniket Limaye
@ 2026-09-19 9:15 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-09-19 9:15 UTC (permalink / raw)
To: Aniket Limaye; +Cc: robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] i2c: designware: unintended register write to offset 0x00 for SMBus
--
commit 756dafe564a9964f945795b25e2b59168e6adcc6
Author: Aniket Limaye <a-limaye@ti.com>
i2c: designware: Add snps,dwc-i2c support and new compatible
This patch adds register offset tables and bit layouts for the DWC_i2c IP
and registers it with a new MODEL_DWC_I2C flag. It handles variations in
interrupt acknowledgments, timing parameters, and capabilities checking.
> diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
> index a21aeb7f415af..cc05c822aa56d 100644
> --- a/drivers/i2c/busses/i2c-designware-common.c
> +++ b/drivers/i2c/busses/i2c-designware-common.c
> @@ -113,6 +113,41 @@ static const u32 dw_i2c_reg_offsets[DW_REG_IDX_MAX] = {
[ ... ]
> +static const u32 dwc_i2c_reg_offsets[DW_REG_IDX_MAX] = {
> + [DW_REG_IDX_ENABLE] = DWC_OPERATION_BLK_OFFSET + 0x04,
> + [DW_REG_IDX_CAPABILITIES] = DWC_OPERATION_BLK_OFFSET + 0x0c,
> + [DW_REG_IDX_CON] = DWC_I2C_BLK_OFFSET + 0x04,
[ ... ]
> + [DW_REG_IDX_COMP_VERSION] = DWC_DEBUG_BLK_OFFSET + 0x08,
> + [DW_REG_IDX_COMP_TYPE] = DWC_DEBUG_BLK_OFFSET + 0x0c,
> +};
[Severity: High]
Does this array lack an initialization for DW_REG_IDX_SMBUS_INTR_MASK?
Since this is designated to have the size DW_REG_IDX_MAX, any omitted
indices will default to 0x00.
[ ... ]
> @@ -536,8 +636,10 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
> * Mask SMBus interrupts to block storms from broken
> * firmware that leaves IC_SMBUS=1; the handler never
> * services them.
> + * For DWC-i2c, need to first check if SMBus is supported
> */
> - regmap_write(dev->map, dev->regs[DW_REG_IDX_SMBUS_INTR_MASK], 0);
> + if (!(dev->flags & MODEL_DWC_I2C) || i2c_dwc_has_smbus(dev))
> + regmap_write(dev->map, dev->regs[DW_REG_IDX_SMBUS_INTR_MASK], 0);
[Severity: High]
If a DWC_I2C hardware instance actually reports SMBus support via the
capabilities register, i2c_dwc_has_smbus() will return true and this will
attempt to mask the SMBus interrupts.
Because DW_REG_IDX_SMBUS_INTR_MASK is missing from dwc_i2c_reg_offsets,
could this unintendedly write the value 0 to the DWC_I2C block at register
offset 0x00?
Even if the TI TDA54 SoC doesn't support SMBus, this code is reachable for
other SoCs or configurations that might report the capability, which could
lead to unintended configuration changes or bus errors.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260919-tda54-upstream-i2c-v1-0-b0b9f77be18b@ti.com?part=3
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: i2c: dw: Add DWC_i2c compatible
2026-09-19 9:06 ` [PATCH 1/3] dt-bindings: i2c: dw: Add DWC_i2c compatible Aniket Limaye
@ 2026-09-20 18:24 ` Krzysztof Kozlowski
2026-09-21 8:25 ` Aniket Limaye
0 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2026-09-20 18:24 UTC (permalink / raw)
To: Aniket Limaye
Cc: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mika Westerberg, Nirujogi Pratap, Bin Du, Andy Shevchenko,
linux-i2c, devicetree, linux-kernel, vigneshr, nm, u-kumar1,
lianfeng.ouyang, Ritwick.Sharma
On Sat, Sep 19, 2026 at 02:36:06PM +0530, Aniket Limaye wrote:
> Add the "snps,dwc-i2c" compatible for Synopsys Advanced I2C Controller
> referred as DWC_i2c. Compared to the existing "snps,designware-i2c"
dwc is designware, so snps,dwc-i2c = snps,designware-i2c
I think this is *the* argument why generic compatibles are wrong.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: i2c: dw: Add DWC_i2c compatible
2026-09-20 18:24 ` Krzysztof Kozlowski
@ 2026-09-21 8:25 ` Aniket Limaye
2026-09-21 11:14 ` Mika Westerberg
0 siblings, 1 reply; 16+ messages in thread
From: Aniket Limaye @ 2026-09-21 8:25 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mika Westerberg, Nirujogi Pratap, Bin Du, Andy Shevchenko,
linux-i2c, devicetree, linux-kernel, vigneshr, nm, u-kumar1,
lianfeng.ouyang, Ritwick.Sharma
On 20/09/26 23:54, Krzysztof Kozlowski wrote:
> On Sat, Sep 19, 2026 at 02:36:06PM +0530, Aniket Limaye wrote:
>> Add the "snps,dwc-i2c" compatible for Synopsys Advanced I2C Controller
>> referred as DWC_i2c. Compared to the existing "snps,designware-i2c"
> dwc is designware, so snps,dwc-i2c = snps,designware-i2c
>
> I think this is *the* argument why generic compatibles are wrong.
These are 2 different controllers with some different registers and
vastly different register offsets.
So, the official product name (short format) for the SNPS Advanced I2C
controller is "DWC_i2c", as per SNPS' own reference document [0].
Whereas the original "snps,designware-i2c" corresponds to the product
name "DW_apb_i2c" as per its own databook [1].
So based on this I though it'd be best to keep "dwc-i2c" close to SNPS'
own naming, despite the fact that it does seem very similar to the
generic "snps,designware-i2c".
If you'd prefer to make it more explicit, I could change this to something
like "snps,dwc-adv-i2c"? Although, again, this does deviate from the
convention that SNPS uses in it's own documents.
[0]: DWC_i2c_reference.pdf
[1]: DW_apb_i2c_databook.pdf
Thanks for your review!
Aniket
>
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] i2c: designware: Introduce per-variant register offset and bit-layout tables
2026-09-19 9:06 ` [PATCH 2/3] i2c: designware: Introduce per-variant register offset and bit-layout tables Aniket Limaye
@ 2026-09-21 11:11 ` Mika Westerberg
2026-09-21 17:10 ` Aniket Limaye
2026-09-25 8:11 ` Andy Shevchenko
0 siblings, 2 replies; 16+ messages in thread
From: Mika Westerberg @ 2026-09-21 11:11 UTC (permalink / raw)
To: Aniket Limaye
Cc: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nirujogi Pratap, Bin Du, Andy Shevchenko, linux-i2c, devicetree,
linux-kernel, vigneshr, nm, u-kumar1, lianfeng.ouyang,
Ritwick.Sharma
Hi,
On Sat, Sep 19, 2026 at 02:36:07PM +0530, Aniket Limaye wrote:
> Every DW_IC_* register offset and CON-register bit position is currently
> baked in as a compile-time constant, which only works while there is a
> single register layout. Introduce a logical register-ID enum (enum
> dw_i2c_reg_idx) plus a per-variant offset table (dev->regs[]) and a
> per-variant CON-register bit-layout descriptor (dev->con_bits), selected
> at probe time via the new i2c_dw_select_variant().
>
> Replace every direct DW_IC_* offset/bit-position reference with a lookup
> through dev->regs[]/dev->con_bits. Also fold the read-to-clear
> interrupt-acknowledgment pattern into a new i2c_dw_ack_intr() helper,
> driven by a per-variant dev->intr_clr[] table.
>
> Only one variant exists at this point (DW_apb_i2c), so this is a
> mechanical, behavior-preserving change: the values in
> dw_i2c_reg_offsets[] and dw_i2c_con_bits match the DW_IC_* macros
> exactly. It lays the groundwork for adding a second register layout
> (DWC_i2c) without duplicating the whole driver.
>
> Signed-off-by: Aniket Limaye <a-limaye@ti.com>
> ---
> drivers/i2c/busses/i2c-designware-amdisp.c | 1 +
> drivers/i2c/busses/i2c-designware-common.c | 158 ++++++++++++++++++++++------
> drivers/i2c/busses/i2c-designware-core.h | 108 ++++++++++++++++++-
> drivers/i2c/busses/i2c-designware-master.c | 125 +++++++++++-----------
> drivers/i2c/busses/i2c-designware-pcidrv.c | 2 +
> drivers/i2c/busses/i2c-designware-platdrv.c | 2 +
> drivers/i2c/busses/i2c-designware-slave.c | 42 ++++----
> 7 files changed, 318 insertions(+), 120 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-amdisp.c b/drivers/i2c/busses/i2c-designware-amdisp.c
> index 9f0ec0fae6f2..f7aa075c9977 100644
> --- a/drivers/i2c/busses/i2c-designware-amdisp.c
> +++ b/drivers/i2c/busses/i2c-designware-amdisp.c
> @@ -46,6 +46,7 @@ static int amd_isp_dw_i2c_plat_probe(struct platform_device *pdev)
> isp_i2c_dev->flags |= ACCESS_POLLING;
> platform_set_drvdata(pdev, isp_i2c_dev);
>
> + i2c_dw_select_variant(isp_i2c_dev);
> isp_i2c_dev->base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(isp_i2c_dev->base))
> return dev_err_probe(&pdev->dev, PTR_ERR(isp_i2c_dev->base),
> diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
> index a1eca6cd4b75..a21aeb7f415a 100644
> --- a/drivers/i2c/busses/i2c-designware-common.c
> +++ b/drivers/i2c/busses/i2c-designware-common.c
> @@ -72,6 +72,95 @@ static const char *const abort_sources[] = {
> "incorrect slave-transmitter mode configuration",
> };
>
> +/* "snps,designware-i2c" flat register layout */
> +static const u32 dw_i2c_reg_offsets[DW_REG_IDX_MAX] = {
> + [DW_REG_IDX_CON] = DW_IC_CON,
> + [DW_REG_IDX_TAR] = DW_IC_TAR,
> + [DW_REG_IDX_SAR] = DW_IC_SAR,
> + [DW_REG_IDX_DATA_CMD] = DW_IC_DATA_CMD,
> + [DW_REG_IDX_SS_SCL_HCNT] = DW_IC_SS_SCL_HCNT,
> + [DW_REG_IDX_SS_SCL_LCNT] = DW_IC_SS_SCL_LCNT,
> + [DW_REG_IDX_FS_SCL_HCNT] = DW_IC_FS_SCL_HCNT,
> + [DW_REG_IDX_FS_SCL_LCNT] = DW_IC_FS_SCL_LCNT,
> + [DW_REG_IDX_HS_SCL_HCNT] = DW_IC_HS_SCL_HCNT,
> + [DW_REG_IDX_HS_SCL_LCNT] = DW_IC_HS_SCL_LCNT,
> + [DW_REG_IDX_INTR_STAT] = DW_IC_INTR_STAT,
> + [DW_REG_IDX_INTR_MASK] = DW_IC_INTR_MASK,
> + [DW_REG_IDX_RAW_INTR_STAT] = DW_IC_RAW_INTR_STAT,
> + [DW_REG_IDX_RX_TL] = DW_IC_RX_TL,
> + [DW_REG_IDX_TX_TL] = DW_IC_TX_TL,
> + [DW_REG_IDX_CLR_INTR] = DW_IC_CLR_INTR,
> + [DW_REG_IDX_CLR_RX_UNDER] = DW_IC_CLR_RX_UNDER,
> + [DW_REG_IDX_CLR_RX_OVER] = DW_IC_CLR_RX_OVER,
> + [DW_REG_IDX_CLR_TX_OVER] = DW_IC_CLR_TX_OVER,
> + [DW_REG_IDX_CLR_RD_REQ] = DW_IC_CLR_RD_REQ,
> + [DW_REG_IDX_CLR_TX_ABRT] = DW_IC_CLR_TX_ABRT,
> + [DW_REG_IDX_CLR_RX_DONE] = DW_IC_CLR_RX_DONE,
> + [DW_REG_IDX_CLR_ACTIVITY] = DW_IC_CLR_ACTIVITY,
> + [DW_REG_IDX_CLR_STOP_DET] = DW_IC_CLR_STOP_DET,
> + [DW_REG_IDX_CLR_START_DET] = DW_IC_CLR_START_DET,
> + [DW_REG_IDX_CLR_GEN_CALL] = DW_IC_CLR_GEN_CALL,
> + [DW_REG_IDX_ENABLE] = DW_IC_ENABLE,
> + [DW_REG_IDX_STATUS] = DW_IC_STATUS,
> + [DW_REG_IDX_TXFLR] = DW_IC_TXFLR,
> + [DW_REG_IDX_RXFLR] = DW_IC_RXFLR,
> + [DW_REG_IDX_SDA_HOLD] = DW_IC_SDA_HOLD,
> + [DW_REG_IDX_TX_ABRT_SOURCE] = DW_IC_TX_ABRT_SOURCE,
> + [DW_REG_IDX_ENABLE_STATUS] = DW_IC_ENABLE_STATUS,
> + [DW_REG_IDX_SMBUS_INTR_MASK] = DW_IC_SMBUS_INTR_MASK,
> + [DW_REG_IDX_COMP_PARAM_1] = DW_IC_COMP_PARAM_1,
> + [DW_REG_IDX_COMP_VERSION] = DW_IC_COMP_VERSION,
> + [DW_REG_IDX_COMP_TYPE] = DW_IC_COMP_TYPE,
> +};
> +
> +static const struct dw_i2c_con_bits dw_i2c_con_bits = {
> + .master = DW_IC_CON_MASTER,
> + .speed_std = DW_IC_CON_SPEED_STD,
> + .speed_fast = DW_IC_CON_SPEED_FAST,
> + .speed_high = DW_IC_CON_SPEED_HIGH,
> + .speed_mask = DW_IC_CON_SPEED_MASK,
> + .bit10_slave = DW_IC_CON_10BITADDR_SLAVE,
> + .bit10_master = DW_IC_CON_10BITADDR_MASTER,
> + .restart_en = DW_IC_CON_RESTART_EN,
> + .slave_disable = DW_IC_CON_SLAVE_DISABLE,
> + .stop_det_ifaddressed = DW_IC_CON_STOP_DET_IFADDRESSED,
> + .tx_empty_ctrl = DW_IC_CON_TX_EMPTY_CTRL,
> + .rx_fifo_full_hld_ctrl = DW_IC_CON_RX_FIFO_FULL_HLD_CTRL,
> + .bus_clear_ctrl = DW_IC_CON_BUS_CLEAR_CTRL,
> +};
> +
> +/* "snps,designware-i2c": dedicated read-to-clear register ID per logical interrupt */
> +static const u32 dw_i2c_intr_clr[DW_INTR_IDX_MAX] = {
> + [DW_INTR_IDX_ALL] = DW_REG_IDX_CLR_INTR,
> + [DW_INTR_IDX_RX_UNDER] = DW_REG_IDX_CLR_RX_UNDER,
> + [DW_INTR_IDX_RX_OVER] = DW_REG_IDX_CLR_RX_OVER,
> + [DW_INTR_IDX_TX_OVER] = DW_REG_IDX_CLR_TX_OVER,
> + [DW_INTR_IDX_RD_REQ] = DW_REG_IDX_CLR_RD_REQ,
> + [DW_INTR_IDX_TX_ABRT] = DW_REG_IDX_CLR_TX_ABRT,
> + [DW_INTR_IDX_RX_DONE] = DW_REG_IDX_CLR_RX_DONE,
> + [DW_INTR_IDX_ACTIVITY] = DW_REG_IDX_CLR_ACTIVITY,
> + [DW_INTR_IDX_STOP_DET] = DW_REG_IDX_CLR_STOP_DET,
> + [DW_INTR_IDX_START_DET] = DW_REG_IDX_CLR_START_DET,
> + [DW_INTR_IDX_GEN_CALL] = DW_REG_IDX_CLR_GEN_CALL,
> +};
> +
> +/**
> + * i2c_dw_select_variant() - Pick the register offset table, CON-register bit
> + * layout and interrupt-ack mapping matching this device's IP variant
> + * @dev: device private data
> + *
> + * Must be called after dev->flags has been populated from
> + * device_get_match_data()/ACPI id data, and before any register access
> + * (including i2c_dw_init_regmap()).
> + */
> +void i2c_dw_select_variant(struct dw_i2c_dev *dev)
> +{
> + dev->regs = dw_i2c_reg_offsets;
> + dev->con_bits = &dw_i2c_con_bits;
> + dev->intr_clr = dw_i2c_intr_clr;
> +}
> +EXPORT_SYMBOL_GPL(i2c_dw_select_variant);
> +
> static int dw_reg_read(void *context, unsigned int reg, unsigned int *val)
> {
> struct dw_i2c_dev *dev = context;
> @@ -147,7 +236,7 @@ static int i2c_dw_init_regmap(struct dw_i2c_dev *dev)
> .disable_locking = true,
> .reg_read = dw_reg_read,
> .reg_write = dw_reg_write,
> - .max_register = DW_IC_COMP_TYPE,
> + .max_register = dev->regs[DW_REG_IDX_COMP_TYPE],
> };
> u32 reg;
> int ret;
> @@ -163,7 +252,7 @@ static int i2c_dw_init_regmap(struct dw_i2c_dev *dev)
> if (ret)
> return ret;
>
> - reg = readl(dev->base + DW_IC_COMP_TYPE);
> + reg = readl(dev->base + dev->regs[DW_REG_IDX_COMP_TYPE]);
> i2c_dw_release_lock(dev);
>
> if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU)
> @@ -365,17 +454,17 @@ static void i2c_dw_configure_mode(struct dw_i2c_dev *dev, int mode)
> {
> switch (mode) {
> case DW_IC_MASTER:
> - regmap_write(dev->map, DW_IC_TX_TL, dev->tx_fifo_depth / 2);
> - regmap_write(dev->map, DW_IC_RX_TL, 0);
> - regmap_write(dev->map, DW_IC_CON, dev->master_cfg);
> + regmap_write(dev->map, dev->regs[DW_REG_IDX_TX_TL], dev->tx_fifo_depth / 2);
> + regmap_write(dev->map, dev->regs[DW_REG_IDX_RX_TL], 0);
> + regmap_write(dev->map, dev->regs[DW_REG_IDX_CON], dev->master_cfg);
Instead of all this. Can't you do this inside the regmap so that here and
elsewhere in the driver we continue to do:
regmap_write(dev->map, DW_IC_RX_TL, 0);
but internally, depending on the hardware it then maps this into the
corresponding register offset.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: i2c: dw: Add DWC_i2c compatible
2026-09-21 8:25 ` Aniket Limaye
@ 2026-09-21 11:14 ` Mika Westerberg
2026-09-21 14:00 ` Krzysztof Kozlowski
0 siblings, 1 reply; 16+ messages in thread
From: Mika Westerberg @ 2026-09-21 11:14 UTC (permalink / raw)
To: Aniket Limaye
Cc: Krzysztof Kozlowski, Andi Shyti, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Nirujogi Pratap, Bin Du, Andy Shevchenko, linux-i2c,
devicetree, linux-kernel, vigneshr, nm, u-kumar1, lianfeng.ouyang,
Ritwick.Sharma
Hi,
On Mon, Sep 21, 2026 at 01:55:23PM +0530, Aniket Limaye wrote:
>
> On 20/09/26 23:54, Krzysztof Kozlowski wrote:
> > On Sat, Sep 19, 2026 at 02:36:06PM +0530, Aniket Limaye wrote:
> > > Add the "snps,dwc-i2c" compatible for Synopsys Advanced I2C Controller
> > > referred as DWC_i2c. Compared to the existing "snps,designware-i2c"
> > dwc is designware, so snps,dwc-i2c = snps,designware-i2c
> >
> > I think this is *the* argument why generic compatibles are wrong.
>
> These are 2 different controllers with some different registers and
> vastly different register offsets.
>
> So, the official product name (short format) for the SNPS Advanced I2C
> controller is "DWC_i2c", as per SNPS' own reference document [0].
> Whereas the original "snps,designware-i2c" corresponds to the product
> name "DW_apb_i2c" as per its own databook [1].
>
> So based on this I though it'd be best to keep "dwc-i2c" close to SNPS'
> own naming, despite the fact that it does seem very similar to the
> generic "snps,designware-i2c".
>
> If you'd prefer to make it more explicit, I could change this to something
> like "snps,dwc-adv-i2c"? Although, again, this does deviate from the
> convention that SNPS uses in it's own documents.
What about just "snps,ai2c" or "snps,advanced-i2c"?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: i2c: dw: Add DWC_i2c compatible
2026-09-21 11:14 ` Mika Westerberg
@ 2026-09-21 14:00 ` Krzysztof Kozlowski
2026-09-21 14:10 ` Mika Westerberg
0 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2026-09-21 14:00 UTC (permalink / raw)
To: Mika Westerberg, Aniket Limaye
Cc: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nirujogi Pratap, Bin Du, Andy Shevchenko, linux-i2c, devicetree,
linux-kernel, vigneshr, nm, u-kumar1, lianfeng.ouyang,
Ritwick.Sharma
On 21/09/2026 13:14, Mika Westerberg wrote:
> Hi,
>
> On Mon, Sep 21, 2026 at 01:55:23PM +0530, Aniket Limaye wrote:
>>
>> On 20/09/26 23:54, Krzysztof Kozlowski wrote:
>>> On Sat, Sep 19, 2026 at 02:36:06PM +0530, Aniket Limaye wrote:
>>>> Add the "snps,dwc-i2c" compatible for Synopsys Advanced I2C Controller
>>>> referred as DWC_i2c. Compared to the existing "snps,designware-i2c"
>>> dwc is designware, so snps,dwc-i2c = snps,designware-i2c
>>>
>>> I think this is *the* argument why generic compatibles are wrong.
>>
>> These are 2 different controllers with some different registers and
>> vastly different register offsets.
>>
>> So, the official product name (short format) for the SNPS Advanced I2C
>> controller is "DWC_i2c", as per SNPS' own reference document [0].
>> Whereas the original "snps,designware-i2c" corresponds to the product
>> name "DW_apb_i2c" as per its own databook [1].
>>
>> So based on this I though it'd be best to keep "dwc-i2c" close to SNPS'
>> own naming, despite the fact that it does seem very similar to the
>> generic "snps,designware-i2c".
>>
>> If you'd prefer to make it more explicit, I could change this to something
>> like "snps,dwc-adv-i2c"? Although, again, this does deviate from the
>> convention that SNPS uses in it's own documents.
>
> What about just "snps,ai2c" or "snps,advanced-i2c"?
Can we just drop generic compatible completely instead? The block cannot
work alone, needs SoC integration.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: i2c: dw: Add DWC_i2c compatible
2026-09-21 14:00 ` Krzysztof Kozlowski
@ 2026-09-21 14:10 ` Mika Westerberg
2026-09-21 17:25 ` Aniket Limaye
0 siblings, 1 reply; 16+ messages in thread
From: Mika Westerberg @ 2026-09-21 14:10 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Aniket Limaye, Andi Shyti, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Nirujogi Pratap, Bin Du, Andy Shevchenko, linux-i2c,
devicetree, linux-kernel, vigneshr, nm, u-kumar1, lianfeng.ouyang,
Ritwick.Sharma
On Mon, Sep 21, 2026 at 04:00:52PM +0200, Krzysztof Kozlowski wrote:
> On 21/09/2026 13:14, Mika Westerberg wrote:
> > Hi,
> >
> > On Mon, Sep 21, 2026 at 01:55:23PM +0530, Aniket Limaye wrote:
> >>
> >> On 20/09/26 23:54, Krzysztof Kozlowski wrote:
> >>> On Sat, Sep 19, 2026 at 02:36:06PM +0530, Aniket Limaye wrote:
> >>>> Add the "snps,dwc-i2c" compatible for Synopsys Advanced I2C Controller
> >>>> referred as DWC_i2c. Compared to the existing "snps,designware-i2c"
> >>> dwc is designware, so snps,dwc-i2c = snps,designware-i2c
> >>>
> >>> I think this is *the* argument why generic compatibles are wrong.
> >>
> >> These are 2 different controllers with some different registers and
> >> vastly different register offsets.
> >>
> >> So, the official product name (short format) for the SNPS Advanced I2C
> >> controller is "DWC_i2c", as per SNPS' own reference document [0].
> >> Whereas the original "snps,designware-i2c" corresponds to the product
> >> name "DW_apb_i2c" as per its own databook [1].
> >>
> >> So based on this I though it'd be best to keep "dwc-i2c" close to SNPS'
> >> own naming, despite the fact that it does seem very similar to the
> >> generic "snps,designware-i2c".
> >>
> >> If you'd prefer to make it more explicit, I could change this to something
> >> like "snps,dwc-adv-i2c"? Although, again, this does deviate from the
> >> convention that SNPS uses in it's own documents.
> >
> > What about just "snps,ai2c" or "snps,advanced-i2c"?
>
> Can we just drop generic compatible completely instead? The block cannot
> work alone, needs SoC integration.
Works for me.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] i2c: designware: Introduce per-variant register offset and bit-layout tables
2026-09-21 11:11 ` Mika Westerberg
@ 2026-09-21 17:10 ` Aniket Limaye
2026-09-25 8:11 ` Andy Shevchenko
1 sibling, 0 replies; 16+ messages in thread
From: Aniket Limaye @ 2026-09-21 17:10 UTC (permalink / raw)
To: Mika Westerberg
Cc: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nirujogi Pratap, Bin Du, Andy Shevchenko, linux-i2c, devicetree,
linux-kernel, vigneshr, nm, u-kumar1, lianfeng.ouyang,
Ritwick.Sharma
On 21/09/26 16:41, Mika Westerberg wrote:
> Hi,
>
> On Sat, Sep 19, 2026 at 02:36:07PM +0530, Aniket Limaye wrote:
>> Every DW_IC_* register offset and CON-register bit position is currently
>> baked in as a compile-time constant, which only works while there is a
>> single register layout. Introduce a logical register-ID enum (enum
>> dw_i2c_reg_idx) plus a per-variant offset table (dev->regs[]) and a
>> per-variant CON-register bit-layout descriptor (dev->con_bits), selected
>> at probe time via the new i2c_dw_select_variant().
>>
>> Replace every direct DW_IC_* offset/bit-position reference with a lookup
>> through dev->regs[]/dev->con_bits. Also fold the read-to-clear
>> interrupt-acknowledgment pattern into a new i2c_dw_ack_intr() helper,
>> driven by a per-variant dev->intr_clr[] table.
>>
>> Only one variant exists at this point (DW_apb_i2c), so this is a
>> mechanical, behavior-preserving change: the values in
>> dw_i2c_reg_offsets[] and dw_i2c_con_bits match the DW_IC_* macros
>> exactly. It lays the groundwork for adding a second register layout
>> (DWC_i2c) without duplicating the whole driver.
>>
>> Signed-off-by: Aniket Limaye <a-limaye@ti.com>
>> ---
>> drivers/i2c/busses/i2c-designware-amdisp.c | 1 +
>> drivers/i2c/busses/i2c-designware-common.c | 158 ++++++++++++++++++++++------
>> drivers/i2c/busses/i2c-designware-core.h | 108 ++++++++++++++++++-
>> drivers/i2c/busses/i2c-designware-master.c | 125 +++++++++++-----------
>> drivers/i2c/busses/i2c-designware-pcidrv.c | 2 +
>> drivers/i2c/busses/i2c-designware-platdrv.c | 2 +
>> drivers/i2c/busses/i2c-designware-slave.c | 42 ++++----
>> 7 files changed, 318 insertions(+), 120 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-designware-amdisp.c b/drivers/i2c/busses/i2c-designware-amdisp.c
>> index 9f0ec0fae6f2..f7aa075c9977 100644
>> --- a/drivers/i2c/busses/i2c-designware-amdisp.c
>> +++ b/drivers/i2c/busses/i2c-designware-amdisp.c
>> @@ -46,6 +46,7 @@ static int amd_isp_dw_i2c_plat_probe(struct platform_device *pdev)
>> isp_i2c_dev->flags |= ACCESS_POLLING;
>> platform_set_drvdata(pdev, isp_i2c_dev);
>>
>> + i2c_dw_select_variant(isp_i2c_dev);
>> isp_i2c_dev->base = devm_platform_ioremap_resource(pdev, 0);
>> if (IS_ERR(isp_i2c_dev->base))
>> return dev_err_probe(&pdev->dev, PTR_ERR(isp_i2c_dev->base),
>> diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
>> index a1eca6cd4b75..a21aeb7f415a 100644
>> --- a/drivers/i2c/busses/i2c-designware-common.c
>> +++ b/drivers/i2c/busses/i2c-designware-common.c
>> @@ -72,6 +72,95 @@ static const char *const abort_sources[] = {
>> "incorrect slave-transmitter mode configuration",
>> };
>>
>> +/* "snps,designware-i2c" flat register layout */
>> +static const u32 dw_i2c_reg_offsets[DW_REG_IDX_MAX] = {
>> + [DW_REG_IDX_CON] = DW_IC_CON,
>> + [DW_REG_IDX_TAR] = DW_IC_TAR,
>> + [DW_REG_IDX_SAR] = DW_IC_SAR,
>> + [DW_REG_IDX_DATA_CMD] = DW_IC_DATA_CMD,
>> + [DW_REG_IDX_SS_SCL_HCNT] = DW_IC_SS_SCL_HCNT,
>> + [DW_REG_IDX_SS_SCL_LCNT] = DW_IC_SS_SCL_LCNT,
>> + [DW_REG_IDX_FS_SCL_HCNT] = DW_IC_FS_SCL_HCNT,
>> + [DW_REG_IDX_FS_SCL_LCNT] = DW_IC_FS_SCL_LCNT,
>> + [DW_REG_IDX_HS_SCL_HCNT] = DW_IC_HS_SCL_HCNT,
>> + [DW_REG_IDX_HS_SCL_LCNT] = DW_IC_HS_SCL_LCNT,
>> + [DW_REG_IDX_INTR_STAT] = DW_IC_INTR_STAT,
>> + [DW_REG_IDX_INTR_MASK] = DW_IC_INTR_MASK,
>> + [DW_REG_IDX_RAW_INTR_STAT] = DW_IC_RAW_INTR_STAT,
>> + [DW_REG_IDX_RX_TL] = DW_IC_RX_TL,
>> + [DW_REG_IDX_TX_TL] = DW_IC_TX_TL,
>> + [DW_REG_IDX_CLR_INTR] = DW_IC_CLR_INTR,
>> + [DW_REG_IDX_CLR_RX_UNDER] = DW_IC_CLR_RX_UNDER,
>> + [DW_REG_IDX_CLR_RX_OVER] = DW_IC_CLR_RX_OVER,
>> + [DW_REG_IDX_CLR_TX_OVER] = DW_IC_CLR_TX_OVER,
>> + [DW_REG_IDX_CLR_RD_REQ] = DW_IC_CLR_RD_REQ,
>> + [DW_REG_IDX_CLR_TX_ABRT] = DW_IC_CLR_TX_ABRT,
>> + [DW_REG_IDX_CLR_RX_DONE] = DW_IC_CLR_RX_DONE,
>> + [DW_REG_IDX_CLR_ACTIVITY] = DW_IC_CLR_ACTIVITY,
>> + [DW_REG_IDX_CLR_STOP_DET] = DW_IC_CLR_STOP_DET,
>> + [DW_REG_IDX_CLR_START_DET] = DW_IC_CLR_START_DET,
>> + [DW_REG_IDX_CLR_GEN_CALL] = DW_IC_CLR_GEN_CALL,
>> + [DW_REG_IDX_ENABLE] = DW_IC_ENABLE,
>> + [DW_REG_IDX_STATUS] = DW_IC_STATUS,
>> + [DW_REG_IDX_TXFLR] = DW_IC_TXFLR,
>> + [DW_REG_IDX_RXFLR] = DW_IC_RXFLR,
>> + [DW_REG_IDX_SDA_HOLD] = DW_IC_SDA_HOLD,
>> + [DW_REG_IDX_TX_ABRT_SOURCE] = DW_IC_TX_ABRT_SOURCE,
>> + [DW_REG_IDX_ENABLE_STATUS] = DW_IC_ENABLE_STATUS,
>> + [DW_REG_IDX_SMBUS_INTR_MASK] = DW_IC_SMBUS_INTR_MASK,
>> + [DW_REG_IDX_COMP_PARAM_1] = DW_IC_COMP_PARAM_1,
>> + [DW_REG_IDX_COMP_VERSION] = DW_IC_COMP_VERSION,
>> + [DW_REG_IDX_COMP_TYPE] = DW_IC_COMP_TYPE,
>> +};
>> +
>> +static const struct dw_i2c_con_bits dw_i2c_con_bits = {
>> + .master = DW_IC_CON_MASTER,
>> + .speed_std = DW_IC_CON_SPEED_STD,
>> + .speed_fast = DW_IC_CON_SPEED_FAST,
>> + .speed_high = DW_IC_CON_SPEED_HIGH,
>> + .speed_mask = DW_IC_CON_SPEED_MASK,
>> + .bit10_slave = DW_IC_CON_10BITADDR_SLAVE,
>> + .bit10_master = DW_IC_CON_10BITADDR_MASTER,
>> + .restart_en = DW_IC_CON_RESTART_EN,
>> + .slave_disable = DW_IC_CON_SLAVE_DISABLE,
>> + .stop_det_ifaddressed = DW_IC_CON_STOP_DET_IFADDRESSED,
>> + .tx_empty_ctrl = DW_IC_CON_TX_EMPTY_CTRL,
>> + .rx_fifo_full_hld_ctrl = DW_IC_CON_RX_FIFO_FULL_HLD_CTRL,
>> + .bus_clear_ctrl = DW_IC_CON_BUS_CLEAR_CTRL,
>> +};
>> +
>> +/* "snps,designware-i2c": dedicated read-to-clear register ID per logical interrupt */
>> +static const u32 dw_i2c_intr_clr[DW_INTR_IDX_MAX] = {
>> + [DW_INTR_IDX_ALL] = DW_REG_IDX_CLR_INTR,
>> + [DW_INTR_IDX_RX_UNDER] = DW_REG_IDX_CLR_RX_UNDER,
>> + [DW_INTR_IDX_RX_OVER] = DW_REG_IDX_CLR_RX_OVER,
>> + [DW_INTR_IDX_TX_OVER] = DW_REG_IDX_CLR_TX_OVER,
>> + [DW_INTR_IDX_RD_REQ] = DW_REG_IDX_CLR_RD_REQ,
>> + [DW_INTR_IDX_TX_ABRT] = DW_REG_IDX_CLR_TX_ABRT,
>> + [DW_INTR_IDX_RX_DONE] = DW_REG_IDX_CLR_RX_DONE,
>> + [DW_INTR_IDX_ACTIVITY] = DW_REG_IDX_CLR_ACTIVITY,
>> + [DW_INTR_IDX_STOP_DET] = DW_REG_IDX_CLR_STOP_DET,
>> + [DW_INTR_IDX_START_DET] = DW_REG_IDX_CLR_START_DET,
>> + [DW_INTR_IDX_GEN_CALL] = DW_REG_IDX_CLR_GEN_CALL,
>> +};
>> +
>> +/**
>> + * i2c_dw_select_variant() - Pick the register offset table, CON-register bit
>> + * layout and interrupt-ack mapping matching this device's IP variant
>> + * @dev: device private data
>> + *
>> + * Must be called after dev->flags has been populated from
>> + * device_get_match_data()/ACPI id data, and before any register access
>> + * (including i2c_dw_init_regmap()).
>> + */
>> +void i2c_dw_select_variant(struct dw_i2c_dev *dev)
>> +{
>> + dev->regs = dw_i2c_reg_offsets;
>> + dev->con_bits = &dw_i2c_con_bits;
>> + dev->intr_clr = dw_i2c_intr_clr;
>> +}
>> +EXPORT_SYMBOL_GPL(i2c_dw_select_variant);
>> +
>> static int dw_reg_read(void *context, unsigned int reg, unsigned int *val)
>> {
>> struct dw_i2c_dev *dev = context;
>> @@ -147,7 +236,7 @@ static int i2c_dw_init_regmap(struct dw_i2c_dev *dev)
>> .disable_locking = true,
>> .reg_read = dw_reg_read,
>> .reg_write = dw_reg_write,
>> - .max_register = DW_IC_COMP_TYPE,
>> + .max_register = dev->regs[DW_REG_IDX_COMP_TYPE],
>> };
>> u32 reg;
>> int ret;
>> @@ -163,7 +252,7 @@ static int i2c_dw_init_regmap(struct dw_i2c_dev *dev)
>> if (ret)
>> return ret;
>>
>> - reg = readl(dev->base + DW_IC_COMP_TYPE);
>> + reg = readl(dev->base + dev->regs[DW_REG_IDX_COMP_TYPE]);
>> i2c_dw_release_lock(dev);
>>
>> if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU)
>> @@ -365,17 +454,17 @@ static void i2c_dw_configure_mode(struct dw_i2c_dev *dev, int mode)
>> {
>> switch (mode) {
>> case DW_IC_MASTER:
>> - regmap_write(dev->map, DW_IC_TX_TL, dev->tx_fifo_depth / 2);
>> - regmap_write(dev->map, DW_IC_RX_TL, 0);
>> - regmap_write(dev->map, DW_IC_CON, dev->master_cfg);
>> + regmap_write(dev->map, dev->regs[DW_REG_IDX_TX_TL], dev->tx_fifo_depth / 2);
>> + regmap_write(dev->map, dev->regs[DW_REG_IDX_RX_TL], 0);
>> + regmap_write(dev->map, dev->regs[DW_REG_IDX_CON], dev->master_cfg);
> Instead of all this. Can't you do this inside the regmap so that here and
> elsewhere in the driver we continue to do:
>
> regmap_write(dev->map, DW_IC_RX_TL, 0);
>
> but internally, depending on the hardware it then maps this into the
> corresponding register offset.
Yeah that's definitely cleaner... will do in v2.
Thanks,
Aniket
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: i2c: dw: Add DWC_i2c compatible
2026-09-21 14:10 ` Mika Westerberg
@ 2026-09-21 17:25 ` Aniket Limaye
2026-09-22 8:50 ` Krzysztof Kozlowski
0 siblings, 1 reply; 16+ messages in thread
From: Aniket Limaye @ 2026-09-21 17:25 UTC (permalink / raw)
To: Mika Westerberg, Krzysztof Kozlowski
Cc: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nirujogi Pratap, Bin Du, Andy Shevchenko, linux-i2c, devicetree,
linux-kernel, vigneshr, nm, u-kumar1, lianfeng.ouyang,
Ritwick.Sharma
On 21/09/26 19:40, Mika Westerberg wrote:
> On Mon, Sep 21, 2026 at 04:00:52PM +0200, Krzysztof Kozlowski wrote:
>> On 21/09/2026 13:14, Mika Westerberg wrote:
>>> Hi,
>>>
>>> On Mon, Sep 21, 2026 at 01:55:23PM +0530, Aniket Limaye wrote:
>>>> On 20/09/26 23:54, Krzysztof Kozlowski wrote:
>>>>> On Sat, Sep 19, 2026 at 02:36:06PM +0530, Aniket Limaye wrote:
>>>>>> Add the "snps,dwc-i2c" compatible for Synopsys Advanced I2C Controller
>>>>>> referred as DWC_i2c. Compared to the existing "snps,designware-i2c"
>>>>> dwc is designware, so snps,dwc-i2c = snps,designware-i2c
>>>>>
>>>>> I think this is *the* argument why generic compatibles are wrong.
>>>> These are 2 different controllers with some different registers and
>>>> vastly different register offsets.
>>>>
>>>> So, the official product name (short format) for the SNPS Advanced I2C
>>>> controller is "DWC_i2c", as per SNPS' own reference document [0].
>>>> Whereas the original "snps,designware-i2c" corresponds to the product
>>>> name "DW_apb_i2c" as per its own databook [1].
>>>>
>>>> So based on this I though it'd be best to keep "dwc-i2c" close to SNPS'
>>>> own naming, despite the fact that it does seem very similar to the
>>>> generic "snps,designware-i2c".
>>>>
>>>> If you'd prefer to make it more explicit, I could change this to something
>>>> like "snps,dwc-adv-i2c"? Although, again, this does deviate from the
>>>> convention that SNPS uses in it's own documents.
>>> What about just "snps,ai2c" or "snps,advanced-i2c"?
>> Can we just drop generic compatible completely instead? The block cannot
>> work alone, needs SoC integration.
> Works for me.
Original intent was to try to develop a generic driver so that it could be
re-used by other SoCs with some more work (referring to an earlier
series [0])
To make sure I understand you guys, you prefer TDA54 SoC based compatible
since the driver itself currently has TDA54 based offsets?
[0]:
https://lore.kernel.org/all/20260527085039.44435-1-lianfeng.ouyang@starfivetech.com/
Thanks,
Aniket
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: i2c: dw: Add DWC_i2c compatible
2026-09-21 17:25 ` Aniket Limaye
@ 2026-09-22 8:50 ` Krzysztof Kozlowski
0 siblings, 0 replies; 16+ messages in thread
From: Krzysztof Kozlowski @ 2026-09-22 8:50 UTC (permalink / raw)
To: Aniket Limaye, Mika Westerberg
Cc: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nirujogi Pratap, Bin Du, Andy Shevchenko, linux-i2c, devicetree,
linux-kernel, vigneshr, nm, u-kumar1, lianfeng.ouyang,
Ritwick.Sharma
On 21/09/2026 19:25, Aniket Limaye wrote:
>
> On 21/09/26 19:40, Mika Westerberg wrote:
>> On Mon, Sep 21, 2026 at 04:00:52PM +0200, Krzysztof Kozlowski wrote:
>>> On 21/09/2026 13:14, Mika Westerberg wrote:
>>>> Hi,
>>>>
>>>> On Mon, Sep 21, 2026 at 01:55:23PM +0530, Aniket Limaye wrote:
>>>>> On 20/09/26 23:54, Krzysztof Kozlowski wrote:
>>>>>> On Sat, Sep 19, 2026 at 02:36:06PM +0530, Aniket Limaye wrote:
>>>>>>> Add the "snps,dwc-i2c" compatible for Synopsys Advanced I2C Controller
>>>>>>> referred as DWC_i2c. Compared to the existing "snps,designware-i2c"
>>>>>> dwc is designware, so snps,dwc-i2c = snps,designware-i2c
>>>>>>
>>>>>> I think this is *the* argument why generic compatibles are wrong.
>>>>> These are 2 different controllers with some different registers and
>>>>> vastly different register offsets.
>>>>>
>>>>> So, the official product name (short format) for the SNPS Advanced I2C
>>>>> controller is "DWC_i2c", as per SNPS' own reference document [0].
>>>>> Whereas the original "snps,designware-i2c" corresponds to the product
>>>>> name "DW_apb_i2c" as per its own databook [1].
>>>>>
>>>>> So based on this I though it'd be best to keep "dwc-i2c" close to SNPS'
>>>>> own naming, despite the fact that it does seem very similar to the
>>>>> generic "snps,designware-i2c".
>>>>>
>>>>> If you'd prefer to make it more explicit, I could change this to something
>>>>> like "snps,dwc-adv-i2c"? Although, again, this does deviate from the
>>>>> convention that SNPS uses in it's own documents.
>>>> What about just "snps,ai2c" or "snps,advanced-i2c"?
>>> Can we just drop generic compatible completely instead? The block cannot
>>> work alone, needs SoC integration.
>> Works for me.
>
> Original intent was to try to develop a generic driver so that it could be
> re-used by other SoCs with some more work (referring to an earlier
> series [0])
>
I did not comment on your driver. I did not even look there.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] i2c: designware: Introduce per-variant register offset and bit-layout tables
2026-09-21 11:11 ` Mika Westerberg
2026-09-21 17:10 ` Aniket Limaye
@ 2026-09-25 8:11 ` Andy Shevchenko
2026-09-25 9:26 ` Aniket Limaye
1 sibling, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2026-09-25 8:11 UTC (permalink / raw)
To: Mika Westerberg
Cc: Aniket Limaye, Andi Shyti, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Nirujogi Pratap, Bin Du, linux-i2c, devicetree,
linux-kernel, vigneshr, nm, u-kumar1, lianfeng.ouyang,
Ritwick.Sharma
On Mon, Sep 21, 2026 at 01:11:48PM +0200, Mika Westerberg wrote:
> On Sat, Sep 19, 2026 at 02:36:07PM +0530, Aniket Limaye wrote:
> > Every DW_IC_* register offset and CON-register bit position is currently
> > baked in as a compile-time constant, which only works while there is a
> > single register layout. Introduce a logical register-ID enum (enum
> > dw_i2c_reg_idx) plus a per-variant offset table (dev->regs[]) and a
> > per-variant CON-register bit-layout descriptor (dev->con_bits), selected
> > at probe time via the new i2c_dw_select_variant().
> >
> > Replace every direct DW_IC_* offset/bit-position reference with a lookup
> > through dev->regs[]/dev->con_bits. Also fold the read-to-clear
> > interrupt-acknowledgment pattern into a new i2c_dw_ack_intr() helper,
> > driven by a per-variant dev->intr_clr[] table.
> >
> > Only one variant exists at this point (DW_apb_i2c), so this is a
> > mechanical, behavior-preserving change: the values in
> > dw_i2c_reg_offsets[] and dw_i2c_con_bits match the DW_IC_* macros
> > exactly. It lays the groundwork for adding a second register layout
> > (DWC_i2c) without duplicating the whole driver.
...
> > - regmap_write(dev->map, DW_IC_TX_TL, dev->tx_fifo_depth / 2);
> > - regmap_write(dev->map, DW_IC_RX_TL, 0);
> > - regmap_write(dev->map, DW_IC_CON, dev->master_cfg);
> > + regmap_write(dev->map, dev->regs[DW_REG_IDX_TX_TL], dev->tx_fifo_depth / 2);
> > + regmap_write(dev->map, dev->regs[DW_REG_IDX_RX_TL], 0);
> > + regmap_write(dev->map, dev->regs[DW_REG_IDX_CON], dev->master_cfg);
>
> Instead of all this. Can't you do this inside the regmap so that here and
> elsewhere in the driver we continue to do:
>
> regmap_write(dev->map, DW_IC_RX_TL, 0);
>
> but internally, depending on the hardware it then maps this into the
> corresponding register offset.
Exactly what I was going to say when I hit "reply".
These series is definitely NAKed (in terms of the approach taken).
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] i2c: designware: Introduce per-variant register offset and bit-layout tables
2026-09-25 8:11 ` Andy Shevchenko
@ 2026-09-25 9:26 ` Aniket Limaye
0 siblings, 0 replies; 16+ messages in thread
From: Aniket Limaye @ 2026-09-25 9:26 UTC (permalink / raw)
To: Andy Shevchenko, Mika Westerberg
Cc: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nirujogi Pratap, Bin Du, linux-i2c, devicetree, linux-kernel,
vigneshr, nm, u-kumar1, lianfeng.ouyang, Ritwick.Sharma
On 25/09/26 13:41, Andy Shevchenko wrote:
> On Mon, Sep 21, 2026 at 01:11:48PM +0200, Mika Westerberg wrote:
>> On Sat, Sep 19, 2026 at 02:36:07PM +0530, Aniket Limaye wrote:
>>> Every DW_IC_* register offset and CON-register bit position is currently
>>> baked in as a compile-time constant, which only works while there is a
>>> single register layout. Introduce a logical register-ID enum (enum
>>> dw_i2c_reg_idx) plus a per-variant offset table (dev->regs[]) and a
>>> per-variant CON-register bit-layout descriptor (dev->con_bits), selected
>>> at probe time via the new i2c_dw_select_variant().
>>>
>>> Replace every direct DW_IC_* offset/bit-position reference with a lookup
>>> through dev->regs[]/dev->con_bits. Also fold the read-to-clear
>>> interrupt-acknowledgment pattern into a new i2c_dw_ack_intr() helper,
>>> driven by a per-variant dev->intr_clr[] table.
>>>
>>> Only one variant exists at this point (DW_apb_i2c), so this is a
>>> mechanical, behavior-preserving change: the values in
>>> dw_i2c_reg_offsets[] and dw_i2c_con_bits match the DW_IC_* macros
>>> exactly. It lays the groundwork for adding a second register layout
>>> (DWC_i2c) without duplicating the whole driver.
> ...
>
>>> - regmap_write(dev->map, DW_IC_TX_TL, dev->tx_fifo_depth / 2);
>>> - regmap_write(dev->map, DW_IC_RX_TL, 0);
>>> - regmap_write(dev->map, DW_IC_CON, dev->master_cfg);
>>> + regmap_write(dev->map, dev->regs[DW_REG_IDX_TX_TL], dev->tx_fifo_depth / 2);
>>> + regmap_write(dev->map, dev->regs[DW_REG_IDX_RX_TL], 0);
>>> + regmap_write(dev->map, dev->regs[DW_REG_IDX_CON], dev->master_cfg);
>> Instead of all this. Can't you do this inside the regmap so that here and
>> elsewhere in the driver we continue to do:
>>
>> regmap_write(dev->map, DW_IC_RX_TL, 0);
>>
>> but internally, depending on the hardware it then maps this into the
>> corresponding register offset.
> Exactly what I was going to say when I hit "reply".
> These series is definitely NAKed (in terms of the approach taken).
>
Hello Mika, Andy,
I have posted a v3 for the series [0] with a different approach,
preserving existing call sites.
Note that v3 version makes changes to all regmaps (native and inherited)
to now handle enums instead of actual offsets.
[0]:
https://lore.kernel.org/all/20260925-tda54-upstream-i2c-v3-0-544d74e992ff@ti.com/
Thanks for your reviews!
Aniket
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-09-25 9:27 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-19 9:06 [PATCH 0/3] i2c: designware: Add DWC_i2c support Aniket Limaye
2026-09-19 9:06 ` [PATCH 1/3] dt-bindings: i2c: dw: Add DWC_i2c compatible Aniket Limaye
2026-09-20 18:24 ` Krzysztof Kozlowski
2026-09-21 8:25 ` Aniket Limaye
2026-09-21 11:14 ` Mika Westerberg
2026-09-21 14:00 ` Krzysztof Kozlowski
2026-09-21 14:10 ` Mika Westerberg
2026-09-21 17:25 ` Aniket Limaye
2026-09-22 8:50 ` Krzysztof Kozlowski
2026-09-19 9:06 ` [PATCH 2/3] i2c: designware: Introduce per-variant register offset and bit-layout tables Aniket Limaye
2026-09-21 11:11 ` Mika Westerberg
2026-09-21 17:10 ` Aniket Limaye
2026-09-25 8:11 ` Andy Shevchenko
2026-09-25 9:26 ` Aniket Limaye
2026-09-19 9:06 ` [PATCH 3/3] i2c: designware: Add snps,dwc-i2c support and new compatible Aniket Limaye
2026-09-19 9:15 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox