diff mbox series

[v2] i2c: sh_mobile: implement atomic transfers

Message ID 20200618150532.2923-1-uli+renesas@fpond.eu (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series [v2] i2c: sh_mobile: implement atomic transfers | expand

Commit Message

Ulrich Hecht June 18, 2020, 3:05 p.m. UTC
Implements atomic transfers to fix reboot/shutdown on r8a7790 Lager and
similar boards.

Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu>
Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
---

Hi!

This revision drops disabling runtime PM as it is unnecessary and implements
a couple of style and logic tweaks. Thanks to Geert and Wolfram for the review.
  
CU
Uli
   
   
Changes since v1:
- don't disable runtime PM operations for atomic transfers
- rename xfer() to sh_mobile_xfer()
- rename timeout to time_left in sh_mobile_xfer() and simplify logic
- minor style tweaks
- rebase
- add Tested-by's


 drivers/i2c/busses/i2c-sh_mobile.c | 95 ++++++++++++++++++++++--------
 1 file changed, 70 insertions(+), 25 deletions(-)

Comments

Geert Uytterhoeven June 18, 2020, 4:39 p.m. UTC | #1
Hi Uli,

On Thu, Jun 18, 2020 at 5:05 PM Ulrich Hecht <uli+renesas@fpond.eu> wrote:
> Implements atomic transfers to fix reboot/shutdown on r8a7790 Lager and
> similar boards.
>
> Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu>
> Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks for your patch!

> --- a/drivers/i2c/busses/i2c-sh_mobile.c
> +++ b/drivers/i2c/busses/i2c-sh_mobile.c

> @@ -581,12 +585,14 @@ static void start_ch(struct sh_mobile_i2c_data *pd, struct i2c_msg *usr_msg,
>         pd->pos = -1;
>         pd->sr = 0;
>

    if (pd->atomic_xfer)
            return;

and be done with it?

> -       pd->dma_buf = i2c_get_dma_safe_msg_buf(pd->msg, 8);
> -       if (pd->dma_buf)
> -               sh_mobile_i2c_xfer_dma(pd);
> -
> -       /* Enable all interrupts to begin with */
> -       iic_wr(pd, ICIC, ICIC_DTEE | ICIC_WAITE | ICIC_ALE | ICIC_TACKE);
> +       if (!pd->atomic_xfer) {
> +               pd->dma_buf = i2c_get_dma_safe_msg_buf(pd->msg, 8);
> +               if (pd->dma_buf)
> +                       sh_mobile_i2c_xfer_dma(pd);
> +               /* Enable all interrupts to begin with */
> +               iic_wr(pd, ICIC,
> +                      ICIC_DTEE | ICIC_WAITE | ICIC_ALE | ICIC_TACKE);
> +       }
>  }
>
>  static int poll_dte(struct sh_mobile_i2c_data *pd)
> @@ -637,15 +643,13 @@ static int poll_busy(struct sh_mobile_i2c_data *pd)
>         return i ? 0 : -ETIMEDOUT;
>  }
>
> -static int sh_mobile_i2c_xfer(struct i2c_adapter *adapter,
> -                             struct i2c_msg *msgs,
> -                             int num)
> +static int sh_mobile_xfer(struct sh_mobile_i2c_data *pd,
> +                        struct i2c_msg *msgs, int num)
>  {
> -       struct sh_mobile_i2c_data *pd = i2c_get_adapdata(adapter);
>         struct i2c_msg  *msg;
>         int err = 0;
>         int i;
> -       long timeout;
> +       long time_left;
>
>         /* Wake up device and enable clock */
>         pm_runtime_get_sync(pd->dev);

pm_runtime_get_sync() is a wrapper around __pm_runtime_resume(), which
does:

        might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe &&
                        dev->power.runtime_status != RPM_ACTIVE);

So if the device is not active (it is not), the might_sleep() is
triggered, and I expect a BUG splat.
However, with CONFIG_DEBUG_ATOMIC_SLEEP disabled (I disabled it on
koelsch, as it increases kernel size beyond the bootloader limit),
might_sleep() is a no-op, so nothing happens.
After enabling it (and disabling drm and media), still nothing...

It turns out ___might_sleep() does:

    if ((preempt_count_equals(preempt_offset) && !irqs_disabled() &&
         !is_idle_task(current) && !current->non_block_count) ||
        system_state == SYSTEM_BOOTING || system_state > SYSTEM_RUNNING ||
        oops_in_progress)
            return;

and as per:

    static inline bool i2c_in_atomic_xfer_mode(void)
    {
            return system_state > SYSTEM_RUNNING && irqs_disabled();
    }

system_state > SYSTEM_RUNNING, and ___might_sleep() just ignores any
issues. Oops...
After removing that check, it starts complaining:

    BUG: sleeping function called from invalid context at
kernel/locking/mutex.c:281
    in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 1, name:
systemd-shutdow

In general, pm_runtime_get_sync() is not safe to call from atomic
context.
For Renesas SoCs, I think both the power and clock domains are safe, as
the respective drivers don't sleep.  The PM core might, though.

> @@ -662,15 +666,35 @@ static int sh_mobile_i2c_xfer(struct i2c_adapter *adapter,
>                 if (do_start)
>                         i2c_op(pd, OP_START);
>
> -               /* The interrupt handler takes care of the rest... */
> -               timeout = wait_event_timeout(pd->wait,
> -                                      pd->sr & (ICSR_TACK | SW_DONE),
> -                                      adapter->timeout);
> +               if (pd->atomic_xfer) {
> +                       unsigned long j = jiffies + pd->adap.timeout;
> +
> +                       time_left = time_before_eq(jiffies, j);
> +                       while (time_left &&

Who's updating time_left?

> +                              !(pd->sr & (ICSR_TACK | SW_DONE))) {
> +                               unsigned char sr = iic_rd(pd, ICSR);
> +
> +                               if (sr & (ICSR_AL   | ICSR_TACK |
> +                                         ICSR_WAIT | ICSR_DTE)) {
> +                                       sh_mobile_i2c_isr(0, pd);
> +                                       udelay(150);
> +                               } else {
> +                                       cpu_relax();
> +                               }
> +                       }
> +               } else {
> +                       /* The interrupt handler takes care of the rest... */
> +                       time_left = wait_event_timeout(pd->wait,
> +                                       pd->sr & (ICSR_TACK | SW_DONE),
> +                                       pd->adap.timeout);
> +
> +                       /* 'stop_after_dma' tells if DMA xfer was complete */
> +                       i2c_put_dma_safe_msg_buf(pd->dma_buf, pd->msg,
> +                                                pd->stop_after_dma);
>
> -               /* 'stop_after_dma' tells if DMA transfer was complete */
> -               i2c_put_dma_safe_msg_buf(pd->dma_buf, pd->msg, pd->stop_after_dma);
> +               }
>
> -               if (!timeout) {
> +               if (!time_left) {
>                         dev_err(pd->dev, "Transfer request timed out\n");
>                         if (pd->dma_direction != DMA_NONE)
>                                 sh_mobile_i2c_cleanup_dma(pd);

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Geert Uytterhoeven June 21, 2020, 11:39 a.m. UTC | #2
Hi Wolfram,

On Thu, Jun 18, 2020 at 6:39 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Thu, Jun 18, 2020 at 5:05 PM Ulrich Hecht <uli+renesas@fpond.eu> wrote:
> > Implements atomic transfers to fix reboot/shutdown on r8a7790 Lager and
> > similar boards.
> >
> > Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu>
> > Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

> > --- a/drivers/i2c/busses/i2c-sh_mobile.c
> > +++ b/drivers/i2c/busses/i2c-sh_mobile.c

> > @@ -637,15 +643,13 @@ static int poll_busy(struct sh_mobile_i2c_data *pd)
> >         return i ? 0 : -ETIMEDOUT;
> >  }
> >
> > -static int sh_mobile_i2c_xfer(struct i2c_adapter *adapter,
> > -                             struct i2c_msg *msgs,
> > -                             int num)
> > +static int sh_mobile_xfer(struct sh_mobile_i2c_data *pd,
> > +                        struct i2c_msg *msgs, int num)
> >  {
> > -       struct sh_mobile_i2c_data *pd = i2c_get_adapdata(adapter);
> >         struct i2c_msg  *msg;
> >         int err = 0;
> >         int i;
> > -       long timeout;
> > +       long time_left;
> >
> >         /* Wake up device and enable clock */
> >         pm_runtime_get_sync(pd->dev);
>
> pm_runtime_get_sync() is a wrapper around __pm_runtime_resume(), which
> does:
>
>         might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe &&
>                         dev->power.runtime_status != RPM_ACTIVE);
>
> So if the device is not active (it is not), the might_sleep() is
> triggered, and I expect a BUG splat.
> However, with CONFIG_DEBUG_ATOMIC_SLEEP disabled (I disabled it on
> koelsch, as it increases kernel size beyond the bootloader limit),
> might_sleep() is a no-op, so nothing happens.
> After enabling it (and disabling drm and media), still nothing...
>
> It turns out ___might_sleep() does:
>
>     if ((preempt_count_equals(preempt_offset) && !irqs_disabled() &&
>          !is_idle_task(current) && !current->non_block_count) ||
>         system_state == SYSTEM_BOOTING || system_state > SYSTEM_RUNNING ||
>         oops_in_progress)
>             return;
>
> and as per:
>
>     static inline bool i2c_in_atomic_xfer_mode(void)
>     {
>             return system_state > SYSTEM_RUNNING && irqs_disabled();

So i2c atomic transfers are really meant to be used during late system suspend
only, and not in atomic context before, when irqs_disabled() is true?

>     }
>
> system_state > SYSTEM_RUNNING, and ___might_sleep() just ignores any
> issues. Oops...
> After removing that check, it starts complaining:
>
>     BUG: sleeping function called from invalid context at
> kernel/locking/mutex.c:281
>     in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 1, name:
> systemd-shutdow

Perhaps we need a checker config option, to make sure the atomic transfer
operation is exercised at least once during boot?
I guess scanning the i2c bus is an unsafe operation, but there may be
something we can do in a safe way, without side effects (e.g. redoing
the first i2c read message using atomic transfers)?

Cfr. CONFIG_ARM_PSCI_CHECKER, which cycles through hotplug and suspend
operations during boot.

> In general, pm_runtime_get_sync() is not safe to call from atomic
> context.
> For Renesas SoCs, I think both the power and clock domains are safe, as
> the respective drivers don't sleep.  The PM core might, though.

Do we need a way to let i2c slaves indicate they plan to use atomic
transfers later, so the i2c core can keep the i2c controller resumed?

Gr{oetje,eeting}s,

                        Geert
Wolfram Sang June 25, 2020, 6:59 a.m. UTC | #3
Hi Geert,

> >     static inline bool i2c_in_atomic_xfer_mode(void)
> >     {
> >             return system_state > SYSTEM_RUNNING && irqs_disabled();
> 
> So i2c atomic transfers are really meant to be used during late system suspend
> only, and not in atomic context before, when irqs_disabled() is true?

Yes. It is all some time ago, I recall we agreed that there shouldn't be
any other I2C communication at irqs_disabled() stage.

> Perhaps we need a checker config option, to make sure the atomic transfer
> operation is exercised at least once during boot?

Testing I2C controllers (in various ways) is a well-desired feature :/

> Do we need a way to let i2c slaves indicate they plan to use atomic
> transfers later, so the i2c core can keep the i2c controller resumed?

I wanted to have this originally, but in the end I gave up on it. IIRC,
you don't want to whitelist a client in general, but only the very late
messages it sends. However, if a message needs to be flagged may be
board specific. It all looked messy and hard to configure, so we ended
up with what we have now.

Take all this with a grain of salt, it's been a while since the
discussions happened.

All the best,

   Wolfram
Wolfram Sang June 25, 2020, 7:06 a.m. UTC | #4
Hi Geert,

thanks for the review!

> > @@ -581,12 +585,14 @@ static void start_ch(struct sh_mobile_i2c_data *pd, struct i2c_msg *usr_msg,
> >         pd->pos = -1;
> >         pd->sr = 0;
> >
> 
>     if (pd->atomic_xfer)
>             return;
> 
> and be done with it?

I like Uli's version a tad better in case we ever add something for both
cases after the following if-block. But I don't care much, we could
change it later.

> > -       pd->dma_buf = i2c_get_dma_safe_msg_buf(pd->msg, 8);
> > -       if (pd->dma_buf)
> > -               sh_mobile_i2c_xfer_dma(pd);
> > -
> > -       /* Enable all interrupts to begin with */
> > -       iic_wr(pd, ICIC, ICIC_DTEE | ICIC_WAITE | ICIC_ALE | ICIC_TACKE);
> > +       if (!pd->atomic_xfer) {
> > +               pd->dma_buf = i2c_get_dma_safe_msg_buf(pd->msg, 8);
> > +               if (pd->dma_buf)
> > +                       sh_mobile_i2c_xfer_dma(pd);
> > +               /* Enable all interrupts to begin with */
> > +               iic_wr(pd, ICIC,
> > +                      ICIC_DTEE | ICIC_WAITE | ICIC_ALE | ICIC_TACKE);
> > +       }

...

> After removing that check, it starts complaining:
> 
>     BUG: sleeping function called from invalid context at
> kernel/locking/mutex.c:281
>     in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 1, name:
> systemd-shutdow
> 
> In general, pm_runtime_get_sync() is not safe to call from atomic
> context.
> For Renesas SoCs, I think both the power and clock domains are safe, as
> the respective drivers don't sleep.  The PM core might, though.

Still, that sounds to me like we should protect these calls as in V1?

> > +                       time_left = time_before_eq(jiffies, j);
> > +                       while (time_left &&
> 
> Who's updating time_left?

Good question :)

Kind regards,

   Wolfram
Geert Uytterhoeven June 25, 2020, 8:18 a.m. UTC | #5
Hi Wolfram,

On Thu, Jun 25, 2020 at 9:06 AM Wolfram Sang <wsa@the-dreams.de> wrote:
> ...
>
> > After removing that check, it starts complaining:
> >
> >     BUG: sleeping function called from invalid context at
> > kernel/locking/mutex.c:281
> >     in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 1, name:
> > systemd-shutdow
> >
> > In general, pm_runtime_get_sync() is not safe to call from atomic
> > context.
> > For Renesas SoCs, I think both the power and clock domains are safe, as
> > the respective drivers don't sleep.  The PM core might, though.
>
> Still, that sounds to me like we should protect these calls as in V1?

And talk to the i2c controller while it is disabled?
That does seem to work on R-Car Gen2 (similar to SMP bringup accessing
registers of a disabled WDT?), though.
Needs testing on R-Mobile A1....

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Wolfram Sang June 25, 2020, 8:33 a.m. UTC | #6
> And talk to the i2c controller while it is disabled?

Hah, stupid me! Coming back from another topic, I overlooked this. You
are right, of course. Hmmm....
Wolfram Sang June 25, 2020, 3:16 p.m. UTC | #7
Hi Geert,

I spend some more thoughts on this.

> > > In general, pm_runtime_get_sync() is not safe to call from atomic
> > > context.
> > > For Renesas SoCs, I think both the power and clock domains are safe, as
> > > the respective drivers don't sleep.  The PM core might, though.
> >
> > Still, that sounds to me like we should protect these calls as in V1?

I still think we should guard these calls just because it is not safe to
call them from atomic contexts.

> And talk to the i2c controller while it is disabled?

Is there maybe some "always-on" property which we could add to the
respective IIC clock?

> That does seem to work on R-Car Gen2 (similar to SMP bringup accessing
> registers of a disabled WDT?), though.

Yes. Uli's patch will not cause a regression because we are already
calling i2c_transfer very late. And we do call the runtime_pm functions
currently. So, it will improve the situation there.

> Needs testing on R-Mobile A1....

That's armadillo, right? I don't have that, sadly.

Thanks,

   Wolfram
Wolfram Sang June 30, 2020, 7:45 p.m. UTC | #8
On Thu, Jun 25, 2020 at 05:16:58PM +0200, Wolfram Sang wrote:
> Hi Geert,
> 
> I spend some more thoughts on this.
> 
> > > > In general, pm_runtime_get_sync() is not safe to call from atomic
> > > > context.
> > > > For Renesas SoCs, I think both the power and clock domains are safe, as
> > > > the respective drivers don't sleep.  The PM core might, though.
> > >
> > > Still, that sounds to me like we should protect these calls as in V1?
> 
> I still think we should guard these calls just because it is not safe to
> call them from atomic contexts.
> 
> > And talk to the i2c controller while it is disabled?
> 
> Is there maybe some "always-on" property which we could add to the
> respective IIC clock?

Ping to this question...

> 
> > That does seem to work on R-Car Gen2 (similar to SMP bringup accessing
> > registers of a disabled WDT?), though.
> 
> Yes. Uli's patch will not cause a regression because we are already
> calling i2c_transfer very late. And we do call the runtime_pm functions
> currently. So, it will improve the situation there.
> 
> > Needs testing on R-Mobile A1....
> 
> That's armadillo, right? I don't have that, sadly.
> 
> Thanks,
> 
>    Wolfram
>
Geert Uytterhoeven June 30, 2020, 7:58 p.m. UTC | #9
Hi Wolfram,

On Tue, Jun 30, 2020 at 9:45 PM Wolfram Sang <wsa@the-dreams.de> wrote:
> On Thu, Jun 25, 2020 at 05:16:58PM +0200, Wolfram Sang wrote:
> > I spend some more thoughts on this.
> >
> > > > > In general, pm_runtime_get_sync() is not safe to call from atomic
> > > > > context.
> > > > > For Renesas SoCs, I think both the power and clock domains are safe, as
> > > > > the respective drivers don't sleep.  The PM core might, though.
> > > >
> > > > Still, that sounds to me like we should protect these calls as in V1?
> >
> > I still think we should guard these calls just because it is not safe to
> > call them from atomic contexts.
> >
> > > And talk to the i2c controller while it is disabled?
> >
> > Is there maybe some "always-on" property which we could add to the
> > respective IIC clock?
>
> Ping to this question...

You mean in DT? DT describes hardware, not software policy.

Anyway, won't help on R-Mobile A1, as there's a real power domain.

> > > That does seem to work on R-Car Gen2 (similar to SMP bringup accessing
> > > registers of a disabled WDT?), though.
> >
> > Yes. Uli's patch will not cause a regression because we are already
> > calling i2c_transfer very late. And we do call the runtime_pm functions
> > currently. So, it will improve the situation there.
> >
> > > Needs testing on R-Mobile A1....
> >
> > That's armadillo, right? I don't have that, sadly.


Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-sh_mobile.c b/drivers/i2c/busses/i2c-sh_mobile.c
index 2cca1b21e26e..c863f9640fcc 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -129,6 +129,7 @@  struct sh_mobile_i2c_data {
 	int sr;
 	bool send_stop;
 	bool stop_after_dma;
+	bool atomic_xfer;
 
 	struct resource *res;
 	struct dma_chan *dma_tx;
@@ -330,13 +331,15 @@  static unsigned char i2c_op(struct sh_mobile_i2c_data *pd, enum sh_mobile_i2c_op
 		ret = iic_rd(pd, ICDR);
 		break;
 	case OP_RX_STOP: /* enable DTE interrupt, issue stop */
-		iic_wr(pd, ICIC,
-		       ICIC_DTEE | ICIC_WAITE | ICIC_ALE | ICIC_TACKE);
+		if (!pd->atomic_xfer)
+			iic_wr(pd, ICIC,
+			       ICIC_DTEE | ICIC_WAITE | ICIC_ALE | ICIC_TACKE);
 		iic_wr(pd, ICCR, ICCR_ICE | ICCR_RACK);
 		break;
 	case OP_RX_STOP_DATA: /* enable DTE interrupt, read data, issue stop */
-		iic_wr(pd, ICIC,
-		       ICIC_DTEE | ICIC_WAITE | ICIC_ALE | ICIC_TACKE);
+		if (!pd->atomic_xfer)
+			iic_wr(pd, ICIC,
+			       ICIC_DTEE | ICIC_WAITE | ICIC_ALE | ICIC_TACKE);
 		ret = iic_rd(pd, ICDR);
 		iic_wr(pd, ICCR, ICCR_ICE | ICCR_RACK);
 		break;
@@ -429,7 +432,8 @@  static irqreturn_t sh_mobile_i2c_isr(int irq, void *dev_id)
 
 	if (wakeup) {
 		pd->sr |= SW_DONE;
-		wake_up(&pd->wait);
+		if (!pd->atomic_xfer)
+			wake_up(&pd->wait);
 	}
 
 	/* defeat write posting to avoid spurious WAIT interrupts */
@@ -581,12 +585,14 @@  static void start_ch(struct sh_mobile_i2c_data *pd, struct i2c_msg *usr_msg,
 	pd->pos = -1;
 	pd->sr = 0;
 
-	pd->dma_buf = i2c_get_dma_safe_msg_buf(pd->msg, 8);
-	if (pd->dma_buf)
-		sh_mobile_i2c_xfer_dma(pd);
-
-	/* Enable all interrupts to begin with */
-	iic_wr(pd, ICIC, ICIC_DTEE | ICIC_WAITE | ICIC_ALE | ICIC_TACKE);
+	if (!pd->atomic_xfer) {
+		pd->dma_buf = i2c_get_dma_safe_msg_buf(pd->msg, 8);
+		if (pd->dma_buf)
+			sh_mobile_i2c_xfer_dma(pd);
+		/* Enable all interrupts to begin with */
+		iic_wr(pd, ICIC,
+		       ICIC_DTEE | ICIC_WAITE | ICIC_ALE | ICIC_TACKE);
+	}
 }
 
 static int poll_dte(struct sh_mobile_i2c_data *pd)
@@ -637,15 +643,13 @@  static int poll_busy(struct sh_mobile_i2c_data *pd)
 	return i ? 0 : -ETIMEDOUT;
 }
 
-static int sh_mobile_i2c_xfer(struct i2c_adapter *adapter,
-			      struct i2c_msg *msgs,
-			      int num)
+static int sh_mobile_xfer(struct sh_mobile_i2c_data *pd,
+			 struct i2c_msg *msgs, int num)
 {
-	struct sh_mobile_i2c_data *pd = i2c_get_adapdata(adapter);
 	struct i2c_msg	*msg;
 	int err = 0;
 	int i;
-	long timeout;
+	long time_left;
 
 	/* Wake up device and enable clock */
 	pm_runtime_get_sync(pd->dev);
@@ -662,15 +666,35 @@  static int sh_mobile_i2c_xfer(struct i2c_adapter *adapter,
 		if (do_start)
 			i2c_op(pd, OP_START);
 
-		/* The interrupt handler takes care of the rest... */
-		timeout = wait_event_timeout(pd->wait,
-				       pd->sr & (ICSR_TACK | SW_DONE),
-				       adapter->timeout);
+		if (pd->atomic_xfer) {
+			unsigned long j = jiffies + pd->adap.timeout;
+
+			time_left = time_before_eq(jiffies, j);
+			while (time_left &&
+			       !(pd->sr & (ICSR_TACK | SW_DONE))) {
+				unsigned char sr = iic_rd(pd, ICSR);
+
+				if (sr & (ICSR_AL   | ICSR_TACK |
+					  ICSR_WAIT | ICSR_DTE)) {
+					sh_mobile_i2c_isr(0, pd);
+					udelay(150);
+				} else {
+					cpu_relax();
+				}
+			}
+		} else {
+			/* The interrupt handler takes care of the rest... */
+			time_left = wait_event_timeout(pd->wait,
+					pd->sr & (ICSR_TACK | SW_DONE),
+					pd->adap.timeout);
+
+			/* 'stop_after_dma' tells if DMA xfer was complete */
+			i2c_put_dma_safe_msg_buf(pd->dma_buf, pd->msg,
+						 pd->stop_after_dma);
 
-		/* 'stop_after_dma' tells if DMA transfer was complete */
-		i2c_put_dma_safe_msg_buf(pd->dma_buf, pd->msg, pd->stop_after_dma);
+		}
 
-		if (!timeout) {
+		if (!time_left) {
 			dev_err(pd->dev, "Transfer request timed out\n");
 			if (pd->dma_direction != DMA_NONE)
 				sh_mobile_i2c_cleanup_dma(pd);
@@ -696,14 +720,35 @@  static int sh_mobile_i2c_xfer(struct i2c_adapter *adapter,
 	return err ?: num;
 }
 
+static int sh_mobile_i2c_xfer(struct i2c_adapter *adapter,
+			      struct i2c_msg *msgs,
+			      int num)
+{
+	struct sh_mobile_i2c_data *pd = i2c_get_adapdata(adapter);
+
+	pd->atomic_xfer = false;
+	return sh_mobile_xfer(pd, msgs, num);
+}
+
+static int sh_mobile_i2c_xfer_atomic(struct i2c_adapter *adapter,
+				     struct i2c_msg *msgs,
+				     int num)
+{
+	struct sh_mobile_i2c_data *pd = i2c_get_adapdata(adapter);
+
+	pd->atomic_xfer = true;
+	return sh_mobile_xfer(pd, msgs, num);
+}
+
 static u32 sh_mobile_i2c_func(struct i2c_adapter *adapter)
 {
 	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
 }
 
 static const struct i2c_algorithm sh_mobile_i2c_algorithm = {
-	.functionality	= sh_mobile_i2c_func,
-	.master_xfer	= sh_mobile_i2c_xfer,
+	.functionality = sh_mobile_i2c_func,
+	.master_xfer = sh_mobile_i2c_xfer,
+	.master_xfer_atomic = sh_mobile_i2c_xfer_atomic,
 };
 
 static const struct i2c_adapter_quirks sh_mobile_i2c_quirks = {