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

[Ming Yu]

Hi Lee,

Thank you for your reviewing.

Lee Jones <lee@xxxxxxxxxx> 於 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.

Thanks,
Ming