public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Scott Wood <scottwood@freescale.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [U-Boot, 02/10] arm: s3c24xx: Fix incorrect CONFIG_SYS_S3C2410_NAND_HWECC name
Date: Fri, 1 May 2015 22:41:44 -0500	[thread overview]
Message-ID: <1430538104.16357.222.camel@freescale.com> (raw)
In-Reply-To: <201505020318.26791.marex@denx.de>

On Sat, 2015-05-02 at 03:18 +0200, Marek Vasut wrote:
> On Saturday, May 02, 2015 at 03:11:41 AM, Scott Wood wrote:
> > On Sat, 2015-05-02 at 02:46 +0200, Marek Vasut wrote:
> > > On Thursday, November 27, 2014 at 03:03:50 AM, Scott Wood wrote:
> > > > On Sat, Oct 11, 2014 at 06:42:50PM +0200, Marek Vasut wrote:
> > > > > The correct name of this symbol is CONFIG_S3C2410_NAND_HWECC , the
> > > > > _SYS is redundant.
> > > > 
> > > > What makes that the correct name?  The symbol is not documented
> > > > anywhere, and while nothing currently tests for the SYS version,
> > > > nothing currently sets the non-SYS version.
> > > > 
> > > > What is SYS redundant with?
> > > > 
> > > > Is this meant to be a user config knob or something that is fixed for a
> > > > given board?
> > > 
> > > u-boot$ git grep CONFIG_SYS_S3C2410_NAND_HWECC
> > > include/configs/VCMA9.h:#define CONFIG_SYS_S3C2410_NAND_HWECC
> > > include/configs/smdk2410.h:#define CONFIG_SYS_S3C2410_NAND_HWECC
> > > 
> > > u-boot$ git grep CONFIG_S3C2410_NAND_HWECC
> > > drivers/mtd/nand/s3c2410_nand.c:#ifdef CONFIG_S3C2410_NAND_HWECC
> > > drivers/mtd/nand/s3c2410_nand.c:#ifdef CONFIG_S3C2410_NAND_HWECC
> > > 
> > > The driver checks the version without _SYS. This is a clear bugfix,
> > > so please apply.
> > 
> > There's a clear bug.  I asked questions to determine whether this is the
> > proper fix (and encourage a better changelog), and you still haven't
> > answered.  It would also be nice if you'd document the symbol while
> > you're at it.
> 
> Adding new stuff (like documentation) is now mandatory part of bugfix ?

I didn't say mandatory.  I said "would be nice".

> I'd expect bugfixes to be taken in to actually fix bugs and not kept out
> of the tree because the submitter didn't also do another random chore.

In this case the lack of documentation feels related to the cause of the
bug -- no authoritative place to say what the proper name is.  I don't
see it as "another random chore".

> Also, I do not know what else should I say besides that the symbol name
> is incorrect and you can clearly see it. If that is not enough ...

The names don't match.  That's a bug.  I was wondering whether the right
thing was to remove SYS in one place or add it in the other (or have we
given up on maintaining the distinction?).  I didn't expect asking that
question to be a huge burden on the patch's progress.  It was also
bundled up in a patchset with a bunch of non-bugfixes, so it didn't seem
like you were asking for urgent handling of this one.

> > In any case, I don't know why you're asking me to apply a patch with an
> > "arm:" subject line, which only touches ARM board config files, instead
> > of asking an ARM custodian.
> 
> You were the only one who responded, 

And look at what I get for doing so. :-)

> but I'm fine if Tom or whoever picks this.

While I would have liked an answer to the questions I asked, it's not
worth arguing over, so:

Acked-by: Scott Wood <scottwood@freescale.com>

-Scott

  reply	other threads:[~2015-05-02  3:41 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-11 16:42 [U-Boot] [PATCH 01/10] video: Add S3C24xx framebuffer support Marek Vasut
2014-10-11 16:42 ` [U-Boot] [PATCH 02/10] arm: s3c24xx: Fix incorrect CONFIG_SYS_S3C2410_NAND_HWECC name Marek Vasut
2014-11-27  2:03   ` [U-Boot] [U-Boot, " Scott Wood
2015-05-02  0:46     ` Marek Vasut
2015-05-02  1:11       ` Scott Wood
2015-05-02  1:18         ` Marek Vasut
2015-05-02  3:41           ` Scott Wood [this message]
2014-10-11 16:42 ` [U-Boot] [PATCH 03/10] mtd: nand: s3c: Fix data type width in debug() Marek Vasut
2014-11-27  2:06   ` [U-Boot] [U-Boot, " Scott Wood
2014-10-11 16:42 ` [U-Boot] [PATCH 04/10] mtd: nand: s3c: Unify the register definition and naming Marek Vasut
2014-10-11 16:42 ` [U-Boot] [PATCH 05/10] mtd: nand: s3c: Add S3C2440 specifics Marek Vasut
2014-11-27  2:20   ` [U-Boot] [U-Boot,05/10] " Scott Wood
2015-05-02  0:48     ` Marek Vasut
2015-05-02  1:05       ` Scott Wood
2014-10-11 16:42 ` [U-Boot] [PATCH 06/10] mtd: nand: s3c: Add S3C2440 buffer reading Marek Vasut
2014-10-11 16:42 ` [U-Boot] [PATCH 07/10] mtd: nand: s3c: Add missing correction and select_chip functions Marek Vasut
2014-10-28 22:45   ` Scott Wood
2016-01-14  1:41     ` Marek Vasut
2016-02-12 23:18       ` Scott Wood
2016-02-19 17:53         ` Marek Vasut
2016-02-19 17:54           ` Scott Wood
2014-10-11 16:42 ` [U-Boot] [PATCH 08/10] i2c: s3c: Implant support for S3C2440 Marek Vasut
2015-05-02  1:12   ` Marek Vasut
2015-05-06  8:02     ` Minkyu Kang
2014-10-11 16:42 ` [U-Boot] [PATCH 09/10] gpio: s3c: Fix the GPIO driver Marek Vasut
2015-05-02  1:12   ` Marek Vasut
2015-05-06  8:02     ` Minkyu Kang
2014-10-11 16:42 ` [U-Boot] [PATCH 10/10] net: smc911x: Keep MAC programmed Marek Vasut
2014-11-10 21:29   ` [U-Boot] [U-Boot,10/10] " Tom Rini
2014-10-16  9:28 ` [U-Boot] [PATCH 01/10] video: Add S3C24xx framebuffer support Anatolij Gustschin
2015-05-02  1:12   ` Marek Vasut
2016-02-19 18:02     ` Anatolij Gustschin
2016-02-19 18:37       ` Marek Vasut
2016-02-22 17:10 ` Anatolij Gustschin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1430538104.16357.222.camel@freescale.com \
    --to=scottwood@freescale.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox