On 02/08/18 06:54, Phil Reid wrote:
On 2/08/2018 03:26, Stefan Lengfeld wrote:
Hi Keerhty,
On Tue, Jul 31, 2018 at 02:04:46PM +0530, Keerthy wrote:
Hi All,
I am looking at the poweroff scenario. At least on ARM based SoCs i see
that machine_power_off first disables the local interrupts and then
shuts off the secondary cores with smp_send_stop.
After this happens we try to shutdown the PMIC that interact over I2C.
I2C writes will need I2C interrupts to be enabled but in an interrupt
disabled context if shutting PMIC is the last thing in the sequence
which needs I2C writes is there an already existing solution to this
scenario? Any pointers would help.
Just to add my two bits of information. I was facing the same problem
two years ago and send a RFC patch set to this mailing list. For me it
was the reboot, not the poweroff scenario, but that should be
interchangeable. You can look up the thread here:
https://www.spinics.net/lists/linux-i2c/msg25401.html
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.
For the first item: I had to patch the I2C core driver to use
polling(=irqless) instead of interrupts.
For the last item: When the poweroff/restart handler code is executed,
the SoC I2C core must not be in use by another driver in the system.
There must be no other bus communication going on, because the code
cannot wait anymore for sending final I2C message to poweroff the
system.
As Wolfram said, a good solution should implement something like
'master_xfer_irqless' and add a new flag I2C_M_IRQLESS, not accessing
the `i2c_algorithm` of the I2C core driver directly as my code did.
This thread piqued my interest. I've got an ARM system that uses a gpio
on an I2C chip
to power the system down. I've been using the gpio-poweroff driver to
drive the line.
So far I haven't had any problems, but looking at the code I'm not sure
how it's working now...
I guess this hw design would complicate things with regards get the gpio
subsystem to do a
master_xfer_irqless to set the pin via the gpio subsystem
Just out of curiosity I hacked around the issue we had with the inlined
patch that follows. It just checks whether irqs are enabled or not in
the platform driver, and switches to polling mode if they are disabled.
However, based on the discussion here, it should be converted to
implement a completely new API or possibly a global variable that tells
the driver that it should operate in polling mode (basically poweroff
has been triggered.) Which one should be the preferable way to go forward?
Dealing with the new API sounds pretty cumbersome, as this would mean
that we need to change everything from the MFD / regmap level down to
the i2c platform drivers (the poweroff handler uses regmap to write to
the PMIC.) I guess we could use a shortcut from the poweroff handler to
write directly to the i2c bus though.
=================>
Subject: [RFC] i2c: omap: add fallback polling mechanism if interrupts are
disabled
If interrupts are disabled while an i2c transfer is initiated, this
most likely means we are executing the poweroff handler. In this case,
attempting to use irqs to handle i2c transfer will fail on SMP systems
due to scheduler getting confused by the situation, and not executing
the interrupt handlers for the i2c completely.
Fix this by adding a fallback polling mechanism for i2c transfer, if
interrupts are disabled.
Signed-off-by: Tero Kristo <t-kristo@xxxxxx>
---
drivers/i2c/busses/i2c-omap.c | 59
++++++++++++++++++++++++++++++++++++-------
1 file changed, 50 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index b9172f0..be75ad4 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -269,6 +269,8 @@ struct omap_i2c_dev {
[OMAP_I2C_IP_V2_IRQENABLE_CLR] = 0x30,
};
+static int omap_i2c_xfer_data(struct omap_i2c_dev *omap);
+
static inline void omap_i2c_write_reg(struct omap_i2c_dev *omap,
int reg, u16 val)
{
@@ -648,6 +650,18 @@ static void omap_i2c_resize_fifo(struct
omap_i2c_dev *omap, u8 size, bool is_rx)
(1000 * omap->speed / 8);
}
+static void omap_i2c_wait(struct omap_i2c_dev *omap)
+{
+ u16 stat;
+ u16 mask = omap_i2c_read_reg(omap, OMAP_I2C_IE_REG);
+ int count = 0;
+
+ do {
+ stat = omap_i2c_read_reg(omap, OMAP_I2C_STAT_REG);
+ count++;
+ } while (!(stat & mask) && count < 5);
+}
+
/*
* Low level master read/write transaction.
*/
@@ -657,10 +671,14 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
struct omap_i2c_dev *omap = i2c_get_adapdata(adap);
unsigned long timeout;
u16 w;
+ bool polling;
+ int ret;
dev_dbg(omap->dev, "addr: 0x%04x, len: %d, flags: 0x%x, stop: %d\n",
msg->addr, msg->len, msg->flags, stop);
+ polling = irqs_disabled();
+
if (msg->len == 0)
return -EINVAL;
@@ -683,7 +701,8 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
w |= OMAP_I2C_BUF_RXFIF_CLR | OMAP_I2C_BUF_TXFIF_CLR;
omap_i2c_write_reg(omap, OMAP_I2C_BUF_REG, w);
- reinit_completion(&omap->cmd_complete);
+ if (!polling)
+ reinit_completion(&omap->cmd_complete);
omap->cmd_err = 0;
w = OMAP_I2C_CON_EN | OMAP_I2C_CON_MST | OMAP_I2C_CON_STT;
@@ -735,8 +754,21 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
* REVISIT: We should abort the transfer on signals, but the bus goes
* into arbitration and we're currently unable to recover from it.
*/
- timeout = wait_for_completion_timeout(&omap->cmd_complete,
- OMAP_I2C_TIMEOUT);
+ if (!polling) {
+ timeout = wait_for_completion_timeout(&omap->cmd_complete,
+ OMAP_I2C_TIMEOUT);
+ } else {
+ do {
+ omap_i2c_wait(omap);
+ ret = omap_i2c_xfer_data(omap);
+ } while (ret == -EAGAIN);
+
+ if (!ret)
+ timeout = 1;
+ else
+ timeout = 0;
+ }
+
if (timeout == 0) {
dev_err(omap->dev, "controller timed out\n");
omap_i2c_reset(omap);
@@ -1038,10 +1070,8 @@ static int omap_i2c_transmit_data(struct
omap_i2c_dev *omap, u8 num_bytes,
return ret;
}
-static irqreturn_t
-omap_i2c_isr_thread(int this_irq, void *dev_id)
+static int omap_i2c_xfer_data(struct omap_i2c_dev *omap)
{
- struct omap_i2c_dev *omap = dev_id;
u16 bits;
u16 stat;
int err = 0, count = 0;
@@ -1059,7 +1089,8 @@ static int omap_i2c_transmit_data(struct
omap_i2c_dev *omap, u8 num_bytes,
if (!stat) {
/* my work here is done */
- goto out;
+ err = -EAGAIN;
+ break;
}
dev_dbg(omap->dev, "IRQ (ISR = 0x%04x)\n", stat);
@@ -1168,9 +1199,19 @@ static int omap_i2c_transmit_data(struct
omap_i2c_dev *omap, u8 num_bytes,
}
} while (stat);
- omap_i2c_complete_cmd(omap, err);
+ return err;
+}
+
+static irqreturn_t
+omap_i2c_isr_thread(int this_irq, void *dev_id)
+{
+ int ret;
+ struct omap_i2c_dev *omap = dev_id;
+
+ ret = omap_i2c_xfer_data(omap);
+ if (ret != -EAGAIN)
+ omap_i2c_complete_cmd(omap, ret);
-out:
return IRQ_HANDLED;
}
--
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
