From: mark.rutland@arm.com (Mark Rutland)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/6] dt/bindings: adjust bindings for Layerscape SCFG MSI
Date: Wed, 26 Oct 2016 11:31:16 +0100 [thread overview]
Message-ID: <20161026103101.GC19965@leverpostej> (raw)
In-Reply-To: <1477398945-22774-1-git-send-email-Minghuan.Lian@nxp.com>
On Tue, Oct 25, 2016 at 08:35:40PM +0800, Minghuan Lian wrote:
> 1. The different version of a SoC may have different MSI
> implementation. But compatible "fsl,<soc-name>-msi" can not describe
> the SoC version.
Surely, "fsl,<soc-name>-<rev>-msi" can describe this?
If the hardware differs, it needs a new compatible string.
If there's some configuration value that varies across revisions (e.g.
number of slots), you can add a proeprty to describe that explciitly.
> The MSI driver will use SoC match interface to get
> SoC type and version instead of compatible string. So all MSI node
> can use the common compatible "fsl,ls-scfg-msi" and the original
> compatible is unnecessary.
>
> 2. Layerscape SoCs may have one or several MSI controllers.
> In order to increase MSI interrupt number of a PCIe, the patch
> moves all MSI node into the parent node "msi-controller". So a
> PCIe can request MSI from all the MSI controllers.
This is not necessary, and does not represent a real block of hardware.
So NAK for this approach.
The msi-parent property can contain a list of MSI controllers. See the
examples in
Documentation/devicetree/bindings/interrupt-controller/msi.txt.
Likewise, the msi-map property can map to a number of MSI controllers.
If the core code can only consider one at a time, then that's an issue
to be addressed in core code, not one to be bodged around in bindings.
>
> Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
> ---
> .../interrupt-controller/fsl,ls-scfg-msi.txt | 57 +++++++++++++++++++---
> 1 file changed, 49 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
> index 9e38949..29f95fd 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
> @@ -1,18 +1,28 @@
> * Freescale Layerscape SCFG PCIe MSI controller
>
> +Layerscape SoCs may have one or multiple MSI controllers.
> +Each MSI controller must be showed as a child node.
> +
> Required properties:
>
> -- compatible: should be "fsl,<soc-name>-msi" to identify
> - Layerscape PCIe MSI controller block such as:
> - "fsl,1s1021a-msi"
> - "fsl,1s1043a-msi"
> +- compatible: should be "fsl,ls-scfg-msi"
This breaks old DTBs, and throws away information which you describe
above as valuable. So another NAK for that.
> +- #address-cells: must be 2
> +- #size-cells: must be 2
> +- ranges: allows valid 1:1 translation between child's address space and
> + parent's address space
> - msi-controller: indicates that this is a PCIe MSI controller node
> +
> +Required child node:
> +A child node must exist to represent the MSI controller.
> +The following are properties specific to those nodes:
Also, as above, the approach of gathering MSI controllers in this manner
is wrong.
Thanks,
Mark.
WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: Minghuan Lian <Minghuan.Lian@nxp.com>
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
Marc Zyngier <marc.zyngier@arm.com>,
Stuart Yoder <stuart.yoder@nxp.com>,
Yang-Leo Li <leoyang.li@nxp.com>, Scott Wood <scott.wood@nxp.com>,
Shawn Guo <shawnguo@kernel.org>, Mingkai Hu <mingkai.hu@nxp.com>
Subject: Re: [PATCH 1/6] dt/bindings: adjust bindings for Layerscape SCFG MSI
Date: Wed, 26 Oct 2016 11:31:16 +0100 [thread overview]
Message-ID: <20161026103101.GC19965@leverpostej> (raw)
In-Reply-To: <1477398945-22774-1-git-send-email-Minghuan.Lian@nxp.com>
On Tue, Oct 25, 2016 at 08:35:40PM +0800, Minghuan Lian wrote:
> 1. The different version of a SoC may have different MSI
> implementation. But compatible "fsl,<soc-name>-msi" can not describe
> the SoC version.
Surely, "fsl,<soc-name>-<rev>-msi" can describe this?
If the hardware differs, it needs a new compatible string.
If there's some configuration value that varies across revisions (e.g.
number of slots), you can add a proeprty to describe that explciitly.
> The MSI driver will use SoC match interface to get
> SoC type and version instead of compatible string. So all MSI node
> can use the common compatible "fsl,ls-scfg-msi" and the original
> compatible is unnecessary.
>
> 2. Layerscape SoCs may have one or several MSI controllers.
> In order to increase MSI interrupt number of a PCIe, the patch
> moves all MSI node into the parent node "msi-controller". So a
> PCIe can request MSI from all the MSI controllers.
This is not necessary, and does not represent a real block of hardware.
So NAK for this approach.
The msi-parent property can contain a list of MSI controllers. See the
examples in
Documentation/devicetree/bindings/interrupt-controller/msi.txt.
Likewise, the msi-map property can map to a number of MSI controllers.
If the core code can only consider one at a time, then that's an issue
to be addressed in core code, not one to be bodged around in bindings.
>
> Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
> ---
> .../interrupt-controller/fsl,ls-scfg-msi.txt | 57 +++++++++++++++++++---
> 1 file changed, 49 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
> index 9e38949..29f95fd 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
> @@ -1,18 +1,28 @@
> * Freescale Layerscape SCFG PCIe MSI controller
>
> +Layerscape SoCs may have one or multiple MSI controllers.
> +Each MSI controller must be showed as a child node.
> +
> Required properties:
>
> -- compatible: should be "fsl,<soc-name>-msi" to identify
> - Layerscape PCIe MSI controller block such as:
> - "fsl,1s1021a-msi"
> - "fsl,1s1043a-msi"
> +- compatible: should be "fsl,ls-scfg-msi"
This breaks old DTBs, and throws away information which you describe
above as valuable. So another NAK for that.
> +- #address-cells: must be 2
> +- #size-cells: must be 2
> +- ranges: allows valid 1:1 translation between child's address space and
> + parent's address space
> - msi-controller: indicates that this is a PCIe MSI controller node
> +
> +Required child node:
> +A child node must exist to represent the MSI controller.
> +The following are properties specific to those nodes:
Also, as above, the approach of gathering MSI controllers in this manner
is wrong.
Thanks,
Mark.
next prev parent reply other threads:[~2016-10-26 10:31 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-25 12:35 [PATCH 1/6] dt/bindings: adjust bindings for Layerscape SCFG MSI Minghuan Lian
2016-10-25 12:35 ` Minghuan Lian
2016-10-25 12:35 ` Minghuan Lian
2016-10-25 12:35 ` [PATCH 2/6] arm: dts: ls1021a: update MSI node Minghuan Lian
2016-10-25 12:35 ` Minghuan Lian
2016-10-25 12:35 ` Minghuan Lian
2016-10-31 2:42 ` Rob Herring
2016-10-31 2:42 ` Rob Herring
2016-10-31 2:42 ` Rob Herring
2016-10-25 12:35 ` [PATCH 3/6] arm64: dts: ls1043a: update MSI and PCIe node Minghuan Lian
2016-10-25 12:35 ` Minghuan Lian
2016-10-25 12:35 ` Minghuan Lian
2016-10-26 10:33 ` Mark Rutland
2016-10-26 10:33 ` Mark Rutland
2016-10-25 12:35 ` [PATCH 4/6] arm64: dts: ls1046a: add MSI dts node Minghuan Lian
2016-10-25 12:35 ` Minghuan Lian
2016-10-25 12:35 ` Minghuan Lian
2016-10-25 12:35 ` [PATCH 5/6] arm64: dts: ls1043a: update gic " Minghuan Lian
2016-10-25 12:35 ` Minghuan Lian
2016-10-25 12:35 ` Minghuan Lian
2016-10-26 10:35 ` Mark Rutland
2016-10-26 10:35 ` Mark Rutland
2016-10-25 12:35 ` [PATCH 6/6] arm64: dts: ls1046a: add PCIe " Minghuan Lian
2016-10-25 12:35 ` Minghuan Lian
2016-10-25 12:35 ` Minghuan Lian
2016-10-25 13:01 ` [PATCH 1/6] dt/bindings: adjust bindings for Layerscape SCFG MSI Robin Murphy
2016-10-25 13:01 ` Robin Murphy
2016-10-26 6:55 ` M.H. Lian
2016-10-26 6:55 ` M.H. Lian
2016-10-26 6:55 ` M.H. Lian
2016-10-26 10:22 ` Mark Rutland
2016-10-26 10:22 ` Mark Rutland
2016-10-26 10:31 ` Mark Rutland [this message]
2016-10-26 10:31 ` Mark Rutland
2016-10-26 22:09 ` Leo Li
2016-10-26 22:09 ` Leo Li
2016-10-26 22:09 ` Leo Li
2016-10-27 14:18 ` Mark Rutland
2016-10-27 14:18 ` Mark Rutland
2016-10-27 14:18 ` Mark Rutland
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=20161026103101.GC19965@leverpostej \
--to=mark.rutland@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
/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 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.