From: Robin Murphy <robin.murphy@arm.com> To: Yong Wu <yong.wu@mediatek.com>, Joerg Roedel <joro@8bytes.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Matthias Brugger <matthias.bgg@gmail.com>, Rob Herring <robh+dt@kernel.org> Cc: youlin.pei@mediatek.com, devicetree@vger.kernel.org, Nicolas Boichat <drinkcat@chromium.org>, arnd@arndb.de, srv_heupstream@mediatek.com, Will Deacon <will.deacon@arm.com>, linux-kernel@vger.kernel.org, Tomasz Figa <tfiga@google.com>, iommu@lists.linux-foundation.org, linux-mediatek@lists.infradead.org, yingjoe.chen@mediatek.com, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 04/13] iommu/mediatek: Add device_link between the consumer and the larb devices Date: Wed, 27 Feb 2019 19:30:10 +0000 [thread overview] Message-ID: <709d7d66-9610-eeb3-128e-e2156bcfb616@arm.com> (raw) In-Reply-To: <1546318276-18993-5-git-send-email-yong.wu@mediatek.com> On 01/01/2019 04:51, Yong Wu wrote: > MediaTek IOMMU don't have its power-domain. all the consumer connect > with smi-larb, then connect with smi-common. > > M4U > | > smi-common > | > ------------- > | | ... > | | > larb1 larb2 > | | > vdec venc > > When the consumer works, it should enable the smi-larb's power which > also need enable the smi-common's power firstly. > > Thus, First of all, use the device link connect the consumer and the > smi-larbs. then add device link between the smi-larb and smi-common. > > This patch adds device_link between the consumer and the larbs. > > Suggested-by: Tomasz Figa <tfiga@chromium.org> > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > --- > drivers/iommu/mtk_iommu.c | 15 +++++++++++++-- > drivers/iommu/mtk_iommu_v1.c | 14 ++++++++++++-- > 2 files changed, 25 insertions(+), 4 deletions(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 202e41b..735ae8d 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -247,6 +247,7 @@ static void mtk_iommu_config(struct mtk_iommu_data *data, > struct mtk_smi_larb_iommu *larb_mmu; > unsigned int larbid, portid; > struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); > + struct device_link *link; > int i; > > for (i = 0; i < fwspec->num_ids; ++i) { > @@ -257,10 +258,20 @@ static void mtk_iommu_config(struct mtk_iommu_data *data, > dev_dbg(dev, "%s iommu port: %d\n", > enable ? "enable" : "disable", portid); > > - if (enable) > + if (enable) { > larb_mmu->mmu |= MTK_SMI_MMU_EN(portid); > - else > + /* Link the consumer with the larb device(supplier) */ > + link = device_link_add(dev, larb_mmu->dev, > + DL_FLAG_PM_RUNTIME | > + DL_FLAG_AUTOREMOVE_CONSUMER); This looks technically wrong, since we get here from mtk_iommu_attach_device(), and in theory a device could get attached to any number of domains over its lifetime, so adding a link at this point when there's no corresponding cleanup on detach is definitely unbalanced. If you want this layer to manage the link on behalf of the consumer, it would probably be better to do it at the point of mtk_iommu_add_device(), and either track it for manual cleanup in mtk_iommu_remove_device() or rely on AUTOREMOVE_SUPPLIER. Robin. > + if (!link) { > + dev_err(dev, "Unable to link %s\n", > + dev_name(larb_mmu->dev)); > + return; > + } > + } else { > larb_mmu->mmu &= ~MTK_SMI_MMU_EN(portid); > + } > } > } > > diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c > index 9386aee..022bad9 100644 > --- a/drivers/iommu/mtk_iommu_v1.c > +++ b/drivers/iommu/mtk_iommu_v1.c > @@ -201,6 +201,7 @@ static void mtk_iommu_config(struct mtk_iommu_data *data, > struct mtk_smi_larb_iommu *larb_mmu; > unsigned int larbid, portid; > struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); > + struct device_link *link; > int i; > > for (i = 0; i < fwspec->num_ids; ++i) { > @@ -211,10 +212,19 @@ static void mtk_iommu_config(struct mtk_iommu_data *data, > dev_dbg(dev, "%s iommu port: %d\n", > enable ? "enable" : "disable", portid); > > - if (enable) > + if (enable) { > larb_mmu->mmu |= MTK_SMI_MMU_EN(portid); > - else > + link = device_link_add(dev, larb_mmu->dev, > + DL_FLAG_PM_RUNTIME | > + DL_FLAG_AUTOREMOVE_CONSUMER); > + if (!link) { > + dev_err(dev, "Unable to link %s\n", > + dev_name(larb_mmu->dev)); > + return; > + } > + } else { > larb_mmu->mmu &= ~MTK_SMI_MMU_EN(portid); > + } > } > } > >
WARNING: multiple messages have this Message-ID (diff)
From: Robin Murphy <robin.murphy@arm.com> To: Yong Wu <yong.wu@mediatek.com>, Joerg Roedel <joro@8bytes.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Matthias Brugger <matthias.bgg@gmail.com>, Rob Herring <robh+dt@kernel.org> Cc: youlin.pei@mediatek.com, devicetree@vger.kernel.org, Nicolas Boichat <drinkcat@chromium.org>, arnd@arndb.de, srv_heupstream@mediatek.com, Will Deacon <will.deacon@arm.com>, linux-kernel@vger.kernel.org, Tomasz Figa <tfiga@google.com>, iommu@lists.linux-foundation.org, linux-mediatek@lists.infradead.org, yingjoe.chen@mediatek.com, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 04/13] iommu/mediatek: Add device_link between the consumer and the larb devices Date: Wed, 27 Feb 2019 19:30:10 +0000 [thread overview] Message-ID: <709d7d66-9610-eeb3-128e-e2156bcfb616@arm.com> (raw) In-Reply-To: <1546318276-18993-5-git-send-email-yong.wu@mediatek.com> On 01/01/2019 04:51, Yong Wu wrote: > MediaTek IOMMU don't have its power-domain. all the consumer connect > with smi-larb, then connect with smi-common. > > M4U > | > smi-common > | > ------------- > | | ... > | | > larb1 larb2 > | | > vdec venc > > When the consumer works, it should enable the smi-larb's power which > also need enable the smi-common's power firstly. > > Thus, First of all, use the device link connect the consumer and the > smi-larbs. then add device link between the smi-larb and smi-common. > > This patch adds device_link between the consumer and the larbs. > > Suggested-by: Tomasz Figa <tfiga@chromium.org> > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > --- > drivers/iommu/mtk_iommu.c | 15 +++++++++++++-- > drivers/iommu/mtk_iommu_v1.c | 14 ++++++++++++-- > 2 files changed, 25 insertions(+), 4 deletions(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 202e41b..735ae8d 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -247,6 +247,7 @@ static void mtk_iommu_config(struct mtk_iommu_data *data, > struct mtk_smi_larb_iommu *larb_mmu; > unsigned int larbid, portid; > struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); > + struct device_link *link; > int i; > > for (i = 0; i < fwspec->num_ids; ++i) { > @@ -257,10 +258,20 @@ static void mtk_iommu_config(struct mtk_iommu_data *data, > dev_dbg(dev, "%s iommu port: %d\n", > enable ? "enable" : "disable", portid); > > - if (enable) > + if (enable) { > larb_mmu->mmu |= MTK_SMI_MMU_EN(portid); > - else > + /* Link the consumer with the larb device(supplier) */ > + link = device_link_add(dev, larb_mmu->dev, > + DL_FLAG_PM_RUNTIME | > + DL_FLAG_AUTOREMOVE_CONSUMER); This looks technically wrong, since we get here from mtk_iommu_attach_device(), and in theory a device could get attached to any number of domains over its lifetime, so adding a link at this point when there's no corresponding cleanup on detach is definitely unbalanced. If you want this layer to manage the link on behalf of the consumer, it would probably be better to do it at the point of mtk_iommu_add_device(), and either track it for manual cleanup in mtk_iommu_remove_device() or rely on AUTOREMOVE_SUPPLIER. Robin. > + if (!link) { > + dev_err(dev, "Unable to link %s\n", > + dev_name(larb_mmu->dev)); > + return; > + } > + } else { > larb_mmu->mmu &= ~MTK_SMI_MMU_EN(portid); > + } > } > } > > diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c > index 9386aee..022bad9 100644 > --- a/drivers/iommu/mtk_iommu_v1.c > +++ b/drivers/iommu/mtk_iommu_v1.c > @@ -201,6 +201,7 @@ static void mtk_iommu_config(struct mtk_iommu_data *data, > struct mtk_smi_larb_iommu *larb_mmu; > unsigned int larbid, portid; > struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); > + struct device_link *link; > int i; > > for (i = 0; i < fwspec->num_ids; ++i) { > @@ -211,10 +212,19 @@ static void mtk_iommu_config(struct mtk_iommu_data *data, > dev_dbg(dev, "%s iommu port: %d\n", > enable ? "enable" : "disable", portid); > > - if (enable) > + if (enable) { > larb_mmu->mmu |= MTK_SMI_MMU_EN(portid); > - else > + link = device_link_add(dev, larb_mmu->dev, > + DL_FLAG_PM_RUNTIME | > + DL_FLAG_AUTOREMOVE_CONSUMER); > + if (!link) { > + dev_err(dev, "Unable to link %s\n", > + dev_name(larb_mmu->dev)); > + return; > + } > + } else { > larb_mmu->mmu &= ~MTK_SMI_MMU_EN(portid); > + } > } > } > > _______________________________________________ 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:[~2019-02-27 19:30 UTC|newest] Thread overview: 125+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-01-01 4:51 [PATCH 00/13] Clean up "mediatek,larb" after adding device_link Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-01-01 4:51 ` [PATCH 01/13] dt-binding: mediatek: Get rid of mediatek,larb for multimedia HW Yong Wu 2019-01-01 4:51 ` [PATCH 01/13] dt-binding: mediatek: Get rid of mediatek, larb " Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-01-11 14:58 ` [PATCH 01/13] dt-binding: mediatek: Get rid of mediatek,larb " Rob Herring 2019-01-11 14:58 ` [PATCH 01/13] dt-binding: mediatek: Get rid of mediatek, larb " Rob Herring 2019-01-11 14:58 ` [PATCH 01/13] dt-binding: mediatek: Get rid of mediatek,larb " Rob Herring 2019-02-25 23:54 ` Evan Green 2019-02-25 23:54 ` [PATCH 01/13] dt-binding: mediatek: Get rid of mediatek, larb " Evan Green 2019-02-25 23:54 ` [PATCH 01/13] dt-binding: mediatek: Get rid of mediatek,larb " Evan Green 2019-01-01 4:51 ` [PATCH 02/13] driver core: Remove the link if there is no driver with AUTO flag Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-02-25 23:53 ` Evan Green 2019-02-25 23:53 ` Evan Green 2019-02-25 23:53 ` Evan Green 2019-02-27 14:33 ` Yong Wu 2019-02-27 14:33 ` Yong Wu 2019-02-27 14:33 ` Yong Wu 2019-03-05 19:03 ` Evan Green 2019-03-05 19:03 ` Evan Green 2019-03-05 19:03 ` Evan Green 2019-03-12 14:21 ` Matthias Brugger 2019-03-12 14:21 ` Matthias Brugger 2019-03-12 14:21 ` Matthias Brugger 2019-03-12 23:17 ` Evan Green 2019-03-12 23:17 ` Evan Green 2019-03-12 23:17 ` Evan Green 2019-03-13 9:08 ` Yong Wu 2019-03-13 9:08 ` Yong Wu 2019-03-13 9:08 ` Yong Wu 2019-01-01 4:51 ` [PATCH 03/13] iommu/mediatek: Add probe_defer for smi-larb Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-02-25 23:54 ` Evan Green 2019-02-25 23:54 ` Evan Green 2019-02-25 23:54 ` Evan Green 2019-02-27 14:33 ` Yong Wu 2019-02-27 14:33 ` Yong Wu 2019-02-27 14:33 ` Yong Wu 2019-03-05 19:02 ` Evan Green 2019-03-05 19:02 ` Evan Green 2019-03-05 19:02 ` Evan Green 2019-01-01 4:51 ` [PATCH 04/13] iommu/mediatek: Add device_link between the consumer and the larb devices Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-02-25 23:54 ` Evan Green 2019-02-25 23:54 ` Evan Green 2019-02-25 23:54 ` Evan Green 2019-02-27 14:34 ` Yong Wu 2019-02-27 14:34 ` Yong Wu 2019-02-27 14:34 ` Yong Wu 2019-02-27 19:30 ` Robin Murphy [this message] 2019-02-27 19:30 ` Robin Murphy 2019-03-13 9:11 ` Yong Wu 2019-03-13 9:11 ` Yong Wu 2019-03-13 9:11 ` Yong Wu 2019-01-01 4:51 ` [PATCH 05/13] memory: mtk-smi: Add device-link between smi-larb and smi-common Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-02-25 23:54 ` Evan Green 2019-02-25 23:54 ` Evan Green 2019-02-25 23:54 ` Evan Green 2019-02-27 14:33 ` Yong Wu 2019-02-27 14:33 ` Yong Wu 2019-02-27 14:33 ` Yong Wu 2019-03-05 19:02 ` Evan Green 2019-03-05 19:02 ` Evan Green 2019-03-05 19:02 ` Evan Green 2019-01-01 4:51 ` [PATCH 06/13] media: mtk-jpeg: Get rid of mtk_smi_larb_get/put Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-02-25 23:55 ` Evan Green 2019-02-25 23:55 ` Evan Green 2019-02-25 23:55 ` Evan Green 2019-01-01 4:51 ` [PATCH 07/13] media: mtk-mdp: " Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-02-25 23:55 ` Evan Green 2019-02-25 23:55 ` Evan Green 2019-02-25 23:55 ` Evan Green 2019-01-01 4:51 ` [PATCH 08/13] media: mtk-vcodec: " Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-02-25 23:55 ` Evan Green 2019-02-25 23:55 ` Evan Green 2019-02-25 23:55 ` Evan Green 2019-01-01 4:51 ` [PATCH 09/13] drm/mediatek: " Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-02-25 23:55 ` Evan Green 2019-02-25 23:55 ` Evan Green 2019-02-25 23:55 ` Evan Green 2019-01-01 4:51 ` [PATCH 10/13] memory: mtk-smi: " Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-02-25 23:56 ` Evan Green 2019-02-25 23:56 ` Evan Green 2019-02-25 23:56 ` Evan Green 2019-01-01 4:51 ` [PATCH 11/13] iommu/mediatek: Use builtin_platform_driver Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-02-25 23:56 ` Evan Green 2019-02-25 23:56 ` Evan Green 2019-02-25 23:56 ` Evan Green 2019-02-27 14:33 ` Yong Wu 2019-02-27 14:33 ` Yong Wu 2019-02-27 14:33 ` Yong Wu 2019-03-05 19:03 ` Evan Green 2019-03-05 19:03 ` Evan Green 2019-03-05 19:03 ` Evan Green 2019-01-01 4:51 ` [PATCH 12/13] arm: dts: mediatek: Get rid of mediatek,larb for MM nodes Yong Wu 2019-01-01 4:51 ` [PATCH 12/13] arm: dts: mediatek: Get rid of mediatek, larb " Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-02-25 23:56 ` [PATCH 12/13] arm: dts: mediatek: Get rid of mediatek,larb " Evan Green 2019-02-25 23:56 ` [PATCH 12/13] arm: dts: mediatek: Get rid of mediatek, larb " Evan Green 2019-02-25 23:56 ` Evan Green 2019-01-01 4:51 ` [PATCH 13/13] arm64: dts: mediatek: Get rid of mediatek,larb " Yong Wu 2019-01-01 4:51 ` [PATCH 13/13] arm64: dts: mediatek: Get rid of mediatek, larb " Yong Wu 2019-01-01 4:51 ` Yong Wu 2019-02-25 23:56 ` [PATCH 13/13] arm64: dts: mediatek: Get rid of mediatek,larb " Evan Green 2019-02-25 23:56 ` [PATCH 13/13] arm64: dts: mediatek: Get rid of mediatek, larb " Evan Green 2019-02-25 23:56 ` Evan Green
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=709d7d66-9610-eeb3-128e-e2156bcfb616@arm.com \ --to=robin.murphy@arm.com \ --cc=arnd@arndb.de \ --cc=devicetree@vger.kernel.org \ --cc=drinkcat@chromium.org \ --cc=gregkh@linuxfoundation.org \ --cc=iommu@lists.linux-foundation.org \ --cc=joro@8bytes.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mediatek@lists.infradead.org \ --cc=matthias.bgg@gmail.com \ --cc=robh+dt@kernel.org \ --cc=srv_heupstream@mediatek.com \ --cc=tfiga@google.com \ --cc=will.deacon@arm.com \ --cc=yingjoe.chen@mediatek.com \ --cc=yong.wu@mediatek.com \ --cc=youlin.pei@mediatek.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.