Re: [PATCH v6 6/7] mfd: nct6694: Introduce regmap-based transport abstraction
Ming Yu <[email protected]> Mon, 10 Aug 2026 13:23:11 +0800
| Newsgroups | org.kernel.vger.linux-i2c,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAOoeyxXgqfts9ytFEYLe=jCpBkPaPwrzwTBQX3oTsTME90CEBw@mail.gmail.com> |
Hi Lee, Thank you for your reviewing. Lee Jones <[email protected]> 於 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