On Mon, Jun 8, 2026 at 12:23 PM Valery Ushakov <[email protected]> wrote: > > On Mon, Jun 08, 2026 at 11:24:17 +0300, Andrius V wrote: > > > > > + int val; > > > > + > > > > + val = (int) pci_conf_read(sc->as_pc, sc->as_tag, > > > > AGP_NVIDIA_0_APSIZE) & 0xff; > > > > + switch (val) { > > > > > > 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. > > -uwe
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... Thanks!
