Re: [U-Boot] [PATCH v2 07/10] dm: i2c: s3c24x0: adjust to dm-i2c api
Hello Simon, On 01/27/2015 04:13 AM, Simon Glass wrote: Hi Przemyslaw, On 26 January 2015 at 08:21, Przemyslaw Marczak p.marc...@samsung.com wrote: This commit adjusts the s3c24x0 driver to new i2c api based on driver-model. The driver supports standard and high-speed i2c as previous. Tested on Trats2, Odroid U3, Arndale, Odroid XU3 Signed-off-by: Przemyslaw Marczak p.marc...@samsung.com Cc: Simon Glass s...@chromium.org Cc: Heiko Schocher h...@denx.de Cc: Minkyu Kang mk7.k...@samsung.com Tested on snow: Tested-by: Simon Glass s...@chromium.org This looks right to me, but I have a number of nits, mostly code that can be deleted, Please see below. If you can respin this I will pick it up. Thank you for the review, I will fix the issues and resend this today. Best regards, -- Przemyslaw Marczak Samsung RD Institute Poland Samsung Electronics p.marc...@samsung.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 07/10] dm: i2c: s3c24x0: adjust to dm-i2c api
This commit adjusts the s3c24x0 driver to new i2c api
based on driver-model. The driver supports standard
and high-speed i2c as previous.
Tested on Trats2, Odroid U3, Arndale, Odroid XU3
Signed-off-by: Przemyslaw Marczak p.marc...@samsung.com
Cc: Simon Glass s...@chromium.org
Cc: Heiko Schocher h...@denx.de
Cc: Minkyu Kang mk7.k...@samsung.com
---
Changes v2:
- use consistent return values on errors
- decrease transaction status timeout, because the previous one was too big
---
drivers/i2c/s3c24x0_i2c.c | 275 +++---
1 file changed, 233 insertions(+), 42 deletions(-)
diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c
index fd328f0..c82640d 100644
--- a/drivers/i2c/s3c24x0_i2c.c
+++ b/drivers/i2c/s3c24x0_i2c.c
@@ -9,8 +9,9 @@
* as they seem to have the same I2C controller inside.
* The different address mapping is handled by the s3c24xx.h files below.
*/
-
#include common.h
+#include errno.h
+#include dm.h
#include fdtdec.h
#if (defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5)
#include asm/arch/clk.h
@@ -111,9 +112,9 @@
#define I2C_START_STOP 0x20/* START / STOP */
#define I2C_TXRX_ENA 0x10/* I2C Tx/Rx enable */
-#define I2C_TIMEOUT_MS 1000/* 1 second */
+#define I2C_TIMEOUT_MS 10 /* 10 ms */
-#defineHSI2C_TIMEOUT_US 10 /* 100 ms, finer granularity */
+#defineHSI2C_TIMEOUT_US 1 /* 10 ms, finer granularity */
/* To support VCMA9 boards and other who dont define max_i2c_num */
@@ -121,13 +122,23 @@
#define CONFIG_MAX_I2C_NUM 1
#endif
+DECLARE_GLOBAL_DATA_PTR;
+
/*
* For SPL boot some boards need i2c before SDRAM is initialised so force
* variables to live in SRAM
*/
+#ifdef CONFIG_SYS_I2C
static struct s3c24x0_i2c_bus i2c_bus[CONFIG_MAX_I2C_NUM]
__attribute__((section(.data)));
+#endif
+
+enum exynos_i2c_type {
+ EXYNOS_I2C_STD,
+ EXYNOS_I2C_HS,
+};
+#ifdef CONFIG_SYS_I2C
/**
* Get a pointer to the given bus index
*
@@ -147,6 +158,7 @@ static struct s3c24x0_i2c_bus *get_bus(unsigned int bus_idx)
debug(Undefined bus: %d\n, bus_idx);
return NULL;
}
+#endif
#if !(defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5)
static int GetI2CSDA(void)
@@ -251,6 +263,7 @@ static void ReadWriteByte(struct s3c24x0_i2c *i2c)
writel(readl(i2c-iiccon) ~I2CCON_IRPND, i2c-iiccon);
}
+#ifdef CONFIG_SYS_I2C
static struct s3c24x0_i2c *get_base_i2c(int bus)
{
#ifdef CONFIG_EXYNOS4
@@ -267,6 +280,7 @@ static struct s3c24x0_i2c *get_base_i2c(int bus)
return s3c24x0_get_base_i2c();
#endif
}
+#endif
static void i2c_ch_init(struct s3c24x0_i2c *i2c, int speed, int slaveadd)
{
@@ -326,7 +340,7 @@ static int hsi2c_get_clk_details(struct s3c24x0_i2c_bus
*i2c_bus)
return 0;
}
}
- return -1;
+ return -EINVAL;
}
static void hsi2c_ch_init(struct s3c24x0_i2c_bus *i2c_bus)
@@ -398,18 +412,20 @@ static void exynos5_i2c_reset(struct s3c24x0_i2c_bus
*i2c_bus)
hsi2c_ch_init(i2c_bus);
}
+#ifdef CONFIG_SYS_I2C
static void s3c24x0_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd)
{
struct s3c24x0_i2c *i2c;
struct s3c24x0_i2c_bus *bus;
-
#if !(defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5)
struct s3c24x0_gpio *gpio = s3c24x0_get_base_gpio();
#endif
ulong start_time = get_timer(0);
- /* By default i2c channel 0 is the current bus */
i2c = get_base_i2c(adap-hwadapnr);
+ bus = i2c_bus[adap-hwadapnr];
+ if (!bus)
+ return;
/*
* In case the previous transfer is still going, wait to give it a
@@ -470,12 +486,13 @@ static void s3c24x0_i2c_init(struct i2c_adapter *adap,
int speed, int slaveadd)
#endif
}
#endif /* #if !(defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5) */
+
i2c_ch_init(i2c, speed, slaveadd);
- bus = i2c_bus[adap-hwadapnr];
bus-active = true;
bus-regs = i2c;
}
+#endif /* CONFIG_SYS_I2C */
/*
* Poll the appropriate bit of the fifo status register until the interface is
@@ -698,40 +715,40 @@ static int hsi2c_read(struct exynos5_hsi2c *i2c,
return rv;
}
+#ifdef CONFIG_SYS_I2C
static unsigned int s3c24x0_i2c_set_bus_speed(struct i2c_adapter *adap,
- unsigned int speed)
+ unsigned int speed)
+#else
+static int s3c24x0_i2c_set_bus_speed(struct udevice *dev, unsigned int speed)
+#endif
{
struct s3c24x0_i2c_bus *i2c_bus;
+#ifdef CONFIG_SYS_I2C
i2c_bus = get_bus(adap-hwadapnr);
+#else
+ if (!dev)
+ return -ENODEV;
+
+ i2c_bus = dev_get_priv(dev);
+#endif
if (!i2c_bus)
- return -1;
+ return -EFAULT;
i2c_bus-clock_frequency = speed;
if (i2c_bus-is_highspeed) {
if
Re: [U-Boot] [PATCH v2 07/10] dm: i2c: s3c24x0: adjust to dm-i2c api
Hi Przemyslaw,
On 26 January 2015 at 08:21, Przemyslaw Marczak p.marc...@samsung.com wrote:
This commit adjusts the s3c24x0 driver to new i2c api
based on driver-model. The driver supports standard
and high-speed i2c as previous.
Tested on Trats2, Odroid U3, Arndale, Odroid XU3
Signed-off-by: Przemyslaw Marczak p.marc...@samsung.com
Cc: Simon Glass s...@chromium.org
Cc: Heiko Schocher h...@denx.de
Cc: Minkyu Kang mk7.k...@samsung.com
Tested on snow:
Tested-by: Simon Glass s...@chromium.org
This looks right to me, but I have a number of nits, mostly code that
can be deleted, Please see below.
If you can respin this I will pick it up.
---
Changes v2:
- use consistent return values on errors
- decrease transaction status timeout, because the previous one was too big
---
drivers/i2c/s3c24x0_i2c.c | 275
+++---
1 file changed, 233 insertions(+), 42 deletions(-)
diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c
index fd328f0..c82640d 100644
--- a/drivers/i2c/s3c24x0_i2c.c
+++ b/drivers/i2c/s3c24x0_i2c.c
@@ -9,8 +9,9 @@
* as they seem to have the same I2C controller inside.
* The different address mapping is handled by the s3c24xx.h files below.
*/
-
#include common.h
+#include errno.h
+#include dm.h
#include fdtdec.h
#if (defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5)
#include asm/arch/clk.h
@@ -111,9 +112,9 @@
#define I2C_START_STOP 0x20/* START / STOP */
#define I2C_TXRX_ENA 0x10/* I2C Tx/Rx enable */
-#define I2C_TIMEOUT_MS 1000/* 1 second */
+#define I2C_TIMEOUT_MS 10 /* 10 ms */
-#defineHSI2C_TIMEOUT_US 10 /* 100 ms, finer granularity */
+#defineHSI2C_TIMEOUT_US 1 /* 10 ms, finer granularity */
Why change the timeouts? In any case that should be a separate patch.
/* To support VCMA9 boards and other who dont define max_i2c_num */
@@ -121,13 +122,23 @@
#define CONFIG_MAX_I2C_NUM 1
#endif
+DECLARE_GLOBAL_DATA_PTR;
+
/*
* For SPL boot some boards need i2c before SDRAM is initialised so force
* variables to live in SRAM
*/
+#ifdef CONFIG_SYS_I2C
static struct s3c24x0_i2c_bus i2c_bus[CONFIG_MAX_I2C_NUM]
__attribute__((section(.data)));
+#endif
+
+enum exynos_i2c_type {
+ EXYNOS_I2C_STD,
+ EXYNOS_I2C_HS,
+};
+#ifdef CONFIG_SYS_I2C
/**
* Get a pointer to the given bus index
*
@@ -147,6 +158,7 @@ static struct s3c24x0_i2c_bus *get_bus(unsigned int
bus_idx)
debug(Undefined bus: %d\n, bus_idx);
return NULL;
}
+#endif
#if !(defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5)
static int GetI2CSDA(void)
@@ -251,6 +263,7 @@ static void ReadWriteByte(struct s3c24x0_i2c *i2c)
writel(readl(i2c-iiccon) ~I2CCON_IRPND, i2c-iiccon);
}
+#ifdef CONFIG_SYS_I2C
static struct s3c24x0_i2c *get_base_i2c(int bus)
{
#ifdef CONFIG_EXYNOS4
@@ -267,6 +280,7 @@ static struct s3c24x0_i2c *get_base_i2c(int bus)
return s3c24x0_get_base_i2c();
#endif
}
+#endif
static void i2c_ch_init(struct s3c24x0_i2c *i2c, int speed, int slaveadd)
{
@@ -326,7 +340,7 @@ static int hsi2c_get_clk_details(struct s3c24x0_i2c_bus
*i2c_bus)
return 0;
}
}
- return -1;
+ return -EINVAL;
}
static void hsi2c_ch_init(struct s3c24x0_i2c_bus *i2c_bus)
@@ -398,18 +412,20 @@ static void exynos5_i2c_reset(struct s3c24x0_i2c_bus
*i2c_bus)
hsi2c_ch_init(i2c_bus);
}
+#ifdef CONFIG_SYS_I2C
static void s3c24x0_i2c_init(struct i2c_adapter *adap, int speed, int
slaveadd)
{
struct s3c24x0_i2c *i2c;
struct s3c24x0_i2c_bus *bus;
-
#if !(defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5)
struct s3c24x0_gpio *gpio = s3c24x0_get_base_gpio();
#endif
ulong start_time = get_timer(0);
- /* By default i2c channel 0 is the current bus */
i2c = get_base_i2c(adap-hwadapnr);
+ bus = i2c_bus[adap-hwadapnr];
+ if (!bus)
+ return;
/*
* In case the previous transfer is still going, wait to give it a
@@ -470,12 +486,13 @@ static void s3c24x0_i2c_init(struct i2c_adapter *adap,
int speed, int slaveadd)
#endif
}
#endif /* #if !(defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5) */
+
i2c_ch_init(i2c, speed, slaveadd);
- bus = i2c_bus[adap-hwadapnr];
bus-active = true;
bus-regs = i2c;
}
+#endif /* CONFIG_SYS_I2C */
/*
* Poll the appropriate bit of the fifo status register until the interface
is
@@ -698,40 +715,40 @@ static int hsi2c_read(struct exynos5_hsi2c *i2c,
return rv;
}
+#ifdef CONFIG_SYS_I2C
static unsigned int s3c24x0_i2c_set_bus_speed(struct i2c_adapter *adap,
- unsigned int speed)
+ unsigned int speed)
+#else
