Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756415AbdLVPxX convert rfc822-to-8bit (ORCPT ); Fri, 22 Dec 2017 10:53:23 -0500 Received: from mail.free-electrons.com ([62.4.15.54]:55402 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751966AbdLVPxU (ORCPT ); Fri, 22 Dec 2017 10:53:20 -0500 Date: Fri, 22 Dec 2017 16:53:08 +0100 From: Miquel RAYNAL To: Chris Packham Cc: ezequiel.garcia@free-electrons.com, boris.brezillon@free-electrons.com, richard@nod.at, dwmw2@infradead.org, computersforpeace@gmail.com, marek.vasut@gmail.com, cyrille.pitchen@wedev4u.fr, linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, Kalyan Kinthada Subject: Re: [PATCHv4 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants Message-ID: <20171222165308.10f2fe15@xps13> In-Reply-To: <20171221231904.21140-2-chris.packham@alliedtelesis.co.nz> References: <20171221231904.21140-1-chris.packham@alliedtelesis.co.nz> <20171221231904.21140-2-chris.packham@alliedtelesis.co.nz> Organization: Free Electrons X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4017 Lines: 104 Hello Chris, On Fri, 22 Dec 2017 12:19:04 +1300 Chris Packham wrote: > From: Kalyan Kinthada > > The Armada-370 based SoCs support arbitration between the NAND Flash > interface and NOR (i.e. devbus) on the same chip select. However there > are two guidelines that must be followed to avoid data corruption in > this scenario. Sorry to bother you again with that but, do you actually face any issue/data corruption with this scenario? This bit is not set by the bootloaders I use and I see no difference when I set it. But by experience on this controller: any change is dangerous and may break users, that is why we are so careful about it. Thank you, Miquèl > > First GL-6843509 which states that only "Don't Care CS" NAND devices > are supported. Secondly GL-5830741 which says that when the > arbitration between NOR and NAND flash is enabled set the FORCE_CSX > bit to 1. This prevents the chip select from being active when the > data flash interface is relinquished to the other slave. > > The NDCR_ND_MODE conflicts with the new NDCR_FORCE_CSX but is unused > so remove it along with NDCR_NAND_MODE. > > Signed-off-by: Kalyan Kinthada > [cp: only apply when arbiter is enabled, reword commit message] > Signed-off-by: Chris Packham > --- > Changes in v4: > - only act when the nand arbiter is enabled > - update commit message with information from Marvell > > Changes in v3: > - "mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants." > Modified commit message to mention that this change does not apply > for pxa3xx variants. > Fixed the missing space in comments. > Removed unused macros "NDCR_ND_MODE" and "NDCR_NAND_MODE". > > Changes in v2: > - Deleted: "dt-bindings: mtd: pxa3xx: Add "marvell,nand-force-csx" > compatible string" Not necessary to create a new compatible string. > - "mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants." > Modified commit message. > This commit sets the FORCE_CSX bit for all ARMADA370 variants. > > drivers/mtd/nand/pxa3xx_nand.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/nand/pxa3xx_nand.c > b/drivers/mtd/nand/pxa3xx_nand.c index 90b9a9ccbe60..1b2ae98311d2 > 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c > +++ b/drivers/mtd/nand/pxa3xx_nand.c > @@ -73,8 +73,7 @@ > #define NDCR_DWIDTH_M (0x1 << 26) > #define NDCR_PAGE_SZ (0x1 << 24) > #define NDCR_NCSX (0x1 << 23) > -#define NDCR_ND_MODE (0x3 << 21) > -#define NDCR_NAND_MODE (0x0) > +#define NDCR_FORCE_CSX (0x1 << 21) > #define NDCR_CLR_PG_CNT (0x1 << 20) > #define NFCV1_NDCR_ARB_CNTL (0x1 << 19) > #define NFCV2_NDCR_STOP_ON_UNCOR (0x1 << 19) > @@ -1477,6 +1476,10 @@ static int pxa3xx_nand_config_ident(struct > pxa3xx_nand_info *info) info->chunk_size = PAGE_CHUNK_SIZE; > info->reg_ndcr = 0x0; /* enable all interrupts */ > info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : > 0; > + /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#: > GL-5830741 */ > + if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370 && > + pdata->enable_arbiter) > + info->reg_ndcr |= NDCR_FORCE_CSX; > info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES); > info->reg_ndcr |= NDCR_SPARE_EN; > > @@ -1511,6 +1514,10 @@ static void pxa3xx_nand_detect_config(struct > pxa3xx_nand_info *info) info->reg_ndcr = ndcr & > ~(NDCR_INT_MASK | NDCR_ND_ARB_EN | > NFCV1_NDCR_ARB_CNTL); info->reg_ndcr |= (pdata->enable_arbiter) ? > NDCR_ND_ARB_EN : 0; > + /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#: > GL-5830741 */ > + if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370 && > + pdata->enable_arbiter) > + info->reg_ndcr |= NDCR_FORCE_CSX; > info->ndtr0cs0 = nand_readl(info, NDTR0CS0); > info->ndtr1cs0 = nand_readl(info, NDTR1CS0); > } -- Miquel Raynal, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com