LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Will Deacon <will.deacon@arm.com>
Cc: Murali Karicheri <m-karicheri2@ti.com>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	Russell King <linux@arm.linux.org.uk>,
	Arnd Bergmann <arnd@arndb.de>, Joerg Roedel <joro@8bytes.org>,
	Rob Herring <robh+dt@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	"suravee.suthikulpanit@amd.com" <suravee.suthikulpanit@amd.com>,
	"grant.likely@linaro.org" <grant.likely@linaro.org>
Subject: Re: [PATCH v4 1/6] of: iommu: add ptr to OF node arg to of_iommu_configure()
Date: Wed, 28 Jan 2015 15:15:10 +0200	[thread overview]
Message-ID: <13812499.ICbNgSDOae@avalon> (raw)
In-Reply-To: <20150128122941.GK1569@arm.com>

On Wednesday 28 January 2015 12:29:42 Will Deacon wrote:
> On Wed, Jan 28, 2015 at 12:23:03PM +0000, Laurent Pinchart wrote:
> > On Wednesday 28 January 2015 11:33:00 Will Deacon wrote:
> >> On Mon, Jan 26, 2015 at 06:49:01PM +0000, Murali Karicheri wrote:
> >>> On 01/25/2015 08:32 AM, Laurent Pinchart wrote:
> >>>> On Friday 23 January 2015 17:32:34 Murali Karicheri wrote:
> >>>>> Function of_iommu_configure() is called from of_dma_configure() to
> >>>>> setup iommu ops using DT property. This API is currently used for
> >>>>> platform devices for which DMA configuration (including iommu ops)
> >>>>> may come from device's parent. To extend this functionality for PCI
> >>>>> devices, this API need to take a parent node ptr as an argument
> >>>>> instead of assuming device's parent. This is needed since for PCI,
> >>>>> the dma configuration may be defined in the DT node of the root bus
> >>>>> bridge's parent device. Currently only dma-range is used for PCI and
> >>>>> iommu is not supported. So return error if the device is PCI.
> >>>>> 
> >>>>> Cc: Joerg Roedel<joro@8bytes.org>
> >>>>> Cc: Grant Likely<grant.likely@linaro.org>
> >>>>> Cc: Rob Herring<robh+dt@kernel.org>
> >>>>> Cc: Bjorn Helgaas<bhelgaas@google.com>
> >>>>> Cc: Will Deacon<will.deacon@arm.com>
> >>>>> Cc: Russell King<linux@arm.linux.org.uk>
> >>>>> Cc: Arnd Bergmann<arnd@arndb.de>
> >>>>> Cc: Suravee Suthikulpanit<Suravee.Suthikulpanit@amd.com>
> >>>>> 
> >>>>> Signed-off-by: Murali Karicheri<m-karicheri2@ti.com>
> >>>>> ---
> >>>>> 
> >>>>>   drivers/iommu/of_iommu.c |   10 ++++++++--
> >>>>>   drivers/of/platform.c    |    2 +-
> >>>>>   include/linux/of_iommu.h |    6 ++++--
> >>>>>   3 files changed, 13 insertions(+), 5 deletions(-)
> >>>>> 
> >>>>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> >>>>> index af1dc6a..439235b 100644
> >>>>> --- a/drivers/iommu/of_iommu.c
> >>>>> +++ b/drivers/iommu/of_iommu.c
> >>>>> @@ -133,19 +133,25 @@ struct iommu_ops *of_iommu_get_ops(struct
> >>>>> device_node *np)
> >>>>>   	return ops;
> >>>>>  }
> >>>>> 
> >>>>> -struct iommu_ops *of_iommu_configure(struct device *dev)
> >>>>> +struct iommu_ops *of_iommu_configure(struct device *dev,
> >>>>> +				     struct device_node *iommu_np)
> >>>>>  {
> >>>>>   	struct of_phandle_args iommu_spec;
> >>>>>   	struct device_node *np;
> >>>>>   	struct iommu_ops *ops = NULL;
> >>>>>   	int idx = 0;
> >>>>> 
> >>>>> +	if (dev_is_pci(dev)) {
> >>>>> +		dev_err(dev, "iommu is currently not supported for PCI\n");
> >>>>> +		return NULL;
> >>>>> +	}
> >>>>> +
> >>>>>   	/*
> >>>>>   	 * We don't currently walk up the tree looking for a parent
> >>>>>   	 IOMMU.
> >>>>>   	 * See the `Notes:' section of
> >>>>>   	 * Documentation/devicetree/bindings/iommu/iommu.txt
> >>>>>   	 */
> >>>> 
> >>>> Wouldn't it be better to fix this rather than passing the device node
> >>>> pointer to the function ? The solution would be more generic.
> >>> 
> >>> Will Deacon (Copied here) is working on this as we alteady discussed
> >>> in this thread. So it will be addressed by him in another series.
> >> 
> >> Well, "working on this" equates to "has it somewhere near the bottom of
> >> a very long list" :)
> > 
> > What's your opinion on this patch then, do you think adding the iommu_np
> > argument makes sense as a temporary hack, or should we instead walk up the
> > tree looking for an iommus attribute in parent nodes ? I don't expect that
> > to be insanely difficult to code.
> 
> If walking up the tree is useful, then I think we should do that and update
> the Documentation to reflect the new behaviour.

If I understand Murali's patch set right (please correct me if that's not the 
case) the PCI code walks up the DT nodes hierarchy to the parent node that 
contains the iommus attribute and passes a pointer to that node to this 
function. It's thus a PCI-specific solution. As a temporary hack that's OK I 
suppose, but if implementing it right straight away isn't difficult that would 
be better.

> The only reason that I didn't code that in of_iommu_configure originally is
> because I didn't want to go against the binding spec for the initial
> version, especially as we didn't have a reason to look at parent nodes.
> 
> The only thing to double-check is that we don't break any existing users
> of the binding with this change, but I don't think that we do.

-- 
Regards,

Laurent Pinchart


  reply	other threads:[~2015-01-28 20:23 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-23 22:32 [PATCH v4 0/6] PCI: get DMA configuration from parent device Murali Karicheri
2015-01-23 22:32 ` [PATCH v4 1/6] of: iommu: add ptr to OF node arg to of_iommu_configure() Murali Karicheri
2015-01-25 13:32   ` Laurent Pinchart
2015-01-26 18:49     ` Murali Karicheri
2015-01-28 11:33       ` Will Deacon
2015-01-28 12:23         ` Laurent Pinchart
2015-01-28 12:29           ` Will Deacon
2015-01-28 13:15             ` Laurent Pinchart [this message]
2015-01-28 13:32               ` Will Deacon
2015-01-28 15:21                 ` Murali Karicheri
2015-01-28 23:32                 ` Laurent Pinchart
2015-01-29 14:59                   ` Murali Karicheri
2015-01-29 16:49                   ` Rob Herring
2015-01-30  0:24                     ` Laurent Pinchart
2015-01-30 15:23                       ` Murali Karicheri
2015-01-23 22:32 ` [PATCH v4 2/6] of: move of_dma_configure() to device.c to help re-use Murali Karicheri
2015-01-23 22:32 ` [PATCH v4 3/6] of: fix size when dma-range is not used Murali Karicheri
2015-01-27 11:27   ` Robin Murphy
2015-01-27 15:44     ` Murali Karicheri
2015-01-27 18:55     ` Murali Karicheri
2015-01-28 11:05       ` Catalin Marinas
2015-01-28 15:45         ` Rob Herring
2015-01-28 17:23           ` Catalin Marinas
2015-01-28 17:34           ` Murali Karicheri
2015-01-28 15:55         ` Robin Murphy
2015-01-28 17:30           ` Catalin Marinas
2015-01-30 18:06             ` Murali Karicheri
2015-02-02 12:18               ` Catalin Marinas
2015-02-02 16:10                 ` Murali Karicheri
2015-02-05 21:42                 ` Murali Karicheri
2015-02-05 22:44                   ` Catalin Marinas
2015-01-23 22:32 ` [PATCH v4 4/6] of/pci: add of_pci_dma_configure() update dma configuration Murali Karicheri
2015-01-23 23:41   ` Bjorn Helgaas
2015-01-26 23:25     ` Murali Karicheri
2015-01-26 23:59       ` Bjorn Helgaas
2015-01-27 18:14         ` Murali Karicheri
2015-01-27 18:42           ` Bjorn Helgaas
2015-01-27 18:45             ` Murali Karicheri
2015-01-23 22:32 ` [PATCH v4 5/6] PCI: update dma configuration from DT Murali Karicheri
2015-01-23 23:27   ` Bjorn Helgaas
2015-01-26 23:28     ` Murali Karicheri
2015-01-23 22:32 ` [PATCH v4 6/6] arm: dma-mapping: updates to limit dma_mask and iommu mapping size Murali Karicheri
2015-01-27 11:12   ` Robin Murphy
2015-01-27 11:34     ` Catalin Marinas
2015-01-27 15:19       ` Murali Karicheri

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=13812499.ICbNgSDOae@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=arnd@arndb.de \
    --cc=bhelgaas@google.com \
    --cc=devicetree@vger.kernel.org \
    --cc=grant.likely@linaro.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-pci@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=m-karicheri2@ti.com \
    --cc=robh+dt@kernel.org \
    --cc=suravee.suthikulpanit@amd.com \
    --cc=will.deacon@arm.com \
    --subject='Re: [PATCH v4 1/6] of: iommu: add ptr to OF node arg to of_iommu_configure()' \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).