RE: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris & Miquel, > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Saturday, September 22, 2018 1:43 PM > To: Miquel Raynal > Cc: Naga Sureshkumar Relli ; rich...@nod.at; > dw...@infradead.org; > computersforpe...@gmail.com; marek.va...@gmail.com; kyungmin.p...@samsung.com; > abs...@codeaurora.org; peterpand...@micron.com; frieder.schre...@exceet.de; > linux- > m...@lists.infradead.org; linux-kernel@vger.kernel.org; Michal Simek > ; > nagasureshkumarre...@gmail.com > Subject: Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for > Arasan NAND > Flash Controller > > On Sat, 22 Sep 2018 09:53:40 +0200 > Miquel Raynal wrote: > > > Hi Naga, > > > > Naga Sureshkumar Relli wrote on Tue, 11 Sep 2018 > > 05:23:09 +: > > > > > Hi Boris, > > > > > > > -Original Message- > > > > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > > > > Sent: Monday, August 20, 2018 10:10 PM > > > > To: Naga Sureshkumar Relli > > > > Cc: miquel.ray...@bootlin.com; rich...@nod.at; > > > > dw...@infradead.org; computersforpe...@gmail.com; > > > > marek.va...@gmail.com; kyungmin.p...@samsung.com; > > > > abs...@codeaurora.org; peterpand...@micron.com; > > > > frieder.schre...@exceet.de; linux- m...@lists.infradead.org; > > > > linux-kernel@vger.kernel.org; Michal Simek ; > > > > nagasureshkumarre...@gmail.com > > > > Subject: Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add > > > > support for Arasan NAND Flash Controller > > > > > > > > Hi Naga, > > > > > > > > On Fri, 17 Aug 2018 18:49:24 +0530 Naga Sureshkumar Relli > > > > wrote: > > > > > > > > > > > > I haven't finished reviewing the driver but there are still a > > > > bunch of things that look strange, for instance, your > > > > ->read/write_page() implementation looks suspicious. Let's discuss that > > > > before you send a > new version. > > Sorry, I've been too busy to finish my review :-/. Please submit the new > version as Miquel > suggested. Ok, I will do. Thanks, Naga Sureshkumar Relli > > > > Could you please review the remaining stuff? > > > I have the changes ready with me which will address all your comments > > > given to this series. > > > > > > Thanks, > > > Naga Sureshkumar Relli > > > > Please submit the new version that is ready, the rest of the review > > will come. > > > > > > Thanks, > > Miquèl
Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
On Sat, 22 Sep 2018 09:53:40 +0200 Miquel Raynal wrote: > Hi Naga, > > Naga Sureshkumar Relli wrote on Tue, 11 Sep 2018 > 05:23:09 +: > > > Hi Boris, > > > > > -Original Message- > > > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > > > Sent: Monday, August 20, 2018 10:10 PM > > > To: Naga Sureshkumar Relli > > > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org; > > > computersforpe...@gmail.com; marek.va...@gmail.com; > > > kyungmin.p...@samsung.com; > > > abs...@codeaurora.org; peterpand...@micron.com; > > > frieder.schre...@exceet.de; linux- > > > m...@lists.infradead.org; linux-kernel@vger.kernel.org; Michal Simek > > > ; > > > nagasureshkumarre...@gmail.com > > > Subject: Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for > > > Arasan NAND > > > Flash Controller > > > > > > Hi Naga, > > > > > > On Fri, 17 Aug 2018 18:49:24 +0530 > > > Naga Sureshkumar Relli wrote: > > > > > > > > > I haven't finished reviewing the driver but there are still a bunch of > > > things that look strange, for > > > instance, your ->read/write_page() implementation looks suspicious. Let's > > > discuss that before > > > you send a new version. Sorry, I've been too busy to finish my review :-/. Please submit the new version as Miquel suggested. > > Could you please review the remaining stuff? > > I have the changes ready with me which will address all your comments given > > to this series. > > > > Thanks, > > Naga Sureshkumar Relli > > Please submit the new version that is ready, the rest of the review will > come. > > > Thanks, > Miquèl
Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga, Naga Sureshkumar Relli wrote on Tue, 11 Sep 2018 05:23:09 +: > Hi Boris, > > > -Original Message- > > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > > Sent: Monday, August 20, 2018 10:10 PM > > To: Naga Sureshkumar Relli > > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org; > > computersforpe...@gmail.com; marek.va...@gmail.com; > > kyungmin.p...@samsung.com; > > abs...@codeaurora.org; peterpand...@micron.com; frieder.schre...@exceet.de; > > linux- > > m...@lists.infradead.org; linux-kernel@vger.kernel.org; Michal Simek > > ; > > nagasureshkumarre...@gmail.com > > Subject: Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for > > Arasan NAND > > Flash Controller > > > > Hi Naga, > > > > On Fri, 17 Aug 2018 18:49:24 +0530 > > Naga Sureshkumar Relli wrote: > > > > > > I haven't finished reviewing the driver but there are still a bunch of > > things that look strange, for > > instance, your ->read/write_page() implementation looks suspicious. Let's > > discuss that before > > you send a new version. > Could you please review the remaining stuff? > I have the changes ready with me which will address all your comments given > to this series. > > Thanks, > Naga Sureshkumar Relli Please submit the new version that is ready, the rest of the review will come. Thanks, Miquèl
RE: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris, > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Monday, August 20, 2018 10:10 PM > To: Naga Sureshkumar Relli > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org; > computersforpe...@gmail.com; marek.va...@gmail.com; kyungmin.p...@samsung.com; > abs...@codeaurora.org; peterpand...@micron.com; frieder.schre...@exceet.de; > linux- > m...@lists.infradead.org; linux-kernel@vger.kernel.org; Michal Simek > ; > nagasureshkumarre...@gmail.com > Subject: Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for > Arasan NAND > Flash Controller > > Hi Naga, > > On Fri, 17 Aug 2018 18:49:24 +0530 > Naga Sureshkumar Relli wrote: > > > I haven't finished reviewing the driver but there are still a bunch of things > that look strange, for > instance, your ->read/write_page() implementation looks suspicious. Let's > discuss that before > you send a new version. Could you please review the remaining stuff? I have the changes ready with me which will address all your comments given to this series. Thanks, Naga Sureshkumar Relli
Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga, > > And why is it an s32 and not a u32? > To monitor NAND_CMD_STATUS. > Sometimes core will just send status command without reading back the status > data and later > It will try to read one byte using ->exec_op(). > So Arasan has FLASH_STS register and whenever we initiate a status command, > the controller > Will update this register with the value returned by the flash device. > So we need to return this value when core is asking about 1 byte status value > without issuing the command. > And in driver we are using memset(nfc_op, 0, sizeof(struct anfc_op)), this > will make cmnds[4] to zeros but 0x0 is also > NAND_CMD_READ0, so inorder to differentiate whether to give status data or > not, I just assigned > nfc_op->cmnds[0] = NAND_CMD_NONE; > > May be this case we can now eliminate as per your suggestion(defining a > separate hook for each pattern) and thanks for that. > > > > > + u32 type; > > > + u32 len; > > > + u32 naddrs; > > > + u32 col; > > > + u32 row; > > > + unsigned int data_instr_idx; > > > + unsigned int rdy_timeout_ms; > > > + unsigned int rdy_delay_ns; > > > + const struct nand_op_instr *data_instr; }; > > > > Please make sure you actually need to redefine all these fields. Looks like > > some them could be > > extracted directly from the nand_op_instr objects. > Ok, all these values are getting updated in anfc_parse_instructions() In anfc_parse_instructions(): + nfc_op->data_instr = instr; + nfc_op->type = NAND_OP_DATA_IN_INSTR; This looks pointless. + nfc_op->data_instr_idx = op_id; + break; + case NAND_OP_DATA_OUT_INSTR: + nfc_op->data_instr = instr; + nfc_op->type = NAND_OP_DATA_IN_INSTR; + nfc_op->data_instr_idx = op_id; + break; + case NAND_OP_WAITRDY_INSTR: + nfc_op->rdy_timeout_ms = instr->ctx.waitrdy.timeout_ms; This one also. + nfc_op->rdy_delay_ns = instr->delay_ns; And this one too. Once you'll have a per pattern callback, I suppose you won't need it anymore. > > > > > + > > > +/** > > > + * struct anfc_nand_chip - Defines the nand chip related information > > > + * @node:used to store NAND chips into a list. > > > + * @chip:NAND chip information structure. > > > + * @bch: Bch / Hamming mode enable/disable. > > > + * @bchmode: Bch mode. > > > > What's the difference between bch and bchmode? > @bch -> to select error correction method(either BCH or Hamming) > @bchmode -> to select ECC correctability (4/8/12/24 bit ECC) Then something like "strength" or "ecc_strength" would be more meaningful. Thanks, Miquèl
RE: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris, Thanks for the review. > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Monday, August 20, 2018 10:10 PM > To: Naga Sureshkumar Relli > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org; > computersforpe...@gmail.com; marek.va...@gmail.com; kyungmin.p...@samsung.com; > abs...@codeaurora.org; peterpand...@micron.com; frieder.schre...@exceet.de; > linux- > m...@lists.infradead.org; linux-kernel@vger.kernel.org; Michal Simek > ; > nagasureshkumarre...@gmail.com > Subject: Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for > Arasan > NAND Flash Controller > > Hi Naga, > > On Fri, 17 Aug 2018 18:49:24 +0530 > Naga Sureshkumar Relli wrote: > > > > > +config MTD_NAND_ARASAN > > + tristate "Support for Arasan Nand Flash controller" > > + depends on HAS_IOMEM > > + depends on HAS_DMA > > Just nitpicking, but you can place them on the same line: Ok, I will update it next version. > > depends on HAS_IOMEM && HAS_DMA > > > + help > > + Enables the driver for the Arasan Nand Flash controller on > > + Zynq Ultrascale+ MPSoC. > > + > > endif # MTD_NAND > > diff --git a/drivers/mtd/nand/raw/Makefile > > b/drivers/mtd/nand/raw/Makefile index d5a5f98..ccb8d56 100644 > > --- a/drivers/mtd/nand/raw/Makefile > > +++ b/drivers/mtd/nand/raw/Makefile > > @@ -57,6 +57,7 @@ obj-$(CONFIG_MTD_NAND_BRCMNAND) += > brcmnand/ > > obj-$(CONFIG_MTD_NAND_QCOM)+= qcom_nandc.o > > obj-$(CONFIG_MTD_NAND_MTK) += mtk_ecc.o mtk_nand.o > > obj-$(CONFIG_MTD_NAND_TEGRA) += tegra_nand.o > > +obj-$(CONFIG_MTD_NAND_ARASAN) += arasan_nand.o > > > > nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o > > nand-objs += nand_amd.o diff --git > > a/drivers/mtd/nand/raw/arasan_nand.c > > b/drivers/mtd/nand/raw/arasan_nand.c > > new file mode 100644 > > index 000..e4f1f80 > > --- /dev/null > > +++ b/drivers/mtd/nand/raw/arasan_nand.c > > @@ -0,0 +1,1350 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Arasan NAND Flash Controller Driver > > + * > > + * Copyright (C) 2014 - 2017 Xilinx, Inc. > > + * Author: Punnaiah Choudary Kalluri > > + * Author: Naga Sureshkumar Relli > > + * > > + */ > > +#include > > +#include > > +#include > > +#include > > +#include #include > > +#include #include #include > > + #include #include > > + #include > > Blank line missing here. Ok, I will update it next version. > > > +#define DRIVER_NAME"arasan_nand" > > Please define drop this definition and pass the string directly to > driver->name. The driver is for a nand controller, so make it clear, > also, it's just a detail but I prefer '-' to '_', so, how about > "arasan-nand-controller"? Yes, it looks good. I will update it next version. > > > +#define EVNT_TIMEOUT_MSEC 1000 > > It's unusual to have EVNT, it's usually EVT or EVENT. Ok, I will update it in next version. > > > +#define STATUS_TIMEOUT 2000 > > Is it in milliseconds? Please add the proper _ prefix here. Yes, it is in milliseconds. Ok I will add. > > > + > > +#define PKT_OFST 0x00 > > +#define MEM_ADDR1_OFST 0x04 > > +#define MEM_ADDR2_OFST 0x08 > > +#define CMD_OFST 0x0C > > +#define PROG_OFST 0x10 > > +#define INTR_STS_EN_OFST 0x14 > > +#define INTR_SIG_EN_OFST 0x18 > > +#define INTR_STS_OFST 0x1C > > +#define READY_STS_OFST 0x20 > > +#define DMA_ADDR1_OFST 0x24 > > +#define FLASH_STS_OFST 0x28 > > +#define DATA_PORT_OFST 0x30 > > +#define ECC_OFST 0x34 > > +#define ECC_ERR_CNT_OFST 0x38 > > +#define ECC_SPR_CMD_OFST 0x3C > > +#define ECC_ERR_CNT_1BIT_OFST 0x40 > > +#define ECC_ERR_CNT_2BIT_OFST 0x44 > > +#define DMA_ADDR0_OFST 0x50 > > +#define DATA_INTERFACE_OFST0x6C > > + > > +#define PKT_CNT_SHIFT 12 > > + > > +#define ECC_ENABLE BIT(31) > > +#define DMA_EN_MASKGENMASK(27, 26) > > +#define DMA_ENABLE 0x
Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga, On Fri, 17 Aug 2018 18:49:24 +0530 Naga Sureshkumar Relli wrote: > > +config MTD_NAND_ARASAN > + tristate "Support for Arasan Nand Flash controller" > + depends on HAS_IOMEM > + depends on HAS_DMA Just nitpicking, but you can place them on the same line: depends on HAS_IOMEM && HAS_DMA > + help > + Enables the driver for the Arasan Nand Flash controller on > + Zynq Ultrascale+ MPSoC. > + > endif # MTD_NAND > diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile > index d5a5f98..ccb8d56 100644 > --- a/drivers/mtd/nand/raw/Makefile > +++ b/drivers/mtd/nand/raw/Makefile > @@ -57,6 +57,7 @@ obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmnand/ > obj-$(CONFIG_MTD_NAND_QCOM) += qcom_nandc.o > obj-$(CONFIG_MTD_NAND_MTK) += mtk_ecc.o mtk_nand.o > obj-$(CONFIG_MTD_NAND_TEGRA) += tegra_nand.o > +obj-$(CONFIG_MTD_NAND_ARASAN)+= arasan_nand.o > > nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o > nand-objs += nand_amd.o > diff --git a/drivers/mtd/nand/raw/arasan_nand.c > b/drivers/mtd/nand/raw/arasan_nand.c > new file mode 100644 > index 000..e4f1f80 > --- /dev/null > +++ b/drivers/mtd/nand/raw/arasan_nand.c > @@ -0,0 +1,1350 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Arasan NAND Flash Controller Driver > + * > + * Copyright (C) 2014 - 2017 Xilinx, Inc. > + * Author: Punnaiah Choudary Kalluri > + * Author: Naga Sureshkumar Relli > + * > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include Blank line missing here. > +#define DRIVER_NAME "arasan_nand" Please define drop this definition and pass the string directly to driver->name. The driver is for a nand controller, so make it clear, also, it's just a detail but I prefer '-' to '_', so, how about "arasan-nand-controller"? > +#define EVNT_TIMEOUT_MSEC1000 It's unusual to have EVNT, it's usually EVT or EVENT. > +#define STATUS_TIMEOUT 2000 Is it in milliseconds? Please add the proper _ prefix here. > + > +#define PKT_OFST 0x00 > +#define MEM_ADDR1_OFST 0x04 > +#define MEM_ADDR2_OFST 0x08 > +#define CMD_OFST 0x0C > +#define PROG_OFST0x10 > +#define INTR_STS_EN_OFST 0x14 > +#define INTR_SIG_EN_OFST 0x18 > +#define INTR_STS_OFST0x1C > +#define READY_STS_OFST 0x20 > +#define DMA_ADDR1_OFST 0x24 > +#define FLASH_STS_OFST 0x28 > +#define DATA_PORT_OFST 0x30 > +#define ECC_OFST 0x34 > +#define ECC_ERR_CNT_OFST 0x38 > +#define ECC_SPR_CMD_OFST 0x3C > +#define ECC_ERR_CNT_1BIT_OFST0x40 > +#define ECC_ERR_CNT_2BIT_OFST0x44 > +#define DMA_ADDR0_OFST 0x50 > +#define DATA_INTERFACE_OFST 0x6C > + > +#define PKT_CNT_SHIFT12 > + > +#define ECC_ENABLE BIT(31) > +#define DMA_EN_MASK GENMASK(27, 26) > +#define DMA_ENABLE 0x2 > +#define DMA_EN_SHIFT 26 > +#define REG_PAGE_SIZE_SHIFT 23 > +#define REG_PAGE_SIZE_5120 > +#define REG_PAGE_SIZE_1K 5 > +#define REG_PAGE_SIZE_2K 1 > +#define REG_PAGE_SIZE_4K 2 > +#define REG_PAGE_SIZE_8K 3 > +#define REG_PAGE_SIZE_16K4 > +#define CMD2_SHIFT 8 > +#define ADDR_CYCLES_SHIFT28 > + > +#define XFER_COMPLETEBIT(2) > +#define READ_READY BIT(1) > +#define WRITE_READY BIT(0) > +#define MBIT_ERROR BIT(3) > + > +#define PROG_PGRDBIT(0) > +#define PROG_ERASE BIT(2) > +#define PROG_STATUS BIT(3) > +#define PROG_PGPROG BIT(4) > +#define PROG_RDIDBIT(6) > +#define PROG_RDPARAM BIT(7) > +#define PROG_RST BIT(8) > +#define PROG_GET_FEATURE BIT(9) > +#define PROG_SET_FEATURE BIT(10) > + > +#define PG_ADDR_SHIFT16 > +#define BCH_MODE_SHIFT 25 > +#define BCH_EN_SHIFT 27 > +#define ECC_SIZE_SHIFT 16 > + > +#define MEM_ADDR_MASKGENMASK(7, 0) > +#define BCH_MODE_MASKGENMASK(27, 25) > + > +#define CS_MASK GENMASK(31, 30) > +#define CS_SHIFT 30 > + > +#define PAGE_ERR_CNT_MASKGENMASK(16, 8) > +#define PKT_ERR_CNT_MASK GENMASK(7, 0) > + > +#define NVDDR_MODE BIT(9) > +#d
Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
On Mon, 20 Aug 2018 12:21:12 + Naga Sureshkumar Relli wrote: > Hi Boris, > > > -Original Message- > > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > > Sent: Monday, August 20, 2018 5:40 PM > > To: Naga Sureshkumar Relli > > Cc: rich...@nod.at; abs...@codeaurora.org; linux-kernel@vger.kernel.org; > > marek.va...@gmail.com; kyungmin.p...@samsung.com; > > frieder.schre...@exceet.de; linux- > > m...@lists.infradead.org; miquel.ray...@bootlin.com; > > nagasureshkumarre...@gmail.com; > > Michal Simek ; computersforpe...@gmail.com; > > dw...@infradead.org; > > peterpand...@micron.com > > Subject: Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for > > Arasan > > NAND Flash Controller > > > > On Mon, 20 Aug 2018 10:49:38 + > > Naga Sureshkumar Relli wrote: > > > > > > > > Thanks for your suggestion and are you saying something like Marvell > > > parser patterns for > > nfcv1 as below? > > > > > > static const struct nand_op_parser marvell_nfcv1_op_parser = > > > NAND_OP_PARSER( > > > /* Naked commands not supported, use a function for each pattern */ > > > NAND_OP_PARSER_PATTERN( > > > marvell_nfc_read_id_type_exec, > > > NAND_OP_PARSER_PAT_CMD_ELEM(false), > > > NAND_OP_PARSER_PAT_ADDR_ELEM(false, > > MAX_ADDRESS_CYC_NFCV1), > > > NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 8)), > > > NAND_OP_PARSER_PATTERN( > > > marvell_nfc_erase_cmd_type_exec, > > > NAND_OP_PARSER_PAT_CMD_ELEM(false), > > > NAND_OP_PARSER_PAT_ADDR_ELEM(false, > > MAX_ADDRESS_CYC_NFCV1), > > > NAND_OP_PARSER_PAT_CMD_ELEM(false), > > > NAND_OP_PARSER_PAT_WAITRDY_ELEM(false)), > > > ); > > > That means, a separate hook for each pattern, is that you are suggesting? > > > > > > > Yes. > > Ok, I will update the driver and will send next version. Wait a bit before sending a new version, there were other things I wanted to comment on, this one was just the main issue.
RE: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris, > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Monday, August 20, 2018 5:40 PM > To: Naga Sureshkumar Relli > Cc: rich...@nod.at; abs...@codeaurora.org; linux-kernel@vger.kernel.org; > marek.va...@gmail.com; kyungmin.p...@samsung.com; frieder.schre...@exceet.de; > linux- > m...@lists.infradead.org; miquel.ray...@bootlin.com; > nagasureshkumarre...@gmail.com; > Michal Simek ; computersforpe...@gmail.com; > dw...@infradead.org; > peterpand...@micron.com > Subject: Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for > Arasan > NAND Flash Controller > > On Mon, 20 Aug 2018 10:49:38 + > Naga Sureshkumar Relli wrote: > > > > > Thanks for your suggestion and are you saying something like Marvell parser > > patterns for > nfcv1 as below? > > > > static const struct nand_op_parser marvell_nfcv1_op_parser = NAND_OP_PARSER( > > /* Naked commands not supported, use a function for each pattern */ > > NAND_OP_PARSER_PATTERN( > > marvell_nfc_read_id_type_exec, > > NAND_OP_PARSER_PAT_CMD_ELEM(false), > > NAND_OP_PARSER_PAT_ADDR_ELEM(false, > MAX_ADDRESS_CYC_NFCV1), > > NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 8)), > > NAND_OP_PARSER_PATTERN( > > marvell_nfc_erase_cmd_type_exec, > > NAND_OP_PARSER_PAT_CMD_ELEM(false), > > NAND_OP_PARSER_PAT_ADDR_ELEM(false, > MAX_ADDRESS_CYC_NFCV1), > > NAND_OP_PARSER_PAT_CMD_ELEM(false), > > NAND_OP_PARSER_PAT_WAITRDY_ELEM(false)), > > ); > > That means, a separate hook for each pattern, is that you are suggesting? > > Yes. Ok, I will update the driver and will send next version. Thanks, Naga Sureshkumar Relli.
Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
On Mon, 20 Aug 2018 10:49:38 + Naga Sureshkumar Relli wrote: > > Thanks for your suggestion and are you saying something like Marvell parser > patterns for nfcv1 as below? > > static const struct nand_op_parser marvell_nfcv1_op_parser = NAND_OP_PARSER( > /* Naked commands not supported, use a function for each pattern */ > NAND_OP_PARSER_PATTERN( > marvell_nfc_read_id_type_exec, > NAND_OP_PARSER_PAT_CMD_ELEM(false), > NAND_OP_PARSER_PAT_ADDR_ELEM(false, MAX_ADDRESS_CYC_NFCV1), > NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 8)), > NAND_OP_PARSER_PATTERN( > marvell_nfc_erase_cmd_type_exec, > NAND_OP_PARSER_PAT_CMD_ELEM(false), > NAND_OP_PARSER_PAT_ADDR_ELEM(false, MAX_ADDRESS_CYC_NFCV1), > NAND_OP_PARSER_PAT_CMD_ELEM(false), > NAND_OP_PARSER_PAT_WAITRDY_ELEM(false)), > ); > That means, a separate hook for each pattern, is that you are suggesting? Yes.
RE: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris,
> -Original Message-
> From: Boris Brezillon [mailto:boris.brezil...@bootlin.com]
> Sent: Monday, August 20, 2018 2:24 PM
> To: Naga Sureshkumar Relli
> Cc: rich...@nod.at; abs...@codeaurora.org; linux-kernel@vger.kernel.org;
> marek.va...@gmail.com; kyungmin.p...@samsung.com; frieder.schre...@exceet.de;
> linux-
> m...@lists.infradead.org; miquel.ray...@bootlin.com;
> nagasureshkumarre...@gmail.com;
> Michal Simek ; computersforpe...@gmail.com;
> dw...@infradead.org;
> peterpand...@micron.com
> Subject: Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for
> Arasan
> NAND Flash Controller
>
> On Sat, 18 Aug 2018 05:49:32 +
> Naga Sureshkumar Relli wrote:
>
> > Hi Boris,
> >
> > Thanks for the review.
> >
> > > -Original Message-
> > > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com]
> > > Sent: Friday, August 17, 2018 11:29 PM
> > > To: Naga Sureshkumar Relli
> > > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org;
> > > computersforpe...@gmail.com; marek.va...@gmail.com;
> > > kyungmin.p...@samsung.com; abs...@codeaurora.org;
> > > peterpand...@micron.com; frieder.schre...@exceet.de; linux-
> > > m...@lists.infradead.org; linux-kernel@vger.kernel.org; Michal Simek
> > > ; nagasureshkumarre...@gmail.com
> > > Subject: Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support
> > > for Arasan NAND Flash Controller
> > >
> > > Hi Naga,
> > >
> > > On Fri, 17 Aug 2018 18:49:24 +0530
> > > Naga Sureshkumar Relli wrote:
> > >
> > > > +static int anfc_exec_op_cmd(struct nand_chip *chip,
> > > > + const struct nand_subop *subop) {
> > > > + const struct nand_op_instr *instr;
> > > > + struct anfc_op nfc_op = {};
> > > > + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > > > + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> > > > + struct mtd_info *mtd = nand_to_mtd(chip);
> > > > + u32 addrcycles;
> > > > + unsigned int op_id, len = 0;
> > > > + bool reading;
> > > > +
> > > > + anfc_parse_instructions(chip, subop, &nfc_op);
> > > > + instr = nfc_op.data_instr;
> > > > + op_id = nfc_op.data_instr_idx;
> > > > + if (nfc_op.data_instr)
> > > > + len = nand_subop_get_data_len(subop, op_id);
> > > > +
> > > > + /*
> > > > +* The switch case is to prepare a command and to set
> > > > page/column
> > > > +* address. Arasan NAND controller has program register(Off:
> > > > 0x10)),
> > > > +* which needs to be set for every command.
> > > > +* Ex: When NAND_CMD_RESET is issued, then we need to set reset
> > > > bit
> > > > +* in program_register. etc..
> > > > +*/
> > > > + switch (nfc_op.cmnds[0]) {
> > > > + case NAND_CMD_SEQIN:
> > > > + addrcycles = achip->raddr_cycles + achip->caddr_cycles;
> > > > +
> > > > + anfc_prepare_cmd(nfc, nfc_op.cmnds[0],
> > > > NAND_CMD_PAGEPROG,
> 1,
> > > > +mtd->writesize, addrcycles);
> > > > + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> > > > + break;
> > > > + case NAND_CMD_READOOB:
> > > > + nfc_op.col += mtd->writesize;
> > > > + case NAND_CMD_READ0:
> > > > + case NAND_CMD_READ1:
> > > > + addrcycles = achip->raddr_cycles + achip->caddr_cycles;
> > > > + anfc_prepare_cmd(nfc, NAND_CMD_READ0,
> NAND_CMD_READSTART,
> > > 1,
> > > > +mtd->writesize, addrcycles);
> > > > + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> > > > + if (!nfc_op.data_instr)
> > > > + return 0;
> > > > +
> > > > + anfc_read_data_op(mtd, instr->ctx.data.buf.in, len);
> > > > + break;
> > > > + case NAND_CMD_RNDOUT:
> > > > + anfc_prepare_cmd(nfc, nfc_op.cmnds[0],
> NAND_CMD_RNDOUTSTART, 1,
> > > > +
Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
On Sat, 18 Aug 2018 05:49:32 +
Naga Sureshkumar Relli wrote:
> Hi Boris,
>
> Thanks for the review.
>
> > -Original Message-
> > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com]
> > Sent: Friday, August 17, 2018 11:29 PM
> > To: Naga Sureshkumar Relli
> > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org;
> > computersforpe...@gmail.com; marek.va...@gmail.com;
> > kyungmin.p...@samsung.com;
> > abs...@codeaurora.org; peterpand...@micron.com; frieder.schre...@exceet.de;
> > linux-
> > m...@lists.infradead.org; linux-kernel@vger.kernel.org; Michal Simek
> > ;
> > nagasureshkumarre...@gmail.com
> > Subject: Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for
> > Arasan
> > NAND Flash Controller
> >
> > Hi Naga,
> >
> > On Fri, 17 Aug 2018 18:49:24 +0530
> > Naga Sureshkumar Relli wrote:
> >
> > > +static int anfc_exec_op_cmd(struct nand_chip *chip,
> > > +const struct nand_subop *subop) {
> > > + const struct nand_op_instr *instr;
> > > + struct anfc_op nfc_op = {};
> > > + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > > + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> > > + struct mtd_info *mtd = nand_to_mtd(chip);
> > > + u32 addrcycles;
> > > + unsigned int op_id, len = 0;
> > > + bool reading;
> > > +
> > > + anfc_parse_instructions(chip, subop, &nfc_op);
> > > + instr = nfc_op.data_instr;
> > > + op_id = nfc_op.data_instr_idx;
> > > + if (nfc_op.data_instr)
> > > + len = nand_subop_get_data_len(subop, op_id);
> > > +
> > > + /*
> > > + * The switch case is to prepare a command and to set page/column
> > > + * address. Arasan NAND controller has program register(Off: 0x10)),
> > > + * which needs to be set for every command.
> > > + * Ex: When NAND_CMD_RESET is issued, then we need to set reset bit
> > > + * in program_register. etc..
> > > + */
> > > + switch (nfc_op.cmnds[0]) {
> > > + case NAND_CMD_SEQIN:
> > > + addrcycles = achip->raddr_cycles + achip->caddr_cycles;
> > > +
> > > + anfc_prepare_cmd(nfc, nfc_op.cmnds[0], NAND_CMD_PAGEPROG, 1,
> > > + mtd->writesize, addrcycles);
> > > + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> > > + break;
> > > + case NAND_CMD_READOOB:
> > > + nfc_op.col += mtd->writesize;
> > > + case NAND_CMD_READ0:
> > > + case NAND_CMD_READ1:
> > > + addrcycles = achip->raddr_cycles + achip->caddr_cycles;
> > > + anfc_prepare_cmd(nfc, NAND_CMD_READ0, NAND_CMD_READSTART,
> > 1,
> > > + mtd->writesize, addrcycles);
> > > + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> > > + if (!nfc_op.data_instr)
> > > + return 0;
> > > +
> > > + anfc_read_data_op(mtd, instr->ctx.data.buf.in, len);
> > > + break;
> > > + case NAND_CMD_RNDOUT:
> > > + anfc_prepare_cmd(nfc, nfc_op.cmnds[0], NAND_CMD_RNDOUTSTART, 1,
> > > + mtd->writesize, 2);
> > > + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> > > + nfc->prog = PROG_PGRD;
> > > + break;
> > > + case NAND_CMD_PARAM:
> > > + anfc_prepare_cmd(nfc, nfc_op.cmnds[0], 0, 0, 0, 1);
> > > + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> > > + nfc->prog = PROG_RDPARAM;
> > > + break;
> > > + case NAND_CMD_READID:
> > > + anfc_prepare_cmd(nfc, nfc_op.cmnds[0], 0, 0, 0, 1);
> > > + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> > > + nfc->prog = PROG_RDID;
> > > + break;
> > > + case NAND_CMD_GET_FEATURES:
> > > + anfc_prepare_cmd(nfc, nfc_op.cmnds[0], 0, 0, 0, 1);
> > > + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> > > + nfc->prog = PROG_GET_FEATURE;
> > > + break;
> > > + case NAND_CMD_SET_FEATURES:
> > > + anfc_prepare_cmd(nfc, nfc_op.cmnds[0], 0, 0, 0, 1);
> > > + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> > > + nfc->prog = PROG_SET_FEATURE;
> > > + break;
> > > + case NAND_CMD_ERASE1:
> > > +
RE: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris,
Thanks for the review.
> -Original Message-
> From: Boris Brezillon [mailto:boris.brezil...@bootlin.com]
> Sent: Friday, August 17, 2018 11:29 PM
> To: Naga Sureshkumar Relli
> Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org;
> computersforpe...@gmail.com; marek.va...@gmail.com; kyungmin.p...@samsung.com;
> abs...@codeaurora.org; peterpand...@micron.com; frieder.schre...@exceet.de;
> linux-
> m...@lists.infradead.org; linux-kernel@vger.kernel.org; Michal Simek
> ;
> nagasureshkumarre...@gmail.com
> Subject: Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for
> Arasan
> NAND Flash Controller
>
> Hi Naga,
>
> On Fri, 17 Aug 2018 18:49:24 +0530
> Naga Sureshkumar Relli wrote:
>
> > +static int anfc_exec_op_cmd(struct nand_chip *chip,
> > + const struct nand_subop *subop) {
> > + const struct nand_op_instr *instr;
> > + struct anfc_op nfc_op = {};
> > + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> > + struct mtd_info *mtd = nand_to_mtd(chip);
> > + u32 addrcycles;
> > + unsigned int op_id, len = 0;
> > + bool reading;
> > +
> > + anfc_parse_instructions(chip, subop, &nfc_op);
> > + instr = nfc_op.data_instr;
> > + op_id = nfc_op.data_instr_idx;
> > + if (nfc_op.data_instr)
> > + len = nand_subop_get_data_len(subop, op_id);
> > +
> > + /*
> > +* The switch case is to prepare a command and to set page/column
> > +* address. Arasan NAND controller has program register(Off: 0x10)),
> > +* which needs to be set for every command.
> > +* Ex: When NAND_CMD_RESET is issued, then we need to set reset bit
> > +* in program_register. etc..
> > +*/
> > + switch (nfc_op.cmnds[0]) {
> > + case NAND_CMD_SEQIN:
> > + addrcycles = achip->raddr_cycles + achip->caddr_cycles;
> > +
> > + anfc_prepare_cmd(nfc, nfc_op.cmnds[0], NAND_CMD_PAGEPROG, 1,
> > +mtd->writesize, addrcycles);
> > + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> > + break;
> > + case NAND_CMD_READOOB:
> > + nfc_op.col += mtd->writesize;
> > + case NAND_CMD_READ0:
> > + case NAND_CMD_READ1:
> > + addrcycles = achip->raddr_cycles + achip->caddr_cycles;
> > + anfc_prepare_cmd(nfc, NAND_CMD_READ0, NAND_CMD_READSTART,
> 1,
> > +mtd->writesize, addrcycles);
> > + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> > + if (!nfc_op.data_instr)
> > + return 0;
> > +
> > + anfc_read_data_op(mtd, instr->ctx.data.buf.in, len);
> > + break;
> > + case NAND_CMD_RNDOUT:
> > + anfc_prepare_cmd(nfc, nfc_op.cmnds[0], NAND_CMD_RNDOUTSTART, 1,
> > +mtd->writesize, 2);
> > + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> > + nfc->prog = PROG_PGRD;
> > + break;
> > + case NAND_CMD_PARAM:
> > + anfc_prepare_cmd(nfc, nfc_op.cmnds[0], 0, 0, 0, 1);
> > + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> > + nfc->prog = PROG_RDPARAM;
> > + break;
> > + case NAND_CMD_READID:
> > + anfc_prepare_cmd(nfc, nfc_op.cmnds[0], 0, 0, 0, 1);
> > + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> > + nfc->prog = PROG_RDID;
> > + break;
> > + case NAND_CMD_GET_FEATURES:
> > + anfc_prepare_cmd(nfc, nfc_op.cmnds[0], 0, 0, 0, 1);
> > + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> > + nfc->prog = PROG_GET_FEATURE;
> > + break;
> > + case NAND_CMD_SET_FEATURES:
> > + anfc_prepare_cmd(nfc, nfc_op.cmnds[0], 0, 0, 0, 1);
> > + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> > + nfc->prog = PROG_SET_FEATURE;
> > + break;
> > + case NAND_CMD_ERASE1:
> > + anfc_erase_function(chip, nfc_op);
> > + break;
> > + default:
> > + break;
> > + }
>
> Looks like you have one of these smart controllers where everything is
> hardcoded and new
> commands (like vendor specific commands) can't be supported, and we're back
> to abusing -
> >exec_op(), just like ->cmdfunc() was abused.
Actually hardcoding commands with
RE: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Miquel,
Thanks for the review.
> -Original Message-
> From: Miquel Raynal [mailto:miquel.ray...@bootlin.com]
> Sent: Friday, August 17, 2018 8:08 PM
> To: Naga Sureshkumar Relli
> Cc: boris.brezil...@bootlin.com; rich...@nod.at; dw...@infradead.org;
> computersforpe...@gmail.com; marek.va...@gmail.com; kyungmin.p...@samsung.com;
> abs...@codeaurora.org; peterpand...@micron.com; frieder.schre...@exceet.de;
> linux-
> m...@lists.infradead.org; linux-kernel@vger.kernel.org; Michal Simek
> ;
> nagasureshkumarre...@gmail.com
> Subject: Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for
> Arasan
> NAND Flash Controller
>
> Hi Naga,
>
> A user of Xilinx BSP reported a problem with lockdep that I can see is not
> yet fixed in this
> code:
I didn't know this. thanks for reporting the issue. let me check.
>
> Naga Sureshkumar Relli wrote on Fri, 17
> Aug 2018
> 18:49:24 +0530:
>
> > +static int anfc_probe(struct platform_device *pdev) {
> > + struct anfc_nand_controller *nfc;
> > + struct anfc_nand_chip *anand_chip;
> > + struct device_node *np = pdev->dev.of_node, *child;
> > + struct resource *res;
> > + int err;
> > +
> > + nfc = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
> > + if (!nfc)
> > + return -ENOMEM;
> > +
> > + init_waitqueue_head(&nfc->controller.wq);
>
> The controller structure has a lock which is not initialized here. You should
> not initialize the
> waitqueue manually neither and instead use
> nand_controller_init(&nfc->controller).
Ok. Just now I saw marvell nand driver. I will update accordingly.
>
> > + INIT_LIST_HEAD(&nfc->chips);
> > + init_completion(&nfc->event);
> > + nfc->dev = &pdev->dev;
> > + platform_set_drvdata(pdev, nfc);
> > + nfc->csnum = -1;
> > + nfc->controller.ops = &anfc_nand_controller_ops;
>
> More to come.
Thanks,
Naga Sureshkumar Relli.
>
> Thanks,
> Miquèl
Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga,
On Fri, 17 Aug 2018 18:49:24 +0530
Naga Sureshkumar Relli wrote:
> +static int anfc_exec_op_cmd(struct nand_chip *chip,
> +const struct nand_subop *subop)
> +{
> + const struct nand_op_instr *instr;
> + struct anfc_op nfc_op = {};
> + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> + struct mtd_info *mtd = nand_to_mtd(chip);
> + u32 addrcycles;
> + unsigned int op_id, len = 0;
> + bool reading;
> +
> + anfc_parse_instructions(chip, subop, &nfc_op);
> + instr = nfc_op.data_instr;
> + op_id = nfc_op.data_instr_idx;
> + if (nfc_op.data_instr)
> + len = nand_subop_get_data_len(subop, op_id);
> +
> + /*
> + * The switch case is to prepare a command and to set page/column
> + * address. Arasan NAND controller has program register(Off: 0x10)),
> + * which needs to be set for every command.
> + * Ex: When NAND_CMD_RESET is issued, then we need to set reset bit
> + * in program_register. etc..
> + */
> + switch (nfc_op.cmnds[0]) {
> + case NAND_CMD_SEQIN:
> + addrcycles = achip->raddr_cycles + achip->caddr_cycles;
> +
> + anfc_prepare_cmd(nfc, nfc_op.cmnds[0], NAND_CMD_PAGEPROG, 1,
> + mtd->writesize, addrcycles);
> + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> + break;
> + case NAND_CMD_READOOB:
> + nfc_op.col += mtd->writesize;
> + case NAND_CMD_READ0:
> + case NAND_CMD_READ1:
> + addrcycles = achip->raddr_cycles + achip->caddr_cycles;
> + anfc_prepare_cmd(nfc, NAND_CMD_READ0, NAND_CMD_READSTART, 1,
> + mtd->writesize, addrcycles);
> + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> + if (!nfc_op.data_instr)
> + return 0;
> +
> + anfc_read_data_op(mtd, instr->ctx.data.buf.in, len);
> + break;
> + case NAND_CMD_RNDOUT:
> + anfc_prepare_cmd(nfc, nfc_op.cmnds[0], NAND_CMD_RNDOUTSTART, 1,
> + mtd->writesize, 2);
> + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> + nfc->prog = PROG_PGRD;
> + break;
> + case NAND_CMD_PARAM:
> + anfc_prepare_cmd(nfc, nfc_op.cmnds[0], 0, 0, 0, 1);
> + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> + nfc->prog = PROG_RDPARAM;
> + break;
> + case NAND_CMD_READID:
> + anfc_prepare_cmd(nfc, nfc_op.cmnds[0], 0, 0, 0, 1);
> + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> + nfc->prog = PROG_RDID;
> + break;
> + case NAND_CMD_GET_FEATURES:
> + anfc_prepare_cmd(nfc, nfc_op.cmnds[0], 0, 0, 0, 1);
> + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> + nfc->prog = PROG_GET_FEATURE;
> + break;
> + case NAND_CMD_SET_FEATURES:
> + anfc_prepare_cmd(nfc, nfc_op.cmnds[0], 0, 0, 0, 1);
> + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> + nfc->prog = PROG_SET_FEATURE;
> + break;
> + case NAND_CMD_ERASE1:
> + anfc_erase_function(chip, nfc_op);
> + break;
> + default:
> + break;
> + }
Looks like you have one of these smart controllers where everything is
hardcoded and new commands (like vendor specific commands) can't be
supported, and we're back to abusing ->exec_op(), just like ->cmdfunc()
was abused.
Don't you have a way to send raw CMD/ADDR/DATA cycles? If not, then
we'll have to consider other options, because I don't want to go back
to the situation we are in with ->cmdfunc().
Maybe I already asked, but is there a public spec for this IP?
Thanks,
Boris
Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on mtd/nand/next]
[also build test WARNING on next-20180817]
[cannot apply to v4.18]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Naga-Sureshkumar-Relli/Add-support-for-Arasan-NAND-Flash-controller/20180817-224117
base: git://git.infradead.org/linux-mtd.git nand/next
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=sh
All warnings (new ones prefixed by >>):
In file included from include/linux/scatterlist.h:9:0,
from include/linux/dma-mapping.h:11,
from drivers/mtd//nand/raw/arasan_nand.c:12:
drivers/mtd//nand/raw/arasan_nand.c: In function 'anfc_rw_dma_op':
>> drivers/mtd//nand/raw/arasan_nand.c:362:16: warning: right shift count >=
>> width of type [-Wshift-count-overflow]
writel((paddr >> 32), nfc->base + DMA_ADDR1_OFST);
^
arch/sh/include/asm/io.h:31:77: note: in definition of macro '__raw_writel'
#define __raw_writel(v,a) (__chk_io_ptr(a), *(volatile u32 __force *)(a) =
(v))
^
arch/sh/include/asm/io.h:46:62: note: in expansion of macro 'ioswabl'
#define writel_relaxed(v,c) ((void)__raw_writel((__force u32)ioswabl(v),c))
^~~
arch/sh/include/asm/io.h:56:32: note: in expansion of macro 'writel_relaxed'
#define writel(v,a) ({ wmb(); writel_relaxed((v),(a)); })
^~
>> drivers/mtd//nand/raw/arasan_nand.c:362:2: note: in expansion of macro
>> 'writel'
writel((paddr >> 32), nfc->base + DMA_ADDR1_OFST);
^~
vim +362 drivers/mtd//nand/raw/arasan_nand.c
330
331 static void anfc_rw_dma_op(struct mtd_info *mtd, uint8_t *buf, int len,
332 bool do_read, u32 prog)
333 {
334 dma_addr_t paddr;
335 struct nand_chip *chip = mtd_to_nand(mtd);
336 struct anfc_nand_controller *nfc = to_anfc(chip->controller);
337 struct anfc_nand_chip *achip = to_anfc_nand(chip);
338 u32 eccintr = 0, dir;
339 u32 pktsize = len, pktcount = 1;
340
341 if (((nfc->curr_cmd == NAND_CMD_READ0)) ||
342 (nfc->curr_cmd == NAND_CMD_SEQIN && !nfc->iswriteoob)) {
343 pktsize = achip->pktsize;
344 pktcount = DIV_ROUND_UP(mtd->writesize, pktsize);
345 }
346 anfc_setpktszcnt(nfc, pktsize, pktcount);
347
348 if (!achip->bch && nfc->curr_cmd == NAND_CMD_READ0)
349 eccintr = MBIT_ERROR;
350
351 if (do_read)
352 dir = DMA_FROM_DEVICE;
353 else
354 dir = DMA_TO_DEVICE;
355
356 paddr = dma_map_single(nfc->dev, buf, len, dir);
357 if (dma_mapping_error(nfc->dev, paddr)) {
358 dev_err(nfc->dev, "Read buffer mapping error");
359 return;
360 }
361 writel(paddr, nfc->base + DMA_ADDR0_OFST);
> 362 writel((paddr >> 32), nfc->base + DMA_ADDR1_OFST);
363 anfc_enable_intrs(nfc, (XFER_COMPLETE | eccintr));
364 writel(prog, nfc->base + PROG_OFST);
365 anfc_wait_for_event(nfc);
366 dma_unmap_single(nfc->dev, paddr, len, dir);
367 }
368
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [LINUX PATCH v10 2/2] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga,
A user of Xilinx BSP reported a problem with lockdep that I can see is
not yet fixed in this code:
Naga Sureshkumar Relli wrote on
Fri, 17 Aug 2018 18:49:24 +0530:
> +static int anfc_probe(struct platform_device *pdev)
> +{
> + struct anfc_nand_controller *nfc;
> + struct anfc_nand_chip *anand_chip;
> + struct device_node *np = pdev->dev.of_node, *child;
> + struct resource *res;
> + int err;
> +
> + nfc = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
> + if (!nfc)
> + return -ENOMEM;
> +
> + init_waitqueue_head(&nfc->controller.wq);
The controller structure has a lock which is not initialized here. You
should not initialize the waitqueue manually neither and instead use
nand_controller_init(&nfc->controller).
> + INIT_LIST_HEAD(&nfc->chips);
> + init_completion(&nfc->event);
> + nfc->dev = &pdev->dev;
> + platform_set_drvdata(pdev, nfc);
> + nfc->csnum = -1;
> + nfc->controller.ops = &anfc_nand_controller_ops;
More to come.
Thanks,
Miquèl
