[U-Boot-Users] soft_i2c
Hi,
a small patch for soft I2C to make it more readable and more general - should not break anything.
best regards Christian
In message 20040423153814.B8298@gateway.bln.innominate.local you wrote:
a small patch for soft I2C to make it more readable and more general - should not break anything.
...
@@ -30,9 +30,17 @@ #include <ioports.h> #endif #include <i2c.h> +#include <asm/hardware.h>
Why do you include <asm/hardware.h>? It was not necessary so far, so why should we add it?
#if defined(CONFIG_SOFT_I2C)
+#ifdef CONFIG_MPC8260
- #define DECLARE_I2C_PORT volatile ioport_t *iop = ioport_addr((immap_t *)CFG_IMMR, I2C_PORT);
+#endif +#ifdef CONFIG_8xx
- #define DECLARE_I2C_PORT volatile immap_t *immr = (immap_t *)CFG_IMMR;
+#endif
/* #define DEBUG_I2C */
@@ -75,12 +83,8 @@ */ static void send_reset(void) { -#ifdef CONFIG_MPC8260
- volatile ioport_t *iop = ioport_addr((immap_t *)CFG_IMMR, I2C_PORT);
-#endif -#ifdef CONFIG_8xx
- volatile immap_t *immr = (immap_t *)CFG_IMMR;
-#endif
- DECLARE_I2C_PORT
Do you really think this is more readable and more general?
To me this is not worth the change.
Best regards,
Wolfgang Denk
On Fri, Apr 23, 2004 at 04:07:28PM +0200, Wolfgang Denk wrote:
In message 20040423153814.B8298@gateway.bln.innominate.local you wrote:
+#ifdef CONFIG_MPC8260
> > + #define DECLARE_I2C_PORT volatile ioport_t *iop = ioport_addr((immap_t *)CFG_IMMR, I2C_PORT); > > +#endif
+#ifdef CONFIG_8xx
- #define DECLARE_I2C_PORT volatile immap_t *immr = (immap_t *)CFG_IMMR;
+#endif
/* #define DEBUG_I2C */
@@ -75,12 +83,8 @@ */ static void send_reset(void) { -#ifdef CONFIG_MPC8260
- volatile ioport_t *iop = ioport_addr((immap_t *)CFG_IMMR, I2C_PORT);
-#endif -#ifdef CONFIG_8xx
- volatile immap_t *immr = (immap_t *)CFG_IMMR;
-#endif
- DECLARE_I2C_PORT
Do you really think this is more readable and more general?
Yes, absoluteley - and if _MPC8260 and _8xx declare their port #define DECLARE_I2C_PORT ........ in their board-configuration file the soft_i2c.c is board independend and other boards can easily use it .....
In the old version he has to add 6 times the same #ifdef s This is ugly copy&paste programming in my eyes.
Your mileage my vary...
(the hardware.h is really obsolete....)
To me this is not worth the change.
Best regards,
Wolfgang Denk
-- Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd@denx.de Beware of the Turing Tar-pit in which everything is possible but nothing of interest is easy.
In message 20040423160543.D8298@gateway.bln.innominate.local you wrote:
Do you really think this is more readable and more general?
Yes, absoluteley - and if _MPC8260 and _8xx declare their port #define DECLARE_I2C_PORT ........ in their board-configuration file the soft_i2c.c is board independend and other boards can easily use it .....
But soft_i2c.c is board independent as is. It's even processor inde- pendent as is.
In the old version he has to add 6 times the same #ifdef s This is ugly copy&paste programming in my eyes.
Your mileage my vary...
If we'd move the code in the board config files we'd have to copy it approx. 100 times - that would be even worse. This code is not board dependent.
I see what you mean - but your patch is even less portable as it will break all boards that fail to define DECLARE_I2C_PORT. OK - you can add a #ifdef, but then I really think that there is no difference in readability left. Another option is to use a file-global declaration of the pointers, but that would increas the memopry footprint of U-boot by 264 bytes - after all, I think the benefit of all these changes is epsilon and not worth the effort.
Best regards,
Wolfgang Denk
participants (2)
-
Christian Hohnstaedt -
Wolfgang Denk