qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 3/6] scsi-disk: add MODE_PAGE_APPLE quirk for Macintosh


From: Mark Cave-Ayland
Subject: Re: [PATCH 3/6] scsi-disk: add MODE_PAGE_APPLE quirk for Macintosh
Date: Sun, 24 Apr 2022 15:50:41 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0

On 21/04/2022 23:00, BALATON Zoltan wrote:

On Thu, 21 Apr 2022, Richard Henderson wrote:
On 4/21/22 08:29, Mark Cave-Ayland wrote:
You need (1 << SCSI_DISK_QUIRK_MODE_PAGE_APPLE) instead.

Doh, you're absolutely right. I believe the current recommendation is to use the BIT() macro in these cases.

I think it's not a recommendation (as in code style) but it often makes things simpler by reducing the number of parenthesis so using it is probably a good idea for readability. But if you never need the bit number only the value then you could define the quirks constants as that in the first place. (Otherwise if you want bit numbers maybe make it an enum.)

We probably need to fix BIT() to use 1ULL.

At present it's using 1UL, to match the other (unfortunate) uses of unsigned long within bitops.h.  The use of BIT() for things unrelated to bitops.h just bit a recent risc-v pull request, in that it failed to build on all 32-bit hosts.

There's already a BIT_ULL(nr) when ULL is needed but in this case quirks was declared uint32_t so probably OK with UL as well. (Was this bitops.h taken from Linux? Keeping it compatible then may be a good idea to avoid confusion.)

It seems there is still a bit of discussion around using BIT() here, so for v2 I'll add the shift directly with (1 << x). Then if the BIT() macro becomes suitable for more general use it can easily be updated as a separate patch later.


ATB,

Mark.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]