From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: Benjamin GAIGNARD <benjamin.gaignard@st.com>,
"lee.jones@linaro.org" <lee.jones@linaro.org>,
"robh+dt@kernel.org" <robh+dt@kernel.org>,
"mark.rutland@arm.com" <mark.rutland@arm.com>,
"mcoquelin.stm32@gmail.com" <mcoquelin.stm32@gmail.com>,
Alexandre TORGUE <alexandre.torgue@st.com>,
"tglx@linutronix.de" <tglx@linutronix.de>,
Fabrice GASNIER <fabrice.gasnier@st.com>
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"linux-stm32@st-md-mailman.stormreply.com"
<linux-stm32@st-md-mailman.stormreply.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Benjamin Gaignard <benjamin.gaignard@linaro.org>,
Pascal PAILLET-LME <p.paillet@st.com>
Subject: Re: [PATCH v4 3/3] clocksource: Add Low Power STM32 timers driver
Date: Thu, 20 Feb 2020 12:05:24 +0100 [thread overview]
Message-ID: <ee131515-cd4c-00b2-5e1f-3abefb634bdd@linaro.org> (raw)
In-Reply-To: <9cc4af9e-27d0-96c3-b3f1-20c88f89b70a@st.com>
On 20/02/2020 11:45, Benjamin GAIGNARD wrote:
[ ... ]
>>> +{
>>> + struct stm32_lp_private *priv = to_priv(clkevt);
>>> +
>>> + /* disable LPTIMER to be able to write into IER register*/
>>> + regmap_write(priv->reg, STM32_LPTIM_CR, 0);
>>> + /* enable ARR interrupt */
>>> + regmap_write(priv->reg, STM32_LPTIM_IER, STM32_LPTIM_ARRMIE);
>>> + /* enable LPTIMER to be able to write into ARR register */
>>> + regmap_write(priv->reg, STM32_LPTIM_CR, STM32_LPTIM_ENABLE);
>>> + /* set next event counter */
>>> + regmap_write(priv->reg, STM32_LPTIM_ARR, evt);
>>> +
>>> + /* start counter */
>>> + if (is_periodic)
>>> + regmap_write(priv->reg, STM32_LPTIM_CR,
>>> + STM32_LPTIM_CNTSTRT | STM32_LPTIM_ENABLE);
>>> + else
>>> + regmap_write(priv->reg, STM32_LPTIM_CR,
>>> + STM32_LPTIM_SNGSTRT | STM32_LPTIM_ENABLE);
>> The regmap config in stm32-lptimer is not defined with the fast_io flag
>> (on purpose or not?) that means we can potentially deadlock here as the
>> lock is a mutex.
>>
>> Isn't it detected with the lock validation scheme?
> It wasn't a problem with other parts of the mfd and I don't notice
> issues so I guess it is ok.
Given your comment below, the case can't happen I agree but there is
still a heavy operation with the locking.
>>> + return 0;
>>> +}
>>> +static int stm32_clkevent_lp_remove(struct platform_device *pdev)
>>> +{
>>> + return -EBUSY; /* cannot unregister clockevent */
>>> +}
>> Won't be the mfd into an inconsistent state here? The other subsystems
>> will be removed but this one will prevent to unload the module leading
>> to a situation where the mfd is partially removed but still there
>> without a possible recovery, no?
> We can't enable the timer part of the mfd at the same time than the
> other features.
Hmm, interesting case. The DT binding does not give this information,
especially in the example. You should fix the DT by giving two examples IMO.
Rob, how do you describe this situation (exclusive properties)?
> It has be exclusive and that exclude the problem you describe above.
Ok, the regmap_write is not a free operation and in this case you can
get rid of all the regmap-ish in this driver.
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
next prev parent reply other threads:[~2020-02-20 11:05 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-17 13:45 [PATCH v4 0/3] clockevent: add low power STM32 timer Benjamin Gaignard
2020-02-17 13:45 ` [PATCH v4 1/3] dt-bindings: mfd: Document STM32 low power timer bindings Benjamin Gaignard
2020-02-18 21:24 ` Rob Herring
2020-02-17 13:45 ` [PATCH v4 2/3] mfd: stm32: Add defines to be used for clkevent purpose Benjamin Gaignard
2020-02-20 9:38 ` Daniel Lezcano
2020-02-20 13:37 ` Lee Jones
2020-03-19 10:10 ` Lee Jones
2020-03-19 10:28 ` Benjamin GAIGNARD
2020-03-19 10:35 ` Daniel Lezcano
2020-02-17 13:45 ` [PATCH v4 3/3] clocksource: Add Low Power STM32 timers driver Benjamin Gaignard
2020-02-20 10:36 ` Daniel Lezcano
2020-02-20 10:45 ` Benjamin GAIGNARD
2020-02-20 11:05 ` Daniel Lezcano [this message]
2020-03-13 13:34 ` Daniel Lezcano
2020-03-13 13:45 ` Benjamin GAIGNARD
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ee131515-cd4c-00b2-5e1f-3abefb634bdd@linaro.org \
--to=daniel.lezcano@linaro.org \
--cc=alexandre.torgue@st.com \
--cc=benjamin.gaignard@linaro.org \
--cc=benjamin.gaignard@st.com \
--cc=devicetree@vger.kernel.org \
--cc=fabrice.gasnier@st.com \
--cc=lee.jones@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=mark.rutland@arm.com \
--cc=mcoquelin.stm32@gmail.com \
--cc=p.paillet@st.com \
--cc=robh+dt@kernel.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).