GPMC omap nand using CS1

Tue Dec 14 00:53:12 PST 2021

Hi Romain,

romain.naour at smile.fr wrote on Mon, 13 Dec 2021 19:53:59 +0100:

> Hello Miquel,
> 
> Le 13/12/2021 à 18:50, Miquel Raynal a écrit :
> > Hello Romain,
> > 
> > romain.naour at smile.fr wrote on Mon, 13 Dec 2021 11:18:03 +0100:
> >   
> >> Hello,
> >>
> >> I'm using an AM3505 cpu with two nand device, nand0 use CS0 and nand1 use CS1.
> >>
> >> The nand0 is ok but nand1 is not detected.
> >> I get "nand: No NAND device found"  
> > 
> > Easy answer (support 101, sorry about that :-) ), did you check that the
> > CS was toggling with a scope? Just to be sure that this is a NAND
> > core/driver issue and not something else?  
> 
> Well I trusted blindly the hardware designer here :)

:)

> 
> I checked from u-boot:
> 
> $ nand info
> 
> Device 0: NAND 256MiB 3,3V 16-bit, sector size 128 KiB
> Device 1: NAND 256MiB 3,3V 16-bit, sector size 128 KiB
> 
> So it seems ok regarding CS1.
> 
> But this is an very old uboot (2011.09) with a lot of vendor patch (TI
> psp04.06.00.08) and additional custom patches.

Can you just check that it is not the same CS that is asserted twice?
Typically by reading both first pages and see if there is something
different? It happened to me already that U-Boot would not complain and
just use the same CS...

But otherwise if U-Boot works correctly, then we're fine from a
hardware point of view.

> >> Regarding the devicetree for nand1, I did something similar to omap3430-sdp.dts
> >> where the nand is on CS1.
> >>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/arm/boot/dts/omap3430-sdp.dts?h=v5.15.6#n102  
> > 
> > And this is precisely the reason why we switched to a new
> > DT representation: as long as you have a single chip, not describing
> > the NAND device inside a bigger "nand" node was fine. But whenever we
> > need to support more than one it starts to be an issue. I saw that the
> > logic here was to add a second nand node which, on a pure DT
> > representation logic is wrong: the GPMC has subnodes which are the
> > different controller (there is only one NAND controller there) which
> > should contain two NAND device subnodes with their specific properties.
> > Anyway, that is not how this controller is currently being described,
> > so let's assume creating two NAND nodes is fine at least to make it
> > work.  
> 
> Sorry, I didn't closely followed the mtd upstream news about the "new DT
> representation" :-/

Oh yeah no problem, I was just pointing out that this very old type of
representation has its limits, and here you typically fall into one.

> So the current GPMC nand binding is not correct (from DT point of view) with
> several nand devices and needs to be changed.

I would say that if you manage, with the current bindings, to get the
second CS working, then I'll shut my eyes. However if you struggle for
too long with them, I would advise to:
- try converting the bindings to the new form
- update the controller driver to support:
    - the legacy way with 1 CS
    - the modern way with >= 1 CS

> > I assume you tried:
> > * booting with only CS0 described -> works
> > * booting with only CS1 described -> No NAND device found error
> > Am I right?  
> 
> Yes I did that.
> 
> >   
> >> I'm not sure this dts actually works since in nand_base.c nand_detect() call
> >> nand_reset(chip, 0) with cs = 0.
> >>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/mtd/nand/raw/nand_base.c?h=v5.15.6#n4945  
> > 
> > This is indeed the first step, we always start with the first CS and
> > then if the first device was discovered, we check if the other CS are
> > wired. This only works of course if nand_scan() requests scanning more
> > than one CS. This is currently not the case and was already like that
> > for quite some time. Maybe that is why the NAND controller was
> > described twice in the link below, to get the probe of the controller
> > called twice?  
> 
> Indeed, the probe of the controller is called twice. I need to dig into the GPMC
> manual to understand if it really support two (or more up to 8) nand devices.

I suppose calling the probe twice is not valid, unless everything is
remapped/requested in the gpmc driver? As this controller is a little
bit different than the others, we might keep the old way of describing
chips if calling this probe twice is considered fine (I didn't dive too
deep into the manual yet, but looking at the code, this driver was also
written a decade ago, its design is really far from ideal and massively
reworking it could be a 15-20 days effort).

Maybe you can also look at my recent changes into the pl353 driver, it
also uses the same kind of building blocks. I fear that the gpmc driver
tries to do too many things that should be delegated to the NAND
controller but let's first try to make it work before cleaning all that
mess.

> Other GPMC use cas I found used one nand, one fram and one nor.
> 
> The board was designed almost a decade ago with two nand devices wired to the
> GPMC controller...
> 
> > 
> > Anyway if the question is 'how do I get CS1 being checked first', you
> > should look at the probe function which instantiate your controller.
> > That is where the driver is supposed to parse the DT, find the right
> > CS to handle and then register the NAND chip. From the core
> > perspective, it can use "target" 0, 1, etc, even though from the
> > controller perspective, target 0 for a given chip might be CS1. You can
> > look at "recent" drivers, like Marvell or Arasan which support several
> > CS.  
> 
> AFAIK, it's fine if CS0 is checked first.

Yes but maybe the driver is not properly written so that starting with
CS1 will not work at all (hence the "No NAND device found" error).

> 
> >   
> >> It seems it was possible to use two nand on gpmc (kernel 3.16):
> >>
> >> https://e2e.ti.com/support/processors-group/processors/f/processors-forum/343017/am335x-gpmc-with-two-nand-flash-in-device-tree  
> > 
> > I honestly do not know if this ever worked or if it was supported
> > upstream, it is very likely, looking at the current code base, that
> > if you want two CS on this board you are going to need to write a
> > couple of patches :)  
> 
> Meh :)
> 
> > 
> > Good luck, do not hesitate if you have other specific questions.  
> 
> Thank you for your feed back.
> 
> Best regards,
> Romain
> 
> > 
> > Thanks,
> > Miquèl
> >   
> 

Thanks,
Miquèl