* [U-Boot] [PATCH V3 1/8] disk: parameterize get_device_and_partition's loop count
2012-09-18 22:37 [U-Boot] [PATCH V3 0/8] disk: "part" command and dependencies Stephen Warren
@ 2012-09-18 22:37 ` Stephen Warren
2012-09-18 22:37 ` [U-Boot] [PATCH V3 2/8] disk: fix get_device_and_partition() bootable search Stephen Warren
` (6 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Stephen Warren @ 2012-09-18 22:37 UTC (permalink / raw)
To: u-boot
From: Stephen Warren <swarren@nvidia.com>
Create #define MAX_SEARCH_PARTITIONS to indicate how many partition IDs
get_device_and_partition()'s automatic mode should search through. Also,
search 1..n not 1..n-1 - it's unlikely anyone has this many partitions,
but given the loop is 1-based, including the limit seems more consistent.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v3: New patch.
---
disk/part.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/disk/part.c b/disk/part.c
index 1284e1a..6caf6d2 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -435,6 +435,7 @@ void print_part (block_dev_desc_t * dev_desc)
#endif
+#define MAX_SEARCH_PARTITIONS 16
int get_device_and_partition(const char *ifname, const char *dev_str,
block_dev_desc_t **dev_desc,
disk_partition_t *info)
@@ -484,7 +485,7 @@ int get_device_and_partition(const char *ifname, const char *dev_str,
} else {
/* find the first bootable partition. If none are bootable,
* fall back to the first valid partition */
- for (p = 1; p < 16; p++) {
+ for (p = 1; p <= MAX_SEARCH_PARTITIONS; p++) {
ret = get_partition_info(desc, p, info);
if (ret)
continue;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [U-Boot] [PATCH V3 2/8] disk: fix get_device_and_partition() bootable search
2012-09-18 22:37 [U-Boot] [PATCH V3 0/8] disk: "part" command and dependencies Stephen Warren
2012-09-18 22:37 ` [U-Boot] [PATCH V3 1/8] disk: parameterize get_device_and_partition's loop count Stephen Warren
@ 2012-09-18 22:37 ` Stephen Warren
2012-09-19 1:18 ` Rob Herring
2012-09-18 22:37 ` [U-Boot] [PATCH V3 3/8] disk: introduce get_device() Stephen Warren
` (5 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Stephen Warren @ 2012-09-18 22:37 UTC (permalink / raw)
To: u-boot
From: Stephen Warren <swarren@nvidia.com>
The existing get_device_and_partition() bootable search loop attempts
to save the current best known partition in variable best_part. However,
it's actually just saving a copy of the (static) "info" variable, and
hence ends up not doing anything much useful if no bootable partition
is found. Fix this by reworking the loop to:
a) When reading information about a partition, always read into the
caller-supplied buffer; that way, if we find a bootable partition,
there's no need to copy any information.
b) Save the first known valid partition's structure content rather than
pointer in the search loop, and restore the structure content rather
than the pointer when the loop exits.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v3: New patch.
---
disk/part.c | 38 +++++++++++++++++++++++++++++---------
1 files changed, 29 insertions(+), 9 deletions(-)
diff --git a/disk/part.c b/disk/part.c
index 6caf6d2..277a243 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -447,7 +447,7 @@ int get_device_and_partition(const char *ifname, const char *dev_str,
int p;
int part = 0;
char *part_str;
- disk_partition_t *best_part = NULL;
+ disk_partition_t tmpinfo;
if (dev_str)
dev = simple_strtoul(dev_str, &ep, 16);
@@ -483,24 +483,44 @@ int get_device_and_partition(const char *ifname, const char *dev_str,
part = (int)simple_strtoul(++part_str, NULL, 16);
ret = get_partition_info(desc, part, info);
} else {
- /* find the first bootable partition. If none are bootable,
- * fall back to the first valid partition */
+ /*
+ * Find the first bootable partition.
+ * If none are bootable, fall back to the first valid partition.
+ */
for (p = 1; p <= MAX_SEARCH_PARTITIONS; p++) {
- ret = get_partition_info(desc, p, info);
+ ret = get_partition_info(*dev_desc, p, info);
if (ret)
continue;
- if (!best_part || info->bootable) {
- best_part = info;
+ /*
+ * First valid partition, or new better partition?
+ * If so, save partition ID.
+ */
+ if (!part || info->bootable)
part = p;
- }
+ /* Best possible partition? Stop searching. */
if (info->bootable)
break;
+
+ /*
+ * We now need to search further for best possible.
+ * If we what we just queried was the best so far,
+ * save the info since we over-write it next loop.
+ */
+ if (part == p)
+ tmpinfo = *info;
}
- info = best_part;
- if (part)
+ /* If we found any acceptable partition */
+ if (part) {
+ /*
+ * If we searched all possible partition IDs,
+ * return the first valid partition we found.
+ */
+ if (p == MAX_SEARCH_PARTITIONS + 1)
+ *info = tmpinfo;
ret = 0;
+ }
}
if (ret) {
printf("** Invalid partition %d, use `dev[:part]' **\n", part);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [U-Boot] [PATCH V3 2/8] disk: fix get_device_and_partition() bootable search
2012-09-18 22:37 ` [U-Boot] [PATCH V3 2/8] disk: fix get_device_and_partition() bootable search Stephen Warren
@ 2012-09-19 1:18 ` Rob Herring
0 siblings, 0 replies; 16+ messages in thread
From: Rob Herring @ 2012-09-19 1:18 UTC (permalink / raw)
To: u-boot
On 09/18/2012 05:37 PM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> The existing get_device_and_partition() bootable search loop attempts
> to save the current best known partition in variable best_part. However,
> it's actually just saving a copy of the (static) "info" variable, and
> hence ends up not doing anything much useful if no bootable partition
> is found. Fix this by reworking the loop to:
>
> a) When reading information about a partition, always read into the
> caller-supplied buffer; that way, if we find a bootable partition,
> there's no need to copy any information.
> b) Save the first known valid partition's structure content rather than
> pointer in the search loop, and restore the structure content rather
> than the pointer when the loop exits.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> v3: New patch.
You might as well squash these into my original commits.
Rob
> ---
> disk/part.c | 38 +++++++++++++++++++++++++++++---------
> 1 files changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/disk/part.c b/disk/part.c
> index 6caf6d2..277a243 100644
> --- a/disk/part.c
> +++ b/disk/part.c
> @@ -447,7 +447,7 @@ int get_device_and_partition(const char *ifname, const char *dev_str,
> int p;
> int part = 0;
> char *part_str;
> - disk_partition_t *best_part = NULL;
> + disk_partition_t tmpinfo;
>
> if (dev_str)
> dev = simple_strtoul(dev_str, &ep, 16);
> @@ -483,24 +483,44 @@ int get_device_and_partition(const char *ifname, const char *dev_str,
> part = (int)simple_strtoul(++part_str, NULL, 16);
> ret = get_partition_info(desc, part, info);
> } else {
> - /* find the first bootable partition. If none are bootable,
> - * fall back to the first valid partition */
> + /*
> + * Find the first bootable partition.
> + * If none are bootable, fall back to the first valid partition.
> + */
> for (p = 1; p <= MAX_SEARCH_PARTITIONS; p++) {
> - ret = get_partition_info(desc, p, info);
> + ret = get_partition_info(*dev_desc, p, info);
> if (ret)
> continue;
>
> - if (!best_part || info->bootable) {
> - best_part = info;
> + /*
> + * First valid partition, or new better partition?
> + * If so, save partition ID.
> + */
> + if (!part || info->bootable)
> part = p;
> - }
>
> + /* Best possible partition? Stop searching. */
> if (info->bootable)
> break;
> +
> + /*
> + * We now need to search further for best possible.
> + * If we what we just queried was the best so far,
> + * save the info since we over-write it next loop.
> + */
> + if (part == p)
> + tmpinfo = *info;
> }
> - info = best_part;
> - if (part)
> + /* If we found any acceptable partition */
> + if (part) {
> + /*
> + * If we searched all possible partition IDs,
> + * return the first valid partition we found.
> + */
> + if (p == MAX_SEARCH_PARTITIONS + 1)
> + *info = tmpinfo;
> ret = 0;
> + }
> }
> if (ret) {
> printf("** Invalid partition %d, use `dev[:part]' **\n", part);
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH V3 3/8] disk: introduce get_device()
2012-09-18 22:37 [U-Boot] [PATCH V3 0/8] disk: "part" command and dependencies Stephen Warren
2012-09-18 22:37 ` [U-Boot] [PATCH V3 1/8] disk: parameterize get_device_and_partition's loop count Stephen Warren
2012-09-18 22:37 ` [U-Boot] [PATCH V3 2/8] disk: fix get_device_and_partition() bootable search Stephen Warren
@ 2012-09-18 22:37 ` Stephen Warren
2012-09-19 1:21 ` Rob Herring
2012-09-21 12:53 ` Rob Herring
2012-09-18 22:37 ` [U-Boot] [PATCH V3 4/8] disk: get_device_and_partition() enhancements Stephen Warren
` (4 subsequent siblings)
7 siblings, 2 replies; 16+ messages in thread
From: Stephen Warren @ 2012-09-18 22:37 UTC (permalink / raw)
To: u-boot
From: Stephen Warren <swarren@nvidia.com>
This patch introduces function get_device(). This looks up a
block_dev_desc_t from an interface name (e.g. mmc) and device number
(e.g. 0). This function is essentially the non-partition-specific
prefix of get_device_and_partition().
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v3: New patch.
---
disk/part.c | 22 ++++++++++++++++++++++
include/part.h | 5 +++++
2 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/disk/part.c b/disk/part.c
index 277a243..9920d48 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -435,6 +435,28 @@ void print_part (block_dev_desc_t * dev_desc)
#endif
+int get_device(const char *ifname, const char *dev_str,
+ block_dev_desc_t **dev_desc)
+{
+ char *ep;
+ int dev;
+
+ dev = simple_strtoul(dev_str, &ep, 16);
+ if (*ep) {
+ printf("** Bad device specification %s %s **\n",
+ ifname, dev_str);
+ return -1;
+ }
+
+ *dev_desc = get_dev(ifname, dev);
+ if (!(*dev_desc) || ((*dev_desc)->type == DEV_TYPE_UNKNOWN)) {
+ printf("** Bad device %s %s **\n", ifname, dev_str);
+ return -1;
+ }
+
+ return dev;
+}
+
#define MAX_SEARCH_PARTITIONS 16
int get_device_and_partition(const char *ifname, const char *dev_str,
block_dev_desc_t **dev_desc,
diff --git a/include/part.h b/include/part.h
index a6d06f3..144df4e 100644
--- a/include/part.h
+++ b/include/part.h
@@ -112,6 +112,8 @@ int get_partition_info (block_dev_desc_t * dev_desc, int part, disk_partition_t
void print_part (block_dev_desc_t *dev_desc);
void init_part (block_dev_desc_t *dev_desc);
void dev_print(block_dev_desc_t *dev_desc);
+int get_device(const char *ifname, const char *dev_str,
+ block_dev_desc_t **dev_desc);
int get_device_and_partition(const char *ifname, const char *dev_str,
block_dev_desc_t **dev_desc,
disk_partition_t *info);
@@ -131,6 +133,9 @@ static inline int get_partition_info (block_dev_desc_t * dev_desc, int part,
static inline void print_part (block_dev_desc_t *dev_desc) {}
static inline void init_part (block_dev_desc_t *dev_desc) {}
static inline void dev_print(block_dev_desc_t *dev_desc) {}
+static inline int get_device(const char *ifname, const char *dev_str,
+ block_dev_desc_t **dev_desc)
+{ return -1; }
static inline int get_device_and_partition(const char *ifname,
const char *dev_str,
block_dev_desc_t **dev_desc,
--
1.7.0.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [U-Boot] [PATCH V3 3/8] disk: introduce get_device()
2012-09-18 22:37 ` [U-Boot] [PATCH V3 3/8] disk: introduce get_device() Stephen Warren
@ 2012-09-19 1:21 ` Rob Herring
2012-09-19 1:25 ` Rob Herring
2012-09-21 12:53 ` Rob Herring
1 sibling, 1 reply; 16+ messages in thread
From: Rob Herring @ 2012-09-19 1:21 UTC (permalink / raw)
To: u-boot
On 09/18/2012 05:37 PM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> This patch introduces function get_device(). This looks up a
> block_dev_desc_t from an interface name (e.g. mmc) and device number
> (e.g. 0). This function is essentially the non-partition-specific
> prefix of get_device_and_partition().
Then shouldn't get_device_and_partition just call get_device. Perhaps
create get_partition() and then get_device_and_partition is just a wrapper.
Rob
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> v3: New patch.
> ---
> disk/part.c | 22 ++++++++++++++++++++++
> include/part.h | 5 +++++
> 2 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/disk/part.c b/disk/part.c
> index 277a243..9920d48 100644
> --- a/disk/part.c
> +++ b/disk/part.c
> @@ -435,6 +435,28 @@ void print_part (block_dev_desc_t * dev_desc)
>
> #endif
>
> +int get_device(const char *ifname, const char *dev_str,
> + block_dev_desc_t **dev_desc)
> +{
> + char *ep;
> + int dev;
> +
> + dev = simple_strtoul(dev_str, &ep, 16);
> + if (*ep) {
> + printf("** Bad device specification %s %s **\n",
> + ifname, dev_str);
> + return -1;
> + }
> +
> + *dev_desc = get_dev(ifname, dev);
> + if (!(*dev_desc) || ((*dev_desc)->type == DEV_TYPE_UNKNOWN)) {
> + printf("** Bad device %s %s **\n", ifname, dev_str);
> + return -1;
> + }
> +
> + return dev;
> +}
> +
> #define MAX_SEARCH_PARTITIONS 16
> int get_device_and_partition(const char *ifname, const char *dev_str,
> block_dev_desc_t **dev_desc,
> diff --git a/include/part.h b/include/part.h
> index a6d06f3..144df4e 100644
> --- a/include/part.h
> +++ b/include/part.h
> @@ -112,6 +112,8 @@ int get_partition_info (block_dev_desc_t * dev_desc, int part, disk_partition_t
> void print_part (block_dev_desc_t *dev_desc);
> void init_part (block_dev_desc_t *dev_desc);
> void dev_print(block_dev_desc_t *dev_desc);
> +int get_device(const char *ifname, const char *dev_str,
> + block_dev_desc_t **dev_desc);
> int get_device_and_partition(const char *ifname, const char *dev_str,
> block_dev_desc_t **dev_desc,
> disk_partition_t *info);
> @@ -131,6 +133,9 @@ static inline int get_partition_info (block_dev_desc_t * dev_desc, int part,
> static inline void print_part (block_dev_desc_t *dev_desc) {}
> static inline void init_part (block_dev_desc_t *dev_desc) {}
> static inline void dev_print(block_dev_desc_t *dev_desc) {}
> +static inline int get_device(const char *ifname, const char *dev_str,
> + block_dev_desc_t **dev_desc)
> +{ return -1; }
> static inline int get_device_and_partition(const char *ifname,
> const char *dev_str,
> block_dev_desc_t **dev_desc,
>
^ permalink raw reply [flat|nested] 16+ messages in thread* [U-Boot] [PATCH V3 3/8] disk: introduce get_device()
2012-09-19 1:21 ` Rob Herring
@ 2012-09-19 1:25 ` Rob Herring
2012-09-19 17:18 ` Stephen Warren
2012-09-19 18:17 ` Tom Rini
0 siblings, 2 replies; 16+ messages in thread
From: Rob Herring @ 2012-09-19 1:25 UTC (permalink / raw)
To: u-boot
On 09/18/2012 08:21 PM, Rob Herring wrote:
> On 09/18/2012 05:37 PM, Stephen Warren wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> This patch introduces function get_device(). This looks up a
>> block_dev_desc_t from an interface name (e.g. mmc) and device number
>> (e.g. 0). This function is essentially the non-partition-specific
>> prefix of get_device_and_partition().
>
> Then shouldn't get_device_and_partition just call get_device. Perhaps
> create get_partition() and then get_device_and_partition is just a wrapper.
>
I should read all the way through the series before replying...
Anyway, I would squash it all unless you want to have restructuring with
current functionality and then enhancements.
Rob
> Rob
>
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>> ---
>> v3: New patch.
>> ---
>> disk/part.c | 22 ++++++++++++++++++++++
>> include/part.h | 5 +++++
>> 2 files changed, 27 insertions(+), 0 deletions(-)
>>
>> diff --git a/disk/part.c b/disk/part.c
>> index 277a243..9920d48 100644
>> --- a/disk/part.c
>> +++ b/disk/part.c
>> @@ -435,6 +435,28 @@ void print_part (block_dev_desc_t * dev_desc)
>>
>> #endif
>>
>> +int get_device(const char *ifname, const char *dev_str,
>> + block_dev_desc_t **dev_desc)
>> +{
>> + char *ep;
>> + int dev;
>> +
>> + dev = simple_strtoul(dev_str, &ep, 16);
>> + if (*ep) {
>> + printf("** Bad device specification %s %s **\n",
>> + ifname, dev_str);
>> + return -1;
>> + }
>> +
>> + *dev_desc = get_dev(ifname, dev);
>> + if (!(*dev_desc) || ((*dev_desc)->type == DEV_TYPE_UNKNOWN)) {
>> + printf("** Bad device %s %s **\n", ifname, dev_str);
>> + return -1;
>> + }
>> +
>> + return dev;
>> +}
>> +
>> #define MAX_SEARCH_PARTITIONS 16
>> int get_device_and_partition(const char *ifname, const char *dev_str,
>> block_dev_desc_t **dev_desc,
>> diff --git a/include/part.h b/include/part.h
>> index a6d06f3..144df4e 100644
>> --- a/include/part.h
>> +++ b/include/part.h
>> @@ -112,6 +112,8 @@ int get_partition_info (block_dev_desc_t * dev_desc, int part, disk_partition_t
>> void print_part (block_dev_desc_t *dev_desc);
>> void init_part (block_dev_desc_t *dev_desc);
>> void dev_print(block_dev_desc_t *dev_desc);
>> +int get_device(const char *ifname, const char *dev_str,
>> + block_dev_desc_t **dev_desc);
>> int get_device_and_partition(const char *ifname, const char *dev_str,
>> block_dev_desc_t **dev_desc,
>> disk_partition_t *info);
>> @@ -131,6 +133,9 @@ static inline int get_partition_info (block_dev_desc_t * dev_desc, int part,
>> static inline void print_part (block_dev_desc_t *dev_desc) {}
>> static inline void init_part (block_dev_desc_t *dev_desc) {}
>> static inline void dev_print(block_dev_desc_t *dev_desc) {}
>> +static inline int get_device(const char *ifname, const char *dev_str,
>> + block_dev_desc_t **dev_desc)
>> +{ return -1; }
>> static inline int get_device_and_partition(const char *ifname,
>> const char *dev_str,
>> block_dev_desc_t **dev_desc,
>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* [U-Boot] [PATCH V3 3/8] disk: introduce get_device()
2012-09-19 1:25 ` Rob Herring
@ 2012-09-19 17:18 ` Stephen Warren
2012-09-19 18:17 ` Tom Rini
1 sibling, 0 replies; 16+ messages in thread
From: Stephen Warren @ 2012-09-19 17:18 UTC (permalink / raw)
To: u-boot
On 09/18/2012 07:25 PM, Rob Herring wrote:
> On 09/18/2012 08:21 PM, Rob Herring wrote:
>> On 09/18/2012 05:37 PM, Stephen Warren wrote:
>>> From: Stephen Warren <swarren@nvidia.com>
>>>
>>> This patch introduces function get_device(). This looks up a
>>> block_dev_desc_t from an interface name (e.g. mmc) and device number
>>> (e.g. 0). This function is essentially the non-partition-specific
>>> prefix of get_device_and_partition().
>>
>> Then shouldn't get_device_and_partition just call get_device. Perhaps
>> create get_partition() and then get_device_and_partition is just a wrapper.
>>
>
> I should read all the way through the series before replying...
>
> Anyway, I would squash it all unless you want to have restructuring with
> current functionality and then enhancements.
OK, I'm happy to squash everything together, and repost a series with
both your and my patches; I separated it out to hopefully make working
out what changes I made a little easier.
I did wonder about creating separate get_device, get_partition, and
get_device_or_partition functions, the latter probably using a common
implementation. I guess if I squash my changes into your original
patches, then creating those 3 separate functions would end up being
less churn. So, unless there are objections, I'll do that.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH V3 3/8] disk: introduce get_device()
2012-09-19 1:25 ` Rob Herring
2012-09-19 17:18 ` Stephen Warren
@ 2012-09-19 18:17 ` Tom Rini
1 sibling, 0 replies; 16+ messages in thread
From: Tom Rini @ 2012-09-19 18:17 UTC (permalink / raw)
To: u-boot
On Tue, Sep 18, 2012 at 08:25:31PM -0500, Rob Herring wrote:
> On 09/18/2012 08:21 PM, Rob Herring wrote:
> > On 09/18/2012 05:37 PM, Stephen Warren wrote:
> >> From: Stephen Warren <swarren@nvidia.com>
> >>
> >> This patch introduces function get_device(). This looks up a
> >> block_dev_desc_t from an interface name (e.g. mmc) and device number
> >> (e.g. 0). This function is essentially the non-partition-specific
> >> prefix of get_device_and_partition().
> >
> > Then shouldn't get_device_and_partition just call get_device. Perhaps
> > create get_partition() and then get_device_and_partition is just a wrapper.
> >
>
> I should read all the way through the series before replying...
>
> Anyway, I would squash it all unless you want to have restructuring with
> current functionality and then enhancements.
IMHO, restucture and then enhancements makes the most sense since it
means we can bisect a latent bug easier. So no need for a v4 to squash
patches down.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120919/f37541a1/attachment.pgp>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH V3 3/8] disk: introduce get_device()
2012-09-18 22:37 ` [U-Boot] [PATCH V3 3/8] disk: introduce get_device() Stephen Warren
2012-09-19 1:21 ` Rob Herring
@ 2012-09-21 12:53 ` Rob Herring
2012-09-21 16:09 ` Stephen Warren
1 sibling, 1 reply; 16+ messages in thread
From: Rob Herring @ 2012-09-21 12:53 UTC (permalink / raw)
To: u-boot
On 09/18/2012 05:37 PM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> This patch introduces function get_device(). This looks up a
> block_dev_desc_t from an interface name (e.g. mmc) and device number
> (e.g. 0). This function is essentially the non-partition-specific
> prefix of get_device_and_partition().
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> v3: New patch.
> ---
> disk/part.c | 22 ++++++++++++++++++++++
> include/part.h | 5 +++++
> 2 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/disk/part.c b/disk/part.c
> index 277a243..9920d48 100644
> --- a/disk/part.c
> +++ b/disk/part.c
> @@ -435,6 +435,28 @@ void print_part (block_dev_desc_t * dev_desc)
>
> #endif
>
> +int get_device(const char *ifname, const char *dev_str,
> + block_dev_desc_t **dev_desc)
> +{
> + char *ep;
> + int dev;
> +
Why don't you look up bootdevice here? That would be more consistent
behavior.
Rob
> + dev = simple_strtoul(dev_str, &ep, 16);
> + if (*ep) {
> + printf("** Bad device specification %s %s **\n",
> + ifname, dev_str);
> + return -1;
> + }
> +
> + *dev_desc = get_dev(ifname, dev);
> + if (!(*dev_desc) || ((*dev_desc)->type == DEV_TYPE_UNKNOWN)) {
> + printf("** Bad device %s %s **\n", ifname, dev_str);
> + return -1;
> + }
> +
> + return dev;
> +}
> +
> #define MAX_SEARCH_PARTITIONS 16
> int get_device_and_partition(const char *ifname, const char *dev_str,
> block_dev_desc_t **dev_desc,
> diff --git a/include/part.h b/include/part.h
> index a6d06f3..144df4e 100644
> --- a/include/part.h
> +++ b/include/part.h
> @@ -112,6 +112,8 @@ int get_partition_info (block_dev_desc_t * dev_desc, int part, disk_partition_t
> void print_part (block_dev_desc_t *dev_desc);
> void init_part (block_dev_desc_t *dev_desc);
> void dev_print(block_dev_desc_t *dev_desc);
> +int get_device(const char *ifname, const char *dev_str,
> + block_dev_desc_t **dev_desc);
> int get_device_and_partition(const char *ifname, const char *dev_str,
> block_dev_desc_t **dev_desc,
> disk_partition_t *info);
> @@ -131,6 +133,9 @@ static inline int get_partition_info (block_dev_desc_t * dev_desc, int part,
> static inline void print_part (block_dev_desc_t *dev_desc) {}
> static inline void init_part (block_dev_desc_t *dev_desc) {}
> static inline void dev_print(block_dev_desc_t *dev_desc) {}
> +static inline int get_device(const char *ifname, const char *dev_str,
> + block_dev_desc_t **dev_desc)
> +{ return -1; }
> static inline int get_device_and_partition(const char *ifname,
> const char *dev_str,
> block_dev_desc_t **dev_desc,
>
^ permalink raw reply [flat|nested] 16+ messages in thread* [U-Boot] [PATCH V3 3/8] disk: introduce get_device()
2012-09-21 12:53 ` Rob Herring
@ 2012-09-21 16:09 ` Stephen Warren
0 siblings, 0 replies; 16+ messages in thread
From: Stephen Warren @ 2012-09-21 16:09 UTC (permalink / raw)
To: u-boot
On 09/21/2012 06:53 AM, Rob Herring wrote:
> On 09/18/2012 05:37 PM, Stephen Warren wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> This patch introduces function get_device(). This looks up a
>> block_dev_desc_t from an interface name (e.g. mmc) and device number
>> (e.g. 0). This function is essentially the non-partition-specific
>> prefix of get_device_and_partition().
>> +int get_device(const char *ifname, const char *dev_str,
>> + block_dev_desc_t **dev_desc)
>> +{
>> + char *ep;
>> + int dev;
>> +
>
> Why don't you look up bootdevice here? That would be more consistent
> behavior.
bootdevice names a partition (or can name a partition), whereas this
function is about retrieving a device handle and never a partition handle.
I'm not sure it makes semantic sense to always fall back to bootdevice
for commands that call get_device() directly. I'd far prefer people to
always just pass the device they want to a command rather than relying
implicitly on environment variables.
If we did read bootdevice here, we'd end up having to read/parse it in
both get_device() and get_device_and_partition(), here to extract just
the device portion and in get_device_and_partition() to extract just the
partition portion. And we'd have to make sure the code here only allowed
the user to specify a partition /if/ this function was called from
get_device_and_partition() and not if a command called it directly. That
all seems a bit complex.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH V3 4/8] disk: get_device_and_partition() enhancements
2012-09-18 22:37 [U-Boot] [PATCH V3 0/8] disk: "part" command and dependencies Stephen Warren
` (2 preceding siblings ...)
2012-09-18 22:37 ` [U-Boot] [PATCH V3 3/8] disk: introduce get_device() Stephen Warren
@ 2012-09-18 22:37 ` Stephen Warren
2012-09-18 22:37 ` [U-Boot] [PATCH V3 5/8] disk: part_efi: range-check partition number Stephen Warren
` (3 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Stephen Warren @ 2012-09-18 22:37 UTC (permalink / raw)
To: u-boot
From: Stephen Warren <swarren@nvidia.com>
Rework get_device_and_partition to:
a) Make use of get_device().
b) Add parameter to indicate whether returning a whole device is
acceptable, or whether a partition is mandatory.
c) Make error-checking of the user's device-/partition-specification
more complete. In particular, if strtoul() doesn't convert all
characters, it's an error rather than just ignored.
d) Require user to explicitly specify "N:auto" as the device/partition
specification to get the automatic "search for a bootable partition"
mode of operation; Rob's patch changed the behaviour of some syntaxes
from defaulting to partition 1.
The resultant device/partition returned by the function will be as
follows, based on whether the disk has a partition table (ptable) or not,
and whether the calling command allows the whole device to be returned
or not.
(D and P are integers, P >= 1)
D
D:
No ptable:
!allow_whole_dev: error
allow_whole_dev: device D
ptable:
device D partition 1
D:0
!allow_whole_dev: error
allow_whole_dev: device D
D:P
No ptable: error
ptable: device D partition P
D:auto
No ptable:
!allow_whole_dev: error
allow_whole_dev: device D
ptable:
first partition in device D with bootable flag set.
If none, first valid paratition in device D.
Note: In order to review this patch, it's probably easiest to simply
look at the file contents post-application, rather than reading the
patch itself.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v3: New patch.
---
common/cmd_disk.c | 2 +-
common/cmd_ext4.c | 2 +-
common/cmd_ext_common.c | 4 +-
common/cmd_fat.c | 8 +-
common/cmd_reiser.c | 4 +-
common/cmd_zfs.c | 4 +-
disk/part.c | 151 ++++++++++++++++++++++++++++++++++-------------
include/part.h | 9 ++-
8 files changed, 127 insertions(+), 57 deletions(-)
diff --git a/common/cmd_disk.c b/common/cmd_disk.c
index e6676b0..3bd1eba 100644
--- a/common/cmd_disk.c
+++ b/common/cmd_disk.c
@@ -31,7 +31,7 @@ int common_diskboot(cmd_tbl_t *cmdtp, const char *intf, int argc,
bootstage_mark(BOOTSTAGE_ID_IDE_BOOT_DEVICE);
part = get_device_and_partition(intf, (argc == 3) ? argv[2] : NULL,
- &dev_desc, &info);
+ &dev_desc, &info, 1);
if (part < 0) {
bootstage_error(BOOTSTAGE_ID_IDE_TYPE);
return 1;
diff --git a/common/cmd_ext4.c b/common/cmd_ext4.c
index 48f9ba3..ca46561 100644
--- a/common/cmd_ext4.c
+++ b/common/cmd_ext4.c
@@ -87,7 +87,7 @@ int do_ext4_write(cmd_tbl_t *cmdtp, int flag, int argc,
if (argc < 6)
return cmd_usage(cmdtp);
- part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+ part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
if (part < 0)
return 1;
diff --git a/common/cmd_ext_common.c b/common/cmd_ext_common.c
index 7d26944..1952f4d 100644
--- a/common/cmd_ext_common.c
+++ b/common/cmd_ext_common.c
@@ -108,7 +108,7 @@ int do_ext_load(cmd_tbl_t *cmdtp, int flag, int argc,
return 1;
}
- part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+ part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
if (part < 0)
return 1;
@@ -166,7 +166,7 @@ int do_ext_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
if (argc < 2)
return cmd_usage(cmdtp);
- part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+ part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
if (part < 0)
return 1;
diff --git a/common/cmd_fat.c b/common/cmd_fat.c
index 90412d6..01e02f5 100644
--- a/common/cmd_fat.c
+++ b/common/cmd_fat.c
@@ -49,7 +49,7 @@ int do_fat_fsload (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
return 1;
}
- part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+ part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
if (part < 0)
return 1;
@@ -101,7 +101,7 @@ int do_fat_ls (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
return 0;
}
- part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+ part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
if (part < 0)
return 1;
@@ -139,7 +139,7 @@ int do_fat_fsinfo (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
return 0;
}
- part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+ part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
if (part < 0)
return 1;
@@ -175,7 +175,7 @@ static int do_fat_fswrite(cmd_tbl_t *cmdtp, int flag,
if (argc < 5)
return cmd_usage(cmdtp);
- part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+ part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
if (part < 0)
return 1;
diff --git a/common/cmd_reiser.c b/common/cmd_reiser.c
index 2865014..e658618 100644
--- a/common/cmd_reiser.c
+++ b/common/cmd_reiser.c
@@ -57,7 +57,7 @@ int do_reiserls (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
if (argc < 3)
return CMD_RET_USAGE;
- part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+ part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
if (part < 0)
return 1;
@@ -140,7 +140,7 @@ int do_reiserload (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
return 1;
}
- part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+ part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
if (part < 0)
return 1;
diff --git a/common/cmd_zfs.c b/common/cmd_zfs.c
index 27f8856..d580f7b 100644
--- a/common/cmd_zfs.c
+++ b/common/cmd_zfs.c
@@ -94,7 +94,7 @@ static int do_zfs_load(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
return 1;
}
- part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+ part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
if (part < 0)
return 1;
@@ -160,7 +160,7 @@ static int do_zfs_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
if (argc == 4)
filename = argv[3];
- part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info);
+ part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
if (part < 0)
return 1;
diff --git a/disk/part.c b/disk/part.c
index 9920d48..9916708 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -24,6 +24,7 @@
#include <common.h>
#include <command.h>
#include <ide.h>
+#include <malloc.h>
#include <part.h>
#undef PART_DEBUG
@@ -457,58 +458,126 @@ int get_device(const char *ifname, const char *dev_str,
return dev;
}
+#define PART_UNSPECIFIED -2
+#define PART_AUTO -1
#define MAX_SEARCH_PARTITIONS 16
-int get_device_and_partition(const char *ifname, const char *dev_str,
+int get_device_and_partition(const char *ifname, const char *dev_part_str,
block_dev_desc_t **dev_desc,
- disk_partition_t *info)
+ disk_partition_t *info, int allow_whole_dev)
{
- int ret;
- char *ep;
+ int ret = -1;
+ const char *part_str;
+ char *dup_str = NULL;
+ const char *dev_str;
int dev;
- block_dev_desc_t *desc;
+ char *ep;
int p;
- int part = 0;
- char *part_str;
+ int part;
disk_partition_t tmpinfo;
- if (dev_str)
- dev = simple_strtoul(dev_str, &ep, 16);
+ /* If no dev_part_str, use bootdevice environment variable */
+ if (!dev_part_str)
+ dev_part_str = getenv("bootdevice");
+
+ /* If still no dev_part_str, it's an error */
+ if (!dev_part_str) {
+ printf("** No device specified **\n");
+ goto cleanup;
+ }
- if (!dev_str || (dev_str == ep)) {
- dev_str = getenv("bootdevice");
- if (dev_str)
- dev = simple_strtoul(dev_str, &ep, 16);
- if (!dev_str || (dev_str == ep))
- goto err;
+ /* Separate device and partition ID specification */
+ part_str = strchr(dev_part_str, ':');
+ if (part_str) {
+ dup_str = strdup(dev_part_str);
+ dup_str[part_str - dev_part_str] = 0;
+ dev_str = dup_str;
+ part_str++;
+ } else {
+ dev_str = dev_part_str;
}
- desc = get_dev(ifname, dev);
- if (!desc || (desc->type == DEV_TYPE_UNKNOWN))
- goto err;
+ /* Look up the device */
+ dev = get_device(ifname, dev_str, dev_desc);
+ if (dev < 0)
+ goto cleanup;
+
+ /* Convert partition ID string to number */
+ if (!part_str || !*part_str) {
+ part = PART_UNSPECIFIED;
+ } else if (!strcmp(part_str, "auto")) {
+ part = PART_AUTO;
+ } else {
+ /* Something specified -> use exactly that */
+ part = (int)simple_strtoul(part_str, &ep, 16);
+ /*
+ * Less than whole string converted,
+ * or request for whole device, but caller requires partition.
+ */
+ if (*ep || (part == 0 && !allow_whole_dev)) {
+ printf("** Bad partition specification %s %s **\n",
+ ifname, dev_part_str);
+ goto cleanup;
+ }
+ }
- if (desc->part_type == PART_TYPE_UNKNOWN) {
- /* disk doesn't use partition table */
- if (!desc->lba) {
- printf("**Bad disk size - %s %d:0 **\n", ifname, dev);
- return -1;
+ /*
+ * No partition table on device,
+ * or user requested partition 0 (entire device).
+ */
+ if (((*dev_desc)->part_type == PART_TYPE_UNKNOWN) ||
+ (part == 0)) {
+ if (!(*dev_desc)->lba) {
+ printf("** Bad device size - %s %s **\n", ifname,
+ dev_str);
+ goto cleanup;
}
+
+ /*
+ * If user specified a partition ID other than 0,
+ * or the calling command only accepts partitions,
+ * it's an error.
+ */
+ if ((part > 0) || (!allow_whole_dev)) {
+ printf("** No partition table - %s %s **\n", ifname,
+ dev_str);
+ goto cleanup;
+ }
+
info->start = 0;
- info->size = desc->lba;
- info->blksz = desc->blksz;
+ info->size = (*dev_desc)->lba;
+ info->blksz = (*dev_desc)->blksz;
+ info->bootable = 0;
+#ifdef CONFIG_PARTITION_UUIDS
+ info->uuid[0] = 0;
+#endif
- *dev_desc = desc;
- return 0;
+ ret = 0;
+ goto cleanup;
}
- part_str = strchr(dev_str, ':');
- if (part_str) {
- part = (int)simple_strtoul(++part_str, NULL, 16);
- ret = get_partition_info(desc, part, info);
+ /*
+ * Now there's known to be a partition table,
+ * not specifying a partition means to pick partition 1.
+ */
+ if (part == PART_UNSPECIFIED)
+ part = 1;
+
+ /*
+ * If user didn't specify a partition number, or did specify something
+ * other than "auto", use that partition number directly.
+ */
+ if (part != PART_AUTO) {
+ ret = get_partition_info(*dev_desc, part, info);
+ if (ret) {
+ printf("** Invalid partition %d **\n", part);
+ goto cleanup;
+ }
} else {
/*
* Find the first bootable partition.
* If none are bootable, fall back to the first valid partition.
*/
+ part = 0;
for (p = 1; p <= MAX_SEARCH_PARTITIONS; p++) {
ret = get_partition_info(*dev_desc, p, info);
if (ret)
@@ -542,23 +611,23 @@ int get_device_and_partition(const char *ifname, const char *dev_str,
if (p == MAX_SEARCH_PARTITIONS + 1)
*info = tmpinfo;
ret = 0;
+ } else {
+ printf("** No valid partitions found **\n");
+ goto cleanup;
}
}
- if (ret) {
- printf("** Invalid partition %d, use `dev[:part]' **\n", part);
- return -1;
- }
if (strncmp((char *)info->type, BOOT_PART_TYPE, sizeof(info->type)) != 0) {
printf("** Invalid partition type \"%.32s\""
" (expect \"" BOOT_PART_TYPE "\")\n",
info->type);
- return -1;
+ ret = -1;
+ goto cleanup;
}
- *dev_desc = desc;
- return part;
+ ret = part;
+ goto cleanup;
- err:
- puts("** Invalid boot device, use `dev[:part]' **\n");
- return -1;
+cleanup:
+ free(dup_str);
+ return ret;
}
diff --git a/include/part.h b/include/part.h
index 144df4e..3f780a1 100644
--- a/include/part.h
+++ b/include/part.h
@@ -114,9 +114,9 @@ void init_part (block_dev_desc_t *dev_desc);
void dev_print(block_dev_desc_t *dev_desc);
int get_device(const char *ifname, const char *dev_str,
block_dev_desc_t **dev_desc);
-int get_device_and_partition(const char *ifname, const char *dev_str,
+int get_device_and_partition(const char *ifname, const char *dev_part_str,
block_dev_desc_t **dev_desc,
- disk_partition_t *info);
+ disk_partition_t *info, int allow_whole_dev);
#else
static inline block_dev_desc_t *get_dev(const char *ifname, int dev)
{ return NULL; }
@@ -137,9 +137,10 @@ static inline int get_device(const char *ifname, const char *dev_str,
block_dev_desc_t **dev_desc)
{ return -1; }
static inline int get_device_and_partition(const char *ifname,
- const char *dev_str,
+ const char *dev_part_str,
block_dev_desc_t **dev_desc,
- disk_partition_t *info)
+ disk_partition_t *info,
+ int allow_whole_dev)
{ *dev_desc = NULL; return -1; }
#endif
--
1.7.0.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [U-Boot] [PATCH V3 5/8] disk: part_efi: range-check partition number
2012-09-18 22:37 [U-Boot] [PATCH V3 0/8] disk: "part" command and dependencies Stephen Warren
` (3 preceding siblings ...)
2012-09-18 22:37 ` [U-Boot] [PATCH V3 4/8] disk: get_device_and_partition() enhancements Stephen Warren
@ 2012-09-18 22:37 ` Stephen Warren
2012-09-18 22:37 ` [U-Boot] [PATCH V3 6/8] disk: part_efi: parse and store partition UUID Stephen Warren
` (2 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Stephen Warren @ 2012-09-18 22:37 UTC (permalink / raw)
To: u-boot
From: Stephen Warren <swarren@nvidia.com>
Enhance get_partition_info_efi() to range-check the partition number.
This prevents invalid partitions being accessed, and prevents access
beyond the end of the gpt_pte[] array.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v3: No change.
v2: New patch.
---
disk/part_efi.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/disk/part_efi.c b/disk/part_efi.c
index 02927a0..2962fd8 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -173,6 +173,13 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc, int part,
return -1;
}
+ if (part > le32_to_int(gpt_head->num_partition_entries) ||
+ !is_pte_valid(&gpt_pte[part - 1])) {
+ printf("%s: *** ERROR: Invalid partition number %d ***\n",
+ __func__, part);
+ return -1;
+ }
+
/* The ulong casting limits the maximum disk size to 2 TB */
info->start = (ulong) le64_to_int(gpt_pte[part - 1].starting_lba);
/* The ending LBA is inclusive, to calculate size, add 1 to it */
--
1.7.0.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [U-Boot] [PATCH V3 6/8] disk: part_efi: parse and store partition UUID
2012-09-18 22:37 [U-Boot] [PATCH V3 0/8] disk: "part" command and dependencies Stephen Warren
` (4 preceding siblings ...)
2012-09-18 22:37 ` [U-Boot] [PATCH V3 5/8] disk: part_efi: range-check partition number Stephen Warren
@ 2012-09-18 22:37 ` Stephen Warren
2012-09-18 22:37 ` [U-Boot] [PATCH V3 7/8] disk: part_msdos: " Stephen Warren
2012-09-18 22:37 ` [U-Boot] [PATCH V3 8/8] cmd_part: add partition-related command Stephen Warren
7 siblings, 0 replies; 16+ messages in thread
From: Stephen Warren @ 2012-09-18 22:37 UTC (permalink / raw)
To: u-boot
From: Stephen Warren <swarren@nvidia.com>
Each EFI partition table entry contains a UUID. Extend U-Boot's struct
disk_partition to be able to store this information, and modify
get_partition_info_efi() to fill it in.
The implementation of uuid_string() was derived from the Linux kernel,
tag v3.6-rc4 file lib/vsprintf.c function uuid_string().
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v3: Provided pointer to source of uuid_string() code in commit description.
v2: Add #ifdef CONFIG_PARTITION_UUIDS around all new code and struct fields.
---
disk/part.c | 5 +++++
disk/part_efi.c | 25 +++++++++++++++++++++++++
include/part.h | 3 +++
3 files changed, 33 insertions(+), 0 deletions(-)
diff --git a/disk/part.c b/disk/part.c
index 9916708..ede888f 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -296,6 +296,11 @@ void init_part (block_dev_desc_t * dev_desc)
int get_partition_info (block_dev_desc_t *dev_desc, int part
, disk_partition_t *info)
{
+#ifdef CONFIG_PARTITION_UUIDS
+ /* The common case is no UUID support */
+ info->uuid[0] = 0;
+#endif
+
switch (dev_desc->part_type) {
#ifdef CONFIG_MAC_PARTITION
case PART_TYPE_MAC:
diff --git a/disk/part_efi.c b/disk/part_efi.c
index 2962fd8..264ea9c 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -154,6 +154,28 @@ void print_part_efi(block_dev_desc_t * dev_desc)
return;
}
+#ifdef CONFIG_PARTITION_UUIDS
+static void uuid_string(unsigned char *uuid, char *str)
+{
+ static const u8 le[16] = {3, 2, 1, 0, 5, 4, 7, 6, 8, 9, 10, 11,
+ 12, 13, 14, 15};
+ int i;
+
+ for (i = 0; i < 16; i++) {
+ sprintf(str, "%02x", uuid[le[i]]);
+ str += 2;
+ switch (i) {
+ case 3:
+ case 5:
+ case 7:
+ case 9:
+ *str++ = '-';
+ break;
+ }
+ }
+}
+#endif
+
int get_partition_info_efi(block_dev_desc_t * dev_desc, int part,
disk_partition_t * info)
{
@@ -190,6 +212,9 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc, int part,
sprintf((char *)info->name, "%s",
print_efiname(&gpt_pte[part - 1]));
sprintf((char *)info->type, "U-Boot");
+#ifdef CONFIG_PARTITION_UUIDS
+ uuid_string(gpt_pte[part - 1].unique_partition_guid.b, info->uuid);
+#endif
debug("%s: start 0x%lX, size 0x%lX, name %s", __func__,
info->start, info->size, info->name);
diff --git a/include/part.h b/include/part.h
index 3f780a1..27ea283 100644
--- a/include/part.h
+++ b/include/part.h
@@ -94,6 +94,9 @@ typedef struct disk_partition {
uchar name[32]; /* partition name */
uchar type[32]; /* string type description */
int bootable; /* Active/Bootable flag is set */
+#ifdef CONFIG_PARTITION_UUIDS
+ char uuid[37]; /* filesystem UUID as string, if exists */
+#endif
} disk_partition_t;
/* Misc _get_dev functions */
--
1.7.0.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [U-Boot] [PATCH V3 7/8] disk: part_msdos: parse and store partition UUID
2012-09-18 22:37 [U-Boot] [PATCH V3 0/8] disk: "part" command and dependencies Stephen Warren
` (5 preceding siblings ...)
2012-09-18 22:37 ` [U-Boot] [PATCH V3 6/8] disk: part_efi: parse and store partition UUID Stephen Warren
@ 2012-09-18 22:37 ` Stephen Warren
2012-09-18 22:37 ` [U-Boot] [PATCH V3 8/8] cmd_part: add partition-related command Stephen Warren
7 siblings, 0 replies; 16+ messages in thread
From: Stephen Warren @ 2012-09-18 22:37 UTC (permalink / raw)
To: u-boot
From: Stephen Warren <swarren@nvidia.com>
The MSDOS/MBR partition table includes a 32-bit unique ID, often referred
to as the NT disk signature. When combined with a partition number within
the table, this can form a unique ID similar in concept to EFI/GPT's
partition UUID.
This patch generates UUIDs in the format 0002dd75-01, which matches the
format expected by the Linux kernel.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v3: No change.
v2: New patch.
---
disk/part_dos.c | 15 ++++++++++++---
disk/part_dos.h | 2 +-
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/disk/part_dos.c b/disk/part_dos.c
index 24ac00c..c9a3e2b 100644
--- a/disk/part_dos.c
+++ b/disk/part_dos.c
@@ -169,7 +169,8 @@ static void print_partition_extended (block_dev_desc_t *dev_desc, int ext_part_s
*/
static int get_partition_info_extended (block_dev_desc_t *dev_desc, int ext_part_sector,
int relative, int part_num,
- int which_part, disk_partition_t *info)
+ int which_part, disk_partition_t *info,
+ unsigned int disksig)
{
ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz);
dos_partition_t *pt;
@@ -188,6 +189,11 @@ static int get_partition_info_extended (block_dev_desc_t *dev_desc, int ext_part
return -1;
}
+#ifdef CONFIG_PARTITION_UUIDS
+ if (!ext_part_sector)
+ disksig = le32_to_int(&buffer[DOS_PART_DISKSIG_OFFSET]);
+#endif
+
/* Print all primary/logical partitions */
pt = (dos_partition_t *) (buffer + DOS_PART_TBL_OFFSET);
for (i = 0; i < 4; i++, pt++) {
@@ -229,6 +235,9 @@ static int get_partition_info_extended (block_dev_desc_t *dev_desc, int ext_part
/* sprintf(info->type, "%d, pt->sys_ind); */
sprintf ((char *)info->type, "U-Boot");
info->bootable = is_bootable(pt);
+#ifdef CONFIG_PARTITION_UUIDS
+ sprintf(info->uuid, "%08x-%02x", disksig, part_num);
+#endif
return 0;
}
@@ -247,7 +256,7 @@ static int get_partition_info_extended (block_dev_desc_t *dev_desc, int ext_part
return get_partition_info_extended (dev_desc, lba_start,
ext_part_sector == 0 ? lba_start : relative,
- part_num, which_part, info);
+ part_num, which_part, info, disksig);
}
}
return -1;
@@ -261,7 +270,7 @@ void print_part_dos (block_dev_desc_t *dev_desc)
int get_partition_info_dos (block_dev_desc_t *dev_desc, int part, disk_partition_t * info)
{
- return get_partition_info_extended (dev_desc, 0, 0, 1, part, info);
+ return get_partition_info_extended(dev_desc, 0, 0, 1, part, info, 0);
}
diff --git a/disk/part_dos.h b/disk/part_dos.h
index de75542..7b77c1d 100644
--- a/disk/part_dos.h
+++ b/disk/part_dos.h
@@ -24,7 +24,7 @@
#ifndef _DISK_PART_DOS_H
#define _DISK_PART_DOS_H
-
+#define DOS_PART_DISKSIG_OFFSET 0x1b8
#define DOS_PART_TBL_OFFSET 0x1be
#define DOS_PART_MAGIC_OFFSET 0x1fe
#define DOS_PBR_FSTYPE_OFFSET 0x36
--
1.7.0.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [U-Boot] [PATCH V3 8/8] cmd_part: add partition-related command
2012-09-18 22:37 [U-Boot] [PATCH V3 0/8] disk: "part" command and dependencies Stephen Warren
` (6 preceding siblings ...)
2012-09-18 22:37 ` [U-Boot] [PATCH V3 7/8] disk: part_msdos: " Stephen Warren
@ 2012-09-18 22:37 ` Stephen Warren
7 siblings, 0 replies; 16+ messages in thread
From: Stephen Warren @ 2012-09-18 22:37 UTC (permalink / raw)
To: u-boot
From: Stephen Warren <swarren@nvidia.com>
This implements the following:
part uuid mmc 0:1
-> print partition UUID
part uuid mmc 0:1 uuid
-> set environment variable to partition UUID
part list mmc 0
-> list the partitions on the specified device
"part uuid" can be useful when writing a bootcmd which searches all
known devices for something bootable, and then wants the kernel to
use the same partition as the root device, e.g.:
part uuid ${devtype} ${devnum}:${rootpart} uuid
setenv bootargs root=PARTUUID=${uuid} ...
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v3:
* Rebased on top of u-boot/ext4 and Rob Herring's get_device_and_partition
patches.
* Implemented "part list" sub-command.
v2:
* validate that CONFIG_PARTITION_UUID is defined when CONFIG_CMD_PART is
---
common/Makefile | 1 +
common/cmd_part.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 106 insertions(+), 0 deletions(-)
create mode 100644 common/cmd_part.c
diff --git a/common/Makefile b/common/Makefile
index 482795e..b56df1d 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -136,6 +136,7 @@ COBJS-$(CONFIG_CMD_NAND) += cmd_nand.o
COBJS-$(CONFIG_CMD_NET) += cmd_net.o
COBJS-$(CONFIG_CMD_ONENAND) += cmd_onenand.o
COBJS-$(CONFIG_CMD_OTP) += cmd_otp.o
+COBJS-$(CONFIG_CMD_PART) += cmd_part.o
ifdef CONFIG_PCI
COBJS-$(CONFIG_CMD_PCI) += cmd_pci.o
endif
diff --git a/common/cmd_part.c b/common/cmd_part.c
new file mode 100644
index 0000000..d997597
--- /dev/null
+++ b/common/cmd_part.c
@@ -0,0 +1,105 @@
+/*
+ * Copyright (c) 2012, NVIDIA CORPORATION. All rights reserved.
+ *
+ * made from cmd_ext2, which was:
+ *
+ * (C) Copyright 2004
+ * esd gmbh <www.esd-electronics.com>
+ * Reinhard Arlt <reinhard.arlt@esd-electronics.com>
+ *
+ * made from cmd_reiserfs by
+ *
+ * (C) Copyright 2003 - 2004
+ * Sysgo Real-Time Solutions, AG <www.elinos.com>
+ * Pavel Bartusek <pba@sysgo.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <common.h>
+#include <config.h>
+#include <command.h>
+#include <part.h>
+#include <vsprintf.h>
+
+#ifndef CONFIG_PARTITION_UUIDS
+#error CONFIG_PARTITION_UUIDS must be enabled for CONFIG_CMD_PART to be enabled
+#endif
+
+int do_part_uuid(int argc, char * const argv[])
+{
+ int part;
+ block_dev_desc_t *dev_desc;
+ disk_partition_t info;
+
+ if (argc < 2)
+ return CMD_RET_USAGE;
+ if (argc > 3)
+ return CMD_RET_USAGE;
+
+ part = get_device_and_partition(argv[0], argv[1], &dev_desc, &info, 0);
+ if (part < 0)
+ return 1;
+
+ if (argc > 2)
+ setenv(argv[2], info.uuid);
+ else
+ printf("%s\n", info.uuid);
+
+ return 0;
+}
+
+int do_part_list(int argc, char * const argv[])
+{
+ int ret;
+ block_dev_desc_t *desc;
+
+ if (argc != 2)
+ return CMD_RET_USAGE;
+
+ ret = get_device(argv[0], argv[1], &desc);
+ if (ret < 0)
+ return 1;
+
+ print_part(desc);
+
+ return 0;
+}
+
+int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+ if (argc < 2)
+ return CMD_RET_USAGE;
+
+ if (!strcmp(argv[1], "uuid"))
+ return do_part_uuid(argc - 2, argv + 2);
+ else if (!strcmp(argv[1], "list"))
+ return do_part_list(argc - 2, argv + 2);
+
+ return CMD_RET_USAGE;
+}
+
+U_BOOT_CMD(
+ part, 5, 1, do_part,
+ "disk partition related commands",
+ "part uuid <interface> <dev>:<part>\n"
+ " - print partition UUID\n"
+ "part uuid <interface> <dev>:<part> <varname>\n"
+ " - set environment variable to partition UUID\n"
+ "part list <interface> <dev>\n"
+ " - print a device's partition table"
+);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 16+ messages in thread