Re: [PATCH] ARM: mach-shmobile: G3EVM FLCTL platform data

[Magnus Damm <magnus.damm@gmail.com>]

On Mon, Feb 22, 2010 at 7:31 PM, Paul Mundt <lethal@linux-sh.org> wrote:
> On Fri, Feb 19, 2010 at 06:54:06PM +0900, Magnus Damm wrote:
>> +static struct mtd_partition nand_partition_info[] = {
>> +     {
>> +             .name   = "system",
>> +             .offset = 0,
>> +             .size   = 64 * 1024 * 1024,
>> +     },
>> +     {
>> +             .name   = "userdata",
>> +             .offset = MTDPART_OFS_APPEND,
>> +             .size   = 128 * 1024 * 1024,
>> +     },
>> +     {
>> +             .name   = "cache",
>> +             .offset = MTDPART_OFS_APPEND,
>> +             .size   = 64 * 1024 * 1024,
>> +     },
>> +};
>> +
> You may want to #include <asm/sizes.h> here and use the SZ_ helpers
> instead. There is far too much existing ambiguity with sz vs sz - 1 for
> resource length that consolidating on common helpers at least makes the
> intent more immediately apparent.

Sounds like a good plan. I guess the existing NOR flash code needs an
update as well then.

>> +static struct resource nand_flash_resources[] = {
>> +     [0] = {
>> +             .start  = 0xe6a30000,
>> +             .end    = 0xe6a3009b,
>> +             .flags  = IORESOURCE_MEM,
>> +     }
>> +};
>> +
>> +static struct sh_flctl_platform_data nand_flash_data = {
>> +     .parts          = nand_partition_info,
>> +     .nr_parts       = ARRAY_SIZE(nand_partition_info),
>> +     .flcmncr_val    = QTSEL_E | FCKSEL_E | TYPESEL_SET | NANWF_E
>> +                     | SHBUSSEL | SEL_16BIT,
>> +};
>
> This is also a bit special, |'s usually are at the end of the previous
> line, not the beginning of the second one.

Ok, let's not be special!

>> +     /* FOE, FCDE, FSC on dedicated pins */
>> +     __raw_writel(__raw_readl(0xe6158048) & ~(1 << 15), 0xe6158048);
>> +
> This looks suspiciously like an obfuscated MSTP toggle that the clock
> framework needs to be handling. I thought I was already pretty clear when
> I said I wouldn't be applying any more of these sorts of patches until
> some basic clock framework support was in place?

I didn't ask you to apply the patch, just wanted to post it so other
people can get the code.

Cheers,

/ magnus