Subject: Re: [PATCH 6/6] of/device: populate platform_device (of_device) resource table on allocation

From: Anton Vorontsov <[email-masked]>
Date: Tue, 8 Jun 2010 19:57:02 +0400

On Tue, Jun 08, 2010 at 08:26:43AM -0600, Grant Likely wrote:
[...]
> +	dev = kzalloc(sizeof(*dev) + (sizeof(struct resource) * i), GFP_KERNEL);
>  	if (!dev)
>  		return NULL;
> -
>  	dev->dev.of_node = of_node_get(np);
>  	dev->dev.dma_mask = &dev->archdata.dma_mask;
>  	dev->dev.parent = parent;
>  	dev->dev.release = of_release_dev;
>  
> +	/* Populate the resource table */
> +	if (num_irq || num_reg) {
> +		dev->resource = (void*)&dev[1];

This is ugly. Why not allocate the memory specifically for
dev->resource? Is this because you plan to get rid of
of_release_dev(), and the generic release_dev() won't
know if it should free the dev->resource? There must
be a better way to handle this.

p.s.

[Two Minutes Hate for Grant.]

Just wonder what happened to of_gpio stuff? You blocked it
in 2.6.34 for no reason saying "I'll pick it into my OF
tree before the 2.6.35 merge window" and it's 2.6.36 merge
window quite soon.

It's still in origin/test-devicetree, which is obviously
not -next. What is your plan now?

Thanks,

> +		dev->num_resources = num_reg + num_irq;
> +		res = dev->resource;
> +		for (i = 0; i < num_reg; i++, res++) {
> +			rc = of_address_to_resource(np, i, res);
> +			WARN_ON(rc);
> +		}
> +		for (i = 0; i < num_irq; i++, res++) {
> +			rc = of_irq_to_resource(np, i, res);
> +			WARN_ON(rc == NO_IRQ);
> +		}
> +	}
> +
>  	if (bus_id)
>  		dev_set_name(&dev->dev, "%s", bus_id);
>  	else

-- 
Anton Vorontsov
email: [email-masked]
irc://irc.freenode.net/bd2