Messages in this thread | | | Subject | Re: [PATCH v2 1/2] reset: brcmstb rescal: implement {de}assert() instead of reset() | From | Philipp Zabel <> | Date | Mon, 09 Nov 2020 18:25:40 +0100 |
| |
On Mon, 2020-11-09 at 11:21 -0500, Jim Quinlan wrote: > On Mon, Nov 9, 2020 at 5:05 AM Philipp Zabel <p.zabel@pengutronix.de> wrote: > > Hi Jim, > > > > On Fri, 2020-11-06 at 14:17 -0500, Jim Quinlan wrote: > > > Before, only control_reset() was implemented. However, the reset core only > > > invokes control_reset() once in its lifetime. Because we need it to invoke > > > control_reset() again after resume out of S2 or S3, we have switched to > > > putting the reset functionality into the control_deassert() method and > > > having an empty control_assert() method. > > > > > > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com> > > > > You are switching to the wrong abstraction to work around a deficiency > > of the reset controller framework. Instead, it would be better to allow > > to "reactivate" shared pulsed resets so they can be triggered again. > > True. > > > > Could you please have a look at [1], which tries to implement this with > > a new API call, and check if this can fix your problem? If so, it would > > be great if you could coordinate with Amjad to see this fixed in the > > core. > > > > [1] https://lore.kernel.org/lkml/20201001132758.12280-1-aouledameur@baylibre.com/ > > Yes, this would work for our usage. Amjad please let me know if I can > help here. The only "nit" I have is that I favor the name 'unreset' > over 'resettable' but truly I don't care one way or the other.
Both unreset and resettable are adjectives, maybe it would be better to have an imperative verb like the other API functions. I would have liked reset_control_trigger/rearm as a pair, but I can't find anything I like that fits with the somewhat unfortunate reset_control_reset name in my mind. In that sense, I don't have a preference one way or the other either.
regards Philipp
| |