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

[Chris Packham]

On 23/05/23 21:39, Miquel Raynal wrote:
> Hi Chris,
>
> chris.packham@xxxxxxxxxxxxxxxxxxx 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@xxxxxxxxxxx>
>> Signed-off-by: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx>
> 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.

>> ---
>>
>> 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èl