[U-Boot] [PATCH 5/6] I2C: Driver changes for FDT support

Rajeshwari Birje rajeshwari.birje at gmail.com
Mon Nov 12 14:03:51 CET 2012


Hi Simon Glass,

Thank you for comments.

On Fri, Nov 9, 2012 at 6:34 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Rajeshwari,
>
> On Wed, Nov 7, 2012 at 9:30 PM, Rajeshwari Shinde
> <rajeshwari.s at samsung.com> wrote:
>> Functions added to get the I2C bus number and reset I2C bus using
>> FDT node.
>>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> Signed-off-by: Rajeshwari Shinde <rajeshwari.s at samsung.com>
>> ---
>>  drivers/i2c/s3c24x0_i2c.c |   89 ++++++++++++++++++++++++++++++++++++++++++++-
>>  drivers/i2c/s3c24x0_i2c.h |    7 ++++
>>  include/i2c.h             |   28 ++++++++++++++
>>  3 files changed, 123 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c
>> index 9bc4c7f..952f76c 100644
>> --- a/drivers/i2c/s3c24x0_i2c.c
>> +++ b/drivers/i2c/s3c24x0_i2c.c
>> @@ -27,6 +27,7 @@
>>   */
>>
>>  #include <common.h>
>> +#include <fdtdec.h>
>>  #ifdef CONFIG_EXYNOS5
>>  #include <asm/arch/clk.h>
>>  #include <asm/arch/cpu.h>
>> @@ -60,7 +61,14 @@
>>  #define I2C_TIMEOUT 1          /* 1 second */
>>
>>
>> -static unsigned int g_current_bus;     /* Stores Current I2C Bus */
>> +/*
>> + * For SPL boot some boards need i2c before SDRAM is initialised so force
>> + * variables to live in SRAM
>> + */
>> +static unsigned int g_current_bus __attribute__((section(".data")));
>> +static struct s3c24x0_i2c_bus i2c_bus[CONFIG_MAX_I2C_NUM]
>> +                       __attribute__((section(".data")));
>> +static int i2c_busses __attribute__((section(".data")));
>>
>>  #ifndef CONFIG_EXYNOS5
>>  static int GetI2CSDA(void)
>> @@ -507,4 +515,83 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
>>                 (i2c, I2C_WRITE, chip << 1, &xaddr[4 - alen], alen, buffer,
>>                  len) != 0);
>>  }
>> +
>> +#ifdef CONFIG_OF_CONTROL
>> +unsigned int i2c_fdt_bus_values(const void *blob)
>> +{
>> +
>> +       int node_list[CONFIG_MAX_I2C_NUM];
>> +       int count, i;
>> +
>> +       count = fdtdec_find_aliases_for_id(blob, "i2c",
>> +               COMPAT_SAMSUNG_S3C2440_I2C, node_list,
>> +               CONFIG_MAX_I2C_NUM);
>> +       for (i = 0; i < count; i++) {
>> +               struct s3c24x0_i2c_bus *bus;
>> +               int node = node_list[i];
>> +
>> +               if (node < 0)
>> +                       continue;
>> +               bus = &i2c_bus[i];
>> +               bus->regs = (struct s3c24x0_i2c *)
>> +                       fdtdec_get_addr(blob, node, "reg");
>> +               bus->node = node;
>> +               bus->bus_num = i2c_busses++;
>> +       }
>> +
>> +       return count;
>> +}
>> +
>
> This seems to be the init function for i2c, so how about calling it
> board_i2c_init() or similar. I don't think you need to return the
> count of i2c ports - you can just use CONFIG_MAX_I2C_NUM anywhere that
> it is needed.
>
>> +static struct s3c24x0_i2c_bus *get_bus(int bus_idx)
>
> should probably be unsigned so that you don't need to check < 0
>
>> +{
>> +       if (bus_idx < i2c_busses)
>> +               return &i2c_bus[bus_idx];
>> +       debug("Undefined bus: %d\n", bus_idx);
>> +       return NULL;
>> +}
>> +
>> +int i2c_get_bus_num_fdt(const void *blob, int node)
>> +{
>> +       enum fdt_compat_id compat;
>> +       fdt_addr_t reg;
>> +       int i;
>> +
>> +       compat = fdtdec_lookup(blob, node);
>> +       if (compat != COMPAT_SAMSUNG_S3C2440_I2C) {
>> +               debug("%s: Not a supported I2C node\n", __func__);
>> +               return -1;
>> +       }
>> +       reg = fdtdec_get_addr(blob, node, "reg");
>> +       for (i = 0; i < i2c_busses; i++) {
>> +               if (reg == (fdt_addr_t)(uintptr_t)i2c_bus[i].regs)
>> +                       return i;
>> +       }
>
> This seems painful. You already have a table of node versus bus
> number, created in the function above, so why not just look in that?
>
You want me to use get_bus function? but that function takes the bus
number and gives the handle to the structure and not the i2c channel
register address. Hence we cannot use that function here, instead I am
using the structures populated in board_i2c_init.
Do clarify if I have misunderstood the comment.

>> +
>> +       debug("%s: Can't find any matched I2C bus\n", __func__);
>> +       return -1;
>> +}
>> +
>> +int i2c_reset_port_fdt(const void *blob, int node)
>> +{
>> +       struct s3c24x0_i2c_bus *i2c;
>> +
>> +       int bus;
>> +
>> +       bus = i2c_get_bus_num_fdt(blob, node);
>> +       if (bus < 0) {
>> +               debug("could not get bus for node %d\n", node);
>> +               return -1;
>> +       }
>> +       i2c = get_bus(bus);
>> +       if (!i2c) {
>> +               debug("get_bus() failed for node node %d\n", node);
>> +               return -1;
>> +       }
>> +
>> +       i2c_ch_init(i2c->regs, CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
>> +
>> +       return 0;
>> +}
>> +#endif
>> +
>>  #endif /* CONFIG_HARD_I2C */
>> diff --git a/drivers/i2c/s3c24x0_i2c.h b/drivers/i2c/s3c24x0_i2c.h
>> index 2dd4b06..bdd5514 100644
>> --- a/drivers/i2c/s3c24x0_i2c.h
>> +++ b/drivers/i2c/s3c24x0_i2c.h
>> @@ -30,4 +30,11 @@ struct s3c24x0_i2c {
>>         u32     iicds;
>>         u32     iiclc;
>>  };
>> +
>> +struct s3c24x0_i2c_bus {
>> +       int node;       /* device tree node */
>> +       int bus_num;    /* i2c bus number */
>> +       struct s3c24x0_i2c *regs;
>> +};
>> +
>>  #endif /* _S3C24X0_I2C_H */
>> diff --git a/include/i2c.h b/include/i2c.h
>> index 16f099d..52be093 100644
>> --- a/include/i2c.h
>> +++ b/include/i2c.h
>> @@ -262,4 +262,32 @@ extern int get_multi_scl_pin(void);
>>  extern int get_multi_sda_pin(void);
>>  extern int multi_i2c_init(void);
>>  #endif
>> +
>> +#ifdef CONFIG_OF_CONTROL
>
> I think you can omit this line - it doesn't hurt to have unused
> prototypes. We will get a link error if someone does something silly.
>
>> +/**
>> + * Get FDT values for i2c bus.
>> + *
>> + * @param blob  Device tree blbo
>> + * @return the number of I2C bus
>> + */
>> +unsigned int i2c_fdt_bus_values(const void *blob);
>> +
>> +/**
>> + * Find the I2C bus number by given a FDT I2C node.
>> + *
>> + * @param blob  Device tree blbo
>> + * @param node  FDT I2C node to find
>> + * @return the number of I2C bus (zero based), or -1 on error
>> + */
>> +int i2c_get_bus_num_fdt(const void *blob, int node);
>> +
>> +/**
>> + * Reset the I2C bus represented by the given a FDT I2C node.
>> + *
>> + * @param blob  Device tree blbo
>> + * @param node  FDT I2C node to find
>> + * @return 0 if port was reset, -1 if not found
>> + */
>> +int i2c_reset_port_fdt(const void *blob, int node);
>> +#endif
>>  #endif /* _I2C_H_ */
>> --
>> 1.7.4.4
>>
>
> Regards,
> Simon
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

Regards,
Rajeshwari


More information about the U-Boot mailing list