Re: [PATCH 6/6] memory: renesas-rpc-if: Add support for RZ/G2L

From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
To: Wolfram Sang <wsa+renesas@sang-engineering.com>,
	"Lad, Prabhakar" <prabhakar.csengg@gmail.com>,
	Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
	Rob Herring <robh+dt@kernel.org>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	Mark Brown <broonie@kernel.org>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Sergei Shtylyov <sergei.shtylyov@gmail.com>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>,
	linux-mtd@lists.infradead.org,
	linux-spi <linux-spi@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	Biju Das <biju.das.jz@bp.renesas.com>
Subject: Re: [PATCH 6/6] memory: renesas-rpc-if: Add support for RZ/G2L
Date: Fri, 1 Oct 2021 10:06:06 +0100	[thread overview]
Message-ID: <CA+V-a8uyQmW3+4hAt4534spKeQHDoeZzuJJE4RY70KLZfYOXoA@mail.gmail.com> (raw)
In-Reply-To: <YVbM3z7x+D0MCkTF@ninjato>

Hi Wolfram,

On Fri, Oct 1, 2021 at 9:55 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> Hi Prabhakar,
>
> > > Is RPCIF_CMNCR_IO3FV and RPCIF_CMNCR_IO2FV actually documented in your
> > > datasheets? I am asking because I have a patch pending to remove writing
> > > to undocumented locations. So, I was aboout to remove the IO3FV and
> > > IO2FV macros.
> > >
> > Yes they are documented, you should be able to download the HW manual from [1]
>
> Great, then I will keep them!
>
> > > > +     regmap_write(rpc->regmap, RPCIF_PHYADJ2, 0x00008080);
> > > > +     regmap_write(rpc->regmap, RPCIF_PHYADJ1, 0x80000024);
> > >
> > > Can't we have defines for these magic values? At least in my latest Gen3
> > > documentation, these values are explained.
> > >
> > RZ/G2L manual doesn't explain these bits. Let me refer to R-Car Gen3
> > and define them as macros.
>
> Seems like we have the best of both worlds then with the documentation
> ;)
>
Indeed :)

> > > > +     if (rpc->type == RPCIF_RCAR_GEN3) {
> > > > +             regmap_write(rpc->regmap, RPCIF_PHYCNT, RPCIF_PHYCNT_STRTIM(7) |
> > > > +                          RPCIF_PHYCNT_PHYMEM(hyperflash ? 3 : 0) | 0x260);
> > > > +     } else {
> > > > +             regmap_read(rpc->regmap, RPCIF_PHYCNT, &dummy);
> > > > +             dummy &= ~RPCIF_PHYCNT_PHYMEM_MASK;
> > > > +             dummy |= RPCIF_PHYCNT_PHYMEM(hyperflash ? 3 : 0) | 0x260;
> > > > +             regmap_write(rpc->regmap, RPCIF_PHYCNT, dummy);
> > >
> > > regmap_update_bits?
> > >
> > Im a bit hesitant to use regmap_update_bits() here as some of the bits
> > are not documented.
>
> Hmm, maybe I should then update the patch avoiding undocumented register
> access beforehand? I will download your docs and see what remains and
> send that out for you to check. Somewhen later today.
>
> Sounds good?
>
Fine with me.

Cheers,
Prabhakar

> Happy hacking,
>
>    Wolfram
>