All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tomasz Figa <tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
To: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Alex Williamson
	<alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	Linux PM <linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	sboyd-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	"Rafael J. Wysocki" <rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org>,
	Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>,
	Linux Kernel Mailing List
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"list-Y9sIeH5OGRo@public.gmane.org:IOMMU DRIVERS
	<iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>,
	Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>,
	"
	<iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	linux-arm-msm
	<linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	freedreno
	<freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>,
	Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
Subject: Re: [PATCH v16 2/5] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
Date: Fri, 7 Sep 2018 18:16:37 +0900	[thread overview]
Message-ID: <CAAFQd5B7YR03Jm1upOg2nsaKOb1iDD7-WGo-qx7S8m-nounCvA@mail.gmail.com> (raw)
In-Reply-To: <20180830144541.17740-3-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

Hi Vivek,

On Thu, Aug 30, 2018 at 11:46 PM Vivek Gautam
<vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>
> From: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>
> The smmu device probe/remove and add/remove master device callbacks
> gets called when the smmu is not linked to its master, that is without
> the context of the master device. So calling runtime apis in those places
> separately.
> Global locks are also initialized before enabling runtime pm as the
> runtime_resume() calls device_reset() which does tlb_sync_global()
> that ultimately requires locks to be initialized.
>
> Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> [vivek: Cleanup pm runtime calls]
> Signed-off-by: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Reviewed-by: Tomasz Figa <tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Tested-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/iommu/arm-smmu.c | 89 +++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 81 insertions(+), 8 deletions(-)
[snip]
> @@ -2215,10 +2281,17 @@ static int arm_smmu_device_remove(struct platform_device *pdev)
>         if (!bitmap_empty(smmu->context_map, ARM_SMMU_MAX_CBS))
>                 dev_err(&pdev->dev, "removing device with active domains!\n");
>
> +       arm_smmu_rpm_get(smmu);
>         /* Turn the thing off */
>         writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
> +       arm_smmu_rpm_put(smmu);
> +
> +       if (pm_runtime_enabled(smmu->dev))
> +               pm_runtime_force_suspend(smmu->dev);
> +       else
> +               clk_bulk_disable(smmu->num_clks, smmu->clks);
>
> -       clk_bulk_disable_unprepare(smmu->num_clks, smmu->clks);
> +       clk_bulk_unprepare(smmu->num_clks, smmu->clks);

Aren't we missing pm_runtime_disable() here? We'll have the enable
count unbalanced if the driver is removed and probed again.

Also, if we add pm_runtime_disable(), we can reorder things a bit and
simplify into:

       arm_smmu_rpm_get(smmu);

       /* Turn the thing off */
       writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);

       if (pm_runtime_enabled())
               pm_runtime_disable();
        arm_smmu_rpm_put(smmu);

        clk_bulk_disable_unprepare(smmu->num_clks, smmu->clks);

Best regards,
Tomasz

WARNING: multiple messages have this Message-ID
From: Tomasz Figa <tfiga@chromium.org>
To: Vivek Gautam <vivek.gautam@codeaurora.org>
Cc: "list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>,
	Joerg Roedel <joro@8bytes.org>," <joro@8bytes.org>,
	Rob Herring <robh+dt@kernel.org>,
	Robin Murphy <robin.murphy@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	"list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>,
	Joerg Roedel <joro@8bytes.org>,"
	<iommu@lists.linux-foundation.org>,
	devicetree@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Alex Williamson <alex.williamson@redhat.com>,
	Mark Rutland <mark.rutland@arm.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Rob Clark <robdclark@gmail.com>,
	Linux PM <linux-pm@vger.kernel.org>,
	freedreno <freedreno@lists.freedesktop.org>,
	sboyd@kernel.org, jcrouse@codeaurora.org,
	Sricharan R <sricharan@codeaurora.org>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Archit Taneja <architt@codeaurora.org>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>
Subject: Re: [PATCH v16 2/5] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
Date: Fri, 7 Sep 2018 18:16:37 +0900	[thread overview]
Message-ID: <CAAFQd5B7YR03Jm1upOg2nsaKOb1iDD7-WGo-qx7S8m-nounCvA@mail.gmail.com> (raw)
In-Reply-To: <20180830144541.17740-3-vivek.gautam@codeaurora.org>

Hi Vivek,

On Thu, Aug 30, 2018 at 11:46 PM Vivek Gautam
<vivek.gautam@codeaurora.org> wrote:
>
> From: Sricharan R <sricharan@codeaurora.org>
>
> The smmu device probe/remove and add/remove master device callbacks
> gets called when the smmu is not linked to its master, that is without
> the context of the master device. So calling runtime apis in those places
> separately.
> Global locks are also initialized before enabling runtime pm as the
> runtime_resume() calls device_reset() which does tlb_sync_global()
> that ultimately requires locks to be initialized.
>
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> [vivek: Cleanup pm runtime calls]
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> Reviewed-by: Tomasz Figa <tfiga@chromium.org>
> Tested-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  drivers/iommu/arm-smmu.c | 89 +++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 81 insertions(+), 8 deletions(-)
[snip]
> @@ -2215,10 +2281,17 @@ static int arm_smmu_device_remove(struct platform_device *pdev)
>         if (!bitmap_empty(smmu->context_map, ARM_SMMU_MAX_CBS))
>                 dev_err(&pdev->dev, "removing device with active domains!\n");
>
> +       arm_smmu_rpm_get(smmu);
>         /* Turn the thing off */
>         writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
> +       arm_smmu_rpm_put(smmu);
> +
> +       if (pm_runtime_enabled(smmu->dev))
> +               pm_runtime_force_suspend(smmu->dev);
> +       else
> +               clk_bulk_disable(smmu->num_clks, smmu->clks);
>
> -       clk_bulk_disable_unprepare(smmu->num_clks, smmu->clks);
> +       clk_bulk_unprepare(smmu->num_clks, smmu->clks);

Aren't we missing pm_runtime_disable() here? We'll have the enable
count unbalanced if the driver is removed and probed again.

Also, if we add pm_runtime_disable(), we can reorder things a bit and
simplify into:

       arm_smmu_rpm_get(smmu);

       /* Turn the thing off */
       writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);

       if (pm_runtime_enabled())
               pm_runtime_disable();
        arm_smmu_rpm_put(smmu);

        clk_bulk_disable_unprepare(smmu->num_clks, smmu->clks);

Best regards,
Tomasz

  parent reply	other threads:[~2018-09-07  9:16 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-30 14:45 [PATCH v16 0/5] iommu/arm-smmu: Add runtime pm/sleep support Vivek Gautam
2018-08-30 14:45 ` Vivek Gautam
     [not found] ` <20180830144541.17740-1-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-08-30 14:45   ` [PATCH v16 1/5] iommu/arm-smmu: Add pm_runtime/sleep ops Vivek Gautam
2018-08-30 14:45     ` Vivek Gautam
     [not found]     ` <20180830144541.17740-2-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-09-26 15:27       ` Robin Murphy
2018-09-26 15:27         ` Robin Murphy
     [not found]         ` <1a918f99-83b0-2f51-7634-126639a220de-5wv7dgnIgG8@public.gmane.org>
2018-09-27  8:31           ` Vivek Gautam
2018-09-27  8:31             ` Vivek Gautam
2018-09-28 11:59     ` Ulf Hansson
2018-10-01  5:49       ` Vivek Gautam
2018-10-01  5:49         ` Vivek Gautam
     [not found]         ` <CAFp+6iHZysBmMZj+pZghPocEwDzU7KfR1gLQAQRBui+-gdiSTQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-10-01  9:38           ` Ulf Hansson
2018-10-01  9:38             ` Ulf Hansson
     [not found]             ` <CAPDyKFrCWaeDLbmbrRb23V_mRk8doBjM3u2X9tm0XNXm=-n5vA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-10-01 10:21               ` Vivek Gautam
2018-10-01 10:21                 ` Vivek Gautam
2018-10-01 10:32           ` Vivek Gautam
2018-10-01 10:32             ` Vivek Gautam
     [not found]             ` <CAFp+6iFJbQZ1MrNWD6g+GNp9TOMXuq0oDdzQ9XUMTmY67tNgWg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-10-01 12:02               ` Ulf Hansson
2018-10-01 12:02                 ` Ulf Hansson
2018-08-30 14:45   ` [PATCH v16 2/5] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device Vivek Gautam
2018-08-30 14:45     ` Vivek Gautam
     [not found]     ` <20180830144541.17740-3-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-09-07  9:16       ` Tomasz Figa [this message]
2018-09-07  9:16         ` Tomasz Figa
     [not found]         ` <CAAFQd5B7YR03Jm1upOg2nsaKOb1iDD7-WGo-qx7S8m-nounCvA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-09-07  9:38           ` Vivek Gautam
2018-09-07  9:38             ` Vivek Gautam
2018-09-07  9:52             ` Tomasz Figa
2018-09-07  9:52               ` Tomasz Figa
     [not found]               ` <CAAFQd5DMDWEeBU=fg6arye3pXnrCqOA4tDtMSGutXk7vSz-EtQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-09-07 10:22                 ` Vivek Gautam
2018-09-07 10:22                   ` Vivek Gautam
     [not found]                   ` <CAFp+6iE3+qn6-gpXEzxfoP5yPMARmUcJNn0AZk-kA2AreTLnng-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-09-18  3:11                     ` Vivek Gautam
2018-09-18  3:11                       ` Vivek Gautam
     [not found]                       ` <CAFp+6iHnHj_PkPFBUSTdJTW57K51wLqSEw6VOMKxAVo7O+ms9g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-09-25  5:56                         ` Vivek Gautam
2018-09-25  5:56                           ` Vivek Gautam
     [not found]                           ` <CAFp+6iFvhdSq7upbvsEWgMewhAaaRM91iw9p7qOFPYrUxis0Hg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-09-25 18:55                             ` Robin Murphy
2018-09-25 18:55                               ` Robin Murphy
     [not found]                               ` <aedf3f32-68ae-566a-3735-ca4ec5c7559a-5wv7dgnIgG8@public.gmane.org>
2018-09-26  6:22                                 ` Vivek Gautam
2018-09-26  6:22                                   ` Vivek Gautam
2018-09-26 15:42       ` Robin Murphy
2018-09-26 15:42         ` Robin Murphy
2018-10-01 12:58       ` Will Deacon
2018-10-01 12:58         ` Will Deacon
     [not found]         ` <20181001125854.GB31488-5wv7dgnIgG8@public.gmane.org>
2018-10-02  4:14           ` Vivek Gautam
2018-10-02  4:14             ` Vivek Gautam
     [not found]             ` <CAFp+6iGkudBtZFfk4ST+k9OMRSGgU=pzVGC4QaSmPb+Xkdp3Xw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-10-02  5:28               ` Vivek Gautam
2018-10-02  5:28                 ` Vivek Gautam
     [not found]                 ` <CAFp+6iFpm05Uu8HkYCC+O+gYwFwvxirc6=e6oOed+ZJk6QLDPw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-10-02  7:12                   ` [PATCH v17 " Vivek Gautam
2018-10-02  7:12                     ` Vivek Gautam
2018-10-02  7:12                     ` Vivek Gautam
2018-08-30 14:45   ` [PATCH v16 3/5] iommu/arm-smmu: Add the device_link between masters and smmu Vivek Gautam
2018-08-30 14:45     ` Vivek Gautam
     [not found]     ` <20180830144541.17740-4-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-09-26 15:44       ` Robin Murphy
2018-09-26 15:44         ` Robin Murphy
2018-08-30 14:45   ` [PATCH v16 4/5] dt-bindings: arm-smmu: Add bindings for qcom, smmu-v2 Vivek Gautam
2018-08-30 14:45     ` [PATCH v16 4/5] dt-bindings: arm-smmu: Add bindings for qcom,smmu-v2 Vivek Gautam
     [not found]     ` <20180830144541.17740-5-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-09-06  3:52       ` [PATCH v16 4/5] dt-bindings: arm-smmu: Add bindings for qcom, smmu-v2 Vivek Gautam
2018-09-06  3:52         ` [PATCH v16 4/5] dt-bindings: arm-smmu: Add bindings for qcom,smmu-v2 Vivek Gautam
2018-09-10 18:02       ` Rob Herring
2018-09-10 18:02         ` Rob Herring
2018-09-11  8:34         ` [PATCH v16 4/5] dt-bindings: arm-smmu: Add bindings for qcom, smmu-v2 Vivek Gautam
2018-09-11  8:34           ` [PATCH v16 4/5] dt-bindings: arm-smmu: Add bindings for qcom,smmu-v2 Vivek Gautam
2018-09-26 15:46       ` [PATCH v16 4/5] dt-bindings: arm-smmu: Add bindings for qcom, smmu-v2 Robin Murphy
2018-09-26 15:46         ` [PATCH v16 4/5] dt-bindings: arm-smmu: Add bindings for qcom,smmu-v2 Robin Murphy
2018-10-01 12:18       ` [PATCH v16 4/5] dt-bindings: arm-smmu: Add bindings for qcom, smmu-v2 Will Deacon
2018-10-01 12:18         ` [PATCH v16 4/5] dt-bindings: arm-smmu: Add bindings for qcom,smmu-v2 Will Deacon
2018-10-01 17:36         ` Rob Herring
     [not found]           ` <CAL_Jsq+dSGXCiWsjLt00wu=s-ZW8M1pkQqKFWboc54M8gHFTYg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-10-01 17:45             ` [PATCH v16 4/5] dt-bindings: arm-smmu: Add bindings for qcom, smmu-v2 Will Deacon
2018-10-01 17:45               ` [PATCH v16 4/5] dt-bindings: arm-smmu: Add bindings for qcom,smmu-v2 Will Deacon
2018-08-30 14:45   ` [PATCH v16 5/5] iommu/arm-smmu: Add support for qcom, smmu-v2 variant Vivek Gautam
2018-08-30 14:45     ` [PATCH v16 5/5] iommu/arm-smmu: Add support for qcom,smmu-v2 variant Vivek Gautam
     [not found]     ` <20180830144541.17740-6-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-09-26 15:59       ` [PATCH v16 5/5] iommu/arm-smmu: Add support for qcom, smmu-v2 variant Robin Murphy
2018-09-26 15:59         ` [PATCH v16 5/5] iommu/arm-smmu: Add support for qcom,smmu-v2 variant Robin Murphy
2018-09-27  6:55         ` Vivek Gautam
2018-09-27  6:55           ` Vivek Gautam
2018-09-28 13:57   ` [PATCH v16 0/5] iommu/arm-smmu: Add runtime pm/sleep support Will Deacon
2018-09-28 13:57     ` Will Deacon
2018-10-01  9:06     ` Vivek Gautam
2018-10-01  9:06       ` Vivek Gautam

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=CAAFQd5B7YR03Jm1upOg2nsaKOb1iDD7-WGo-qx7S8m-nounCvA@mail.gmail.com \
    --to=tfiga-f7+t8e8rja9g9huczpvpmw@public.gmane.org \
    --cc=alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
    --cc=iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
    --cc=rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=robin.murphy-5wv7dgnIgG8@public.gmane.org \
    --cc=sboyd-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=will.deacon-5wv7dgnIgG8@public.gmane.org \
    --subject='Re: [PATCH v16 2/5] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device' \
    /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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.