Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752203AbdGDJzg (ORCPT ); Tue, 4 Jul 2017 05:55:36 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:37964 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751631AbdGDJze (ORCPT ); Tue, 4 Jul 2017 05:55:34 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 4136F6074C Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=architt@codeaurora.org Subject: Re: [PATCH 12/14] qcom: mtd: nand: change register offset defines with enums To: Abhishek Sahu , dwmw2@infradead.org, computersforpeace@gmail.com, boris.brezillon@free-electrons.com, marek.vasut@gmail.com, richard@nod.at, cyrille.pitchen@wedev4u.fr, robh+dt@kernel.org, mark.rutland@arm.com Cc: linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, andy.gross@linaro.org, sricharan@codeaurora.org References: <1498720566-20782-1-git-send-email-absahu@codeaurora.org> <1498720566-20782-13-git-send-email-absahu@codeaurora.org> From: Archit Taneja Message-ID: Date: Tue, 4 Jul 2017 15:25:26 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <1498720566-20782-13-git-send-email-absahu@codeaurora.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8537 Lines: 263 On 06/29/2017 12:46 PM, Abhishek Sahu wrote: > The current driver defines the register offset with preprocessor > macro which is defined crossponding to NAND controller version > 1.4.0. This patch changes these macro with enumeration. It also > adds mapping array which contains controller register offsets for > each register offset enumeration. This mapping array will be > referenced before each register read and writes, where the register > offset enumeration is being replaced with actual register offsets. > > Signed-off-by: Abhishek Sahu > --- > drivers/mtd/nand/qcom_nandc.c | 136 +++++++++++++++++++++++++++--------------- > 1 file changed, 89 insertions(+), 47 deletions(-) > > diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c > index 6d749b6..24750e6 100644 > --- a/drivers/mtd/nand/qcom_nandc.c > +++ b/drivers/mtd/nand/qcom_nandc.c > @@ -24,43 +24,6 @@ > #include > #include > > -/* NANDc reg offsets */ > -#define NAND_FLASH_CMD 0x00 > -#define NAND_ADDR0 0x04 > -#define NAND_ADDR1 0x08 > -#define NAND_FLASH_CHIP_SELECT 0x0c > -#define NAND_EXEC_CMD 0x10 > -#define NAND_FLASH_STATUS 0x14 > -#define NAND_BUFFER_STATUS 0x18 > -#define NAND_DEV0_CFG0 0x20 > -#define NAND_DEV0_CFG1 0x24 > -#define NAND_DEV0_ECC_CFG 0x28 > -#define NAND_DEV1_ECC_CFG 0x2c > -#define NAND_DEV1_CFG0 0x30 > -#define NAND_DEV1_CFG1 0x34 > -#define NAND_READ_ID 0x40 > -#define NAND_READ_STATUS 0x44 > -#define NAND_DEV_CMD0 0xa0 > -#define NAND_DEV_CMD1 0xa4 > -#define NAND_DEV_CMD2 0xa8 > -#define NAND_DEV_CMD_VLD 0xac > -#define SFLASHC_BURST_CFG 0xe0 > -#define NAND_ERASED_CW_DETECT_CFG 0xe8 > -#define NAND_ERASED_CW_DETECT_STATUS 0xec > -#define NAND_EBI2_ECC_BUF_CFG 0xf0 > -#define FLASH_BUF_ACC 0x100 > - > -#define NAND_CTRL 0xf00 > -#define NAND_VERSION 0xf08 > -#define NAND_READ_LOCATION_0 0xf20 > -#define NAND_READ_LOCATION_1 0xf24 > -#define NAND_READ_LOCATION_2 0xf28 > -#define NAND_READ_LOCATION_3 0xf2c > - > -/* dummy register offsets, used by write_reg_dma */ > -#define NAND_DEV_CMD1_RESTORE 0xdead > -#define NAND_DEV_CMD_VLD_RESTORE 0xbeef > - > /* NAND_FLASH_CMD bits */ > #define PAGE_ACC BIT(4) > #define LAST_PAGE BIT(5) > @@ -204,6 +167,44 @@ > #define QPIC_PER_CW_MAX_CMD_SGL (32) > #define QPIC_PER_CW_MAX_DATA_SGL (8) > > +/* NANDc reg offsets enumeration */ > +enum { > + NAND_FLASH_CMD, > + NAND_ADDR0, > + NAND_ADDR1, > + NAND_FLASH_CHIP_SELECT, > + NAND_EXEC_CMD, > + NAND_FLASH_STATUS, > + NAND_BUFFER_STATUS, > + NAND_DEV0_CFG0, > + NAND_DEV0_CFG1, > + NAND_DEV0_ECC_CFG, > + NAND_DEV1_ECC_CFG, > + NAND_DEV1_CFG0, > + NAND_DEV1_CFG1, > + NAND_READ_ID, > + NAND_READ_STATUS, > + NAND_DEV_CMD0, > + NAND_DEV_CMD1, > + NAND_DEV_CMD2, > + NAND_DEV_CMD_VLD, > + SFLASHC_BURST_CFG, > + NAND_ERASED_CW_DETECT_CFG, > + NAND_ERASED_CW_DETECT_STATUS, > + NAND_EBI2_ECC_BUF_CFG, > + FLASH_BUF_ACC, > + NAND_CTRL, > + NAND_VERSION, > + NAND_READ_LOCATION_0, > + NAND_READ_LOCATION_1, > + NAND_READ_LOCATION_2, > + NAND_READ_LOCATION_3, > + > + /* dummy register offsets, used by write_reg_dma */ > + NAND_DEV_CMD1_RESTORE, > + NAND_DEV_CMD_VLD_RESTORE, > +}; > + > /* > * This data type corresponds to the BAM transaction which will be used for all > * NAND transfers. > @@ -326,6 +327,7 @@ struct nandc_regs { > * bam dma > * @max_cwperpage: maximum qpic codeword required. calcualted > * from all nand device pagesize > + * @regs_offsets: register offset mapping array > */ > struct qcom_nand_controller { > struct nand_hw_control controller; > @@ -371,6 +373,7 @@ struct qcom_nand_controller { > > u32 cmd1, vld; > u32 ecc_modes; > + const u32 *regs_offsets; minor quirk: s/regs_offsets/reg_offsets > }; > > /* > @@ -434,6 +437,40 @@ struct qcom_nand_driver_data { > bool dma_bam_enabled; > }; > > +/* Mapping table which contains the actual register offsets */ > +static const u32 regs_offsets[] = { > + [NAND_FLASH_CMD] = 0x00, > + [NAND_ADDR0] = 0x04, > + [NAND_ADDR1] = 0x08, > + [NAND_FLASH_CHIP_SELECT] = 0x0c, > + [NAND_EXEC_CMD] = 0x10, > + [NAND_FLASH_STATUS] = 0x14, > + [NAND_BUFFER_STATUS] = 0x18, > + [NAND_DEV0_CFG0] = 0x20, > + [NAND_DEV0_CFG1] = 0x24, > + [NAND_DEV0_ECC_CFG] = 0x28, > + [NAND_DEV1_ECC_CFG] = 0x2c, > + [NAND_DEV1_CFG0] = 0x30, > + [NAND_DEV1_CFG1] = 0x34, > + [NAND_READ_ID] = 0x40, > + [NAND_READ_STATUS] = 0x44, > + [NAND_DEV_CMD0] = 0xa0, > + [NAND_DEV_CMD1] = 0xa4, > + [NAND_DEV_CMD2] = 0xa8, > + [NAND_DEV_CMD_VLD] = 0xac, > + [SFLASHC_BURST_CFG] = 0xe0, > + [NAND_ERASED_CW_DETECT_CFG] = 0xe8, > + [NAND_ERASED_CW_DETECT_STATUS] = 0xec, > + [NAND_EBI2_ECC_BUF_CFG] = 0xf0, > + [FLASH_BUF_ACC] = 0x100, > + [NAND_CTRL] = 0xf00, > + [NAND_VERSION] = 0xf08, > + [NAND_READ_LOCATION_0] = 0xf20, > + [NAND_READ_LOCATION_1] = 0xf24, > + [NAND_READ_LOCATION_2] = 0xf28, > + [NAND_READ_LOCATION_3] = 0xf2c, > +}; > + > /* Frees the BAM transaction memory */ > static void free_bam_transaction(struct qcom_nand_controller *nandc) > { > @@ -516,13 +553,13 @@ static inline struct qcom_nand_host *to_qcom_nand_host(struct nand_chip *chip) > > static inline u32 nandc_read(struct qcom_nand_controller *nandc, int offset) > { > - return ioread32(nandc->base + offset); > + return ioread32(nandc->base + nandc->regs_offsets[offset]); > } > > static inline void nandc_write(struct qcom_nand_controller *nandc, int offset, > u32 val) > { > - iowrite32(val, nandc->base + offset); > + iowrite32(val, nandc->base + nandc->regs_offsets[offset]); > } > > static __le32 *offset_to_nandc_reg(struct nandc_regs *regs, int offset) > @@ -903,11 +940,12 @@ static int read_reg_dma(struct qcom_nand_controller *nandc, int first, > nandc->reg_read_pos += num_regs; > > if (nandc->dma_bam_enabled) > - return prep_dma_desc_command(nandc, true, first, vaddr, > + return prep_dma_desc_command(nandc, true, > + nandc->regs_offsets[first], vaddr, > num_regs, flags); > > - return prep_dma_desc(nandc, true, first, vaddr, num_regs * sizeof(u32), > - flow_control); > + return prep_dma_desc(nandc, true, nandc->regs_offsets[first], vaddr, > + num_regs * sizeof(u32), flow_control); > } > > /* > @@ -946,11 +984,12 @@ static int write_reg_dma(struct qcom_nand_controller *nandc, int first, > first = NAND_DEV_CMD_VLD; > > if (nandc->dma_bam_enabled) > - return prep_dma_desc_command(nandc, false, first, vaddr, > + return prep_dma_desc_command(nandc, false, > + nandc->regs_offsets[first], vaddr, > num_regs, flags); > > - return prep_dma_desc(nandc, false, first, vaddr, num_regs * sizeof(u32), > - flow_control); > + return prep_dma_desc(nandc, false, nandc->regs_offsets[first], vaddr, > + num_regs * sizeof(u32), flow_control); > } > > /* > @@ -968,7 +1007,8 @@ static int read_data_dma(struct qcom_nand_controller *nandc, int reg_off, > return prep_dma_desc_data_bam(nandc, true, reg_off, vaddr, size, > flags); > > - return prep_dma_desc(nandc, true, reg_off, vaddr, size, false); > + return prep_dma_desc(nandc, true, nandc->regs_offsets[FLASH_BUF_ACC] + > + reg_off - FLASH_BUF_ACC, vaddr, size, false); This doesn't make sense. The integer corresponding to FLASH_BUF_ACC enum constant shouldn't be involved in any calculations. It would be better to pass the correct param to reg_off in all the call sites to read_data_dma() and write_data_dma() Looks good otherwise. Archit > } > > /* > @@ -986,7 +1026,8 @@ static int write_data_dma(struct qcom_nand_controller *nandc, int reg_off, > return prep_dma_desc_data_bam(nandc, false, reg_off, vaddr, > size, flags); > > - return prep_dma_desc(nandc, false, reg_off, vaddr, size, false); > + return prep_dma_desc(nandc, false, nandc->regs_offsets[FLASH_BUF_ACC] + > + reg_off - FLASH_BUF_ACC, vaddr, size, false); > } > > /* > @@ -2791,6 +2832,7 @@ static int qcom_nandc_probe(struct platform_device *pdev) > > nandc->ecc_modes = driver_data->ecc_modes; > nandc->dma_bam_enabled = driver_data->dma_bam_enabled; > + nandc->regs_offsets = regs_offsets; > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > nandc->base = devm_ioremap_resource(dev, res); > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project