Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1210261 > unrolled thread

Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver

Started byMarek Vasut <marex@denx.de>
First post2015-08-20 10:10 +0200
Last post2015-08-21 00:10 +0200
Articles 13 — 5 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver Marek Vasut <marex@denx.de> - 2015-08-20 10:10 +0200
    Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver Viet Nga Dao <vndao@altera.com> - 2015-08-20 10:20 +0200
      Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver Marek Vasut <marex@denx.de> - 2015-08-20 11:10 +0200
    Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver Nga Chi <ngachi86@gmail.com> - 2015-08-20 10:20 +0200
      Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver Marek Vasut <marex@denx.de> - 2015-08-20 11:10 +0200
        Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver Viet Nga Dao <vndao@altera.com> - 2015-08-20 11:20 +0200
          Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver Marek Vasut <marex@denx.de> - 2015-08-20 11:50 +0200
            Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver Brian Norris <computersforpeace@gmail.com> - 2015-08-20 22:40 +0200
          Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver Brian Norris <computersforpeace@gmail.com> - 2015-08-20 22:40 +0200
            Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver Viet Nga Dao <vndao@altera.com> - 2015-08-21 03:40 +0200
    Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver Alexander Stein <alexander.stein@systec-electronic.com> - 2015-08-20 12:10 +0200
      Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver Brian Norris <computersforpeace@gmail.com> - 2015-08-20 22:20 +0200
        Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver Marek Vasut <marex@denx.de> - 2015-08-21 00:10 +0200

#1210261 — Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver

FromMarek Vasut <marex@denx.de>
Date2015-08-20 10:10 +0200
SubjectRe: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver
Message-ID<pZt58-7r8-25@gated-at.bofh.it>
On Thursday, August 20, 2015 at 08:55:05 AM, vndao@altera.com wrote:
> From: VIET NGA DAO <vndao@altera.com>
> 
> Altera Quad SPI Controller is a soft IP which enables access to
> Altera EPCS and EPCQ flash chips. This patch adds driver
> for these devices.
> 
> Signed-off-by: VIET NGA DAO <vndao@altera.com>
> 
> ---
> v5:
> - Remove Micron support
> - Add multiple flashes probe failure handle
> 
> v4:
> - Add more flash devices support ( EPCQL and Micron)
> - Remove redundant messages
> - Change EPCQ_OPCODE_ID to NON_EPCS_OPCODE_ID
> - Replace get_flash_name to altera_quadspi_scan
> - Remove clk related parts
> - Remove altera_quadspi_plat
> - Change device tree reg name and remove opcode-id
> 
> v3:
> - Change altera_epcq driver name to altera_quadspi for more generic name
> - Implement flash name searching in altera_quadspi.c instead of spi-nor
> - Edit the altra quadspi info table in spi-nor
> - Remove wait_til_ready in all read,write, erase, lock, unlock functions
> - Merge .h and .c into 1 file
> 
> 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-quadspi.txt     |   45 ++
>  drivers/mtd/spi-nor/Kconfig                        |    8 +
>  drivers/mtd/spi-nor/Makefile                       |    1 +
>  drivers/mtd/spi-nor/altera-quadspi.c               |  557
> ++++++++++++++++++++ drivers/mtd/spi-nor/spi-nor.c                      | 
>  18 +
>  5 files changed, 629 insertions(+), 0 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/mtd/altera-quadspi.txt create mode
> 100644 drivers/mtd/spi-nor/altera-quadspi.c
> 
> diff --git a/Documentation/devicetree/bindings/mtd/altera-quadspi.txt
> b/Documentation/devicetree/bindings/mtd/altera-quadspi.txt new file mode
> 100644
> index 0000000..e1bcf18
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/altera-quadspi.txt
> @@ -0,0 +1,45 @@
> +* MTD Altera QUADSPI driver
> +
> +Required properties:
> +- compatible: Should be "altr,quadspi-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
> +  "avl_csr": Should contain the register configuration base address
> +  "avl_mem": Should contain the data base address
> +- #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:
> +	  1. EPCS:   epcs16, epcs64, epcs128
> +	  2. EPCQ:   epcq16, epcq32, epcq64, epcq128, epcq256, epcq512, epcq1024
> +	  3. EPCQ-L: epcql256, epcql512, epcql1024
> +	- #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:
> +
> +			quadspi_controller_0: quadspi@0x180014a0 {
> +				compatible = "altr,quadspi-1.0";
> +				reg = <0x180014a0 0x00000020>,
> +				      <0x14000000 0x04000000>;
> +				reg-names = "avl_csr", "avl_mem";
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				flash0: epcq256@0 {
> +					compatible = "altr,epcq256";
> +					#address-cells = <1>;
> +					#size-cells = <1>;
> +					partition@0 {
> +						/* 16 MB for raw data. */
> +						label = "EPCQ Flash 0 raw data";
> +						reg = <0x0 0x1000000>;
> +					};
> +					partition@1000000 {
> +						/* 16 MB for jffs2 data. */
> +						label = "EPCQ Flash 0 JFFS 2";
> +						reg = <0x1000000 0x1000000>;
> +					};

IIRC, encoding partitions into OF is deprecated (and it shouldn't be
part of the example anyway, so please remove this bit).

> +				};
> +			}; //end quadspi@0x180014a0 (quadspi_controller_0)

Remove this incorrect comment.


[...]

> +static struct flash_device flash_devices[] = {
> +	FLASH_ID("epcs16",              EPCS_OPCODE_ID,     0x14),
> +	FLASH_ID("epcs64",              EPCS_OPCODE_ID,     0x16),
> +	FLASH_ID("epcs128",             EPCS_OPCODE_ID,     0x18),
> +
> +	FLASH_ID("epcq16",              NON_EPCS_OPCODE_ID, 0x15),
> +	FLASH_ID("epcq32",              NON_EPCS_OPCODE_ID, 0x16),
> +	FLASH_ID("epcq64",              NON_EPCS_OPCODE_ID, 0x17),
> +	FLASH_ID("epcq128",             NON_EPCS_OPCODE_ID, 0x18),
> +	FLASH_ID("epcq256",             NON_EPCS_OPCODE_ID, 0x19),
> +	FLASH_ID("epcq512",             NON_EPCS_OPCODE_ID, 0x20),
> +	FLASH_ID("epcq1024",            NON_EPCS_OPCODE_ID, 0x21),
> +
> +	FLASH_ID("epcql256",            NON_EPCS_OPCODE_ID, 0x19),
> +	FLASH_ID("epcql512",            NON_EPCS_OPCODE_ID, 0x20),
> +	FLASH_ID("epcql1024",           NON_EPCS_OPCODE_ID, 0x21),
> +
> +};

I think it'd be better to wait until the IP block is fixed and can
issue READID correctly.

> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 14a5d23..2ab7279 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -687,6 +687,24 @@ 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 are non-JEDEC */

Are they really ? Last time I checked on CV SoC, I was able to read their
JEDEC ID just fine ; though it's true I used that EPCS core.

> +	{ "epcs16",   INFO(0, 0, 0x10000, 32,   0) },
> +	{ "epcs64",   INFO(0, 0, 0x10000, 128,  0) },
> +	{ "epcs128",  INFO(0, 0, 0x40000, 256,  0) },
> +
> +	{ "epcq16",   INFO(0, 0, 0x10000, 32,   0) },
> +	{ "epcq32",   INFO(0, 0, 0x10000, 64,   0) },
> +	{ "epcq64",   INFO(0, 0, 0x10000, 128,  0) },
> +	{ "epcq128",  INFO(0, 0, 0x10000, 256,  0) },
> +	{ "epcq256",  INFO(0, 0, 0x10000, 512,  0) },
> +	{ "epcq512",  INFO(0, 0, 0x10000, 1024, 0) },
> +	{ "epcq1024", INFO(0, 0, 0x10000, 2048, 0) },
> +
> +	{ "epcql256",  INFO(0, 0, 0x10000, 512,  0) },
> +	{ "epcql512",  INFO(0, 0, 0x10000, 1024, 0) },
> +	{ "epcql1024", INFO(0, 0, 0x10000, 2048, 0) },
> +
>  	{ },
>  };
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1210269

FromViet Nga Dao <vndao@altera.com>
Date2015-08-20 10:20 +0200
Message-ID<pZteO-7Cz-13@gated-at.bofh.it>
In reply to#1210261
Sorry for missing to reply the last question

On Thu, Aug 20, 2015 at 4:13 PM, Nga Chi <ngachi86@gmail.com> wrote:
> On Thu, Aug 20, 2015 at 4:03 PM, Marek Vasut <marex@denx.de> wrote:
>> On Thursday, August 20, 2015 at 08:55:05 AM, vndao@altera.com wrote:
>>> From: VIET NGA DAO <vndao@altera.com>
>>>
>>> Altera Quad SPI Controller is a soft IP which enables access to
>>> Altera EPCS and EPCQ flash chips. This patch adds driver
>>> for these devices.
>>>
>>> Signed-off-by: VIET NGA DAO <vndao@altera.com>
>>>
>>> ---
>>> v5:
>>> - Remove Micron support
>>> - Add multiple flashes probe failure handle
>>>
>>> v4:
>>> - Add more flash devices support ( EPCQL and Micron)
>>> - Remove redundant messages
>>> - Change EPCQ_OPCODE_ID to NON_EPCS_OPCODE_ID
>>> - Replace get_flash_name to altera_quadspi_scan
>>> - Remove clk related parts
>>> - Remove altera_quadspi_plat
>>> - Change device tree reg name and remove opcode-id
>>>
>>> v3:
>>> - Change altera_epcq driver name to altera_quadspi for more generic name
>>> - Implement flash name searching in altera_quadspi.c instead of spi-nor
>>> - Edit the altra quadspi info table in spi-nor
>>> - Remove wait_til_ready in all read,write, erase, lock, unlock functions
>>> - Merge .h and .c into 1 file
>>>
>>> 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-quadspi.txt     |   45 ++
>>>  drivers/mtd/spi-nor/Kconfig                        |    8 +
>>>  drivers/mtd/spi-nor/Makefile                       |    1 +
>>>  drivers/mtd/spi-nor/altera-quadspi.c               |  557
>>> ++++++++++++++++++++ drivers/mtd/spi-nor/spi-nor.c                      |
>>>  18 +
>>>  5 files changed, 629 insertions(+), 0 deletions(-)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/mtd/altera-quadspi.txt create mode
>>> 100644 drivers/mtd/spi-nor/altera-quadspi.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/altera-quadspi.txt
>>> b/Documentation/devicetree/bindings/mtd/altera-quadspi.txt new file mode
>>> 100644
>>> index 0000000..e1bcf18
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mtd/altera-quadspi.txt
>>> @@ -0,0 +1,45 @@
>>> +* MTD Altera QUADSPI driver
>>> +
>>> +Required properties:
>>> +- compatible: Should be "altr,quadspi-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
>>> +  "avl_csr": Should contain the register configuration base address
>>> +  "avl_mem": Should contain the data base address
>>> +- #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:
>>> +       1. EPCS:   epcs16, epcs64, epcs128
>>> +       2. EPCQ:   epcq16, epcq32, epcq64, epcq128, epcq256, epcq512, epcq1024
>>> +       3. EPCQ-L: epcql256, epcql512, epcql1024
>>> +     - #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:
>>> +
>>> +                     quadspi_controller_0: quadspi@0x180014a0 {
>>> +                             compatible = "altr,quadspi-1.0";
>>> +                             reg = <0x180014a0 0x00000020>,
>>> +                                   <0x14000000 0x04000000>;
>>> +                             reg-names = "avl_csr", "avl_mem";
>>> +                             #address-cells = <1>;
>>> +                             #size-cells = <0>;
>>> +                             flash0: epcq256@0 {
>>> +                                     compatible = "altr,epcq256";
>>> +                                     #address-cells = <1>;
>>> +                                     #size-cells = <1>;
>>> +                                     partition@0 {
>>> +                                             /* 16 MB for raw data. */
>>> +                                             label = "EPCQ Flash 0 raw data";
>>> +                                             reg = <0x0 0x1000000>;
>>> +                                     };
>>> +                                     partition@1000000 {
>>> +                                             /* 16 MB for jffs2 data. */
>>> +                                             label = "EPCQ Flash 0 JFFS 2";
>>> +                                             reg = <0x1000000 0x1000000>;
>>> +                                     };
>>
>> IIRC, encoding partitions into OF is deprecated (and it shouldn't be
>> part of the example anyway, so please remove this bit).
>>
>>> +                             };
>>> +                     }; //end quadspi@0x180014a0 (quadspi_controller_0)
>>
>> Remove this incorrect comment.
>>
>>
>> [...]
>>
>
>Do you mean the partition part below should not be in example?
>   partition@0 {
>                                             /* 16 MB for raw data. */
>                                             label = "EPCQ Flash 0 raw data";
>                                              reg = <0x0 0x1000000>;
>                                     };
>                                     partition@1000000 {
>                                            /* 16 MB for jffs2 data. */
>                                             label = "EPCQ Flash 0 JFFS 2";
>                                           reg = <0x1000000 0x1000000>;
>                                      };
>
>>> +static struct flash_device flash_devices[] = {
>>> +     FLASH_ID("epcs16",              EPCS_OPCODE_ID,     0x14),
>>> +     FLASH_ID("epcs64",              EPCS_OPCODE_ID,     0x16),
>>> +     FLASH_ID("epcs128",             EPCS_OPCODE_ID,     0x18),
>>> +
>>> +     FLASH_ID("epcq16",              NON_EPCS_OPCODE_ID, 0x15),
>>> +     FLASH_ID("epcq32",              NON_EPCS_OPCODE_ID, 0x16),
>>> +     FLASH_ID("epcq64",              NON_EPCS_OPCODE_ID, 0x17),
>>> +     FLASH_ID("epcq128",             NON_EPCS_OPCODE_ID, 0x18),
>>> +     FLASH_ID("epcq256",             NON_EPCS_OPCODE_ID, 0x19),
>>> +     FLASH_ID("epcq512",             NON_EPCS_OPCODE_ID, 0x20),
>>> +     FLASH_ID("epcq1024",            NON_EPCS_OPCODE_ID, 0x21),
>>> +
>>> +     FLASH_ID("epcql256",            NON_EPCS_OPCODE_ID, 0x19),
>>> +     FLASH_ID("epcql512",            NON_EPCS_OPCODE_ID, 0x20),
>>> +     FLASH_ID("epcql1024",           NON_EPCS_OPCODE_ID, 0x21),
>>> +
>>> +};
>>
>> I think it'd be better to wait until the IP block is fixed and can
>> issue READID correctly.
>>
>
> The hardware IP fix will take time while there are lot of customer are
> waiting for this driver. That is why we decided to upstream the
> driver. If the hardware fix, there might not need to have any changes
> in driver to support or if yes, it will be just minor.
>
>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>> index 14a5d23..2ab7279 100644
>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>> @@ -687,6 +687,24 @@ 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 are non-JEDEC */
>>
>> Are they really ? Last time I checked on CV SoC, I was able to read their
>> JEDEC ID just fine ; though it's true I used that EPCS core.
>>

Altera EPCS flash is non-jedec device. And this new controller is not
EPCS controller. If you look at the documentation i sent in another
email, they are not the same.

>>> +     { "epcs16",   INFO(0, 0, 0x10000, 32,   0) },
>>> +     { "epcs64",   INFO(0, 0, 0x10000, 128,  0) },
>>> +     { "epcs128",  INFO(0, 0, 0x40000, 256,  0) },
>>> +
>>> +     { "epcq16",   INFO(0, 0, 0x10000, 32,   0) },
>>> +     { "epcq32",   INFO(0, 0, 0x10000, 64,   0) },
>>> +     { "epcq64",   INFO(0, 0, 0x10000, 128,  0) },
>>> +     { "epcq128",  INFO(0, 0, 0x10000, 256,  0) },
>>> +     { "epcq256",  INFO(0, 0, 0x10000, 512,  0) },
>>> +     { "epcq512",  INFO(0, 0, 0x10000, 1024, 0) },
>>> +     { "epcq1024", INFO(0, 0, 0x10000, 2048, 0) },
>>> +
>>> +     { "epcql256",  INFO(0, 0, 0x10000, 512,  0) },
>>> +     { "epcql512",  INFO(0, 0, 0x10000, 1024, 0) },
>>> +     { "epcql1024", INFO(0, 0, 0x10000, 2048, 0) },
>>> +
>>>       { },
>>>  };
>
>
>
> --
> Nga
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1210320

FromMarek Vasut <marex@denx.de>
Date2015-08-20 11:10 +0200
Message-ID<pZu1e-l9-47@gated-at.bofh.it>
In reply to#1210269
On Thursday, August 20, 2015 at 10:17:57 AM, Viet Nga Dao wrote:
> Sorry for missing to reply the last question

Hi!

> >>> diff --git a/drivers/mtd/spi-nor/spi-nor.c
> >>> b/drivers/mtd/spi-nor/spi-nor.c index 14a5d23..2ab7279 100644
> >>> --- a/drivers/mtd/spi-nor/spi-nor.c
> >>> +++ b/drivers/mtd/spi-nor/spi-nor.c
> >>> @@ -687,6 +687,24 @@ 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 are non-JEDEC */
> >> 
> >> Are they really ? Last time I checked on CV SoC, I was able to read
> >> their JEDEC ID just fine ; though it's true I used that EPCS core.
> 
> Altera EPCS flash is non-jedec device. And this new controller is not
> EPCS controller. If you look at the documentation i sent in another
> email, they are not the same.

https://www.altera.com/content/dam/altera-
www/global/en_US/pdfs/literature/hb/cfg/cfg_cf52012.pdf page 15 ; seems
like opcode 0x9f is supported. All the other opcodes seems compatible too.
What is the problem ?

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

[toc] | [prev] | [next] | [standalone]


#1210271

FromNga Chi <ngachi86@gmail.com>
Date2015-08-20 10:20 +0200
Message-ID<pZteO-7Cz-15@gated-at.bofh.it>
In reply to#1210261
On Thu, Aug 20, 2015 at 4:03 PM, Marek Vasut <marex@denx.de> wrote:
> On Thursday, August 20, 2015 at 08:55:05 AM, vndao@altera.com wrote:
>> From: VIET NGA DAO <vndao@altera.com>
>>
>> Altera Quad SPI Controller is a soft IP which enables access to
>> Altera EPCS and EPCQ flash chips. This patch adds driver
>> for these devices.
>>
>> Signed-off-by: VIET NGA DAO <vndao@altera.com>
>>
>> ---
>> v5:
>> - Remove Micron support
>> - Add multiple flashes probe failure handle
>>
>> v4:
>> - Add more flash devices support ( EPCQL and Micron)
>> - Remove redundant messages
>> - Change EPCQ_OPCODE_ID to NON_EPCS_OPCODE_ID
>> - Replace get_flash_name to altera_quadspi_scan
>> - Remove clk related parts
>> - Remove altera_quadspi_plat
>> - Change device tree reg name and remove opcode-id
>>
>> v3:
>> - Change altera_epcq driver name to altera_quadspi for more generic name
>> - Implement flash name searching in altera_quadspi.c instead of spi-nor
>> - Edit the altra quadspi info table in spi-nor
>> - Remove wait_til_ready in all read,write, erase, lock, unlock functions
>> - Merge .h and .c into 1 file
>>
>> 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-quadspi.txt     |   45 ++
>>  drivers/mtd/spi-nor/Kconfig                        |    8 +
>>  drivers/mtd/spi-nor/Makefile                       |    1 +
>>  drivers/mtd/spi-nor/altera-quadspi.c               |  557
>> ++++++++++++++++++++ drivers/mtd/spi-nor/spi-nor.c                      |
>>  18 +
>>  5 files changed, 629 insertions(+), 0 deletions(-)
>>  create mode 100644
>> Documentation/devicetree/bindings/mtd/altera-quadspi.txt create mode
>> 100644 drivers/mtd/spi-nor/altera-quadspi.c
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/altera-quadspi.txt
>> b/Documentation/devicetree/bindings/mtd/altera-quadspi.txt new file mode
>> 100644
>> index 0000000..e1bcf18
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/altera-quadspi.txt
>> @@ -0,0 +1,45 @@
>> +* MTD Altera QUADSPI driver
>> +
>> +Required properties:
>> +- compatible: Should be "altr,quadspi-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
>> +  "avl_csr": Should contain the register configuration base address
>> +  "avl_mem": Should contain the data base address
>> +- #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:
>> +       1. EPCS:   epcs16, epcs64, epcs128
>> +       2. EPCQ:   epcq16, epcq32, epcq64, epcq128, epcq256, epcq512, epcq1024
>> +       3. EPCQ-L: epcql256, epcql512, epcql1024
>> +     - #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:
>> +
>> +                     quadspi_controller_0: quadspi@0x180014a0 {
>> +                             compatible = "altr,quadspi-1.0";
>> +                             reg = <0x180014a0 0x00000020>,
>> +                                   <0x14000000 0x04000000>;
>> +                             reg-names = "avl_csr", "avl_mem";
>> +                             #address-cells = <1>;
>> +                             #size-cells = <0>;
>> +                             flash0: epcq256@0 {
>> +                                     compatible = "altr,epcq256";
>> +                                     #address-cells = <1>;
>> +                                     #size-cells = <1>;
>> +                                     partition@0 {
>> +                                             /* 16 MB for raw data. */
>> +                                             label = "EPCQ Flash 0 raw data";
>> +                                             reg = <0x0 0x1000000>;
>> +                                     };
>> +                                     partition@1000000 {
>> +                                             /* 16 MB for jffs2 data. */
>> +                                             label = "EPCQ Flash 0 JFFS 2";
>> +                                             reg = <0x1000000 0x1000000>;
>> +                                     };
>
> IIRC, encoding partitions into OF is deprecated (and it shouldn't be
> part of the example anyway, so please remove this bit).
>
>> +                             };
>> +                     }; //end quadspi@0x180014a0 (quadspi_controller_0)
>
> Remove this incorrect comment.
>
>
> [...]
>

Do you mean the partition part below should not be in example?
  partition@0 {
                                            /* 16 MB for raw data. */
                                            label = "EPCQ Flash 0 raw data";
                                             reg = <0x0 0x1000000>;
                                    };
                                    partition@1000000 {
                                           /* 16 MB for jffs2 data. */
                                            label = "EPCQ Flash 0 JFFS 2";
                                          reg = <0x1000000 0x1000000>;
                                     };

>> +static struct flash_device flash_devices[] = {
>> +     FLASH_ID("epcs16",              EPCS_OPCODE_ID,     0x14),
>> +     FLASH_ID("epcs64",              EPCS_OPCODE_ID,     0x16),
>> +     FLASH_ID("epcs128",             EPCS_OPCODE_ID,     0x18),
>> +
>> +     FLASH_ID("epcq16",              NON_EPCS_OPCODE_ID, 0x15),
>> +     FLASH_ID("epcq32",              NON_EPCS_OPCODE_ID, 0x16),
>> +     FLASH_ID("epcq64",              NON_EPCS_OPCODE_ID, 0x17),
>> +     FLASH_ID("epcq128",             NON_EPCS_OPCODE_ID, 0x18),
>> +     FLASH_ID("epcq256",             NON_EPCS_OPCODE_ID, 0x19),
>> +     FLASH_ID("epcq512",             NON_EPCS_OPCODE_ID, 0x20),
>> +     FLASH_ID("epcq1024",            NON_EPCS_OPCODE_ID, 0x21),
>> +
>> +     FLASH_ID("epcql256",            NON_EPCS_OPCODE_ID, 0x19),
>> +     FLASH_ID("epcql512",            NON_EPCS_OPCODE_ID, 0x20),
>> +     FLASH_ID("epcql1024",           NON_EPCS_OPCODE_ID, 0x21),
>> +
>> +};
>
> I think it'd be better to wait until the IP block is fixed and can
> issue READID correctly.
>

The hardware IP fix will take time while there are lot of customer are
waiting for this driver. That is why we decided to upstream the
driver. If the hardware fix, there might not need to have any changes
in driver to support or if yes, it will be just minor.

>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index 14a5d23..2ab7279 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -687,6 +687,24 @@ 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 are non-JEDEC */
>
> Are they really ? Last time I checked on CV SoC, I was able to read their
> JEDEC ID just fine ; though it's true I used that EPCS core.
>
>> +     { "epcs16",   INFO(0, 0, 0x10000, 32,   0) },
>> +     { "epcs64",   INFO(0, 0, 0x10000, 128,  0) },
>> +     { "epcs128",  INFO(0, 0, 0x40000, 256,  0) },
>> +
>> +     { "epcq16",   INFO(0, 0, 0x10000, 32,   0) },
>> +     { "epcq32",   INFO(0, 0, 0x10000, 64,   0) },
>> +     { "epcq64",   INFO(0, 0, 0x10000, 128,  0) },
>> +     { "epcq128",  INFO(0, 0, 0x10000, 256,  0) },
>> +     { "epcq256",  INFO(0, 0, 0x10000, 512,  0) },
>> +     { "epcq512",  INFO(0, 0, 0x10000, 1024, 0) },
>> +     { "epcq1024", INFO(0, 0, 0x10000, 2048, 0) },
>> +
>> +     { "epcql256",  INFO(0, 0, 0x10000, 512,  0) },
>> +     { "epcql512",  INFO(0, 0, 0x10000, 1024, 0) },
>> +     { "epcql1024", INFO(0, 0, 0x10000, 2048, 0) },
>> +
>>       { },
>>  };



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

[toc] | [prev] | [next] | [standalone]


#1210317

FromMarek Vasut <marex@denx.de>
Date2015-08-20 11:10 +0200
Message-ID<pZu1d-l9-45@gated-at.bofh.it>
In reply to#1210271
On Thursday, August 20, 2015 at 10:13:30 AM, Nga Chi wrote:
> On Thu, Aug 20, 2015 at 4:03 PM, Marek Vasut <marex@denx.de> wrote:
> > On Thursday, August 20, 2015 at 08:55:05 AM, vndao@altera.com wrote:
> >> From: VIET NGA DAO <vndao@altera.com>
> >> 
> >> Altera Quad SPI Controller is a soft IP which enables access to
> >> Altera EPCS and EPCQ flash chips. This patch adds driver
> >> for these devices.
> >> 
> >> Signed-off-by: VIET NGA DAO <vndao@altera.com>
> >> 
> >> ---
> >> v5:
> >> - Remove Micron support
> >> - Add multiple flashes probe failure handle
> >> 
> >> v4:
> >> - Add more flash devices support ( EPCQL and Micron)
> >> - Remove redundant messages
> >> - Change EPCQ_OPCODE_ID to NON_EPCS_OPCODE_ID
> >> - Replace get_flash_name to altera_quadspi_scan
> >> - Remove clk related parts
> >> - Remove altera_quadspi_plat
> >> - Change device tree reg name and remove opcode-id
> >> 
> >> v3:
> >> - Change altera_epcq driver name to altera_quadspi for more generic name
> >> - Implement flash name searching in altera_quadspi.c instead of spi-nor
> >> - Edit the altra quadspi info table in spi-nor
> >> - Remove wait_til_ready in all read,write, erase, lock, unlock functions
> >> - Merge .h and .c into 1 file
> >> 
> >> 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-quadspi.txt     |   45 ++
> >>  drivers/mtd/spi-nor/Kconfig                        |    8 +
> >>  drivers/mtd/spi-nor/Makefile                       |    1 +
> >>  drivers/mtd/spi-nor/altera-quadspi.c               |  557
> >> 
> >> ++++++++++++++++++++ drivers/mtd/spi-nor/spi-nor.c                     
> >> |
> >> 
> >>  18 +
> >>  5 files changed, 629 insertions(+), 0 deletions(-)
> >>  create mode 100644
> >> 
> >> Documentation/devicetree/bindings/mtd/altera-quadspi.txt create mode
> >> 100644 drivers/mtd/spi-nor/altera-quadspi.c
> >> 
> >> diff --git a/Documentation/devicetree/bindings/mtd/altera-quadspi.txt
> >> b/Documentation/devicetree/bindings/mtd/altera-quadspi.txt new file mode
> >> 100644
> >> index 0000000..e1bcf18
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mtd/altera-quadspi.txt
> >> @@ -0,0 +1,45 @@
> >> +* MTD Altera QUADSPI driver
> >> +
> >> +Required properties:
> >> +- compatible: Should be "altr,quadspi-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
> >> +  "avl_csr": Should contain the register configuration base address
> >> +  "avl_mem": Should contain the data base address
> >> +- #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:
> >> +       1. EPCS:   epcs16, epcs64, epcs128
> >> +       2. EPCQ:   epcq16, epcq32, epcq64, epcq128, epcq256, epcq512,
> >> epcq1024 +       3. EPCQ-L: epcql256, epcql512, epcql1024
> >> +     - #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:
> >> +
> >> +                     quadspi_controller_0: quadspi@0x180014a0 {
> >> +                             compatible = "altr,quadspi-1.0";
> >> +                             reg = <0x180014a0 0x00000020>,
> >> +                                   <0x14000000 0x04000000>;
> >> +                             reg-names = "avl_csr", "avl_mem";
> >> +                             #address-cells = <1>;
> >> +                             #size-cells = <0>;
> >> +                             flash0: epcq256@0 {
> >> +                                     compatible = "altr,epcq256";
> >> +                                     #address-cells = <1>;
> >> +                                     #size-cells = <1>;
> >> +                                     partition@0 {
> >> +                                             /* 16 MB for raw data. */
> >> +                                             label = "EPCQ Flash 0 raw
> >> data"; +                                             reg = <0x0
> >> 0x1000000>; +                                     };
> >> +                                     partition@1000000 {
> >> +                                             /* 16 MB for jffs2 data.
> >> */ +                                             label = "EPCQ Flash 0
> >> JFFS 2"; +                                             reg = <0x1000000
> >> 0x1000000>; +                                     };
> > 
> > IIRC, encoding partitions into OF is deprecated (and it shouldn't be
> > part of the example anyway, so please remove this bit).
> > 
> >> +                             };
> >> +                     }; //end quadspi@0x180014a0 (quadspi_controller_0)
> > 
> > Remove this incorrect comment.
> > 
> > 
> > [...]
> 
> Do you mean the partition part below should not be in example?
>   partition@0 {
>                                             /* 16 MB for raw data. */
>                                             label = "EPCQ Flash 0 raw
> data"; reg = <0x0 0x1000000>; };
>                                     partition@1000000 {
>                                            /* 16 MB for jffs2 data. */
>                                             label = "EPCQ Flash 0 JFFS 2";
>                                           reg = <0x1000000 0x1000000>;
>                                      };

Yes, it's not really relevant.

> >> +static struct flash_device flash_devices[] = {
> >> +     FLASH_ID("epcs16",              EPCS_OPCODE_ID,     0x14),
> >> +     FLASH_ID("epcs64",              EPCS_OPCODE_ID,     0x16),
> >> +     FLASH_ID("epcs128",             EPCS_OPCODE_ID,     0x18),
> >> +
> >> +     FLASH_ID("epcq16",              NON_EPCS_OPCODE_ID, 0x15),
> >> +     FLASH_ID("epcq32",              NON_EPCS_OPCODE_ID, 0x16),
> >> +     FLASH_ID("epcq64",              NON_EPCS_OPCODE_ID, 0x17),
> >> +     FLASH_ID("epcq128",             NON_EPCS_OPCODE_ID, 0x18),
> >> +     FLASH_ID("epcq256",             NON_EPCS_OPCODE_ID, 0x19),
> >> +     FLASH_ID("epcq512",             NON_EPCS_OPCODE_ID, 0x20),
> >> +     FLASH_ID("epcq1024",            NON_EPCS_OPCODE_ID, 0x21),
> >> +
> >> +     FLASH_ID("epcql256",            NON_EPCS_OPCODE_ID, 0x19),
> >> +     FLASH_ID("epcql512",            NON_EPCS_OPCODE_ID, 0x20),
> >> +     FLASH_ID("epcql1024",           NON_EPCS_OPCODE_ID, 0x21),
> >> +
> >> +};
> > 
> > I think it'd be better to wait until the IP block is fixed and can
> > issue READID correctly.
> 
> The hardware IP fix will take time while there are lot of customer are
> waiting for this driver.

Does that justify pushing serious crap into mainline Linux in any way ?

> That is why we decided to upstream the
> driver. If the hardware fix, there might not need to have any changes
> in driver to support or if yes, it will be just minor.

If the hardware can do proper READID opcode, this entire nonsense table
will go away and a proper integration into the SPI NOR framework will
take place.

You might consider submitting this driver for staging, but I definitely
am not a big fan of that.

> >> diff --git a/drivers/mtd/spi-nor/spi-nor.c
> >> b/drivers/mtd/spi-nor/spi-nor.c index 14a5d23..2ab7279 100644
> >> --- a/drivers/mtd/spi-nor/spi-nor.c
> >> +++ b/drivers/mtd/spi-nor/spi-nor.c
> >> @@ -687,6 +687,24 @@ 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 are non-JEDEC */
> > 
> > Are they really ? Last time I checked on CV SoC, I was able to read their
> > JEDEC ID just fine ; though it's true I used that EPCS core.
> > 
> >> +     { "epcs16",   INFO(0, 0, 0x10000, 32,   0) },
> >> +     { "epcs64",   INFO(0, 0, 0x10000, 128,  0) },
> >> +     { "epcs128",  INFO(0, 0, 0x40000, 256,  0) },
> >> +
> >> +     { "epcq16",   INFO(0, 0, 0x10000, 32,   0) },
> >> +     { "epcq32",   INFO(0, 0, 0x10000, 64,   0) },
> >> +     { "epcq64",   INFO(0, 0, 0x10000, 128,  0) },
> >> +     { "epcq128",  INFO(0, 0, 0x10000, 256,  0) },
> >> +     { "epcq256",  INFO(0, 0, 0x10000, 512,  0) },
> >> +     { "epcq512",  INFO(0, 0, 0x10000, 1024, 0) },
> >> +     { "epcq1024", INFO(0, 0, 0x10000, 2048, 0) },
> >> +
> >> +     { "epcql256",  INFO(0, 0, 0x10000, 512,  0) },
> >> +     { "epcql512",  INFO(0, 0, 0x10000, 1024, 0) },
> >> +     { "epcql1024", INFO(0, 0, 0x10000, 2048, 0) },
> >> +
> >> 
> >>       { },
> >>  
> >>  };
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1210336

FromViet Nga Dao <vndao@altera.com>
Date2015-08-20 11:20 +0200
Message-ID<pZuaT-wx-45@gated-at.bofh.it>
In reply to#1210317
On Thu, Aug 20, 2015 at 4:52 PM, Marek Vasut <marex@denx.de> wrote:
> On Thursday, August 20, 2015 at 10:13:30 AM, Nga Chi wrote:
>> On Thu, Aug 20, 2015 at 4:03 PM, Marek Vasut <marex@denx.de> wrote:
>> > On Thursday, August 20, 2015 at 08:55:05 AM, vndao@altera.com wrote:
>> >> From: VIET NGA DAO <vndao@altera.com>
>> >>
>> >> Altera Quad SPI Controller is a soft IP which enables access to
>> >> Altera EPCS and EPCQ flash chips. This patch adds driver
>> >> for these devices.
>> >>
>> >> Signed-off-by: VIET NGA DAO <vndao@altera.com>
>> >>
>> >> ---
>> >> v5:
>> >> - Remove Micron support
>> >> - Add multiple flashes probe failure handle
>> >>
>> >> v4:
>> >> - Add more flash devices support ( EPCQL and Micron)
>> >> - Remove redundant messages
>> >> - Change EPCQ_OPCODE_ID to NON_EPCS_OPCODE_ID
>> >> - Replace get_flash_name to altera_quadspi_scan
>> >> - Remove clk related parts
>> >> - Remove altera_quadspi_plat
>> >> - Change device tree reg name and remove opcode-id
>> >>
>> >> v3:
>> >> - Change altera_epcq driver name to altera_quadspi for more generic name
>> >> - Implement flash name searching in altera_quadspi.c instead of spi-nor
>> >> - Edit the altra quadspi info table in spi-nor
>> >> - Remove wait_til_ready in all read,write, erase, lock, unlock functions
>> >> - Merge .h and .c into 1 file
>> >>
>> >> 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-quadspi.txt     |   45 ++
>> >>  drivers/mtd/spi-nor/Kconfig                        |    8 +
>> >>  drivers/mtd/spi-nor/Makefile                       |    1 +
>> >>  drivers/mtd/spi-nor/altera-quadspi.c               |  557
>> >>
>> >> ++++++++++++++++++++ drivers/mtd/spi-nor/spi-nor.c
>> >> |
>> >>
>> >>  18 +
>> >>  5 files changed, 629 insertions(+), 0 deletions(-)
>> >>  create mode 100644
>> >>
>> >> Documentation/devicetree/bindings/mtd/altera-quadspi.txt create mode
>> >> 100644 drivers/mtd/spi-nor/altera-quadspi.c
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/mtd/altera-quadspi.txt
>> >> b/Documentation/devicetree/bindings/mtd/altera-quadspi.txt new file mode
>> >> 100644
>> >> index 0000000..e1bcf18
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/mtd/altera-quadspi.txt
>> >> @@ -0,0 +1,45 @@
>> >> +* MTD Altera QUADSPI driver
>> >> +
>> >> +Required properties:
>> >> +- compatible: Should be "altr,quadspi-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
>> >> +  "avl_csr": Should contain the register configuration base address
>> >> +  "avl_mem": Should contain the data base address
>> >> +- #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:
>> >> +       1. EPCS:   epcs16, epcs64, epcs128
>> >> +       2. EPCQ:   epcq16, epcq32, epcq64, epcq128, epcq256, epcq512,
>> >> epcq1024 +       3. EPCQ-L: epcql256, epcql512, epcql1024
>> >> +     - #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:
>> >> +
>> >> +                     quadspi_controller_0: quadspi@0x180014a0 {
>> >> +                             compatible = "altr,quadspi-1.0";
>> >> +                             reg = <0x180014a0 0x00000020>,
>> >> +                                   <0x14000000 0x04000000>;
>> >> +                             reg-names = "avl_csr", "avl_mem";
>> >> +                             #address-cells = <1>;
>> >> +                             #size-cells = <0>;
>> >> +                             flash0: epcq256@0 {
>> >> +                                     compatible = "altr,epcq256";
>> >> +                                     #address-cells = <1>;
>> >> +                                     #size-cells = <1>;
>> >> +                                     partition@0 {
>> >> +                                             /* 16 MB for raw data. */
>> >> +                                             label = "EPCQ Flash 0 raw
>> >> data"; +                                             reg = <0x0
>> >> 0x1000000>; +                                     };
>> >> +                                     partition@1000000 {
>> >> +                                             /* 16 MB for jffs2 data.
>> >> */ +                                             label = "EPCQ Flash 0
>> >> JFFS 2"; +                                             reg = <0x1000000
>> >> 0x1000000>; +                                     };
>> >
>> > IIRC, encoding partitions into OF is deprecated (and it shouldn't be
>> > part of the example anyway, so please remove this bit).
>> >
>> >> +                             };
>> >> +                     }; //end quadspi@0x180014a0 (quadspi_controller_0)
>> >
>> > Remove this incorrect comment.
>> >
>> >
>> > [...]
>>
>> Do you mean the partition part below should not be in example?
>>   partition@0 {
>>                                             /* 16 MB for raw data. */
>>                                             label = "EPCQ Flash 0 raw
>> data"; reg = <0x0 0x1000000>; };
>>                                     partition@1000000 {
>>                                            /* 16 MB for jffs2 data. */
>>                                             label = "EPCQ Flash 0 JFFS 2";
>>                                           reg = <0x1000000 0x1000000>;
>>                                      };
>
> Yes, it's not really relevant.
>

OK

>> >> +static struct flash_device flash_devices[] = {
>> >> +     FLASH_ID("epcs16",              EPCS_OPCODE_ID,     0x14),
>> >> +     FLASH_ID("epcs64",              EPCS_OPCODE_ID,     0x16),
>> >> +     FLASH_ID("epcs128",             EPCS_OPCODE_ID,     0x18),
>> >> +
>> >> +     FLASH_ID("epcq16",              NON_EPCS_OPCODE_ID, 0x15),
>> >> +     FLASH_ID("epcq32",              NON_EPCS_OPCODE_ID, 0x16),
>> >> +     FLASH_ID("epcq64",              NON_EPCS_OPCODE_ID, 0x17),
>> >> +     FLASH_ID("epcq128",             NON_EPCS_OPCODE_ID, 0x18),
>> >> +     FLASH_ID("epcq256",             NON_EPCS_OPCODE_ID, 0x19),
>> >> +     FLASH_ID("epcq512",             NON_EPCS_OPCODE_ID, 0x20),
>> >> +     FLASH_ID("epcq1024",            NON_EPCS_OPCODE_ID, 0x21),
>> >> +
>> >> +     FLASH_ID("epcql256",            NON_EPCS_OPCODE_ID, 0x19),
>> >> +     FLASH_ID("epcql512",            NON_EPCS_OPCODE_ID, 0x20),
>> >> +     FLASH_ID("epcql1024",           NON_EPCS_OPCODE_ID, 0x21),
>> >> +
>> >> +};
>> >
>> > I think it'd be better to wait until the IP block is fixed and can
>> > issue READID correctly.
>>
>> The hardware IP fix will take time while there are lot of customer are
>> waiting for this driver.
>
> Does that justify pushing serious crap into mainline Linux in any way ?
>
>> That is why we decided to upstream the
>> driver. If the hardware fix, there might not need to have any changes
>> in driver to support or if yes, it will be just minor.
>
> If the hardware can do proper READID opcode, this entire nonsense table
> will go away and a proper integration into the SPI NOR framework will
> take place.
>
> You might consider submitting this driver for staging, but I definitely
> am not a big fan of that.
>

You might misunderstand the hardware problem i mention here. This soft
IP controller is able to provide the ID for our Altera EPCS/EPCQ flash
chips, which are non JEDEC chips. As from EPCQ device data sheet
(https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/hb/cfg/cfg_cf52012.pdf),
the device ID is 8 bit data. The remaining problem here is that this
controller is able to support Micron chips but it currently has
limitation in providing only 8 bit ID, which is not full JEDEC ID for
Micron chips. Hence, we are asking hardware engineer to find out the
solution so that the driver does not need to do any dirty hacking. And
so, this table should still be here even hardware fix will take place
or not.

>> >> diff --git a/drivers/mtd/spi-nor/spi-nor.c
>> >> b/drivers/mtd/spi-nor/spi-nor.c index 14a5d23..2ab7279 100644
>> >> --- a/drivers/mtd/spi-nor/spi-nor.c
>> >> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> >> @@ -687,6 +687,24 @@ 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 are non-JEDEC */
>> >
>> > Are they really ? Last time I checked on CV SoC, I was able to read their
>> > JEDEC ID just fine ; though it's true I used that EPCS core.
>> >
>> >> +     { "epcs16",   INFO(0, 0, 0x10000, 32,   0) },
>> >> +     { "epcs64",   INFO(0, 0, 0x10000, 128,  0) },
>> >> +     { "epcs128",  INFO(0, 0, 0x40000, 256,  0) },
>> >> +
>> >> +     { "epcq16",   INFO(0, 0, 0x10000, 32,   0) },
>> >> +     { "epcq32",   INFO(0, 0, 0x10000, 64,   0) },
>> >> +     { "epcq64",   INFO(0, 0, 0x10000, 128,  0) },
>> >> +     { "epcq128",  INFO(0, 0, 0x10000, 256,  0) },
>> >> +     { "epcq256",  INFO(0, 0, 0x10000, 512,  0) },
>> >> +     { "epcq512",  INFO(0, 0, 0x10000, 1024, 0) },
>> >> +     { "epcq1024", INFO(0, 0, 0x10000, 2048, 0) },
>> >> +
>> >> +     { "epcql256",  INFO(0, 0, 0x10000, 512,  0) },
>> >> +     { "epcql512",  INFO(0, 0, 0x10000, 1024, 0) },
>> >> +     { "epcql1024", INFO(0, 0, 0x10000, 2048, 0) },
>> >> +
>> >>
>> >>       { },
>> >>
>> >>  };
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1210351

FromMarek Vasut <marex@denx.de>
Date2015-08-20 11:50 +0200
Message-ID<pZuDV-15a-27@gated-at.bofh.it>
In reply to#1210336
On Thursday, August 20, 2015 at 11:18:14 AM, Viet Nga Dao wrote:

Hi,

[...]

> >> That is why we decided to upstream the
> >> driver. If the hardware fix, there might not need to have any changes
> >> in driver to support or if yes, it will be just minor.
> > 
> > If the hardware can do proper READID opcode, this entire nonsense table
> > will go away and a proper integration into the SPI NOR framework will
> > take place.
> > 
> > You might consider submitting this driver for staging, but I definitely
> > am not a big fan of that.
> 
> You might misunderstand the hardware problem i mention here. This soft
> IP controller is able to provide the ID for our Altera EPCS/EPCQ flash
> chips, which are non JEDEC chips. As from EPCQ device data sheet
> (https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature
> /hb/cfg/cfg_cf52012.pdf), the device ID is 8 bit data.

So what exactly is the output of READID instruction followed by 6 Byte read
on an EPCQ chip?

> The remaining
> problem here is that this controller is able to support Micron chips but
> it currently has
> limitation in providing only 8 bit ID, which is not full JEDEC ID for
> Micron chips.

OK

> Hence, we are asking hardware engineer to find out the
> solution so that the driver does not need to do any dirty hacking. And
> so, this table should still be here even hardware fix will take place
> or not.

[...]

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

[toc] | [prev] | [next] | [standalone]


#1210722

FromBrian Norris <computersforpeace@gmail.com>
Date2015-08-20 22:40 +0200
Message-ID<pZEMW-7lF-19@gated-at.bofh.it>
In reply to#1210351
On Thu, Aug 20, 2015 at 11:42:18AM +0200, Marek Vasut wrote:
> On Thursday, August 20, 2015 at 11:18:14 AM, Viet Nga Dao wrote:
> > You might misunderstand the hardware problem i mention here. This soft
> > IP controller is able to provide the ID for our Altera EPCS/EPCQ flash
> > chips, which are non JEDEC chips. As from EPCQ device data sheet
> > (https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature
> > /hb/cfg/cfg_cf52012.pdf), the device ID is 8 bit data.
> 
> So what exactly is the output of READID instruction followed by 6 Byte read
> on an EPCQ chip?

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

[toc] | [prev] | [next] | [standalone]


#1210721

FromBrian Norris <computersforpeace@gmail.com>
Date2015-08-20 22:40 +0200
Message-ID<pZEMW-7lF-17@gated-at.bofh.it>
In reply to#1210336
On Thu, Aug 20, 2015 at 05:18:14PM +0800, Viet Nga Dao wrote:
> You might misunderstand the hardware problem i mention here. This soft
> IP controller is able to provide the ID for our Altera EPCS/EPCQ flash
> chips, which are non JEDEC chips. As from EPCQ device data sheet
> (https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/hb/cfg/cfg_cf52012.pdf),
> the device ID is 8 bit data.

8 bits of data is vastly insufficient for uniquely identifying a flash.
This is not extendible and is not really a candidate for inclusion in
mainline, unless it's somehow guaranteed that these flash can only be
used with your controller (and I'm not sure how you would do that).
Otherwise, you need to augment every flash with more out-of-band
device-tree information.

> The remaining problem here is that this
> controller is able to support Micron chips but it currently has
> limitation in providing only 8 bit ID, which is not full JEDEC ID for
> Micron chips.

You're just proving my point. I will not support "READ ID" detection
that is based on only 8 bits of ID.

> Hence, we are asking hardware engineer to find out the
> solution so that the driver does not need to do any dirty hacking.

OK, then I wish your hardware team great speed.

> And
> so, this table should still be here even hardware fix will take place
> or not.

I'm not sure how you come to this conclusion.

BTW, to reiterate one other question that I feel wasn't adequately
answered: what does the full ID string look like for these EPCS/EPCQ
flash chips? Not the ID as seen by your limited controller, but the ID
that can be reported by the flash chip. Is it really only an 8-bit
number? Or does it have a longer string that your controller just can't
read?

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

[toc] | [prev] | [next] | [standalone]


#1210825

FromViet Nga Dao <vndao@altera.com>
Date2015-08-21 03:40 +0200
Message-ID<pZJtg-5K8-1@gated-at.bofh.it>
In reply to#1210721
On Fri, Aug 21, 2015 at 4:37 AM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Thu, Aug 20, 2015 at 05:18:14PM +0800, Viet Nga Dao wrote:
>> You might misunderstand the hardware problem i mention here. This soft
>> IP controller is able to provide the ID for our Altera EPCS/EPCQ flash
>> chips, which are non JEDEC chips. As from EPCQ device data sheet
>> (https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/hb/cfg/cfg_cf52012.pdf),
>> the device ID is 8 bit data.
>
> 8 bits of data is vastly insufficient for uniquely identifying a flash.
> This is not extendible and is not really a candidate for inclusion in
> mainline, unless it's somehow guaranteed that these flash can only be
> used with your controller (and I'm not sure how you would do that).
> Otherwise, you need to augment every flash with more out-of-band
> device-tree information.
>
>> The remaining problem here is that this
>> controller is able to support Micron chips but it currently has
>> limitation in providing only 8 bit ID, which is not full JEDEC ID for
>> Micron chips.
>
> You're just proving my point. I will not support "READ ID" detection
> that is based on only 8 bits of ID.
>
>> Hence, we are asking hardware engineer to find out the
>> solution so that the driver does not need to do any dirty hacking.
>
> OK, then I wish your hardware team great speed.
>
>> And
>> so, this table should still be here even hardware fix will take place
>> or not.
>
> I'm not sure how you come to this conclusion.
>

I have this conclusion is because Altera EPCQ/EPCS flashes are non
JEDEC. Thus on the spi_device_id table,
the ID in the INFO struct must be filled up with zeros in order to
matches the current framework.
However, since i still persist to have the device id check in my
driver, as suggested by Rash,
I should implement it locally in my driver. And this table is the look
up table for the device ID of supported flashes.

Or maybe you can give me any other better idea?

> BTW, to reiterate one other question that I feel wasn't adequately
> answered: what does the full ID string look like for these EPCS/EPCQ
> flash chips? Not the ID as seen by your limited controller, but the ID
> that can be reported by the flash chip. Is it really only an 8-bit
> number? Or does it have a longer string that your controller just can't
> read?
>

Yes, As you can refer to page 32 of EPCQ flash datasheet
(https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/hb/cfg/cfg_cf52012.pdf),
 "This operation reads the 8-bit device identification of the EPCQ
device from the DATA1 output pin".
Table 29 lists the EPCQ device identifications

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

[toc] | [prev] | [next] | [standalone]


#1210378

FromAlexander Stein <alexander.stein@systec-electronic.com>
Date2015-08-20 12:10 +0200
Message-ID<pZuXg-1Hx-25@gated-at.bofh.it>
In reply to#1210261
Hello Marek,

On Thursday 20 August 2015 10:03:38, Marek Vasut wrote:
> > +Example:
> > +
> > +			quadspi_controller_0: quadspi@0x180014a0 {
> > +				compatible = "altr,quadspi-1.0";
> > +				reg = <0x180014a0 0x00000020>,
> > +				      <0x14000000 0x04000000>;
> > +				reg-names = "avl_csr", "avl_mem";
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +				flash0: epcq256@0 {
> > +					compatible = "altr,epcq256";
> > +					#address-cells = <1>;
> > +					#size-cells = <1>;
> > +					partition@0 {
> > +						/* 16 MB for raw data. */
> > +						label = "EPCQ Flash 0 raw data";
> > +						reg = <0x0 0x1000000>;
> > +					};
> > +					partition@1000000 {
> > +						/* 16 MB for jffs2 data. */
> > +						label = "EPCQ Flash 0 JFFS 2";
> > +						reg = <0x1000000 0x1000000>;
> > +					};
> 
> IIRC, encoding partitions into OF is deprecated (and it shouldn't be
> part of the example anyway, so please remove this bit).

Do you mean specifying partitions in OF is deprecated in general? Is there any link for that?
What would be an alternative to it?

Best regards,
Alexander
-- 
Dipl.-Inf. Alexander Stein
SYS TEC electronic GmbH
alexander.stein@systec-electronic.com

Legal and Commercial Address:
Am Windrad 2
08468 Heinsdorfergrund
Germany

Office: +49 (0) 3765 38600-0
Fax:    +49 (0) 3765 38600-4100
 
Managing Directors:
	Director Technology/CEO: Dipl.-Phys. Siegmar Schmidt;
	Director Commercial Affairs/COO: Dipl. Ing. (FH) Armin von Collrepp
Commercial Registry:
	Amtsgericht Chemnitz, HRB 28082; USt.-Id Nr. DE150534010

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

[toc] | [prev] | [next] | [standalone]


#1210715

FromBrian Norris <computersforpeace@gmail.com>
Date2015-08-20 22:20 +0200
Message-ID<pZEtA-6XY-9@gated-at.bofh.it>
In reply to#1210378
On Thu, Aug 20, 2015 at 12:06:36PM +0200, Alexander Stein wrote:
> On Thursday 20 August 2015 10:03:38, Marek Vasut wrote:
> > > +Example:
> > > +
> > > +			quadspi_controller_0: quadspi@0x180014a0 {
> > > +				compatible = "altr,quadspi-1.0";
> > > +				reg = <0x180014a0 0x00000020>,
> > > +				      <0x14000000 0x04000000>;
> > > +				reg-names = "avl_csr", "avl_mem";
> > > +				#address-cells = <1>;
> > > +				#size-cells = <0>;
> > > +				flash0: epcq256@0 {
> > > +					compatible = "altr,epcq256";
> > > +					#address-cells = <1>;
> > > +					#size-cells = <1>;
> > > +					partition@0 {
> > > +						/* 16 MB for raw data. */
> > > +						label = "EPCQ Flash 0 raw data";
> > > +						reg = <0x0 0x1000000>;
> > > +					};
> > > +					partition@1000000 {
> > > +						/* 16 MB for jffs2 data. */
> > > +						label = "EPCQ Flash 0 JFFS 2";
> > > +						reg = <0x1000000 0x1000000>;
> > > +					};
> > 
> > IIRC, encoding partitions into OF is deprecated (and it shouldn't be
> > part of the example anyway, so please remove this bit).
> 
> Do you mean specifying partitions in OF is deprecated in general? Is there any link for that?
> What would be an alternative to it?

This is the first I've heard of such a deprecation. I would argue that
it's still probably one of the best ways to specify partitions. It's not
exactly perfect (partitions aren't really "hardware", so may not belong
in the "hardware description"), but IMO it's better than cmdlineparts,
for instance. I also don't really know of any good-enough, generically
useful on-flash partition formats, especially ones that can handle the
complexities of NAND.

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

[toc] | [prev] | [next] | [standalone]


#1210755

FromMarek Vasut <marex@denx.de>
Date2015-08-21 00:10 +0200
Message-ID<pZGc3-16Z-31@gated-at.bofh.it>
In reply to#1210715
On Thursday, August 20, 2015 at 10:19:25 PM, Brian Norris wrote:
> On Thu, Aug 20, 2015 at 12:06:36PM +0200, Alexander Stein wrote:
> > On Thursday 20 August 2015 10:03:38, Marek Vasut wrote:
> > > > +Example:
> > > > +
> > > > +			quadspi_controller_0: quadspi@0x180014a0 {
> > > > +				compatible = "altr,quadspi-1.0";
> > > > +				reg = <0x180014a0 0x00000020>,
> > > > +				      <0x14000000 0x04000000>;
> > > > +				reg-names = "avl_csr", "avl_mem";
> > > > +				#address-cells = <1>;
> > > > +				#size-cells = <0>;
> > > > +				flash0: epcq256@0 {
> > > > +					compatible = "altr,epcq256";
> > > > +					#address-cells = <1>;
> > > > +					#size-cells = <1>;
> > > > +					partition@0 {
> > > > +						/* 16 MB for raw data. 
*/
> > > > +						label = "EPCQ Flash 0 
raw data";
> > > > +						reg = <0x0 0x1000000>;
> > > > +					};
> > > > +					partition@1000000 {
> > > > +						/* 16 MB for jffs2 data. 
*/
> > > > +						label = "EPCQ Flash 0 
JFFS 2";
> > > > +						reg = <0x1000000 
0x1000000>;
> > > > +					};
> > > 
> > > IIRC, encoding partitions into OF is deprecated (and it shouldn't be
> > > part of the example anyway, so please remove this bit).
> > 
> > Do you mean specifying partitions in OF is deprecated in general? Is
> > there any link for that? What would be an alternative to it?
> 
> This is the first I've heard of such a deprecation. I would argue that
> it's still probably one of the best ways to specify partitions. It's not
> exactly perfect (partitions aren't really "hardware", so may not belong
> in the "hardware description")

Which is the reasoning why I heard it was deprecated, but it seems my
knowledge is not correct. I apologize for the misinformation, sorry.

> , but IMO it's better than cmdlineparts,
> for instance. I also don't really know of any good-enough, generically
> useful on-flash partition formats, especially ones that can handle the
> complexities of NAND.

Right.

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

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web