[PATCH 04/13] ARM: clps711x: Add clocksource driver

[Sascha Hauer]

On Mon, Mar 11, 2013 at 08:22:05PM +0400, Alexander Shiyan wrote:
> > On Mon, Mar 11, 2013 at 12:01:58PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > > +static int clps711x_cs_probe(struct device_d *dev)
> > > > +{
> > > > +	u32 rate;
> > > > +	struct clk *timer_clk;
> > > > +
> > > > +	timer_clk = clk_get(dev, NULL);
> > > > +	if (IS_ERR(timer_clk))
> > > > +		return PTR_ERR(timer_clk);
> > > > +
> > > > +	rate = clk_get_rate(timer_clk);
> > > > +	clps711x_timer_base = dev_request_mem_region(dev, 0);
> > > > +	if (!clps711x_timer_base) {
> > > > +		clk_put(timer_clk);
> > > > +		return -ENOENT;
> > > > +	}
> > > this deserve a nice crash
> > 
> > No, it doesn't. First of all we are very early here, so we might not even
> > see the crash. Then, with devicetree probing we may often end up with
> > the same devices registered from the devicetree and from the
> > platform/soc. While this should find a way to avoid duplicate device
> > registration, it is not nice having barebox crash in this case.
> 
> So what is the solution you propose in this case?

For the device duplication I don't have a solution yet. Basically I
wanted to say that your patch looks good the way you did it.

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 |