Re: [PATCH v3 09/10] mtd: rawnand: brcmnand: update log level messages

[David Regan]

On Wed, Jan 24, 2024 at 9:37 AM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:
>
> Hi David,
>
> dregan@xxxxxxxxxxxx wrote on Tue, 23 Jan 2024 19:04:57 -0800:
>
> > Update log level messages so that more critical messages
> > can be seen.
> >
> > Signed-off-by: David Regan <dregan@xxxxxxxxxxxx>
> > Reviewed-by: William Zhang <william.zhang@xxxxxxxxxxxx>
> > ---
> > Changes in v3: None
> > ---
> > Changes in v2:
> > - Added to patch series
> > ---
> >  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > index 6b5d76eff0ec..a4e311b6798c 100644
> > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > @@ -1143,7 +1143,7 @@ static int bcmnand_ctrl_poll_status(struct brcmnand_host *host,
> >       if ((val & mask) == expected_val)
> >               return 0;
> >
> > -     dev_warn(ctrl->dev, "timeout on status poll (expected %x got %x)\n",
> > +     dev_err(ctrl->dev, "timeout on status poll (expected %x got %x)\n",
>
> I don't see the point but if you want.
>
> >                expected_val, val & mask);
> >
> >       return -ETIMEDOUT;
> > @@ -2196,7 +2196,7 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip,
> >                               return err;
> >               }
> >
> > -             dev_dbg(ctrl->dev, "uncorrectable error at 0x%llx\n",
> > +             dev_err(ctrl->dev, "uncorrectable error at 0x%llx\n",
>
> Upper layer will complain, you can keep this at the debug level.

I've discovered that the upper layer will complain, but typically
with information that is maybe not useful for debugging.

>
> >                       (unsigned long long)err_addr);
> >               mtd->ecc_stats.failed++;
> >               /* NAND layer expects zero on ECC errors */
> > @@ -2211,7 +2211,7 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip,
> >                       err = brcmnand_read_by_pio(mtd, chip, addr, trans, buf,
> >                                                  oob, &err_addr);
> >
> > -             dev_dbg(ctrl->dev, "corrected error at 0x%llx\n",
> > +             dev_info(ctrl->dev, "corrected error at 0x%llx\n",
>
> Definitely not! Way too verbose. Please keep this one as it is.

ok

>
> >                       (unsigned long long)err_addr);
> >               mtd->ecc_stats.corrected += corrected;
> >               /* Always exceed the software-imposed threshold */
>
>
> Thanks,
> Miquèl

Thanks!

-Dave