Received-SPF: pass (google.com: domain of linux-kernel+bounces-39148-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240124030458.98408-1-dregan@broadcom.com> <20240124030458.98408-10-dregan@broadcom.com>
 <20240124183706.0af8fb90@xps-13>
In-Reply-To: <20240124183706.0af8fb90@xps-13>
From: David Regan <dregan@broadcom.com>
Date: Thu, 25 Jan 2024 10:47:18 -0800
Message-ID: <CAA_RMS6VeAmJUYxVYknYrcf5vVWGzEx3npSpSujFhKKL15zjVA@mail.gmail.com>
Subject: Re: [PATCH v3 09/10] mtd: rawnand: brcmnand: update log level messages
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: David Regan <dregan@broadcom.com>, dregan@mail.com, 
	Richard Weinberger <richard@nod.at>, Vignesh Raghavendra <vigneshr@ti.com>, robh+dt@kernel.org, 
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, 
	computersforpeace@gmail.com, kdasu.kdev@gmail.com, 
	linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, 
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Joel Peshkin <joel.peshkin@broadcom.com>, 
	Tomer Yacoby <tomer.yacoby@broadcom.com>, Dan Beygelman <dan.beygelman@broadcom.com>, 
	William Zhang <william.zhang@broadcom.com>, Anand Gore <anand.gore@broadcom.com>, 
	Kursad Oney <kursad.oney@broadcom.com>, Florian Fainelli <florian.fainelli@broadcom.com>, 
	rafal@milecki.pl, bcm-kernel-feedback-list@broadcom.com, 
	andre.przywara@arm.com, baruch@tkos.co.il, 
	linux-arm-kernel@lists.infradead.org, 
	Dan Carpenter <dan.carpenter@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Jan 24, 2024 at 9:37=E2=80=AFAM Miquel Raynal <miquel.raynal@bootli=
n.com> wrote:
>
> Hi David,
>
> dregan@broadcom.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@broadcom.com>
> > Reviewed-by: William Zhang <william.zhang@broadcom.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/nan=
d/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 brcmna=
nd_host *host,
> >       if ((val & mask) =3D=3D 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, st=
ruct 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, st=
ruct nand_chip *chip,
> >                       err =3D brcmnand_read_by_pio(mtd, chip, addr, tra=
ns, 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 +=3D corrected;
> >               /* Always exceed the software-imposed threshold */
>
>
> Thanks,
> Miqu=C3=A8l

Thanks!

-Dave