mmc: dw_mmc: init desc in dw_mci_idmac_init

Message ID 1430314316-15971-1-git-send-email-zhangfei.gao@linaro.org
State New
Headers show

Commit Message

zhangfei April 29, 2015, 1:31 p.m.
Set 0 to des1 in 32bit case.
Otherwise the random value of des1 will be used in
dw_mci_translate_sglist: IDMAC_64ADDR_SET_BUFFER1_SIZE(desc, length)

Signed-off-by: Fei Wang <w.f@huawei.com>
Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
---
 drivers/mmc/host/dw_mmc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

zhangfei April 30, 2015, 8:37 a.m. | #1
On 04/30/2015 04:01 PM, Jaehoon Chung wrote:
> Hi.
>
> On 04/29/2015 10:31 PM, Zhangfei Gao wrote:
>> Set 0 to des1 in 32bit case.
>> Otherwise the random value of des1 will be used in
>> dw_mci_translate_sglist: IDMAC_64ADDR_SET_BUFFER1_SIZE(desc, length)
>
> I'm not understanding this comment.
> how does it use the IDMAC_64ADDR_SET_BUFFER1_SIZE(desc, length)?

p->des1 contains random value, coming from dmam_alloc_coherent in 
dw_mci_init_dma

In dw_mci_translate_sglist -> IDMAC_SET_BUFFER1_SIZE(desc, length);
#define IDMAC_SET_BUFFER1_SIZE(d, s) \
         ((d)->des1 = ((d)->des1 & 0x03ffe000) | ((s) & 0x1fff))

The random value will be set to des1 as well as length, causing 
unexpected behavior.

So des1 need to be init to 0.
While des0, des2, des3 are set in dw_mci_idmac_init and 
dw_mci_translate_sglist.

Thanks


>
> Best Regards,
> Jaehoon Chung
>>
>> Signed-off-by: Fei Wang <w.f@huawei.com>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>> ---
>>   drivers/mmc/host/dw_mmc.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 241454f..488a8af 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -589,9 +589,11 @@ static int dw_mci_idmac_init(struct dw_mci *host)
>>   		host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
>>
>>   		/* Forward link the descriptor list */
>> -		for (i = 0, p = host->sg_cpu; i < host->ring_size - 1; i++, p++)
>> +		for (i = 0, p = host->sg_cpu; i < host->ring_size - 1; i++, p++) {
>>   			p->des3 = cpu_to_le32(host->sg_dma +
>>   					(sizeof(struct idmac_desc) * (i + 1)));
>> +			p->des1 = 0;
>> +		}
>>
>>   		/* Set the last descriptor as the end-of-ring descriptor */
>>   		p->des3 = cpu_to_le32(host->sg_dma);
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
zhangfei April 30, 2015, 2:13 p.m. | #2
On 30 April 2015 at 18:28, Jae hoon Chung <jh80.chung@gmail.com> wrote:
>
>
> 2015-04-30 17:37 GMT+09:00 zhangfei <zhangfei.gao@linaro.org>:
>>
>>
>>
>> On 04/30/2015 04:01 PM, Jaehoon Chung wrote:
>>>
>>> Hi.
>>>
>>> On 04/29/2015 10:31 PM, Zhangfei Gao wrote:
>>>>
>>>> Set 0 to des1 in 32bit case.
>>>> Otherwise the random value of des1 will be used in
>>>> dw_mci_translate_sglist: IDMAC_64ADDR_SET_BUFFER1_SIZE(desc, length)
>>>
>>>
>>> I'm not understanding this comment.
>>> how does it use the IDMAC_64ADDR_SET_BUFFER1_SIZE(desc, length)?
>>
>>
>> p->des1 contains random value, coming from dmam_alloc_coherent in
>> dw_mci_init_dma
>>
>> In dw_mci_translate_sglist -> IDMAC_SET_BUFFER1_SIZE(desc, length);
>> #define IDMAC_SET_BUFFER1_SIZE(d, s) \
>>         ((d)->des1 = ((d)->des1 & 0x03ffe000) | ((s) & 0x1fff))
>>
> I think you need to change the commit message
> (IDMAC_SET_BUFFER1_SIZE instead of IDMAC_64ADDR_SET_BUFFER1_SIZE.
>
> You mentioned about desc1, but IDMAC_64ADDR_SET_BUFFER1_SIZE assign to
> desc2.

Yes, made mistake, will change the log.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 241454f..488a8af 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -589,9 +589,11 @@  static int dw_mci_idmac_init(struct dw_mci *host)
 		host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
 
 		/* Forward link the descriptor list */
-		for (i = 0, p = host->sg_cpu; i < host->ring_size - 1; i++, p++)
+		for (i = 0, p = host->sg_cpu; i < host->ring_size - 1; i++, p++) {
 			p->des3 = cpu_to_le32(host->sg_dma +
 					(sizeof(struct idmac_desc) * (i + 1)));
+			p->des1 = 0;
+		}
 
 		/* Set the last descriptor as the end-of-ring descriptor */
 		p->des3 = cpu_to_le32(host->sg_dma);