Re: [PATCH] mtd: rawnand: marvell: ensure timing values are written

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Chris,

Chris.Packham@alliedtelesis.co.nz wrote on Tue, 23 May 2023 20:42:45
+0000:

> On 23/05/23 21:39, Miquel Raynal wrote:
> > Hi Chris,
> >
> > chris.packham@alliedtelesis.co.nz wrote on Tue, 23 May 2023 15:21:03
> > +1200:
> >  
> >> When new timing values are calculated in marvell_nfc_setup_interface()
> >> ensure that they will be applied in marvell_nfc_select_target() by
> >> clearing the selected_chip pointer.
> >>
> >> Suggested-by: Miquel Raynal <miquel.raynal@bootlin.com>
> >> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>  
> > I believe this patch deserves Fixes+Cc:stable?  
> 
> I agree. I just wasn't sure what to point at with the fixes tag since 
> you mentioned that it's probably a result in some of the core NAND 
> infrastructure changing.
> 
> Maybe b25251414f6e00 ("mtd: rawnand: marvell: Stop implementing 
> ->select_chip()") is the correct change to point at.  

Sounds reasonable.

> 
> >> ---
> >>
> >> Notes:
> >>      This at least gets me to a point where I can illustrated the problem
> >>      reported to me. It appears that despite the chip correctly reporting
> >>      support for SDR timing modes up to 4 the observed tWC is 20ns. I've not
> >>      seen any actual problem running in this state the only complaint is that
> >>      the datasheet says the minimum tWC is 25ns.
> >>      
> >>      If I make a change to my bootloader such that the NAND Clock Frequency
> >>      Select bit (0xF2440700:0) to 1 before booting the kernel _and_ I remove
> >>      the extra factor of 2 from the period_ns calculation I observe tWC of
> >>      about 60ns. If I don't remove the factor of 2 the NAND interface doesn't
> >>      work (can't write BBT).
> >>
> >>   drivers/mtd/nand/raw/marvell_nand.c | 6 ++++++
> >>   1 file changed, 6 insertions(+)
> >>
> >> diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
> >> index afb424579f0b..3b5e4d5d220f 100644
> >> --- a/drivers/mtd/nand/raw/marvell_nand.c
> >> +++ b/drivers/mtd/nand/raw/marvell_nand.c
> >> @@ -2457,6 +2457,12 @@ static int marvell_nfc_setup_interface(struct nand_chip *chip, int chipnr,
> >>   			NDTR1_WAIT_MODE;
> >>   	}
> >>   
> >> +	/*
> >> +	 * Reset nfc->selected_chip so the next command will cause the timing
> >> +	 * registers to be restored in marvell_nfc_select_target().
> >> +	 */  
> > s/restored/updated/ ?
> >
> > Restored feels like the content vanished.  
> OK. Will send a v2 later today.
> >> +	nfc->selected_chip = NULL;
> >> +
> >>   	return 0;
> >>   }
> >>     
> >
> > Thanks,
> > Miquè  


Thanks,
Miquèl