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

[Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>]

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.