* [PATCH 0/2] gmsl: Use unsigned constants
@ 2020-09-22 15:52 Kieran Bingham
2020-09-22 15:52 ` [PATCH 1/2] media: i2c: max9286: " Kieran Bingham
2020-09-22 15:52 ` [PATCH 2/2] media: i2c: max9271: " Kieran Bingham
0 siblings, 2 replies; 6+ messages in thread
From: Kieran Bingham @ 2020-09-22 15:52 UTC (permalink / raw)
To: sakari.ailus, linux-media, linux-renesas-soc; +Cc: Kieran Bingham
During pre-integration reviews of the GMSL (MAX9286/MAX9271) patches,
one comment was to mark the constants used in bitfield shifts as
explicitly unsigned. This ensures that any use of a register write that
updates bit 31 is operated correctly.
Whilst none of these register shifts affect bit 31 in any of the
registers, update the values to follow best practices and ensure
consistency if further additions are made.
Please note, this does not remove or clean up the 'Prefer BIT' check
patch warnings, as the use of the BIT macro is still not applicable to
these entries.
Kieran Bingham (2):
media: i2c: max9286: Use unsigned constants
media: i2c: max9271: Use unsigned constants
drivers/media/i2c/max9271.h | 60 +++++++++++------------
drivers/media/i2c/max9286.c | 96 ++++++++++++++++++-------------------
2 files changed, 78 insertions(+), 78 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] media: i2c: max9286: Use unsigned constants
2020-09-22 15:52 [PATCH 0/2] gmsl: Use unsigned constants Kieran Bingham
@ 2020-09-22 15:52 ` Kieran Bingham
2020-09-23 7:42 ` Geert Uytterhoeven
2020-09-22 15:52 ` [PATCH 2/2] media: i2c: max9271: " Kieran Bingham
1 sibling, 1 reply; 6+ messages in thread
From: Kieran Bingham @ 2020-09-22 15:52 UTC (permalink / raw)
To: sakari.ailus, linux-media, linux-renesas-soc; +Cc: Kieran Bingham
Convert the bitfield definitions to use unsigned integers.
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
drivers/media/i2c/max9286.c | 96 ++++++++++++++++++-------------------
1 file changed, 48 insertions(+), 48 deletions(-)
diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index 7c8212071df9..079d5d7cfc2d 100644
--- 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)
#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))
/* Register 0x01 */
-#define MAX9286_FSYNCMODE_ECU (3 << 6)
-#define MAX9286_FSYNCMODE_EXT (2 << 6)
-#define MAX9286_FSYNCMODE_INT_OUT (1 << 6)
-#define MAX9286_FSYNCMODE_INT_HIZ (0 << 6)
+#define MAX9286_FSYNCMODE_ECU (3U << 6)
+#define MAX9286_FSYNCMODE_EXT (2U << 6)
+#define MAX9286_FSYNCMODE_INT_OUT (1U << 6)
+#define MAX9286_FSYNCMODE_INT_HIZ (0U << 6)
#define MAX9286_GPIEN BIT(5)
#define MAX9286_ENLMO_RSTFSYNC BIT(2)
-#define MAX9286_FSYNCMETH_AUTO (2 << 0)
-#define MAX9286_FSYNCMETH_SEMI_AUTO (1 << 0)
-#define MAX9286_FSYNCMETH_MANUAL (0 << 0)
+#define MAX9286_FSYNCMETH_AUTO (2U << 0)
+#define MAX9286_FSYNCMETH_SEMI_AUTO (1U << 0)
+#define MAX9286_FSYNCMETH_MANUAL (0U << 0)
#define MAX9286_REG_FSYNC_PERIOD_L 0x06
#define MAX9286_REG_FSYNC_PERIOD_M 0x07
#define MAX9286_REG_FSYNC_PERIOD_H 0x08
/* Register 0x0a */
-#define MAX9286_FWDCCEN(n) (1 << ((n) + 4))
-#define MAX9286_REVCCEN(n) (1 << (n))
+#define MAX9286_FWDCCEN(n) (1U << ((n) + 4))
+#define MAX9286_REVCCEN(n) (1U << (n))
/* Register 0x0c */
#define MAX9286_HVEN BIT(7)
-#define MAX9286_EDC_6BIT_HAMMING (2 << 5)
-#define MAX9286_EDC_6BIT_CRC (1 << 5)
-#define MAX9286_EDC_1BIT_PARITY (0 << 5)
+#define MAX9286_EDC_6BIT_HAMMING (2U << 5)
+#define MAX9286_EDC_6BIT_CRC (1U << 5)
+#define MAX9286_EDC_1BIT_PARITY (0U << 5)
#define MAX9286_DESEL BIT(4)
#define MAX9286_INVVS BIT(3)
#define MAX9286_INVHS BIT(2)
-#define MAX9286_HVSRC_D0 (2 << 0)
-#define MAX9286_HVSRC_D14 (1 << 0)
-#define MAX9286_HVSRC_D18 (0 << 0)
+#define MAX9286_HVSRC_D0 (2U << 0)
+#define MAX9286_HVSRC_D14 (1U << 0)
+#define MAX9286_HVSRC_D18 (0U << 0)
/* Register 0x0f */
#define MAX9286_0X0F_RESERVED BIT(3)
/* Register 0x12 */
#define MAX9286_CSILANECNT(n) (((n) - 1) << 6)
#define MAX9286_CSIDBL BIT(5)
#define MAX9286_DBL BIT(4)
-#define MAX9286_DATATYPE_USER_8BIT (11 << 0)
-#define MAX9286_DATATYPE_USER_YUV_12BIT (10 << 0)
-#define MAX9286_DATATYPE_USER_24BIT (9 << 0)
-#define MAX9286_DATATYPE_RAW14 (8 << 0)
-#define MAX9286_DATATYPE_RAW11 (7 << 0)
-#define MAX9286_DATATYPE_RAW10 (6 << 0)
-#define MAX9286_DATATYPE_RAW8 (5 << 0)
-#define MAX9286_DATATYPE_YUV422_10BIT (4 << 0)
-#define MAX9286_DATATYPE_YUV422_8BIT (3 << 0)
-#define MAX9286_DATATYPE_RGB555 (2 << 0)
-#define MAX9286_DATATYPE_RGB565 (1 << 0)
-#define MAX9286_DATATYPE_RGB888 (0 << 0)
+#define MAX9286_DATATYPE_USER_8BIT (11U << 0)
+#define MAX9286_DATATYPE_USER_YUV_12BIT (10U << 0)
+#define MAX9286_DATATYPE_USER_24BIT (9U << 0)
+#define MAX9286_DATATYPE_RAW14 (8U << 0)
+#define MAX9286_DATATYPE_RAW11 (7U << 0)
+#define MAX9286_DATATYPE_RAW10 (6U << 0)
+#define MAX9286_DATATYPE_RAW8 (5U << 0)
+#define MAX9286_DATATYPE_YUV422_10BIT (4U << 0)
+#define MAX9286_DATATYPE_YUV422_8BIT (3U << 0)
+#define MAX9286_DATATYPE_RGB555 (2U << 0)
+#define MAX9286_DATATYPE_RGB565 (1U << 0)
+#define MAX9286_DATATYPE_RGB888 (0U << 0)
/* Register 0x15 */
#define MAX9286_VC(n) ((n) << 5)
#define MAX9286_VCTYPE BIT(4)
#define MAX9286_CSIOUTEN BIT(3)
-#define MAX9286_0X15_RESV (3 << 0)
+#define MAX9286_0X15_RESV (3U << 0)
/* Register 0x1b */
-#define MAX9286_SWITCHIN(n) (1 << ((n) + 4))
-#define MAX9286_ENEQ(n) (1 << (n))
+#define MAX9286_SWITCHIN(n) (1U << ((n) + 4))
+#define MAX9286_ENEQ(n) (1U << (n))
/* Register 0x27 */
#define MAX9286_LOCKED BIT(7)
/* Register 0x31 */
#define MAX9286_FSYNC_LOCKED BIT(6)
/* Register 0x34 */
#define MAX9286_I2CLOCACK BIT(7)
-#define MAX9286_I2CSLVSH_1046NS_469NS (3 << 5)
-#define MAX9286_I2CSLVSH_938NS_352NS (2 << 5)
-#define MAX9286_I2CSLVSH_469NS_234NS (1 << 5)
-#define MAX9286_I2CSLVSH_352NS_117NS (0 << 5)
-#define MAX9286_I2CMSTBT_837KBPS (7 << 2)
-#define MAX9286_I2CMSTBT_533KBPS (6 << 2)
-#define MAX9286_I2CMSTBT_339KBPS (5 << 2)
-#define MAX9286_I2CMSTBT_173KBPS (4 << 2)
-#define MAX9286_I2CMSTBT_105KBPS (3 << 2)
-#define MAX9286_I2CMSTBT_84KBPS (2 << 2)
-#define MAX9286_I2CMSTBT_28KBPS (1 << 2)
-#define MAX9286_I2CMSTBT_8KBPS (0 << 2)
-#define MAX9286_I2CSLVTO_NONE (3 << 0)
-#define MAX9286_I2CSLVTO_1024US (2 << 0)
-#define MAX9286_I2CSLVTO_256US (1 << 0)
-#define MAX9286_I2CSLVTO_64US (0 << 0)
+#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)
+#define MAX9286_I2CMSTBT_837KBPS (7U << 2)
+#define MAX9286_I2CMSTBT_533KBPS (6U << 2)
+#define MAX9286_I2CMSTBT_339KBPS (5U << 2)
+#define MAX9286_I2CMSTBT_173KBPS (4U << 2)
+#define MAX9286_I2CMSTBT_105KBPS (3U << 2)
+#define MAX9286_I2CMSTBT_84KBPS (2U << 2)
+#define MAX9286_I2CMSTBT_28KBPS (1U << 2)
+#define MAX9286_I2CMSTBT_8KBPS (0U << 2)
+#define MAX9286_I2CSLVTO_NONE (3U << 0)
+#define MAX9286_I2CSLVTO_1024US (2U << 0)
+#define MAX9286_I2CSLVTO_256US (1U << 0)
+#define MAX9286_I2CSLVTO_64US (0U << 0)
/* Register 0x3b */
#define MAX9286_REV_TRF(n) ((n) << 4)
#define MAX9286_REV_AMP(n) ((((n) - 30) / 10) << 1) /* in mV */
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] media: i2c: max9271: Use unsigned constants
2020-09-22 15:52 [PATCH 0/2] gmsl: Use unsigned constants Kieran Bingham
2020-09-22 15:52 ` [PATCH 1/2] media: i2c: max9286: " Kieran Bingham
@ 2020-09-22 15:52 ` Kieran Bingham
1 sibling, 0 replies; 6+ messages in thread
From: Kieran Bingham @ 2020-09-22 15:52 UTC (permalink / raw)
To: sakari.ailus, linux-media, linux-renesas-soc; +Cc: Kieran Bingham
Convert the bitfield definitions to use unsigned integers.
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
drivers/media/i2c/max9271.h | 60 ++++++++++++++++++-------------------
1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/drivers/media/i2c/max9271.h b/drivers/media/i2c/max9271.h
index d78fb21441e9..4ef36a90c746 100644
--- a/drivers/media/i2c/max9271.h
+++ b/drivers/media/i2c/max9271.h
@@ -13,24 +13,24 @@
#define MAX9271_DEFAULT_ADDR 0x40
/* Register 0x02 */
-#define MAX9271_SPREAD_SPECT_0 (0 << 5)
-#define MAX9271_SPREAD_SPECT_05 (1 << 5)
-#define MAX9271_SPREAD_SPECT_15 (2 << 5)
-#define MAX9271_SPREAD_SPECT_1 (5 << 5)
-#define MAX9271_SPREAD_SPECT_2 (3 << 5)
-#define MAX9271_SPREAD_SPECT_3 (6 << 5)
-#define MAX9271_SPREAD_SPECT_4 (7 << 5)
+#define MAX9271_SPREAD_SPECT_0 (0U << 5)
+#define MAX9271_SPREAD_SPECT_05 (1U << 5)
+#define MAX9271_SPREAD_SPECT_15 (2U << 5)
+#define MAX9271_SPREAD_SPECT_1 (5U << 5)
+#define MAX9271_SPREAD_SPECT_2 (3U << 5)
+#define MAX9271_SPREAD_SPECT_3 (6U << 5)
+#define MAX9271_SPREAD_SPECT_4 (7U << 5)
#define MAX9271_R02_RES BIT(4)
-#define MAX9271_PCLK_AUTODETECT (3 << 2)
+#define MAX9271_PCLK_AUTODETECT (3U << 2)
#define MAX9271_SERIAL_AUTODETECT (0x03)
/* Register 0x04 */
#define MAX9271_SEREN BIT(7)
#define MAX9271_CLINKEN BIT(6)
#define MAX9271_PRBSEN BIT(5)
#define MAX9271_SLEEP BIT(4)
-#define MAX9271_INTTYPE_I2C (0 << 2)
-#define MAX9271_INTTYPE_UART (1 << 2)
-#define MAX9271_INTTYPE_NONE (2 << 2)
+#define MAX9271_INTTYPE_I2C (0U << 2)
+#define MAX9271_INTTYPE_UART (1U << 2)
+#define MAX9271_INTTYPE_NONE (2U << 2)
#define MAX9271_REVCCEN BIT(1)
#define MAX9271_FWDCCEN BIT(0)
/* Register 0x07 */
@@ -39,9 +39,9 @@
#define MAX9271_BWS BIT(5)
#define MAX9271_ES BIT(4)
#define MAX9271_HVEN BIT(2)
-#define MAX9271_EDC_1BIT_PARITY (0 << 0)
-#define MAX9271_EDC_6BIT_CRC (1 << 0)
-#define MAX9271_EDC_6BIT_HAMMING (2 << 0)
+#define MAX9271_EDC_1BIT_PARITY (0U << 0)
+#define MAX9271_EDC_6BIT_CRC (1U << 0)
+#define MAX9271_EDC_6BIT_HAMMING (2U << 0)
/* Register 0x08 */
#define MAX9271_INVVS BIT(7)
#define MAX9271_INVHS BIT(6)
@@ -51,22 +51,22 @@
#define MAX9271_ID 0x09
/* Register 0x0d */
#define MAX9271_I2CLOCACK BIT(7)
-#define MAX9271_I2CSLVSH_1046NS_469NS (3 << 5)
-#define MAX9271_I2CSLVSH_938NS_352NS (2 << 5)
-#define MAX9271_I2CSLVSH_469NS_234NS (1 << 5)
-#define MAX9271_I2CSLVSH_352NS_117NS (0 << 5)
-#define MAX9271_I2CMSTBT_837KBPS (7 << 2)
-#define MAX9271_I2CMSTBT_533KBPS (6 << 2)
-#define MAX9271_I2CMSTBT_339KBPS (5 << 2)
-#define MAX9271_I2CMSTBT_173KBPS (4 << 2)
-#define MAX9271_I2CMSTBT_105KBPS (3 << 2)
-#define MAX9271_I2CMSTBT_84KBPS (2 << 2)
-#define MAX9271_I2CMSTBT_28KBPS (1 << 2)
-#define MAX9271_I2CMSTBT_8KBPS (0 << 2)
-#define MAX9271_I2CSLVTO_NONE (3 << 0)
-#define MAX9271_I2CSLVTO_1024US (2 << 0)
-#define MAX9271_I2CSLVTO_256US (1 << 0)
-#define MAX9271_I2CSLVTO_64US (0 << 0)
+#define MAX9271_I2CSLVSH_1046NS_469NS (3U << 5)
+#define MAX9271_I2CSLVSH_938NS_352NS (2U << 5)
+#define MAX9271_I2CSLVSH_469NS_234NS (1U << 5)
+#define MAX9271_I2CSLVSH_352NS_117NS (0U << 5)
+#define MAX9271_I2CMSTBT_837KBPS (7U << 2)
+#define MAX9271_I2CMSTBT_533KBPS (6U << 2)
+#define MAX9271_I2CMSTBT_339KBPS (5U << 2)
+#define MAX9271_I2CMSTBT_173KBPS (4U << 2)
+#define MAX9271_I2CMSTBT_105KBPS (3U << 2)
+#define MAX9271_I2CMSTBT_84KBPS (2U << 2)
+#define MAX9271_I2CMSTBT_28KBPS (1U << 2)
+#define MAX9271_I2CMSTBT_8KBPS (0U << 2)
+#define MAX9271_I2CSLVTO_NONE (3U << 0)
+#define MAX9271_I2CSLVTO_1024US (2U << 0)
+#define MAX9271_I2CSLVTO_256US (1U << 0)
+#define MAX9271_I2CSLVTO_64US (0U << 0)
/* Register 0x0f */
#define MAX9271_GPIO5OUT BIT(5)
#define MAX9271_GPIO4OUT BIT(4)
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] media: i2c: max9286: Use unsigned constants
2020-09-22 15:52 ` [PATCH 1/2] media: i2c: max9286: " Kieran Bingham
@ 2020-09-23 7:42 ` Geert Uytterhoeven
2020-09-23 9:41 ` Kieran Bingham
0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2020-09-23 7:42 UTC (permalink / raw)
To: Kieran Bingham; +Cc: Sakari Ailus, Linux Media Mailing List, Linux-Renesas
Hi Kieran,
On Tue, Sep 22, 2020 at 5:52 PM Kieran Bingham
<kieran.bingham+renesas@ideasonboard.com> wrote:
> Convert the bitfield definitions to use unsigned integers.
>
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
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.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] media: i2c: max9286: Use unsigned constants
2020-09-23 7:42 ` Geert Uytterhoeven
@ 2020-09-23 9:41 ` Kieran Bingham
2020-09-23 11:51 ` Geert Uytterhoeven
0 siblings, 1 reply; 6+ messages in thread
From: Kieran Bingham @ 2020-09-23 9:41 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Sakari Ailus, Linux Media Mailing List, Linux-Renesas
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@ideasonboard.com> wrote:
>> Convert the bitfield definitions to use unsigned integers.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>
> 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@linux-m68k.org
>
> 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] media: i2c: max9286: Use unsigned constants
2020-09-23 9:41 ` Kieran Bingham
@ 2020-09-23 11:51 ` Geert Uytterhoeven
0 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2020-09-23 11:51 UTC (permalink / raw)
To: Kieran Bingham; +Cc: Sakari Ailus, Linux Media Mailing List, Linux-Renesas
Hi Kieran,
On Wed, Sep 23, 2020 at 11:41 AM Kieran Bingham
<kieran.bingham+renesas@ideasonboard.com> wrote:
> On 23/09/2020 08:42, Geert Uytterhoeven wrote:
> > On Tue, Sep 22, 2020 at 5:52 PM Kieran Bingham
> > <kieran.bingham+renesas@ideasonboard.com> wrote:
> >> Convert the bitfield definitions to use unsigned integers.
> >>
> >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >
> > 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@linux-m68k.org
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-23 11:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-22 15:52 [PATCH 0/2] gmsl: Use unsigned constants Kieran Bingham
2020-09-22 15:52 ` [PATCH 1/2] media: i2c: max9286: " Kieran Bingham
2020-09-23 7:42 ` Geert Uytterhoeven
2020-09-23 9:41 ` Kieran Bingham
2020-09-23 11:51 ` Geert Uytterhoeven
2020-09-22 15:52 ` [PATCH 2/2] media: i2c: max9271: " Kieran Bingham
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).