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

[Bjorn Andersson]

On Thu, Apr 23, 2026 at 08:25:51PM +0530, Mukesh Kumar Savaliya wrote:
> 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

"For GPI mode transfers" is there any other form?

> 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@xxxxxxxxxxxxxxxx>
> ---
>  drivers/i2c/busses/i2c-qcom-geni.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index ae609bdd2ec4..a396ddc7d8f4 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -815,6 +815,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;

You say above that single-messages case is handled, but if num == 1 then
we will hit i == 0, set the acquire, we will not hit else, and then we
will exit the loop. What am I missing?

> +		}
> +
>  		peripheral.addr = msgs[i].addr;
>  		if (i > 0 && (!(msgs[i].flags & I2C_M_RD)))
>  			peripheral.multi_msg = false;
> @@ -1014,6 +1022,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;

gi2c->se.multi_owner = of_property_read_bool(pdev->dev.of_node, "qcom,qup-multi-owner");

> +		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));
>  
> @@ -1089,7 +1102,9 @@ static int geni_i2c_probe(struct platform_device *pdev)
>  	}
>  
>  	if (fifo_disable) {
> -		/* FIFO is disabled, so we can only use GPI DMA */
> +		/* FIFO is disabled, so we can only use GPI DMA.

That's not how we format comments outside the network subsystem.

Regards,
Bjorn

> +		 * SE can be shared in GSI mode between subsystems, each SS owns a GPII.
> +		 */
>  		gi2c->gpi_mode = true;
>  		ret = setup_gpi_dma(gi2c);
>  		if (ret)
> @@ -1098,6 +1113,11 @@ static int geni_i2c_probe(struct platform_device *pdev)
>  		dev_dbg(dev, "Using GPI DMA mode for I2C\n");
>  	} else {
>  		gi2c->gpi_mode = false;
> +
> +		if (gi2c->se.multi_owner)
> +			return dev_err_probe(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 */
> -- 
> 2.43.0
>