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:
signature.asc
Description: PGP signature
