[U-Boot] [PATCH v3 6/7] tegra: nand: Add Tegra NAND driver

[Scott Wood <scottwood@freescale.com>]

On 05/22/2012 03:04 PM, Simon Glass wrote:
> Hi Scott,
> 
> On Mon, May 21, 2012 at 8:47 AM, Scott Wood <scottwood@freescale.com
> <mailto:scottwood@freescale.com>> wrote:
> 
>     On 05/21/2012 05:07 AM, Jim Lin wrote:
>     >> diff --git a/drivers/mtd/nand/tegra2_nand.h
>     b/drivers/mtd/nand/tegra2_nand.h
>     >> new file mode 100644
>     >> index 0000000..7e74be7
>     >> --- /dev/null
>     >> +++ b/drivers/mtd/nand/tegra2_nand.h
>     >> @@ -0,0 +1,257 @@
>     >> +/*
>     >> + * (C) Copyright 2011 NVIDIA Corporation <www.nvidia.com
>     <http://www.nvidia.com>>
>     >> + *
>     >> + * See file CREDITS for list of people who contributed to this
>     >> + * project.
>     >> + *
>     >> + * This program is free software; you can redistribute it and/or
>     >> + * modify it under the terms of the GNU General Public License as
>     >> + * published by the Free Software Foundation; either version 2 of
>     >> + * the License, or (at your option) any later version.
>     >> + *
>     >> + * This program is distributed in the hope that it will be useful,
>     >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>     >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>     >> + * GNU General Public License for more details.
>     >> + *
>     >> + * You should have received a copy of the GNU General Public License
>     >> + * along with this program; if not, write to the Free Software
>     >> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>     >> + * MA 02111-1307 USA
>     >> + */
>     >> +
>     >> +/* register offset */
>     >> +#define COMMAND_0           0x00
>     >> +#define CMD_GO                      (1 << 31)
>     >> +#define CMD_CLE                     (1 << 30)
>     >> +#define CMD_ALE                     (1 << 29)
>     >> +#define CMD_PIO                     (1 << 28)
>     >> +#define CMD_TX                      (1 << 27)
>     >> +#define CMD_RX                      (1 << 26)
>     >> +#define CMD_SEC_CMD         (1 << 25)
>     >> +#define CMD_AFT_DAT_MASK    (1 << 24)
>     >> +#define CMD_AFT_DAT_DISABLE 0
>     >> +#define CMD_AFT_DAT_ENABLE  (1 << 24)
>     >> +#define CMD_TRANS_SIZE_SHIFT        20
>     >> +#define CMD_TRANS_SIZE_PAGE 8
>     >
>     > Please use proper namespacing on symbols defined in headers.
> 
>     >
>     > [Jim] Scott, I don't know what you meant. Could you give me an
>     example?
>     >  Thanks.
>     >  The names like AFT_DAT_MASK, TRANS_SIZE_SHIFT, and
>     TRANS_SIZE_PAGE come from field names of register in data sheet.
> 
>     Names in your data sheet are presumably unique within that chip, but not
>     within U-Boot as a whole.  Names like CMD_CLE, CMD_TX, etc. are
>     particularly likely to conflict (at least, if at least one other driver
>     were similarly loose with its naming).
> 
>     Prefix these with TEGRA_ or TEGRA_NAND_
> 
> 
> I'm not thrilled with this idea. It adds a lot of needless boilerplate
> to the code IMO.
> 
> This header can only be included by the tegra NAND driver after all.
> Please can you reconsider this?

If it can only be included in the NAND driver, why does it need to be in
a header at all?

-Scott