Re: [U-Boot] [PATCH] mmc: seperate block number into small parts for multi-write cmd

2010-10-11 Thread Wolfgang Denk
Dear Reinhard Meyer, dear Lei Wen,

In message <4cb2ea80.2090...@emk-elektronik.de> you wrote:
>
> > Ok, I am also fine with not include the 512KiB restriction.
> > So we comes to a conclusion that we still use v1 patch, but cut the
> > 512KiB limitation?
> 
> Considering the comments that were given to that one, yes.

I would like to point out that, from the previous discussion, I
actually think that a 512 KiB limit would be wrong. If the limitation
is really in the alignment, i. e. when crossing a 512 KiB boundary,
then the allowable transfer size depends on the start address, and
512 KiB is actually only the maximum possible value.

> > As mmc host limitation, the max number of block in one go
> > should be limited to 65535, and the max buffer size should
> > not excceed 512k bytes.
> 
> Better reads somehow like this:
> Since some hardware has a 16 bit block counter, split the multiple block
> write into a maximum of (2**16 - 1) blocks to be written at one go.

Agreed.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Einstein argued that there must be simplified explanations of nature,
because God is not capricious or arbitrary. No  such  faith  comforts
the software engineer. - Fred Brooks, Jr.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mmc: seperate block number into small parts for multi-write cmd

2010-10-11 Thread Reinhard Meyer
Dear Lei Wen,

> Ok, I am also fine with not include the 512KiB restriction.
> So we comes to a conclusion that we still use v1 patch, but cut the
> 512KiB limitation?

Considering the comments that were given to that one, yes.

> As mmc host limitation, the max number of block in one go
> should be limited to 65535, and the max buffer size should
> not excceed 512k bytes.

Better reads somehow like this:
Since some hardware has a 16 bit block counter, split the multiple block
write into a maximum of (2**16 - 1) blocks to be written at one go.

> 
> Signed-off-by: Lei Wen 
> ---
>  drivers/mmc/mmc.c |   56 +---
>  1 files changed, 35 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 94d1ac2..ea398a5 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -77,29 +77,16 @@ struct mmc *find_mmc_device(int dev_num)
>   return NULL;
>  }
>  
> -static ulong
> -mmc_bwrite(int dev_num, ulong start, lbaint_t blkcnt, const void*src)
> +int mmc_write_block(struct mmc *mmc, ulong *src, ulong start, uint blocknum)
should be:
static ulong mmc_write_blocks(struct_mmc *mmc, ulong start, lbaint_t blkcnt, 
const void*src)
(keep parameter types and attributes (const) intact and similar to mmc_bwrite)

>  {
>   struct mmc_cmd cmd;
>   struct mmc_data data;
> - int err;
> - int stoperr = 0;
> - struct mmc *mmc = find_mmc_device(dev_num);
> - int blklen;
> -
> - if (!mmc)
> - return -1;
> -
> + int blklen, err;
>   blklen = mmc->write_bl_len;
>  
> - err = mmc_set_blocklen(mmc, mmc->write_bl_len);
> -
> - if (err) {
> - printf("set write bl len failed\n\r");
> - return err;
> - }
> -
> - if (blkcnt > 1)
> + BUG_ON(blklen * blocknum > 524288);
> + BUG_ON(blocknum > 65535);

remove the BUG_ONs

> + if (blocknum > 1)
>   cmd.cmdidx = MMC_CMD_WRITE_MULTIPLE_BLOCK;
>   else
>   cmd.cmdidx = MMC_CMD_WRITE_SINGLE_BLOCK;
> @@ -113,7 +100,7 @@ mmc_bwrite(int dev_num, ulong start, lbaint_t blkcnt, 
> const void*src)
>   cmd.flags = 0;
>  
>   data.src = src;
> - data.blocks = blkcnt;
> + data.blocks = blocknum;

that change not necessary when above parameters are kept.

>   data.blocksize = blklen;
>   data.flags = MMC_DATA_WRITE;
>  
> @@ -124,14 +111,41 @@ mmc_bwrite(int dev_num, ulong start, lbaint_t blkcnt, 
> const void*src)
>   return err;
>   }
>  
> - if (blkcnt > 1) {
> + if (blocknum > 1) {

same here.

>   cmd.cmdidx = MMC_CMD_STOP_TRANSMISSION;
>   cmd.cmdarg = 0;
>   cmd.resp_type = MMC_RSP_R1b;
>   cmd.flags = 0;
> - stoperr = mmc_send_cmd(mmc, &cmd, NULL);
> + mmc_send_cmd(mmc, &cmd, NULL);

should be not keep checking the result for errors?

>   }
>  
> + return blocknum;
> +}
> +



> +static ulong
> +mmc_bwrite(int dev_num, ulong start, lbaint_t blkcnt, const void*src)
> +{
> + int err;
> + struct mmc *mmc = find_mmc_device(dev_num);
> + unsigned int max;

keep related variables the same type: lbainit_t

> + lbaint_t tmp = blkcnt, cur;

tmp is better named: blocks_todo
put cur into the loop

> +
> + if (!mmc)
> + return -1;
> +
> + err = mmc_set_blocklen(mmc, mmc->write_bl_len);
> + if (err) {
> + printf("set write bl len failed\n\r");
> + return err;
> + }
> +
> + max = 524288 / mmc->write_bl_len;
remove
> + do {
> + cur = (tmp > max) ? max : tmp;
lbaint_t chunk = blocks_todo;
if (chunk > 65535)
chunk = 65535;
> + tmp -= cur;
do that at end of loop
> + if(mmc_write_block(mmc, src, start, cur) != cur)
if (mmc_write_blocks(mmc, start, chunk, src) != chunk)
> + return -1;

I think src, start should be adjusted, too...
src += chunk * mmc->write_bl_len;
start += chunk;
blocks_todo -= chunk;
> + } while (blocks_todo > 0);
>   return blkcnt;
>  }
>  

could the functions be reordered such that diff does not mess up so much?
It would be easier to read if the really new function would not be diffed
with the existing function and the modified existing function not seen as
a new one.

I had similar issues with one of my patches and reordered the functions so
diff did "see" it the way it should be... even if that requires a forward
declaration.

Best Regards
Reinhard

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mmc: seperate block number into small parts for multi-write cmd

2010-10-10 Thread Lei Wen
On Sun, Oct 10, 2010 at 5:39 PM, Reinhard Meyer
 wrote:
> Dear Lei Wen,
>>
>> I think it is a generic problem, and I am also curios why other people
>> don't complain for it...
>
> Because it is NOT a generic problem.
>
>> In Linux code, the multi-write write semantic also has such limitation
>> in platform code.
>> So if UBOOT also could have this, that would be so nice...
>
> In PLATFORM code? Not in GENERIC code?
>
>>
>> As you also says that the mmc also server greatly for SD, while the
>> published SD host controller
>> spec has such limitation, I think It is general, right? ...
>
> You REPEATEDLY state this since weeks, but that "specification" is just
> an example of how a minimal controller could be.


Sorry for repeating it again and again, what I intend to do is to let
the issue solved
gracefully.

So should I base on your suggestion add two additional member in struct mmc to
get a final solution?

Thanks,
Lei
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mmc: seperate block number into small parts for multi-write cmd

2010-10-10 Thread Lei Wen
Hi Reinhard,

On Sun, Oct 10, 2010 at 5:33 PM, Reinhard Meyer
 wrote:
> Dear all concerned,
>>
>> But my (limited!) understanding is that it's not a platform problem
>> you are solving, but one of this special kind of controller, which
>> nobody else would ever run into.
>
> I would vouch to extend
>
> struct mmc {
>        struct list_head link;
>        char name[32];
>        void *priv;
>        uint voltages;
>        uint version;
>        uint f_min;
>        uint f_max;
>        int high_capacity;
>        uint bus_width;
>        uint clock;
>        uint card_caps;
>        uint host_caps;
> +       uint host_maxblk;
> +       uint host_maxdmalen;
>        uint ocr;
>        uint scr[2];
>        uint csd[4];
>        uint cid[4];
>        ushort rca;
>        uint tran_speed;
>        uint read_bl_len;
>        uint write_bl_len;
>        u64 capacity;
>        block_dev_desc_t block_dev;
>        int (*send_cmd)(struct mmc *mmc,
>                        struct mmc_cmd *cmd, struct mmc_data *data);
>        void (*set_ios)(struct mmc *mmc);
>        int (*init)(struct mmc *mmc);
> };
>
> have the driver fill those values and the common mmc part act upon them,
> provided they are non-zero. Zero means no limitation on the host side.

I like this modification.
>
> This should not break existing code.
>
> I fail, however, to understand the 512 KiB limit on that hardware. It
> would allow for only 1024 blocks, despite the 65535 allowed by the register.
> Also I assume that limit is about a 512k boundary, not a 512k length???
>
Yes, the 512KiB is intend to be a dma boudary.
The 512 KiB is used by Linux, for it didn't want to handle the DMA
interrupt, I guess so.

Best regards,
Lei
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mmc: seperate block number into small parts for multi-write cmd

2010-10-10 Thread Reinhard Meyer
Dear Lei Wen,
> I think it is a generic problem, and I am also curios why other people
> don't complain for it...

Because it is NOT a generic problem.

> In Linux code, the multi-write write semantic also has such limitation
> in platform code.
> So if UBOOT also could have this, that would be so nice...

In PLATFORM code? Not in GENERIC code?

>
> As you also says that the mmc also server greatly for SD, while the
> published SD host controller
> spec has such limitation, I think It is general, right? ...

You REPEATEDLY state this since weeks, but that "specification" is just
an example of how a minimal controller could be.

Reinhard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mmc: seperate block number into small parts for multi-write cmd

2010-10-10 Thread Reinhard Meyer
Dear all concerned,
> But my (limited!) understanding is that it's not a platform problem
> you are solving, but one of this special kind of controller, which
> nobody else would ever run into.

I would vouch to extend

struct mmc {
struct list_head link;
char name[32];
void *priv;
uint voltages;
uint version;
uint f_min;
uint f_max;
int high_capacity;
uint bus_width;
uint clock;
uint card_caps;
uint host_caps;
+   uint host_maxblk;
+   uint host_maxdmalen;
uint ocr;
uint scr[2];
uint csd[4];
uint cid[4];
ushort rca;
uint tran_speed;
uint read_bl_len;
uint write_bl_len;
u64 capacity;
block_dev_desc_t block_dev;
int (*send_cmd)(struct mmc *mmc,
struct mmc_cmd *cmd, struct mmc_data *data);
void (*set_ios)(struct mmc *mmc);
int (*init)(struct mmc *mmc);
};

have the driver fill those values and the common mmc part act upon them,
provided they are non-zero. Zero means no limitation on the host side.

This should not break existing code.

I fail, however, to understand the 512 KiB limit on that hardware. It
would allow for only 1024 blocks, despite the 65535 allowed by the register.
Also I assume that limit is about a 512k boundary, not a 512k length???

Reinhard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mmc: seperate block number into small parts for multi-write cmd

2010-10-10 Thread Lei Wen
Hi Wolfgang,

On Sun, Oct 10, 2010 at 5:22 PM, Wolfgang Denk  wrote:
> Dear Lei Wen,
>
> In message  you 
> wrote:
>>
>> > Please explain why that shouldnot be possible. It's the driver that
>> > accesses your hardware, so it has full control over everything going
>> > on.
>> >
>> You are right, certainly driver cuold make this, but this would bring
>> a lot of complexity for it...
>> As I mentioned in the last mail, the multi-write command semantic
>> consist by multi-write command + stop transmission command.
>>
>> Send what kind of command is the job of platform, and how to send the
>> command is the job of driver.
>> If I implement this seperation in the driver level, the driver must
>> send another stop transmission command during each internal
>> seperation,
>> which is so ugly and error prone..
>>
>> So this is why I intend to put the code in the platform level...
>
> But my (limited!) understanding is that it's not a platform problem
> you are solving, but one of this special kind of controller, which
> nobody else would ever run into.
>
> Is this understanding correct?
>

I think it is a generic problem, and I am also curios why other people
don't complain for it...
In Linux code, the multi-write write semantic also has such limitation
in platform code.
So if UBOOT also could have this, that would be so nice...

As you also says that the mmc also server greatly for SD, while the
published SD host controller
spec has such limitation, I think It is general, right? ...

Best regards,
Lei
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mmc: seperate block number into small parts for multi-write cmd

2010-10-10 Thread Wolfgang Denk
Dear Lei Wen,

In message  you 
wrote:
> 
> > Please explain why that shouldnot be possible. It's the driver that
> > accesses your hardware, so it has full control over everything going
> > on.
> >
> You are right, certainly driver cuold make this, but this would bring
> a lot of complexity for it...
> As I mentioned in the last mail, the multi-write command semantic
> consist by multi-write command + stop transmission command.
> 
> Send what kind of command is the job of platform, and how to send the
> command is the job of driver.
> If I implement this seperation in the driver level, the driver must
> send another stop transmission command during each internal
> seperation,
> which is so ugly and error prone..
> 
> So this is why I intend to put the code in the platform level...

But my (limited!) understanding is that it's not a platform problem
you are solving, but one of this special kind of controller, which
nobody else would ever run into.

Is this understanding correct?


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Real computer scientists despise the idea of actual  hardware.  Hard-
ware has limitations, software doesn't. It's a real shame that Turing
machines are so poor at I/O.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mmc: seperate block number into small parts for multi-write cmd

2010-10-10 Thread Lei Wen
Hi Wolfgang,

On Sun, Oct 10, 2010 at 2:48 PM, Wolfgang Denk  wrote:
> Dear Lei Wen,
>
> In message  you 
> wrote:
>>
>> Although the common code is named as mmc.c, I think a lot of people use it
>> to work for sd too, since they are compatible in most case.
>
> Right. And where SD drivers differ, such differeng code should be
> provided by the SD drivers, and not pulled into the common code parts.
>
>> the driver cannot do nothing unless a mess change.
>
> Please explain why that shouldnot be possible. It's the driver that
> accesses your hardware, so it has full control over everything going
> on.
>
You are right, certainly driver cuold make this, but this would bring
a lot of complexity for it...
As I mentioned in the last mail, the multi-write command semantic
consist by multi-write command + stop transmission command.

Send what kind of command is the job of platform, and how to send the
command is the job of driver.
If I implement this seperation in the driver level, the driver must
send another stop transmission command during each internal
seperation,
which is so ugly and error prone..

So this is why I intend to put the code in the platform level...

Best regards,
Lei
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mmc: seperate block number into small parts for multi-write cmd

2010-10-09 Thread Wolfgang Denk
Dear Lei Wen,

In message  you 
wrote:
> 
> Although the common code is named as mmc.c, I think a lot of people use it
> to work for sd too, since they are compatible in most case.

Right. And where SD drivers differ, such differeng code should be
provided by the SD drivers, and not pulled into the common code parts.

> the driver cannot do nothing unless a mess change.

Please explain why that shouldnot be possible. It's the driver that
accesses your hardware, so it has full control over everything going
on.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Dealing with failure is easy: work hard to improve. Success  is  also
easy  to  handle:  you've  solved  the  wrong  problem.  Work hard to
improve.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mmc: seperate block number into small parts for multi-write cmd

2010-10-09 Thread Lei Wen
Hi Reinhard,

On Mon, Sep 6, 2010 at 7:50 PM, Reinhard Meyer  wrote:
> Lei Wen schrieb:
> Where does this limitation supposedly come from?
 This limitation comes from the SD/MMC sepc. You could find one and
 check the 0x6 offset register(BLOCK COUNT REGISTER).
>>> This might refer to certain HOST controllers, but not to Cards!
>>
>> You are right, this comes from HOST, not card. But since the host spec
>> define so, then is there any host don't follow the sepc? ...
>
> Any that use bit banging, or SPI, and at least: ATMELs MCI.
> Probably any number of others that do not copy Intel/PC centered
> register structures...
>
> If you had read my first mail on this, there was an example of a 98304
> blocks transfer...

I know that maybe the mmc specific hardware may have no such limitation, but
for our hardware it is sd&mmc cocompatible one, which should follow both spec
and their limitation.

>
>>
>>> And you still do not explain why the buffer size shall be limited to 512KB?
>>
>> The 512 KB comes from the SDMA boundary, and this value is also adopt by 
>> Linux.
>> You could refer to drivers/mmc/host/sdhci.c in Linux code.
>
> What hardware, again? sdhci.c? SDMA?

My hardware is pxa168, which has sdhci controller for control both mmc and sd.

>
> You are trying to change a GENERIC function of U-Boot here to suit a
> particular hardware, not a particular hardware driver.
>
> As such, changes should be generic/configurable and not suited to a specific
> hardware.
>

Although the common code is named as mmc.c, I think a lot of people use it
to work for sd too, since they are compatible in most case.

I also try to put this limitation in the driver level, but no lucky,
without the generic change,
the driver cannot do nothing unless a mess change.
For multi-write, it should first send multi-write command, then follow
stop transmission command.
If I implement the seperation in driver, I need to send the stop
transmission command by self...
Then the "generic" one would make no sense...

> If a specific hardware needs the generic part to obey some limitations, those
> limitations should be configurable, like maybe:
>
> #define CONFIG_SYS_MMC_BLOCKLIMIT 65535
> #define CONFIG_SYS_MMC_BUFFERLIMIT (512<<10)
>

I am ok with set a macro for this.

Best regards,
Lei
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mmc: seperate block number into small parts for multi-write cmd

2010-09-06 Thread Reinhard Meyer
Lei Wen schrieb:
 Where does this limitation supposedly come from?
>>> This limitation comes from the SD/MMC sepc. You could find one and
>>> check the 0x6 offset register(BLOCK COUNT REGISTER).
>> This might refer to certain HOST controllers, but not to Cards!
> 
> You are right, this comes from HOST, not card. But since the host spec
> define so, then is there any host don't follow the sepc? ...

Any that use bit banging, or SPI, and at least: ATMELs MCI.
Probably any number of others that do not copy Intel/PC centered
register structures...

If you had read my first mail on this, there was an example of a 98304
blocks transfer...

> 
>> And you still do not explain why the buffer size shall be limited to 512KB?
> 
> The 512 KB comes from the SDMA boundary, and this value is also adopt by 
> Linux.
> You could refer to drivers/mmc/host/sdhci.c in Linux code.

What hardware, again? sdhci.c? SDMA?

You are trying to change a GENERIC function of U-Boot here to suit a
particular hardware, not a particular hardware driver.

As such, changes should be generic/configurable and not suited to a specific
hardware.

If a specific hardware needs the generic part to obey some limitations, those
limitations should be configurable, like maybe:

#define CONFIG_SYS_MMC_BLOCKLIMIT 65535
#define CONFIG_SYS_MMC_BUFFERLIMIT (512<<10)

Or maybe the splitting/chunking could be resolved in the hardware driver?

Also a clarification of those reasons in the patch comment would be appropriate.

That's my 2 cents for this. Let the community decide if there shall be a general
limitation be put into the generic part...

Best Regards,
Reinhard

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mmc: seperate block number into small parts for multi-write cmd

2010-09-06 Thread Lei Wen
On Mon, Sep 6, 2010 at 6:16 PM, Reinhard Meyer  wrote:
> Lei Wen schrieb:
>> On Mon, Sep 6, 2010 at 5:23 PM, Reinhard Meyer  
>> wrote:
>>> Dear Lei Wen,
 As mmc host limitation, the max number of block in one go
>
> You already write it's a HOST limitation.
>
 should be limited to 65535, and the max buffer size should
 not excceed 512k bytes.
>
> Which would limit the #blocks to 1024 (assuming a 512 byte blocks)
>
>>> Where does this limitation supposedly come from?
>>
>> This limitation comes from the SD/MMC sepc. You could find one and
>> check the 0x6 offset register(BLOCK COUNT REGISTER).
>
> This might refer to certain HOST controllers, but not to Cards!

You are right, this comes from HOST, not card. But since the host spec
define so, then is there any host don't follow the sepc? ...

>
>> You could get that register is only 16bit width and only cound contain
>> the 65535 block.
>>
>> This register is only validate for multi-operation, like multi-read or
>> multi-write command.
>
> Hmm, CMD25 states: "Continuously writes blocks of data until a
> STOP_TRANSMISSION follows."
>
> So the Card itself does not limit the number of blocks written in one go.
>
> I do not mind the transfer to be split into chunks if required by some hosts,
> but the "number of blocks in a go" should be configurable.
>
> And you still do not explain why the buffer size shall be limited to 512KB?

The 512 KB comes from the SDMA boundary, and this value is also adopt by Linux.
You could refer to drivers/mmc/host/sdhci.c in Linux code.

This way makes UBOOT don't need to check the boundary interrupt and
make the process more smooth.

>
> Best Regards,
> Reinhard
>
>
Best regards,
Lei
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mmc: seperate block number into small parts for multi-write cmd

2010-09-06 Thread Reinhard Meyer
Lei Wen schrieb:
> On Mon, Sep 6, 2010 at 5:23 PM, Reinhard Meyer  
> wrote:
>> Dear Lei Wen,
>>> As mmc host limitation, the max number of block in one go

You already write it's a HOST limitation.

>>> should be limited to 65535, and the max buffer size should
>>> not excceed 512k bytes.

Which would limit the #blocks to 1024 (assuming a 512 byte blocks)

>> Where does this limitation supposedly come from?
> 
> This limitation comes from the SD/MMC sepc. You could find one and
> check the 0x6 offset register(BLOCK COUNT REGISTER).

This might refer to certain HOST controllers, but not to Cards!

> You could get that register is only 16bit width and only cound contain
> the 65535 block.
> 
> This register is only validate for multi-operation, like multi-read or
> multi-write command.

Hmm, CMD25 states: "Continuously writes blocks of data until a
STOP_TRANSMISSION follows."

So the Card itself does not limit the number of blocks written in one go.

I do not mind the transfer to be split into chunks if required by some hosts,
but the "number of blocks in a go" should be configurable.

And you still do not explain why the buffer size shall be limited to 512KB?

Best Regards,
Reinhard

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mmc: seperate block number into small parts for multi-write cmd

2010-09-06 Thread Lei Wen
On Mon, Sep 6, 2010 at 5:23 PM, Reinhard Meyer  wrote:
> Dear Lei Wen,
>> As mmc host limitation, the max number of block in one go
>> should be limited to 65535, and the max buffer size should
>> not excceed 512k bytes.
>
> Where does this limitation supposedly come from?

This limitation comes from the SD/MMC sepc. You could find one and
check the 0x6 offset register(BLOCK COUNT REGISTER).
You could get that register is only 16bit width and only cound contain
the 65535 block.

This register is only validate for multi-operation, like multi-read or
multi-write command.

>
> And, 65535 blocks (of 512) are already near 32 MB.
>
> TOP9000> mmci
> mci: setting clock 194000 Hz, block size 512
> mci: setting clock 24832000 Hz, block size 512
> mci: setting clock 194000 Hz, block size 512
> mci: setting clock 194000 Hz, block size 512
> mci: setting clock 24832000 Hz, block size 512
> Device: mci
> Manufacturer ID: 3
> OEM: 5344
> Name: SD32G
> Tran Speed: 2500
> Rd Block Len: 512
> SD version 2.0
> High Capacity: Yes
> Capacity: 31914983424
> Bus Width: 4-bit
> TOP9000> mmc write 0 2000 1000 18000
>
> MMC write: dev # 0, block # 4096, count 98304 ... mci: setting clock 194000 
> Hz,
> block size 512
> mci: setting clock 24832000 Hz, block size 512
> mci: setting clock 194000 Hz, block size 512
> mci: setting clock 194000 Hz, block size 512
> mci: setting clock 24832000 Hz, block size 512
> 98304 blocks written: OK
> TOP9000>
>
> Reinhard
>
>
> ___
> 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


Re: [U-Boot] [PATCH] mmc: seperate block number into small parts for multi-write cmd

2010-09-06 Thread Reinhard Meyer
Dear Lei Wen,
> As mmc host limitation, the max number of block in one go
> should be limited to 65535, and the max buffer size should
> not excceed 512k bytes.

Where does this limitation supposedly come from?

And, 65535 blocks (of 512) are already near 32 MB.

TOP9000> mmci
mci: setting clock 194000 Hz, block size 512
mci: setting clock 24832000 Hz, block size 512
mci: setting clock 194000 Hz, block size 512
mci: setting clock 194000 Hz, block size 512
mci: setting clock 24832000 Hz, block size 512
Device: mci
Manufacturer ID: 3
OEM: 5344
Name: SD32G
Tran Speed: 2500
Rd Block Len: 512
SD version 2.0
High Capacity: Yes
Capacity: 31914983424
Bus Width: 4-bit
TOP9000> mmc write 0 2000 1000 18000

MMC write: dev # 0, block # 4096, count 98304 ... mci: setting clock 194000 Hz,
block size 512
mci: setting clock 24832000 Hz, block size 512
mci: setting clock 194000 Hz, block size 512
mci: setting clock 194000 Hz, block size 512
mci: setting clock 24832000 Hz, block size 512
98304 blocks written: OK
TOP9000>

Reinhard


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mmc: seperate block number into small parts for multi-write cmd

2010-09-06 Thread Lei Wen
On Mon, Sep 6, 2010 at 4:28 PM, Wolfgang Denk  wrote:
> Dear Lei Wen,
>
> In message <1283757567-15161-1-git-send-email-lei...@marvell.com> you wrote:
>> As mmc host limitation, the max number of block in one go
>> should be limited to 65535, and the max buffer size should
>> not excceed 512k bytes.
>
> There is a typo in the Subject ("separate"), and actually I think the
> Subject is a bit misleading. How about "limit block count for
> multi-block write commands" or so?

I agree with that. :-)
>
>
>> -     data.blocks = blkcnt;
>> +     data.blocks = blocknum;
>
> Be careful with your variable names. Most people will read "blocknum"
> as "block number" (and interpret it as an index, i. e. as the logical
> block address on the device), while you probably mean "number of
> block" - the old name "blkcnt" was more appropriate.
>
>
> Why is mmc_write_block() not static?

For mmc.c provide a mmc read api as:
int mmc_read_block(struct mmc *mmc, void *dst, uint blocknum)

So I think it is reasonable to also provide a write api like that,
should it be like?...
Variable blocknum is allow following mmc_read_block style.

>
>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
> SW engineering is a race between programmers trying  to  make  better
> idiot-proof programs and the universe producing greater idiots.
> ___
> 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


Re: [U-Boot] [PATCH] mmc: seperate block number into small parts for multi-write cmd

2010-09-06 Thread Wolfgang Denk
Dear Lei Wen,

In message <1283757567-15161-1-git-send-email-lei...@marvell.com> you wrote:
> As mmc host limitation, the max number of block in one go
> should be limited to 65535, and the max buffer size should
> not excceed 512k bytes.

There is a typo in the Subject ("separate"), and actually I think the
Subject is a bit misleading. How about "limit block count for
multi-block write commands" or so?


> - data.blocks = blkcnt;
> + data.blocks = blocknum;

Be careful with your variable names. Most people will read "blocknum"
as "block number" (and interpret it as an index, i. e. as the logical
block address on the device), while you probably mean "number of
block" - the old name "blkcnt" was more appropriate.


Why is mmc_write_block() not static?


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
SW engineering is a race between programmers trying  to  make  better
idiot-proof programs and the universe producing greater idiots.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot