subject:"\[PATCH v2\] mtd\:spi\-nor\: Add Altera EPCQ Driver"

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

2015-03-16 Thread Viet Nga Dao

On Fri, Mar 13, 2015 at 3:38 PM, Brian Norris
 wrote:
> Hi Viet,
>
> On Wed, Feb 11, 2015 at 12:53:00PM +0800, Viet Nga Dao wrote:
>> From: Viet Nga Dao 
>>
>> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
>> EPCS flash chips. This patch adds driver for these devices.
>>
>> Signed-off-by: VIET NGA DAO 
>>
>> ---
>> v2:
>> - Change to spi_nor structure
>> - Add lock and unlock functions for spi_nor
>> - Simplify the altera_epcq_lock function
>> - Replace reg by compatible in device tree
>> ---
>>  .../devicetree/bindings/mtd/altera_epcq.txt|   45 ++
>>  drivers/mtd/spi-nor/Kconfig|   12 +
>>  drivers/mtd/spi-nor/Makefile   |1 +
>>  drivers/mtd/spi-nor/altera_epcq.c  |  507 
>> 
>>  drivers/mtd/spi-nor/altera_epcq.h  |  116 +
>>  drivers/mtd/spi-nor/spi-nor.c  |   86 -
>>  include/linux/mtd/spi-nor.h|3 +-
>>  7 files changed, 764 insertions(+), 6 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/mtd/altera_epcq.txt
>>  create mode 100644 drivers/mtd/spi-nor/altera_epcq.c
>>  create mode 100644 drivers/mtd/spi-nor/altera_epcq.h
>
> Please split this up into at least 2 patches, if not 3. One or more for
> the spi-nor changes, split logically; one for your new driver; and maybe
> a separate one for the binding documentation.
>
> Also, as Rafal noted, your patche is whitespace damaged, so I can't
> apply or build it.
>
> Please make sure you can apply, build, and run your patch on top of
> l2-mtd.git:
>
>   http://git.infradead.org/l2-mtd.git
>   git://git.infradead.org/l2-mtd.git
>
> ...
>> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
>> index 64a4f0e..83178b9 100644
>> --- a/drivers/mtd/spi-nor/Kconfig
>> +++ b/drivers/mtd/spi-nor/Kconfig
>> @@ -28,4 +28,16 @@ config SPI_FSL_QUADSPI
>>This enables support for the Quad SPI controller in master mode.
>>We only connect the NOR to this controller now.
>>
>> +config SPI_ALTERA_EPCQ
>> +tristate "Support Altera EPCQ/EPCS Flash chips"
>> +depends on OF
>> +help
>> +  This enables access to Altera EPCQ/EPCS flash chips, used for data
>> +  storage. See the driver source for the current list,
>> +  or to add other chips.
>> +
>> +  If you want to compile this driver as a module ( = code which can be
>> +  inserted in and removed from the running kernel whenever you want),
>> +  say M here and read .
>
> I don't think you need to explain what a module is here. Look at other
> Kconfig entries as examples, but I think you can just drop that last
> paragraph.
>
>> +
>>  endif # MTD_SPI_NOR
> ...
>> diff --git a/drivers/mtd/spi-nor/altera_epcq.c
>> b/drivers/mtd/spi-nor/altera_epcq.c
>> new file mode 100644
>> index 000..9db0d05
>> --- /dev/null
>> +++ b/drivers/mtd/spi-nor/altera_epcq.c
>> @@ -0,0 +1,507 @@
>> +/*
>> + * Copyright (C) 2014 Altera Corporation. All rights reserved
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope 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, see .
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "altera_epcq.h"
>> +
>> +static int altera_epcq_write_reg(struct spi_nor *nor, u8 opcode, u8 *val,
>> + int len, int wr_en)
>> +{
>> +return 0;
>> +}
>> +
>> +static int altera_epcq_read_reg(struct spi_nor *nor, u8 opcode, u8 *val,
>> +int len)
>> +{
>> +struct altera_epcq_flash *flash = nor->priv;
>> +struct altera_epcq *dev = flash->priv;
>> +u32 data = 0;
>> +u8 id[5];
>> +
>> +switch (opcode) {
>> +case SPINOR_OP_RDSR:
>> +data = readl(dev->csr_base + EPCQ_SR_REG);
>> +*val = (u8)data & EPCQ_SR_MASK;
>> +break;
>> +case SPINOR_OP_RDID:
>> +/* opcode id */
>> +if (dev->is_epcs) {
>> +data = readl(dev->csr_base + EPCQ_SID_REG);
>> +id[4] = EPCS_OPCODE_ID;
>> +} else {
>> +data = readl(dev->csr_base + EPCQ_RDID_REG);
>> +id[4] = EPCQ_OPCODE_ID;
>> +}
>
> I think Rafal had some suggestions

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

2015-03-13 Thread Viet Nga Dao

Hi Brian,
Thanks for your detail comment. I will make modification and submit
the separated patches

On Fri, Mar 13, 2015 at 3:38 PM, Brian Norris
 wrote:
> Hi Viet,
>
> On Wed, Feb 11, 2015 at 12:53:00PM +0800, Viet Nga Dao wrote:
>> From: Viet Nga Dao 
>>
>> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
>> EPCS flash chips. This patch adds driver for these devices.
>>
>> Signed-off-by: VIET NGA DAO 
>>
>> ---
>> v2:
>> - Change to spi_nor structure
>> - Add lock and unlock functions for spi_nor
>> - Simplify the altera_epcq_lock function
>> - Replace reg by compatible in device tree
>> ---
>>  .../devicetree/bindings/mtd/altera_epcq.txt|   45 ++
>>  drivers/mtd/spi-nor/Kconfig|   12 +
>>  drivers/mtd/spi-nor/Makefile   |1 +
>>  drivers/mtd/spi-nor/altera_epcq.c  |  507 
>> 
>>  drivers/mtd/spi-nor/altera_epcq.h  |  116 +
>>  drivers/mtd/spi-nor/spi-nor.c  |   86 -
>>  include/linux/mtd/spi-nor.h|3 +-
>>  7 files changed, 764 insertions(+), 6 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/mtd/altera_epcq.txt
>>  create mode 100644 drivers/mtd/spi-nor/altera_epcq.c
>>  create mode 100644 drivers/mtd/spi-nor/altera_epcq.h
>
> Please split this up into at least 2 patches, if not 3. One or more for
> the spi-nor changes, split logically; one for your new driver; and maybe
> a separate one for the binding documentation.
>
> Also, as Rafal noted, your patche is whitespace damaged, so I can't
> apply or build it.
>
> Please make sure you can apply, build, and run your patch on top of
> l2-mtd.git:
>
>   http://git.infradead.org/l2-mtd.git
>   git://git.infradead.org/l2-mtd.git
>
> ...
>> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
>> index 64a4f0e..83178b9 100644
>> --- a/drivers/mtd/spi-nor/Kconfig
>> +++ b/drivers/mtd/spi-nor/Kconfig
>> @@ -28,4 +28,16 @@ config SPI_FSL_QUADSPI
>>This enables support for the Quad SPI controller in master mode.
>>We only connect the NOR to this controller now.
>>
>> +config SPI_ALTERA_EPCQ
>> +tristate "Support Altera EPCQ/EPCS Flash chips"
>> +depends on OF
>> +help
>> +  This enables access to Altera EPCQ/EPCS flash chips, used for data
>> +  storage. See the driver source for the current list,
>> +  or to add other chips.
>> +
>> +  If you want to compile this driver as a module ( = code which can be
>> +  inserted in and removed from the running kernel whenever you want),
>> +  say M here and read .
>
> I don't think you need to explain what a module is here. Look at other
> Kconfig entries as examples, but I think you can just drop that last
> paragraph.
>
>> +
>>  endif # MTD_SPI_NOR
> ...
>> diff --git a/drivers/mtd/spi-nor/altera_epcq.c
>> b/drivers/mtd/spi-nor/altera_epcq.c
>> new file mode 100644
>> index 000..9db0d05
>> --- /dev/null
>> +++ b/drivers/mtd/spi-nor/altera_epcq.c
>> @@ -0,0 +1,507 @@
>> +/*
>> + * Copyright (C) 2014 Altera Corporation. All rights reserved
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope 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, see .
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "altera_epcq.h"
>> +
>> +static int altera_epcq_write_reg(struct spi_nor *nor, u8 opcode, u8 *val,
>> + int len, int wr_en)
>> +{
>> +return 0;
>> +}
>> +
>> +static int altera_epcq_read_reg(struct spi_nor *nor, u8 opcode, u8 *val,
>> +int len)
>> +{
>> +struct altera_epcq_flash *flash = nor->priv;
>> +struct altera_epcq *dev = flash->priv;
>> +u32 data = 0;
>> +u8 id[5];
>> +
>> +switch (opcode) {
>> +case SPINOR_OP_RDSR:
>> +data = readl(dev->csr_base + EPCQ_SR_REG);
>> +*val = (u8)data & EPCQ_SR_MASK;
>> +break;
>> +case SPINOR_OP_RDID:
>> +/* opcode id */
>> +if (dev->is_epcs) {
>> +data = readl(dev->csr_base + EPCQ_SID_REG);
>> +id[4] = EPCS_OPCODE_ID;
>> +} else {
>> +data = readl(dev->csr_base + EPCQ_RD

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

2015-03-13 Thread Brian Norris

Hi Viet,

On Wed, Feb 11, 2015 at 12:53:00PM +0800, Viet Nga Dao wrote:
> From: Viet Nga Dao 
> 
> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
> EPCS flash chips. This patch adds driver for these devices.
> 
> Signed-off-by: VIET NGA DAO 
> 
> ---
> v2:
> - Change to spi_nor structure
> - Add lock and unlock functions for spi_nor
> - Simplify the altera_epcq_lock function
> - Replace reg by compatible in device tree
> ---
>  .../devicetree/bindings/mtd/altera_epcq.txt|   45 ++
>  drivers/mtd/spi-nor/Kconfig|   12 +
>  drivers/mtd/spi-nor/Makefile   |1 +
>  drivers/mtd/spi-nor/altera_epcq.c  |  507 
> 
>  drivers/mtd/spi-nor/altera_epcq.h  |  116 +
>  drivers/mtd/spi-nor/spi-nor.c  |   86 -
>  include/linux/mtd/spi-nor.h|3 +-
>  7 files changed, 764 insertions(+), 6 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mtd/altera_epcq.txt
>  create mode 100644 drivers/mtd/spi-nor/altera_epcq.c
>  create mode 100644 drivers/mtd/spi-nor/altera_epcq.h

Please split this up into at least 2 patches, if not 3. One or more for
the spi-nor changes, split logically; one for your new driver; and maybe
a separate one for the binding documentation.

Also, as Rafal noted, your patche is whitespace damaged, so I can't
apply or build it.

Please make sure you can apply, build, and run your patch on top of
l2-mtd.git:

  http://git.infradead.org/l2-mtd.git
  git://git.infradead.org/l2-mtd.git

...
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 64a4f0e..83178b9 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -28,4 +28,16 @@ config SPI_FSL_QUADSPI
>This enables support for the Quad SPI controller in master mode.
>We only connect the NOR to this controller now.
> 
> +config SPI_ALTERA_EPCQ
> +tristate "Support Altera EPCQ/EPCS Flash chips"
> +depends on OF
> +help
> +  This enables access to Altera EPCQ/EPCS flash chips, used for data
> +  storage. See the driver source for the current list,
> +  or to add other chips.
> +
> +  If you want to compile this driver as a module ( = code which can be
> +  inserted in and removed from the running kernel whenever you want),
> +  say M here and read .

I don't think you need to explain what a module is here. Look at other
Kconfig entries as examples, but I think you can just drop that last
paragraph.

> +
>  endif # MTD_SPI_NOR
...
> diff --git a/drivers/mtd/spi-nor/altera_epcq.c
> b/drivers/mtd/spi-nor/altera_epcq.c
> new file mode 100644
> index 000..9db0d05
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/altera_epcq.c
> @@ -0,0 +1,507 @@
> +/*
> + * Copyright (C) 2014 Altera Corporation. All rights reserved
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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, see .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "altera_epcq.h"
> +
> +static int altera_epcq_write_reg(struct spi_nor *nor, u8 opcode, u8 *val,
> + int len, int wr_en)
> +{
> +return 0;
> +}
> +
> +static int altera_epcq_read_reg(struct spi_nor *nor, u8 opcode, u8 *val,
> +int len)
> +{
> +struct altera_epcq_flash *flash = nor->priv;
> +struct altera_epcq *dev = flash->priv;
> +u32 data = 0;
> +u8 id[5];
> +
> +switch (opcode) {
> +case SPINOR_OP_RDSR:
> +data = readl(dev->csr_base + EPCQ_SR_REG);
> +*val = (u8)data & EPCQ_SR_MASK;
> +break;
> +case SPINOR_OP_RDID:
> +/* opcode id */
> +if (dev->is_epcs) {
> +data = readl(dev->csr_base + EPCQ_SID_REG);
> +id[4] = EPCS_OPCODE_ID;
> +} else {
> +data = readl(dev->csr_base + EPCQ_RDID_REG);
> +id[4] = EPCQ_OPCODE_ID;
> +}

I think Rafal had some suggestions on changing how you match
IDs/devices. This, at least, doesn't look very good; you're faking a
particular ID, then matching it in spi-nor.c?

You might be better off doing something like the *-nonjedec entries in
sp

Re: FW: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

2015-03-12 Thread Viet Nga Dao

On Fri, Mar 13, 2015 at 1:45 PM, Rafał Miłecki  wrote:
> On 11 March 2015 at 09:41, Viet Nga Dao  wrote:
>> On Tue, Mar 10, 2015 at 4:09 PM, Viet Nga Dao  wrote:
>>> Ok. I will modify the code the way you suggest.
>>
>> I just realize that the opcode for RDID is handled by hardware in my
>> case, therefore i dont really need to worry about 0xab opcode.
>
> Nice :) Maybe just put a comment in code so ppl will know (in future)
> why we don't strictly follow specs.
>
>
>> Is there any other comments about the modified part i did the lock and
>> unlock in spi-nor.c?
>
> I can't, I don't know spi-nor well enough :|
>
> --
> Rafał
I have to wait for Brian then. Anway, thanks Rafal :)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: FW: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

2015-03-12 Thread Rafał Miłecki

On 11 March 2015 at 09:41, Viet Nga Dao  wrote:
> On Tue, Mar 10, 2015 at 4:09 PM, Viet Nga Dao  wrote:
>> Ok. I will modify the code the way you suggest.
>
> I just realize that the opcode for RDID is handled by hardware in my
> case, therefore i dont really need to worry about 0xab opcode.

Nice :) Maybe just put a comment in code so ppl will know (in future)
why we don't strictly follow specs.


> Is there any other comments about the modified part i did the lock and
> unlock in spi-nor.c?

I can't, I don't know spi-nor well enough :|

-- 
Rafał
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

2015-03-12 Thread Viet Nga Dao

Hi,
Could anyone please help me review the lock, unlock modification part
i did in spi-nor?
Thanks,
Viet Nga

On Mon, Feb 23, 2015 at 9:30 AM, Viet Nga Dao  wrote:
> Hi,
> It has been nearly 2 weeks since i submitted this patch.  Could you
> please help to review?
> Thanks,
>
> On Tue, Feb 17, 2015 at 9:33 AM, Viet Nga Dao  wrote:
>> Hi Brian,
>> Could you please help me to review through this 2nd version?
>>
>> On Wed, Feb 11, 2015 at 12:53 PM, Viet Nga Dao  wrote:
>>> From: Viet Nga Dao 
>>>
>>> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
>>> EPCS flash chips. This patch adds driver for these devices.
>>>
>>> Signed-off-by: VIET NGA DAO 
>>>
>>> ---
>>> v2:
>>> - Change to spi_nor structure
>>> - Add lock and unlock functions for spi_nor
>>> - Simplify the altera_epcq_lock function
>>> - Replace reg by compatible in device tree
>>> ---
>>>  .../devicetree/bindings/mtd/altera_epcq.txt|   45 ++
>>>  drivers/mtd/spi-nor/Kconfig|   12 +
>>>  drivers/mtd/spi-nor/Makefile   |1 +
>>>  drivers/mtd/spi-nor/altera_epcq.c  |  507 
>>> 
>>>  drivers/mtd/spi-nor/altera_epcq.h  |  116 +
>>>  drivers/mtd/spi-nor/spi-nor.c  |   86 -
>>>  include/linux/mtd/spi-nor.h|3 +-
>>>  7 files changed, 764 insertions(+), 6 deletions(-)
>>>  create mode 100644 Documentation/devicetree/bindings/mtd/altera_epcq.txt
>>>  create mode 100644 drivers/mtd/spi-nor/altera_epcq.c
>>>  create mode 100644 drivers/mtd/spi-nor/altera_epcq.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/altera_epcq.txt
>>> b/Documentation/devicetree/bindings/mtd/altera_epcq.txt
>>> new file mode 100644
>>> index 000..b6b5e61
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mtd/altera_epcq.txt
>>> @@ -0,0 +1,45 @@
>>> +* MTD Altera EPCQ driver
>>> +
>>> +Required properties:
>>> +- compatible: Should be "altr,epcq-1.0"
>>> +- reg: Address and length of the register set  for the device. It contains
>>> +  the information of registers in the same order as described by reg-names
>>> +- reg-names: Should contain the reg names
>>> +  "csr_base": Should contain the register configuration base address
>>> +  "data_base": Should contain the data base address
>>> +- is-epcs: boolean type.
>>> +If present, the device contains EPCS flashes.
>>> +Otherwise, it contains EPCQ flashes.
>>> +- #address-cells: Must be <1>.
>>> +- #size-cells: Must be <0>.
>>> +- flash device tree subnode, there must be a node with the following 
>>> fields:
>>> +- compatible: Should contain the flash name
>>> +- #address-cells: please refer to /mtd/partition.txt
>>> +- #size-cells: please refer to /mtd/partition.txt
>>> +For partitions inside each flash, please refer to /mtd/partition.txt
>>> +
>>> +Example:
>>> +
>>> +epcq_controller_0: epcq@0x0 {
>>> +compatible = "altr,epcq-1.0";
>>> +reg = <0x0001 0x 0x0020>,
>>> +<0x 0x 0x0200>;
>>> +reg-names = "csr_base", "data_base";
>>> +#address-cells = <1>;
>>> +#size-cells = <0>;
>>> +flash0: epcq256@0 {
>>> +compatible = "epcq256";
>>> +#address-cells = <1>;
>>> +#size-cells = <1>;
>>> +partition@0 {
>>> +/* 16 MB for raw data. */
>>> +label = "EPCQ Flash 0 raw data";
>>> +reg = <0x0 0x100>;
>>> +};
>>> +partition@100 {
>>> +/* 16 MB for jffs2 data. */
>>> +label = "EPCQ Flash 0 JFFS 2";
>>> +reg = <0x100 0x100>;
>>> +};
>>> +};
>>> +}; //end epcq@0x0 (epcq_controller_0)
>>> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
>>> index 64a4f0e..83178b9 100644
>>> --- a/drivers/mtd/spi-nor/Kconfig
>>> +++ b/drivers/mtd/spi-nor/Kconfig
>>> @@ -28,4 +28,16 @@ config SPI_FSL_QUADSPI
>>>This enables support for the Quad SPI controller in master mode.
>>>We only connect the NOR to this controller now.
>>>
>>> +config SPI_ALTERA_EPCQ
>>> +tristate "Support Altera EPCQ/EPCS Flash chips"
>>> +depends on OF
>>> +help
>>> +  This enables access to Altera EPCQ/EPCS flash chips, used for data
>>> +  storage. See the driver source for the current list,
>>> +  or to add other chips.
>>> +
>>> +  If you want to compile this driver as a module ( = code which can be
>>> +  inserted in and removed from the running kernel whenever you want),
>>> +  say M here and read .
>>> +
>>>  endif # MTD_SPI_NOR
>>> diff --git a/drivers/mtd/spi-nor/Makefile b/dr

Re: FW: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

2015-03-11 Thread Viet Nga Dao

Hi Rafal,

On Tue, Mar 10, 2015 at 4:09 PM, Viet Nga Dao  wrote:
> +
> +/* Altera EPCQ/EPCS Flashes */
> +{ "epcq16"  , EPCQ_INFO(2, 0x15, 0x1, 32,   0x100) },
> +{ "epcq32"  , EPCQ_INFO(2, 0x16, 0x1, 64,   0x100) },
> +{ "epcq64"  , EPCQ_INFO(2, 0x17, 0x1, 128,  0x100) },
> +{ "epcq128" , EPCQ_INFO(2, 0x18, 0x1, 256,  0x100) },
> +{ "epcq256" , EPCQ_INFO(2, 0x19, 0x1, 512,  0x100) },
> +{ "epcq512" , EPCQ_INFO(2, 0x20, 0x1, 1024, 0x100) },
> +{ "epcs16"  , EPCQ_INFO(1, 0x14, 0x1, 32,   0x100) },
> +{ "epcs64"  , EPCQ_INFO(1, 0x16, 0x1, 128,  0x100) },
> +{ "epcs128" , EPCQ_INFO(1, 0x18, 0x4, 256,  0x100) },
>  { },
>  };
>
> @@ -666,6 +731,14 @@ static const struct spi_device_id
> *spi_nor_read_id(struct spi_nor *nor)
>  if (info->jedec_id == jedec) {
>  if (info->ext_id == 0 || info->ext_id == ext_jedec)
>  return &spi_nor_ids[tmp];
> +
> +/* altera epcq which is non jedec device
> + * use id[4] as opcode id to differentiate
> + * epcs and epcq devices
> + */
> +} else if (info->altera_flash_opcode_id == id[4] &&
> +  info->ext_id == id[3]) {
> +return &spi_nor_ids[tmp];

 This is the part I don't like. I think it's fishy, and that this
 check may result in false positives. Looks too generic.

 Also the logic of your behavior there seems unclear to me. On the one
 hand you don't have JEDEC, so you provide chip name using DT. But in
 place above you stop trusting DT info and you try to (kind of)
 auto-detect used chip anyway.

 I guess we should finally think about some more generic way of
 passing flash info.
>>>
>>> Actually, i just want fo follow the way current spi-nor doing as much
>>> as possible. Like to read the device id and compare with info table.
>>> Like double checking from both dtb and the device id. Since the
>>> flashes i support do not have JEDEC id but only extended id. But the
>>> problem is that some of them have the same extended id, for example
>>> epcs64 and epcq32). That is why in my driver, i have to decode 1st
>>> byte of ext id to differentiate epcs and ecpq.
>>
>> I see your point and it makes sense, I just think it shouldn't be part of 
>> spi-nor. By adding chip specific code to spi-nor we'll end with hacky code 
>> and possible false chips identifications. I can really easily imagine some 
>> random chip having the same id[3] and id[4] as one of Altera flashes.
>>
>> Moreover your patch has not working support for epcs16 and epcs64.
>> They don't support 0x9f opcode (SPINOR_OP_RDID), so you would need to add 
>> support for 0xab ("Read silicon ID") to the spi-nor.
>>
>> It's the same problem I have with Broadcom's "w25q128" that doesn't support 
>> 0x9f opcode but a 0x90 with 16b reply. You may see my tiny bcm53xxspiflash.c 
>> driver:
>> http://git.openwrt.org/?p=openwrt.git;a=blob;f=target/linux/bcm53xx/files/drivers/mtd/spi-nor/bcm53xxspiflash.c;h=f192f4e59b71a2444833b5c62dd2239d28f9435d;hb=d105c51a428a72a9af42759c472df9960c496d67
>>
>> So I'm afraid that if spi-nor gets support for:
>> 1) 0xab opcode
>> 2) 0x90 opcode
>> 3) Some uncommon replies for 0x9f opcode (like Altera ones) it will quickly 
>> get hacky & buggy.
>>
>> So what about:
>> 1) Using 0x9f and 0xab in altera_epcq.c
>> 2) Finding chip name in altera_epcq.c
>> 3) Adding Altera chip names & all sizes to spi-nor.c
>> 4) Just passing a chip name to spi-nor.c
>>
>> It's something I do in bcm53xxspiflash.c. I detect w25q128 using some 
>> specific 0x90 opcode and just pass a chip name to the spi-nor.
>>
>> --
>> Rafał
>>
> Ok. I will modify the code the way you suggest.

I just realize that the opcode for RDID is handled by hardware in my
case, therefore i dont really need to worry about 0xab opcode.
Is there any other comments about the modified part i did the lock and
unlock in spi-nor.c?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: FW: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

2015-03-10 Thread Viet Nga Dao

 +
 +/* Altera EPCQ/EPCS Flashes */
 +{ "epcq16"  , EPCQ_INFO(2, 0x15, 0x1, 32,   0x100) },
 +{ "epcq32"  , EPCQ_INFO(2, 0x16, 0x1, 64,   0x100) },
 +{ "epcq64"  , EPCQ_INFO(2, 0x17, 0x1, 128,  0x100) },
 +{ "epcq128" , EPCQ_INFO(2, 0x18, 0x1, 256,  0x100) },
 +{ "epcq256" , EPCQ_INFO(2, 0x19, 0x1, 512,  0x100) },
 +{ "epcq512" , EPCQ_INFO(2, 0x20, 0x1, 1024, 0x100) },
 +{ "epcs16"  , EPCQ_INFO(1, 0x14, 0x1, 32,   0x100) },
 +{ "epcs64"  , EPCQ_INFO(1, 0x16, 0x1, 128,  0x100) },
 +{ "epcs128" , EPCQ_INFO(1, 0x18, 0x4, 256,  0x100) },
  { },
  };

 @@ -666,6 +731,14 @@ static const struct spi_device_id
 *spi_nor_read_id(struct spi_nor *nor)
  if (info->jedec_id == jedec) {
  if (info->ext_id == 0 || info->ext_id == ext_jedec)
  return &spi_nor_ids[tmp];
 +
 +/* altera epcq which is non jedec device
 + * use id[4] as opcode id to differentiate
 + * epcs and epcq devices
 + */
 +} else if (info->altera_flash_opcode_id == id[4] &&
 +  info->ext_id == id[3]) {
 +return &spi_nor_ids[tmp];
>>>
>>> This is the part I don't like. I think it's fishy, and that this
>>> check may result in false positives. Looks too generic.
>>>
>>> Also the logic of your behavior there seems unclear to me. On the one
>>> hand you don't have JEDEC, so you provide chip name using DT. But in
>>> place above you stop trusting DT info and you try to (kind of)
>>> auto-detect used chip anyway.
>>>
>>> I guess we should finally think about some more generic way of
>>> passing flash info.
>>
>> Actually, i just want fo follow the way current spi-nor doing as much
>> as possible. Like to read the device id and compare with info table.
>> Like double checking from both dtb and the device id. Since the
>> flashes i support do not have JEDEC id but only extended id. But the
>> problem is that some of them have the same extended id, for example
>> epcs64 and epcq32). That is why in my driver, i have to decode 1st
>> byte of ext id to differentiate epcs and ecpq.
>
> I see your point and it makes sense, I just think it shouldn't be part of 
> spi-nor. By adding chip specific code to spi-nor we'll end with hacky code 
> and possible false chips identifications. I can really easily imagine some 
> random chip having the same id[3] and id[4] as one of Altera flashes.
>
> Moreover your patch has not working support for epcs16 and epcs64.
> They don't support 0x9f opcode (SPINOR_OP_RDID), so you would need to add 
> support for 0xab ("Read silicon ID") to the spi-nor.
>
> It's the same problem I have with Broadcom's "w25q128" that doesn't support 
> 0x9f opcode but a 0x90 with 16b reply. You may see my tiny bcm53xxspiflash.c 
> driver:
> http://git.openwrt.org/?p=openwrt.git;a=blob;f=target/linux/bcm53xx/files/drivers/mtd/spi-nor/bcm53xxspiflash.c;h=f192f4e59b71a2444833b5c62dd2239d28f9435d;hb=d105c51a428a72a9af42759c472df9960c496d67
>
> So I'm afraid that if spi-nor gets support for:
> 1) 0xab opcode
> 2) 0x90 opcode
> 3) Some uncommon replies for 0x9f opcode (like Altera ones) it will quickly 
> get hacky & buggy.
>
> So what about:
> 1) Using 0x9f and 0xab in altera_epcq.c
> 2) Finding chip name in altera_epcq.c
> 3) Adding Altera chip names & all sizes to spi-nor.c
> 4) Just passing a chip name to spi-nor.c
>
> It's something I do in bcm53xxspiflash.c. I detect w25q128 using some 
> specific 0x90 opcode and just pass a chip name to the spi-nor.
>
> --
> Rafał
>
Ok. I will modify the code the way you suggest.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

2015-03-09 Thread Rafał Miłecki

On 10 March 2015 at 07:11, Viet Nga Dao  wrote:
> On Mon, Mar 9, 2015 at 2:31 PM, Rafał Miłecki  wrote:
>> On 11 February 2015 at 05:53, Viet Nga Dao  wrote:
>>>  /* NOTE: double check command sets and memory organization when you add
>>>   * more nor chips.  This current list focusses on newer chips, which
>>>   * have been converging on command sets which including JEDEC ID.
>>> @@ -637,6 +691,17 @@ static const struct spi_device_id spi_nor_ids[] = {
>>>  { "cat25c09", CAT25_INFO( 128, 8, 32, 2, SPI_NOR_NO_ERASE |
>>> SPI_NOR_NO_FR) },
>>>  { "cat25c17", CAT25_INFO( 256, 8, 32, 2, SPI_NOR_NO_ERASE |
>>> SPI_NOR_NO_FR) },
>>>  { "cat25128", CAT25_INFO(2048, 8, 64, 2, SPI_NOR_NO_ERASE |
>>> SPI_NOR_NO_FR) },
>>> +
>>> +/* Altera EPCQ/EPCS Flashes */
>>> +{ "epcq16"  , EPCQ_INFO(2, 0x15, 0x1, 32,   0x100) },
>>> +{ "epcq32"  , EPCQ_INFO(2, 0x16, 0x1, 64,   0x100) },
>>> +{ "epcq64"  , EPCQ_INFO(2, 0x17, 0x1, 128,  0x100) },
>>> +{ "epcq128" , EPCQ_INFO(2, 0x18, 0x1, 256,  0x100) },
>>> +{ "epcq256" , EPCQ_INFO(2, 0x19, 0x1, 512,  0x100) },
>>> +{ "epcq512" , EPCQ_INFO(2, 0x20, 0x1, 1024, 0x100) },
>>> +{ "epcs16"  , EPCQ_INFO(1, 0x14, 0x1, 32,   0x100) },
>>> +{ "epcs64"  , EPCQ_INFO(1, 0x16, 0x1, 128,  0x100) },
>>> +{ "epcs128" , EPCQ_INFO(1, 0x18, 0x4, 256,  0x100) },
>>>  { },
>>>  };
>>>
>>> @@ -666,6 +731,14 @@ static const struct spi_device_id
>>> *spi_nor_read_id(struct spi_nor *nor)
>>>  if (info->jedec_id == jedec) {
>>>  if (info->ext_id == 0 || info->ext_id == ext_jedec)
>>>  return &spi_nor_ids[tmp];
>>> +
>>> +/* altera epcq which is non jedec device
>>> + * use id[4] as opcode id to differentiate
>>> + * epcs and epcq devices
>>> + */
>>> +} else if (info->altera_flash_opcode_id == id[4] &&
>>> +  info->ext_id == id[3]) {
>>> +return &spi_nor_ids[tmp];
>>
>> This is the part I don't like. I think it's fishy, and that this check
>> may result in false positives. Looks too generic.
>>
>> Also the logic of your behavior there seems unclear to me. On the one
>> hand you don't have JEDEC, so you provide chip name using DT. But in
>> place above you stop trusting DT info and you try to (kind of)
>> auto-detect used chip anyway.
>>
>> I guess we should finally think about some more generic way of passing
>> flash info.
>
> Actually, i just want fo follow the way current spi-nor doing as much
> as possible. Like to read the device id and compare with info table.
> Like double checking from both dtb and the device id. Since the
> flashes i support do not have JEDEC id but only extended id. But the
> problem is that some of them have the same extended id, for example
> epcs64 and epcq32). That is why in my driver, i have to decode 1st
> byte of ext id to differentiate epcs and ecpq.

I see your point and it makes sense, I just think it shouldn't be part
of spi-nor. By adding chip specific code to spi-nor we'll end with
hacky code and possible false chips identifications. I can really
easily imagine some random chip having the same id[3] and id[4] as one
of Altera flashes.

Moreover your patch has not working support for epcs16 and epcs64.
They don't support 0x9f opcode (SPINOR_OP_RDID), so you would need to
add support for 0xab ("Read silicon ID") to the spi-nor.

It's the same problem I have with Broadcom's "w25q128" that doesn't
support 0x9f opcode but a 0x90 with 16b reply. You may see my tiny
bcm53xxspiflash.c driver:
http://git.openwrt.org/?p=openwrt.git;a=blob;f=target/linux/bcm53xx/files/drivers/mtd/spi-nor/bcm53xxspiflash.c;h=f192f4e59b71a2444833b5c62dd2239d28f9435d;hb=d105c51a428a72a9af42759c472df9960c496d67

So I'm afraid that if spi-nor gets support for:
1) 0xab opcode
2) 0x90 opcode
3) Some uncommon replies for 0x9f opcode (like Altera ones)
it will quickly get hacky & buggy.

So what about:
1) Using 0x9f and 0xab in altera_epcq.c
2) Finding chip name in altera_epcq.c
3) Adding Altera chip names & all sizes to spi-nor.c
4) Just passing a chip name to spi-nor.c

It's something I do in bcm53xxspiflash.c. I detect w25q128 using some
specific 0x90 opcode and just pass a chip name to the spi-nor.

-- 
Rafał
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

2015-03-09 Thread Viet Nga Dao

Hi Rafal,
Thanks for your review.

On Mon, Mar 9, 2015 at 2:31 PM, Rafał Miłecki  wrote:
> Hi Viet,
>
> I'm not too active in mtd subsystem, so I didn't notice your patch
> earlier. However I would like to share few comments.
>
> On 11 February 2015 at 05:53, Viet Nga Dao  wrote:
>> From: Viet Nga Dao 
>>
>> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
>> EPCS flash chips. This patch adds driver for these devices.
>
> First of all, your whole patch is white-space damaged. It can't be
> applied, seems all tabs were replaced with spaces. It's what I got in
> my GMail and what was also received by patchwork, see
> https://patchwork.ozlabs.org/patch/438684/
>
> You'll need to resend using some smarter e-mail client, you may e.g.
> try git send-email.
>
>
>> +#define EPCQ_INFO(_opcode_id, _ext_id, _sector_size, _n_sectors, 
>> _page_size) \
>> +((kernel_ulong_t)&(struct flash_info) {\
>> +.altera_flash_opcode_id = (_opcode_id),\
>> +.ext_id = (_ext_id),\
>> +.sector_size = (_sector_size),\
>> +.n_sectors = (_n_sectors),\
>> +.page_size = (_page_size),\
>> +})
>
> Starting with kernel 3.19 we don't have ext_id in struct anymore, see:
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=d928a259385dc6fca3956b7775c588f21c0b50fc
>
>

Noted. I will make the modification accordingly.

>>  /* NOTE: double check command sets and memory organization when you add
>>   * more nor chips.  This current list focusses on newer chips, which
>>   * have been converging on command sets which including JEDEC ID.
>> @@ -637,6 +691,17 @@ static const struct spi_device_id spi_nor_ids[] = {
>>  { "cat25c09", CAT25_INFO( 128, 8, 32, 2, SPI_NOR_NO_ERASE |
>> SPI_NOR_NO_FR) },
>>  { "cat25c17", CAT25_INFO( 256, 8, 32, 2, SPI_NOR_NO_ERASE |
>> SPI_NOR_NO_FR) },
>>  { "cat25128", CAT25_INFO(2048, 8, 64, 2, SPI_NOR_NO_ERASE |
>> SPI_NOR_NO_FR) },
>> +
>> +/* Altera EPCQ/EPCS Flashes */
>> +{ "epcq16"  , EPCQ_INFO(2, 0x15, 0x1, 32,   0x100) },
>> +{ "epcq32"  , EPCQ_INFO(2, 0x16, 0x1, 64,   0x100) },
>> +{ "epcq64"  , EPCQ_INFO(2, 0x17, 0x1, 128,  0x100) },
>> +{ "epcq128" , EPCQ_INFO(2, 0x18, 0x1, 256,  0x100) },
>> +{ "epcq256" , EPCQ_INFO(2, 0x19, 0x1, 512,  0x100) },
>> +{ "epcq512" , EPCQ_INFO(2, 0x20, 0x1, 1024, 0x100) },
>> +{ "epcs16"  , EPCQ_INFO(1, 0x14, 0x1, 32,   0x100) },
>> +{ "epcs64"  , EPCQ_INFO(1, 0x16, 0x1, 128,  0x100) },
>> +{ "epcs128" , EPCQ_INFO(1, 0x18, 0x4, 256,  0x100) },
>>  { },
>>  };
>>
>> @@ -666,6 +731,14 @@ static const struct spi_device_id
>> *spi_nor_read_id(struct spi_nor *nor)
>>  if (info->jedec_id == jedec) {
>>  if (info->ext_id == 0 || info->ext_id == ext_jedec)
>>  return &spi_nor_ids[tmp];
>> +
>> +/* altera epcq which is non jedec device
>> + * use id[4] as opcode id to differentiate
>> + * epcs and epcq devices
>> + */
>> +} else if (info->altera_flash_opcode_id == id[4] &&
>> +  info->ext_id == id[3]) {
>> +return &spi_nor_ids[tmp];
>
> This is the part I don't like. I think it's fishy, and that this check
> may result in false positives. Looks too generic.
>
> Also the logic of your behavior there seems unclear to me. On the one
> hand you don't have JEDEC, so you provide chip name using DT. But in
> place above you stop trusting DT info and you try to (kind of)
> auto-detect used chip anyway.
>
> I guess we should finally think about some more generic way of passing
> flash info.

Actually, i just want fo follow the way current spi-nor doing as much
as possible. Like to read the device id and compare with info table.
Like double checking from both dtb and the device id. Since the
flashes i support do not have JEDEC id but only extended id. But the
problem is that some of them have the same extended id, for example
epcs64 and epcq32). That is why in my driver, i have to decode 1st
byte of ext id to differentiate epcs and ecpq.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

2015-03-08 Thread Rafał Miłecki

Hi Viet,

I'm not too active in mtd subsystem, so I didn't notice your patch
earlier. However I would like to share few comments.

On 11 February 2015 at 05:53, Viet Nga Dao  wrote:
> From: Viet Nga Dao 
>
> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
> EPCS flash chips. This patch adds driver for these devices.

First of all, your whole patch is white-space damaged. It can't be
applied, seems all tabs were replaced with spaces. It's what I got in
my GMail and what was also received by patchwork, see
https://patchwork.ozlabs.org/patch/438684/

You'll need to resend using some smarter e-mail client, you may e.g.
try git send-email.


> +#define EPCQ_INFO(_opcode_id, _ext_id, _sector_size, _n_sectors, _page_size) 
> \
> +((kernel_ulong_t)&(struct flash_info) {\
> +.altera_flash_opcode_id = (_opcode_id),\
> +.ext_id = (_ext_id),\
> +.sector_size = (_sector_size),\
> +.n_sectors = (_n_sectors),\
> +.page_size = (_page_size),\
> +})

Starting with kernel 3.19 we don't have ext_id in struct anymore, see:
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=d928a259385dc6fca3956b7775c588f21c0b50fc


>  /* NOTE: double check command sets and memory organization when you add
>   * more nor chips.  This current list focusses on newer chips, which
>   * have been converging on command sets which including JEDEC ID.
> @@ -637,6 +691,17 @@ static const struct spi_device_id spi_nor_ids[] = {
>  { "cat25c09", CAT25_INFO( 128, 8, 32, 2, SPI_NOR_NO_ERASE |
> SPI_NOR_NO_FR) },
>  { "cat25c17", CAT25_INFO( 256, 8, 32, 2, SPI_NOR_NO_ERASE |
> SPI_NOR_NO_FR) },
>  { "cat25128", CAT25_INFO(2048, 8, 64, 2, SPI_NOR_NO_ERASE |
> SPI_NOR_NO_FR) },
> +
> +/* Altera EPCQ/EPCS Flashes */
> +{ "epcq16"  , EPCQ_INFO(2, 0x15, 0x1, 32,   0x100) },
> +{ "epcq32"  , EPCQ_INFO(2, 0x16, 0x1, 64,   0x100) },
> +{ "epcq64"  , EPCQ_INFO(2, 0x17, 0x1, 128,  0x100) },
> +{ "epcq128" , EPCQ_INFO(2, 0x18, 0x1, 256,  0x100) },
> +{ "epcq256" , EPCQ_INFO(2, 0x19, 0x1, 512,  0x100) },
> +{ "epcq512" , EPCQ_INFO(2, 0x20, 0x1, 1024, 0x100) },
> +{ "epcs16"  , EPCQ_INFO(1, 0x14, 0x1, 32,   0x100) },
> +{ "epcs64"  , EPCQ_INFO(1, 0x16, 0x1, 128,  0x100) },
> +{ "epcs128" , EPCQ_INFO(1, 0x18, 0x4, 256,  0x100) },
>  { },
>  };
>
> @@ -666,6 +731,14 @@ static const struct spi_device_id
> *spi_nor_read_id(struct spi_nor *nor)
>  if (info->jedec_id == jedec) {
>  if (info->ext_id == 0 || info->ext_id == ext_jedec)
>  return &spi_nor_ids[tmp];
> +
> +/* altera epcq which is non jedec device
> + * use id[4] as opcode id to differentiate
> + * epcs and epcq devices
> + */
> +} else if (info->altera_flash_opcode_id == id[4] &&
> +  info->ext_id == id[3]) {
> +return &spi_nor_ids[tmp];

This is the part I don't like. I think it's fishy, and that this check
may result in false positives. Looks too generic.

Also the logic of your behavior there seems unclear to me. On the one
hand you don't have JEDEC, so you provide chip name using DT. But in
place above you stop trusting DT info and you try to (kind of)
auto-detect used chip anyway.

I guess we should finally think about some more generic way of passing
flash info.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

2015-03-08 Thread Viet Nga Dao

Hi Brian,
I understand that you are busy but is it possible if you could spend
some time to help me review my patch? My work progress really depends
on your review.
Thanks,
Viet Nga

On Tue, Feb 24, 2015 at 11:59 AM, Brian Norris
 wrote:
> On Mon, Feb 23, 2015 at 09:30:09AM +0800, Viet Nga Dao wrote:
>> Hi,
>> It has been nearly 2 weeks since i submitted this patch.  Could you
>> please help to review?
>
> Those two weeks were during the merge window, so I wasn't queueing
> anything up. And there are things that have waited longer, anyway. My
> time is unfortunately finite.
>
> I'll get to your patch eventually.
>
>> On Tue, Feb 17, 2015 at 9:33 AM, Viet Nga Dao  wrote:
>> > Hi Brian,
>> > Could you please help me to review through this 2nd version?
>> >
>> > On Wed, Feb 11, 2015 at 12:53 PM, Viet Nga Dao  wrote:
>> >> From: Viet Nga Dao 
>> >>
>> >> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ 
>> >> and
>> >> EPCS flash chips. This patch adds driver for these devices.
>> >>
>> >> Signed-off-by: VIET NGA DAO 
>> >>
>> >> ---
>> >> v2:
>> >> - Change to spi_nor structure
>> >> - Add lock and unlock functions for spi_nor
>> >> - Simplify the altera_epcq_lock function
>> >> - Replace reg by compatible in device tree
> ...
>
> Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

2015-02-23 Thread Brian Norris

On Mon, Feb 23, 2015 at 09:30:09AM +0800, Viet Nga Dao wrote:
> Hi,
> It has been nearly 2 weeks since i submitted this patch.  Could you
> please help to review?

Those two weeks were during the merge window, so I wasn't queueing
anything up. And there are things that have waited longer, anyway. My
time is unfortunately finite.

I'll get to your patch eventually.

> On Tue, Feb 17, 2015 at 9:33 AM, Viet Nga Dao  wrote:
> > Hi Brian,
> > Could you please help me to review through this 2nd version?
> >
> > On Wed, Feb 11, 2015 at 12:53 PM, Viet Nga Dao  wrote:
> >> From: Viet Nga Dao 
> >>
> >> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
> >> EPCS flash chips. This patch adds driver for these devices.
> >>
> >> Signed-off-by: VIET NGA DAO 
> >>
> >> ---
> >> v2:
> >> - Change to spi_nor structure
> >> - Add lock and unlock functions for spi_nor
> >> - Simplify the altera_epcq_lock function
> >> - Replace reg by compatible in device tree
...

Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

2015-02-22 Thread Viet Nga Dao

Hi,
It has been nearly 2 weeks since i submitted this patch.  Could you
please help to review?
Thanks,

On Tue, Feb 17, 2015 at 9:33 AM, Viet Nga Dao  wrote:
> Hi Brian,
> Could you please help me to review through this 2nd version?
>
> On Wed, Feb 11, 2015 at 12:53 PM, Viet Nga Dao  wrote:
>> From: Viet Nga Dao 
>>
>> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
>> EPCS flash chips. This patch adds driver for these devices.
>>
>> Signed-off-by: VIET NGA DAO 
>>
>> ---
>> v2:
>> - Change to spi_nor structure
>> - Add lock and unlock functions for spi_nor
>> - Simplify the altera_epcq_lock function
>> - Replace reg by compatible in device tree
>> ---
>>  .../devicetree/bindings/mtd/altera_epcq.txt|   45 ++
>>  drivers/mtd/spi-nor/Kconfig|   12 +
>>  drivers/mtd/spi-nor/Makefile   |1 +
>>  drivers/mtd/spi-nor/altera_epcq.c  |  507 
>> 
>>  drivers/mtd/spi-nor/altera_epcq.h  |  116 +
>>  drivers/mtd/spi-nor/spi-nor.c  |   86 -
>>  include/linux/mtd/spi-nor.h|3 +-
>>  7 files changed, 764 insertions(+), 6 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/mtd/altera_epcq.txt
>>  create mode 100644 drivers/mtd/spi-nor/altera_epcq.c
>>  create mode 100644 drivers/mtd/spi-nor/altera_epcq.h
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/altera_epcq.txt
>> b/Documentation/devicetree/bindings/mtd/altera_epcq.txt
>> new file mode 100644
>> index 000..b6b5e61
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/altera_epcq.txt
>> @@ -0,0 +1,45 @@
>> +* MTD Altera EPCQ driver
>> +
>> +Required properties:
>> +- compatible: Should be "altr,epcq-1.0"
>> +- reg: Address and length of the register set  for the device. It contains
>> +  the information of registers in the same order as described by reg-names
>> +- reg-names: Should contain the reg names
>> +  "csr_base": Should contain the register configuration base address
>> +  "data_base": Should contain the data base address
>> +- is-epcs: boolean type.
>> +If present, the device contains EPCS flashes.
>> +Otherwise, it contains EPCQ flashes.
>> +- #address-cells: Must be <1>.
>> +- #size-cells: Must be <0>.
>> +- flash device tree subnode, there must be a node with the following fields:
>> +- compatible: Should contain the flash name
>> +- #address-cells: please refer to /mtd/partition.txt
>> +- #size-cells: please refer to /mtd/partition.txt
>> +For partitions inside each flash, please refer to /mtd/partition.txt
>> +
>> +Example:
>> +
>> +epcq_controller_0: epcq@0x0 {
>> +compatible = "altr,epcq-1.0";
>> +reg = <0x0001 0x 0x0020>,
>> +<0x 0x 0x0200>;
>> +reg-names = "csr_base", "data_base";
>> +#address-cells = <1>;
>> +#size-cells = <0>;
>> +flash0: epcq256@0 {
>> +compatible = "epcq256";
>> +#address-cells = <1>;
>> +#size-cells = <1>;
>> +partition@0 {
>> +/* 16 MB for raw data. */
>> +label = "EPCQ Flash 0 raw data";
>> +reg = <0x0 0x100>;
>> +};
>> +partition@100 {
>> +/* 16 MB for jffs2 data. */
>> +label = "EPCQ Flash 0 JFFS 2";
>> +reg = <0x100 0x100>;
>> +};
>> +};
>> +}; //end epcq@0x0 (epcq_controller_0)
>> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
>> index 64a4f0e..83178b9 100644
>> --- a/drivers/mtd/spi-nor/Kconfig
>> +++ b/drivers/mtd/spi-nor/Kconfig
>> @@ -28,4 +28,16 @@ config SPI_FSL_QUADSPI
>>This enables support for the Quad SPI controller in master mode.
>>We only connect the NOR to this controller now.
>>
>> +config SPI_ALTERA_EPCQ
>> +tristate "Support Altera EPCQ/EPCS Flash chips"
>> +depends on OF
>> +help
>> +  This enables access to Altera EPCQ/EPCS flash chips, used for data
>> +  storage. See the driver source for the current list,
>> +  or to add other chips.
>> +
>> +  If you want to compile this driver as a module ( = code which can be
>> +  inserted in and removed from the running kernel whenever you want),
>> +  say M here and read .
>> +
>>  endif # MTD_SPI_NOR
>> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
>> index 6a7ce14..ff9437b 100644
>> --- a/drivers/mtd/spi-nor/Makefile
>> +++ b/drivers/mtd/spi-nor/Makefile
>> @@ -1,2 +1,3 @@
>>  obj-$(CONFIG_MTD_SPI_NOR)+= spi-nor.o
>>  obj-$(CONFIG_SPI_FSL_QUADSPI)+= fsl-quadspi.o
>> +obj-$(CONFIG_SPI_ALTERA_EP

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

2015-02-16 Thread Viet Nga Dao

Hi Brian,
Could you please help me to review through this 2nd version?

On Wed, Feb 11, 2015 at 12:53 PM, Viet Nga Dao  wrote:
> From: Viet Nga Dao 
>
> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
> EPCS flash chips. This patch adds driver for these devices.
>
> Signed-off-by: VIET NGA DAO 
>
> ---
> v2:
> - Change to spi_nor structure
> - Add lock and unlock functions for spi_nor
> - Simplify the altera_epcq_lock function
> - Replace reg by compatible in device tree
> ---
>  .../devicetree/bindings/mtd/altera_epcq.txt|   45 ++
>  drivers/mtd/spi-nor/Kconfig|   12 +
>  drivers/mtd/spi-nor/Makefile   |1 +
>  drivers/mtd/spi-nor/altera_epcq.c  |  507 
> 
>  drivers/mtd/spi-nor/altera_epcq.h  |  116 +
>  drivers/mtd/spi-nor/spi-nor.c  |   86 -
>  include/linux/mtd/spi-nor.h|3 +-
>  7 files changed, 764 insertions(+), 6 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mtd/altera_epcq.txt
>  create mode 100644 drivers/mtd/spi-nor/altera_epcq.c
>  create mode 100644 drivers/mtd/spi-nor/altera_epcq.h
>
> diff --git a/Documentation/devicetree/bindings/mtd/altera_epcq.txt
> b/Documentation/devicetree/bindings/mtd/altera_epcq.txt
> new file mode 100644
> index 000..b6b5e61
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/altera_epcq.txt
> @@ -0,0 +1,45 @@
> +* MTD Altera EPCQ driver
> +
> +Required properties:
> +- compatible: Should be "altr,epcq-1.0"
> +- reg: Address and length of the register set  for the device. It contains
> +  the information of registers in the same order as described by reg-names
> +- reg-names: Should contain the reg names
> +  "csr_base": Should contain the register configuration base address
> +  "data_base": Should contain the data base address
> +- is-epcs: boolean type.
> +If present, the device contains EPCS flashes.
> +Otherwise, it contains EPCQ flashes.
> +- #address-cells: Must be <1>.
> +- #size-cells: Must be <0>.
> +- flash device tree subnode, there must be a node with the following fields:
> +- compatible: Should contain the flash name
> +- #address-cells: please refer to /mtd/partition.txt
> +- #size-cells: please refer to /mtd/partition.txt
> +For partitions inside each flash, please refer to /mtd/partition.txt
> +
> +Example:
> +
> +epcq_controller_0: epcq@0x0 {
> +compatible = "altr,epcq-1.0";
> +reg = <0x0001 0x 0x0020>,
> +<0x 0x 0x0200>;
> +reg-names = "csr_base", "data_base";
> +#address-cells = <1>;
> +#size-cells = <0>;
> +flash0: epcq256@0 {
> +compatible = "epcq256";
> +#address-cells = <1>;
> +#size-cells = <1>;
> +partition@0 {
> +/* 16 MB for raw data. */
> +label = "EPCQ Flash 0 raw data";
> +reg = <0x0 0x100>;
> +};
> +partition@100 {
> +/* 16 MB for jffs2 data. */
> +label = "EPCQ Flash 0 JFFS 2";
> +reg = <0x100 0x100>;
> +};
> +};
> +}; //end epcq@0x0 (epcq_controller_0)
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 64a4f0e..83178b9 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -28,4 +28,16 @@ config SPI_FSL_QUADSPI
>This enables support for the Quad SPI controller in master mode.
>We only connect the NOR to this controller now.
>
> +config SPI_ALTERA_EPCQ
> +tristate "Support Altera EPCQ/EPCS Flash chips"
> +depends on OF
> +help
> +  This enables access to Altera EPCQ/EPCS flash chips, used for data
> +  storage. See the driver source for the current list,
> +  or to add other chips.
> +
> +  If you want to compile this driver as a module ( = code which can be
> +  inserted in and removed from the running kernel whenever you want),
> +  say M here and read .
> +
>  endif # MTD_SPI_NOR
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index 6a7ce14..ff9437b 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -1,2 +1,3 @@
>  obj-$(CONFIG_MTD_SPI_NOR)+= spi-nor.o
>  obj-$(CONFIG_SPI_FSL_QUADSPI)+= fsl-quadspi.o
> +obj-$(CONFIG_SPI_ALTERA_EPCQ)+= altera_epcq.o
> diff --git a/drivers/mtd/spi-nor/altera_epcq.c
> b/drivers/mtd/spi-nor/altera_epcq.c
> new file mode 100644
> index 000..9db0d05
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/altera_epcq.c
> @@ -0,0 +1,507 @@
> +/*
> + * Copyright (C) 2014 A

[PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

2015-02-10 Thread Viet Nga Dao

From: Viet Nga Dao 

Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
EPCS flash chips. This patch adds driver for these devices.

Signed-off-by: VIET NGA DAO 

---
v2:
- Change to spi_nor structure
- Add lock and unlock functions for spi_nor
- Simplify the altera_epcq_lock function
- Replace reg by compatible in device tree
---
 .../devicetree/bindings/mtd/altera_epcq.txt|   45 ++
 drivers/mtd/spi-nor/Kconfig|   12 +
 drivers/mtd/spi-nor/Makefile   |1 +
 drivers/mtd/spi-nor/altera_epcq.c  |  507 
 drivers/mtd/spi-nor/altera_epcq.h  |  116 +
 drivers/mtd/spi-nor/spi-nor.c  |   86 -
 include/linux/mtd/spi-nor.h|3 +-
 7 files changed, 764 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/altera_epcq.txt
 create mode 100644 drivers/mtd/spi-nor/altera_epcq.c
 create mode 100644 drivers/mtd/spi-nor/altera_epcq.h

diff --git a/Documentation/devicetree/bindings/mtd/altera_epcq.txt
b/Documentation/devicetree/bindings/mtd/altera_epcq.txt
new file mode 100644
index 000..b6b5e61
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/altera_epcq.txt
@@ -0,0 +1,45 @@
+* MTD Altera EPCQ driver
+
+Required properties:
+- compatible: Should be "altr,epcq-1.0"
+- reg: Address and length of the register set  for the device. It contains
+  the information of registers in the same order as described by reg-names
+- reg-names: Should contain the reg names
+  "csr_base": Should contain the register configuration base address
+  "data_base": Should contain the data base address
+- is-epcs: boolean type.
+If present, the device contains EPCS flashes.
+Otherwise, it contains EPCQ flashes.
+- #address-cells: Must be <1>.
+- #size-cells: Must be <0>.
+- flash device tree subnode, there must be a node with the following fields:
+- compatible: Should contain the flash name
+- #address-cells: please refer to /mtd/partition.txt
+- #size-cells: please refer to /mtd/partition.txt
+For partitions inside each flash, please refer to /mtd/partition.txt
+
+Example:
+
+epcq_controller_0: epcq@0x0 {
+compatible = "altr,epcq-1.0";
+reg = <0x0001 0x 0x0020>,
+<0x 0x 0x0200>;
+reg-names = "csr_base", "data_base";
+#address-cells = <1>;
+#size-cells = <0>;
+flash0: epcq256@0 {
+compatible = "epcq256";
+#address-cells = <1>;
+#size-cells = <1>;
+partition@0 {
+/* 16 MB for raw data. */
+label = "EPCQ Flash 0 raw data";
+reg = <0x0 0x100>;
+};
+partition@100 {
+/* 16 MB for jffs2 data. */
+label = "EPCQ Flash 0 JFFS 2";
+reg = <0x100 0x100>;
+};
+};
+}; //end epcq@0x0 (epcq_controller_0)
diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 64a4f0e..83178b9 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -28,4 +28,16 @@ config SPI_FSL_QUADSPI
   This enables support for the Quad SPI controller in master mode.
   We only connect the NOR to this controller now.

+config SPI_ALTERA_EPCQ
+tristate "Support Altera EPCQ/EPCS Flash chips"
+depends on OF
+help
+  This enables access to Altera EPCQ/EPCS flash chips, used for data
+  storage. See the driver source for the current list,
+  or to add other chips.
+
+  If you want to compile this driver as a module ( = code which can be
+  inserted in and removed from the running kernel whenever you want),
+  say M here and read .
+
 endif # MTD_SPI_NOR
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 6a7ce14..ff9437b 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_MTD_SPI_NOR)+= spi-nor.o
 obj-$(CONFIG_SPI_FSL_QUADSPI)+= fsl-quadspi.o
+obj-$(CONFIG_SPI_ALTERA_EPCQ)+= altera_epcq.o
diff --git a/drivers/mtd/spi-nor/altera_epcq.c
b/drivers/mtd/spi-nor/altera_epcq.c
new file mode 100644
index 000..9db0d05
--- /dev/null
+++ b/drivers/mtd/spi-nor/altera_epcq.c
@@ -0,0 +1,507 @@
+/*
+ * Copyright (C) 2014 Altera Corporation. All rights reserved
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without ev

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

Re: FW: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

Re: FW: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

Re: FW: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

Re: FW: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

[PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver

16 matches

Site Navigation

Mail list logo

Footer information