Re: [U-Boot-Users] Please pull u-boot-83xx.git (I2C rework)
-----Original Message----- From: u-boot-users-bounces@lists.sourceforge.net [mailto:u-boot-users-bounces@lists.sourceforge.net] On Behalf Of Stefan Roese Sent: 27 November 2006 07:22 To: u-boot-users@lists.sourceforge.net Cc: Kim Phillips; Ben Warren; Wolfgang Denk Subject: Re: [U-Boot-Users] Please pull u-boot-83xx.git (I2C rework)
Hi Ben,
On Monday 27 November 2006 03:45, Ben Warren wrote:
Sorry about the formatting. I've been influenced by many coding 'standards' over the years and haven't quite adapted to the kernel way yet... Old habits die hard.
;-)
To keep things moving, I'll fix this up and forward to Kim on Monday morning. I'm sure he has better things to do than fix my sloppy styling.
Good idea. If I remember correctly, Kim is not available until mid of this week.
Speaking of keeping things moving - I submitted a SPI driver for the MPC834x several months back. I'm sure it has the same formatting problems. Will it be faster for me to resubmit?
Yes. You would save us at least one round of code review & resubmit. It's also easier to have a patch based on the current git tree.
Best regards, Stefan
I think that the I2C_READ and I2C_WRITE #defines in fsl_i2c.h conflicts with soft I2C.
Jocke
Joakim Tjernlund wrote:
I think that the I2C_READ and I2C_WRITE #defines in fsl_i2c.h conflicts with soft I2C.
Can you be more specific? These two macros are defined in a variety of ways in U-Boot. Soft I2C is not used on any Freescale parts (AFAIK).
On Tue, 2006-11-28 at 12:04 -0600, Timur Tabi wrote:
Joakim Tjernlund wrote:
I think that the I2C_READ and I2C_WRITE #defines in fsl_i2c.h conflicts with soft I2C.
Can you be more specific? These two macros are defined in a variety of ways in U-Boot. Soft I2C is not used on any Freescale parts (AFAIK).
While it's not used on any Freescale evaluation boards, it could certainly be implemented on boards with Freescale CPUs. I'm not sure why you'd bit-bang I2C if you have nice hardware controllers, but there may be situations where this makes sense. On the other hand, I don't know if SOFT_I2C and HARD_I2C can co-exist.
Either way, I don't think we should preclude the use of SOFT I2C on Freescale CPUs.
regards, Ben
Ben Warren wrote:
On Tue, 2006-11-28 at 12:04 -0600, Timur Tabi wrote:
Joakim Tjernlund wrote:
I think that the I2C_READ and I2C_WRITE #defines in fsl_i2c.h conflicts with soft I2C.
Can you be more specific? These two macros are defined in a variety of ways in U-Boot. Soft I2C is not used on any Freescale parts (AFAIK).
While it's not used on any Freescale evaluation boards, it could certainly be implemented on boards with Freescale CPUs. I'm not sure why you'd bit-bang I2C if you have nice hardware controllers, but there may be situations where this makes sense. On the other hand, I don't know if SOFT_I2C and HARD_I2C can co-exist.
Either way, I don't think we should preclude the use of SOFT I2C on Freescale CPUs.
regards, Ben
It is used on some boards with Mot/Freescale CPUs, the WindRiver (EST) SBC8260 for example (although configurable hard vs. soft IIRC). The hardware i2c can be more effort than it is worth compared to bit-banging it. Perhaps the new QUICCs are easier to use, perhaps y'all are just smarter than we were back then. :-)
gvb
On 28 Νοε 2006, at 8:17 ΜΜ, Ben Warren wrote:
On Tue, 2006-11-28 at 12:04 -0600, Timur Tabi wrote:
Joakim Tjernlund wrote:
I think that the I2C_READ and I2C_WRITE #defines in fsl_i2c.h conflicts with soft I2C.
Can you be more specific? These two macros are defined in a variety of ways in U-Boot. Soft I2C is not used on any Freescale parts (AFAIK).
While it's not used on any Freescale evaluation boards, it could certainly be implemented on boards with Freescale CPUs. I'm not sure why you'd bit-bang I2C if you have nice hardware controllers, but there may be situations where this makes sense. On the other hand, I don't know if SOFT_I2C and HARD_I2C can co-exist.
Either way, I don't think we should preclude the use of SOFT I2C on Freescale CPUs.
regards, Ben
In the same note, definitely, definitely there are designs with multiple I2C buses. Just because something does not exist on an evaluation board, it doesn't mean it's not used in the real world.
So please make sure that both HW I2C & SW I2C can coexist.
-- Pantelis
Pantelis Antoniou wrote:
In the same note, definitely, definitely there are designs with multiple I2C buses. Just because something does not exist on an evaluation board, it doesn't mean it's not used in the real world.
The new fsl_i2c.c supports multiple HW I2C buses. Well, it supports only 2 buses, but I have plans to fix that.
So please make sure that both HW I2C & SW I2C can coexist.
I'm going to need some help with that. All I'm hearing so far are comments that the two should work together. I don't see how any of the I2C changes I made affect soft I2C. On top of that, I've never used soft I2C, so I don't even know how it's supposed to work.
-----Original Message----- From: Ben Warren [mailto:bwarren@qstreams.com] Sent: den 28 november 2006 19:18 To: Timur Tabi Cc: Joakim Tjernlund; Stefan Roese; u-boot-users@lists.sourceforge.net; Kim Phillips; Wolfgang Denk Subject: Re: [U-Boot-Users] Please pull u-boot-83xx.git (I2C rework)
On Tue, 2006-11-28 at 12:04 -0600, Timur Tabi wrote:
Joakim Tjernlund wrote:
I think that the I2C_READ and I2C_WRITE #defines in
fsl_i2c.h conflicts
with soft I2C.
Can you be more specific? These two macros are defined in
a variety of ways in
U-Boot. Soft I2C is not used on any Freescale parts (AFAIK).
While it's not used on any Freescale evaluation boards, it could certainly be implemented on boards with Freescale CPUs. I'm not sure why you'd bit-bang I2C if you have nice hardware controllers, but there may be situations where this makes sense. On the other hand, I don't know if SOFT_I2C and HARD_I2C can co-exist.
I use it in u-boot to send an I2C reset sequence* which the hardware controller can't do(or so Freescale tells me). Futhermore, one can not use the I2C pins as GPIO on these parts, so I had to connect two pins from PORT D** to the I2C bus to do the bitbanging. If someone knows if this can be done differently, I am all ears.
so I removed I2C_READ/I2C_WRITE from asm-ppc/i2c.h and renamed I2C_READ to I2C_READ_BIT(same for write) locally in cpu/mpc83xx/i2c.c(now renamed to fsl-i2c.c)
* The reset sequence in soft i2c isn't complete. One has to send a start in each clock cycle as well.
** Something isn't working properly with PORTD in ODR mode. I have change direction from output to input for SDA in I2C_TRISTATE. It should be enough to set SDA high since its open drain.
Jocke
Either way, I don't think we should preclude the use of SOFT I2C on Freescale CPUs.
In message 1164737878.31193.38.camel@saruman.qstreams.net you wrote:
Can you be more specific? These two macros are defined in a variety of ways in U-Boot. Soft I2C is not used on any Freescale parts (AFAIK).
While it's not used on any Freescale evaluation boards, it could certainly be implemented on boards with Freescale CPUs. I'm not sure why you'd bit-bang I2C if you have nice hardware controllers, but there
...because the bitbanging code is much smaller and easier to implement and debug than the code that uses the HW controller?
may be situations where this makes sense. On the other hand, I don't know if SOFT_I2C and HARD_I2C can co-exist.
No, theu=y are exclusive. But it should be possible to select any of these interfaces.
Either way, I don't think we should preclude the use of SOFT I2C on Freescale CPUs.
Agreed.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
In message 1164737878.31193.38.camel@saruman.qstreams.net you wrote:
Can you be more specific? These two macros are defined in a variety of ways in U-Boot. Soft I2C is not used on any Freescale parts (AFAIK).
While it's not used on any Freescale evaluation boards, it could certainly be implemented on boards with Freescale CPUs. I'm not sure why you'd bit-bang I2C if you have nice hardware controllers, but there
...because the bitbanging code is much smaller and easier to implement and debug than the code that uses the HW controller?
may be situations where this makes sense. On the other hand, I don't know if SOFT_I2C and HARD_I2C can co-exist.
No, theu=y are exclusive. But it should be possible to select any of these interfaces.
How do we handle the case that there is one hard I2C interface and another soft I2C interface (bus) via a pair of GPIO port pins? I have a PPC405EP based custom board that has such a case and I was looking forward to enabling multiple I2C bus support in U-Boot via both SOFT_I2C as well as HARD_I2C defined.
Tolunay
In message 456CA573.4020103@orkun.us you wrote:
How do we handle the case that there is one hard I2C interface and another soft I2C interface (bus) via a pair of GPIO port pins? I have a PPC405EP based custom board that has such a case and I was looking forward to enabling multiple I2C bus support in U-Boot via both SOFT_I2C as well as HARD_I2C defined.
Good question. I guess you're the first one who wants to do this, so you will have to sort out the problems. I agree that this is a reasonable request, it's just that it has never been attempted before.
Best regards,
Wolfgang Denk
In message 456C7A18.1090309@freescale.com you wrote:
Can you be more specific? These two macros are defined in a variety of ways in U-Boot. Soft I2C is not used on any Freescale parts (AFAIK).
But it should be possible to use it. Please don't use incompatible definitions.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
In message 456C7A18.1090309@freescale.com you wrote:
Can you be more specific? These two macros are defined in a variety of ways in U-Boot. Soft I2C is not used on any Freescale parts (AFAIK).
But it should be possible to use it. Please don't use incompatible definitions.
So you're saying that in MPC8349ITX.h, I should be able to delete this line:
#define CONFIG_HARC_I2C
and replace it with this line:
#define CONFIG_SOFT_I2C
and everything should still work????
On Tue, 2006-11-28 at 15:08 -0600, Timur Tabi wrote:
Wolfgang Denk wrote:
In message 456C7A18.1090309@freescale.com you wrote:
Can you be more specific? These two macros are defined in a variety of ways in U-Boot. Soft I2C is not used on any Freescale parts (AFAIK).
But it should be possible to use it. Please don't use incompatible definitions.
So you're saying that in MPC8349ITX.h, I should be able to delete this line:
#define CONFIG_HARC_I2C
and replace it with this line:
#define CONFIG_SOFT_I2C
and everything should still work????
No, see attached patch(s)
Not tested in your tree as I don't use that one (yet)
Jocke
Joakim Tjernlund wrote:
No, see attached patch(s)
Ah, I see.
Not tested in your tree as I don't use that one (yet)
Git didn't like your patches, for some reason, so I had to apply them by hand, but everything seems to be okay. I will apply them to our tree for Wolfgang's convenience.
-----Original Message----- From: u-boot-users-bounces@lists.sourceforge.net [mailto:u-boot-users-bounces@lists.sourceforge.net] On Behalf Of Timur Tabi Sent: den 28 november 2006 23:16 To: joakim.tjernlund@transmode.se Cc: u-boot-users@lists.sourceforge.net; Stefan Roese; Ben Warren; Wolfgang Denk; Kim Phillips Subject: Re: [U-Boot-Users] Please pull u-boot-83xx.git (I2C rework)
Joakim Tjernlund wrote:
No, see attached patch(s)
Ah, I see.
:)
Not tested in your tree as I don't use that one (yet)
Git didn't like your patches, for some reason, so I had to apply them by hand, but everything seems to be okay. I will apply them to our tree for Wolfgang's convenience.
eeh? I just did them aginst your tree with git-format-patch. Are you using an old git perhaps? I think something changed with git's diff format.
Jocke
Joakim Tjernlund wrote:
eeh? I just did them aginst your tree with git-format-patch. Are you using an old git perhaps? I think something changed with git's diff format.
I do have one patch in my sandbox that's not in the tree, but even so, I can't explain why git-apply and git-am complained.
On Tue, 2006-11-28 at 16:16 -0600, Timur Tabi wrote:
Joakim Tjernlund wrote:
No, see attached patch(s)
Ah, I see.
Not tested in your tree as I don't use that one (yet)
Git didn't like your patches, for some reason, so I had to apply them by hand, but everything seems to be okay. I will apply them to our tree for Wolfgang's convenience.
While I am at it, I would also like to see this in u-boot We use I2C as HRCW since we wan't to haw our flash reset connetced to HRESET, otherwise you might be unable to boot if the flash is in non read array mode when the board resets. We also need to have the version info in the begining of the flash so we can identify what version of u-boot we have installed.
Joakim Tjernlund wrote:
On Tue, 2006-11-28 at 16:16 -0600, Timur Tabi wrote:
Joakim Tjernlund wrote:
No, see attached patch(s)
Ah, I see.
Not tested in your tree as I don't use that one (yet)
Git didn't like your patches, for some reason, so I had to apply them by hand, but everything seems to be okay. I will apply them to our tree for Wolfgang's convenience.
While I am at it, I would also like to see this in u-boot We use I2C as HRCW since we wan't to haw our flash reset connetced to HRESET, otherwise you might be unable to boot if the flash is in non read array mode when the board resets. We also need to have the version info in the begining of the flash so we can identify what version of u-boot we have installed.
From 89b60f21af0d04959d93ccb70fd781c8aba9e66c Mon Sep 17 00:00:00 2001 From: Joakim Tjernlund Joakim.Tjernlund@transmode.se Date: Tue, 28 Nov 2006 23:42:31 +0100 Subject: [PATCH] Make HRCW and version info in data segment configurable.
Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se
cpu/mpc83xx/start.S | 28 +++++++++++++++++----------- 1 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/cpu/mpc83xx/start.S b/cpu/mpc83xx/start.S index 0f27bb6..44bca26 100644 --- a/cpu/mpc83xx/start.S +++ b/cpu/mpc83xx/start.S @@ -77,20 +77,12 @@ END_GOT
/*
- Version string - must be in data segment because MPC83xx uses the
- first 256 bytes for the Hard Reset Configuration Word table (see
- below). Similarly, can't have the U-Boot Magic Number as the first
- thing in the image - don't know how this will affect the image tools,
- but I guess I'll find out soon.
- MPC83xx can use the first 0x40 bytes for the Hard Reset Configuration Word
*/
- table (see below) if so configured.
- .data
- .globl version_string
-version_string:
.ascii U_BOOT_VERSION
.ascii " (", __DATE__, " - ", __TIME__, ")"
.ascii " ", CONFIG_IDENT_STRING, "\0"
.text
+#ifndef CFG_HRCW_IN_I2C_EEPROM #define _HRCW_TABLE_ENTRY(w) \ .fill 8,1,(((w)>>24)&0xff); \ .fill 8,1,(((w)>>16)&0xff); \ @@ -99,7 +91,21 @@ version_string:
_HRCW_TABLE_ENTRY(CFG_HRCW_LOW) _HRCW_TABLE_ENTRY(CFG_HRCW_HIGH) +#endif
+/*
- Version string - May be in data segment if one wants to reserve the
- space left to address 0x100 for future expansion of HRCW bytes.
- */
+#ifdef CFG_VERSION_STRING_IN_DATA
- .data
+#endif
.long 0x27051956 /* U-Boot Magic Number */- .globl version_string
+version_string:
- .ascii U_BOOT_VERSION
- .ascii " (", __DATE__, " - ", __TIME__, ")"
- .ascii " ", CONFIG_IDENT_STRING, "\0"
#ifndef CONFIG_DEFAULT_IMMR #error CONFIG_DEFAULT_IMMR must be defined
Hi Timur,
I don't believe you want to do this: you are removing the HRCW entirely from flash if you have it in I2C (CFG_HRCW_IN_I2C_EEPROM). Unless I'm missing something (possible), I don't see why you even need a configuration option CFG_HRCW_IN_I2C_EEPROM.
* You can chose to ignore the flash-HRCW and use the I2C one simply by pulling a config pin up/down on power up (if I understand the manuals correctly), so you can store your HRCW in both places and chose which you use via a hardware configuration jumper. This is a Very Good Thing[tm]. Suppressing the flash version simply because you chose not to use it is not something _I_ want to do on my 8360.
* If you put your HRCW in I2C _and_ in flash, you will be able to boot via the flash HRCW if your I2C version gets hosed. If you don't, you will be hosed.
* Having a redundant HRCW in flash is essentially free: it costs you 64 bytes of otherwise unused memory (well, other than the version string which you moved there, but there should be room for both).
My suggestion for putting the u-boot version string in a fixed place in flash was to put it _after_ the HRCW (e.g. at offset 0x40). 0x60 (96) bytes should be plenty for the version string. Instead, you've unwisely (IMHO) _replaced_ the HRCW. Browsing some of the other PowerPC CPU types, the ones that can appear to put the version string at the very start of flash. This implies putting the version string in flash at offset 0x40 is OK (I was concerned about relocation fixup issues with pointers to flash vs. RAM after relocation).
On a related topic, the comment at the head of start.S says /* * Version string - must be in data segment because MPC83xx uses the * first 256 bytes for the Hard Reset Configuration Word table (see * below). Similarly, can't have the U-Boot Magic Number as the first * thing in the image - don't know how this will affect the image tools, * but I guess I'll find out soon. */ I see the comment was cut'n'pasted from the 8260 start.S. Actually only the first 64 (0x40) bytes of memory are used for the HRCW, _not_ all 256 bytes.
Looks like I (or somebody) needs to submit a clean up patch for that comment for the 8260 and 8360. Sigh. I just _had_ to look. Curiosity will be the death of me yet. ;-)
gvb
On Tue, 2006-11-28 at 15:08 -0600, Timur Tabi wrote:
Wolfgang Denk wrote:
In message 456C7A18.1090309@freescale.com you wrote:
Can you be more specific? These two macros are defined in a variety of ways in U-Boot. Soft I2C is not used on any Freescale parts (AFAIK).
But it should be possible to use it. Please don't use incompatible definitions.
So you're saying that in MPC8349ITX.h, I should be able to delete this line:
#define CONFIG_HARC_I2C
and replace it with this line:
#define CONFIG_SOFT_I2C
and everything should still work????
No. To implement Soft I2C (bit-banging) you need to provide information about the I/O that you're using, among other things.
regards, Ben
In message 456CA564.8060509@freescale.com you wrote:
But it should be possible to use it. Please don't use incompatible definitions.
So you're saying that in MPC8349ITX.h, I should be able to delete this line:
#define CONFIG_HARC_I2C
and replace it with this line:
#define CONFIG_SOFT_I2C
and everything should still work????
Yes - assuming you provide the required definitions for port pin initialization and access in you rboard config file.
Best regards,
Wolfgang Denk
participants (7)
-
Ben Warren -
Jerry Van Baren -
Joakim Tjernlund -
Pantelis Antoniou -
Timur Tabi -
Tolunay Orkun -
Wolfgang Denk