Hi Geert, Thanks for the feedback. > -----Original Message----- > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> > Subject: Re: [PATCH v3 7/9] memory: renesas-rpc-if: Add wrapper functions > > Hi Biju, > > Thanks for your patch! > > On Tue, 11 Mar 2025 at 12:36, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote: > > Even though XSPI and RPCIF has different register layout, reuse the > > code by adding wrapper functions to support both XSPI and RPC-IF. > > > > While at it, replace error check for pm_runtime_resume_and_get() as it > > can return positive value as well. > > While the change is fine for me, the function cannot return strict positive values: > https://elixir.bootlin.com/linux/v6.13.7/source/include/linux/pm_runtime.h#L418 OK. > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > > > --- a/drivers/memory/renesas-rpc-if.c > > +++ b/drivers/memory/renesas-rpc-if.c > > > @@ -519,20 +543,15 @@ static void memcpy_fromio_readw(void *to, > > } > > } > > > > -ssize_t rpcif_dirmap_read(struct device *dev, u64 offs, size_t len, > > void *buf) > > +static ssize_t rpcif_dirmap_read_helper(struct rpcif_priv *rpc, u64 offs, > > + size_t len, void *buf) > > Now the function can no longer fail, it can return size_t. OK. > > > { > > - struct rpcif_priv *rpc = dev_get_drvdata(dev); > > loff_t from = offs & (rpc->size - 1); > > size_t size = rpc->size - from; > > - int ret; > > > > if (len > size) > > len = size; > > > > - ret = pm_runtime_resume_and_get(dev); > > - if (ret < 0) > > - return ret; > > - > > regmap_update_bits(rpc->regmap, RPCIF_CMNCR, RPCIF_CMNCR_MD, 0); > > regmap_write(rpc->regmap, RPCIF_DRCR, 0); > > regmap_write(rpc->regmap, RPCIF_DRCMR, rpc->command); @@ > > -549,9 +568,24 @@ ssize_t rpcif_dirmap_read(struct device *dev, u64 offs, size_t len, void *buf) > > else > > memcpy_fromio(buf, rpc->dirmap + from, len); > > > > + return len; > > +} > > + > > +ssize_t rpcif_dirmap_read(struct device *dev, u64 offs, size_t len, > > +void *buf) { > > + struct rpcif_priv *rpc = dev_get_drvdata(dev); > > + ssize_t length; > > size_t read? OK. Will fix this in next version. Cheers, Biju > > > + int ret; > > + > > + ret = pm_runtime_resume_and_get(dev); > > + if (ret) > > + return ret; > > + > > + length = rpcif_dirmap_read_helper(rpc, offs, len, buf); > > + > > pm_runtime_put(dev); > > > > - return len; > > + return length; > > } > > EXPORT_SYMBOL(rpcif_dirmap_read); > > Nothing critical, so > Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx > > In personal conversations with technical people, I call myself a hacker. But when I'm talking to > journalists I just say "programmer" or something like that. > -- Linus Torvalds
