Hi Kieran,
On Wed, Sep 23, 2020 at 11:41 AM Kieran Bingham
<kieran.bingham+renesas@xxxxxxxxxxxxxxxx> wrote:
> On 23/09/2020 08:42, Geert Uytterhoeven wrote:
> > 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.
Personally, I'm not such a big fan of them.
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
