Messages in this thread | | | From | Jim Quinlan <> | Date | Mon, 9 Nov 2020 11:21:59 -0500 | Subject | Re: [PATCH v2 1/2] reset: brcmstb rescal: implement {de}assert() instead of reset() |
| |
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.
Thanks and kind regards, Jim Quinaln Broadcom STB > > > > regards > Philipp > > > --- > > drivers/reset/reset-brcmstb-rescal.c | 13 ++++++++++--- > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/reset/reset-brcmstb-rescal.c b/drivers/reset/reset-brcmstb-rescal.c > > index b6f074d6a65f..1f54ae4f91fe 100644 > > --- a/drivers/reset/reset-brcmstb-rescal.c > > +++ b/drivers/reset/reset-brcmstb-rescal.c > > @@ -20,8 +20,8 @@ struct brcm_rescal_reset { > > struct reset_controller_dev rcdev; > > }; > > > > -static int brcm_rescal_reset_set(struct reset_controller_dev *rcdev, > > - unsigned long id) > > +static int brcm_rescal_reset_deassert(struct reset_controller_dev *rcdev, > > + unsigned long id) > > { > > struct brcm_rescal_reset *data = > > container_of(rcdev, struct brcm_rescal_reset, rcdev); > > @@ -52,6 +52,12 @@ static int brcm_rescal_reset_set(struct reset_controller_dev *rcdev, > > return 0; > > } > > > > +static int brcm_rescal_reset_assert(struct reset_controller_dev *rcdev, > > + unsigned long id) > > +{ > > + return 0; > > +} > > + > > static int brcm_rescal_reset_xlate(struct reset_controller_dev *rcdev, > > const struct of_phandle_args *reset_spec) > > { > > @@ -60,7 +66,8 @@ static int brcm_rescal_reset_xlate(struct reset_controller_dev *rcdev, > > } > > > > static const struct reset_control_ops brcm_rescal_reset_ops = { > > - .reset = brcm_rescal_reset_set, > > + .deassert = brcm_rescal_reset_deassert, > > + .assert = brcm_rescal_reset_assert, > > }; > > > > static int brcm_rescal_reset_probe(struct platform_device *pdev) [unhandled content-type:application/pkcs7-signature] | |