Re: [PATCH] i2c: qcom-geni: Keep comment why interrupts start disabled

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Wolfram,

On Mon, Sep 16, 2024 at 02:15:17PM GMT, Wolfram Sang wrote:
> The to-be-fixed commit rightfully reduced a race window, but also
> removed a comment which is still helpful after the fix. Bring the
> comment back.
> 
> Fixes: e2c85d85a05f ("i2c: qcom-geni: Use IRQF_NO_AUTOEN flag in request_irq()")
> Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/i2c/busses/i2c-qcom-geni.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 4c9050a4d58e..03c05dcc2202 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -818,6 +818,8 @@ static int geni_i2c_probe(struct platform_device *pdev)
>  	init_completion(&gi2c->done);
>  	spin_lock_init(&gi2c->lock);
>  	platform_set_drvdata(pdev, gi2c);
> +
> +	/* Disable the interrupt so that the system can enter low-power mode */

Thanks for the patch! However, I wouldn’t typically consider this
a fix, and I don’t think it would qualify for stable release
inclusion.

That said, I agree that a comment should be added back. The original
comment no longer fits as well as it did before. A more
appropriate comment would be:

/*
 * Do not enable interrupts by default to allow the system to enter
 * low-power mode. The driver will explicitly enable interrupts when
 * needed using enable_irq().
 */

Does it make sense?

Thanks,
Andi

PS If you want I can add it to the current i2c-host-fixes and
retrigger the pull request.

>  	ret = devm_request_irq(dev, gi2c->irq, geni_i2c_irq, IRQF_NO_AUTOEN,
>  			       dev_name(dev), gi2c);
>  	if (ret) {
> -- 
> 2.45.2
> 




[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux