[PATCH 1/4] mtd: bcm47xxnflash: fix typo in freq calculation

[PATCH 1/4] mtd: bcm47xxnflash: fix typo in freq calculation

[Rafał Miłecki]

We are supposed to mask value, not multiply it. Add some comments btw.

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c b/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c
index b2ab373..dc204f3 100644
--- a/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c
+++ b/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c
@@ -364,11 +364,13 @@ int bcm47xxnflash_ops_bcm4706_init(struct bcm47xxnflash *b47n)
 
 	/* Configure wait counters */
 	if (b47n->cc->status & BCMA_CC_CHIPST_4706_PKG_OPTION) {
-		freq = 100000000;
+		/* 400 MHz */
+		freq = 400000000 / 4;
 	} else {
 		freq = bcma_chipco_pll_read(b47n->cc, 4);
-		freq = (freq * 0xFFF) >> 3;
-		freq = (freq * 25000000) >> 3;
+		freq = (freq & 0xFFF) >> 3;
+		/* Fixed reference clock 25 MHz and m = 2 */
+		freq = (freq * 25000000 / 2) / 4;
 	}
 	clock = freq / 1000000;
 	w0 = bcm47xxnflash_ops_bcm4706_ns_to_cycle(15, clock);
-- 
1.8.4.5

[PATCH 2/4] mtd: bcm47xxnflash: add dev_ready and fill chip_delay

[Rafał Miłecki]

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c b/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c
index dc204f3..1ea5e77 100644
--- a/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c
+++ b/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c
@@ -174,6 +174,14 @@ static void bcm47xxnflash_ops_bcm4706_select_chip(struct mtd_info *mtd,
 	return;
 }
 
+static int bcm47xxnflash_ops_bcm4706_dev_ready(struct mtd_info *mtd)
+{
+	struct nand_chip *nand_chip = (struct nand_chip *)mtd->priv;
+	struct bcm47xxnflash *b47n = (struct bcm47xxnflash *)nand_chip->priv;
+
+	return !!(bcma_cc_read32(b47n->cc, BCMA_CC_NFLASH_CTL) & NCTL_READY);
+}
+
 /*
  * Default nand_command and nand_command_lp don't match BCM4706 hardware layout.
  * For example, reading chip id is performed in a non-standard way.
@@ -341,6 +349,7 @@ static void bcm47xxnflash_ops_bcm4706_write_buf(struct mtd_info *mtd,
 
 int bcm47xxnflash_ops_bcm4706_init(struct bcm47xxnflash *b47n)
 {
+	struct nand_chip *nand_chip = (struct nand_chip *)&b47n->nand_chip;
 	int err;
 	u32 freq;
 	u16 clock;
@@ -351,10 +360,13 @@ int bcm47xxnflash_ops_bcm4706_init(struct bcm47xxnflash *b47n)
 	u32 val;
 
 	b47n->nand_chip.select_chip = bcm47xxnflash_ops_bcm4706_select_chip;
+	nand_chip->dev_ready = bcm47xxnflash_ops_bcm4706_dev_ready;
 	b47n->nand_chip.cmdfunc = bcm47xxnflash_ops_bcm4706_cmdfunc;
 	b47n->nand_chip.read_byte = bcm47xxnflash_ops_bcm4706_read_byte;
 	b47n->nand_chip.read_buf = bcm47xxnflash_ops_bcm4706_read_buf;
 	b47n->nand_chip.write_buf = bcm47xxnflash_ops_bcm4706_write_buf;
+
+	nand_chip->chip_delay = 50;
 	b47n->nand_chip.bbt_options = NAND_BBT_USE_FLASH;
 	b47n->nand_chip.ecc.mode = NAND_ECC_NONE; /* TODO: implement ECC */
 
-- 
1.8.4.5

Re: [PATCH 2/4] mtd: bcm47xxnflash: add dev_ready and fill chip_delay

[Brian Norris]

On Tue, Aug 19, 2014 at 09:14:14AM +0200, Rafał Miłecki wrote:
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
>  drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c b/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c
> index dc204f3..1ea5e77 100644
> --- a/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c
> +++ b/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c
> @@ -174,6 +174,14 @@ static void bcm47xxnflash_ops_bcm4706_select_chip(struct mtd_info *mtd,
>  	return;
>  }
>  
> +static int bcm47xxnflash_ops_bcm4706_dev_ready(struct mtd_info *mtd)
> +{
> +	struct nand_chip *nand_chip = (struct nand_chip *)mtd->priv;
> +	struct bcm47xxnflash *b47n = (struct bcm47xxnflash *)nand_chip->priv;
> +
> +	return !!(bcma_cc_read32(b47n->cc, BCMA_CC_NFLASH_CTL) & NCTL_READY);

BTW, you don't actually need the double negation, since
chip->dev_ready() is always used as a bool anyway. But no matter.

> +}
> +
>  /*
>   * Default nand_command and nand_command_lp don't match BCM4706 hardware layout.
>   * For example, reading chip id is performed in a non-standard way.
> @@ -341,6 +349,7 @@ static void bcm47xxnflash_ops_bcm4706_write_buf(struct mtd_info *mtd,
>  
>  int bcm47xxnflash_ops_bcm4706_init(struct bcm47xxnflash *b47n)
>  {
> +	struct nand_chip *nand_chip = (struct nand_chip *)&b47n->nand_chip;
>  	int err;
>  	u32 freq;
>  	u16 clock;
> @@ -351,10 +360,13 @@ int bcm47xxnflash_ops_bcm4706_init(struct bcm47xxnflash *b47n)
>  	u32 val;
>  
>  	b47n->nand_chip.select_chip = bcm47xxnflash_ops_bcm4706_select_chip;
> +	nand_chip->dev_ready = bcm47xxnflash_ops_bcm4706_dev_ready;
>  	b47n->nand_chip.cmdfunc = bcm47xxnflash_ops_bcm4706_cmdfunc;
>  	b47n->nand_chip.read_byte = bcm47xxnflash_ops_bcm4706_read_byte;
>  	b47n->nand_chip.read_buf = bcm47xxnflash_ops_bcm4706_read_buf;
>  	b47n->nand_chip.write_buf = bcm47xxnflash_ops_bcm4706_write_buf;
> +
> +	nand_chip->chip_delay = 50;

I'm curious, did you have a good reason to use something other than the
default provided by nand_base?

>  	b47n->nand_chip.bbt_options = NAND_BBT_USE_FLASH;
>  	b47n->nand_chip.ecc.mode = NAND_ECC_NONE; /* TODO: implement ECC */
>  

Brian

Re: [PATCH 2/4] mtd: bcm47xxnflash: add dev_ready and fill chip_delay

[Rafał Miłecki]

On 18 September 2014 08:27, Brian Norris <computersforpeace@gmail.com> wrote:
> On Tue, Aug 19, 2014 at 09:14:14AM +0200, Rafał Miłecki wrote:
>> +
>> +     nand_chip->chip_delay = 50;
>
> I'm curious, did you have a good reason to use something other than the
> default provided by nand_base?

I simply followed what Broadcom used in their ugly code (don't look to
deep into that file, or you'll go crazy):
http://svn.dd-wrt.com/browser/src/linux/universal/linux-3.8/drivers/mtd/bcm947xx/nand/brcmnand_47xx.c?rev=22249#L2240

They set "dev_ready" as well as "chip_delay" (to the 50), so the only
case where they use this delay is
NAND_CMD_STATUS_ERROR
NAND_CMD_STATUS_ERROR[0-3]

Do you think we should try to stay with the default value?

-- 
Rafał

Re: [PATCH 2/4] mtd: bcm47xxnflash: add dev_ready and fill chip_delay

[Brian Norris]

On Wed, Sep 17, 2014 at 11:43 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
> On 18 September 2014 08:27, Brian Norris <computersforpeace@gmail.com> wrote:
>> On Tue, Aug 19, 2014 at 09:14:14AM +0200, Rafał Miłecki wrote:
>>> +
>>> +     nand_chip->chip_delay = 50;
>>
>> I'm curious, did you have a good reason to use something other than the
>> default provided by nand_base?
>
> I simply followed what Broadcom used in their ugly code (don't look to
> deep into that file, or you'll go crazy):
> http://svn.dd-wrt.com/browser/src/linux/universal/linux-3.8/drivers/mtd/bcm947xx/nand/brcmnand_47xx.c?rev=22249#L2240
>
> They set "dev_ready" as well as "chip_delay" (to the 50), so the only
> case where they use this delay is
> NAND_CMD_STATUS_ERROR
> NAND_CMD_STATUS_ERROR[0-3]
>
> Do you think we should try to stay with the default value?

No, it's fine as-is.

Brian

[PATCH 3/4] mtd: bcm47xxnflash: add cmd_ctrl handler

[Rafał Miłecki]

This won't be used by NAND subsystem as we implement cmdfunc on our
own, but will allow us to write a bit cleaner code.

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c b/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c
index 1ea5e77..30df67a 100644
--- a/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c
+++ b/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c
@@ -167,6 +167,26 @@ static void bcm47xxnflash_ops_bcm4706_write(struct mtd_info *mtd,
  * NAND chip ops
  **************************************************/
 
+static void bcm47xxnflash_ops_bcm4706_cmd_ctrl(struct mtd_info *mtd, int cmd,
+					       unsigned int ctrl)
+{
+	struct nand_chip *nand_chip = (struct nand_chip *)mtd->priv;
+	struct bcm47xxnflash *b47n = (struct bcm47xxnflash *)nand_chip->priv;
+	u32 code = 0;
+
+	if (cmd == NAND_CMD_NONE)
+		return;
+
+	if (cmd & NAND_CTRL_CLE)
+		code = cmd | NCTL_CMD0;
+
+	/* nCS is not needed for reset command */
+	if (cmd != NAND_CMD_RESET)
+		code |= NCTL_CSA;
+
+	bcm47xxnflash_ops_bcm4706_ctl_cmd(b47n->cc, code);
+}
+
 /* Default nand_select_chip calls cmd_ctrl, which is not used in BCM4706 */
 static void bcm47xxnflash_ops_bcm4706_select_chip(struct mtd_info *mtd,
 						  int chip)
@@ -360,6 +380,7 @@ int bcm47xxnflash_ops_bcm4706_init(struct bcm47xxnflash *b47n)
 	u32 val;
 
 	b47n->nand_chip.select_chip = bcm47xxnflash_ops_bcm4706_select_chip;
+	nand_chip->cmd_ctrl = bcm47xxnflash_ops_bcm4706_cmd_ctrl;
 	nand_chip->dev_ready = bcm47xxnflash_ops_bcm4706_dev_ready;
 	b47n->nand_chip.cmdfunc = bcm47xxnflash_ops_bcm4706_cmdfunc;
 	b47n->nand_chip.read_byte = bcm47xxnflash_ops_bcm4706_read_byte;
-- 
1.8.4.5

[PATCH 4/4] mtd: bcm47xxnflash: NAND_CMD_RESET support

[Rafał Miłecki]

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c b/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c
index 30df67a..82844ef 100644
--- a/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c
+++ b/drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c
@@ -14,6 +14,7 @@
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/slab.h>
+#include <linux/delay.h>
 #include <linux/bcma/bcma.h>
 
 /* Broadcom uses 1'000'000 but it seems to be too many. Tests on WNDR4500 has
@@ -226,7 +227,10 @@ static void bcm47xxnflash_ops_bcm4706_cmdfunc(struct mtd_info *mtd,
 
 	switch (command) {
 	case NAND_CMD_RESET:
-		pr_warn("Chip reset not implemented yet\n");
+		nand_chip->cmd_ctrl(mtd, command, NAND_CTRL_CLE);
+
+		ndelay(100);
+		nand_wait_ready(mtd);
 		break;
 	case NAND_CMD_READID:
 		ctlcode = NCTL_CSA | 0x01000000 | NCTL_CMD1W | NCTL_CMD0;
-- 
1.8.4.5

Re: [PATCH 1/4] mtd: bcm47xxnflash: fix typo in freq calculation

[Brian Norris]

On Tue, Aug 19, 2014 at 09:14:13AM +0200, Rafał Miłecki wrote:
> We are supposed to mask value, not multiply it. Add some comments btw.
> 
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>

Pushed all 4 to l2-mtd.git. Thanks!

Brian