Hello Tatha,

Le 02/28/12 07:50, Tathagata Das a écrit :
Hi,
   Attached is the updated kernel patch to support brcm47xx BCMA NAND flash.
I have used latest trunk source code to create this patch. I have tested it on 
Netgear WNR3500Lv2.

Thanks to Florian and Hauke for their comments.

And thank you for your efforts on this, I am sorry, but I found a couple of small issues in your patch, which I attached inline for easier commenting:

9991-bcm47xx-bcma-nandflash.patch


diff -Naur a/arch/mips/bcm47xx/bus.c b/arch/mips/bcm47xx/bus.c
--- a/arch/mips/bcm47xx/bus.c   2012-02-17 16:34:21.000000000 +0530
+++ b/arch/mips/bcm47xx/bus.c   2012-02-23 18:22:17.000000000 +0530
@@ -2,6 +2,7 @@
  * BCM947xx nvram variable access
  *
  * Copyright (C) 2011 Hauke Mehrtens<ha...@hauke-m.de>
+ * Copyright (C) 2011-2012 Tathagata Das<tathag...@alumnux.com>
  *
  * 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
@@ -92,3 +93,9 @@
        sflash->numblocks = scc->sflash.numblocks;
        sflash->size = scc->sflash.size;
 }
+
+void bcm47xx_nflash_struct_bcma_init(struct bcm47xx_nflash *nflash, struct 
bcma_drv_cc *bcc)
+{
+       nflash->nflash_type = BCM47XX_BUS_TYPE_BCMA;
+       nflash->bcc = bcc;
+}
diff -Naur a/arch/mips/bcm47xx/nvram.c b/arch/mips/bcm47xx/nvram.c
--- a/arch/mips/bcm47xx/nvram.c 2012-02-17 16:34:22.000000000 +0530
+++ b/arch/mips/bcm47xx/nvram.c 2012-02-23 18:20:57.000000000 +0530
@@ -4,6 +4,7 @@
  * Copyright (C) 2005 Broadcom Corporation
  * Copyright (C) 2006 Felix Fietkau<n...@openwrt.org>
  * Copyright (C) 2010-2011 Hauke Mehrtens<ha...@hauke-m.de>
+ * Copyright (C) 2011-2012 Tathagata Das<tathag...@alumnux.com>
  *
  * 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
@@ -21,6 +22,7 @@
 #include<asm/mach-bcm47xx/nvram.h>
 #include<asm/mach-bcm47xx/bcm47xx.h>
 #include<asm/mach-bcm47xx/bus.h>
+#include<linux/mtd/bcm47xx_nand.h>

 char nvram_buf[NVRAM_SPACE];
 EXPORT_SYMBOL(nvram_buf);
@@ -159,6 +161,53 @@
        return 0;
 }

+static int early_nvram_init_nflash(void)
+{
+       struct nvram_header *header;
+       u32 off;
+       int ret;
+       int len;
+       u32 flash_size = bcm47xx_nflash.size;
+       u8 tmpbuf[NFL_SECTOR_SIZE];
+       int i;
+       u32 *src, *dst;
+
+       /* check if the struct is already initilized */
+       if (!flash_size)
+               return -1;
+       
+       cfe_env = 0;
+
+       off = FLASH_MIN;
+       while (off<= flash_size) {
+               ret = bcma_nflash_read(bcm47xx_nflash.bcc, off, 
NFL_SECTOR_SIZE, tmpbuf);
+               if (ret != NFL_SECTOR_SIZE) {
+                       goto done;
+               }

The brackets here are not required.

 +              header = (struct nvram_header *)tmpbuf;
+               if (header->magic == NVRAM_HEADER) {
+                       goto found;
+               }

They are not required here too.

 +              off<<= 1;
+       }
+
+       ret = -1;
+       goto done;
+
+found:
+       len = header->len;
+       header = (struct nvram_header *) KSEG1ADDR(NAND_FLASH1 + off);
+       src = (u32 *) header;
+       dst = (u32 *) nvram_buf;
+       for (i = 0; i<  sizeof(struct nvram_header); i += 4)
+               *dst++ = *src++;
+       for (; i<  len&&  i<  NVRAM_SPACE; i += 4)
+               *dst++ = *src++;

Did you have to do this because memcpy() did not work, due to byte access 
instead of word access?

 +      ret = 0;
+done:
+       return ret;
+}
+
[snip]

+
+/* Issue a nand flash command */
+static inline void bcma_nflash_cmd(struct bcma_drv_cc *cc, u32 opcode)
+{
+       bcma_cc_write32(cc, NAND_CMD_START, opcode);
+       bcma_cc_read32(cc,  NAND_CMD_START);
+}
+
+/* Check offset and length */
+static int bcma_nflash_offset_is_valid(struct bcma_drv_cc *cc, u32 offset, u32 
len, u32 mask)
+{
+       if ((offset&  mask) != 0 || (len&  mask) != 0) {
+               pr_err("%s(): Address is not aligned. offset: %x, len: %x, mask: 
%x\n", __func__, offset, len, mask);
+               BUG_ON(1);
+               return 1;
+       }

a BUG_ON() here is a little hard, just the printk should be sufficient in case 
someone is doing unaligned accesses.

 +
+       if ((((offset + len)>>  20)>  cc->nflash.size) ||
+               ((((offset + len)>>  20) == cc->nflash.size)&&

these two lines are equal to:

((offset + len)>>  20)>= cc->nflash.size

 +              (((offset + len)&  ((1<<  20) - 1)) != 0))) {
+               pr_err("%s(): Address is outside Flash memory region. offset: %x, 
len: %x, mask: %x\n", __func__, offset, len, mask);
+               return 1;
+       }
+
+       return 0;
+}

Everything else is fine from my perspective otherwise.

Thank you.
--
Florian

_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Reply via email to