Hi Dale,
Thanks for the comments, I will re-submit the patch on Monday without
the of_node_put and split in 2.
Remi
On Fri, 2008-04-18 at 13:58 -0700, Dale Farnsworth wrote:
> On Thu, Apr 17, 2008 at 04:35:55PM -0700, Remi Machet wrote:
> > This patch affects only the mv64x60 driver. It fixes 2 problem:
>
> Hi Remi,
>
> Thanks for finding and addressing these issues.
> BTW, since you're fixing 2 problems, you might split this into 2 patches.
>
> > -If one of the devices of the mv64x60 init fails, the remaining
> > devices are not initialized => I changed the code to display an
> > error and continue the initialization.
>
> I agree that this is a good idea. A small comment on details below.
>
> > -I2C parameters freq_m and freq_n are assigned default in the code
> > but if those properties are not found in the open firmware description
> > the init returns an error=> the code now uses the default
> > values if the properties are not found.
>
> Yes. Sigh, I can't believe I missed this.
>
> > diff --git a/arch/powerpc/sysdev/mv64x60_dev.c
> > b/arch/powerpc/sysdev/mv64x60_dev.c
> > index 047b310..ef0fc99 100644
> > --- a/arch/powerpc/sysdev/mv64x60_dev.c
> > +++ b/arch/powerpc/sysdev/mv64x60_dev.c
> > @@ -433,9 +431,15 @@ static int __init mv64x60_device_setup(void)
> > int err;
> >
> > id = 0;
> > - for_each_compatible_node(np, "serial", "marvell,mv64360-mpsc")
> > - if ((err = mv64x60_mpsc_device_setup(np, id++)))
> > - goto error;
> > + for_each_compatible_node(np, "serial", "marvell,mv64360-mpsc") {
> > + err = mv64x60_mpsc_device_setup(np, id++);
> > + if (err) {
> > + printk(KERN_ERR "Failed to initialize MV64x60 " \
> > + "serial device %s: error %d.\n",
> > + np->full_name, err);
> > + of_node_put(np);
>
> This of_node_put call (and the others you added) are not needed.
>
> Thanks,
> -Dale
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
