lkml.org 
[lkml]   [2017]   [Jul]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 07/14] qcom: mtd: nand: support for passing flags in transfer functions
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@codeaurora.org>
>>> ---
>>> 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

\
 
 \ /
  Last update: 2017-07-17 09:00    [W:0.106 / U:5.880 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site