On Mon, 2020-08-24 at 16:26 +0200, Jerome Brunet wrote: [...] > In practice, I think your proposition would work since the drivers > sharing this USB reset line are likely to be probed/suspended/resumed at > the same time but ... > > If we imagine a situation where 2 device share a reset line, 1 go in > suspend, the other does not - if the first device as control of the > reset, it could trigger it and break the 2nd device. Same goes for > probe/remove() > > I agree it could be seen as unlikely but leaving such race condition > open looks dangerous to me. You are right, this is not good enough. > > Is this something that would be feasible for your combination of > > drivers? Otherwise it is be unclear to me under which condition a driver > > should be allowed to call the proposed reset_control_clear(). > > I was thinking of reset_control_clear() as the counter part of > reset_control_reset(). I'm not particularly fond of reset_control_clear as a name, because it is very close to "clearing a reset bit" which usually would deassert a reset line (or the inverse). > When a reset_control_reset() has been called once, "triggered_count" is > incremented which signals that the ressource under the reset is > "in_use" and the reset should not be done again. "triggered_count" would then have to be renamed to something like "trigger_requested_count", or "use_count". I wonder it might be possible to merge this with "deassert_count" as they'd share the same semantics (while the count is > 0, the reset line must stay deasserted). > reset_control_clear() > would be the way to state that the ressource is no longer used and, that > from the caller perspective, the reset can fired again if necessary. > > If we take the probe / suspend / resume example: > * 1st device using the shared will actually trigger it (as it is now) > * following device just increase triggered_count > > If all devices go to suspend (calling reset_control_clear()) then > triggered_count will reach zero, allowing the first device resuming to > trigger the reset again ... this is important since it might not be the > one which would have got the exclusive control > > If any device don't go to suspend, meaning the ressource under reset > keep on being used, no reset will performed. With exlusive control, > there is a risk that the resuming device resets something already in use. > > Regarding the condition, on shared resets, call reset_control_reset() > should be balanced reset_control_clear() - no clear before reset. Martin, is this something that would be useful for the current users of the shared reset trigger functionality (phy-meson-gxl-usb2 and phy- meson8b-usb2 with reset-meson)? regards Philipp
