devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andreas Herrmann <andreas.herrmann@calxeda.com>
To: Will Deacon <will.deacon@arm.com>
Cc: Olav Haugan <ohaugan@codeaurora.org>,
	"devicetree-discuss@lists.ozlabs.org"
	<devicetree-discuss@lists.ozlabs.org>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v2] documentation: iommu: add description of ARM System MMU binding
Date: Fri, 17 May 2013 22:16:39 +0200	[thread overview]
Message-ID: <20130517201639.GL10369@alberich> (raw)
In-Reply-To: <20130513104147.GF10369@alberich>

On Mon, May 13, 2013 at 12:41:47PM +0200, Andreas Herrmann wrote:
> On Mon, May 13, 2013 at 05:58:46AM -0400, Will Deacon wrote:
> > On Mon, May 13, 2013 at 10:50:20AM +0100, Andreas Herrmann wrote:

[snip]

> > > I also think that it is more useful to move the stream-id property to
> > > the device node of a master device. (It's a characteristic of the
> > > master device not of the SMMU.) Currently with multiple stream IDs per
> > > master device you have repeated entries in the mmu-master property.
> > 
> > The problem with that approach is how to handle StreamID remastering. This
> > can and will happen, so the StreamID for a device is actually a property of
> > both the device *and* a particular point in the bus topology. Putting this
> > information in the device nodes will drag topology information all over the
> > place and I don't think it will make things clearer to read or easier to parse.
> 
> Ok, good point, didn't think about that.
> And agreed, adding remastered StreamIDs as a property to a device node is odd.
> 
> > > But all that is needed is to point (once) to each mmu-master in the
> > > SMMU device node. Then you should be able to look up the corresponding
> > > stream IDs in the device node for each mmu-master.
> > 
> > Again, you also need to tie in topology information if you go down this
> > route.

I still don't like the approach of having two independend lists that
must be in sync to associate a master with its stream-ids.

Why? Say you have 8 masters for an SMMU with 1 or 2 stream-ids each:

     	 smmu {
		...
                mmu-masters = <&dma0>, <&dma0>, <&dma1>, <&dma1>,
			      <&dma2>, <&dma2>, <&dma4>, <&dma4>,
			      <&dma5>, <&dma6>, <&dma7>, <&dma8>;
                stream-ids =	<0>, <1>, <2>, <3>,
				<4>, <5>, <6>, <7>,
		                <8>, <9>, <0xa>, <0xb>;
	}

Couldn't we use of_phandle_args for this purpose? So your example

+        smmu {
		 ...
+                mmu-masters = <&dma0>,
+                              <&dma0>,
+                              <&dma1>;
+                stream-ids  = <0xd01d>,
+                              <0xd01e>,
+                              <0xd11c>;
+        };

would look like

	dma0 {
		...
		#stream-id-cells = <2>
		...
	}

	dma1 {
		...
		#stream-id-cells = <1>
		...
	}

        smmu {
		...
		mmu-masters = <&dma0 0xd01d 0xd01e
			       &dma1 0xd11c>,
       };

and my example would be converted to

	smmu {
		...
                mmu-masters = <&dma0 0 1 &dma1 2 3 &dma2 4 5
			       &dma4 6 7 &dma5 8 &dma6 9
			       &dma7 0xa &dma8 0xb>
		...
	}

where each master has #stream-id-cells property with value 1 or 2.
And if dma4 has #stream-id-cells = <1>, the parsing code quickly
notifies us about an error whereas such an error can't be noticed
right from the beginning with the two-list-approach. In this example
stream-id 6 belongs to dma3 which was completely ommitted in both
descriptions.

Of course usage of of_phandle_args would restrict the number of
stream-ids per master to 8 (which is currently used as
MAX_PHANDLE_ARGS). But I don't think that this is a restriction in
practice or do you expect to have more than 8 stream-ids per master
(ie. per struct device in Linux)?


Andreas

  reply	other threads:[~2013-05-17 20:16 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-12 18:02 [PATCH v2] documentation: iommu: add description of ARM System MMU binding Will Deacon
2013-05-13  9:50 ` Andreas Herrmann
2013-05-13  9:58   ` Will Deacon
2013-05-13 10:41     ` Andreas Herrmann
2013-05-17 20:16       ` Andreas Herrmann [this message]
2013-05-20 10:18         ` Will Deacon
     [not found]           ` <20130520101841.GK31359-MRww78TxoiP5vMa5CHWGZ34zcgK1vI+I0E9HWUfgJXw@public.gmane.org>
2013-05-21 10:25             ` Andreas Herrmann
2013-05-21 17:33               ` Will Deacon
     [not found]                 ` <20130521173357.GA26251-MRww78TxoiP5vMa5CHWGZ34zcgK1vI+I0E9HWUfgJXw@public.gmane.org>
2013-05-21 18:35                   ` Andreas Herrmann

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=20130517201639.GL10369@alberich \
    --to=andreas.herrmann@calxeda.com \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=iommu@lists.linux-foundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=ohaugan@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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).