[U-Boot] MMC: proposal to support multiple physical partitions

In addition to using the MMC "user area" (the device's physical partition), I am also using the "first MMC boot partition" and the "second MMC boot partition"... As a result, I am currently using the following code snippet in a number of places:
err = -1; if (mmc->part_num != part_num) { if (mmc_switch_part(dev_num, part_num)) { printf("%s: MMC partition switch to %d failed\n", __func__, part_num); err = 0; } }
if (err != 0) { err = mmc->block_dev.block_read(dev_num, start, blkcnt, buffer); }
if (mmc->part_num != part_num) { if (mmc_switch_part(dev_num, mmc->part_num)) { printf("%s: MMC partition switching back from %d failed\n", __func__, part_num); } }
I have two different proposals: 1) overload the "int dev_num" argument with encoded "dev_num" and "part_num" fields
- the dev_num in the [15:0] bits,
- the part_num in the [30:16] bits,
- a flag to indicate this encoding in [31] bit.
- and modify mmc_bread() to handle this encoded argument, and implement the above code... 2) create a wrapper function to perform the above code, with an added argument "int part_num", possibly named:
- mmc_block_dev_block_read() -- so that it is similar to the original calling convention [mmc->block_dev.block_read], or
- mmc_pbread() [PartitionBlockRead] -- so that it is similar to the mmc_bread() [which is the implementation of the callback function]
Also, would implement this solution for mmc->block_dev.block_write() and mmc->block_dev.block_erase() too.
Either proposals would affect:
include/mmc.h
drivers/mmc/mmc.c
drivers/mmc/mmc_write.c
Would either of these proposals be acceptable to upstream?
Thanks in advance, Steve

Hi Steve,
On Apr 24, 2014, at 8:50 PM, Steve Rae wrote:
In addition to using the MMC "user area" (the device's physical partition), I am also using the "first MMC boot partition" and the "second MMC boot partition"... As a result, I am currently using the following code snippet in a number of places:
err = -1; if (mmc->part_num != part_num) { if (mmc_switch_part(dev_num, part_num)) { printf("%s: MMC partition switch to %d failed\n", __func__, part_num); err = 0; } }
if (err != 0) { err = mmc->block_dev.block_read(dev_num, start, blkcnt, buffer); }
if (mmc->part_num != part_num) { if (mmc_switch_part(dev_num, mmc->part_num)) { printf("%s: MMC partition switching back from %d failed\n", __func__, part_num); } }
I have two different proposals:
- overload the "int dev_num" argument with encoded "dev_num" and "part_num" fields
the dev_num in the [15:0] bits,
the part_num in the [30:16] bits,
a flag to indicate this encoding in [31] bit.
and modify mmc_bread() to handle this encoded argument, and implement the above code...
- create a wrapper function to perform the above code, with an added argument "int part_num", possibly named:
mmc_block_dev_block_read() -- so that it is similar to the original calling convention [mmc->block_dev.block_read], or
mmc_pbread() [PartitionBlockRead] -- so that it is similar to the mmc_bread() [which is the implementation of the callback function]
I'd rather go with the wrapper function. Perhaps it's not even needed. The function called is part of the block_dev (block_read/write etc).
Overwrite those with functions that implemented the switching first, and then call the original block* function.
Also, would implement this solution for mmc->block_dev.block_write() and mmc->block_dev.block_erase() too. Either proposals would affect: include/mmc.h drivers/mmc/mmc.c drivers/mmc/mmc_write.c
Would either of these proposals be acceptable to upstream? Thanks in advance, Steve
Anything that cleans things up is acceptable.
Regards
-- Pantelis

Thanks for the response,
-----Original Message----- From: Pantelis Antoniou [mailto:panto@antoniou-consulting.com] Sent: Tuesday, April 29, 2014 10:25 To: Steve Rae Cc: u-boot@lists.denx.de; trini@ti.com; albert.u.boot@aribaud.net Subject: Re: MMC: proposal to support multiple physical partitions
Hi Steve,
On Apr 24, 2014, at 8:50 PM, Steve Rae wrote:
[... snip ...]
I have two different proposals:
- overload the "int dev_num" argument with encoded "dev_num" and
"part_num" fields
the dev_num in the [15:0] bits,
the part_num in the [30:16] bits,
a flag to indicate this encoding in [31] bit.
and modify mmc_bread() to handle this encoded argument, and
implement the above code...
- create a wrapper function to perform the above code, with an added
argument "int part_num", possibly named:
mmc_block_dev_block_read() -- so that it is similar to the original
calling convention [mmc->block_dev.block_read], or
mmc_pbread() [PartitionBlockRead] -- so that it is similar to the
mmc_bread() [which is the implementation of the callback function]
I'd rather go with the wrapper function. Perhaps it's not even needed. The function called is part of the block_dev (block_read/write etc).
Overwrite those with functions that implemented the switching first, and then call the original block* function.
The callback function is: mmc->block_dev.block_read = mmc_bread and it accepts four arguments: include/part.h: unsigned long (*block_read)(int dev, include/part.h- lbaint_t start, include/part.h- lbaint_t blkcnt, include/part.h- void *buffer); Are you suggesting that I should add another argument to this callback?
[...snip...]
Regards
-- Pantelis

Pantelis,
As per comments below, I suspect that adding another argument to the callback function is unacceptable. Please clarify!
Thanks, Steve
On 14-04-29 11:08 AM, Steve Rae wrote:
Thanks for the response,
-----Original Message----- From: Pantelis Antoniou [mailto:panto@antoniou-consulting.com] Sent: Tuesday, April 29, 2014 10:25 To: Steve Rae Cc: u-boot@lists.denx.de; trini@ti.com; albert.u.boot@aribaud.net Subject: Re: MMC: proposal to support multiple physical partitions
Hi Steve,
On Apr 24, 2014, at 8:50 PM, Steve Rae wrote:
[... snip ...]
I have two different proposals:
- overload the "int dev_num" argument with encoded "dev_num" and
"part_num" fields
the dev_num in the [15:0] bits,
the part_num in the [30:16] bits,
a flag to indicate this encoding in [31] bit.
and modify mmc_bread() to handle this encoded argument, and
implement the above code...
- create a wrapper function to perform the above code, with an added
argument "int part_num", possibly named:
mmc_block_dev_block_read() -- so that it is similar to the original
calling convention [mmc->block_dev.block_read], or
mmc_pbread() [PartitionBlockRead] -- so that it is similar to the
mmc_bread() [which is the implementation of the callback function]
I'd rather go with the wrapper function. Perhaps it's not even needed. The function called is part of the block_dev (block_read/write etc).
Overwrite those with functions that implemented the switching first, and then call the original block* function.
The callback function is: mmc->block_dev.block_read = mmc_bread and it accepts four arguments: include/part.h: unsigned long (*block_read)(int dev, include/part.h- lbaint_t start, include/part.h- lbaint_t blkcnt, include/part.h- void *buffer); Are you suggesting that I should add another argument to this callback?
[...snip...]
Regards
-- Pantelis
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
participants (2)
-
Pantelis Antoniou
-
Steve Rae