| Message ID | 20230523032103.208213-1-chris.packham@alliedtelesis.co.nz |
|---|---|
| State | Changes Requested |
| Delegated to: | Miquel Raynal |
| Headers | show |
| Series | mtd: rawnand: marvell: ensure timing values are written | expand |
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? > --- > > 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. > + nfc->selected_chip = NULL; > + > return 0; > } > Thanks, Miquèl
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. >> --- >> >> 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
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
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(). + */ + nfc->selected_chip = NULL; + return 0; }
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> --- 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(+)