[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+-6iNzMAbvhODSX_Kn_DWe_T=9F3xzu5khdh-zdGNPyPs6FdA@mail.gmail.com>
Date: Mon, 9 Nov 2020 11:21:59 -0500
From: Jim Quinlan <james.quinlan@...adcom.com>
To: Philipp Zabel <p.zabel@...gutronix.de>,
Amjad Ouled-Ameur <aouledameur@...libre.com>
Cc: Hans de Goede <hdegoede@...hat.com>, Jens Axboe <axboe@...nel.dk>,
"maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE"
<bcm-kernel-feedback-list@...adcom.com>,
Florian Fainelli <f.fainelli@...il.com>,
"moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE"
<linux-arm-kernel@...ts.infradead.org>,
open list <linux-kernel@...r.kernel.org>
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@...gutronix.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@...adcom.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)
Download attachment "smime.p7s" of type "application/pkcs7-signature" (4167 bytes)
Powered by blists - more mailing lists