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 <shawn.lin at rock-chips.com>
>> ---
>>
>> 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
