All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Aniket Limaye <a-limaye@ti.com>,
	Andi Shyti <andi.shyti@kernel.org>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Nirujogi Pratap <pratap.nirujogi@amd.com>,
	Bin Du <bin.du@amd.com>,
	linux-i2c@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, vigneshr@ti.com, nm@ti.com,
	u-kumar1@ti.com, lianfeng.ouyang@starfivetech.com,
	Ritwick.Sharma@arm.com
Subject: Re: [PATCH 2/3] i2c: designware: Introduce per-variant register offset and bit-layout tables
Date: Fri, 25 Sep 2026 11:11:54 +0300	[thread overview]
Message-ID: <arYsyiu4KG-m23hh@ashevche-desk.local> (raw)
In-Reply-To: <20260921111148.GT106095@black.igk.intel.com>

On Mon, Sep 21, 2026 at 01:11:48PM +0200, Mika Westerberg wrote:
> On Sat, Sep 19, 2026 at 02:36:07PM +0530, Aniket Limaye wrote:
> > Every DW_IC_* register offset and CON-register bit position is currently
> > baked in as a compile-time constant, which only works while there is a
> > single register layout. Introduce a logical register-ID enum (enum
> > dw_i2c_reg_idx) plus a per-variant offset table (dev->regs[]) and a
> > per-variant CON-register bit-layout descriptor (dev->con_bits), selected
> > at probe time via the new i2c_dw_select_variant().
> > 
> > Replace every direct DW_IC_* offset/bit-position reference with a lookup
> > through dev->regs[]/dev->con_bits. Also fold the read-to-clear
> > interrupt-acknowledgment pattern into a new i2c_dw_ack_intr() helper,
> > driven by a per-variant dev->intr_clr[] table.
> > 
> > Only one variant exists at this point (DW_apb_i2c), so this is a
> > mechanical, behavior-preserving change: the values in
> > dw_i2c_reg_offsets[] and dw_i2c_con_bits match the DW_IC_* macros
> > exactly. It lays the groundwork for adding a second register layout
> > (DWC_i2c) without duplicating the whole driver.

...

> > -		regmap_write(dev->map, DW_IC_TX_TL, dev->tx_fifo_depth / 2);
> > -		regmap_write(dev->map, DW_IC_RX_TL, 0);
> > -		regmap_write(dev->map, DW_IC_CON, dev->master_cfg);
> > +		regmap_write(dev->map, dev->regs[DW_REG_IDX_TX_TL], dev->tx_fifo_depth / 2);
> > +		regmap_write(dev->map, dev->regs[DW_REG_IDX_RX_TL], 0);
> > +		regmap_write(dev->map, dev->regs[DW_REG_IDX_CON], dev->master_cfg);
> 
> Instead of all this. Can't you do this inside the regmap so that here and
> elsewhere in the driver we continue to do:
> 
> 	regmap_write(dev->map, DW_IC_RX_TL, 0);
> 
> but internally, depending on the hardware it then maps this into the
> corresponding register offset.

Exactly what I was going to say when I hit "reply".
These series is definitely NAKed (in terms of the approach taken).

-- 
With Best Regards,
Andy Shevchenko



  parent reply	other threads:[~2026-09-25  8:12 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-19  9:06 [PATCH 0/3] i2c: designware: Add DWC_i2c support Aniket Limaye
2026-09-19  9:06 ` [PATCH 1/3] dt-bindings: i2c: dw: Add DWC_i2c compatible Aniket Limaye
2026-09-20 18:24   ` Krzysztof Kozlowski
2026-09-21  8:25     ` Aniket Limaye
2026-09-21 11:14       ` Mika Westerberg
2026-09-21 14:00         ` Krzysztof Kozlowski
2026-09-21 14:10           ` Mika Westerberg
2026-09-21 17:25             ` Aniket Limaye
2026-09-22  8:50               ` Krzysztof Kozlowski
2026-09-19  9:06 ` [PATCH 2/3] i2c: designware: Introduce per-variant register offset and bit-layout tables Aniket Limaye
2026-09-21 11:11   ` Mika Westerberg
2026-09-21 17:10     ` Aniket Limaye
2026-09-25  8:11     ` Andy Shevchenko [this message]
2026-09-25  9:26       ` Aniket Limaye
2026-09-19  9:06 ` [PATCH 3/3] i2c: designware: Add snps,dwc-i2c support and new compatible Aniket Limaye
2026-09-19  9:15   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=arYsyiu4KG-m23hh@ashevche-desk.local \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=Ritwick.Sharma@arm.com \
    --cc=a-limaye@ti.com \
    --cc=andi.shyti@kernel.org \
    --cc=bin.du@amd.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=lianfeng.ouyang@starfivetech.com \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=nm@ti.com \
    --cc=pratap.nirujogi@amd.com \
    --cc=robh@kernel.org \
    --cc=u-kumar1@ti.com \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.