From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joerg Roedel Subject: Re: [PATCH v6 4/5] iommu/mediatek: Add mt8173 IOMMU driver Date: Mon, 14 Dec 2015 15:16:57 +0100 Message-ID: <20151214141656.GG18805@8bytes.org> References: <1449568153-15643-1-git-send-email-yong.wu@mediatek.com> <1449568153-15643-5-git-send-email-yong.wu@mediatek.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1449568153-15643-5-git-send-email-yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Yong Wu Cc: Mark Rutland , Catalin Marinas , Will Deacon , yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org, Thierry Reding , devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sasha Hauer , arnd-r2nGTMty4D4@public.gmane.org, Tomasz Figa , Rob Herring , linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Matthias Brugger , linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, pebolle-IWqWACnzNjzz+pZb47iToQ@public.gmane.org, srv_heupstream-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, Daniel Kurtz , p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org, Lucas Stach List-Id: iommu@lists.linux-foundation.org On Tue, Dec 08, 2015 at 05:49:12PM +0800, Yong Wu wrote: > +static int mtk_iommu_attach_device(struct iommu_domain *domain, > + struct device *dev) > +{ > + struct mtk_iommu_domain *dom = to_mtk_domain(domain); > + struct mtk_iommu_client_priv *priv = dev->archdata.iommu; > + struct mtk_iommu_data *data; > + int ret; > + > + if (!priv) > + return -ENODEV; > + > + data = dev_get_drvdata(priv->m4udev); > + if (!data) { > + /* > + * The DMA core will run earlier than this probe, and it will > + * create a default iommu domain for each a iommu device. > + * But here there is only one domain called the m4u domain > + * which all the multimedia HW share. > + * The default domain isn't needed here. > + */ The iommu core creates one domain per iommu-group. In your case this means one default domain per iommu in the system. > + iommu_domain_free(domain); This function is not supposed to free the domain passed to it. > +static int mtk_iommu_add_device(struct device *dev) > +{ > + struct iommu_group *group; > + > + if (!dev->archdata.iommu) /* Not a iommu client device */ > + return -ENODEV; > + > + group = iommu_group_get_for_dev(dev); > + if (IS_ERR(group)) > + return PTR_ERR(group); > + > + iommu_group_put(group); > + return 0; > +} [...] > +static struct iommu_group *mtk_iommu_device_group(struct device *dev) > +{ > + struct mtk_iommu_data *data; > + struct mtk_iommu_client_priv *priv; > + > + priv = dev->archdata.iommu; > + if (!priv) > + return ERR_PTR(-ENODEV); > + > + /* All the client devices are in the same m4u iommu-group */ > + data = dev_get_drvdata(priv->m4udev); > + if (!data->m4u_group) { > + data->m4u_group = iommu_group_alloc(); > + if (IS_ERR(data->m4u_group)) > + dev_err(dev, "Failed to allocate M4U IOMMU group\n"); > + } > + return data->m4u_group; > +} This looks much better than before, thanks. From mboxrd@z Thu Jan 1 00:00:00 1970 From: joro@8bytes.org (Joerg Roedel) Date: Mon, 14 Dec 2015 15:16:57 +0100 Subject: [PATCH v6 4/5] iommu/mediatek: Add mt8173 IOMMU driver In-Reply-To: <1449568153-15643-5-git-send-email-yong.wu@mediatek.com> References: <1449568153-15643-1-git-send-email-yong.wu@mediatek.com> <1449568153-15643-5-git-send-email-yong.wu@mediatek.com> Message-ID: <20151214141656.GG18805@8bytes.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tue, Dec 08, 2015 at 05:49:12PM +0800, Yong Wu wrote: > +static int mtk_iommu_attach_device(struct iommu_domain *domain, > + struct device *dev) > +{ > + struct mtk_iommu_domain *dom = to_mtk_domain(domain); > + struct mtk_iommu_client_priv *priv = dev->archdata.iommu; > + struct mtk_iommu_data *data; > + int ret; > + > + if (!priv) > + return -ENODEV; > + > + data = dev_get_drvdata(priv->m4udev); > + if (!data) { > + /* > + * The DMA core will run earlier than this probe, and it will > + * create a default iommu domain for each a iommu device. > + * But here there is only one domain called the m4u domain > + * which all the multimedia HW share. > + * The default domain isn't needed here. > + */ The iommu core creates one domain per iommu-group. In your case this means one default domain per iommu in the system. > + iommu_domain_free(domain); This function is not supposed to free the domain passed to it. > +static int mtk_iommu_add_device(struct device *dev) > +{ > + struct iommu_group *group; > + > + if (!dev->archdata.iommu) /* Not a iommu client device */ > + return -ENODEV; > + > + group = iommu_group_get_for_dev(dev); > + if (IS_ERR(group)) > + return PTR_ERR(group); > + > + iommu_group_put(group); > + return 0; > +} [...] > +static struct iommu_group *mtk_iommu_device_group(struct device *dev) > +{ > + struct mtk_iommu_data *data; > + struct mtk_iommu_client_priv *priv; > + > + priv = dev->archdata.iommu; > + if (!priv) > + return ERR_PTR(-ENODEV); > + > + /* All the client devices are in the same m4u iommu-group */ > + data = dev_get_drvdata(priv->m4udev); > + if (!data->m4u_group) { > + data->m4u_group = iommu_group_alloc(); > + if (IS_ERR(data->m4u_group)) > + dev_err(dev, "Failed to allocate M4U IOMMU group\n"); > + } > + return data->m4u_group; > +} This looks much better than before, thanks. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751960AbbLNORC (ORCPT ); Mon, 14 Dec 2015 09:17:02 -0500 Received: from 8bytes.org ([81.169.241.247]:44082 "EHLO theia.8bytes.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751832AbbLNOQ7 (ORCPT ); Mon, 14 Dec 2015 09:16:59 -0500 Date: Mon, 14 Dec 2015 15:16:57 +0100 From: Joerg Roedel To: Yong Wu Cc: Thierry Reding , Mark Rutland , Matthias Brugger , Robin Murphy , Will Deacon , Daniel Kurtz , Tomasz Figa , Lucas Stach , Rob Herring , Catalin Marinas , linux-mediatek@lists.infradead.org, Sasha Hauer , srv_heupstream@mediatek.com, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, iommu@lists.linux-foundation.org, pebolle@tiscali.nl, arnd@arndb.de, mitchelh@codeaurora.org, p.zabel@pengutronix.de, yingjoe.chen@mediatek.com Subject: Re: [PATCH v6 4/5] iommu/mediatek: Add mt8173 IOMMU driver Message-ID: <20151214141656.GG18805@8bytes.org> References: <1449568153-15643-1-git-send-email-yong.wu@mediatek.com> <1449568153-15643-5-git-send-email-yong.wu@mediatek.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1449568153-15643-5-git-send-email-yong.wu@mediatek.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Dec 08, 2015 at 05:49:12PM +0800, Yong Wu wrote: > +static int mtk_iommu_attach_device(struct iommu_domain *domain, > + struct device *dev) > +{ > + struct mtk_iommu_domain *dom = to_mtk_domain(domain); > + struct mtk_iommu_client_priv *priv = dev->archdata.iommu; > + struct mtk_iommu_data *data; > + int ret; > + > + if (!priv) > + return -ENODEV; > + > + data = dev_get_drvdata(priv->m4udev); > + if (!data) { > + /* > + * The DMA core will run earlier than this probe, and it will > + * create a default iommu domain for each a iommu device. > + * But here there is only one domain called the m4u domain > + * which all the multimedia HW share. > + * The default domain isn't needed here. > + */ The iommu core creates one domain per iommu-group. In your case this means one default domain per iommu in the system. > + iommu_domain_free(domain); This function is not supposed to free the domain passed to it. > +static int mtk_iommu_add_device(struct device *dev) > +{ > + struct iommu_group *group; > + > + if (!dev->archdata.iommu) /* Not a iommu client device */ > + return -ENODEV; > + > + group = iommu_group_get_for_dev(dev); > + if (IS_ERR(group)) > + return PTR_ERR(group); > + > + iommu_group_put(group); > + return 0; > +} [...] > +static struct iommu_group *mtk_iommu_device_group(struct device *dev) > +{ > + struct mtk_iommu_data *data; > + struct mtk_iommu_client_priv *priv; > + > + priv = dev->archdata.iommu; > + if (!priv) > + return ERR_PTR(-ENODEV); > + > + /* All the client devices are in the same m4u iommu-group */ > + data = dev_get_drvdata(priv->m4udev); > + if (!data->m4u_group) { > + data->m4u_group = iommu_group_alloc(); > + if (IS_ERR(data->m4u_group)) > + dev_err(dev, "Failed to allocate M4U IOMMU group\n"); > + } > + return data->m4u_group; > +} This looks much better than before, thanks.