| Message ID | 1360063473-26176-1-git-send-email-ezequiel.garcia@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Feb 05, 2013 at 08:24:33AM -0300, Ezequiel Garcia wrote: > This patch adds an SPI master device node for Armada XP-GP board. > This master node is an SPI flash controller 'n25q128a13'. > > Since there is no 'partitions' node declared, one full sized > partition named as the device will be created. > > Cc: Gregory Clement <gregory.clement@free-electrons.com> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > Cc: Lior Amsalem <alior@marvell.com> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > --- > This patch depends on: > > 1. Gregory's patch for Armada XP GP board: > arm: mvebu: support for the new Armada XP development board(DB-MV784MP-GP) > > 2. My previous patch for SPI on Armada 370/XP: > arm: mvebu: Add support for SPI controller in Armada 370/XP > > And don't forget to compile the SPI flash driver, CONFIG_MTD_M25P80=y > > arch/arm/boot/dts/armada-xp-gp.dts | 12 ++++++++++++ > 1 files changed, 12 insertions(+), 0 deletions(-) Very nice, thanks for listing the dependencies. That makes things a lot easier on this end. thx, Jason.
> And don't forget to compile the SPI flash driver, CONFIG_MTD_M25P80=y
Hi Ezequiel
Maybe it makes sense to provide a patch to mvebu_defconfig to add
CONFIG_MTD_M25P80=y ?
Andrew
Morning-ish Andrew, On Tue, Feb 05, 2013 at 01:38:27PM +0100, Andrew Lunn wrote: > > And don't forget to compile the SPI flash driver, CONFIG_MTD_M25P80=y > > Maybe it makes sense to provide a patch to mvebu_defconfig to add > CONFIG_MTD_M25P80=y ? I thought this as well, then reconsidered when he said there was nothing on it. So it's not really needed to boot. I don't have a strong opinion on it though, so =y, =m, or =n. thx, Jason.
On Tue, Feb 05, 2013 at 07:48:33AM -0500, Jason Cooper wrote: > Morning-ish Andrew, > > On Tue, Feb 05, 2013 at 01:38:27PM +0100, Andrew Lunn wrote: > > > And don't forget to compile the SPI flash driver, CONFIG_MTD_M25P80=y > > > > Maybe it makes sense to provide a patch to mvebu_defconfig to add > > CONFIG_MTD_M25P80=y ? > > I thought this as well, then reconsidered when he said there was nothing > on it. So it's not really needed to boot. I don't have a strong > opinion on it though, so =y, =m, or =n. > Well, the SPI flash may not be empty, if that's what you meant. For instnace, my Armada XP GP board has the bootloader on that SPI, and boots from there. After booting, the SPI flash is no longer needed. IMHO, I don't think we should include the option CONFIG_MTD_M25P80 as it is just a particular SPI master device present in a particular board. On the other hand, I don't have a strong opinion on this. I've always wondered the *true* meaning of defconfigs, so feel free to enligthen me :)
On Tue, Feb 05, 2013 at 11:27:21AM -0300, Ezequiel Garcia wrote: > On Tue, Feb 05, 2013 at 07:48:33AM -0500, Jason Cooper wrote: > > Morning-ish Andrew, > > > > On Tue, Feb 05, 2013 at 01:38:27PM +0100, Andrew Lunn wrote: > > > > And don't forget to compile the SPI flash driver, CONFIG_MTD_M25P80=y > > > > > > Maybe it makes sense to provide a patch to mvebu_defconfig to add > > > CONFIG_MTD_M25P80=y ? > > > > I thought this as well, then reconsidered when he said there was nothing > > on it. So it's not really needed to boot. I don't have a strong > > opinion on it though, so =y, =m, or =n. > > > > Well, the SPI flash may not be empty, if that's what you meant. > For instnace, my Armada XP GP board has the bootloader on that SPI, > and boots from there. After booting, the SPI flash is no longer needed. Is the bootloader environment on there? Is there room for the dtb once u-boot is upgraded to support fdt? > IMHO, I don't think we should include the option CONFIG_MTD_M25P80 > as it is just a particular SPI master device present in a particular board. > > On the other hand, I don't have a strong opinion on this. > I've always wondered the *true* meaning of defconfigs, > so feel free to enligthen me :) Hmm, a quick look through Documentation/* didn't produce anything authoritative. There's Documentation/sh/new-machine.txt which says " Now all that's left to do is providing a defconfig for your new board. This way, other people who end up with this board can simply use this config for reference instead of trying to guess what settings are supposed to be used on it. " One thing we've been doing a lot of with mvebu is using it for build-testing. For that use case, since the bootloader is there, I'd recommend setting it. This will reduce the likelyhood of nasty build breakages futher down the line as we'll catch them before pushing to arm-soc. thx, Jason.
Dear Jason Cooper, On Tue, 5 Feb 2013 10:31:35 -0500, Jason Cooper wrote: > One thing we've been doing a lot of with mvebu is using it for > build-testing. For that use case, since the bootloader is there, I'd > recommend setting it. This will reduce the likelyhood of nasty build > breakages futher down the line as we'll catch them before pushing to > arm-soc. To me, the defconfig should enable all the options needed for the drivers of on-board devices. So we should enable the driver for this SPI flash in the mvebu_defconfig. Best regards, Thomas
On 02/05/2013 05:17 PM, Thomas Petazzoni wrote: > Dear Jason Cooper, > > On Tue, 5 Feb 2013 10:31:35 -0500, Jason Cooper wrote: > >> One thing we've been doing a lot of with mvebu is using it for >> build-testing. For that use case, since the bootloader is there, I'd >> recommend setting it. This will reduce the likelyhood of nasty build >> breakages futher down the line as we'll catch them before pushing to >> arm-soc. > > To me, the defconfig should enable all the options needed for the > drivers of on-board devices. So we should enable the driver for this > SPI flash in the mvebu_defconfig. > So that means we should enable the drivers for all the devices present on all the boards? > Best regards, > > Thomas >
On Tue, Feb 05, 2013 at 05:17:01PM +0100, Thomas Petazzoni wrote: > Dear Jason Cooper, > > On Tue, 5 Feb 2013 10:31:35 -0500, Jason Cooper wrote: > > > One thing we've been doing a lot of with mvebu is using it for > > build-testing. For that use case, since the bootloader is there, I'd > > recommend setting it. This will reduce the likelyhood of nasty build > > breakages futher down the line as we'll catch them before pushing to > > arm-soc. > > To me, the defconfig should enable all the options needed for the > drivers of on-board devices. So we should enable the driver for this > SPI flash in the mvebu_defconfig. I agree. thx, Jason.
Hi Ezequiel, On 02/05/2013 12:24 PM, Ezequiel Garcia wrote: > This patch adds an SPI master device node for Armada XP-GP board. > This master node is an SPI flash controller 'n25q128a13'. > > Since there is no 'partitions' node declared, one full sized > partition named as the device will be created. > > Cc: Gregory Clement <gregory.clement@free-electrons.com> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > Cc: Lior Amsalem <alior@marvell.com> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > --- > This patch depends on: > > 1. Gregory's patch for Armada XP GP board: > arm: mvebu: support for the new Armada XP development board(DB-MV784MP-GP) > > 2. My previous patch for SPI on Armada 370/XP: > arm: mvebu: Add support for SPI controller in Armada 370/XP > > And don't forget to compile the SPI flash driver, CONFIG_MTD_M25P80=y > > arch/arm/boot/dts/armada-xp-gp.dts | 12 ++++++++++++ > 1 files changed, 12 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts > index 3eea531..1c8afe2 100644 > --- a/arch/arm/boot/dts/armada-xp-gp.dts > +++ b/arch/arm/boot/dts/armada-xp-gp.dts > @@ -97,5 +97,17 @@ > phy = <&phy3>; > phy-mode = "rgmii-id"; > }; > + > + spi0: spi@d0010600 { > + status = "okay"; > + > + spi-flash@0 { > + #address-cells = <1>; > + #size-cells = <1>; > + compatible = "n25q128a13"; > + reg = <0>; /* Chip select 0 */ > + spi-max-frequency = <108000000>; > + }; > + }; > }; > }; > I applied, compiled and try this patch it worked well, at least for reading, I didn't try to write on the flash where my boolader is stored. So you can add my: Tested-by: Gregory Clement <gregory.clement@free-electrons.com>
Dear Gregory CLEMENT, On Tue, 05 Feb 2013 17:20:02 +0100, Gregory CLEMENT wrote: > So that means we should enable the drivers for all the devices present > on all the boards? Yes. For example my mvsdio patch set enables some Wifi driver that corresponds to a particular SDIO Wifi device that is present in the Mirabox. But not on other boards. Of course, it doesn't make sense to have drivers for pluggable devices like PCIe devices that are not soldered, USB devices and things like that. But for all the rest, I would say it should be in the defconfig. Best regards, Thomas
On Tue, Feb 05, 2013 at 06:07:05PM +0100, Thomas Petazzoni wrote: > Of course, it doesn't make sense to have drivers for pluggable devices > like PCIe devices that are not soldered, USB devices and things like > that. But for all the rest, I would say it should be in the defconfig. hmmm, so for the new devboard we should set CONFIG_RAM=n? ;-) thx, Jason.
On Tue, Feb 05, 2013 at 06:07:05PM +0100, Thomas Petazzoni wrote: > Dear Gregory CLEMENT, > > On Tue, 05 Feb 2013 17:20:02 +0100, Gregory CLEMENT wrote: > > > So that means we should enable the drivers for all the devices present > > on all the boards? > > Yes. For example my mvsdio patch set enables some Wifi driver that > corresponds to a particular SDIO Wifi device that is present in the > Mirabox. But not on other boards. > Okey, I'll add everything needed to access and test the SPI flash, including MTD_CHAR. Thanks,
On 02/05/2013 05:28 PM, Gregory CLEMENT wrote: > Hi Ezequiel, > > On 02/05/2013 12:24 PM, Ezequiel Garcia wrote: >> This patch adds an SPI master device node for Armada XP-GP board. >> This master node is an SPI flash controller 'n25q128a13'. >> >> Since there is no 'partitions' node declared, one full sized >> partition named as the device will be created. >> >> Cc: Gregory Clement <gregory.clement@free-electrons.com> >> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> >> Cc: Lior Amsalem <alior@marvell.com> >> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> >> --- >> This patch depends on: >> >> 1. Gregory's patch for Armada XP GP board: >> arm: mvebu: support for the new Armada XP development board(DB-MV784MP-GP) >> >> 2. My previous patch for SPI on Armada 370/XP: >> arm: mvebu: Add support for SPI controller in Armada 370/XP >> >> And don't forget to compile the SPI flash driver, CONFIG_MTD_M25P80=y >> >> arch/arm/boot/dts/armada-xp-gp.dts | 12 ++++++++++++ >> 1 files changed, 12 insertions(+), 0 deletions(-) >> >> diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts >> index 3eea531..1c8afe2 100644 >> --- a/arch/arm/boot/dts/armada-xp-gp.dts >> +++ b/arch/arm/boot/dts/armada-xp-gp.dts >> @@ -97,5 +97,17 @@ >> phy = <&phy3>; >> phy-mode = "rgmii-id"; >> }; >> + >> + spi0: spi@d0010600 { >> + status = "okay"; >> + >> + spi-flash@0 { >> + #address-cells = <1>; >> + #size-cells = <1>; >> + compatible = "n25q128a13"; >> + reg = <0>; /* Chip select 0 */ >> + spi-max-frequency = <108000000>; I had a remark about it, according to the datasheet, 108MHz is the maximum frequency for the all the instructions but the READ instruction. For the READ the maximum frequency is 54MHz. So I wonder if we shouldn't use 54000000 here. >> + }; >> + }; >> }; >> }; >> > > I applied, compiled and try this patch it worked well, at least > for reading, I didn't try to write on the flash where my boolader > is stored. So you can add my: > > Tested-by: Gregory Clement <gregory.clement@free-electrons.com> > >
> From: Jason Cooper [mailto:jason@lakedaemon.net] > Sent: Tuesday, February 05, 2013 8:09 PM > > On Tue, Feb 05, 2013 at 06:07:05PM +0100, Thomas Petazzoni wrote: > > Of course, it doesn't make sense to have drivers for pluggable devices > > like PCIe devices that are not soldered, USB devices and things like > > that. But for all the rest, I would say it should be in the defconfig. > > hmmm, so for the new devboard we should set CONFIG_RAM=n? ;-) I think something like CONFIG_RAM=m suit better :) > > thx, > > Jason. Regards, Lior Amsalem
Hi Gregory, On Tue, Feb 05, 2013 at 09:17:02PM +0100, Gregory CLEMENT wrote: > On 02/05/2013 05:28 PM, Gregory CLEMENT wrote: > > Hi Ezequiel, > > > > On 02/05/2013 12:24 PM, Ezequiel Garcia wrote: > >> This patch adds an SPI master device node for Armada XP-GP board. > >> This master node is an SPI flash controller 'n25q128a13'. > >> > >> Since there is no 'partitions' node declared, one full sized > >> partition named as the device will be created. > >> > >> Cc: Gregory Clement <gregory.clement@free-electrons.com> > >> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > >> Cc: Lior Amsalem <alior@marvell.com> > >> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > >> --- > >> This patch depends on: > >> > >> 1. Gregory's patch for Armada XP GP board: > >> arm: mvebu: support for the new Armada XP development board(DB-MV784MP-GP) > >> > >> 2. My previous patch for SPI on Armada 370/XP: > >> arm: mvebu: Add support for SPI controller in Armada 370/XP > >> > >> And don't forget to compile the SPI flash driver, CONFIG_MTD_M25P80=y > >> > >> arch/arm/boot/dts/armada-xp-gp.dts | 12 ++++++++++++ > >> 1 files changed, 12 insertions(+), 0 deletions(-) > >> > >> diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts > >> index 3eea531..1c8afe2 100644 > >> --- a/arch/arm/boot/dts/armada-xp-gp.dts > >> +++ b/arch/arm/boot/dts/armada-xp-gp.dts > >> @@ -97,5 +97,17 @@ > >> phy = <&phy3>; > >> phy-mode = "rgmii-id"; > >> }; > >> + > >> + spi0: spi@d0010600 { > >> + status = "okay"; > >> + > >> + spi-flash@0 { > >> + #address-cells = <1>; > >> + #size-cells = <1>; > >> + compatible = "n25q128a13"; > >> + reg = <0>; /* Chip select 0 */ > >> + spi-max-frequency = <108000000>; > > I had a remark about it, according to the datasheet, 108MHz is the > maximum frequency for the all the instructions but the READ > instruction. For the READ the maximum frequency is 54MHz. So I wonder > if we shouldn't use 54000000 here. > Mmm... nice catch. The mtd driver for the spi flash (m25p80) will use FAST_READ opcode if CONFIG_M25PXX_USE_FAST_READ is selected, and this option is selected by default. However we cannot count on this option being selected, of course. On the other side, after some testing with spi-max-frequency = 50 MHz and also with spi-max-frequency = 108 MHz I'm seeing the flash often (not always) stalls when trying to read the full device through dd: / # dd if=/dev/mtd0ro of=/dev/null This happens regardless of CONFIG_M25PXX_USE_FAST_READ, and regardless of spi-max-frequency = 50 MHz or 54 MHz or 108 MHz. Note that setting 108 MHz is useless in our case, because there's an upper limit of 62.5 MHz set by the core clock of the SoC. Using a read block size of 1M, the read completes OK -- always: / # dd if=/dev/mtd0 of=/dev/null bs=1M 16+0 records in 16+0 records out 16777216 bytes (16.0MB) copied, 27.306048 seconds, 600.0KB/s Moreover, when the read completes, the achieved bandwidth is the same for 40, 50, 54, or 108 MHz. If we set spi-max-frequency to 27 MHz (108 MHz / 4), it looks like the stalling disappear and the read always completes OK, at almost half the speed: / # dd if=/dev/mtd0ro of=/dev/null 32768+0 records in 32768+0 records out 16777216 bytes (16.0MB) copied, 49.005122 seconds, 334.3KB/s So according to this analysis, we whould set 27 MHz as the max frequency, based on nothing but in these tests. I'm not sure the stalling is due to the clocking or due to some driver bug (I don't like the tricks pulled by the busy loop in orion_spi_wait_till_ready() but probably this is completely unrelated). Adding some SPI people in Cc, hoping they can shed a light on this. What do you think?
On 02/06/2013 11:54 AM, Ezequiel Garcia wrote: > Hi Gregory, > > On Tue, Feb 05, 2013 at 09:17:02PM +0100, Gregory CLEMENT wrote: >> On 02/05/2013 05:28 PM, Gregory CLEMENT wrote: >>> Hi Ezequiel, >>> >>> On 02/05/2013 12:24 PM, Ezequiel Garcia wrote: >>>> This patch adds an SPI master device node for Armada XP-GP board. >>>> This master node is an SPI flash controller 'n25q128a13'. >>>> >>>> Since there is no 'partitions' node declared, one full sized >>>> partition named as the device will be created. >>>> >>>> Cc: Gregory Clement <gregory.clement@free-electrons.com> >>>> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> >>>> Cc: Lior Amsalem <alior@marvell.com> >>>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> >>>> --- >>>> This patch depends on: >>>> >>>> 1. Gregory's patch for Armada XP GP board: >>>> arm: mvebu: support for the new Armada XP development board(DB-MV784MP-GP) >>>> >>>> 2. My previous patch for SPI on Armada 370/XP: >>>> arm: mvebu: Add support for SPI controller in Armada 370/XP >>>> >>>> And don't forget to compile the SPI flash driver, CONFIG_MTD_M25P80=y >>>> >>>> arch/arm/boot/dts/armada-xp-gp.dts | 12 ++++++++++++ >>>> 1 files changed, 12 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts >>>> index 3eea531..1c8afe2 100644 >>>> --- a/arch/arm/boot/dts/armada-xp-gp.dts >>>> +++ b/arch/arm/boot/dts/armada-xp-gp.dts >>>> @@ -97,5 +97,17 @@ >>>> phy = <&phy3>; >>>> phy-mode = "rgmii-id"; >>>> }; >>>> + >>>> + spi0: spi@d0010600 { >>>> + status = "okay"; >>>> + >>>> + spi-flash@0 { >>>> + #address-cells = <1>; >>>> + #size-cells = <1>; >>>> + compatible = "n25q128a13"; >>>> + reg = <0>; /* Chip select 0 */ >>>> + spi-max-frequency = <108000000>; >> >> I had a remark about it, according to the datasheet, 108MHz is the >> maximum frequency for the all the instructions but the READ >> instruction. For the READ the maximum frequency is 54MHz. So I wonder >> if we shouldn't use 54000000 here. >> > > Mmm... nice catch. > > The mtd driver for the spi flash (m25p80) will use FAST_READ opcode > if CONFIG_M25PXX_USE_FAST_READ is selected, and this option > is selected by default. OK I didn't notice that thre wre a FAST_READ which was supported by Linux. So I think we can keep 108000000 as spi-max-frequency. > However we cannot count on this option being selected, of course. > > On the other side, after some testing with spi-max-frequency = 50 MHz > and also with spi-max-frequency = 108 MHz I'm seeing the flash often > (not always) stalls when trying to read the full device through dd: > > / # dd if=/dev/mtd0ro of=/dev/null > > This happens regardless of CONFIG_M25PXX_USE_FAST_READ, and regardless > of spi-max-frequency = 50 MHz or 54 MHz or 108 MHz. > > Note that setting 108 MHz is useless in our case, because there's > an upper limit of 62.5 MHz set by the core clock of the SoC. > > Using a read block size of 1M, the read completes OK -- always: > > / # dd if=/dev/mtd0 of=/dev/null bs=1M > 16+0 records in > 16+0 records out > 16777216 bytes (16.0MB) copied, 27.306048 seconds, 600.0KB/s > > Moreover, when the read completes, the achieved bandwidth > is the same for 40, 50, 54, or 108 MHz. > > If we set spi-max-frequency to 27 MHz (108 MHz / 4), it looks like the stalling > disappear and the read always completes OK, at almost half the speed: > > / # dd if=/dev/mtd0ro of=/dev/null > 32768+0 records in > 32768+0 records out > 16777216 bytes (16.0MB) copied, 49.005122 seconds, 334.3KB/s > > So according to this analysis, we whould set 27 MHz as the max > frequency, based on nothing but in these tests. > > I'm not sure the stalling is due to the clocking or due to > some driver bug (I don't like the tricks pulled by the busy loop > in orion_spi_wait_till_ready() but probably this is completely unrelated). > > Adding some SPI people in Cc, hoping they can shed a light on this. > > What do you think? We have just seen this on IRC, so it seemed it was actually a problem in userspace not in kernel space.
(Adding mtd in Cc) On Wed, Feb 06, 2013 at 07:54:31AM -0300, Ezequiel Garcia wrote: > Hi Gregory, > > On Tue, Feb 05, 2013 at 09:17:02PM +0100, Gregory CLEMENT wrote: > > On 02/05/2013 05:28 PM, Gregory CLEMENT wrote: > > > Hi Ezequiel, > > > > > > On 02/05/2013 12:24 PM, Ezequiel Garcia wrote: > > >> This patch adds an SPI master device node for Armada XP-GP board. > > >> This master node is an SPI flash controller 'n25q128a13'. > > >> > > >> Since there is no 'partitions' node declared, one full sized > > >> partition named as the device will be created. > > >> > > >> Cc: Gregory Clement <gregory.clement@free-electrons.com> > > >> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > > >> Cc: Lior Amsalem <alior@marvell.com> > > >> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > > >> --- > > >> This patch depends on: > > >> > > >> 1. Gregory's patch for Armada XP GP board: > > >> arm: mvebu: support for the new Armada XP development board(DB-MV784MP-GP) > > >> > > >> 2. My previous patch for SPI on Armada 370/XP: > > >> arm: mvebu: Add support for SPI controller in Armada 370/XP > > >> > > >> And don't forget to compile the SPI flash driver, CONFIG_MTD_M25P80=y > > >> > > >> arch/arm/boot/dts/armada-xp-gp.dts | 12 ++++++++++++ > > >> 1 files changed, 12 insertions(+), 0 deletions(-) > > >> > > >> diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts > > >> index 3eea531..1c8afe2 100644 > > >> --- a/arch/arm/boot/dts/armada-xp-gp.dts > > >> +++ b/arch/arm/boot/dts/armada-xp-gp.dts > > >> @@ -97,5 +97,17 @@ > > >> phy = <&phy3>; > > >> phy-mode = "rgmii-id"; > > >> }; > > >> + > > >> + spi0: spi@d0010600 { > > >> + status = "okay"; > > >> + > > >> + spi-flash@0 { > > >> + #address-cells = <1>; > > >> + #size-cells = <1>; > > >> + compatible = "n25q128a13"; > > >> + reg = <0>; /* Chip select 0 */ > > >> + spi-max-frequency = <108000000>; > > > > I had a remark about it, according to the datasheet, 108MHz is the > > maximum frequency for the all the instructions but the READ > > instruction. For the READ the maximum frequency is 54MHz. So I wonder > > if we shouldn't use 54000000 here. > > > > Mmm... nice catch. > > The mtd driver for the spi flash (m25p80) will use FAST_READ opcode > if CONFIG_M25PXX_USE_FAST_READ is selected, and this option > is selected by default. > However we cannot count on this option being selected, of course. > > On the other side, after some testing with spi-max-frequency = 50 MHz > and also with spi-max-frequency = 108 MHz I'm seeing the flash often > (not always) stalls when trying to read the full device through dd: > > / # dd if=/dev/mtd0ro of=/dev/null > FWIW, using CONFIG_DETECT_HUNG_TASK we can see the hung happens inside spi_sync(). It seems the completion handler is never called, though I still don't understand under what circumstances that can happen. dd D c02d79c8 0 964 961 0x00000000 [<c02d79c8>] (__schedule+0x1c8/0x504) from [<c02d61d0>] (schedule_timeout+0x158/0x200) [<c02d61d0>] (schedule_timeout+0x158/0x200) from [<c02d7504>] (wait_for_common+0xe0/0x194) [<c02d7504>] (wait_for_common+0xe0/0x194) from [<c0215b44>] (spi_sync+0x74/0x90) [<c0215b44>] (spi_sync+0x74/0x90) from [<c0214c80>] (m25p80_read+0x100/0x17c) [<c0214c80>] (m25p80_read+0x100/0x17c) from [<c020edac>] (mtd_read+0x8c/0xc0) [<c020edac>] (mtd_read+0x8c/0xc0) from [<c021404c>] (mtdchar_read+0xe0/0x224) [<c021404c>] (mtdchar_read+0xe0/0x224) from [<c00c757c>] (vfs_read+0xa4/0x148) [<c00c757c>] (vfs_read+0xa4/0x148) from [<c00c7664>] (sys_read+0x44/0x78) [<c00c7664>] (sys_read+0x44/0x78) from [<c000ede0>] (ret_fast_syscall+0x0/0x30)
diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts index 3eea531..1c8afe2 100644 --- a/arch/arm/boot/dts/armada-xp-gp.dts +++ b/arch/arm/boot/dts/armada-xp-gp.dts @@ -97,5 +97,17 @@ phy = <&phy3>; phy-mode = "rgmii-id"; }; + + spi0: spi@d0010600 { + status = "okay"; + + spi-flash@0 { + #address-cells = <1>; + #size-cells = <1>; + compatible = "n25q128a13"; + reg = <0>; /* Chip select 0 */ + spi-max-frequency = <108000000>; + }; + }; }; };
This patch adds an SPI master device node for Armada XP-GP board. This master node is an SPI flash controller 'n25q128a13'. Since there is no 'partitions' node declared, one full sized partition named as the device will be created. Cc: Gregory Clement <gregory.clement@free-electrons.com> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Cc: Lior Amsalem <alior@marvell.com> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> --- This patch depends on: 1. Gregory's patch for Armada XP GP board: arm: mvebu: support for the new Armada XP development board(DB-MV784MP-GP) 2. My previous patch for SPI on Armada 370/XP: arm: mvebu: Add support for SPI controller in Armada 370/XP And don't forget to compile the SPI flash driver, CONFIG_MTD_M25P80=y arch/arm/boot/dts/armada-xp-gp.dts | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)