From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756740AbcHaGzV (ORCPT ); Wed, 31 Aug 2016 02:55:21 -0400 Received: from mailout1.samsung.com ([203.254.224.24]:59855 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752294AbcHaGzR (ORCPT ); Wed, 31 Aug 2016 02:55:17 -0400 X-AuditID: cbfee68d-f79286d000007a9a-16-57c67f53fbe8 Subject: Re: [PATCH 1/2] mmc: dw_mmc: split out preparation of desc for IDMAC32 and IDMAC64 To: Shawn Lin References: <1471599615-6249-1-git-send-email-shawn.lin@rock-chips.com> Cc: Ulf Hansson , linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org, Doug Anderson , Brian Norris , Heiko Stuebner , linux-rockchip@lists.infradead.org From: Jaehoon Chung Message-id: <6e8fbd6b-c3ce-c360-663d-70aeaf87f641@samsung.com> Date: Wed, 31 Aug 2016 15:55:15 +0900 User-Agent: Mozilla/5.0 (X11; Linux i686; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-version: 1.0 In-reply-to: <1471599615-6249-1-git-send-email-shawn.lin@rock-chips.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrFIsWRmVeSWpSXmKPExsWyRsSkQDe4/li4wbU1uhabPr5ntTi77CCb xf9Hr1ktLu+aw2Zx5H8/o8WnB/+ZLe48Wc9qcXxtuAOHx+yGiywed67tYfPYvKTe4++s/Swe 26/NY/b4vEkugC2KyyYlNSezLLVI3y6BK+PNusNMBStMKh5tmM7UwDhJvYuRg0NCwESi8bxU FyMnkCkmceHeerYuRi4OIYEVjBIHp6xjg0iYSPw9+x8qMYtRYnHvfCjnAaPEnXMnmEGqhAVi JV69mwjWISKgIXHj7HUwW0hgLqNE44RwkAZmgb+MEkc/PmAHSbAJ6Ehs/3acCcTmFbCTOPS8 mwXEZhFQlTh6bCWYLSoQJnHy3Dl2iBpBiR+T74HFOQU8JI7+v8sC8gKzgJ7E/YtaIGFmAXmJ zWveMoPskhC4xy7xeN0jdoiZAhLfJh9igXhZVmLTAWaIzyQlDq64wTKBUWwWkg2zEKbOQjJ1 ASPzKkbR1ILkguKk9CJDveLE3OLSvHS95PzcTYzAKDz971nvDsbbB6wPMQpwMCrx8GbMOBou xJpYVlyZe4jRFOiIicxSosn5wFjPK4k3NDYzsjA1MTU2Mrc0UxLnVZT6GSwkkJ5YkpqdmlqQ WhRfVJqTWnyIkYmDU6qBcYf2IcXpDFXr5e9e9F+cKDT/oorsKx3uyDd/p6svk1RwuvnoquSP FnOFd8fufnh2ttRQibWXy4Tll3LLzX8h9+cYKC/dd7Kz5MgCM56AvtRsj4vuSicezHTKbDYw CnoWfILb0YL3QvqB7TK9hyVtWLhazkmeFVBM6otl/L0463ShSWvJAqF2JZbijERDLeai4kQA jHhm570CAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrBIsWRmVeSWpSXmKPExsVy+t9jQd3g+mPhBi8mqlts+vie1eLssoNs Fv8fvWa1uLxrDpvFkf/9jBafHvxntrjzZD2rxfG14Q4cHrMbLrJ43Lm2h81j85J6j7+z9rN4 bL82j9nj8ya5ALaoBkabjNTElNQihdS85PyUzLx0WyXv4HjneFMzA0NdQ0sLcyWFvMTcVFsl F58AXbfMHKB7lBTKEnNKgUIBicXFSvp2mCaEhrjpWsA0Ruj6hgTB9RgZoIGENYwZb9YdZipY YVLxaMN0pgbGSepdjJwcEgImEn/P/meDsMUkLtxbD2RzcQgJzGKUWNw7H8p5wChx59wJZpAq YYFYiVfvJoJ1iAhoSNw4ex3MFhKYyyjROCEcpIFZ4C+jxNGPD9hBEmwCOhLbvx1nArF5Bewk Dj3vZgGxWQRUJY4eWwlmiwqESZw8d44dokZQ4sfke2BxTgEPiaP/7wLZHEBD9STuX9QCCTML yEtsXvOWeQIj0JkIHbMQqmYhqVrAyLyKUSK1ILmgOCk91zAvtVyvODG3uDQvXS85P3cTIzjW n0ntYDy4y/0QowAHoxIP7wPGY+FCrIllxZW5hxglOJiVRHgnVwOFeFMSK6tSi/Lji0pzUosP MZoCvTGRWUo0OR+YhvJK4g2NTcyMLI3MDS2MjM2VxHkf/18XJiSQnliSmp2aWpBaBNPHxMEp 1cBYHOPrub116oOj05zOpwaras0rjnwmz3Rhp1bSTf/fG1Tq29YGWS3sWdI3X4W30XeNgpk6 1+oVF6YK/Hnp029ZsnrBQSc5n/M8hu8PJW1Ze7tNRjlh6krpvOeb0zd1aQpcaYxpio4rcfj3 MWmtaPCS67ofdt9TZWdeyvmxmmPJtb5nj6/vibVRYinOSDTUYi4qTgQAjRwMYgsDAAA= DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > > 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? Best Regards, Jaehoon Chung > > wmb(); /* drain writebuffer */ > } >