* [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; 13+ 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] 13+ 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; 13+ 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 [flat|nested] 13+ 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; 13+ 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 [flat|nested] 13+ 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
2 siblings, 0 replies; 13+ 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 [flat|nested] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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
0 siblings, 1 reply; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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
0 siblings, 0 replies; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ messages in thread
end of thread, other threads:[~2026-09-22 8:50 UTC | newest]
Thread overview: 13+ 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-19 9:06 ` [PATCH 3/3] i2c: designware: Add snps,dwc-i2c support and new compatible Aniket Limaye
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®