[U-Boot] [PATCH 0/2] arm nomadik: gpio and i2c

[Daniel Gorsulowski]

Hello Heiko,

Heiko Schocher wrote:
> Hello Alessandro,
> 
> Alessandro Rubini wrote:
>> This adds gpio and i2c support for the Nomadik evaluation kit. They
>> are needed to turn on the LCD backlight in order to later add LCD
>> support.
>>
>> I have one doubt and some questions on gpio:
>>
>> To use soft_i2c I need to define some macros in the config file.
>> Instead of writing hard numbers there I called the gpio functions, but
>> the config file is inluded from asm sources as well.  I don't think my
>> approach is beautiful at all (both #ifndef __ASSEMBLY__ and #include
>> "../board/"), but I didn't find a better solution.
> 
> Yes, thats a problem ... if we had a GPIO Framework it would be solved
> by including gpio.h ...
> 
> Or, maybe, we can make a soft_i2c.h which which gets only included
> if saying CONFIG_I2C_SOFT_INCLUDE is defined. soft_i2c.h defines for
> example:
> 
> #define I2C_SDA(x)	i2c_soft_sda(bit)
> 
> void i2c_soft_sda(int pin);
> 
> and you can define this function i2c_soft_sda(int pin) in your board
> specific code ... maybe a cleaner option?
> 
> to speak in c, I tried the following patch on the suen3 plattform,
> where I have actually a similiar problem, and this worked fine :-)
> 
> [PATCH] i2c, soft: added soft_i2c.h
> 
> In case you must define functions for the I2C_XXX
> defines, it is necessary to have a soft_i2c.h, which
> defines functions for this macros. This functions can
> then be programmed in board specific code. To activate
> this it must be CONFIG_I2C_SOFT_INCLUDE defined in the
> board config file.
> 
> Signed-off-by: Heiko Schocher <hs at denx.de>
> ---
>  drivers/i2c/soft_i2c.c |    3 +++
>  include/soft_i2c.h     |   16 ++++++++++++++++
>  2 files changed, 19 insertions(+), 0 deletions(-)
>  create mode 100644 include/soft_i2c.h
> 
> diff --git a/drivers/i2c/soft_i2c.c b/drivers/i2c/soft_i2c.c
> index 59883a5..30e24f3 100644
> --- a/drivers/i2c/soft_i2c.c
> +++ b/drivers/i2c/soft_i2c.c
> @@ -43,6 +43,9 @@
>  #if defined(CONFIG_MPC852T) || defined(CONFIG_MPC866)
>  #include <asm/io.h>
>  #endif
> +#if defined(CONFIG_SOFT_I2C_INCLUDE)
> +#include <soft_i2c.h>
> +#endif
>  #include <i2c.h>
> 
>  /* #define	DEBUG_I2C	*/
> diff --git a/include/soft_i2c.h b/include/soft_i2c.h
> new file mode 100644
> index 0000000..39f9a35
> --- /dev/null
> +++ b/include/soft_i2c.h
> @@ -0,0 +1,16 @@
> +#ifndef _CONFIG_SOFT_I2C
> +#define _CONFIG_SOFT_I2C
> +#define I2C_ACTIVE	i2c_soft_active();
> +#define I2C_TRISTATE	i2c_soft_tristate();
> +#define I2C_READ	i2c_soft_read();
> +#define I2C_SDA(bit)	i2c_soft_sda(bit);
> +#define I2C_SCL(bit)	i2c_soft_scl(bit);
> +#define	I2C_DELAY	i2c_soft_delay();
> +
> +void	i2c_soft_active(void);
> +void	i2c_soft_tristate(void);
> +int	i2c_soft_read(void);
> +void	i2c_soft_sda(int value);
> +void	i2c_soft_scl(int value);
> +void	i2c_soft_delay(void);
> +#endif
> -- 1.6.0.GIT Maybe you can try it too?
> > I would like to add a gpio command, and I've found no generic gpio
> > stuff.  Only one board (cm-bf527) has a gpio commands, but quite a few
> > have similar commands to set leds or other bits.  Is time ripe for a
> > generic gpio driver with board-specific limits and operations? Would
> > that be interesting for u-boot-next?  Should I process with a board-specific
> > gpio command by now?
>
> I vote for making a gpio framework, but that will take a while I think ...
> Hmm.. maybe we use my proposal for such a soft_i2c.h, so I think, it is
> okay for such a board specific gpio (unless we have a gpio framework).
>
> bye
> Heiko

I tried your patch on AT91SAM9263EK based OTC-570 board (patch will come to ML
within next merge window). It worked fine on it.
But some boards (the OTC-570 too) need a i2c_soft_init() function. So I extended
your patch as follows:

[PATCH] i2c, soft: added soft_i2c.h

In case you must define functions for the I2C_XXX
defines, it is necessary to have a soft_i2c.h, which
defines functions for this macros. This functions can
then be programmed in board specific code. To activate
this it must be CONFIG_I2C_SOFT_INCLUDE defined in the
board config file.

Signed-off-by: Heiko Schocher <hs at denx.de>
---
 drivers/i2c/soft_i2c.c |    3 +++
 include/soft_i2c.h     |   20 ++++++++++++++++++++
 2 files changed, 23 insertions(+), 0 deletions(-)
 create mode 100644 include/soft_i2c.h

diff --git a/drivers/i2c/soft_i2c.c b/drivers/i2c/soft_i2c.c
index 59883a5..30e24f3 100644
--- a/drivers/i2c/soft_i2c.c
+++ b/drivers/i2c/soft_i2c.c
@@ -43,6 +43,9 @@
 #if defined(CONFIG_MPC852T) || defined(CONFIG_MPC866)
 #include <asm/io.h>
 #endif
+#if defined(CONFIG_SOFT_I2C_INCLUDE)
+#include <soft_i2c.h>
+#endif
 #include <i2c.h>

 /* #define	DEBUG_I2C	*/
diff --git a/include/soft_i2c.h b/include/soft_i2c.h
new file mode 100644
index 0000000..c1b770d
--- /dev/null
+++ b/include/soft_i2c.h
@@ -0,0 +1,20 @@
+#ifndef _CONFIG_SOFT_I2C
+#define _CONFIG_SOFT_I2C
+#ifdef CONFIG_SOFT_I2C_INIT
+#define I2C_INIT	i2c_soft_init()
+#endif
+#define I2C_ACTIVE	i2c_soft_active()
+#define I2C_TRISTATE	i2c_soft_tristate()
+#define I2C_READ	i2c_soft_read()
+#define I2C_SDA(bit)	i2c_soft_sda(bit)
+#define I2C_SCL(bit)	i2c_soft_scl(bit)
+#define	I2C_DELAY	i2c_soft_delay()
+
+void	i2c_soft_init(void);
+void	i2c_soft_active(void);
+void	i2c_soft_tristate(void);
+int	i2c_soft_read(void);
+void	i2c_soft_sda(int value);
+void	i2c_soft_scl(int value);
+void	i2c_soft_delay(void);
+#endif
-- 
1.5.3

Moreover I removed the semicolons at the end of the #define lines,
they are not needed.
Maybe you apply these changes to your I2C tree?
(Or is there a better solution in the meantime?)

Bye
Daniel