Hi Jaehoon,

On 2016/8/31 14:55, Jaehoon Chung wrote:
Hi Shawn,

On 08/19/2016 06:40 PM, Shawn Lin wrote:
We intend to add more check for descriptors when
preparing desc. Let's spilt out the separate body
to make the dw_mci_translate_sglist not so lengthy.

Sorry for reviewing late.


Signed-off-by: Shawn Lin <[email protected]>
---

 drivers/mmc/host/dw_mmc.c | 148 +++++++++++++++++++++++++---------------------
 1 file changed, 81 insertions(+), 67 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 32380d5..0a5a49f 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -467,103 +467,117 @@ static void dw_mci_dmac_complete_dma(void *arg)
        }
 }

-static void dw_mci_translate_sglist(struct dw_mci *host, struct mmc_data *data,
-                                   unsigned int sg_len)
+static inline void dw_mci_prepare_desc64(struct dw_mci *host,
+                                        struct mmc_data *data,
+                                        unsigned int sg_len)
 {
        unsigned int desc_len;
+       struct idmac_desc_64addr *desc_first, *desc_last, *desc;
        int i;

-       if (host->dma_64bit_address == 1) {
-               struct idmac_desc_64addr *desc_first, *desc_last, *desc;
-
-               desc_first = desc_last = desc = host->sg_cpu;
+       desc_first = desc_last = desc = host->sg_cpu;

-               for (i = 0; i < sg_len; i++) {
-                       unsigned int length = sg_dma_len(&data->sg[i]);
+       for (i = 0; i < sg_len; i++) {
+               unsigned int length = sg_dma_len(&data->sg[i]);

-                       u64 mem_addr = sg_dma_address(&data->sg[i]);
+               u64 mem_addr = sg_dma_address(&data->sg[i]);

-                       for ( ; length ; desc++) {
-                               desc_len = (length <= DW_MCI_DESC_DATA_LENGTH) ?
-                                          length : DW_MCI_DESC_DATA_LENGTH;
+               for ( ; length ; desc++) {
+                       desc_len = (length <= DW_MCI_DESC_DATA_LENGTH) ?
+                                  length : DW_MCI_DESC_DATA_LENGTH;

-                               length -= desc_len;
+                       length -= desc_len;

-                               /*
-                                * Set the OWN bit and disable interrupts
-                                * for this descriptor
-                                */
-                               desc->des0 = IDMAC_DES0_OWN | IDMAC_DES0_DIC |
-                                                       IDMAC_DES0_CH;
+                       /*
+                        * Set the OWN bit and disable interrupts
+                        * for this descriptor
+                        */
+                       desc->des0 = IDMAC_DES0_OWN | IDMAC_DES0_DIC |
+                                               IDMAC_DES0_CH;

-                               /* Buffer length */
-                               IDMAC_64ADDR_SET_BUFFER1_SIZE(desc, desc_len);
+                       /* Buffer length */
+                       IDMAC_64ADDR_SET_BUFFER1_SIZE(desc, desc_len);

-                               /* Physical address to DMA to/from */
-                               desc->des4 = mem_addr & 0xffffffff;
-                               desc->des5 = mem_addr >> 32;
+                       /* Physical address to DMA to/from */
+                       desc->des4 = mem_addr & 0xffffffff;
+                       desc->des5 = mem_addr >> 32;

-                               /* Update physical address for the next desc */
-                               mem_addr += desc_len;
+                       /* Update physical address for the next desc */
+                       mem_addr += desc_len;

-                               /* Save pointer to the last descriptor */
-                               desc_last = desc;
-                       }
+                       /* Save pointer to the last descriptor */
+                       desc_last = desc;
                }
+       }

-               /* Set first descriptor */
-               desc_first->des0 |= IDMAC_DES0_FD;
+       /* Set first descriptor */
+       desc_first->des0 |= IDMAC_DES0_FD;

-               /* Set last descriptor */
-               desc_last->des0 &= ~(IDMAC_DES0_CH | IDMAC_DES0_DIC);
-               desc_last->des0 |= IDMAC_DES0_LD;
+       /* Set last descriptor */
+       desc_last->des0 &= ~(IDMAC_DES0_CH | IDMAC_DES0_DIC);
+       desc_last->des0 |= IDMAC_DES0_LD;
+}

-       } else {
-               struct idmac_desc *desc_first, *desc_last, *desc;

-               desc_first = desc_last = desc = host->sg_cpu;
+static inline void dw_mci_prepare_desc32(struct dw_mci *host,
+                                        struct mmc_data *data,
+                                        unsigned int sg_len)
+{
+       unsigned int desc_len;
+       struct idmac_desc *desc_first, *desc_last, *desc;
+       int i;

-               for (i = 0; i < sg_len; i++) {
-                       unsigned int length = sg_dma_len(&data->sg[i]);
+       desc_first = desc_last = desc = host->sg_cpu;

-                       u32 mem_addr = sg_dma_address(&data->sg[i]);
+       for (i = 0; i < sg_len; i++) {
+               unsigned int length = sg_dma_len(&data->sg[i]);

-                       for ( ; length ; desc++) {
-                               desc_len = (length <= DW_MCI_DESC_DATA_LENGTH) ?
-                                          length : DW_MCI_DESC_DATA_LENGTH;
+               u32 mem_addr = sg_dma_address(&data->sg[i]);

-                               length -= desc_len;
+               for ( ; length ; desc++) {
+                       desc_len = (length <= DW_MCI_DESC_DATA_LENGTH) ?
+                                  length : DW_MCI_DESC_DATA_LENGTH;

-                               /*
-                                * Set the OWN bit and disable interrupts
-                                * for this descriptor
-                                */
-                               desc->des0 = cpu_to_le32(IDMAC_DES0_OWN |
-                                                        IDMAC_DES0_DIC |
-                                                        IDMAC_DES0_CH);
+                       length -= desc_len;

-                               /* Buffer length */
-                               IDMAC_SET_BUFFER1_SIZE(desc, desc_len);
+                       /*
+                        * Set the OWN bit and disable interrupts
+                        * for this descriptor
+                        */
+                       desc->des0 = cpu_to_le32(IDMAC_DES0_OWN |
+                                                IDMAC_DES0_DIC |
+                                                IDMAC_DES0_CH);

-                               /* Physical address to DMA to/from */
-                               desc->des2 = cpu_to_le32(mem_addr);
+                       /* Buffer length */
+                       IDMAC_SET_BUFFER1_SIZE(desc, desc_len);

-                               /* Update physical address for the next desc */
-                               mem_addr += desc_len;
+                       /* Physical address to DMA to/from */
+                       desc->des2 = cpu_to_le32(mem_addr);

-                               /* Save pointer to the last descriptor */
-                               desc_last = desc;
-                       }
+                       /* Update physical address for the next desc */
+                       mem_addr += desc_len;
+
+                       /* Save pointer to the last descriptor */
+                       desc_last = desc;
                }
+       }

-               /* Set first descriptor */
-               desc_first->des0 |= cpu_to_le32(IDMAC_DES0_FD);
+       /* Set first descriptor */
+       desc_first->des0 |= cpu_to_le32(IDMAC_DES0_FD);

-               /* Set last descriptor */
-               desc_last->des0 &= cpu_to_le32(~(IDMAC_DES0_CH |
-                                              IDMAC_DES0_DIC));
-               desc_last->des0 |= cpu_to_le32(IDMAC_DES0_LD);
-       }
+       /* Set last descriptor */
+       desc_last->des0 &= cpu_to_le32(~(IDMAC_DES0_CH |
+                                      IDMAC_DES0_DIC));
+       desc_last->des0 |= cpu_to_le32(IDMAC_DES0_LD);
+}
+
+static void dw_mci_translate_sglist(struct dw_mci *host, struct mmc_data *data,
+                                   unsigned int sg_len)
+{
+       if (host->dma_64bit_address == 1)
+               dw_mci_prepare_desc64(host, data, sg_len);
+       else
+               dw_mci_prepare_desc32(host, data, sg_len);

I think dw_mci_translate_sglist can be removed.
Instead, these functions should be called in dw_mci_idamc_start_dma().
How about?


Sounds fine, I will do it.

Best Regards,
Jaehoon Chung


        wmb(); /* drain writebuffer */
 }







--
Best Regards
Shawn Lin

Reply via email to