Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752307AbcKRKOI (ORCPT ); Fri, 18 Nov 2016 05:14:08 -0500 Received: from mail-by2nam03on0069.outbound.protection.outlook.com ([104.47.42.69]:51456 "EHLO NAM03-BY2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751586AbcKRKOG (ORCPT ); Fri, 18 Nov 2016 05:14:06 -0500 From: Rafal Ozieblo To: Harini Katakam CC: Nicolas Ferre , Andrei Pistirica , "harini.katakam@xilinx.com" , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: RE: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM Thread-Topic: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM Thread-Index: AQHSQM0mlMF9RYMdA0O6HcZ/ouUay6DdK3SAgAAAdXCAAPtGgIAASw+QgAADTYCAAAKqIIAABr6AgAAB/KA= Date: Fri, 18 Nov 2016 09:59:24 +0000 Message-ID: References: In-Reply-To: Accept-Language: pl-PL, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=rafalo@cadence.com; x-originating-ip: [213.131.238.28] x-microsoft-exchange-diagnostics: 1;BN3PR07MB2514;7:cQIhFTO+0wx07eHl6Ba76QUPM/Xau7E6NokuxnEUV6FruoVTHT7jgkizTkUfiCQNUCu6DPxStgVAKm0BD0notBxyh4HXUTSBNccqwbMcD3qaBKH3g9FbyXq+zWCTWhhKcNtcezU4PR1x/nj5X143uJGFpiMu/G0sVbM13d5S5XCqVKV54a5Oq8vbmFCJarl9fwMqx1thMYz83qBIEOFclaJjEUd1+zeU8R/MvNFbnkckoeQRdxyg/+v2buatzpAMu85p7Fh8XxyIiS1GvBO+s3kclxsjurttCcCeipTckbKFCIPVyLCIbo3gewKIpcUrrcU8BGiNSeDgPuFgvgYE0yJ3WgGH7QRVal/cCUChAhA=;20:XS+9VPOdLw1LIN5f0iA6GzSQQH4whaB/VR6Sx4jFdK+8J/O0yEI4zKy4oF2KwppYJ0wFeLwvz1xNTF3ffmSkqRQnkXZZ79hY5HjxpiA3f9ZvCqSUplbt8bNfNgx9AVLnPguIJT9PYYDchhe9uM18RoNlL6mavTvNH3q2vXWc6HUnkrvo7S9mODkUWIPGFWMg/9gvrwk5kZ1v6bAC2NPL1GKbhAtrFtDHMfDm8reJXZFUVn1P0gDgkK0h78Mzy6a2 x-ms-office365-filtering-correlation-id: 77726284-474d-449d-3c51-08d40f999358 x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001);SRVR:BN3PR07MB2514; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(10436049006162)(9452136761055)(192813158149592)(72806322054110); x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(6060326)(6040281)(601004)(2401047)(5005006)(8121501046)(10201501046)(3002001)(6041223)(6061324);SRVR:BN3PR07MB2514;BCL:0;PCL:0;RULEID:;SRVR:BN3PR07MB2514; x-forefront-prvs: 01304918F3 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(6009001)(7916002)(377454003)(199003)(13464003)(36092001)(24454002)(189002)(50986999)(122556002)(4326007)(2906002)(575784001)(86362001)(229853002)(33656002)(3660700001)(1411001)(6506003)(54356999)(76176999)(3280700002)(38730400001)(9686002)(8936002)(81156014)(68736007)(66066001)(3846002)(101416001)(6116002)(102836003)(81166006)(93886004)(87936001)(8666005)(110136003)(7846002)(6916009)(74316002)(7736002)(97736004)(305945005)(5660300001)(7696004)(8676002)(92566002)(2900100001)(189998001)(77096005)(76576001)(105586002)(106356001)(106116001)(99286002)(2950100002)(7059030)(217873001);DIR:OUT;SFP:1101;SCL:1;SRVR:BN3PR07MB2514;H:BN3PR07MB2516.namprd07.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 X-OriginatorOrg: cadence.com X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Nov 2016 09:59:24.0736 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: d36035c5-6ce6-4662-a3dc-e762e61ae4c9 X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN3PR07MB2514 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 base64 to 8bit by mail.home.local id uAIAEE9x017157 Content-Length: 15875 Lines: 178 > >-----Original Message----- >From: Harini Katakam [mailto:harinikatakamlinux@gmail.com] >Sent: 18 listopada 2016 10:44 >To: Rafal Ozieblo >Cc: Nicolas Ferre; Andrei Pistirica; harini.katakam@xilinx.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org >Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM > >Hi Rafal, > >On Fri, Nov 18, 2016 at 3:00 PM, Rafal Ozieblo wrote: >>>-----Original Message----- >>>From: Nicolas Ferre [mailto:nicolas.ferre@atmel.com] >>>Sent: 18 listopada 2016 10:10 >>>To: Rafal Ozieblo; Harini Katakam; Andrei Pistirica >>>Cc: harini.katakam@xilinx.com; netdev@vger.kernel.org; >>>linux-kernel@vger.kernel.org >>>Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support >>>for GEM >>> >>>Le 18/11/2016 à 09:59, Rafal Ozieblo a écrit : >>>> Hello, >>>> >>>>> From: Harini Katakam [mailto:harinikatakamlinux@gmail.com] >>>>> Sent: 18 listopada 2016 05:30 >>>>> To: Rafal Ozieblo >>>>> Cc: Nicolas Ferre; harini.katakam@xilinx.com; >>>>> netdev@vger.kernel.org; linux-kernel@vger.kernel.org >>>>> Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing >>>>> support for GEM >>>>> >>>>> Hi Rafal, >>>>> >>>>> On Thu, Nov 17, 2016 at 7:05 PM, Rafal Ozieblo wrote: >>>>>> -----Original Message----- >>>>>> From: Nicolas Ferre [mailto:nicolas.ferre@atmel.com] >>>>>> Sent: 17 listopada 2016 14:29 >>>>>> To: Harini Katakam; Rafal Ozieblo >>>>>> Cc: harini.katakam@xilinx.com; netdev@vger.kernel.org; >>>>>> linux-kernel@vger.kernel.org >>>>>> Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing >>>>>> support for GEM >>>>>> >>>>>>> Le 17/11/2016 à 13:21, Harini Katakam a écrit : >>>>>>>> Hi Rafal, >>>>>>>> >>>>>>>> On Thu, Nov 17, 2016 at 5:20 PM, Rafal Ozieblo wrote: >>>>>>>>> Hello, >>>>>>>>> I think, there could a bug in your patch. >>>>>>>>> >>>>>>>>>> + >>>>>>>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT >>>>>>>>>> + dmacfg |= GEM_BIT(ADDR64); #endif >>>>>>>>> >>>>>>>>> You enable 64 bit addressing (64b dma bus width) always when appropriate architecture config option is enabled. >>>>>>>>> But there are some legacy controllers which do not support that feature. According Cadence hardware team: >>>>>>>>> "64 bit addressing was added in July 2013. Earlier version do not have it. >>>>>>>>> This feature was enhanced in release August 2014 to have separate upper address values for transmit and receive." >>>>>>>>> >>>>>>>>>> /* Bitfields in NSR */ >>>>>>>>>> @@ -474,6 +479,10 @@ >>>>>>>>>> struct macb_dma_desc { >>>>>>>>> > u32 addr; >>>>>>>>>> u32 ctrl; >>>>>>>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT >>>>>>>>>> + u32 addrh; >>>>>>>>>> + u32 resvd; >>>>>>>>>> +#endif >>>>>>>>>> }; >>>>>>>>> >>>>>>>>> It will not work for legacy hardware. Old descriptor is 2 words wide, the new one is 4 words wide. >>>>>>>>> If you enable CONFIG_ARCH_DMA_ADDR_T_64BIT but hardware doesn't >>>>>>>>> support it at all, you will miss every second descriptor. >>>>>>>>> >>>>>>>> >>>>>>>> True, this feature is not available in all of Cadence IP versions. >>>>>>>> In fact, the IP version Zynq does not support this. But the one in ZynqMP does. >>>>>>>> So, we enable kernel config for 64 bit DMA addressing for this >>>>>>>> SoC and hence the driver picks it up. My assumption was that if >>>>>>>> the legacy IP does not support >>>>>>>> 64 bit addressing, then this DMA option wouldn't be enabled. >>>>>>>> >>>>>>>> There is a design config register in Cadence IP which is being >>>>>>>> read to check for 64 bit address support - DMA mask is set based on that. >>>>>>>> But the addition of two descriptor words cannot be based on this runtime check. >>>>>>>> For this reason, all the static changes were placed under this check. >>>>>>> >>>>>>> We have quite a bunch of options in this driver to determinate what is the real capacity of the underlying hardware. >>>>>>> If HW configuration registers are not appropriate, and it seems they are not, I would advice to simply use the DT compatibility string. >>>>>>> >>>>>>> Best regards, >>>>>>> -- >>>>>>> Nicolas Ferre >>>>>> >>>>>> HW configuration registers are appropriate. The issue is that this code doesn’t use the capability bit to switch between different dma descriptors (2 words vs. 4 words). >>>>>> DMA descriptor size is chosen based on kernel configuration, not based on hardware capabilities. >>>>> >>>>> HW configuration register does give appropriate information. >>>>> But addition of two address words in the macb descriptor structure is a static change. >>>>> >>>>> +static inline void macb_set_addr(struct macb_dma_desc *desc, >>>>> +dma_addr_t >>>>> +addr) { >>>>> + desc->addr = (u32)addr; >>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT >>>>> + desc->addrh = (u32)(addr >> 32); #endif >>>>> + >>>>> >>>>> Even if the #ifdef condition here is changed to HW config check, addr and addrh are different. >>>>> And "addrh" entry has to be present for 64 bit desc case to be handled separately. >>>>> Can you please tell me how you propose change in DMA descriptor >>>>> structure from >>>>> 4 to 2 or 2 to 4 words *after* reading the DCFG register? >>>> >>>> It is very complex problem. I wrote to you because I faced the same issue. I'm working on PTP implementation for macb. When PTP is enabled there are additional two words in buffer descriptor. >>> >>>Moreover, we can use PTP without the 64bits descriptor support (early GEM revisions). >>> >>>BTW, note that there is an initiative ongoing with Andrei to support >>>PTP/1588 on these devices with patches already send: please synchronize with him. >>>https://urldefense.proofpoint.com/v2/url?u=https-3A__marc.info_-3Fl-3D >>>linux-2Dnetdev-26m-3D147282092309112-26w-3D3&d=DgIDaQ&c=aUq983L2pue2Fq >>>KFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtC >>>g4L5E&m=jUmZzqwn1uRrvcqF3BHCHFC6ZsKoZkU3vT8gJ-7fnsE&s=kr_km0MKQBzpkaOt >>>0fkM-FIajN1-pOylzzTjsXi-vak&e= >>>or >>>https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel. >>>org_patch_9310989_&d=DgIDaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_ha >>>XqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtCg4L5E&m=jUmZzqwn1uRrvcqF3B >>>HCHFC6ZsKoZkU3vT8gJ-7fnsE&s=ZbXVD5Lb5zGn7wUKIPYHxagIEKp_vJvrnkRa4qJfmT >>>Y&e= >>>and >>>https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel. >>>org_patch_9310991_&d=DgIDaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_ha >>>XqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtCg4L5E&m=jUmZzqwn1uRrvcqF3B >>>HCHFC6ZsKoZkU3vT8gJ-7fnsE&s=Z0kRqUqh5Is4TmuaY7A4hnpizfdeq3HrgPhyDAMLuA >>>8&e= >>> >>>Regards, >>>-- >>>Nicolas Ferre >> >> Above patch doesn’t use hardware timestamps in descriptors at all. It doesn't use appropriate accuracy. >> We have our PTP patch ready, which use timestamps from descriptor. But we have not sent it yet because of compatibility issue. >> Of course, we can do it like Harini did: >> >> struct macb_dma_desc { >> u32 addr; >> u32 ctrl; >> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT >> u32 addrh; >> u32 resvd; >> #endif >> #if IS_ENABLED(CONFIG_PTP_1588_CLOCK) >> u32 dma_desc_ts_1; >> u32 dma_desc_ts_2; >> #endif >> }; >> >> But in that approach we lose backward compatibility. >> >> What are your suggestion? Can we send patch like it is or wait for some common solution with backward compatibility? >> > >Yes, Andre's version of Cadence does not ability to report have RX timestamp. >The version I worked with did. This is the old series I sent: >https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2015_9_11_92&d=DgIFaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtCg4L5E&m=1A1qxdlwY3kMlJ1JUJJL1EqLzWz4AKtfOxZf_vu8bWo&s=NGpEbbC4bYXUZjJ6n0Ud8F8p3fPz5EhTJ1O9-NKwCkQ&e= >But right now, i'm working on building on top of Andre's changes. > >Regards, >Harini > I can’t see a place where you enable extended descriptor for PTP. Did you add support for extended PTP descriptor? "DMA Configuration Register" 0x010: 29 tx_bd_extended_mode_en "Enable TX extended BD mode. See TX BD control register definition for description of feature." 28 rx_bd_extended_mode_en "Enable RX extended BD mode. See RX BD control register definition for description of feature." Can I send you our patch for comparison ?