tmio_mmc: Revise a limit of the data size

PATCH In review Yusuke Goda 5 participants 10 messages Jul 7, 2010 – Aug 26, 2010

Subsystems (2)

MMC, SD and SDIO TMIO/SDHI MMC driver

lore

Share & tools

lore

Yusuke Goda

author

2010-07-07 02:01 UTC #0 lore raw

PATCH 1/1 tmio_mmc: Revise a limit of the data size

Signed-off-by Yusuke Goda <[email protected]>

---
 drivers/mmc/host/tmio_mmc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/mmc/host/tmio_mmc.c +1 -1

index ee7d0a5..cac1c97 100644
@@ -661,7 +661,7 @@ static int tmio_mmc_start_data(struct tmio_mmc_host *host,
 		 data->blksz, data->blocks);
 
 	/* Hardware cannot perform 1 and 2 byte requests in 4 bit mode */
-	if (data->blksz < 4 && host->mmc->ios.bus_width == MMC_BUS_WIDTH_4) {
+	if (data->blksz < 2 && host->mmc->ios.bus_width == MMC_BUS_WIDTH_4) {
 		pr_err("%s: %d byte block unsupported in 4 bit mode\n",
 		       mmc_hostname(host->mmc), data->blksz);
 		return -EINVAL;

Signature · 1 line

1.7.0

Andrew Morton

2010-07-08 21:47 UTC #1 lore raw

Quoted 10 lines · On Wed, 07 Jul 2010 11:01:20 +0900 Yusuke Goda <[email protected]>…

Signed-off-by Yusuke Goda <[email protected]>
---
 drivers/mmc/host/tmio_mmc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/mmc/host/tmio_mmc.c +0 -0
index ee7d0a5..cac1c97 100644

drivers/mmc/host/tmio_mmc.c static int tmio_mmc_start_data(struct tmio_mmc_host *host, · line 667

Quoted 8 lines

drivers/mmc/host/tmio_mmc.c +1 -1

@@ -661,7 +661,7 @@ static int tmio_mmc_start_data(struct tmio_mmc_host *host,
 		 data->blksz, data->blocks);
 
 	/* Hardware cannot perform 1 and 2 byte requests in 4 bit mode */
-	if (data->blksz < 4 && host->mmc->ios.bus_width == MMC_BUS_WIDTH_4) {
+	if (data->blksz < 2 && host->mmc->ios.bus_width == MMC_BUS_WIDTH_4) {
 		pr_err("%s: %d byte block unsupported in 4 bit mode\n",
 		       mmc_hostname(host->mmc), data->blksz);
 		return -EINVAL;

Again, please provide a suitable description for this change.

Yusuke Goda

author

2010-07-13 02:33 UTC #2 lore raw

Hi Andrew

Quoted 15 lines · Andrew Morton

Quoted 12 lines · On Wed, 07 Jul 2010 11:01:20 +0900 Yusuke Goda <[email protected]>…
Signed-off-by Yusuke Goda <[email protected]>
---
 drivers/mmc/host/tmio_mmc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/mmc/host/tmio_mmc.c +0 -0
index ee7d0a5..cac1c97 100644
@@ -661,7 +661,7 @@ static int tmio_mmc_start_data(struct tmio_mmc_host *host,
 		 data->blksz, data->blocks);
 

Quoted 8 lines

drivers/mmc/host/tmio_mmc.c static int tmio_mmc_start_data(struct tmio_mmc_host *host, · line 667
Quoted 6 lines
drivers/mmc/host/tmio_mmc.c +1 -1
 	/* Hardware cannot perform 1 and 2 byte requests in 4 bit mode */
-	if (data->blksz < 4 && host->mmc->ios.bus_width == MMC_BUS_WIDTH_4) {
+	if (data->blksz < 2 && host->mmc->ios.bus_width == MMC_BUS_WIDTH_4) {
 		pr_err("%s: %d byte block unsupported in 4 bit mode\n",
 		       mmc_hostname(host->mmc), data->blksz);
 		return -EINVAL;
Again, please provide a suitable description for this change.

I think the data size is not limited by MMC_BUS_WIDTH_x.
I confirmed that data transmission of 2Byte was performed without a problem.

Thanks,
Goda

Matt Fleming

2010-08-25 21:52 UTC #3 lore raw

Quoted 26 lines · On Tue, 13 Jul 2010 11:32:33 +0900 Yusuke Goda <[email protected]>…

Hi Andrew
Quoted 15 lines · Andrew Morton
Quoted 12 lines · On Wed, 07 Jul 2010 11:01:20 +0900 Yusuke Goda <[email protected]>…
Signed-off-by Yusuke Goda <[email protected]>
---
 drivers/mmc/host/tmio_mmc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/mmc/host/tmio_mmc.c +0 -0
index ee7d0a5..cac1c97 100644
@@ -661,7 +661,7 @@ static int tmio_mmc_start_data(struct tmio_mmc_host *host,
 		 data->blksz, data->blocks);
 
Quoted 8 lines
drivers/mmc/host/tmio_mmc.c static int tmio_mmc_start_data(struct tmio_mmc_host *host, · line 667
Quoted 6 lines
drivers/mmc/host/tmio_mmc.c +1 -1
 	/* Hardware cannot perform 1 and 2 byte requests in 4 bit mode */
-	if (data->blksz < 4 && host->mmc->ios.bus_width == MMC_BUS_WIDTH_4) {
+	if (data->blksz < 2 && host->mmc->ios.bus_width == MMC_BUS_WIDTH_4) {
 		pr_err("%s: %d byte block unsupported in 4 bit mode\n",
 		       mmc_hostname(host->mmc), data->blksz);
 		return -EINVAL;
Again, please provide a suitable description for this change.

Quoted 2 lines

I think the data size is not limited by MMC_BUS_WIDTH_x.
I confirmed that data transmission of 2Byte was performed without a problem.

This patch hasn't been picked up. I'm assuming that's because Andrew is still unhappy with the changelog.

Andrew, how about something like,

"When running in 4-bit bus width mode, it is entirely possible to transfer data in block sizes of 2 bytes and larger. Relax the conditional check to allow 2-byte data block transfers which were previously disallowed."

Yusuke, have I interpreted your changelog correctly? Also note that your patch should remove the comment above the conditional that says, "Hardware cannot perform 1 and 2 byte requests in 4 bit mode".

Andrew Morton

2010-08-25 22:05 UTC #4 lore raw

Quoted 29 lines · On Wed, 25 Aug 2010 22:52:08 +0100 Matt Fleming <[email protected]> wrote:

Quoted 26 lines · On Tue, 13 Jul 2010 11:32:33 +0900 Yusuke Goda <[email protected]>…
Hi Andrew
Quoted 23 lines · Andrew Morton
Quoted 10 lines · On Wed, 07 Jul 2010 11:01:20 +0900 Yusuke Goda <[email protected]>…
Signed-off-by Yusuke Goda <[email protected]>
---
 drivers/mmc/host/tmio_mmc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/mmc/host/tmio_mmc.c +0 -0
index ee7d0a5..cac1c97 100644
drivers/mmc/host/tmio_mmc.c static int tmio_mmc_start_data(struct tmio_mmc_host *host, · line 667
Quoted 8 lines
drivers/mmc/host/tmio_mmc.c +1 -1
@@ -661,7 +661,7 @@ static int tmio_mmc_start_data(struct tmio_mmc_host *host,
 		 data->blksz, data->blocks);
 
 	/* Hardware cannot perform 1 and 2 byte requests in 4 bit mode */
-	if (data->blksz < 4 && host->mmc->ios.bus_width == MMC_BUS_WIDTH_4) {
+	if (data->blksz < 2 && host->mmc->ios.bus_width == MMC_BUS_WIDTH_4) {
 		pr_err("%s: %d byte block unsupported in 4 bit mode\n",
 		       mmc_hostname(host->mmc), data->blksz);
 		return -EINVAL;
Again, please provide a suitable description for this change.

Quoted 5 lines

Quoted 2 lines
I think the data size is not limited by MMC_BUS_WIDTH_x.
I confirmed that data transmission of 2Byte was performed without a problem.
This patch hasn't been picked up. I'm assuming that's because Andrew is still unhappy with the changelog.

Actually I don't know what happened with this. I merged it on July 8 and appear to have dropped it on July 27, but I can't find its removed-from-mm email so I don't know why I dropped it. Weird.

Oh well, I merged it again. Is 2.6.37 an appropriate merge schedule?

Also, I tend not to handle tmio_mmc patches - usually Paul patches that driver.

Quoted 6 lines

Andrew, how about something like,
"When running in 4-bit bus width mode, it is entirely possible to transfer data in block sizes of 2 bytes and larger. Relax the conditional check to allow 2-byte data block transfers which were previously disallowed."

thanks.

Quoted 3 lines

Yusuke, have I interpreted your changelog correctly? Also note that your patch should remove the comment above the conditional that says, "Hardware cannot perform 1 and 2 byte requests in 4 bit mode".

this?

drivers/mmc/host/tmio_mmc.c renamed from drivers/mmc/host/tmio_mmc.c~tmio_mmc-revise-a-limit-of-the-data-size-fix +0 -1

@@ -660,7 +660,6 @@ static int tmio_mmc_start_data(struct tm
 	pr_debug("setup data transfer: blocksize %08x  nr_blocks %d\n",
 		 data->blksz, data->blocks);
 
-	/* Hardware cannot perform 1 and 2 byte requests in 4 bit mode */
 	if (data->blksz < 2 && host->mmc->ios.bus_width == MMC_BUS_WIDTH_4) {
 		pr_err("%s: %d byte block unsupported in 4 bit mode\n",
 		       mmc_hostname(host->mmc), data->blksz);

Matt Fleming

2010-08-25 22:19 UTC #5 lore raw

Quoted 3 lines · On Wed, 25 Aug 2010 15:05:02 -0700 Andrew Morton <[email protected]>…

Actually I don't know what happened with this. I merged it on July 8 and appear to have dropped it on July 27, but I can't find its removed-from-mm email so I don't know why I dropped it. Weird.

Oh, interesting.

Quoted 1 line

Oh well, I merged it again. Is 2.6.37 an appropriate merge schedule?

I'll defer that to Goda and Paul but that seems reasonable to me.

Quoted 16 lines

Quoted 3 lines
Yusuke, have I interpreted your changelog correctly? Also note that your patch should remove the comment above the conditional that says, "Hardware cannot perform 1 and 2 byte requests in 4 bit mode".
this?
drivers/mmc/host/tmio_mmc.c renamed from drivers/mmc/host/tmio_mmc.c~tmio_mmc-revise-a-limit-of-the-data-size-fix +0 -1
@@ -660,7 +660,6 @@ static int tmio_mmc_start_data(struct tm
 	pr_debug("setup data transfer: blocksize %08x  nr_blocks %d\n",
 		 data->blksz, data->blocks);
 
-	/* Hardware cannot perform 1 and 2 byte requests in 4 bit mode */
 	if (data->blksz < 2 && host->mmc->ios.bus_width == MMC_BUS_WIDTH_4) {
 		pr_err("%s: %d byte block unsupported in 4 bit mode\n",
 		       mmc_hostname(host->mmc), data->blksz);

Quoted 2 lines

_

I was thinking more like changing the comment to say "Hardware cannot perform 1 byte requests in 4 bit mode" but that's evident from the check. So sure, removing it seems sane.

Ian Molton

2010-08-26 01:50 UTC #6 lore raw

In reply to Andrew Morton

Probably dropped because I complained about two things,

1) The patch doesnt check what the hardware is underneath before arbitrarily changing the parameters relied on by some hardware

2) Removing a comment I put there for a good reason without explaining why.

"It works on my hardware" is not a good enough excuse.

Signature · 3 lines

Ian Molton
Linux, Automotive, and other hacking:
http://www.mnementh.co.uk/

Quoted 75 lines · Andrew Morton

Quoted 29 lines · On Wed, 25 Aug 2010 22:52:08 +0100 Matt Fleming <[email protected]> wrote:
Quoted 26 lines · On Tue, 13 Jul 2010 11:32:33 +0900 Yusuke Goda <[email protected]>…
Hi Andrew
Quoted 23 lines · Andrew Morton
Quoted 10 lines · On Wed, 07 Jul 2010 11:01:20 +0900 Yusuke Goda <[email protected]>…
Signed-off-by Yusuke Goda <[email protected]>
---
drivers/mmc/host/tmio_mmc.c | 2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/mmc/host/tmio_mmc.c +0 -0
index ee7d0a5..cac1c97 100644
drivers/mmc/host/tmio_mmc.c static int tmio_mmc_start_data(struct tmio_mmc_host *host, · line 667
Quoted 8 lines
drivers/mmc/host/tmio_mmc.c +1 -1
@@ -661,7 +661,7 @@ static int tmio_mmc_start_data(struct tmio_mmc_host *host,
                   data->blksz, data->blocks);
 
          /* Hardware cannot perform 1 and 2 byte requests in 4 bit mode */
-        if (data->blksz < 4 && host->mmc->ios.bus_width == MMC_BUS_WIDTH_4) {
+        if (data->blksz < 2 && host->mmc->ios.bus_width == MMC_BUS_WIDTH_4) {
                  pr_err("%s: %d byte block unsupported in 4 bit mode\n",
                         mmc_hostname(host->mmc), data->blksz);
                  return -EINVAL;
Again, please provide a suitable description for this change.
Quoted 5 lines
Quoted 2 lines
I think the data size is not limited by MMC_BUS_WIDTH_x.
I confirmed that data transmission of 2Byte was performed without a problem.
This patch hasn't been picked up. I'm assuming that's because Andrew is still unhappy with the changelog.
Actually I don't know what happened with this. I merged it on July 8 and appear to have dropped it on July 27, but I can't find its removed-from-mm email so I don't know why I dropped it. Weird.
Oh well, I merged it again. Is 2.6.37 an appropriate merge schedule?
Also, I tend not to handle tmio_mmc patches - usually Paul patches that driver.
Quoted 6 lines
Andrew, how about something like,
"When running in 4-bit bus width mode, it is entirely possible to transfer data in block sizes of 2 bytes and larger. Relax the conditional check to allow 2-byte data block transfers which were previously disallowed."
thanks.
Quoted 3 lines
Yusuke, have I interpreted your changelog correctly? Also note that your patch should remove the comment above the conditional that says, "Hardware cannot perform 1 and 2 byte requests in 4 bit mode".
this?
drivers/mmc/host/tmio_mmc.c renamed from drivers/mmc/host/tmio_mmc.c~tmio_mmc-revise-a-limit-of-the-data-size-fix +0 -1
@@ -660,7 +660,6 @@ static int tmio_mmc_start_data(struct tm
        pr_debug("setup data transfer: blocksize %08x  nr_blocks %d\n",
                 data->blksz, data->blocks);
 
-       /* Hardware cannot perform 1 and 2 byte requests in 4 bit mode */
        if (data->blksz < 2 && host->mmc->ios.bus_width == MMC_BUS_WIDTH_4) {
                pr_err("%s: %d byte block unsupported in 4 bit mode\n",
                       mmc_hostname(host->mmc), data->blksz);
_

Matt Fleming

2010-08-26 07:24 UTC #7 lore raw

Quoted 4 lines · Ian Molton

Probably dropped because I complained about two things,
1) The patch doesnt check what the hardware is underneath before arbitrarily changing the parameters relied on by some hardware

Has anyone thought about adding a caps or flags field to the tmio_mmc_host structure so that platform code set a bit if it can't do 2-byte data block transfers?

Magnus Damm

2010-08-26 07:26 UTC #8 lore raw

Quoted 9 lines · Matt Fleming

Quoted 4 lines · Ian Molton
Probably dropped because I complained about two things,
1) The patch doesnt check what the hardware is underneath before arbitrarily changing the parameters relied on by some hardware
Has anyone thought about adding a caps or flags field to the tmio_mmc_host structure so that platform code set a bit if it can't do 2-byte data block transfers?

Good idea! There is already a good place available:

Quoted 1 line

From include/linux/mfd/tmio.h:

...
/* tmio MMC platform flags */
#define TMIO_MMC_WRPROTECT_DISABLE (1 << 0)

Cheers,

/ magnus

Ian Molton

2010-08-26 10:28 UTC #9 lore raw

@Matt: yes, thats what I was suggesting before.

@Magnus: that looks fine.

Signature · 3 lines

Ian Molton
Linux, Automotive, and other hacking:
http://www.mnementh.co.uk/

Quoted 22 lines · Magnus Damm

Quoted 9 lines · Matt Fleming
Quoted 4 lines · Ian Molton
Probably dropped because I complained about two things,
1) The patch doesnt check what the hardware is underneath before arbitrarily changing the parameters relied on by some hardware
Has anyone thought about adding a caps or flags field to the tmio_mmc_host structure so that platform code set a bit if it can't do 2-byte data block transfers?
Good idea! There is already a good place available:
From include/linux/mfd/tmio.h:
...
/* tmio MMC platform flags */
#define TMIO_MMC_WRPROTECT_DISABLE (1 << 0)
Cheers,
/ magnus