Re: [PATCH v3 0/3] i2c: designware: Add TI TDA54 I2C support

[Aniket Limaye <a-limaye@ti.com>]

On 25/09/26 15:40, Andy Shevchenko wrote:
> On Fri, Sep 25, 2026 at 03:29:27PM +0530, Aniket Limaye wrote:
>> On 25/09/26 15:11, Andy Shevchenko wrote:
>>> On Fri, Sep 25, 2026 at 12:26:27PM +0530, Aniket Limaye wrote:
> ...
>
>>> Still doesn't look good. The current register layout may be left as is. What
>>> you need is translate it in the respective regmap callbacks in case we are
>>> enumerated on the different IP. Also possible to have a different regmap
>>> config for the different HW where you translate them only in one place.
>> Is it confusing to keep using existing offsets in regmap_read/write() call
>> sites for TDA54, and let the regmap silently handle the translation?
>>
>> Given that we do *not* have any new registers in use on the tda54 version
>> that were not there in the original one, I guess it works... will send a v4
>> as per your suggestion.
> Depends on the mapping. Your series also forgot to provide the differences
> Check this as an example: e539f435cb9c ("spi: dw: Add support for DesignWare DWC_ssi").
>
Ahh sorry about that, will remember to add a clean mapping in cover 
letter of next version.

For now here are the structural differences:

1.  Reg offsets  Existing        New TDA54

     [DW_IC_CON]                     0x00           0x2c
     [DW_IC_TAR]                     0x04            0x30
     [DW_IC_SAR]                     0x08            0x34
     [DW_IC_DATA_CMD]                0x10            0x80
     [DW_IC_SS_SCL_HCNT]             0x14            0x4c
     [DW_IC_SS_SCL_LCNT]             0x18            0x50
     [DW_IC_FS_SCL_HCNT]             0x1c            0x4c  /* same as SS */
     [DW_IC_FS_SCL_LCNT]             0x20            0x50  /* same as SS */
     [DW_IC_HS_SCL_HCNT]             0x24         0x54
     [DW_IC_HS_SCL_LCNT]             0x28            0x58
     [DW_IC_INTR_STAT]               0x2c            0xc0
     [DW_IC_INTR_MASK]               0x30            0xc4
     [DW_IC_RAW_INTR_STAT]           0x34            0xc8
     [DW_IC_RX_TL]                   0x38            0x84
     [DW_IC_TX_TL]                   0x3c            0x88
     [DW_IC_CLR_INTR]                0x40            0xcc
     [DW_IC_CLR_RX_UNDER]            0x44            NA
     [DW_IC_CLR_RX_OVER]             0x48            NA
     [DW_IC_CLR_TX_OVER]             0x4c            NA
     [DW_IC_CLR_RD_REQ]              0x50            NA
     [DW_IC_CLR_TX_ABRT]             0x54            NA
     [DW_IC_CLR_RX_DONE]             0x58            NA
     [DW_IC_CLR_ACTIVITY]            0x5c            NA
     [DW_IC_CLR_STOP_DET]            0x60            NA
     [DW_IC_CLR_START_DET]           0x64            NA
     [DW_IC_CLR_GEN_CALL]            0x68            NA
     [DW_IC_ENABLE]                  0x6c            0x04
     [DW_IC_STATUS]                  0x70            0xd8
     [DW_IC_TXFLR]                   0x74            0xdc
     [DW_IC_RXFLR]                   0x78            0xe0
     [DW_IC_SDA_HOLD]                0x7c            0x5c
     [DW_IC_TX_ABRT_SOURCE]          0x80            0xd4
     [DW_IC_ENABLE_STATUS]           0x9c            0xd0
     [DW_IC_CLR_RESTART_DET]         0xa8            NA
     [DW_IC_SMBUS_INTR_STAT]         0xc8            NA
     [DW_IC_SMBUS_INTR_MASK]         0xcc            NA
     [DW_IC_CLR_SMBUS_INTR]          0xd4            NA
     [DW_IC_COMP_PARAM_1]            0xf4            NA
     [DW_IC_COMP_VERSION]            0xf8            0x100
     [DW_IC_COMP_TYPE]               0xfc            0x104

2. DW_IC_CON bitfields:

     DW_IC_CON_MASTER BIT(0)                  BIT(0)
     DW_IC_CON_SPEED_STD                 (1 << 1)       (1 << 4)
     DW_IC_CON_SPEED_FAST                (2 << 1)       (2 << 4)
     DW_IC_CON_SPEED_HIGH                (3 << 1)       (3 << 4)
     DW_IC_CON_SPEED_MASK                GENMASK(2, 1)  GENMASK(5, 4)
     DW_IC_CON_10BITADDR_SLAVE           BIT(3) BIT(8)
     DW_IC_CON_10BITADDR_MASTER          BIT(4) BIT(9)
     DW_IC_CON_RESTART_EN                BIT(5) NA
     DW_IC_CON_SLAVE_DISABLE             BIT(6) NA
     DW_IC_CON_STOP_DET_IFADDRESSED      BIT(7) BIT(10)
     DW_IC_CON_TX_EMPTY_CTRL             BIT(8) BIT(11)
     DW_IC_CON_RX_FIFO_FULL_HLD_CTRL     BIT(9) BIT(12)
     DW_IC_CON_BUS_CLEAR_CTRL            BIT(11)  BIT(14)

3. To clear INTR,  Read DW_IC_CLR_* reg    Write bit to DW_IC_CLR_INTR

As you can see, it's an entirely different mapping, which is why I had 2 
independent enum -> reg offset maps instead of offset -> offset 
translation.
Similarly, selecting a CON register bitfield layout too.

Let me know what you would prefer based on this...

Thanks,
Aniket