Re: [PATCH v7 1/2] i2c: qcom-geni: Handle runtime PM disabled state during early resume

[Andi Shyti]

Hi Mukesh,

...

> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 96dbf04138be..4bc00922cd97 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -917,6 +917,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;
> +	}

I have to agree with Praveen here. Ignoring -EACCES is not safe:
geni_i2c_runtime_resume() is not guaranteed to have run in that
case.

Also, pm_runtime_force_resume() already re-enables runtime PM
before returning, so the additional pm_runtime_enable() is
unnecessary.

>  	if (ret < 0) {
>  		dev_err(gi2c->se.dev, "error turning SE resources:%d\n", ret);
>  		pm_runtime_put_noidle(gi2c->se.dev);
> @@ -1115,7 +1119,8 @@ static int geni_i2c_probe(struct platform_device *pdev)
>  		return ret;
>  
>  	/* 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);

IRQF_EARLY_RESUME and IRQF_NO_SUSPEND are reundant. Perhaps for
this use case we should leave only IRQF_EARLY_RESUME?

Thanks,
Andi

>  	if (ret)
>  		return dev_err_probe(dev, ret,