DaVinci/i2c: allow multiple buses

Hi Tom,

I'm requesting comments on the following (untested) patch.
It adds support for multiple i2c buses on davinci_i2c, without altering one line of code. What I don't like is that I'm doing a bit of macro black magic to transform constants into variables. But I don't know whether refactoring the code would be accepted, so this is a first shot.

Regards,

--
cJ

commit 53d99d331dc7fc4bf3384a59bbca564bf1b85f6b
Author: Jerome Carretero <[email protected]>
Date: ? Thu Dec 22 00:26:13 2011 -0500

? ?DaVinci/i2c: Support for multiple buses

arch/arm/include/asm/arch-davinci/i2c_defs.h +0 -0

index 24cd268..1d24fbf 100644
@@ -25,13 +25,18 @@

?#ifndef _DAVINCI_I2C_H_
?#define _DAVINCI_I2C_H_

arch/arm/include/asm/arch-davinci/i2c_defs.h +2 -0

+#include <asm/arch/hardware.h>
+

?#define I2C_WRITE ? ? ? ? ? ? ?0
?#define I2C_READ ? ? ? ? ? ? ? 1

?#ifndef CONFIG_SOC_DA8XX

arch/arm/include/asm/arch-davinci/i2c_defs.h +2 -1

-#define I2C_BASE ? ? ? ? ? ? ? 0x01c21000
+# define I2C_BASE ? ? ? ? ? ? ?DAVINCI_I2C_BASE
+# define I2C_DEFAULT_BASE I2C_BASE

?#else

arch/arm/include/asm/arch-davinci/i2c_defs.h +3 -1

-#define I2C_BASE ? ? ? ? ? ? ? 0x01c22000
+# define I2C_BASE0 ? ? ? ? ? ? DAVINCI_I2C0_BASE
+# define I2C_BASE1 ? ? ? ? ? ? DAVINCI_I2C1_BASE
+# define I2C_DEFAULT_BASE I2C_BASE0

?#endif

?#define ? ? ? ?I2C_OA ? ? ? ? ? ? ? ? ?(I2C_BASE + 0x00)

drivers/i2c/davinci_i2c.c +0 -0

index 2abddfb..be7fb7a 100644
@@ -29,6 +29,44 @@

?#include <asm/arch/hardware.h>
?#include <asm/arch/i2c_defs.h>

drivers/i2c/davinci_i2c.c +38 -0

+#if defined(CONFIG_I2C_MULTI_BUS)
+
+static unsigned int bus_initialized[I2C_BUS_MAX];
+static unsigned int current_bus;
+static unsigned long i2c_base = I2C_DEFAULT_BASE;
+
+#ifndef I2C_BASE
+# define I2C_BASE i2c_base
+#else
+# error CONFIG_I2C_MULTI_BUS *and* I2C_BASE defined !
+#endif
+
+int i2c_set_bus_num(unsigned int bus)
+{
+ ? ? ? if ((bus < 0) || (bus >= I2C_BUS_MAX)) { // TODO where to put I2C_BUS_MAX ?
+ ? ? ? ? ? ? ? printf("Bad bus: %d\n", bus);
+ ? ? ? ? ? ? ? return -1;
+ ? ? ? }
+
+ ? ? ? if (bus == 1)
+ ? ? ? ? ? ? ? i2c_base = I2C_BASE1;
+ ? ? ? else
+ ? ? ? ? ? ? ? i2c_base = I2C_BASE0;
+
+ ? ? ? current_bus = bus;
+
+ ? ? ? if (!bus_initialized[current_bus])
+ ? ? ? ? ? ? ? i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
+ ? ? ? ? ? ? ? return 0;
+}
+
+unsigned int i2c_get_bus_num(void)
+{
+ ? ? ? return current_bus;
+}
+
+#endif
+

?#define CHECK_NACK() \
? ? ? ?do {\
? ? ? ? ? ? ? ?if (tmp & (I2C_TIMEOUT | I2C_STAT_NACK)) {\

Quoted 6 lines · J?r?me Carretero

Hi Tom,

I'm requesting comments on the following (untested) patch.
It adds support for multiple i2c buses on davinci_i2c, without altering one line of code. What I don't like is that I'm doing a bit of macro black magic to transform constants into variables. But I don't know whether refactoring the code would be accepted, so this is a first shot.

So, this follows the example of other i2c drivers, but as Wolfgang likes to point out, following another bad example isn't a valid reason to do something. So, is this the right way or a bad example to follow, Wolfgang or Heiko? Thanks!

Yes, that is the actual (not really nice) state to implement the "multibus" feature for i2c. The better way would be to go this way:

http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=shortlog;h=refs/heads/multibus_v2_20111112

but this branch is not up to date, some test on at last powerpc and arm architectures should be done, and there are a lof of checkpatch errors in this branch (any help is welcome ;-) ... so there is some work to do, before this can go to mainline ...