On Tue, Jun 9, 2026 at 9:47 AM Valery Ushakov <[email protected]> wrote: > > On Tue, Jun 09, 2026 at 01:29:10 +0300, Andrius V wrote: > > > > > > Why the intermediate val, and the cast? Why not just: > > > > > > > > > > switch (pci_conf_read(sc->as_pc, sc->as_tag, > > > > > AGP_NVIDIA_0_APSIZE) & > > > > > 0xff) { > > > > > > > > > > > > > I think I felt that it was a very "bulky" statement for a switch key. > > > > > > I'd agree. A better variable name than "val" would also help. For a > > > drive-by reader it's also not so clear what does 0xff do. Is it an > > > expected part of obtaining the `apsize` value, or does does it select > > > a subfield, or something... In the one-liner - "APSIZE" is hidden in > > > the middle of a long line, closer to the rigtht marging, and further > > > obscured by the all-caps prefix before it and the bitmask after it. > > > `switch (apsize)` OTOH, is easier to read. > > > > Renamed variable to reg and used pcireg_t type to make consistent with > > agp_nvidia_set_aperture() variable name. > > The mask was supposed to be 0x0f to take the lower four bits of the > > aperture size field. Fixed that... > > Thats's a _horrible_ name :) > > Looking at agp_nvidia_set_aperture I guess the right name for this > variable is "key", if only for consistency with the inverse function. > > prcireg_t is what you "physically" read (or write, in the setter), but > the "key" you are extracting from it and looking at here is something > else. > > -uwe
Yeah, you are right! But "key" is likely also bad name :). Will check how to change later today. Regards, Andrius V
