> > > I think this might be as simple as adding: > > > > > > if (WARN_ON(adap->dev.parent->power.is_suspended)) > > > return -ESHUTDOWN; Peter, I think this should work for muxes, too, or? The i2c_transfer() call to the mux will not be rejected, but it will be later when we reach the root adapter. And then the error code will be pushed down the tree until we arrive at the mux again. So, the rejection will not happen at the earliest time, but it will happen. Is my understanding correct? > > I have seen this flag but decided against it. One reason is because it > > is marked as "PM core only". > > Right and we definitely should not be touching it, but reading it should > be fine. Seems like it. So far, no rejection from the other PM people :) > No you are right, there is a race here, but I don't think we are likely to > hit that race. Normally there won't be any ongoing i2c-transfers during > a system suspend and more over, the goal of adding this check is to help > find problems, so even if the check sometimes does not trigger because > of the race that is not really a big deal. You are right that the impact of a missed detection is not fatal. That helps. The low likeliness was not an argument for me, though, because detecting rare things is very important IMO. Because, well, they are rare and especially hard to debug. > I think we need to get really unlucky to have both a suspend ordering > problem in the first case (already a somewhat rare thing) combined with > hitting this race in such a way *each time* that we don't trigger the > WARN_ON. And here you convinced me: even if we miss a detection once, I agree it is super super unlikely to hit the race every time. > To me this seems a case of perfect being the enemy of good. When we > first started discussing this you wanted to not have to modify the > adapter/bus drivers for the check, using adap->dev.parent->power.is_suspended > gives us that and it will also work for complex cases like > the i2c-designware case, so I believe the benefits outway the downsides. I'll try it.
Attachment:
signature.asc
Description: PGP signature
