Hi Benjamin,
With preemption disabled, this boils down to
return system_state > SYSTEM_RUNNING (&& !0)
and will then generate a backtrace splash on each reboot on our
board:
# reboot -f
[ 12.687169] No atomic I2C transfer handler for 'i2c-0'
...
[ 12.806359] Call trace:
[ 12.808793] i2c_smbus_xfer+0x100/0x118
...
I'm not sure if this is now the expected behavior or not. There will
be
no backtraces, if I build a preemptible kernel, nor will there be
backtraces if I revert this patch.
thanks for the report.
In your case, the warning comes from shutting down a regulator during
device_shutdown(), so nothing really problematic here.
I tend to disagree. Yes it's not problematic. But from a users point of
view, you get a splash of *many* backtraces on every reboot. Btw, one
should really turn this into a WARN_ONCE(). But even in this case you
might scare users which will eventually lead to more bug reports.
However, later in
the "restart sequence", IRQs are disabled before the restart handlers
are called. If the reboot handlers would rely on irq-based
("non-atomic") i2c transfer, they might not work properly.
I get this from a technical point of view and agree that the correct
fix is to add the atomic variant to the i2c driver, which begs the
question, if adding the atomic variant to the driver will be considered
as a Fixes patch.
Do I get it correct, that in my case the interrupts are still enabled?
Otherwise I'd have gotten this warning even before your patch, correct?
Excuse my ignorance, but when are the interrupts actually disabled
during shutdown?
OTOH, the driver I'm using (drivers/i2c/busses/i2c-mt65xx.c) has no
*_atomic(). So the warning is correct. There is also [1], which seems
to
be the same issue I'm facing.
-michael
[1]
https://lore.kernel.org/linux-i2c/13271b9b-4132-46ef-abf8-2c311967bb46@xxxxxxxxxxx/
I tried to implement an atomic handler for the mt65xx, but I don't have
the respective hardware available to test it. I decided to use a
similar
approach as done in drivers/i2c/busses/i2c-rk3x.c, which calls the IRQ
handler in a while loop if an atomic xfer is requested. IMHO, this
should work with IRQs enabled and disabled, but I am not sure if this
is
the best approach...
Thanks for already looking into that. Do you want to submit it as an
actual patch? If so, you can add
Tested-by: Michael Walle <mwalle@xxxxxxxxxx>
But again, it would be nice if we somehow can get rid of this huge
splash
of backtraces on 6.7.x (I guess it's already too late 6.7).
-michael
