[PATCH v8 0/4] Enable multi-owner I2C support for QCOM GENI controllers

[PATCH v8 0/4] Enable multi-owner I2C support for QCOM GENI controllers

[Mukesh Kumar Savaliya]

The QUP-based GENI I2C controller driver currently assumes exclusive
ownership of the controller by a single system processor. This prevents
safe use of a single I2C controller by multiple system processors
(e.g. APPS and a DSP) running the same or different operating systems.

One practical example is an EEPROM connected to an I2C controller that
needs to be accessed independently by firmware running on a DSP and by
Linux running on the application processor, without causing bus-level
interference during transfers.

This series adds support for operating a QUP GENI I2C Serial Engine in a
multi-owner configuration. Each system processor uses its own dedicated
GPI instance (GPII) as the data path between the Serial Engine and the
GSI DMA engine. As a result, controller sharing is supported only when
the I2C controller operates in GPI mode; FIFO/CPU DMA modes are not
supported for this configuration.

To serialize access at the hardware level, the GPI DMA engine is used to
emit lock and unlock Transfer Ring Elements (TREs) around I2C transfers.
The lock is acquired before the first transfer and released after the
last transfer, ensuring uninterrupted access to the controller while a
processor owns it.

In addition, when a controller is shared, the GENI common layer avoids
placing the associated GPIOs into the pinctrl "sleep" state during
runtime suspend. This prevents disruption of transfers that may still
be in progress on another system processor using the same controller
pins.

The multi-owner behavior is enabled via a DeviceTree property,
`qcom,qup-multi-owner`, on the I2C controller node. This property must be
used only when the hardware configuration requires controller sharing
and when GPI mode is enabled.

Patch overview:
  1. Document the `qcom,qup-multi-owner` DeviceTree property for GENI I2C.
  2. Extend the QCOM GPI DMA driver to support lock and unlock TREs with a
     simplified single-field API.
  3. Update the GENI common layer to keep pinctrl active for shared
     controllers during runtime suspend.
  4. Enable multi-owner operation in the GENI I2C driver using the new
     DeviceTree property and GPI lock/unlock support.

Signed-off-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>

---
Link to V7 : https://lore.kernel.org/all/20260423145705.545552-1-mukesh.savaliya@oss.qualcomm.com/
Changes in V8:
- Documented that multi-owner controllers must not assume exclusive ownership of GPIOs and
  therefore sleep pinctrl state must not be selected.
- Documented that each owner is responsible for maintaining required resource votes for
  shared-controller operation.
- Clarified commit message to state that lock/unlock sequencing applies only to
  GPI DMA mode transfers, as FIFO mode does not use the GPI engine.
- Fixed GPI lock/unlock handling for single-message transfer, ensure both ACQUIRE
  and RELEASE are issued when num == 1.
- Simplified DT property assignment by directly assigning of_property_read_bool()
  result to se.multi_owner.
- Improved error message to clearly indicate that multi-owner configuration requires
  GPI DMA mode.
- Updated code comments to follow kernel style guidelines and improved wording for clarity.
- Rebased patch on tip.
- Added ACked by tag for Patch 1 and reveiwed by tag for Patch 3.
 
Link to V6 : https://lore.kernel.org/all/20260331114742.2896317-1-mukesh.savaliya@oss.qualcomm.com/
Changes in V7:
 - Added Acked-by for dt-biding patch 1 given by Rob.
 - Minor description change for multi_owner variable in patch 3 and added RB tag from Konrad.
 - Removed description of multi_owner DT property from code as it's part of kernel doc.
 - Returned with dev_err_probe() in geni_i2c_probe() - Konrad's suggestion.


Link to V5 : https://lore.kernel.org/lkml/20241129144357.2008465-2-mukesh.savaliya@oss.qualcomm.com/
Changes in V6:
 - Addressed review feedback from Krzysztof Kozlowski and other reviewers, primarily
   around clarifying the feature semantics and improving the DeviceTree flag naming.
 - Renamed the DeviceTree property from qcom,shared-se to qcom,qup-multi-owner to
   better describe the multi-owner controller use case.
 - Updated the cover letter to clearly describe the multi-owner I2C design, the
   GPI-only limitation, and the role of the new qcom,qup-multi-owner flag.
 - Updated the DeviceTree binding documentation to reflect the new qcom,qup-multi-owner
   property and refined its description for clarity and correctness.
 - [Patch 2/4] Simplify the GPI I2C interface by replacing multiple shared SE related
   state flags with a single internal lock/unlock control managed entirely in the GPI
   driver - Suggested by Vinod Koul.
 - [Patch 3/4] Updated the GENI common layer to avoid selecting the pinctrl “sleep”
   state for multi-owner controllers, preventing disruption of transfers initiated by
   another system processor during runtime suspend.
 - [Patch 4/4] Updated the GENI I2C driver to: 
    - Detect the qcom,qup-multi-owner DeviceTree property.
	- Mark the underlying serial engine as shared.
	- Request GPI lock and unlock TRE sequencing around I2C transfers using the
	  simplified single field API.
 - Clarified commit messages across all patches to avoid ambiguous terminology
   (such as “subsystem”), expand abbreviations, and better explain functional
   requirements rather than optimizations.
 - Updated copyright headers across all files wherever applicable.
 - Renamed variable shared_geni_se to multi_owner to match the DT property naming.
 - Changed dev_err(print_log) during probe() to dev_err_probe().
 

Link to V4 : https://lore.kernel.org/lkml/20241113161413.3821858-1-quic_msavaliy@quicinc.com/
Changes in V5:
 - Corrected name as qcom,shared-se instead of qcom,is-shared.
 - Added description for the SE acronyms into yaml file and commit log.
 - Renamed TRE_I2C_UNLOCK to TRE_UNLOCK being generic.
 - Log an error and return if non GPI mode goes into shared usecase.


Link to V3: https://lore.kernel.org/lkml/20240927063108.2773304-4-quic_msavaliy@quicinc.com/T/
Changes in V4:
 - Fixed Typo to dt-bindings in subject line of PATCH 1.
 - Replaced SS (subsystem) as multiprocessor as per Bryan's suggestions.
 - Replied to Krzysztof's comments and replaced SS with Multiprocessor system.
 - Removed Abbreviations and also bullet point list from  PATCH 1.
 - Changed feature flag name from qcom,shared-se to qcom,is-shared.
 - Removed bullet points from example of usecase and explained in paragraph.
 - Changed title suffix to dmaengine from dma for Patch 2.
 - Rename TRE_I2C_LOCK to TRE_LOCK in PATCH 2.
 - Enhanced comments about not modifying the pin states on shared SE for PATCH 3.
 - Enhanced shared_geni_se struct member explanation as per Bjorn's comment in PATCH 3.
 - Moved GPIO unconfiguration description from patch 4 to patch 3 as pointed by Bjorn.
 - Removed debug log which was unrelated to this feature change.
 - Added usecase exmaple of shared SE in commit log.


Link to V2: https://lore.kernel.org/lkml/a88a16ff-3537-4396-b2ea-4ba02b4850e9@quicinc.com/T/
Changes in V3:
 - Added missing maintainers which i forgot to add.
 - Add cover letter with description of SS and EE for dt-bindings patch.
 - Added acronyms expansion to commit log.
 - [PATCH v2 3/4] : Removed exported symbol geni_se_clks_off(). 
   Instead added changes to bypass pinctrl sleep configuration from
   geni_se_resources_off() function.
 - Changed title name of [PATCH v2 3/4] to reflect the suggested changes.
 - [PATCH v2 4/4] kept geni_i2c_runtime_suspend() as is and removed 
   explicit call to geni_se_clks_off().
 - Removed is_shared variable from i2c driver and instead used common 
   shared_geni_se variable from qcom-geni-se.h so that other protocols
   can also extend for similar feature.
 - I2C driver log changed from dev_err() to dev_dbg() for timeout.
 - set gpi_mode = true if shared_geni_se is set for this usecase. Enhanced
   comments around code and commit log.


Link to V1: https://lore.kernel.org/lkml/cb7613d0-586e-4089-a1b6-2405f4dc4883@quicinc.com/T/
Changes in V2:
 - Enhanced commit log grammatically for PATCH v1 3/4 as suggested by Bryan.
 - Updated Cover letter along with acronyms expansion.
 - Added maintainers list from other subsystems for review, which was missing.
   Thanks to Krzysztof for pointing out.
 - Added cover letter with an example of Serial Engine sharing.
 - Addressed review comments for all the patches.
---

Mukesh Kumar Savaliya (4):
  dt-bindings: i2c: qcom,i2c-geni: Document multi-owner controller
    support
  dmaengine: qcom: gpi: Add lock/unlock TREs for multi-owner I2C
    transfers
  soc: qcom: geni-se: Keep pinctrl active for multi-owner controllers
  i2c: qcom-geni: Support multi-owner controllers in GPI mode

 .../bindings/i2c/qcom,i2c-geni-qcom.yaml      | 16 +++++++
 drivers/dma/qcom/gpi.c                        | 44 ++++++++++++++++++-
 drivers/i2c/busses/i2c-qcom-geni.c            | 24 +++++++++-
 drivers/soc/qcom/qcom-geni-se.c               | 14 ++++--
 include/linux/dma/qcom-gpi-dma.h              | 20 ++++++++-
 include/linux/soc/qcom/geni-se.h              |  2 +
 6 files changed, 114 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH v8 1/4] dt-bindings: i2c: qcom,i2c-geni: Document multi-owner controller support

[Mukesh Kumar Savaliya]

Document a DeviceTree property to describe QUP-based I2C controllers that
are shared with one or more other system processors.

On some Qualcomm platforms, a QUP-based I2C controller may be accessed by
multiple system processors (for example, APPS and DSP). In such
configurations, the operating system must not assume exclusive ownership
of the controller or its associated hardware resources.

The new qcom,qup-multi-owner property indicates that the controller is
externally shared and that the operating system must avoid operations
which rely on sole control of the hardware.

Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
---
 .../bindings/i2c/qcom,i2c-geni-qcom.yaml         | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
index 51534953a69c..ed9b029603fd 100644
--- a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
+++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
@@ -60,6 +60,22 @@ properties:
   power-domains:
     maxItems: 1
 
+  qcom,qup-multi-owner:
+    type: boolean
+    description:
+      Indicates that the QUP-based controller is shared with one or more
+      other system processors and must not be assumed to have exclusive
+      ownership by the operating system.
+
+      The associated GPIOs must not be reconfigured into a sleep state
+      during runtime suspend, as doing so may disrupt transactions
+      initiated by another owner of the controller.
+
+      Each owner is responsible for maintaining any resource votes
+      required for operation of the shared controller (for example clocks,
+      power domains, interconnect bandwidth, or other platform-specific
+      resources)
+
   reg:
     maxItems: 1
 
-- 
2.43.0

[PATCH v8 2/4] dmaengine: qcom: gpi: Add lock/unlock TREs for multi-owner I2C transfers

[Mukesh Kumar Savaliya]

Some platforms use a QUP-based I2C controller in a configuration where the
controller is shared with another system processor (described in DT using
qcom,qup-multi-owner). In such setups, GPI hardware lock/unlock TREs can be
used to serialize access to the controller.

Add support to emit lock and unlock TREs around I2C transfers and increase
the maximum TRE count to account for the additional elements.

Also simplify the client interface by replacing multiple boolean fields
(shared flag and message position tracking) with a single lock_action
selector (acquire/release/none), as the GPI driver only needs to know
whether to emit lock/unlock TREs for a given transfer.

Signed-off-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
---
 drivers/dma/qcom/gpi.c           | 44 +++++++++++++++++++++++++++++++-
 include/linux/dma/qcom-gpi-dma.h | 20 ++++++++++++++-
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
index a5055a6273af..1e70d2adfdff 100644
--- a/drivers/dma/qcom/gpi.c
+++ b/drivers/dma/qcom/gpi.c
@@ -2,6 +2,7 @@
 /*
  * Copyright (c) 2017-2020, The Linux Foundation. All rights reserved.
  * Copyright (c) 2020, Linaro Limited
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
  */
 
 #include <dt-bindings/dma/qcom-gpi.h>
@@ -67,6 +68,14 @@
 #define TRE_DMA_LEN		GENMASK(23, 0)
 #define TRE_DMA_IMMEDIATE_LEN	GENMASK(3, 0)
 
+/* Lock TRE */
+#define TRE_LOCK		BIT(0)
+#define TRE_MINOR_TYPE		GENMASK(19, 16)
+#define TRE_MAJOR_TYPE		GENMASK(23, 20)
+
+/* Unlock TRE */
+#define TRE_UNLOCK		BIT(8)
+
 /* Register offsets from gpi-top */
 #define GPII_n_CH_k_CNTXT_0_OFFS(n, k)	(0x20000 + (0x4000 * (n)) + (0x80 * (k)))
 #define GPII_n_CH_k_CNTXT_0_EL_SIZE	GENMASK(31, 24)
@@ -518,7 +527,7 @@ struct gpii {
 	bool ieob_set;
 };
 
-#define MAX_TRE 3
+#define MAX_TRE 5
 
 struct gpi_desc {
 	struct virt_dma_desc vd;
@@ -1625,12 +1634,27 @@ static int gpi_create_i2c_tre(struct gchan *chan, struct gpi_desc *desc,
 			      unsigned long flags)
 {
 	struct gpi_i2c_config *i2c = chan->config;
+	enum gpi_lock_action lock_action = i2c->lock_action;
 	struct device *dev = chan->gpii->gpi_dev->dev;
 	unsigned int tre_idx = 0;
 	dma_addr_t address;
 	struct gpi_tre *tre;
 	unsigned int i;
 
+	/* Optional lock TRE before transfer */
+	if (lock_action == GPI_LOCK_ACQUIRE) {
+		tre = &desc->tre[tre_idx];
+		tre_idx++;
+
+		tre->dword[0] = 0;
+		tre->dword[1] = 0;
+		tre->dword[2] = 0;
+		tre->dword[3] = u32_encode_bits(1, TRE_LOCK);
+		tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_IEOB);
+		tre->dword[3] |= u32_encode_bits(0, TRE_MINOR_TYPE);
+		tre->dword[3] |= u32_encode_bits(3, TRE_MAJOR_TYPE);
+	}
+
 	/* first create config tre if applicable */
 	if (i2c->set_config) {
 		tre = &desc->tre[tre_idx];
@@ -1690,6 +1714,24 @@ static int gpi_create_i2c_tre(struct gchan *chan, struct gpi_desc *desc,
 
 		if (!(flags & DMA_PREP_INTERRUPT))
 			tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_BEI);
+
+		/* If multi-owner and this is the release boundary, chain it */
+		if (i2c->lock_action == GPI_LOCK_RELEASE)
+			tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_CHAIN);
+	}
+
+	/* Optional unlock TRE after transfer */
+	if (lock_action == GPI_LOCK_RELEASE && i2c->op != I2C_READ) {
+		tre = &desc->tre[tre_idx];
+		tre_idx++;
+
+		tre->dword[0] = 0;
+		tre->dword[1] = 0;
+		tre->dword[2] = 0;
+		tre->dword[3] = u32_encode_bits(1, TRE_UNLOCK);
+		tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_IEOB);
+		tre->dword[3] |= u32_encode_bits(1, TRE_MINOR_TYPE);
+		tre->dword[3] |= u32_encode_bits(3, TRE_MAJOR_TYPE);
 	}
 
 	for (i = 0; i < tre_idx; i++)
diff --git a/include/linux/dma/qcom-gpi-dma.h b/include/linux/dma/qcom-gpi-dma.h
index 332be28427e4..f10fa93713f9 100644
--- a/include/linux/dma/qcom-gpi-dma.h
+++ b/include/linux/dma/qcom-gpi-dma.h
@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 /*
  * Copyright (c) 2020, Linaro Limited
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
  */
 
 #ifndef QCOM_GPI_DMA_H
@@ -54,6 +55,21 @@ enum i2c_op {
 	I2C_READ,
 };
 
+/**
+ * enum gpi_lock_action - request lock/unlock TRE sequencing
+ * @GPI_LOCK_NONE: No lock/unlock TRE requested for this transfer
+ * @GPI_LOCK_ACQUIRE: Emit a lock TRE before the transfer
+ * @GPI_LOCK_RELEASE: Emit an unlock TRE after the transfer
+ *
+ * Used by protocol drivers for multi-owner controller setups (e.g. when
+ * DeviceTree indicates the controller is shared via qcom,qup-multi-owner).
+ */
+enum gpi_lock_action {
+	GPI_LOCK_NONE = 0,
+	GPI_LOCK_ACQUIRE,
+	GPI_LOCK_RELEASE,
+};
+
 /**
  * struct gpi_i2c_config - i2c config for peripheral
  *
@@ -67,7 +83,8 @@ enum i2c_op {
  * @set_config: set peripheral config
  * @rx_len: receive length for buffer
  * @op: i2c cmd
- * @multi_msg: is part of multi i2c r-w msgs
+ * @muli-msg: is part of multi i2c r-w msgs
+ * @lock_action: request lock/unlock TRE sequencing for this transfer
  */
 struct gpi_i2c_config {
 	u8 set_config;
@@ -81,6 +98,7 @@ struct gpi_i2c_config {
 	u32 rx_len;
 	enum i2c_op op;
 	bool multi_msg;
+	enum gpi_lock_action lock_action;
 };
 
 #endif /* QCOM_GPI_DMA_H */
-- 
2.43.0

[PATCH v8 3/4] soc: qcom: geni-se: Keep pinctrl active for multi-owner controllers

[Mukesh Kumar Savaliya]

On platforms where a GENI Serial Engine is shared with another system
processor, selecting the "sleep" pinctrl state can disrupt ongoing
transfers initiated by the other processor.

Teach geni_se_resources_off() to skip selecting the pinctrl sleep state
when the Serial Engine is marked as shared, while still allowing the
rest of the resource shutdown sequence to proceed.

This is required for multi-owner configurations (described via DeviceTree
with qcom,qup-multi-owner on the protocol controller node).

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
---
 drivers/soc/qcom/qcom-geni-se.c  | 14 +++++++++++---
 include/linux/soc/qcom/geni-se.h |  2 ++
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
index 15636a8dc907..3441ae3431e6 100644
--- a/drivers/soc/qcom/qcom-geni-se.c
+++ b/drivers/soc/qcom/qcom-geni-se.c
@@ -607,9 +607,17 @@ int geni_se_resources_off(struct geni_se *se)
 	if (has_acpi_companion(se->dev))
 		return 0;
 
-	ret = pinctrl_pm_select_sleep_state(se->dev);
-	if (ret)
-		return ret;
+	/*
+	 * Select the "sleep" pinctrl state only when the serial engine is
+	 * exclusively owned by this system processor. For shared controller
+	 * configurations, another system processor may still be using the pins,
+	 * and switching them to "sleep" can disrupt ongoing transfers.
+	 */
+	if (!se->multi_owner) {
+		ret = pinctrl_pm_select_sleep_state(se->dev);
+		if (ret)
+			return ret;
+	}
 
 	geni_se_clks_off(se);
 	return 0;
diff --git a/include/linux/soc/qcom/geni-se.h b/include/linux/soc/qcom/geni-se.h
index c5e6ab85df09..9571da2c51a8 100644
--- a/include/linux/soc/qcom/geni-se.h
+++ b/include/linux/soc/qcom/geni-se.h
@@ -66,6 +66,7 @@ struct geni_icc_path {
  * @icc_paths:		Array of ICC paths for SE
  * @pd_list:		Power domain list for managing power domains
  * @has_opp:		Indicates if OPP is supported
+ * @multi_owner:	True if SE is shared between multiple owners.
  */
 struct geni_se {
 	void __iomem *base;
@@ -78,6 +79,7 @@ struct geni_se {
 	struct geni_icc_path icc_paths[3];
 	struct dev_pm_domain_list *pd_list;
 	bool has_opp;
+	bool multi_owner;
 };
 
 /* Common SE registers */
-- 
2.43.0

[PATCH v8 4/4] i2c: qcom-geni: Support multi-owner controllers in GPI mode

[Mukesh Kumar Savaliya]

Some platforms use a QUP-based I2C controller in a configuration where the
controller is shared with another system processor. In this setup the
operating system must not assume exclusive ownership of the controller or
its associated pins.

Add support for enabling multi-owner operation when DeviceTree specifies
qcom,qup-multi-owner. When enabled, mark the underlying serial engine as
shared so the common GENI resource handling avoids selecting the "sleep"
pinctrl state, which could disrupt transfers initiated by the other
processor.

For GPI mode transfers, request lock/unlock TRE sequencing from the GPI
driver by setting a single lock_action selector per message, emitting lock
before the first message and unlock after the last message (handling the
single-message case as well). This serializes access to the shared
controller without requiring message-position flags to be passed into the
DMA engine layer.

Signed-off-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
---
 drivers/i2c/busses/i2c-qcom-geni.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 96dbf04138be..757c2c8eb207 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -832,6 +832,14 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
 		if (i < num - 1)
 			peripheral.stretch = 1;
 
+		peripheral.lock_action = GPI_LOCK_NONE;
+		if (gi2c->se.multi_owner) {
+			if (i == 0)
+				peripheral.lock_action = GPI_LOCK_ACQUIRE;
+			else if (i == num - 1)
+				peripheral.lock_action = GPI_LOCK_RELEASE;
+		}
+
 		peripheral.addr = msgs[i].addr;
 		if (i > 0 && (!(msgs[i].flags & I2C_M_RD)))
 			peripheral.multi_msg = false;
@@ -1019,7 +1027,11 @@ static int geni_i2c_init(struct geni_i2c_dev *gi2c)
 	}
 
 	if (fifo_disable) {
-		/* FIFO is disabled, so we can only use GPI DMA */
+		/*
+		 * FIFO is disabled, so only GPI DMA can be used.
+		 * In multi-owner configurations, the SE may be shared between subsystems,
+		 * with each subsystem owning a separate GPII.
+		 */
 		gi2c->gpi_mode = true;
 		ret = setup_gpi_dma(gi2c);
 		if (ret)
@@ -1028,6 +1040,11 @@ static int geni_i2c_init(struct geni_i2c_dev *gi2c)
 		dev_dbg(gi2c->se.dev, "Using GPI DMA mode for I2C\n");
 	} else {
 		gi2c->gpi_mode = false;
+
+		if (gi2c->se.multi_owner)
+			return dev_err_probe(gi2c->se.dev, -EINVAL,
+					     "I2C sharing not supported in non GSI mode\n");
+
 		tx_depth = geni_se_get_tx_fifo_depth(&gi2c->se);
 
 		/* I2C Master Hub Serial Elements doesn't have the HW_PARAM_0 register */
@@ -1098,6 +1115,11 @@ static int geni_i2c_probe(struct platform_device *pdev)
 		gi2c->clk_freq_out = I2C_MAX_STANDARD_MODE_FREQ;
 	}
 
+	if (of_property_read_bool(pdev->dev.of_node, "qcom,qup-multi-owner")) {
+		gi2c->se.multi_owner = true;
+		dev_dbg(&pdev->dev, "I2C controller is shared with another system processor\n");
+	}
+
 	if (has_acpi_companion(dev))
 		ACPI_COMPANION_SET(&gi2c->adap.dev, ACPI_COMPANION(dev));
 
-- 
2.43.0

Re: [PATCH v8 1/4] dt-bindings: i2c: qcom,i2c-geni: Document multi-owner controller support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] YAML syntax error in `qcom,qup-multi-owner` description due to missing block scalar indicator.
--

--- Patch [1]: [PATCH v8 1/4] dt-bindings: i2c: qcom,i2c-geni: Document multi-owner controller support ---
commit d38e4d9aefae5285416967f29ecb244ec4f2d7e0
Author: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>

dt-bindings: i2c: qcom,i2c-geni: Document multi-owner controller support

This commit adds a boolean property to the qcom,i2c-geni device tree binding
to describe QUP-based I2C controllers that are shared across multiple system
processors. It notes that the operating system must not assume exclusive
ownership of such controllers or reconfigure associated GPIOs into sleep
states.

> diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
> --- a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
> +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
> @@ -60,6 +60,22 @@ properties:
>    power-domains:
>      maxItems: 1
>  
> +  qcom,qup-multi-owner:
> +    type: boolean
> +    description:
> +      Indicates that the QUP-based controller is shared with one or more
> +      other system processors and must not be assumed to have exclusive
> +      ownership by the operating system.
> +
> +      The associated GPIOs must not be reconfigured into a sleep state

[Severity: Low]
Will this description format cause a YAML parsing error during a schema check?

Because the description spans multiple paragraphs separated by blank lines,
it appears to need a block scalar indicator (such as '|' or '>') immediately
following the 'description:' key. 

Without it, a YAML 1.2 parser may treat the blank line as the end of the
scalar and fail to parse the next paragraph, throwing an error when it cannot
find a mapping key for the subsequent text.

> +      during runtime suspend, as doing so may disrupt transactions
> +      initiated by another owner of the controller.
> +
> +      Each owner is responsible for maintaining any resource votes
> +      required for operation of the shared controller (for example clocks,
> +      power domains, interconnect bandwidth, or other platform-specific
> +      resources)
> +
>    reg:
>      maxItems: 1

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708051023.2872304-1-mukesh.savaliya@oss.qualcomm.com?part=1

Re: [PATCH v8 4/4] i2c: qcom-geni: Support multi-owner controllers in GPI mode

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Single-message transfers on a multi-owner controller acquire the hardware lock but never release it, deadlocking the shared controller.
- [High] Extraneous lock command TRE is emitted on the RX DMA channel for read transfers, violating hardware protocol.
- [Medium] PM runtime usage counter is leaked when rejecting multi-owner configurations in non-GPI mode.
- [Low] Usage of `of_property_read_bool()` ignores ACPI configurations for multi-owner support.
--

--- Patch [4]: [PATCH v8 4/4] i2c: qcom-geni: Support multi-owner controllers in GPI mode ---
commit 6f8199761192397d64a43320106d79fa18ae0f1f
Author: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>

i2c: qcom-geni: Support multi-owner controllers in GPI mode

This commit introduces support for sharing a QUP-based I2C controller with
another system processor. It enables multi-owner operation using the
qcom,qup-multi-owner property and serializes GPI mode transfers using
hardware lock/unlock commands.

> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 96dbf04138bee..757c2c8eb2071 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -832,6 +832,14 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
>  		if (i < num - 1)
>  			peripheral.stretch = 1;
>  
> +		peripheral.lock_action = GPI_LOCK_NONE;
> +		if (gi2c->se.multi_owner) {
> +			if (i == 0)
> +				peripheral.lock_action = GPI_LOCK_ACQUIRE;
> +			else if (i == num - 1)
> +				peripheral.lock_action = GPI_LOCK_RELEASE;
> +		}
> +

[Severity: High]
Does this code deadlock the shared controller for single-message transfers?

If num == 1, i == 0 evaluates to true and peripheral.lock_action is set
to GPI_LOCK_ACQUIRE. The else if (i == num - 1) branch is skipped entirely,
meaning GPI_LOCK_RELEASE is never requested. Since lock_action is a single 
enum value, it doesn't appear possible to emit both lock and unlock commands
for a single message this way.

[Severity: High]
Can this cause an extraneous lock command TRE to be emitted on the RX DMA
channel?

Because config.peripheral_config points to peripheral, the lock_action is
passed to the TX channel when calling geni_i2c_gpi() for I2C_WRITE. However,
if this is a read transfer (msgs[i].flags & I2C_M_RD), the same config
structure is immediately reused to configure the RX channel. 

Because peripheral.lock_action is not cleared back to GPI_LOCK_NONE, the RX
channel will also receive a GPI_LOCK_ACQUIRE command TRE, which could violate
the hardware protocol expecting command TREs only on the TX channel.

>  		peripheral.addr = msgs[i].addr;
>  		if (i > 0 && (!(msgs[i].flags & I2C_M_RD)))
>  			peripheral.multi_msg = false;

[ ... ]

> @@ -1028,6 +1040,11 @@ static int geni_i2c_init(struct geni_i2c_dev *gi2c)
>  		dev_dbg(gi2c->se.dev, "Using GPI DMA mode for I2C\n");
>  	} else {
>  		gi2c->gpi_mode = false;
> +
> +		if (gi2c->se.multi_owner)
> +			return dev_err_probe(gi2c->se.dev, -EINVAL,
> +					     "I2C sharing not supported in non GSI mode\n");
> +

[Severity: Medium]
Does this direct return leak the PM runtime usage counter?

At the beginning of geni_i2c_init(), pm_runtime_resume_and_get() is called. 
By returning directly here instead of using the goto err; cleanup path, 
the driver bypasses pm_runtime_put(), leaving the device power domain active.

>  		tx_depth = geni_se_get_tx_fifo_depth(&gi2c->se);
>  
>  		/* I2C Master Hub Serial Elements doesn't have the HW_PARAM_0 register */

[ ... ]

> @@ -1098,6 +1115,11 @@ static int geni_i2c_probe(struct platform_device *pdev)
>  		gi2c->clk_freq_out = I2C_MAX_STANDARD_MODE_FREQ;
>  	}
>  
> +	if (of_property_read_bool(pdev->dev.of_node, "qcom,qup-multi-owner")) {
> +		gi2c->se.multi_owner = true;
> +		dev_dbg(&pdev->dev, "I2C controller is shared with another system processor\n");
> +	}
> +

[Severity: Low]
Does this hardcode the multi-owner feature strictly to DeviceTree?

The driver supports ACPI companions (checked right below this via
has_acpi_companion(dev)). Using of_property_read_bool() ignores ACPI
platforms. Could this use device_property_read_bool(dev, "qcom,qup-multi-owner")
to uniformly support both ACPI _DSD and DeviceTree?

>  	if (has_acpi_companion(dev))
>  		ACPI_COMPANION_SET(&gi2c->adap.dev, ACPI_COMPANION(dev));
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708051023.2872304-1-mukesh.savaliya@oss.qualcomm.com?part=4

Re: [PATCH v8 3/4] soc: qcom: geni-se: Keep pinctrl active for multi-owner controllers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing Initialization / Dead Code
- [High] Incomplete Fix
- [High] Buffer Overflow / API Misuse
--

--- Patch [3]: [PATCH v8 3/4] soc: qcom: geni-se: Keep pinctrl active for multi-owner controllers ---
commit be30e0e06b9485a4fb9a672b4ecaed4d0dc8ff46
Author: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>

soc: qcom: geni-se: Keep pinctrl active for multi-owner controllers

This patch updates the GENI Serial Engine driver to avoid selecting the
"sleep" pinctrl state when the engine is shared with another processor. It
introduces a multi_owner flag to conditionally skip the pinctrl state change
during shutdown.

> diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
> index 15636a8dc907a..3441ae3431e6a 100644
> --- a/drivers/soc/qcom/qcom-geni-se.c
> +++ b/drivers/soc/qcom/qcom-geni-se.c
> @@ -607,9 +607,17 @@ int geni_se_resources_off(struct geni_se *se)
>  	if (has_acpi_companion(se->dev))
>  		return 0;
>  
> -	ret = pinctrl_pm_select_sleep_state(se->dev);
> -	if (ret)
> -		return ret;
> +	/*
> +	 * Select the "sleep" pinctrl state only when the serial engine is
> +	 * exclusively owned by this system processor. For shared controller
> +	 * configurations, another system processor may still be using the pins,
> +	 * and switching them to "sleep" can disrupt ongoing transfers.
> +	 */
> +	if (!se->multi_owner) {
> +		ret = pinctrl_pm_select_sleep_state(se->dev);
> +		if (ret)
> +			return ret;
> +	}

[Severity: High]
Does this regression also affect geni_se_resources_deactivate()?

Shared I2C and SPI buses use geni_se_resources_deactivate() as their active
power management callback. If a shared I2C or SPI controller initiates runtime
suspend, that function still unconditionally changes the pinctrl state:

drivers/soc/qcom/qcom-geni-se.c:geni_se_resources_deactivate() {
    ...
    ret = pinctrl_pm_select_sleep_state(se->dev);
    if (ret)
        return ret;
    ...
}

Could this leave the primary protocol drivers vulnerable to the exact
disruption described in the commit message?

> diff --git a/include/linux/soc/qcom/geni-se.h b/include/linux/soc/qcom/geni-se.h
> index c5e6ab85df099..9571da2c51a88 100644
> --- a/include/linux/soc/qcom/geni-se.h
> +++ b/include/linux/soc/qcom/geni-se.h
> @@ -66,6 +66,7 @@ struct geni_icc_path {
>   * @icc_paths:		Array of ICC paths for SE
>   * @pd_list:		Power domain list for managing power domains
>   * @has_opp:		Indicates if OPP is supported
> + * @multi_owner:	True if SE is shared between multiple owners.
>   */
>  struct geni_se {
>  	void __iomem *base;
> @@ -78,6 +79,7 @@ struct geni_se {
>  	struct geni_icc_path icc_paths[3];
>  	struct dev_pm_domain_list *pd_list;
>  	bool has_opp;
> +	bool multi_owner;
>  };

[Severity: High]
Does this missing initialization cause a regression where the fix fails to
take effect?

The commit message mentions it is described via DeviceTree with
qcom,qup-multi-owner, but it doesn't appear that of_property_read_bool()
is called anywhere to populate multi_owner.

Without this, will the flag always remain false, preventing the fix from
taking effect for shared controllers?

[Severity: High]
Can this regression cause the driver to queue a truncated or corrupted
transfer chain to the DMA engine under heavy IO traffic?

In drivers/dma/qcom/gpi.c, gpi_prep_slave_sg() statically calculates the
required number of TREs without accounting for lock_action, which can add up
to 2 extra TREs (total 5):

drivers/dma/qcom/gpi.c:gpi_prep_slave_sg() {
    ...
    nr_tre = 3;
    set_config = *(u32 *)gchan->config;
    if (!set_config)
        nr_tre = 2;
    ...
}

If the transfer ring has 3 or 4 slots available, validation passes against the
smaller bound. However, when ring space runs out, gpi_queue_xfer() silently
drops the TRE:

drivers/dma/qcom/gpi.c:gpi_queue_xfer() {
    ...
    ret = gpi_ring_add_element(&gchan->ch_ring, (void **)&ch_tre);
    if (unlikely(ret)) {
        dev_err(gpii->gpi_dev->dev, "Error adding ring element to xfer ring\n");
        return;
    }
    ...
}

Will this cause the caller to submit a truncated chain, leading to hardware
hangs or data corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708051023.2872304-1-mukesh.savaliya@oss.qualcomm.com?part=3

Re: [PATCH v8 2/4] dmaengine: qcom: gpi: Add lock/unlock TREs for multi-owner I2C transfers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] Setting TRE_FLAGS_IEOB on Lock/Unlock TREs causes multiple completion events per descriptor, corrupting the vchan queue.
- [High] Dangling CHAIN flag on the final DMA TRE for I2C Read transfers.
- [High] Asymmetric Lock/Unlock TRE generation causes the RX channel to permanently lock.
- [High] Insufficient DMA transfer ring space check leads to partial TRE enqueue and hardware lockup.
- [High] Missing CHAIN bit on GO TRE before Unlock TRE.
--

--- Patch [2]: [PATCH v8 2/4] dmaengine: qcom: gpi: Add lock/unlock TREs for multi-owner I2C transfers ---
commit cc47faf2611d1c6105d08f28b502e364ce9bde15
Author: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
Subject: dmaengine: qcom: gpi: Add lock/unlock TREs for multi-owner I2C transfers

This commit adds support for emitting GPI hardware lock and unlock TREs to
serialize access to a shared QUP-based I2C controller. It simplifies the
client interface with a lock_action selector and increases the maximum TRE
count to accommodate the newly added elements.

> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
> index a5055a6273af6..1e70d2adfdff0 100644
> --- a/drivers/dma/qcom/gpi.c
> +++ b/drivers/dma/qcom/gpi.c
> @@ -518,7 +527,7 @@ struct gpii {
>  	bool ieob_set;
>  };
>  
> -#define MAX_TRE 3
> +#define MAX_TRE 5
>  
>  struct gpi_desc {
>  	struct virt_dma_desc vd;

[Severity: High]
Since the maximum number of TREs has increased, does gpi_prep_slave_sg()
also need to be updated to account for the new elements?

It appears gpi_prep_slave_sg() still hardcodes the maximum required TRE
slots to 3:

gpi_prep_slave_sg() {
        ...
        nr_tre = 3;
        ...
        nr = gpi_ring_num_elements_avail(ch_ring);
        if (nr < nr_tre) {
        ...
}

If the ring only has 3 slots available but 5 TREs are generated, could this
cause gpi_queue_xfer() to silently drop the extra TREs and dispatch a
truncated chain to the hardware?

> [ ... ]
> @@ -1625,12 +1634,27 @@ static int gpi_create_i2c_tre(struct gchan *chan, struct gpi_desc *desc,
>  			      unsigned long flags)
>  {
>  	struct gpi_i2c_config *i2c = chan->config;
> +	enum gpi_lock_action lock_action = i2c->lock_action;
>  	struct device *dev = chan->gpii->gpi_dev->dev;
>  	unsigned int tre_idx = 0;
>  	dma_addr_t address;
>  	struct gpi_tre *tre;
>  	unsigned int i;
>  
> +	/* Optional lock TRE before transfer */
> +	if (lock_action == GPI_LOCK_ACQUIRE) {
> +		tre = &desc->tre[tre_idx];
> +		tre_idx++;
> +
> +		tre->dword[0] = 0;
> +		tre->dword[1] = 0;
> +		tre->dword[2] = 0;
> +		tre->dword[3] = u32_encode_bits(1, TRE_LOCK);
> +		tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_IEOB);

[Severity: Critical]
Could setting the Interrupt on End of Block (IEOB) flag here on the LOCK
TRE cause multiple completion events to fire for a single descriptor?

The DMA TRE also sets the IEOT flag later in this function. If both events
are generated, gpi_process_xfer_compl_event() might call vchan_next_desc()
multiple times, popping unrelated descriptors from the queue and potentially
causing premature completions or use-after-free conditions.

[Severity: High]
Will this logic cause the RX channel to acquire a lock that it never
releases?

For an I2C read transfer, lock_action == GPI_LOCK_ACQUIRE unconditionally
appends a LOCK TRE to the descriptor for both the TX and RX channels.
However, the logic later in this function explicitly suppresses the UNLOCK
TRE when i2c->op != I2C_READ. Is it possible the RX channel sequencer gets
permanently locked this way?

> +		tre->dword[3] |= u32_encode_bits(0, TRE_MINOR_TYPE);
> +		tre->dword[3] |= u32_encode_bits(3, TRE_MAJOR_TYPE);
> +	}
> +
> [ ... ]
> @@ -1690,6 +1714,24 @@ static int gpi_create_i2c_tre(struct gchan *chan, struct gpi_desc *desc,
>  
>  		if (!(flags & DMA_PREP_INTERRUPT))
>  			tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_BEI);
> +
> +		/* If multi-owner and this is the release boundary, chain it */
> +		if (i2c->lock_action == GPI_LOCK_RELEASE)
> +			tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_CHAIN);
> +	}
> +
> +	/* Optional unlock TRE after transfer */
> +	if (lock_action == GPI_LOCK_RELEASE && i2c->op != I2C_READ) {
> +		tre = &desc->tre[tre_idx];
> +		tre_idx++;

[Severity: High]
Can this sequence leave a dangling CHAIN flag on the final DMA TRE?

When preparing an I2C read transfer with GPI_LOCK_RELEASE, the DMA block
unconditionally sets the CHAIN flag on the DMA TRE. However, the UNLOCK TRE
is explicitly skipped here because of the (i2c->op != I2C_READ) condition.

Could this cause the hardware DMA sequencer to fetch an out-of-bounds or
stale TRE since it expects another descriptor to follow?

[Severity: High]
When configuring the TX channel for a multi-message read transfer, does
the GO TRE need the CHAIN bit instead of the LINK bit if an UNLOCK TRE
is going to follow it?

If multi_msg is true, earlier code in gpi_create_i2c_tre() sets
TRE_FLAGS_LINK on the GO TRE. Since the DMA TRE is skipped for TX channels
during reads, the next TRE appended will be this UNLOCK TRE. Without the
CHAIN bit on the GO TRE, might the hardware sequencer halt before processing
the UNLOCK TRE?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708051023.2872304-1-mukesh.savaliya@oss.qualcomm.com?part=2