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

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] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 22 Apr 2009 01:35:29 +0800
From:	<JosephChan@....com.tw>
To:	<drzeus-mmc@...eus.cx>
Cc:	<linux-kernel@...r.kernel.org>, <arnd@...db.de>,
	<ben-linux@...ff.org>
Subject: RE: [Patch 1/1 v5] via-sdmmc: VIA MSP card reader driver support

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@...eus.cx] 
>> +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);