On 14/12/2020 10:48, Qii Wang wrote:
On Thu, 2020-12-10 at 15:03 +0200, Grygorii Strashko wrote:
On 10/12/2020 03:56, Qii Wang wrote:
On Mon, 2020-12-07 at 18:35 +0200, Grygorii Strashko wrote:
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@xxxxxxxxxxxx>
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?
Because you must not use any IRQ driven operations after _noirq suspend state as it might (and most probably will)
cause unpredictable behavior later in suspend_enter():
arch_suspend_disable_irqs();
BUG_ON(!irqs_disabled());
^after this point any IRQ driven I2C transfer will cause IRQ to be re-enabled
if you need turn off device from platform callbacks - .master_xfer_atomic() has to be implemented and used.
Maybe my comment is a bit disturbing.Our purpose is not to call i2c and
use interrupts after _noirq pauses.So We use
i2c_mark_adapter_suspended&i2c_mark_adapter_resumed to block these i2c
transfers， There will not have any IRQ driven I2C transfer after this
point:
arch_suspend_disable_irqs();
BUG_ON(!irqs_disabled());
But some device driver will do i2c transfer after
dpm_noirq_resume_devices in dpm_resume_noirq(PMSG_RESUME) when our
driver irq hasn't resume.
void dpm_resume_noirq(pm_message_t state)
{
dpm_noirq_resume_devices(state);
Just to clarify. You have resume sequence in dpm_noirq_resume_devices
dpm_noirq_resume_devices -> resume I2C -> resume some device -> do i2c transfer after?
Yes.
huh. First consider IRQF_EARLY_RESUME - it's better, but still will be a hack
Is "some device" in Kernel mainline?
The problematic device driver is drivers/regulator/da9211-regulator.c in
Kernel mainline.
regulator is passive device, somebody should call it !?
And da9211-regulator IRQ handler should remain disabled till resume_device_irqs() call.
note. regulator_class implements only
static const struct dev_pm_ops __maybe_unused regulator_pm_ops = {
.suspend = regulator_suspend,
.resume = regulator_resume,
};
resume_device_irqs();
device_wakeup_disarm_wake_irqs();
cpuidle_resume();
}
.master_xfer_atomic() seems to be invalid for this question at this
time?
--
Best regards,
grygorii
