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
>
