[U-Boot] [PATCH] ARM: i.MX51: Config option to disable PLL1

2011-05-26 Thread David Jander
i.MX51 PLL1 seems to have stability problems. It is advised to not use it,
although it is unclear whether all boards and/or chip revisions have this
problem. Using PLL2 for the core and DDR2 seems to fix the problem.
No official errata yet.

Signed-off-by: David Jander 
---
 arch/arm/cpu/armv7/mx5/lowlevel_init.S |   16 
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/arch/arm/cpu/armv7/mx5/lowlevel_init.S 
b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
index 96ebfe2..e1d6c35 100644
--- a/arch/arm/cpu/armv7/mx5/lowlevel_init.S
+++ b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
@@ -153,7 +153,11 @@
mov r1, #0x4
str r1, [r0, #CLKCTL_CCSR]
 
+#if defined(CONFIG_MX51_AVOID_PLL1)
+   setup_pll PLL1_BASE_ADDR, 216
+#else
setup_pll PLL1_BASE_ADDR, 800
+#endif
 
 #if defined(CONFIG_MX51)
setup_pll PLL3_BASE_ADDR, 665
@@ -165,7 +169,11 @@
str r1, [r0, #CLKCTL_CBCMR]
ldr r1, =0x13239145
str r1, [r0, #CLKCTL_CBCDR]
+#if defined(CONFIG_MX51_AVOID_PLL1)
+   setup_pll PLL2_BASE_ADDR, 800
+#else
setup_pll PLL2_BASE_ADDR, 665
+#endif
 
/* Switch peripheral to PLL2 */
ldr r0, =CCM_BASE_ADDR
@@ -197,7 +205,11 @@
 #endif
str r1, [r0, #CLKCTL_CACRR]
/* Switch ARM back to PLL 1 */
+#if defined(CONFIG_MX51_AVOID_PLL1)
+   mov r1, #0x0104 /* Set ARM/DDR to PLL2 */
+#else
mov r1, #0
+#endif
str r1, [r0, #CLKCTL_CCSR]
 
 #if defined(CONFIG_MX51)
@@ -228,7 +240,11 @@
/* Use PLL 2 for UART's, get 66.5MHz from it */
ldr r1, =0xA5A2A020
str r1, [r0, #CLKCTL_CSCMR1]
+#if defined(CONFIG_MX51_AVOID_PLL1)
+   ldr r1, =0x0104041a /* Adjust dividers for 800MHz on PLL2 */
+#else
ldr r1, =0x00C30321
+#endif
str r1, [r0, #CLKCTL_CSCDR1]
 #elif defined(CONFIG_MX53)
ldr r1, [r0, #CLKCTL_CSCDR1]
-- 
1.7.4.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] ARM: i.MX51: Config option to disable PLL1

2011-05-26 Thread David Jander
On Thu, 26 May 2011 19:00:14 +0200
David Jander  wrote:

> i.MX51 PLL1 seems to have stability problems. It is advised to not use it,
> although it is unclear whether all boards and/or chip revisions have this
> problem. Using PLL2 for the core and DDR2 seems to fix the problem.
> No official errata yet.

Forgot to mention this in the commit message:

All boards that need this fix (all of them?) should change their board config
header file to include this:

#define CONFIG_MX51_AVOID_PLL1

...

#ifdef CONFIG_MX51_AVOID_PLL1
#define CONFIG_SYS_CLKTL_CBCDR  0x59EC7580
#else
#define CONFIG_SYS_CLKTL_CBCDR  0x59E35100
#endif

This is the case for mx51evk.h, and the exact value of CONFIG_SYS_CLKTL_CBCDR
may vary depending on crystal frequency, type of RAM, NFC clocks, etc...

I would like to have some feedback before resubmitting the patch with the
amended commit message. I would also like to know whether I should include
another patch fixing all affected board-config headers? I guess this should be
decided by the respective maintainers, since this requires fixing the linux
kernel clock driver also...

Beste regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] ARM: i.MX51: Config option to disable PLL1

2011-05-27 Thread David Jander
On Fri, 27 May 2011 12:13:32 +0200
Stefano Babic  wrote:

> On 05/26/2011 07:00 PM, David Jander wrote:
> > i.MX51 PLL1 seems to have stability problems. It is advised to not use it,
> > although it is unclear whether all boards and/or chip revisions have this
> > problem. Using PLL2 for the core and DDR2 seems to fix the problem.
> > No official errata yet.
> > 
> 
> Hi David,
> 
> do you get some info from Freescale's FAE ?

Yes.

> Is this issue strictly
> related to the processor or can be board related ?

AFAIK, this issue could also be board-related. In other words, if one designs
a board that powers vpll* from a higher voltage than nominal mentioned
in the datasheet, chances could be lower.

> I hope someone from Freescale can help us to understand this issue.

I think I already know quite a lot about it (feel free to ask me off-list).

> > Signed-off-by: David Jander 
> > ---
> >  arch/arm/cpu/armv7/mx5/lowlevel_init.S |   16 
> >  1 files changed, 16 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/cpu/armv7/mx5/lowlevel_init.S
> > b/arch/arm/cpu/armv7/mx5/lowlevel_init.S index 96ebfe2..e1d6c35 100644
> > --- a/arch/arm/cpu/armv7/mx5/lowlevel_init.S
> > +++ b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
> > @@ -153,7 +153,11 @@
> > mov r1, #0x4
> > str r1, [r0, #CLKCTL_CCSR]
> >  
> > +#if defined(CONFIG_MX51_AVOID_PLL1)
> 
> If you add a new CONFIG_, you must document it in the README file.

Ah, ok, thanks for pointing out.

> Rather I cannot get a better feedback, I do not know this issue on the
> i.MX51. As you reported, it seems still unclear what happens.

Symptoms are sudden complete freeze of the ARM core, and either stable or
unstable image corruption on the LCD.

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] Linux 2.6.33.9 (RT) on mpc5121

2011-05-27 Thread David Jander
On Fri, 27 May 2011 11:34:53 +
Einar Már Björgvinsson  wrote:

> hi
> 
> I'm not sure if this is the proper list for posting this question but it
> seems to me that there are some folks here that have vast experience of the
> mpc5121 chip, so here it goes.

It is indeed not the correct list. You might want to try
linuxppc-...@lists.ozlabs.org for better results.

> I'm currently trying to port an older kernel version (2.6.33-rc6 non
> realtime) to a newer one (2.6.33.9 with RT-PREEMPT 2.6.33.9). The initial
> support for USB and CAN was done by Denx.

Have you checked latest mainline linux for that processor? I don't know what
drivers you need, but there has been a lot of progress, and chances are it
works better than what you have now :-)

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] USB: Ethernet: asix.c: Added support for AX88178 based devices

2011-05-31 Thread David Jander
Completed command definitions copied from linux driver source.
Implemented support for AX88178 by copying and rewriting bits and pieces
from the linux asix driver.

Signed-off-by: David Jander 
---
 drivers/usb/eth/asix.c |  236 +---
 include/usb_ether.h|1 +
 2 files changed, 224 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c
index 9b012e4..cecbc76 100644
--- a/drivers/usb/eth/asix.c
+++ b/drivers/usb/eth/asix.c
@@ -24,6 +24,8 @@
 #include 
 #include "usb_ether.h"
 
+#define ASIX_MODEL_AX88772 0
+#define ASIX_MODEL_AX88178 1
 
 /* ASIX AX8817X based USB 2.0 Ethernet Devices */
 
@@ -31,14 +33,29 @@
 #define AX_CMD_READ_MII_REG0x07
 #define AX_CMD_WRITE_MII_REG   0x08
 #define AX_CMD_SET_HW_MII  0x0a
+#define AX_CMD_READ_EEPROM 0x0b
+#define AX_CMD_WRITE_EEPROM0x0c
+#define AX_CMD_WRITE_ENABLE0x0d
+#define AX_CMD_WRITE_DISABLE   0x0e
 #define AX_CMD_READ_RX_CTL 0x0f
 #define AX_CMD_WRITE_RX_CTL0x10
+#define AX_CMD_READ_IPG012 0x11
 #define AX_CMD_WRITE_IPG0  0x12
+#define AX_CMD_WRITE_IPG1  0x13
 #define AX_CMD_READ_NODE_ID0x13
+#define AX_CMD_WRITE_NODE_ID   0x14
+#define AX_CMD_WRITE_IPG2  0x14
+#define AX_CMD_WRITE_MULTI_FILTER  0x16
+#define AX88172_CMD_READ_NODE_ID   0x17
 #define AX_CMD_READ_PHY_ID 0x19
+#define AX_CMD_READ_MEDIUM_STATUS  0x1a
 #define AX_CMD_WRITE_MEDIUM_MODE   0x1b
+#define AX_CMD_READ_MONITOR_MODE   0x1c
+#define AX_CMD_WRITE_MONITOR_MODE  0x1d
+#define AX_CMD_READ_GPIOS  0x1e
 #define AX_CMD_WRITE_GPIOS 0x1f
 #define AX_CMD_SW_RESET0x20
+#define AX_CMD_SW_PHY_STATUS   0x21
 #define AX_CMD_SW_PHY_SELECT   0x22
 
 #define AX_SWRESET_CLEAR   0x00
@@ -83,6 +100,10 @@
(AX_RX_CTL_SO | AX_RX_CTL_AB)
 
 /* GPIO 2 toggles */
+#define AX_GPIO_GPO0EN 0x01/* GPIO0 Output enable */
+#define AX_GPIO_GPO_0  0x02/* GPIO0 Output value */
+#define AX_GPIO_GPO1EN 0x04/* GPIO1 Output enable */
+#define AX_GPIO_GPO_1  0x08/* GPIO1 Output value */
 #define AX_GPIO_GPO2EN 0x10/* GPIO2 Output enable */
 #define AX_GPIO_GPO_2  0x20/* GPIO2 Output value */
 #define AX_GPIO_RSE0x80/* Reload serial EEPROM */
@@ -310,7 +331,7 @@ static int mii_nway_restart(struct ueth_data *dev)
 /*
  * Asix callbacks
  */
-static int asix_init(struct eth_device *eth, bd_t *bd)
+static int asix_init_ax88772(struct eth_device *eth, bd_t *bd)
 {
int embd_phy;
unsigned char buf[ETH_ALEN];
@@ -419,6 +440,190 @@ out_err:
return -1;
 }
 
+/**
+ * mii_check_media - check the MII interface for a duplex change
+ * @mii: the MII interface
+ * @ok_to_print: OK to print link up/down messages
+ * @init_media: OK to save duplex mode in @mii
+ *
+ * Returns 1 if the duplex mode changed, 0 if not.
+ * If the media type is forced, always returns 0.
+ */
+unsigned int mii_check_media (struct eth_device *eth,
+ unsigned int ok_to_print,
+ unsigned int init_media)
+{
+   int advertise, lpa, media, duplex;
+   int lpa2 = 0;
+   struct ueth_data *dev = (struct ueth_data *)eth->priv;
+
+   /* get MII advertise and LPA values */
+   advertise = asix_mdio_read(dev, dev->phy_id, MII_ADVERTISE);
+   lpa = asix_mdio_read(dev, dev->phy_id, MII_LPA);
+   lpa2 = asix_mdio_read(dev, dev->phy_id, MII_STAT1000);
+
+   /* figure out media and duplex from advertise and LPA values */
+   media = mii_nway_result(lpa & advertise);
+   duplex = (media & ADVERTISE_FULL) ? 1 : 0;
+   if (lpa2 & LPA_1000FULL)
+   duplex = 1;
+
+   printf("link up, %uMbps, %s-duplex, lpa 0x%04X\n",
+   lpa2 & (LPA_1000FULL | LPA_1000HALF) ? 1000 :
+   media & (ADVERTISE_100FULL | ADVERTISE_100HALF) ?
+   100 : 10,
+   duplex ? "full" : "half",
+   lpa);
+
+   return 0; /* duplex did not change */
+}
+
+static int ax88178_link_reset(struct eth_device *eth)
+{
+   u16 mode;
+   struct ueth_data*dev = (struct ueth_data *)eth->priv;
+
+   debug("ax88178_link_reset()\n");
+
+   mii_check_media(eth, 1, 1);
+   mode = AX88178_MEDIUM_DEFAULT;
+
+   /* All supported ax88178 dongles have GMII */
+   mode |= AX_MEDIUM_GM;
+   mode |= AX_MEDIUM_ENCK;
+   mode |= AX_MEDIUM_FD;
+
+   asix_write_medium_mode(dev, mode);
+
+   return 0;
+}
+
+static int asix_init_ax88178(struct eth_device *eth, bd_t *bd)
+{
+   unsigned char 

[U-Boot] New chapter in i.MX51 datasheet an issue?

2012-05-04 Thread David Jander

Hi all,

I discovered a bug in u-boot, that got evident after Freescale updated the
i.MX51 datasheets to revision 5 in March this year. I don't know if it is a
serious problem or not, but if I believe the wording of the datasheet many of
the boards that use a i.MX51 processor and running u-boot as of latest git,
can potentially suffer "permanent damage", whatever that means.

I am referring to the new paragraphs at the end of chapter 4.3.4 of the
datasheet, and the wrong interpretation of the meaning of the HVE bit in the
iomuxc.h header file of u-boot here:

arch/arm/include/asm/arch-mx5/iomux.h:

...
  69 PAD_CTL_DRV_VOT_LOW = 0x0 << 13, /* Low voltage mode */
  70 PAD_CTL_DRV_VOT_HIGH = 0x1 << 13,/* High voltage mode */
...

According to the reference manual, the correct meaning of this bit is negated:

"Bit 13:
High / Low Output Voltage Range. This bit selects the output voltage mode for 
SD2_CMD.
0 High output voltage mode
1 Low output voltage mode"

Added to the new paragraph in the datasheet:

"The UHVIO type of I/O cells have to be configured properly according to their
supply voltage level, in order to prevent permanent damage to them and in
order to not degrade their timing performance."

Seems like we may have a problem here!

I would like to know if anyone is aware of this? Does anyone know of a board
that is actually destroyed this way?

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [resent] New chapter in i.MX51 datasheet an issue?

2012-05-04 Thread David Jander

Hi all,

I discovered a bug in u-boot, that got evident after Freescale updated the
i.MX51 datasheets to revision 5 in March this year. I don't know if it is a
serious problem or not, but if I believe the wording of the datasheet many of
the boards that use a i.MX51 processor and running u-boot as of latest git,
can potentially suffer "permanent damage", whatever that means.

I am referring to the new paragraphs at the end of chapter 4.3.4 of the
datasheet, and the wrong interpretation of the meaning of the HVE bit in the
iomuxc.h header file of u-boot here:

arch/arm/include/asm/arch-mx5/iomux.h:

...
  69 PAD_CTL_DRV_VOT_LOW = 0x0 << 13, /* Low voltage mode */
  70 PAD_CTL_DRV_VOT_HIGH = 0x1 << 13,/* High voltage mode */
...

According to the reference manual, the correct meaning of this bit is negated:

"Bit 13:
High / Low Output Voltage Range. This bit selects the output voltage mode for
SD2_CMD. 0 High output voltage mode
1 Low output voltage mode"

Added to the new paragraph in the datasheet:

"The UHVIO type of I/O cells have to be configured properly according to their
supply voltage level, in order to prevent permanent damage to them and in
order to not degrade their timing performance."

Seems like we may have a problem here!

I would like to know if anyone is aware of this? Does anyone know of a board
that is actually destroyed this way?

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [resent] New chapter in i.MX51 datasheet an issue?

2012-05-07 Thread David Jander

Dear Stefano,

On Sun, 06 May 2012 18:15:18 +0200
Stefano Babic  wrote:

> On 04/05/2012 12:08, David Jander wrote:
> > 
> > Hi all,
> > 
> 
> Hi David,
> 
> > I discovered a bug in u-boot, that got evident after Freescale updated the
> > i.MX51 datasheets to revision 5 in March this year. I don't know if it is a
> > serious problem or not, but if I believe the wording of the datasheet many 
> > of
> > the boards that use a i.MX51 processor and running u-boot as of latest git,
> > can potentially suffer "permanent damage", whatever that means.
> > 
> > I am referring to the new paragraphs at the end of chapter 4.3.4 of the
> > datasheet, and the wrong interpretation of the meaning of the HVE bit in the
> > iomuxc.h header file of u-boot here:
> > 
> > arch/arm/include/asm/arch-mx5/iomux.h:
> > 
> > ...
> >   69 PAD_CTL_DRV_VOT_LOW = 0x0 << 13, /* Low voltage mode */
> >   70 PAD_CTL_DRV_VOT_HIGH = 0x1 << 13,/* High voltage mode */
> > ...
> > 
> 
> Agree. The header defines the bit wrongly.
> 
> It seems to me that the Reference Manual is correct. It is from
> 24/2/2010 and it was not updated.
> 
> After a reset, value is set to 1, and this means low-voltage for the
> most pins.

Yes, but is none of those boards using 3.15 or 3.3V? If they are, those bits
must be cleared!

> > According to the reference manual, the correct meaning of this bit is 
> > negated:
> > 
> > "Bit 13:
> > High / Low Output Voltage Range. This bit selects the output voltage mode 
> > for
> > SD2_CMD. 0 High output voltage mode
> > 1 Low output voltage mode"
> > 
> > Added to the new paragraph in the datasheet:
> > 
> > "The UHVIO type of I/O cells have to be configured properly according to 
> > their
> > supply voltage level, in order to prevent permanent damage to them and in
> > order to not degrade their timing performance."
> > 
> > Seems like we may have a problem here!
> > 
> > I would like to know if anyone is aware of this? Does anyone know of a board
> > that is actually destroyed this way?
> 
> At the moment, we have no problems and I can explain why. The only
> boards setting these pins (for SD card) are mx51evk and vision2. Both
> are setting PAD_CTL_DRV_VOT_HIGH, and because the define is wrong, they
> are really setting the pin to low output voltage mode.

AFAIK, most SD-cards need 3.1V or more to work, and the EVK boots from an SD
card. That means, the rails NVCC_PER15 and/or NVCC_PER17 are probably powered
from a 3.15 or 3.3V supply, so the HVE bits for those pins must be cleared in
able to avoid damage (3.3V is what they call "Ultra High Voltage").

> For other boards and other pins, voltage is not explicitely set : this
> means they work in low voltage mode after a reset.

Everyone reading the documentation as it was available before 03-2012 would
most probably think this is ok, even for 3.1...3.6V powered pins, but according
to the new documentation, it isn't. So probably many boards need to be
re-designed or at least u-boot board-code needs to be fixed for them. This is
a different issue though, and needs to be addressed by the different BSP
maintainers or board manufacturers.

> To fix arch/arm/include/asm/arch-mx5/iomux.h and synchronize it with the
> documentation, we need also to change mx51evk / vision2, setting the
> pins to PAD_CTL_DRV_VOT_LOW, and they will work as now.

... and probably die. The setting is most probably wrong. All we need to do,
is change the define in the header files, and not touch the mx51evk / vision2
BSP files. But the respective maintainers need to be warned IMHO.

> We can also drop completely PAD_CTL_DRV_VOT_HIGH from these two boards
> and use the reset value.

PAD_CTL_DRV_VOT_HIGH should be a NOP in a bit mask, since its value should be
0.
PAD_CTL_DRV_VOT_LOW should be 1. Board code most probably shouldn't be
changed, because it most probably assumes that the "meaning" of the define is
right, and not its value.

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [resent] New chapter in i.MX51 datasheet an issue?

2012-05-08 Thread David Jander
On Tue, 08 May 2012 10:46:10 +0200
Stefano Babic  wrote:

> On 07/05/2012 09:11, David Jander wrote:
> > 
> > Dear Stefano,
> > 
> 
> Hi David,
> 
> > Yes, but is none of those boards using 3.15 or 3.3V? If they are, those bits
> > must be cleared!
> 
> This is a good question - also because SD was tested and it is working
> on these cards. I am asking to myself how it can work if voltage is wrong.

That is the whole point: You probably won't notice anything with the wrong
settings besides slightly lower drive-strength on the pin. Things should
just work with either setting. The problem is that now Freescale is telling us
that using the incorrect settings can cause "permanent damage" to the chip!!!
No idea what sort of damage nor whether it occurs frequently.

> >> At the moment, we have no problems and I can explain why. The only
> >> boards setting these pins (for SD card) are mx51evk and vision2. Both
> >> are setting PAD_CTL_DRV_VOT_HIGH, and because the define is wrong, they
> >> are really setting the pin to low output voltage mode.
> > 
> > AFAIK, most SD-cards need 3.1V or more to work, and the EVK boots from an SD
> > card. That means, the rails NVCC_PER15 and/or NVCC_PER17 are probably 
> > powered
> > from a 3.15 or 3.3V supply, so the HVE bits for those pins must be cleared 
> > in
> > able to avoid damage (3.3V is what they call "Ultra High Voltage").
> 
> Really I have expected that SD does not work if the voltage is lower as
> specified, not that theree is a damage - I can understand this in the
> opposite case (setting high voltage when low voltage is required).

The setting doesn't affect the actual output voltage or something like that.
This bit only changes some electrical characteristics of the PAD-IO circuitry,
in order to perform better for lower or for higher rail power... depending on
its value. Apparently Freescale now discovered that its not only affecting
minor timing details that probably are irrelevant for most uses, but also that
the chip can malfunction ("permanent damage can occur").

> >> For other boards and other pins, voltage is not explicitely set : this
> >> means they work in low voltage mode after a reset.
> > 
> > Everyone reading the documentation as it was available before 03-2012 would
> > most probably think this is ok, even for 3.1...3.6V powered pins, but 
> > according
> > to the new documentation, it isn't. So probably many boards need to be
> > re-designed or at least u-boot board-code needs to be fixed for them. This 
> > is
> > a different issue though, and needs to be addressed by the different BSP
> > maintainers or board manufacturers.
> 
> I think only u-boot code should be fixed.

Potentially also board code, in the cases where the settings are not matching
the actual hardware not because it doesn't work, but rather to avoid
possible "permanent damage". That's something board designers/BSP maintainers
should care about.

> >> To fix arch/arm/include/asm/arch-mx5/iomux.h and synchronize it with the
> >> documentation, we need also to change mx51evk / vision2, setting the
> >> pins to PAD_CTL_DRV_VOT_LOW, and they will work as now.
> > 
> > ... and probably die.
> 
> well, they will work as now - with low voltage instead of high voltage.
> Anyway, I agree that this should be wrong, because when the code was
> written was thought that the voltage should be high.

See above.

> > The setting is most probably wrong. All we need to do,
> > is change the define in the header files, and not touch the mx51evk / 
> > vision2
> > BSP files. But the respective maintainers need to be warned IMHO.
> 
> Go ahead in this direction. Then we can test on these boards (mx51evk /
> vision2 / efikamx), the only ones having these issue).

I agree we should only change the headers, but... if there are more i.MX51
boards, they may all potentially need to fix their board code and that is
up to them. Unfortunately this warning/wake-up call should come from Freescale
themselves, but I have not heared anything from them.

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [resent] New chapter in i.MX51 datasheet an issue?

2012-05-09 Thread David Jander
On Tue, 8 May 2012 14:51:52 +0200
Marek Vasut  wrote:

> Dear David Jander,
> 
> > On Tue, 08 May 2012 10:46:10 +0200
> > 
> > Stefano Babic  wrote:
> > > On 07/05/2012 09:11, David Jander wrote:
> > > > Dear Stefano,
> > > 
> > > Hi David,
> > > 
> > > > Yes, but is none of those boards using 3.15 or 3.3V? If they are, those
> > > > bits must be cleared!
> > > 
> > > This is a good question - also because SD was tested and it is working
> > > on these cards. I am asking to myself how it can work if voltage is
> > > wrong.
> > 
> > That is the whole point: You probably won't notice anything with the wrong
> > settings besides slightly lower drive-strength on the pin. Things
> > should just work with either setting. The problem is that now Freescale is
> > telling us that using the incorrect settings can cause "permanent damage"
> > to the chip!!! No idea what sort of damage nor whether it occurs
> > frequently.
> 
> I think it might have something to do with ESD. It's probably unlikely to 
> happen 
> anyway.

Why do you think that?
ESD protection is not something that could possibly have something to do with
a running board and wrong PAD configuration bit settings. Or do you mean that
IO-pad ESD protection is the thing that gets destroyed by wrong settings?

AFAICS, this bit probably enables and disables a different set of output
transistors on the pad.

Whether damage is likely to happen or not, is not clear to me yet. I am waiting
on a reply from Freescale about this issue right now. I will post any important
findings here as soon as I know more.

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [resent] New chapter in i.MX51 datasheet an issue?

2012-05-09 Thread David Jander
On Tue, 08 May 2012 15:14:47 +0200
Stefano Babic  wrote:

> On 08/05/2012 14:06, David Jander wrote:
> 
> >> Go ahead in this direction. Then we can test on these boards (mx51evk /
> >> vision2 / efikamx), the only ones having these issue).
> > 
> > I agree we should only change the headers, but... if there are more i.MX51
> > boards, they may all potentially need to fix their board code and that 
> > is
> > up to them. Unfortunately this warning/wake-up call should come from 
> > Freescale
> > themselves, but I have not heared anything from them.
> 
> What we can do is to change all MX51 boards in mainline (the three
> boards we have already mentioned), explaining in the commit-id of the
> patch why we change. We do not care about boards that were not pushed to
> mainline.

Seems reasonable. Are you also implying that you are willing to propose a
patch? (Swapping the two values seems fairly trivial)

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] MX5: PAD_CTL_DRV_VOT_LOW and PAD_CTL_DRV_VOT_HIGH exchanged

2012-05-09 Thread David Jander
On Wed,  9 May 2012 12:13:04 +0200
Stefano Babic  wrote:

> After an update to the MX51 reference manual (Rev. 5), the
> values of the PAD_CTL_DRV_VOT_LOW and PAD_CTL_DRV_VOT_HIGH
> are now clearly wrong:
> 
> "Bit 13:
> High / Low Output Voltage Range. This bit selects the output voltage mode for
> SD2_CMD. 0 High output voltage mode
> 1 Low output voltage mode"
> 
> The values are currently negated in code - fixed.
> 
> Reported-by: David Jander 
> Signed-off-by: Stefano Babic 
> CC: Marek Vasut 
> CC: David Jander 
> ---
>  arch/arm/include/asm/arch-mx5/iomux.h |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-mx5/iomux.h 
> b/arch/arm/include/asm/arch-mx5/iomux.h
> index 760371b..e3765a3 100644
> --- a/arch/arm/include/asm/arch-mx5/iomux.h
> +++ b/arch/arm/include/asm/arch-mx5/iomux.h
> @@ -66,8 +66,8 @@ typedef enum iomux_pad_config {
>   PAD_CTL_HYS_ENABLE = 0x1 << 8,  /* Hysteresis enabled */
>   PAD_CTL_DDR_INPUT_CMOS = 0x0 << 9,/* DDR input CMOS */
>   PAD_CTL_DDR_INPUT_DDR = 0x1 << 9,/* DDR input DDR */
> - PAD_CTL_DRV_VOT_LOW = 0x0 << 13, /* Low voltage mode */
> - PAD_CTL_DRV_VOT_HIGH = 0x1 << 13,/* High voltage mode */
> + PAD_CTL_DRV_VOT_LOW = 0x1 << 13, /* Low voltage mode */
> + PAD_CTL_DRV_VOT_HIGH = 0x0 << 13,/* High voltage mode */
>  } iomux_pad_config_t;
>  
>  /* various IOMUX input functions */

Acked!

thanks.

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] ARM: MX5: Remove broken leftover TO-2 errata workaround

2011-07-14 Thread David Jander
This check is broken. r3 does not contain the silicon revision.

Signed-off-by: David Jander 
---
 arch/arm/cpu/armv7/mx5/lowlevel_init.S |5 -
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/arch/arm/cpu/armv7/mx5/lowlevel_init.S 
b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
index ee4150d..f17d200 100644
--- a/arch/arm/cpu/armv7/mx5/lowlevel_init.S
+++ b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
@@ -39,11 +39,6 @@
orr r0, r0, #(1 << 23)  /* disable write allocate combine */
orr r0, r0, #(1 << 22)  /* disable write allocate */
 
-   cmp r3, #0x10/* r3 contains the silicon rev */
-
-   /* disable write combine for TO 2 and lower revs */
-   orrls r0, r0, #(1 << 25)
-
mcr 15, 1, r0, c9, c0, 2
 .endm /* init_l2cc */
 
-- 
1.7.4.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] ARM: MX51: PLL errata workaround

2011-07-14 Thread David Jander
This is a port of the official PLL errata workaround from Freescale to
mainline u-boot.
The PLL's in the i.MX51 processor can go out of lock due to a metastable
condition in an analog flip-flop when used at high frequencies.
This workaround implements an undocumented feature in the PLL (dither
mode), which causes the effect of this failure to be much lower (in terms
of frequency deviation), avoiding system failure, or at least decreasing
the likelihood of system failure.

Signed-off-by: David Jander 
---
 arch/arm/cpu/armv7/mx5/lowlevel_init.S   |   38 ++
 arch/arm/include/asm/arch-mx5/imx-regs.h |5 
 doc/README.imx5  |   18 ++
 3 files changed, 61 insertions(+), 0 deletions(-)
 create mode 100644 doc/README.imx5

diff --git a/arch/arm/cpu/armv7/mx5/lowlevel_init.S 
b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
index 96ebfe2..ee4150d 100644
--- a/arch/arm/cpu/armv7/mx5/lowlevel_init.S
+++ b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
@@ -117,6 +117,35 @@
beq 1b
 .endm
 
+.macro setup_pll_errata pll, freq
+   ldr r2, =\pll
+   mov r1, #0x0
+   str r1, [r2, #PLL_DP_CONFIG] /* Disable auto-restart AREN bit */
+   ldr r1, =0x1236
+   str r1, [r2, #PLL_DP_CTL]/* Restart PLL with PLM=1 */
+1: ldr r1, [r2, #PLL_DP_CTL]/* Wait for lock */
+   ands r1, r1, #0x1
+   beq 1b
+
+   ldr r5, \freq
+   str r5, [r2, #PLL_DP_MFN]/* Modify MFN value */
+   str r5, [r2, #PLL_DP_HFS_MFN]
+
+   mov r1, #0x1
+   str r1, [r2, #PLL_DP_CONFIG] /* Reload MFN value */
+
+2: ldr r1, [r2, #PLL_DP_CONFIG]
+   tst r1, #1
+   bne 2b
+
+   ldr r1, =100 /* Wait at least 4 us */
+3: subs r1, r1, #1
+   bge 3b
+
+   mov r1, #0x2
+   str r1, [r2, #PLL_DP_CONFIG] /* Enable auto-restart AREN bit */
+.endm
+
 .macro init_clock
ldr r0, =CCM_BASE_ADDR
 
@@ -153,7 +182,12 @@
mov r1, #0x4
str r1, [r0, #CLKCTL_CCSR]
 
+#if defined(CONFIG_MX51_PLL_ERRATA)
+   setup_pll PLL1_BASE_ADDR, 864
+   setup_pll_errata PLL1_BASE_ADDR, W_DP_MFN_800_DIT
+#else
setup_pll PLL1_BASE_ADDR, 800
+#endif
 
 #if defined(CONFIG_MX51)
setup_pll PLL3_BASE_ADDR, 665
@@ -283,6 +317,10 @@ lowlevel_init:
mov pc,lr
 
 /* Board level setting value */
+W_DP_OP_864:  .word DP_OP_864
+W_DP_MFD_864: .word DP_MFD_864
+W_DP_MFN_864: .word DP_MFN_864
+W_DP_MFN_800_DIT: .word DP_MFN_800_DIT
 W_DP_OP_800:  .word DP_OP_800
 W_DP_MFD_800: .word DP_MFD_800
 W_DP_MFN_800: .word DP_MFN_800
diff --git a/arch/arm/include/asm/arch-mx5/imx-regs.h 
b/arch/arm/include/asm/arch-mx5/imx-regs.h
index d7f83c5..0b1caf0 100644
--- a/arch/arm/include/asm/arch-mx5/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx5/imx-regs.h
@@ -235,6 +235,11 @@
 
 /* Assuming 24MHz input clock with doubler ON */
 /*MFI PDF */
+#define DP_OP_864  ((8 << 4) + ((1 - 1)  << 0))
+#define DP_MFD_864 (180 - 1) /* PL Dither mode */
+#define DP_MFN_864 180
+#define DP_MFN_800_DIT 60 /* PL Dither mode */
+
 #define DP_OP_850  ((8 << 4) + ((1 - 1)  << 0))
 #define DP_MFD_850 (48 - 1)
 #define DP_MFN_850 41
diff --git a/doc/README.imx5 b/doc/README.imx5
new file mode 100644
index 000..e9b2c88
--- /dev/null
+++ b/doc/README.imx5
@@ -0,0 +1,18 @@
+U-Boot for Freescale i.MX5x
+
+This file contains information for the port of U-Boot to the Freescale
+i.MX5x SoCs.
+
+1. CONFIGURATION OPTIONS/SETTINGS
+-
+
+1.1 CONFIG_MX51_PLL_ERRATA: Workaround for i.MX51 PLL errata.
+This option should be enabled by all boards using the i.MX51 silicon
+version up until (including) 3.0 running at 800MHz.
+The PLL's in the i.MX51 processor can go out of lock due to a metastable
+condition in an analog flip-flop when used at high frequencies.
+This workaround implements an undocumented feature in the PLL (dither
+mode), which causes the effect of this failure to be much lower (in terms
+of frequency deviation), avoiding system failure, or at least decreasing
+the likelihood of system failure.
+
-- 
1.7.4.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] ARM: MX5: Remove broken leftover TO-2 errata workaround

2011-07-14 Thread David Jander
On Thu, 14 Jul 2011 10:16:51 +0200
Stefano Babic  wrote:

> On 07/14/2011 09:13 AM, David Jander wrote:
> > This check is broken. r3 does not contain the silicon revision.
> > 
> > Signed-off-by: David Jander 
> > ---
> 
> Hi David,
> 
> >  arch/arm/cpu/armv7/mx5/lowlevel_init.S |5 -
> >  1 files changed, 0 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm/cpu/armv7/mx5/lowlevel_init.S
> > b/arch/arm/cpu/armv7/mx5/lowlevel_init.S index ee4150d..f17d200 100644
> > --- a/arch/arm/cpu/armv7/mx5/lowlevel_init.S
> > +++ b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
> > @@ -39,11 +39,6 @@
> > orr r0, r0, #(1 << 23)  /* disable write allocate
> > combine */ orr r0, r0, #(1 << 22)   /* disable write allocate
> > */ 
> > -   cmp r3, #0x10/* r3 contains the silicon rev */
> 
> You are right. Nobody sets the r3 register, the test can be wrong.
> 
> > -
> > -   /* disable write combine for TO 2 and lower revs */
> > -   orrls r0, r0, #(1 << 25)
> 
> However, you also remove the setup for TO2. To fix the TO2 issue, we
> should read correctly the revision number (from IIM or from a fixed
> address, I do not remember now), and then apply the compare to the read
> value.

Yes, you are right.
But I don't know how to do it correctly.
OTOH, it is broken now for all platforms. My patch fixes it for TO3 and
newer. L2 write-combine has a significant performance impact, and I wonder how
many boards there are still that use such an old (prototype silicon) processor.
IMHO, the vast majority of MX51 users will benefit from this patch, and the
rest shouldn't have any more problems than they have already, so can we just
apply this, please?

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] ARM: MX51: PLL errata workaround

2011-07-14 Thread David Jander
On Thu, 14 Jul 2011 10:30:11 +0200
Stefano Babic  wrote:

> On 07/14/2011 09:11 AM, David Jander wrote:
> > This is a port of the official PLL errata workaround from Freescale to
> > mainline u-boot.
> > The PLL's in the i.MX51 processor can go out of lock due to a metastable
> > condition in an analog flip-flop when used at high frequencies.
> > This workaround implements an undocumented feature in the PLL (dither
> > mode), which causes the effect of this failure to be much lower (in terms
> > of frequency deviation), avoiding system failure, or at least decreasing
> > the likelihood of system failure.
> > 
> > Signed-off-by: David Jander 
> 
> Hi David,
> 
> do you have now also an official Errata number from Freescale to be
> added to your documentation ?

Needless to say that this supersedes my patch sent back in May, 26th... which
btw did not help much, since _all_ PLL's are affected by this problem. Not
only PLL1.

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] ARM: MX51: PLL errata workaround

2011-07-14 Thread David Jander

Hi Stefano,

On Thu, 14 Jul 2011 10:30:11 +0200
Stefano Babic  wrote:

> On 07/14/2011 09:11 AM, David Jander wrote:
> > This is a port of the official PLL errata workaround from Freescale to
> > mainline u-boot.
> > The PLL's in the i.MX51 processor can go out of lock due to a metastable
> > condition in an analog flip-flop when used at high frequencies.
> > This workaround implements an undocumented feature in the PLL (dither
> > mode), which causes the effect of this failure to be much lower (in terms
> > of frequency deviation), avoiding system failure, or at least decreasing
> > the likelihood of system failure.
> > 
> > Signed-off-by: David Jander 
> 
> Hi David,
> 
> do you have now also an official Errata number from Freescale to be
> added to your documentation ?

Freescale promised an official errata a week ago, but released the errata just
yesterday (hadn't seen it until now).
The number is ENGcm12051.
Do you mind including it in the commit yourself, or do you want me to re-post?

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] ARM: MX5: Remove broken leftover TO-2 errata workaround

2011-07-14 Thread David Jander
On Thu, 14 Jul 2011 12:49:15 +0200
Stefano Babic  wrote:

> On 07/14/2011 12:20 PM, David Jander wrote:
> 
> >> However, you also remove the setup for TO2. To fix the TO2 issue, we
> >> should read correctly the revision number (from IIM or from a fixed
> >> address, I do not remember now), and then apply the compare to the read
> >> value.
> > 
> > Yes, you are right.
> > But I don't know how to do it correctly.
> 
> There is a similar code always in lowlevel_init.S
> 
> 189 ldr r1, =0x0
> 190 ldr r3, [r1, #ROM_SI_REV]
> 191 cmp r3, #0x10
> 
> As we can suppose this is correct, the same code can be used in the macro.

Hmmm. Hadn't seen that part. Can we trust this?... because I have no means of
testing for the TO2 case.

> > OTOH, it is broken now for all platforms.
> 
> Agree we have to fix it. I only dislike to break some boards. As far as
> I know, there is many mx51evk boards sold by Freescale with the TO2 chip.

Ah, ok. AFAICR, our EVK has a TO3, but I agree there might be low quantities
of EVKs with TO2 still in use somewhere.

> > My patch fixes it for TO3 and
> > newer. L2 write-combine has a significant performance impact, and I wonder
> > how many boards there are still that use such an old (prototype silicon)
> > processor.
> 
> I think only on the evaluation boards, but they were sold and delivered.

Ok.

> > IMHO, the vast majority of MX51 users will benefit from this patch, and the
> > rest shouldn't have any more problems than they have already, so can we
> > just apply this, please?
> 
> Not as it is  - I prefer we fix the test. Can you resubmit with the
> proposed changes ?

Ok, thanks for pointing out the missing code. I will fix and re-submit.

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] ARM: MX5: Fix broken leftover TO-2 errata workaround

2011-07-14 Thread David Jander
This check was broken. r3 does not contain the silicon revision anymore, so
we need to reload it.

Signed-off-by: David Jander 
---
 arch/arm/cpu/armv7/mx5/lowlevel_init.S |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/arm/cpu/armv7/mx5/lowlevel_init.S 
b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
index ee4150d..6bb398f 100644
--- a/arch/arm/cpu/armv7/mx5/lowlevel_init.S
+++ b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
@@ -39,7 +39,9 @@
orr r0, r0, #(1 << 23)  /* disable write allocate combine */
orr r0, r0, #(1 << 22)  /* disable write allocate */
 
-   cmp r3, #0x10/* r3 contains the silicon rev */
+   ldr r1, =0x0
+   ldr r3, [r1, #ROM_SI_REV]
+   cmp r3, #0x10
 
/* disable write combine for TO 2 and lower revs */
orrls r0, r0, #(1 << 25)
-- 
1.7.4.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] ARM: MX5: Fix broken leftover TO-2 errata workaround

2011-07-14 Thread David Jander
On Thu, 14 Jul 2011 14:14:52 +0200
Stefano Babic  wrote:

> On 07/14/2011 01:56 PM, David Jander wrote:
> > This check was broken. r3 does not contain the silicon revision anymore, so
> > we need to reload it.
> > 
> > Signed-off-by: David Jander 
> > ---
> 
> Hi David,
> 
> >  arch/arm/cpu/armv7/mx5/lowlevel_init.S |4 +++-
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/arm/cpu/armv7/mx5/lowlevel_init.S
> > b/arch/arm/cpu/armv7/mx5/lowlevel_init.S index ee4150d..6bb398f 100644
> > --- a/arch/arm/cpu/armv7/mx5/lowlevel_init.S
> > +++ b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
> > @@ -39,7 +39,9 @@
> > orr r0, r0, #(1 << 23)  /* disable write allocate
> > combine */ orr r0, r0, #(1 << 22)   /* disable write allocate
> > */ 
> > -   cmp r3, #0x10/* r3 contains the silicon rev */
> > +   ldr r1, =0x0
> > +   ldr r3, [r1, #ROM_SI_REV]
> > +   cmp r3, #0x10
> 
> You have to protect the code related to TO2 with CONFIG_MX51. As I can
> see, the macro is called for MX.53, too, and the test produces wrong
> results.

Wow, you are right! So this code was actually broken in more ways than I
initially thought :-)
Ok, will fix it (again).
Btw, may I congratulate you for your good work in improving u-boot code
quality? It is starting to get to a level only known from LKML ;-)
Clearly the code was not reviewed as well when this was initially submitted.

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH V2] ARM: MX5: Fix broken leftover TO-2 errata workaround

2011-07-14 Thread David Jander
This check was broken. r3 does not contain the silicon revision anymore, so
we need to reload it. Also, this errata only applies to i.MX51.

Changed in this version:
 - Added #ifdef CONFIG_MX51 around the workaround

Signed-off-by: David Jander 
---
 arch/arm/cpu/armv7/mx5/lowlevel_init.S |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/arm/cpu/armv7/mx5/lowlevel_init.S 
b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
index ee4150d..6c66b42 100644
--- a/arch/arm/cpu/armv7/mx5/lowlevel_init.S
+++ b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
@@ -39,10 +39,14 @@
orr r0, r0, #(1 << 23)  /* disable write allocate combine */
orr r0, r0, #(1 << 22)  /* disable write allocate */
 
-   cmp r3, #0x10/* r3 contains the silicon rev */
+#if defined(CONFIG_MX51)
+   ldr r1, =0x0
+   ldr r3, [r1, #ROM_SI_REV]
+   cmp r3, #0x10
 
/* disable write combine for TO 2 and lower revs */
orrls r0, r0, #(1 << 25)
+#endif
 
mcr 15, 1, r0, c9, c0, 2
 .endm /* init_l2cc */
-- 
1.7.4.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH V3] ARM: MX5: Fix broken leftover TO-2 errata workaround

2011-07-14 Thread David Jander
This check was broken. r3 does not contain the silicon revision anymore, so
we need to reload it. Also, this errata only applies to i.MX51.

Signed-off-by: David Jander 
---

Changed in this version:
  - Move patch changelog below '---' line.

 arch/arm/cpu/armv7/mx5/lowlevel_init.S |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/arm/cpu/armv7/mx5/lowlevel_init.S 
b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
index ee4150d..6c66b42 100644
--- a/arch/arm/cpu/armv7/mx5/lowlevel_init.S
+++ b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
@@ -39,10 +39,14 @@
orr r0, r0, #(1 << 23)  /* disable write allocate combine */
orr r0, r0, #(1 << 22)  /* disable write allocate */
 
-   cmp r3, #0x10/* r3 contains the silicon rev */
+#if defined(CONFIG_MX51)
+   ldr r1, =0x0
+   ldr r3, [r1, #ROM_SI_REV]
+   cmp r3, #0x10
 
/* disable write combine for TO 2 and lower revs */
orrls r0, r0, #(1 << 25)
+#endif
 
mcr 15, 1, r0, c9, c0, 2
 .endm /* init_l2cc */
-- 
1.7.4.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] i.MX51: FEC: Cache coherency problem?

2011-07-18 Thread David Jander

Hi all,

I am busy debugging a problem with the i.MX51 FEC ethernet driver, that
stopped working after upgrading u-boot. Before the upgrade I used
v2010.06-rc3, which worked fine.
I gave up on trying to find the difference beween the old version and this one
that broke it due to BSP issues, git bisecting seems a monumental task I
am not considering yet.
The funny part is that it seems to work fine if I disable data-caches!
With data caches enabled, it hangs in the following while loop in fec_send(),
at line 592:

...
584 /*
585  * Enable SmartDMA transmit task
586  */
587 fec_tx_task_enable(fec);
588 
589 /*
590  * wait until frame is sent .
591  */
592 while (readw(&fec->tbd_base[fec->tbd_index].status) & 
FEC_TBD_READY) {
593 udelay(1);
594 }
...

If I change this code in the following way, the while loop exits successfully:

...
584 /*
585  * Enable SmartDMA transmit task
586  */
587 flush_cache(&fec->tbd_base[fec->tbd_index], 4);
588 fec_tx_task_enable(fec);
589 flash_dcache_all();
590 
591 /*
592  * wait until frame is sent .
593  */
594 while (readw(&fec->tbd_base[fec->tbd_index].status) & 
FEC_TBD_READY) {
595 udelay(1);
596 }
...

Notice the cache flush calls at line 587 and 589. With these, sending
succeeds. The driver still hangs in receiving afterwards, but at least this
part seems to work. If I remove either of the two added lines, it stops
working again.

What is going on here? Why did this work with caches enabled before??

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] i.MX51: FEC: Cache coherency problem?

2011-07-19 Thread David Jander
On Mon, 18 Jul 2011 21:46:28 +0530
Aneesh V  wrote:

> Hi David,
> 
> On Monday 18 July 2011 08:48 PM, David Jander wrote:
> >
> > Hi all,
> >
> > I am busy debugging a problem with the i.MX51 FEC ethernet driver, that
> > stopped working after upgrading u-boot. Before the upgrade I used
> > v2010.06-rc3, which worked fine.
> > I gave up on trying to find the difference beween the old version and this
> > one that broke it due to BSP issues, git bisecting seems a monumental
> > task I am not considering yet.
> > The funny part is that it seems to work fine if I disable data-caches!
> > With data caches enabled, it hangs in the following while loop in
> > fec_send(), at line 592:
> >
> > ...
> > 584 /*
> > 585  * Enable SmartDMA transmit task
> > 586  */
> > 587 fec_tx_task_enable(fec);
> > 588
> > 589 /*
> > 590  * wait until frame is sent .
> > 591  */
> > 592 while (readw(&fec->tbd_base[fec->tbd_index].status)&
> > FEC_TBD_READY) { 593 udelay(1);
> > 594 }
> > ...
> >
> > If I change this code in the following way, the while loop exits
> > successfully:
> >
> > ...
> > 584 /*
> > 585  * Enable SmartDMA transmit task
> > 586  */
> > 587 flush_cache(&fec->tbd_base[fec->tbd_index], 4);
> > 588 fec_tx_task_enable(fec);
> > 589 flash_dcache_all();
> > 590
> > 591 /*
> > 592  * wait until frame is sent .
> > 593  */
> > 594 while (readw(&fec->tbd_base[fec->tbd_index].status)&
> > FEC_TBD_READY) { 595 udelay(1);
> > 596 }
> > ...
> >
> > Notice the cache flush calls at line 587 and 589. With these, sending
> > succeeds. The driver still hangs in receiving afterwards, but at least this
> > part seems to work. If I remove either of the two added lines, it stops
> > working again.
> >
> > What is going on here? Why did this work with caches enabled before??
> 
> Are you sure caches were enabled before? Until recently absence of
> CONFIG_SYS_DCACHE_OFF did not mean that your D-cache is enabled because
> you also had to call the function dcache_enable(). Some platforms were
> calling this from board file or some drivers but not all.

We did. I am always calling dcache_enable(), icache_enable() and
l2_cache_enable() from board code. The latter function I implemented myself,
because nor u-boot nor the kernel does otherwise activate l2-cache, which has a
big performance impact.

Just in case, at the moment I am testing with only the L1 caches enabled.

> The following patch changed this for ARM:
> 
> commit c2dd0d45540397704de9b13287417d21049d34c6
> armv7: integrate cache maintenance support
> 
> In this patch I added a call to dcache_enable() at the beginning of
> board_init_r() for ARM(i.e. as soon as relocation is over). As a result
> D-cache will be enabled for all ARM platforms now unless
> CONFIG_SYS_DCACHE_OFF is set.

Yes, I noticed that.

> Looks like this is a real coherency issue that is brought out because
> d-cache is really enabled for you now.

AFAICS, it was always enabled for our board. At least I did always call
i/dcache_enable() in board setup code was it broken before? Are the L1
caches enabled by (mainline) linux code?

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] i.MX51: FEC: Cache coherency problem?

2011-07-19 Thread David Jander

Hi Stefano,

On Mon, 18 Jul 2011 18:55:05 +0200
Stefano Babic  wrote:

> On 07/18/2011 05:18 PM, David Jander wrote:
> > 
> > Hi all,
> 
> Hi David,
> 
> > What is going on here? Why did this work with caches enabled before??
> 
> I think cache was always disabled..

I had even L2-caches enabled in u-boot (copied/adapted some code from OMAP
cache.S), and called i/dcache_enable() from board code like this:

int board_late_init(void)
{
power_init();
probe_board_type();
icache_enable();
dcache_enable();

return 0;
}

Is there a reason this wouldn't have worked before?

Suppose it didn't. Does that mean we need to use the MMU to properly mark
regions of register space and specially FEC BD's as not-cached? Or do we need
to flash caches manually each time such a memory region is accessed?
I am kind of a CPU-speed-junkie, so I am not sure I want to live without
caches enabled in u-boot ;-)

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] i.MX51: FEC: Cache coherency problem?

2011-07-19 Thread David Jander
On Tue, 19 Jul 2011 10:21:12 +0200
Albert ARIBAUD  wrote:

> Hi David,
> 
> Le 19/07/2011 09:44, David Jander a écrit :
> >
> > Hi Stefano,
> >
> > On Mon, 18 Jul 2011 18:55:05 +0200
> > Stefano Babic  wrote:
> >
> >> On 07/18/2011 05:18 PM, David Jander wrote:
> >>>
> >>> Hi all,
> >>
> >> Hi David,
> >>
> >>> What is going on here? Why did this work with caches enabled before??
> >>
> >> I think cache was always disabled..
> >
> > I had even L2-caches enabled in u-boot (copied/adapted some code from OMAP
> > cache.S), and called i/dcache_enable() from board code like this:
> >
> > int board_late_init(void)
> > {
> >  power_init();
> >  probe_board_type();
> >  icache_enable();
> >  dcache_enable();
> >
> >  return 0;
> > }
> >
> > Is there a reason this wouldn't have worked before?
> >
> > Suppose it didn't. Does that mean we need to use the MMU to properly mark
> > regions of register space and specially FEC BD's as not-cached? Or do we
> > need to flash caches manually each time such a memory region is accessed?
> > I am kind of a CPU-speed-junkie, so I am not sure I want to live without
> > caches enabled in u-boot ;-)
> 
> You would have to flush (before sending packets / starting external 
> memory-to-device DMA) and invalidate (before reading received packets / 
> after external device-to-memory DMA is done); using MMU and mapping 
> cached/non-cached areas is IMO overkill, and will hurt CPU accesses to 
> the xmit/receive buffers and descriptors.

So, you say actually what I did while exploring the problem would have been a
correct way of solving this problem?

Like this:

587 flush_cache(&fec->tbd_base[fec->tbd_index], 4);
588 fec_tx_task_enable(fec);
589 flush_dcache_all();
590 
591 /*
592  * wait until frame is sent .
593      */
594 while (readw(&fec->tbd_base[fec->tbd_index].status) & 
FEC_TBD_READY) {
595 udelay(1);
596 }

I am still not sure why I need line 587 above.
Would a patch to fec_mxc.c that does the necessary cache handwork be
acceptable for u-boot?

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] i.MX51: FEC: Cache coherency problem?

2011-07-19 Thread David Jander

Dear Aneesh,

Thanks a lot for your replies.

On Tue, 19 Jul 2011 14:13:34 +0530
Aneesh V  wrote:
> On Tuesday 19 July 2011 02:07 PM, David Jander wrote:
> > On Tue, 19 Jul 2011 10:21:12 +0200
> > Albert ARIBAUD  wrote:
> >
> >> Hi David,
> >>
> >> Le 19/07/2011 09:44, David Jander a écrit :
> >>>
> >>> Hi Stefano,
> >>>
> >>> On Mon, 18 Jul 2011 18:55:05 +0200
> >>> Stefano Babic   wrote:
> >>>
> >>>> On 07/18/2011 05:18 PM, David Jander wrote:
> >>>>>
> >>>>> Hi all,
> >>>>
> >>>> Hi David,
> >>>>
> >>>>> What is going on here? Why did this work with caches enabled before??
> >>>>
> >>>> I think cache was always disabled..
> >>>
> >>> I had even L2-caches enabled in u-boot (copied/adapted some code from
> >>> OMAP cache.S), and called i/dcache_enable() from board code like this:
> >>>
> >>> int board_late_init(void)
> >>> {
> >>>   power_init();
> >>>   probe_board_type();
> >>>   icache_enable();
> >>>   dcache_enable();
> >>>
> >>>   return 0;
> >>> }
> >>>
> >>> Is there a reason this wouldn't have worked before?
> >>>
> >>> Suppose it didn't. Does that mean we need to use the MMU to properly mark
> >>> regions of register space and specially FEC BD's as not-cached? Or do we
> >>> need to flash caches manually each time such a memory region is accessed?
> >>> I am kind of a CPU-speed-junkie, so I am not sure I want to live without
> >>> caches enabled in u-boot ;-)
> 
> Are you talking about some memory-mapped IO region(register space). If
> that is the case, that region won't be cached. ARM mmu implementation
> makes only the SDRAM region cached. Rest is non-cached non-buffered.

Ah, thanks for pointing out. I already suspected something like that...

> >> You would have to flush (before sending packets / starting external
> >> memory-to-device DMA) and invalidate (before reading received packets /
> >> after external device-to-memory DMA is done); using MMU and mapping
> >> cached/non-cached areas is IMO overkill, and will hurt CPU accesses to
> >> the xmit/receive buffers and descriptors.
> >
> > So, you say actually what I did while exploring the problem would have
> > been a correct way of solving this problem?
> >
> > Like this:
> >
> > 587 flush_cache(&fec->tbd_base[fec->tbd_index], 4);
> 
> This is what is needed assuming the below is initiating a memory to
> peripheral DMA. Is your buffer only 4 bytes long?

No it isn't. I know, I should flush the whole buffer area, but this was just
enough to get the status field flushed, so the FEC started transmitting, and
the while loop ended eventually. The result was still not correct, but at
least it won't hang.
What would be more expensive, flushing just the buffer area, or
flush_dcache_all()?

> Also, please check if flush_cache() is correctly supported for your
> CPU. The default implementation in in arch/arm/lib/cache.c has support
> for only a handful of cpus. AFAIK, only armv7 is over-riding this
> default implementation at the moment.

There is cache_v7.c which implements these... they are supposed to work
correctly I guess?

> The fact that it's helping you indicates that it may be working for
> you. But still worth a check.
> 
> > 588 fec_tx_task_enable(fec);
> > 589 flush_dcache_all();
> 
> This should not be needed.

I agree, but without it the while loop below still gets stuck.

> > 590
> > 591 /*
> > 592  * wait until frame is sent .
> > 593  */
> > 594 while (readw(&fec->tbd_base[fec->tbd_index].status)&
> > FEC_TBD_READY) {
> > 595 udelay(1);
> > 596 }
> >
> > I am still not sure why I need line 587 above.
> 
> Did you try keeping 587 and removing 589?

Yes, I did. These two lines really is the minimum necessary to exit the while
loop. It won't work if I leave out either of those.
Now that I know a little more, I guess this is because of the status field in
the buffer-descriptor being checked in the while loop, and that is still in
cache. So the only thing line 589 does, is invalidate the caches, so the next
readw() returns the value stored by the FEC, which is apparently faster than
this piece of code :-)

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] i.MX51: FEC: Cache coherency problem?

2011-07-19 Thread David Jander
On Tue, 19 Jul 2011 13:07:52 +0200
Matthias Weißer  wrote:

> Dear Aneesh
> 
> Am 18.07.2011 18:16, schrieb Aneesh V:
> > commit c2dd0d45540397704de9b13287417d21049d34c6
> > armv7: integrate cache maintenance support
> >
> > In this patch I added a call to dcache_enable() at the beginning of
> > board_init_r() for ARM(i.e. as soon as relocation is over). As a result
> > D-cache will be enabled for all ARM platforms now unless
> > CONFIG_SYS_DCACHE_OFF is set.
> 
> Is this really a good idea? This will break a couple of boards using 
> non-cache-aware drivers. And there are a couple of them in u-boot. I 
> think d-cache should be opt-in rather then opt-out as long as there are 
> any drivers which didn't handle cached memory regions correct. i-cache 
> is much less problematic and can be enabled by default.
> 
> If d-cache will be enabled by default on ARM I think I have to send a 
> patch for one of my boards :-)

Exactly the issue I am having now... the fact that I was unaware of this patch
and generally don't have much experience with solving cache-related issues
apparently, made me waste a lot of time debugging the fec_mxc.c driver while
the driver itself wasn't really broken (at least not more than before).
Now I finally know what's wrong and am working on a proposed fix to make this
one driver cache-aware.

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] i.MX51: FEC: Cache coherency problem?

2011-07-19 Thread David Jander
On Tue, 19 Jul 2011 13:20:26 +0200
Wolfgang Denk  wrote:

> Dear David Jander,
> 
> In message <20110719131744.403a81e6@archvile> you wrote:
> > 
> > Now I finally know what's wrong and am working on a proposed fix to make
> > this one driver cache-aware.
> 
> I would just like to point out that these efforts are highly
> appreciated!

Thanks.
I'll try my best, but I am running out of time, and it is still not as it
should. I still have trouble identifying the right place where receive buffer
descriptors should be cache-invalidated. At one point, a magic
flush_dcache_all() at a certain place in code made it work, but that can't be
entirely correct, and if I replace it with only the necessary ranges, it
doesn't work correctly anymore, so I guess this flush is also invalidating
something I need elsewhere :-(
I keep searching through the IMO fairly convoluted network code.

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] i.MX51: FEC: Cache coherency problem?

2011-07-19 Thread David Jander
On Tue, 19 Jul 2011 07:36:32 -0700
"J. William Campbell"  wrote:

> On 7/19/2011 2:05 AM, Albert ARIBAUD wrote:
> > Le 19/07/2011 10:43, Aneesh V a écrit :
> >
> >>>> You would have to flush (before sending packets / starting external
> >>>> memory-to-device DMA) and invalidate (before reading received packets /
> >>>> after external device-to-memory DMA is done); using MMU and mapping
> >>>> cached/non-cached areas is IMO overkill, and will hurt CPU accesses to
> >>>> the xmit/receive buffers and descriptors.
> >>> So, you say actually what I did while exploring the problem would have
> >>> been a
> >>> correct way of solving this problem?
> >>>
> >>> Like this:
> >>>
> >>> 587 flush_cache(&fec->tbd_base[fec->tbd_index], 4);
> >> This is what is needed assuming the below is initiating a memory to
> >> peripheral DMA. Is your buffer only 4 bytes long?
> > Generally:
> >
> > - for sending data through a device that has its own, external, DMA
> > engine, you'll obviously need to flush the data buffer(s) but also any
> > DMA descriptors used by the engine, before you start the engine;
> >
> > - for rceiving, if you have to set up receive descriptors, you must
> > flush that before telling the device to enter receive mode (so that the
> > device reads the descriptors as you wrote them), and you should
> > invalidate the receive buffers at the latest when the device signals
> > that data has been received,
> Hi All,
> 
> >   or preferably long before (at the same time
> > you flushed the read descriptor, so that cache-related actions are
> > grouped in the same place in the code).
> I think this last statement is incorrect. You should invalidate the 
> cache for the receive buffers just before you intend to reference them. 
> If you do it right after receive mode is entered, subsequent access to 
> items NEAR the receive buffer may reload the cache with receive buffer 
> data before the dma is done, re-creating the problem you are trying to 
> avoid. Also, I don't know if ARM cache is write-back or write -thru, but 
> if it is write-back, the only way to avoid problems is to allocate the 
> receive buffers on cache line boundaries, so no "nearby" writes can 
> cause something in the DMA buffer to be corrupted. If all receive 
> buffers are allocated on cache-line boundaries (both start and end of 
> each buffer), you can invalidate the cache "early" under the assumption 
> that there will be no read accesses to the read buffers themselves until 
> after DMA is complete. IMHO it is better, even in this case., to 
> invalidate cache after dma is done but before referencing the read data.

Hey, thanks a lot for this bit of information! I really needed it! :-)
Indeed I hadn't thought about the situation of nearby memory accesses
allocating critical cache-lines...

Coming to think of it... would it really be that hard to enable the MMU in
u-boot and configure just two pools of flat, direct mapped linear memory to
malloc and dma_malloc from?
That would make fixing broken drivers trivial, and have other benefits (like
video memory coherence when caches are on)!
IMHO, changing the necessary malloc()'s for dma_malloc()'s is a lot less work
and easier to oversee than obscure cache-manipulation and alignment tricks all
over the place What do you think?

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] i.MX51: FEC: Cache coherency problem?

2011-07-19 Thread David Jander
On Tue, 19 Jul 2011 14:10:48 +0200
David Jander  wrote:

> On Tue, 19 Jul 2011 13:20:26 +0200
> Wolfgang Denk  wrote:
> 
> > Dear David Jander,
> > 
> > In message <20110719131744.403a81e6@archvile> you wrote:
> > > 
> > > Now I finally know what's wrong and am working on a proposed fix to make
> > > this one driver cache-aware.
> > 
> > I would just like to point out that these efforts are highly
> > appreciated!
> 
> Thanks.
> I'll try my best, but I am running out of time, and it is still not as it
> should. I still have trouble identifying the right place where receive buffer
> descriptors should be cache-invalidated. At one point, a magic
> flush_dcache_all() at a certain place in code made it work, but that can't be
> entirely correct, and if I replace it with only the necessary ranges, it
> doesn't work correctly anymore, so I guess this flush is also invalidating
> something I need elsewhere :-(
> I keep searching through the IMO fairly convoluted network code.

Ok, that was not such a problem. I found few nice spots, where
cache-maintenance code should go IMO:

In fec_rx_task_enable() cleaned RX BD's should be flushed.
In fec_tx_task_enable() dirty TX BD's should be flushed, as well as data
buffers.
In fec_send() Invalidate BD status fields before reading, also in the while
loop.
In fec_recv() Invalidate BD and data buffers just before every read.

And of course align all buffers to cache-line size and pad around the end of
each buffer to cache-line size.

Problem: It doesn't work (tftp transfer stops after a few packets)! :-(
I am giving up!

Plan B: I'll now try to allocate all buffers in internal RAM (which is not
cached AFAIK).

Any ideas?

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] i.MX51: FEC: Cache coherency problem?

2011-07-20 Thread David Jander
On Wed, 20 Jul 2011 10:56:07 +0200
Albert ARIBAUD  wrote:

> Hi David,
> 
> Le 20/07/2011 08:29, David Jander a écrit :
> > On Tue, 19 Jul 2011 14:10:48 +0200
> > David Jander  wrote:
> >
> >> On Tue, 19 Jul 2011 13:20:26 +0200
> >> Wolfgang Denk  wrote:
> >>
> >>> Dear David Jander,
> >>>
> >>> In message<20110719131744.403a81e6@archvile>  you wrote:
> >>>>
> >>>> Now I finally know what's wrong and am working on a proposed fix to make
> >>>> this one driver cache-aware.
> >>>
> >>> I would just like to point out that these efforts are highly
> >>> appreciated!
> >>
> >> Thanks.
> >> I'll try my best, but I am running out of time, and it is still not as it
> >> should. I still have trouble identifying the right place where receive
> >> buffer descriptors should be cache-invalidated. At one point, a magic
> >> flush_dcache_all() at a certain place in code made it work, but that
> >> can't be entirely correct, and if I replace it with only the necessary
> >> ranges, it doesn't work correctly anymore, so I guess this flush is also
> >> invalidating something I need elsewhere :-(
> >> I keep searching through the IMO fairly convoluted network code.
> >
> > Ok, that was not such a problem. I found few nice spots, where
> > cache-maintenance code should go IMO:
> >
> > In fec_rx_task_enable() cleaned RX BD's should be flushed.
> > In fec_tx_task_enable() dirty TX BD's should be flushed, as well as data
> > buffers.
> > In fec_send() Invalidate BD status fields before reading, also in the while
> > loop.
> > In fec_recv() Invalidate BD and data buffers just before every read.
> >
> > And of course align all buffers to cache-line size and pad around the end
> > of each buffer to cache-line size.
> >
> > Problem: It doesn't work (tftp transfer stops after a few packets)! :-(
> > I am giving up!
> >
> > Plan B: I'll now try to allocate all buffers in internal RAM (which is not
> > cached AFAIK).
> >
> > Any ideas?
> 
> Yes, one: I had issues with the Marvell Ethernet adapter, which has DMA 
> as well, not because of cache (it was not active at the time) but 
> because of instruction reordering done by the compiler when optimizing, 
> which resulted in out-of-order accesses to the descritpors and DMA 
> registered, so that the DMA start was written before the descriptors 
> were set up. The (proper and clean) solution was to introduce memory 
> barriers at strategic points of the driver to ensure that specific DMA 
> starts were done only after all writes to the descriptors (and possibly 
> buffers).

Hmmm. I know that issue, but AFAICS, the driver already uses readX() and
writeX() macros when accessing register and DMA memory. Those macros have
compiler barriers included and armv7 is still in-order execution ;-)
Thanks for the suggestion anyway :-)

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] i.MX51: FEC: Cache coherency problem?

2011-07-20 Thread David Jander

Hi Aneesh,

On Wed, 20 Jul 2011 15:59:42 +0530
Aneesh V  wrote:
> On Wednesday 20 July 2011 02:51 PM, David Jander wrote:
> > On Wed, 20 Jul 2011 10:56:07 +0200
> 
> [snip ..]
> 
> >>> Any ideas?
> >>
> >> Yes, one: I had issues with the Marvell Ethernet adapter, which has DMA
> >> as well, not because of cache (it was not active at the time) but
> >> because of instruction reordering done by the compiler when optimizing,
> >> which resulted in out-of-order accesses to the descritpors and DMA
> >> registered, so that the DMA start was written before the descriptors
> >> were set up. The (proper and clean) solution was to introduce memory
> >> barriers at strategic points of the driver to ensure that specific DMA
> >> starts were done only after all writes to the descriptors (and possibly
> >> buffers).
> >
> > Hmmm. I know that issue, but AFAICS, the driver already uses readX() and
> > writeX() macros when accessing register and DMA memory. Those macros have
> > compiler barriers included and armv7 is still in-order execution ;-)
> 
> 1. armv7 is not necessarily in-order. Cortex-A8 is in-order while
> Cortex-A9 is out-of-order.
> 2. I am not sure if in-order execution guarantees memory ordering. My
> understanding is that it doesn't.
> 3. In u-boot's implementation of MMU for ARM, the register
> space(everything other than SDRAM) is 'strongly-ordered' memory.
> Strongly ordered accesses will be correctly ordered w.r.to all other
> accesses in program order.

Wow, I hadn't noticed dcache_enable() also enables the MMU. Cool. One step
closer to splitting the mapping between a cached and uncached, strongly-ordered
region of RAM.

> 4. Compiler barriers prevent the compiler from doing certain
> optimizations, but doesn't help in these situations.

Thanks for this excellent summary!

> In short, I think you don't need to worry about ordering, but not
> because of compiler barriers or armv7 being in-order.

Well, I do need to worry about the buffer-descriptors and buffers then, since
they are in SDRAM, right?

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] i.MX51: FEC: Cache coherency problem?

2011-07-20 Thread David Jander
cached areas, because of burst reads vs. individual 
> reads. However, I doubt that the u-boot user can tell the difference, as 
> the network latency will far exceed the difference in copy time. The 
> question is, which is easier to do, and that is probably a matter of 
> opinion. However, it is safe to say that so far a cached solution has 
> eluded us. That may be changing, but it would still be nice to know how 
> to allocate a section of un-cached RAM in the ARM processor, in so far 
> as the question has a single answer! That would allow easy portability 
> of drivers that do not know about caches, of which there seems to be many.

I agree. Unfortunately, my time is up for now, and I can't go on with trying
to fix this driver. Maybe I'll pick up after my vacation.
As for now I settled for the ugly solution of keeping dcache disabled while
ethernet is being used :-(
IMHO, doing cache maintenance all over the driver is not an easy or nice
solution. Implementing a non-cached memory pool in the MMU and a corresponding
dma_malloc() sounds like much more universally applicable to any driver.

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 0/4] Add support for PRTLVT2 boards

2010-08-19 Thread David Jander
The following patches add support for PRTLVT2 based boards (i.MX515 SoC).

[PATCH 1/4] MX51: iomux: Added support for mxc_iomux_set_input()
[PATCH 2/4] MX51: Added missing pin definition
[PATCH 3/4] mc13982 driver: corrected/added some definitions according to 
latest user-manual
[PATCH 4/4] Added initial support for PRTLVT2-based boards.

Signed-off-by: David Jander 

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 1/4] MX51: iomux: Added support for mxc_iomux_set_input()

2010-08-19 Thread David Jander
Signed-off-by: David Jander 
---
 arch/arm/cpu/armv7/mx51/iomux.c |8 +++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/arch/arm/cpu/armv7/mx51/iomux.c b/arch/arm/cpu/armv7/mx51/iomux.c
index 62b2954..fb48f1c 100644
--- a/arch/arm/cpu/armv7/mx51/iomux.c
+++ b/arch/arm/cpu/armv7/mx51/iomux.c
@@ -34,7 +34,7 @@ enum iomux_reg_addr {
IOMUXSW_MUX_CTL = IOMUXC_BASE_ADDR,
IOMUXSW_MUX_END = IOMUXC_BASE_ADDR + MUX_I_END,
IOMUXSW_PAD_CTL = IOMUXC_BASE_ADDR + PAD_I_START,
-   IOMUXSW_INPUT_CTL = IOMUXC_BASE_ADDR,
+   IOMUXSW_INPUT_CTL = IOMUXC_BASE_ADDR + INPUT_CTL_START,
 };
 
 #define MUX_PIN_NUM_MAX (((MUX_I_END - MUX_I_START) >> 2) + 1)
@@ -164,3 +164,9 @@ unsigned int mxc_iomux_get_pad(iomux_pin_name_t pin)
u32 pad_reg = get_pad_reg(pin);
return readl(pad_reg);
 }
+
+void mxc_iomux_set_input(iomux_input_select_t input, u32 config)
+{
+   u32 pad_reg = IOMUXSW_INPUT_CTL+(input*4);
+   writel(config, pad_reg);
+}
-- 
1.6.3.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 2/4] MX51: Added missing pin definition

2010-08-19 Thread David Jander
Signed-off-by: David Jander 
---
 arch/arm/include/asm/arch-mx51/mx51_pins.h |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/arch-mx51/mx51_pins.h 
b/arch/arm/include/asm/arch-mx51/mx51_pins.h
index ca26f41..c443f13 100644
--- a/arch/arm/include/asm/arch-mx51/mx51_pins.h
+++ b/arch/arm/include/asm/arch-mx51/mx51_pins.h
@@ -320,6 +320,7 @@ enum iomux_pins {
MX51_PIN_DISP1_DAT22 = _MXC_BUILD_NON_GPIO_PIN(0x324, 0x724),
MX51_PIN_DISP1_DAT23 = _MXC_BUILD_NON_GPIO_PIN(0x328, 0x728),
MX51_PIN_DI1_PIN3 = _MXC_BUILD_NON_GPIO_PIN(0x32C, 0x72C),
+   MX51_PIN_DI1_DISP_CLK = _MXC_BUILD_NON_GPIO_PIN(0, 0x730), /* No MUX 
register!! */
MX51_PIN_DI1_PIN2 = _MXC_BUILD_NON_GPIO_PIN(0x330, 0x734),
MX51_PIN_DI_GP1 = _MXC_BUILD_NON_GPIO_PIN(0x334, 0x73C),
MX51_PIN_DI_GP2 = _MXC_BUILD_NON_GPIO_PIN(0x338, 0x740),
-- 
1.6.3.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 3/4] mc13982 driver: corrected/added some definitions according to latest user-manual

2010-08-19 Thread David Jander
Signed-off-by: David Jander 
---
 include/fsl_pmic.h |2 +-
 include/mc13892.h  |   41 +
 2 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/include/fsl_pmic.h b/include/fsl_pmic.h
index e3abde6..2f2aa7d 100644
--- a/include/fsl_pmic.h
+++ b/include/fsl_pmic.h
@@ -112,7 +112,7 @@ enum {
 #define GPO4STBY   (1 << 13)
 #define PWGT1SPIEN (1 << 15)
 #define PWGT2SPIEN (1 << 16)
-#define PWUP   (1 << 21)
+#define GPO4ADIN   (1 << 21)
 
 /* Power Control 0 */
 #define COINCHEN   (1 << 23)
diff --git a/include/mc13892.h b/include/mc13892.h
index b291757..4eea6af 100644
--- a/include/mc13892.h
+++ b/include/mc13892.h
@@ -29,29 +29,51 @@
 
 /* REG_CHARGE */
 
-#define VCHRG0 0
+#define VCHRG0 (1 << 0)
 #define VCHRG1 (1 << 1)
 #define VCHRG2 (1 << 2)
 #define ICHRG0 (1 << 3)
 #define ICHRG1 (1 << 4)
 #define ICHRG2 (1 << 5)
 #define ICHRG3 (1 << 6)
-#define ICHRGTR0   (1 << 7)
-#define ICHRGTR1   (1 << 8)
-#define ICHRGTR2   (1 << 9)
+#define TREN   (1 << 7)
+#define ACKLPB (1 << 8)
+#define THCHKB (1 << 9)
 #define FETOVRD(1 << 10)
 #define FETCTRL(1 << 11)
 #define RVRSMODE   (1 << 13)
-#define OVCTRL0(1 << 15)
-#define OVCTRL1(1 << 16)
-#define UCHEN  (1 << 17)
+#define PLIM0  (1 << 15)
+#define PLIM1  (1 << 16)
+#define PLIMDIS(1 << 17)
 #define CHRGLEDEN  (1 << 18)
-#define CHRGRAWPDEN(1 << 19)
+#define CHGTMRRST  (1 << 19)
 #define CHGRESTART (1 << 20)
 #define CHGAUTOB   (1 << 21)
 #define CYCLB  (1 << 22)
 #define CHGAUTOVIB (1 << 23)
 
+
+/* Power Control 2 (reg 15) */
+#define RESTARTEN (1 << 0)
+#define PWRON1RSTEN   (1 << 1)
+#define PWRON2RSTEN   (1 << 2)
+#define PWRON3RSTEN   (1 << 3)
+#define PWRON1DBNC0   (1 << 4)
+#define PWRON1DBNC1   (1 << 5)
+#define PWRON2DBNC0   (1 << 6)
+#define PWRON2DBNC1   (1 << 7)
+#define PWRON3DBNC0   (1 << 8)
+#define PWRON3DBNC1   (1 << 9)
+#define STANDBYINV(1 << 10)
+#define STANDBYSECINV (1 << 11)
+#define WDIRESET  (1 << 12)
+#define SPIDRV0   (1 << 13)
+#define SPIDRV1   (1 << 14)
+#define CLK32KDRV0(1 << 17)
+#define CLK32KDRV1(1 << 18)
+#define STBYDLY0  (1 << 22)
+#define STBYDLY1  (1 << 23)
+
 /* REG_SETTING_0/1 */
 #define VO_1_20V   0
 #define VO_1_30V   1
@@ -84,6 +106,9 @@
 #define SWMODE_PFM_PFM 15
 #define SWMODE_MASK0x0F
 
+/* Switchers 4 (reg 28) */
+#define SWILIMB(1 << 22)
+
 #define SWMODE1_SHIFT  0
 #define SWMODE2_SHIFT  10
 #define SWMODE3_SHIFT  0
-- 
1.6.3.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 4/4] Added initial support for PRTLVT2-based boards.

2010-08-19 Thread David Jander
Signed-off-by: David Jander 
---
 board/Protonic/prtlvt2/Makefile |   48 
 board/Protonic/prtlvt2/config.mk|   25 ++
 board/Protonic/prtlvt2/imximage.cfg |  171 
 board/Protonic/prtlvt2/prtlvt2.c|  513 +++
 board/Protonic/prtlvt2/prtlvt2.h|   50 
 boards.cfg  |1 +
 include/configs/prtlvt2.h   |  203 ++
 7 files changed, 1011 insertions(+), 0 deletions(-)
 create mode 100644 board/Protonic/prtlvt2/Makefile
 create mode 100644 board/Protonic/prtlvt2/config.mk
 create mode 100644 board/Protonic/prtlvt2/imximage.cfg
 create mode 100644 board/Protonic/prtlvt2/prtlvt2.c
 create mode 100644 board/Protonic/prtlvt2/prtlvt2.h
 create mode 100644 include/configs/prtlvt2.h

diff --git a/board/Protonic/prtlvt2/Makefile b/board/Protonic/prtlvt2/Makefile
new file mode 100644
index 000..e1f1157
--- /dev/null
+++ b/board/Protonic/prtlvt2/Makefile
@@ -0,0 +1,48 @@
+#
+# Copyright (C) 2007, Guennadi Liakhovetski 
+#
+# (C) Copyright 2009 Freescale Semiconductor, Inc.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that 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, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+include $(TOPDIR)/config.mk
+
+LIB= $(obj)lib$(BOARD).a
+
+COBJS  := prtlvt2.o
+
+SRCS   := $(SOBJS:.o=.S) $(COBJS:.o=.c)
+OBJS   := $(addprefix $(obj),$(COBJS))
+SOBJS  := $(addprefix $(obj),$(SOBJS))
+
+$(LIB):$(obj).depend $(OBJS) $(SOBJS)
+   $(AR) $(ARFLAGS) $@ $(OBJS) $(SOBJS)
+
+clean:
+   rm -f $(SOBJS) $(OBJS)
+
+distclean: clean
+   rm -f $(LIB) core *.bak .depend
+
+#
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#
diff --git a/board/Protonic/prtlvt2/config.mk b/board/Protonic/prtlvt2/config.mk
new file mode 100644
index 000..af70ec2
--- /dev/null
+++ b/board/Protonic/prtlvt2/config.mk
@@ -0,0 +1,25 @@
+#
+# Copyright 2009 Freescale Semiconductor, Inc. All Rights Reserved.
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that 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, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+LDSCRIPT = $(CPUDIR)/$(SOC)/u-boot.lds
+TEXT_BASE = 0x9780
+IMX_CONFIG = $(SRCTREE)/board/$(BOARDDIR)/imximage.cfg
diff --git a/board/Protonic/prtlvt2/imximage.cfg 
b/board/Protonic/prtlvt2/imximage.cfg
new file mode 100644
index 000..a89168e
--- /dev/null
+++ b/board/Protonic/prtlvt2/imximage.cfg
@@ -0,0 +1,171 @@
+#
+# (C Copyright 2009
+# Stefano Babic DENX Software Engineering sba...@denx.de.
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License or (at your option) any later version.
+#
+# This program is distributed in the hope that 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 write to the Free Software
+# Foundation Inc. 51 Franklin Street Fifth Floor Boston,
+# MA 02110-1301 USA
+#
+# Refer docs/README.imxmage for more details about how-to configure
+# and create imximage boot image
+#
+# The syntax is taken as close as possible with the kwbimage
+
+# Boot Device : one of
+# spi, sd (the board has no nand neither onenand)
+
+# BOOT_FROMspi

Re: [U-Boot] [PATCH 4/4] Added initial support for PRTLVT2-based boards.

2010-08-19 Thread David Jander
SD1 bootcmd_SD2\0" \
> 
> Indentation by TAB only, please. [Check & fix globally, please.]

Ok.

> > +   "ethaddr=00:00:00:00:00:00\0" \
> > +   "ipaddr=192.168.1.244\0" \
> > +   "serverip=192.168.1.132\0" \
> > +   "nfsserverip=192.168.1.160\0" \
> 
> NAK!!
> 
> We do not allow network parameters in the default environment (and
> especially not broken/incorrect MAC addresses.

Ooops. This wasn't meant to be here :-(
Anyway, I am currently working on implementing access to SPI-NOR-flash, and 
this was there only for convinience while developing on one board. It was 
meant to be removed, though.
Btw, I am also trying to fix the awfully broken mxc_spi driver. The spi_xfer() 
function assumes 32-bit word transfers (because the hardware does it like 
this, and FSL didn't bother adapting the driver to linux/u-boot spi 
conventions), so I need to introduce a second spi_xfer_fsl() function for all 
existing, but broken device drivers that assume fsl-byte-ordering (like the 
PMIC driver) and fix the original spi_xfer(). Is that acceptable?

> > +   "nfsroot=/srv/home/david/Devel/Sandboxes/LEL/XDH/nfsroot\0" \
> > +   "kernel=linux-2.6.31-prtlvt2.uImage\0" \

This also wasn't supposed to be here

> > +#define CONFIG_ARP_TIMEOUT 200UL
> 
> Is this really needed on your hardware?

Honestly I didn't even know what this is for. It's from MX51EVK.

> > +#define CONFIG_ENV_SECT_SIZE(128 * 1024)
> > +#define CONFIG_ENV_SIZECONFIG_ENV_SECT_SIZE
> 
> Please use TABs for vertical alignment.

Excuse this dumb question about u-boot coding-style: Are tabs supposed to be 
used 8-spaces wide?

And I thought my version was much cleaner than the MX51EVK BSP which is 
already in u-boot :-(

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 1/4] MX51: iomux: Added su pport for mxc_iomux_set_input ()

2010-08-20 Thread David Jander

Hi Stefano,

On Friday 20 August 2010 10:10:49 am Stefano Babic wrote: 
> Hi Dave,
> 
> >  arch/arm/cpu/armv7/mx51/iomux.c |8 +++-
> >  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> Probably iut is better you set a more useful comment in your commit.
> Instead of "Added support for mxc_iomux_set_input()", you can explain
> which is the new feature you provide. Something to explain you add a
> utility for the "daisy chain" pins, to control the input path to a
> module when the module can be connected to more as one pin.

Hmmm. I thought it was a trivial and obviously missing function to make 
iomux.c complete. Someone just needed to write it. I didn't think it needed 
any more explaining than that, but I'll do it in the next version of the patch 
set (will take a while).

> The patch is part of a series. However, I can see only the first two
> patches. Is there something missing ? I do not see any relation between
> these two patches, too.

I sent 4 patches and received them all on the mailing-list. Are you sure you 
miss two of them?
The first three patches introduce some minimal fixes/additions in order to 
implement the BSP for PRTLVT2 boards (patch 4/4).

> > +void mxc_iomux_set_input(iomux_input_select_t input, u32 config)
> > +{
> > +   u32 pad_reg = IOMUXSW_INPUT_CTL+(input*4);
> 
> Code styling, you should add spaces:
> 
>   u32 pad_reg = IOMUXSW_INPUT_CTL + (input * 4);

Ok.

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 2/4] MX51: Added missing pin definition

2010-08-20 Thread David Jander
On Friday 20 August 2010 10:12:16 am Stefano Babic wrote:
> David Jander wrote:
> > Signed-off-by: David Jander 
> > ---
> >  arch/arm/include/asm/arch-mx51/mx51_pins.h |1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/arch-mx51/mx51_pins.h
> > b/arch/arm/include/asm/arch-mx51/mx51_pins.h index ca26f41..c443f13
> > 100644
> > --- a/arch/arm/include/asm/arch-mx51/mx51_pins.h
> > +++ b/arch/arm/include/asm/arch-mx51/mx51_pins.h
> > @@ -320,6 +320,7 @@ enum iomux_pins {
> > MX51_PIN_DISP1_DAT22 = _MXC_BUILD_NON_GPIO_PIN(0x324, 0x724),
> > MX51_PIN_DISP1_DAT23 = _MXC_BUILD_NON_GPIO_PIN(0x328, 0x728),
> > MX51_PIN_DI1_PIN3 = _MXC_BUILD_NON_GPIO_PIN(0x32C, 0x72C),
> > +   MX51_PIN_DI1_DISP_CLK = _MXC_BUILD_NON_GPIO_PIN(0, 0x730), /* No MUX
> > register!! */
> 
> Line too long.

Ok, will fix.

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

2010-08-20 Thread David Jander
On Friday 20 August 2010 10:20:11 am Stefano Babic wrote:
> The patch adds support for setting gpios to the
> MX51 processor and change name to the corresponding
> functions for MX31. In this way, it is possible to get rid
> of nasty #ifdef switches related to the processor type.

Argh! I was just writing the exact same code as in this patch :-(
Thanks a lot anyway.

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

2010-08-20 Thread David Jander

Hi Stefano,

On Friday 20 August 2010 10:20:11 am Stefano Babic wrote:
> diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c
> index e15a63c..54af2e3 100644
> --- a/drivers/spi/mxc_spi.c
> +++ b/drivers/spi/mxc_spi.c
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #ifdef CONFIG_MX27
>  /* i.MX27 has a completely wrong register layout and register definitions
>  in the @@ -68,9 +69,6 @@ static unsigned long spi_bases[] = {
>   0x53f84000,
>  };
> 
> -#define OUT  MX31_GPIO_DIRECTION_OUT
> -#define mxc_gpio_direction   mx31_gpio_direction
> -#define mxc_gpio_set mx31_gpio_set
>  #elif defined(CONFIG_MX51)
>  #include 
>  #include 
> @@ -111,13 +109,12 @@ static unsigned long spi_bases[] = {
>   CSPI2_BASE_ADDR,
>   CSPI3_BASE_ADDR,
>  };
> -#define mxc_gpio_direction(gpio, dir)(0)
> -#define mxc_gpio_set(gpio, value){}
> -#define OUT  1

After this change, it seems something else is missing:
GCC somehow removed the following code for i.MX51 without actually compiling 
the arguments to the functions (???), but now it becomes evident this only 
compiles for i.MX31:

void spi_cs_activate(struct spi_slave *slave)
{
struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
if (mxcs->gpio > 0)
mxc_gpio_set(mxcs->gpio, mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL);
}

void spi_cs_deactivate(struct spi_slave *slave)
{
struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
if (mxcs->gpio > 0)
mxc_gpio_set(mxcs->gpio,
  !(mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL));
}

On i.MX51 SSPOL is set in the config register, and per SS individually. 
Therefore, MXC_CSPICTRL_SSPOL isn't defined for i.MX51.

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

2010-08-20 Thread David Jander

Stefano,

On Friday 20 August 2010 12:01:00 pm Stefano Babic wrote:
> > After this change, it seems something else is missing:
> > GCC somehow removed the following code for i.MX51 without actually
> > compiling the arguments to the functions (???), but now it becomes
> > evident this only compiles for i.MX31:
> 
> Understood, in SPI driver. Really I had another patch in my private
> tree, but I have not yet sent. However, as you note, it makes no sense
> to split changes in two patches.
> 
> I will rebase my tree and send V2 version of the pacth, inclusive the
> changes for mxc_spi.c

Ok, thanks!

> > void spi_cs_activate(struct spi_slave *slave)
> > {
> > struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
> > if (mxcs->gpio > 0)
> > mxc_gpio_set(mxcs->gpio, mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL);
> 
> I know, I had already change this code...I store ss_pol in the priivate
> structure to be independent from the processor register.
> 
> I will send the compound patch soon.

Great. I'll wait.
In the meantime I have just done this to get it working:

#ifdef CONFIG_MX31
void spi_cs_activate(struct spi_slave *slave)
{
struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
if (mxcs->gpio > 0)
mxc_gpio_set(mxcs->gpio, mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL);
}

void spi_cs_deactivate(struct spi_slave *slave)
{
struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
if (mxcs->gpio > 0)
mxc_gpio_set(mxcs->gpio,
  !(mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL));
}
#elif defined (CONFIG_MX51)
void spi_cs_activate(struct spi_slave *slave)
{
struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
unsigned int val = mxcs->cfg_reg & 
(1 << (slave->cs + MXC_CSPICON_SSPOL));
if (mxcs->gpio > 0)
mxc_gpio_set(mxcs->gpio, val);
}

void spi_cs_deactivate(struct spi_slave *slave)
{
struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
unsigned int val = !(mxcs->cfg_reg & 
(1 << (slave->cs + MXC_CSPICON_SSPOL)));
if (mxcs->gpio > 0)
mxc_gpio_set(mxcs->gpio, val);
}
#endif

Seems to work, but never mind...

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 4/4] Added initial support for PRTLVT2-based boards.

2010-08-20 Thread David Jander
On Friday 20 August 2010 11:51:18 am Stefano Babic wrote:
> Hi David,
> 
> in addition to Wolfgang's comments:
> > +static u32 system_rev;
> > +struct io_board_ctrl *mx51_io_board;
> 
> Structure is not used, and probably does not match your board. You
> should drop it.

Ok, I was already wondering what this did here I'll just remove it.

> > +#define POUT_HS (PAD_CTL_DRV_HIGH | PAD_CTL_SRE_FAST)
> > +#define POUT_MS (PAD_CTL_DRV_MAX | PAD_CTL_SRE_FAST)
> > +#define POUT_LS (PAD_CTL_DRV_MEDIUM)
> > +#define PIN_HYS (PAD_CTL_HYS_ENABLE)
> > +#define PIN_HYS_PULL (PAD_CTL_HYS_ENABLE | PAD_CTL_PKE_ENABLE |
> > PAD_CTL_PUE_PULL) +#define PIN_HYS_KEEP (PAD_CTL_HYS_ENABLE |
> > PAD_CTL_PKE_ENABLE)
> > +#define PIO_OD  (PIN_HYS_PULL | PAD_CTL_22K_PU |
> > PAD_CTL_ODE_OPENDRAIN_ENABLE | PAD_CTL_DRV_MEDIUM)
> 
> Consider to put these defines in include/asm/arch-mx51/iomux.h. They
> could be usefult for other boards, too.

Hmmm. In that case they should have other names, and one should probably make 
the set complete with all combinations, and I doubt it'll make sense anymore 
then
This subset of possibilities is too specific to our board I fear.
I had already been complaining about some seemingly arbitrary combinations of 
IO-pad settings being used in SoC-driver code in the linux kernel. IMHO, one 
should never choose a specific combination of settings without excactly 
knowing it is electrically correct for that specific pin on that specific 
board... not only because it works, but also from EMC considerations.
I also think, that in theory this would make the kernel board-specific in that 
case... not something one would want I believe. For that reason, I keep 
thinking that correct and thorough IO-pad setup specific for each board should 
be done in the boot-loader never in the kernel.

OTOH, maybe one could come up with some commonly-used combinations... but 
which ones?

> > +   /* Raise the core frequency to 800MHz */
> > +   /* printf("Core at 400 MHz!\n"); */
> > +   /* writel(0x1, &mxc_ccm->cacrr); */
> > +   writel(0x0, &mxc_ccm->cacrr);
> 
> Comment is misleading. Remove what is not needed.

Sorry, I was too lazy to make a configuration option out of this... will fix 
it.

> > +   /* Setup other io's */
> > +   for(t=0; other_io_conf[t].pin>=0; t++) {
> 
> Add spaces when needed.

Ok.

> > +#ifdef BOARD_LATE_INIT
> 
> Probably it is better (and in mx51evk, too) to remove the #ifdef,
> because we need to initialize the pmic, else some functionalities cannot
> work. It is mandatory that power_init is called.

I agree. I had always been "about to remove it" ;-)

> > diff --git a/board/Protonic/prtlvt2/prtlvt2.h
> > b/board/Protonic/prtlvt2/prtlvt2.h +#ifndef
> > __BOARD_FREESCALE_MX51_EVK_H__
> > +#define __BOARD_FREESCALE_MX51_EVK_H__
> > +
> > +#ifndef __ASSEMBLY__
> > +struct io_board_ctrl {
> > +   u16 led_ctrl;   /* 0x00 */
> > +   u16 resv1[0x03];
> > +   u16 sb_stat;/* 0x08 */
> > +   u16 resv2[0x03];
> > +   u16 int_stat;   /* 0x10 */
> > +   u16 resv3[0x07];
> > +   u16 int_rest;   /* 0x20 */
> > +   u16 resv4[0x0B];
> > +   u16 int_mask;   /* 0x38 */
> > +   u16 resv5[0x03];
> > +   u16 id1;/* 0x40 */
> > +   u16 resv6[0x03];
> > +   u16 id2;/* 0x48 */
> > +   u16 resv7[0x03];
> > +   u16 version;/* 0x50 */
> > +   u16 resv8[0x03];
> > +   u16 id3;    /* 0x58 */
> > +   u16 resv9[0x03];
> > +   u16 sw_reset;   /* 0x60 */
> > +};
> > +#endif
> 
> Is this structure really used ? I have not seen in code. Or does it come
> only from mx51evk ? You can remove the whole file, if it is not needed.

No idea what it does. It's copied from MX51EVK. I will try removing it.

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

2010-08-20 Thread David Jander
On Friday 20 August 2010 12:20:25 pm Stefano Babic wrote:
> David Jander wrote:
> > Great. I'll wait.
> > In the meantime I have just done this to get it working:
> >
> > #ifdef CONFIG_MX31
> > void spi_cs_activate(struct spi_slave *slave)
> > {
> > struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
> > if (mxcs->gpio > 0)
> > mxc_gpio_set(mxcs->gpio, mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL);
> > }
> 
> Ok, but one goal I have is to get rid of nasty #ifdef CONFIG_MX*. I
> introduce general gpio functions to make code more common, and I do not
> want to fall back adding processor switches.

Absolutely right. I just posted it as reference for your patch eventually, not 
because I thought it was good that way.

> > Seems to work, but never mind...
> 
> Ok, I will resend my patch, I hope you can give a chance a test it on
> your target.

Will do.
Btw, do you have any idea why spi_xchg_single() hangs while transmitting the 
second word without claiming the bus again?

Also, I don't know if you already fixed mxc_spi.c, to use the correct byte-
ordering when sending u8 buffers. I have a fix, but it is not yet ready.
I essentially renamed spi_xfer() to spi_xfer_fsl(), to be used in the (broken) 
pmic driver, and wrote a new spi_xfer() function which works correcly for u8 
buffers.

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

2010-08-20 Thread David Jander
ntroller, and the endianess is
> consistent. However, it is common for a SPI driver to allocate a "char"
> buffer, and the first byte in the buffer is the first byte sent to the
> SPI bus. This is not the case with mxc_spi.c

I know. I thought to do it in two steps: Fix mxc_spi.c with a workaround for 
the pmic driver (which amounts to '#define spi_xfer spi_xfer_fsl' at the 
beginning of this driver basically) and fix the pmic driver later, since it is 
probably not trivial, and needs to be done carefully (you know, one can smoke 
a board by mistake :-)

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

2010-08-23 Thread David Jander

Hi again,

On Friday 20 August 2010 03:35:57 pm Stefano Babic wrote:
> > Just figured out one big mistake. I was debugging spi_flash.c, and had
> > CONFIG_ENV_IS_IN_SPI_FLASH set. That means, first SPI access is done
> > before malloc is available, and guess what? spi_setup_slave() uses malloc
> > ;-)
> 
> I have CONFIG_ENV_IS_IN_SPI_FLASH set, too.
> I try to figure out how the functions are called, but I do not get the
> same issue. I set with the debugger two breakpoints, one in
> mem_malloc_init and the second one in spi_setup_slave. I see that
> mem_alloc_init is hit first, and when spi_setup_slave is called, malloc
> is available. I get a valid pointer for the private structure. It seems
> there something in our config files that makes the things different. I
> do not yet know why.

Are you sure? In arch/arm/lib/board.c function start_armboot(), init_sequence 
is processed first, which contains env_init() before dram_init() and just 
after completing init_sequence, mem_malloc_init() is called. How can you have 
working malloc then?

> > The C++ comments show original code, the line below is new.
> 
> Understood, if malloc is not called, we have to use static or (better)
> try to call mem_malloc_init() first

I don't know if that is possible. I know that physically RAM is initialized 
before u-boot even starts (it runs from RAM), but logically, dram_init() is 
called _after_ env_init(), so I'm not sure if you are supposed to call 
mem_malloc_init() in env_init()...

> > I see a one byte access followed by a 5 byte access,
> 
> That is correct, you see the code in spi_flash.c. First the ID command
> is sent (0x97), without reading nothing (that is, din is NULL). Then the
> answer is read (dout is NULL), and the id buffer is 5 bytes long.

I am slowly progressing... now the transfer succeeds, but I only read FF ;-)

> > [...]
> I am trying another approach. As the MX51 has 32 bytes FIFO, it makes
> sense to use it and not send a single word, if we can. This must not
> change the behavior for the MX31, because this processor has no FIFO and
> a single word can be sent.
> So I replaced completely spi_xfer, and the logic you put in spi_xfer I
> have (more or less, I have not checked in details) moved inside the
> spi_xcgh:single, that now has the meaning for me as single transation:
> up to 1 word for i.MX31, up to 32 words (128 bytes) for i.MX51.
> 
> Take into account that loading the kernel using a single word takes a
> lot of time..

A good point. I was following the premise that u-boot drivers should be 
simple, but a little bit of speed for booting is surely not a bad idea ;-)

> >> However, I am currently working on several issues for MX51. It should be
> >> nice to know which are your plans to save both some time ;-)
> >
> > Well, I am in a bit of a hurry, and essentially what I need is to be able
> > to access SPI-nor flash (spansion type) for environment and booting
> > linux. MMC/SD access would be nice, but is not yet necessary.
> 
> Ok, quite the same. I have a ST flash, but we get the same problems, I see.

I am just now picking up where I left last week, so give me a few hours and I  
should have something working, I guess.

> > I know. I thought to do it in two steps: Fix mxc_spi.c with a workaround
> > for the pmic driver (which amounts to '#define spi_xfer spi_xfer_fsl' at
> > the beginning of this driver basically) and fix the pmic driver later,
> > since it is probably not trivial, and needs to be done carefully (you
> > know, one can smoke a board by mistake :-)
> 
> I know, this makes funny setting voltages via software

I always say: "Electronics work on smoke. If the smoke escapes, it stops 
working" :-)

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

2010-08-23 Thread David Jander
On Monday 23 August 2010 10:50:53 am David Jander wrote:
> I am just now picking up where I left last week, so give me a few hours and
>  I should have something working, I guess.

Ok, I guess I was pessimistic. There is a weird bug in mxc_spi.c: CPOL is 
negated!
I just saw that in the mx51evk.h header file CONFIG_FSL_PMIC_MODE was set to 
low-active clock (CPOL=1), which is not supposed to work. But it did work, and 
on the scope clock-polarity was active-high.

In spi_cfg(), I saw this line:

if (!(mode & SPI_CPOL))
sclkpol = 1;

AFAIK, this should be:

if (mode & SPI_CPOL)
sclkpol = 1;

At least for the MX51. Can you confirm that this is different on the MX31?

Now I get a correct flash id.

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

2010-08-23 Thread David Jander

Hi Stefano,

On Monday 23 August 2010 12:37:16 pm Stefano Babic wrote:
>[...]
> > In spi_cfg(), I saw this line:
> >
> > if (!(mode & SPI_CPOL))
> > sclkpol = 1;
> >
> > AFAIK, this should be:
> >
> > if (mode & SPI_CPOL)
> > sclkpol = 1;
> >
> > At least for the MX51. Can you confirm that this is different on the
> > MX31?
> 
> Agree. According to the MX51 Manual, the register must be set with:
>   0 Active high polarity (0 = Idle).
>   1 Active low polarity (1 = Idle).

I just checked in the reference manual of the i.MX31, and there the meaning of 
this bit has the same polarity as on the i.MX51, so you'll need to fix this 
also at the end of the spi_setup_slave() function, in the #else path of the 
#ifdef CONFIG_MX51 directive.

if (!(mode & SPI_CPOL))
ctrl_reg |= MXC_CSPICTRL_POL;

should be:

if (mode & SPI_CPOL)
ctrl_reg |= MXC_CSPICTRL_POL;

Would be nice if someone with a MX31 board could verify this.

> So we need to change both CONFIG_FSL_PMIC_MODE in config and in
> mxc_spi.c. Do you send a patch or do you prefer I will do this job ? I
> will add your signed-off-by if you agree.

I agree, and I guess you can better include it in your patch-set, otherwise 
I'd have to wait for your patches and then provide my own patch on top of 
that too complicated :-)

I am also adding support for S25FL032P chips to the spansion driver. Will post 
a patch later.

Right now I have correct detection of the chip, but the environment is not 
saved and read back correctly. Still investigating... maybe some chip 
configuration prolem in the spansion driver?

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] MTD: Add support for S25FL032P spi nor-flash

2010-08-23 Thread David Jander
This patch introduces an extra mask-field in spansion_spi_flash_params
to support flash chips with 1-byte extended ID (like the S25FL032P).

Signed-off-by: David Jander 
---
 drivers/mtd/spi/spansion.c |   19 ++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/spi/spansion.c b/drivers/mtd/spi/spansion.c
index d6c1a5f..14cd6d3 100644
--- a/drivers/mtd/spi/spansion.c
+++ b/drivers/mtd/spi/spansion.c
@@ -52,12 +52,14 @@
 #define SPSN_ID_S25FL128P  0x2018
 #define SPSN_EXT_ID_S25FL128P_256KB0x0300
 #define SPSN_EXT_ID_S25FL128P_64KB 0x0301
+#define SPSN_EXT_ID_S25FL032P  0x4d00
 
 #define SPANSION_SR_WIP(1 << 0)/* Write-in-Progress */
 
 struct spansion_spi_flash_params {
u16 idcode1;
u16 idcode2;
+   u16 idmask2;
u16 page_size;
u16 pages_per_sector;
u16 nr_sectors;
@@ -79,6 +81,7 @@ static const struct spansion_spi_flash_params 
spansion_spi_flash_table[] = {
{
.idcode1 = SPSN_ID_S25FL008A,
.idcode2 = 0,
+   .idmask2 = 0x,
.page_size = 256,
.pages_per_sector = 256,
.nr_sectors = 16,
@@ -87,6 +90,7 @@ static const struct spansion_spi_flash_params 
spansion_spi_flash_table[] = {
{
.idcode1 = SPSN_ID_S25FL016A,
.idcode2 = 0,
+   .idmask2 = 0x,
.page_size = 256,
.pages_per_sector = 256,
.nr_sectors = 32,
@@ -95,6 +99,7 @@ static const struct spansion_spi_flash_params 
spansion_spi_flash_table[] = {
{
.idcode1 = SPSN_ID_S25FL032A,
.idcode2 = 0,
+   .idmask2 = 0x,
.page_size = 256,
.pages_per_sector = 256,
.nr_sectors = 64,
@@ -103,6 +108,7 @@ static const struct spansion_spi_flash_params 
spansion_spi_flash_table[] = {
{
.idcode1 = SPSN_ID_S25FL064A,
.idcode2 = 0,
+   .idmask2 = 0x,
.page_size = 256,
.pages_per_sector = 256,
.nr_sectors = 128,
@@ -111,6 +117,7 @@ static const struct spansion_spi_flash_params 
spansion_spi_flash_table[] = {
{
.idcode1 = SPSN_ID_S25FL128P,
.idcode2 = SPSN_EXT_ID_S25FL128P_64KB,
+   .idmask2 = 0x,
.page_size = 256,
.pages_per_sector = 256,
.nr_sectors = 256,
@@ -119,11 +126,21 @@ static const struct spansion_spi_flash_params 
spansion_spi_flash_table[] = {
{
.idcode1 = SPSN_ID_S25FL128P,
.idcode2 = SPSN_EXT_ID_S25FL128P_256KB,
+   .idmask2 = 0x,
.page_size = 256,
.pages_per_sector = 1024,
.nr_sectors = 64,
.name = "S25FL128P_256K",
},
+   {
+   .idcode1 = SPSN_ID_S25FL032A,
+   .idcode2 = SPSN_EXT_ID_S25FL032P,
+   .idmask2 = 0xff00,
+   .page_size = 256,
+   .pages_per_sector = 256,
+   .nr_sectors = 64,
+   .name = "S25FL032P",
+   },
 };
 
 static int spansion_wait_ready(struct spi_flash *flash, unsigned long 
timeout)
@@ -317,7 +334,7 @@ struct spi_flash *spi_flash_probe_spansion(struct 
spi_slave *spi, u8 *idcode)
for (i = 0; i < ARRAY_SIZE(spansion_spi_flash_table); i++) {
params = &spansion_spi_flash_table[i];
if (params->idcode1 == jedec) {
-   if (params->idcode2 == ext_jedec)
+   if (params->idcode2 == (ext_jedec & params->idmask2))
break;
}
}
-- 
1.6.3.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] MTD: Add support for S25FL032P spi nor-flash

2010-08-23 Thread David Jander
On Monday 23 August 2010 06:31:26 pm Mike Frysinger wrote:
> On Monday, August 23, 2010 09:12:16 David Jander wrote:
> > +   {
> > +   .idcode1 = SPSN_ID_S25FL032A,
> > +   .idcode2 = SPSN_EXT_ID_S25FL032P,
> > +   .idmask2 = 0xff00,
> 
> what does the idcode2 look like such that you need a mask ?
> -mike

According to the datasheet the RDID command (0x9f) returns the following 
bytes:

byte 0: Manufacturer ID = 0x01
byte 1,2: Device ID = 0x02, 0x15 (same as S25FL032A)
byte 3: Extended ID = 0x4d
byte 4,5,6: Spansion reserved (do not use).

byte 4 is read as 0x00 on my part, but due to the above explaination, I cannot 
say for sure it is always the same, so I had to implement a mask to check for 
it.

Best regards,

-- 
David Jander
Protonic Holland.
tel.: +31 (0) 229 212928
fax.: +31 (0) 229 210930
Factorij 36 / 1689 AL Zwaag
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] MTD: Add support for S25FL032P spi nor-flash

2010-08-23 Thread David Jander
This patch introduces an extra mask-field in spansion_spi_flash_params
to support flash chips with 1-byte extended ID (like the S25FL032P).

Signed-off-by: David Jander 
---
 drivers/mtd/spi/spansion.c |   19 ++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/spi/spansion.c b/drivers/mtd/spi/spansion.c
index d6c1a5f..14cd6d3 100644
--- a/drivers/mtd/spi/spansion.c
+++ b/drivers/mtd/spi/spansion.c
@@ -52,12 +52,14 @@
 #define SPSN_ID_S25FL128P  0x2018
 #define SPSN_EXT_ID_S25FL128P_256KB0x0300
 #define SPSN_EXT_ID_S25FL128P_64KB 0x0301
+#define SPSN_EXT_ID_S25FL032P  0x4d00
 
 #define SPANSION_SR_WIP(1 << 0)/* Write-in-Progress */
 
 struct spansion_spi_flash_params {
u16 idcode1;
u16 idcode2;
+   u16 idmask2;
u16 page_size;
u16 pages_per_sector;
u16 nr_sectors;
@@ -79,6 +81,7 @@ static const struct spansion_spi_flash_params 
spansion_spi_flash_table[] = {
{
.idcode1 = SPSN_ID_S25FL008A,
.idcode2 = 0,
+   .idmask2 = 0x,
.page_size = 256,
.pages_per_sector = 256,
.nr_sectors = 16,
@@ -87,6 +90,7 @@ static const struct spansion_spi_flash_params 
spansion_spi_flash_table[] = {
{
.idcode1 = SPSN_ID_S25FL016A,
.idcode2 = 0,
+   .idmask2 = 0x,
.page_size = 256,
.pages_per_sector = 256,
.nr_sectors = 32,
@@ -95,6 +99,7 @@ static const struct spansion_spi_flash_params 
spansion_spi_flash_table[] = {
{
.idcode1 = SPSN_ID_S25FL032A,
.idcode2 = 0,
+   .idmask2 = 0x,
.page_size = 256,
.pages_per_sector = 256,
.nr_sectors = 64,
@@ -103,6 +108,7 @@ static const struct spansion_spi_flash_params 
spansion_spi_flash_table[] = {
{
.idcode1 = SPSN_ID_S25FL064A,
.idcode2 = 0,
+   .idmask2 = 0x,
.page_size = 256,
.pages_per_sector = 256,
.nr_sectors = 128,
@@ -111,6 +117,7 @@ static const struct spansion_spi_flash_params 
spansion_spi_flash_table[] = {
{
.idcode1 = SPSN_ID_S25FL128P,
.idcode2 = SPSN_EXT_ID_S25FL128P_64KB,
+   .idmask2 = 0x,
.page_size = 256,
.pages_per_sector = 256,
.nr_sectors = 256,
@@ -119,11 +126,21 @@ static const struct spansion_spi_flash_params 
spansion_spi_flash_table[] = {
{
.idcode1 = SPSN_ID_S25FL128P,
.idcode2 = SPSN_EXT_ID_S25FL128P_256KB,
+   .idmask2 = 0x,
.page_size = 256,
.pages_per_sector = 1024,
.nr_sectors = 64,
.name = "S25FL128P_256K",
},
+   {
+   .idcode1 = SPSN_ID_S25FL032A,
+   .idcode2 = SPSN_EXT_ID_S25FL032P,
+   .idmask2 = 0xff00,
+   .page_size = 256,
+   .pages_per_sector = 256,
+   .nr_sectors = 64,
+   .name = "S25FL032P",
+   },
 };
 
 static int spansion_wait_ready(struct spi_flash *flash, unsigned long timeout)
@@ -317,7 +334,7 @@ struct spi_flash *spi_flash_probe_spansion(struct spi_slave 
*spi, u8 *idcode)
for (i = 0; i < ARRAY_SIZE(spansion_spi_flash_table); i++) {
params = &spansion_spi_flash_table[i];
if (params->idcode1 == jedec) {
-   if (params->idcode2 == ext_jedec)
+   if (params->idcode2 == (ext_jedec & params->idmask2))
break;
}
}
-- 
1.6.3.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] MTD: Add support for S25FL032P spi nor-flash

2010-08-24 Thread David Jander
On Tuesday 24 August 2010 06:06:26 pm Mike Frysinger wrote:
> On Tuesday, August 24, 2010 02:39:16 David Jander wrote:
> > On Monday 23 August 2010 06:31:26 pm Mike Frysinger wrote:
> > > On Monday, August 23, 2010 09:12:16 David Jander wrote:
> > > > +   {
> > > > +   .idcode1 = SPSN_ID_S25FL032A,
> > > > +   .idcode2 = SPSN_EXT_ID_S25FL032P,
> > > > +   .idmask2 = 0xff00,
> > >
> > > what does the idcode2 look like such that you need a mask ?
> >
> > According to the datasheet the RDID command (0x9f) returns the following
> > bytes:
> >
> > byte 0: Manufacturer ID = 0x01
> > byte 1,2: Device ID = 0x02, 0x15 (same as S25FL032A)
> > byte 3: Extended ID = 0x4d
> > byte 4,5,6: Spansion reserved (do not use).
> >
> > byte 4 is read as 0x00 on my part, but due to the above explaination, I
> > cannot say for sure it is always the same, so I had to implement a mask
> > to check for it.
> 
> i'd rather we delay adding code to support something that may never change.
> so drop the whole idmask2 stuff and wait for it to become an actual problem
> -mike

I agree that chances this ever breaks might seem rather tiny, but if it ever 
does break, waiting for it to happen could trigger a much bigger problem than 
it is to add these few lines; in the worst case, in some distant future, some 
boards will just not work for no apparent reason (if spansion decided to do 
something with byte 4 without notifying), and nobody will remember this 
discussion anymore...

OTOH, you decide. It's ok with me if you want to leave it out. Given the fact 
that you had already accepted this patch, should I send a new version (without 
the mask)?

Best regards,

-- 
David Jander
Protonic Holland.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] MTD: Add support for S25FL032P spi nor-flash

2010-08-25 Thread David Jander
Signed-off-by: David Jander 
---
 drivers/mtd/spi/spansion.c |9 +
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/spi/spansion.c b/drivers/mtd/spi/spansion.c
index d6c1a5f..42cebf6 100644
--- a/drivers/mtd/spi/spansion.c
+++ b/drivers/mtd/spi/spansion.c
@@ -52,6 +52,7 @@
 #define SPSN_ID_S25FL128P  0x2018
 #define SPSN_EXT_ID_S25FL128P_256KB0x0300
 #define SPSN_EXT_ID_S25FL128P_64KB 0x0301
+#define SPSN_EXT_ID_S25FL032P  0x4d00
 
 #define SPANSION_SR_WIP(1 << 0)/* Write-in-Progress */
 
@@ -124,6 +125,14 @@ static const struct spansion_spi_flash_params 
spansion_spi_flash_table[] = {
.nr_sectors = 64,
.name = "S25FL128P_256K",
},
+   {
+   .idcode1 = SPSN_ID_S25FL032A,
+   .idcode2 = SPSN_EXT_ID_S25FL032P,
+   .page_size = 256,
+   .pages_per_sector = 256,
+   .nr_sectors = 64,
+   .name = "S25FL032P",
+   },
 };
 
 static int spansion_wait_ready(struct spi_flash *flash, unsigned long timeout)
-- 
1.6.3.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot