Subject: Re: [PATCH 1/3] [POWERPC] FSL UPM: routines to manage FSL UPMs

From: Anton Vorontsov <[email-masked]>
Date: Sun, 23 Dec 2007 14:59:35 +0300

On Sun, Dec 23, 2007 at 01:24:57PM +1100, Stephen Rothwell wrote:
> On Fri, 21 Dec 2007 23:39:25 +0300 Anton Vorontsov <[email-masked]> wrote:
> >
> > +int fsl_upm_get_for(struct device_node *node, const char *name,
> > +		    struct fsl_upm *upm)
> > +{
> > +	int ret;
> > +	struct device_node *lbus;
> > +	struct resource lbc_res;
> > +	ptrdiff_t mxmr_offs;
> > +
> > +	lbus = of_get_parent(node);
> > +	if (!lbus) {
> > +		pr_err("FSL UPM: can't get parent local bus node\n");
> > +		return -ENOENT;
> > +	}
> > +
> > +	ret = of_address_to_resource(lbus, 0, &lbc_res);
> 
> of_node_put(lbus) as of_get_parent() gets a reference.

Thanks, will fix.

> > +static inline void fsl_upm_start_pattern(struct fsl_upm *upm, u32 pat_offset)
> > +{
> > +	spin_lock_irqsave(&upm_lock, upm_lock_flags);
> 
> I may be wrong, but don't we need the "flags" argument to
> spin_lock_irqsave to be on the stack?  And the save and restore to be in
> the same function?

In general case, yes. Here, not exactly. We have to grab a lock at the
start(), do runs(), and release a lock at the end():

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