On Fri, Sep 25, 2026 at 04:46:39PM +0530, Aniket Limaye wrote:
> 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)
Thanks for providing this mapping!
> 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...
This clears a bit the whole picture and what I would like to say is that better
to have the separate driver for it. On top of completely reworked RTL I believe
you won't need tons of hacks and workarounds that are applied during all these
years against the old IP.
TL;DR: Make a new clean and simple driver, which is hack-less and done properly
(using the all modern APIs and frameworks in the kernel).
--
With Best Regards,
Andy Shevchenko
