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

Piotr Wilczek
2013-11-08T07:45:56Z

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@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.

>
> Thanks for your work!
>
> bye,
> Heiko

Best regards,
Piotr Wilczek