Hi Andrew, Peng, > On Mar 23, 2015, at 01:23 , Andrew Gabbasov <andrew_gabba...@mentor.com> > wrote: > > Hi Peng, > >> From: Peng Fan [mailto:peng....@freescale.com] >> Sent: Saturday, March 21, 2015 1:34 PM >> To: Gabbasov, Andrew; u-boot@lists.denx.de >> Subject: Re: [U-Boot] [PATCH] mmc: fix OCR Polling >> >> [skipped] >> >>> After this patch, if the busy flag is indeed already set (so that the >>> loop body is not executed), and it is not an SPI case >>> (mmc_host_is_spi(mmc) is false), the "cmd" structure (which is local >>> to mmc_complete_op_cond() function) is left uninitialized, and using >>> cmd.response[0] later in the function becomes incorrect. And the OCR >>> register value and the high capacity flag may be set incorrectly. >> Yeah. you are right. >> Maybe the following piece of code should be added to replace mmc->ocr = >> cmd.response[0]: >> " >> if (mmc_host_is_spi(mmc)) >> mmc->ocr = cmd.response[0]; >> else >> mmc->ocr = mmc->op_cond_response; >> " >> Thanks for correcting me. > > Well, there can be several ways to correct that. The easiest would be > something > similar to what you propose, but, just to avoid extra "if", we could add > mmc->op_cond_response = cmd.response[0]; > to the end of existing "if(mmc_host_is_spi(mmc))" and change > mmc->ocr = cmd.response[0]; > to > mmc->ocr = mmc->op_cond_response; > at the end of function. Since op_cond_response should be already set from > the function beginning, this can be used immediately. > > And, going further, since op_cond_response is actually the same contents > as mmc->ocr, we could combine them and use mmc->ocr at once, from > the beginning of polling loops. This is a little more complex, but makes > the code cleaner. This is what is done in one of other patches in my series > ;-) >
This does seem like a case where a simple accessor structure would help until we figure out how to process. Something like mmc_get_ocr() as a private API perhaps? > Thanks. > > Best regards, > Andrew > Regards — Pantelis > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
