RE: [Patch 1/1 v5] via-sdmmc: VIA MSP card reader driver support

[JosephChan]

Pierre,

Sorry for the late reply due to my HDD was crashed few days ago.
We will do what you suggest in next release. Please also see the reply in lines. 
Thanks.

BRs,
Joseph 

-----Original Message-----
From: Pierre Ossman [mailto:drzeus-mmc@xxxxxxxxx] 
>> +nodata:
>> +	if (cmd->opcode == MMC_STOP_TRANSMISSION)
>> +		cmdctrl |= VIA_CRDR_SDCTRL_STOP;
> >+

>I didn't check this part properly the last time. I'm assuming you're telling the hardware that this is a command that terminates a data transmission? If so, you cannot check the opcode like that. There might be other stop commands.
>What you need to do is to either pass another parameter to your function when you're calling it with data->stop, or simply have a test in there if cmd == data->stop (or mrq->stop, whichever is most convenient).

OK, we will change to 
       if(cmd == host->mrq->stop)
             cmdctrl |= VIA_CRDR_SDCTRL_STOP;

>> +	else if (ios->clock >=  5000000)
>> +		clock = PCI_CLK_8M;
>>+	else
>>+		clock = PCI_CLK_375K;
>>+

> This looks a bit strange. Why are you not comparing with the clock frequency you'll be setting?

Do you mean that we should check the current clock frequency before setting?  But our engineer thinks that the following lines can do that work normally too:
Any comment? 

              else if (ios->clock >=  5000000)
                     clock = PCI_CLK_8M;
              else
                     clock = PCI_CLK_375K;

              addrbase = host->pcictrl_mmiobase; 

              if (readb(addrbase + VIA_CRDR_PCISDCCLK) != clock)
                     writeb(clock, addrbase + VIA_CRDR_PCISDCCLK);


>> +
>> +	mmc->f_min = VIA_CRDR_MIN_CLOCK;
>> +	mmc->f_max = VIA_CRDR_MAX_CLOCK;
>> +	mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
>> +	mmc->caps = MMC_CAP_4_BIT_DATA;

> You also need to set the bits stating if the controller supports the high-speed SD and/or MMC mode, otherwise the core won't go over 25/20 MHz.


OK, we will add the following line in the via_init_mmc_host function:

       mmc->caps |= MMC_CAP_MMC_HIGHSPEED |MMC_CAP_SD_HIGHSPEED;

And, we will add the following source code to the via_sdc_set_ios function: 

       org_data = readl(addrbase + VIA_CRDR_SDEXTCTRL);

       if ((ios->timing == MMC_TIMING_SD_HS) || (ios->timing == MMC_TIMING_MMC_HS))
              org_data |= VIA_CRDR_SDEXTCTRL_HISPD;
       else
              org_data &= ~VIA_CRDR_SDEXTCTRL_HISPD;

       writel(org_data, addrbase + VIA_CRDR_SDEXTCTRL);       
¢éì®&Þ~º&¶¬–+-±éÝ¥Šw®žË±Êâmébžìdz¹Þ)í…æèw*jg¬±¨¶‰šŽŠÝj/êäz¹ÞŠà2ŠÞ¨è­Ú&¢)ß«a¶Úþø®G«éh®æj:+v‰¨Šwè†Ù>Wš±êÞiÛaxPjØm¶ŸÿÃ-»+ƒùdš_