Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver
On Thu, Aug 20, 2015 at 2:55 PM, <vn...@altera.com> wrote: > From: VIET NGA DAO <vn...@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 <vn...@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 > --- Hi, i decided to wait for the hardware fixed and tested before submitting new patch. Thanks for all for your reviews and comments. Viet Nga -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [PATCH v4] mtd:spi-nor: Add Altera Quad SPI Driver
On Tue, Aug 18, 2015 at 2:55 PM, Viet Nga Dao vn...@altera.com wrote: -Original Message- From: ma...@denx.de Sent: Tuesday, August 18, 2015 9:33 AM To: Brian Norris Cc: linux-...@lists.infradead.org; Viet Nga Dao; devicetree@vger.kernel.org; Rafa?? Mi??ecki; linux-ker...@vger.kernel.org; David Woodhouse; Graham Moore Subject: Re: [PATCH] [PATCH v4] mtd:spi-nor: Add Altera Quad SPI Driver On Tuesday, August 18, 2015 at 03:24:44 AM, Brian Norris wrote: I'm not very helpful here, so hopefully Viet can be of more use: Yup :) On Mon, Aug 17, 2015 at 07:53:23PM +0200, Marek Vasut wrote: On Monday, August 17, 2015 at 06:03:38 PM, Brian Norris wrote: Also, I cannot find any documentation for this IP block even if I search through Quartus/QSys, is there any proper documentation available anywhere? I never found proper documentation, but I didn't look too hard. I've mostly been going off of Viet's comments and code. Me neither, and I looked through the altera stuff in fact. I'm trying to learn whether this is just an Soft IP, in which case it certainly can be fixed ; or if there is actually some chip shipping with this crap synthesised into actual silicon. But FWIW, I did find some relevant info for the peculiar Altera EPCQ flash here: https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/litera ture/ hb/cfg/cfg_cf52012.pdf Altera EPCS/EPCQ flashes are just rebranded micron flashes, they just have different JEDEC ID and are a bit more expensive. You can find the document at here (https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/ug/ug_embedded_ip.pdf) Chapter 42.Page 407. For the soft IP issue, i've requested hardware engineer to come out the solution. So in the mean way, our driver will NOT support Micron flashes until hardware fix is completed. Hence, i just submitted version 5 for this driver with eliminating micron device support. Hope this version will get ACKed by you. Thanks, Viet Nga -- To unsubscribe from this list: send the line unsubscribe devicetree in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver
On Thu, Aug 20, 2015 at 4:52 PM, Marek Vasut ma...@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 ma...@denx.de wrote: On Thursday, August 20, 2015 at 08:55:05 AM, vn...@altera.com wrote: From: VIET NGA DAO vn...@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 vn...@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 000..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 0x0020, + 0x1400 0x0400; + 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 0x100; + }; + partition@100 { + /* 16 MB for jffs2 data. */ + label = EPCQ Flash 0 JFFS 2; + reg = 0x100 0x100; + }; 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
Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver
Sorry for missing to reply the last question On Thu, Aug 20, 2015 at 4:13 PM, Nga Chi ngach...@gmail.com wrote: On Thu, Aug 20, 2015 at 4:03 PM, Marek Vasut ma...@denx.de wrote: On Thursday, August 20, 2015 at 08:55:05 AM, vn...@altera.com wrote: From: VIET NGA DAO vn...@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 vn...@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 000..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 0x0020, + 0x1400 0x0400; + 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 0x100; + }; + partition@100 { + /* 16 MB for jffs2 data. */ + label = EPCQ Flash 0 JFFS 2; + reg = 0x100 0x100; + }; 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 0x100
Re: [PATCH] [PATCH v4] mtd:spi-nor: Add Altera Quad SPI Driver
Hi, On Tuesday, August 18, 2015 at 03:24:44 AM, Brian Norris wrote: I'm not very helpful here, so hopefully Viet can be of more use: Yup :) On Mon, Aug 17, 2015 at 07:53:23PM +0200, Marek Vasut wrote: On Monday, August 17, 2015 at 06:03:38 PM, Brian Norris wrote: Also, I cannot find any documentation for this IP block even if I search through Quartus/QSys, is there any proper documentation available anywhere? I never found proper documentation, but I didn't look too hard. I've mostly been going off of Viet's comments and code. Me neither, and I looked through the altera stuff in fact. I'm trying to learn whether this is just an Soft IP, in which case it certainly can be fixed ; or if there is actually some chip shipping with this crap synthesised into actual silicon. But FWIW, I did find some relevant info for the peculiar Altera EPCQ flash here: https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/litera ture/ hb/cfg/cfg_cf52012.pdf Altera EPCS/EPCQ flashes are just rebranded micron flashes, they just have different JEDEC ID and are a bit more expensive. You can find the document at here (https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/ug/ug_embedded_ip.pdf) Chapter 42.Page 407. For the soft IP issue, i've requested hardware engineer to come out the solution. So in the mean way, our driver will NOT support Micron flashes until hardware fix is completed. Hence, i just submitted version 5 for this driver with eliminating micron device support. Hope this version will get ACKed by you. Thanks, Viet Nga -- To unsubscribe from this list: send the line unsubscribe devicetree in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [PATCH v4] mtd:spi-nor: Add Altera Quad SPI Driver
On Thu, Aug 20, 2015 at 3:55 PM, Marek Vasut ma...@denx.de wrote: On Thursday, August 20, 2015 at 09:37:33 AM, Viet Nga Dao wrote: Hi, Hi, On Tuesday, August 18, 2015 at 03:24:44 AM, Brian Norris wrote: I'm not very helpful here, so hopefully Viet can be of more use: Yup :) On Mon, Aug 17, 2015 at 07:53:23PM +0200, Marek Vasut wrote: On Monday, August 17, 2015 at 06:03:38 PM, Brian Norris wrote: Also, I cannot find any documentation for this IP block even if I search through Quartus/QSys, is there any proper documentation available anywhere? I never found proper documentation, but I didn't look too hard. I've mostly been going off of Viet's comments and code. Me neither, and I looked through the altera stuff in fact. I'm trying to learn whether this is just an Soft IP, in which case it certainly can be fixed ; or if there is actually some chip shipping with this crap synthesised into actual silicon. But FWIW, I did find some relevant info for the peculiar Altera EPCQ flash here: https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/litera ture/ hb/cfg/cfg_cf52012.pdf Altera EPCS/EPCQ flashes are just rebranded micron flashes, they just have different JEDEC ID and are a bit more expensive. You can find the document at here (https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literatu re/ug/ug_embedded_ip.pdf) Chapter 42.Page 407. For the soft IP issue, i've requested hardware engineer to come out the solution. That's good :) So in the mean way, our driver will NOT support Micron flashes until hardware fix is completed. This doesn't answer my question, so let me reiterate. Is this controller only Soft IP (as in, FPGA core) or is this controller shipping in some chip as Hard IP (as in, piece of silicon) ? This is new soft IP. Hence, i just submitted version 5 for this driver with eliminating micron device support. Hope this version will get ACKed by you. We'll see. Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe devicetree in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver
On Fri, Aug 21, 2015 at 4:37 AM, Brian Norris computersforpe...@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 devicetree in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [PATCH v4] mtd:spi-nor: Add Altera Quad SPI Driver
Thanks Brian for your reply! On Sat, Jul 25, 2015 at 2:37 AM, Brian Norris computersforpe...@gmail.com wrote: On Wed, Jun 03, 2015 at 12:30:44AM -0700, vn...@altera.com wrote: From: VIET NGA DAO vn...@altera.com Altera Quad SPI Controller is a soft IP which enables access to Altera EPCS, EPCQ and Mircon flash chips. This patch adds driver for these devices. Signed-off-by: VIET NGA DAO vn...@altera.com --- v4: - Add more flash devices support ( EPCQL and Micron) ^^ Unfortunately, I think you've added yourself another burden with this one. Most of the rest actually is looking pretty good, so it's sad to see this hold your driver back. Comments below. - 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 | 49 ++ drivers/mtd/spi-nor/Kconfig|8 + drivers/mtd/spi-nor/Makefile |1 + drivers/mtd/spi-nor/altera-quadspi.c | 568 drivers/mtd/spi-nor/spi-nor.c | 30 + 5 files changed, 656 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 000..2873319 --- /dev/null +++ b/Documentation/devicetree/bindings/mtd/altera-quadspi.txt @@ -0,0 +1,49 @@ +* 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 + 4. Mircon: n25q016-nonjedec, n25q032-nonjedec, n25q064-nonjedec, + n25q128a13-nonjedec, n25q128a11-nonjedec, n25q256a-nonjedec, + n25q256a11-nonjedec, n25q512a-nonjedec, n25q512ax3-nonjedec, + mt25ql512-nonjedec, n25q00-nonjedec, n25q00a11-nonjedec OK, so you're adding a bunch of Micron flashes which already have support via standard DT bindings and spi-nor library code, except now you're adding -nonjedec to all of them. You better have a *really* good reason for this. Are these flash not compatible with the JEDEC READ ID opcode, by which every other system identifies these parts? Or are you adding these names because of limitations in your controller? For the former, I might be able to understand the need, but for the latter, I'm much disinclined to support this. There's got to be a better way. Hi Brian, It is really unfortunate that this controller is not able to read full JEDEC ID. It only can provide 1 byte ID. I did discuss with IP designer about this, but it is really unfortunate that they are not able to fix that issue. Hence it requires software to make changes. + - #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 0x0020, + 0x1400 0x0400; + reg-names = avl_csr, avl_mem; + #address-cells = 1; + #size-cells = 0; + flash0: epcq256@0 { + compatible = altr,epcq256; + #address-cells = 1; + #size-cells = 1
Re: [PATCH] [PATCH v3] mtd:spi-nor: Add Altera Quad SPI Driver
Hi, Could you please help me to review this patch? Thanks On Mon, Mar 16, 2015 at 4:40 PM, Viet Nga Dao vn...@altera.com wrote: On Mon, Mar 16, 2015 at 4:35 PM, Rafał Miłecki zaj...@gmail.com wrote: On 16 March 2015 at 09:16, vn...@altera.com wrote: +static struct flash_device flash_devices[] = { + FLASH_ID(epcq16-nonjedec, 2, 0x15), + FLASH_ID(epcq32-nonjedec, 2, 0x16), + FLASH_ID(epcq64-nonjedec, 2, 0x17), + FLASH_ID(epcq128-nonjedec, 2, 0x18), + FLASH_ID(epcq256-nonjedec, 2, 0x19), + FLASH_ID(epcq512-nonjedec, 2, 0x20), You could probably use EPCQ_OPCODE_ID + + FLASH_ID(epcs16-nonjedec, 1, 0x14), + FLASH_ID(epcs64-nonjedec, 1, 0x16), + FLASH_ID(epcs128-nonjedec, 1, 0x18), And EPCS_OPCODE_ID here. Noted diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index 43bb552..ad0c274 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -683,6 +683,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-nonjedec, INFO(0, 0, 0x1, 32, 0) }, + { epcq32-nonjedec, INFO(0, 0, 0x1, 64, 0) }, + { epcq64-nonjedec, INFO(0, 0, 0x1, 128, 0) }, + { epcq128-nonjedec, INFO(0, 0, 0x1, 256, 0) }, + { epcq256-nonjedec, INFO(0, 0, 0x1, 512, 0) }, + { epcq512-nonjedec, INFO(0, 0, 0x1, 1024, 0) }, + { epcs16-nonjedec, INFO(0, 0, 0x1, 32, 0) }, + { epcs64-nonjedec, INFO(0, 0, 0x1, 128, 0) }, + { epcs128-nonjedec, INFO(0, 0, 0x4, 256, 0) }, { }, }; But mostly, I just wanted to say I like your integration with spi-nor. Nice work :) -- Rafał Thank you. This is all thanks to you and Brian for helpful comments. I learned a lot :) -- To unsubscribe from this list: send the line unsubscribe devicetree in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [PATCH v3] mtd:spi-nor: Add Altera Quad SPI Driver
Hi Brian, Can you help me to review this patch of mine? Thanks, Nga On Mon, Mar 16, 2015 at 4:16 PM, vn...@altera.com wrote: From: VIET NGA DAO vn...@altera.com Altera Quad SPI 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 vn...@altera.com --- 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 | 608 + drivers/mtd/spi-nor/spi-nor.c | 11 + 5 files changed, 673 insertions(+) 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 000..f5bdd35 --- /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 + 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: + + quadspi_controller_0: quadspi@0x0 { + compatible = altr,quadspi-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-nonjedec; + #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 quadspi@0x0 (quadspi_controller_0) diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig index 64a4f0e..b9eed6d 100644 --- a/drivers/mtd/spi-nor/Kconfig +++ b/drivers/mtd/spi-nor/Kconfig @@ -28,4 +28,12 @@ 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_QUADSPI + 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. + endif # MTD_SPI_NOR diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile index 6a7ce14..1a36a72 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
Re: [PATCH] mtd:spi-nor: Add lock and unlock callback functions to struct spi_nor
Hi Brian, On Fri, Mar 20, 2015 at 1:49 AM, Brian Norris computersforpe...@gmail.com wrote: Hi Viet, On Mon, Mar 16, 2015 at 01:15:14AM -0700, vn...@altera.com wrote: From: VIET NGA DAO vn...@altera.com This patch introduces a properly-replaceable spi_nor callback that does flash specific lock and unlock. The existing code for spi_nor_lock and spi_nor_unlock is moved into their own functions which are stm_lock and stm_unlock. I'm curious; is this a complete ripoff of my code [1]? You haven't credited my authorship at all. That's a big no-no. Typically you keep the 'From:' and Signed-off-by of the original author if you're going to modify/redistribute it. (Admittedly, I didn't provide the S-o-b on my informal patch.) Anyway, that's all fine this time, but please avoid doing this in the future; I can fix up the authorship, etc., and apply it, if it gets an Ack/Tested-by from one or more reviewers (e.g., you). BTW, I hope you at least tested this, right? Brian Hi Brian, I am so sorry for this mistake. it is not my intention. :( I am new to kernel driver and up-streaming thing, that is why i do not know the proper way. Yes, please change the authorship to you. Yes, i tested it. [1] http://lists.infradead.org/pipermail/linux-mtd/2015-March/058301.html Signed-off-by: VIET NGA DAO vn...@altera.com --- drivers/mtd/spi-nor/spi-nor.c | 56 --- include/linux/mtd/spi-nor.h | 4 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index b6a5a0c..43bb552 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -369,17 +369,13 @@ erase_err: return ret; } -static int spi_nor_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len) +static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len) { - struct spi_nor *nor = mtd_to_spi_nor(mtd); + struct mtd_info *mtd = nor-mtd; uint32_t offset = ofs; uint8_t status_old, status_new; int ret = 0; - ret = spi_nor_lock_and_prep(nor, SPI_NOR_OPS_LOCK); - if (ret) - return ret; - status_old = read_sr(nor); if (offset mtd-size - (mtd-size / 2)) @@ -402,26 +398,18 @@ static int spi_nor_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len) (status_old (SR_BP2 | SR_BP1 | SR_BP0))) { write_enable(nor); ret = write_sr(nor, status_new); - if (ret) - goto err; } -err: - spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_LOCK); return ret; } -static int spi_nor_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len) +static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len) { - struct spi_nor *nor = mtd_to_spi_nor(mtd); + struct mtd_info *mtd = nor-mtd; uint32_t offset = ofs; uint8_t status_old, status_new; int ret = 0; - ret = spi_nor_lock_and_prep(nor, SPI_NOR_OPS_UNLOCK); - if (ret) - return ret; - status_old = read_sr(nor); if (offset+len mtd-size - (mtd-size / 64)) @@ -444,15 +432,41 @@ static int spi_nor_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len) (status_old (SR_BP2 | SR_BP1 | SR_BP0))) { write_enable(nor); ret = write_sr(nor, status_new); - if (ret) - goto err; } -err: + return ret; +} + +static int spi_nor_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len) +{ + struct spi_nor *nor = mtd_to_spi_nor(mtd); + int ret; + + ret = spi_nor_lock_and_prep(nor, SPI_NOR_OPS_LOCK); + if (ret) + return ret; + + ret = nor-flash_lock(nor, ofs, len); + spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_UNLOCK); return ret; } +static int spi_nor_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len) +{ + struct spi_nor *nor = mtd_to_spi_nor(mtd); + int ret; + + ret = spi_nor_lock_and_prep(nor, SPI_NOR_OPS_UNLOCK); + if (ret) + return ret; + + ret = nor-flash_unlock(nor, ofs, len); + + spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_LOCK); + return ret; +} + /* Used when the _ext_id is two bytes at most */ #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags) \ ((kernel_ulong_t)(struct flash_info) { \ @@ -1045,6 +1059,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode) /* nor protection support for STmicro chips */ if (JEDEC_MFR(info) == CFI_MFR_ST) { + nor-flash_lock = stm_lock; + nor-flash_unlock = stm_unlock; + } + if (nor-flash_lock nor-flash_unlock) { mtd-_lock = spi_nor_lock; mtd-_unlock = spi_nor_unlock; } diff --git a/include/linux
Re: [PATCH] [PATCH v3] mtd:spi-nor: Add Altera Quad SPI Driver
On Mon, Mar 16, 2015 at 4:35 PM, Rafał Miłecki zaj...@gmail.com wrote: On 16 March 2015 at 09:16, vn...@altera.com wrote: +static struct flash_device flash_devices[] = { + FLASH_ID(epcq16-nonjedec, 2, 0x15), + FLASH_ID(epcq32-nonjedec, 2, 0x16), + FLASH_ID(epcq64-nonjedec, 2, 0x17), + FLASH_ID(epcq128-nonjedec, 2, 0x18), + FLASH_ID(epcq256-nonjedec, 2, 0x19), + FLASH_ID(epcq512-nonjedec, 2, 0x20), You could probably use EPCQ_OPCODE_ID + + FLASH_ID(epcs16-nonjedec, 1, 0x14), + FLASH_ID(epcs64-nonjedec, 1, 0x16), + FLASH_ID(epcs128-nonjedec, 1, 0x18), And EPCS_OPCODE_ID here. Noted diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index 43bb552..ad0c274 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -683,6 +683,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-nonjedec, INFO(0, 0, 0x1, 32, 0) }, + { epcq32-nonjedec, INFO(0, 0, 0x1, 64, 0) }, + { epcq64-nonjedec, INFO(0, 0, 0x1, 128, 0) }, + { epcq128-nonjedec, INFO(0, 0, 0x1, 256, 0) }, + { epcq256-nonjedec, INFO(0, 0, 0x1, 512, 0) }, + { epcq512-nonjedec, INFO(0, 0, 0x1, 1024, 0) }, + { epcs16-nonjedec, INFO(0, 0, 0x1, 32, 0) }, + { epcs64-nonjedec, INFO(0, 0, 0x1, 128, 0) }, + { epcs128-nonjedec, INFO(0, 0, 0x4, 256, 0) }, { }, }; But mostly, I just wanted to say I like your integration with spi-nor. Nice work :) -- Rafał Thank you. This is all thanks to you and Brian for helpful comments. I learned a lot :) -- To unsubscribe from this list: send the line unsubscribe devicetree in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
On Fri, Mar 13, 2015 at 3:38 PM, Brian Norris computersforpe...@gmail.com wrote: Hi Viet, On Wed, Feb 11, 2015 at 12:53:00PM +0800, Viet Nga Dao wrote: From: Viet Nga Dao vn...@altera.com 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 vn...@altera.com --- 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 file:Documentation/kbuild/modules.txt. 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 http://www.gnu.org/licenses/. + */ + +#include linux/clk.h +#include linux/delay.h +#include linux/device.h +#include linux/err.h +#include linux/errno.h +#include linux/interrupt.h +#include linux/io.h +#include linux/ioport.h +#include linux/jiffies.h +#include linux/kernel.h +#include linux/log2.h +#include linux/module.h +#include linux/mod_devicetable.h +#include linux/mtd/mtd.h +#include linux/mtd/partitions.h +#include linux/mtd/spi-nor.h +#include linux/mutex.h +#include linux/of.h +#include linux/of_address.h +#include linux/platform_device.h +#include linux/sched.h + +#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
Re: FW: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
On Fri, Mar 13, 2015 at 1:45 PM, Rafał Miłecki zaj...@gmail.com wrote: On 11 March 2015 at 09:41, Viet Nga Dao vn...@altera.com wrote: On Tue, Mar 10, 2015 at 4:09 PM, Viet Nga Dao vn...@altera.com 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 devicetree in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
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 computersforpe...@gmail.com wrote: Hi Viet, On Wed, Feb 11, 2015 at 12:53:00PM +0800, Viet Nga Dao wrote: From: Viet Nga Dao vn...@altera.com 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 vn...@altera.com --- 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 file:Documentation/kbuild/modules.txt. 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 http://www.gnu.org/licenses/. + */ + +#include linux/clk.h +#include linux/delay.h +#include linux/device.h +#include linux/err.h +#include linux/errno.h +#include linux/interrupt.h +#include linux/io.h +#include linux/ioport.h +#include linux/jiffies.h +#include linux/kernel.h +#include linux/log2.h +#include linux/module.h +#include linux/mod_devicetable.h +#include linux/mtd/mtd.h +#include linux/mtd/partitions.h +#include linux/mtd/spi-nor.h +#include linux/mutex.h +#include linux/of.h +#include linux/of_address.h +#include linux/platform_device.h +#include linux/sched.h + +#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
Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
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 vn...@altera.com 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 vn...@altera.com 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 vn...@altera.com wrote: From: Viet Nga Dao vn...@altera.com 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 vn...@altera.com --- 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 file:Documentation/kbuild/modules.txt. + 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
Re: FW: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
Hi Rafal, On Tue, Mar 10, 2015 at 4:09 PM, Viet Nga Dao vn...@altera.com 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 devicetree in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
Hi Rafal, Thanks for your review. On Mon, Mar 9, 2015 at 2:31 PM, Rafał Miłecki zaj...@gmail.com 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 vn...@altera.com wrote: From: Viet Nga Dao vn...@altera.com 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 devicetree in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: FW: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
+ +/* 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 devicetree in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
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 vn...@altera.com 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 vn...@altera.com wrote: From: Viet Nga Dao vn...@altera.com 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 vn...@altera.com --- 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 file:Documentation/kbuild/modules.txt. + 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
Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
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 vn...@altera.com wrote: From: Viet Nga Dao vn...@altera.com 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 vn...@altera.com --- 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 file:Documentation/kbuild/modules.txt. + 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
[PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
From: Viet Nga Dao vn...@altera.com 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 vn...@altera.com --- 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 file:Documentation/kbuild/modules.txt. + 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
Re: [PATCH mtd] mtd:devices: Add Altera EPCQ Driver
Sorry for multiple emails. On Mon, Feb 9, 2015 at 2:42 PM, Viet Nga Dao vn...@altera.com wrote: Please ignore previous 2 emails of mine ^^ On Thu, Feb 5, 2015 at 4:37 AM, Brian Norris computersforpe...@gmail.com wrote: On Tue, Jan 27, 2015 at 02:53:59PM +0800, Viet Nga Dao wrote: On Tue, Jan 20, 2015 at 10:05 AM, Viet Nga Dao vn...@altera.com wrote: On Thu, Jan 15, 2015 at 5:27 PM, Viet Nga Dao vn...@altera.com wrote: On Tue, Jan 13, 2015 at 11:33 AM, Brian Norris computersforpe...@gmail.com wrote: On Thu, Dec 18, 2014 at 12:23:16AM -0800, vn...@altera.com wrote: From: Viet Nga Dao vn...@altera.com That is why if rewrite the drivers to follow spi-nor structure, it will require extra decoding works for the only few used opcodes. I think you'd only have some very trivial work here. There would be some small work to reintroduce a properly-replaceable ID table, and callbacks like -lock() and -unlock(), but those should be implemented in spi-nor.c sooner or later anyway. I am trying to modify the code to follow your recommendation. However, for lock and unlock functions, i have to implement it in spi_nor.c , am i right? If yes, currently we already have existing spi_nor_lock and spi_nor_unlock functions but they could not apply for my driver. Should i create a new functions named altera_epcq_lock and unlock and then map it to callback functions or i should the put those code sunder existing spi_nor_lock/unlock? We probably want a replaceable spi_nor callback that does the flash-specific unlock. spi_nor_lock/unlock would then call the nor-unlock() / nor-lock() functions for you, when present. Some of the existing code should move into their own spi_nor_st_lock() / spi_nor_st_unlock() functions. This changes will be made by me or maintainers? If current this functions are not supported, i decide not to implement my lock, unlock function as well. I made the changes at my side here. However, there are something i want to confirm with you: - in spi-nor.h, i add 2 functions. _lock and _unlock instead of lock and unlock. It is because we already have struct mutex lock with the same name. int (*_lock)(struct spi_nor *nor, loff_t offs, uint64_t len); int (*_unlock)(struct spi_nor *nor, loff_t offs, uint64_t len); - in spi-nor.c, i change the current spi_nor_lock and spi_nor_unlock to stmicro_spi_nor_lock and stmirco_spi_nor_unlock - in spi-nor.c, i add 2 functions: static int spi_nor_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len) { struct spi_nor *nor = mtd_to_spi_nor(mtd); int ret = 0; ret = spi_nor_lock_and_prep(nor, SPI_NOR_OPS_LOCK); if (ret) return ret; /* Wait until finished previous command */ ret = wait_till_ready(nor); if (ret) goto err; ret = nor-_lock(nor, ofs, len); err: spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_LOCK); return ret; } static int spi_nor_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len) { struct spi_nor *nor = mtd_to_spi_nor(mtd); int ret = 0; ret = spi_nor_lock_and_prep(nor, SPI_NOR_OPS_LOCK); if (ret) return ret; /* Wait until finished previous command */ ret = wait_till_ready(nor); if (ret) goto err; ret = nor-_unlock(nor, ofs, len); err: spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_LOCK); return ret; } - Then under spi_nor_scan function, i add these few lines: /* nor protection support for STmicro chips */ if (JEDEC_MFR(info-jedec_id) == CFI_MFR_ST) { mtd-_lock = stmicro_spi_nor_lock; mtd-_unlock = stmicro_spi_nor_unlock; } else { mtd-_lock = spi_nor_lock; mtd-_unlock = spi_nor_unlock; } What is your comment? diff --git a/Documentation/devicetree/bindings/mtd/altera_epcq.txt b/Documentation/devicetree/bindings/mtd/altera_epcq.txt new file mode 100644 index 000..d14f50e --- /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: These subnodes definitely require a 'compatible' string. Perhaps they should be used to differentiate EPCS vs. EPCQ. Does is-epcs really need to be in the top-level controller node? + - reg: Should contain the flash id Should, or must? (This question
Re: [PATCH mtd] mtd:devices: Add Altera EPCQ Driver
Please ignore previous 2 emails of mine ^^ On Thu, Feb 5, 2015 at 4:37 AM, Brian Norris computersforpe...@gmail.com wrote: On Tue, Jan 27, 2015 at 02:53:59PM +0800, Viet Nga Dao wrote: On Tue, Jan 20, 2015 at 10:05 AM, Viet Nga Dao vn...@altera.com wrote: On Thu, Jan 15, 2015 at 5:27 PM, Viet Nga Dao vn...@altera.com wrote: On Tue, Jan 13, 2015 at 11:33 AM, Brian Norris computersforpe...@gmail.com wrote: On Thu, Dec 18, 2014 at 12:23:16AM -0800, vn...@altera.com wrote: From: Viet Nga Dao vn...@altera.com That is why if rewrite the drivers to follow spi-nor structure, it will require extra decoding works for the only few used opcodes. I think you'd only have some very trivial work here. There would be some small work to reintroduce a properly-replaceable ID table, and callbacks like -lock() and -unlock(), but those should be implemented in spi-nor.c sooner or later anyway. I am trying to modify the code to follow your recommendation. However, for lock and unlock functions, i have to implement it in spi_nor.c , am i right? If yes, currently we already have existing spi_nor_lock and spi_nor_unlock functions but they could not apply for my driver. Should i create a new functions named altera_epcq_lock and unlock and then map it to callback functions or i should the put those code sunder existing spi_nor_lock/unlock? We probably want a replaceable spi_nor callback that does the flash-specific unlock. spi_nor_lock/unlock would then call the nor-unlock() / nor-lock() functions for you, when present. Some of the existing code should move into their own spi_nor_st_lock() / spi_nor_st_unlock() functions. This changes will be made by me or maintainers? If current this functions are not supported, i decide not to implement my lock, unlock function as well. diff --git a/Documentation/devicetree/bindings/mtd/altera_epcq.txt b/Documentation/devicetree/bindings/mtd/altera_epcq.txt new file mode 100644 index 000..d14f50e --- /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: These subnodes definitely require a 'compatible' string. Perhaps they should be used to differentiate EPCS vs. EPCQ. Does is-epcs really need to be in the top-level controller node? + - reg: Should contain the flash id Should, or must? (This question is relevant, because you seem to make it optional in your code.) And what does the flash ID mean? It seems like you're using as a chip-select or bank index. Yes, this is used for chip select. It is required, not optional. This to ID for each flash in the device OK, so correct the language here and enforce this in your driver. Right now, you don't fail gracefully if this is missing. Sorry, you are right. This field is unnecessary for my driver. Instead, compatible is replaced. I will update it with 2nd version. + if (sector_start num_sectors-(num_sectors / 4)) + sr_bp = __ilog2_u32(num_sectors); + else if (sector_start num_sectors-(num_sectors / 8)) + sr_bp = __ilog2_u32(num_sectors) - 1; + else if (sector_start num_sectors-(num_sectors / 16)) + sr_bp = __ilog2_u32(num_sectors) - 2; + else if (sector_start num_sectors-(num_sectors / 32)) + sr_bp = __ilog2_u32(num_sectors) - 3; + else if (sector_start num_sectors-(num_sectors / 64)) + sr_bp = __ilog2_u32(num_sectors) - 4; + else if (sector_start num_sectors-(num_sectors / 128)) + sr_bp = __ilog2_u32(num_sectors) - 5; + else if (sector_start num_sectors-(num_sectors / 256)) + sr_bp = __ilog2_u32(num_sectors) - 6; + else if (sector_start num_sectors-(num_sectors / 512)) + sr_bp = __ilog2_u32(num_sectors) - 7; + else if (sector_start num_sectors-(num_sectors / 1024)) + sr_bp = __ilog2_u32(num_sectors) - 8; + else + sr_bp = 0; /* non area protected */ Yikes, that's ugly! And I'm not sure it matches the EPCQ doc I found. I'm pretty sure you can rewrite
Re: [PATCH mtd] mtd:devices: Add Altera EPCQ Driver
On Tue, Jan 20, 2015 at 10:05 AM, Viet Nga Dao vn...@altera.com wrote: On Thu, Jan 15, 2015 at 5:27 PM, Viet Nga Dao vn...@altera.com wrote: On Tue, Jan 13, 2015 at 11:33 AM, Brian Norris computersforpe...@gmail.com wrote: On Thu, Dec 18, 2014 at 12:23:16AM -0800, vn...@altera.com wrote: From: Viet Nga Dao vn...@altera.com 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 vn...@altera.com This drivers seems awfully similar to (and so I infer it is likely copy-and-pasted from) m25p80.c / spi-nor.c. Do you think it can be rewritten as a SPI NOR driver, under drivers/mtd/spi-nor/ ? It looks like these flash share most (all?) the same basic opcodes. For Altera EPCQ flashes, almost operations are performed underline hardware. Right, that's understandable. But that's what spi-nor.c was designed for: implementing hardware-specific functionality that is targeted directly at SPI flash. Did you take a look at the callbacks available in 'struct spi_nor'? Software only able to perform the following through registers: - read status register - read id - write status registers bit SR_BP0,SR_BP1, SR_BP2,SR_BP3, SR_TB (http://www.altera.com.my/literature/hb/cfg/cfg_cf52012.pdf) OK For read/write data: all the operations like QUAD_READ/WRITE, FAST_READ/WRITE are handled by hardware as well. From software point of view, there is no difference between these 2 modes. OK, so you don't have to hook up the dual/quad mode infrastructure. And you'd just implement dead-simple spi_nor.{read,write,erase}() callbacks. That is why if rewrite the drivers to follow spi-nor structure, it will require extra decoding works for the only few used opcodes. I think you'd only have some very trivial work here. There would be some small work to reintroduce a properly-replaceable ID table, and callbacks like -lock() and -unlock(), but those should be implemented in spi-nor.c sooner or later anyway. I am trying to modify the code to follow your recommendation. However, for lock and unlock functions, i have to implement it in spi_nor.c , am i right? If yes, currently we already have existing spi_nor_lock and spi_nor_unlock functions but they could not apply for my driver. Should i create a new functions named altera_epcq_lock and unlock and then map it to callback functions or i should the put those code sunder existing spi_nor_lock/unlock? Anyway, if you take another look at spi-nor.{c,h} and determine that it's too difficult, then I suppose I don't mind accepting your driver under its current design. Your hardware is pretty esoteric anyway, the driver is still pretty simple, and it'll never be supporting any common SPI NOR vendors (right?), so it's not too big of a maintenance problem, I expect. But I do want to make sure that we don't copy/paste to repeat the mistakes of old drivers. As I noted already, your driver inherited some of the quirks of the old m25p80.c code. So please, give drivers/mtd/spi-nor/ another look, and then we can resume this discussion. --- .../devicetree/bindings/mtd/altera_epcq.txt| 45 ++ drivers/mtd/devices/Kconfig| 12 + drivers/mtd/devices/Makefile |2 +- drivers/mtd/devices/altera_epcq.c | 804 drivers/mtd/devices/altera_epcq.h | 130 5 files changed, 992 insertions(+), 1 deletions(-) create mode 100644 Documentation/devicetree/bindings/mtd/altera_epcq.txt create mode 100644 drivers/mtd/devices/altera_epcq.c create mode 100644 drivers/mtd/devices/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..d14f50e --- /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: These subnodes definitely require a 'compatible' string. Perhaps they should be used to differentiate EPCS vs. EPCQ. Does is-epcs really need to be in the top-level controller node? + - reg: Should contain the flash id Should, or must? (This question is relevant, because you seem to make it optional in your code
Re: [PATCH mtd] mtd:devices: Add Altera EPCQ Driver
On Tue, Jan 20, 2015 at 10:05 AM, Viet Nga Dao vn...@altera.com wrote: On Thu, Jan 15, 2015 at 5:27 PM, Viet Nga Dao vn...@altera.com wrote: On Tue, Jan 13, 2015 at 11:33 AM, Brian Norris computersforpe...@gmail.com wrote: On Thu, Dec 18, 2014 at 12:23:16AM -0800, vn...@altera.com wrote: From: Viet Nga Dao vn...@altera.com 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 vn...@altera.com This drivers seems awfully similar to (and so I infer it is likely copy-and-pasted from) m25p80.c / spi-nor.c. Do you think it can be rewritten as a SPI NOR driver, under drivers/mtd/spi-nor/ ? It looks like these flash share most (all?) the same basic opcodes. For Altera EPCQ flashes, almost operations are performed underline hardware. Software only able to perform the following through registers: - read status register - read id - write status registers bit SR_BP0,SR_BP1, SR_BP2,SR_BP3, SR_TB (http://www.altera.com.my/literature/hb/cfg/cfg_cf52012.pdf) For read/write data: all the operations like QUAD_READ/WRITE, FAST_READ/WRITE are handled by hardware as well. From software point of view, there is no difference between these 2 modes. That is why if rewrite the drivers to follow spi-nor structure, it will require extra decoding works for the only few used opcodes. Is it OK to remain this driver structure? Can someone please reply my question as it is been a while? Viet Nga -- To unsubscribe from this list: send the line unsubscribe devicetree in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH mtd] mtd:devices: Add Altera EPCQ Driver
On Thu, Jan 15, 2015 at 5:27 PM, Viet Nga Dao vn...@altera.com wrote: On Tue, Jan 13, 2015 at 11:33 AM, Brian Norris computersforpe...@gmail.com wrote: On Thu, Dec 18, 2014 at 12:23:16AM -0800, vn...@altera.com wrote: From: Viet Nga Dao vn...@altera.com 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 vn...@altera.com This drivers seems awfully similar to (and so I infer it is likely copy-and-pasted from) m25p80.c / spi-nor.c. Do you think it can be rewritten as a SPI NOR driver, under drivers/mtd/spi-nor/ ? It looks like these flash share most (all?) the same basic opcodes. For Altera EPCQ flashes, almost operations are performed underline hardware. Software only able to perform the following through registers: - read status register - read id - write status registers bit SR_BP0,SR_BP1, SR_BP2,SR_BP3, SR_TB (http://www.altera.com.my/literature/hb/cfg/cfg_cf52012.pdf) For read/write data: all the operations like QUAD_READ/WRITE, FAST_READ/WRITE are handled by hardware as well. From software point of view, there is no difference between these 2 modes. That is why if rewrite the drivers to follow spi-nor structure, it will require extra decoding works for the only few used opcodes. Is it OK to remain this driver structure? --- .../devicetree/bindings/mtd/altera_epcq.txt| 45 ++ drivers/mtd/devices/Kconfig| 12 + drivers/mtd/devices/Makefile |2 +- drivers/mtd/devices/altera_epcq.c | 804 drivers/mtd/devices/altera_epcq.h | 130 5 files changed, 992 insertions(+), 1 deletions(-) create mode 100644 Documentation/devicetree/bindings/mtd/altera_epcq.txt create mode 100644 drivers/mtd/devices/altera_epcq.c create mode 100644 drivers/mtd/devices/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..d14f50e --- /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: These subnodes definitely require a 'compatible' string. Perhaps they should be used to differentiate EPCS vs. EPCQ. Does is-epcs really need to be in the top-level controller node? + - reg: Should contain the flash id Should, or must? (This question is relevant, because you seem to make it optional in your code.) And what does the flash ID mean? It seems like you're using as a chip-select or bank index. + - #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 { + reg = 0; + #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/devices/Kconfig b/drivers
Re: [PATCH mtd] mtd:devices: Add Altera EPCQ Driver
On Tue, Jan 13, 2015 at 11:33 AM, Brian Norris computersforpe...@gmail.com wrote: On Thu, Dec 18, 2014 at 12:23:16AM -0800, vn...@altera.com wrote: From: Viet Nga Dao vn...@altera.com 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 vn...@altera.com This drivers seems awfully similar to (and so I infer it is likely copy-and-pasted from) m25p80.c / spi-nor.c. Do you think it can be rewritten as a SPI NOR driver, under drivers/mtd/spi-nor/ ? It looks like these flash share most (all?) the same basic opcodes. For Altera EPCQ flashes, almost operations are performed underline hardware. Software only able to perform the following through registers: - read status register - read id - write status registers bit SR_BP0,SR_BP1, SR_BP2,SR_BP3, SR_TB (http://www.altera.com.my/literature/hb/cfg/cfg_cf52012.pdf) For read/write data: all the operations like QUAD_READ/WRITE, FAST_READ/WRITE are handled by hardware as well. From software point of view, there is no difference between these 2 modes. That is why if rewrite the drivers to follow spi-nor structure, it will require extra decoding works for the only few used opcodes. --- .../devicetree/bindings/mtd/altera_epcq.txt| 45 ++ drivers/mtd/devices/Kconfig| 12 + drivers/mtd/devices/Makefile |2 +- drivers/mtd/devices/altera_epcq.c | 804 drivers/mtd/devices/altera_epcq.h | 130 5 files changed, 992 insertions(+), 1 deletions(-) create mode 100644 Documentation/devicetree/bindings/mtd/altera_epcq.txt create mode 100644 drivers/mtd/devices/altera_epcq.c create mode 100644 drivers/mtd/devices/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..d14f50e --- /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: These subnodes definitely require a 'compatible' string. Perhaps they should be used to differentiate EPCS vs. EPCQ. Does is-epcs really need to be in the top-level controller node? + - reg: Should contain the flash id Should, or must? (This question is relevant, because you seem to make it optional in your code.) And what does the flash ID mean? It seems like you're using as a chip-select or bank index. + - #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 { + reg = 0; + #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/devices/Kconfig b/drivers/mtd/devices/Kconfig index c49d0b1..020b864 100644 --- a/drivers/mtd/devices/Kconfig +++ b/drivers/mtd/devices/Kconfig
RE: [PATCH mtd] mtd:devices: Add Altera EPCQ Driver
Hi Brian, For Altera EPCQ flashes, almost operations are performed underline hardware. Software only able to perform the following through registers: - read status register - read id - write status registers bit SR_BP0,SR_BP1, SR_BP2,SR_BP3, SR_TB (http://www.altera.com.my/literature/hb/cfg/cfg_cf52012.pdf) For read/write data: all the operations like QUAD_READ/WRITE, FAST_READ/WRITE are handled by hardware as well. From software point of view, there is no difference between these 2 modes. That is why if rewrite the drivers to follow spi-nor structure, it will require extra decoding works for the only few used opcodes. Thanks, Viet Nga -Original Message- From: Brian Norris [mailto:computersforpe...@gmail.com] Sent: Tuesday, January 13, 2015 11:33 AM To: Viet Nga Dao Cc: dw...@infradead.org; linux-...@lists.infradead.org; linux-ker...@vger.kernel.org; devicetree@vger.kernel.org; ngach...@gmail.com Subject: Re: [PATCH mtd] mtd:devices: Add Altera EPCQ Driver On Thu, Dec 18, 2014 at 12:23:16AM -0800, vn...@altera.com wrote: From: Viet Nga Dao vn...@altera.com 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 vn...@altera.com This drivers seems awfully similar to (and so I infer it is likely copy-and-pasted from) m25p80.c / spi-nor.c. Do you think it can be rewritten as a SPI NOR driver, under drivers/mtd/spi-nor/ ? It looks like these flash share most (all?) the same basic opcodes. --- .../devicetree/bindings/mtd/altera_epcq.txt| 45 ++ drivers/mtd/devices/Kconfig| 12 + drivers/mtd/devices/Makefile |2 +- drivers/mtd/devices/altera_epcq.c | 804 drivers/mtd/devices/altera_epcq.h | 130 5 files changed, 992 insertions(+), 1 deletions(-) create mode 100644 Documentation/devicetree/bindings/mtd/altera_epcq.txt create mode 100644 drivers/mtd/devices/altera_epcq.c create mode 100644 drivers/mtd/devices/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..d14f50e --- /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: These subnodes definitely require a 'compatible' string. Perhaps they should be used to differentiate EPCS vs. EPCQ. Does is-epcs really need to be in the top-level controller node? + - reg: Should contain the flash id Should, or must? (This question is relevant, because you seem to make it optional in your code.) And what does the flash ID mean? It seems like you're using as a chip-select or bank index. + - #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 { + reg = 0; + #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
RE: [PATCH mtd] mtd:devices: Add Altera EPCQ Driver
Hi Linux Community, It is been a while since I submitted this patch. Could you please help me to review this driver? Thanks and Regards, Viet Nga Dao -Original Message- From: Viet Nga Dao Sent: Thursday, December 18, 2014 4:23 PM To: dw...@infradead.org; computersforpe...@gmail.com Cc: linux-...@lists.infradead.org; linux-ker...@vger.kernel.org; devicetree@vger.kernel.org; ngach...@gmail.com; Viet Nga Dao Subject: [PATCH mtd] mtd:devices: Add Altera EPCQ Driver From: Viet Nga Dao vn...@altera.com 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 vn...@altera.com --- .../devicetree/bindings/mtd/altera_epcq.txt| 45 ++ drivers/mtd/devices/Kconfig| 12 + drivers/mtd/devices/Makefile |2 +- drivers/mtd/devices/altera_epcq.c | 804 drivers/mtd/devices/altera_epcq.h | 130 5 files changed, 992 insertions(+), 1 deletions(-) create mode 100644 Documentation/devicetree/bindings/mtd/altera_epcq.txt create mode 100644 drivers/mtd/devices/altera_epcq.c create mode 100644 drivers/mtd/devices/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..d14f50e --- /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: + - reg: Should contain the flash id + - #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 { + reg = 0; + #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/devices/Kconfig b/drivers/mtd/devices/Kconfig index c49d0b1..020b864 100644 --- a/drivers/mtd/devices/Kconfig +++ b/drivers/mtd/devices/Kconfig @@ -218,6 +218,18 @@ config MTD_ST_SPI_FSM SPI Fast Sequence Mode (FSM) Serial Flash Controller and support for a subset of connected Serial Flash devices. +config MTD_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 file:Documentation/kbuild/modules.txt. + if MTD_DOCG3 config BCH_CONST_M default 14 diff --git a/drivers/mtd/devices/Makefile b/drivers/mtd/devices/Makefile index f0b0e61..b429c4d 100644 --- a/drivers/mtd/devices/Makefile +++ b/drivers/mtd/devices/Makefile @@ -16,6 +16,6 @@ obj