Re: [PATCH v2 1/2] reset: brcmstb rescal: implement {de}assert() instead of reset()

[Amjad Ouled-Ameur]

Hi everyone,


On 09/11/2020 18:25, Philipp Zabel wrote:
> 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@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> > > 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@xxxxxxxxxxxx/
> > 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.

My pleasure, I will send a V2 soon of the patch, when it is done, please
let me know if I can add anything that would suit best your use case as well.

> 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.

I think reset_control_rearm would be a very good candidate, resettable
is quite representative but I think it is best to keep using verbs for
the sake of homogeneity

> regards
> Philipp

Sincerely,

Amjad