linux-kernel - Re: [PATCH v2] mfd:rtsx: do retry when dma transfer error

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <E865DE34E934EF47ACC616C10D554F76A80B32E2@RS-MBS01.realsil.com.cn>
Date:   Fri, 6 Jan 2017 03:16:43 +0000
From:   冯伟linux <steven_feng@...lsil.com.cn>
To:     Lee Jones <lee.jones@...aro.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] mfd:rtsx: do retry when dma transfer error

> So this is function is SD card specific?
>
> In which case, why isn't it in the SD card driver?
Dma tans error really because of the wrong  byte sequence rather than
the invalid argument,
so we use EILSEQ instead of EINVAL.
SD and memstick driver are all use this to transmit data, so it was put
in here,

steven feng
Realsil Microelectronics CO. LTD.
Mobile：181-6899-0403  Ext：57594

在 2017年01月05日 22:50, Lee Jones 写道:
> On Thu, 05 Jan 2017, 冯伟linux wrote:
>
>>> +		err = -EILSEQ;
>>> Why are you using this error code?
>>>
>> when kernel receive the error number is EILSEQ, the SD card requeset
>> will be retried for some times, instead of dropping it directly.
>> so we use EILSEQ and frequency reduction to ensure data can be 
>> transmitted correctly as much as possible.
> So this is function is SD card specific?
>
> In which case, why isn't it in the SD card driver?
>
>> steven feng
>> Realsil Microelectronics CO. LTD.
>> Mobile：181-6899-0403  Ext：57594
>>
>> 在 2017年01月04日 20:12, Lee Jones 写道:
>>> On Wed, 04 Jan 2017, steven_feng@...lsil.com.cn wrote:
>>>
>>>> From: steven_feng <steven_feng@...lsil.com.cn>
>>>>
>>>> the request should be reissued when dma transfer error.
>>>> for rts5227, the clock freq need to step reduce when error occurred.
>>>>
>>>> Signed-off-by: steven_feng <steven_feng@...lsil.com.cn>
>>>> ---
>>>>  drivers/mfd/rtsx_pcr.c       | 16 ++++++++++++++--
>>>>  include/linux/mfd/rtsx_pci.h |  4 ++++
>>>>  2 files changed, 18 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/mfd/rtsx_pcr.c b/drivers/mfd/rtsx_pcr.c
>>>> index 98029ee..63ee0ac 100644
>>>> --- a/drivers/mfd/rtsx_pcr.c
>>>> +++ b/drivers/mfd/rtsx_pcr.c
>>>> @@ -30,6 +30,7 @@
>>>>  #include <linux/platform_device.h>
>>>>  #include <linux/mfd/core.h>
>>>>  #include <linux/mfd/rtsx_pci.h>
>>>> +#include <linux/mmc/card.h>
>>>>  #include <asm/unaligned.h>
>>>>  
>>>>  #include "rtsx_pcr.h"
>>>> @@ -452,8 +453,12 @@ int rtsx_pci_dma_transfer(struct rtsx_pcr *pcr, struct scatterlist *sglist,
>>>>  	}
>>>>  
>>>>  	spin_lock_irqsave(&pcr->lock, flags);
>>>> -	if (pcr->trans_result == TRANS_RESULT_FAIL)
>>>> -		err = -EINVAL;
>>>> +	if (pcr->trans_result == TRANS_RESULT_FAIL) {
>>>> +		err = -EILSEQ;
>>> Why are you using this error code?
>>>
>>>> +		if (pcr->dma_error_count < 8)
>>> Please define the 8.
>>>
>>> RTSX_MAX_FREQ_REDUCTIONS or similar.
>>>
>>>> +			pcr->dma_error_count++;
>>>> +	}
>>>> +
>>>>  	else if (pcr->trans_result == TRANS_NO_DEVICE)
>>>>  		err = -ENODEV;
>>>>  	spin_unlock_irqrestore(&pcr->lock, flags);
>>>> @@ -659,6 +664,12 @@ int rtsx_pci_switch_clock(struct rtsx_pcr *pcr, unsigned int card_clock,
>>>>  	if (err < 0)
>>>>  		return err;
>>>>  
>>>> +	/* each time dma transfer error occurs, card clock decreased by 20MHz */
>>> "Each"
>>>
>>>> +	if (card_clock == UHS_SDR104_MAX_DTR &&
>>>> +		pcr->dma_error_count && PCI_PID(pcr) == RTS5227_DEVICE_ID)
>>> Suggest placing "PCI_PID(pcr) == RTS5227_DEVICE_ID" on a new line.
>>>
>>>> +		card_clock = UHS_SDR104_MAX_DTR -
>>>> +			pcr->dma_error_count * 20000000;
>>> Suggest placing parantheis around:
>>>
>>> (UHS_SDR104_MAX_DTR - pcr->dma_error_count)
>>>
>>>>  	card_clock /= 1000000;
>>>>  	pcr_dbg(pcr, "Switch card clock to %dMHz\n", card_clock);
>>>>  
>>>> @@ -894,6 +905,7 @@ static irqreturn_t rtsx_pci_isr(int irq, void *dev_id)
>>>>  			pcr->card_removed |= SD_EXIST;
>>>>  			pcr->card_inserted &= ~SD_EXIST;
>>>>  		}
>>>> +		pcr->dma_error_count = 0;
>>>>  	}
>>>>  
>>>>  	if (int_reg & MS_INT) {
>>>> diff --git a/include/linux/mfd/rtsx_pci.h b/include/linux/mfd/rtsx_pci.h
>>>> index 7eb7cba..751c4042 100644
>>>> --- a/include/linux/mfd/rtsx_pci.h
>>>> +++ b/include/linux/mfd/rtsx_pci.h
>>>> @@ -850,6 +850,8 @@
>>>>  
>>>>  #define rtsx_pci_init_cmd(pcr)		((pcr)->ci = 0)
>>>>  
>>>> +#define RTS5227_DEVICE_ID		0x5227
>>>> +
>>>>  struct rtsx_pcr;
>>>>  
>>>>  struct pcr_handle {
>>>> @@ -957,6 +959,8 @@ struct rtsx_pcr {
>>>>  
>>>>  	int				num_slots;
>>>>  	struct rtsx_slot		*slots;
>>>> +
>>>> +	u8				dma_error_count;
>>>>  };
>>>>  
>>>>  #define CHK_PCI_PID(pcr, pid)		((pcr)->pci->device == (pid))
>> begin:vcard
>> fn;quoted-printable:=E5=86=AF=E4=BC=9F
>> n;quoted-printable:;=E5=86=AF=E4=BC=9F
>> email;internet:steven_feng@...lsil.com.cn
>> tel;cell:18168990403
>> version:2.1
>> end:vcard
>>
>


View attachment "steven_feng.vcf" of type "text/x-vcard" (184 bytes)