Hi Biju, On Thu, Sep 14, 2023 at 11:37 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote: > > Subject: Re: [PATCH] memory: renesas-rpc-if: Fix IO state based on flash > > type > > On Thu, Sep 14, 2023 at 11:12 AM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> > > wrote: > > > geert@xxxxxxxxxxxxxx wrote on Thu, 14 Sep 2023 11:04:01 +0200: > > > > On Thu, Sep 14, 2023 at 10:59 AM Miquel Raynal > > > > <miquel.raynal@xxxxxxxxxxx> wrote: > > > > > geert@xxxxxxxxxxxxxx wrote on Thu, 14 Sep 2023 10:34:50 +0200: > > > > > > On Thu, Sep 14, 2023 at 10:08 AM Krzysztof Kozlowski > > > > > > <krzysztof.kozlowski@xxxxxxxxxx> 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. > > > > > > > > > > > > > > > > > > > > > > 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? > > > > I'm not sure we can do that, as this code is part of the hardware > > initialization during probing. > > Biju: is this needed that early, or can it be done later, after the > > connected device has been identified? > > I need to check that. > > You mean patch drivers/spi/spi-rpc-if.c > to identify the flash type from sfdp info and pass as a parameter to rpcif_hw_init?? Something like that. That configuration should be saved somewhere, as rpcif_hw_init() is also called from rpcif_resume(), and when recovering from an error in rpcif_manual_xfer(). 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