Re: port-vax/58261

[David Brownlee <abs%absd.org@localhost>]

On Sun, 9 Jun 2024 at 19:37, matthew green <mrg%eterna23.net@localhost> wrote:
>
> > https://sync.absd.org/vax/vax-sie.diff (also via http:// for those who
>
> thanks for working on this.  looks pretty good to me.

Thanks for the check

> couple of minor comments:
>
> - switch is a keyword, so add/retain the spaces after it before the (.
> - ka46.c change could do with a line wrap to retain 80 cols

Done & done

> - VAX_BTYP_46 check now assumes not 1 is 2, instead of checking for 1,
>   2, and then deciding unknown.  also VAX_BTYP_48.  also VAX_BTYP_420
>   for 0 and 1..
> - if you want to clean up a little more, changing these '1', '2', etc.,

(as mentioned before) I've never seen nor heard anyone report that
output, but it will be easy enough to put back the behaviour (though
I'll make it "Unknown
Mariah model VAX" or similar). Will do that next.

>   magic numbers could be named, "vax_cpudata & 0xff" could become a
>   macro (GET_SIE_MICROCODE_VER(x)?)

Sometimes the ucode rev is in 0xff (which is accessed sometimes as "&
0xff", "& 0377", and my favourite "% 0377" :), and sometimes the
hardware3 rev is in 0xff, and ucode rev in 0xff00. Its common enough
that I think it's worth a macro, with a /* usually, not always */
comment

Actually, looking again I really rather like the approach in
vax/ka780.c:345 - create a bitflag struct for sid

struct ka78x {
        unsigned snr:12,
                 plant:3,
                 eco:8,
                 v785:1,
                 type:8;
};

and then use

        aprint_normal(": KA%s, S/N %d(%d), hardware ECO level %d(%d)\n",
            cpu_getmodel() + 7, ka78->snr, ka78->plant, ka78->eco >>
4, ka78->eco);

I think for the cpu specific files which just print out hardware rev
and similar, that could be a lot cleaner. Need to think more.

Updated diff for most changes at original url, will look at changing
the '46 & '48 back to switch with "unknown". (The '402 is using
GET_SIE_STYP for a similar but unrelated check :),

Thanks

David