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

[Scott Wood]

On 05/22/2012 03:06 PM, Scott Wood wrote:
> On 05/22/2012 03:04 PM, Simon Glass wrote:
>> Hi Scott,
>>
>> On Mon, May 21, 2012 at 8:47 AM, Scott Wood <scottwood at freescale.com
>> <mailto:scottwood at 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?

That said, I see it's in drivers/mtd/nand/ and not include/, so if you
really want to structure it that way, it's OK with me.

-Scott