[U-Boot] [PATCH 1/3] dm: i2c soft: enable driver model for software i2c driver

[Przemyslaw Marczak <p.marczak@samsung.com>]

Hello,

On 03/24/2015 07:01 AM, Heiko Schocher wrote:
> Hello Simon, Przemyslaw,
>
> Am 24.03.2015 00:38, schrieb Simon Glass:
>> Hi Przemyslaw,
>>
>> On 10 March 2015 at 04:30, Przemyslaw Marczak <p.marczak@samsung.com>
>> wrote:
>>> This change adds driver model support to software emulated
>>> i2c bus driver. To bind the driver, proper device-tree node
>>> must be defined, with the following attributes:
>>> - alias: to keep proper bus order
>>> - compatible: 'soft-i2c'
>>> - clock-frequency [Hz]
>>> - clock-pin, data-pin: gpio phandles
>>>
>>> /* Define the alias number to keep bus numbering order */
>>> aliases {
>>>          [...]
>>>          i2c5 = "/i2c at 13500000";
>>>          i2c6 = "/soft-i2c at 1";
>>>          [...]
>>> };
>>>
>>> /* Define the basic bus attributes */
>>> soft-i2c at 1 {
>>>          #address-cells = <1>;
>>>          #size-cells = <0>;
>>>          compatible = "soft-i2c";
>>>          clock-frequency = <50000>;
>>>
>>>          /* Define the proper GPIO pins */
>>>          clock-pin = <&gpa0 0 GPIO_ACTIVE_HIGH>;
>>>          data-pin = <&gpa0 1 GPIO_ACTIVE_HIGH>;
>>>
>>>          /* Optionally, define some driver node (bus child) */
>>>          somedev at 0x44 {
>>>                  compatible = "somedev";
>>>                  reg = <0x44>;
>>>                  [...]
>>>          };
>>> };
>>>
>>> The device can be accessed by the i2c command:
>>>   i2c dev 8                   (bus number set by alias)
>>>   i2c probe <0x44>            (address is optionally)
>>>   i2c md 0x44 0x0             (dump dev registers at address 0x0)
>>>   Valid chip addresses: 0x44  (success!)
>>>   ...
>>>
>>> The previous driver functionality stays unchanged. Driving the bus lines
>>> is done by dm gpio calls in the preprocessor macros. Each, can be
>>> redefined
>>> by the user as previous.
>>>
>>> Tested on Trats2 and Odroid U3.
>>
>> Is the intention to retire the old (non-driver-model) code? What
>> boards use it?
>
> I think not ... Hmm ... maybe you move the new code into
> a new file?
>
Ok, I will do this for v2.

>>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>>> Cc: Masahiro Yamada <yamada.m@jp.panasonic.com>
>>> Cc: Mike Frysinger <vapier@gentoo.org>
>>> Cc: Simon Glass <sjg@chromium.org>
>>> Cc: Heiko Schocher <hs@denx.de>
>
> Hups... seems I missed this patches ...
>
>>> ---
>>>   drivers/i2c/Makefile   |   1 +
>>>   drivers/i2c/soft_i2c.c | 410
>>> +++++++++++++++++++++++++++++++++++++++++++++++--
>>>   2 files changed, 400 insertions(+), 11 deletions(-)
>
> Thanks for this work!
>
>>> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
>>> index 774bc94..7039b6d 100644
>>> --- a/drivers/i2c/Makefile
>>> +++ b/drivers/i2c/Makefile
>>> @@ -6,6 +6,7 @@
>>>   #
>>>   obj-$(CONFIG_DM_I2C) += i2c-uclass.o
>>>   obj-$(CONFIG_DM_I2C_COMPAT) += i2c-uclass-compat.o
>>> +obj-$(CONFIG_DM_I2C_SOFT) += soft_i2c.o
>>>
>>>   obj-$(CONFIG_SYS_I2C_ADI) += adi_i2c.o
>>>   obj-$(CONFIG_I2C_MV) += mv_i2c.o
>>> diff --git a/drivers/i2c/soft_i2c.c b/drivers/i2c/soft_i2c.c
>>> index db9b402..7afae0b 100644
>>> --- a/drivers/i2c/soft_i2c.c
>>> +++ b/drivers/i2c/soft_i2c.c
>>> @@ -1,4 +1,7 @@
>>>   /*
>>> + * (C) Copyright 2015, Samsung Electronics
>>> + * Przemyslaw Marczak <p.marczak@samsung.com>
>>> + *
>>>    * (C) Copyright 2009
>>>    * Heiko Schocher, DENX Software Engineering, hs at denx.de.
>>>    * Changes for multibus/multiadapter I2C support.
>>> @@ -14,6 +17,11 @@
>>>    */
>>>
>>>   #include <common.h>
>>> +#include <errno.h>
>>> +#include <dm.h>
>>> +#include <i2c.h>
>>> +#include <div64.h>
>>> +#include <asm/gpio.h>
>>>   #ifdef CONFIG_MPC8260                  /* only valid for MPC8260 */
>>>   #include <ioports.h>
>>>   #include <asm/io.h>
>>> @@ -32,11 +40,9 @@
>>>   #if defined(CONFIG_MPC852T) || defined(CONFIG_MPC866)
>>>   #include <asm/io.h>
>>>   #endif
>>> -#include <i2c.h>
>>>
>>> +#if defined(CONFIG_SYS_I2C)
>>>   #if defined(CONFIG_SOFT_I2C_GPIO_SCL)
>>> -# include <asm/gpio.h>
>>> -
>>>   # ifndef I2C_GPIO_SYNC
>>>   #  define I2C_GPIO_SYNC
>>>   # endif
>>> @@ -85,6 +91,7 @@
>>>   # endif
>>>
>>>   #endif
>>> +#endif
>>>
>>>   /* #define     DEBUG_I2C       */
>>>
>>> @@ -109,6 +116,65 @@ DECLARE_GLOBAL_DATA_PTR;
>>>   #define CONFIG_SYS_I2C_SOFT_SLAVE CONFIG_SYS_I2C_SLAVE
>>>   #endif
>>>
>>> +/* DM SOFT I2C */
>>> +#ifdef CONFIG_DM_I2C_SOFT
>>> +# ifndef I2C_GPIO_SYNC
>>> +#  define I2C_GPIO_SYNC(gpio)
>>> +# endif
>>> +
>>> +# ifndef I2C_INIT
>>> +#  define I2C_INIT(scl, sda)  \
>>> +       do { } while (0)
>>> +# endif
>>> +
>>> +# ifndef I2C_ACTIVE
>>> +#  define I2C_ACTIVE(sda) \
>>> +       do { } while (0)
>>> +# endif
>>> +
>>> +# ifndef I2C_TRISTATE
>>> +#  define I2C_TRISTATE(sda) \
>>> +       do { } while (0)
>>> +# endif
>>> +
>>> +# ifndef I2C_READ
>>> +#  define I2C_READ(sda) dm_gpio_get_value(sda);
>>> +# endif
>>> +
>>> +# ifndef I2C_SDA
>>> +#  define I2C_SDA(sda, bit) \
>>> +       do { \
>>> +               if (bit) { \
>>> +                       sda->flags &= ~GPIOD_IS_OUT; \
>>> +                       sda->flags |= GPIOD_IS_IN; \
>>> +                       dm_gpio_set_dir(sda); \
>>> +               } else { \
>>> +                       sda->flags &= ~GPIOD_IS_IN; \
>>> +                       sda->flags |= GPIOD_IS_OUT; \
>>> +                       dm_gpio_set_dir(sda); \
>>> +                       dm_gpio_set_value(sda, 0); \
>>> +               } \
>>> +               I2C_GPIO_SYNC(sda); \
>>> +       } while (0)
>>> +# endif
>>> +
>>> +# ifndef I2C_SCL
>>> +#  define I2C_SCL(scl, bit) \
>>> +       do { \
>>> +               scl->flags &= ~GPIOD_IS_IN; \
>>> +               scl->flags |= GPIOD_IS_OUT; \
>>> +               dm_gpio_set_dir(scl); \
>>> +               dm_gpio_set_value(scl, bit); \
>>> +               I2C_GPIO_SYNC(scl); \
>>> +       } while (0)
>>> +# endif
>>> +
>>> +# ifndef I2C_DELAY
>>> +#  define I2C_DELAY(us) udelay(us)     /* 1/4 I2C clock duration */
>>> +# endif
>>> +
>>> +#endif /* CONFIG_DM_I2C_SOFT */
>>> +
>>>
>>> /*-----------------------------------------------------------------------
>>>
>>>    * Definitions
>>>    */
>>> @@ -117,7 +183,6 @@ DECLARE_GLOBAL_DATA_PTR;
>>>   #define I2C_ACK                0               /* PD_SDA level to
>>> ack a byte */
>>>   #define I2C_NOACK      1               /* PD_SDA level to noack a
>>> byte */
>>>
>>> -
>>>   #ifdef DEBUG_I2C
>>>   #define PRINTD(fmt,args...)    do {    \
>>>                  printf (fmt ,##args);   \
>>> @@ -129,21 +194,30 @@ DECLARE_GLOBAL_DATA_PTR;
>>>
>>> /*-----------------------------------------------------------------------
>>>
>>>    * Local functions
>>>    */
>>> +#ifdef CONFIG_SYS_I2C
>>>   #if !defined(CONFIG_SYS_I2C_INIT_BOARD)
>>> -static void  send_reset        (void);
>>> +static void send_reset(void);
>>> +#endif
>>> +static void send_start(void);
>>> +static void send_stop(void);
>>> +static void send_ack(int);
>>> +static int write_byte(uchar byte);
>>> +static uchar read_byte(int);
>>> +#else
>>> +static void send_reset(struct gpio_desc *, struct gpio_desc *, int);
>>> +static void send_start(struct gpio_desc *, struct gpio_desc *, int);
>>> +static void send_stop(struct gpio_desc *, struct gpio_desc *, int);
>>> +static void send_ack(struct gpio_desc *, struct gpio_desc *, int, int);
>>> +static int write_byte(struct gpio_desc *, struct gpio_desc *, int,
>>> uchar);
>>> +static uchar read_byte(struct gpio_desc *, struct gpio_desc *, int,
>>> int);
>>>   #endif
>>> -static void  send_start        (void);
>>> -static void  send_stop (void);
>>> -static void  send_ack  (int);
>>> -static int   write_byte        (uchar byte);
>>> -static uchar read_byte (int);
>>> -
>>>   #if !defined(CONFIG_SYS_I2C_INIT_BOARD)
>>>
>>> /*-----------------------------------------------------------------------
>>>
>>>    * Send a reset sequence consisting of 9 clocks with the data
>>> signal high
>>>    * to clock any confused device back into an idle state.  Also send a
>>>    * <stop> at the end of the sequence for belts & suspenders.
>>>    */
>>> +#ifdef CONFIG_SYS_I2C_SOFT
>>>   static void send_reset(void)
>>>   {
>>>          I2C_SOFT_DECLARATIONS   /* intentional without ';' */
>>> @@ -166,11 +240,36 @@ static void send_reset(void)
>>>          send_stop();
>>>          I2C_TRISTATE;
>>>   }
>>> +#else
>>> +static void send_reset(struct gpio_desc *scl, struct gpio_desc *sda,
>>> int delay)
>>> +{
>>> +       I2C_SOFT_DECLARATIONS   /* intentional without ';' */
>>> +       int j;
>>> +
>>> +       I2C_SCL(scl, 1);
>>> +       I2C_SDA(sda, 1);
>>> +#ifdef I2C_INIT
>>> +       I2C_INIT(scl, sda);
>>> +#endif
>>> +       I2C_TRISTATE(sda);
>>> +       for (j = 0; j < 9; j++) {
>>> +               I2C_SCL(scl, 0);
>>> +               I2C_DELAY(delay);
>>> +               I2C_DELAY(delay);
>>> +               I2C_SCL(scl, 1);
>>> +               I2C_DELAY(delay);
>>> +               I2C_DELAY(delay);
>>> +       }
>>> +       send_stop(scl, sda, delay);
>>> +       I2C_TRISTATE(sda);
>>
>> For the new code I would much prefer that these become functions
>> rather than macros. What is the benefit of using a macro?
>
> I do not know the real reason, but I think it comes from powerpc
> times, where we had not yet such powerful SoCs and it saved some
> instructions ... I tend to say, lets move the dm part into a new
> file ... and let it us do like linux ...
>
> @Simon: Do you pick up this patches through DM tree, or should
> I pick them up?
>
> bye,
> Heiko

Yes, I will make the new file.


Best regards,
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com