From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Warren <swarren@wwwdotorg.org>
Date: Tue, 12 Apr 2016 14:56:39 -0600
Subject: [U-Boot] [PATCH 40/44] dm: blk: Allow blk_create_device() to
 allocate the device number
In-Reply-To: <1460256336-30436-41-git-send-email-sjg@chromium.org>
References: <1460256336-30436-1-git-send-email-sjg@chromium.org>
 <1460256336-30436-41-git-send-email-sjg@chromium.org>
Message-ID: <570D6107.70509@wwwdotorg.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 04/09/2016 08:45 PM, Simon Glass wrote:
> Allow a devnum parameter of -1 to indicate that the device number should be
> alocated automatically. The next highest available device number for that
> interface type is used.

> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c

> +int blk_find_max_devnum(enum if_type if_type)
> +{
> +	struct udevice *dev;
> +	int max_devnum = -ENODEV;
> +	struct uclass *uc;
> +	int ret;
> +
> +	ret = uclass_get(UCLASS_BLK, &uc);
> +	if (ret)
> +		return ret;

I'm tempted to suggest that the -ENODEV special case be place here, and 
return 0 instead. Of course, that would require the devnum to be a 
pointer/output parameter rather than encoded into the return value, so 
that other errors could still be returned. Perhaps not worth it.

My reasoning is that clients of this function should just be able to ask 
"what's the next device number" and not care about implementing the 
special cases, but rather rely on this function encoding everything. 
This doesn't matter so much if there's only one call-site though. If 
there is only one call-site, should this be static?

Hmm. This could all be solved by having the function return the next 
free devnum (0..n) (or -ve error) rather than the highest used devnum. 
That way, the -ENODEV special case would return 0, and clients could 
just do:

devnum = blk_find_next_free_devnum(...);
if (devnum < 0)
     return devnum;

and nothing else.

> @@ -428,6 +448,15 @@ int blk_create_device(struct udevice *parent, const char *drv_name,
>   	desc->lba = size / blksz;
>   	desc->part_type = PART_TYPE_UNKNOWN;
>   	desc->bdev = dev;
> +	if (devnum == -1) {
> +		ret = blk_find_max_devnum(if_type);
> +		if (ret == -ENODEV)
> +			devnum = 0;

I'm thinking that clients should have to care about implementing that 
ever time they use the function.

> +		else if (ret < 0)
> +			return ret;
> +		else
> +			devnum = ret + 1;
> +	}
>   	desc->devnum = devnum;