Re: [PATCH] memory: renesas-rpc-if: Fix IO state based on flash type

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	Biju Das <biju.das.jz@bp.renesas.com>,
	Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	"linux-renesas-soc@vger.kernel.org" 
	<linux-renesas-soc@vger.kernel.org>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Tudor Ambarus <tudor.ambarus@linaro.org>,
	Mark Brown <broonie@kernel.org>,
	MTD Maling List <linux-mtd@lists.infradead.org>,
	linux-spi <linux-spi@vger.kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Michael Walle <michael@walle.cc>
Subject: Re: [PATCH] memory: renesas-rpc-if: Fix IO state based on flash type
Date: Thu, 14 Sep 2023 11:12:00 +0200	[thread overview]
Message-ID: <20230914111200.6e6832ca@xps-13> (raw)
In-Reply-To: <CAMuHMdX8Vg2M2K3U_sbAtVk+6VDvL+GNYtbs5FTTRZzRvVGPCQ@mail.gmail.com>

Hi Geert,

geert@linux-m68k.org wrote on Thu, 14 Sep 2023 11:04:01 +0200:

> Hi Miquel,
> 
> On Thu, Sep 14, 2023 at 10:59 AM Miquel Raynal
> <miquel.raynal@bootlin.com> wrote:
> > geert@linux-m68k.org wrote on Thu, 14 Sep 2023 10:34:50 +0200:  
> > > On Thu, Sep 14, 2023 at 10:08 AM Krzysztof Kozlowski
> > > <krzysztof.kozlowski@linaro.org> wrote:  
> > > > On 30/08/2023 17:18, Biju Das wrote:  
> > > > >>>                 regmap_update_bits(rpc->regmap, RPCIF_CMNCR, @@ -774,6
> > > > >>> +776,12 @@ static int rpcif_probe(struct platform_device *pdev)
> > > > >>>                 return ret;
> > > > >>>         }
> > > > >>>
> > > > >>> +       if (rpc->info->type == RPCIF_RZ_G2L &&  
> > > > >>
> > > > >> Wouldn't this apply to non-RZ/G2L systems, too?  
> > > > >
> > > > > It applies, if the device uses the flash[1] or [2] and it needs
> > > > > 4-bit tx support.
> > > > >
> > > > > [1] Figure 20: QUAD INPUT/OUTPUT FAST READ – EBh/ECh
> > > > > https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-a/mt25q_qlks_u_512_aba_0.pdf?rev=3e5b2a574f7b4790b6e58dacf4c889b2
> > > > >
> > > > > [2] section 8.14
> > > > >
> > > > > https://www.renesas.com/eu/en/document/dst/at25ql128a-datasheet?r=1608586  
> > > >
> > > > Geert,
> > > >
> > > > Does it answer your comment or do you expect here some changes?  
> > >
> > > Well, now it has been confirmed this applies to non-RZ/G2L systems, too,
> > > the check for RPCIF_RZ_G2L should probably be removed.  In upstream,
> > > only arch/arm64/boot/dts/renesas/rzg2l{,c}-smarc-som.dtsi have devices
> > > that are compatible with "micron,mt25qu512a", but obviously they can
> > > appear elsewhere, too.
> > >
> > > Now, the presence of that compatible value in rzg2l{,c}-smarc-som.dtsi
> > > currently causes a dtbs_check warning, as it is not documented.
> > > However, there has been some pushback against adding more compatible
> > > values, cfr. my patch to add mt25qu512a[1], and Miquel's commit [2].  
> >
> > Just FYI, I sent [2] after an unsuccessful attempt to update that list
> > too, see [3]. The idea is: if you don't have anything useful to add,  
> 
> Oh, I didn't know that.
> 
> > just use the generic compatible. If you need specific changes, you can
> > add an entry.  
> 
> The problem is that usually these things are discovered too late,
> so the only prudent way is to be proactive, and always add them.
> Initially I thought that the different handling on RZ/G2L was due
> to a difference in the RPC-IF block.  But now we know it's due to the
> type of FLASH attached.

Actually what I say is wrong, we are not supposed to touch that list
anymore and prefer to handle the issues in the drivers by
auto-discovery. Can't we do that in your case?

> > [3] https://lore.kernel.org/linux-mtd/d816499e-baab-6200-0780-17a8205b252e@linaro.org/
> >  
> > > But the issue Biju is seeing proves there is a need to add these.
> > >
> > > In addition, I had hoped to gather some feedback or guidance from the
> > > hyperbus and/or spi people, as issues w.r.t. pin states will eventually
> > > pop up on other systems, too, and thus may need handling in the core,
> > > instead of in each individual device driver.  But of course that can
> > > be done later, when the need arises.
> > >
> > > Thanks!
> > >
> > > [1] "[PATCH] dt-bindings: mtd: jedec,spi-nor: Document support for
> > > more MT25QU parts"
> > >     https://lore.kernel.org/all/363186079b4269891073f620e3e2353cf7d2559a.1669988238.git.geert+renesas@glider.be
> > > [2] 4b0cb4e7ab2f777c ("dt-bindings: mtd: spi-nor: clarify the need for
> > > spi-nor compatibles").  
> 

Thanks,
Miquèl