> This seems strange. What is the reason for using this error? > >> + if (pcr->dma_error_count < 8) > Why 8?
dma_error_count record the number of dma transfer error, card clock decreased by 20MHz *dma_error_count, when setting card clock again. while dma_error_count == 8, the clock had reduced from 208MHz to 40MHz The frequency is low enough, if transfer error still occurs, card reset is needed. > What does 0x5227 mean? No magic numbers. Please define it. > >> + card_clock = UHS_SDR104_MAX_DTR - >> + pcr->dma_error_count * 20000000; > What does this do? 0x5227 is the rts5227's device ID, which is a pcie card reader. card clock need to decreased by 20MHz * dma_error_count, because of dma transfer error occured at current frequency. > Looks like you need a comment to describe what you're trying to > achieve. add it at patch v2 steven feng Realsil Microelectronics CO. LTD. Mobile：181-6899-0403 Ext：57594 在 2017年01月04日 01:13, Lee Jones 写道: > On Fri, 09 Dec 2016, [email protected] wrote: > >> From: steven_feng <[email protected]> >> >> 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 <[email protected]> >> --- >> drivers/mfd/rtsx_pcr.c | 15 +++++++++++++-- >> include/linux/mfd/rtsx_pci.h | 2 ++ >> 2 files changed, 15 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mfd/rtsx_pcr.c b/drivers/mfd/rtsx_pcr.c >> index 98029ee..1684e61 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; > This seems strange. What is the reason for using this error? > >> + if (pcr->dma_error_count < 8) > Why 8? > >> + pcr->dma_error_count++; >> + } >> + >> else if (pcr->trans_result == TRANS_NO_DEVICE) >> err = -ENODEV; >> spin_unlock_irqrestore(&pcr->lock, flags); >> @@ -659,6 +664,11 @@ int rtsx_pci_switch_clock(struct rtsx_pcr *pcr, >> unsigned int card_clock, >> if (err < 0) >> return err; >> >> + if (card_clock == UHS_SDR104_MAX_DTR && >> + pcr->dma_error_count && PCI_PID(pcr) == 0x5227) > Tabbing is off here. > > What does 0x5227 mean? No magic numbers. Please define it. > >> + card_clock = UHS_SDR104_MAX_DTR - >> + pcr->dma_error_count * 20000000; > What does this do? > > Looks like you need a comment to describe what you're trying to > achieve. > >> card_clock /= 1000000; >> pcr_dbg(pcr, "Switch card clock to %dMHz\n", card_clock); >> >> @@ -894,6 +904,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..1d65f4e 100644 >> --- a/include/linux/mfd/rtsx_pci.h >> +++ b/include/linux/mfd/rtsx_pci.h >> @@ -957,6 +957,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))
<<attachment: steven_feng.vcf>>
