diff mbox

[2/4] dt/bindings: Introduce the FSL QorIQ DPAA BMan portal(s)

Message ID 1413986972-621-2-git-send-email-Emilian.Medve@Freescale.com
State Superseded, archived
Headers show

Commit Message

Emil Medve Oct. 22, 2014, 2:09 p.m. UTC
Portals are used by software running on processor cores, accelerators and
network interfaces to communicate with the BMan

Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
Change-Id: I6d245ffc14ba3d0e91d403ac7c3b91b75a9e6a95
---
 .../bindings/powerpc/fsl/bman-portals.txt          | 50 ++++++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt

Comments

Emil Medve Oct. 22, 2014, 8:04 p.m. UTC | #1
Hello Mark,


Thanks for having a look at this

On 10/22/2014 09:29 AM, Mark Rutland wrote:
> On Wed, Oct 22, 2014 at 03:09:30PM +0100, Emil Medve wrote:
>> Portals are used by software running on processor cores, accelerators and
>> network interfaces to communicate with the BMan
> 
> What exactly is a portal?
> 
> Is it a region of shared memory? A device?

In a nutshell a (software, i.e. processor core accessible) portal is a
memory mapped interface to the B/QMan that allows low latency, lock-less
interaction by logically separated units of software. The original
intent was to have one affine portal per core. As of now we're
sprinkling portals to use from various (core affine) contexts:
hypervisor, guests, user-space, containers, etc.

I'll make the definition more palatable in the next round

> I only received emails 2 and 3 of this series, so I'm lacking the
> context necessary to understand the bindings.

Some bubble in the pipe... As of now they all seem to have hit the
e-mail list(s), patchwork and hopefully your Inbox

>> Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
>> Change-Id: I6d245ffc14ba3d0e91d403ac7c3b91b75a9e6a95
>> ---
>>  .../bindings/powerpc/fsl/bman-portals.txt          | 50 ++++++++++++++++++++++
>>  1 file changed, 50 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt
>>
>> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt b/Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt
>> new file mode 100644
>> index 0000000..40e607e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt
>> @@ -0,0 +1,50 @@
>> +QorIQ DPAA Buffer Manager Portals Device Tree Binding
>> +
>> +Copyright (C) 2008 - 2014 Freescale Semiconductor Inc.
>> +
>> +CONTENTS
>> +
>> +	- BMan Portal
>> +	- Example
>> +
>> +NOTE:	The bindings described in this document are preliminary and subject to
>> +	change
> 
> While we've tried that elsewhere, unstable DT bindings have been shown
> to be a major source of pain.

Agreed

> I'd feel rather uncomfortable accepting a
> binding that we already believe to be insufficient to describe the
> hardware.
> 
> What do you expect to change?

Related bindings seem incomplete. As such, the PAMU binding (pamu.txt)
covers incompletely a dynamic LIODN assignment/programming model. The
current driver uses a static assignment scheme that the binding needs to
include. I also suspect that once the driver starts supporting the
dynamic LIODN assignment/programming we might find some wrinkles

>> +
>> +BMan Portal Node
>> +
>> +PROPERTIES
>> +
>> +- compatible
>> +	Usage:		Required
>> +	Value type:	<stringlist>
>> +	Definition:	Must include "fsl,bman-portal-<hardware revision>"
>> +			May include "fsl,<SoC>-bman-portal" or "fsl,bman-portal"
>> +
>> +- reg
>> +	Usage:		Required
>> +	Value type:	<prop-encoded-array>
>> +	Definition:	Two regions. The first is the cache-enabled region of
>> +			the portal. The second is the cache-inhibited region of
>> +			the portal
>> +
>> +EXAMPLE
>> +
>> +The example below shows a (P4080) BMan portals container/bus node with two portals
> 
> Is there any particular reason to place these under a simple-bus?

I think they fit the ePAPR definition for simple-bus container. They can
be accessed directly and can be used independently. What are you suggesting?

>> +
>> +	bman-portals@ff4000000 {
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		compatible = "simple-bus";
>> +		ranges = <0 0xf 0xf4000000 0x200000>;
>> +
>> +		bman-portal@0 {
>> +			compatible = "fsl,bman-portal-1.0.0", "fsl,bman-portal";
>> +			reg = <0x0 0x4000 0x100000 0x1000>;
> 
> It would be easier to read is each entry had its own set of brackets.
> Initially this looked to me like a single 64-bit address/size pair.

Something like <>, <>? It doesn't seem widely used but I agree is more
readable. I can include it in the the next spin

>> +			interrupts = <105 2 0 0>;
>> +		};
> 
> Given the description above, surely you need to know what the portal is
> used for? Or is that queried from the portal?

We don't need/want to include such information in the DT. Portals are
"allocated" dynamically and used by the respective context


Cheers,


> Thanks,
> Mark.
> 
>> +		bman-portal@4000 {
>> +			compatible = "fsl,bman-portal-1.0.0", "fsl,bman-portal";
>> +			reg = <0x4000 0x4000 0x101000 0x1000>;
>> +			interrupts = <107 2 0 0>;
>> +		};
>> +	};
>> -- 
>> 2.1.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Rutland Oct. 23, 2014, 11:16 a.m. UTC | #2
On Wed, Oct 22, 2014 at 09:04:54PM +0100, Emil Medve wrote:
> Hello Mark,
> 
> 
> Thanks for having a look at this
> 
> On 10/22/2014 09:29 AM, Mark Rutland wrote:
> > On Wed, Oct 22, 2014 at 03:09:30PM +0100, Emil Medve wrote:
> >> Portals are used by software running on processor cores, accelerators and
> >> network interfaces to communicate with the BMan
> > 
> > What exactly is a portal?
> > 
> > Is it a region of shared memory? A device?
> 
> In a nutshell a (software, i.e. processor core accessible) portal is a
> memory mapped interface to the B/QMan that allows low latency, lock-less
> interaction by logically separated units of software. The original
> intent was to have one affine portal per core. As of now we're
> sprinkling portals to use from various (core affine) contexts:
> hypervisor, guests, user-space, containers, etc.
> 
> I'll make the definition more palatable in the next round

Thanks,

> > I only received emails 2 and 3 of this series, so I'm lacking the
> > context necessary to understand the bindings.
> 
> Some bubble in the pipe... As of now they all seem to have hit the
> e-mail list(s), patchwork and hopefully your Inbox

They've all come through now, yes.

I'd expected to see the relevant code too -- in isolation the bindings
are somewhat difficult to grok.

> 
> >> Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
> >> Change-Id: I6d245ffc14ba3d0e91d403ac7c3b91b75a9e6a95
> >> ---
> >>  .../bindings/powerpc/fsl/bman-portals.txt          | 50 ++++++++++++++++++++++
> >>  1 file changed, 50 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt b/Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt
> >> new file mode 100644
> >> index 0000000..40e607e
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt
> >> @@ -0,0 +1,50 @@
> >> +QorIQ DPAA Buffer Manager Portals Device Tree Binding
> >> +
> >> +Copyright (C) 2008 - 2014 Freescale Semiconductor Inc.
> >> +
> >> +CONTENTS
> >> +
> >> +	- BMan Portal
> >> +	- Example
> >> +
> >> +NOTE:	The bindings described in this document are preliminary and subject to
> >> +	change
> > 
> > While we've tried that elsewhere, unstable DT bindings have been shown
> > to be a major source of pain.
> 
> Agreed
> 
> > I'd feel rather uncomfortable accepting a
> > binding that we already believe to be insufficient to describe the
> > hardware.
> > 
> > What do you expect to change?
> 
> Related bindings seem incomplete. As such, the PAMU binding (pamu.txt)
> covers incompletely a dynamic LIODN assignment/programming model. The
> current driver uses a static assignment scheme that the binding needs to
> include. I also suspect that once the driver starts supporting the
> dynamic LIODN assignment/programming we might find some wrinkles

Ok. Given that the driver doesn't seem to be in mainline (per a quick
grep), just put "DO NOT MERGE" or something like that in the commit
message. Once this is merged you're going to need to support it.

> >> +BMan Portal Node
> >> +
> >> +PROPERTIES
> >> +
> >> +- compatible
> >> +	Usage:		Required
> >> +	Value type:	<stringlist>
> >> +	Definition:	Must include "fsl,bman-portal-<hardware revision>"
> >> +			May include "fsl,<SoC>-bman-portal" or "fsl,bman-portal"
> >> +
> >> +- reg
> >> +	Usage:		Required
> >> +	Value type:	<prop-encoded-array>
> >> +	Definition:	Two regions. The first is the cache-enabled region of
> >> +			the portal. The second is the cache-inhibited region of
> >> +			the portal
> >> +
> >> +EXAMPLE
> >> +
> >> +The example below shows a (P4080) BMan portals container/bus node with two portals
> > 
> > Is there any particular reason to place these under a simple-bus?
> 
> I think they fit the ePAPR definition for simple-bus container. They can
> be accessed directly and can be used independently. What are you suggesting?
> 
> >> +
> >> +	bman-portals@ff4000000 {
> >> +		#address-cells = <1>;
> >> +		#size-cells = <1>;
> >> +		compatible = "simple-bus";
> >> +		ranges = <0 0xf 0xf4000000 0x200000>;
> >> +
> >> +		bman-portal@0 {
> >> +			compatible = "fsl,bman-portal-1.0.0", "fsl,bman-portal";
> >> +			reg = <0x0 0x4000 0x100000 0x1000>;
> > 
> > It would be easier to read is each entry had its own set of brackets.
> > Initially this looked to me like a single 64-bit address/size pair.
> 
> Something like <>, <>?

So here we'd have:

	reg = <0x000000 0x4000>,
	      <0x100000 0x1000>;

> It doesn't seem widely used but I agree is more readable. I can
> include it in the the next spin

Thanks.

> 
> >> +			interrupts = <105 2 0 0>;
> >> +		};
> > 
> > Given the description above, surely you need to know what the portal is
> > used for? Or is that queried from the portal?
> 
> We don't need/want to include such information in the DT. Portals are
> "allocated" dynamically and used by the respective context

Ok. So they have no fixed purpose and software allocates as necessary?
There are no constraints on how each portal may be used, or which portal
another agetn might use?

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Geoff Thorpe Oct. 23, 2014, 1:28 p.m. UTC | #3
Hi,

Leaping in here a little after the fact, to add a bit of background info on the hardware in case it helps.

> -----Original Message-----

> From: Mark Rutland [mailto:mark.rutland@arm.com]

> Sent: October-23-14 7:16 AM

> To: Medve Emilian-EMMEDVE1

> 

> On Wed, Oct 22, 2014 at 09:04:54PM +0100, Emil Medve wrote:

> > Hello Mark,

> >

> >

> > Thanks for having a look at this

> >

> > On 10/22/2014 09:29 AM, Mark Rutland wrote:

> > > On Wed, Oct 22, 2014 at 03:09:30PM +0100, Emil Medve wrote:

> > >> Portals are used by software running on processor cores,

> accelerators and

> > >> network interfaces to communicate with the BMan

> > >

> > > What exactly is a portal?

> > >

> > > Is it a region of shared memory? A device?


Just to supplement what Emil already mentioned, the portals each contain two memory-mapped regions and a single interrupt. One of the regions is a conventional 4K page of 32-bit cache-inhibited registers (and this includes all the controls and status for the portal's interrupt line, which conveys a variety of individually-configurable interrupt source conditions). The other region is a 16K cache-enabled space, so the "registers" are cacheline width - interaction with this relies on cacheline manipulation instructions (flushing, prefetching, etc), and in some configurations hardware will initiate stashing (cache-warming) for some of those cachelines, which is one of the reasons that these portals have "LIODN" information configured into them (stashing transactions go through the IOMMU). The other reason for LIODN/IOMMU context is that all the accelerator and networking functions accessed via a portal will be subjected to the IOMMU context info of that portal. In this way, a portal that is mapped/used by a given software context can be setup such that DMA generated by its hardware interactions will respect the same guest-physical translations and restrictions as apply to the software context itself.

BTW, a subset of the CCSR registers for the QMan and BMan blocks are used to configure these LIODN/IOMMU attributes of the portals, as opposed to those attributes being exposed in the register space of the portals themselves. In this way, the user of a memory-mapped portal cannot set their own LIODN/IOMMU attributes, that decision belongs to whoever twiddles with CCSR.

> > Given the description above, surely you need to know what the

> portal is

> > > used for? Or is that queried from the portal?

> >

> > We don't need/want to include such information in the DT. Portals

> are

> > "allocated" dynamically and used by the respective context

> 

> Ok. So they have no fixed purpose and software allocates as necessary?

> There are no constraints on how each portal may be used, or which

> portal

> another agetn might use?


Yes, precisely. They're interchangeable windows/contexts into the hardware, for issuing various kinds of commands and obtaining various kinds of results/status. The SoC provides a certain number of these portals, and software can allocate them out and configure them in whatever manner (static, dynamic, ...). Because of the CCSR-controlled LIODN/IOMMU portal attributes, two portals can be set up to have different "access" and operate in different contexts (eg. apps, virtual machines), but that is determined by control software (kernel driver) configuration of those portals before their use. In short, there's nothing intrinsic to the portals (ie. from a hardware perspective) that distinguishes them or predetermines their use.

Cheers,
Geoff
Emil Medve Oct. 24, 2014, 9:26 a.m. UTC | #4
Hello Mark,


On 10/23/2014 06:16 AM, Mark Rutland wrote:
>>>> +NOTE:	The bindings described in this document are preliminary and subject to
>>>> +	change
>>>
>>> While we've tried that elsewhere, unstable DT bindings have been shown
>>> to be a major source of pain.
>>
>> Agreed
>>
>>> I'd feel rather uncomfortable accepting a
>>> binding that we already believe to be insufficient to describe the
>>> hardware.
>>>
>>> What do you expect to change?
>>
>> Related bindings seem incomplete. As such, the PAMU binding (pamu.txt)
>> covers incompletely a dynamic LIODN assignment/programming model. The
>> current driver uses a static assignment scheme that the binding needs to
>> include. I also suspect that once the driver starts supporting the
>> dynamic LIODN assignment/programming we might find some wrinkles
> 
> Ok. Given that the driver doesn't seem to be in mainline (per a quick
> grep), just put "DO NOT MERGE" or something like that in the commit
> message. Once this is merged you're going to need to support it.

We have a driver and once the bindings get accepted we'll (update the
driver as needed and) post the code


Cheers,
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood Oct. 28, 2014, 6:09 p.m. UTC | #5
On Wed, 2014-10-22 at 15:04 -0500, Emil Medve wrote:
> Hello Mark,
> 
> 
> Thanks for having a look at this
> 
> On 10/22/2014 09:29 AM, Mark Rutland wrote:
> > I'd feel rather uncomfortable accepting a
> > binding that we already believe to be insufficient to describe the
> > hardware.
> > 
> > What do you expect to change?
> 
> Related bindings seem incomplete. As such, the PAMU binding (pamu.txt)
> covers incompletely a dynamic LIODN assignment/programming model. The
> current driver uses a static assignment scheme that the binding needs to
> include. I also suspect that once the driver starts supporting the
> dynamic LIODN assignment/programming we might find some wrinkles

How is this different from any of the other QorIQ bindings that have
been merged without such a disclaimer?  The static LIODN model is
already there, even if documentation is missing, and should continue to
be supported even if we eventually implement a dynamic LIODN model.

> >> +
> >> +	bman-portals@ff4000000 {
> >> +		#address-cells = <1>;
> >> +		#size-cells = <1>;
> >> +		compatible = "simple-bus";
> >> +		ranges = <0 0xf 0xf4000000 0x200000>;
> >> +
> >> +		bman-portal@0 {
> >> +			compatible = "fsl,bman-portal-1.0.0", "fsl,bman-portal";
> >> +			reg = <0x0 0x4000 0x100000 0x1000>;
> > 
> > It would be easier to read is each entry had its own set of brackets.
> > Initially this looked to me like a single 64-bit address/size pair.
> 
> Something like <>, <>? It doesn't seem widely used but I agree is more
> readable. I can include it in the the next spin

The older PPC device trees haven't used it much but I think it's pretty
common in the newer ARM trees.

-Scott


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt b/Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt
new file mode 100644
index 0000000..40e607e
--- /dev/null
+++ b/Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt
@@ -0,0 +1,50 @@ 
+QorIQ DPAA Buffer Manager Portals Device Tree Binding
+
+Copyright (C) 2008 - 2014 Freescale Semiconductor Inc.
+
+CONTENTS
+
+	- BMan Portal
+	- Example
+
+NOTE:	The bindings described in this document are preliminary and subject to
+	change
+
+BMan Portal Node
+
+PROPERTIES
+
+- compatible
+	Usage:		Required
+	Value type:	<stringlist>
+	Definition:	Must include "fsl,bman-portal-<hardware revision>"
+			May include "fsl,<SoC>-bman-portal" or "fsl,bman-portal"
+
+- reg
+	Usage:		Required
+	Value type:	<prop-encoded-array>
+	Definition:	Two regions. The first is the cache-enabled region of
+			the portal. The second is the cache-inhibited region of
+			the portal
+
+EXAMPLE
+
+The example below shows a (P4080) BMan portals container/bus node with two portals
+
+	bman-portals@ff4000000 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		ranges = <0 0xf 0xf4000000 0x200000>;
+
+		bman-portal@0 {
+			compatible = "fsl,bman-portal-1.0.0", "fsl,bman-portal";
+			reg = <0x0 0x4000 0x100000 0x1000>;
+			interrupts = <105 2 0 0>;
+		};
+		bman-portal@4000 {
+			compatible = "fsl,bman-portal-1.0.0", "fsl,bman-portal";
+			reg = <0x4000 0x4000 0x101000 0x1000>;
+			interrupts = <107 2 0 0>;
+		};
+	};