From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757781AbcK3JSH (ORCPT ); Wed, 30 Nov 2016 04:18:07 -0500 Received: from mail-dm3nam03on0051.outbound.protection.outlook.com ([104.47.41.51]:6848 "EHLO NAM03-DM3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1757709AbcK3JRY (ORCPT ); Wed, 30 Nov 2016 04:17:24 -0500 Authentication-Results: spf=pass (sender IP is 149.199.60.83) smtp.mailfrom=xilinx.com; vger.kernel.org; dkim=none (message not signed) header.d=none;vger.kernel.org; dmarc=bestguesspass action=none header.from=xilinx.com; X-IncomingTopHeaderMarker: OriginalChecksum:;UpperCasedChecksum:;SizeAsReceived:2611;Count:27 From: Naga Sureshkumar Relli To: Cyrille Pitchen , "broonie@kernel.org" , "michal.simek@xilinx.com" , Soren Brinkmann , Harini Katakam , Punnaiah Choudary Kalluri CC: "linux-spi@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "linux-mtd@lists.infradead.org" Subject: RE: [LINUX RFC v4 3/4] mtd: spi-nor: add stripe support Thread-Topic: [LINUX RFC v4 3/4] mtd: spi-nor: add stripe support Thread-Index: AQHSSIkeNvVdd1JJQUqN/2aam+6s+aDvv18AgAF68rA= Date: Wed, 30 Nov 2016 09:17:14 +0000 Message-ID: References: <1480235616-34038-1-git-send-email-nagasure@xilinx.com> <68ca0f19-e534-3361-11f0-6566626380fe@atmel.com> In-Reply-To: <68ca0f19-e534-3361-11f0-6566626380fe@atmel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [172.23.18.72] Content-Type: text/plain; charset="iso-8859-1" MIME-Version: 1.0 X-RCIS-Action: ALLOW X-TM-AS-Product-Ver: IMSS-7.1.0.1224-8.0.0.1202-22732.000 X-TM-AS-User-Approved-Sender: Yes;Yes X-IncomingHeaderCount: 27 X-EOPAttributedMessage: 0 X-MS-Office365-Filtering-HT: Tenant X-Forefront-Antispam-Report: CIP:149.199.60.83;IPV:NLI;CTRY:US;EFV:NLI;SFV:NSPM;SFS:(10009020)(6009001)(7916002)(2980300002)(438002)(199003)(52314003)(189002)(63266004)(575784001)(2900100001)(2920100001)(2201001)(6636002)(626004)(5660300001)(229853002)(54356999)(33656002)(50466002)(50986999)(76176999)(356003)(7736002)(2950100002)(305945005)(92566002)(7846002)(38730400001)(6862003)(23756003)(2501003)(81166006)(189998001)(106466001)(81156014)(4326007)(5250100002)(8676002)(3846002)(6116002)(102836003)(5001770100001)(7696004)(8936002)(55846006)(47776003)(106116001)(39410400001)(2906002)(39450400002)(8746002)(107986001)(217873001);DIR:OUT;SFP:1101;SCL:1;SRVR:BN3PR02MB1110;H:xsj-pvapsmtpgw01;FPR:;SPF:Pass;PTR:unknown-60-83.xilinx.com;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;BL2NAM02FT022;1:+28HwLZs9oMVJ4uZTnmX0whlLKW3ii+ditHDUI+1Uo55AYUQLCpdAOyDsKZW2feflg5ymLFej7ZV3nVg0FSn4ho4akZO/J14UNn2iyWU7hAd7TC4fUnc0X4jp64R/S49hw+43KwcYWc9AX/yZxOh3PVPuDZXahTawmeYMeaty84hWWBE6fB9tbY0YW7QSwu0txkZc/+8Ola8PgvCPDtwlzpPCGa4KOAWKaZPig+oWFUjdjODfmwLp3rKjZLX/w/Kol+laU6N6IF/ylFMme9QOHLxfxbTBsPfAiDyck4LMwSFCzVBlNeSw2iPEyoK6gK7EN6dE4cHSRjkTu7b2R2SNpRBS4r5ETWThNf1lrOVevvVXfNXAg0BQ0qgHECs/8TgZutii3Wi/1J3yrvCFSTEfnQl5w3Lk+h94TDRjwrBx/iBEgfPIcF9rSESGMhbCQDhyqA8brz27mjgRbmUouIsjuHcGv5Ly4bIECLHgNVd8EKs6wa2sia8LKANJTOPwKMAH4iZxVYvq9e4BM89PitZkcnq7OOuPJ1MgA49UGxL6hC5TMCfSVGPw+TMqlLLQlSodEFtqAl8eu8L8F+eeuBar9mZEQk7hH/+jLF+waL48Kw= X-MS-Office365-Filtering-Correlation-Id: 5efb8714-9f7f-4ab2-9f0a-08d41901af02 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(8251501002);SRVR:BN3PR02MB1110; X-Microsoft-Exchange-Diagnostics: 1;BN3PR02MB1110;3:f8qHHZ+goYzlyKBWUDqmVAlewMnoS2+WJdOE/9M/Z7ySf1DxhCs52c4hHHeojBoPHeImHJu9R25Lb5U4+K8nSGHMRV3O8WDKFLzotZU9RauFtuHCeeTsZZeNMjMNdiAKWu0I3JV1mez6u7KfNZBGxYDENL0h2IqpOYMrqFpuswCbv3tEyBUSuj3wTQAGN5PoGeY4IdVuYc8ykkBcKjWwE2FKfRbjFtETaD4e06JxvEQiQy10BWtEmNEdwlcnyhvw93HtBYb7N0kuDUWTRpXnxrBnHHqj0DgRAsX2ocKQeBorl2Gd0ez7eZIO+yfzUAkMsDEp3Vtu9A1zVa5jDrqqBSUOMHfje+/JL8YkXEP3kn2oqXihCNEfJKLGAEmOpspf1e3lAPC+DogunJdJpxDerA==;25:0rC0Bcu67MAdCarsEJ+91UutUB6XEFOoFPf8kr5jl01IDczIMzZgq8bCpR3l9H5SHptaTU6fTzZKK6NhljSAsl2O5Cabn8PyMvYpboG2Xt+WgfsSqsb06YHx/3cxRFFltoi69V7DxUot+ShHiVKrCQKXHyMS+glYCsm42WEQnnJT7JnXwbueBd5cay62CTZs90iSom/BFug9sEMmrweSMimQ08g8GNgHLc+wAvf2vAellSAmKGpmSJyOrWc40Gkp3+CI43ZyhB3FNXryD3Yuwl9uwip2G1NoaIbqzRubFGdTD12hwu2/jYDTVo9jDpCRgZzxag354+QenH7bMhTzL2ToaQK4/WFMLG72KQNduO1xAtk4yIr084g/3W/YkPUaC/9ysZCYxxbsltgCNOi6zcoe2L2kbdKSIeVBUC1qjBPAgjakkh23F7WQtlIHU3DiE1jpriJ9IwGW4GpOOAozNw== X-Microsoft-Exchange-Diagnostics: 1;BN3PR02MB1110;31:TddPEYI/FxHMmk5yHaqBUwEJWAHOI4puBeaSfAN0QtoNazd6KtzmE250uBiwN+ypOFRElfIQZ4ErYgzWBbkvL+d5wHx4Gb03VXNUQg3QembVeOyXBO5F9fx3n0UswA7Mjuk/XCPTEU8JIzieY8B7PXgXK74ozXqbK3wugs7FZknxu1p0epOGyTSiUyEC+JTA2ZjjGerYWm1hwuoRGbPNidJBFuUXd4TyZltFb6Kq8EdrDDz7yfnQzHpkTO2cI0mce8gAKC21g14Bzr5ob0pglmVDeuE554zxRYKo/gG0/hU=;20:rEcCV2cqzllq8ThgjT//xqUle8lqC7xXKM5foq44Is3llQyW5luq5kTTVv0EzxBtUTJCS5TqvCgn74YOWACFL64qotX7+Qq5nlAG5sdCylPWqyB9wIqsmLQuMlFrvC3QzhAPIGRG7SdOc+FeIzyWnSRKol1395b8hZBHoaObcruu7ifVrilmKEbbJlVsFbIAp/4IbQ5IYnLa5M6SroE9RodpuAorbLmx8FMstASPflGAdqwqU7tjte8rN139PzT25d5bRBHRoU/BSBB/49tSZCCk/gEESfZ+nA4oNB+uco0kdaEx7QIiL59bE3nqFqPrQ5ZGAWPXSfd5deRMygJ+mYT7wAl2SsWYEWLFRL5TfjvFbrcQqCOXQVpfOGMsa2lI+cC0mmSaVAbxlaXhMTAGfB6oifETfD8gsVTc0qXlaFbP66r9VtRe3jaFVdQTjVtjIqea/wjpZZoucU9PXn/DDKOHx10ZGaCW/Vab0XQKrWxxFkzgtDtlXeC+X92oVUCO X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(192813158149592); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040375)(601004)(2401047)(13024025)(13023025)(13018025)(13017025)(5005006)(13015025)(8121501046)(3002001)(10201501046)(6055026)(6041248)(20161123555025)(20161123560025)(20161123562025)(20161123564025)(6072148);SRVR:BN3PR02MB1110;BCL:0;PCL:0;RULEID:;SRVR:BN3PR02MB1110; X-Microsoft-Exchange-Diagnostics: 1;BN3PR02MB1110;4:oWwiASMlGnOFGGBA1agUOX02VqQGs6dVZneEDDrVsQNGCHolfBmFOqHHFnIoCfCpD4vjbO8R/wLsRB4zsV9lakZA/TGeknlu1304tRLeAoJxLuUcXDc37OB8rs9P5kzxFnVLyEKGX0dFOxDLSeAhZxdHhLf0jNWwI2AtamdPVEDOuu57Z2DqUMmvHXkVDTe2EFNupX4Qaeg5TbjHvkXwHQMo+3KZzSJeleka5UKbA7T1Fis3mTaDYUbfRlW1WfLnBXhfskOewmXk8DB8aMLRfU2hgxUlcZ08fq+NCjzgw7Vh6NuW1FIv++OAuGok453JhatTl5pqlbwWHCr1qya/rBEpmXkSwEkpcgfltIbsQ66RrhsH0am9ZHZ0D3Co7llzrGkWrZko3xWMfdP7chDKAIfhy04pXLcenr/aP7d8uAC/ZNw59vtuRPeRhgHDbUkGUcVjkIuN72ycwCwEGYYK+AvToTm3njFiVURiKAWfalQsZxmpmOh9T/r7e/H+HKvDwOFoeDCSZtLlugn48o6jbVQPRmmxsXQf2ah5yNcl26g6hg/uJeP3tcM49sSrY2SrtdlayBvqh9tLw+L9XMqnUaZl/EuK4xE2Zgh0eMmC91/vjpwX3e/7bh2E057E8N775e5GHt4iYpKaAy3TF0VmVp0oOhBak9DQXe2K4stXW2c/khVIMIwcq4g/AZzX/aJiuLuK5Um8VFWbbHLH9L+IR4Q2UwO72fWh1mdLw1S8EXI= X-Forefront-PRVS: 0142F22657 X-Microsoft-Exchange-Diagnostics: =?iso-8859-1?Q?1;BN3PR02MB1110;23:XovMxGGGWPpLn55z46jdcuBE8SdtQyWTqERHc+L?= =?iso-8859-1?Q?OZF1c1rXnFTSLDJ+dU2+chtCfHc0sKLBz+K415sNZDy6nRz/Xf1hjxgKfo?= =?iso-8859-1?Q?SZoby/r9ogkBlojFaK+XVpHZ8czhoaJZVaDDPocBmCQzQN37VzNS6GqP/e?= =?iso-8859-1?Q?ZdXAMj36aaLyqaNnGWvVyW7lkrxVwQwpupgTf+HsGK/fmmIAVAVhspsSuk?= =?iso-8859-1?Q?G0HGChCvrKbZWQqthncm0szl+/g1NcPEHs4jqsxdp0eHaQIsk3tlk98BJy?= =?iso-8859-1?Q?3tDfb3ByokY+odR4Obr49RtJrxBpun+DOV42Ju2m4xGsznREEmty8agmZV?= =?iso-8859-1?Q?XeeHtyCZ8AU9nUMGmNyI1mYFGVtzld2cQbX86VvTlvBECVp7e5zpgPCXTM?= =?iso-8859-1?Q?j1RWWSIfpdUztKrawFPf0Xa/v6BF/g/BdiO459cVj6En6FYkBEAO4qq8an?= =?iso-8859-1?Q?HbHsS7RxYsy3IgWYjWaG/pUn9lrSHuYOt4C76GwCIuthyom6CJSBPDqgJ2?= =?iso-8859-1?Q?gYBT4AxRfUV5UKFGZf085BRmqN5nVSA7WLD2RBR33AMfPENx1y/kx9XZSD?= =?iso-8859-1?Q?fsdZftpPXHkCKVWY0Wd33SGWrbZdgAhUzIdH0Us+JPGCw3QEKhak141sne?= =?iso-8859-1?Q?FVjHvJ1P8hXEUUwXyAxe3YPNtzuHNmP6quOh3AHBffWZXHyAkFGCBSP/aB?= =?iso-8859-1?Q?d4CfOfudH+dFXth37lMYb2p9yn1yZftdQjliI/vxaulrC1TdoTVrYQDhh8?= =?iso-8859-1?Q?EaDHp2sON426sq8fFFpaYDocrV641T/A10bqiwphtpZsekV22EEuIlVaOB?= =?iso-8859-1?Q?7azLFBcwZ5nFZHshVedB/H0G41EKJ/MzDBTmiFiBKeDj/qRPJ0JX7Ap8Hc?= =?iso-8859-1?Q?m9FMkjBy4L0CIyi1xiooEQpzE3iMtFuEQZjpoQTQX+aKm92dzOHBg4I0Y8?= =?iso-8859-1?Q?WMXVZxn6B+hhI+9fxZeSEMoSFdwyDC6IBYQ66i0iuy31iUWK03xeJ1ffR7?= =?iso-8859-1?Q?+k1NQFDUKAok1UJ4qT14noRF4sRGJAhlG8xMlIflWsY0Z1bWP2mJk8C+Lm?= =?iso-8859-1?Q?upFKMGuG0M9H6dhVr+EWPXyXE3q3aSEL8BQexNSsmITVCwcdFMO4r0QXPQ?= =?iso-8859-1?Q?6cu7jdAYSVyCSp4o0evOa/ZBd2VbPKLgHQIDPh5etSJWlgzSdsVn/7rsxc?= =?iso-8859-1?Q?hY0b1UkdNMEK4QvUNitoQEeH2HHK9kGXM1PB1vSsQXVeGfFwCwDu0uqZk+?= =?iso-8859-1?Q?TghBRHDf9VnaICsqLfZwiCO8WUb9vYBmA35mjGD4WMAghtYyE+7lh/rVSL?= =?iso-8859-1?Q?iyG2QTCxsR7NLpQNLNLRxO2?= X-Microsoft-Exchange-Diagnostics: 1;BN3PR02MB1110;6:h4ZhZapTYySrHdJCYexWubGwx849fx7FrVu4NdwH5IDi/j2QHIpJ8EY6ILbAFP6uPn2RwcAJct3iqFUnq+uQ+FVR/wq649MoJCfqFkXCdbSFQpKUthMLJrqOOFNNK792HwO/2qmTpar0YXfH3AoMfT+++1oboJ+62Jk9HeOe1YPYKkXW0ZDlBkv9ERYdQSV7LIKSwoHAtHCQlVceMfFi4jOI4d8EENoP94UErMEX1/6JENs2KrkWYgjq2iHDG7NKESKUTzT3JSs/Pr8eeNlvL7hIruPXnOgeEuikDDzqXHUOymw4jQLPdxq3Y8mOu00wQnBHrPWozhf1qxrt98/bqPv1ST6TwPYUKcT3xEIJv/8ztdmsQ3B0rzKr8PmFq1xUfYWH2TzbLkVw367kcAb53mgZ8wBWNsTqheWc8Qbzghm4qUkiM/fjb/lqDT+frNMEyMoi+GZOExk3evLWpwlogxYcwNkFq2fYtgKHyhWfTfbQo3rPq/OSLzQkovX3K4Cb;5:vyLLye6YlvAXy/wfS//CbkUSH3hmaqhgWXH4QPyR3NppfueBI1TTcBn+GCuePgg4Mo7t6RK5m6kaEcQKYJRWn8NaMXfe94DS5DqELX3HbLbg3xRP1kbtUty7iG7ghoAkF5/R1B2mVffowHqUvLRykg==;24:rjHs+9STwgj77eqcjKYduaZhSwwqis1LxNzTc3qOnXK4z+xdIau7geJxS2sjvbhPgtCKjZgXfdC2FHHZvDR3MqUGv6dwdn4kGJPvLn87oRU= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;BN3PR02MB1110;7:WWWU/2WpkuPHb7RKBiFS5sCJkxDsfSojTH9XAzcWVtv9v8XUfeWNRw2jx7LA3KqUNLA4gnAVVBPgyGLv/d4o6NniADhMuPhsChzhqp6CIJU7dJ018OBJKKpzXHn7x/AmjK4FTH31i+ww06ouop9cYzCz1iWqwjuqfGiJmMESVENZ9a1mMvWWK6dOB+6fo9LCIZVVRzXi9sq9ncza21SB0Srm+99B8dfwkipqNeBJh/U2Q9gIBNUTqEJuJSd9Pe9M/aFkkdZCtJGk5IDLuuGVnj3jgG87Mnq/4P3rHzjQj/XrImG4Zs2ZpG9ibM9qHcXLKmbPyVOZWDmvmVXSh5/nzOF6uWj0OOQ4WTjMmgonfvBuukpKy7W/v5LIul+WpXhkuLUhOg+g5xR0obr2drRueYx7p/YWVzDiH+3CYX2S08LML9HLy3OeR1rxRPQLCo3HENoxUGO/b+ERd2mGlX3ylQ== X-OriginatorOrg: xilinx.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Nov 2016 09:17:18.4335 (UTC) X-MS-Exchange-CrossTenant-Id: 657af505-d5df-48d0-8300-c31994686c5c X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=657af505-d5df-48d0-8300-c31994686c5c;Ip=[149.199.60.83];Helo=[xsj-pvapsmtpgw01] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN3PR02MB1110 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 uAU9IDtq025116 Hi Cyrille, > I have not finished to review the whole series yet but here some first > comments: Thanks for reviewing these patch series. > > Le 27/11/2016 à 09:33, Naga Sureshkumar Relli a écrit : > > This patch adds stripe support and it is needed for GQSPI parallel > > configuration mode by: > > > > - Adding required parameters like stripe and shift to spi_nor > > structure. > > - Initializing all added parameters in spi_nor_scan() > > - Updating read_sr() and read_fsr() for getting status from both > > flashes > > - Increasing page_size, sector_size, erase_size and toatal flash > > size as and when required. > > - Dividing address by 2 > > - Updating spi->master->flags for qspi driver to change CS > > > > Signed-off-by: Naga Sureshkumar Relli > > --- > > Changes for v4: > > - rename isparallel to stripe > > Changes for v3: > > - No change > > Changes for v2: > > - Splitted to separate MTD layer changes from SPI core changes > > --- > > drivers/mtd/spi-nor/spi-nor.c | 130 > ++++++++++++++++++++++++++++++++---------- > > include/linux/mtd/spi-nor.h | 2 + > > 2 files changed, 103 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c > > b/drivers/mtd/spi-nor/spi-nor.c index d0fc165..4252239 100644 > > --- a/drivers/mtd/spi-nor/spi-nor.c > > +++ b/drivers/mtd/spi-nor/spi-nor.c > > @@ -22,6 +22,7 @@ > > #include > > #include > > #include > > +#include > > > > /* Define max times to check status register before we give up. */ > > > > @@ -89,15 +90,24 @@ static const struct flash_info > > *spi_nor_match_id(const char *name); static int read_sr(struct > > spi_nor *nor) { > > int ret; > > - u8 val; > > + u8 val[2]; > > > > - ret = nor->read_reg(nor, SPINOR_OP_RDSR, &val, 1); > > - if (ret < 0) { > > - pr_err("error %d reading SR\n", (int) ret); > > - return ret; > > + if (nor->stripe) { > > + ret = nor->read_reg(nor, SPINOR_OP_RDSR, &val[0], 2); > > + if (ret < 0) { > > + pr_err("error %d reading SR\n", (int) ret); > > + return ret; > > + } > > + val[0] |= val[1]; > Why '|' rather than '&' ? > I guess because of the 'Write In Progress/Busy' bit: when called by > spi_nor_sr_ready(), you want to be sure that this 'BUSY' bit is cleared on > both memories. > > But what about when the Status Register is read for purpose other than > checking the state of the 'BUSY' bit? > Yes you are correct, I will change this. > What about SPI controllers supporting more than 2 memories in parallel? > > This solution might fit the ZynqMP controller but doesn't look so generic. > > > + } else { > > + ret = nor->read_reg(nor, SPINOR_OP_RDSR, &val[0], 1); > > + if (ret < 0) { > > + pr_err("error %d reading SR\n", (int) ret); > > + return ret; > > + } > > } > > > > - return val; > > + return val[0]; > > } > > > > /* > > @@ -108,15 +118,24 @@ static int read_sr(struct spi_nor *nor) static > > int read_fsr(struct spi_nor *nor) { > > int ret; > > - u8 val; > > + u8 val[2]; > > > > - ret = nor->read_reg(nor, SPINOR_OP_RDFSR, &val, 1); > > - if (ret < 0) { > > - pr_err("error %d reading FSR\n", ret); > > - return ret; > > + if (nor->stripe) { > > + ret = nor->read_reg(nor, SPINOR_OP_RDFSR, &val[0], 2); > > + if (ret < 0) { > > + pr_err("error %d reading FSR\n", ret); > > + return ret; > > + } > > + val[0] &= val[1]; > Same comment here: why '&' rather than '|'? > Surely because of the the 'READY' bit which should be set for both memories. I will update this also. > > > + } else { > > + ret = nor->read_reg(nor, SPINOR_OP_RDFSR, &val[0], 1); > > + if (ret < 0) { > > + pr_err("error %d reading FSR\n", ret); > > + return ret; > > + } > > } > > > > - return val; > > + return val[0]; > > } > > > > /* > > @@ -290,9 +309,16 @@ static int spi_nor_wait_till_ready(struct spi_nor > *nor) > > */ > > static int erase_chip(struct spi_nor *nor) { > > + u32 ret; > > + > > dev_dbg(nor->dev, " %lldKiB\n", (long long)(nor->mtd.size >> 10)); > > > > - return nor->write_reg(nor, SPINOR_OP_CHIP_ERASE, NULL, 0); > > + ret = nor->write_reg(nor, SPINOR_OP_CHIP_ERASE, NULL, 0); > > + if (ret) > > + return ret; > > + > > + return ret; > > + > > if (ret) > return ret; > else > return ret; > > This chunk should be removed, it doesn't ease the patch review ;) Ok, I will remove. > > > } > > > > static int spi_nor_lock_and_prep(struct spi_nor *nor, enum > > spi_nor_ops ops) @@ -349,7 +375,7 @@ static int > > spi_nor_erase_sector(struct spi_nor *nor, u32 addr) static int > > spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) { > > struct spi_nor *nor = mtd_to_spi_nor(mtd); > > - u32 addr, len; > > + u32 addr, len, offset; > > uint32_t rem; > > int ret; > > > > @@ -399,9 +425,13 @@ static int spi_nor_erase(struct mtd_info *mtd, > struct erase_info *instr) > > /* "sector"-at-a-time erase */ > > } else { > > while (len) { > > + > > write_enable(nor); > > + offset = addr; > > + if (nor->stripe) > > + offset /= 2; > > I guess this should be /= 4 for controllers supporting 4 memories in parallel. > Shouldn't you use nor->shift and define shift as an unsigned int instead of a > bool? > offset >>= nor->shift; > Yes we can use this shift, I will update > Anyway, by tuning the address here in spi-nor.c rather than in the SPI > controller driver, you impose a model to support parallel memories that > might not be suited to other controllers. For this ZynqMP GQSPI controller parallel configuration, globally spi-nor should know about this stripe feature And based on that address has to change. As I mentioned in cover letter, this controller in parallel configuration will work with even addresses only. i.e. Before creating address format(m25p_addr2cmd) in mtd layer, spi-nor should change that address based on stripe option. I am updating this offset based on stripe option, and stripe option will update by reading dt property in nor_scan(). So the controller which doesn't support, then the stripe will be zero. Or Can you please suggest any other way? > > > > > - ret = spi_nor_erase_sector(nor, addr); > > + ret = spi_nor_erase_sector(nor, offset); > > if (ret) > > goto erase_err; > > > > @@ -525,6 +555,8 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, > uint64_t len) > > bool use_top; > > int ret; > > > > + ofs = ofs >> nor->shift; > > + > > status_old = read_sr(nor); > > if (status_old < 0) > > return status_old; > > @@ -610,6 +642,8 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, > uint64_t len) > > bool use_top; > > int ret; > > > > + ofs = ofs >> nor->shift; > > + > > status_old = read_sr(nor); > > if (status_old < 0) > > return status_old; > > @@ -709,6 +743,8 @@ static int spi_nor_lock(struct mtd_info *mtd, loff_t > ofs, uint64_t len) > > if (ret) > > return ret; > > > > + ofs = ofs >> nor->shift; > > + > > ret = nor->flash_lock(nor, ofs, len); > > > > spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_UNLOCK); @@ - > 724,6 +760,8 > > @@ static int spi_nor_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t > len) > > if (ret) > > return ret; > > > > + ofs = ofs >> nor->shift; > > + > > ret = nor->flash_unlock(nor, ofs, len); > > > > spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_LOCK); @@ - > 1018,6 +1056,9 > > @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor) > > u8 id[SPI_NOR_MAX_ID_LEN]; > > const struct flash_info *info; > > > > + nor->spi->master->flags &= ~(SPI_MASTER_BOTH_CS | > > + SPI_MASTER_DATA_STRIPE); > > + > > tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, > SPI_NOR_MAX_ID_LEN); > > if (tmp < 0) { > > dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp); > @@ -1041,6 > > +1082,7 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, > > size_t len, { > > struct spi_nor *nor = mtd_to_spi_nor(mtd); > > int ret; > > + u32 offset = from; > > > > dev_dbg(nor->dev, "from 0x%08x, len %zd\n", (u32)from, len); > > > > @@ -1049,7 +1091,13 @@ static int spi_nor_read(struct mtd_info *mtd, > loff_t from, size_t len, > > return ret; > > > > while (len) { > > - ret = nor->read(nor, from, len, buf); > > + > > + offset = from; > > + > > + if (nor->stripe) > > + offset /= 2; > > + > > + ret = nor->read(nor, offset, len, buf); > > if (ret == 0) { > > /* We shouldn't see 0-length reads */ > > ret = -EIO; > > @@ -1161,6 +1209,7 @@ static int spi_nor_write(struct mtd_info *mtd, > loff_t to, size_t len, > > struct spi_nor *nor = mtd_to_spi_nor(mtd); > > size_t page_offset, page_remain, i; > > ssize_t ret; > > + u32 offset; > > > > dev_dbg(nor->dev, "to 0x%08x, len %zd\n", (u32)to, len); > > > > @@ -1178,9 +1227,13 @@ static int spi_nor_write(struct mtd_info *mtd, > loff_t to, size_t len, > > /* the size of data remaining on the first page */ > > page_remain = min_t(size_t, > > nor->page_size - page_offset, len - i); > > + offset = (to + i); > > + > > + if (nor->stripe) > > + offset /= 2; > > > > write_enable(nor); > > - ret = nor->write(nor, to + i, page_remain, buf + i); > > + ret = nor->write(nor, (offset), page_remain, buf + i); > > if (ret < 0) > > goto write_err; > > written = ret; > > @@ -1302,22 +1355,22 @@ static int spi_nor_check(struct spi_nor *nor) > > > > int spi_nor_scan(struct spi_nor *nor, const char *name, enum > > read_mode mode) { > > - const struct flash_info *info = NULL; > > + struct flash_info *info = NULL; > > You should not remove the const and should not try to modify members of > *info. > > > struct device *dev = nor->dev; > > struct mtd_info *mtd = &nor->mtd; > > struct device_node *np = spi_nor_get_flash_node(nor); > > - int ret; > > - int i; > > + struct device_node *np_spi; > > + int ret, i, xlnx_qspi_mode; > > > > ret = spi_nor_check(nor); > > if (ret) > > return ret; > > > > if (name) > > - info = spi_nor_match_id(name); > > + info = (struct flash_info *)spi_nor_match_id(name); > > /* Try to auto-detect if chip name wasn't specified or not found */ > > if (!info) > > - info = spi_nor_read_id(nor); > > + info = (struct flash_info *)spi_nor_read_id(nor); > > if (IS_ERR_OR_NULL(info)) > > return -ENOENT; > > > Both spi_nor_match_id() and spi_nor_read_id(), when they succeed, return > a pointer to an entry of the spi_nor_ids[] array, which is located in a read- > only memory area. > > Since your patch doesn't remove the const attribute of the spi_nor_ids[], I > wonder whether it has been tested. I expect it not to work on most > architecture. > > Anyway spi_nor_ids[] should remain const. Let's take the case of eXecution > In Place (XIP) from an external memory: if spi_nor_ids[] is const, it can be > read directly from this external (read-only) memory and we never need to > copy the array in RAM, so we save some KB of RAM. > This is just an example but I guess we can find other reasons to keep this > array const. > > > @@ -1341,7 +1394,7 @@ int spi_nor_scan(struct spi_nor *nor, const char > *name, enum read_mode mode) > > */ > > dev_warn(dev, "found %s, expected %s\n", > > jinfo->name, info->name); > > - info = jinfo; > > + info = (struct flash_info *)jinfo; > > } > > } > > > > @@ -1370,6 +1423,27 @@ int spi_nor_scan(struct spi_nor *nor, const char > *name, enum read_mode mode) > > mtd->size = info->sector_size * info->n_sectors; > > mtd->_erase = spi_nor_erase; > > mtd->_read = spi_nor_read; > > +#ifdef CONFIG_OF > > + np_spi = of_get_next_parent(np); > > + > > + if (of_property_read_u32(np_spi, "xlnx,qspi-mode", > > + &xlnx_qspi_mode) < 0) { > This really looks controller specific so should not be placed in the generic spi- > nor.c file. Const is removed in info struct, because to update info members based parallel configuration. As I mentioned above, for this parallel configuration, mtd and spi-nor should know the details like mtd->size, info->sectors, sector_size and page_size. So during spi_nor_scan only mtd will update all these details, that's why I have added controller specific check to update those. Can you please suggest, any other way to let mtd and spi-nor to know about this configuration without touching the core layers? Please let me know, if I missed providing any required info. > > > + nor->shift = 0; > > + nor->stripe = 0; > > + } else if (xlnx_qspi_mode == 2) { > > + nor->shift = 1; > > + info->sector_size <<= nor->shift; > > + info->page_size <<= nor->shift; > Just don't modify *info members! :) > > > > + mtd->size <<= nor->shift; > > + nor->stripe = 1; > > + nor->spi->master->flags |= (SPI_MASTER_BOTH_CS | > > + SPI_MASTER_DATA_STRIPE); > > + } > > +#else > > + /* Default to single */ > > + nor->shift = 0; > > + nor->stripe = 0; > > +#endif > Best regards, > > Cyrille Thanks, Naga Sureshkumar Relli