lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAN1oZWwLRYxs=eeWi8jh-jo8+C7WckdW1hfyunmq5_1qb1Jb8w@mail.gmail.com>
Date:	Thu, 20 Aug 2015 16:13:30 +0800
From:	Nga Chi <ngachi86@...il.com>
To:	Marek Vasut <marex@...x.de>
Cc:	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	Viet Nga Dao <vndao@...era.com>,
	Brian Norris <computersforpeace@...il.com>,
	David Woodhouse <dwmw2@...radead.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver

On Thu, Aug 20, 2015 at 4:03 PM, Marek Vasut <marex@...x.de> wrote:
> On Thursday, August 20, 2015 at 08:55:05 AM, vndao@...era.com wrote:
>> From: VIET NGA DAO <vndao@...era.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@...era.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@...80014a0 {
>> +                             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@...0000 {
>> +                                             /* 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@...80014a0 (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@...0000 {
                                           /* 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ