From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S940708AbcKQDnL (ORCPT ); Wed, 16 Nov 2016 22:43:11 -0500 Received: from mail-db5eur01on0079.outbound.protection.outlook.com ([104.47.2.79]:37381 "EHLO EUR01-DB5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932423AbcKQDnG (ORCPT ); Wed, 16 Nov 2016 22:43:06 -0500 From: Scott Wood To: Sriram Dash , "linux-kernel@vger.kernel.org" , "linux-usb@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" CC: "mark.rutland@arm.com" , "felipe.balbi@linux.intel.com" , "mathias.nyman@intel.com" , "catalin.marinas@arm.com" , "will.deacon@arm.com" , "kishon@ti.com" , "robh+dt@kernel.org" , "stern@rowland.harvard.edu" , Suresh Gupta , "gregkh@linuxfoundation.org" , "pku.leo@gmail.com" Subject: Re: [upstream-release] [PATCH 1/2] drivers: usb: phy: Add qoriq usb 3.0 phy driver support Thread-Topic: [upstream-release] [PATCH 1/2] drivers: usb: phy: Add qoriq usb 3.0 phy driver support Thread-Index: AQHSPjfIdBocuEaXMU2jHpnkfUNzKg== Date: Wed, 16 Nov 2016 21:07:57 +0000 Message-ID: References: <1479101215-26954-1-git-send-email-sriram.dash@nxp.com> <1479101215-26954-2-git-send-email-sriram.dash@nxp.com> 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=scott.wood@nxp.com; x-originating-ip: [2601:449:8400:923b:12bf:48ff:fe84:c9a0] x-microsoft-exchange-diagnostics: 1;DB5PR0401MB1925;7:2ELf2mDZRXVbfsQ1Q6TonUSNVxIcdKMk1AypUgRLHLKc/gsn+7387uFv682VOYWegcd1jbxULgZMciJ+UM4XEhwjr8kFy1E60+gUv+ovzQEKMZpgN7KG50rnulpa4FDRRodH1WTytrxN2XoIlCRDQPrDqxLODmhXuxsZpiv0ps9NT4dFZzqO6w9oohxXUgYVfs8Ynqt6eeLYBm/aG890PRBEiY2dm6fmnXgewFALDEmHteog1QWoFGRBqqsgueornsSc00Jk+PfL+T+N72/ZMheyC/6QXD8bAoj9xG9Oi77ZJaV3MJaDeOIePpLOHeXd0L70XFUubCHZ/cjOMUcUboI9FQfcmWC3cmojVOQc1c4= x-forefront-antispam-report: SFV:SKI;SCL:-1SFV:NSPM;SFS:(10009020)(6009001)(7916002)(43544003)(24454002)(189002)(377454003)(199003)(106116001)(106356001)(105586002)(86362001)(189998001)(97736004)(5001770100001)(2906002)(4326007)(92566002)(6116002)(102836003)(5660300001)(3280700002)(3660700001)(87936001)(76176999)(33656002)(2201001)(6506003)(7416002)(229853002)(7696004)(50986999)(76576001)(54356999)(93886004)(101416001)(74316002)(7846002)(122556002)(7736002)(8936002)(2501003)(81156014)(68736007)(81166006)(9686002)(305945005)(77096005)(2900100001);DIR:OUT;SFP:1101;SCL:1;SRVR:DB5PR0401MB1925;H:DB5PR0401MB1928.eurprd04.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; x-ms-office365-filtering-correlation-id: 36f127c1-526b-4dc9-9da3-08d40e64a3fa x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001);SRVR:DB5PR0401MB1925; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(6045074)(6060326)(6040281)(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046)(6055026)(6046074)(6061324)(6041223)(6047074);SRVR:DB5PR0401MB1925;BCL:0;PCL:0;RULEID:;SRVR:DB5PR0401MB1925; x-forefront-prvs: 01283822F8 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: 16 Nov 2016 21:07:57.5450 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB5PR0401MB1925 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 uAH3hG5X007574 On 11/16/2016 05:33 AM, Sriram Dash wrote: >> From: Scott Wood >> On 11/15/2016 06:39 AM, Sriram Dash wrote: >>>> From: Scott Wood >>>> On 11/13/2016 11:27 PM, Sriram Dash wrote: >>>>> diff --git >>>>> a/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt >>>>> b/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt >>>>> new file mode 100644 >>>>> index 0000000..d934c80 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt >>>>> @@ -0,0 +1,36 @@ >>>>> +Driver for Freescale USB 3.0 PHY >>>>> + >>>>> +Required properties: >>>>> + >>>>> +- compatible : fsl,qoriq-usb3-phy >>>> >>> >>> Hi Scott, >>> >>>> This is a very vague compatible. Are there versioning registers >>>> within this register block? >>>> >>> >>> There are versioning registers for the phy (1.0 and 1.1). But the >>> current erratum >>> A008751 does not require the mentioning of the version numbers. Was >>> planning to take care of the versioning when there is code diversity >>> on the basis of the version number. >> >> That is not how device tree bindings work. The describe the hardware, not the >> driver. >> >> That said, is the block version sufficient to tell whether a given chip has this >> erratum? If so, you don't need a special property for the erratum. If not, what is >> different about the PHY that is not described by the versioning? Can you find out the answer to this? >> In any case, it would be nice to mention the version register and its offset in the >> binding, just so that it becomes part of the definition of this compatible string, and >> if we come out with some QorIQ chip with a >> USB3 PHY that is totally different and doesn't have that version register, it'll be >> clear that it needs a different compatible. >> > > Okay. Will include version number in the next rev for Documentation and dt. I'm not asking you to put the version number in the dt if it can be read from a register. I'm asking you to have the binding describe the version register, as part of the definition of what "fsl,qoriq-usb3-phy" means. >>> The only reason for __raw_writel is to make the code faster. >> >> Does that really matter here? >> >>> However, shall I use writel(with both barriers and byte swap) instead >> >> Yes, if the registers are little-endian on all chips. >> > > The endianness is not same for all Socs. But for most Socs, it is big-endian. > Is "iowrite32be" better instead? Then the device tree node needs to say what endian it is, and you need to choose the appropriate accessor at runtime. But is it really big endian for most or even any SoCs? This hardware isn't present on our PPC chips, right (I checked a couple of the most recent PPC RMs and it shows USB 2.0 only)? So it'd just be the few ARM chips that made almost everything big endian, and even there the couple RMs I checked (ls1021a and ls1043a) have these registers described as little-endian. >>> and then make appropriate changes in the value 32'h27672B2A? >> >> Not sure what you mean here. >> >>> In my knowledge, there are more than 5 errata in pipeline, >> >> Then please get all of these errata described in the device tree ASAP (unless their >> presence can be reliably inferred from the block version, as discussed above). >> > > Yes. We will push the errata asap. My point is that the device tree node should be complete when you submit it. Don't wait for the implementation of a workaround to advertise the existence of an erratum in the device tree. -Scott From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [upstream-release] [PATCH 1/2] drivers: usb: phy: Add qoriq usb 3.0 phy driver support Date: Wed, 16 Nov 2016 21:07:57 +0000 Message-ID: References: <1479101215-26954-1-git-send-email-sriram.dash@nxp.com> <1479101215-26954-2-git-send-email-sriram.dash@nxp.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: 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: Sriram Dash , "linux-kernel@vger.kernel.org" , "linux-usb@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" Cc: "mark.rutland@arm.com" , "felipe.balbi@linux.intel.com" , "mathias.nyman@intel.com" , "catalin.marinas@arm.com" , "will.deacon@arm.com" , "kishon@ti.com" , "robh+dt@kernel.org" , "stern@rowland.harvard.edu" , Suresh Gupta , "gregkh@linuxfoundation.org" , "pku.leo@gmail.com" List-Id: devicetree@vger.kernel.org On 11/16/2016 05:33 AM, Sriram Dash wrote: >> From: Scott Wood >> On 11/15/2016 06:39 AM, Sriram Dash wrote: >>>> From: Scott Wood >>>> On 11/13/2016 11:27 PM, Sriram Dash wrote: >>>>> diff --git >>>>> a/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt >>>>> b/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt >>>>> new file mode 100644 >>>>> index 0000000..d934c80 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt >>>>> @@ -0,0 +1,36 @@ >>>>> +Driver for Freescale USB 3.0 PHY >>>>> + >>>>> +Required properties: >>>>> + >>>>> +- compatible : fsl,qoriq-usb3-phy >>>> >>> >>> Hi Scott, >>> >>>> This is a very vague compatible. Are there versioning registers >>>> within this register block? >>>> >>> >>> There are versioning registers for the phy (1.0 and 1.1). But the >>> current erratum >>> A008751 does not require the mentioning of the version numbers. Was >>> planning to take care of the versioning when there is code diversity >>> on the basis of the version number. >> >> That is not how device tree bindings work. The describe the hardware, not the >> driver. >> >> That said, is the block version sufficient to tell whether a given chip has this >> erratum? If so, you don't need a special property for the erratum. If not, what is >> different about the PHY that is not described by the versioning? Can you find out the answer to this? >> In any case, it would be nice to mention the version register and its offset in the >> binding, just so that it becomes part of the definition of this compatible string, and >> if we come out with some QorIQ chip with a >> USB3 PHY that is totally different and doesn't have that version register, it'll be >> clear that it needs a different compatible. >> > > Okay. Will include version number in the next rev for Documentation and dt. I'm not asking you to put the version number in the dt if it can be read from a register. I'm asking you to have the binding describe the version register, as part of the definition of what "fsl,qoriq-usb3-phy" means. >>> The only reason for __raw_writel is to make the code faster. >> >> Does that really matter here? >> >>> However, shall I use writel(with both barriers and byte swap) instead >> >> Yes, if the registers are little-endian on all chips. >> > > The endianness is not same for all Socs. But for most Socs, it is big-endian. > Is "iowrite32be" better instead? Then the device tree node needs to say what endian it is, and you need to choose the appropriate accessor at runtime. But is it really big endian for most or even any SoCs? This hardware isn't present on our PPC chips, right (I checked a couple of the most recent PPC RMs and it shows USB 2.0 only)? So it'd just be the few ARM chips that made almost everything big endian, and even there the couple RMs I checked (ls1021a and ls1043a) have these registers described as little-endian. >>> and then make appropriate changes in the value 32'h27672B2A? >> >> Not sure what you mean here. >> >>> In my knowledge, there are more than 5 errata in pipeline, >> >> Then please get all of these errata described in the device tree ASAP (unless their >> presence can be reliably inferred from the block version, as discussed above). >> > > Yes. We will push the errata asap. My point is that the device tree node should be complete when you submit it. Don't wait for the implementation of a workaround to advertise the existence of an erratum in the device tree. -Scott From mboxrd@z Thu Jan 1 00:00:00 1970 From: scott.wood@nxp.com (Scott Wood) Date: Wed, 16 Nov 2016 21:07:57 +0000 Subject: [upstream-release] [PATCH 1/2] drivers: usb: phy: Add qoriq usb 3.0 phy driver support References: <1479101215-26954-1-git-send-email-sriram.dash@nxp.com> <1479101215-26954-2-git-send-email-sriram.dash@nxp.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 11/16/2016 05:33 AM, Sriram Dash wrote: >> From: Scott Wood >> On 11/15/2016 06:39 AM, Sriram Dash wrote: >>>> From: Scott Wood >>>> On 11/13/2016 11:27 PM, Sriram Dash wrote: >>>>> diff --git >>>>> a/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt >>>>> b/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt >>>>> new file mode 100644 >>>>> index 0000000..d934c80 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt >>>>> @@ -0,0 +1,36 @@ >>>>> +Driver for Freescale USB 3.0 PHY >>>>> + >>>>> +Required properties: >>>>> + >>>>> +- compatible : fsl,qoriq-usb3-phy >>>> >>> >>> Hi Scott, >>> >>>> This is a very vague compatible. Are there versioning registers >>>> within this register block? >>>> >>> >>> There are versioning registers for the phy (1.0 and 1.1). But the >>> current erratum >>> A008751 does not require the mentioning of the version numbers. Was >>> planning to take care of the versioning when there is code diversity >>> on the basis of the version number. >> >> That is not how device tree bindings work. The describe the hardware, not the >> driver. >> >> That said, is the block version sufficient to tell whether a given chip has this >> erratum? If so, you don't need a special property for the erratum. If not, what is >> different about the PHY that is not described by the versioning? Can you find out the answer to this? >> In any case, it would be nice to mention the version register and its offset in the >> binding, just so that it becomes part of the definition of this compatible string, and >> if we come out with some QorIQ chip with a >> USB3 PHY that is totally different and doesn't have that version register, it'll be >> clear that it needs a different compatible. >> > > Okay. Will include version number in the next rev for Documentation and dt. I'm not asking you to put the version number in the dt if it can be read from a register. I'm asking you to have the binding describe the version register, as part of the definition of what "fsl,qoriq-usb3-phy" means. >>> The only reason for __raw_writel is to make the code faster. >> >> Does that really matter here? >> >>> However, shall I use writel(with both barriers and byte swap) instead >> >> Yes, if the registers are little-endian on all chips. >> > > The endianness is not same for all Socs. But for most Socs, it is big-endian. > Is "iowrite32be" better instead? Then the device tree node needs to say what endian it is, and you need to choose the appropriate accessor at runtime. But is it really big endian for most or even any SoCs? This hardware isn't present on our PPC chips, right (I checked a couple of the most recent PPC RMs and it shows USB 2.0 only)? So it'd just be the few ARM chips that made almost everything big endian, and even there the couple RMs I checked (ls1021a and ls1043a) have these registers described as little-endian. >>> and then make appropriate changes in the value 32'h27672B2A? >> >> Not sure what you mean here. >> >>> In my knowledge, there are more than 5 errata in pipeline, >> >> Then please get all of these errata described in the device tree ASAP (unless their >> presence can be reliably inferred from the block version, as discussed above). >> > > Yes. We will push the errata asap. My point is that the device tree node should be complete when you submit it. Don't wait for the implementation of a workaround to advertise the existence of an erratum in the device tree. -Scott