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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAA_RMS6VeAmJUYxVYknYrcf5vVWGzEx3npSpSujFhKKL15zjVA@mail.gmail.com>
Date: Thu, 25 Jan 2024 10:47:18 -0800
From: David Regan <dregan@...adcom.com>
To: Miquel Raynal <miquel.raynal@...tlin.com>
Cc: David Regan <dregan@...adcom.com>, dregan@...l.com, 
	Richard Weinberger <richard@....at>, Vignesh Raghavendra <vigneshr@...com>, robh+dt@...nel.org, 
	krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org, 
	computersforpeace@...il.com, kdasu.kdev@...il.com, 
	linux-mtd@...ts.infradead.org, devicetree@...r.kernel.org, 
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, Joel Peshkin <joel.peshkin@...adcom.com>, 
	Tomer Yacoby <tomer.yacoby@...adcom.com>, Dan Beygelman <dan.beygelman@...adcom.com>, 
	William Zhang <william.zhang@...adcom.com>, Anand Gore <anand.gore@...adcom.com>, 
	Kursad Oney <kursad.oney@...adcom.com>, Florian Fainelli <florian.fainelli@...adcom.com>, 
	rafal@...ecki.pl, bcm-kernel-feedback-list@...adcom.com, 
	andre.przywara@....com, baruch@...s.co.il, 
	linux-arm-kernel@...ts.infradead.org, 
	Dan Carpenter <dan.carpenter@...aro.org>
Subject: Re: [PATCH v3 09/10] mtd: rawnand: brcmnand: update log level messages

On Wed, Jan 24, 2024 at 9:37 AM Miquel Raynal <miquel.raynal@...tlin.com> wrote:
>
> Hi David,
>
> dregan@...adcom.com 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@...adcom.com>
> > Reviewed-by: William Zhang <william.zhang@...adcom.com>
> > ---
> > 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