Re: [PATCH RFC] clk: mxs: Fix invalid 32-bit access to frac registers
On Sunday, December 14, 2014 at 06:16:17 PM, Stefan Wahren wrote: Hi Marek, Marek Vasut ma...@denx.de hat am 14. Dezember 2014 um 17:12 geschrieben: static void __iomem *digctrl; #define DIGCTRL digctrl @@ -118,11 +119,12 @@ static void __init clk_misc_init(void) /* * 480 MHz seems too high to be ssp clock source directly, * so set frac0 to get a 288 MHz ref_io0 and ref_io1. + * According to reference manual we must access frac0 bytewise. */ - val = readl_relaxed(FRAC0); - val = ~((0x3f BP_FRAC0_IO0FRAC) | (0x3f BP_FRAC0_IO1FRAC)); - val |= (30 BP_FRAC0_IO0FRAC) | (30 BP_FRAC0_IO1FRAC); - writel_relaxed(val, FRAC0); + writeb_relaxed(0x3f, FRAC0 + FRAC0_IO0 + CLR); + writeb_relaxed(30, FRAC0 + FRAC0_IO0 + SET); + writeb_relaxed(0x3f, FRAC0 + FRAC0_IO1 + CLR); + writeb_relaxed(30, FRAC0 + FRAC0_IO1 + SET); This used to be a R-M-W sequence, but now it's changed to multiple writes. This changes the behavior and seeing you use the CLR register, I am worried this might be prone to clock glitches. What do you think please ? you are right. I adapt the imx23 init to the imx28 to make code simple. But it would be better to avoid glitches. I hope it's okay for this bugfix to introduce a R-M-W sequence for the imx23 init. So it's consequent. It should be OK. Make sure to document it in the commit message. [...] Also, it might be a good idea to zap the 0x3f mask and use HEX and DEC numbers consistently, but this is an idea for another patch. Yes. Btw i hope this patch also fixes a SPI communication issue with our hardware which forces us to bypass ref_io1 for ssp2. But i will have access to that hardware tomorrow. Which issue would that be please ? What are the symptoms ? 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 RFC] clk: mxs: Fix invalid 32-bit access to frac registers
On Thursday, December 18, 2014 at 04:58:28 PM, Stefan Wahren wrote: Hi Marek, Hello Stefan, Am 17.12.2014 um 17:00 schrieb Marek Vasut: On Wednesday, December 17, 2014 at 08:58:23 AM, Stefan Wahren wrote: Hi Fabio, Am 17.12.2014 um 03:44 schrieb Fabio Estevam: Hi Stefan, On Sun, Dec 14, 2014 at 3:16 PM, Stefan Wahren stefan.wah...@i2se.com wrote: Btw i hope this patch also fixes a SPI communication issue with our hardware which forces us to bypass ref_io1 for ssp2. Does this patch fix the SPI issue? unfortunately not. If the 3.19-rc1 has been released, i send the fixed patch. I've the theory that's an initialization issue. I've never heard of SPI problems from the U-Boot users and we are using imx-bootlets. I'll try to test it with U-Boot. Please keep me in the loop, I'd be interested in what you find. Thanks! Best regards, Marek Vasut i tested it with U-Boot, but the issue still exists. If there is any progress, i'll inform you. Thanks! 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] clk: mxs: Fix invalid 32-bit access to frac registers
On Sunday, December 21, 2014 at 02:46:39 PM, Stefan Wahren wrote:
Hi!
[...]
diff --git a/drivers/clk/mxs/clk-ref.c b/drivers/clk/mxs/clk-ref.c
index 4adeed6..bdecec1 100644
--- a/drivers/clk/mxs/clk-ref.c
+++ b/drivers/clk/mxs/clk-ref.c
@@ -16,6 +16,8 @@
#include linux/slab.h
#include clk.h
+#define BF_CLKGATE BIT(7)
+
/**
* struct clk_ref - mxs reference clock
* @hw: clk_hw for the reference clock
@@ -39,7 +41,7 @@ static int clk_ref_enable(struct clk_hw *hw)
{
struct clk_ref *ref = to_clk_ref(hw);
- writel_relaxed(1 ((ref-idx + 1) * 8 - 1), ref-reg + CLR);
+ writeb(BF_CLKGATE, ref-reg + ref-idx + CLR);
Should this be writeb_relaxed() maybe ?
return 0;
}
@@ -48,7 +50,7 @@ static void clk_ref_disable(struct clk_hw *hw)
{
struct clk_ref *ref = to_clk_ref(hw);
- writel_relaxed(1 ((ref-idx + 1) * 8 - 1), ref-reg + SET);
+ writeb(BF_CLKGATE, ref-reg + ref-idx + SET);
Same here and all around the place ?
Other than that, it looks pretty OK :)
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 RFC] clk: mxs: Fix invalid 32-bit access to frac registers
On Wednesday, December 17, 2014 at 08:58:23 AM, Stefan Wahren wrote: Hi Fabio, Am 17.12.2014 um 03:44 schrieb Fabio Estevam: Hi Stefan, On Sun, Dec 14, 2014 at 3:16 PM, Stefan Wahren stefan.wah...@i2se.com wrote: Btw i hope this patch also fixes a SPI communication issue with our hardware which forces us to bypass ref_io1 for ssp2. Does this patch fix the SPI issue? unfortunately not. If the 3.19-rc1 has been released, i send the fixed patch. I've the theory that's an initialization issue. I've never heard of SPI problems from the U-Boot users and we are using imx-bootlets. I'll try to test it with U-Boot. Please keep me in the loop, I'd be interested in what you find. Thanks! 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/
[PATCH] doc: Fix comparison operator
From: Olaf Mandel o.man...@menlosystems.com Align the documentation with the include/linux/etherdevice.h , which is where this example comes from. The return value from the check was inverted in the documentation. Signed-off-by: Olaf Mandel o.man...@menlosystems.com Signed-off-by: Marek Vasut ma...@denx.de Cc: Jiri Kosina jkos...@suse.cz Cc: Joe Perches j...@perches.com Cc: linux-...@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- Documentation/unaligned-memory-access.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/unaligned-memory-access.txt b/Documentation/unaligned-memory-access.txt index a445da0..3f76c0c 100644 --- a/Documentation/unaligned-memory-access.txt +++ b/Documentation/unaligned-memory-access.txt @@ -151,7 +151,7 @@ bool ether_addr_equal(const u8 *addr1, const u8 *addr2) #else const u16 *a = (const u16 *)addr1; const u16 *b = (const u16 *)addr2; - return ((a[0] ^ b[0]) | (a[1] ^ b[1]) | (a[2] ^ b[2])) != 0; + return ((a[0] ^ b[0]) | (a[1] ^ b[1]) | (a[2] ^ b[2])) == 0; #endif } -- 2.0.0 -- 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] m25p80, spi-nor: add SFDP detect method
On Wednesday, February 04, 2015 at 05:06:55 PM, Rafał Miłecki wrote: On 4 February 2015 at 16:50, Jim-Ting Kuo jimting...@gmail.com wrote: The serial flash discoverable parameters (SFDP) is needed to spi nor devices for some specific features. I added some sfdp structure and detect method. And I hope the method will be useful in the future. The code have been tested on lower version Linux and can successfully work with Macronix chips. So I also modified the chip support list of Macronix. Please resend without broken white spaces, so we can review/apply it. btw. after a brief skim over the code, I'd suggest to send the new functionality and new chip IDs in separate patches. 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 V2] clk: mxs: Fix invalid 32-bit access to frac registers
On Wednesday, January 21, 2015 at 05:16:03 PM, Zhi Li wrote: On Sun, Dec 28, 2014 at 4:26 AM, Stefan Wahren stefan.wah...@i2se.com wrote: According to i.MX23 and i.MX28 reference manual the fractional clock control registers must be addressed by byte instructions. I don't think mx23 and mx28 have such limitation. I will double check with IC team about this. RTL is generated from a xml file. All registers implement is unified. I don't think only clock control register have such limitation and other registers not. Hi, Section 10.8.24 in the MX28 datasheet (Fractional Clock Control Register 0) states otherwise, but maybe the documentation is simply not matching the silicon. Here's a quote: This register controls the 9-phase fractional clock dividers. The fractional clock frequencies are a product of the values in these registers. NOTE: This register can only be addressed by byte instructions. Addressing word or half- word are not allowed. I also recall seeing weird behavior when these registers were accessed by word access in U-Boot, so I believe the datasheet is correct. 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 V2 RESEND] clk: mxs: Fix invalid 32-bit access to frac registers
On Monday, February 16, 2015 at 09:24:51 PM, Stefan Wahren wrote: Hi Fabio, Fabio Estevam feste...@gmail.com hat am 12. Februar 2015 um 20:08 geschrieben: Hi Stefan, On Thu, Feb 12, 2015 at 4:59 PM, Stefan Wahren stefan.wah...@i2se.com wrote: Hi Fabio, Fabio Estevam feste...@gmail.com hat am 11. Februar 2015 um 22:10 geschrieben: On Wed, Feb 11, 2015 at 6:31 PM, Stefan Wahren stefan.wah...@i2se.com wrote: I always get 0x5e5b5513 with or without your patch. very strange. Do you have any idea why IO1_STABLE is permanent low? On my case it is always 1. i expected the same behavior on my hardware. Do you use u-boot as bootloader? Yes, I do. i will try to test it with u-boot. Can you confirm the behavior according to your flash issue? In my tests IO1_STABLE stays the same. This wasn't the intension of my second question. I wanted to know about the state of the SPI NOR flash detection process. Does it sucessed if you apply the patch, but revert the changes in clk_ref_set_rate() from clk-ref.c? I don't have my mx28evk setup available at the moment, but when I applied your patch and reverted all the changes in clk-ref.c, then the SPI flash detection works. I haven't tested to only reverting the changes inside clk_ref_set_rate(), but I can do it tomorrow. I think the reason for the problem in the flash detection is caused by the misaligned access on the frac register. Misaligned ? In which way ? Can you please elaborate on this ? -- 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] staging: iio: adc: mxs-lradc: Change type in printf format string
On Tuesday, January 27, 2015 at 11:23:33 PM, Rickard Strandqvist wrote: Wrong type in printf format string, requires 'int' but the argument type is 'unsigned int' This was found using a static code analysis program called cppcheck Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se Makes sense, thanks! Reviewed-by: Marek Vasut ma...@denx.de 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 V2 RESEND] clk: mxs: Fix invalid 32-bit access to frac registers
On Tuesday, February 10, 2015 at 10:54:52 PM, Fabio Estevam wrote: Hi Stefan, On Tue, Feb 10, 2015 at 7:24 PM, Stefan Wahren stefan.wah...@i2se.com wrote: thanks this is very helpful. I built the linux-next for my Duckbill and add the SSP2 section from imx28-evk.dts into imx28-duckbill.dts. Without my patch i get the following for HW_CLKCTRL_FRAC0: ./memwatch -a 0x800401B0 0x800401b0: 0x5e5b5513 With my patch i get: ./memwatch -a 0x800401B0 0x800401b0: 0x5e1b5513 So it looks like a problem in my patch. Yes, let's try to get the same value HW_CLKCTRL_FRAC0. I orded such a flash chip, but it will take some time until i can use it with my hardware. Thanks for doing this. Maybe if you find out a way to fix the calculation of HW_CLKCTRL_FRAC0, then I can test it on my board. Hi, the difference is this 0x40 bit, right ? That's _STABLE bit, so it means the clock are not stable, right ? Why would that happen in the first place? 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/
[PATCH] rtlwifi: rtl8192cu: Add new device ID
Add new ID for ASUS N10 WiFi dongle.
Signed-off-by: Marek Vasut ma...@denx.de
Cc: Larry Finger larry.fin...@lwfinger.net
Cc: John W. Linville linvi...@tuxdriver.com
Cc: Chaoming Li chaoming...@realsil.com.cn
---
drivers/net/wireless/rtlwifi/rtl8192cu/sw.c | 1 +
1 file changed, 1 insertion(+)
Bus 001 Device 004: ID 0b05:17ba ASUSTek Computer, Inc.
Device Descriptor:
bLength18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass0 (Defined at Interface level)
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize064
idVendor 0x0b05 ASUSTek Computer, Inc.
idProduct 0x17ba
bcdDevice2.00
iManufacturer 1 Realtek
iProduct2 802.11n WLAN Adapter
iSerial 3 00e04c01
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 46
bNumInterfaces 1
bConfigurationValue 1
iConfiguration 0
bmAttributes 0x80
(Bus Powered)
MaxPower 500mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber0
bAlternateSetting 0
bNumEndpoints 4
bInterfaceClass 255 Vendor Specific Class
bInterfaceSubClass255 Vendor Specific Subclass
bInterfaceProtocol255 Vendor Specific Protocol
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes2
Transfer TypeBulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0200 1x 512 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x02 EP 2 OUT
bmAttributes2
Transfer TypeBulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0200 1x 512 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes2
Transfer TypeBulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0200 1x 512 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x84 EP 4 IN
bmAttributes3
Transfer TypeInterrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 1
Device Qualifier (for other device speed):
bLength10
bDescriptorType 6
bcdUSB 2.00
bDeviceClass0 (Defined at Interface level)
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize064
bNumConfigurations 1
Device Status: 0x
(Bus Powered)
diff --git a/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c
b/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c
index 90a714c..d451d2b 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c
@@ -360,6 +360,7 @@ static struct usb_device_id rtl8192c_usb_ids[] = {
{RTL_USB_DEVICE(0x04f2, 0xaffb, rtl92cu_hal_cfg)}, /*Xavi*/
{RTL_USB_DEVICE(0x04f2, 0xaffc, rtl92cu_hal_cfg)}, /*Xavi*/
{RTL_USB_DEVICE(0x2019, 0x1201, rtl92cu_hal_cfg)}, /*Planex-Vencer*/
+ {RTL_USB_DEVICE(0x0b05, 0x17ba, rtl92cu_hal_cfg)}, /*ASUS-Edimax*/
/** 8192CU /
{RTL_USB_DEVICE(0x050d, 0x1004, rtl92cu_hal_cfg)}, /*Belcom-SurfN300*/
--
2.1.4
--
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 2/2] phy: driver for Conexant Digicolor internal USB PHY
On Friday, March 27, 2015 at 05:36:29 AM, Baruch Siach wrote: Add a driver for the USB PHY on the Conexant CX92755 SoC, from the Digicolor series of SoCs. The PHY is connected to the on-chip chipidea usb2 host. The hardware is somewhat similar to the phy-mxs-usb.c usb_phy, but it is different enough to merit its own driver. Also, this driver uses the generic phy infrastructure. Hi, the register set looks very similar to MXS one indeed. How is it different please ? The driver looks OK. 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] rtlwifi: rtl8192cu: Add new device ID
On Thursday, March 26, 2015 at 01:50:59 AM, Larry Finger wrote:
On 03/25/2015 04:18 PM, Marek Vasut wrote:
Add new ID for ASUS N10 WiFi dongle.
Signed-off-by: Marek Vasut ma...@denx.de
Cc: Larry Finger larry.fin...@lwfinger.net
Cc: John W. Linville linvi...@tuxdriver.com
Cc: Chaoming Li chaoming...@realsil.com.cn
---
drivers/net/wireless/rtlwifi/rtl8192cu/sw.c | 1 +
1 file changed, 1 insertion(+)
Bus 001 Device 004: ID 0b05:17ba ASUSTek Computer, Inc.
Device Descriptor:
bLength18
bDescriptorType 1
bcdUSB 2.00
[...]
diff --git a/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c
b/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c index 90a714c..d451d2b
100644
--- a/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c
@@ -360,6 +360,7 @@ static struct usb_device_id rtl8192c_usb_ids[] = {
{RTL_USB_DEVICE(0x04f2, 0xaffb, rtl92cu_hal_cfg)}, /*Xavi*/
{RTL_USB_DEVICE(0x04f2, 0xaffc, rtl92cu_hal_cfg)}, /*Xavi*/
{RTL_USB_DEVICE(0x2019, 0x1201, rtl92cu_hal_cfg)}, /*Planex-Vencer*/
+ {RTL_USB_DEVICE(0x0b05, 0x17ba, rtl92cu_hal_cfg)}, /*ASUS-Edimax*/
/** 8192CU /
{RTL_USB_DEVICE(0x050d, 0x1004, rtl92cu_hal_cfg)}, /*Belcom-SurfN300*/
Hi!
The full dump from lsusb is a bit much for a commit message. All you need
is the one line from lsusb with no switches set.
That's why I added it below the diffstat, so that it doesn't polute the commit
message.
According to the Vendor driver, which the comment indicates as the source
of part of your patch,
In fact, the comment came from that same source file, from 0b05:17ab line.
The vendor is the same though, so I kept the comment.
this particular device belongs in the Customer ID
section under 8188CU, not where you placed it. In addition, I like to keep
the USB IDs in numerical order within each section. Accordingly, your new
ID should be between 0846:9041 and 0bda:5088.
OK.
I am assuming that you have such a device. A statement that this change
allows it to work should be in the commit message. If you have not tested,
then you need to state that.
I have one and I tested it, I mostly checked the other commit messages and
wrote the same thing. It's a rather mindless change afterall.
--
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/
[PATCH V2] rtlwifi: rtl8192cu: Add new device ID
Add new ID for ASUS N10 WiFi dongle.
Signed-off-by: Marek Vasut ma...@denx.de
Tested-by: Marek Vasut ma...@denx.de
Cc: Larry Finger larry.fin...@lwfinger.net
Cc: John W. Linville linvi...@tuxdriver.com
---
drivers/net/wireless/rtlwifi/rtl8192cu/sw.c | 1 +
1 file changed, 1 insertion(+)
V2: - Move the ID into correct position
- Add my Tested-by to indicate this change was tested on real HW
- Drop the confusing USB device listing from the diffstat section
diff --git a/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c
b/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c
index 90a714c..252eb76 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c
@@ -321,6 +321,7 @@ static struct usb_device_id rtl8192c_usb_ids[] = {
{RTL_USB_DEVICE(0x07b8, 0x8188, rtl92cu_hal_cfg)}, /*Abocom - Abocom*/
{RTL_USB_DEVICE(0x07b8, 0x8189, rtl92cu_hal_cfg)}, /*Funai - Abocom*/
{RTL_USB_DEVICE(0x0846, 0x9041, rtl92cu_hal_cfg)}, /*NetGear WNA1000M*/
+ {RTL_USB_DEVICE(0x0b05, 0x17ba, rtl92cu_hal_cfg)}, /*ASUS-Edimax*/
{RTL_USB_DEVICE(0x0bda, 0x5088, rtl92cu_hal_cfg)}, /*Thinkware-CCC*/
{RTL_USB_DEVICE(0x0df6, 0x0052, rtl92cu_hal_cfg)}, /*Sitecom - Edimax*/
{RTL_USB_DEVICE(0x0df6, 0x005c, rtl92cu_hal_cfg)}, /*Sitecom - Edimax*/
--
2.1.4
--
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] iio: acpi: Add ACPI0008 Ambient Light Sensor
On Wednesday, April 29, 2015 at 05:36:32 PM, Gabriele Mazzotta wrote: [...] I'm sorry, I've just noticed that I haven't changed the value of realbits in acpi_als_channels. This makes me wonder what would be the proper value, given that this is a generic driver and all the information I have are those in the ACPI specification (which states what I reported here above). Should I just set realbits to 32? I believe the ALS reports only 16bit signel value, no ? My observation with a strong coherent light source is that the saturated sensor reported 0x . Probably it's the same for me. I couldn't get to the point where ALI reports 0x, just really close, I will have to try with some stronger lights. However, looking at my ACPI table, I can see that the value returned by _ALI is just the composition of two 8 bits variables put side by side, so yes, I can say that even on my system it's a 16bit value. What kind of hardware are you testing this on ? The problem here is that I'm not sure we can assume this as true in general since the ACPI specification doesn't say anything. Maybe someone more knowledgable can speak up. 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] iio: acpi: Add ACPI0008 Ambient Light Sensor
On Thursday, April 30, 2015 at 01:27:43 PM, Gabriele Mazzotta wrote: On Thursday 30 April 2015 11:44:36 Marek Vasut wrote: On Wednesday, April 29, 2015 at 05:36:32 PM, Gabriele Mazzotta wrote: [...] I'm sorry, I've just noticed that I haven't changed the value of realbits in acpi_als_channels. This makes me wonder what would be the proper value, given that this is a generic driver and all the information I have are those in the ACPI specification (which states what I reported here above). Should I just set realbits to 32? I believe the ALS reports only 16bit signel value, no ? My observation with a strong coherent light source is that the saturated sensor reported 0x . Probably it's the same for me. I couldn't get to the point where ALI reports 0x, just really close, I will have to try with some stronger lights. However, looking at my ACPI table, I can see that the value returned by _ALI is just the composition of two 8 bits variables put side by side, so yes, I can say that even on my system it's a 16bit value. What kind of hardware are you testing this on ? Hi! Dell XPS13 9333. Cool, even better :) I tested it on the following hardware: Acer W500 Retina MacBook Pro 10,1 MacBook Air (I don't know the revision, sorry) The problem here is that I'm not sure we can assume this as true in general since the ACPI specification doesn't say anything. Maybe someone more knowledgable can speak up. I'm looking at some documentation from Microsoft [1] on how to use these sensors in Windows and values as high as 100,000 lux are mentioned. I see, thank you for investigating ! [1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn613948%28v=vs. 85%29.aspx 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: Linux device driver for USB WIFI needs integrating
On Monday, April 27, 2015 at 04:49:09 PM, Anders Larsen wrote: On 2015-04-27 16:23, Chris Ward wrote: With the patch actually applied, the WiFi adapter is recognised and works, both on boot and if the adapter is plugged in later. So, all is well. glad to hear that - and thanks for testing! What mainline kernel level will contain the patch ? Marek (CC'ed) added the USB ID some 4 weeks ago, so it's scheduled for the upcoming 4.1 @Marek: can we get your commit 9374e7d2 added to the stable kernels, please? (the full discussion with Chris is archived here: https://lkml.org/lkml/2015/4/24/156) Feel free to apply for stable. I also tested the patch on Linux 3.10.5x so it ought to work all the way back to at least 3.10. 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] iio: acpi: Add ACPI0008 Ambient Light Sensor
On Thursday, April 30, 2015 at 10:33:46 PM, Paul Bolle wrote: Just a nit: a license mismatch. On Wed, 2015-04-29 at 13:27 +0200, Gabriele Mazzotta wrote: --- /dev/null +++ b/drivers/iio/light/acpi-als.c + * 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/. This states the license is GPL v2. +MODULE_LICENSE(GPL); And, according to include/linux/module.h, this states the license is GPL v2 or later. So I think either the comment at the top of this file or the ident used in the MODULE_LICENSE() macro should be changed. I'm OK with v2+ or later . 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 0/3] Using SPI NOR flah on sunxi.
On Thursday, April 30, 2015 at 06:56:18 PM, Michal Suchanek wrote: On 30 April 2015 at 18:30, thomas.bet...@rohde-schwarz.com wrote: Hello Michal: I tried to connect a SPI NOR flash to my sunxi board and due to the current sunxi SPI driver limitations it does not work. The SPI driver returns an error when more than 64 bytes are transferred at once due to lack of DMA support. Wouldn't it be easier to fix the SPI driver to handle transfers larger than 64 bytes, filling and draining the FIFO multiple times if neccessary? (As far as I can tell, most SPI drivers do this.) Yes, the intent is to fix this by adding dma support to the driver, eventually. The patch might be still useful for other hardware with developing SPI support. Please just fix the controller driver to correctly handle arbitrary transfer lengths. 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 1/3] MTD: m25p80: fix write return value.
On Thursday, April 30, 2015 at 03:33:47 PM, Michal Suchanek wrote: The size of written data was added to user supplied value rather than written at the provided address. You might want to work on the commit message a little, something like the following, but feel free to reword as seen fit. The 'retlen' points to a variable representing the number of data bytes written/read (see include/linux/mtd/mtd.h) by the current invocation of the function. This variable must be set, not incremented. Otherwise, the patch is correct I believe: Acked-by: Marek Vasut ma...@denx.de Signed-off-by: Michal Suchanek hramr...@gmail.com --- drivers/mtd/devices/m25p80.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c index 7c8b169..0b2bc21 100644 --- a/drivers/mtd/devices/m25p80.c +++ b/drivers/mtd/devices/m25p80.c @@ -102,7 +102,7 @@ static void m25p80_write(struct spi_nor *nor, loff_t to, size_t len, spi_sync(spi, m); - *retlen += m.actual_length - cmd_sz; + *retlen = m.actual_length - cmd_sz; } static inline unsigned int m25p80_rx_nbits(struct spi_nor *nor) 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 3/3] MTD: spi-nor: add flag to not use sector erase.
On Friday, May 01, 2015 at 09:05:15 AM, Michal Suchanek wrote: On 1 May 2015 at 01:13, Marek Vasut ma...@denx.de wrote: On Thursday, April 30, 2015 at 11:13:12 PM, Michal Suchanek wrote: The sector size of the flash memory is unclear from datasheet or may possibly vary between chips so add a flag to always use 4k blocks. Currently 4k blocks are always used when possible but in the future somebody might want to do some optimizations with sector erase. Signed-off-by: Michal Suchanek hramr...@gmail.com I _think_ you might be able to determine the size, no ? One way is to ask the vendor, but you can also try something like: 1) erase the whole SPI NOR 2) overwrite it with zeroes (or ones ? I think it should be all ones after erasing). 3) Erase sector 0 4) Read some 128 KiB back 5) Observe what is the difference. I can determine it for this particular chip. However, when the vendor datasheet says the block is 64/32K it might mean that chips with this ID can have either block size. http://www.chingistek.com/img/Product_Files/Pm25LD010020datasheet%20v04.pdf page 21: SECTOR_ER (20h) erases 4kByte sector. BLOCK_ER (d8h) erases 64kByte sector. http://www.gigadevice.com/product/download/366.html?locale=en_US page 27-28: Sector Erase (SE) (20h) erases 4kByte sector 64KB Block Erase (BE) (d8h) erases 64kByte sector It's a value that we don't use anyway so I just mark it as unknown here for future reference. Looks like standard SPI NOR opcodes [1], nothing unknown there ;-) [1] include/linux/mtd/spi-nor.h 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 3/3] MTD: spi-nor: add flag to not use sector erase.
On Monday, May 04, 2015 at 01:11:03 PM, Michal Suchanek wrote: Hello, Hi! On 1 May 2015 at 16:20, Marek Vasut ma...@denx.de wrote: On Friday, May 01, 2015 at 09:05:15 AM, Michal Suchanek wrote: On 1 May 2015 at 01:13, Marek Vasut ma...@denx.de wrote: I can determine it for this particular chip. However, when the vendor datasheet says the block is 64/32K it might mean that chips with this ID can have either block size. http://www.chingistek.com/img/Product_Files/Pm25LD010020datasheet%20v04.p df page 21: SECTOR_ER (20h) erases 4kByte sector. BLOCK_ER (d8h) erases 64kByte sector. http://www.gigadevice.com/product/download/366.html?locale=en_US page 27-28: Sector Erase (SE) (20h) erases 4kByte sector 64KB Block Erase (BE) (d8h) erases 64kByte sector It's pretty much the same as the datasheet I used http://www.elm-tech.com/en/products/spi-flash-memory/gd25q41/gd25q41.pdf It mentions both 32KB Block Erase (BE) (52H) and 64KB Block Erase (BE) (D8H) The SPI NOR framework will use 0xbe opcode, no problem. So the chip probably tries its best to be compatible with any command set and this last patch is not needed. The memory organization table on page 7 is not all that reassuring, though. Which exact part do you refer to please ? 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 3/3] MTD: spi-nor: add flag to not use sector erase.
On Monday, May 04, 2015 at 03:18:56 PM, Michal Suchanek wrote: On 4 May 2015 at 14:12, Marek Vasut ma...@denx.de wrote: On Monday, May 04, 2015 at 01:11:03 PM, Michal Suchanek wrote: Hello, Hi! On 1 May 2015 at 16:20, Marek Vasut ma...@denx.de wrote: On Friday, May 01, 2015 at 09:05:15 AM, Michal Suchanek wrote: On 1 May 2015 at 01:13, Marek Vasut ma...@denx.de wrote: I can determine it for this particular chip. However, when the vendor datasheet says the block is 64/32K it might mean that chips with this ID can have either block size. http://www.chingistek.com/img/Product_Files/Pm25LD010020datasheet%20v0 4.p df page 21: SECTOR_ER (20h) erases 4kByte sector. BLOCK_ER (d8h) erases 64kByte sector. http://www.gigadevice.com/product/download/366.html?locale=en_US page 27-28: Sector Erase (SE) (20h) erases 4kByte sector 64KB Block Erase (BE) (d8h) erases 64kByte sector It's pretty much the same as the datasheet I used http://www.elm-tech.com/en/products/spi-flash-memory/gd25q41/gd25q41.pdf It mentions both 32KB Block Erase (BE) (52H) and 64KB Block Erase (BE) (D8H) The SPI NOR framework will use 0xbe opcode, no problem. So the chip probably tries its best to be compatible with any command set and this last patch is not needed. The memory organization table on page 7 is not all that reassuring, though. Which exact part do you refer to please ? Start of page 7 where it says sector size 32/64K in either datasheet. It can refer to both BE opcode variants being supported but it's quite unclear. My guess here would be that the internal organisation of the SPI NOR is in 4k blocks, which is no surprise really. My understanding is that opcode 0x52 erases 8x4k sector (ie. 32k of data) while 0xd8 erases 16x4k sector (ie. 64k of data). I don't see any problem here -- there are two different opcodes which do two different things and their behavior matches the one on various other SPI NORs. Write protection seems to be calculated in 4k sectors and not blocks so the block size does not seem very relevant. See above. Does it make sense now please ? 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 3/3] MTD: spi-nor: add flag to not use sector erase.
On Monday, May 04, 2015 at 03:39:44 PM, Michal Suchanek wrote: On 4 May 2015 at 15:35, Marek Vasut ma...@denx.de wrote: On Monday, May 04, 2015 at 03:18:56 PM, Michal Suchanek wrote: On 4 May 2015 at 14:12, Marek Vasut ma...@denx.de wrote: On Monday, May 04, 2015 at 01:11:03 PM, Michal Suchanek wrote: It mentions both 32KB Block Erase (BE) (52H) and 64KB Block Erase (BE) (D8H) The SPI NOR framework will use 0xbe opcode, no problem. So the chip probably tries its best to be compatible with any command set and this last patch is not needed. The memory organization table on page 7 is not all that reassuring, though. Which exact part do you refer to please ? Start of page 7 where it says sector size 32/64K in either datasheet. It can refer to both BE opcode variants being supported but it's quite unclear. My guess here would be that the internal organisation of the SPI NOR is in 4k blocks, which is no surprise really. My understanding is that opcode 0x52 erases 8x4k sector (ie. 32k of data) while 0xd8 erases 16x4k sector (ie. 64k of data). I don't see any problem here -- there are two different opcodes which do two different things and their behavior matches the one on various other SPI NORs. Write protection seems to be calculated in 4k sectors and not blocks so the block size does not seem very relevant. See above. Does it make sense now please ? Yes, makes sense. I'm glad to hear this got cleared up, thanks ! :) 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 1/3] MTD: m25p80: fix write return value.
On Thursday, April 30, 2015 at 03:33:47 PM, Michal Suchanek wrote: The 'retlen' points to a variable representing the number of data bytes written/read (see include/linux/mtd/mtd.h) by the current invocation of the function. This variable must be set, not incremented. v2: clearer commit message This V2 goes past the diffstat, so that when the patch is applied, it doesn't end up in the log. Signed-off-by: Michal Suchanek hramr...@gmail.com Acked-by: Marek Vasut ma...@denx.de --- drivers/mtd/devices/m25p80.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Aka. Vn changes go here. I don't think there's a need to repost tho :) diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c index 7c8b169..0b2bc21 100644 --- a/drivers/mtd/devices/m25p80.c +++ b/drivers/mtd/devices/m25p80.c @@ -102,7 +102,7 @@ static void m25p80_write(struct spi_nor *nor, loff_t to, size_t len, spi_sync(spi, m); - *retlen += m.actual_length - cmd_sz; + *retlen = m.actual_length - cmd_sz; } static inline unsigned int m25p80_rx_nbits(struct spi_nor *nor) 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 3/3] MTD: spi-nor: add flag to not use sector erase.
On Thursday, April 30, 2015 at 11:13:12 PM, Michal Suchanek wrote: The sector size of the flash memory is unclear from datasheet or may possibly vary between chips so add a flag to always use 4k blocks. Currently 4k blocks are always used when possible but in the future somebody might want to do some optimizations with sector erase. Signed-off-by: Michal Suchanek hramr...@gmail.com I _think_ you might be able to determine the size, no ? One way is to ask the vendor, but you can also try something like: 1) erase the whole SPI NOR 2) overwrite it with zeroes (or ones ? I think it should be all ones after erasing). 3) Erase sector 0 4) Read some 128 KiB back 5) Observe what is the difference. 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] iio: acpi: Add ACPI0008 Ambient Light Sensor
On Wednesday, April 29, 2015 at 01:51:21 PM, Gabriele Mazzotta wrote: On Wednesday 29 April 2015 13:27:25 Gabriele Mazzotta wrote: Add basic implementation of the ACPI0008 Ambient Light Sensor driver. This driver currently supports only the ALI property, yet is ready to be easily extended to handle ALC, ALT, ALP ones as well. Signed-off-by: Martin Liska marxin.li...@gmail.com Signed-off-by: Marek Vasut ma...@denx.de Signed-off-by: Gabriele Mazzotta gabriele@gmail.com Cc: Zhang Rui rui.zh...@intel.com --- This continues http://marc.info/?t=14016346322 I've made few adjustments over the original patch: - Code aligned with 4.1-rc1 and cleaned up - Use signed integers to store values: sensors report 32bit signed values. In particular, -1 is reported when the current reading is above the supported range of sensitivity. Most of the changes are just a consequence of the changes in the iio subsystem. Gabriele I'm sorry, I've just noticed that I haven't changed the value of realbits in acpi_als_channels. This makes me wonder what would be the proper value, given that this is a generic driver and all the information I have are those in the ACPI specification (which states what I reported here above). Should I just set realbits to 32? I believe the ALS reports only 16bit signel value, no ? My observation with a strong coherent light source is that the saturated sensor reported 0x . 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 v9] iio: acpi: Add support for ACPI0008 Ambient Light Sensor
On Friday, May 08, 2015 at 11:34:35 PM, Jonathan Cameron wrote: On 8 May 2015 20:20:33 BST, Gabriele Mazzotta gabriele@gmail.com wrote: On Friday 08 May 2015 10:58:29 Jonathan Cameron wrote: On 02/05/15 08:30, Gabriele Mazzotta wrote: This driver adds the initial support for the ACPI Ambient Light Sensor as defined in Section 9.2 of the ACPI specification (Revision 5.0) [1]. Sensors complying with the standard are exposed as ACPI devices with ACPI0008 as hardware ID and provide standard methods by which the OS can query properties of the ambient light environment the system is currently operating in. This driver currently allows only to get the current ambient light illuminance reading through the _ALI method, but is ready to be extended extended to handle _ALC, _ALT and _ALP as well. [1] http://www.acpi.info/DOWNLOADS/ACPIspec50.pdf Signed-off-by: Martin Liska marxin.li...@gmail.com Signed-off-by: Marek Vasut ma...@denx.de Signed-off-by: Gabriele Mazzotta gabriele@gmail.com Cc: Zhang Rui rui.zh...@intel.com Thank you guys for finally getting this mainline :) 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: simple framebuffer slower by factor of 20, on socfpga (arm) platform
On Tuesday, April 07, 2015 at 02:19:33 PM, Geert Uytterhoeven wrote: Hi Pavel, On Tue, Apr 7, 2015 at 2:12 PM, Pavel Machek pa...@ucw.cz wrote: I have an socfpga board, which uses has simple framebuffer implemented in the FPGA. On 3.15, framebuffer is fast: root@wagabuibui:~# time cat /dev/fb0 /dev/null real 0m 0.00s user 0m 0.00s sys0m 0.00s on 3.18, this takes 220msec. Similar slowdown exists for writes. Simple framebuffer did not change at all between 3.15 and 3.18; resource flags of the framebuffer are still same (0x200). If I enable caching on 3.18, it speeds up a bit, to 70msec or so... Which means problem is not only in caching. Any ideas? My first guess was commit 67dc0d4758e5 (vt_buffer: drop console buffer copying optimisations), but this was introduced only in v4.0-rc1. Just in case you encounter another performance regression after upgrading to a more modern kernel ;-) Why don't you use the Altera VIP FB on SoCFPGA ? 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 10/11] spi: add more debug prints in s3c64xx
On Wednesday, June 03, 2015 at 11:26:42 PM, Michal Suchanek wrote: The SPI NOR transfers mysteriously fail so add more debug prints about SPI transactions. Signed-off-by: Michal Suchanek hramr...@gmail.com dev_dbg() and friends would certainly be nicer here. 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 00/11] Enable access to SPI NOR flash on Samsung Snow board
On Wednesday, June 03, 2015 at 11:26:39 PM, Michal Suchanek wrote: Hello, Hi, this patch series makes it possible to access the SPI NOR flash in the Samsung XE303 'Snow' Chromebook. Unfortunately not all issues are resolved. To work around an issue with the pl330 dma engine I respun the patch for limiting transfer size in m25p80 driver. Looks like fixing a bug at the wrong place to me ... As the flash does not contain any sane filesystem and is only likely to be accessed with mtd_debug or similar tool the limit of the dma engine is easily reached. Filesystems using shorter data transfers are less likely to be affected. Sounds like the DMA engine driver should be fixed. 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 04/11] mtd: ofpart: do not fail probe when no partitions exist
On Wednesday, June 03, 2015 at 11:26:40 PM, Michal Suchanek wrote: On Exynos it is necessary to set SPI controller parameters that apply to a SPI slave in a DT subnode of the slave device. The ofpart code returns an error when there are subnodes of the SPI flash but no partitions are found. Change this condition to a warning so that flash without partitions can be accessed on Exynos. I have to admit the rationale for this patch is not very clear to me, sorry. Can you please explain this a bit more ? 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 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, June 03, 2015 at 11:26:41 PM, Michal Suchanek wrote: On sunxi the SPI controller currently does not have DMA support and fails any transfer larger than 63 bytes. On Exynos the pl330 DMA controller fails any transfer larger than 64kb when using slower speed like 40MHz and any transfer larger than 128bytes when running at 133MHz. This smells more like some corruption of the data on the bus or something even more obscure. The best thing is that in both cases the controller can just lock up and never finish potentially leaving the hardware in unusable state. So it is required that the m25p80 driver actively prevents doing transfers that are too large for the current driver state on a particular piece of hardware. Signed-off-by: Michal Suchanek hramr...@gmail.com -- Update commit message and documentation text. --- .../devicetree/bindings/mtd/jedec,spi-nor.txt | 6 ++ drivers/mtd/devices/m25p80.c | 24 -- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt index 2bee681..4e63ae8 100644 --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt @@ -19,6 +19,12 @@ Optional properties: all chips and support for it can not be detected at runtime. Refer to your chips' datasheet to check if this is supported by your chip. +- linux,max_tx_len : With some SPI controller drivers possible transfer size is + limited. This may be hardware or driver bug. + Transfer data in chunks no larger than this value. + Using this option may severely degrade performance and + possibly flash memory life when max_tx_len is smaller than + flash page size (typically 256 bytes) Will we need similar patch for all other SPI slave drivers, like SPI NAND ? 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 04/11] mtd: ofpart: do not fail probe when no partitions exist
On Thursday, June 04, 2015 at 05:40:58 PM, Michal Suchanek wrote:
On 4 June 2015 at 17:28, Marek Vasut ma...@denx.de wrote:
On Thursday, June 04, 2015 at 06:54:00 AM, Michal Suchanek wrote:
On 4 June 2015 at 00:58, Marek Vasut ma...@denx.de wrote:
On Wednesday, June 03, 2015 at 11:26:40 PM, Michal Suchanek wrote:
On Exynos it is necessary to set SPI controller parameters that apply
to a SPI slave in a DT subnode of the slave device. The ofpart code
returns an error when there are subnodes of the SPI flash but no
partitions are found. Change this condition to a warning so that
flash without partitions can be accessed on Exynos.
I have to admit the rationale for this patch is not very clear to me,
sorry. Can you please explain this a bit more ?
This is how the DT entry for SPI slave looks with s3c64xx:
flash: m25p80@0 {
#address-cells = 1;
#size-cells = 1;
compatible = jedec,spi-nor;
reg = 0;
spi-max-frequency = 4000;
linux,max_tx_len = 65536;
SIDENOTE: I thought this was actually added by your patch #8 in this
series. The underscores in the name of the property are not really
consistent with the rest of the names.
m25p,fast-read;
controller-data {
samsung,spi-feedback-delay = 0;
};
};
this is example of flash partitions:
flash@0 {
#address-cells = 1;
#size-cells = 1;
partition@0 {
label = u-boot;
reg = 0x000 0x10;
read-only;
};
uimage@10 {
reg = 0x010 0x20;
};
};
The parser ignores any flash without subnodes and returns 0 (no
partititon). When there is a subnode it assumes the flash is
partitioned and tries to parse the subnodes as partitions. When there
are subnodes and none parses as partition an error is returned. As
shown above it is valid to have subnodes on unpartitioned flash.
When an error is returned from a partition parser the mtdpart code
passes on this error to the flash probe function and the proble of the
flash fails.
What does /proc/mtd tell you when you have no partitions defined
in the DT ? It should provide you with the entire MTD device and
the code shouldn't even try to parse any OF partitions, since you
don't have any.
mtdinfo shows I have no mtd devices and the log shows that probe
failed unless I patch the kernel.
When there is *support* for of partitions the ofparser is run on mtd
probe. If it fails because it considers
controller-data {
samsung,spi-feedback-delay = 0;
};
an invalid partition and does not find any valid partition the probe fails.
OK, now it has become clearer to me, thanks!
There are two problems here
1) the above is not invalid. It just is not a partition. The parser
should not fail seeing this
The parser should complain verbosely and ignore this I guess ?
2) the mtd probe should not fail when a partition parser fails and
should present the unpartitioned device
OK
Both are addressed in separate patches.
Thanks
Michal
--
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 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Thursday, June 04, 2015 at 06:31:45 AM, Michal Suchanek wrote: On 4 June 2015 at 01:03, Marek Vasut ma...@denx.de wrote: On Wednesday, June 03, 2015 at 11:26:41 PM, Michal Suchanek wrote: On sunxi the SPI controller currently does not have DMA support and fails any transfer larger than 63 bytes. On Exynos the pl330 DMA controller fails any transfer larger than 64kb when using slower speed like 40MHz and any transfer larger than 128bytes when running at 133MHz. This smells more like some corruption of the data on the bus or something even more obscure. If the data was corrupted you would get corrupted data and not transfer failure. AFAIK there is no checksum or anything. I actually do get corrupted data when I use wrong feedback delay setting. OK The best thing is that in both cases the controller can just lock up and never finish potentially leaving the hardware in unusable state. [...] --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt @@ -19,6 +19,12 @@ Optional properties: all chips and support for it can not be detected at runtime. Refer to your chips' datasheet to check if this is supported by your chip. +- linux,max_tx_len : With some SPI controller drivers possible transfer size is + limited. This may be hardware or driver bug. + Transfer data in chunks no larger than this value. + Using this option may severely degrade performance and + possibly flash memory life when max_tx_len is smaller than + flash page size (typically 256 bytes) Will we need similar patch for all other SPI slave drivers, like SPI NAND ? Probably. Some SPI slave drivers already do have similar option. So the SPI device drivers are implementing workarounds for buggy SPI hosts, even if those SPI devices themselves don't suffer from such limitations. Yes, this seems like the seriously wrong thing to do. In general it cannot be expected that you can reliably transfer arbitrarily large data over SPI it seems. This is what should be expected because this is how the bus is supposed to work in fact. You can transfer an arbitrary amount of data over SPI bus. If some driver has a limitation, it's that (bus, dma) driver which needs to implement a fix or a workaround. However, if the nand driver transfers data a page at a time it should be fine. ... until someone issues multi-block read, in which case it all falls down like a house of cards. It is impossible to build a reliable system on should and similar assumptions ; the driver must be solid. 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 11/11] dt: Exynos: add Snow SPI NOR node.
On Thursday, June 04, 2015 at 04:04:18 AM, Krzysztof Kozlowski wrote:
On 04.06.2015 06:26, Michal Suchanek wrote:
The Snow has onboard SPI NOR flash which contains the bootloader.
Add DT node for this flash chip. The flash is rated 133MHz but the pl330
controller can transfer only up to 128 bytes at this speed so use more
conservative settings. Even at 40MHz pl330 can transfer at most 64k with
the current driver.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
arch/arm/boot/dts/exynos5250-snow.dts | 12
1 file changed, 12 insertions(+)
diff --git a/arch/arm/boot/dts/exynos5250-snow.dts
b/arch/arm/boot/dts/exynos5250-snow.dts index 1fa72cf..38e4cda 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -691,6 +691,18 @@
num-cs = 1;
cs-gpios = gpa2 5 0;
status = okay;
+ flash: m25p80@0 {
The indentation looks odd. This should be at the same level as status.
+ #address-cells = 1;
+ #size-cells = 1;
+ compatible = jedec,spi-nor;
+ reg = 0;
+ spi-max-frequency = 4000;
So actually you wanted 133 MHz but as a workaround for DMA issue you use
40 MHz, right? Could you add here a small TODO note in comment about it?
I disagree. There is a problem, the problem should actually be analyzed and
fixed, not just postponed with some TODO nonsense.
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 00/11] Enable access to SPI NOR flash on Samsung Snow board
On Thursday, June 04, 2015 at 06:21:32 AM, Michal Suchanek wrote: On 4 June 2015 at 00:53, Marek Vasut ma...@denx.de wrote: On Wednesday, June 03, 2015 at 11:26:39 PM, Michal Suchanek wrote: Hello, Hi, this patch series makes it possible to access the SPI NOR flash in the Samsung XE303 'Snow' Chromebook. Unfortunately not all issues are resolved. To work around an issue with the pl330 dma engine I respun the patch for limiting transfer size in m25p80 driver. Looks like fixing a bug at the wrong place to me ... As the flash does not contain any sane filesystem and is only likely to be accessed with mtd_debug or similar tool the limit of the dma engine is easily reached. Filesystems using shorter data transfers are less likely to be affected. Sounds like the DMA engine driver should be fixed. I looked at the pl330 datasheet and don't see anything obviusly wrong with the pl330 driver in Linux. Ideally it would be fixed but I have no idea what's wrong with it. Ask maybe Catalin from ARM to help you -- he might be able to point you to the right hardware guy. 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 04/11] mtd: ofpart: do not fail probe when no partitions exist
On Thursday, June 04, 2015 at 06:54:00 AM, Michal Suchanek wrote:
On 4 June 2015 at 00:58, Marek Vasut ma...@denx.de wrote:
On Wednesday, June 03, 2015 at 11:26:40 PM, Michal Suchanek wrote:
On Exynos it is necessary to set SPI controller parameters that apply to
a SPI slave in a DT subnode of the slave device. The ofpart code returns
an error when there are subnodes of the SPI flash but no partitions are
found. Change this condition to a warning so that flash without
partitions can be accessed on Exynos.
I have to admit the rationale for this patch is not very clear to me,
sorry. Can you please explain this a bit more ?
This is how the DT entry for SPI slave looks with s3c64xx:
flash: m25p80@0 {
#address-cells = 1;
#size-cells = 1;
compatible = jedec,spi-nor;
reg = 0;
spi-max-frequency = 4000;
linux,max_tx_len = 65536;
SIDENOTE: I thought this was actually added by your patch #8 in this
series. The underscores in the name of the property are not really
consistent with the rest of the names.
m25p,fast-read;
controller-data {
samsung,spi-feedback-delay = 0;
};
};
this is example of flash partitions:
flash@0 {
#address-cells = 1;
#size-cells = 1;
partition@0 {
label = u-boot;
reg = 0x000 0x10;
read-only;
};
uimage@10 {
reg = 0x010 0x20;
};
};
The parser ignores any flash without subnodes and returns 0 (no
partititon). When there is a subnode it assumes the flash is
partitioned and tries to parse the subnodes as partitions. When there
are subnodes and none parses as partition an error is returned. As
shown above it is valid to have subnodes on unpartitioned flash.
When an error is returned from a partition parser the mtdpart code
passes on this error to the flash probe function and the proble of the
flash fails.
What does /proc/mtd tell you when you have no partitions defined
in the DT ? It should provide you with the entire MTD device and
the code shouldn't even try to parse any OF partitions, since you
don't have any.
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: Official bugreport 4.1 kernel (audio gadget and ChipIdea)
On Tuesday, June 30, 2015 at 04:23:01 AM, Peter Chen wrote: On Fri, Jun 26, 2015 at 07:15:18PM +0200, Sébastien Pruvost wrote: Hello, I'm sending this mail to report a bug concerning the latest kernel 4.1. Here is the problem (and the test I've done): I have firstly used the 3.10.53 kernel for my two sabrelites in order to use the audio gadget driver with the Dual Role ChipIdea Controller (in order to switch roles between my two IMX6 sabreLite). After loading g_audio in my two sabreLite and plugging the cable (microA – microB), there is an error “ci_hdrc.0 request length too big for isochronous snd_uac2.0 1116 Error”. And even after running aplay command, I still got this error and there is no sound getting out of the jack port. I've switched roles between the two boards by following this: https:// www.kernel.org/doc/Documentation/usb/chipidea.txt. This works fine with the serial driver, I can see a new serial interface (host side) and after switching role a new serial interfaces at device side. Same thing for ethernet gadget: this works fine too. But not with the audio gadget. In fact, there is a new audio interface at host side but I can not interact with it (even alsamixer doesn’t see any controls on this new sound card). I’ve tested that audio gadget works fine if I don’t use ChipIdea HighSpeed Dual Role Controller. Secondly I have tested this audio gadget with the latest Kernel 4.1 for my two IMX6 sabrelites (imx_v6_v7_defconfig). Now these previous errors are gone but there are still no sound getting out of the jack port (even if there are a new sound card in host side) It is may not a role switch problem, please check if the g_audio can work well with an ubuntu PC (make sure your codec works well). ci_hdrc.0 request length too big for isochronous Doesn't this just mean it cannot transfer such a long buffer via ISO pipe ? I guess the UAC should send smaller buffers to work with the CI HDRC? 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] Input: stmpe-ts - enforce device tree only mode
On Saturday, May 23, 2015 at 06:41:50 PM, Dmitry Torokhov wrote: On May 23, 2015 9:38:54 AM PDT, Marek Vasut ma...@denx.de wrote: On Saturday, May 23, 2015 at 12:58:32 AM, Dmitry Torokhov wrote: The STMPE MFD is only used with device tree configured systems (and STMPE MFD core depends on OF), so force the configuration to come from device tree only. Signed-off-by: Dmitry Torokhov dmitry.torok...@gmail.com --- Hi! [...] @@ -351,14 +336,13 @@ static int stmpe_input_probe(struct platform_device *pdev) idev-name = STMPE_TS_NAME; idev-phys = STMPE_TS_NAME/input0; idev-id.bustype = BUS_I2C; - idev-evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS); - idev-keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH); idev-open = stmpe_ts_open; idev-close = stmpe_ts_close; input_set_drvdata(idev, ts); + input_set_capability(idev, EV_KEY, BTN_TOUCH); Isn't this part of the patch dropping the EV_ABS evbit ? No, on newer kernels input_set_abs_paramd() takes care of setting we EV_ABS in evbit as well. Oh, I see, thanks for enlightening me! Reviewed-by: Marek Vasut ma...@denx.de 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] Input: stmpe-ts - enforce device tree only mode
On Saturday, May 23, 2015 at 12:58:32 AM, Dmitry Torokhov wrote: The STMPE MFD is only used with device tree configured systems (and STMPE MFD core depends on OF), so force the configuration to come from device tree only. Signed-off-by: Dmitry Torokhov dmitry.torok...@gmail.com --- Hi! [...] @@ -351,14 +336,13 @@ static int stmpe_input_probe(struct platform_device *pdev) idev-name = STMPE_TS_NAME; idev-phys = STMPE_TS_NAME/input0; idev-id.bustype = BUS_I2C; - idev-evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS); - idev-keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH); idev-open = stmpe_ts_open; idev-close = stmpe_ts_close; input_set_drvdata(idev, ts); + input_set_capability(idev, EV_KEY, BTN_TOUCH); Isn't this part of the patch dropping the EV_ABS evbit ? input_set_abs_params(idev, ABS_X, 0, XY_MASK, 0, 0); input_set_abs_params(idev, ABS_Y, 0, XY_MASK, 0, 0); input_set_abs_params(idev, ABS_PRESSURE, 0x0, 0xff, 0, 0); [...] 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
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/literature/ 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. 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
On Monday, August 17, 2015 at 06:03:38 PM, Brian Norris wrote: Hi! [...] 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. Thanks for CCing me, I assume this is a driver for that altera_epcq_controller which you can synthesise into your FPGA, is that correct? Is there an Hard IP variant of this controller in the public or is this purely Soft IP? Also, I cannot find any documentation for this IP block even if I search through Quartus/QSys, is there any proper documentation available anywhere? [...] 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 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.
[...]
+static struct flash_device flash_devices[] = {
+ FLASH_ID(epcs16, EPCS_OPCODE_ID, 0x14),
+ FLASH_ID(epcs64, EPCS_OPCODE_ID, 0x16),
+ FLASH_ID(epcs128, EPCS_OPCODE_ID, 0x18),
+
+ FLASH_ID(epcq16, NON_EPCS_OPCODE_ID, 0x15),
+ FLASH_ID(epcq32, NON_EPCS_OPCODE_ID, 0x16),
+ FLASH_ID(epcq64, NON_EPCS_OPCODE_ID, 0x17),
+ FLASH_ID(epcq128, NON_EPCS_OPCODE_ID, 0x18),
Re: [PATCH linux-next v4 5/5] mtd: atmel-quadspi: add driver for Atmel QSPI controller
On Monday, August 24, 2015 at 02:49:24 PM, Russell King - ARM Linux wrote: Hi Russell, On Mon, Aug 24, 2015 at 01:03:51PM +0200, Marek Vasut wrote: These are functions, not macros :) btw is there any reason for these ? I'd say, just put the read*() and write*() functions directly into the code and be done with it, it is much less confusing. Also, why do you use the _relaxed() versions of the functions ? Now that the _relaxed() accessors are available throughout the kernel, everyone should be using the _relaxed() versions unless they need the properties of the non-relaxed versions. You mean the memory barrier, right ? Remember that the non-relaxed versions are rather expensive on ARM due to the need to go all the way out to the L2 cache - it at least doubles the number of accesses for every read*/write*(). I think in case of this driver, we don't need the non-relaxed version anywhere, right ? Thanks for the educational writeup :) btw. is [1] still the current study material on the I/O accessor best practices please ? [1] http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117644 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 linux-next v4 5/5] mtd: atmel-quadspi: add driver for Atmel QSPI controller
On Monday, August 24, 2015 at 07:04:38 PM, Cyrille Pitchen wrote:
Hi Marek,
Hi!
Le 24/08/2015 13:03, Marek Vasut a écrit :
On Monday, August 24, 2015 at 12:14:00 PM, Cyrille Pitchen wrote:
This driver add support to the new Atmel QSPI controller embedded into
sama5d2x SoCs. It expects a NOR memory to be connected to the QSPI
controller.
[...]
+ /* Compute address parameters */
+ switch (cmd-enable.bits.address) {
+ case 4:
+ ifr |= QSPI_IFR_ADDRL;
+ /*break;*/ /* fallback to the 24bit address case */
What's this commented out bit of code for ? :-)
I just wanted to stress out there was no missing break;.
I've reworded the comment to:
/* No break on purpose: fallback to the 24bit address case. */
Oh, the address is in bytes . I see, yes, it makes sense to be more
explicit here about the purpose of the fallback. I think this change
in the comment will make it easier for everyone who comes back in a
few years and reads this code.
+ case 3:
+ iar = (cmd-enable.bits.data) ? 0 : cmd-address;
+ ifr |= QSPI_IFR_ADDREN;
+ break;
+ case 0:
+ break;
+ default:
+ return -EINVAL;
+ }
[...]
+no_data:
+ /* Poll INSTRuction End status */
+ sr = qspi_readl(aq, QSPI_SR);
+ if (sr QSPI_SR_INSTRE)
+ return err;
+
+ /* Wait for INSTRuction End interrupt */
+ init_completion(aq-completion);
You should use reinit_completion() in the code. init_completion()
should be used only in the probe() function and nowhere else.
Alright. In the next version I'll rename the completion member of
struct atmel_qspi into cmd_completion. Also I'll add another
dma_completion member in this very same structure to replace the local
struct completion completion in atmel_qspi_run_dma_transfer().
Then I'll call init_completion() on both cmd_completion and dma_completion
only from atmel_qspi_probe() and reinit_completion() elsewhere.
+ aq-pending = 0;
+ qspi_writel(aq, QSPI_IER, QSPI_SR_INSTRE);
+ if (!wait_for_completion_timeout(aq-completion,
+ msecs_to_jiffies(1000)))
+ err = -ETIMEDOUT;
+ qspi_writel(aq, QSPI_IDR, QSPI_SR_INSTRE);
+
+ return err;
+}
[...]
Hope this helps :)
Indeed, it does! I still work on the next version of this series to take
all your comments into account.
Thanks :)
--
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 linux-next v4 3/5] mtd: spi-nor: allow to tune the number of dummy cycles
On Monday, August 24, 2015 at 06:42:46 PM, Cyrille Pitchen wrote:
Hi Marek,
Hi!
[...]
- * Dummy Cycle calculation for different type of read.
- * It can be used to support more commands with
- * different dummy cycle requirements.
- */
-static inline int spi_nor_read_dummy_cycles(struct spi_nor *nor)
-{
- switch (nor-flash_read) {
- case SPI_NOR_FAST:
- case SPI_NOR_DUAL:
- case SPI_NOR_QUAD:
- return 8;
- case SPI_NOR_NORMAL:
- return 0;
- }
- return 0;
-}
You can probably just soup up this function so that it sets the
nor-read_dummy, no ?
Actually, this is what the patch does: spi_nor_read_dummy_cycles() was
reused and enhanced few lines below where you've pointed out the
switch (nor-flash_read) block should be move after the else block.
You know what? I'll go get some sleep, coffee doesn't cut it anymore :)
I think when I wrote the code I've chosen to move the definition of this
function instead of adding forward declarations of functions such as
read_cr() or write_sr_cr(), which are now called by
micron_set_dummy_cycles().
Yep, that's all right, sorry for the confusion.
-/*
* Write status register 1 byte
* Returns negative if error occurred.
*/
@@ -1012,6 +994,81 @@ static int set_quad_mode(struct spi_nor *nor,
struct flash_info *info) }
}
[...]
+/*
+ * Dummy Cycle calculation for different type of read.
+ * It can be used to support more commands with
+ * different dummy cycle requirements.
+ */
+static int spi_nor_read_dummy_cycles(struct spi_nor *nor,
+ const struct flash_info *info)
+{
+ struct device_node *np = nor-dev-of_node;
+ u32 num_dummy_cycles;
+
+ if (np !of_property_read_u32(np, m25p,num-dummy-cycles,
+ num_dummy_cycles)) {
+ nor-read_dummy = num_dummy_cycles;
+
+ /*
+ * This switch block might be moved after the if...then...else
+ * statement but it was not tested with all Spansion or Micron
+ * memories.
+ * Now the m25p,num-dummy-cycles property needs to be
+ * explicitly set in the device tree so the switch statement is
+ * executed. This should avoid unwanted side effects and keep
+ * backward compatibility.
+ */
+ switch (JEDEC_MFR(info)) {
+ case CFI_MFR_ST:
+ return micron_set_dummy_cycles(nor);
+ default:
If you do have m25p,num-dummy-cycles set for non-micron flash, you have a
problem here I believe.
+ break;
+ }
+ } else {
The solution would be to drop this else {} bit here, so that if you fail
in the DT-based configuration, you fall back to this old behavior. What
do you think please ? :)
Good idea!
I also add a trace for the default case of switch (JEDEC_MFR(info)):
dev_warn(dev, can't set the number of dummy cycles\n);
Maybe change this to setting the number of dummy cycles not supported by chip,
ignoring or something, to be explicit about the fallback and that this is not
supported by the chip. But this is just an idea, feel free to ignore it.
So the user is notified that the driver could not use the value of
m25p,num-dummy-cycles from the DT before falling back to the legacy
code.
Yup.
+ switch (nor-flash_read) {
+ case SPI_NOR_FAST:
+ case SPI_NOR_DUAL:
+ case SPI_NOR_QUAD:
+ nor-read_dummy = 8;
+ case SPI_NOR_NORMAL:
+ nor-read_dummy = 0;
+ }
+ }
+
+ return 0;
+}
[...]
thanks for the review!
Im glad it helped ;-)
--
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 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) ? 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 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 Flash 0 raw
data; reg = 0x0
Re: [PATCH] [PATCH v4] mtd:spi-nor: Add Altera Quad SPI Driver
On Thursday, August 20, 2015 at 10:06:29 AM, Viet Nga Dao wrote: 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/lite ra 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/liter atu 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. I see, thanks ! 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 Thursday, August 20, 2015 at 10:17:57 AM, Viet Nga Dao wrote:
Sorry for missing to reply the last question
Hi!
diff --git a/drivers/mtd/spi-nor/spi-nor.c
b/drivers/mtd/spi-nor/spi-nor.c index 14a5d23..2ab7279 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -687,6 +687,24 @@ static const struct spi_device_id spi_nor_ids[] =
{
{ cat25c09, CAT25_INFO( 128, 8, 32, 2, SPI_NOR_NO_ERASE |
SPI_NOR_NO_FR) }, { cat25c17, CAT25_INFO( 256, 8, 32, 2,
SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) }, { cat25128, CAT25_INFO(2048, 8,
64, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) }, +
+ /* Altera EPCQ/EPCS Flashes are non-JEDEC */
Are they really ? Last time I checked on CV SoC, I was able to read
their JEDEC ID just fine ; though it's true I used that EPCS core.
Altera EPCS flash is non-jedec device. And this new controller is not
EPCS controller. If you look at the documentation i sent in another
email, they are not the same.
https://www.altera.com/content/dam/altera-
www/global/en_US/pdfs/literature/hb/cfg/cfg_cf52012.pdf page 15 ; seems
like opcode 0x9f is supported. All the other opcodes seems compatible too.
What is the problem ?
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to 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 Thursday, August 20, 2015 at 11:18:14 AM, Viet Nga Dao wrote: Hi, [...] That is why we decided to upstream the driver. If the hardware fix, there might not need to have any changes in driver to support or if yes, it will be just minor. If the hardware can do proper READID opcode, this entire nonsense table will go away and a proper integration into the SPI NOR framework will take place. You might consider submitting this driver for staging, but I definitely am not a big fan of that. You might misunderstand the hardware problem i mention here. This soft IP controller is able to provide the ID for our Altera EPCS/EPCQ flash chips, which are non JEDEC chips. As from EPCQ device data sheet (https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature /hb/cfg/cfg_cf52012.pdf), the device ID is 8 bit data. So what exactly is the output of READID instruction followed by 6 Byte read on an EPCQ chip? The remaining problem here is that this controller is able to support Micron chips but it currently has limitation in providing only 8 bit ID, which is not full JEDEC ID for Micron chips. OK Hence, we are asking hardware engineer to find out the solution so that the driver does not need to do any dirty hacking. And so, this table should still be here even hardware fix will take place or not. [...] Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to 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 linux-next v4 1/5] mtd: spi-nor: notify (Q)SPI controller about protocol change
On Monday, August 24, 2015 at 12:13:56 PM, Cyrille Pitchen wrote: Once the Quad SPI mode has been enabled on a Micron flash memory, this device expects ALL the following commands to use the SPI 4-4-4 protocol. The (Q)SPI controller needs to be notified about the protocol change so it can adapt and keep on dialoging with the Micron memory. Signed-off-by: Cyrille Pitchen cyrille.pitc...@atmel.com Awesome, Acked-by: Marek Vasut ma...@denx.de If this could be applied separately (since I want to use the same functionality for the Cadence QSPI driver), I'd be really happy too :) 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 linux-next v4 4/5] Documentation: atmel-quadspi: add binding file for Atmel QSPI driver
On Monday, August 24, 2015 at 12:13:59 PM, Cyrille Pitchen wrote: This patch documents the DT bindings for the driver of the Atmel QSPI controller embedded inside sama5d2x SoCs. Signed-off-by: Cyrille Pitchen cyrille.pitc...@atmel.com Acked-by: Nicolas Ferre nicolas.fe...@atmel.com If my ack has any value in here, feel free to add it, the bindings look pretty standard anyway: Acked-by: Marek Vasut ma...@denx.de 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 linux-next v4 3/5] mtd: spi-nor: allow to tune the number of dummy cycles
On Monday, August 24, 2015 at 12:13:58 PM, Cyrille Pitchen wrote:
The number of dummy cycles used during Fast Read commands can be reduced
to improve transfer performances. Each manufacturer has a dedicated set of
registers to provide the memory with the exact number of dummy cycles it
should expect. Both the memory and the (Q)SPI controller must agree on
this number of dummy cycles.
The number of dummy cycles can be found into the memory datasheet and
mostly depends on the SPI clock frequency, the Fast Read op code and the
Single/Dual Data Rate mode.
Probing JEDEC Serial Flash Discoverable Parameters (SFDP) tables would
only provide the driver with a high enough number of dummy cycles for each
Fast Read command to be used for all clock frequencies: this solution
would not be optimized.
Signed-off-by: Cyrille Pitchen cyrille.pitc...@atmel.com
Hi!
drivers/mtd/spi-nor/spi-nor.c | 97
++- include/linux/mtd/spi-nor.h
| 2 +
2 files changed, 80 insertions(+), 19 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index e2a6029dc056..869e098a6841 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -119,24 +119,6 @@ static int read_cr(struct spi_nor *nor)
}
/*
- * Dummy Cycle calculation for different type of read.
- * It can be used to support more commands with
- * different dummy cycle requirements.
- */
-static inline int spi_nor_read_dummy_cycles(struct spi_nor *nor)
-{
- switch (nor-flash_read) {
- case SPI_NOR_FAST:
- case SPI_NOR_DUAL:
- case SPI_NOR_QUAD:
- return 8;
- case SPI_NOR_NORMAL:
- return 0;
- }
- return 0;
-}
You can probably just soup up this function so that it sets the
nor-read_dummy, no ?
-/*
* Write status register 1 byte
* Returns negative if error occurred.
*/
@@ -1012,6 +994,81 @@ static int set_quad_mode(struct spi_nor *nor, struct
flash_info *info) }
}
+static int micron_set_dummy_cycles(struct spi_nor *nor)
+{
+ int ret;
+ u8 val, mask;
+
+ /* read the Volatile Configuration Register (VCR) */
NIT: If this is a sentence, start it with capital letter and end it with
fullstop :)
+ ret = nor-read_reg(nor, SPINOR_OP_RD_VCR, val, 1);
+ if (ret 0) {
+ dev_err(nor-dev, error %d reading VCR\n, ret);
+ return ret;
+ }
+
+ write_enable(nor);
+
+ /* update the number of dummy into the VCR */
DTTO
+ mask = GENMASK(7, 4);
+ val = ~mask;
+ val |= (nor-read_dummy 4) mask;
+ ret = nor-write_reg(nor, SPINOR_OP_WR_VCR, val, 1, 0);
+ if (ret 0) {
+ dev_err(nor-dev, error while writing VCR register\n);
+ return ret;
+ }
+
+ ret = spi_nor_wait_till_ready(nor);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+/*
+ * Dummy Cycle calculation for different type of read.
+ * It can be used to support more commands with
+ * different dummy cycle requirements.
+ */
+static int spi_nor_read_dummy_cycles(struct spi_nor *nor,
+ const struct flash_info *info)
+{
+ struct device_node *np = nor-dev-of_node;
+ u32 num_dummy_cycles;
+
+ if (np !of_property_read_u32(np, m25p,num-dummy-cycles,
+ num_dummy_cycles)) {
+ nor-read_dummy = num_dummy_cycles;
+
+ /*
+ * This switch block might be moved after the if...then...else
+ * statement but it was not tested with all Spansion or Micron
+ * memories.
+ * Now the m25p,num-dummy-cycles property needs to be
+ * explicitly set in the device tree so the switch statement is
+ * executed. This should avoid unwanted side effects and keep
+ * backward compatibility.
+ */
+ switch (JEDEC_MFR(info)) {
+ case CFI_MFR_ST:
+ return micron_set_dummy_cycles(nor);
+ default:
If you do have m25p,num-dummy-cycles set for non-micron flash, you have a
problem here I believe.
+ break;
+ }
+ } else {
The solution would be to drop this else {} bit here, so that if you fail in
the DT-based configuration, you fall back to this old behavior. What do you
think please ? :)
+ switch (nor-flash_read) {
+ case SPI_NOR_FAST:
+ case SPI_NOR_DUAL:
+ case SPI_NOR_QUAD:
+ nor-read_dummy = 8;
+ case SPI_NOR_NORMAL:
+ nor-read_dummy = 0;
+ }
+ }
+
+ return 0;
+}
[...]
--
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
Re: [PATCH linux-next v4 5/5] mtd: atmel-quadspi: add driver for Atmel QSPI controller
On Monday, August 24, 2015 at 12:14:00 PM, Cyrille Pitchen wrote:
This driver add support to the new Atmel QSPI controller embedded into
sama5d2x SoCs. It expects a NOR memory to be connected to the QSPI
controller.
Signed-off-by: Cyrille Pitchen cyrille.pitc...@atmel.com
Acked-by: Nicolas Ferre nicolas.fe...@atmel.com
Hi,
[...]
+/* Register access macros */
These are functions, not macros :)
btw is there any reason for these ? I'd say, just put the read*() and
write*() functions directly into the code and be done with it, it is
much less confusing.
Also, why do you use the _relaxed() versions of the functions ?
+static inline u32 qspi_readl(struct atmel_qspi *aq, u32 reg)
+{
+ return readl_relaxed(aq-regs + reg);
+}
+
+static inline void qspi_writel(struct atmel_qspi *aq, u32 reg, u32 value)
+{
+ writel_relaxed(value, aq-regs + reg);
+}
+
+static inline u16 qspi_readw(struct atmel_qspi *aq, u32 reg)
+{
+ return readw_relaxed(aq-regs + reg);
+}
+
+static inline void qspi_writew(struct atmel_qspi *aq, u32 reg, u16 value)
+{
+ writew_relaxed(value, aq-regs + reg);
+}
+
+static inline u8 qspi_readb(struct atmel_qspi *aq, u32 reg)
+{
+ return readb_relaxed(aq-regs + reg);
+}
+
+static inline void qspi_writeb(struct atmel_qspi *aq, u32 reg, u8 value)
+{
+ writeb_relaxed(value, aq-regs + reg);
+}
[...]
+static int atmel_qspi_run_command(struct atmel_qspi *aq,
+ const struct atmel_qspi_command *cmd)
+{
+ u32 iar, icr, ifr, sr;
+ int err = 0;
+
+ iar = 0;
+ icr = 0;
+ ifr = aq-ifr_width | cmd-ifr_tfrtyp;
+
+ /* Compute instruction parameters */
+ if (cmd-enable.bits.instruction) {
+ icr |= QSPI_ICR_INST(cmd-instruction);
+ ifr |= QSPI_IFR_INSTEN;
+ }
+
+ /* Compute address parameters */
+ switch (cmd-enable.bits.address) {
+ case 4:
+ ifr |= QSPI_IFR_ADDRL;
+ /*break;*/ /* fallback to the 24bit address case */
What's this commented out bit of code for ? :-)
+ case 3:
+ iar = (cmd-enable.bits.data) ? 0 : cmd-address;
+ ifr |= QSPI_IFR_ADDREN;
+ break;
+ case 0:
+ break;
+ default:
+ return -EINVAL;
+ }
[...]
+no_data:
+ /* Poll INSTRuction End status */
+ sr = qspi_readl(aq, QSPI_SR);
+ if (sr QSPI_SR_INSTRE)
+ return err;
+
+ /* Wait for INSTRuction End interrupt */
+ init_completion(aq-completion);
You should use reinit_completion() in the code. init_completion()
should be used only in the probe() function and nowhere else.
+ aq-pending = 0;
+ qspi_writel(aq, QSPI_IER, QSPI_SR_INSTRE);
+ if (!wait_for_completion_timeout(aq-completion,
+ msecs_to_jiffies(1000)))
+ err = -ETIMEDOUT;
+ qspi_writel(aq, QSPI_IDR, QSPI_SR_INSTRE);
+
+ return err;
+}
[...]
Hope this helps :)
--
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 Thursday, August 20, 2015 at 10:19:25 PM, Brian Norris wrote:
On Thu, Aug 20, 2015 at 12:06:36PM +0200, Alexander Stein wrote:
On Thursday 20 August 2015 10:03:38, Marek Vasut wrote:
+Example:
+
+ quadspi_controller_0: quadspi@0x180014a0 {
+ compatible = altr,quadspi-1.0;
+ reg = 0x180014a0 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).
Do you mean specifying partitions in OF is deprecated in general? Is
there any link for that? What would be an alternative to it?
This is the first I've heard of such a deprecation. I would argue that
it's still probably one of the best ways to specify partitions. It's not
exactly perfect (partitions aren't really hardware, so may not belong
in the hardware description)
Which is the reasoning why I heard it was deprecated, but it seems my
knowledge is not correct. I apologize for the misinformation, sorry.
, but IMO it's better than cmdlineparts,
for instance. I also don't really know of any good-enough, generically
useful on-flash partition formats, especially ones that can handle the
complexities of NAND.
Right.
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to 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 1/2] mtd: spi-nor: rework spi nor read and write.
On Wednesday, July 29, 2015 at 06:16:21 AM, Michal Suchanek wrote:
On 28 July 2015 at 20:15, Marek Vasut ma...@denx.de wrote:
On Tuesday, July 28, 2015 at 11:23:02 AM, Michal Suchanek wrote:
The spi_nor read and write functions pass thru the mtd retlen to the
chip-specific read and write function. This makes it difficult to check
for errors in read and write functions and these errors are not checked.
This leads to silent data corruption.
This patch styles the chip-specific read and write function as unix
read(2) and write(2) and updates the retlen in the spi-nor generic
driver.
This also makes it possible to check for write errors.
When pl330 fails to transfer the flash data over SPI I get I/O error
instead of 4M of zeroes.
I do not have sst and fsl hardware to test with.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
drivers/mtd/devices/m25p80.c | 33 ++-
drivers/mtd/spi-nor/fsl-quadspi.c | 29 ++--
drivers/mtd/spi-nor/spi-nor.c | 57
--- include/linux/mtd/spi-nor.h
| 8 +++---
4 files changed, 81 insertions(+), 46 deletions(-)
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index d313f948b..d8f064b 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -75,14 +75,15 @@ static int m25p80_write_reg(struct spi_nor *nor, u8
opcode, u8 *buf, int len, return spi_write(spi, flash-command, len +
1);
}
-static void m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
- size_t *retlen, const u_char *buf)
+static ssize_t m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
+ const u_char *buf)
{
struct m25p *flash = nor-priv;
struct spi_device *spi = flash-spi;
struct spi_transfer t[2] = {};
struct spi_message m;
int cmd_sz = m25p_cmdsz(nor);
+ ssize_t ret;
spi_message_init(m);
@@ -100,9 +101,14 @@ static void m25p80_write(struct spi_nor *nor,
loff_t to, size_t len, t[1].len = len;
spi_message_add_tail(t[1], m);
- spi_sync(spi, m);
+ ret = spi_sync(spi, m);
+ if (ret)
+ return ret;
- *retlen += m.actual_length - cmd_sz;
+ ret = m.actual_length - cmd_sz;
+ if (ret 0)
+ return -EIO;
+ return ret;
I'd prefer to just add the return value and keep the retlen to keep error
codes and transfer length separate.
I prefer to not pass around retlen because passing it around
- causes code duplication
- makes the code harder to understand
Not sure I want to argue on this one, since it looks like a matter of taste.
Adding the error return first and removing the retlen second might make the
patch more readable though.
btw. you change the transfer length from unsigned to signed type -- long
transfer might get interpreted as an error.
Note that ssize_t is supposed to be enough for write(2) so when it
does not suffice you have a real system-wide problem.
That aside NOR flash sizes tend to be in the order of megabytes so
this concern is very theoretical.
I do hope so :)
--
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 1/2] mtd: spi-nor: Add GD25LQ32C 1.8V SPI NOR flash ID
On Tuesday, July 28, 2015 at 11:07:57 AM, Michal Suchanek wrote:
This 1.8V SPI NOR flash is found on ARM Chromebook XE303C and reads
something like 25LQ32VIG in the middle.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
drivers/mtd/spi-nor/spi-nor.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index d78831b..cba3bd0 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -559,6 +559,7 @@ static const struct spi_device_id spi_nor_ids[] = {
/* GigaDevice */
{ gd25q32, INFO(0xc84016, 0, 64 * 1024, 64, SECT_4K) },
+ { gd25lq32c, INFO(0xc86016, 0, 64 * 1024, 64, SECT_4K) },
{ gd25q64, INFO(0xc84017, 0, 64 * 1024, 128, SECT_4K) },
{ gd25q128, INFO(0xc84018, 0, 64 * 1024, 256, SECT_4K) },
Don't we have DT bindings, so we can use jedec,spi-nor prop for all
new SPI NORs without growing this table ?
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 2/2] mtd: spi-nor: Add Pm25LD020 and GD25Q41B chip ID.
On Tuesday, July 28, 2015 at 11:07:58 AM, Michal Suchanek wrote:
First chip reads Pm25LD020 or Pm25L0020. Found on some WD HDD PCB.
Identified as PMC Pm25LD020.
Flash read does not return consistent data which explains why the disk
died.
Second chip reads something like 25Q41BT. Found on Esspif ESP8266 based
ESP-01 board. Identified as Giga Devices GD25Q41B.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
drivers/mtd/spi-nor/spi-nor.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index cba3bd0..e55689d 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -558,6 +558,7 @@ static const struct spi_device_id spi_nor_ids[] = {
{ mb85rs1mt, INFO(0x047f27, 0, 128 * 1024, 1, SPI_NOR_NO_ERASE) },
/* GigaDevice */
+ { gd25q41b, INFO(0xc84013, 0, 64 * 1024, 8, SECT_4K) },
{ gd25q32, INFO(0xc84016, 0, 64 * 1024, 64, SECT_4K) },
{ gd25lq32c, INFO(0xc86016, 0, 64 * 1024, 64, SECT_4K) },
{ gd25q64, INFO(0xc84017, 0, 64 * 1024, 128, SECT_4K) },
@@ -601,6 +602,7 @@ static const struct spi_device_id spi_nor_ids[] = {
/* PMC */
{ pm25lv512, INFO(0,0, 32 * 1024,2, SECT_4K_PMC) },
{ pm25lv010, INFO(0,0, 32 * 1024,4, SECT_4K_PMC) },
+ { pm25ld020, INFO(0x7f9d22, 0, 64 * 1024,4, SECT_4K) },
{ pm25lq032, INFO(0x7f9d46, 0, 64 * 1024, 64, SECT_4K) },
/* Spansion -- single (large) sector size only, at least
You might want to split this into two patches, but see my comment on 1/2
and please wait for Brian's confirmation there before you do anything.
Thanks!
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 1/2] mtd: spi-nor: Add GD25LQ32C 1.8V SPI NOR flash ID
On Tuesday, July 28, 2015 at 04:36:29 PM, Michal Suchanek wrote:
On 28 July 2015 at 16:33, Marek Vasut ma...@denx.de wrote:
On Tuesday, July 28, 2015 at 11:07:57 AM, Michal Suchanek wrote:
This 1.8V SPI NOR flash is found on ARM Chromebook XE303C and reads
something like 25LQ32VIG in the middle.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
drivers/mtd/spi-nor/spi-nor.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/spi-nor/spi-nor.c
b/drivers/mtd/spi-nor/spi-nor.c index d78831b..cba3bd0 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -559,6 +559,7 @@ static const struct spi_device_id spi_nor_ids[] = {
/* GigaDevice */
{ gd25q32, INFO(0xc84016, 0, 64 * 1024, 64, SECT_4K) },
+ { gd25lq32c, INFO(0xc86016, 0, 64 * 1024, 64, SECT_4K) },
{ gd25q64, INFO(0xc84017, 0, 64 * 1024, 128, SECT_4K) },
{ gd25q128, INFO(0xc84018, 0, 64 * 1024, 256, SECT_4K) },
Don't we have DT bindings, so we can use jedec,spi-nor prop for all
new SPI NORs without growing this table ?
When jedec,spi-nor compatible is used an attempt is made to identify
the chip using this table.
When the ID is not in the table (or is misread due to transfer error)
spi-nor probe fails.
Dang, I was under the impression we want to avoid growing this table :(
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 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Thursday, July 30, 2015 at 02:18:09 PM, Michal Suchanek wrote: On 30 July 2015 at 13:24, Marek Vasut ma...@denx.de wrote: On Monday, July 27, 2015 at 10:43:05 PM, Michal Suchanek wrote: On 27 July 2015 at 19:43, Marek Vasut ma...@denx.de wrote: On Monday, July 27, 2015 at 11:46:25 AM, Michal Suchanek wrote: On 24 July 2015 at 10:34, Marek Vasut ma...@denx.de wrote: On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: Ok, so here is some summary. I have a NOR flash attached to a s3c64xx SPI controller with 64byte fifo and a pl330 dma controller. Normally DMA controller is used for transfers that do not fit into the FIFO. I tried adding the flash memory ID to the spi-nor driver table and adding a DT node for it. The flash is rated at 120MHz so I used that speed but the ID was bit-shifted and identification failed. There is DT property samsung,spi-feedback-delay for addressing this and at 120MHz it must be 2 or 3 on this board. 40MHz works with default value 0. The next step after identification worked was to try reading the flash content. For this the DMA controller is used because data is transferred in blocks larger than 64 bytes. When reading the whole 4MB flash the transfer failed silently. I got a 4MB file of all ones or all zeroes. It turns out that - the pl330 locks up when transfering large amount of data. Specifically, the maximum power of 2 sized transfer at 120MHz is 128 bytes and 64k at 40MHz. Transferring more than this results in the pl330 locking up and never signalling completion of the transfer. Hypothesis: I think this might just be that the controller didn't catch all the inbound clock ticks and thus counted less inbound data than it was set up to receive. The controller thus waits for more data. The flash chip can transfer data as long as you keep the clock going, especially when you transfer 64k off a 4M flash. The read command is bound just by stopping the clock. There is always more data to be had. Sure, if you keep clocking the chip, the data will keep going. Is the chip being clocked ? There is always data. Like in whenever you want to read SPI data you sample the input pin and watever you sample is data so far as SPI bus is concerned. Aren't you forgetting that the data are synchronous to the clock which you (your SPI block) generate ? Doesn't the PL330 have some kind of a counter register where you can check how much data were received so far ? That should be sufficient to verify this hypothesis of mine. Could check that, yes. Maybe I could read the counter in a function which dmaengine client uses to tear down the dma transfer. Might be a good idea. Check if the register is stuck for too long and then zap the transfer. On a related note I enabled more prints on the spidev test program. I have code that tests maximum transfer size up to the 4k spidev limit and it can transfer full 4k buffer of NOPs at 80MHz. The recieved data is all ones except a 00 at the start. So it looks like read-only transfers fail but full-duplex sort of work. And it reproduces the 00 prefix that was seen in the dma-only setup in otherwise working setup :-S An attempt to read a page of data using the fast-read command fails with timeout. It would be a good idea to detail what your program exactly does on the bus (like, my program sends 3 bytes of data 0xf0 0x0b 0xar and then receives 60 bytes of data). Maybe I'm lost, but your test is really not clear. Data is left in FIFO which causes subsequent commands to fail since garbage is returned instead of command reply. Also subsequent read may cause I/O error or or return an empty page depending on the garbage around. So the driver for the DMA controller might need to be augmented to handle this case -- add a timeout and in case DMA times out, drain the FIFO to make sure subsequent transfers do not fail. There is no way to add timeout in the DMA driver since it does not know the SPI clock. The timeout is handled by the SPI driver and it should be possible to augment it to drain the FIFO when DMA fails so long as FIFO level is readable somewhere. If the DMA doesn't complete in certain amount of time, it should time out I'd say. Don't you think ? No. The DMA driver has no idea if the transfer is at 133MHz, 40MHz, 1MHz, 1kHz, whatever. That's not really important, is it ? If the transfer doesn't finish in, say, 1 second, and it is a 4096b transfer, then it is most likely timeout. On the other hand, adding a flush_fifo in the SPI driver after DMA failure allows probing the chip reliably by realoding the driver even just after a failed transfer. OK - the I/O errors are not checked in spi-nor at all which leads to silent data corruption. On a suggestion that this may improve reliability I changed
Re: [PATCH v2 1/6] mtd: spi-nor: change return value of read/write
On Monday, August 03, 2015 at 08:39:01 PM, Michal Suchanek wrote:
Change the return value of spi-nor device read and write methods to
allow returning amount of data transferred and errors as
read(2)/write(2) does.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
include/linux/mtd/spi-nor.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index e540952..7d782cb 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -185,9 +185,9 @@ struct spi_nor {
int (*write_reg)(struct spi_nor *nor, u8 opcode, u8 *buf, int len,
int write_enable);
- int (*read)(struct spi_nor *nor, loff_t from,
+ ssize_t (*read)(struct spi_nor *nor, loff_t from,
size_t len, size_t *retlen, u_char *read_buf);
- void (*write)(struct spi_nor *nor, loff_t to,
+ ssize_t (*write)(struct spi_nor *nor, loff_t to,
size_t len, size_t *retlen, const u_char *write_buf);
int (*erase)(struct spi_nor *nor, loff_t offs);
You realize that if someone does bisect and has only this patch applied,
the compiler will complain loudly about mismatching data types, right ? :)
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 1/2] mtd: spi-nor: Add GD25LQ32C 1.8V SPI NOR flash ID
On Tuesday, July 28, 2015 at 05:19:18 PM, Rafał Miłecki wrote:
On 28 July 2015 at 16:33, Marek Vasut ma...@denx.de wrote:
On Tuesday, July 28, 2015 at 11:07:57 AM, Michal Suchanek wrote:
This 1.8V SPI NOR flash is found on ARM Chromebook XE303C and reads
something like 25LQ32VIG in the middle.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
drivers/mtd/spi-nor/spi-nor.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/spi-nor/spi-nor.c
b/drivers/mtd/spi-nor/spi-nor.c index d78831b..cba3bd0 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -559,6 +559,7 @@ static const struct spi_device_id spi_nor_ids[] = {
/* GigaDevice */
{ gd25q32, INFO(0xc84016, 0, 64 * 1024, 64, SECT_4K) },
+ { gd25lq32c, INFO(0xc86016, 0, 64 * 1024, 64, SECT_4K) },
{ gd25q64, INFO(0xc84017, 0, 64 * 1024, 128, SECT_4K) },
{ gd25q128, INFO(0xc84018, 0, 64 * 1024, 256, SECT_4K) },
Don't we have DT bindings, so we can use jedec,spi-nor prop for all
new SPI NORs without growing this table ?
You confused this table with the one in m25p80.c.
Uh oh, sorry.
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 2/2] mtd: spi-nor: Add Pm25LD020 and GD25Q41B chip ID.
On Tuesday, July 28, 2015 at 05:22:17 PM, Rafał Miłecki wrote:
On 28 July 2015 at 16:33, Marek Vasut ma...@denx.de wrote:
On Tuesday, July 28, 2015 at 11:07:58 AM, Michal Suchanek wrote:
First chip reads Pm25LD020 or Pm25L0020. Found on some WD HDD PCB.
Identified as PMC Pm25LD020.
Flash read does not return consistent data which explains why the disk
died.
Second chip reads something like 25Q41BT. Found on Esspif ESP8266 based
ESP-01 board. Identified as Giga Devices GD25Q41B.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
drivers/mtd/spi-nor/spi-nor.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/mtd/spi-nor/spi-nor.c
b/drivers/mtd/spi-nor/spi-nor.c index cba3bd0..e55689d 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -558,6 +558,7 @@ static const struct spi_device_id spi_nor_ids[] = {
{ mb85rs1mt, INFO(0x047f27, 0, 128 * 1024, 1, SPI_NOR_NO_ERASE)
},
/* GigaDevice */
+ { gd25q41b, INFO(0xc84013, 0, 64 * 1024, 8, SECT_4K) },
{ gd25q32, INFO(0xc84016, 0, 64 * 1024, 64, SECT_4K) },
{ gd25lq32c, INFO(0xc86016, 0, 64 * 1024, 64, SECT_4K) },
{ gd25q64, INFO(0xc84017, 0, 64 * 1024, 128, SECT_4K) },
@@ -601,6 +602,7 @@ static const struct spi_device_id spi_nor_ids[] = {
/* PMC */
{ pm25lv512, INFO(0,0, 32 * 1024,2, SECT_4K_PMC)
}, { pm25lv010, INFO(0,0, 32 * 1024,4,
SECT_4K_PMC) },
+ { pm25ld020, INFO(0x7f9d22, 0, 64 * 1024,4, SECT_4K) },
{ pm25lq032, INFO(0x7f9d46, 0, 64 * 1024, 64, SECT_4K) },
/* Spansion -- single (large) sector size only, at least
You might want to split this into two patches, but see my comment on 1/2
and please wait for Brian's confirmation there before you do anything.
Since these flashes are from 2 different vendors, it may make sense to
use 2 separated patches. I don't care too much however.
Adding these entries is OK in general.
Thanks for clarifying, I'll make sure to remember this :)
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 V6 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
;
+
+ cqspi-pdev = pdev;
+ platform_set_drvdata(pdev, cqspi);
+
+ ret = cqspi_of_get_pdata(pdev);
+ if (ret) {
+ dev_err(dev, Get platform data failed.\n);
+ return -ENODEV;
+ }
+
+ cqspi-clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(cqspi-clk)) {
+ dev_err(dev, cannot get qspi clk\n);
+ ret = PTR_ERR(cqspi-clk);
+ goto probe_failed;
+ }
+ cqspi-master_ref_clk_hz = clk_get_rate(cqspi-clk);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ cqspi-iobase = devm_ioremap_resource(dev, res);
+ if (IS_ERR(cqspi-iobase)) {
+ dev_err(dev, devm_ioremap_resource res 0 failed\n);
+ ret = PTR_ERR(cqspi-iobase);
+ goto probe_failed;
+ }
+
+ res_ahb = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ cqspi-ahb_phy_addr = res_ahb-start;
+ cqspi-ahb_base = devm_ioremap_resource(dev, res_ahb);
+ if (IS_ERR(cqspi-ahb_base)) {
+ dev_err(dev, devm_ioremap_resource res 1 failed\n);
+ ret = PTR_ERR(cqspi-ahb_base);
+ goto probe_failed;
+ }
+
+ init_completion(cqspi-transfer_complete);
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq 0) {
+ dev_err(dev, platform_get_irq failed\n);
+ ret = -ENXIO;
+ goto probe_failed;
+ }
+ ret = devm_request_irq(dev, irq,
+cqspi_irq_handler, 0, pdev-name, cqspi);
+ if (ret) {
+ dev_err(dev, devm_request_irq failed\n);
+ goto probe_failed;
+ }
+
+ cqspi_wait_idle(cqspi);
+ cqspi_controller_init(cqspi);
+ cqspi-current_cs = -1;
+ cqspi-sclk = 0;
+
+ /* Get flash device data */
+ for_each_available_child_of_node(dev-of_node, np) {
+ unsigned int cs;
+ struct cqspi_flash_pdata *f_pdata;
+
+ if (of_property_read_u32(np, reg, cs)) {
+ dev_err(dev, couldn't determine chip select\n);
+ return -ENXIO;
+ }
+ if (cs = CQSPI_MAX_CHIPSELECT) {
+ dev_err(dev, chip select %d out of range\n, cs);
+ return -ENXIO;
+ }
+ f_pdata = cqspi-f_pdata[cs];
+
+ ret = cqspi_of_get_flash_pdata(pdev, f_pdata, np);
+ if (ret)
+ goto probe_failed;
+
+ nor = f_pdata-nor;
+ mtd = f_pdata-mtd;
+
+ nor-mtd = mtd;
+ nor-dev = dev;
+ nor-priv = cqspi;
+ mtd-priv = nor;
+
+ nor-read_reg = cqspi_read_reg;
+ nor-write_reg = cqspi_write_reg;
+ nor-read = cqspi_read;
+ nor-write = cqspi_write;
+ nor-erase = cqspi_erase;
+
+ nor-prepare = cqspi_prep;
+
+ /*
+ * Here is a 'nasty hack' from Marek Vasut to pick
+ * up OF properties from flash device subnode.
+ */
+ nor-dev-of_node = np;
WARNING: HERE IS A HACK! This is true indeed, can someone please give
feedback on how to do this right ?
+ ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
+ if (ret)
+ goto probe_failed;
+
+ if (nor-read_dummy CQSPI_DUMMY_CLKS_MAX)
+ nor-read_dummy = CQSPI_DUMMY_CLKS_MAX;
+
+ ppdata.of_node = np;
+ ret = mtd_device_parse_register(mtd, NULL, ppdata, NULL, 0);
+ if (ret)
+ goto probe_failed;
+ }
[...]
--
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 1/2] mtd: spi-nor: rework spi nor read and write.
On Tuesday, July 28, 2015 at 11:23:02 AM, Michal Suchanek wrote:
The spi_nor read and write functions pass thru the mtd retlen to the
chip-specific read and write function. This makes it difficult to check
for errors in read and write functions and these errors are not checked.
This leads to silent data corruption.
This patch styles the chip-specific read and write function as unix
read(2) and write(2) and updates the retlen in the spi-nor generic driver.
This also makes it possible to check for write errors.
When pl330 fails to transfer the flash data over SPI I get I/O error
instead of 4M of zeroes.
I do not have sst and fsl hardware to test with.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
drivers/mtd/devices/m25p80.c | 33 ++-
drivers/mtd/spi-nor/fsl-quadspi.c | 29 ++--
drivers/mtd/spi-nor/spi-nor.c | 57
--- include/linux/mtd/spi-nor.h
| 8 +++---
4 files changed, 81 insertions(+), 46 deletions(-)
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index d313f948b..d8f064b 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -75,14 +75,15 @@ static int m25p80_write_reg(struct spi_nor *nor, u8
opcode, u8 *buf, int len, return spi_write(spi, flash-command, len + 1);
}
-static void m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
- size_t *retlen, const u_char *buf)
+static ssize_t m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
+ const u_char *buf)
{
struct m25p *flash = nor-priv;
struct spi_device *spi = flash-spi;
struct spi_transfer t[2] = {};
struct spi_message m;
int cmd_sz = m25p_cmdsz(nor);
+ ssize_t ret;
spi_message_init(m);
@@ -100,9 +101,14 @@ static void m25p80_write(struct spi_nor *nor, loff_t
to, size_t len, t[1].len = len;
spi_message_add_tail(t[1], m);
- spi_sync(spi, m);
+ ret = spi_sync(spi, m);
+ if (ret)
+ return ret;
- *retlen += m.actual_length - cmd_sz;
+ ret = m.actual_length - cmd_sz;
+ if (ret 0)
+ return -EIO;
+ return ret;
I'd prefer to just add the return value and keep the retlen to keep error
codes and transfer length separate.
btw. you change the transfer length from unsigned to signed type -- long
transfer might get interpreted as an error.
[...]
--
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 V6 1/2] mtd: spi-nor: Bindings for Cadence Quad SPI Flash Controller driver.
On Tuesday, July 28, 2015 at 07:38:02 PM, Graham Moore wrote: A commit message would be really nice :-) Signed-off-by: Graham Moore grmo...@opensource.altera.com --- V2: Add cdns prefix to driver-specific bindings. V3: Use existing property is-decoded-cs instead of creating a duplicate, ext-decoder. Timing parameters are in nanoseconds, not master reference clocks. Remove bus-num completely. V4: Add new properties fifo-width and trigger-address --- .../devicetree/bindings/mtd/cadence_quadspi.txt| 54 1 file changed, 54 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/cadence_quadspi.txt diff --git a/Documentation/devicetree/bindings/mtd/cadence_quadspi.txt b/Documentation/devicetree/bindings/mtd/cadence_quadspi.txt new file mode 100644 index 000..d7e6fdd --- /dev/null +++ b/Documentation/devicetree/bindings/mtd/cadence_quadspi.txt @@ -0,0 +1,54 @@ +* Cadence Quad SPI controller + +Required properties: +- compatible : Should be cdns,qspi-nor. +- reg : Contains two entries, each of which is a tuple consisting of a + physical address and length. The first entry is the address and + length of the controller register set. The second entry is the + address and length of the QSPI Controller data area. +- interrupts : Unit interrupt specifier for the controller interrupt. +- clocks : phandle to the Quad SPI clock. +- fifo-depth : Size of the data FIFO in words. This and the following two are absolutelly Cadence specific, so prefix them that way please: cdns,fifo-depth +- fifo-width: Bus width of the data FIFO in bytes. cdns,fifo-width +- trigger-address : 32-bit indirect AHB trigger address. cdns,trigger-address What is this address exactly? Is this documented somewhere what the value should be precisely ? [...] -- 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 V6 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
On Tuesday, July 28, 2015 at 08:22:05 PM, Graham Moore wrote: On 07/28/2015 01:07 PM, Marek Vasut wrote: On Tuesday, July 28, 2015 at 07:38:03 PM, Graham Moore wrote: DTTO here. Thanks a lot for working on the driver though -- would you like me to continue reviewing or just take over please ? Aha, I see your strategy :) You must *really* want to take this over. Fine, you can have it. I don't care either way, it's just a driver on my radar which I'd like to see mainline because I use it on my board. I can keep reviewing until we bash it into sensible state :) And thanks, I'm glad somebody actually gives a hoot. You're welcome, I do care indeed. I use UBI/UBIFS on a QSPI NOR here. 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 v2 1/6] mtd: spi-nor: change return value of read/write
On Wednesday, August 05, 2015 at 10:27:05 AM, Michal Suchanek wrote:
On 4 August 2015 at 18:42, Marek Vasut ma...@denx.de wrote:
On Tuesday, August 04, 2015 at 08:42:51 AM, Michal Suchanek wrote:
On 3 August 2015 at 23:46, Marek Vasut ma...@denx.de wrote:
On Monday, August 03, 2015 at 08:39:01 PM, Michal Suchanek wrote:
Change the return value of spi-nor device read and write methods to
allow returning amount of data transferred and errors as
read(2)/write(2) does.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
include/linux/mtd/spi-nor.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/mtd/spi-nor.h
b/include/linux/mtd/spi-nor.h index e540952..7d782cb 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -185,9 +185,9 @@ struct spi_nor {
int (*write_reg)(struct spi_nor *nor, u8 opcode, u8 *buf, int
len,
int write_enable);
- int (*read)(struct spi_nor *nor, loff_t from,
+ ssize_t (*read)(struct spi_nor *nor, loff_t from,
size_t len, size_t *retlen, u_char *read_buf);
- void (*write)(struct spi_nor *nor, loff_t to,
+ ssize_t (*write)(struct spi_nor *nor, loff_t to,
size_t len, size_t *retlen, const u_char
*write_buf);
int (*erase)(struct spi_nor *nor, loff_t offs);
You realize that if someone does bisect and has only this patch
applied, the compiler will complain loudly about mismatching data
types, right ?
:)
Yes, the compiler prints a warning. However, only the return value
which is not used changes so it should not cause any real problem.
Are you certain that ssize_t is equal to int , always , everywhere ? :-)
No, it's larger or equal.
Not per any standard to my knowledge, but yeah. I don't think it's a good
idea to do this change alone.
Note that it is possible to do such conversion even without introducing
compiler warnings along the way -- just do the data type conversion first
and the other changes in subsequent patches. It will be clear that one
patch does one thing then.
If this is the only concern with these patches it is easily amended ;-)
It'd be nice if the others reviewed it as well, additional feedback would be
nice.
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 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Monday, July 27, 2015 at 10:43:05 PM, Michal Suchanek wrote: On 27 July 2015 at 19:43, Marek Vasut ma...@denx.de wrote: On Monday, July 27, 2015 at 11:46:25 AM, Michal Suchanek wrote: On 24 July 2015 at 10:34, Marek Vasut ma...@denx.de wrote: On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: Ok, so here is some summary. I have a NOR flash attached to a s3c64xx SPI controller with 64byte fifo and a pl330 dma controller. Normally DMA controller is used for transfers that do not fit into the FIFO. I tried adding the flash memory ID to the spi-nor driver table and adding a DT node for it. The flash is rated at 120MHz so I used that speed but the ID was bit-shifted and identification failed. There is DT property samsung,spi-feedback-delay for addressing this and at 120MHz it must be 2 or 3 on this board. 40MHz works with default value 0. The next step after identification worked was to try reading the flash content. For this the DMA controller is used because data is transferred in blocks larger than 64 bytes. When reading the whole 4MB flash the transfer failed silently. I got a 4MB file of all ones or all zeroes. It turns out that - the pl330 locks up when transfering large amount of data. Specifically, the maximum power of 2 sized transfer at 120MHz is 128 bytes and 64k at 40MHz. Transferring more than this results in the pl330 locking up and never signalling completion of the transfer. Hypothesis: I think this might just be that the controller didn't catch all the inbound clock ticks and thus counted less inbound data than it was set up to receive. The controller thus waits for more data. The flash chip can transfer data as long as you keep the clock going, especially when you transfer 64k off a 4M flash. The read command is bound just by stopping the clock. There is always more data to be had. Sure, if you keep clocking the chip, the data will keep going. Is the chip being clocked ? Doesn't the PL330 have some kind of a counter register where you can check how much data were received so far ? That should be sufficient to verify this hypothesis of mine. Data is left in FIFO which causes subsequent commands to fail since garbage is returned instead of command reply. Also subsequent read may cause I/O error or or return an empty page depending on the garbage around. So the driver for the DMA controller might need to be augmented to handle this case -- add a timeout and in case DMA times out, drain the FIFO to make sure subsequent transfers do not fail. There is no way to add timeout in the DMA driver since it does not know the SPI clock. The timeout is handled by the SPI driver and it should be possible to augment it to drain the FIFO when DMA fails so long as FIFO level is readable somewhere. If the DMA doesn't complete in certain amount of time, it should time out I'd say. Don't you think ? - the I/O errors are not checked in spi-nor at all which leads to silent data corruption. On a suggestion that this may improve reliability I changed the s3c64xx driver to use DMA for all transfers. This caused identification to fail in spin-nor because the ID was prefixed with extra 00 byte. Testing with spidev confirmed that everything is prefixed with extra 00. The determinism of this is in fact excellent news. Also when the DMA controller locked up no transfers were possible anymore. When DMA was not used for sending commands the controller would recover on next command. I made the option to always use DMA configurable and it turns out that the returned data is prefixed with 00 only when no transfer without DMA was ever made. Loading the spi-nor driver with the dma-only option off and then with dma-only option on results in correct operation. Only loading the dma-only driver first causes the 00 prefix. Can we conclude that the PIO codepath somehow programs a register somewhere which gets rid of this 0x00 prefix ? If so, this should then also be part of the DMA codepath, or even better, this should be set in probe() somewhere. Yes, it looks like it. Did you find what this could be please ? -- 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 1/6] mtd: spi-nor: change return value of read/write
On Tuesday, August 04, 2015 at 08:42:51 AM, Michal Suchanek wrote:
On 3 August 2015 at 23:46, Marek Vasut ma...@denx.de wrote:
On Monday, August 03, 2015 at 08:39:01 PM, Michal Suchanek wrote:
Change the return value of spi-nor device read and write methods to
allow returning amount of data transferred and errors as
read(2)/write(2) does.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
include/linux/mtd/spi-nor.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index e540952..7d782cb 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -185,9 +185,9 @@ struct spi_nor {
int (*write_reg)(struct spi_nor *nor, u8 opcode, u8 *buf, int len,
int write_enable);
- int (*read)(struct spi_nor *nor, loff_t from,
+ ssize_t (*read)(struct spi_nor *nor, loff_t from,
size_t len, size_t *retlen, u_char *read_buf);
- void (*write)(struct spi_nor *nor, loff_t to,
+ ssize_t (*write)(struct spi_nor *nor, loff_t to,
size_t len, size_t *retlen, const u_char
*write_buf);
int (*erase)(struct spi_nor *nor, loff_t offs);
You realize that if someone does bisect and has only this patch applied,
the compiler will complain loudly about mismatching data types, right ?
:)
Yes, the compiler prints a warning. However, only the return value
which is not used changes so it should not cause any real problem.
Are you certain that ssize_t is equal to int , always , everywhere ? :-)
Note that it is possible to do such conversion even without introducing
compiler warnings along the way -- just do the data type conversion first
and the other changes in subsequent patches. It will be clear that one
patch does one thing then.
The
data type in the fsl-quadspi and m25p80 drivers is matched in the
following two patches.
Thanks
Michal
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] mtd: spi-nor: Add support for sst25wf020a.
On Friday, August 14, 2015 at 07:35:39 PM, Alexis Ballier wrote:
It is a 256KiB flash with 4 KiB erase sectors
and 64KiB overlay blocks.
This is the one available on Hardkernel's Odroid U3 shield.
Signed-off-by: Alexis Ballier aball...@gentoo.org
---
drivers/mtd/spi-nor/spi-nor.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index d78831b..e521b35 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -636,6 +636,7 @@ static const struct spi_device_id spi_nor_ids[] = {
{ sst25wf512, INFO(0xbf2501, 0, 64 * 1024, 1, SECT_4K | SST_WRITE)
},
{ sst25wf010, INFO(0xbf2502, 0, 64 * 1024, 2, SECT_4K | SST_WRITE)
},
{ sst25wf020, INFO(0xbf2503, 0, 64 * 1024, 4, SECT_4K | SST_WRITE)
},
+ { sst25wf020a, INFO(0x621612, 0, 64 * 1024, 4, SECT_4K) },
Is the SST_WRITE not needed on this device ?
Otherwise, looks pretty obvious :)
{ sst25wf040, INFO(0xbf2504, 0, 64 * 1024, 8, SECT_4K | SST_WRITE)
},
{ sst25wf080, INFO(0xbf2505, 0, 64 * 1024, 16, SECT_4K | SST_WRITE)
},
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] mtd: spi-nor: Add support for sst25wf020a.
On Saturday, August 15, 2015 at 10:38:59 AM, Alexis Ballier wrote:
On Fri, 14 Aug 2015 22:45:14 +0200
Marek Vasut ma...@denx.de wrote:
On Friday, August 14, 2015 at 07:35:39 PM, Alexis Ballier wrote:
It is a 256KiB flash with 4 KiB erase sectors
and 64KiB overlay blocks.
This is the one available on Hardkernel's Odroid U3 shield.
Signed-off-by: Alexis Ballier aball...@gentoo.org
---
drivers/mtd/spi-nor/spi-nor.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/spi-nor/spi-nor.c
b/drivers/mtd/spi-nor/spi-nor.c index d78831b..e521b35 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -636,6 +636,7 @@ static const struct spi_device_id spi_nor_ids[]
= { { sst25wf512, INFO(0xbf2501, 0, 64 * 1024, 1, SECT_4K |
SST_WRITE)
},
{ sst25wf010, INFO(0xbf2502, 0, 64 * 1024, 2, SECT_4K
| SST_WRITE)
},
{ sst25wf020, INFO(0xbf2503, 0, 64 * 1024, 4, SECT_4K
| SST_WRITE)
},
+ { sst25wf020a, INFO(0x621612, 0, 64 * 1024, 4,
SECT_4K) },
Is the SST_WRITE not needed on this device ?
Tried that at first and it didn't work (writes were silently not
performed).
This is the worst possible behavior.
I'm no expert here, but I don't see anything about auto address
increment in the spec:
http://ww1.microchip.com/downloads/en/DeviceDoc/20005139E.pdf
and it uses the page program command (0x02).
They probably decided to start rolling out a standard SPI NOR, which is good :)
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 V6 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
On Wednesday, August 12, 2015 at 03:05:48 AM, Vikas MANOCHA wrote:
Hi Marek,
Hi,
-Original Message-
From: Marek Vasut [mailto:ma...@denx.de]
Sent: Wednesday, August 05, 2015 4:15 PM
To: Vikas MANOCHA
Cc: Graham Moore; linux-...@lists.infradead.org; David Woodhouse; Brian
Norris; linux-kernel@vger.kernel.org; Alan Tull; Dinh Nguyen; Yves
Vandervennet
Subject: Re: [PATCH V6 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI
Flash Controller.
On Wednesday, August 05, 2015 at 08:29:11 PM, vikasm wrote:
Hi Graham,
Hi vikasm,
On 07/28/2015 10:38 AM, Graham Moore wrote:
Signed-off-by: Graham Moore grmo...@opensource.altera.com
---
V2: use NULL instead of modalias in spi_nor_scan call
V3: Use existing property is-decoded-cs instead of creating
duplicate. V4: Support Micron quad mode by snooping command stream
for EVCR command and subsequently configuring Cadence controller for
quad
mode.
V5: Clean up sparse and smatch complaints. Remove snooping of
Micron quad mode. Add comment on XIP mode bit and dummy clock
cycles. Set up SRAM partition at 1:1 during init.
V6: Remove dts patch that was included by mistake. Incorporate
Vikas's comments regarding fifo width, SRAM partition setting, and
trigger address. Trigger address was added as an unsigned int, as
it is not an IO resource per se, and does not need to be mapped.
Also add Marek Vasut's workaround for picking up OF properties on
subnodes.
I am still not able to apply this patch to master. It seems to be
rebased on master for ..spi-nor/Kconfig spi-nor/makefile.
I'm able to apply this on next/master just fine.
I think my thunderbird client is messing up with the patch, I am trying to
fix it.
OK
Also I still see spaces are still not replaced by tabs in this version.
Which exact spots are you talking about ? I don't see that many indent
flubs in this patch to be honest. Sometimes, it is better to indent the
last piece with spaces (mandated by kernel coding style btw) to increase
the readability.
This is in particular the case with stuff like this:
pr_err(formating string that is almost 80 chars long %i!\n,
parameter_that_is_indented_with_7_spaces);
The sole purpose is to align stuff right under the opening parenthesis.
---
btw. would you please learn to use [...] and keep only the part you're
commenting on with a bit of context in your reply? It is really hard to
locate your comment if it's inbetween 500 lines of nothing.
[...]
Thanks Marek, I am not sure how to use it. I would appreciate any help.
That's rather simple in fact, but it really depends on your UI.
TL;DR, select the irrelevant piece of email and replace it with [...] .
[...]
+ writel(remaining, reg_base + CQSPI_REG_INDIRECTRDBYTES);
+
+ /* Clear all interrupts. */
+ writel(CQSPI_IRQ_STATUS_MASK, reg_base +
+ CQSPI_REG_IRQSTATUS);
+
+ cqspi-irq_mask = CQSPI_IRQ_MASK_RD;
+ writel(cqspi-irq_mask, reg_base + CQSPI_REG_IRQMASK);
+
+ reinit_completion(cqspi-transfer_complete);
+ writel(CQSPI_REG_INDIRECTRD_START_MASK,
+ reg_base + CQSPI_REG_INDIRECTRD);
+
+ while (remaining 0) {
+ ret =
wait_for_completion_timeout(cqspi-transfer_complete, +
msecs_to_jiffies +
(CQSPI_READ_TIMEOUT_MS)); +
+ bytes_to_read = CQSPI_GET_RD_SRAM_LEVEL(reg_base);
There should not be any need to read SRAM level, we should be able to
get rid of it like in u-boot.
Can you explain why ?
There is no need to check for sram fill level. If sram is empty, cpu will
go in the wait state till the time data is available from flash.
You mean reading from SRAM will stall the CPU ?
Also Relying on SRAM fill level only for deciding when the data should
befetched from the local SRAM is not most efficient approach, particularly
if we are working on high data rates.
After one SRAM word is collected, the information is forwarded into system
domain and then synchronized into register domain (apb). If we are using
slow APB and AHB clks, SRAM fill level might not be up-to-dated because of
latency cycles needed for synchronization. For example in case we are
getting SRAM fill level equal to 10 locations but in reality there were 2
another words completed and actual level is 12 but information may not be
synchronized yet because of the synchronization latency on APB domain.
We use the SRAM level only to determine how many bytes at minimum can we safely
read. If there are more data waiting, but the SRAM level is not yet updated, we
will keep looping here until the update happens, right ?
Though I agree this is not the most efficient method either. Can
Re: [PATCH V6 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
On Wednesday, August 05, 2015 at 08:29:11 PM, vikasm wrote:
Hi Graham,
Hi vikasm,
On 07/28/2015 10:38 AM, Graham Moore wrote:
Signed-off-by: Graham Moore grmo...@opensource.altera.com
---
V2: use NULL instead of modalias in spi_nor_scan call
V3: Use existing property is-decoded-cs instead of creating duplicate.
V4: Support Micron quad mode by snooping command stream for EVCR command
and subsequently configuring Cadence controller for quad mode.
V5: Clean up sparse and smatch complaints. Remove snooping of Micron
quad mode. Add comment on XIP mode bit and dummy clock cycles. Set
up SRAM partition at 1:1 during init.
V6: Remove dts patch that was included by mistake. Incorporate Vikas's
comments regarding fifo width, SRAM partition setting, and trigger
address. Trigger address was added as an unsigned int, as it is not
an IO resource per se, and does not need to be mapped. Also add
Marek Vasut's workaround for picking up OF properties on subnodes.
I am still not able to apply this patch to master. It seems to be rebased
on master for ..spi-nor/Kconfig spi-nor/makefile.
I'm able to apply this on next/master just fine.
Also I still see spaces are still not replaced by tabs in this version.
Which exact spots are you talking about ? I don't see that many indent flubs
in this patch to be honest. Sometimes, it is better to indent the last piece
with spaces (mandated by kernel coding style btw) to increase the readability.
This is in particular the case with stuff like this:
pr_err(formating string that is almost 80 chars long %i!\n,
parameter_that_is_indented_with_7_spaces);
The sole purpose is to align stuff right under the opening parenthesis.
---
btw. would you please learn to use [...] and keep only the part you're
commenting on with a bit of context in your reply? It is really hard
to locate your comment if it's inbetween 500 lines of nothing.
[...]
+static int cqspi_indirect_read_setup(struct spi_nor *nor,
+unsigned int from_addr)
+{
+ unsigned int reg;
+ unsigned int dummy_clk = 0;
+ struct cqspi_st *cqspi = nor-priv;
+ void __iomem *reg_base = cqspi-iobase;
+
+ writel(cqspi-trigger_address,
+ reg_base + CQSPI_REG_INDIRECTTRIGGER);
move indirect trigger configuration in init, no need to do it for every
read write.
Fixed.
+ writel(from_addr, reg_base + CQSPI_REG_INDIRECTRDSTARTADDR);
+
+ reg = nor-read_opcode CQSPI_REG_RD_INSTR_OPCODE_LSB;
+ reg |= cqspi_calc_rdreg(nor, nor-read_opcode);
+
+ /* Setup dummy clock cycles */
+#define CQSPI_SUPPORT_XIP_CHIPS
+#ifdef CQSPI_SUPPORT_XIP_CHIPS
+ /*
+* Set mode bits high to ensure chip doesn't enter XIP.
+* This results in an extra 8 dummy clocks so
+* we must account for them.
+*/
+ writel(0xFF, reg_base + CQSPI_REG_MODE_BIT);
+ reg |= (1 CQSPI_REG_RD_INSTR_MODE_EN_LSB);
+ if (nor-read_dummy = 8)
+ dummy_clk = nor-read_dummy - 8;
+ else
+ dummy_clk = 0;
+#else
+ dummy_clk = nor-read_dummy;
+#endif
+ reg |= (dummy_clk CQSPI_REG_RD_INSTR_DUMMY_MASK)
+CQSPI_REG_RD_INSTR_DUMMY_LSB;
+
+ writel(reg, reg_base + CQSPI_REG_RD_INSTR);
+
+ /* Set address width */
+ reg = readl(reg_base + CQSPI_REG_SIZE);
+ reg = ~CQSPI_REG_SIZE_ADDRESS_MASK;
+ reg |= (nor-addr_width - 1);
+ writel(reg, reg_base + CQSPI_REG_SIZE);
+ return 0;
+}
+
+static int cqspi_indirect_read_execute(struct spi_nor *nor,
+ u8 *rxbuf, unsigned n_rx)
+{
+ int ret = 0;
+ unsigned int reg = 0;
+ unsigned int bytes_to_read = 0;
+ unsigned int timeout;
+ unsigned int watermark;
+ struct cqspi_st *cqspi = nor-priv;
+ void __iomem *reg_base = cqspi-iobase;
+ void __iomem *ahb_base = cqspi-ahb_base;
+ int remaining = (int)n_rx;
+
+ watermark = cqspi-fifo_depth * cqspi-fifo_width / 2;
+ writel(watermark, reg_base + CQSPI_REG_INDIRECTRDWATERMARK);
I think watermark configuration can be moved to init also there should
not be any need to configuration it for every read.
Fixed.
+ writel(remaining, reg_base + CQSPI_REG_INDIRECTRDBYTES);
+
+ /* Clear all interrupts. */
+ writel(CQSPI_IRQ_STATUS_MASK, reg_base + CQSPI_REG_IRQSTATUS);
+
+ cqspi-irq_mask = CQSPI_IRQ_MASK_RD;
+ writel(cqspi-irq_mask, reg_base + CQSPI_REG_IRQMASK);
+
+ reinit_completion(cqspi-transfer_complete);
+ writel(CQSPI_REG_INDIRECTRD_START_MASK,
+ reg_base + CQSPI_REG_INDIRECTRD);
+
+ while (remaining 0) {
+ ret =
wait_for_completion_timeout(cqspi-transfer_complete, +
Re: [PATCH v2 5/5] mtd: atmel-quadspi: add driver for Atmel QSPI controller
On Monday, July 27, 2015 at 10:41:37 AM, Cyrille Pitchen wrote: Hi Marek, Le 22/07/2015 15:50, Marek Vasut a écrit : On Wednesday, July 22, 2015 at 03:17:10 PM, Cyrille Pitchen wrote: This driver add support to the new Atmel QSPI controller embedded into sama5d2x SoCs. It expects a NOR memory to be connected to the QSPI controller. Signed-off-by: Cyrille Pitchen cyrille.pitc...@atmel.com --- [...] +/* QSPI register offsets */ +#define QSPI_CR 0x /* Control Register */ +#define QSPI_MR 0x0004 /* Mode Register */ +#define QSPI_RD 0x0008 /* Receive Data Register */ +#define QSPI_TD 0x000c /* Transmit Data Register */ +#define QSPI_SR 0x0010 /* Status Register */ +#define QSPI_IER 0x0014 /* Interrupt Enable Register */ +#define QSPI_IDR 0x0018 /* Interrupt Disable Register */ +#define QSPI_IMR 0x001c /* Interrupt Mask Register */ +#define QSPI_SCR 0x0020 /* Serial Clock Register */ + +#define QSPI_IAR 0x0030 /* Instruction Address Register */ +#define QSPI_ICR 0x0034 /* Instruction Code Register */ +#define QSPI_IFR 0x0038 /* Instruction Frame Register */ + +#define QSPI_SMR 0x0040 /* Scrambling Mode Register */ +#define QSPI_SKR 0x0044 /* Scrambling Key Register */ + +#define QSPI_WPMR 0x00E4 /* Write Protection Mode Register */ +#define QSPI_WPSR 0x00E8 /* Write Protection Status Register */ + +#define QSPI_VERSION 0x00FC /* Version Register */ + +/* Bitfields in QSPI_CR (Control Register) */ +#define QSPI_CR_QSPIEN_OFFSET 0 +#define QSPI_CR_QSPIEN_SIZE 1 +#define QSPI_CR_QSPIDIS_OFFSET1 +#define QSPI_CR_QSPIDIS_SIZE 1 +#define QSPI_CR_SWRST_OFFSET 7 +#define QSPI_CR_SWRST_SIZE1 +#define QSPI_CR_LASTXFER_OFFSET 24 +#define QSPI_CR_LASTXFER_SIZE 1 + +/* Bitfields in QSPI_MR (Mode Register) */ +#define QSPI_MR_SSM_OFFSET0 +#define QSPI_MR_SSM_SIZE 1 +#define QSPI_MR_LLB_OFFSET1 +#define QSPI_MR_LLB_SIZE 1 +#define QSPI_MR_WDRBT_OFFSET 2 +#define QSPI_MR_WDRBT_SIZE1 +#define QPSI_MR_SMRM_OFFSET 3 +#define QSPI_MR_SMRM_SIZE 1 +#define QSPI_MR_CSMODE_OFFSET 4 +#define QSPI_MR_CSMODE_SIZE 2 +#define QSPI_MR_NBBITS_OFFSET 8 +#define QSPI_MR_NBBITS_SIZE 4 +#define QSPI_MR_NBBITS_8_BIT0 +#define QSPI_MR_NBBITS_9_BIT1 +#define QSPI_MR_NBBITS_10_BIT 2 +#define QSPI_MR_NBBITS_11_BIT 3 +#define QSPI_MR_NBBITS_12_BIT 4 +#define QSPI_MR_NBBITS_13_BIT 5 +#define QSPI_MR_NBBITS_14_BIT 6 +#define QSPI_MR_NBBITS_15_BIT 7 +#define QSPI_MR_NBBITS_16_BIT 8 You might want to turn this into something like: #define QSPI_NR_NBBITS(n) ((n) - 8) done in the next series +#define QSPI_MR_DLYBCT_OFFSET 16 +#define QSPI_MR_DLYBCT_SIZE 8 +#define QSPI_MR_DLYCS_OFFSET 24 +#define QSPI_MR_DLYCS_SIZE8 [...] +/* Bitfields in QSPI_IFR (Instruction Frame Register) */ +#define QSPI_IFR_WIDTH_OFFSET 0 +#define QSPI_IFR_WIDTH_SIZE 3 +#define QSPI_IFR_WIDTH_SINGLE_BIT_SPI 0 +#define QSPI_IFR_WIDTH_DUAL_OUTPUT 1 +#define QSPI_IFR_WIDTH_QUAD_OUTPUT 2 +#define QSPI_IFR_WIDTH_DUAL_IO 3 +#define QSPI_IFR_WIDTH_QUAD_IO 4 +#define QSPI_IFR_WIDTH_DUAL_CMD 5 +#define QSPI_IFR_WIDTH_QUAD_CMD 6 Please use #define[SPACE] instead of inconsistent #define[TAB] done in the next series. I also use BIT and GENMASK macros as much as possible to define the register bit fields. [...] +/* Bit manipulation macros */ +#define QSPI_BIT(name) \ + (1 QSPI_##name##_OFFSET) +#define QSPI_BF(name, value) \ + (((value) ((1 QSPI_##name##_SIZE) - 1)) QSPI_##name##_OFFSET) +#define QSPI_BFEXT(name, value) \ + (((value) QSPI_##name##_OFFSET) ((1 QSPI_##name##_SIZE) - 1)) +#define QSPI_BFINS(name, value, old) \ + (((old) ~(((1 QSPI_##name##_SIZE) - 1) QSPI_##name##_OFFSET)) \ + | QSPI_BF(name, value)) + +/* Register access macros */ +#define qspi_readl(port, reg) \ + readl_relaxed((port)-regs + QSPI_##reg) +#define qspi_writel(port, reg, value) \ + writel_relaxed((value), (port)-regs + QSPI_##reg) + +#define qspi_readw(port, reg) \ + readw_relaxed((port)-regs + QSPI_##reg) +#define qspi_writew(port, reg, value) \ + writew_relaxed((value), (port)-regs
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Monday, July 27, 2015 at 11:46:25 AM, Michal Suchanek wrote: On 24 July 2015 at 10:34, Marek Vasut ma...@denx.de wrote: On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: Hi! [...] It's probably slower to set up DMA for 2-byte commands but it might work nonetheless. It is, the overhead will be considerable. It might help the stability though. I'm really looking forward to the results! Hello, this does not quite work. My test with spidev: # ./spinor /dev/spidev1.0 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 Sending 90 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff ff ff ff c8 15 c8 15 c8 15 c8 15 c8 15 c8 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 I receive correct ID but spi-nor complains it does not know ID 00 c8 60. IIRC garbage should be sent only at the time command is transferred so only one byte of garbage should be received. Also the garbage tends to be the last state of the data output - all 0 or all 1. So it seems using DMA for all transfers including 1-byte commands results in (some?) received data getting an extra 00 prefix. I also managed to lock up the controller completely since there is some error passing the SPI speed somewhere :( [ 1352.977530] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977540] spidev spi1.0: spi mode 0 [ 1352.977576] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977582] spidev spi1.0: msb first [ 1352.977614] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977620] spidev spi1.0: 0 bits per word [ 1352.977652] spidev spi1.0: setup mode 0, 8 bits/w, 2690588672 Hz max -- 0 [ 1352.977726] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 src_clk sclk_spi1 mode bpw 8 [ 1352.977753] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: xfer bpw 8 speed -1604378624 [ 1352.977760] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 src_clk sclk_spi1 mode bpw 8 [ 1352.977781] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: using dma [ 1352.977797] dma-pl330 121b.pdma: setting up request on thread 1 Hmm, on a second thought it probably works as expected more or less. The nonsensical value was passed from application and there is no guard against that. Since I don't do PIO the controller remains locked up indefinitely. I have to admit, I don't quite understand the above. I also don't quite know what your spidev test does. Can you possibly just bind a regular SPI NOR driver and run mtdtests to see if it is stable ? Ok, so here is some summary. I have a NOR flash attached to a s3c64xx SPI controller with 64byte fifo and a pl330 dma controller. Normally DMA controller is used for transfers that do not fit into the FIFO. I tried adding the flash memory ID to the spi-nor driver table and adding a DT node for it. The flash is rated at 120MHz so I used that speed but the ID was bit-shifted and identification failed. There is DT property samsung,spi-feedback-delay for addressing this and at 120MHz it must be 2 or 3 on this board. 40MHz works with default value 0. The next step after identification worked was to try reading the flash content. For this the DMA controller is used because data is transferred in blocks larger than 64 bytes. When reading the whole 4MB flash the transfer failed silently. I got a 4MB file of all ones or all zeroes. It turns out that - the pl330 locks up when transfering large amount of data. Specifically, the maximum power of 2 sized transfer at 120MHz is 128 bytes and 64k at 40MHz. Transferring more than this results in the pl330 locking up and never signalling completion of the transfer. Hypothesis: I think this might just be that the controller didn't catch all the inbound clock ticks and thus counted less inbound data than it was set up to receive. The controller thus waits for more data. Data is left in FIFO which causes subsequent commands to fail since garbage is returned instead of command reply. Also subsequent read may cause I/O error or or return an empty page depending on the garbage around. So the driver for the DMA controller might need to be augmented to handle this case -- add a timeout and in case DMA times out, drain the FIFO to make sure subsequent transfers do not fail. - the I/O errors are not checked in spi-nor at all which leads to silent data corruption. On a suggestion that this may improve reliability I changed the s3c64xx driver to use DMA for all transfers. This caused identification to fail in spin-nor because the ID was prefixed with extra 00 byte. Testing with spidev confirmed that everything is prefixed with extra 00. The determinism of this is in fact
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: Hi! [...] It's probably slower to set up DMA for 2-byte commands but it might work nonetheless. It is, the overhead will be considerable. It might help the stability though. I'm really looking forward to the results! Hello, this does not quite work. My test with spidev: # ./spinor /dev/spidev1.0 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 Sending 90 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff ff ff ff c8 15 c8 15 c8 15 c8 15 c8 15 c8 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 I receive correct ID but spi-nor complains it does not know ID 00 c8 60. IIRC garbage should be sent only at the time command is transferred so only one byte of garbage should be received. Also the garbage tends to be the last state of the data output - all 0 or all 1. So it seems using DMA for all transfers including 1-byte commands results in (some?) received data getting an extra 00 prefix. I also managed to lock up the controller completely since there is some error passing the SPI speed somewhere :( [ 1352.977530] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977540] spidev spi1.0: spi mode 0 [ 1352.977576] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977582] spidev spi1.0: msb first [ 1352.977614] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977620] spidev spi1.0: 0 bits per word [ 1352.977652] spidev spi1.0: setup mode 0, 8 bits/w, 2690588672 Hz max -- 0 [ 1352.977726] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 src_clk sclk_spi1 mode bpw 8 [ 1352.977753] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: xfer bpw 8 speed -1604378624 [ 1352.977760] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 src_clk sclk_spi1 mode bpw 8 [ 1352.977781] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: using dma [ 1352.977797] dma-pl330 121b.pdma: setting up request on thread 1 Hmm, on a second thought it probably works as expected more or less. The nonsensical value was passed from application and there is no guard against that. Since I don't do PIO the controller remains locked up indefinitely. I have to admit, I don't quite understand the above. I also don't quite know what your spidev test does. Can you possibly just bind a regular SPI NOR driver and run mtdtests to see if it is stable ? -- 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 V4 1/2] mtd: spi-nor: Bindings for Cadence Quad SPI Flash Controller driver.
On Monday, March 23, 2015 at 02:36:21 PM, Graham Moore wrote: Signed-off-by: Graham Moore grmo...@opensource.altera.com --- V2: Add cdns prefix to driver-specific bindings. V3: Use existing property is-decoded-cs instead of creating a duplicate, ext-decoder. Timing parameters are in nanoseconds, not master reference clocks. Remove bus-num completely. Hi! do you plan to continue on this driver any soon? If not, I'd like to take over the mainlining if you're not opposed. I'd like to see this in mainline soon. btw you'll need some kind of a variation on the attached two patches in the next iteration. Best regards, Marek Vasut From e305b9a9cd80e56aeaa19b3c2a5bb26ba3adf8d7 Mon Sep 17 00:00:00 2001 From: Marek Vasut ma...@denx.de Date: Fri, 24 Jul 2015 10:10:23 +0200 Subject: [PATCH 1/2] mtd: spi-nor: Fix SRAM config on CQSPI Make sure the SRAM configuration register is loaded with correct data when initializing the controller. This might not always be the case, since for example U-Boot configures this register and even toggling the controller reset doesn't reset it to default value. Thus, explicitly set the register. Signed-off-by: Marek Vasut ma...@denx.de --- drivers/mtd/spi-nor/cadence-quadspi.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c index fa7b421..a18732c 100644 --- a/drivers/mtd/spi-nor/cadence-quadspi.c +++ b/drivers/mtd/spi-nor/cadence-quadspi.c @@ -1100,6 +1100,9 @@ static void cqspi_controller_init(struct cqspi_st *cqspi) /* Disable all interrupts. */ writel(0, cqspi-iobase + CQSPI_REG_IRQMASK); + /* Configure the SRAM split to 1:1 . */ + writel(0x40, cqspi-iobase + CQSPI_REG_SRAMPARTITION); + cqspi_controller_enable(cqspi); } -- 2.1.4 From fac766b916a64c263fdb044bdaba80a52f582ecb Mon Sep 17 00:00:00 2001 From: Marek Vasut ma...@denx.de Date: Fri, 24 Jul 2015 14:10:09 +0200 Subject: [PATCH 2/2] mtd: spi-nor: Pass OF node into subdevs The entire mechanism by which the CQSPI driver probes the SPI NORs is probably broken, in particular because it uses dev pointer of the CQSPI and passes it into spi_nor_scan(). Since the dev-of_node therefore points into the of_node of the CQSPI, the spi-nor driver cannot properly parse the OF properties of the subnode and thus can not configure itself accordingly. Add a nasty hack to work around this for now. Signed-off-by: Marek Vasut ma...@denx.de --- drivers/mtd/spi-nor/cadence-quadspi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c index a18732c..017460e 100644 --- a/drivers/mtd/spi-nor/cadence-quadspi.c +++ b/drivers/mtd/spi-nor/cadence-quadspi.c @@ -1203,6 +1203,7 @@ static int cqspi_probe(struct platform_device *pdev) nor-mtd = mtd; nor-dev = dev; + nor-dev-of_node = np; nor-priv = cqspi; mtd-priv = nor; -- 2.1.4
Re: [PATCH V4 1/2] mtd: spi-nor: Bindings for Cadence Quad SPI Flash Controller driver.
On Friday, July 24, 2015 at 06:12:01 PM, Graham Moore wrote: On 07/24/2015 07:45 AM, Marek Vasut wrote: On Monday, March 23, 2015 at 02:36:21 PM, Graham Moore wrote: Signed-off-by: Graham Moore grmo...@opensource.altera.com --- V2: Add cdns prefix to driver-specific bindings. V3: Use existing property is-decoded-cs instead of creating a duplicate, ext-decoder. Timing parameters are in nanoseconds, not master reference clocks. Remove bus-num completely. Hi! do you plan to continue on this driver any soon? If not, I'd like to take over the mainlining if you're not opposed. I'd like to see this in mainline soon. I don't have a problem with you taking over the mainlining. I've been stalled on the micron quad protocol issue that Cyrille Pitchen has solved. Do you mean the protocol change notification please? btw you'll need some kind of a variation on the attached two patches in the next iteration. I have a V5 with fixes for sparse and smatch issues. Do you want me to post it and you add those two patches? Alternatively I can incorporate them into my V5. Please post those patches, yup, that'd be nice :) Also, just squash those two micropatches into yours, no problem. Be careful about the 0002 patch, see the commit message. I do NOT think it is correct to pass the pdev-dev which represents the controller, not the subnode, into spi_nor_scan() in cqspi_probe() . I might be wrong here though, I am not sure. I think Brian or David would be much better people to comment on this part. 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 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: On 22 July 2015 at 06:49, Vinod Koul vinod.k...@intel.com wrote: On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. In practice on the board I have the maximum transfer length before it fails depends on SPI bus speed which is set up per slave. I did not try searching the space of possible settings thorougly and settled for a setting that gives reasonable speed and transfer length. This looks more like a signal integrity issue though. However, if this was not tied to the particular slave setting picked in the current DT a formula would be needed that translates arbitrary client settings to transfer size limit and there would be need to somehow get the client settings to the formula in the dmaengine driver. Thanks Michal 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 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 22, 2015 at 10:38:14 AM, Michal Suchanek wrote: On 22 July 2015 at 10:24, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 10:18:04 AM, Michal Suchanek wrote: On 22 July 2015 at 09:58, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: On 22 July 2015 at 09:33, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: On 22 July 2015 at 06:49, Vinod Koul vinod.k...@intel.com wrote: On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. In practice on the board I have the maximum transfer length before it fails depends on SPI bus speed which is set up per slave. I did not try searching the space of possible settings thorougly and settled for a setting that gives reasonable speed and transfer length. This looks more like a signal integrity issue though. It certainly does on the surface. However, when wrong data is delivered over the SPI bus (such as when I use wrong phase setting) the SPI controller happily delivers wrong data over PIO. The failure I am seeing is that the pl330 DMA program which repeatedly waits for data from the SPI controller never finishes the read loop and does not signal the interrupt. It seems it also leaves some data in a FIFO somewhere so next command on the flash returns garbage and fails. I observed something similar on MXS (mx28) SPI block. Do you use mixed PIO/DMA mode perhaps ? The SPI driver uses PIO for short transfers and DMA for transfers longer than the controller FIFO. This seems to be the standard way to do things.It works flawlessly so long as submitting overly long DMA programs is avoided. Can you try doing JUST DMA, no PIO ? I remember seeing some bus synchronisation issues when I did mixed PIO/DMA on the MXS and it was nasty to track down. Just give pure DMA a go to see if the thing stabilizes somehow. It's probably slower to set up DMA for 2-byte commands but it might work nonetheless. It is, the overhead will be considerable. It might help the stability though. I'm really looking forward to the results! I will give it a go. Do you have the option to connect a bus analyzer? I can probably offer you some tools, I'm in Prague ... The flash chip is accessible when removing the bottom cover. It is VSOP8 package slightly lower than SOP8 so attaching clips to it might be a bit problematic. That's the only accessible part. Everything other than SPI is inside the SoC. That might be doable, though you might want to try the above thing first. Since SPI has no verification whatsoever the chip might be completely dead and you can still read fine all zeroes or all ones when attempting a read from it. I observed this behaviour when I used a flash chip in a socket and it was not firmly seated. It was with a different SPI controller, though. You should run into issues as soon as the SPI NOR framework tries to read status register, no ? Yes, when the DMA transfer fails the next command fails due to garbage lying around. However, you can unload the SPI NOR driver, load spidev driver, and read enough garbage to empty the fifos. Then the flash identifies as normal again and you can access it. Yikes :( When the flash is not seated properly and acts autistic you get all 0xff or all 0 back whatever you send to it, obvously. The identification by the SPI NOR driver fails then. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: On 22 July 2015 at 09:33, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: On 22 July 2015 at 06:49, Vinod Koul vinod.k...@intel.com wrote: On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. In practice on the board I have the maximum transfer length before it fails depends on SPI bus speed which is set up per slave. I did not try searching the space of possible settings thorougly and settled for a setting that gives reasonable speed and transfer length. This looks more like a signal integrity issue though. It certainly does on the surface. However, when wrong data is delivered over the SPI bus (such as when I use wrong phase setting) the SPI controller happily delivers wrong data over PIO. The failure I am seeing is that the pl330 DMA program which repeatedly waits for data from the SPI controller never finishes the read loop and does not signal the interrupt. It seems it also leaves some data in a FIFO somewhere so next command on the flash returns garbage and fails. I observed something similar on MXS (mx28) SPI block. Do you use mixed PIO/DMA mode perhaps ? Do you have the option to connect a bus analyzer? I can probably offer you some tools, I'm in Prague ... 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 v2 5/5] mtd: atmel-quadspi: add driver for Atmel QSPI controller
On Wednesday, July 22, 2015 at 03:17:10 PM, Cyrille Pitchen wrote: This driver add support to the new Atmel QSPI controller embedded into sama5d2x SoCs. It expects a NOR memory to be connected to the QSPI controller. Signed-off-by: Cyrille Pitchen cyrille.pitc...@atmel.com --- [...] +/* QSPI register offsets */ +#define QSPI_CR 0x /* Control Register */ +#define QSPI_MR 0x0004 /* Mode Register */ +#define QSPI_RD 0x0008 /* Receive Data Register */ +#define QSPI_TD 0x000c /* Transmit Data Register */ +#define QSPI_SR 0x0010 /* Status Register */ +#define QSPI_IER 0x0014 /* Interrupt Enable Register */ +#define QSPI_IDR 0x0018 /* Interrupt Disable Register */ +#define QSPI_IMR 0x001c /* Interrupt Mask Register */ +#define QSPI_SCR 0x0020 /* Serial Clock Register */ + +#define QSPI_IAR 0x0030 /* Instruction Address Register */ +#define QSPI_ICR 0x0034 /* Instruction Code Register */ +#define QSPI_IFR 0x0038 /* Instruction Frame Register */ + +#define QSPI_SMR 0x0040 /* Scrambling Mode Register */ +#define QSPI_SKR 0x0044 /* Scrambling Key Register */ + +#define QSPI_WPMR0x00E4 /* Write Protection Mode Register */ +#define QSPI_WPSR0x00E8 /* Write Protection Status Register */ + +#define QSPI_VERSION 0x00FC /* Version Register */ + +/* Bitfields in QSPI_CR (Control Register) */ +#define QSPI_CR_QSPIEN_OFFSET0 +#define QSPI_CR_QSPIEN_SIZE 1 +#define QSPI_CR_QSPIDIS_OFFSET 1 +#define QSPI_CR_QSPIDIS_SIZE 1 +#define QSPI_CR_SWRST_OFFSET 7 +#define QSPI_CR_SWRST_SIZE 1 +#define QSPI_CR_LASTXFER_OFFSET 24 +#define QSPI_CR_LASTXFER_SIZE1 + +/* Bitfields in QSPI_MR (Mode Register) */ +#define QSPI_MR_SSM_OFFSET 0 +#define QSPI_MR_SSM_SIZE 1 +#define QSPI_MR_LLB_OFFSET 1 +#define QSPI_MR_LLB_SIZE 1 +#define QSPI_MR_WDRBT_OFFSET 2 +#define QSPI_MR_WDRBT_SIZE 1 +#define QPSI_MR_SMRM_OFFSET 3 +#define QSPI_MR_SMRM_SIZE1 +#define QSPI_MR_CSMODE_OFFSET4 +#define QSPI_MR_CSMODE_SIZE 2 +#define QSPI_MR_NBBITS_OFFSET8 +#define QSPI_MR_NBBITS_SIZE 4 +#define QSPI_MR_NBBITS_8_BIT0 +#define QSPI_MR_NBBITS_9_BIT1 +#define QSPI_MR_NBBITS_10_BIT 2 +#define QSPI_MR_NBBITS_11_BIT 3 +#define QSPI_MR_NBBITS_12_BIT 4 +#define QSPI_MR_NBBITS_13_BIT 5 +#define QSPI_MR_NBBITS_14_BIT 6 +#define QSPI_MR_NBBITS_15_BIT 7 +#define QSPI_MR_NBBITS_16_BIT 8 You might want to turn this into something like: #define QSPI_NR_NBBITS(n) ((n) - 8) +#define QSPI_MR_DLYBCT_OFFSET16 +#define QSPI_MR_DLYBCT_SIZE 8 +#define QSPI_MR_DLYCS_OFFSET 24 +#define QSPI_MR_DLYCS_SIZE 8 [...] +/* Bitfields in QSPI_IFR (Instruction Frame Register) */ +#define QSPI_IFR_WIDTH_OFFSET0 +#define QSPI_IFR_WIDTH_SIZE 3 +#define QSPI_IFR_WIDTH_SINGLE_BIT_SPI 0 +#define QSPI_IFR_WIDTH_DUAL_OUTPUT 1 +#define QSPI_IFR_WIDTH_QUAD_OUTPUT 2 +#define QSPI_IFR_WIDTH_DUAL_IO 3 +#define QSPI_IFR_WIDTH_QUAD_IO 4 +#define QSPI_IFR_WIDTH_DUAL_CMD 5 +#define QSPI_IFR_WIDTH_QUAD_CMD 6 Please use #define[SPACE] instead of inconsistent #define[TAB] [...] +/* Bit manipulation macros */ +#define QSPI_BIT(name) \ + (1 QSPI_##name##_OFFSET) +#define QSPI_BF(name, value) \ + (((value) ((1 QSPI_##name##_SIZE) - 1)) QSPI_##name##_OFFSET) +#define QSPI_BFEXT(name, value) \ + (((value) QSPI_##name##_OFFSET) ((1 QSPI_##name##_SIZE) - 1)) +#define QSPI_BFINS(name, value, old) \ + (((old) ~(((1 QSPI_##name##_SIZE) - 1) QSPI_##name##_OFFSET)) \ + | QSPI_BF(name, value)) + +/* Register access macros */ +#define qspi_readl(port, reg) \ + readl_relaxed((port)-regs + QSPI_##reg) +#define qspi_writel(port, reg, value) \ + writel_relaxed((value), (port)-regs + QSPI_##reg) + +#define qspi_readw(port, reg) \ + readw_relaxed((port)-regs + QSPI_##reg) +#define qspi_writew(port, reg, value) \ + writew_relaxed((value), (port)-regs + QSPI_##reg) + +#define qspi_readb(port, reg) \ + readb_relaxed((port)-regs + QSPI_##reg) +#define qspi_writeb(port, reg, value) \ + writeb_relaxed((value), (port)-regs + QSPI_##reg) I cannot say I'd be very fond of those preprocessor concatenations. Can't you do something about them? It's really hard to look for
Re: [PATCH v2 1/5] mtd: spi-nor: notify (Q)SPI controller about protocol change
On Wednesday, July 22, 2015 at 03:17:06 PM, Cyrille Pitchen wrote:
Once the Quad SPI mode has been enabled on a Micron flash memory, this
device expects ALL the following commands to use the SPI 4-4-4 protocol.
The (Q)SPI controller needs to be notified about the protocol change so it
can adapt and keep on dialoging with the Micron memory.
Signed-off-by: Cyrille Pitchen cyrille.pitc...@atmel.com
---
drivers/mtd/spi-nor/spi-nor.c | 17 +
include/linux/mtd/spi-nor.h | 13 +
2 files changed, 30 insertions(+)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index d78831b4422b..93627d4e6be8 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -163,6 +163,18 @@ static inline int write_disable(struct spi_nor *nor)
return nor-write_reg(nor, SPINOR_OP_WRDI, NULL, 0, 0);
}
+/*
+ * Notify the (Q)SPI controller about the new protocol to be used.
Hi!
Can you please just reword this a little, so that it is absolutelly clear
even to the less bright of us (like me) that this is a notification coming
from the upper layers (ie. the spi-nor framework) toward the hardware ?
+ */
+static inline int spi_nor_set_protocol(struct spi_nor *nor,
+enum spi_protocol proto)
+{
+ if (nor-set_protocol)
+ return nor-set_protocol(nor, proto);
+
+ return 0;
+}
+
static inline struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
{
return mtd-priv;
[...]
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 v2 2/5] Documentation: mtd: add a DT property to set the number of dummy cycles
On Wednesday, July 22, 2015 at 03:17:07 PM, Cyrille Pitchen wrote: Depending on the SPI clock frequency, the Fast Read op code and the Single/Dual Data Rate mode, the number of dummy cycles can be tuned to improve transfer speed. The actual number of dummy cycles is specific for each memory model and is provided by the manufacturer thanks to the memory datasheet. Signed-off-by: Cyrille Pitchen cyrille.pitc...@atmel.com --- Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt index 2bee68103b01..4387567d8024 100644 --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt @@ -19,6 +19,11 @@ Optional properties: all chips and support for it can not be detected at runtime. Refer to your chips' datasheet to check if this is supported by your chip. +- m25p,num-dummy-cycles : Set the number of dummy cycles for Fast Read commands. + Depending on the manufacturer additional dedicated + commands are sent to the flash memory so the + controller and the memory can agree on the number of + dummy cycles to use. Can't you just try negotiating this value at probe time, starting with some high value and see how low you can get with the negotiations ? This way, you'd be able to effectively auto-detect this value at probe-time. I might be wrong though :) 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 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 22, 2015 at 10:18:04 AM, Michal Suchanek wrote: On 22 July 2015 at 09:58, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: On 22 July 2015 at 09:33, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: On 22 July 2015 at 06:49, Vinod Koul vinod.k...@intel.com wrote: On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. In practice on the board I have the maximum transfer length before it fails depends on SPI bus speed which is set up per slave. I did not try searching the space of possible settings thorougly and settled for a setting that gives reasonable speed and transfer length. This looks more like a signal integrity issue though. It certainly does on the surface. However, when wrong data is delivered over the SPI bus (such as when I use wrong phase setting) the SPI controller happily delivers wrong data over PIO. The failure I am seeing is that the pl330 DMA program which repeatedly waits for data from the SPI controller never finishes the read loop and does not signal the interrupt. It seems it also leaves some data in a FIFO somewhere so next command on the flash returns garbage and fails. I observed something similar on MXS (mx28) SPI block. Do you use mixed PIO/DMA mode perhaps ? The SPI driver uses PIO for short transfers and DMA for transfers longer than the controller FIFO. This seems to be the standard way to do things.It works flawlessly so long as submitting overly long DMA programs is avoided. Can you try doing JUST DMA, no PIO ? I remember seeing some bus synchronisation issues when I did mixed PIO/DMA on the MXS and it was nasty to track down. Just give pure DMA a go to see if the thing stabilizes somehow. Do you have the option to connect a bus analyzer? I can probably offer you some tools, I'm in Prague ... The flash chip is accessible when removing the bottom cover. It is VSOP8 package slightly lower than SOP8 so attaching clips to it might be a bit problematic. That's the only accessible part. Everything other than SPI is inside the SoC. That might be doable, though you might want to try the above thing first. Since SPI has no verification whatsoever the chip might be completely dead and you can still read fine all zeroes or all ones when attempting a read from it. I observed this behaviour when I used a flash chip in a socket and it was not firmly seated. It was with a different SPI controller, though. You should run into issues as soon as the SPI NOR framework tries to read status register, no ? -- 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 V4 1/2] mtd: spi-nor: Bindings for Cadence Quad SPI Flash Controller driver.
On Friday, July 24, 2015 at 07:16:48 PM, Graham Moore wrote: On 07/24/2015 11:25 AM, Marek Vasut wrote: On Friday, July 24, 2015 at 06:12:01 PM, Graham Moore wrote: On 07/24/2015 07:45 AM, Marek Vasut wrote: On Monday, March 23, 2015 at 02:36:21 PM, Graham Moore wrote: Signed-off-by: Graham Moore grmo...@opensource.altera.com --- V2: Add cdns prefix to driver-specific bindings. V3: Use existing property is-decoded-cs instead of creating a duplicate, ext-decoder. Timing parameters are in nanoseconds, not master reference clocks. Remove bus-num completely. Hi! do you plan to continue on this driver any soon? If not, I'd like to take over the mainlining if you're not opposed. I'd like to see this in mainline soon. I don't have a problem with you taking over the mainlining. I've been stalled on the micron quad protocol issue that Cyrille Pitchen has solved. Do you mean the protocol change notification please? Yes, that's what I mean, the protocol change notification. OK btw you'll need some kind of a variation on the attached two patches in the next iteration. I have a V5 with fixes for sparse and smatch issues. Do you want me to post it and you add those two patches? Alternatively I can incorporate them into my V5. Please post those patches, yup, that'd be nice :) Also, just squash those two micropatches into yours, no problem. Be careful about the 0002 patch, see the commit message. I do NOT think it is correct to pass the pdev-dev which represents the controller, not the subnode, into spi_nor_scan() in cqspi_probe() . I might be wrong here though, I am not sure. I think Brian or David would be much better people to comment on this part. I'm doing it the way fsl_quadspi.c does it. It works on my Cyclone5 test system with and without that 0002 patch. So maybe I'll leave it out for now. It indeed works, but that doesn't mean it is correct. The most simple test to show you why I needed that 0002 patch would be for you to check if the m25p,fast-read prop, which you are actually adding in this patchset to the QSPI flash device, is set from within the spi-nor driver. It will never be set because you're not passing the DT of_node pointer into the spi-nor driver. V5 patches on the way... Thanks! 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 v2 2/5] Documentation: mtd: add a DT property to set the number of dummy cycles
On Wednesday, July 22, 2015 at 06:59:21 PM, Cyrille Pitchen wrote: Hi Marek, Le 22/07/2015 15:43, Marek Vasut a écrit : On Wednesday, July 22, 2015 at 03:17:07 PM, Cyrille Pitchen wrote: Depending on the SPI clock frequency, the Fast Read op code and the Single/Dual Data Rate mode, the number of dummy cycles can be tuned to improve transfer speed. The actual number of dummy cycles is specific for each memory model and is provided by the manufacturer thanks to the memory datasheet. Signed-off-by: Cyrille Pitchen cyrille.pitc...@atmel.com --- Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt index 2bee68103b01..4387567d8024 100644 --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt @@ -19,6 +19,11 @@ Optional properties: all chips and support for it can not be detected at runtime. Refer to your chips' datasheet to check if this is supported by your chip. +- m25p,num-dummy-cycles : Set the number of dummy cycles for Fast Read commands. + Depending on the manufacturer additional dedicated + commands are sent to the flash memory so the + controller and the memory can agree on the number of + dummy cycles to use. Can't you just try negotiating this value at probe time, starting with some high value and see how low you can get with the negotiations ? This way, you'd be able to effectively auto-detect this value at probe-time. I might be wrong though :) I don't know whether it would be reliable enough. It is the exact same idea as for the latency code used by Spansion QSPI memories. Micron memories allow to skip the step of converting the number of dummy cycles into a latency code, you directly program the right number of dummy cycles into a Micron specific register, the Volatile Configuration Register. However for both manufacturers the number of dummy cycles to use during Fast Read commands is given though tables found into the memory datasheet. The number of dummy cycles depends on the Fast Read command, the SPI bus clock frequency and the Single/Dual Data Rate mode. It should be confirmed by Quad SPI memory manufacturers but since the number of dummy cycles depends on the bus clock frequency, I guess the values provided by the datasheets are recommendations. I think a too low value should not be so easy to detect. For a given frequency one Fast Read command may succeed whereas the same command with the very same number of dummy cycles might fail on the next try. To be honest, I'm not sure about the memory behavior in limit conditions so maybe the command will always succeed or always fail. Also we can't be sure the read data are valid if we don't write them first. So we would have to save the original data to restore them at the end of the probing. Writing data at each probe would also reduce the memory lifetime. We should also be aware of the bad blocks, which is more a job for upper layers. I see, understood, OK. I really like how you explain those things btw :) It would be interesting to have some feedbacks from Micron, Spansion or other QSPI memory manufacturer :) Definitelly! -- 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 1/5] mtd: spi-nor: notify (Q)SPI controller about protocol change
On Wednesday, July 22, 2015 at 06:25:20 PM, Cyrille Pitchen wrote: Hi Marek, Le 22/07/2015 15:41, Marek Vasut a écrit : On Wednesday, July 22, 2015 at 03:17:06 PM, Cyrille Pitchen wrote: Once the Quad SPI mode has been enabled on a Micron flash memory, this device expects ALL the following commands to use the SPI 4-4-4 protocol. The (Q)SPI controller needs to be notified about the protocol change so it can adapt and keep on dialoging with the Micron memory. Signed-off-by: Cyrille Pitchen cyrille.pitc...@atmel.com --- drivers/mtd/spi-nor/spi-nor.c | 17 + include/linux/mtd/spi-nor.h | 13 + 2 files changed, 30 insertions(+) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index d78831b4422b..93627d4e6be8 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -163,6 +163,18 @@ static inline int write_disable(struct spi_nor *nor) return nor-write_reg(nor, SPINOR_OP_WRDI, NULL, 0, 0); } +/* + * Notify the (Q)SPI controller about the new protocol to be used. Hi! Can you please just reword this a little, so that it is absolutelly clear even to the less bright of us (like me) that this is a notification coming from the upper layers (ie. the spi-nor framework) toward the hardware ? Sure, no problem! what about the following? /* * Let the spi-nor framework notify lower layers, especially the driver of the * (Q)SPI controller, about the new protocol to be used. Indeed, once the * spi-nor framework has sent manufacturer specific commands to a memory to * enable its Quad SPI mode, it should immediately after tell the QSPI * controller to use the very same Quad SPI protocol as expected by the memory. */ Sure, excellent, thanks ! :-) -- 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 4/7] Documentation: mtd: add a DT property to set the latency code of Spansion memory
On Thursday, July 16, 2015 at 05:27:51 PM, Cyrille Pitchen wrote: Hi! Both the SPI controller and the NOR flash memory need to agree on the number of dummy cycles to use for Fast Read commands. For Spansion memories, this number of dummy cycles is not given directly but through a so called latency code. The latency code can be found into the memory datasheet and depends on the SPI clock frequency, the Fast Read op code and the Single/Dual Data Rate mode. Shouldn't you be able to derive the latency code from the above information, which you already know then ? 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 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Thursday, July 16, 2015 at 03:19:35 AM, Brian Norris wrote: On Wed, Jul 15, 2015 at 07:15:50PM +0200, Marek Vasut wrote: On Wednesday, July 15, 2015 at 05:59:46 PM, Brian Norris wrote: 1. Fix up the SPI driver so that it knows how to break large SPI transfers up into smaller segments that its constituent hardware (DMA controllers, fast clocks, etc.) can handle. BTW, Mark Brown already commented on this approach: http://lists.infradead.org/pipermail/linux-mtd/2015-May/059364.html I quote: | With modern drivers using transfer_one() where we have control of the | chip select we do also have the ability to add support to the core for | chopping large transfers up into smaller ones that the hardware can | handle transparently. That won't for everything though since it depends | on us being able to manually control the chip select which not all | hardware can do. | I think this might actually be easier -- just do a transfer where you don't toggle CS and just stops the clock at the last bit, then do another (multiple) transfers which don't toggle CS at all, then finally do a transfer which toggles a CS at the end. Sounds OK to me. And as Mark noted, this could probably be done in the core. I suppose Mark could suggest whether the most expedient path is to hack the buggy driver to do this, or whether reworking the SPI core to have the appropriate spi_master field(s) (and caveats) to support this. Yep, agreed. This should be pretty trivial to do and I think for example spi-mxs.c does this. I don't think spi-mxs.c really does this; it chooses between PIO and DMA based on length, but with either option, it runs through each SPI transfer in one go. You might be confused by the fact that this driver implements -transfer_one_message instead of -transfer_one, so it has to loop through all transfers in the message. It does chop up the DMA transfers between multiple runs of the DMA engine IIRC. But it does so within the driver, which apparently is not the way to go :) (IIUC, spi-mxs.c could easily be rewritten to use -transfer, and some of that not-too-complicated boilerplate could be killed off.) Most likely, yes. 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 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 15, 2015 at 05:59:46 PM, Brian Norris wrote: Hi Michal, Hi all, On Wed, Jul 15, 2015 at 01:52:27PM +0200, Marek Vasut wrote: The problem is, if you add a new DT binding, you'd have to support it forever, no matter how bad idea that binding turned out to be. Agreed, and a solid NAK to this patch. I could have sworn I gave such a response when this was originally being discussed a month ago. AFAICT, you have one of two general approaches available to you: 1. Fix up the SPI driver so that it knows how to break large SPI transfers up into smaller segments that its constituent hardware (DMA controllers, fast clocks, etc.) can handle. I think this might actually be easier -- just do a transfer where you don't toggle CS and just stops the clock at the last bit, then do another (multiple) transfers which don't toggle CS at all, then finally do a transfer which toggles a CS at the end. This should be pretty trivial to do and I think for example spi-mxs.c does this. 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 4/7] Documentation: mtd: add a DT property to set the latency code of Spansion memory
On Monday, July 20, 2015 at 11:23:39 AM, Cyrille Pitchen wrote: Hi Marek, Hi! Le 16/07/2015 19:44, Marek Vasut a écrit : On Thursday, July 16, 2015 at 05:27:51 PM, Cyrille Pitchen wrote: Hi! Both the SPI controller and the NOR flash memory need to agree on the number of dummy cycles to use for Fast Read commands. For Spansion memories, this number of dummy cycles is not given directly but through a so called latency code. The latency code can be found into the memory datasheet and depends on the SPI clock frequency, the Fast Read op code and the Single/Dual Data Rate mode. Shouldn't you be able to derive the latency code from the above information, which you already know then ? Yes I agree with you; this could have been done adding static tables inside the driver instead of creating a new DT property dedicated to Spansion memories. OK, I see now. The latency code can not be calculed from SPI clock frequency, the Fast Read op code and the Single/Dual Data Rate mode easily, you need to index into some table to obtain some ad-hoc value. Got it. Sorry for the noise! When I wrote this patch, I had a close look at the s25fl512s datasheet but only overviewed few datasheets for other Spansion QSPI flash memories. So I don't know whether a single latency code table could be shared among all Spansion memories or many tables should be added to support different memory models. That's why I've chosen to add a dedicated DT property to support Spansion memories as it avoids to add tables to guess the proper latency code to be used. I thought it would be more flexible. Maybe I will remove the support of Spansion QSPI memories from this series for now. Their support can still be implemented later. Anyway, thanks for your review :) Let's wait for more comments :) 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 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 15, 2015 at 11:45:07 AM, Michal Suchanek wrote: On 4 June 2015 at 19:15, Richard Cochran richardcoch...@gmail.com wrote: On Thu, Jun 04, 2015 at 10:31:45AM +0200, Michal Suchanek wrote: You might want to try to run the bus at 60MHz or 80MHz and then the values would probably again be different. The first two values are set in DT so the logical place for setting the third is also in DT. Otherwise you would need some in-kernel table of these settings. Or a formula. This formula probably needs to take into account - the unknown reason for the pl330 to fail transfer Shouldn't that be fixed at the PL330 level ? This looks like fixing a problem at the wrong place :) - the device transfer speed and transfer phase as set in DT - possibly device-specific latency and board-specific trace design and assembly tolerances If the design is broken, then cap the speed as for normal SPI device. Seriously, until I have at least a vague idea why the transfer fails I am not comfortable pulling some formula out of thin air and pretending I have a working patch. On the other hand, a parameter you can set in the DT and which comes with a suggested value which can be tuned depending on the system seems more viable. The problem is, if you add a new DT binding, you'd have to support it forever, no matter how bad idea that binding turned out to be. 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 1/5] mtd: ofpart: grab device tree node directly from master device node
On Thursday, October 29, 2015 at 08:24:48 AM, Boris Brezillon wrote: > Hi Robert, Hi! > On Thu, 29 Oct 2015 07:32:33 +0100 > > Robert Jarzmik <robert.jarz...@free.fr> wrote: > > Marek Vasut <ma...@denx.de> writes: > > >> Isn't there the case of a single NAND controller with 2 identical > > >> chips, each a 8 bit NAND chip, and the controller aggregating them to > > >> offer the OS a single 16-bit NAND chip ? > > Honestly, I don't know how this can possibly work, do you have a real > example of that use case. > > Here are a few reasons making it impossible: > > 1/ NAND are accessed using specific command sequences, and those > commands and addresses cycles are sent on through the data bus (AFAIR > only the lower 8bits of a 16bits bus are used for those > command/address cycles), so even if you connect the CLE/ALE/CS/RB pins > on both chips, the one connected on the MSB side of the data bus will > just receive garbage during the command/address sequences, and your > program/read operations won't work Unless you duplicate the command to both MSB and LSB. > 2/ NAND chips can have bad blocks, so even if you were able to address > 2 chips (which according to #1 is impossible), you might try to write > on a bad block on the chip connected on the MSB side of the data bus. This one is a valid problem. The other valid issue here is where the command might fail on one chip and pass on the other. > 3/ There probably are plenty of other reasons why this is not > possible ;-). It's possible, implementable, but a really bad idea. 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 1/5] mtd: ofpart: grab device tree node directly from master device node
On Wednesday, October 28, 2015 at 08:58:13 AM, Boris Brezillon wrote: > Hi Brian, Hi, [...] > > Are > > there ever cases we want more than one (master) MTD per nand_chip? Or > > vice versa? > > Nope, I'd say that you always have a 1:1 relationship between a master > MTD device and a NAND device. Do some sorts of chipselects come into play here ? Ie. you can have one master with multiple NAND chips connected to it. 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 1/5] mtd: ofpart: grab device tree node directly from master device node
On Wednesday, October 28, 2015 at 05:32:15 PM, Boris Brezillon wrote: > Hi Marek, Hi Boris, > On Wed, 28 Oct 2015 17:11:14 +0100 > > Marek Vasut <ma...@denx.de> wrote: > > On Wednesday, October 28, 2015 at 08:58:13 AM, Boris Brezillon wrote: > > > Hi Brian, > > > > Hi, > > > > [...] > > > > > > Are > > > > there ever cases we want more than one (master) MTD per nand_chip? Or > > > > vice versa? > > > > > > Nope, I'd say that you always have a 1:1 relationship between a master > > > MTD device and a NAND device. > > > > Do some sorts of chipselects come into play here ? Ie. you can have one > > master with multiple NAND chips connected to it. > > Most NAND controllers support interacting with several chips (or > dies in case your chip embeds several NAND dies), but I keep thinking > each physical chip should have its own instance of nand_chip + mtd_info. > If you want to have a single mtd device aggregating several chips you > can use mtdconcat. I agree. > This leaves the multi-dies chip case, and IHMO we should represent those > chips as a single entity, and I guess that's the purpose of the > ->numchips field in nand_chip (if your chip embeds 2 dies with 2 CS > lines, then ->numchips should be 2). I'd expect this to be represented as two physical chips, no ? > Anyway, I think the whole problem here is that most NAND drivers are > mixing the concepts of NAND controller (the controller driving one or > several NAND chips) and NAND chip (a chip connected to a NAND > controller). > The NAND controller should not be represented with a nand_chip > instance, but with a nand_hw_control instance, which is rarely done > except in a few drivers. OK, understood. > I sent an RFC a while ago [1] to clarify that, but didn't have time to > post a new version. > > Best Regards, > > Boris > > [1]http://thread.gmane.org/gmane.linux.drivers.mtd/57614/focus=58552 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 1/5] mtd: ofpart: grab device tree node directly from master device node
On Wednesday, October 28, 2015 at 09:55:24 PM, Robert Jarzmik wrote: > Brian Norris <computersforpe...@gmail.com> writes: > >> > Do some sorts of chipselects come into play here ? Ie. you can have > >> > one master with multiple NAND chips connected to it. > >> > >> Most NAND controllers support interacting with several chips (or > >> dies in case your chip embeds several NAND dies), but I keep thinking > >> each physical chip should have its own instance of nand_chip + mtd_info. > >> If you want to have a single mtd device aggregating several chips you > >> can use mtdconcat. > >> > >> This leaves the multi-dies chip case, and IHMO we should represent those > >> chips as a single entity, and I guess that's the purpose of the > >> ->numchips field in nand_chip (if your chip embeds 2 dies with 2 CS > >> lines, then ->numchips should be 2). > > > > Yes, I think that's some of the intention there. And so even in that > > case, a multi-die chip gets represented as a single struct nand_chip. > > Isn't there the case of a single NAND controller with 2 identical chips, > each a 8 bit NAND chip, and the controller aggregating them to offer the > OS a single 16-bit NAND chip ? Is that using 1 or 2 physical chipselect lines on the CPU (controller) ? > In this case, the controller (pxa3xx is a good example) will be programmed > to handle both chips at the same time, and calculate CRC on both chips, > etc ... I hope the assertion "physical chip should have its own instance > of nand_chip + mtd_info" does take into account this example. > > I don't know if there is actually any user of this for either pxa3xx or > another controller, nor if there is any value in this. > > Cheers. 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/
