[linux-sunxi] Re: [PATCH v3 5/9] mtd: nand: add sunxi NAND flash controller support

2014-05-20 Thread Boris BREZILLON

On 20/05/2014 21:21, Brian Norris wrote:
> On Tue, May 20, 2014 at 11:49:42AM -0700, Brian Norris wrote:
>> On Fri, May 09, 2014 at 06:47:22PM +0200, Boris BREZILLON wrote:
>>> On 09/05/2014 18:03, Ezequiel Garcia wrote:
 On 12 Mar 07:07 PM, Boris BREZILLON wrote:
> --- /dev/null
> +++ b/drivers/mtd/nand/sunxi_nand.c
> @@ -0,0 +1,1276 @@
>> ...
> +static int sunxi_nand_ecc_init(struct mtd_info *mtd, struct 
> nand_ecc_ctrl *ecc,
> +struct device_node *np)
> +{
> + struct nand_chip *nand = mtd->priv;
> + int ecc_step_size, ecc_strength;
> + int ret;
> +
> + ecc_step_size = of_get_nand_ecc_step_size(np);
> + ecc_strength = of_get_nand_ecc_strength(np);
> + if (ecc_step_size > 0 && ecc_strength > 0) {
> + ecc->size = ecc_step_size;
> + ecc->strength = ecc_strength;
> + } else {
> + ecc->size = nand->ecc_step_ds;
> + ecc->strength = nand->ecc_strength_ds;
> + }
> +
 Shouldn't you check the devicetree value is not weaker than the 
 ONFI-obtained?
>>> I can definitely do that.
>> You can do that here, but take a look at the discussion Ezequiel and I
>> had about this:
>>
>>   http://thread.gmane.org/gmane.linux.drivers.devicetree/67462
>>
>> We probably don't want to be doing anything drastic like overriding the
>> device tree configuration. Instead, we might want to stick a warning
>> into the core nand_base code that does the proper comparison of the
>> '*_ds' values with the actual values chosen in
>> chip->ecc->{size,strength}. The comparison is kind of subtle, actually,
>> so it'd be good to do it exactly once for everyone.

Fair enough, I'll follow your suggestions on this specific point ;)

> I forgot, Ezequiel already submitted this. I'll look at it soon:
>
>   http://patchwork.ozlabs.org/patch/348901/

Thanks for pointing this out.


>
> Brian

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[linux-sunxi] Re: [PATCH v3 5/9] mtd: nand: add sunxi NAND flash controller support

2014-05-20 Thread Brian Norris
On Tue, May 20, 2014 at 11:49:42AM -0700, Brian Norris wrote:
> On Fri, May 09, 2014 at 06:47:22PM +0200, Boris BREZILLON wrote:
> > On 09/05/2014 18:03, Ezequiel Garcia wrote:
> > > On 12 Mar 07:07 PM, Boris BREZILLON wrote:
> > >> --- /dev/null
> > >> +++ b/drivers/mtd/nand/sunxi_nand.c
> > >> @@ -0,0 +1,1276 @@
> ...
> > >> +static int sunxi_nand_ecc_init(struct mtd_info *mtd, struct 
> > >> nand_ecc_ctrl *ecc,
> > >> +   struct device_node *np)
> > >> +{
> > >> +struct nand_chip *nand = mtd->priv;
> > >> +int ecc_step_size, ecc_strength;
> > >> +int ret;
> > >> +
> > >> +ecc_step_size = of_get_nand_ecc_step_size(np);
> > >> +ecc_strength = of_get_nand_ecc_strength(np);
> > >> +if (ecc_step_size > 0 && ecc_strength > 0) {
> > >> +ecc->size = ecc_step_size;
> > >> +ecc->strength = ecc_strength;
> > >> +} else {
> > >> +ecc->size = nand->ecc_step_ds;
> > >> +ecc->strength = nand->ecc_strength_ds;
> > >> +}
> > >> +
> > > Shouldn't you check the devicetree value is not weaker than the 
> > > ONFI-obtained?
> > 
> > I can definitely do that.
> 
> You can do that here, but take a look at the discussion Ezequiel and I
> had about this:
> 
>   http://thread.gmane.org/gmane.linux.drivers.devicetree/67462
> 
> We probably don't want to be doing anything drastic like overriding the
> device tree configuration. Instead, we might want to stick a warning
> into the core nand_base code that does the proper comparison of the
> '*_ds' values with the actual values chosen in
> chip->ecc->{size,strength}. The comparison is kind of subtle, actually,
> so it'd be good to do it exactly once for everyone.

I forgot, Ezequiel already submitted this. I'll look at it soon:

  http://patchwork.ozlabs.org/patch/348901/

Brian

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[linux-sunxi] Re: [PATCH v3 5/9] mtd: nand: add sunxi NAND flash controller support

2014-05-20 Thread Brian Norris
On Fri, May 09, 2014 at 06:47:22PM +0200, Boris BREZILLON wrote:
> On 09/05/2014 18:03, Ezequiel Garcia wrote:
> > On 12 Mar 07:07 PM, Boris BREZILLON wrote:
> >> --- /dev/null
> >> +++ b/drivers/mtd/nand/sunxi_nand.c
> >> @@ -0,0 +1,1276 @@
...
> >> +static int sunxi_nand_ecc_init(struct mtd_info *mtd, struct nand_ecc_ctrl 
> >> *ecc,
> >> + struct device_node *np)
> >> +{
> >> +  struct nand_chip *nand = mtd->priv;
> >> +  int ecc_step_size, ecc_strength;
> >> +  int ret;
> >> +
> >> +  ecc_step_size = of_get_nand_ecc_step_size(np);
> >> +  ecc_strength = of_get_nand_ecc_strength(np);
> >> +  if (ecc_step_size > 0 && ecc_strength > 0) {
> >> +  ecc->size = ecc_step_size;
> >> +  ecc->strength = ecc_strength;
> >> +  } else {
> >> +  ecc->size = nand->ecc_step_ds;
> >> +  ecc->strength = nand->ecc_strength_ds;
> >> +  }
> >> +
> > Shouldn't you check the devicetree value is not weaker than the 
> > ONFI-obtained?
> 
> I can definitely do that.

You can do that here, but take a look at the discussion Ezequiel and I
had about this:

  http://thread.gmane.org/gmane.linux.drivers.devicetree/67462

We probably don't want to be doing anything drastic like overriding the
device tree configuration. Instead, we might want to stick a warning
into the core nand_base code that does the proper comparison of the
'*_ds' values with the actual values chosen in
chip->ecc->{size,strength}. The comparison is kind of subtle, actually,
so it'd be good to do it exactly once for everyone.

Brian

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[linux-sunxi] Re: [PATCH v3 5/9] mtd: nand: add sunxi NAND flash controller support

2014-05-09 Thread Ezequiel Garcia
On 09 May 06:47 PM, Boris BREZILLON wrote:
> > Can you add some documentation about this read-back stuff?
> 
> This has nothing to do with read-back, RB means ready/busy :-).
> 

Hehe... right... I guess I was thinking in read/busy.

See? That's why you need to document it: so it's fool-proof.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[linux-sunxi] Re: [PATCH v3 5/9] mtd: nand: add sunxi NAND flash controller support

2014-05-09 Thread Boris BREZILLON

On 09/05/2014 18:03, Ezequiel Garcia wrote:
> Hello Boris,
>
> Sorry for the review delay.
>
> Emilio, if you have hardware to test this, it would be nice to give
> Boris some Tested-by?
>
> On 12 Mar 07:07 PM, Boris BREZILLON wrote:
>> Add support for the sunxi NAND Flash Controller (NFC).
>>
>> Signed-off-by: Boris BREZILLON 
>> ---
>>  drivers/mtd/nand/Kconfig  |6 +
>>  drivers/mtd/nand/Makefile |1 +
>>  drivers/mtd/nand/sunxi_nand.c | 1276 
>> +
>>  3 files changed, 1283 insertions(+)
>>  create mode 100644 drivers/mtd/nand/sunxi_nand.c
>>
>> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
>> index 90ff447..8a28c06 100644
>> --- a/drivers/mtd/nand/Kconfig
>> +++ b/drivers/mtd/nand/Kconfig
>> @@ -510,4 +510,10 @@ config MTD_NAND_XWAY
>>Enables support for NAND Flash chips on Lantiq XWAY SoCs. NAND is 
>> attached
>>to the External Bus Unit (EBU).
>>  
>> +config MTD_NAND_SUNXI
>> +tristate "Support for NAND on Allwinner SoCs"
>> +depends on ARCH_SUNXI
>> +help
>> +  Enables support for NAND Flash chips on Allwinner SoCs.
>> +
>>  endif # MTD_NAND
>> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
>> index 0b8a822..34f45d8 100644
>> --- a/drivers/mtd/nand/Makefile
>> +++ b/drivers/mtd/nand/Makefile
>> @@ -49,5 +49,6 @@ obj-$(CONFIG_MTD_NAND_JZ4740)  += jz4740_nand.o
>>  obj-$(CONFIG_MTD_NAND_GPMI_NAND)+= gpmi-nand/
>>  obj-$(CONFIG_MTD_NAND_XWAY) += xway_nand.o
>>  obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)+= bcm47xxnflash/
>> +obj-$(CONFIG_MTD_NAND_SUNXI)+= sunxi_nand.o
>>  
>>  nand-objs := nand_base.o nand_bbt.o
>> diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
>> new file mode 100644
>> index 000..e93cc44
>> --- /dev/null
>> +++ b/drivers/mtd/nand/sunxi_nand.c
>> @@ -0,0 +1,1276 @@
>> +/*
>> + * Copyright (C) 2013 Boris BREZILLON 
>> + *
>> + * Derived from:
>> + *  https://github.com/yuq/sunxi-nfc-mtd
>> + *  Copyright (C) 2013 Qiang Yu 
>> + *
>> + *  https://github.com/hno/Allwinner-Info
>> + *  Copyright (C) 2013 Henrik Nordström 
>> + *
>> + *  Copyright (C) 2013 Dmitriy B. 
>> + *  Copyright (C) 2013 Sergey Lapin 
>> + *
>> + * 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.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define NFC_REG_CTL 0x
>> +#define NFC_REG_ST  0x0004
>> +#define NFC_REG_INT 0x0008
>> +#define NFC_REG_TIMING_CTL  0x000C
>> +#define NFC_REG_TIMING_CFG  0x0010
>> +#define NFC_REG_ADDR_LOW0x0014
>> +#define NFC_REG_ADDR_HIGH   0x0018
>> +#define NFC_REG_SECTOR_NUM  0x001C
>> +#define NFC_REG_CNT 0x0020
>> +#define NFC_REG_CMD 0x0024
>> +#define NFC_REG_RCMD_SET0x0028
>> +#define NFC_REG_WCMD_SET0x002C
>> +#define NFC_REG_IO_DATA 0x0030
>> +#define NFC_REG_ECC_CTL 0x0034
>> +#define NFC_REG_ECC_ST  0x0038
>> +#define NFC_REG_DEBUG   0x003C
>> +#define NFC_REG_ECC_CNT00x0040
>> +#define NFC_REG_ECC_CNT10x0044
>> +#define NFC_REG_ECC_CNT20x0048
>> +#define NFC_REG_ECC_CNT30x004c
>> +#define NFC_REG_USER_DATA_BASE  0x0050
>> +#define NFC_REG_SPARE_AREA  0x00A0
>> +#define NFC_RAM0_BASE   0x0400
>> +#define NFC_RAM1_BASE   0x0800
>> +
>> +/*define bit use in NFC_CTL*/
> nit: Use BIT() for these?

Okay.

>
>> +#define NFC_EN  (1 << 0)
>> +#define NFC_RESET   (1 << 1)
>> +#define NFC_BUS_WIDYH   (1 << 2)
>> +#define NFC_RB_SEL  (1 << 3)
>> +#define NFC_CE_SEL  (7 << 24)
>> +#define NFC_CE_CTL  (1 << 6)
>> +#define NFC_CE_CTL1 (1 << 7)
>> +#define NFC_PAGE_SIZE   (0xf << 8)
>> +#define NFC_SAM (1 << 12)
>> +#define NFC_RAM_METHOD  (1 << 14)
>> +#define NFC_DEBUG_CTL   (1 << 31)
>> +
>> +/*define bit use in NFC_ST*/
>> +#define NFC_RB_B2R  (1 << 0)
>> +#define NFC_CMD_INT_FLAG(1 << 1)
>> +#define NFC_DMA_INT_FLAG(1 << 2)
>> +#define NFC_CMD_FIFO_STATUS (1 << 3)
>> +#define NFC_STA 

[linux-sunxi] Re: [PATCH v3 5/9] mtd: nand: add sunxi NAND flash controller support

2014-05-09 Thread Ezequiel Garcia
Hello Boris,

Sorry for the review delay.

Emilio, if you have hardware to test this, it would be nice to give
Boris some Tested-by?

On 12 Mar 07:07 PM, Boris BREZILLON wrote:
> Add support for the sunxi NAND Flash Controller (NFC).
> 
> Signed-off-by: Boris BREZILLON 
> ---
>  drivers/mtd/nand/Kconfig  |6 +
>  drivers/mtd/nand/Makefile |1 +
>  drivers/mtd/nand/sunxi_nand.c | 1276 
> +
>  3 files changed, 1283 insertions(+)
>  create mode 100644 drivers/mtd/nand/sunxi_nand.c
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 90ff447..8a28c06 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -510,4 +510,10 @@ config MTD_NAND_XWAY
> Enables support for NAND Flash chips on Lantiq XWAY SoCs. NAND is 
> attached
> to the External Bus Unit (EBU).
>  
> +config MTD_NAND_SUNXI
> + tristate "Support for NAND on Allwinner SoCs"
> + depends on ARCH_SUNXI
> + help
> +   Enables support for NAND Flash chips on Allwinner SoCs.
> +
>  endif # MTD_NAND
> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index 0b8a822..34f45d8 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -49,5 +49,6 @@ obj-$(CONFIG_MTD_NAND_JZ4740)   += jz4740_nand.o
>  obj-$(CONFIG_MTD_NAND_GPMI_NAND) += gpmi-nand/
>  obj-$(CONFIG_MTD_NAND_XWAY)  += xway_nand.o
>  obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH) += bcm47xxnflash/
> +obj-$(CONFIG_MTD_NAND_SUNXI) += sunxi_nand.o
>  
>  nand-objs := nand_base.o nand_bbt.o
> diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
> new file mode 100644
> index 000..e93cc44
> --- /dev/null
> +++ b/drivers/mtd/nand/sunxi_nand.c
> @@ -0,0 +1,1276 @@
> +/*
> + * Copyright (C) 2013 Boris BREZILLON 
> + *
> + * Derived from:
> + *   https://github.com/yuq/sunxi-nfc-mtd
> + *   Copyright (C) 2013 Qiang Yu 
> + *
> + *   https://github.com/hno/Allwinner-Info
> + *   Copyright (C) 2013 Henrik Nordström 
> + *
> + *   Copyright (C) 2013 Dmitriy B. 
> + *   Copyright (C) 2013 Sergey Lapin 
> + *
> + * 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.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define NFC_REG_CTL  0x
> +#define NFC_REG_ST   0x0004
> +#define NFC_REG_INT  0x0008
> +#define NFC_REG_TIMING_CTL   0x000C
> +#define NFC_REG_TIMING_CFG   0x0010
> +#define NFC_REG_ADDR_LOW 0x0014
> +#define NFC_REG_ADDR_HIGH0x0018
> +#define NFC_REG_SECTOR_NUM   0x001C
> +#define NFC_REG_CNT  0x0020
> +#define NFC_REG_CMD  0x0024
> +#define NFC_REG_RCMD_SET 0x0028
> +#define NFC_REG_WCMD_SET 0x002C
> +#define NFC_REG_IO_DATA  0x0030
> +#define NFC_REG_ECC_CTL  0x0034
> +#define NFC_REG_ECC_ST   0x0038
> +#define NFC_REG_DEBUG0x003C
> +#define NFC_REG_ECC_CNT0 0x0040
> +#define NFC_REG_ECC_CNT1 0x0044
> +#define NFC_REG_ECC_CNT2 0x0048
> +#define NFC_REG_ECC_CNT3 0x004c
> +#define NFC_REG_USER_DATA_BASE   0x0050
> +#define NFC_REG_SPARE_AREA   0x00A0
> +#define NFC_RAM0_BASE0x0400
> +#define NFC_RAM1_BASE0x0800
> +
> +/*define bit use in NFC_CTL*/

nit: Use BIT() for these?

> +#define NFC_EN   (1 << 0)
> +#define NFC_RESET(1 << 1)
> +#define NFC_BUS_WIDYH(1 << 2)
> +#define NFC_RB_SEL   (1 << 3)
> +#define NFC_CE_SEL   (7 << 24)
> +#define NFC_CE_CTL   (1 << 6)
> +#define NFC_CE_CTL1  (1 << 7)
> +#define NFC_PAGE_SIZE(0xf << 8)
> +#define NFC_SAM  (1 << 12)
> +#define NFC_RAM_METHOD   (1 << 14)
> +#define NFC_DEBUG_CTL(1 << 31)
> +
> +/*define bit use in NFC_ST*/
> +#define NFC_RB_B2R   (1 << 0)
> +#define NFC_CMD_INT_FLAG (1 << 1)
> +#define NFC_DMA_INT_FLAG (1 << 2)
> +#define NFC_CMD_FIFO_STATUS  (1 << 3)
> +#define NFC_STA  (1 << 4)
> +#define NFC_NATCH_INT_FLAG   (1 << 5)
> +#define NFC_RB_STATE0(1 << 8)
> +#define NFC_RB_STA