Re: port-vax/58261

[Johnny Billquist <bqt%softjar.se@localhost>]

The following reply was made to PR port-vax/58261; it has been noted by GNATS.

From: Johnny Billquist <bqt%softjar.se@localhost>
To: David Brownlee <abs%absd.org@localhost>, matthew green <mrg%eterna23.net@localhost>
Cc: port-vax-maintainer%netbsd.org@localhost, gnats-admin%netbsd.org@localhost,
 netbsd-bugs%netbsd.org@localhost, ncommander@restless.systems, gnats-bugs%netbsd.org@localhost,
 port-vax List <port-vax%netbsd.org@localhost>
Subject: Re: port-vax/58261
Date: Mon, 10 Jun 2024 00:29:32 +0200

 On 2024-06-10 00:18, David Brownlee wrote:
 > On Sun, 9 Jun 2024 at 19:37, matthew green <mrg%eterna23.net@localhost> wrote:
 >>
 >>    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
 
 If anyone did "% 0377" that would be a bug. It's a different thing. 
 However, if they did "% 0400" it would be fine, and I would suspect the 
 compiler would reduce that to an AND (or actually a BICL in VAX 
 assembly) in the end anyway.
 
 > 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 do agree that using a bitfield is a very reasonable approach for this, 
 and I did the same for the ka86. But looking at the quoted lines above, 
 I really think eco should be split in two parts instead of the shifting. 
 That seems just silly, if that really is two different numbers to the eco.
 Also, cpu_getmodel() returns a string, and then 7 is added to that 
 pointer. That seems rather horrible coding, and I think that should also 
 be cleaned up a bit. But that's not really a part of your work, so I 
 wouldn't worry about that here.
 
 However, again - I think changing to use bitfields for these kind of 
 things make the code much cleaner.
 
    Johnny
 
 -- 
 Johnny Billquist                  || "I'm on a bus
                                    ||  on a psychedelic trip
 email: bqt%softjar.se@localhost             ||  Reading murder books
 pdp is alive!                     ||  tryin' to stay hip" - B. Idol