* [U-Boot] MMC: proposal to support multiple physical partitions
@ 2014-04-24 17:50 Steve Rae
2014-04-29 17:24 ` Pantelis Antoniou
0 siblings, 1 reply; 4+ messages in thread
From: Steve Rae @ 2014-04-24 17:50 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] MMC: proposal to support multiple physical partitions
2014-04-24 17:50 [U-Boot] MMC: proposal to support multiple physical partitions Steve Rae
@ 2014-04-29 17:24 ` Pantelis Antoniou
2014-04-29 18:08 ` Steve Rae
0 siblings, 1 reply; 4+ messages in thread
From: Pantelis Antoniou @ 2014-04-29 17:24 UTC (permalink / raw)
To: u-boot
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:
> 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]
>
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] MMC: proposal to support multiple physical partitions
2014-04-29 17:24 ` Pantelis Antoniou
@ 2014-04-29 18:08 ` Steve Rae
2014-05-05 16:53 ` Steve Rae
0 siblings, 1 reply; 4+ messages in thread
From: Steve Rae @ 2014-04-29 18:08 UTC (permalink / raw)
To: u-boot
Thanks for the response,
> -----Original Message-----
> From: Pantelis Antoniou [mailto:panto at antoniou-consulting.com]
> Sent: Tuesday, April 29, 2014 10:25
> To: Steve Rae
> Cc: u-boot at lists.denx.de; trini at ti.com; albert.u.boot at 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:
> > 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]
> >
>
>
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] MMC: proposal to support multiple physical partitions
2014-04-29 18:08 ` Steve Rae
@ 2014-05-05 16:53 ` Steve Rae
0 siblings, 0 replies; 4+ messages in thread
From: Steve Rae @ 2014-05-05 16:53 UTC (permalink / raw)
To: u-boot
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 at antoniou-consulting.com]
>> Sent: Tuesday, April 29, 2014 10:25
>> To: Steve Rae
>> Cc: u-boot at lists.denx.de; trini at ti.com; albert.u.boot at 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:
>>> 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]
>>
>> 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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-05-05 16:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-24 17:50 [U-Boot] MMC: proposal to support multiple physical partitions Steve Rae
2014-04-29 17:24 ` Pantelis Antoniou
2014-04-29 18:08 ` Steve Rae
2014-05-05 16:53 ` Steve Rae
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.