Re: [PATCH 1/2] media: i2c: max9286: Use unsigned constants

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

 



Hi Geert,

On 23/09/2020 08:42, Geert Uytterhoeven wrote:
> Hi Kieran,
> 
> On Tue, Sep 22, 2020 at 5:52 PM Kieran Bingham
> <kieran.bingham+renesas@xxxxxxxxxxxxxxxx> wrote:
>> Convert the bitfield definitions to use unsigned integers.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
> 
> Thanks for your patch!
> 
>> --- a/drivers/media/i2c/max9286.c
>> +++ b/drivers/media/i2c/max9286.c
>> @@ -31,85 +31,85 @@
>>  #include <media/v4l2-subdev.h>
>>
>>  /* Register 0x00 */
>> -#define MAX9286_MSTLINKSEL_AUTO                (7 << 5)
>> +#define MAX9286_MSTLINKSEL_AUTO                (7U << 5)
> 
> While using this format for multi-bit fields makes sense...
> 
>>  #define MAX9286_MSTLINKSEL(n)          ((n) << 5)
>>  #define MAX9286_EN_VS_GEN              BIT(4)
>> -#define MAX9286_LINKEN(n)              (1 << (n))
>> +#define MAX9286_LINKEN(n)              (1U << (n))
> 
> ... I think single-bit fields (more below) make better use of the BIT() macro.

Ooops, I missed that, indeed that certainly looks like a BIT.

I was really trying to make sure all the 'bit-field enum' type values
are consistent here, i.e.:

#define MAX9286_I2CSLVSH_1046NS_469NS	(3U << 5)
#define MAX9286_I2CSLVSH_938NS_352NS	(2U << 5)
#define MAX9286_I2CSLVSH_469NS_234NS	(1U << 5)
#define MAX9286_I2CSLVSH_352NS_117NS	(0U << 5)

I'll sift out the single bit fields that are more appropriate for BIT().

There is also the FIELD_PREP, FIELD_GET macros that could be used
instead from include/linux/bitfield.h which are new to me, and seem
interesting but I haven't worked out if it's worth converting the whole
driver to use that yet or not.

--
Kieran


> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux