Date: Wed, 18 May 2016 21:32:17 +0200 From: Zbigniew Bodek <zbb@semihalf.com> To: Andrew Turner <andrew@fubar.geek.nz> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r300149 - in head/sys: arm/allwinner arm/allwinner/a10 arm/arm arm/broadcom/bcm2835 arm/mv arm/nvidia arm/ti arm/ti/omap4 arm64/arm64 kern mips/mediatek mips/mips sys Message-ID: <CAG7dG%2BygwdVqJGDjnVxnZSmp=nX==swVWwYxdNozrFSgc_xyWw@mail.gmail.com> In-Reply-To: <20160518162409.1074344e@zapp> References: <201605181505.u4IF5ixZ018827@repo.freebsd.org> <CAG7dG%2BxBi8BxoDMr_OnDeRFXWzfro%2BVqOQXWKhPtzMfuePx09g@mail.gmail.com> <20160518162409.1074344e@zapp>
next in thread | previous in thread | raw e-mail | index | archive | help
Hello Andrew,
Thanks for your reply. Please see my comments in-line.
Kind regards
zbb
2016-05-18 17:24 GMT+02:00 Andrew Turner <andrew@fubar.geek.nz>:
> On Wed, 18 May 2016 17:15:10 +0200
> Zbigniew Bodek <zbb@semihalf.com> wrote:
>
> > 2016-05-18 17:05 GMT+02:00 Andrew Turner <andrew@freebsd.org>:
> ...
> > > #ifdef INTRNG
> > > xref = OF_xref_from_node(ofw_bus_get_node(dev));
> > > - if (intr_pic_register(dev, xref) != 0) {
> > > + if (intr_pic_register(dev, xref) == NULL) {
> > > device_printf(dev, "could not register PIC\n");
> > > goto error;
> > > }
> > > @@ -172,7 +172,7 @@ error:
> > > /* Failure so free resources */
> > > gic_v3_detach(dev);
> > >
> > > - return (err);
> > > + return (ENXIO);
> > >
> >
> >
> > Few line above we have:
> > err = gic_v3_attach(dev);
> > if (err != 0)
> > goto error;
> >
> > So now we would not return error code from gic_v3_attach() but always
> > ENXIO...
>
> The error value doesn't matter, as long as it's non-zero. This also
> fixes the return value in the intrng case if either of
> intr_pic_register or intr_pic_claim_root fail. You might get a little
> information from the return value being printed, however it would be
> more useful to have verbose logging to print an error message.
>
>
So if you wanted to fix INTRNG part why didn't you just set an error value
in case of intr_pic_register() failure before "goto error"?
Simply:
+ if (intr_pic_register(dev, xref) == NULL) {
device_printf(dev, "could not register PIC\n");
+ err = ENXIO;
goto error;
}
Because now we set an error value and in case of real error (not when
gic_v3_attach() returns 0) we print ENXIO while doing nothing with the err
variable...
> In all of these failure cases there isn't much we can do as there is no
> root interrupt controller, the boot will fail later on when we enable
> interrupts.
>
I know but that is not the explanation for breaking the logic here.
>
> Andrew
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG7dG%2BygwdVqJGDjnVxnZSmp=nX==swVWwYxdNozrFSgc_xyWw>