Hi, Chris > -----Original Message----- > From: Chris Ball [mailto:cjb@xxxxxxxxxx] > Sent: Sunday, February 12, 2012 5:20 AM > To: Huang Changming-R66093 > Cc: linux-mmc@xxxxxxxxxxxxxxx; Jain Priyanka-B32167 > Subject: Re: [PATCH v7] ESDHC: Workaround for data crc error on p1010rdb > > Hi, > > On Wed, Feb 08 2012, Huang Changming-R66093 wrote: > >> > +static void esdhc_of_set_clock(struct sdhci_host *host, unsigned > >> > +int > >> clock) > >> > +{ > >> > + /* Workaround to reduce the clock frequency for p1010 esdhc > */ > >> > + if (of_find_compatible_node(NULL, NULL, "fsl,p1010-esdhc")) { > >> > + if (clock == 0) { > >> > + host->clock = clock; > >> > + return; > >> > + } > >> > >> Is this if (clock == 0) necessary? You'll be calling > >> esdhc_set_clock() immediately afterwards, and it performs the same > test. > > > > Yes, I perform this test two times. > > The first time, I test the original value (as the parameter from > > set_clock) The second time, I test it again, that is because the clock > > value has been changed or reduced, I think it is necessary to test it > > again. > > I don't see how it's necessary to test *if it is zero* again, because > you're not going to change or reduce it to zero. You're going to > subtract 5M/10M from it if it was over 20M/40M, so there is no chance of > it reaching zero. You are right. I want to use the below code in header file "drivers/mmc/host/sdhci-esdhc.h": if (of_find_compatible_node(NULL, NULL, "fsl,p1010-esdhc")) then I can avoid the twice test of the clock. do you think if it is reasonable? Thanks Jerry Huang -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
