Re: [PATCH v3 04/14] i2c: add nexell driver
Hello Heiko,
thank you for your proposals. I'll make the appropriate changes.
Regards
Stefan
Am 03.07.20 um 08:03 schrieb Heiko Schocher:
Hello Stefan,
Am 29.06.2020 um 19:46 schrieb Stefan Bosch:
Changes in relation to FriendlyARM's U-Boot nanopi2-v2016.01:
- i2c/nx_i2c.c: Some adaptions mainly because of changes in
"struct udevice".
- several Bugfixes in nx_i2c.c.
- the driver has been for s5p6818 only. Code extended appropriately
in order s5p4418 is also working.
- "probe_chip" added.
- pinctrl-driver/dt is used instead of configuring the i2c I/O-pins
in the i2c-driver.
- '#ifdef CONFIG...' changed to 'if (IS_ENABLED(CONFIG...))' where
possible (and similar).
- livetree API (dev_read_...) is used instead of fdt one (fdt...).
Signed-off-by: Stefan Bosch
Reviewed-by: Heiko Schocher
Nitpick only ...
[...]
diff --git a/drivers/i2c/nx_i2c.c b/drivers/i2c/nx_i2c.c
new file mode 100644
index 000..cefc774
--- /dev/null
+++ b/drivers/i2c/nx_i2c.c
@@ -0,0 +1,624 @@
[...]
+static uint i2c_set_clk(struct nx_i2c_bus *bus, uint enb)
+{
+ struct clk *clk;
+ char name[50];
+
+ sprintf(name, "%s.%d", DEV_NAME_I2C, bus->bus_num);
+ clk = clk_get((const char *)name);
+ if (!clk) {
+ debug("%s(): clk_get(%s) error!\n",
+ __func__, (const char *)name);
+ return -EINVAL;
+ }
+
+ if (enb) {
+ clk_disable(clk);
+ clk_enable(clk);
+ } else {
+ clk_disable(clk);
+ }
You can do here:
clk_disable(clk);
if (enb)
clk_enable(clk);
+
+ return 0;
+}
+
+#ifdef CONFIG_ARCH_S5P6818
+/* Set SDA line delay, not available at S5P4418 */
+static int nx_i2c_set_sda_delay(struct nx_i2c_bus *bus)
+{
+ struct nx_i2c_regs *i2c = bus->regs;
+ uint pclk = 0;
+ uint t_pclk = 0;
+ uint delay = 0;
+
+ /* get input clock of the I2C-controller */
+ pclk = i2c_get_clkrate(bus);
+
+ if (bus->sda_delay) {
+ /* t_pclk = period time of one pclk [ns] */
+ t_pclk = DIV_ROUND_UP(1000, pclk / 100);
+ /* delay = number of pclks required for sda_delay [ns] */
+ delay = DIV_ROUND_UP(bus->sda_delay, t_pclk);
+ /* delay = register value (step of 5 clocks) */
+ delay = DIV_ROUND_UP(delay, 5);
+ /* max. possible register value = 3 */
+ if (delay > 3) {
May you use defines here?
+ delay = 3;
+ debug("%s(): sda-delay des.: %dns, sat. to max.: %dns
(granularity: %dns)\n",
+ __func__, bus->sda_delay, t_pclk * delay * 5,
+ t_pclk * 5);
+ } else {
+ debug("%s(): sda-delay des.: %dns, act.: %dns
(granularity: %dns)\n",
+ __func__, bus->sda_delay, t_pclk * delay * 5,
+ t_pclk * 5);
+ }
+
+ delay |= I2CLC_FILTER;
+ } else {
+ delay = 0;
+ debug("%s(): sda-delay = 0\n", __func__);
+ }
+
+ delay &= 0x7;
+ writel(delay, &i2c->iiclc);
+
+ return 0;
+}
+#endif
+
+static int nx_i2c_set_bus_speed(struct udevice *dev, uint speed)
+{
+ struct nx_i2c_bus *bus = dev_get_priv(dev);
+ struct nx_i2c_regs *i2c = bus->regs;
+ unsigned long pclk, pres = 16, div;
+
+ if (i2c_set_clk(bus, 1))
+ return -EINVAL;
+
+ /* get input clock of the I2C-controller */
+ pclk = i2c_get_clkrate(bus);
+
+ /* calculate prescaler and divisor values */
+ if ((pclk / pres / (16 + 1)) > speed)
+ /* set prescaler to 256 */
+ pres = 256;
+
+ div = 0;
+ /* actual divider = div + 1 */
+ while ((pclk / pres / (div + 1)) > speed)
+ div++;
+
+ if (div > 0xF) {
+ debug("%s(): pres==%ld, div==0x%lx is saturated to 0xF !)\n",
+ __func__, pres, div);
+ div = 0xF;
+ } else {
+ debug("%s(): pres==%ld, div==0x%lx)\n", __func__, pres, div);
+ }
+
+ /* set prescaler, divisor according to pclk, also set ACKGEN, IRQ */
+ writel((div & 0x0F) | ((pres == 256) ? 0x40 : 0), &i2c->iiccon);
Ok, I am really nitpicking now ... can you use defines here instead
this magic values?
[...]
Thanks!
bye,
Heiko
Re: [PATCH v3 04/14] i2c: add nexell driver
Hello Stefan,
Am 29.06.2020 um 19:46 schrieb Stefan Bosch:
Changes in relation to FriendlyARM's U-Boot nanopi2-v2016.01:
- i2c/nx_i2c.c: Some adaptions mainly because of changes in
"struct udevice".
- several Bugfixes in nx_i2c.c.
- the driver has been for s5p6818 only. Code extended appropriately
in order s5p4418 is also working.
- "probe_chip" added.
- pinctrl-driver/dt is used instead of configuring the i2c I/O-pins
in the i2c-driver.
- '#ifdef CONFIG...' changed to 'if (IS_ENABLED(CONFIG...))' where
possible (and similar).
- livetree API (dev_read_...) is used instead of fdt one (fdt...).
Signed-off-by: Stefan Bosch
Reviewed-by: Heiko Schocher
Nitpick only ...
[...]
diff --git a/drivers/i2c/nx_i2c.c b/drivers/i2c/nx_i2c.c
new file mode 100644
index 000..cefc774
--- /dev/null
+++ b/drivers/i2c/nx_i2c.c
@@ -0,0 +1,624 @@
[...]
+static uint i2c_set_clk(struct nx_i2c_bus *bus, uint enb)
+{
+ struct clk *clk;
+ char name[50];
+
+ sprintf(name, "%s.%d", DEV_NAME_I2C, bus->bus_num);
+ clk = clk_get((const char *)name);
+ if (!clk) {
+ debug("%s(): clk_get(%s) error!\n",
+ __func__, (const char *)name);
+ return -EINVAL;
+ }
+
+ if (enb) {
+ clk_disable(clk);
+ clk_enable(clk);
+ } else {
+ clk_disable(clk);
+ }
You can do here:
clk_disable(clk);
if (enb)
clk_enable(clk);
+
+ return 0;
+}
+
+#ifdef CONFIG_ARCH_S5P6818
+/* Set SDA line delay, not available at S5P4418 */
+static int nx_i2c_set_sda_delay(struct nx_i2c_bus *bus)
+{
+ struct nx_i2c_regs *i2c = bus->regs;
+ uint pclk = 0;
+ uint t_pclk = 0;
+ uint delay = 0;
+
+ /* get input clock of the I2C-controller */
+ pclk = i2c_get_clkrate(bus);
+
+ if (bus->sda_delay) {
+ /* t_pclk = period time of one pclk [ns] */
+ t_pclk = DIV_ROUND_UP(1000, pclk / 100);
+ /* delay = number of pclks required for sda_delay [ns] */
+ delay = DIV_ROUND_UP(bus->sda_delay, t_pclk);
+ /* delay = register value (step of 5 clocks) */
+ delay = DIV_ROUND_UP(delay, 5);
+ /* max. possible register value = 3 */
+ if (delay > 3) {
May you use defines here?
+ delay = 3;
+ debug("%s(): sda-delay des.: %dns, sat. to max.: %dns
(granularity: %dns)\n",
+ __func__, bus->sda_delay, t_pclk * delay * 5,
+ t_pclk * 5);
+ } else {
+ debug("%s(): sda-delay des.: %dns, act.: %dns (granularity:
%dns)\n",
+ __func__, bus->sda_delay, t_pclk * delay * 5,
+ t_pclk * 5);
+ }
+
+ delay |= I2CLC_FILTER;
+ } else {
+ delay = 0;
+ debug("%s(): sda-delay = 0\n", __func__);
+ }
+
+ delay &= 0x7;
+ writel(delay, &i2c->iiclc);
+
+ return 0;
+}
+#endif
+
+static int nx_i2c_set_bus_speed(struct udevice *dev, uint speed)
+{
+ struct nx_i2c_bus *bus = dev_get_priv(dev);
+ struct nx_i2c_regs *i2c = bus->regs;
+ unsigned long pclk, pres = 16, div;
+
+ if (i2c_set_clk(bus, 1))
+ return -EINVAL;
+
+ /* get input clock of the I2C-controller */
+ pclk = i2c_get_clkrate(bus);
+
+ /* calculate prescaler and divisor values */
+ if ((pclk / pres / (16 + 1)) > speed)
+ /* set prescaler to 256 */
+ pres = 256;
+
+ div = 0;
+ /* actual divider = div + 1 */
+ while ((pclk / pres / (div + 1)) > speed)
+ div++;
+
+ if (div > 0xF) {
+ debug("%s(): pres==%ld, div==0x%lx is saturated to 0xF !)\n",
+ __func__, pres, div);
+ div = 0xF;
+ } else {
+ debug("%s(): pres==%ld, div==0x%lx)\n", __func__, pres, div);
+ }
+
+ /* set prescaler, divisor according to pclk, also set ACKGEN, IRQ */
+ writel((div & 0x0F) | ((pres == 256) ? 0x40 : 0), &i2c->iiccon);
Ok, I am really nitpicking now ... can you use defines here instead
this magic values?
[...]
Thanks!
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: h...@denx.de
[PATCH v3 04/14] i2c: add nexell driver
Changes in relation to FriendlyARM's U-Boot nanopi2-v2016.01:
- i2c/nx_i2c.c: Some adaptions mainly because of changes in
"struct udevice".
- several Bugfixes in nx_i2c.c.
- the driver has been for s5p6818 only. Code extended appropriately
in order s5p4418 is also working.
- "probe_chip" added.
- pinctrl-driver/dt is used instead of configuring the i2c I/O-pins
in the i2c-driver.
- '#ifdef CONFIG...' changed to 'if (IS_ENABLED(CONFIG...))' where
possible (and similar).
- livetree API (dev_read_...) is used instead of fdt one (fdt...).
Signed-off-by: Stefan Bosch
---
Changes in v3:
- pinctrl-driver/dt is used now instead of configuring the i2c I/O-pins
in the i2c-driver.
- drivers/i2c/nx_i2c.c: '#include ' inserted because it has been
removed from common.h.
- '#ifdef...' changed to 'if (IS_ENABLED(CONFIG...))' where possible
because of appropriate warnings of patman.
- Changed to livetree API as proposed by patman:
fdtdec_get_int() --> dev_read_s32_default()
Changes in v2:
- commit "i2c: mmc: add nexell driver (gpio, i2c, mmc, pwm)" splitted
into separate commits for gpio, i2c, mmc, pwm.
- several Bugfixes in nx_i2c.c.
- the i2c-driver has been for s5p6818 only. Code extended approriately
in order s5p4418 is also working.
- "probe_chip" added to the i2c-driver.
- doc/device-tree-bindings/i2c/nx_i2c.txt added.
doc/device-tree-bindings/i2c/nx_i2c.txt | 28 ++
drivers/i2c/Kconfig | 9 +
drivers/i2c/Makefile| 1 +
drivers/i2c/nx_i2c.c| 624
4 files changed, 662 insertions(+)
create mode 100644 doc/device-tree-bindings/i2c/nx_i2c.txt
create mode 100644 drivers/i2c/nx_i2c.c
diff --git a/doc/device-tree-bindings/i2c/nx_i2c.txt
b/doc/device-tree-bindings/i2c/nx_i2c.txt
new file mode 100644
index 000..9f3abe7
--- /dev/null
+++ b/doc/device-tree-bindings/i2c/nx_i2c.txt
@@ -0,0 +1,28 @@
+I2C controller embedded in Nexell's/Samsung's SoC S5P4418 and S5P6818
+
+Driver:
+- drivers/i2c/nx_i2c.c
+
+Required properties:
+- #address-cells = <1>;
+- #size-cells = <0>;
+- compatible = "nexell,s5pxx18-i2c";
+- reg = ;
+Where i2c_base has to be the base address of the i2c-register set.
+I2C0: 0xc00a4000
+I2C1: 0xc00a5000
+I2C2: 0xc00a6000
+
+Optional properties:
+- clock-frequency: Desired I2C bus frequency in Hz, default value is 10.
+- i2c-sda-delay-ns (S5P6818 only): SDA delay in ns, default value is 0.
+- Child nodes conforming to i2c bus binding.
+
+Example:
+ i2c0:i2c@c00a4000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "nexell,s5pxx18-i2c";
+ reg = <0xc00a4000 0x100>;
+ clock-frequency = <40>;
+ };
diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index f8b18de..b276f0b 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -325,6 +325,15 @@ config SYS_MXC_I2C8_SLAVE
MXC I2C8 Slave
endif
+config SYS_I2C_NEXELL
+ bool "Nexell I2C driver"
+ depends on DM_I2C
+ help
+ Add support for the Nexell I2C driver. This is used with various
+ Nexell parts such as S5Pxx18 series SoCs. All chips
+ have several I2C ports and all are provided, controlled by the
+ device tree.
+
config SYS_I2C_OMAP24XX
bool "TI OMAP2+ I2C driver"
depends on ARCH_OMAP2PLUS || ARCH_K3
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index 62935b7..6ebc1aa 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_SYS_I2C_LPC32XX) += lpc32xx_i2c.o
obj-$(CONFIG_SYS_I2C_MESON) += meson_i2c.o
obj-$(CONFIG_SYS_I2C_MVTWSI) += mvtwsi.o
obj-$(CONFIG_SYS_I2C_MXC) += mxc_i2c.o
+obj-$(CONFIG_SYS_I2C_NEXELL) += nx_i2c.o
obj-$(CONFIG_SYS_I2C_OMAP24XX) += omap24xx_i2c.o
obj-$(CONFIG_SYS_I2C_RCAR_I2C) += rcar_i2c.o
obj-$(CONFIG_SYS_I2C_RCAR_IIC) += rcar_iic.o
diff --git a/drivers/i2c/nx_i2c.c b/drivers/i2c/nx_i2c.c
new file mode 100644
index 000..cefc774
--- /dev/null
+++ b/drivers/i2c/nx_i2c.c
@@ -0,0 +1,624 @@
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define I2C_WRITE 0
+#define I2C_READ1
+
+#define I2CSTAT_MTM 0xC0/* Master Transmit Mode */
+#define I2CSTAT_MRM 0x80/* Master Receive Mode */
+#define I2CSTAT_BSY 0x20/* Read: Bus Busy */
+#define I2CSTAT_SS 0x20/* Write: START (1) / STOP (0) */
+#define I2CSTAT_RXTXEN 0x10/* Rx/Tx enable */
+#define I2CSTAT_ABT0x08/* Arbitration bit */
+#define I2CSTAT_NACK0x01/* Nack bit */
+#define I2CCON_IRCLR0x100 /* Interrupt Clear bit */
+#define I2CCON_ACKGEN 0x80/* Acknowledge generation */
+#define I2CCON_IRENB 0x20/* Interrupt Enable bit */
+#define I2CCON_IRPND0x10/* Interrupt pending bit */
+
+#ifdef CONFIG_ARCH_S5P6818
+#define I2CLC_FILTER 0x04/*
