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

2014-05-05 Thread Steve Rae 

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:pa...@antoniou-consulting.com]
Sent: Tuesday, April 29, 2014 10:25
To: Steve Rae
Cc: u-boot@lists.denx.de; tr...@ti.com; albert.u.b...@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@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] MMC: proposal to support multiple physical partitions

2014-04-29 Thread Pantelis Antoniou 
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

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

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

2014-04-29 Thread Steve Rae 
Thanks for the response,

 -Original Message-
 From: Pantelis Antoniou [mailto:pa...@antoniou-consulting.com]
 Sent: Tuesday, April 29, 2014 10:25
 To: Steve Rae
 Cc: u-boot@lists.denx.de; tr...@ti.com; albert.u.b...@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@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2014-04-24 Thread Steve Rae 
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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot