| 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: <76b81dad9dc710f57245051fee72707f@codeaurora.org>
Date: Mon, 17 Jul 2017 13:01:11 +0530
From: Abhishek Sahu <absahu@...eaurora.org>
To: Archit Taneja <architt@...eaurora.org>
Cc: 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, 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 2017-07-04 15:25, Archit Taneja wrote:
> 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
>
Sure. I will fix this in v2.
>> };
>>
>> /*
>> @@ -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()
>
Yes. I will do the same in v2.
> 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);
>>
--
Abhishek Sahu
Powered by blists - more mailing lists