Re: [v2] i2c: mediatek: Move suspend and resume handling to NOIRQ phase

From: Qii Wang <qii.wang@mediatek.com>
To: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: srv_heupstream@mediatek.com, Wolfram Sang <wsa@the-dreams.de>,
	leilk.liu@mediatek.com, linux-kernel@vger.kernel.org,
	linux-mediatek@lists.infradead.org, linux-i2c@vger.kernel.org,
	matthias.bgg@gmail.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [v2] i2c: mediatek: Move suspend and resume handling to NOIRQ phase
Date: Mon, 7 Dec 2020 15:33:51 +0800	[thread overview]
Message-ID: <1607326431.25719.33.camel@mhfsdcap03> (raw)
In-Reply-To: <629d171a-0e77-3d74-ae23-e6439dcf17b7@ti.com>

Hi:
Thank you very much for your patience review.
There are two main purposes of this patch:
1.i2c_mark_adapter_suspended&i2c_mark_adapter_resumed
Avoid accessing the adapter while it is suspended by marking it
suspended during suspend.  This allows the I2C core to catch this, and
print a warning.
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20181219164827.20985-2-wsa+renesas@sang-engineering.com/

2. IRQF_NO_SUSPEND.
Having interrupts disabled means not only that an interrupt will not
occur at an awkward time, but also that using any functionality that
requires interrupts will not work. So if the driver uses an I2C bus or
similar to tell the device to turn off, and if the I2C bus uses
interrupts to indicate completion (which is normal), then either the
device must be powered-off in suspend_late, so the I2C interrupt must be
marked IRQF_NO_SUSPEND.
https://patchwork.kernel.org/project/linux-acpi/patch/20180923135812.29574-8-hdegoede@redhat.com/

On Thu, 2020-12-03 at 10:01 +0200, Grygorii Strashko wrote:
> 
> On 03/12/2020 03:25, Qii Wang wrote:
> > On Wed, 2020-12-02 at 16:35 +0100, Wolfram Sang wrote:
> >> Hi,
> >>
> >>> Some i2c device driver indirectly uses I2C driver when it is now
> >>> being suspended. The i2c devices driver is suspended during the
> >>> NOIRQ phase and this cannot be changed due to other dependencies.
> >>> Therefore, we also need to move the suspend handling for the I2C
> >>> controller driver to the NOIRQ phase as well.
> >>>
> >>> Signed-off-by: Qii Wang <qii.wang@mediatek.com>
> >>
> >> Is this a bugfix and should go into 5.10? Or can it wait for 5.11?
> >>
> > 
> > Yes, Can you help to apply it into 5.10? Thanks
> 
> To be honest if you still do have any i2c device which accessing i2c buss after _noirq
> stage and your driver does not implement .master_xfer_atomic() - you definitely have a bigger problem.
> So adding IRQF_NO_SUSPEND sound like a hack and probably works just by luck.
> 

At present, it is only a problem caused by missing interrupts,
and .master_xfer_atomic() just a implement in polling mode. Why not set
the interrupt to a state that can always be triggered?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel