From mboxrd@z Thu Jan 1 00:00:00 1970 From: Minkyu Kang Date: Thu, 1 Sep 2011 11:26:30 +0900 Subject: [U-Boot] [PATCH] s5p-mmc: Fix ambiguous setting of data transfer width In-Reply-To: References: <1314701731-19099-1-git-send-email-chander.kashyap@linaro.org> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Dear Andy Fleming, On 1 September 2011 10:23, Andy Fleming wrote: > On Tue, Aug 30, 2011 at 5:55 AM, Chander Kashyap > wrote: >> mmc data transfer width is set as following: >> WIDE8[5]: >> 0 = Depend on WIDE4 >> 1 = 8-bit mode >> WIDE4[1]: >> 1 = 4-bit mode >> 0 = 1-bit mode >> >> In case of 4-bit mode reset 8-bit mode and >> in case of 1-bit mode reset 8-bit mode and 4-bit mode >> >> Signed-off-by: Chander Kashyap >> --- >> ?drivers/mmc/s5p_mmc.c | ? 10 +++++++--- >> ?1 files changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/mmc/s5p_mmc.c b/drivers/mmc/s5p_mmc.c >> index 7786ecf..6e6ad08 100644 >> --- a/drivers/mmc/s5p_mmc.c >> +++ b/drivers/mmc/s5p_mmc.c >> @@ -368,12 +368,16 @@ static void mmc_set_ios(struct mmc *mmc) >> ? ? ? ? * 1 = 4-bit mode >> ? ? ? ? * 0 = 1-bit mode >> ? ? ? ? */ >> - ? ? ? if (mmc->bus_width == 8) >> + ? ? ? if (mmc->bus_width == 8) { >> ? ? ? ? ? ? ? ?ctrl |= (1 << 5); >> - ? ? ? else if (mmc->bus_width == 4) >> + ? ? ? ? ? ? ? ctrl &= ~(1 << 1); > > > I know these were like this before, but those numbers are awfully > magical. You should really define constants for them. We decided to use comments instead of defines. > > Also, this seems like a very confusing way to do this? Why not clear > both bits at the start, and then set the one that is needed? Agreed. Thanks Minkyu Kang -- from. prom. www.promsoft.net