Re: [PATCH v6] i2c: i2c-qcom-geni: serve transfers during early resume

[Aniket RANDIVE <aniket.randive@oss.qualcomm.com>]

On 6/29/2026 7:11 PM, Mukesh Kumar Savaliya wrote:
> I2C transfers issued during the resume_noirq phase can fail on GENI I2C
> controllers.
> 
> Some devices require I2C communication before their resume sequence can
> complete. One example is a USB Ethernet device attached through PCIe,
> where device configuration must be restored over I2C before PCIe link
> initialization can proceed. Since such accesses occur during
> resume_noirq(), the underlying I2C controller must be able to service
> transfers at that stage.
> 
> However, GENI I2C transfers rely on interrupts for command completion,
> while IRQ handling is still suspended during early resume. Additionally,
> runtime PM may remain disabled until later in the resume sequence, causing
> pm_runtime_get_sync() to return -EACCES and preventing controller
> resources from being enabled.
> 
> Allow the controller to operate during early resume by requesting the IRQ
> with IRQF_NO_SUSPEND and IRQF_EARLY_RESUME so completion interrupts can be
> delivered during the noirq phase. Also restore runtime PM from
> resume_noirq() when it is disabled and tolerate transient -EACCES failures
> from pm_runtime_get_sync() during the PM state transition.
> 
> This enables GENI I2C transfers to complete successfully during the
> resume_noirq phase and allows dependent devices to finish their resume
> sequence.
> 
> Co-developed-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
> Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
> Signed-off-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
> ---
> v5->v6 :
> - Modified commit log to start with problem description as suggested by Bjorn.
> - Moved to new implementation of the logic while earlier replied to comments on
>    older design and considers latest fix added recently.
> - Made change generic to I2C including GPI mode transfer, this was not done earlier.
> - Changed email address to oss.qualcomm.com domain.
> Link to V5: https://lore.kernel.org/lkml/20241227130236.755794-1-quic_msavaliy@quicinc.com/
> 
> ---
> v4->v5:
> - Commit log enhanced considering Bjorn's comments by explaining PCIe usecase.
> - Enhanced comment with reason when using pm_runtime_force_resume().
> - Corrected IS_ENABLED(CONFIG_PM) condition inside geni_i2c_xfer().
> - Improved debug log as per Bjorn's suggestions during suspend, resume.
> - Reverted back comment before devm_request_irq().
> Link to V4: https://lore.kernel.org/lkml/bd699719-4958-445a-a685-4b5f6a8ad81f@quicinc.com/
> 
> ---
> v3->v4 :
>   - Enhanced commit log by explaining client usecase scenario during early resume.
>   - Covered 'usage_count' of 'struct dev_pm_info' under CONFIG_PM to compile non PM CONFIG.
> Link to V3: https://lore.kernel.org/all/20241119143031.3331753-1-quic_msavaliy@quicinc.com/T/
> 
> ---
> v2 -> v3:
>   - Updated exact usecase and scenario in the commit log description.
>   - Removed bulleted points from technical description, added details in free flow.
>   - Used pm_runtime_force_resume/suspend() instead customized local implementation.
>   - Added debug log after pm_runtime_force_suspend().
> Link to V2: https://lore.kernel.org/lkml/202410132233.P25W2vKq-lkp@intel.com/T/
> 
> ---
> v1 -> v2:
>   - Changed gi2c->se.dev to dev during dev_dbg() calls.
>   - Addressed review comments from Andi and Bjorn.
>   - Returned 0 instead garbage inside geni_i2c_force_resume().
>   - Added comments explaining forced resume transfer when runtime PM
>     remains disabled.
> Link to V1: https://patches.linaro.org/project/linux-i2c/patch/20240328123743.1713696-1-quic_msavaliy@quicinc.com/
> ---
> ---
>   drivers/dma/qcom/gpi.c             |  3 ++-
>   drivers/i2c/busses/i2c-qcom-geni.c | 12 +++++++++++-
>   2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
> index c9a6f610ffd9..332f3aa40628 100644
> --- a/drivers/dma/qcom/gpi.c
> +++ b/drivers/dma/qcom/gpi.c
> @@ -615,7 +615,8 @@ static int gpi_config_interrupts(struct gpii *gpii, enum gpii_irq_settings setti
>   
>   	if (!gpii->configured_irq) {
>   		ret = devm_request_irq(gpii->gpi_dev->dev, gpii->irq,
> -				       gpi_handle_irq, IRQF_TRIGGER_HIGH,
> +				       gpi_handle_irq,
> +				       IRQF_TRIGGER_HIGH | IRQF_NO_SUSPEND | IRQF_EARLY_RESUME,
>   				       "gpi-dma", gpii);
>   		if (ret < 0) {
>   			dev_err(gpii->gpi_dev->dev, "error request irq:%d ret:%d\n",
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index d2f5055b0b10..d56b36bd1d26 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -913,6 +913,10 @@ static int geni_i2c_xfer(struct i2c_adapter *adap,
>   	gi2c->err = 0;
>   	reinit_completion(&gi2c->done);
>   	ret = pm_runtime_get_sync(gi2c->se.dev);
> +	if (ret == -EACCES) {
> +		dev_warn(gi2c->se.dev, "Runtime PM is disabled:%d\n", ret);
> +		ret = 0;
> +	}

Why we are checking specific error code here? Why can't we use the below 
error check directly?
if get sync itself is failed with pm runtime disabled then why we are 
going ahead by making ret = 0 here? How you will make sure resources are 
enabled?

>   	if (ret < 0) {
>   		dev_err(gi2c->se.dev, "error turning SE resources:%d\n", ret);
>   		pm_runtime_put_noidle(gi2c->se.dev);
> @@ -1045,7 +1049,8 @@ static int geni_i2c_probe(struct platform_device *pdev)
>   	platform_set_drvdata(pdev, gi2c);
>   
>   	/* Keep interrupts disabled initially to allow for low-power modes */
> -	ret = devm_request_irq(dev, gi2c->irq, geni_i2c_irq, IRQF_NO_AUTOEN,
> +	ret = devm_request_irq(dev, gi2c->irq, geni_i2c_irq,
> +			       IRQF_NO_AUTOEN | IRQF_NO_SUSPEND | IRQF_EARLY_RESUME,
>   			       dev_name(dev), gi2c);
>   	if (ret)
>   		return dev_err_probe(dev, ret,
> @@ -1257,7 +1262,12 @@ static int __maybe_unused geni_i2c_resume_noirq(struct device *dev)
>   	if (ret)
>   		return ret;
>   
> +	/* Enforced disable_depth = 0 to actually enable runtime PM during noirq phase */
> +	if (!pm_runtime_enabled(dev))
> +		pm_runtime_enable(dev);

Here we are enabling the pm runtime so where we are disabling it?

> +
>   	i2c_mark_adapter_resumed(&gi2c->adap);
> +
>   	return 0;
>   }
>