Skip to contentSkip to conversation

MMC: logical-bitwise and confusion in tifm_sd_transfer_data()?

PATCH In review Roel Kluin 3 participants 5 messages Mar 9, 2008 – Mar 15, 2008

Subsystems (2)
lore
Share & tools
lore

Roel Kluin

author
#0 lore raw

from drivers/mmc/host/tifm_sd.c:183:

if ((r_data->flags & MMC_DATA_WRITE)

    && DATA_CARRY)

shouldn't this be bit-wise &?

#1 lore raw
Quoted 6 lines · On Sun, 09 Mar 2008 21:31:08 +0100 Roel Kluin <[email protected]> wrote:

from drivers/mmc/host/tifm_sd.c:183:

if ((r_data->flags & MMC_DATA_WRITE)

    && DATA_CARRY)

shouldn't this be bit-wise &?

First off, I'm not the maintainer of that driver, Alex Dubov is.

Second, the code seems broken, but not in the way you suggest. It should probably have been:

if ((r_data->flags & MMC_DATA_WRITE)

	&& (host->cmd_flags & DATA_CARRY))

Alex, could you have a look?

Rgds

Signature · 5 lines
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org
maintainer
#2 lore raw
Quoted 8 lines · --- Pierre Ossman
Quoted 5 lines · On Sun, 09 Mar 2008 21:31:08 +0100 Roel Kluin <[email protected]> wrote:

from drivers/mmc/host/tifm_sd.c:183:

if ((r_data->flags & MMC_DATA_WRITE)

    && DATA_CARRY)
Quoted 9 lines
Quoted 1 line

shouldn't this be bit-wise &?

First off, I'm not the maintainer of that driver, Alex Dubov is.

Second, the code seems broken, but not in the way you suggest. It should probably have been:

if ((r_data->flags & MMC_DATA_WRITE)

	&& (host->cmd_flags & DATA_CARRY))

Yes, this is the intended meaning. Can you fix it yourself or should I send a separate patch?

      ____________________________________________________________________________________

Looking for last minute shopping deals?
Find them fast with Yahoo! Search. http://tools.search.yahoo.com/newsearch/category.php?category=shopping

author
#3 lore raw

PATCH 1/1 MMC: DATA_CARRY is not boolean in tifm_sd_transfer_data() (was: MMC: logical-bitwise and confusion in tifm_sd_transfer_data()?)

Quoted 13 lines · Alex Dubov
Quoted 11 lines · --- Pierre Ossman
Quoted 6 lines · On Sun, 09 Mar 2008 21:31:08 +0100 Roel Kluin <[email protected]> wrote:

from drivers/mmc/host/tifm_sd.c:183:

if ((r_data->flags & MMC_DATA_WRITE)

    && DATA_CARRY)

shouldn't this be bit-wise &?

First off, I'm not the maintainer of that driver, Alex Dubov is.

Quoted 7 lines
Quoted 5 lines

Second, the code seems broken, but not in the way you suggest. It should probably have been:

if ((r_data->flags & MMC_DATA_WRITE)

	&& (host->cmd_flags & DATA_CARRY))

Yes, this is the intended meaning. Can you fix it yourself or should I send a separate patch?

Thanks,
---
DATA_CARRY is not boolean

---

drivers/mmc/host/tifm_sd.c +1 -1
index 20d5c7b..1c14a18 100644
@@ -180,7 +180,7 @@ static void tifm_sd_transfer_data(struct tifm_sd *host)
 			host->sg_pos++;
 			if (host->sg_pos == host->sg_len) {
 				if ((r_data->flags & MMC_DATA_WRITE)
-				    && DATA_CARRY)
+				    && (host->cmd_flags & DATA_CARRY))
 					writel(host->bounce_buf_data[0],
 					       host->dev->addr
 					       + SOCK_MMCSD_DATA);
#4 lore raw

Re: [PATCH] MMC: DATA_CARRY is not boolean in tifm_sd_transfer_data() (was: MMC: logical-bitwise and confusion in tifm_sd_transfer_data()?)

Quoted 10 lines · On Tue, 11 Mar 2008 09:00:41 +0100 Roel Kluin <[email protected]> wrote:
Quoted 2 lines · Alex Dubov

Yes, this is the intended meaning. Can you fix it yourself or should I send a separate patch?

Thanks,
---
DATA_CARRY is not boolean

---

Sorry about the delay. I've sent this off to Linus now.

Rgds

Signature · 5 lines
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org