Re: [PATCH] mtd: rawnand: marvell: Use correct logic for nand-keep-config
From: Miquel Raynal
Date: Wed Oct 05 2022 - 03:35:11 EST
Hi Chris,
Chris.Packham@xxxxxxxxxxxxxxxxxxx wrote on Tue, 4 Oct 2022 21:21:37
+0000:
> On 5/10/22 08:41, Chris Packham wrote:
> >
> > On 4/10/22 23:02, Miquel Raynal wrote:
> >> Hi Chris,
> >>
> >> Chris.Packham@xxxxxxxxxxxxxxxxxxx wrote on Tue, 27 Sep 2022 02:54:40
> >> +0000:
> >>
> >>> On 27/09/22 15:47, Chris Packham wrote:
> >>>> From: Tony O'Brien <tony.obrien@xxxxxxxxxxxxxxxxxxx>
> >>>>
> >>>> Originally the absence of the marvell,nand-keep-config property caused
> >>>> the setup_data_interface function to be provided. However when
> >>>> setup_data_interface was moved into nand_controller_ops the logic was
> >>>> unintentionally inverted. Update the logic so that only if the
> >>>> marvell,nand-keep-config property is present the bootloader NAND
> >>>> config
> >>>> kept.
> >>>>
> >>>> Fixes: 7a08dbaedd36 ("mtd: rawnand: Move ->setup_data_interface()
> >>>> to nand_controller_ops")
> >>>> Signed-off-by: Tony O'Brien <tony.obrien@xxxxxxxxxxxxxxxxxxx>
> >>>> Signed-off-by: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx>
> >>>> ---
> >>>>
> >>>> Notes:
> >>>> I think this is a bug that's been lurking for 4 years or so.
> >>>> I'm not
> >>>> sure that's particularly long in the life of an embedded
> >>>> device but it
> >>>> does make me wonder if there have been other bug reports
> >>>> about it.
> >>>> We noticed this because we had a bootloader that used
> >>>> maxed out NAND
> >>>> timings which made the time it took the kernel to do anything
> >>>> on the
> >>>> file system longer than we expected.
> >>> I think there might be a similar logic inversion bug in
> >>> drivers/mtd/nand/raw/denali.c but I lack the ability to test for that
> >>> platform.
> >> Agreed, the denali driver has the same issue. Could you please send a
> >> patch?
> > Sure although it'll be compile tested only.
> Actually looks like it was already fixed in commit d311e0c27b8f ("mtd:
> rawnand: denali: get ->setup_data_interface() working again").
Ok, perfect.
Thanks,
Miquèl