Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver
On Thu, Aug 20, 2015 at 2:55 PM, wrote: > From: VIET NGA DAO > > 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 > > --- > 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 linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [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 linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver
On Fri, Aug 21, 2015 at 4:37 AM, Brian Norris wrote: > On Thu, Aug 20, 2015 at 05:18:14PM +0800, Viet Nga Dao wrote: >> You might misunderstand the hardware problem i mention here. This soft >> IP controller is able to provide the ID for our Altera EPCS/EPCQ flash >> chips, which are non JEDEC chips. As from EPCQ device data sheet >> (https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/hb/cfg/cfg_cf52012.pdf), >> the device ID is 8 bit data. > > 8 bits of data is vastly insufficient for uniquely identifying a flash. > This is not extendible and is not really a candidate for inclusion in > mainline, unless it's somehow guaranteed that these flash can only be > used with your controller (and I'm not sure how you would do that). > Otherwise, you need to augment every flash with more out-of-band > device-tree information. > >> The remaining problem here is that this >> controller is able to support Micron chips but it currently has >> limitation in providing only 8 bit ID, which is not full JEDEC ID for >> Micron chips. > > You're just proving my point. I will not support "READ ID" detection > that is based on only 8 bits of ID. > >> Hence, we are asking hardware engineer to find out the >> solution so that the driver does not need to do any dirty hacking. > > OK, then I wish your hardware team great speed. > >> And >> so, this table should still be here even hardware fix will take place >> or not. > > I'm not sure how you come to this conclusion. > I have this conclusion is because Altera EPCQ/EPCS flashes are non JEDEC. Thus on the spi_device_id table, the ID in the INFO struct must be filled up with zeros in order to matches the current framework. However, since i still persist to have the device id check in my driver, as suggested by Rash, I should implement it locally in my driver. And this table is the look up table for the device ID of supported flashes. Or maybe you can give me any other better idea? > BTW, to reiterate one other question that I feel wasn't adequately > answered: what does the full ID string look like for these EPCS/EPCQ > flash chips? Not the ID as seen by your limited controller, but the ID > that can be reported by the flash chip. Is it really only an 8-bit > number? Or does it have a longer string that your controller just can't > read? > Yes, As you can refer to page 32 of EPCQ flash datasheet (https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/hb/cfg/cfg_cf52012.pdf), "This operation reads the 8-bit device identification of the EPCQ device from the DATA1 output pin". Table 29 lists the EPCQ device identifications Nga -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [PATCH v5] mtd:spi-nor: Add Altera Quad SPI Driver
On Thu, Aug 20, 2015 at 4:52 PM, Marek Vasut 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 wrote:
>> > On Thursday, August 20, 2015 at 08:55:05 AM, vn...@altera.com wrote:
>> >> From: VIET NGA DAO
>> >>
>> >> 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
>> >>
>> >> ---
>> >> 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-
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 wrote:
> On Thu, Aug 20, 2015 at 4:03 PM, Marek Vasut wrote:
>> On Thursday, August 20, 2015 at 08:55:05 AM, vn...@altera.com wrote:
>>> From: VIET NGA DAO
>>>
>>> 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
>>>
>>> ---
>>> 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 {
>>> +
Re: [PATCH] [PATCH v4] mtd:spi-nor: Add Altera Quad SPI Driver
On Thu, Aug 20, 2015 at 3:55 PM, Marek Vasut 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 linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [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 linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [PATCH v4] mtd:spi-nor: Add Altera Quad SPI Driver
On Tue, Aug 18, 2015 at 2:55 PM, Viet Nga Dao 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; devicet...@vger.kernel.org; > Rafa?? Mi??ecki; linux-kernel@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 linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [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; devicet...@vger.kernel.org; Rafa?? Mi??ecki; linux-kernel@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 linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [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 linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [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 linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [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 linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [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 wrote: > On Wed, Jun 03, 2015 at 12:30:44AM -0700, vn...@altera.com wrote: >> From: VIET NGA DAO >> >> 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 >> >> --- >> 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,
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,
It has been long time since this patch was up. Could you please help
me to review?
Thanks,
Viet Nga
On Thu, Apr 23, 2015 at 2:39 PM, Nga Chi wrote:
> Hi Brian,
> Could you please spend some time help me to review this patch? It has
> been more than 1 month and i really hope can get this patch upstream
> by end of this month.
> Thanks,
> Viet Nga
>
> On Mon, Mar 16, 2015 at 4:16 PM, wrote:
>> From: VIET NGA DAO
>>
>> 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
>>
>> ---
>> 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";
>> + r
Re: [PATCH] [PATCH v3] mtd:spi-nor: Add Altera Quad SPI Driver
Hi,
It has been long time since this patch was up. Could you please help
me to review?
Thanks,
Viet Nga
On Thu, Apr 23, 2015 at 2:39 PM, Nga Chi ngach...@gmail.com wrote:
Hi Brian,
Could you please spend some time help me to review this patch? It has
been more than 1 month and i really hope can get this patch upstream
by end of this month.
Thanks,
Viet 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
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 wrote:
> On Mon, Mar 16, 2015 at 4:35 PM, Rafał Miłecki wrote:
>> On 16 March 2015 at 09:16, 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 linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [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 linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [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, wrote:
> From: VIET NGA DAO
>
> 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
>
> ---
> 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
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
wrote:
> Hi Viet,
>
> On Mon, Mar 16, 2015 at 01:15:14AM -0700, vn...@altera.com wrote:
>> From: VIET NGA DAO
>>
>> 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
>> ---
>> 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
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 v2] mtd:spi-nor: Add Altera EPCQ Driver
On Fri, Mar 13, 2015 at 3:38 PM, Brian Norris
wrote:
> Hi Viet,
>
> On Wed, Feb 11, 2015 at 12:53:00PM +0800, Viet Nga Dao wrote:
>> From: Viet Nga Dao
>>
>> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
>> EPCS flash chips. This patch adds driver for these devices.
>>
>> Signed-off-by: VIET NGA DAO
>>
>> ---
>> v2:
>> - Change to spi_nor structure
>> - Add lock and unlock functions for spi_nor
>> - Simplify the altera_epcq_lock function
>> - Replace reg by compatible in device tree
>> ---
>> .../devicetree/bindings/mtd/altera_epcq.txt| 45 ++
>> drivers/mtd/spi-nor/Kconfig| 12 +
>> drivers/mtd/spi-nor/Makefile |1 +
>> drivers/mtd/spi-nor/altera_epcq.c | 507
>>
>> drivers/mtd/spi-nor/altera_epcq.h | 116 +
>> drivers/mtd/spi-nor/spi-nor.c | 86 -
>> include/linux/mtd/spi-nor.h|3 +-
>> 7 files changed, 764 insertions(+), 6 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/mtd/altera_epcq.txt
>> create mode 100644 drivers/mtd/spi-nor/altera_epcq.c
>> create mode 100644 drivers/mtd/spi-nor/altera_epcq.h
>
> Please split this up into at least 2 patches, if not 3. One or more for
> the spi-nor changes, split logically; one for your new driver; and maybe
> a separate one for the binding documentation.
>
> Also, as Rafal noted, your patche is whitespace damaged, so I can't
> apply or build it.
>
> Please make sure you can apply, build, and run your patch on top of
> l2-mtd.git:
>
> http://git.infradead.org/l2-mtd.git
> git://git.infradead.org/l2-mtd.git
>
> ...
>> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
>> index 64a4f0e..83178b9 100644
>> --- a/drivers/mtd/spi-nor/Kconfig
>> +++ b/drivers/mtd/spi-nor/Kconfig
>> @@ -28,4 +28,16 @@ config SPI_FSL_QUADSPI
>>This enables support for the Quad SPI controller in master mode.
>>We only connect the NOR to this controller now.
>>
>> +config SPI_ALTERA_EPCQ
>> +tristate "Support Altera EPCQ/EPCS Flash chips"
>> +depends on OF
>> +help
>> + This enables access to Altera EPCQ/EPCS flash chips, used for data
>> + storage. See the driver source for the current list,
>> + or to add other chips.
>> +
>> + If you want to compile this driver as a module ( = code which can be
>> + inserted in and removed from the running kernel whenever you want),
>> + say M here and read .
>
> I don't think you need to explain what a module is here. Look at other
> Kconfig entries as examples, but I think you can just drop that last
> paragraph.
>
>> +
>> endif # MTD_SPI_NOR
> ...
>> diff --git a/drivers/mtd/spi-nor/altera_epcq.c
>> b/drivers/mtd/spi-nor/altera_epcq.c
>> new file mode 100644
>> index 000..9db0d05
>> --- /dev/null
>> +++ b/drivers/mtd/spi-nor/altera_epcq.c
>> @@ -0,0 +1,507 @@
>> +/*
>> + * Copyright (C) 2014 Altera Corporation. All rights reserved
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along
>> with
>> + * this program. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include "altera_epcq.h"
>> +
>> +static int altera_epcq_write_reg(struct spi_nor *nor, u8 opcode, u8 *val,
>> + int len, int wr_en)
>> +{
>> +return 0;
>> +}
>> +
>> +static int altera_epcq_read_reg(struct spi_nor *nor, u8 opcode, u8
Re: [PATCH] [PATCH v3] mtd:spi-nor: Add Altera Quad SPI Driver
On Mon, Mar 16, 2015 at 4:35 PM, Rafał Miłecki wrote:
> On 16 March 2015 at 09:16, 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 linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [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 linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
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: [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
wrote:
> Hi Viet,
>
> On Wed, Feb 11, 2015 at 12:53:00PM +0800, Viet Nga Dao wrote:
>> From: Viet Nga Dao
>>
>> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
>> EPCS flash chips. This patch adds driver for these devices.
>>
>> Signed-off-by: VIET NGA DAO
>>
>> ---
>> v2:
>> - Change to spi_nor structure
>> - Add lock and unlock functions for spi_nor
>> - Simplify the altera_epcq_lock function
>> - Replace reg by compatible in device tree
>> ---
>> .../devicetree/bindings/mtd/altera_epcq.txt| 45 ++
>> drivers/mtd/spi-nor/Kconfig| 12 +
>> drivers/mtd/spi-nor/Makefile |1 +
>> drivers/mtd/spi-nor/altera_epcq.c | 507
>>
>> drivers/mtd/spi-nor/altera_epcq.h | 116 +
>> drivers/mtd/spi-nor/spi-nor.c | 86 -
>> include/linux/mtd/spi-nor.h|3 +-
>> 7 files changed, 764 insertions(+), 6 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/mtd/altera_epcq.txt
>> create mode 100644 drivers/mtd/spi-nor/altera_epcq.c
>> create mode 100644 drivers/mtd/spi-nor/altera_epcq.h
>
> Please split this up into at least 2 patches, if not 3. One or more for
> the spi-nor changes, split logically; one for your new driver; and maybe
> a separate one for the binding documentation.
>
> Also, as Rafal noted, your patche is whitespace damaged, so I can't
> apply or build it.
>
> Please make sure you can apply, build, and run your patch on top of
> l2-mtd.git:
>
> http://git.infradead.org/l2-mtd.git
> git://git.infradead.org/l2-mtd.git
>
> ...
>> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
>> index 64a4f0e..83178b9 100644
>> --- a/drivers/mtd/spi-nor/Kconfig
>> +++ b/drivers/mtd/spi-nor/Kconfig
>> @@ -28,4 +28,16 @@ config SPI_FSL_QUADSPI
>>This enables support for the Quad SPI controller in master mode.
>>We only connect the NOR to this controller now.
>>
>> +config SPI_ALTERA_EPCQ
>> +tristate "Support Altera EPCQ/EPCS Flash chips"
>> +depends on OF
>> +help
>> + This enables access to Altera EPCQ/EPCS flash chips, used for data
>> + storage. See the driver source for the current list,
>> + or to add other chips.
>> +
>> + If you want to compile this driver as a module ( = code which can be
>> + inserted in and removed from the running kernel whenever you want),
>> + say M here and read .
>
> I don't think you need to explain what a module is here. Look at other
> Kconfig entries as examples, but I think you can just drop that last
> paragraph.
>
>> +
>> endif # MTD_SPI_NOR
> ...
>> diff --git a/drivers/mtd/spi-nor/altera_epcq.c
>> b/drivers/mtd/spi-nor/altera_epcq.c
>> new file mode 100644
>> index 000..9db0d05
>> --- /dev/null
>> +++ b/drivers/mtd/spi-nor/altera_epcq.c
>> @@ -0,0 +1,507 @@
>> +/*
>> + * Copyright (C) 2014 Altera Corporation. All rights reserved
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along
>> with
>> + * this program. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include "altera_epcq.h"
>> +
>> +static int altera_epcq_write_reg(struct spi_nor *nor, u8 opcode, u8 *val,
>> + int len, int wr_en)
>> +{
>> +return 0;
&
Re: FW: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
On Fri, Mar 13, 2015 at 1:45 PM, Rafał Miłecki wrote: > On 11 March 2015 at 09:41, Viet Nga Dao wrote: >> On Tue, Mar 10, 2015 at 4:09 PM, Viet Nga Dao wrote: >>> Ok. I will modify the code the way you suggest. >> >> I just realize that the opcode for RDID is handled by hardware in my >> case, therefore i dont really need to worry about 0xab opcode. > > Nice :) Maybe just put a comment in code so ppl will know (in future) > why we don't strictly follow specs. > > >> Is there any other comments about the modified part i did the lock and >> unlock in spi-nor.c? > > I can't, I don't know spi-nor well enough :| > > -- > Rafał I have to wait for Brian then. Anway, thanks Rafal :) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: FW: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
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 linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
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 wrote:
> Hi,
> It has been nearly 2 weeks since i submitted this patch. Could you
> please help to review?
> Thanks,
>
> On Tue, Feb 17, 2015 at 9:33 AM, Viet Nga Dao wrote:
>> Hi Brian,
>> Could you please help me to review through this 2nd version?
>>
>> On Wed, Feb 11, 2015 at 12:53 PM, Viet Nga Dao wrote:
>>> From: Viet Nga Dao
>>>
>>> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
>>> EPCS flash chips. This patch adds driver for these devices.
>>>
>>> Signed-off-by: VIET NGA DAO
>>>
>>> ---
>>> v2:
>>> - Change to spi_nor structure
>>> - Add lock and unlock functions for spi_nor
>>> - Simplify the altera_epcq_lock function
>>> - Replace reg by compatible in device tree
>>> ---
>>> .../devicetree/bindings/mtd/altera_epcq.txt| 45 ++
>>> drivers/mtd/spi-nor/Kconfig| 12 +
>>> drivers/mtd/spi-nor/Makefile |1 +
>>> drivers/mtd/spi-nor/altera_epcq.c | 507
>>>
>>> drivers/mtd/spi-nor/altera_epcq.h | 116 +
>>> drivers/mtd/spi-nor/spi-nor.c | 86 -
>>> include/linux/mtd/spi-nor.h|3 +-
>>> 7 files changed, 764 insertions(+), 6 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/mtd/altera_epcq.txt
>>> create mode 100644 drivers/mtd/spi-nor/altera_epcq.c
>>> create mode 100644 drivers/mtd/spi-nor/altera_epcq.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/altera_epcq.txt
>>> b/Documentation/devicetree/bindings/mtd/altera_epcq.txt
>>> new file mode 100644
>>> index 000..b6b5e61
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mtd/altera_epcq.txt
>>> @@ -0,0 +1,45 @@
>>> +* MTD Altera EPCQ driver
>>> +
>>> +Required properties:
>>> +- compatible: Should be "altr,epcq-1.0"
>>> +- reg: Address and length of the register set for the device. It contains
>>> + the information of registers in the same order as described by reg-names
>>> +- reg-names: Should contain the reg names
>>> + "csr_base": Should contain the register configuration base address
>>> + "data_base": Should contain the data base address
>>> +- is-epcs: boolean type.
>>> +If present, the device contains EPCS flashes.
>>> +Otherwise, it contains EPCQ flashes.
>>> +- #address-cells: Must be <1>.
>>> +- #size-cells: Must be <0>.
>>> +- flash device tree subnode, there must be a node with the following
>>> fields:
>>> +- compatible: Should contain the flash name
>>> +- #address-cells: please refer to /mtd/partition.txt
>>> +- #size-cells: please refer to /mtd/partition.txt
>>> +For partitions inside each flash, please refer to /mtd/partition.txt
>>> +
>>> +Example:
>>> +
>>> +epcq_controller_0: epcq@0x0 {
>>> +compatible = "altr,epcq-1.0";
>>> +reg = <0x0001 0x 0x0020>,
>>> +<0x 0x 0x0200>;
>>> +reg-names = "csr_base", "data_base";
>>> +#address-cells = <1>;
>>> +#size-cells = <0>;
>>> +flash0: epcq256@0 {
>>> +compatible = "epcq256";
>>> +#address-cells = <1>;
>>> +#size-cells = <1>;
>>> +partition@0 {
>>> +/* 16 MB for raw data. */
>>> +label = "EPCQ Flash 0 raw data";
>>> +reg = <0x0 0x100>;
>>> +};
>>> +partition@100 {
>>> +/* 16 MB for jffs2 data. */
>>> +label = "EPCQ Flash 0 JFFS 2";
>>> +reg = <0x100 0x100>;
>>> +};
>>> +};
>>> +}; //end epcq@0x0 (epcq_controller_0)
>>> diff --git a/driver
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 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 _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 _nor_ids[tmp];
>>>>
>>>> This is the part I don't like. I think it's fishy, and that this
>>>> check may result in false positives. Looks too generic.
>>>>
>>>> Also the logic of your behavior there seems unclear to me. On the one
>>>> hand you don't have JEDEC, so you provide chip name using DT. But in
>>>> place above you stop trusting DT info and you try to (kind of)
>>>> auto-detect used chip anyway.
>>>>
>>>> I guess we should finally think about some more generic way of
>>>> passing flash info.
>>>
>>> Actually, i just want fo follow the way current spi-nor doing as much
>>> as possible. Like to read the device id and compare with info table.
>>> Like double checking from both dtb and the device id. Since the
>>> flashes i support do not have JEDEC id but only extended id. But the
>>> problem is that some of them have the same extended id, for example
>>> epcs64 and epcq32). That is why in my driver, i have to decode 1st
>>> byte of ext id to differentiate epcs and ecpq.
>>
>> I see your point and it makes sense, I just think it shouldn't be part of
>> spi-nor. By adding chip specific code to spi-nor we'll end with hacky code
>> and possible false chips identifications. I can really easily imagine some
>> random chip having the same id[3] and id[4] as one of Altera flashes.
>>
>> Moreover your patch has not working support for epcs16 and epcs64.
>> They don't support 0x9f opcode (SPINOR_OP_RDID), so you would need to add
>> support for 0xab ("Read silicon ID") to the spi-nor.
>>
>> It's the same problem I have with Broadcom's "w25q128" that doesn't support
>> 0x9f opcode but a 0x90 with 16b reply. You may see my tiny bcm53xxspiflash.c
>> driver:
>> http://git.openwrt.org/?p=openwrt.git;a=blob;f=target/linux/bcm53xx/files/drivers/mtd/spi-nor/bcm53xxspiflash.c;h=f192f4e59b71a2444833b5c62dd2239d28f9435d;hb=d105c51a428a72a9af42759c472df9960c496d67
>>
>> So I'm afraid that if spi-nor gets support for:
>> 1) 0xab opcode
>> 2) 0x90 opcode
>> 3) Some uncommon replies for 0x9f opcode (like Altera ones) it will quickly
>> get hacky & buggy.
>>
>> So what about:
>> 1) Using 0x9f and 0xab in altera_epcq.c
>> 2) Finding chip name in altera_epcq.c
>> 3) Adding Altera chip names & all sizes to spi-nor.c
>> 4) Just passing a chip name to spi-nor.c
>>
>> It's something I do in bcm53xxspiflash.c. I detect w25q128 using some
>> specific 0x90 opcode and just pass a chip name to the spi-nor.
>>
>> --
>> Rafał
>>
> Ok. I will modify the code the way you suggest.
I just realize that the opcode for RDID is handled by hardware in my
case, therefore i dont really need to worry about 0xab opcode.
Is there any other comments about the modified part i did the lock and
unlock in spi-nor.c?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: FW: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
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 linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: FW: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
+
+/* 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 _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 _nor_ids[tmp];
>>>
>>> This is the part I don't like. I think it's fishy, and that this
>>> check may result in false positives. Looks too generic.
>>>
>>> Also the logic of your behavior there seems unclear to me. On the one
>>> hand you don't have JEDEC, so you provide chip name using DT. But in
>>> place above you stop trusting DT info and you try to (kind of)
>>> auto-detect used chip anyway.
>>>
>>> I guess we should finally think about some more generic way of
>>> passing flash info.
>>
>> Actually, i just want fo follow the way current spi-nor doing as much
>> as possible. Like to read the device id and compare with info table.
>> Like double checking from both dtb and the device id. Since the
>> flashes i support do not have JEDEC id but only extended id. But the
>> problem is that some of them have the same extended id, for example
>> epcs64 and epcq32). That is why in my driver, i have to decode 1st
>> byte of ext id to differentiate epcs and ecpq.
>
> I see your point and it makes sense, I just think it shouldn't be part of
> spi-nor. By adding chip specific code to spi-nor we'll end with hacky code
> and possible false chips identifications. I can really easily imagine some
> random chip having the same id[3] and id[4] as one of Altera flashes.
>
> Moreover your patch has not working support for epcs16 and epcs64.
> They don't support 0x9f opcode (SPINOR_OP_RDID), so you would need to add
> support for 0xab ("Read silicon ID") to the spi-nor.
>
> It's the same problem I have with Broadcom's "w25q128" that doesn't support
> 0x9f opcode but a 0x90 with 16b reply. You may see my tiny bcm53xxspiflash.c
> driver:
> http://git.openwrt.org/?p=openwrt.git;a=blob;f=target/linux/bcm53xx/files/drivers/mtd/spi-nor/bcm53xxspiflash.c;h=f192f4e59b71a2444833b5c62dd2239d28f9435d;hb=d105c51a428a72a9af42759c472df9960c496d67
>
> So I'm afraid that if spi-nor gets support for:
> 1) 0xab opcode
> 2) 0x90 opcode
> 3) Some uncommon replies for 0x9f opcode (like Altera ones) it will quickly
> get hacky & buggy.
>
> So what about:
> 1) Using 0x9f and 0xab in altera_epcq.c
> 2) Finding chip name in altera_epcq.c
> 3) Adding Altera chip names & all sizes to spi-nor.c
> 4) Just passing a chip name to spi-nor.c
>
> It's something I do in bcm53xxspiflash.c. I detect w25q128 using some
> specific 0x90 opcode and just pass a chip name to the spi-nor.
>
> --
> Rafał
>
Ok. I will modify the code the way you suggest.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
Hi Rafal,
Thanks for your review.
On Mon, Mar 9, 2015 at 2:31 PM, Rafał Miłecki wrote:
> Hi Viet,
>
> I'm not too active in mtd subsystem, so I didn't notice your patch
> earlier. However I would like to share few comments.
>
> On 11 February 2015 at 05:53, Viet Nga Dao wrote:
>> From: Viet Nga Dao
>>
>> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
>> EPCS flash chips. This patch adds driver for these devices.
>
> First of all, your whole patch is white-space damaged. It can't be
> applied, seems all tabs were replaced with spaces. It's what I got in
> my GMail and what was also received by patchwork, see
> https://patchwork.ozlabs.org/patch/438684/
>
> You'll need to resend using some smarter e-mail client, you may e.g.
> try git send-email.
>
>
>> +#define EPCQ_INFO(_opcode_id, _ext_id, _sector_size, _n_sectors,
>> _page_size) \
>> +((kernel_ulong_t)&(struct flash_info) {\
>> +.altera_flash_opcode_id = (_opcode_id),\
>> +.ext_id = (_ext_id),\
>> +.sector_size = (_sector_size),\
>> +.n_sectors = (_n_sectors),\
>> +.page_size = (_page_size),\
>> +})
>
> Starting with kernel 3.19 we don't have ext_id in struct anymore, see:
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=d928a259385dc6fca3956b7775c588f21c0b50fc
>
>
Noted. I will make the modification accordingly.
>> /* NOTE: double check command sets and memory organization when you add
>> * more nor chips. This current list focusses on newer chips, which
>> * have been converging on command sets which including JEDEC ID.
>> @@ -637,6 +691,17 @@ static const struct spi_device_id spi_nor_ids[] = {
>> { "cat25c09", CAT25_INFO( 128, 8, 32, 2, SPI_NOR_NO_ERASE |
>> SPI_NOR_NO_FR) },
>> { "cat25c17", CAT25_INFO( 256, 8, 32, 2, SPI_NOR_NO_ERASE |
>> SPI_NOR_NO_FR) },
>> { "cat25128", CAT25_INFO(2048, 8, 64, 2, SPI_NOR_NO_ERASE |
>> SPI_NOR_NO_FR) },
>> +
>> +/* Altera EPCQ/EPCS Flashes */
>> +{ "epcq16" , EPCQ_INFO(2, 0x15, 0x1, 32, 0x100) },
>> +{ "epcq32" , EPCQ_INFO(2, 0x16, 0x1, 64, 0x100) },
>> +{ "epcq64" , EPCQ_INFO(2, 0x17, 0x1, 128, 0x100) },
>> +{ "epcq128" , EPCQ_INFO(2, 0x18, 0x1, 256, 0x100) },
>> +{ "epcq256" , EPCQ_INFO(2, 0x19, 0x1, 512, 0x100) },
>> +{ "epcq512" , EPCQ_INFO(2, 0x20, 0x1, 1024, 0x100) },
>> +{ "epcs16" , EPCQ_INFO(1, 0x14, 0x1, 32, 0x100) },
>> +{ "epcs64" , EPCQ_INFO(1, 0x16, 0x1, 128, 0x100) },
>> +{ "epcs128" , EPCQ_INFO(1, 0x18, 0x4, 256, 0x100) },
>> { },
>> };
>>
>> @@ -666,6 +731,14 @@ static const struct spi_device_id
>> *spi_nor_read_id(struct spi_nor *nor)
>> if (info->jedec_id == jedec) {
>> if (info->ext_id == 0 || info->ext_id == ext_jedec)
>> return _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 _nor_ids[tmp];
>
> This is the part I don't like. I think it's fishy, and that this check
> may result in false positives. Looks too generic.
>
> Also the logic of your behavior there seems unclear to me. On the one
> hand you don't have JEDEC, so you provide chip name using DT. But in
> place above you stop trusting DT info and you try to (kind of)
> auto-detect used chip anyway.
>
> I guess we should finally think about some more generic way of passing
> flash info.
Actually, i just want fo follow the way current spi-nor doing as much
as possible. Like to read the device id and compare with info table.
Like double checking from both dtb and the device id. Since the
flashes i support do not have JEDEC id but only extended id. But the
problem is that some of them have the same extended id, for example
epcs64 and epcq32). That is why in my driver, i have to decode 1st
byte of ext id to differentiate epcs and ecpq.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
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 linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: FW: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
+
+/* Altera EPCQ/EPCS Flashes */
+{ epcq16 , EPCQ_INFO(2, 0x15, 0x1, 32, 0x100) },
+{ epcq32 , EPCQ_INFO(2, 0x16, 0x1, 64, 0x100) },
+{ epcq64 , EPCQ_INFO(2, 0x17, 0x1, 128, 0x100) },
+{ epcq128 , EPCQ_INFO(2, 0x18, 0x1, 256, 0x100) },
+{ epcq256 , EPCQ_INFO(2, 0x19, 0x1, 512, 0x100) },
+{ epcq512 , EPCQ_INFO(2, 0x20, 0x1, 1024, 0x100) },
+{ epcs16 , EPCQ_INFO(1, 0x14, 0x1, 32, 0x100) },
+{ epcs64 , EPCQ_INFO(1, 0x16, 0x1, 128, 0x100) },
+{ epcs128 , EPCQ_INFO(1, 0x18, 0x4, 256, 0x100) },
{ },
};
@@ -666,6 +731,14 @@ static const struct spi_device_id
*spi_nor_read_id(struct spi_nor *nor)
if (info-jedec_id == jedec) {
if (info-ext_id == 0 || info-ext_id == ext_jedec)
return spi_nor_ids[tmp];
+
+/* altera epcq which is non jedec device
+ * use id[4] as opcode id to differentiate
+ * epcs and epcq devices
+ */
+} else if (info-altera_flash_opcode_id == id[4]
+ info-ext_id == id[3]) {
+return spi_nor_ids[tmp];
This is the part I don't like. I think it's fishy, and that this
check may result in false positives. Looks too generic.
Also the logic of your behavior there seems unclear to me. On the one
hand you don't have JEDEC, so you provide chip name using DT. But in
place above you stop trusting DT info and you try to (kind of)
auto-detect used chip anyway.
I guess we should finally think about some more generic way of
passing flash info.
Actually, i just want fo follow the way current spi-nor doing as much
as possible. Like to read the device id and compare with info table.
Like double checking from both dtb and the device id. Since the
flashes i support do not have JEDEC id but only extended id. But the
problem is that some of them have the same extended id, for example
epcs64 and epcq32). That is why in my driver, i have to decode 1st
byte of ext id to differentiate epcs and ecpq.
I see your point and it makes sense, I just think it shouldn't be part of
spi-nor. By adding chip specific code to spi-nor we'll end with hacky code
and possible false chips identifications. I can really easily imagine some
random chip having the same id[3] and id[4] as one of Altera flashes.
Moreover your patch has not working support for epcs16 and epcs64.
They don't support 0x9f opcode (SPINOR_OP_RDID), so you would need to add
support for 0xab (Read silicon ID) to the spi-nor.
It's the same problem I have with Broadcom's w25q128 that doesn't support
0x9f opcode but a 0x90 with 16b reply. You may see my tiny bcm53xxspiflash.c
driver:
http://git.openwrt.org/?p=openwrt.git;a=blob;f=target/linux/bcm53xx/files/drivers/mtd/spi-nor/bcm53xxspiflash.c;h=f192f4e59b71a2444833b5c62dd2239d28f9435d;hb=d105c51a428a72a9af42759c472df9960c496d67
So I'm afraid that if spi-nor gets support for:
1) 0xab opcode
2) 0x90 opcode
3) Some uncommon replies for 0x9f opcode (like Altera ones) it will quickly
get hacky buggy.
So what about:
1) Using 0x9f and 0xab in altera_epcq.c
2) Finding chip name in altera_epcq.c
3) Adding Altera chip names all sizes to spi-nor.c
4) Just passing a chip name to spi-nor.c
It's something I do in bcm53xxspiflash.c. I detect w25q128 using some
specific 0x90 opcode and just pass a chip name to the spi-nor.
--
Rafał
Ok. I will modify the code the way you suggest.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
Hi Brian, I understand that you are busy but is it possible if you could spend some time to help me review my patch? My work progress really depends on your review. Thanks, Viet Nga On Tue, Feb 24, 2015 at 11:59 AM, Brian Norris wrote: > On Mon, Feb 23, 2015 at 09:30:09AM +0800, Viet Nga Dao wrote: >> Hi, >> It has been nearly 2 weeks since i submitted this patch. Could you >> please help to review? > > Those two weeks were during the merge window, so I wasn't queueing > anything up. And there are things that have waited longer, anyway. My > time is unfortunately finite. > > I'll get to your patch eventually. > >> On Tue, Feb 17, 2015 at 9:33 AM, Viet Nga Dao wrote: >> > Hi Brian, >> > Could you please help me to review through this 2nd version? >> > >> > On Wed, Feb 11, 2015 at 12:53 PM, Viet Nga Dao wrote: >> >> From: Viet Nga Dao >> >> >> >> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ >> >> and >> >> EPCS flash chips. This patch adds driver for these devices. >> >> >> >> Signed-off-by: VIET NGA DAO >> >> >> >> --- >> >> v2: >> >> - Change to spi_nor structure >> >> - Add lock and unlock functions for spi_nor >> >> - Simplify the altera_epcq_lock function >> >> - Replace reg by compatible in device tree > ... > > Brian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
Hi Brian, I understand that you are busy but is it possible if you could spend some time to help me review my patch? My work progress really depends on your review. Thanks, Viet Nga On Tue, Feb 24, 2015 at 11:59 AM, Brian Norris computersforpe...@gmail.com wrote: On Mon, Feb 23, 2015 at 09:30:09AM +0800, Viet Nga Dao wrote: Hi, It has been nearly 2 weeks since i submitted this patch. Could you please help to review? Those two weeks were during the merge window, so I wasn't queueing anything up. And there are things that have waited longer, anyway. My time is unfortunately finite. I'll get to your patch eventually. On Tue, Feb 17, 2015 at 9:33 AM, Viet Nga Dao 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 ... Brian -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] mtd:spi-nor: Add Altera EPCQ Driver
Hi,
It has been nearly 2 weeks since i submitted this patch. Could you
please help to review?
Thanks,
On Tue, Feb 17, 2015 at 9:33 AM, Viet Nga Dao wrote:
> Hi Brian,
> Could you please help me to review through this 2nd version?
>
> On Wed, Feb 11, 2015 at 12:53 PM, Viet Nga Dao wrote:
>> From: Viet Nga Dao
>>
>> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
>> EPCS flash chips. This patch adds driver for these devices.
>>
>> Signed-off-by: VIET NGA DAO
>>
>> ---
>> v2:
>> - Change to spi_nor structure
>> - Add lock and unlock functions for spi_nor
>> - Simplify the altera_epcq_lock function
>> - Replace reg by compatible in device tree
>> ---
>> .../devicetree/bindings/mtd/altera_epcq.txt| 45 ++
>> drivers/mtd/spi-nor/Kconfig| 12 +
>> drivers/mtd/spi-nor/Makefile |1 +
>> drivers/mtd/spi-nor/altera_epcq.c | 507
>>
>> drivers/mtd/spi-nor/altera_epcq.h | 116 +
>> drivers/mtd/spi-nor/spi-nor.c | 86 -
>> include/linux/mtd/spi-nor.h|3 +-
>> 7 files changed, 764 insertions(+), 6 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/mtd/altera_epcq.txt
>> create mode 100644 drivers/mtd/spi-nor/altera_epcq.c
>> create mode 100644 drivers/mtd/spi-nor/altera_epcq.h
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/altera_epcq.txt
>> b/Documentation/devicetree/bindings/mtd/altera_epcq.txt
>> new file mode 100644
>> index 000..b6b5e61
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/altera_epcq.txt
>> @@ -0,0 +1,45 @@
>> +* MTD Altera EPCQ driver
>> +
>> +Required properties:
>> +- compatible: Should be "altr,epcq-1.0"
>> +- reg: Address and length of the register set for the device. It contains
>> + the information of registers in the same order as described by reg-names
>> +- reg-names: Should contain the reg names
>> + "csr_base": Should contain the register configuration base address
>> + "data_base": Should contain the data base address
>> +- is-epcs: boolean type.
>> +If present, the device contains EPCS flashes.
>> +Otherwise, it contains EPCQ flashes.
>> +- #address-cells: Must be <1>.
>> +- #size-cells: Must be <0>.
>> +- flash device tree subnode, there must be a node with the following fields:
>> +- compatible: Should contain the flash name
>> +- #address-cells: please refer to /mtd/partition.txt
>> +- #size-cells: please refer to /mtd/partition.txt
>> +For partitions inside each flash, please refer to /mtd/partition.txt
>> +
>> +Example:
>> +
>> +epcq_controller_0: epcq@0x0 {
>> +compatible = "altr,epcq-1.0";
>> +reg = <0x0001 0x 0x0020>,
>> +<0x 0x 0x0200>;
>> +reg-names = "csr_base", "data_base";
>> +#address-cells = <1>;
>> +#size-cells = <0>;
>> +flash0: epcq256@0 {
>> +compatible = "epcq256";
>> +#address-cells = <1>;
>> +#size-cells = <1>;
>> +partition@0 {
>> +/* 16 MB for raw data. */
>> +label = "EPCQ Flash 0 raw data";
>> +reg = <0x0 0x100>;
>> +};
>> +partition@100 {
>> +/* 16 MB for jffs2 data. */
>> +label = "EPCQ Flash 0 JFFS 2";
>> +reg = <0x100 0x100>;
>> +};
>> +};
>> +}; //end epcq@0x0 (epcq_controller_0)
>> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
>> index 64a4f0e..83178b9 100644
>> --- a/drivers/mtd/spi-nor/Kconfig
>> +++ b/drivers/mtd/spi-nor/Kconfig
>> @@ -28,4 +28,16 @@ config SPI_FSL_QUADSPI
>>This enables support for the Quad SPI controller in master mode.
>>We only connect the NOR to this controller now.
>>
>> +config SPI_ALTERA_EPCQ
>> +tristate "Support Altera EPCQ/EPCS Flash chips"
>> +depends on OF
>>
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 wrote:
> From: Viet Nga Dao
>
> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
> EPCS flash chips. This patch adds driver for these devices.
>
> Signed-off-by: VIET NGA DAO
>
> ---
> v2:
> - Change to spi_nor structure
> - Add lock and unlock functions for spi_nor
> - Simplify the altera_epcq_lock function
> - Replace reg by compatible in device tree
> ---
> .../devicetree/bindings/mtd/altera_epcq.txt| 45 ++
> drivers/mtd/spi-nor/Kconfig| 12 +
> drivers/mtd/spi-nor/Makefile |1 +
> drivers/mtd/spi-nor/altera_epcq.c | 507
>
> drivers/mtd/spi-nor/altera_epcq.h | 116 +
> drivers/mtd/spi-nor/spi-nor.c | 86 -
> include/linux/mtd/spi-nor.h|3 +-
> 7 files changed, 764 insertions(+), 6 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mtd/altera_epcq.txt
> create mode 100644 drivers/mtd/spi-nor/altera_epcq.c
> create mode 100644 drivers/mtd/spi-nor/altera_epcq.h
>
> diff --git a/Documentation/devicetree/bindings/mtd/altera_epcq.txt
> b/Documentation/devicetree/bindings/mtd/altera_epcq.txt
> new file mode 100644
> index 000..b6b5e61
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/altera_epcq.txt
> @@ -0,0 +1,45 @@
> +* MTD Altera EPCQ driver
> +
> +Required properties:
> +- compatible: Should be "altr,epcq-1.0"
> +- reg: Address and length of the register set for the device. It contains
> + the information of registers in the same order as described by reg-names
> +- reg-names: Should contain the reg names
> + "csr_base": Should contain the register configuration base address
> + "data_base": Should contain the data base address
> +- is-epcs: boolean type.
> +If present, the device contains EPCS flashes.
> +Otherwise, it contains EPCQ flashes.
> +- #address-cells: Must be <1>.
> +- #size-cells: Must be <0>.
> +- flash device tree subnode, there must be a node with the following fields:
> +- compatible: Should contain the flash name
> +- #address-cells: please refer to /mtd/partition.txt
> +- #size-cells: please refer to /mtd/partition.txt
> +For partitions inside each flash, please refer to /mtd/partition.txt
> +
> +Example:
> +
> +epcq_controller_0: epcq@0x0 {
> +compatible = "altr,epcq-1.0";
> +reg = <0x0001 0x 0x0020>,
> +<0x 0x 0x0200>;
> +reg-names = "csr_base", "data_base";
> +#address-cells = <1>;
> +#size-cells = <0>;
> +flash0: epcq256@0 {
> +compatible = "epcq256";
> +#address-cells = <1>;
> +#size-cells = <1>;
> +partition@0 {
> +/* 16 MB for raw data. */
> +label = "EPCQ Flash 0 raw data";
> +reg = <0x0 0x100>;
> +};
> +partition@100 {
> +/* 16 MB for jffs2 data. */
> +label = "EPCQ Flash 0 JFFS 2";
> +reg = <0x100 0x100>;
> +};
> +};
> +}; //end epcq@0x0 (epcq_controller_0)
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 64a4f0e..83178b9 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -28,4 +28,16 @@ config SPI_FSL_QUADSPI
>This enables support for the Quad SPI controller in master mode.
>We only connect the NOR to this controller now.
>
> +config SPI_ALTERA_EPCQ
> +tristate "Support Altera EPCQ/EPCS Flash chips"
> +depends on OF
> +help
> + This enables access to Altera EPCQ/EPCS flash chips, used for data
> + storage. See the driver source for the current list,
> + or to add other chips.
> +
> + If you want to compile this driver as a module ( = code which can be
> + inserted in and removed from the running kernel whenever you want),
> + say M here and read .
> +
> endif # MTD_SPI_NOR
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index 6a7ce14..ff9437b 100644
> --- a/drivers/mtd/s
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
Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
EPCS flash chips. This patch adds driver for these devices.
Signed-off-by: VIET NGA DAO
---
v2:
- Change to spi_nor structure
- Add lock and unlock functions for spi_nor
- Simplify the altera_epcq_lock function
- Replace reg by compatible in device tree
---
.../devicetree/bindings/mtd/altera_epcq.txt| 45 ++
drivers/mtd/spi-nor/Kconfig| 12 +
drivers/mtd/spi-nor/Makefile |1 +
drivers/mtd/spi-nor/altera_epcq.c | 507
drivers/mtd/spi-nor/altera_epcq.h | 116 +
drivers/mtd/spi-nor/spi-nor.c | 86 -
include/linux/mtd/spi-nor.h|3 +-
7 files changed, 764 insertions(+), 6 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mtd/altera_epcq.txt
create mode 100644 drivers/mtd/spi-nor/altera_epcq.c
create mode 100644 drivers/mtd/spi-nor/altera_epcq.h
diff --git a/Documentation/devicetree/bindings/mtd/altera_epcq.txt
b/Documentation/devicetree/bindings/mtd/altera_epcq.txt
new file mode 100644
index 000..b6b5e61
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/altera_epcq.txt
@@ -0,0 +1,45 @@
+* MTD Altera EPCQ driver
+
+Required properties:
+- compatible: Should be "altr,epcq-1.0"
+- reg: Address and length of the register set for the device. It contains
+ the information of registers in the same order as described by reg-names
+- reg-names: Should contain the reg names
+ "csr_base": Should contain the register configuration base address
+ "data_base": Should contain the data base address
+- is-epcs: boolean type.
+If present, the device contains EPCS flashes.
+Otherwise, it contains EPCQ flashes.
+- #address-cells: Must be <1>.
+- #size-cells: Must be <0>.
+- flash device tree subnode, there must be a node with the following fields:
+- compatible: Should contain the flash name
+- #address-cells: please refer to /mtd/partition.txt
+- #size-cells: please refer to /mtd/partition.txt
+For partitions inside each flash, please refer to /mtd/partition.txt
+
+Example:
+
+epcq_controller_0: epcq@0x0 {
+compatible = "altr,epcq-1.0";
+reg = <0x0001 0x 0x0020>,
+<0x 0x 0x0200>;
+reg-names = "csr_base", "data_base";
+#address-cells = <1>;
+#size-cells = <0>;
+flash0: epcq256@0 {
+compatible = "epcq256";
+#address-cells = <1>;
+#size-cells = <1>;
+partition@0 {
+/* 16 MB for raw data. */
+label = "EPCQ Flash 0 raw data";
+reg = <0x0 0x100>;
+};
+partition@100 {
+/* 16 MB for jffs2 data. */
+label = "EPCQ Flash 0 JFFS 2";
+reg = <0x100 0x100>;
+};
+};
+}; //end epcq@0x0 (epcq_controller_0)
diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 64a4f0e..83178b9 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -28,4 +28,16 @@ config SPI_FSL_QUADSPI
This enables support for the Quad SPI controller in master mode.
We only connect the NOR to this controller now.
+config SPI_ALTERA_EPCQ
+tristate "Support Altera EPCQ/EPCS Flash chips"
+depends on OF
+help
+ This enables access to Altera EPCQ/EPCS flash chips, used for data
+ storage. See the driver source for the current list,
+ or to add other chips.
+
+ If you want to compile this driver as a module ( = code which can be
+ inserted in and removed from the running kernel whenever you want),
+ say M here and read .
+
endif # MTD_SPI_NOR
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 6a7ce14..ff9437b 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_MTD_SPI_NOR)+= spi-nor.o
obj-$(CONFIG_SPI_FSL_QUADSPI)+= fsl-quadspi.o
+obj-$(CONFIG_SPI_ALTERA_EPCQ)+= altera_epcq.o
diff --git a/drivers/mtd/spi-nor/altera_epcq.c
b/drivers/mtd/spi-nor/altera_epcq.c
new file mode 100644
index 000..9db0d05
--- /dev/null
+++ b/drivers/mtd/spi-nor/altera_epcq.c
@@ -0,0 +1,507 @@
+/*
+ * Copyright (C) 2014 Altera Corporation. All rights reserved
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * v
[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 wrote:
> Please ignore previous 2 emails of mine ^^
>
> On Thu, Feb 5, 2015 at 4:37 AM, Brian Norris
> 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 wrote:
>>> > On Thu, Jan 15, 2015 at 5:27 PM, Viet Nga Dao wrote:
>>> >> On Tue, Jan 13, 2015 at 11:33 AM, Brian Norris
>>> >> wrote:
>>> >>> On Thu, Dec 18, 2014 at 12:23:16AM -0800, vn...@altera.com wrote:
>>> >>>> From: Viet Nga Dao
>>
>>> >> 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
>>> >>&g
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 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 wrote: >> > On Thu, Jan 15, 2015 at 5:27 PM, Viet Nga Dao wrote: >> >> On Tue, Jan 13, 2015 at 11:33 AM, Brian Norris >> >> wrote: >> >>> On Thu, Dec 18, 2014 at 12:23:16AM -0800, vn...@altera.com wrote: >> >>>> From: Viet Nga Dao > >> >> 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); >> >>>> +
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 Thu, Feb 5, 2015 at 4:37 AM, Brian Norris 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 wrote: >> > On Thu, Jan 15, 2015 at 5:27 PM, Viet Nga Dao wrote: >> >> On Tue, Jan 13, 2015 at 11:33 AM, Brian Norris >> >> wrote: >> >>> On Thu, Dec 18, 2014 at 12:23:16AM -0800, vn...@altera.com wrote: >> >>>> From: Viet Nga Dao > >> >> 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. > When this will be implemented? As for time being, what should i do for these 2 functions? >> >>>> 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. > >> >>>> + 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_se
Re: [PATCH mtd] mtd:devices: Add Altera EPCQ Driver
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. When this will be implemented? As for time being, what should i do for these 2 functions? 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. + 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 this if/else-if/else block in about 1 line though. Yes, i understand that it looks ugly. But it is the best i can do since this function has to satisfy for all the supported devices (http://www.altera.com.my/literature/hb/cfg/cfg_cf52012.pdf
Re: [PATCH mtd] mtd:devices: Add Altera EPCQ Driver
On Tue, Jan 20, 2015 at 10:05 AM, Viet Nga Dao wrote:
> On Thu, Jan 15, 2015 at 5:27 PM, Viet Nga Dao wrote:
>> On Tue, Jan 13, 2015 at 11:33 AM, Brian Norris
>> wrote:
>>> On Thu, Dec 18, 2014 at 12:23:16AM -0800, vn...@altera.com wrote:
>>>> From: Viet Nga Dao
>>>>
>>>> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
>>>> EPCS flash chips. This patch adds driver for these devices.
>>>>
>>>> Signed-off-by: Viet Nga Dao
>>>
>>> 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
>>>
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 wrote: > On Thu, Jan 15, 2015 at 5:27 PM, Viet Nga Dao wrote: >> On Tue, Jan 13, 2015 at 11:33 AM, Brian Norris >> wrote: >>> On Thu, Dec 18, 2014 at 12:23:16AM -0800, vn...@altera.com wrote: >>>> From: Viet Nga Dao >>>> >>>> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and >>>> EPCS flash chips. This patch adds driver for these devices. >>>> >>>> Signed-off-by: Viet Nga Dao >>> >>> 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 linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 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 linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH mtd] mtd:devices: Add Altera EPCQ Driver
On Thu, Jan 15, 2015 at 5:27 PM, Viet Nga Dao wrote:
> On Tue, Jan 13, 2015 at 11:33 AM, Brian Norris
> wrote:
>> On Thu, Dec 18, 2014 at 12:23:16AM -0800, vn...@altera.com wrote:
>>> From: Viet Nga Dao
>>>
>>> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
>>> EPCS flash chips. This patch adds driver for these devices.
>>>
>>> Signed-off-by: Viet Nga Dao
>>
>> 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>;
>>> +
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
wrote:
> On Thu, Dec 18, 2014 at 12:23:16AM -0800, vn...@altera.com wrote:
>> From: Viet Nga Dao
>>
>> Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
>> EPCS flash chips. This patch adds driver for these devices.
>>
>> Signed-off-by: Viet Nga Dao
>
> 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";
>> +
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-kernel@vger.kernel.org; devicet...@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
>
> 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
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&
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-kernel@vger.kernel.org; devicet...@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-kernel@vger.kernel.org;
devicet...@vger.kernel.org; ngach...@gmail.com; Viet Nga Dao
Subject: [PATCH mtd] mtd:devices: Add Altera EPCQ Driver
From: Viet Nga Dao
Altera EPCQ Controller is a soft IP which enables access to Altera EPCQ and
EPCS flash chips. This patch adds driver for these devices.
Signed-off-by: Viet Nga Dao
---
.../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 .
+
if MTD_DOCG3
config BCH_CONST_M
default 14
diff --git a/drivers/mtd/devices/Makefile b/drivers/mtd/devices/Makefile index
f0b0e
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-kernel@vger.kernel.org;
devicet...@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
