Linux-Clk Archive on lore.kernel.org
 help / color / Atom feed
From: Philipp Zabel <p.zabel@pengutronix.de>
To: Abel Vesa <abel.vesa@nxp.com>
Cc: Mike Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>, Rob Herring <robh@kernel.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <kernel@pengutronix.de>,
	Fabio Estevam <fabio.estevam@nxp.com>,
	Anson Huang <anson.huang@nxp.com>,
	Dong Aisheng <aisheng.dong@nxp.com>, Peng Fan <peng.fan@nxp.com>,
	Fugang Duan <fugang.duan@nxp.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	linux-arm-kernel@lists.infradead.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-clk@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 11/17] clk: imx: Add blk_ctrl combo driver
Date: Thu, 30 Jul 2020 11:39:22 +0200
Message-ID: <3f4fd963bdf58e61715524fdb246481fb2b2d137.camel@pengutronix.de> (raw)
In-Reply-To: <20200730085508.ddxhb4rjnzwooh2z@fsr-ub1664-175>

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?

> > > +	}
> > > +
> > > +	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

  reply index

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 [this message]
2020-08-12  7:28         ` Abel Vesa
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=3f4fd963bdf58e61715524fdb246481fb2b2d137.camel@pengutronix.de \
    --to=p.zabel@pengutronix.de \
    --cc=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=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

Linux-Clk Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-clk/0 linux-clk/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-clk linux-clk/ https://lore.kernel.org/linux-clk \
		linux-clk@vger.kernel.org
	public-inbox-index linux-clk

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-clk


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git