Re: [PATCH 1/7] device: introduce resource structure to simplify resource declaration

[Sascha Hauer <s.hauer@pengutronix.de>]

On Mon, Jul 18, 2011 at 06:43:47AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > +#endif /* __ASSEMBLY__ */
> > > +#endif	/* _LINUX_IOPORT_H */
> > > diff --git a/lib/driver.c b/lib/driver.c
> > > index 4c10a49..ee5e850 100644
> > > --- a/lib/driver.c
> > > +++ b/lib/driver.c
> > > @@ -103,6 +103,15 @@ int register_device(struct device_d *new_device)
> > >  {
> > >  	struct driver_d *drv;
> > >  
> > > +	/* if no map_base available use the first resource if available
> > > +	 * so we do not need to duplicate it
> > > +	 * Temporary fixup until we get rid of map_base and size
> > > +	 */
> > > +	if (new_device->map_base == 0 && new_device->resource) {
> > > +		new_device->map_base = new_device->resource[0].start;
> > > +		new_device->size = new_device->resource[0].size;
> > > +	}
> > 
> > I think we don't need this. What we need though is the other way round:
> > 
> > 	if (new_device->map_base) {
> > 		dev_warn(new_device, "uses map_base. Please convert to use resources\n")
> > 		new_device->resource = xzalloc(sizeof(struct resource));
> > 		new_device->resource[0].start = new_device->map_base;
> > 		new_device->resource[0].size = new_device->size;
> > 	}
> > 
> > This way all devices have resources, we can convert the drivers and
> > devices to use resources one by one.
> ok for the warning
> 
> but if we do not keep the map_base uptodate the core device drivers will be
> broken so if no map_base is specified we must update it with the first
> resource

You're right.

> 
> how about this
> 
> 	if (new_device->map_base) {
> 		if (new_device->resource) {
> 			dev_err(new_device, "map_base and resource specifed\n");
> 			return -EIO;
> 		}
> 		dev_warn(new_device, "uses map_base. Please convert to use resources\n")
> 		new_device->resource = xzalloc(sizeof(struct resource));
> 		new_device->resource[0].start = new_device->map_base;
> 		new_device->resource[0].size = new_device->size;
> 		new_device->numresources = 1;
> 	} else if (new_device->resource) {
> 		new_device->map_base = new_device->resource[0].start;
> 		new_device->size = new_device->resource[0].size;
> 	}

Ok, this looks good.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox