Re: [PATCH 07/14] qcom: mtd: nand: support for passing flags in transfer functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2017-07-10 19:40, Sricharan R wrote:
Hi,

On 7/4/2017 12:19 PM, Archit Taneja wrote:


On 06/29/2017 12:45 PM, Abhishek Sahu wrote:
The BAM has multiple flags to control the transfer. This patch
adds flags parameter in register and data transfer functions and
modifies all these function call with appropriate flags.

Signed-off-by: Abhishek Sahu <absahu@xxxxxxxxxxxxxx>
---
  drivers/mtd/nand/qcom_nandc.c | 114
++++++++++++++++++++++++------------------
  1 file changed, 65 insertions(+), 49 deletions(-)

diff --git a/drivers/mtd/nand/qcom_nandc.c
b/drivers/mtd/nand/qcom_nandc.c
index 7042a65..65c9059 100644
--- a/drivers/mtd/nand/qcom_nandc.c
+++ b/drivers/mtd/nand/qcom_nandc.c
@@ -170,6 +170,14 @@
  #define    ECC_BCH_4BIT    BIT(2)
  #define    ECC_BCH_8BIT    BIT(3)
  +/* Flags used for BAM DMA desc preparation*/
+/* Don't set the EOT in current tx sgl */
+#define NAND_BAM_NO_EOT            (0x0001)
+/* Set the NWD flag in current sgl */
+#define NAND_BAM_NWD            (0x0002)
+/* Finish writing in the current sgl and start writing in another sgl */
+#define NAND_BAM_NEXT_SGL        (0x0004)
+
  #define QPIC_PER_CW_MAX_CMD_ELEMENTS    (32)
  #define QPIC_PER_CW_MAX_CMD_SGL        (32)
  #define QPIC_PER_CW_MAX_DATA_SGL    (8)

 I will remove the braces and use the bit macros.

@@ -712,7 +720,7 @@ static int prep_dma_desc(struct qcom_nand_controller
*nandc, bool read,
   * @num_regs:        number of registers to read
   */
static int read_reg_dma(struct qcom_nand_controller *nandc, int first,
-            int num_regs)
+            int num_regs, unsigned int flags)
  {
      bool flow_control = false;
      void *vaddr;
@@ -736,7 +744,7 @@ static int read_reg_dma(struct qcom_nand_controller
*nandc, int first,
   * @num_regs:        number of registers to write
   */
static int write_reg_dma(struct qcom_nand_controller *nandc, int first,
-             int num_regs)
+             int num_regs, unsigned int flags)

Adding flags to read_reg_dma and write_reg_dma is making things a bit
messy. I can't
think of a better way to share the code either, though.

One thing we could consider doing is something like below. I don't know if
it would
make things more legible.

union nand_dma_props {
    bool adm_flow_control;
    unsigned int bam_flags;
};

config_cw_read()
{
    union nand_dma_props dma_props;
    ...
    ...

    if (is_bam)
        dma_props.bam_flags = NAND_BAM_NWD;
    else
        dma_props.adm_flow_control = false;

    write_reg_dma(nandc, NAND_EXEC_CMD, 1, &dma_props);
    ...
    ...
}

 The flags for each write_reg_dma and read_reg_dma will be different.
 Normally, for all the API's which uses flags
 (like dmaengine_prep_slave_sg), we are passing the flags directly.
 this union won't help us making this code more readable.


 Right, with this , i think we can have two different indirections for
functions like,
 prep_dma_desc_command and prep_dma_desc. That will help to reduce the
bam_dma_enabled
 checks.

 Since common code changes are intermixed with bam_dma_enabled check
so taking function pointer won't help much in making code more readable.

 anyway, I will analyze the final code for v2 and will check the
 possibility of using function pointers.


Regards,
 Sricharan

--
Abhishek Sahu
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux