Hi Heiko,
On 19.05.20 08:07, Heiko Schocher wrote:
Hello Stefan,
Am 14.05.2020 um 09:23 schrieb Stefan Roese:
From: Suneel Garapati <sgarap...@marvell.com>
Add support for I2C controllers found on Octeon II/III and Octeon TX
TX2 SoC platforms.
Signed-off-by: Aaron Williams <awilli...@marvell.com>
Signed-off-by: Suneel Garapati <sgarap...@marvell.com>
Signed-off-by: Stefan Roese <s...@denx.de>
Cc: Heiko Schocher <h...@denx.de>
Cc: Simon Glass <s...@chromium.org>
Cc: Daniel Schwierzeck <daniel.schwierz...@gmail.com>
Cc: Aaron Williams <awilli...@marvell.com>
Cc: Chandrakala Chavva <ccha...@marvell.com>
---
RFC -> v1 (Stefan):
- Separated this patch from the OcteonTX/TX2 RFC patch series into a
single patch. This is useful, as the upcoming MIPS Octeon support will
use this I2C driver.
- Added MIPS Octeon II/III support (big endian). Rename driver and its
function names from "octeontx" to "octeon" to better match all Octeon
platforms.
- Moved from union to defines / bitmasks as suggested by Simon. This
makes
the driver usage on little- and big-endian platforms much easier.
- Enhanced Kconfig text
- Removed all clock macros (use values from DT)
- Removed long driver debug strings. This is only available when a debug
version of this driver is built. The user / developer can lookup the
descriptive error messages in the driver in this case anyway.
- Removed static "last_id"
- Dropped misc blank lines. Misc reformatting.
- Dropped "!= 0"
- Added missing function comments
- Added missing strut comments
- Changed comment style
- Renames "result" to "ret"
- Hex numbers uppercase
- Minor other changes
- Reword commit text and subject
drivers/i2c/Kconfig | 10 +
drivers/i2c/Makefile | 1 +
drivers/i2c/octeon_i2c.c | 803 +++++++++++++++++++++++++++++++++++++++
3 files changed, 814 insertions(+)
create mode 100644 drivers/i2c/octeon_i2c.c
nitpick only ...
Please add a documentation of the device tree bindings.
Okay.
[...]
diff --git a/drivers/i2c/octeon_i2c.c b/drivers/i2c/octeon_i2c.c
new file mode 100644
index 0000000000..210f98655e
--- /dev/null
+++ b/drivers/i2c/octeon_i2c.c
@@ -0,0 +1,803 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Marvell International Ltd.
+ */
+
+#include <common.h>
+#include <i2c.h>
+#include <dm.h>
+#include <pci_ids.h>
+#include <asm/io.h>
+#include <asm/arch/clock.h>
+#include <linux/bitfield.h>
+
+/*
+ * Octeon II/III (MIPS) have different register offsets than the ARM
based
+ * Octeon TX/TX2 SoCs
+ */
+#if defined(CONFIG_ARCH_OCTEON)
+#define REG_OFFS 0x0000
+#else
+#define REG_OFFS 0x1000
+#endif
+
+#define TWSI_SW_TWSI (REG_OFFS + 0x00)
+#define TWSI_TWSI_SW (REG_OFFS + 0x08)
Is there no clearer name ?
Just wonderung about "TWSI" .. we already have an i2c driver with TWSI
defines:
https://gitlab.denx.de/u-boot/u-boot/-/blob/master/drivers/i2c/mvtwsi.c
But it seems they have no common parts.
TWSI is another abbreviation for I2C: Two Wire Serial Interface. I
would like to keep these macros as they reflect the register names in
the Cavium / Marvell manuals.
[...]
+#if defined(CONFIG_ARCH_OCTEON)
+static int get_io_clock(void)
+{
+ return octeon_get_io_clock();
+}
+#else
+static int get_io_clock(void)
+{
+ return octeontx_get_io_clock();
+}
+#endif
Here would be good to have the clk framework...
Yes, it would be great. I'm thinking about adding a minimal clk driver
for Octeon - not sure, if I get it done shortly though.
+
+static int twsi_thp(void)
+{
+ if (IS_ENABLED(CONFIG_ARCH_OCTEON) ||
IS_ENABLED(CONFIG_ARCH_OCTEONTX))
+ return 24;
+ else
+ return 3;
+}
May you can add here some comments for this magic numbers?
I didn't write the initial version of this driver, so its hard for
me to come up with details here. But I'll try.
[...]
+#define RST_BOOT_PNR_MUL(val) (((val) >> 33) & 0x1F)
not used define, please remove.
Ah, thanks for spotting.
Thanks,
Stefan
