From: Tony O'Brien <[email protected]>
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 <[email protected]>
Signed-off-by: Chris Packham <[email protected]>
---
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.
drivers/mtd/nand/raw/marvell_nand.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
index 2455a581fd70..b248c5f657d5 100644
--- a/drivers/mtd/nand/raw/marvell_nand.c
+++ b/drivers/mtd/nand/raw/marvell_nand.c
@@ -2672,7 +2672,7 @@ static int marvell_nand_chip_init(struct device *dev, struct marvell_nfc *nfc,
chip->controller = &nfc->controller;
nand_set_flash_node(chip, np);
- if (!of_property_read_bool(np, "marvell,nand-keep-config"))
+ if (of_property_read_bool(np, "marvell,nand-keep-config"))
chip->options |= NAND_KEEP_TIMINGS;
mtd = nand_to_mtd(chip);
--
2.37.3
Hi All,
On 27/09/22 15:47, Chris Packham wrote:
> From: Tony O'Brien <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Chris Packham <[email protected]>
Just following up on this. I know things have probably been busy with
the 6.0 release but it's been a week so I figured I'd give this a bump.
> ---
>
> 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.
>
> drivers/mtd/nand/raw/marvell_nand.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
> index 2455a581fd70..b248c5f657d5 100644
> --- a/drivers/mtd/nand/raw/marvell_nand.c
> +++ b/drivers/mtd/nand/raw/marvell_nand.c
> @@ -2672,7 +2672,7 @@ static int marvell_nand_chip_init(struct device *dev, struct marvell_nfc *nfc,
> chip->controller = &nfc->controller;
> nand_set_flash_node(chip, np);
>
> - if (!of_property_read_bool(np, "marvell,nand-keep-config"))
> + if (of_property_read_bool(np, "marvell,nand-keep-config"))
> chip->options |= NAND_KEEP_TIMINGS;
>
> mtd = nand_to_mtd(chip);
On Mon, 3 Oct 2022 21:46:31 +0000
Chris Packham <[email protected]> wrote:
> Hi All,
>
> On 27/09/22 15:47, Chris Packham wrote:
> > From: Tony O'Brien <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Chris Packham <[email protected]>
>
> Just following up on this. I know things have probably been busy with
> the 6.0 release but it's been a week so I figured I'd give this a bump.
Reviewed-by: Boris Brezillon <[email protected]>
>
> > ---
> >
> > 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.
> >
> > drivers/mtd/nand/raw/marvell_nand.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
> > index 2455a581fd70..b248c5f657d5 100644
> > --- a/drivers/mtd/nand/raw/marvell_nand.c
> > +++ b/drivers/mtd/nand/raw/marvell_nand.c
> > @@ -2672,7 +2672,7 @@ static int marvell_nand_chip_init(struct device *dev, struct marvell_nfc *nfc,
> > chip->controller = &nfc->controller;
> > nand_set_flash_node(chip, np);
> >
> > - if (!of_property_read_bool(np, "marvell,nand-keep-config"))
> > + if (of_property_read_bool(np, "marvell,nand-keep-config"))
> > chip->options |= NAND_KEEP_TIMINGS;
> >
> > mtd = nand_to_mtd(chip)
Hi Chris,
[email protected] wrote on Mon, 3 Oct 2022 21:46:31
+0000:
> Hi All,
>
> On 27/09/22 15:47, Chris Packham wrote:
> > From: Tony O'Brien <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Chris Packham <[email protected]>
>
> Just following up on this. I know things have probably been busy with
> the 6.0 release but it's been a week so I figured I'd give this a bump.
I was just off the past week :)
I will queue it soon.
>
> > ---
> >
> > 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.
I don't remember any... Indeed this must be fixed.
> > 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.
> >
> > drivers/mtd/nand/raw/marvell_nand.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
> > index 2455a581fd70..b248c5f657d5 100644
> > --- a/drivers/mtd/nand/raw/marvell_nand.c
> > +++ b/drivers/mtd/nand/raw/marvell_nand.c
> > @@ -2672,7 +2672,7 @@ static int marvell_nand_chip_init(struct device *dev, struct marvell_nfc *nfc,
> > chip->controller = &nfc->controller;
> > nand_set_flash_node(chip, np);
> >
> > - if (!of_property_read_bool(np, "marvell,nand-keep-config"))
> > + if (of_property_read_bool(np, "marvell,nand-keep-config"))
> > chip->options |= NAND_KEEP_TIMINGS;
> >
> > mtd = nand_to_mtd(chip)
Thanks,
Miquèl
On Tue, 2022-09-27 at 02:47:28 UTC, Chris Packham wrote:
> From: Tony O'Brien <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Chris Packham <[email protected]>
> Reviewed-by: Boris Brezillon <[email protected]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/fixes, thanks.
Miquel