On Wed, Feb 06, 2019 at 07:12:04PM +0100, Philipp Zabel wrote: > On Wed, 2019-02-06 at 17:00 +0100, Thierry Reding wrote: [...] > > That way we operate on the same reset control, but we wouldn't need to > > iterate over all existing reset controls in order to determine whether > > we can acquire or not. > > How could we decide in reset_control_assert whether the provided rstc is > allowed to change the reset line if both rstc handles point to the same > struct reset_control? The idea was that acquire/release would basically act as lock/unlock for the reset control. So consumers would always have to call acquire() before assert()/deassert()/reset() and they would be allowed to continue only if acquire() returns success. Of course that's something you can only enforce during code review, but that's pretty much the same as with any type of locking. So basically the idea is that if a consumers acquire() call succeeds, the acquired flag gets set on the reset control and that consumer becomes the only user allowed to modify the reset control. Any other consumers would call acquire() and fail because the acquired is already true. But what you proposed works for me. We can always find constructive ways to optimize this later if we need or want to. Another possibility would be to keep an additional, separate list of the temporarily exclusive resets so that only that list would have to be iterated to find the ones that are relevant. > > > if (WARN_ON(!rstc->shared || !shared)) > > > return ERR_PTR(-EBUSY); > > > > With the above I think we could just extend this list of conditions with > > > > || acquired > > > > and things should work the same. Or perhaps I'm missing something. > > > > Other than that this really looks like a very nice solution. > > Well, apart from the API function names... > devm_reset_control_get_optional_exclusive_released(dev, "id"); > would be a mouthful. Yeah, the combinations are somewhat awkward. However, I would expect the temporarily exclusive resets to be required in most cases, so that would at least make the name a little shorter. Thierry
Attachment:
signature.asc
Description: PGP signature
