Re: [PATCH 6/8] x86/platform/uv: Decode UVsystab Info

[Mike Travis]

On 9/5/2019 7:47 AM, Mike Travis wrote:
> Also, nit:
>
> > --- linux.orig/arch/x86/kernel/apic/x2apic_uv_x.c
> > +++ linux/arch/x86/kernel/apic/x2apic_uv_x.c
> > @@ -1303,7 +1303,8 @@ static int __init decode_uv_systab(void)
> >       struct uv_systab *st;
> >       int i;
> > -    if (uv_hub_info->hub_revision < UV4_HUB_REVISION_BASE)
> > +    /* Select only UV4 (hubbed or hubless) and higher */
> > +    if (is_uv_hubbed(-2) < uv(4) && is_uv_hubless(-2) < uv(4))
> >           return 0;    /* No extended UVsystab required */
> >       st = uv_systab;
> > @@ -1554,8 +1555,19 @@ static __init int uv_system_init_hubless
> >       /* Init kernel/BIOS interface */
> >       rc = uv_bios_init();
> > +    if (rc < 0) {
> > +        pr_err("UV: BIOS init error:%d\n", rc);
>
> Why isn't that function printing an error?
>
>
> > +        return rc;
> > +    }
> > +
> > +    /* Process UVsystab */
> > +    rc = decode_uv_systab();
> > +    if (rc < 0) {
> > +        pr_err("UV: UVsystab decode error:%d\n", rc);
>
> Same here, have the function itself print the error, makes this type of
> stuff much cleaner.

Turns out both functions already print an error message for each 
instance of an error.  The only redundancy is the caller also printing 
an error with just the numeric error code.  Shall I remove that?