[U-Boot] [PATCH] RFC: samsung: i2c: Enable new CONFIG_SYS_I2C framework

Naveen Krishna Ch naveenkrishna.ch at gmail.com
Fri Nov 8 08:59:11 CET 2013


Hello Piotr,

On 8 November 2013 13:15, Piotr Wilczek <p.wilczek at samsung.com> wrote:
> Dear Naveen,
>
>
> On 10/14/2013 08:06 AM, Heiko Schocher wrote:
>>
>> Hello Naveen,
>>
>> Am 30.09.2013 08:58, schrieb Naveen Krishna Chatradhi:
>>>
>>> This enables CONFIG_SYS_I2C on Samsung, updating existing s3c24x0
>>> i2c driver to support this.
>>>
>>> Note: Not for merge, Just for review and suggestions.
>>>
>>> Signed-off-by: Naveen Krishna Chatradhi<ch.naveen at samsung.com>
>>> ---
>>>   drivers/i2c/Makefile      |    2 +-
>>>   drivers/i2c/s3c24x0_i2c.c |  156
>>> +++++++++++++++++++++++++++++++--------------
>>>   2 files changed, 108 insertions(+), 50 deletions(-)
>>>
>>> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
>>> index 37ccbd1..96448a6 100644
>>> --- a/drivers/i2c/Makefile
>>> +++ b/drivers/i2c/Makefile
>>> @@ -20,7 +20,7 @@ COBJS-$(CONFIG_DRIVER_OMAP1510_I2C) += omap1510_i2c.o
>>>   COBJS-$(CONFIG_DRIVER_OMAP24XX_I2C) += omap24xx_i2c.o
>>>   COBJS-$(CONFIG_DRIVER_OMAP34XX_I2C) += omap24xx_i2c.o
>>>   COBJS-$(CONFIG_PCA9564_I2C) += pca9564_i2c.o
>>> -COBJS-$(CONFIG_DRIVER_S3C24X0_I2C) += s3c24x0_i2c.o
>>> +COBJS-$(CONFIG_SYS_I2C_S3C24X0_I2C) += s3c24x0_i2c.o
>>
>>
>> Please keep lists sorted.
>>
>>>   COBJS-$(CONFIG_S3C44B0_I2C) += s3c44b0_i2c.o
>>>   COBJS-$(CONFIG_TSI108_I2C) += tsi108_i2c.o
>>>   COBJS-$(CONFIG_U8500_I2C) += u8500_i2c.o
>>> diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c
>>> index 604d43d..89cb2d2 100644
>>> --- a/drivers/i2c/s3c24x0_i2c.c
>>> +++ b/drivers/i2c/s3c24x0_i2c.c
>>> @@ -117,6 +117,8 @@
>>>
>>>   #define    HSI2C_TIMEOUT_US 100000 /* 100 ms, finer granularity */
>>>
>>> +DECLARE_GLOBAL_DATA_PTR;
>>> +
>>>   /*
>>>    * For SPL boot some boards need i2c before SDRAM is initialised so
>>> force
>>>    * variables to live in SRAM
>>> @@ -126,6 +128,9 @@ static unsigned int g_current_bus
>>> __attribute__((section(".data")));
>>>   static struct s3c24x0_i2c_bus i2c_bus[CONFIG_MAX_I2C_NUM]
>>>               __attribute__((section(".data")));
>>>   #endif
>>> +static void i2c_ch_init(struct s3c24x0_i2c *i2c, int speed, int
>>> slaveadd);
>>> +static int hsi2c_get_clk_details(struct s3c24x0_i2c_bus *);
>>> +static void hsi2c_ch_init(struct s3c24x0_i2c_bus *);
>>>
>>>   /**
>>>    * Get a pointer to the given bus index
>>> @@ -133,20 +138,40 @@ static struct s3c24x0_i2c_bus
>>> i2c_bus[CONFIG_MAX_I2C_NUM]
>>>    * @bus_idx: Bus index to look up
>>>    * @return pointer to bus, or NULL if invalid or not available
>>>    */
>>> -static struct s3c24x0_i2c_bus *get_bus(unsigned int bus_idx)
>>> +static struct s3c24x0_i2c_bus *s3c_i2c_get_bus(struct i2c_adapter *adap)
>>>   {
>>> -    if (bus_idx<  ARRAY_SIZE(i2c_bus)) {
>>> -        struct s3c24x0_i2c_bus *bus;
>>> +    struct s3c24x0_i2c_bus *bus;
>>>
>>> -        bus =&i2c_bus[bus_idx];
>>> -        if (bus->active)
>>> -            return bus;
>>> -    }
>>> +    bus =&i2c_bus[adap->hwadapnr];
>>> +    if (bus->active)
>>> +        return bus;
>>>
>>> -    debug("Undefined bus: %d\n", bus_idx);
>>> +    debug("Undefined bus: %d\n", adap->hwadapnr);
>>>       return NULL;
>>>   }
>>>
>>> +static unsigned int s3c_i2c_set_bus_speed(struct i2c_adapter *adap,
>>> +                      unsigned int speed)
>>> +{
>>> +    struct s3c24x0_i2c_bus *i2c_bus;
>>> +
>>> +    i2c_bus = s3c_i2c_get_bus(adap);
>>> +    if (!i2c_bus)
>>> +        return -1;
>>> +    i2c_bus->clock_frequency = speed;
>>> +
>>> +    if (i2c_bus->is_highspeed) {
>>> +        if (hsi2c_get_clk_details(i2c_bus))
>>> +            return -1;
>>> +        hsi2c_ch_init(i2c_bus);
>>> +    } else {
>>> +        i2c_ch_init(i2c_bus->regs, i2c_bus->clock_frequency,
>>> +                        CONFIG_SYS_I2C_SLAVE);
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>   #if !(defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5)
>>>   static int GetI2CSDA(void)
>>>   {
>>> @@ -392,39 +417,7 @@ static void exynos5_i2c_reset(struct
>>> s3c24x0_i2c_bus *i2c_bus)
>>>       hsi2c_ch_init(i2c_bus);
>>>   }
>>>
>>> -/*
>>> - * MULTI BUS I2C support
>>> - */
>>> -
>>> -#ifdef CONFIG_I2C_MULTI_BUS
>>> -int i2c_set_bus_num(unsigned int bus)
>>> -{
>>> -    struct s3c24x0_i2c_bus *i2c_bus;
>>> -
>>> -    i2c_bus = get_bus(bus);
>>> -    if (!i2c_bus)
>>> -        return -1;
>>> -    g_current_bus = bus;
>>> -
>>> -    if (i2c_bus->is_highspeed) {
>>> -        if (hsi2c_get_clk_details(i2c_bus))
>>> -            return -1;
>>> -        hsi2c_ch_init(i2c_bus);
>>> -    } else {
>>> -        i2c_ch_init(i2c_bus->regs, i2c_bus->clock_frequency,
>>> -                        CONFIG_SYS_I2C_SLAVE);
>>> -    }
>>> -
>>> -    return 0;
>>> -}
>>> -
>>> -unsigned int i2c_get_bus_num(void)
>>> -{
>>> -    return g_current_bus;
>>> -}
>>> -#endif
>>> -
>>> -void i2c_init(int speed, int slaveadd)
>>> +static void s3c_i2c_init(struct i2c_adapter *adap, int speed, int
>>> slaveaddr)
>>>   {
>>>       struct s3c24x0_i2c *i2c;
>>>   #if !(defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5)
>>> @@ -493,9 +486,15 @@ void i2c_init(int speed, int slaveadd)
>>>   #endif
>>>       }
>>>   #endif /* #if !(defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5) */
>>> -    i2c_ch_init(i2c, speed, slaveadd);
>>> +    /* This will override the speed selected in the fdt for that port */
>>> +    debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
>>> +    i2c_ch_init(i2c_bus->regs, speed, slaveaddr);
>>>   }
>>>
>>> +static void exynos_i2c_init(struct i2c_adapter *adap, int speed, int
>>> slaveaddr)
>>> +{
>>> +    i2c_set_bus_speed(speed);
>>> +}
>>>   /*
>>>    * Poll the appropriate bit of the fifo status register until the
>>> interface is
>>>    * ready to process the next byte or timeout expires.
>>> @@ -829,13 +828,13 @@ bailout:
>>>       return result;
>>>   }
>>>
>>> -int i2c_probe(uchar chip)
>>> +int s3c_i2c_probe(struct i2c_adapter *adap, uchar chip)
>>>   {
>>>       struct s3c24x0_i2c_bus *i2c_bus;
>>>       uchar buf[1];
>>>       int ret;
>>>
>>> -    i2c_bus = get_bus(g_current_bus);
>>> +    i2c_bus = s3c_i2c_get_bus(adap);
>>>       if (!i2c_bus)
>>>           return -1;
>>>       buf[0] = 0;
>>> @@ -857,7 +856,8 @@ int i2c_probe(uchar chip)
>>>       return ret != I2C_OK;
>>>   }
>>>
>>> -int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
>>> +int s3c_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, int
>>> alen,
>>> +         uchar *buffer, int len)
>>>   {
>>>       struct s3c24x0_i2c_bus *i2c_bus;
>>>       uchar xaddr[4];
>>> @@ -891,7 +891,7 @@ int i2c_read(uchar chip, uint addr, int alen,
>>> uchar *buffer, int len)
>>>           chip |= ((addr>>  (alen * 8))&
>>>               CONFIG_SYS_I2C_EEPROM_ADDR_OVERFLOW);
>>>   #endif
>>> -    i2c_bus = get_bus(g_current_bus);
>>> +    i2c_bus = s3c_i2c_get_bus(adap);
>>>       if (!i2c_bus)
>>>           return -1;
>>>
>>> @@ -911,7 +911,8 @@ int i2c_read(uchar chip, uint addr, int alen,
>>> uchar *buffer, int len)
>>>       return 0;
>>>   }
>>>
>>> -int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
>>> +int s3c_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
>>> int alen,
>>> +          uchar *buffer, int len)
>>>   {
>>>       struct s3c24x0_i2c_bus *i2c_bus;
>>>       uchar xaddr[4];
>>> @@ -944,7 +945,7 @@ int i2c_write(uchar chip, uint addr, int alen,
>>> uchar *buffer, int len)
>>>           chip |= ((addr>>  (alen * 8))&
>>>               CONFIG_SYS_I2C_EEPROM_ADDR_OVERFLOW);
>>>   #endif
>>> -    i2c_bus = get_bus(g_current_bus);
>>> +    i2c_bus = s3c_i2c_get_bus(adap);
>>>       if (!i2c_bus)
>>>           return -1;
>>>
>>> @@ -1000,6 +1001,25 @@ static void process_nodes(const void *blob, int
>>> node_list[], int count,
>>>           node_list[i] = -1;
>>>       }
>>>   }
>>> +void i2c_init_board()
>>> +{
>>> +    int node_list[CONFIG_MAX_I2C_NUM];
>>> +    const void *blob = gd->fdt_blob;
>>> +    int count;
>>> +
>>> +    /* First get the normal i2c ports */
>>> +    count = fdtdec_find_aliases_for_id(blob, "i2c",
>>> +        COMPAT_SAMSUNG_S3C2440_I2C, node_list,
>>> +        CONFIG_MAX_I2C_NUM);
>>> +    process_nodes(blob, node_list, count, 0);
>>> +
>>> +    /* Now look for high speed i2c ports */
>>> +    count = fdtdec_find_aliases_for_id(blob, "i2c",
>>> +        COMPAT_SAMSUNG_EXYNOS5_I2C, node_list,
>>> +        CONFIG_MAX_I2C_NUM);
>>> +    process_nodes(blob, node_list, count, 1);
>>> +
>>> +}
>>>
>>>   void board_i2c_init(const void *blob)
>>>   {
>>> @@ -1017,9 +1037,10 @@ void board_i2c_init(const void *blob)
>>>           COMPAT_SAMSUNG_EXYNOS5_I2C, node_list,
>>>           CONFIG_MAX_I2C_NUM);
>>>       process_nodes(blob, node_list, count, 1);
>>> -
>>>   }
>>>
>>> +#ifndef CONFIG_SYS_I2C
>>> +/* TODO: fix this as per comments from upstream */
>>
>>
>> Why this? If we switch to the new i2c framework, please convert all
>> boards,
>> which using this driver ...
>>
>>>   int i2c_get_bus_num_fdt(int node)
>>>   {
>>>       int i;
>>> @@ -1062,5 +1083,42 @@ int i2c_reset_port_fdt(const void *blob, int node)
>>>       return 0;
>>>   }
>>>   #endif
>>> +#endif
>>>
>>> +/*
>>> + * Register soft i2c adapters
>>> + */
>>> +U_BOOT_I2C_ADAP_COMPLETE(s3c0, s3c_i2c_init, s3c_i2c_probe,
>>> +             s3c_i2c_read, s3c_i2c_write,
>>> +             s3c_i2c_set_bus_speed, 100000, 0, 0)
>>> +U_BOOT_I2C_ADAP_COMPLETE(s3c1, s3c_i2c_init, s3c_i2c_probe,
>>> +             s3c_i2c_read, s3c_i2c_write,
>>> +             s3c_i2c_set_bus_speed, 100000, 0, 1)
>>> +U_BOOT_I2C_ADAP_COMPLETE(s3c2, s3c_i2c_init, s3c_i2c_probe,
>>> +             s3c_i2c_read, s3c_i2c_write,
>>> +             s3c_i2c_set_bus_speed, 100000, 0, 2)
>>> +U_BOOT_I2C_ADAP_COMPLETE(s3c3, exynos_i2c_init, s3c_i2c_probe,
>>> +             s3c_i2c_read, s3c_i2c_write,
>>> +             s3c_i2c_set_bus_speed, 100000, 0, 3)
>>> +U_BOOT_I2C_ADAP_COMPLETE(s3c4, exynos_i2c_init, s3c_i2c_probe,
>>> +             s3c_i2c_read, s3c_i2c_write,
>>> +             s3c_i2c_set_bus_speed, 100000, 0, 4)
>>> +U_BOOT_I2C_ADAP_COMPLETE(s3c5, exynos_i2c_init, s3c_i2c_probe,
>>> +             s3c_i2c_read, s3c_i2c_write,
>>> +             s3c_i2c_set_bus_speed, 100000, 0, 5)
>>> +U_BOOT_I2C_ADAP_COMPLETE(s3c6, exynos_i2c_init, s3c_i2c_probe,
>>> +             s3c_i2c_read, s3c_i2c_write,
>>> +             s3c_i2c_set_bus_speed, 100000, 0, 6)
>>> +U_BOOT_I2C_ADAP_COMPLETE(s3c7, exynos_i2c_init, s3c_i2c_probe,
>>> +             s3c_i2c_read, s3c_i2c_write,
>>> +             s3c_i2c_set_bus_speed, 100000, 0, 7)
>>> +U_BOOT_I2C_ADAP_COMPLETE(s3c8, exynos_i2c_init, s3c_i2c_probe,
>>> +             s3c_i2c_read, s3c_i2c_write,
>>> +             s3c_i2c_set_bus_speed, 100000, 0, 8)
>>> +U_BOOT_I2C_ADAP_COMPLETE(s3c9, exynos_i2c_init, s3c_i2c_probe,
>>> +             s3c_i2c_read, s3c_i2c_write,
>>> +             s3c_i2c_set_bus_speed, 100000, 0, 9)
>>> +U_BOOT_I2C_ADAP_COMPLETE(s3c10, exynos_i2c_init, s3c_i2c_probe,
>>> +             s3c_i2c_read, s3c_i2c_write,
>>> +             s3c_i2c_set_bus_speed, 100000, 0, 10)
>>>   #endif /* CONFIG_HARD_I2C */
>>
>>
>> Beside of that, it looks good to me.
>
> I have some patches based on this patch. Do you plan send updated version?
> It would be nice to switch that driver to the new framework.

I don't, have the updated patch ready.

I had a problem with the bus id's being scrambled with the new i2c framework.
Also, we need to update the new config in all the boards which are
using this driver.
I was in and out of that work.

If you have a better patch, feel free to update this patch as well.

>
>
>>
>> Thanks for your work!
>>
>> bye,
>> Heiko
>
>
> Best regards,
> Piotr Wilczek
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot



-- 
Shine bright,
(: Nav :)


More information about the U-Boot mailing list