Re: [PATCH v6 6/7] mfd: nct6694: Introduce regmap-based transport abstraction

[Lee Jones <lee@kernel.org>]

On Mon, 10 Aug 2026, Ming Yu wrote:

> Hi Lee,
> 
> Thank you for your reviewing.
> 
> Lee Jones <lee@kernel.org> 於 2026年7月9日週四 下午5:28寫道：
> >
> ...
> > > +/*
> > > + * A few firmware commands (e.g. the I2C "deliver") transmit a request and
> > > + * read the reply back in a single transaction. Model this as a regmap read of
> > > + * a SET register: @buf carries the request on entry and holds the reply on
> > > + * return, so the firmware exchange stays a single message as before.
> > > + */
> > > +static inline int nct6694_write_read_msg(struct nct6694 *nct6694,
> > > +                                      const struct nct6694_cmd_header *cmd_hd,
> > > +                                      void *buf)
> > > +{
> > > +     return regmap_bulk_read(nct6694->regmap,
> > > +                             nct6694_cmd_to_reg(cmd_hd, NCT6694_HCTRL_SET),
> > > +                             buf, le16_to_cpu(cmd_hd->len));
> > > +}
> >
> > [Severity: Medium]
> > Are we misusing the regmap API by passing an input payload through
> > regmap_bulk_read() in nct6694_write_read_msg()?
> >
> > The driver handles command exchanges by modeling them as a regmap_bulk_read()
> > of a SET register, passing the request payload inside the buf parameter.
> > However, the regmap API strictly defines this as an output buffer.
> >
> > This currently works because regmap bypasses bounce buffers and cache for this
> > specific configuration. Any future changes in the regmap subsystem, such as
> > initializing output buffers, enabling a regmap cache, or adding bounce buffers
> > for DMA alignment, will silently discard the outgoing request payload.
> >
> 
> On the write_read finding: it's safe as-is (REGCACHE_NONE, val_bits 8,
> so val_buf passes straight to bus->read), but I agree the read
> semantics are bent for this one request/response command.
> 
> Would you prefer I keep nct6694_write_read_msg() and document the
> in/out contract, plus a note that this regmap must never enable
> caching, so everything stays behind regmap as you asked in v5?
> 
> Or would you rather I add a thin transport helper just for the
> "deliver" command? That reintroduces the ops-style you pushed back on
> in v5, so I didn't want to go that way without checking first.

Please don't re-introduce the driver-level "transport layer".

Yes, if you fully understand it, document it.  If you know of a better
way to handle it that is within the bounds of Regmap, then please do
that.

-- 
Lee Jones