From: will.deacon@arm•com (Will Deacon)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 1/6] iommu/arm-smmu: add support for specifying clocks
Date: Tue, 19 Aug 2014 13:58:34 +0100 [thread overview]
Message-ID: <20140819125833.GO23128@arm.com> (raw)
In-Reply-To: <1407891099-24641-2-git-send-email-mitchelh@codeaurora.org>
On Wed, Aug 13, 2014 at 01:51:34AM +0100, Mitchel Humpherys wrote:
> On some platforms with tight power constraints it is polite to only
> leave your clocks on for as long as you absolutely need them. Currently
> we assume that all clocks necessary for SMMU register access are always
> on.
>
> Add some optional device tree properties to specify any clocks that are
> necessary for SMMU register access and turn them on and off as needed.
>
> If no clocks are specified in the device tree things continue to work
> the way they always have: we assume all necessary clocks are always
> turned on.
How does this interact with an SMMU in bypass mode?
[...]
> +static int arm_smmu_enable_clocks(struct arm_smmu_device *smmu)
> +{
> + int i, ret = 0;
> +
> + for (i = 0; i < smmu->num_clocks; ++i) {
> + ret = clk_prepare_enable(smmu->clocks[i]);
> + if (ret) {
> + dev_err(smmu->dev, "Couldn't enable clock #%d\n", i);
> + while (i--)
> + clk_disable_unprepare(smmu->clocks[i]);
> + break;
> + }
> + }
> +
> + return ret;
> +}
> +
> +static void arm_smmu_disable_clocks(struct arm_smmu_device *smmu)
> +{
> + int i;
> +
> + for (i = 0; i < smmu->num_clocks; ++i)
> + clk_disable_unprepare(smmu->clocks[i]);
> +}
What stops theses from racing with each other when there are multiple
clocks? I also assume that the clk API ignores calls to clk_enable_prepare
for a clk that's already enabled? I couldn't find that code...
> /* Wait for any pending TLB invalidations to complete */
> static void arm_smmu_tlb_sync(struct arm_smmu_device *smmu)
> {
> @@ -644,11 +672,15 @@ static irqreturn_t arm_smmu_context_fault(int irq, void *dev)
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> void __iomem *cb_base;
>
> + arm_smmu_enable_clocks(smmu);
How can I get a context interrupt from an SMMU without its clocks enabled?
[...]
> +int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
> {
> unsigned long size;
> void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
> @@ -2027,10 +2124,16 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
> }
> dev_notice(dev, "registered %d master devices\n", i);
>
> - err = arm_smmu_device_cfg_probe(smmu);
> + err = arm_smmu_init_clocks(smmu);
> if (err)
> goto out_put_masters;
>
> + arm_smmu_enable_clocks(smmu);
> +
> + err = arm_smmu_device_cfg_probe(smmu);
> + if (err)
> + goto out_disable_clocks;
> +
> parse_driver_options(smmu);
>
> if (smmu->version > 1 &&
> @@ -2039,7 +2142,7 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
> "found only %d context interrupt(s) but %d required\n",
> smmu->num_context_irqs, smmu->num_context_banks);
> err = -ENODEV;
> - goto out_put_masters;
> + goto out_disable_clocks;
> }
>
> for (i = 0; i < smmu->num_global_irqs; ++i) {
> @@ -2061,12 +2164,16 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
> spin_unlock(&arm_smmu_devices_lock);
>
> arm_smmu_device_reset(smmu);
> + arm_smmu_disable_clocks(smmu);
I wonder if this is really the right thing to do. Rather than the
fine-grained clock enable/disable you have, why don't we just enable in
domain_init and disable in domain_destroy, with refcounting for the clocks?
Will
next prev parent reply other threads:[~2014-08-19 12:58 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-13 0:51 [PATCH 0/6] iommu/arm-smmu: misc features, new DT bindings Mitchel Humpherys
2014-08-13 0:51 ` [PATCH 1/6] iommu/arm-smmu: add support for specifying clocks Mitchel Humpherys
2014-08-13 21:07 ` Mitchel Humpherys
2014-08-19 12:58 ` Will Deacon [this message]
2014-08-19 19:03 ` Olav Haugan
2014-08-26 14:27 ` Will Deacon
2014-09-10 1:29 ` Mitchel Humpherys
2014-09-10 18:27 ` Will Deacon
2014-09-10 19:09 ` Mitchel Humpherys
2014-09-15 18:38 ` Mitchel Humpherys
2014-08-19 19:28 ` Mitchel Humpherys
2014-08-13 0:51 ` [PATCH 2/6] iommu/arm-smmu: add support for specifying regulators Mitchel Humpherys
2014-08-13 21:17 ` Mitchel Humpherys
2014-08-19 13:00 ` Will Deacon
2014-08-13 0:51 ` [PATCH 3/6] iommu/arm-smmu: add support for iova_to_phys through ATS1PR Mitchel Humpherys
2014-08-19 12:44 ` Will Deacon
2014-08-19 18:12 ` Mitchel Humpherys
2014-08-26 13:54 ` Will Deacon
2014-09-01 16:15 ` Will Deacon
2014-08-13 0:51 ` [PATCH 4/6] iommu/arm-smmu: implement generic DT bindings Mitchel Humpherys
2014-08-13 16:53 ` Mitchel Humpherys
2014-08-19 12:54 ` Will Deacon
2014-08-19 15:54 ` Hiroshi Doyu
2014-08-20 3:18 ` Arnd Bergmann
2014-08-13 0:51 ` [PATCH 5/6] iommu/arm-smmu: support buggy implementations with invalidate-on-map Mitchel Humpherys
2014-11-12 18:26 ` Will Deacon
2014-11-12 18:58 ` Mitchel Humpherys
2014-11-13 9:48 ` Will Deacon
2014-11-14 23:08 ` Mitchel Humpherys
2014-08-13 0:51 ` [PATCH 6/6] iommu/arm-smmu: add .domain_{set, get}_attr for coherent walk control Mitchel Humpherys
2014-08-19 12:48 ` [PATCH 6/6] iommu/arm-smmu: add .domain_{set,get}_attr " Will Deacon
2014-08-19 19:19 ` [PATCH 6/6] iommu/arm-smmu: add .domain_{set, get}_attr " Mitchel Humpherys
2014-08-13 17:22 ` [PATCH 0/6] iommu/arm-smmu: misc features, new DT bindings Mitchel Humpherys
2014-08-15 17:25 ` Will Deacon
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=20140819125833.GO23128@arm.com \
--to=will.deacon@arm$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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