[PATCH v2 0/2] Enable Samsung I2C drivers on newer platforms
This set of patches should enable the Samsung I2C drivers to work on platforms other than EXYNOS4/EXYNOS5.
This has been tested on Exynos7885 with the S3C I2C driver. With the clocks for it implemented in it's driver, this should also enable S3C I2C to work on Exynos850.
While at it, clean up some dead code as well.
Changes in v2: - Fix compile on Exynos4/Exynos5 platform
David Virag (2): i2c: samsung: Drop s3c24x0 specific code. i2c: samsung: Support platforms other than EXYNOS4 and EXYNOS5
drivers/i2c/Kconfig | 2 +- drivers/i2c/exynos_hs_i2c.c | 25 +++++++++++++++++++++---- drivers/i2c/s3c24x0_i2c.c | 32 ++++++++++++++++++++++++-------- drivers/i2c/s3c24x0_i2c.h | 2 ++ 4 files changed, 48 insertions(+), 13 deletions(-)
This has been dead code for many years now. Remove it.
Signed-off-by: David Virag virag.david003@gmail.com --- drivers/i2c/exynos_hs_i2c.c | 4 ---- drivers/i2c/s3c24x0_i2c.c | 8 -------- 2 files changed, 12 deletions(-)
diff --git a/drivers/i2c/exynos_hs_i2c.c b/drivers/i2c/exynos_hs_i2c.c index 2ab0bae449..189ce6d509 100644 --- a/drivers/i2c/exynos_hs_i2c.c +++ b/drivers/i2c/exynos_hs_i2c.c @@ -145,11 +145,7 @@ static int hsi2c_get_clk_details(struct s3c24x0_i2c_bus *i2c_bus) unsigned int i = 0, utemp0 = 0, utemp1 = 0; unsigned int t_ftl_cycle;
-#if defined(CONFIG_ARCH_EXYNOS4) || defined(CONFIG_ARCH_EXYNOS5) clkin = get_i2c_clk(); -#else - clkin = get_PCLK(); -#endif /* FPCLK / FI2C = * (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2) + 8 + 2 * FLT_CYCLE * uTemp0 = (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2) diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c index 72d2ab0f73..ae3a801cad 100644 --- a/drivers/i2c/s3c24x0_i2c.c +++ b/drivers/i2c/s3c24x0_i2c.c @@ -8,14 +8,10 @@ #include <dm.h> #include <fdtdec.h> #include <time.h> -#if defined(CONFIG_ARCH_EXYNOS4) || defined(CONFIG_ARCH_EXYNOS5) #include <log.h> #include <asm/arch/clk.h> #include <asm/arch/cpu.h> #include <asm/arch/pinmux.h> -#else -#include <asm/arch/s3c24x0_cpu.h> -#endif #include <asm/global_data.h> #include <asm/io.h> #include <i2c.h> @@ -53,11 +49,7 @@ static void read_write_byte(struct s3c24x0_i2c *i2c) static void i2c_ch_init(struct s3c24x0_i2c *i2c, int speed, int slaveadd) { ulong freq, pres = 16, div; -#if defined(CONFIG_ARCH_EXYNOS4) || defined(CONFIG_ARCH_EXYNOS5) freq = get_i2c_clk(); -#else - freq = get_PCLK(); -#endif /* calculate prescaler and divisor values */ if ((freq / pres / (16 + 1)) > speed) /* set prescaler to 512 */
Hello David,
On 02.08.24 21:19, David Virag wrote:
This has been dead code for many years now. Remove it.
Signed-off-by: David Virag virag.david003@gmail.com
drivers/i2c/exynos_hs_i2c.c | 4 ---- drivers/i2c/s3c24x0_i2c.c | 8 -------- 2 files changed, 12 deletions(-)
Reviewed-by: Heiko Schocher hs@denx.de
Thanks!
bye, Heiko
Newer Samsung SoCs (including newer Exynos, ExynosAuto, Google Tensor) still use these IPs, or slightly newer versions of it.
Make these drivers available on these platforms by guarding EXYNOS4/EXYNOS5 specific code behind their configs, and using CCF for clocks on other platforms.
Tested S3C I2C driver on Exynos7885. This along with extended clock driver should enable S3C I2C on Exynos850.
Signed-off-by: David Virag virag.david003@gmail.com --- drivers/i2c/Kconfig | 2 +- drivers/i2c/exynos_hs_i2c.c | 25 +++++++++++++++++++++++-- drivers/i2c/s3c24x0_i2c.c | 30 +++++++++++++++++++++++++++--- drivers/i2c/s3c24x0_i2c.h | 2 ++ 4 files changed, 53 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig index cba7f84894..52067fa7c1 100644 --- a/drivers/i2c/Kconfig +++ b/drivers/i2c/Kconfig @@ -650,7 +650,7 @@ config SYS_I2C_GENI
config SYS_I2C_S3C24X0 bool "Samsung I2C driver" - depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && DM_I2C + depends on DM_I2C help Support for Samsung I2C controller as Samsung SoCs.
diff --git a/drivers/i2c/exynos_hs_i2c.c b/drivers/i2c/exynos_hs_i2c.c index 189ce6d509..fa0d1c8f64 100644 --- a/drivers/i2c/exynos_hs_i2c.c +++ b/drivers/i2c/exynos_hs_i2c.c @@ -9,11 +9,15 @@ #include <dm.h> #include <i2c.h> #include <log.h> +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) #include <asm/arch/clk.h> #include <asm/arch/cpu.h> #include <asm/arch/pinmux.h> +#endif #include <asm/global_data.h> +#include <asm/io.h> #include <linux/delay.h> +#include <clk.h> #include "s3c24x0_i2c.h"
DECLARE_GLOBAL_DATA_PTR; @@ -137,15 +141,26 @@ static int hsi2c_wait_for_trx(struct exynos5_hsi2c *i2c) return I2C_NOK_TOUT; }
-static int hsi2c_get_clk_details(struct s3c24x0_i2c_bus *i2c_bus) +static int hsi2c_get_clk_details(struct udevice *dev) { + struct s3c24x0_i2c_bus *i2c_bus = dev_get_priv(dev); struct exynos5_hsi2c *hsregs = i2c_bus->hsregs; ulong clkin; unsigned int op_clk = i2c_bus->clock_frequency; unsigned int i = 0, utemp0 = 0, utemp1 = 0; unsigned int t_ftl_cycle;
+#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) clkin = get_i2c_clk(); +#else + struct clk clk; + int ret; + + ret = clk_get_by_name(dev, "hsi2c", &clk); + if (ret < 0) + return ret; + clkin = clk_get_rate(&clk); +#endif /* FPCLK / FI2C = * (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2) + 8 + 2 * FLT_CYCLE * uTemp0 = (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2) @@ -487,7 +502,7 @@ static int s3c24x0_i2c_set_bus_speed(struct udevice *dev, unsigned int speed)
i2c_bus->clock_frequency = speed;
- if (hsi2c_get_clk_details(i2c_bus)) + if (hsi2c_get_clk_details(dev)) return -EFAULT; hsi2c_ch_init(i2c_bus);
@@ -514,7 +529,9 @@ static int s3c24x0_i2c_probe(struct udevice *dev, uint chip, uint chip_flags)
static int s3c_i2c_of_to_plat(struct udevice *dev) { +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) const void *blob = gd->fdt_blob; +#endif struct s3c24x0_i2c_bus *i2c_bus = dev_get_priv(dev); int node;
@@ -522,7 +539,9 @@ static int s3c_i2c_of_to_plat(struct udevice *dev)
i2c_bus->hsregs = dev_read_addr_ptr(dev);
+#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) i2c_bus->id = pinmux_decode_periph_id(blob, node); +#endif
i2c_bus->clock_frequency = dev_read_u32_default(dev, "clock-frequency", @@ -530,7 +549,9 @@ static int s3c_i2c_of_to_plat(struct udevice *dev) i2c_bus->node = node; i2c_bus->bus_num = dev_seq(dev);
+#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) exynos_pinmux_config(i2c_bus->id, PINMUX_FLAG_HS_MODE); +#endif
i2c_bus->active = true;
diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c index ae3a801cad..ade1ad6cef 100644 --- a/drivers/i2c/s3c24x0_i2c.c +++ b/drivers/i2c/s3c24x0_i2c.c @@ -9,12 +9,15 @@ #include <fdtdec.h> #include <time.h> #include <log.h> +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) #include <asm/arch/clk.h> #include <asm/arch/cpu.h> #include <asm/arch/pinmux.h> +#endif #include <asm/global_data.h> #include <asm/io.h> #include <i2c.h> +#include <clk.h> #include "s3c24x0_i2c.h"
DECLARE_GLOBAL_DATA_PTR; @@ -46,10 +49,23 @@ static void read_write_byte(struct s3c24x0_i2c *i2c) clrbits_le32(&i2c->iiccon, I2CCON_IRPND); }
-static void i2c_ch_init(struct s3c24x0_i2c *i2c, int speed, int slaveadd) +static int i2c_ch_init(struct udevice *dev, int speed, int slaveadd) { + struct s3c24x0_i2c_bus *i2c_bus = dev_get_priv(dev); + struct s3c24x0_i2c *i2c = i2c_bus->regs; ulong freq, pres = 16, div; + +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || defined(CONFIG_ARCH_EXYNOS5) freq = get_i2c_clk(); +#else + struct clk clk; + int ret; + + ret = clk_get_by_name(dev, "i2c", &clk); + if (ret < 0) + return ret; + freq = clk_get_rate(&clk); +#endif /* calculate prescaler and divisor values */ if ((freq / pres / (16 + 1)) > speed) /* set prescaler to 512 */ @@ -67,6 +83,7 @@ static void i2c_ch_init(struct s3c24x0_i2c *i2c, int speed, int slaveadd) writel(slaveadd, &i2c->iicadd); /* program Master Transmit (and implicit STOP) */ writel(I2C_MODE_MT | I2C_TXRX_ENA, &i2c->iicstat); + return 0; }
#define SYS_I2C_S3C24X0_SLAVE_ADDR 0 @@ -77,8 +94,9 @@ static int s3c24x0_i2c_set_bus_speed(struct udevice *dev, unsigned int speed)
i2c_bus->clock_frequency = speed;
- i2c_ch_init(i2c_bus->regs, i2c_bus->clock_frequency, - SYS_I2C_S3C24X0_SLAVE_ADDR); + if (i2c_ch_init(dev, i2c_bus->clock_frequency, + SYS_I2C_S3C24X0_SLAVE_ADDR)) + return -EFAULT;
return 0; } @@ -293,7 +311,9 @@ static int s3c24x0_i2c_xfer(struct udevice *dev, struct i2c_msg *msg,
static int s3c_i2c_of_to_plat(struct udevice *dev) { +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) const void *blob = gd->fdt_blob; +#endif struct s3c24x0_i2c_bus *i2c_bus = dev_get_priv(dev); int node;
@@ -301,7 +321,9 @@ static int s3c_i2c_of_to_plat(struct udevice *dev)
i2c_bus->regs = dev_read_addr_ptr(dev);
+#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) i2c_bus->id = pinmux_decode_periph_id(blob, node); +#endif
i2c_bus->clock_frequency = dev_read_u32_default(dev, "clock-frequency", @@ -309,7 +331,9 @@ static int s3c_i2c_of_to_plat(struct udevice *dev) i2c_bus->node = node; i2c_bus->bus_num = dev_seq(dev);
+#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) exynos_pinmux_config(i2c_bus->id, 0); +#endif
i2c_bus->active = true;
diff --git a/drivers/i2c/s3c24x0_i2c.h b/drivers/i2c/s3c24x0_i2c.h index ec8f1acaef..12249d5c14 100644 --- a/drivers/i2c/s3c24x0_i2c.h +++ b/drivers/i2c/s3c24x0_i2c.h @@ -54,7 +54,9 @@ struct s3c24x0_i2c_bus { struct exynos5_hsi2c *hsregs; int is_highspeed; /* High speed type, rather than I2C */ unsigned clock_frequency; +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) int id; +#endif unsigned clk_cycle; unsigned clk_div; };
Hi David,
On Fri, Aug 02, 2024 at 09:19:16PM +0200, David Virag wrote:
Newer Samsung SoCs (including newer Exynos, ExynosAuto, Google Tensor) still use these IPs, or slightly newer versions of it.
Make these drivers available on these platforms by guarding EXYNOS4/EXYNOS5 specific code behind their configs, and using CCF for clocks on other platforms.
Tested S3C I2C driver on Exynos7885. This along with extended clock driver should enable S3C I2C on Exynos850.
Signed-off-by: David Virag virag.david003@gmail.com
Tested-by: Henrik Grimler henrik@grimler.se
Tested on exynos4412-odroid-u2 and exynos5422-odroid-xu4. On both devices i2c still works fine, thanks!
Best regards, Henrik Grimler
drivers/i2c/Kconfig | 2 +- drivers/i2c/exynos_hs_i2c.c | 25 +++++++++++++++++++++++-- drivers/i2c/s3c24x0_i2c.c | 30 +++++++++++++++++++++++++++--- drivers/i2c/s3c24x0_i2c.h | 2 ++ 4 files changed, 53 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig index cba7f84894..52067fa7c1 100644 --- a/drivers/i2c/Kconfig +++ b/drivers/i2c/Kconfig @@ -650,7 +650,7 @@ config SYS_I2C_GENI
config SYS_I2C_S3C24X0 bool "Samsung I2C driver"
- depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && DM_I2C
- depends on DM_I2C help Support for Samsung I2C controller as Samsung SoCs.
diff --git a/drivers/i2c/exynos_hs_i2c.c b/drivers/i2c/exynos_hs_i2c.c index 189ce6d509..fa0d1c8f64 100644 --- a/drivers/i2c/exynos_hs_i2c.c +++ b/drivers/i2c/exynos_hs_i2c.c @@ -9,11 +9,15 @@ #include <dm.h> #include <i2c.h> #include <log.h> +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) #include <asm/arch/clk.h> #include <asm/arch/cpu.h> #include <asm/arch/pinmux.h> +#endif #include <asm/global_data.h> +#include <asm/io.h> #include <linux/delay.h> +#include <clk.h> #include "s3c24x0_i2c.h"
DECLARE_GLOBAL_DATA_PTR; @@ -137,15 +141,26 @@ static int hsi2c_wait_for_trx(struct exynos5_hsi2c *i2c) return I2C_NOK_TOUT; }
-static int hsi2c_get_clk_details(struct s3c24x0_i2c_bus *i2c_bus) +static int hsi2c_get_clk_details(struct udevice *dev) {
- struct s3c24x0_i2c_bus *i2c_bus = dev_get_priv(dev); struct exynos5_hsi2c *hsregs = i2c_bus->hsregs; ulong clkin; unsigned int op_clk = i2c_bus->clock_frequency; unsigned int i = 0, utemp0 = 0, utemp1 = 0; unsigned int t_ftl_cycle;
+#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) clkin = get_i2c_clk(); +#else
- struct clk clk;
- int ret;
- ret = clk_get_by_name(dev, "hsi2c", &clk);
- if (ret < 0)
return ret;- clkin = clk_get_rate(&clk);
+#endif /* FPCLK / FI2C = * (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2) + 8 + 2 * FLT_CYCLE * uTemp0 = (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2) @@ -487,7 +502,7 @@ static int s3c24x0_i2c_set_bus_speed(struct udevice *dev, unsigned int speed)
i2c_bus->clock_frequency = speed;
- if (hsi2c_get_clk_details(i2c_bus))
- if (hsi2c_get_clk_details(dev)) return -EFAULT; hsi2c_ch_init(i2c_bus);
@@ -514,7 +529,9 @@ static int s3c24x0_i2c_probe(struct udevice *dev, uint chip, uint chip_flags)
static int s3c_i2c_of_to_plat(struct udevice *dev) { +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) const void *blob = gd->fdt_blob; +#endif struct s3c24x0_i2c_bus *i2c_bus = dev_get_priv(dev); int node;
@@ -522,7 +539,9 @@ static int s3c_i2c_of_to_plat(struct udevice *dev)
i2c_bus->hsregs = dev_read_addr_ptr(dev);
+#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) i2c_bus->id = pinmux_decode_periph_id(blob, node); +#endif
i2c_bus->clock_frequency = dev_read_u32_default(dev, "clock-frequency", @@ -530,7 +549,9 @@ static int s3c_i2c_of_to_plat(struct udevice *dev) i2c_bus->node = node; i2c_bus->bus_num = dev_seq(dev);
+#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) exynos_pinmux_config(i2c_bus->id, PINMUX_FLAG_HS_MODE); +#endif
i2c_bus->active = true;
diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c index ae3a801cad..ade1ad6cef 100644 --- a/drivers/i2c/s3c24x0_i2c.c +++ b/drivers/i2c/s3c24x0_i2c.c @@ -9,12 +9,15 @@ #include <fdtdec.h> #include <time.h> #include <log.h> +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) #include <asm/arch/clk.h> #include <asm/arch/cpu.h> #include <asm/arch/pinmux.h> +#endif #include <asm/global_data.h> #include <asm/io.h> #include <i2c.h> +#include <clk.h> #include "s3c24x0_i2c.h"
DECLARE_GLOBAL_DATA_PTR; @@ -46,10 +49,23 @@ static void read_write_byte(struct s3c24x0_i2c *i2c) clrbits_le32(&i2c->iiccon, I2CCON_IRPND); }
-static void i2c_ch_init(struct s3c24x0_i2c *i2c, int speed, int slaveadd) +static int i2c_ch_init(struct udevice *dev, int speed, int slaveadd) {
- struct s3c24x0_i2c_bus *i2c_bus = dev_get_priv(dev);
- struct s3c24x0_i2c *i2c = i2c_bus->regs; ulong freq, pres = 16, div;
+#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || defined(CONFIG_ARCH_EXYNOS5) freq = get_i2c_clk(); +#else
- struct clk clk;
- int ret;
- ret = clk_get_by_name(dev, "i2c", &clk);
- if (ret < 0)
return ret;- freq = clk_get_rate(&clk);
+#endif /* calculate prescaler and divisor values */ if ((freq / pres / (16 + 1)) > speed) /* set prescaler to 512 */ @@ -67,6 +83,7 @@ static void i2c_ch_init(struct s3c24x0_i2c *i2c, int speed, int slaveadd) writel(slaveadd, &i2c->iicadd); /* program Master Transmit (and implicit STOP) */ writel(I2C_MODE_MT | I2C_TXRX_ENA, &i2c->iicstat);
- return 0;
}
#define SYS_I2C_S3C24X0_SLAVE_ADDR 0 @@ -77,8 +94,9 @@ static int s3c24x0_i2c_set_bus_speed(struct udevice *dev, unsigned int speed)
i2c_bus->clock_frequency = speed;
- i2c_ch_init(i2c_bus->regs, i2c_bus->clock_frequency,
SYS_I2C_S3C24X0_SLAVE_ADDR);
if (i2c_ch_init(dev, i2c_bus->clock_frequency,
SYS_I2C_S3C24X0_SLAVE_ADDR))return -EFAULT;return 0;
} @@ -293,7 +311,9 @@ static int s3c24x0_i2c_xfer(struct udevice *dev, struct i2c_msg *msg,
static int s3c_i2c_of_to_plat(struct udevice *dev) { +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) const void *blob = gd->fdt_blob; +#endif struct s3c24x0_i2c_bus *i2c_bus = dev_get_priv(dev); int node;
@@ -301,7 +321,9 @@ static int s3c_i2c_of_to_plat(struct udevice *dev)
i2c_bus->regs = dev_read_addr_ptr(dev);
+#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) i2c_bus->id = pinmux_decode_periph_id(blob, node); +#endif
i2c_bus->clock_frequency = dev_read_u32_default(dev, "clock-frequency", @@ -309,7 +331,9 @@ static int s3c_i2c_of_to_plat(struct udevice *dev) i2c_bus->node = node; i2c_bus->bus_num = dev_seq(dev);
+#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) exynos_pinmux_config(i2c_bus->id, 0); +#endif
i2c_bus->active = true;
diff --git a/drivers/i2c/s3c24x0_i2c.h b/drivers/i2c/s3c24x0_i2c.h index ec8f1acaef..12249d5c14 100644 --- a/drivers/i2c/s3c24x0_i2c.h +++ b/drivers/i2c/s3c24x0_i2c.h @@ -54,7 +54,9 @@ struct s3c24x0_i2c_bus { struct exynos5_hsi2c *hsregs; int is_highspeed; /* High speed type, rather than I2C */ unsigned clock_frequency; +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) int id; +#endif unsigned clk_cycle; unsigned clk_div; }; -- 2.46.0
Hello David,
On 02.08.24 21:19, David Virag wrote:
Newer Samsung SoCs (including newer Exynos, ExynosAuto, Google Tensor) still use these IPs, or slightly newer versions of it.
Make these drivers available on these platforms by guarding EXYNOS4/EXYNOS5 specific code behind their configs, and using CCF for clocks on other platforms.
Tested S3C I2C driver on Exynos7885. This along with extended clock driver should enable S3C I2C on Exynos850.
Signed-off-by: David Virag virag.david003@gmail.com
drivers/i2c/Kconfig | 2 +- drivers/i2c/exynos_hs_i2c.c | 25 +++++++++++++++++++++++-- drivers/i2c/s3c24x0_i2c.c | 30 +++++++++++++++++++++++++++--- drivers/i2c/s3c24x0_i2c.h | 2 ++ 4 files changed, 53 insertions(+), 6 deletions(-)
Reviewed-by: Heiko Schocher hs@denx.de
Thanks!
bye, Heiko
Hi David,
Thinking about this a bit more...
On Fri, Aug 02, 2024 at 09:19:16PM +0200, David Virag wrote:
Newer Samsung SoCs (including newer Exynos, ExynosAuto, Google Tensor) still use these IPs, or slightly newer versions of it.
Make these drivers available on these platforms by guarding EXYNOS4/EXYNOS5 specific code behind their configs, and using CCF for clocks on other platforms.
Tested S3C I2C driver on Exynos7885. This along with extended clock driver should enable S3C I2C on Exynos850.
Signed-off-by: David Virag virag.david003@gmail.com
drivers/i2c/Kconfig | 2 +- drivers/i2c/exynos_hs_i2c.c | 25 +++++++++++++++++++++++-- drivers/i2c/s3c24x0_i2c.c | 30 +++++++++++++++++++++++++++--- drivers/i2c/s3c24x0_i2c.h | 2 ++ 4 files changed, 53 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig index cba7f84894..52067fa7c1 100644 --- a/drivers/i2c/Kconfig +++ b/drivers/i2c/Kconfig @@ -650,7 +650,7 @@ config SYS_I2C_GENI
config SYS_I2C_S3C24X0 bool "Samsung I2C driver"
- depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && DM_I2C
- depends on DM_I2C help Support for Samsung I2C controller as Samsung SoCs.
diff --git a/drivers/i2c/exynos_hs_i2c.c b/drivers/i2c/exynos_hs_i2c.c index 189ce6d509..fa0d1c8f64 100644 --- a/drivers/i2c/exynos_hs_i2c.c +++ b/drivers/i2c/exynos_hs_i2c.c @@ -9,11 +9,15 @@ #include <dm.h> #include <i2c.h> #include <log.h> +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) #include <asm/arch/clk.h> #include <asm/arch/cpu.h> #include <asm/arch/pinmux.h> +#endif
We want to try to move the Exynos 4 and 5 devices to CCF and OF_UPSTREAM as well, which will make this messy. Can we check CONFIG_CLK_EXYNOS and CONFIG_PINCTRL_EXYNOS instead?
#include <asm/global_data.h> +#include <asm/io.h> #include <linux/delay.h> +#include <clk.h> #include "s3c24x0_i2c.h"
DECLARE_GLOBAL_DATA_PTR; @@ -137,15 +141,26 @@ static int hsi2c_wait_for_trx(struct exynos5_hsi2c *i2c) return I2C_NOK_TOUT; }
-static int hsi2c_get_clk_details(struct s3c24x0_i2c_bus *i2c_bus) +static int hsi2c_get_clk_details(struct udevice *dev) {
- struct s3c24x0_i2c_bus *i2c_bus = dev_get_priv(dev); struct exynos5_hsi2c *hsregs = i2c_bus->hsregs; ulong clkin; unsigned int op_clk = i2c_bus->clock_frequency; unsigned int i = 0, utemp0 = 0, utemp1 = 0; unsigned int t_ftl_cycle;
+#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5)
As above, can we check CONFIG_CLK_EXYNOS instead?
clkin = get_i2c_clk(); +#else
- struct clk clk;
- int ret;
- ret = clk_get_by_name(dev, "hsi2c", &clk);
- if (ret < 0)
return ret;- clkin = clk_get_rate(&clk);
+#endif /* FPCLK / FI2C = * (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2) + 8 + 2 * FLT_CYCLE * uTemp0 = (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2) @@ -487,7 +502,7 @@ static int s3c24x0_i2c_set_bus_speed(struct udevice *dev, unsigned int speed)
i2c_bus->clock_frequency = speed;
- if (hsi2c_get_clk_details(i2c_bus))
- if (hsi2c_get_clk_details(dev)) return -EFAULT; hsi2c_ch_init(i2c_bus);
@@ -514,7 +529,9 @@ static int s3c24x0_i2c_probe(struct udevice *dev, uint chip, uint chip_flags)
static int s3c_i2c_of_to_plat(struct udevice *dev) { +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) const void *blob = gd->fdt_blob; +#endif struct s3c24x0_i2c_bus *i2c_bus = dev_get_priv(dev); int node;
@@ -522,7 +539,9 @@ static int s3c_i2c_of_to_plat(struct udevice *dev)
i2c_bus->hsregs = dev_read_addr_ptr(dev);
+#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) i2c_bus->id = pinmux_decode_periph_id(blob, node); +#endif
i2c_bus->clock_frequency = dev_read_u32_default(dev, "clock-frequency", @@ -530,7 +549,9 @@ static int s3c_i2c_of_to_plat(struct udevice *dev) i2c_bus->node = node; i2c_bus->bus_num = dev_seq(dev);
+#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) exynos_pinmux_config(i2c_bus->id, PINMUX_FLAG_HS_MODE); +#endif
I think these three #if def's can be consolidated in one #if def group towards the end of s3c_i2c_of_to_plat to make it easier to follow.
Also, can we check CONFIG_PINCTRL_EXYNOS instead? That would make it easier when migrating Exynos 4 and 5 devices to pinctrl driver and OF_UPSTREAM.
Same comments apply to s3c24x0_i2c.c below.
Best regards, Henrik Grimler
i2c_bus->active = true;
diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c index ae3a801cad..ade1ad6cef 100644 --- a/drivers/i2c/s3c24x0_i2c.c +++ b/drivers/i2c/s3c24x0_i2c.c @@ -9,12 +9,15 @@ #include <fdtdec.h> #include <time.h> #include <log.h> +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) #include <asm/arch/clk.h> #include <asm/arch/cpu.h> #include <asm/arch/pinmux.h> +#endif #include <asm/global_data.h> #include <asm/io.h> #include <i2c.h> +#include <clk.h> #include "s3c24x0_i2c.h"
DECLARE_GLOBAL_DATA_PTR; @@ -46,10 +49,23 @@ static void read_write_byte(struct s3c24x0_i2c *i2c) clrbits_le32(&i2c->iiccon, I2CCON_IRPND); }
-static void i2c_ch_init(struct s3c24x0_i2c *i2c, int speed, int slaveadd) +static int i2c_ch_init(struct udevice *dev, int speed, int slaveadd) {
- struct s3c24x0_i2c_bus *i2c_bus = dev_get_priv(dev);
- struct s3c24x0_i2c *i2c = i2c_bus->regs; ulong freq, pres = 16, div;
+#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || defined(CONFIG_ARCH_EXYNOS5) freq = get_i2c_clk(); +#else
- struct clk clk;
- int ret;
- ret = clk_get_by_name(dev, "i2c", &clk);
- if (ret < 0)
return ret;- freq = clk_get_rate(&clk);
+#endif /* calculate prescaler and divisor values */ if ((freq / pres / (16 + 1)) > speed) /* set prescaler to 512 */ @@ -67,6 +83,7 @@ static void i2c_ch_init(struct s3c24x0_i2c *i2c, int speed, int slaveadd) writel(slaveadd, &i2c->iicadd); /* program Master Transmit (and implicit STOP) */ writel(I2C_MODE_MT | I2C_TXRX_ENA, &i2c->iicstat);
- return 0;
}
#define SYS_I2C_S3C24X0_SLAVE_ADDR 0 @@ -77,8 +94,9 @@ static int s3c24x0_i2c_set_bus_speed(struct udevice *dev, unsigned int speed)
i2c_bus->clock_frequency = speed;
- i2c_ch_init(i2c_bus->regs, i2c_bus->clock_frequency,
SYS_I2C_S3C24X0_SLAVE_ADDR);
if (i2c_ch_init(dev, i2c_bus->clock_frequency,
SYS_I2C_S3C24X0_SLAVE_ADDR))return -EFAULT;return 0;
} @@ -293,7 +311,9 @@ static int s3c24x0_i2c_xfer(struct udevice *dev, struct i2c_msg *msg,
static int s3c_i2c_of_to_plat(struct udevice *dev) { +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) const void *blob = gd->fdt_blob; +#endif struct s3c24x0_i2c_bus *i2c_bus = dev_get_priv(dev); int node;
@@ -301,7 +321,9 @@ static int s3c_i2c_of_to_plat(struct udevice *dev)
i2c_bus->regs = dev_read_addr_ptr(dev);
+#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) i2c_bus->id = pinmux_decode_periph_id(blob, node); +#endif
i2c_bus->clock_frequency = dev_read_u32_default(dev, "clock-frequency", @@ -309,7 +331,9 @@ static int s3c_i2c_of_to_plat(struct udevice *dev) i2c_bus->node = node; i2c_bus->bus_num = dev_seq(dev);
+#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) exynos_pinmux_config(i2c_bus->id, 0); +#endif
i2c_bus->active = true;
diff --git a/drivers/i2c/s3c24x0_i2c.h b/drivers/i2c/s3c24x0_i2c.h index ec8f1acaef..12249d5c14 100644 --- a/drivers/i2c/s3c24x0_i2c.h +++ b/drivers/i2c/s3c24x0_i2c.h @@ -54,7 +54,9 @@ struct s3c24x0_i2c_bus { struct exynos5_hsi2c *hsregs; int is_highspeed; /* High speed type, rather than I2C */ unsigned clock_frequency; +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) int id; +#endif unsigned clk_cycle; unsigned clk_div; }; -- 2.46.0
Hi Henrik,
On Fri, 2024-08-09 at 23:08 +0200, Henrik Grimler wrote:
Hi David,
Thinking about this a bit more...
On Fri, Aug 02, 2024 at 09:19:16PM +0200, David Virag wrote:
[snip]
@@ -9,11 +9,15 @@ #include <dm.h> #include <i2c.h> #include <log.h> +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) #include <asm/arch/clk.h> #include <asm/arch/cpu.h> #include <asm/arch/pinmux.h> +#endif
We want to try to move the Exynos 4 and 5 devices to CCF and OF_UPSTREAM as well, which will make this messy. Can we check CONFIG_CLK_EXYNOS and CONFIG_PINCTRL_EXYNOS instead?
Happy to hear that move, but you are a bit late, this got merged into master not long ago. Sending a patch for changing it should be fine, the 7885 port I'm working on uses mostly the same things as the E850- 96board (just has some support for 2 boards right now, though I may submit jackpotlte only first, will see)
#include <asm/global_data.h> +#include <asm/io.h> #include <linux/delay.h> +#include <clk.h> #include "s3c24x0_i2c.h" DECLARE_GLOBAL_DATA_PTR; @@ -137,15 +141,26 @@ static int hsi2c_wait_for_trx(struct exynos5_hsi2c *i2c) return I2C_NOK_TOUT; } -static int hsi2c_get_clk_details(struct s3c24x0_i2c_bus *i2c_bus) +static int hsi2c_get_clk_details(struct udevice *dev) {
- struct s3c24x0_i2c_bus *i2c_bus = dev_get_priv(dev);
struct exynos5_hsi2c *hsregs = i2c_bus->hsregs; ulong clkin; unsigned int op_clk = i2c_bus->clock_frequency; unsigned int i = 0, utemp0 = 0, utemp1 = 0; unsigned int t_ftl_cycle; +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5)
As above, can we check CONFIG_CLK_EXYNOS instead?
Should be fine
clkin = get_i2c_clk(); +#else
- struct clk clk;
- int ret;
- ret = clk_get_by_name(dev, "hsi2c", &clk);
- if (ret < 0)
return ret;- clkin = clk_get_rate(&clk);
+#endif /* FPCLK / FI2C = * (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2) + 8 + 2 * FLT_CYCLE * uTemp0 = (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2) @@ -487,7 +502,7 @@ static int s3c24x0_i2c_set_bus_speed(struct udevice *dev, unsigned int speed) i2c_bus->clock_frequency = speed;
- if (hsi2c_get_clk_details(i2c_bus))
- if (hsi2c_get_clk_details(dev))
return -EFAULT; hsi2c_ch_init(i2c_bus); @@ -514,7 +529,9 @@ static int s3c24x0_i2c_probe(struct udevice *dev, uint chip, uint chip_flags) static int s3c_i2c_of_to_plat(struct udevice *dev) { +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) const void *blob = gd->fdt_blob; +#endif struct s3c24x0_i2c_bus *i2c_bus = dev_get_priv(dev); int node; @@ -522,7 +539,9 @@ static int s3c_i2c_of_to_plat(struct udevice *dev) i2c_bus->hsregs = dev_read_addr_ptr(dev); +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) i2c_bus->id = pinmux_decode_periph_id(blob, node); +#endif i2c_bus->clock_frequency = dev_read_u32_default(dev, "clock-frequency", @@ -530,7 +549,9 @@ static int s3c_i2c_of_to_plat(struct udevice *dev) i2c_bus->node = node; i2c_bus->bus_num = dev_seq(dev); +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) exynos_pinmux_config(i2c_bus->id, PINMUX_FLAG_HS_MODE); +#endif
I think these three #if def's can be consolidated in one #if def group towards the end of s3c_i2c_of_to_plat to make it easier to follow.
Hm, sounds good, though I don't know if moving the variable declaration from the top would work. I don't know which C standard uboot is compiled with, but I do know that some don't like that. If it works, it works.
Also, can we check CONFIG_PINCTRL_EXYNOS instead? That would make it easier when migrating Exynos 4 and 5 devices to pinctrl driver and OF_UPSTREAM.
Yeah, that works too. Only reason I used EXYNOS4/5 is that that's really what it directly depends on, and I didn't know it was planned to move Exynos4/5 specific stuff to CCF/pinctrl.
My only real nitpick is that these changes would mean we'd have to reintroduce the dependency on exynos platforms, since if we say "if we don't have exynos ccf driver, use exynos specific stuff", then if we just add the driver to some other non-exynos platform without exynos ccf driver enabled for some reason, it would not compile, since it'd be trying to use the old exynos4/5 stuff. Though for now it should be fine if we limit it to Exynos only.
Same comments apply to s3c24x0_i2c.c below.
Best regards, Henrik Grimler
Best regards, David
Hi David,
On Sat, Aug 10, 2024 at 01:05:06AM +0200, David Virag wrote:
Hi Henrik,
On Fri, 2024-08-09 at 23:08 +0200, Henrik Grimler wrote:
Hi David,
Thinking about this a bit more...
On Fri, Aug 02, 2024 at 09:19:16PM +0200, David Virag wrote:
[snip]
@@ -9,11 +9,15 @@ #include <dm.h> #include <i2c.h> #include <log.h> +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) #include <asm/arch/clk.h> #include <asm/arch/cpu.h> #include <asm/arch/pinmux.h> +#endif
We want to try to move the Exynos 4 and 5 devices to CCF and OF_UPSTREAM as well, which will make this messy. Can we check CONFIG_CLK_EXYNOS and CONFIG_PINCTRL_EXYNOS instead?
Happy to hear that move, but you are a bit late, this got merged into master not long ago. Sending a patch for changing it should be fine, the 7885 port I'm working on uses mostly the same things as the E850- 96board (just has some support for 2 boards right now, though I may submit jackpotlte only first, will see)
Ah, oops, missed that! Yeah, no problem, I will touch it in an upcoming patchset.
#include <asm/global_data.h> +#include <asm/io.h> #include <linux/delay.h> +#include <clk.h> #include "s3c24x0_i2c.h" DECLARE_GLOBAL_DATA_PTR; @@ -137,15 +141,26 @@ static int hsi2c_wait_for_trx(struct exynos5_hsi2c *i2c) return I2C_NOK_TOUT; } -static int hsi2c_get_clk_details(struct s3c24x0_i2c_bus *i2c_bus) +static int hsi2c_get_clk_details(struct udevice *dev) {
- struct s3c24x0_i2c_bus *i2c_bus = dev_get_priv(dev);
struct exynos5_hsi2c *hsregs = i2c_bus->hsregs; ulong clkin; unsigned int op_clk = i2c_bus->clock_frequency; unsigned int i = 0, utemp0 = 0, utemp1 = 0; unsigned int t_ftl_cycle; +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5)
As above, can we check CONFIG_CLK_EXYNOS instead?
Should be fine
clkin = get_i2c_clk(); +#else
- struct clk clk;
- int ret;
- ret = clk_get_by_name(dev, "hsi2c", &clk);
- if (ret < 0)
return ret;- clkin = clk_get_rate(&clk);
+#endif /* FPCLK / FI2C = * (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2) + 8 + 2 * FLT_CYCLE * uTemp0 = (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2) @@ -487,7 +502,7 @@ static int s3c24x0_i2c_set_bus_speed(struct udevice *dev, unsigned int speed) i2c_bus->clock_frequency = speed;
- if (hsi2c_get_clk_details(i2c_bus))
- if (hsi2c_get_clk_details(dev))
return -EFAULT; hsi2c_ch_init(i2c_bus); @@ -514,7 +529,9 @@ static int s3c24x0_i2c_probe(struct udevice *dev, uint chip, uint chip_flags) static int s3c_i2c_of_to_plat(struct udevice *dev) { +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) const void *blob = gd->fdt_blob; +#endif struct s3c24x0_i2c_bus *i2c_bus = dev_get_priv(dev); int node; @@ -522,7 +539,9 @@ static int s3c_i2c_of_to_plat(struct udevice *dev) i2c_bus->hsregs = dev_read_addr_ptr(dev); +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) i2c_bus->id = pinmux_decode_periph_id(blob, node); +#endif i2c_bus->clock_frequency = dev_read_u32_default(dev, "clock-frequency", @@ -530,7 +549,9 @@ static int s3c_i2c_of_to_plat(struct udevice *dev) i2c_bus->node = node; i2c_bus->bus_num = dev_seq(dev); +#if IS_ENABLED(CONFIG_ARCH_EXYNOS4) || IS_ENABLED(CONFIG_ARCH_EXYNOS5) exynos_pinmux_config(i2c_bus->id, PINMUX_FLAG_HS_MODE); +#endif
I think these three #if def's can be consolidated in one #if def group towards the end of s3c_i2c_of_to_plat to make it easier to follow.
Hm, sounds good, though I don't know if moving the variable declaration from the top would work. I don't know which C standard uboot is compiled with, but I do know that some don't like that. If it works, it works.
We can probably drop "const void *blob" altogether and use gd->fdt_blob straight away.
Also, can we check CONFIG_PINCTRL_EXYNOS instead? That would make it easier when migrating Exynos 4 and 5 devices to pinctrl driver and OF_UPSTREAM.
Yeah, that works too. Only reason I used EXYNOS4/5 is that that's really what it directly depends on, and I didn't know it was planned to move Exynos4/5 specific stuff to CCF/pinctrl.
I am working on migrating exynos4412-odroid-u2 and exyno5422-odroid-xu4, as well as adding support for exynos4210-i9100.
My only real nitpick is that these changes would mean we'd have to reintroduce the dependency on exynos platforms, since if we say "if we don't have exynos ccf driver, use exynos specific stuff", then if we just add the driver to some other non-exynos platform without exynos ccf driver enabled for some reason, it would not compile, since it'd be trying to use the old exynos4/5 stuff. Though for now it should be fine if we limit it to Exynos only.
I guess only exynos and older s3c/s5p devices will ever use the driver, based on current situation in Linux at least. Even if we fix compilation for a none-exynos4/5 device without CCF I suppose driver would not work without modifications to deal with clocks.
Anyways, thanks! Will CC you on future changes!
Best regards, Henrik Grimler
participants (3)
-
David Virag -
Heiko Schocher -
Henrik Grimler