* [U-Boot] [RFC] [PATCH] DaVinci/i2c: allow multiple buses
@ 2011-12-22 5:36 Jérôme Carretero
2012-01-05 19:46 ` Tom Rini
0 siblings, 1 reply; 4+ messages in thread
From: Jérôme Carretero @ 2011-12-22 5:36 UTC (permalink / raw)
To: u-boot
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 <cJ-uboot@zougloub.eu>
Date: Thu Dec 22 00:26:13 2011 -0500
DaVinci/i2c: Support for multiple buses
diff --git a/arch/arm/include/asm/arch-davinci/i2c_defs.h b/arch/arm/include/asm/arch-davinci/i2c_defs.h
index 24cd268..1d24fbf 100644
--- a/arch/arm/include/asm/arch-davinci/i2c_defs.h
+++ b/arch/arm/include/asm/arch-davinci/i2c_defs.h
@@ -25,13 +25,18 @@
#ifndef _DAVINCI_I2C_H_
#define _DAVINCI_I2C_H_
+#include <asm/arch/hardware.h>
+
#define I2C_WRITE 0
#define I2C_READ 1
#ifndef CONFIG_SOC_DA8XX
-#define I2C_BASE 0x01c21000
+# define I2C_BASE DAVINCI_I2C_BASE
+# define I2C_DEFAULT_BASE I2C_BASE
#else
-#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)
diff --git a/drivers/i2c/davinci_i2c.c b/drivers/i2c/davinci_i2c.c
index 2abddfb..be7fb7a 100644
--- a/drivers/i2c/davinci_i2c.c
+++ b/drivers/i2c/davinci_i2c.c
@@ -29,6 +29,44 @@
#include <asm/arch/hardware.h>
#include <asm/arch/i2c_defs.h>
+#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)) {\
^ permalink raw reply related [flat|nested] 4+ messages in thread* [U-Boot] [RFC] [PATCH] DaVinci/i2c: allow multiple buses
2011-12-22 5:36 [U-Boot] [RFC] [PATCH] DaVinci/i2c: allow multiple buses Jérôme Carretero
@ 2012-01-05 19:46 ` Tom Rini
2012-01-11 6:52 ` Heiko Schocher
0 siblings, 1 reply; 4+ messages in thread
From: Tom Rini @ 2012-01-05 19:46 UTC (permalink / raw)
To: u-boot
On Wed, Dec 21, 2011 at 10:36 PM, J?r?me Carretero <cJ-uboot@zougloub.eu> wrote:
> 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!
>
> Regards,
>
> --
> cJ
>
>
> commit 53d99d331dc7fc4bf3384a59bbca564bf1b85f6b
> Author: Jerome Carretero <cJ-uboot@zougloub.eu>
> Date: ? Thu Dec 22 00:26:13 2011 -0500
>
> ? ?DaVinci/i2c: Support for multiple buses
>
> diff --git a/arch/arm/include/asm/arch-davinci/i2c_defs.h b/arch/arm/include/asm/arch-davinci/i2c_defs.h
> index 24cd268..1d24fbf 100644
> --- a/arch/arm/include/asm/arch-davinci/i2c_defs.h
> +++ b/arch/arm/include/asm/arch-davinci/i2c_defs.h
> @@ -25,13 +25,18 @@
> ?#ifndef _DAVINCI_I2C_H_
> ?#define _DAVINCI_I2C_H_
>
> +#include <asm/arch/hardware.h>
> +
> ?#define I2C_WRITE ? ? ? ? ? ? ?0
> ?#define I2C_READ ? ? ? ? ? ? ? 1
>
> ?#ifndef CONFIG_SOC_DA8XX
> -#define I2C_BASE ? ? ? ? ? ? ? 0x01c21000
> +# define I2C_BASE ? ? ? ? ? ? ?DAVINCI_I2C_BASE
> +# define I2C_DEFAULT_BASE I2C_BASE
> ?#else
> -#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)
> diff --git a/drivers/i2c/davinci_i2c.c b/drivers/i2c/davinci_i2c.c
> index 2abddfb..be7fb7a 100644
> --- a/drivers/i2c/davinci_i2c.c
> +++ b/drivers/i2c/davinci_i2c.c
> @@ -29,6 +29,44 @@
> ?#include <asm/arch/hardware.h>
> ?#include <asm/arch/i2c_defs.h>
>
> +#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)) {\
>
--
Tom
^ permalink raw reply [flat|nested] 4+ messages in thread* [U-Boot] [RFC] [PATCH] DaVinci/i2c: allow multiple buses
2012-01-05 19:46 ` Tom Rini
@ 2012-01-11 6:52 ` Heiko Schocher
2012-01-11 13:30 ` Jérôme Carretero
0 siblings, 1 reply; 4+ messages in thread
From: Heiko Schocher @ 2012-01-11 6:52 UTC (permalink / raw)
To: u-boot
Hello Tom, J?r?me,
Sorry for the late reply, but I just starting to look through my
backlog, after my vacation ...
Tom Rini wrote:
> On Wed, Dec 21, 2011 at 10:36 PM, J?r?me Carretero <cJ-uboot@zougloub.eu> wrote:
>> 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 ...
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-01-11 13:30 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-22 5:36 [U-Boot] [RFC] [PATCH] DaVinci/i2c: allow multiple buses Jérôme Carretero
2012-01-05 19:46 ` Tom Rini
2012-01-11 6:52 ` Heiko Schocher
2012-01-11 13:30 ` Jérôme Carretero
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox