Hello Stefan,
Am 14.05.2020 um 09:23 schrieb Stefan Roese:
From: Suneel Garapati sgarapati@marvell.com
Add support for I2C controllers found on Octeon II/III and Octeon TX TX2 SoC platforms.
Signed-off-by: Aaron Williams awilliams@marvell.com Signed-off-by: Suneel Garapati sgarapati@marvell.com Signed-off-by: Stefan Roese sr@denx.de Cc: Heiko Schocher hs@denx.de Cc: Simon Glass sjg@chromium.org Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Aaron Williams awilliams@marvell.com Cc: Chandrakala Chavva cchavva@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.
[...]
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.
[...]
+#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...
+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?
[...]
+#define RST_BOOT_PNR_MUL(val) (((val) >> 33) & 0x1F)
not used define, please remove.
[...]
bye, Heiko