Re: [PATCH 1/2] i2c: Add support for Qualcomm Generic Interface (GENI) I2C controller
On 19/04/2024 13:47, Caleb Connolly wrote:
Hi Neil,
On 18/04/2024 23:47, Neil Armstrong wrote:
Add Support for the Qualcomm Generic Interface (GENI) I2C interface
found on newer Qualcomm SoCs.
\o/
The Generic Interface (GENI) is a firmware based Qualcomm Universal
Peripherals (QUP) Serial Engine (SE) Wrapper which can support multiple
bus protocols depending on the firmware type loaded at early boot time
based on system configuration.
It also supports the "I2C Master Hub" which is a single function Wrapper
that only FIFO mode I2C.
It replaces the fixed-function QUP Wrapper found on older SoCs.
The geni-se.h containing the generic GENI Serial Engine registers defines
is imported from Linux.
Only FIFO mode is implemented, nor SE DMA nor GPI DMA is implemented.
nit: "neither SE DMA nor GPI DMA are implemented"
Thx!
A few minor things below, but otherwise LGTM!
Signed-off-by: Neil Armstrong
---
drivers/i2c/Kconfig| 10 +
drivers/i2c/Makefile | 1 +
drivers/i2c/geni_i2c.c | 576 +
include/soc/qcom/geni-se.h | 265 +
4 files changed, 852 insertions(+)
[...]
diff --git a/drivers/i2c/geni_i2c.c b/drivers/i2c/geni_i2c.c
new file mode 100644
index 000..8c3ed3bef89
--- /dev/null
+++ b/drivers/i2c/geni_i2c.c
@@ -0,0 +1,576 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2024, Linaro Limited
+ * Author: Neil Armstrong
+ *
+ * Based on Linux driver: drivers/i2c/busses/i2c-qcom-geni.c
+ */
+
[...]
+static int geni_i2c_fifo_tx_fill(struct geni_i2c_priv *geni, struct i2c_msg
*msg)
+{
+ ulong start = get_timer(0);
+ ulong cur_xfer = 0;
+ int i;
+
+ while (get_timer(start) < I2C_TIMEOUT_MS) {
+ u32 status = readl(geni->base + SE_GENI_M_IRQ_STATUS);
+
+ if (status & (M_CMD_ABORT_EN |
+ M_CMD_OVERRUN_EN |
+ M_ILLEGAL_CMD_EN |
+ M_CMD_FAILURE_EN |
+ M_GP_IRQ_1_EN |
+ M_GP_IRQ_3_EN |
+ M_GP_IRQ_4_EN)) {
+ debug("%s:%d cmd err\n", __func__, __LINE__);
How likely are we to hit this? Would it make sense to promote it to a
pr_warn()?
Please drop the __LINE__ and (if it makes sense to?) print the value of
status.
It's used when the tranactions is nacked, so it would spam, so I rather remove
the print entirely. It's verly unlikely we see any of the other errors.
+ writel(status, geni->base + SE_GENI_M_IRQ_CLEAR);
+ writel(0, geni->base + SE_GENI_TX_WATERMARK_REG);
+ return -EREMOTEIO;
+ }
+
+ if ((status & M_TX_FIFO_WATERMARK_EN) == 0) {
+ udelay(1);
+ goto skip_fill;
+ }
+
+ for (i = 0; i < geni->tx_wm; i++) {
+ u32 temp, tx = 0;
+ unsigned int p = 0;
+
+ while (cur_xfer < msg->len && p < sizeof(tx)) {
+ temp = msg->buf[cur_xfer++];
+ tx |= temp << (p * 8);
+ p++;
+ }
+
+ writel(tx, geni->base + SE_GENI_TX_FIFOn);
+
+ if (cur_xfer == msg->len) {
+ writel(0, geni->base +
SE_GENI_TX_WATERMARK_REG);
+ break;
+ }
+ }
+
+skip_fill:
+ writel(status, geni->base + SE_GENI_M_IRQ_CLEAR);
+
+ if (status & M_CMD_DONE_EN)
+ return 0;
+ }
+
+ return -ETIMEDOUT;
+}
+
+static int geni_i2c_fifo_rx_drain(struct geni_i2c_priv *geni, struct i2c_msg
*msg)
+{
+ ulong start = get_timer(0);
+ ulong cur_xfer = 0;
+ int i;
+
+ while (get_timer(start) < I2C_TIMEOUT_MS) {
+ u32 status = readl(geni->base + SE_GENI_M_IRQ_STATUS);
+ u32 rxstatus = readl(geni->base + SE_GENI_RX_FIFO_STATUS);
+ u32 rxcnt = rxstatus & RX_FIFO_WC_MSK;
+
+ if (status & (M_CMD_ABORT_EN |
+ M_CMD_FAILURE_EN |
+ M_CMD_OVERRUN_EN |
+ M_ILLEGAL_CMD_EN |
+ M_GP_IRQ_1_EN |
+ M_GP_IRQ_3_EN |
+ M_GP_IRQ_4_EN)) {
+ debug("%s:%d cmd err\n", __func__, __LINE__);
Ditto
+ writel(status, geni->base + SE_GENI_M_IRQ_CLEAR);
+ return -EIO;
+ }
+
+ if ((status & (M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN)) ==
0) {
+ udelay(1);
+ g
Re: [PATCH 1/2] i2c: Add support for Qualcomm Generic Interface (GENI) I2C controller
Hi Neil,
On 18/04/2024 23:47, Neil Armstrong wrote:
> Add Support for the Qualcomm Generic Interface (GENI) I2C interface
> found on newer Qualcomm SoCs.
\o/
>
> The Generic Interface (GENI) is a firmware based Qualcomm Universal
> Peripherals (QUP) Serial Engine (SE) Wrapper which can support multiple
> bus protocols depending on the firmware type loaded at early boot time
> based on system configuration.
>
> It also supports the "I2C Master Hub" which is a single function Wrapper
> that only FIFO mode I2C.
>
> It replaces the fixed-function QUP Wrapper found on older SoCs.
>
> The geni-se.h containing the generic GENI Serial Engine registers defines
> is imported from Linux.
>
> Only FIFO mode is implemented, nor SE DMA nor GPI DMA is implemented.
nit: "neither SE DMA nor GPI DMA are implemented"
A few minor things below, but otherwise LGTM!
>
> Signed-off-by: Neil Armstrong
> ---
> drivers/i2c/Kconfig| 10 +
> drivers/i2c/Makefile | 1 +
> drivers/i2c/geni_i2c.c | 576
> +
> include/soc/qcom/geni-se.h | 265 +
> 4 files changed, 852 insertions(+)
>
[...]
> diff --git a/drivers/i2c/geni_i2c.c b/drivers/i2c/geni_i2c.c
> new file mode 100644
> index 000..8c3ed3bef89
> --- /dev/null
> +++ b/drivers/i2c/geni_i2c.c
> @@ -0,0 +1,576 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2024, Linaro Limited
> + * Author: Neil Armstrong
> + *
> + * Based on Linux driver: drivers/i2c/busses/i2c-qcom-geni.c
> + */
> +
[...]
> +static int geni_i2c_fifo_tx_fill(struct geni_i2c_priv *geni, struct i2c_msg
> *msg)
> +{
> + ulong start = get_timer(0);
> + ulong cur_xfer = 0;
> + int i;
> +
> + while (get_timer(start) < I2C_TIMEOUT_MS) {
> + u32 status = readl(geni->base + SE_GENI_M_IRQ_STATUS);
> +
> + if (status & (M_CMD_ABORT_EN |
> + M_CMD_OVERRUN_EN |
> + M_ILLEGAL_CMD_EN |
> + M_CMD_FAILURE_EN |
> + M_GP_IRQ_1_EN |
> + M_GP_IRQ_3_EN |
> + M_GP_IRQ_4_EN)) {
> + debug("%s:%d cmd err\n", __func__, __LINE__);
How likely are we to hit this? Would it make sense to promote it to a
pr_warn()?
Please drop the __LINE__ and (if it makes sense to?) print the value of
status.
> + writel(status, geni->base + SE_GENI_M_IRQ_CLEAR);
> + writel(0, geni->base + SE_GENI_TX_WATERMARK_REG);
> + return -EREMOTEIO;
> + }
> +
> + if ((status & M_TX_FIFO_WATERMARK_EN) == 0) {
> + udelay(1);
> + goto skip_fill;
> + }
> +
> + for (i = 0; i < geni->tx_wm; i++) {
> + u32 temp, tx = 0;
> + unsigned int p = 0;
> +
> + while (cur_xfer < msg->len && p < sizeof(tx)) {
> + temp = msg->buf[cur_xfer++];
> + tx |= temp << (p * 8);
> + p++;
> + }
> +
> + writel(tx, geni->base + SE_GENI_TX_FIFOn);
> +
> + if (cur_xfer == msg->len) {
> + writel(0, geni->base +
> SE_GENI_TX_WATERMARK_REG);
> + break;
> + }
> + }
> +
> +skip_fill:
> + writel(status, geni->base + SE_GENI_M_IRQ_CLEAR);
> +
> + if (status & M_CMD_DONE_EN)
> + return 0;
> + }
> +
> + return -ETIMEDOUT;
> +}
> +
> +static int geni_i2c_fifo_rx_drain(struct geni_i2c_priv *geni, struct i2c_msg
> *msg)
> +{
> + ulong start = get_timer(0);
> + ulong cur_xfer = 0;
> + int i;
> +
> + while (get_timer(start) < I2C_TIMEOUT_MS) {
> + u32 status = readl(geni->base + SE_GENI_M_IRQ_STATUS);
> + u32 rxstatus = readl(geni->base + SE_GENI_RX_FIFO_STATUS);
> + u32 rxcnt = rxstatus & RX_FIFO_WC_MSK;
> +
> + if (status & (M_CMD_ABORT_EN |
> + M_CMD_FAILURE_EN |
> + M_CMD_OVERRUN_EN |
> + M_ILLEGAL_CMD_EN |
> + M_GP_IRQ_1_EN |
> + M_GP_IRQ_3_EN |
> + M_GP_IRQ_4_EN)) {
> + debug("%s:%d cmd err\n", __func__, __LINE__);
Ditto
> + writel(status, geni->base + SE_GENI_M_IRQ_CLEAR);
> + return -EIO;
> + }
> +
> + if ((status & (M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN)) ==
> 0) {
> + udelay(1);
> + goto skip_drain;
> + }
> +
> + for (i = 0; cur_xfer < msg->len && i <
[PATCH 1/2] i2c: Add support for Qualcomm Generic Interface (GENI) I2C controller
Add Support for the Qualcomm Generic Interface (GENI) I2C interface
found on newer Qualcomm SoCs.
The Generic Interface (GENI) is a firmware based Qualcomm Universal
Peripherals (QUP) Serial Engine (SE) Wrapper which can support multiple
bus protocols depending on the firmware type loaded at early boot time
based on system configuration.
It also supports the "I2C Master Hub" which is a single function Wrapper
that only FIFO mode I2C.
It replaces the fixed-function QUP Wrapper found on older SoCs.
The geni-se.h containing the generic GENI Serial Engine registers defines
is imported from Linux.
Only FIFO mode is implemented, nor SE DMA nor GPI DMA is implemented.
Signed-off-by: Neil Armstrong
---
drivers/i2c/Kconfig| 10 +
drivers/i2c/Makefile | 1 +
drivers/i2c/geni_i2c.c | 576 +
include/soc/qcom/geni-se.h | 265 +
4 files changed, 852 insertions(+)
diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index 59c635af80b..34b02114dc6 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -638,6 +638,16 @@ config SYS_I2C_QUP
Technical Reference Manual, chapter "6.1 Qualcomm Universal
Peripherals Engine (QUP)".
+config SYS_I2C_GENI
+ bool "Qualcomm Generic Interface (GENI) I2C controller"
+ depends on ARCH_SNAPDRAGON
+ help
+ Support for the Qualcomm Generic Interface (GENI) I2C interface.
+ The Generic Interface (GENI) is a firmware based Qualcomm Universal
+ Peripherals (QUP) Serial Engine (SE) Wrapper which can support
multiple
+ bus protocols depending on the firmware type loaded at early boot time
+ based on system configuration.
+
config SYS_I2C_S3C24X0
bool "Samsung I2C driver"
depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && DM_I2C
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index 692f63bafd0..00b90523c62 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_SYS_I2C_DAVINCI) += davinci_i2c.o
obj-$(CONFIG_SYS_I2C_DW) += designware_i2c.o
obj-$(CONFIG_SYS_I2C_DW_PCI) += designware_i2c_pci.o
obj-$(CONFIG_SYS_I2C_FSL) += fsl_i2c.o
+obj-$(CONFIG_SYS_I2C_GENI) += geni_i2c.o
obj-$(CONFIG_SYS_I2C_IHS) += ihs_i2c.o
obj-$(CONFIG_SYS_I2C_INTEL) += intel_i2c.o
obj-$(CONFIG_SYS_I2C_IMX_LPI2C) += imx_lpi2c.o
diff --git a/drivers/i2c/geni_i2c.c b/drivers/i2c/geni_i2c.c
new file mode 100644
index 000..8c3ed3bef89
--- /dev/null
+++ b/drivers/i2c/geni_i2c.c
@@ -0,0 +1,576 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2024, Linaro Limited
+ * Author: Neil Armstrong
+ *
+ * Based on Linux driver: drivers/i2c/busses/i2c-qcom-geni.c
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define SE_I2C_TX_TRANS_LEN0x26c
+#define SE_I2C_RX_TRANS_LEN0x270
+#define SE_I2C_SCL_COUNTERS0x278
+
+#define SE_I2C_ERR (M_CMD_OVERRUN_EN | M_ILLEGAL_CMD_EN | M_CMD_FAILURE_EN |\
+ M_GP_IRQ_1_EN | M_GP_IRQ_3_EN | M_GP_IRQ_4_EN)
+#define SE_I2C_ABORT BIT(1)
+
+/* M_CMD OP codes for I2C */
+#define I2C_WRITE 0x1
+#define I2C_READ 0x2
+#define I2C_WRITE_READ 0x3
+#define I2C_ADDR_ONLY 0x4
+#define I2C_BUS_CLEAR 0x6
+#define I2C_STOP_ON_BUS0x7
+/* M_CMD params for I2C */
+#define PRE_CMD_DELAY BIT(0)
+#define TIMESTAMP_BEFORE BIT(1)
+#define STOP_STRETCH BIT(2)
+#define TIMESTAMP_AFTERBIT(3)
+#define POST_COMMAND_DELAY BIT(4)
+#define IGNORE_ADD_NACKBIT(6)
+#define READ_FINISHED_WITH_ACK BIT(7)
+#define BYPASS_ADDR_PHASE BIT(8)
+#define SLV_ADDR_MSK GENMASK(15, 9)
+#define SLV_ADDR_SHFT 9
+/* I2C SCL COUNTER fields */
+#define HIGH_COUNTER_MSK GENMASK(29, 20)
+#define HIGH_COUNTER_SHFT 20
+#define LOW_COUNTER_MSKGENMASK(19, 10)
+#define LOW_COUNTER_SHFT 10
+#define CYCLE_COUNTER_MSK GENMASK(9, 0)
+
+#define I2C_PACK_TXBIT(0)
+#define I2C_PACK_RXBIT(1)
+
+#define PACKING_BYTES_PW 4
+
+#define GENI_I2C_IS_MASTER_HUB BIT(0)
+
+#define I2C_TIMEOUT_MS 100
+
+struct geni_i2c_clk_fld {
+ u32 clk_freq_out;
+ u8 clk_div;
+ u8 t_high_cnt;
+ u8 t_low_cnt;
+ u8 t_cycle_cnt;
+};
+
+struct geni_i2c_priv {
+ fdt_addr_t wrapper;
+ phys_addr_t base;
+ struct clk core;
+ struct clk se;
+ u32 tx_wm;
+ bool is_master_hub;
+ const struct geni_i2c_clk_fld *clk_fld;
+};
+
+/*
+ * Hardware uses the underlying formula to calculate time periods of
+ * SCL clock cycle. Firmware uses some additional cycles excluded from t
