> IMHO the is_suspended flag should be set by the adapter driver from its
> own suspend callback (which may be a normal, late or noirq suspend callback),
> as this is the point past which the bus should no longer be used because
> e.g. the suspend callbacks has disabled clocks and/or put the controller
> in reset, etc.
That's what I meant with "not seeing the woods for the trees". Thanks
for clearing my view!
> the i2c-controller device where necessary. But you're hooking into the
> child-device of the actual "hardware" device which represents the i2c_adapter
> bits.
That was intentional because this is the device the I2C core has access
to. Because the callbacks deal only with 'is_suspended' and not with HW,
I thought the logic device could be suitable. I didn't think of
additional relations only affecting its parent.
> on the adapter-device. I believe that instead we should add
> i2c_adapter_mark_suspended and i2c_adapter_mark_resumed helpers and let
> the adapter-driver suspend/resume callbacks call these. This will ensure
> that they get called at the right phase and at the exact moment that
> the adapter is actually suspending.
Agreed. And these helpers are basically this:
> > + i2c_lock_bus(adap, I2C_LOCK_ROOT_ADAPTER);
> > + adap->is_suspended = true;
> > + i2c_unlock_bus(adap, I2C_LOCK_ROOT_ADAPTER);
and this:
> > + i2c_lock_bus(adap, I2C_LOCK_ROOT_ADAPTER);
> > + adap->is_suspended = false;
> > + i2c_unlock_bus(adap, I2C_LOCK_ROOT_ADAPTER);
together with this still in the I2C core:
> > @@ -1867,6 +1904,9 @@ int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> > if (WARN_ON(!msgs || num < 1))
> > return -EINVAL;
> > + if (WARN_ON(adap->is_suspended))
> > + return -ESHUTDOWN;
> > +
> > if (adap->quirks && i2c_check_for_quirks(adap, msgs, num))
> > return -EOPNOTSUPP;
> > diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> > index 65b4eaed1d96..ee46295a67d4 100644
> > --- a/include/linux/i2c.h
> > +++ b/include/linux/i2c.h
> > @@ -692,6 +692,8 @@ struct i2c_adapter {
> > const struct i2c_adapter_quirks *quirks;
> > struct irq_domain *host_notify_domain;
> > +
> > + int is_suspended:1;
Are we aligned?
Thanks,
Wolfram
Attachment:
signature.asc
Description: PGP signature
