| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <a8961294-c72b-035c-0924-f0f901821ea4@codeaurora.org>
Date: Tue, 4 Jul 2017 15:25:26 +0530
From: Archit Taneja <architt@...eaurora.org>
To: Abhishek Sahu <absahu@...eaurora.org>, dwmw2@...radead.org,
computersforpeace@...il.com, boris.brezillon@...e-electrons.com,
marek.vasut@...il.com, richard@....at, cyrille.pitchen@...ev4u.fr,
robh+dt@...nel.org, mark.rutland@....com
Cc: linux-mtd@...ts.infradead.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
andy.gross@...aro.org, sricharan@...eaurora.org
Subject: Re: [PATCH 12/14] qcom: mtd: nand: change register offset defines
with enums
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 <absahu@...eaurora.org>
> ---
> 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 <linux/delay.h>
> #include <linux/dma/qcom_bam_dma.h>
>
> -/* 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
Powered by blists - more mailing lists