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 <stefa...@posteo.net>
Reviewed-by: Heiko Schocher <h...@denx.de>
Nitpick only ...
[...]
diff --git a/drivers/i2c/nx_i2c.c b/drivers/i2c/nx_i2c.c
new file mode 100644
index 0000000..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 / 1000000);
+ /* 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
