Re: [PATCH v6 2/3] i3c: master: Add Qualcomm I3C controller driver

ALOK TIWARI
2025-07-01T08:50:23Z

On 7/1/2025 12:48 PM, Mukesh Kumar Savaliya wrote:
> Add support for the Qualcomm I3C controller driver, which implements
> I3C master functionality as defined in the MIPI Alliance Specification
> for I3C, Version 1.0.
>
> This driver supports master role in SDR mode.
>
> Unlike some other I3C master controllers, this implementation
> does not support In-Band Interrupts (IBI) and Hot-join requests.
>
> Signed-off-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
> ---
> drivers/i3c/master/Kconfig | 12 +
> drivers/i3c/master/Makefile | 1 +
> drivers/i3c/master/i3c-qcom-geni.c | 1158 ++++++++++++++++++++++++++++
> 3 files changed, 1171 insertions(+)
> create mode 100644 drivers/i3c/master/i3c-qcom-geni.c
>
> diff --git a/drivers/i3c/master/Kconfig b/drivers/i3c/master/Kconfig
> index 7b30db3253af..3e062d4b8f85 100644
> --- a/drivers/i3c/master/Kconfig
> +++ b/drivers/i3c/master/Kconfig
> @@ -41,6 +41,18 @@ config SVC_I3C_MASTER
> help
> Support for Silvaco I3C Dual-Role Master Controller.
>
> +config I3C_QCOM_GENI
> + tristate "Qualcomm Technologies Inc.'s I3C controller driver"
> + depends on QCOM_GENI_SE
> + help
> + This driver supports QUPV3 GENI based I3C controller in master
> + mode on the Qualcomm Technologies Inc.s SoCs. If you say yes to
> + this option, support will be included for the built-in I3C interface
> + on the Qualcomm Technologies Inc.s SoCs.

is this .s have any meaning

> +
> + This driver can also be built as a module. If so, the module
> + will be called i3c-qcom-geni.
> +
> config MIPI_I3C_HCI
> tristate "MIPI I3C Host Controller Interface driver (EXPERIMENTAL)"
> depends on HAS_IOMEM
> diff --git a/drivers/i3c/master/Makefile b/drivers/i3c/master/Makefile
> index 3e97960160bc..0e3ad9d96424 100644
> --- a/drivers/i3c/master/Makefile
> +++ b/drivers/i3c/master/Makefile
> @@ -1,5 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0-only
> obj-$(CONFIG_CDNS_I3C_MASTER) += i3c-master-cdns.o
> +obj-$(CONFIG_I3C_QCOM_GENI) += i3c-qcom-geni.o
> obj-$(CONFIG_DW_I3C_MASTER) += dw-i3c-master.o
> obj-$(CONFIG_AST2600_I3C_MASTER) += ast2600-i3c-master.o
> obj-$(CONFIG_SVC_I3C_MASTER) += svc-i3c-master.o
> diff --git a/drivers/i3c/master/i3c-qcom-geni.c b/drivers/i3c/master/i3c-qcom-geni.c
> new file mode 100644
> index 000000000000..c8bb40cdaaf4
> --- /dev/null
> +++ b/drivers/i3c/master/i3c-qcom-geni.c
> @@ -0,0 +1,1158 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
> + *
> + * Author: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/i3c/master.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slab.h>
> +#include <linux/soc/qcom/geni-se.h>
> +#include <linux/units.h>
> +
> +#define SE_I3C_SCL_HIGH 0x268
> +#define SE_I3C_TX_TRANS_LEN 0x26c
> +#define SE_I3C_RX_TRANS_LEN 0x270
> +#define SE_I3C_DELAY_COUNTER 0x274
> +#define SE_I2C_SCL_COUNTERS 0x278
> +#define SE_I3C_SCL_CYCLE 0x27c
> +#define SE_GENI_HW_IRQ_EN 0x920
> +#define SE_GENI_HW_IRQ_IGNORE_ON_ACTIVE 0x924
> +#define SE_GENI_HW_IRQ_CMD_PARAM_0 0x930
> +
> +/* HW I3C IBI interrupt enable */
> +#define M_IBI_IRQ_EN BIT(0)
> +
> +/* M_IBI_IRQ_IGNORE */
> +#define M_IBI_IRQ_IGNORE BIT(0)
> +
> +/* SE_GENI_M_CLK_CFG field shifts */
> +#define CLK_DIV_VALUE_MASK GENMASK(23, 4)
> +#define SER_CLK_EN BIT(0)
> +
> +/* SE_GENI_HW_IRQ_CMD_PARAM_0 field bits */
> +#define M_IBI_IRQ_PARAM_7E BIT(0)
> +#define M_IBI_IRQ_PARAM_STOP_STALL BIT(1)
> +
> +/* SE_I2C_SCL_COUNTERS field shifts */
> +#define I2C_SCL_HIGH_COUNTER_MASK GENMASK(29, 20)
> +#define I2C_SCL_LOW_COUNTER_MASK GENMASK(19, 10)
> +#define I2C_SCL_CYCLE_COUNTER_MASK GENMASK(9, 0)
> +
> +#define SE_I3C_ERR (M_CMD_OVERRUN_EN | M_ILLEGAL_CMD_EN | M_CMD_FAILURE_EN |\
> + M_CMD_ABORT_EN | M_GP_IRQ_0_EN | M_GP_IRQ_1_EN | M_GP_IRQ_2_EN | \
> + M_GP_IRQ_3_EN | M_GP_IRQ_4_EN)
> +
> +/* M_CMD OP codes for I2C/I3C */
> +#define I3C_READ_IBI_HW 0
> +#define I2C_WRITE 1
> +#define I2C_READ 2
> +#define I2C_WRITE_READ 3
> +#define I2C_ADDR_ONLY 4
> +#define I3C_INBAND_RESET 5
> +#define I2C_BUS_CLEAR 6
> +#define I2C_STOP_ON_BUS 7
> +#define I3C_HDR_DDR_EXIT 8
> +#define I3C_PRIVATE_WRITE 9
> +#define I3C_PRIVATE_READ 10
> +#define I3C_HDR_DDR_WRITE 11
> +#define I3C_HDR_DDR_READ 12
> +#define I3C_DIRECT_CCC_ADDR_ONLY 13
> +#define I3C_BCAST_CCC_ADDR_ONLY 14
> +#define I3C_READ_IBI 15
> +#define I3C_BCAST_CCC_WRITE 16
> +#define I3C_DIRECT_CCC_WRITE 17
> +#define I3C_DIRECT_CCC_READ 18
> +
> +/* M_CMD params for I3C */
> +#define PRE_CMD_DELAY BIT(0)
> +#define TIMESTAMP_BEFORE BIT(1)
> +#define STOP_STRETCH BIT(2)
> +#define TIMESTAMP_AFTER BIT(3)
> +#define POST_COMMAND_DELAY BIT(4)
> +#define IGNORE_ADD_NACK BIT(6)
> +#define READ_FINISHED_WITH_ACK BIT(7)
> +#define CONTINUOUS_MODE_DAA BIT(8)
> +
> +#define SLAVE_ADDR_MASK GENMASK(15, 9)
> +
> +#define CCC_HDR_CMD_MSK GENMASK(23, 16)
> +#define IBI_NACK_TBL_CTRL BIT(24)
> +#define USE_7E BIT(25)
> +#define BYPASS_ADDR_PHASE BIT(26)
> +
> +/* GSI callback error fields - DMA_TX_IRQ_STAT */
> +#define GP_IRQ0 BIT(5)
> +#define GP_IRQ1 BIT(6)
> +#define GP_IRQ2 BIT(7)
> +#define GP_IRQ3 BIT(8)
> +#define GP_IRQ4 BIT(9)
> +#define GP_IRQ5 BIT(10)
> +#define DM_I3C_CB_ERR GENMASK(10, 5)
> +
> +#define I3C_AUTO_SUSPEND_DELAY 250
> +#define PACKING_BYTES_PER_WORD 4
> +#define XFER_TIMEOUT 250
> +#define DFS_INDEX_MAX 7
> +
> +#define I3C_ADDR_MASK I2C_MAX_ADDR
> +
[clip]
> +
> +static int geni_i3c_master_i2c_xfers(struct i2c_dev_desc *dev, struct i2c_msg *msgs, int num)
> +{
> + struct i3c_master_controller *m = i2c_dev_get_master(dev);
> + struct geni_i3c_dev *gi3c = to_geni_i3c_master(m);
> + int i, ret;
> +
> + ret = i3c_geni_runtime_get_mutex_lock(gi3c);
> + if (ret)
> + return ret;
> +
> + qcom_geni_i3c_conf(gi3c, PUSH_PULL_MODE);
> +
> + for (i = 0; i < num; i++) {
> + struct geni_i3c_xfer_params xfer;
> +
> + xfer.m_cmd = (msgs[i].flags & I2C_M_RD) ? I2C_READ : I2C_WRITE;
> + xfer.m_param = (i < (num - 1)) ? STOP_STRETCH : 0;
> + xfer.m_param |= FIELD_PREP(SLAVE_ADDR_MASK, msgs[i].addr);
> + xfer.mode = msgs[i].len > 32 ? GENI_SE_DMA : GENI_SE_FIFO;

is this 32 assumptions ,can tx_depth decide this?

> + if (msgs[i].flags & I2C_M_RD)
> + ret = i3c_geni_execute_read_command(gi3c, &xfer, msgs[i].buf, msgs[i].len);
> + else
> + ret = i3c_geni_execute_write_command(gi3c, &xfer, msgs[i].buf, msgs[i].len);
> + if (ret)
> + break;
> + }
> +
> + dev_dbg(gi3c->se.dev, "i2c: txn ret:%d\n", ret);
> + i3c_geni_runtime_put_mutex_unlock(gi3c);
> +
> + return ret;
> +}
> +
[clip]
> +static int geni_i3c_master_bus_init(struct i3c_master_controller *m)
> +{
> + struct geni_i3c_dev *gi3c = to_geni_i3c_master(m);
> + struct i3c_bus *bus = i3c_master_get_bus(m);
> + struct i3c_device_info info = { };
> + int ret;
> +
> + /* Get an address for the master. */
> + ret = i3c_master_get_free_addr(m, 0);
> + if (ret < 0)
> + dev_err(gi3c->se.dev, "%s: error No free addr:%d\n", __func__, ret);
> +
> + info.dyn_addr = ret;
> + info.dcr = I3C_DCR_GENERIC_DEVICE;
> + info.bcr = I3C_BCR_I3C_MASTER | I3C_BCR_HDR_CAP;
> + info.pid = 0;
> +
> + ret = geni_i3c_clk_map_idx(gi3c);
> + if (ret) {
> + dev_err(gi3c->se.dev,
> + "Invalid clk frequency %d Hz src for %ld Hz bus: %d\n",
> + gi3c->clk_src_freq, bus->scl_rate.i3c, ret);
> + return ret; //This was missed in upstream : TBD

why this TBD

> + }
> +
> + ret = i3c_geni_runtime_get_mutex_lock(gi3c);
> + if (ret)
> + return ret;
> +
> + qcom_geni_i3c_conf(gi3c, OPEN_DRAIN_MODE);
> +
> + ret = i3c_master_set_info(&gi3c->ctrlr, &info);
> + i3c_geni_runtime_put_mutex_unlock(gi3c);
> +
> + return ret;
> +}

Thanks,
Alok