lkml.org 
[lkml]   [2021]   [Feb]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH v5 10/14] clk: imx: Add generic blk-ctl driver
    Date


    > -----Original Message-----
    > From: Frieder Schrempf [mailto:frieder.schrempf@kontron.de]
    > Sent: Thursday, February 25, 2021 4:23 PM
    > To: Abel Vesa <abel.vesa@nxp.com>; Dong Aisheng <dongas86@gmail.com>
    > Cc: Aisheng Dong <aisheng.dong@nxp.com>; Rob Herring <robh@kernel.org>;
    > Peng Fan <peng.fan@nxp.com>; Jacky Bai <ping.bai@nxp.com>; Anson Huang
    > <anson.huang@nxp.com>; devicetree <devicetree@vger.kernel.org>;
    > Stephen Boyd <sboyd@kernel.org>; Shawn Guo <shawnguo@kernel.org>;
    > Mike Turquette <mturquette@baylibre.com>; Linux Kernel Mailing List
    > <linux-kernel@vger.kernel.org>; Marek Vasut <marek.vasut@gmail.com>;
    > dl-linux-imx <linux-imx@nxp.com>; Sascha Hauer <kernel@pengutronix.de>;
    > Fabio Estevam <fabio.estevam@nxp.com>; Philipp Zabel
    > <p.zabel@pengutronix.de>; Adam Ford <aford173@gmail.com>; linux-clk
    > <linux-clk@vger.kernel.org>; moderated list:ARM/FREESCALE IMX / MXC
    > ARM ARCHITECTURE <linux-arm-kernel@lists.infradead.org>; Lucas Stach
    > <l.stach@pengutronix.de>
    > Subject: Re: [PATCH v5 10/14] clk: imx: Add generic blk-ctl driver
    >
    > 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@nxp.com> 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.
    >

    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
    \
     
     \ /
      Last update: 2021-02-25 09:31    [W:4.708 / U:0.840 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site