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>, Mark Rutland <mark.rutland@arm.com>, "Rafael J. Wysocki" <rjw@rjwysocki.net>, Robin Murphy <robin.murphy@arm.com>, Will Deacon <will.deacon@arm.com>, Rob Clark <robdclark@gmail.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@vger.kernel.org, linux-pm@vger.kernel.org, dri-devel <dri-devel@lists.freedesktop.org>, freedreno@lists.freedesktop.org, David Airlie <airlied@linux.ie>, Greg KH <gregkh@linuxfoundation.org>, sboyd@codeaurora.org, linux-arm-msm@vger.kernel.org Subject: Re: [PATCH v7 4/6] iommu/arm-smmu: Add the device_link between masters and smmu Date: Tue, 13 Feb 2018 17:31:57 +0900 [thread overview] Message-ID: <CAAFQd5BQAs9=N27_Z0pJNSrndFY3vFin2KBP44UtiW+tMXy5nQ@mail.gmail.com> (raw) In-Reply-To: <1517999482-17317-5-git-send-email-vivek.gautam@codeaurora.org> Hi Vivek, Thanks for the patch. Please see my comments inline. On Wed, Feb 7, 2018 at 7:31 PM, Vivek Gautam <vivek.gautam@codeaurora.org> wrote: > From: Sricharan R <sricharan@codeaurora.org> > > Finally add the device link between the master device and > smmu, so that the smmu gets runtime enabled/disabled only when the > master needs it. This is done from add_device callback which gets > called once when the master is added to the smmu. > > Signed-off-by: Sricharan R <sricharan@codeaurora.org> > Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org> > --- > drivers/iommu/arm-smmu.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index c024f69c1682..c7e924d553bd 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -215,6 +215,9 @@ struct arm_smmu_device { > > /* IOMMU core code handle */ > struct iommu_device iommu; > + > + /* runtime PM link to master */ > + struct device_link *link; > }; > > enum arm_smmu_context_fmt { > @@ -1425,6 +1428,17 @@ static int arm_smmu_add_device(struct device *dev) > > pm_runtime_put_sync(smmu->dev); > > + /* > + * Establish the link between smmu and master, so that the > + * smmu gets runtime enabled/disabled as per the master's > + * needs. > + */ > + smmu->link = device_link_add(dev, smmu->dev, DL_FLAG_PM_RUNTIME); > + if (!smmu->link) > + dev_warn(smmu->dev, > + "Unable to create device link between %s and %s\n", > + dev_name(smmu->dev), dev_name(dev)); How likely it is that the master can work normally even if the link add fails? Perhaps we should just return an error here? > + > return 0; > > out_rpm_put: > @@ -1449,6 +1463,8 @@ static void arm_smmu_remove_device(struct device *dev) > cfg = fwspec->iommu_priv; > smmu = cfg->smmu; > > + device_link_del(smmu->link); We allowed smmu->link in arm_smmu_add_device(), but here we don't check it. Looking at the code, device_link_del() doesn't seem to check either. Note that this problem would go away if we fail add_device on device_link_add() failure, as I suggested above, so no change would be necessary. Best regards, Tomasz
WARNING: multiple messages have this Message-ID (diff)
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>, Mark Rutland <mark.rutland@arm.com>, "Rafael J. Wysocki" <rjw@rjwysocki.net>, Robin Murphy <robin.murphy@arm.com>, Will Deacon <will.deacon@arm.com>, Rob Clark <robdclark@gmail.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@vger.kernel.org, linux-pm@vger.kernel.org, dri-devel <dri-devel@lists.freedesktop.org>, freedreno@lists.freedesktop.org, David Airlie <airlied@linux.ie>, Greg KH <gregkh@linuxfoundation.org>, sboyd@codeaurora.org, linux-arm-msm@vger.kernel.org Subject: Re: [PATCH v7 4/6] iommu/arm-smmu: Add the device_link between masters and smmu Date: Tue, 13 Feb 2018 17:31:57 +0900 [thread overview] Message-ID: <CAAFQd5BQAs9=N27_Z0pJNSrndFY3vFin2KBP44UtiW+tMXy5nQ@mail.gmail.com> (raw) In-Reply-To: <1517999482-17317-5-git-send-email-vivek.gautam@codeaurora.org> Hi Vivek, Thanks for the patch. Please see my comments inline. On Wed, Feb 7, 2018 at 7:31 PM, Vivek Gautam <vivek.gautam@codeaurora.org> wrote: > From: Sricharan R <sricharan@codeaurora.org> > > Finally add the device link between the master device and > smmu, so that the smmu gets runtime enabled/disabled only when the > master needs it. This is done from add_device callback which gets > called once when the master is added to the smmu. > > Signed-off-by: Sricharan R <sricharan@codeaurora.org> > Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org> > --- > drivers/iommu/arm-smmu.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index c024f69c1682..c7e924d553bd 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -215,6 +215,9 @@ struct arm_smmu_device { > > /* IOMMU core code handle */ > struct iommu_device iommu; > + > + /* runtime PM link to master */ > + struct device_link *link; > }; > > enum arm_smmu_context_fmt { > @@ -1425,6 +1428,17 @@ static int arm_smmu_add_device(struct device *dev) > > pm_runtime_put_sync(smmu->dev); > > + /* > + * Establish the link between smmu and master, so that the > + * smmu gets runtime enabled/disabled as per the master's > + * needs. > + */ > + smmu->link = device_link_add(dev, smmu->dev, DL_FLAG_PM_RUNTIME); > + if (!smmu->link) > + dev_warn(smmu->dev, > + "Unable to create device link between %s and %s\n", > + dev_name(smmu->dev), dev_name(dev)); How likely it is that the master can work normally even if the link add fails? Perhaps we should just return an error here? > + > return 0; > > out_rpm_put: > @@ -1449,6 +1463,8 @@ static void arm_smmu_remove_device(struct device *dev) > cfg = fwspec->iommu_priv; > smmu = cfg->smmu; > > + device_link_del(smmu->link); We allowed smmu->link in arm_smmu_add_device(), but here we don't check it. Looking at the code, device_link_del() doesn't seem to check either. Note that this problem would go away if we fail add_device on device_link_add() failure, as I suggested above, so no change would be necessary. Best regards, Tomasz
next prev parent reply other threads:[~2018-02-13 8:31 UTC|newest] Thread overview: 103+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-02-07 10:31 [PATCH v7 0/6] iommu/arm-smmu: Add runtime pm/sleep support Vivek Gautam 2018-02-07 10:31 ` Vivek Gautam [not found] ` <1517999482-17317-1-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2018-02-07 10:31 ` [PATCH v7 1/6] base: power: runtime: Export pm_runtime_get/put_suppliers Vivek Gautam 2018-02-07 10:31 ` Vivek Gautam 2018-02-13 7:44 ` Tomasz Figa 2018-02-13 7:44 ` Tomasz Figa [not found] ` <CAAFQd5BmroRf-C8dQkvTKHWK1psGnNi1t7g-q=Xce6KjrGTsdw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-13 12:00 ` Robin Murphy 2018-02-13 12:00 ` Robin Murphy 2018-02-13 12:54 ` Tomasz Figa 2018-02-13 12:54 ` Tomasz Figa 2018-02-13 13:37 ` Robin Murphy 2018-02-13 13:37 ` Robin Murphy 2018-02-07 10:31 ` [PATCH v7 2/6] iommu/arm-smmu: Add pm_runtime/sleep ops Vivek Gautam 2018-02-07 10:31 ` Vivek Gautam 2018-02-13 8:03 ` Tomasz Figa 2018-02-13 8:03 ` Tomasz Figa [not found] ` <CAAFQd5B2u8RL-tdB3qgPxVUcXnsBSEhXRBZWxqO-w6rYKAiOtg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-13 10:25 ` Vivek Gautam 2018-02-13 10:25 ` Vivek Gautam 2018-02-14 3:45 ` Tomasz Figa 2018-02-14 3:45 ` Tomasz Figa 2018-02-07 10:31 ` [PATCH v7 3/6] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device Vivek Gautam 2018-02-07 10:31 ` Vivek Gautam 2018-02-13 8:24 ` Tomasz Figa 2018-02-13 8:24 ` Tomasz Figa [not found] ` <CAAFQd5DxYhkK61VDAesby6bT+FtG2nqsbHQRvxkhrsSS0KWtog-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-13 12:57 ` Robin Murphy 2018-02-13 12:57 ` Robin Murphy [not found] ` <906051dd-8898-ec6f-5ad4-3f37716292cf-5wv7dgnIgG8@public.gmane.org> 2018-02-13 13:52 ` Tomasz Figa 2018-02-13 13:52 ` Tomasz Figa [not found] ` <CAAFQd5DJtQYPg5S3Ep2bK27+D5rQiKuA-uPfMDUon3FudmGF0w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-14 8:24 ` Vivek Gautam 2018-02-14 8:24 ` Vivek Gautam 2018-02-14 8:28 ` Vivek Gautam 2018-02-14 8:28 ` Vivek Gautam [not found] ` <1517999482-17317-4-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2018-02-22 23:52 ` Jordan Crouse 2018-02-22 23:52 ` Jordan Crouse [not found] ` <20180222235200.GA18743-9PYrDHPZ2Orvke4nUoYGnHL1okKdlPRT@public.gmane.org> 2018-02-23 10:36 ` Vivek Gautam 2018-02-23 10:36 ` Vivek Gautam [not found] ` <CAFp+6iGQ5Vckui14Jb=V0uk_Pjes95hOxo=KBijR4yxPeDDzFg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-23 15:40 ` [Freedreno] " Jordan Crouse 2018-02-23 15:40 ` Jordan Crouse [not found] ` <20180223154048.GB18743-9PYrDHPZ2Orvke4nUoYGnHL1okKdlPRT@public.gmane.org> 2018-02-23 17:43 ` Vivek Gautam 2018-02-23 17:43 ` [Freedreno] " Vivek Gautam 2018-02-07 10:31 ` [PATCH v7 4/6] iommu/arm-smmu: Add the device_link between masters and smmu Vivek Gautam 2018-02-07 10:31 ` Vivek Gautam 2018-02-13 8:31 ` Tomasz Figa [this message] 2018-02-13 8:31 ` Tomasz Figa [not found] ` <CAAFQd5BQAs9=N27_Z0pJNSrndFY3vFin2KBP44UtiW+tMXy5nQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-13 10:14 ` Vivek Gautam 2018-02-13 10:14 ` Vivek Gautam 2018-02-07 10:31 ` [PATCH v7 5/6] iommu/arm-smmu: Add support for qcom, smmu-v2 variant Vivek Gautam 2018-02-07 10:31 ` [PATCH v7 5/6] iommu/arm-smmu: Add support for qcom,smmu-v2 variant Vivek Gautam 2018-02-09 10:57 ` [PATCH v8 " Vivek Gautam 2018-02-13 8:57 ` Tomasz Figa 2018-02-13 8:57 ` Tomasz Figa 2018-02-07 10:31 ` [PATCH v7 6/6] drm/msm: iommu: Replace runtime calls with runtime suppliers Vivek Gautam 2018-02-07 10:31 ` Vivek Gautam 2018-02-13 9:10 ` Tomasz Figa 2018-02-13 9:10 ` Tomasz Figa [not found] ` <CAAFQd5AjopiX6fDgD+mO+-+d0yj-swEnVCNvccWRBSMO+XVJkA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-13 16:42 ` Jordan Crouse 2018-02-13 16:42 ` Jordan Crouse 2018-02-14 3:31 ` Tomasz Figa [not found] ` <CAAFQd5CjQRFATfh-mRQv5J=WefYuxBVTkk=Ju09FoqA-Or5Cvg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-14 15:48 ` Jordan Crouse 2018-02-14 15:48 ` Jordan Crouse [not found] ` <20180214154850.GA25422-9PYrDHPZ2Orvke4nUoYGnHL1okKdlPRT@public.gmane.org> 2018-02-14 16:12 ` [Freedreno] " Rob Clark 2018-02-14 16:12 ` Rob Clark 2018-02-15 4:09 ` Tomasz Figa 2018-02-15 4:09 ` Tomasz Figa [not found] ` <CAAFQd5C-9mbd3hDSvz10a1oiO0--FT-L4EpsAYcALxxUvk6Fjg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-15 14:14 ` Rob Clark 2018-02-15 14:14 ` [Freedreno] " Rob Clark 2018-02-13 18:03 ` Rob Clark 2018-02-13 18:03 ` Rob Clark 2018-02-14 1:59 ` Tomasz Figa 2018-02-14 1:59 ` Tomasz Figa [not found] ` <CAAFQd5BKRumpEfAKNF_RKS-ZZ8D671DfOz4vB2+w1SV3aG9NxQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-14 2:13 ` Rob Clark 2018-02-14 2:13 ` Rob Clark [not found] ` <CAF6AEGuNZJKtwGZ5mLfqNND2jtU+HYM11UONfAtVTzoM0QVpdg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-14 3:01 ` Tomasz Figa 2018-02-14 3:01 ` Tomasz Figa [not found] ` <CAAFQd5BZJ1G0RG32hYErNzPRvisBhhiSNCBsjbzfm0WzO=DnsQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-14 4:17 ` Vivek Gautam 2018-02-14 4:17 ` Vivek Gautam [not found] ` <CAFp+6iHaycK=CcE1S15EeuMkaw8LnW0ebptU0hM6tUtWdeEOtA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-14 5:38 ` Tomasz Figa 2018-02-14 5:38 ` Tomasz Figa [not found] ` <CAAFQd5Afj-Bj+3wHwmF2tT7y=46EsYEtO_mXfY6stXBgHutEUg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-14 9:13 ` Vivek Gautam 2018-02-14 9:13 ` Vivek Gautam [not found] ` <CAFp+6iGX6pr+MdPSSHHG=qOnhHky_8OHiDqAcJ9UudEUv=JMHg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-14 9:16 ` Tomasz Figa 2018-02-14 9:16 ` Tomasz Figa [not found] ` <CAAFQd5DiwAugGnPOTw0+XrEfef9x-n-vx59JFuXpNawjiXHwCw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-14 10:33 ` Vivek Gautam 2018-02-14 10:33 ` Vivek Gautam [not found] ` <CAFp+6iEW0faeHDfzN_F1bRrHGcVo3sPCk4HSY=t9dnEvHkDkYw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-14 16:03 ` Robin Murphy 2018-02-14 16:03 ` Robin Murphy 2018-02-15 3:17 ` Tomasz Figa [not found] ` <CAAFQd5AmG1zSm+CouXOCJbs8SNGFk1-RqfU1nWGjMGJMB-qfvw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-15 4:17 ` Tomasz Figa 2018-02-15 4:17 ` Tomasz Figa [not found] ` <CAAFQd5A9B-di9svtiJbvk2hz1U1xo61rTY5vt6AD+KR5iMcG-A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-15 17:14 ` Robin Murphy 2018-02-15 17:14 ` Robin Murphy [not found] ` <7406f1ce-c2c9-a6bd-2886-5a34de45add6-5wv7dgnIgG8@public.gmane.org> 2018-02-16 0:13 ` Tomasz Figa 2018-02-16 0:13 ` Tomasz Figa [not found] ` <CAAFQd5DkLtq2w00=Zd4sMDB4QOWqi7R-zgydECJXLdTmaHty+g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-22 8:13 ` Tomasz Figa 2018-02-22 8:13 ` Tomasz Figa [not found] ` <CAAFQd5CQoDqunAunwoVo7W=QXa=ET=eJ2s_j9j+3YgAR2EGgCg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-22 13:30 ` Rob Clark 2018-02-22 13:30 ` [Freedreno] " Rob Clark 2018-02-22 13:45 ` Robin Murphy 2018-02-22 13:45 ` Robin Murphy 2018-02-22 14:12 ` Tomasz Figa 2018-02-22 14:12 ` Tomasz Figa [not found] ` <CAAFQd5CzOJ0oS8LLcKP-DgyXSXgzafg5CWPAvnR-QDPS+DZAUg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-22 17:24 ` Vivek Gautam 2018-02-22 17:24 ` 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='CAAFQd5BQAs9=N27_Z0pJNSrndFY3vFin2KBP44UtiW+tMXy5nQ@mail.gmail.com' \ --to=tfiga@chromium.org \ --cc=joro@8bytes.org \ --cc=mark.rutland@arm.com \ --cc=rjw@rjwysocki.net \ --cc=robdclark@gmail.com \ --cc=robh+dt@kernel.org \ --cc=robin.murphy@arm.com \ --cc=vivek.gautam@codeaurora.org \ --cc=will.deacon@arm.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.