Re: [PATCH 05/13] nvmem: add mt6323 PMIC EFUSE driver

Andy Shevchenko
2026-05-06T07:30:26Z

On Tue, May 05, 2026 at 04:24:41PM +0000, Roman Vivchar wrote:
> On Tuesday, May 5th, 2026 at 11:00 AM, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > On Mon, May 04, 2026 at 09:24:57PM +0300, Roman Vivchar via B4 Relay wrote:

First of all, please drop the pieces you are agree with or not going to comment
on (for whatever reason). It makes easier not miss anything important and saves
reviewer's time on doing that for you.

...

> > for (size_t i = 0; i < bytes; i += sizeof(*buf)) {
> >
> > > + ret = regmap_read(efuse->regmap,
> > > + MT6323_EFUSE_DOUT_BASE + offset + i, &tmp);
> > > + if (ret)
> > > + return ret;
> > > + buf[i / 2] = tmp;
> > > + }
> >
> > + blank line.
> >
> > Isn't this reimplementation of bulk read? Why the latter may not be used?
>
> Yes, but the PWRAP (the regmap which the driver uses) doesn't support read
> callback, only reg_read is implemented in the driver.
> It hits the map->cache_type == REGCACHE_NONE in the regmap_read_raw, and falls
> for the !map->read check, so regmap_bulk_read neither the regmap_read_raw
> can be used.
>
> Technically the PMIC is not continuous address space, but rather non-MMIO FSM,
> so it makes sense to not implement the read callback. I guess the custom
> implementation is fine then?

No objections from me, but please make sure the summary of this is mentioned
in the comment in the code and/or in the commit message.

--
With Best Regards,
Andy Shevchenko