-----Original Message-----
From: Frieder Schrempf [mailto:frieder.schrempf@kontron.de]
Sent: Thursday, February 25, 2021 4:23 PM
To: Abel Vesa <redacted>; Dong Aisheng <redacted>
Cc: Aisheng Dong <aisheng.dong@nxp.com>; Rob Herring <robh@kernel.org>;
Peng Fan [off-list ref]; Jacky Bai [off-list ref]; Anson Huang
[off-list ref]; devicetree [off-list ref];
Stephen Boyd [off-list ref]; Shawn Guo [off-list ref];
Mike Turquette [off-list ref]; Linux Kernel Mailing List
[off-list ref]; Marek Vasut [off-list ref];
dl-linux-imx [off-list ref]; Sascha Hauer [off-list ref];
Fabio Estevam [off-list ref]; Philipp Zabel
[off-list ref]; Adam Ford [off-list ref]; linux-clk
[off-list ref]; moderated list:ARM/FREESCALE IMX / MXC
ARM ARCHITECTURE [off-list ref]; Lucas Stach
[off-list ref]
Subject: Re: [PATCH v5 10/14] clk: imx: Add generic blk-ctl driver

Hi Abel,

On 17.11.20 15:48, Abel Vesa wrote:

quoted

On 20-11-11 17:13:25, Dong Aisheng wrote:

quoted

On Tue, Nov 3, 2020 at 7:22 PM Abel Vesa [off-list ref] wrote:
...

quoted

+static int imx_blk_ctl_reset_set(struct reset_controller_dev *rcdev,
+                                 unsigned long id, bool assert) {
+       struct imx_blk_ctl_drvdata *drvdata = container_of(rcdev,
+                       struct imx_blk_ctl_drvdata, rcdev);
+       unsigned int offset = drvdata->rst_hws[id].offset;
+       unsigned int shift = drvdata->rst_hws[id].shift;
+       unsigned int mask = drvdata->rst_hws[id].mask;
+       void __iomem *reg_addr = drvdata->base + offset;
+       unsigned long flags;
+       u32 reg;
+
+       if (!assert && !test_bit(1, &drvdata->rst_hws[id].asserted))
+               return -ENODEV;

What if consumers call deassert first in probe which seems common in

kernel?

quoted

quoted

It seems will fail.
e.g.
probe() {
     reset_control_get()
     reset_control_deassert()
}

Regards
Aisheng

OK, I'm trying to explain here how I know the resets are supposed to
be working and how the BLK_CTL IP is working.


First of, the BLK_CTL bits (resets and clocks) all have the HW init
(default) values as 0. Basically, after the blk_ctl PD is powered on,
the resets are deasserted and clocks are gated by default. Since the
blk_ctl is not the parent of any of the consumers in devicetree (the
reg maps are entirely different anyway), there is no way of ordering
the runtime callbacks between the consumer and the blk_ctl. So we
might end up having the runtime resume callback after the one from
EARC (consumer), for example, which will basically overwrite the value

written by EARC driver with whatever was saved on suspend.

quoted

Now, about the usage of the reset bits. AFAICT, it would make more
sense to assert the reset, then enable the clock, then deassert. This
way, you're keeping the EARC (consumer) in reset (with the clocks on)
until you eventually release it out of reset by deasserting. This is
how the runtime resume should deal with the reset and the clock. As
for the runtime suspend, the reset can be entirely ignored as long as you're

disabling the clock.

quoted

This last part will allow the blk_ctl to make the following assumption:
if all the clocks are disabled and none of the reset bits are asserted, I can

power off.

quoted

Now, I know there are drivers outthere that do assert on suspend, but
as long as the clocks are disabled, the assert will have no impact.
But maybe in their case the reset controller cannot power down itself.

As for the safekeeping of the register, I'll just drop it due to the following

arguments:

quoted

1. all the clocks are gated by default 2. all resets are deasserted by
default 3. when blk_ctl goes down, all the consumers go down. (all
have the same PD)

 From 1 and 2 results the IP will not be running and from 3 results
the HW state of every IP becomes HW init state.

Are there any plans to continue this work? As BLK-CTL it is not only relevant
for the i.MX8MP, but also for i.MX8MM and i.MX8MN, it would be nice to get
this ready in order to prepare for proper graphics/display support.

Before continuing this work, we need to find out a way to resolve the cycling dependency issue between power domain and blk-ctrl.
it is indeed introduced some troubles in NXP latest internal release when the blk-ctrl driver is added.

BR
Jacky Bai

Thanks
Frieder