Re: [PATCH v2] i2c: sh_mobile: implement atomic transfers

[Wolfram Sang <wsa@the-dreams.de>]

[-- Attachment #1: Type: text/plain, Size: 1886 bytes --]

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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]