[U-Boot,02/11] DM: add support for scanning DOS partitions to blockdev core

Enable blockdev core to automatically create partitions based on DOS partition
table. This code is based on /disk/, and should eventually replace it.

Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>
---
 drivers/blockdev/Makefile                |   1 +
 drivers/blockdev/core.c                  | 752 +++++++++++++++++++++++++++++++
 drivers/blockdev/part_types/part_dos.c   | 148 ++++++
 drivers/blockdev/part_types/part_dos.h   |  49 ++
 drivers/blockdev/part_types/part_types.h |  34 ++
 5 files changed, 984 insertions(+)
 create mode 100644 drivers/blockdev/core.c
 create mode 100644 drivers/blockdev/part_types/part_dos.c
 create mode 100644 drivers/blockdev/part_types/part_dos.h
 create mode 100644 drivers/blockdev/part_types/part_types.h

Dear Pavel Herrmann,

[..]

> +#define BLOCKDEV_IFTYPE_BITS 4
> +#define BLOCKDEV_IFTYPE_COUNT (1<<BLOCKDEV_IFTYPE_BITS)
> +#define BLOCKDEV_IFTYPE_MAX BLOCKDEV_IFTYPE_COUNT-1

I saw this in blockdev.h

> +struct blockdev_id {
> +	struct {
> +		unsigned type:BLOCKDEV_IFTYPE_BITS;
> +		unsigned number:(16-BLOCKDEV_IFTYPE_BITS);
> +	} disk;
> +	unsigned partition:16;
> +};
> +
> +static struct blockdev_id invalid_id = {{0xf, 0xfff}, 0xffff};
> +static struct blockdev_id empty_id = {{0x0, 0x0}, 0x0};
> +
> +static inline int id_cmp(const struct blockdev_id *left,
> +	const struct blockdev_id *right)
> +{
> +	return memcmp(left, right, sizeof(struct blockdev_id));
> +}
> +
> +struct blockdev_core_entry {
> +	struct list_head	list;
> +	struct instance		*instance;
> +	struct blockdev_ops	*ops;
> +	struct blockdev_id	name;
> +};
> +
> +struct blockdev_core_private {
> +	/* Put cache here */
> +};
> +
> +struct bdid_instance_pair {
> +	struct blockdev_id	id;
> +	struct instance		*inst;
> +};
> +
> +static struct blockdev_core_entry *get_entry_by_instance(struct instance
> *i) +{
> +	struct blockdev_core_entry *tmp;
> +	struct core_instance *core = get_core_instance(CORE_BLOCKDEV);
> +
> +	if (!core)
> +		return NULL;
> +
> +	list_for_each_entry(tmp, &core->succ, list)
> +		if (tmp->instance == i)
> +			return tmp;
> +
> +	return NULL;
> +}
> +
> +static struct blockdev_core_entry *get_entry_by_id(struct blockdev_id id)
> +{
> +	struct blockdev_core_entry *tmp;
> +	struct core_instance *core = get_core_instance(CORE_BLOCKDEV);
> +
> +	if (!core)
> +		return NULL;
> +
> +	list_for_each_entry(tmp, &core->succ, list)
> +		if (!id_cmp(&tmp->name, &id))
> +			return tmp;
> +
> +	return NULL;
> +}
> +
> +static inline int try_match_name(enum blockdev_iftype type,
> +	struct blockdev_id *id, char **name)
> +{
> +	int len = strlen(blockdev_name[type]);
> +
> +	if (!strncmp(*name, blockdev_name[type], len)) {
> +		id->disk.type = type;
> +		*name += len;
> +		return 0;
> +	}
> +	return 1;
> +}
> +
> +static struct blockdev_id get_id_from_name(char *name)
> +{
> +	struct blockdev_id disk_id = empty_id;
> +
> +	if (!try_match_name(BLOCKDEV_IFTYPE_UNKNOWN, &disk_id, &name))
> +		goto get_number;
> +	if (!try_match_name(BLOCKDEV_IFTYPE_ATA, &disk_id, &name))
> +		goto get_number;
> +	if (!try_match_name(BLOCKDEV_IFTYPE_SD, &disk_id, &name))
> +		goto get_number;
> +	if (!try_match_name(BLOCKDEV_IFTYPE_USB, &disk_id, &name))
> +		goto get_number;
> +
> +	return invalid_id;
> +
> +get_number:
> +	/* get disk number from name */
> +	if ((*name < '0') || (*name > '9'))
> +		return invalid_id;
> +
> +	disk_id.disk.number *= 10;
> +	disk_id.disk.number += (*name-'0');
> +	name++;
> +
> +	switch (*name) {
> +	case 0:
> +		return disk_id;
> +	case BLOCKDEV_PARTITION_SEPARATOR:
> +		name += 1;
> +		goto get_part;
> +	default:
> +		goto get_number;
> +	}
> +	return invalid_id;
> +
> +get_part:
> +	/* get partition number fron name */
> +	if ((*name < '0') || (*name > '9'))
> +		return invalid_id;
> +
> +	disk_id.partition *= 10;
> +	disk_id.partition += (*name-'0');
> +	name++;
> +
> +	switch (*name) {
> +	case 0:
> +		return disk_id;
> +	default:
> +		goto get_part;
> +	}
> +
> +	return invalid_id;
> +}
> +
> +static int get_free_index(struct core_instance *core, enum blockdev_iftype
> type) +{
> +	int retval = 0;
> +	struct blockdev_core_entry *entry;
> +
> +	list_for_each_entry(entry, &core->succ, list) {
> +		if ((entry->name.disk.type == type) &&
> +			(entry->name.disk.number >= retval))
> +				retval = entry->name.disk.number + 1;
> +	}
> +
> +	return retval;
> +}
> +
> +static struct blockdev_id create_id_from_hint(struct core_instance *core,
> +	struct instance *dev, void* data)
> +{
> +	struct blockdev_id retval = empty_id;
> +	struct blockdev_core_entry *entry;
> +	struct blockdev_core_hint *hint = data;
> +
> +	/* no hint means we have no idea what type of device we have */
> +	if (!hint)
> +		retval.disk.type = BLOCKDEV_IFTYPE_UNKNOWN;

return here ... you don't need else then.

> +	else {
> +		/* for a partition, we find its parent and use its name */
> +		if (hint->iftype == BLOCKDEV_IFTYPE_PARTITION) {
> +			entry = get_entry_by_instance(dev->bus);
> +			retval.disk = entry->name.disk;
> +			retval.partition = hint->part_number;
> +			return retval;
> +		/* if we have a valid hint for a disk, get a free index */
> +		} else {
> +			retval.disk.type = hint->iftype;
> +			retval.disk.number = get_free_index(core, hint->iftype);
> +			retval.partition = 0;
> +		}
> +	}
> +
> +	return retval;
> +}
> +
> +/* Core API functions */
> +static int get_count(struct core_instance *core)
> +{
> +	int cnt = 0;
> +	struct blockdev_core_entry *entry = NULL;
> +
> +	list_for_each_entry(entry, &core->succ, list)
> +		cnt++;
> +
> +	return cnt;
> +}
> +
> +static struct instance *get_child(struct core_instance *core, int index)
> +{
> +	struct blockdev_core_entry *entry = NULL;
> +
> +	list_for_each_entry(entry, &core->succ, list) {
> +		if (!index)
> +			return entry->instance;
> +		index--;
> +	}
> +
> +	return NULL;
> +}
> +
> +static int bind(struct core_instance *core, struct instance *dev, void
> *ops, +	void *data)
> +{
> +	struct blockdev_core_entry *entry;
> +
> +	if (ops == NULL)
> +		return -EINVAL;
> +
> +	entry = malloc(sizeof(*entry));
> +	if (entry == NULL)
> +		return -ENOMEM;
> +
> +	INIT_LIST_HEAD(&entry->list);
> +	entry->instance = dev;
> +	entry->ops = ops;
> +	entry->name = create_id_from_hint(core, dev, data);
> +	list_add_tail(&entry->list, &core->succ);
> +
> +	return 0;
> +}
> +
> +static int unbind(struct core_instance *core, struct instance *dev)
> +{
> +	struct blockdev_core_entry *entry, *n;
> +
> +	list_for_each_entry_safe(entry, n, &core->succ, list) {
> +		if (entry->instance == dev) {
> +			list_del(&entry->list);
> +			free(entry);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int replace(struct core_instance *core, struct instance *new,
> +	struct instance *old)
> +{
> +	struct blockdev_core_entry *entry = get_entry_by_instance(old);
> +
> +	if (!entry)
> +		return -ENOENT;
> +
> +	entry->instance = new;
> +
> +	return 0;
> +}
> +
> +static int init(struct core_instance *core)

I'd say, rename it to block_core_init() or something, so the syms in u-boot.map 
are unique.

> +{
> +	INIT_LIST_HEAD(&core->succ);
> +	core->private_data = NULL;
> +
> +	return 0;
> +}
> +
> +static int reloc(struct core_instance *core, struct core_instance *old)
> +{
> +	struct blockdev_core_entry *entry, *new;
> +
> +	/* no private_data to copy, yet */
> +
> +	/* fixup links in old list and prepare new list head */
> +	/* FIXME */
> +	/* list_fix_reloc(&old->succ); */
> +	INIT_LIST_HEAD(&core->succ);
> +	core->private_data = NULL;
> +
> +	/* copy list entries to new memory */
> +	list_for_each_entry(entry, &old->succ, list) {
> +		new = malloc(sizeof(*new));
> +		if (!new)
> +			return -ENOMEM;
> +
> +		INIT_LIST_HEAD(&new->list);
> +		new->instance = entry->instance;
> +		new->ops = entry->ops;
> +		new->name = entry->name;
> +		list_add_tail(&new->list, &core->succ);
> +		/*no free at this point, old memory should not be freed*/
> +	}
> +
> +	return 0;
> +}
> +
> +static int destroy(struct core_instance *core)
> +{
> +	struct blockdev_core_entry *entry, *n;
> +
> +	/* destroy private data */
> +	free(core->private_data);
> +	core->private_data = NULL;
> +
> +	/* destroy successor list */
> +	list_for_each_entry_safe(entry, n, &core->succ, list) {
> +		list_del(&entry->list);
> +		free(entry);
> +	}
> +
> +	return 0;
> +}
> +
> +U_BOOT_CORE(CORE_BLOCKDEV,
> +	init,
> +	reloc,
> +	destroy,
> +	get_count,
> +	get_child,
> +	bind,
> +	unbind,
> +	replace);

Sep the stuff below away into separate file. Conditionally compile in one or the 
other.

> +/* Driver wrapping API */
> +lbaint_t blockdev_read(struct instance *i, lbaint_t start, lbaint_t
> blkcnt, +	void *buffer)
> +{
> +	struct blockdev_core_entry *entry = NULL;
> +	struct blockdev_ops *device_ops = NULL;
> +	int error;
> +
> +	entry = get_entry_by_instance(i);
> +	if (!entry)
> +		return -ENOENT;
> +
> +	error = driver_activate(i);
> +	if (error)
> +		return error;
> +
> +	device_ops = entry->ops;
> +	if (!device_ops || !device_ops->read)
> +		return -EINVAL;
> +
> +	return device_ops->read(i, start, blkcnt, buffer);
> +}
> +
> +lbaint_t blockdev_write(struct instance *i, lbaint_t start, lbaint_t
> blkcnt, +	void *buffer)
> +{
> +	struct blockdev_core_entry *entry = NULL;
> +	struct blockdev_ops *device_ops = NULL;
> +	int error;
> +
> +	entry = get_entry_by_instance(i);
> +	if (!entry)
> +		return -ENOENT;
> +
> +	error = driver_activate(i);
> +	if (error)
> +		return error;
> +
> +	device_ops = entry->ops;
> +	if (!device_ops || !device_ops->write)
> +		return -EINVAL;
> +
> +	return device_ops->write(i, start, blkcnt, buffer);
> +}
> +
> +lbaint_t blockdev_erase(struct instance *i, lbaint_t start, lbaint_t
> blkcnt) +{
> +	struct blockdev_core_entry *entry = NULL;
> +	struct blockdev_ops *device_ops = NULL;
> +	int error;
> +
> +	entry = get_entry_by_instance(i);
> +	if (!entry)
> +		return -ENOENT;
> +
> +	error = driver_activate(i);
> +	if (error)
> +		return error;
> +
> +	device_ops = entry->ops;
> +	if (!device_ops)
> +		return -EINVAL;
> +
> +	return device_ops->erase(i, start, blkcnt);
> +}
> +
> +int blockdev_get_option(struct instance *i, enum blockdev_option_code op,
> +	struct option *result)
> +{
> +	struct blockdev_core_entry *entry = NULL;
> +	struct blockdev_ops *device_ops = NULL;
> +	int error;
> +
> +	entry = get_entry_by_instance(i);
> +	if (!entry)
> +		return -ENOENT;
> +
> +	error = driver_activate(i);
> +	if (error)
> +		return error;
> +
> +	device_ops = entry->ops;
> +	if (!device_ops)
> +		return -EINVAL;
> +
> +	return device_ops->get_option(i, op, result);
> +}
> +
> +int blockdev_set_option(struct instance *i, enum blockdev_option_code op,
> +	struct option *value)
> +{
> +	struct blockdev_core_entry *entry = NULL;
> +	struct blockdev_ops *device_ops = NULL;
> +	int error;
> +
> +	entry = get_entry_by_instance(i);
> +	if (!entry)
> +		return -ENOENT;
> +
> +	error = driver_activate(i);
> +	if (error)
> +		return error;
> +
> +	device_ops = entry->ops;
> +	if (!device_ops)
> +		return -EINVAL;
> +
> +	return device_ops->set_option(i, op, value);
> +}
> +
> +/* Command related functions */
> +struct instance *get_blockdev_by_name(char *name)
> +{
> +	struct blockdev_id disk_id = empty_id;
> +	struct blockdev_core_entry *entry;
> +
> +	if (!name)
> +		return NULL;
> +
> +	disk_id = get_id_from_name(name);
> +
> +	if (id_cmp(&disk_id, &invalid_id)) {
> +		entry = get_entry_by_id(disk_id);
> +		if (entry)
> +			return entry->instance;
> +	}
> +
> +	return NULL;
> +}
> +
> +int scan_partitions(struct instance *dev)
> +{
> +	struct blockdev_core_entry *entry;
> +	struct driver_instance *di, *tmp;
> +
> +	entry = get_entry_by_instance(dev);
> +	/* ignore if instance is partition or not a blockdev */
> +	if (!entry || (entry->name.partition != 0))
> +		return -EINVAL;
> +
> +	/* remove all children */
> +	list_for_each_entry_safe(di, tmp, &dev->succ, list) {
> +		driver_remove(&di->i);
> +		driver_unbind(&di->i);
> +	}
> +
> +	/* determine type of partition table and scan for partitions */
> +#ifdef CONFIG_DOS_PARTITION
> +	if (!test_partitions_dos(dev))
> +		return scan_partitions_dos(dev);
> +#endif
> +
> +	return -ENOENT;
> +}
> +
> +static inline int part_number_overflow(int number)
> +{
> +	/* just support 128 partitions for now */
> +	return (number > 128);
> +}
> +
> +int add_partition(struct instance *parent, lbaint_t start, lbaint_t
> length, +	unsigned int number)
> +{
> +	struct blockdev_partition_platform_data *platform = NULL;
> +	struct driver_info *info = NULL;
> +
> +	/* check for overflow in partition number */
> +	if (part_number_overflow(number))
> +		return -EINVAL;
> +
> +	platform = malloc(sizeof(*platform));
> +	info = malloc(sizeof(*info));
> +	if (!platform || !info) {
> +		/* malloc went wrong, cleanup and indicate imminent death */
> +		free(platform);
> +		free(info);
> +		return -ENOMEM;
> +	}
> +
> +	platform->offset = start;
> +	platform->block_count = length;
> +	platform->part_number = number;
> +	info->name = "blockdev_partition";
> +	info->platform_data = platform;
> +	if (!driver_bind(parent, info))
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
> +/* Info printing stuff */
> +static char *type_name(unsigned type)
> +{
> +	switch (type) {
> +	case BLOCKDEV_TYPE_UNKNOWN:
> +		return "Unknown/Not Connected";
> +	case BLOCKDEV_TYPE_HARDDISK:
> +		return "Hard drive";
> +	case BLOCKDEV_TYPE_TAPE:
> +		return "Tape";
> +	case BLOCKDEV_TYPE_CDROM:
> +		return "CDROM";
> +	case BLOCKDEV_TYPE_OPDISK:
> +		return "Optical disk";
> +	default:
> +		return "Unknown";
> +	};
> +}
> +
> +static inline int get_opt_u(struct blockdev_ops *ops, struct instance
> *dev, +	enum blockdev_option_code code, struct option *opt)
> +{
> +	int retval = ops->get_option(dev, code, opt);
> +	if (retval)
> +		return retval;
> +
> +	/* If we dont get the correct type we fail. */
> +	if (OPTION_TYPE(*opt) != OPTION_TYPE_U)
> +		retval = -EINVAL;
> +
> +	/* If we get a mallocated string we should free it. */
> +	if (opt->flags & OPTION_PTR_MALLOCED)
> +		free(opt->data.data_s);
> +
> +	return retval;
> +}
> +
> +static inline int get_opt_s(struct blockdev_ops *ops, struct instance
> *dev, +	enum blockdev_option_code code, struct option *opt)
> +{
> +	int retval = ops->get_option(dev, code, opt);
> +	if (retval)
> +		return retval;
> +
> +	/* If we dont get the correct type we fail. */
> +	if (OPTION_TYPE(*opt) != OPTION_TYPE_S)
> +		retval = -EINVAL;
> +
> +	return retval;
> +}
> +
> +int print_blockdev_info(struct instance *dev)
> +{
> +	struct option opt;
> +	unsigned int type = 0;
> +	unsigned int block_size = 0;
> +	lbaint_t offset = 0;
> +	lbaint_t block_count = 0;
> +	char *vendor = NULL;
> +	char *product = NULL;
> +	char *revision = NULL;
> +	int vendor_malloc = 0;
> +	int product_malloc = 0;
> +	int revision_malloc = 0;
> +	struct blockdev_id id = empty_id;
> +	struct blockdev_core_entry *entry = NULL;
> +	struct blockdev_ops *ops = NULL;
> +	int retval = 0;
> +	enum blockdev_iftype iftype;
> +
> +	entry = get_entry_by_instance(dev);
> +	if (!entry)
> +		return -ENOENT;
> +	else {
> +		id = entry->name;
> +		ops = entry->ops;
> +	}
> +
> +	/* we are not using blockdev_get_option, so we activate manually here */
> +	retval = driver_activate(dev);
> +	if (retval)
> +		return retval;
> +
> +	retval = get_opt_u(ops, dev, BLKD_OPT_TYPE, &opt);
> +	if (retval)
> +		return retval;
> +	type = opt.data.data_u;
> +
> +	if (type == BLOCKDEV_TYPE_PARTITION) {
> +		/* get options that make sense for a partition */
> +		retval = get_opt_u(ops, dev, BLKD_OPT_BLOCKCOUNT, &opt);
> +		if (retval)
> +			return retval;
> +		block_count = opt.data.data_u;
> +
> +		retval = get_opt_u(ops, dev, BLKD_OPT_OFFSET, &opt);
> +		if (retval)
> +			return retval;
> +		offset = opt.data.data_u;
> +
> +		/* print some information message */
> +		printf("%s%d:%d\n\tpartition on %s%d\n\t"
> +			"offset: %lu\n\tblock count: %lu\n\n",
> +			blockdev_name[id.disk.type], id.disk.number,
> +			id.partition, blockdev_name[id.disk.type],
> +			id.disk.number, offset, block_count);
> +
> +	} else {
> +		/* get options that make sense for a disk */
> +		retval = get_opt_u(ops, dev, BLKD_OPT_IFTYPE, &opt);
> +		if (retval)
> +			return retval;
> +		iftype = opt.data.data_u;
> +
> +		retval = get_opt_u(ops, dev, BLKD_OPT_BLOCKSIZE, &opt);
> +		if (retval)
> +			return retval;
> +		block_size = opt.data.data_u;
> +
> +		retval = get_opt_u(ops, dev, BLKD_OPT_BLOCKCOUNT, &opt);
> +		if (retval)
> +			return retval;
> +		block_count = opt.data.data_u;
> +
> +		retval = get_opt_s(ops, dev, BLKD_OPT_VENDOR, &opt);
> +		if (retval)
> +			return retval;
> +		vendor = opt.data.data_s;
> +		vendor_malloc = opt.flags & OPTION_PTR_MALLOCED;
> +
> +		retval = get_opt_s(ops, dev, BLKD_OPT_PRODUCT, &opt);
> +		if (retval)
> +			return retval;
> +		product = opt.data.data_s;
> +		product_malloc = opt.flags & OPTION_PTR_MALLOCED;
> +
> +		retval = get_opt_s(ops, dev, BLKD_OPT_REVISION, &opt);
> +		if (retval)
> +			return retval;
> +		revision = opt.data.data_s;
> +		revision_malloc = opt.flags & OPTION_PTR_MALLOCED;
> +
> +		/* print some information message */
> +		printf("%s%d\n\tvendor: %s\n\tproduct: %s\n\t"
> +			"revision: %s\n\ttype: %s\n\tiftype: %s\n\t"
> +			"block size: %d\n\tblock count: %lu\n\n",
> +			blockdev_name[id.disk.type], id.disk.number,
> +			vendor, product, revision, type_name(type),
> +			iftype_name[iftype], block_size, block_count);
> +
> +		/*cleanup if we got dynamic memory pointers*/
> +		if (vendor_malloc)
> +			free(vendor);
> +
> +		if (product_malloc)
> +			free(product);
> +
> +		if (revision_malloc)
> +			free(revision);
> +	}
> +
> +	return retval;
> +}
> +
> +static void sort_bdid_i(struct bdid_instance_pair *data, size_t count)
> +{
> +	/* use bubble sort for now */
> +	int a, b;
> +	struct bdid_instance_pair tswap;
> +
> +	for (a = 1; a < count; a++) {
> +		for (b = a; b > 0; b--) {
> +			if (id_cmp(&data[b].id, &data[b-1].id) < 0) {
> +				/*swap position b and b-1 */
> +				tswap = data[b-1];
> +				data[b-1] = data[b];
> +				data[b] = tswap;
> +			}
> +		}
> +	}
> +}
> +
> +int print_blockdev_info_all(void)
> +{
> +	struct core_instance *core = NULL;
> +	struct bdid_instance_pair *sorted_pairs = NULL;
> +	struct blockdev_core_entry *entry = NULL;
> +	int count = 0;
> +	int idx = 0;
> +
> +	core = get_core_instance(CORE_BLOCKDEV);
> +	if (!core)
> +		return -ENOMEM;
> +
> +	count = core_get_count(CORE_BLOCKDEV);
> +	sorted_pairs = malloc(sizeof(*sorted_pairs) * count);
> +	if (!sorted_pairs)
> +		return -ENOMEM;
> +
> +	/* get list of all instances and associated IDs */
> +	list_for_each_entry(entry, &core->succ, list) {
> +		sorted_pairs[idx].id = entry->name;
> +		sorted_pairs[idx].inst = entry->instance;
> +		idx++;
> +	}
> +
> +	/* sort isntances by ID */
> +	sort_bdid_i(sorted_pairs, count);
> +
> +	/* print info about each instance */
> +	for (idx = 0; idx < count; idx++)
> +		print_blockdev_info(sorted_pairs[idx].inst);
> +	return 0;
> +}
> diff --git a/drivers/blockdev/part_types/part_dos.c
> b/drivers/blockdev/part_types/part_dos.c new file mode 100644
> index 0000000..7d19818
> --- /dev/null
> +++ b/drivers/blockdev/part_types/part_dos.c
> @@ -0,0 +1,148 @@
> +/*
> + * (C) Copyright 2001
> + * Raymond Lo, lo@routefree.com
> + * Wolfgang Denk, DENX Software Engineering, wd@denx.de.
> + *
> + * 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, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +/*
> + * Support for harddisk partitions.
> + *
> + * To be compatible with LinuxPPC and Apple we use the standard Apple
> + * SCSI disk partitioning scheme. For more information see:
> + *
> http://developer.apple.com/techpubs/mac/Devices/Devices-126.html#MARKER-14
> -92 + */
> +
> +#include <common.h>
> +#include <ide.h>
> +#include "part_dos.h"
> +#include <dm/blockdev.h>
> +
> +#if defined(CONFIG_CMD_IDE) || \
> +	defined(CONFIG_CMD_MG_DISK) || \
> +	defined(CONFIG_CMD_SATA) || \
> +	defined(CONFIG_CMD_SCSI) || \
> +	defined(CONFIG_CMD_USB) || \
> +	defined(CONFIG_MMC) || \
> +	defined(CONFIG_SYSTEMACE)
> +
> +/* Convert char[4] in little endian format to the host format integer
> + */
> +static inline int le32_to_int(unsigned char *le32)
> +{
> +	return ((le32[3] << 24) +
> +		(le32[2] << 16) +
> +		(le32[1] << 8) +
> +		le32[0]);
> +}
> +
> +static inline int is_extended(int part_type)
> +{
> +	return (part_type == 0x5 ||
> +		part_type == 0xf ||
> +		part_type == 0x85);
> +}
> +
> +int test_partitions_dos(struct instance *dev)
> +{
> +	struct option blksz;
> +	int error = blockdev_get_option(dev, BLKD_OPT_BLOCKSIZE, &blksz);
> +	if (error)
> +		return error;
> +
> +	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, blksz.data.data_u);
> +
> +	if ((blockdev_read(dev, 0, 1, buffer) != 1) ||
> +	    (buffer[DOS_PART_MAGIC_OFFSET + 0] != 0x55) ||
> +	    (buffer[DOS_PART_MAGIC_OFFSET + 1] != 0xaa)) {
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +
> +
> +int scan_partitions_dos(struct instance *dev)
> +{
> +	struct dos_partition *pt;
> +	lbaint_t extpt_sector = 0;
> +	struct option blksz;
> +	int error = blockdev_get_option(dev, BLKD_OPT_BLOCKSIZE, &blksz);
> +	if (error)
> +		return error;
> +
> +	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, blksz.data.data_u);
> +
> +	if (blockdev_read(dev, 0, 1, buffer) != 1) {
> +		printf("** Can't read partition table **\n");
> +		return -EINVAL;
> +	}
> +
> +	if (buffer[DOS_PART_MAGIC_OFFSET] != 0x55 ||
> +		buffer[DOS_PART_MAGIC_OFFSET + 1] != 0xaa) {
> +		printf("bad MBR sector signature 0x%02x%02x\n",
> +			buffer[DOS_PART_MAGIC_OFFSET],
> +			buffer[DOS_PART_MAGIC_OFFSET + 1]);
> +		return -EINVAL;
> +	}
> +
> +	pt = (struct dos_partition *) (buffer + DOS_PART_TBL_OFFSET);
> +	int i;
> +	for (i = 0; i < 4; i++, pt++) {
> +		if (((pt->boot_ind & ~0x80) == 0) &&
> +				(pt->sys_ind != 0) &&
> +				(is_extended(pt->sys_ind) == 0)) {
> +			add_partition(dev, le32_to_int(pt->start4),
> +				le32_to_int(pt->size4), i+1);
> +		}
> +		if (is_extended(pt->sys_ind))
> +			extpt_sector = le32_to_int(pt->start4);
> +	}
> +
> +	if (extpt_sector == 0)
> +		return 0;
> +
> +	/* repeat once for extended partitions */
> +	if (blockdev_read(dev, extpt_sector, 1, buffer) != 1) {
> +		printf("** Can't read extended partition table **\n");
> +		return -EINVAL;
> +	}
> +
> +	if (buffer[DOS_PART_MAGIC_OFFSET] != 0x55 ||
> +		buffer[DOS_PART_MAGIC_OFFSET + 1] != 0xaa) {
> +		printf("bad MBR sector signature 0x%02x%02x\n",
> +			buffer[DOS_PART_MAGIC_OFFSET],
> +			buffer[DOS_PART_MAGIC_OFFSET + 1]);
> +		return -EINVAL;
> +	}
> +
> +	pt = (struct dos_partition *) (buffer + DOS_PART_TBL_OFFSET);
> +	for (i = 0; i < 4; i++, pt++) {
> +		if (((pt->boot_ind & ~0x80) == 0) &&
> +				(pt->sys_ind != 0) &&
> +				(is_extended(pt->sys_ind) == 0)) {
> +			add_partition(dev, le32_to_int(pt->start4),
> +				le32_to_int(pt->size4), i+5);
> +		}
> +	}
> +
> +	return 0;
> +}
> +#endif
> diff --git a/drivers/blockdev/part_types/part_dos.h
> b/drivers/blockdev/part_types/part_dos.h new file mode 100644
> index 0000000..98b0293
> --- /dev/null
> +++ b/drivers/blockdev/part_types/part_dos.h
> @@ -0,0 +1,49 @@
> +/*
> + * (C) Copyright 2000
> + * Wolfgang Denk, DENX Software Engineering, wd@denx.de.
> + *
> + * 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, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#ifndef _DISK_PART_DOS_H
> +#define _DISK_PART_DOS_H
> +
> +
> +#define DOS_PART_TBL_OFFSET	0x1be
> +#define DOS_PART_MAGIC_OFFSET	0x1fe
> +#define DOS_PBR_FSTYPE_OFFSET	0x36
> +#define DOS_PBR32_FSTYPE_OFFSET	0x52
> +#define DOS_PBR_MEDIA_TYPE_OFFSET	0x15
> +#define DOS_MBR	0
> +#define DOS_PBR	1
> +
> +struct dos_partition {
> +	unsigned char boot_ind;		/* 0x80 - active		*/
> +	unsigned char head;		/* starting head		*/
> +	unsigned char sector;		/* starting sector		*/
> +	unsigned char cyl;		/* starting cylinder		*/
> +	unsigned char sys_ind;		/* What partition type		*/
> +	unsigned char end_head;		/* end head			*/
> +	unsigned char end_sector;	/* end sector			*/
> +	unsigned char end_cyl;		/* end cylinder			*/
> +	unsigned char start4[4];	/* starting sector counting from 0*/
> +	unsigned char size4[4];		/* nr of sectors in partition	*/
> +};
> +
> +#endif	/* _DISK_PART_DOS_H */
> diff --git a/drivers/blockdev/part_types/part_types.h
> b/drivers/blockdev/part_types/part_types.h new file mode 100644
> index 0000000..d5e6f61
> --- /dev/null
> +++ b/drivers/blockdev/part_types/part_types.h
> @@ -0,0 +1,34 @@
> +/*
> + * (C) Copyright 2012
> + * Pavel Herrmann <morpheus.ibis@gmail.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, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#ifndef _BLOCKDEV_PART_TYPES_H_
> +#define _BLOCKDEV_PART_TYPES_H_ 1
> +
> +#include <dm/structures.h>
> +
> +#ifdef CONFIG_DOS_PARTITION
> +int test_partitions_dos(struct instance *i);
> +int scan_partitions_dos(struct instance *i);
> +#endif
> +
> +#endif

On Thursday 20 of September 2012 22:03:05 Marek Vasut wrote:
> Dear Pavel Herrmann,
> 
> [..]
> 
> > +#define BLOCKDEV_IFTYPE_BITS 4
> > +#define BLOCKDEV_IFTYPE_COUNT (1<<BLOCKDEV_IFTYPE_BITS)
> > +#define BLOCKDEV_IFTYPE_MAX BLOCKDEV_IFTYPE_COUNT-1
> 
> I saw this in blockdev.h

My bad then, sorry.

...
> > +}
> > +
> > +static int replace(struct core_instance *core, struct instance *new,
> > +	struct instance *old)
> > +{
> > +	struct blockdev_core_entry *entry = get_entry_by_instance(old);
> > +
> > +	if (!entry)
> > +		return -ENOENT;
> > +
> > +	entry->instance = new;
> > +
> > +	return 0;
> > +}
> > +
> > +static int init(struct core_instance *core)
> 
> I'd say, rename it to block_core_init() or something, so the syms in
> u-boot.map are unique.

thic being static, how could it show in u-boot.map?

> > +{
> > +	INIT_LIST_HEAD(&core->succ);
> > +	core->private_data = NULL;
> > +
> > +	return 0;
> > +}
> > +
> > +static int reloc(struct core_instance *core, struct core_instance *old)
> > +{
> > +	struct blockdev_core_entry *entry, *new;
> > +
> > +	/* no private_data to copy, yet */
> > +
> > +	/* fixup links in old list and prepare new list head */
> > +	/* FIXME */
> > +	/* list_fix_reloc(&old->succ); */
> > +	INIT_LIST_HEAD(&core->succ);
> > +	core->private_data = NULL;
> > +
> > +	/* copy list entries to new memory */
> > +	list_for_each_entry(entry, &old->succ, list) {
> > +		new = malloc(sizeof(*new));
> > +		if (!new)
> > +			return -ENOMEM;
> > +
> > +		INIT_LIST_HEAD(&new->list);
> > +		new->instance = entry->instance;
> > +		new->ops = entry->ops;
> > +		new->name = entry->name;
> > +		list_add_tail(&new->list, &core->succ);
> > +		/*no free at this point, old memory should not be freed*/
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int destroy(struct core_instance *core)
> > +{
> > +	struct blockdev_core_entry *entry, *n;
> > +
> > +	/* destroy private data */
> > +	free(core->private_data);
> > +	core->private_data = NULL;
> > +
> > +	/* destroy successor list */
> > +	list_for_each_entry_safe(entry, n, &core->succ, list) {
> > +		list_del(&entry->list);
> > +		free(entry);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +U_BOOT_CORE(CORE_BLOCKDEV,
> > +	init,
> > +	reloc,
> > +	destroy,
> > +	get_count,
> > +	get_child,
> > +	bind,
> > +	unbind,
> > +	replace);
> 
> Sep the stuff below away into separate file. Conditionally compile in one or
> the other.

I distinctly remember you saying to put all this into one file (as opposed to 3 
it was before), so why the turn-around now?
No idea what you mean by "one or the other" - you need all this code for it to 
work.

> > +/* Driver wrapping API */
> > +lbaint_t blockdev_read(struct instance *i, lbaint_t start, lbaint_t
> > blkcnt, +	void *buffer)
> > +{
> > +	struct blockdev_core_entry *entry = NULL;
> > +	struct blockdev_ops *device_ops = NULL;
> > +	int error;
> > +
> > +	entry = get_entry_by_instance(i);
> > +	if (!entry)
> > +		return -ENOENT;
> > +
> > +	error = driver_activate(i);
> > +	if (error)
> > +		return error;
> > +
> > +	device_ops = entry->ops;
> > +	if (!device_ops || !device_ops->read)
> > +		return -EINVAL;
> > +
> > +	return device_ops->read(i, start, blkcnt, buffer);
> > +}

Pavel Herrmann

Dear Pavel Herrmann,

[...]

> > > +static int init(struct core_instance *core)
> > 
> > I'd say, rename it to block_core_init() or something, so the syms in
> > u-boot.map are unique.
> 
> thic being static, how could it show in u-boot.map?

Argh, not u-boot.map, sorry. But it's much easier for git grep to look up unique 
syms than this.

> > > +{
> > > +	INIT_LIST_HEAD(&core->succ);
> > > +	core->private_data = NULL;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int reloc(struct core_instance *core, struct core_instance
> > > *old) +{
> > > +	struct blockdev_core_entry *entry, *new;
> > > +
> > > +	/* no private_data to copy, yet */
> > > +
> > > +	/* fixup links in old list and prepare new list head */
> > > +	/* FIXME */
> > > +	/* list_fix_reloc(&old->succ); */
> > > +	INIT_LIST_HEAD(&core->succ);
> > > +	core->private_data = NULL;
> > > +
> > > +	/* copy list entries to new memory */
> > > +	list_for_each_entry(entry, &old->succ, list) {
> > > +		new = malloc(sizeof(*new));
> > > +		if (!new)
> > > +			return -ENOMEM;
> > > +
> > > +		INIT_LIST_HEAD(&new->list);
> > > +		new->instance = entry->instance;
> > > +		new->ops = entry->ops;
> > > +		new->name = entry->name;
> > > +		list_add_tail(&new->list, &core->succ);
> > > +		/*no free at this point, old memory should not be freed*/
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int destroy(struct core_instance *core)
> > > +{
> > > +	struct blockdev_core_entry *entry, *n;
> > > +
> > > +	/* destroy private data */
> > > +	free(core->private_data);
> > > +	core->private_data = NULL;
> > > +
> > > +	/* destroy successor list */
> > > +	list_for_each_entry_safe(entry, n, &core->succ, list) {
> > > +		list_del(&entry->list);
> > > +		free(entry);
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +U_BOOT_CORE(CORE_BLOCKDEV,
> > > +	init,
> > > +	reloc,
> > > +	destroy,
> > > +	get_count,
> > > +	get_child,
> > > +	bind,
> > > +	unbind,
> > > +	replace);
> > 
> > Sep the stuff below away into separate file. Conditionally compile in one
> > or the other.
> 
> I distinctly remember you saying to put all this into one file (as opposed
> to 3 it was before), so why the turn-around now?

Well, I didn't see the code before, so I couldn't make a firm decision, sorry.

> No idea what you mean by "one or the other" - you need all this code for it
> to work.

You need the "driver wrapping API" if DM is enabled? I do not understand this, 
please elaborate!

What about having a common part for both cases and then compile either the DM 
part or non-DM part conditionally?

> > > +/* Driver wrapping API */
> > > +lbaint_t blockdev_read(struct instance *i, lbaint_t start, lbaint_t
> > > blkcnt, +	void *buffer)
> > > +{
> > > +	struct blockdev_core_entry *entry = NULL;
> > > +	struct blockdev_ops *device_ops = NULL;
> > > +	int error;
> > > +
> > > +	entry = get_entry_by_instance(i);
> > > +	if (!entry)
> > > +		return -ENOENT;
> > > +
> > > +	error = driver_activate(i);
> > > +	if (error)
> > > +		return error;
> > > +
> > > +	device_ops = entry->ops;
> > > +	if (!device_ops || !device_ops->read)
> > > +		return -EINVAL;
> > > +
> > > +	return device_ops->read(i, start, blkcnt, buffer);
> > > +}
> 
> Pavel Herrmann

Best regards,
Marek Vasut

On Friday 21 of September 2012 14:47:24 Marek Vasut wrote:
> Dear Pavel Herrmann,
> 
> [...]
> 
> > > > +static int init(struct core_instance *core)
> > > 
> > > I'd say, rename it to block_core_init() or something, so the syms in
> > > u-boot.map are unique.
> > 
> > thic being static, how could it show in u-boot.map?
> 
> Argh, not u-boot.map, sorry. But it's much easier for git grep to look up
> unique syms than this.

ah, OK then

> > > > +{
> > > > +	INIT_LIST_HEAD(&core->succ);
> > > > +	core->private_data = NULL;
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static int reloc(struct core_instance *core, struct core_instance
> > > > *old) +{
> > > > +	struct blockdev_core_entry *entry, *new;
> > > > +
> > > > +	/* no private_data to copy, yet */
> > > > +
> > > > +	/* fixup links in old list and prepare new list head */
> > > > +	/* FIXME */
> > > > +	/* list_fix_reloc(&old->succ); */
> > > > +	INIT_LIST_HEAD(&core->succ);
> > > > +	core->private_data = NULL;
> > > > +
> > > > +	/* copy list entries to new memory */
> > > > +	list_for_each_entry(entry, &old->succ, list) {
> > > > +		new = malloc(sizeof(*new));
> > > > +		if (!new)
> > > > +			return -ENOMEM;
> > > > +
> > > > +		INIT_LIST_HEAD(&new->list);
> > > > +		new->instance = entry->instance;
> > > > +		new->ops = entry->ops;
> > > > +		new->name = entry->name;
> > > > +		list_add_tail(&new->list, &core->succ);
> > > > +		/*no free at this point, old memory should not be freed*/
> > > > +	}
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static int destroy(struct core_instance *core)
> > > > +{
> > > > +	struct blockdev_core_entry *entry, *n;
> > > > +
> > > > +	/* destroy private data */
> > > > +	free(core->private_data);
> > > > +	core->private_data = NULL;
> > > > +
> > > > +	/* destroy successor list */
> > > > +	list_for_each_entry_safe(entry, n, &core->succ, list) {
> > > > +		list_del(&entry->list);
> > > > +		free(entry);
> > > > +	}
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +U_BOOT_CORE(CORE_BLOCKDEV,
> > > > +	init,
> > > > +	reloc,
> > > > +	destroy,
> > > > +	get_count,
> > > > +	get_child,
> > > > +	bind,
> > > > +	unbind,
> > > > +	replace);
> > > 
> > > Sep the stuff below away into separate file. Conditionally compile in
> > > one
> > > or the other.
> > 
> > I distinctly remember you saying to put all this into one file (as opposed
> > to 3 it was before), so why the turn-around now?
> 
> Well, I didn't see the code before, so I couldn't make a firm decision,
> sorry.
> > No idea what you mean by "one or the other" - you need all this code for
> > it
> > to work.
> 
> You need the "driver wrapping API" if DM is enabled? I do not understand
> this, please elaborate!
> 
> What about having a common part for both cases and then compile either the
> DM part or non-DM part conditionally?

this is all DM-only.
driver-wrapping API is wrapper/proxy functions to driver ops (aka it wraps 
around the ops, doing lookup and activate and whatnot), not a wrapper for old 
API (which would be in a driver mind you)

> > > > +/* Driver wrapping API */
> > > > +lbaint_t blockdev_read(struct instance *i, lbaint_t start, lbaint_t
> > > > blkcnt, +	void *buffer)
> > > > +{
> > > > +	struct blockdev_core_entry *entry = NULL;
> > > > +	struct blockdev_ops *device_ops = NULL;
> > > > +	int error;
> > > > +
> > > > +	entry = get_entry_by_instance(i);
> > > > +	if (!entry)
> > > > +		return -ENOENT;
> > > > +
> > > > +	error = driver_activate(i);
> > > > +	if (error)
> > > > +		return error;
> > > > +
> > > > +	device_ops = entry->ops;
> > > > +	if (!device_ops || !device_ops->read)
> > > > +		return -EINVAL;
> > > > +
> > > > +	return device_ops->read(i, start, blkcnt, buffer);
> > > > +}
> > 
> > Pavel Herrmann
> 
> Best regards,
> Marek Vasut

Dear Pavel Herrmann,

[...]

> > You need the "driver wrapping API" if DM is enabled? I do not understand
> > this, please elaborate!
> > 
> > What about having a common part for both cases and then compile either
> > the DM part or non-DM part conditionally?
> 
> this is all DM-only.
> driver-wrapping API is wrapper/proxy functions to driver ops (aka it wraps
> around the ops, doing lookup and activate and whatnot), not a wrapper for
> old API (which would be in a driver mind you)

I see ... do you still fail to understand why I'm being such a 
documentation/comments bitch?

[...]

Best regards,
Marek Vasut