From: Abel Vesa <abel.vesa@nxp.com>
To: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Dong Aisheng <aisheng.dong@nxp.com>,
Rob Herring <robh@kernel.org>, Peng Fan <peng.fan@nxp.com>,
Fugang Duan <fugang.duan@nxp.com>,
Anson Huang <anson.huang@nxp.com>,
devicetree@vger.kernel.org, Stephen Boyd <sboyd@kernel.org>,
Mike Turquette <mturquette@baylibre.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
NXP Linux Team <linux-imx@nxp.com>,
Sascha Hauer <kernel@pengutronix.de>,
Fabio Estevam <fabio.estevam@nxp.com>,
Shawn Guo <shawnguo@kernel.org>,
linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 11/17] clk: imx: Add blk_ctrl combo driver
Date: Wed, 12 Aug 2020 10:28:54 +0300 [thread overview]
Message-ID: <20200812072854.4r4d6cz5cprezlof@fsr-ub1664-175> (raw)
In-Reply-To: <3f4fd963bdf58e61715524fdb246481fb2b2d137.camel@pengutronix.de>
On 20-07-30 11:39:22, Philipp Zabel wrote:
> On Thu, 2020-07-30 at 11:55 +0300, Abel Vesa wrote:
> > On 20-07-29 14:46:28, Philipp Zabel wrote:
> > > Hi Abel,
> > >
> > > On Wed, 2020-07-29 at 15:07 +0300, Abel Vesa wrote:
> > > > On i.MX8MP, there is a new type of IP which is called BLK_CTRL in
> >
> > [...]
> >
> > > > +
> > > > +static int imx_blk_ctrl_reset_set(struct reset_controller_dev *rcdev,
> > > > + unsigned long id, bool assert)
> > > > +{
> > > > + struct imx_blk_ctrl_drvdata *drvdata = container_of(rcdev,
> > > > + struct imx_blk_ctrl_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) {
> > > > + pm_runtime_get_sync(rcdev->dev);
> > > > + spin_lock_irqsave(&drvdata->lock, flags);
> > > > + reg = readl(reg_addr);
> > > > + writel(reg & ~(mask << shift), reg_addr);
> > > > + spin_unlock_irqrestore(&drvdata->lock, flags);
> > > > + } else {
> > > > + spin_lock_irqsave(&drvdata->lock, flags);
> > > > + reg = readl(reg_addr);
> > > > + writel(reg | (mask << shift), reg_addr);
> > > > + spin_unlock_irqrestore(&drvdata->lock, flags);
> > > > + pm_runtime_put(rcdev->dev);
> > >
> > > This still has the issue of potentially letting exclusive reset control
> > > users break the device usage counter.
> > >
> > > Also shared reset control users start with deassert(), and you end probe
> > > with pm_runtime_put(), so the first shared reset control user that
> > > deasserts its reset will decrement the dev->power.usage_count to -1 ?
> > > For multiple resets being initially deasserted this would decrement
> > > multiple times.
> > >
> > > I think you'll have to track the (number of) asserted reset bits in this
> > > reset controller and limit when to call pm_runtime_get/put_sync().
> > >
> >
> > Yes, you're right.
> >
> > I'll add a mask, and for each assert, the according bit will get set, and
> > for each deasssert the same bit will get cleared.
>
> > And when the mask has at least one bit set, the pm_runtime_get gets called
>
> ^ When the mask was 0 before but now has a bit set.
>
> > and when the mask is 0, the pm_runtime_put_sync will be called.
>
> ^ When the mask had a bit set but now is 0.
>
> > Does that sound OK ?
>
> And the mask starts out as 0, as after the pm_runtime_put() in probe all
> reset lines are deasserted?
>
Yes, that is correct.
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int imx_blk_ctrl_reset_reset(struct reset_controller_dev *rcdev,
> > > > + unsigned long id)
> > > > +{
> > > > + imx_blk_ctrl_reset_set(rcdev, id, true);
> > > > + return imx_blk_ctrl_reset_set(rcdev, id, false);
> > >
> > > Does this work for all peripherals? Are there none that require the
> > > reset line to be asserted for a certain number of bus clocks or similar?
> >
> > As of now, there is no user that calls reset. All the users call the assert
> > and then deassert. As for the number of clocks for reset, I'll try to have a
> > chat to the HW design team and then come back with the information.
>
> Ok. If this is not required or can't be guaranteed to work, it may be
> better to just leave it out.
>
> regards
> Philipp
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-08-12 7:32 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-29 12:07 [PATCH 00/17] Add BLK_CTRL support for i.MX8MP Abel Vesa
2020-07-29 12:07 ` [PATCH 01/17] dt-bindings: clocks: imx8mp: Rename audiomix ids clocks to audio_blk_ctrl Abel Vesa
2020-07-29 19:47 ` Stephen Boyd
2020-07-30 7:29 ` Abel Vesa
2020-07-29 12:07 ` [PATCH 02/17] dt-bindings: reset: imx8mp: Add audio blk_ctrl reset IDs Abel Vesa
2020-07-29 12:07 ` [PATCH 03/17] dt-bindings: clock: imx8mp: Add ids for the audio shared gate Abel Vesa
2020-07-31 22:22 ` Rob Herring
2020-07-29 12:07 ` [PATCH 04/17] dt-bindings: clock: imx8mp: Add media blk_ctrl clock IDs Abel Vesa
2020-07-31 22:22 ` Rob Herring
2020-07-29 12:07 ` [PATCH 05/17] dt-bindings: reset: imx8mp: Add media blk_ctrl reset IDs Abel Vesa
2020-07-31 22:23 ` Rob Herring
2020-07-29 12:07 ` [PATCH 06/17] dt-bindings: clock: imx8mp: Add hdmi blk_ctrl clock IDs Abel Vesa
2020-07-31 22:23 ` Rob Herring
2020-07-29 12:07 ` [PATCH 07/17] dt-bindings: reset: imx8mp: Add hdmi blk_ctrl reset IDs Abel Vesa
2020-07-31 22:24 ` Rob Herring
2020-07-29 12:07 ` [PATCH 08/17] clk: imx8mp: Add audio shared gate Abel Vesa
2020-07-29 12:07 ` [PATCH 09/17] arm64: dts: Remove imx-hdmimix-reset header file Abel Vesa
2020-07-29 12:07 ` [PATCH 10/17] Documentation: bindings: clk: Add bindings for i.MX BLK_CTRL Abel Vesa
2020-07-29 19:49 ` Stephen Boyd
2020-07-30 7:30 ` Abel Vesa
2020-07-31 18:20 ` Rob Herring
2020-07-29 12:07 ` [PATCH 11/17] clk: imx: Add blk_ctrl combo driver Abel Vesa
2020-07-29 12:46 ` Philipp Zabel
2020-07-30 8:55 ` Abel Vesa
2020-07-30 9:39 ` Philipp Zabel
2020-08-12 7:28 ` Abel Vesa [this message]
2020-07-29 12:07 ` [PATCH 12/17] clk: imx8mp: Add audio blk_ctrl clocks and resets Abel Vesa
2020-07-29 12:07 ` [PATCH 13/17] clk: imx8mp: Add hdmi " Abel Vesa
2020-07-29 12:08 ` [PATCH 14/17] clk: imx8mp: Add media " Abel Vesa
2020-07-29 12:08 ` [PATCH 15/17] arm64: dts: imx8mp: Add audio_blk_ctrl node Abel Vesa
2020-07-29 12:16 ` Abel Vesa
2020-07-29 12:08 ` [PATCH 16/17] arm64: dts: imx8mp: Add media_blk_ctrl node Abel Vesa
2020-07-29 12:17 ` Abel Vesa
2020-07-29 12:08 ` [PATCH 17/17] arm64: dts: imx8mp: Add hdmi_blk_ctrl node Abel Vesa
2020-07-29 12:17 ` Abel Vesa
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=20200812072854.4r4d6cz5cprezlof@fsr-ub1664-175 \
--to=abel.vesa@nxp.com \
--cc=aisheng.dong@nxp.com \
--cc=anson.huang@nxp.com \
--cc=devicetree@vger.kernel.org \
--cc=fabio.estevam@nxp.com \
--cc=fugang.duan@nxp.com \
--cc=kernel@pengutronix.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-imx@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mturquette@baylibre.com \
--cc=p.zabel@pengutronix.de \
--cc=peng.fan@nxp.com \
--cc=robh@kernel.org \
--cc=sboyd@kernel.org \
--cc=shawnguo@kernel.org \
/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).