Re: BD71847 clk driver disables clk-32k-out causing RTC/WDT failure
From: Peng Fan
Date: Mon Sep 12 2022 - 22:31:01 EST
On 9/13/2022 1:15 AM, Tim Harvey wrote:
On Mon, Sep 12, 2022 at 12:40 AM Peng Fan <peng.fan@xxxxxxxxxxx> wrote:
On 9/9/2022 1:06 PM, Matti Vaittinen wrote:
Hi dee Ho peeps,
On 9/9/22 05:35, Marek Vasut wrote:
On 9/9/22 04:06, Peng Fan wrote:
Subject: Re: BD71847 clk driver disables clk-32k-out causing RTC/WDT
failure
On 9/8/22 21:25, Tim Harvey wrote:
On Thu, Sep 8, 2022 at 9:55 AM Marek Vasut <marex@xxxxxxx> wrote:
On 9/8/22 18:00, Tim Harvey wrote:
On Thu, Sep 1, 2022 at 9:14 PM Matti Vaittinen
<mazziesaccount@xxxxxxxxx> wrote:
Hi Tim,
On 9/2/22 01:23, Tim Harvey wrote:
Greetings,
I've found that the bd71847 clk driver
(CONFIG_COMMON_CLK_BD718XX
drivers/clk/clk-bd718x7.c) disables clk-32k-out (the BD71847
C32K_OUT
pin) which is connected IMX8MM RTC_XTALI which ends up disabling
the IMX RTC as well as the IMX WDOG functionality.
//snip
This happens via clk_unprepare_unused() as nothing is flagging the
clk-32k-out as being used. What should be added to the device-tree
to signify that this clk is indeed necessary and should not be
disabled?
I have seen following proposal from Marek Vasut:
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fl
ore.kernel.org%2Fall%2F20220517235919.200375-1-
marex%40denx.de%2FT%
2F%23m52d6d0831bf43d5f293e35cb27f3021f278d0564&data=05%7C0
1%7Cp
eng.fan%40nxp.com%7C07d48edcc47c4694e08208da91da2bf4%7C686ea1d
3bc2b
4c6fa92cd99c5c301635%7C0%7C0%7C637982664162868785%7CUnknown%
7CTWFpb
GZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI
6
Mn0%3D%7C3000%7C%7C%7C&sdata=uF26u9g4onuqCWzPRAvD%2F%
2FLByaEhh5
Dtah9K8CcAOAM%3D&reserved=0
I am not sure if the discussion is completed though. I guess it was
agreed this was needed/usefull and maybe the remaining thing to
decide was just the property naming.
Best Regards
-- Matti
Thanks Matti,
Marek - has there been any progress on determining how best to keep
certain clocks from being disabled?
No. You can read the discussion above.
Marek,
I wasn't on the linux-clk list at that time so can't respond to the
thread but the discussion seems to have died out a couple of months
ago with no agreement between you or Stephen on how to deal with it.
So where do we take this from here? It looks like there are about 18
boards with dt's using "rohm,bd718*" which would all have non working
RTC/WDOG with CONFIG_COMMON_CLK_BD718XX enabled (which it is in
arch/arm64/configs/defconfig) right?
Yeah. The ROHM BD71837 and BD71847 (and BD71850 - which is one of the
variants) are used quite a lot. I am pretty sure not fixing this in
upstream is increasing downstream solutions. I don't think that should
be preferred.
Is there any requirement that the bd718xx clk needs to be runtime
on/off?
Yes, the 32kHz clock on BD71xxx should behave like any other clock,
unless specified otherwise, see below.
I suppose the clk should always be never be off, if yes, why not have
something:
What is needed in this specific case of BD718xx is I think clock
consumer on the MX8M clock driver side which would claim the 32kHz
input from the PMIC and up the clock enable count to keep the 32 kHz
clock always on.
This sounds like a solution that would describe the actual HW setup. I
don't know the CCF of the i.MX8 well enough to tell whether this can
ensure the clk is not disabled before the consumer is found or when the
consumer is going down though. Simplest thing to me would really be to
just mark the clk as "do-not-touch" one on the boards where it must not
be touched.
The PMIC is most likely supplying 32 kHz clock to the MX8M,
which if the 32 kHz clock are turned off would hang (I observed that
before too).
What I tried to address in this thread is a generic problem which
commonly appears on various embedded systems, except every time anyone
tried to solve it in a generic manner, it was rejected or they gave up.
I agree with Marek - generic solution would be nice. I don't think this
is something specific to this PMIC.
The problem is this -- you have an arbitrary clock, and you need to
keep it running always otherwise the system fails, and you do not have
a clock consumer in the DT for whatever reason e.g. because the SoC is
only used as a clock source for some unrelated clock net. There must
be a way to mark the clock as "never disable these", i.e.
critical-clock. (I feel like I keep repeating this over and over in
this thread, so please read the whole thread backlog)
Thanks for the explanation and effor you did Marek.
My take on this is that from a (generic) component vendor perspective it
is a bad idea to hard-code the clock status (enable/disable) in the PMIC
driver. A vendor wants to provide a driver which allows use of the
component in wide variety of systems/boards. When the PMIC contains a
clock gate, the PMIC driver should provide the means of controlling it.
Some setups may want it enabled, other disabled and some want runtime
control. This "use-policy" must not be hard coded in the driver - it
needs to come from HW description which explains how the clk line is
wired and potentially also from the consumer drivers. This enables the
same PMIC driver to support all different setups with their own needs,
right?
I am not sure if some non email discussions have been ongoing around
this topic but just by reading the emails it seemed to me that Marek's
suggestion was acked by the DT folks - and I don't think that Stephen
was (at the end of the day) against that either(?). Maybe I missed
something.
After a thought, maybe an easier way is to add a optional property
xxx,32k-always-on to the pmic node/driver.
Regards,
Peng.
Is there simply a way to add the clk to the snvs_rtc and the wdog dt
nodes so they have a use count and don't get disabled?
I just see snvs_rtc requires osc 32k, but I not see wdog requires 32KHz.
"
The 32KHz XTAL module uses a different IP and it is used as the clock
source for the
RTC, located in the SNVS.
"
Regards,
Peng.
Best Regards,
Tim