Hi Abel,
On 17.11.20 15:48, Abel Vesa wrote:
On 20-11-11 17:13:25, Dong Aisheng wrote:
On Tue, Nov 3, 2020 at 7:22 PM Abel Vesa <abel.vesa@xxxxxxx> wrote:
...
+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?
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.
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.
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.
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:
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.
Thanks
Frieder
