Hi all,
to continue the discussion: Here is a new spin of an I2C IRQ less patch
set.
The approach does not introduce a new I2C transfer callback like
master_xfer_irqless(). Instead if follows Tero Kristo first patch and
the I2C driver code should detect itself, whether it's called in IRQ
disabled context with code like
bool polling = irqs_disabled();
Using this function seems appropriate, because the I2C core does the
same in the function i2c_transfer().
Furthermore the patchset introduces a boolean flag 'irq_safe' in the
i2c_adapter. Every driver author should announce in the driver's probe
function whether the driver is safe to be called in atomic or IRQ
disabled contexts with the function i2c_adapter_irq_safe().
The idea is taken from the PM subsystem and the function
pm_runtime_irq_safe().
The i2c_adapter_irq_safe() function should address Wolfram Sang's
comment:
> * Also, there is no white-listing, all transfers will be executed, so
> buggy drivers will go unnoticed.
Drivers that are not coded with IRQ less operation in mind get noticed
at once.
Responding to Tero Kristo's comment:
> So, what is the plan going forward?
>
> 1) Modify i2c core APIs? And modify all relevant i2c drivers to support this?
> 2) Just add the shutdown handler switch to the relevant drivers? (My RFCv2
> patch)
> 3) Something else?
This patchset combines your RFCv1 patch with a minimal I2C core change.
For Wolfram Sang's comment:
> IIRC the latest design we discussed is to add a new callback to struct
> i2c_algorithm like 'master_xfer_irqless' and teach the I2C core when to
> call which callback. Which might not be so super straightforward because
> for most drivers (except PMICs probably) using I2C when interrupts are
> disabled is a bug and we also shouldn't hide that by providing a generic
> fallback.
To detect buggy users of the I2C API, we would need to change the I2C
API and add a function like:
i2c_transfer(); // existing function
i2c_transfer_irqless(); // new function for PMIC drivers
Using a flag like 'I2C_M_IRQLESS' seems wrong, because IRQ less
operation is a per-transfer, not per message thing.
Nevertheless, as you already said, such a new API function would
required tree-wide changes, e.g in the regmap framework.
For my comment:
> There are two distinct problems:
> * Sending I2C messages in an atomic/irqless/sleep-free context
> * Blocking the I2C-adapter for other users before starting the
> sequence poweroff/reboot.
The second issue I cannot reproduce anymore. Maybe it's due to having a
single core i.MX6 or something else. So the corresponding patch to
modify the I2C API is left out in the patchset.
I
Just a not for development the code. Without the help of the LOCKDEP
framework and config options:
CONFIG_PROVE_LOCKING=y
CONFIG_DEBUG_ATOMIC_SLEEP=y
it's nearly impossible two write sleep/schedule free code. There are
just to many kernel functions that sleep internally.
The patches are tested on a kernel v4.18-rc8 and a phyCORE-i.MX6 Solo
board.
Kind regards,
Stefan Lengfeld
Stefan Christ (1):
ARM: dts: phyboard-mira-dl: rely on PMIC for reboot and watchdog
Stefan Lengfeld (3):
i2c: allow drivers to announce that they are IRQ safe
i2c: imx: implement IRQ less master_xfer function
watchdog: da9062: avoid regmap in restart handler
arch/arm/boot/dts/imx6dl-phytec-mira-rdk-nand.dts | 8 ++
drivers/i2c/busses/i2c-imx.c | 113 ++++++++++++++++------
drivers/i2c/i2c-core-base.c | 8 ++
drivers/mfd/da9062-core.c | 1 +
drivers/watchdog/da9062_wdt.c | 17 +++-
include/linux/i2c.h | 16 +++
include/linux/mfd/da9062/core.h | 1 +
7 files changed, 128 insertions(+), 36 deletions(-)
--
2.16.4
