Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v2 1/2] reset: brcmstb rescal: implement {de}assert()
 instead of reset()
To:     Philipp Zabel <p.zabel@pengutronix.de>,
        Jim Quinlan <james.quinlan@broadcom.com>
Cc:     Hans de Goede <hdegoede@redhat.com>, Jens Axboe <axboe@kernel.dk>,
        "maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE" 
        <bcm-kernel-feedback-list@broadcom.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        "moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE" 
        <linux-arm-kernel@lists.infradead.org>,
        open list <linux-kernel@vger.kernel.org>
References: <20201106191743.40457-1-james.quinlan@broadcom.com>
 <20201106191743.40457-2-james.quinlan@broadcom.com>
 <64aa1f860bf60ddac7a2015c87896c8a7b13b161.camel@pengutronix.de>
 <CA+-6iNzMAbvhODSX_Kn_DWe_T=9F3xzu5khdh-zdGNPyPs6FdA@mail.gmail.com>
 <2ee2fb9170e127d583e19062a791f3b1767d44fc.camel@pengutronix.de>
From:   Amjad Ouled-Ameur <aouledameur@baylibre.com>
Message-ID: <eb0d2af6-a7df-6ab1-1dfe-615675a841da@baylibre.com>
Date:   Thu, 12 Nov 2020 18:28:34 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <2ee2fb9170e127d583e19062a791f3b1767d44fc.camel@pengutronix.de>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Precedence: bulk

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

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