From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753146AbcJ0Gme (ORCPT ); Thu, 27 Oct 2016 02:42:34 -0400 Received: from mail-db5eur01on0067.outbound.protection.outlook.com ([104.47.2.67]:61408 "EHLO EUR01-DB5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751836AbcJ0Gmb (ORCPT ); Thu, 27 Oct 2016 02:42:31 -0400 X-Greylist: delayed 7237 seconds by postgrey-1.27 at vger.kernel.org; Thu, 27 Oct 2016 02:42:30 EDT From: Leo Li To: Mark Rutland , "M.H. Lian" CC: "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "devicetree@vger.kernel.org" , Marc Zyngier , "Stuart Yoder" , Scott Wood , Shawn Guo , Mingkai Hu Subject: RE: [PATCH 1/6] dt/bindings: adjust bindings for Layerscape SCFG MSI Thread-Topic: [PATCH 1/6] dt/bindings: adjust bindings for Layerscape SCFG MSI Thread-Index: AQHSLrwGeftAD0bBxEOPo1Z6AL0Tg6C6irEAgADAVUA= Date: Wed, 26 Oct 2016 22:09:07 +0000 Message-ID: References: <1477398945-22774-1-git-send-email-Minghuan.Lian@nxp.com> <20161026103101.GC19965@leverpostej> In-Reply-To: <20161026103101.GC19965@leverpostej> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=leoyang.li@nxp.com; x-originating-ip: [192.88.168.1] x-ms-office365-filtering-correlation-id: 06a56528-acbc-4f36-d2d2-08d3fdecb4f0 x-microsoft-exchange-diagnostics: 1;HE1PR0401MB2634;7:OFQ10k9Tu3m+wLnCVIRWuiskBi5olhBdPmS5ib0QPMw5VxyA3WBy1FRJP9usi8q1yCDOxhB494wZm4u7UCZBXIfBxUg683Hqz6CQGyGygNILfDSrL1yaB84+IUJ9A0YZ/wQFwe9bLT7UwBTyETrw7WGCkIOqOEVYvedkvfS/9ejcT75xFGmJ19gVR75yVS1h4X8h0DBqdkF7TR0ajvdYacJHYymKlq+FRSxS70TydS1Nw1TCXVvOSnu0LbigtbM/ovZf2HchnlIVXJY0SZ2ocL4goKFWwpIpFjuakwZdZKCe5zYh/n+LP6CkpA8nKxU3zEaMm4w47c3n1Y0z57wCKZxBYA/5hxmGYdMPOoaUJAg= x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:HE1PR0401MB2634; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(180628864354917)(9452136761055)(185117386973197)(258649278758335); x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(6040176)(6045074)(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046)(6055026)(6046074)(6072074);SRVR:HE1PR0401MB2634;BCL:0;PCL:0;RULEID:;SRVR:HE1PR0401MB2634; x-forefront-prvs: 0107098B6C x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(6009001)(7916002)(13464003)(24454002)(377454003)(189002)(199003)(86362001)(74316002)(2900100001)(586003)(77096005)(97736004)(6116002)(5001770100001)(305945005)(68736007)(92566002)(3846002)(33656002)(102836003)(7736002)(7846002)(189998001)(6636002)(2950100002)(101416001)(54356999)(76176999)(8936002)(50986999)(2906002)(87936001)(4326007)(9686002)(106116001)(19580405001)(19580395003)(105586002)(7696004)(8676002)(66066001)(81166006)(106356001)(81156014)(3660700001)(10400500002)(76576001)(5002640100001)(5660300001)(3280700002)(122556002);DIR:OUT;SFP:1101;SCL:1;SRVR:HE1PR0401MB2634;H:DB4PR04MB0781.eurprd04.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Oct 2016 22:09:07.6330 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0401MB2634 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id u9R6gc2f005865 > -----Original Message----- > From: Mark Rutland [mailto:mark.rutland@arm.com] > Sent: Wednesday, October 26, 2016 5:31 AM > To: M.H. Lian > Cc: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; > devicetree@vger.kernel.org; Marc Zyngier ; Stuart > Yoder ; Leo Li ; Scott Wood > ; Shawn Guo ; Mingkai Hu > > Subject: Re: [PATCH 1/6] dt/bindings: adjust bindings for Layerscape SCFG MSI > > 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,-msi" can not describe > > the SoC version. > > Surely, "fsl,--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 > > --- > > .../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-m > > si.txt > > b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-m > > si.txt > > index 9e38949..29f95fd 100644 > > --- > > a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-m > > si.txt > > +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-sc > > +++ fg-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,-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. I agree with you that we should maintain the backward compatibility. But on the other hand, I just found that there is a silly typo in the original binding that "ls" is wrongly spelled as "1s" and they look too close to be noticed in previous patch reviews. :( The driver and all the DTSes used the binding with the typo which covered up the problem. So even if we want to keep the "fsl,-msi" binding, we probably want to fix the typo, right? And that breaks the backward compatibility too. Regards, Leo From mboxrd@z Thu Jan 1 00:00:00 1970 From: Leo Li Subject: RE: [PATCH 1/6] dt/bindings: adjust bindings for Layerscape SCFG MSI Date: Wed, 26 Oct 2016 22:09:07 +0000 Message-ID: References: <1477398945-22774-1-git-send-email-Minghuan.Lian@nxp.com> <20161026103101.GC19965@leverpostej> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20161026103101.GC19965@leverpostej> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Mark Rutland , "M.H. Lian" Cc: "devicetree@vger.kernel.org" , Marc Zyngier , Stuart Yoder , "linux-kernel@vger.kernel.org" , Scott Wood , Mingkai Hu , Shawn Guo , "linux-arm-kernel@lists.infradead.org" List-Id: devicetree@vger.kernel.org > -----Original Message----- > From: Mark Rutland [mailto:mark.rutland@arm.com] > Sent: Wednesday, October 26, 2016 5:31 AM > To: M.H. Lian > Cc: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; > devicetree@vger.kernel.org; Marc Zyngier ; Stuart > Yoder ; Leo Li ; Scott Wood > ; Shawn Guo ; Mingkai Hu > > Subject: Re: [PATCH 1/6] dt/bindings: adjust bindings for Layerscape SCFG MSI > > 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,-msi" can not describe > > the SoC version. > > Surely, "fsl,--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 > > --- > > .../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-m > > si.txt > > b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-m > > si.txt > > index 9e38949..29f95fd 100644 > > --- > > a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-m > > si.txt > > +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-sc > > +++ fg-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,-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. I agree with you that we should maintain the backward compatibility. But on the other hand, I just found that there is a silly typo in the original binding that "ls" is wrongly spelled as "1s" and they look too close to be noticed in previous patch reviews. :( The driver and all the DTSes used the binding with the typo which covered up the problem. So even if we want to keep the "fsl,-msi" binding, we probably want to fix the typo, right? And that breaks the backward compatibility too. Regards, Leo From mboxrd@z Thu Jan 1 00:00:00 1970 From: leoyang.li@nxp.com (Leo Li) Date: Wed, 26 Oct 2016 22:09:07 +0000 Subject: [PATCH 1/6] dt/bindings: adjust bindings for Layerscape SCFG MSI In-Reply-To: <20161026103101.GC19965@leverpostej> References: <1477398945-22774-1-git-send-email-Minghuan.Lian@nxp.com> <20161026103101.GC19965@leverpostej> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org > -----Original Message----- > From: Mark Rutland [mailto:mark.rutland at arm.com] > Sent: Wednesday, October 26, 2016 5:31 AM > To: M.H. Lian > Cc: linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org; > devicetree at vger.kernel.org; Marc Zyngier ; Stuart > Yoder ; Leo Li ; Scott Wood > ; Shawn Guo ; Mingkai Hu > > Subject: Re: [PATCH 1/6] dt/bindings: adjust bindings for Layerscape SCFG MSI > > 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,-msi" can not describe > > the SoC version. > > Surely, "fsl,--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 > > --- > > .../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-m > > si.txt > > b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-m > > si.txt > > index 9e38949..29f95fd 100644 > > --- > > a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-m > > si.txt > > +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-sc > > +++ fg-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,-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. I agree with you that we should maintain the backward compatibility. But on the other hand, I just found that there is a silly typo in the original binding that "ls" is wrongly spelled as "1s" and they look too close to be noticed in previous patch reviews. :( The driver and all the DTSes used the binding with the typo which covered up the problem. So even if we want to keep the "fsl,-msi" binding, we probably want to fix the typo, right? And that breaks the backward compatibility too. Regards, Leo