| Message ID | 20230821223001.28480-13-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: i2c: imx219: Miscellaneous cleanups and improvements | expand |
On Mon, 21 Aug 2023 at 23:30, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > The IMX219_VTS_* macros define default VTS values for the modes > supported by the driver. They are used in a single place, and hinder > readability compared to using the value directly as a decimal number. > Drop them. Personally I don't see it as a huge hindrance to readability, but it's not that significant either way. > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > --- > drivers/media/i2c/imx219.c | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c > index 67a30dc39641..165c5e8473f7 100644 > --- a/drivers/media/i2c/imx219.c > +++ b/drivers/media/i2c/imx219.c > @@ -71,10 +71,6 @@ > > /* V_TIMING internal */ > #define IMX219_REG_VTS CCI_REG16(0x0160) > -#define IMX219_VTS_15FPS 0x0dc6 > -#define IMX219_VTS_30FPS_1080P 0x06e3 > -#define IMX219_VTS_30FPS_BINNED 0x06e3 > -#define IMX219_VTS_30FPS_640x480 0x06e3 > #define IMX219_VTS_MAX 0xffff > > #define IMX219_VBLANK_MIN 4 > @@ -302,7 +298,7 @@ static const struct imx219_mode supported_modes[] = { > .width = 3280, > .height = 2464 > }, > - .vts_def = IMX219_VTS_15FPS, > + .vts_def = 3526, > }, > { > /* 1080P 30fps cropped */ > @@ -314,7 +310,7 @@ static const struct imx219_mode supported_modes[] = { > .width = 1920, > .height = 1080 > }, > - .vts_def = IMX219_VTS_30FPS_1080P, > + .vts_def = 1763, > }, > { > /* 2x2 binned 30fps mode */ > @@ -326,7 +322,7 @@ static const struct imx219_mode supported_modes[] = { > .width = 3280, > .height = 2464 > }, > - .vts_def = IMX219_VTS_30FPS_BINNED, > + .vts_def = 1763, > }, > { > /* 640x480 30fps mode */ > @@ -338,7 +334,7 @@ static const struct imx219_mode supported_modes[] = { > .width = 1280, > .height = 960 > }, > - .vts_def = IMX219_VTS_30FPS_640x480, > + .vts_def = 1763, > }, > }; > > -- > Regards, > > Laurent Pinchart >
Hi Laurent On Tue, Aug 22, 2023 at 06:27:06PM +0100, Dave Stevenson wrote: > On Mon, 21 Aug 2023 at 23:30, Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: > > > > The IMX219_VTS_* macros define default VTS values for the modes > > supported by the driver. They are used in a single place, and hinder > > readability compared to using the value directly as a decimal number. > > Drop them. > > Personally I don't see it as a huge hindrance to readability, but it's > not that significant either way. > Agreed, a matter of tastes mostly > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > --- > > drivers/media/i2c/imx219.c | 12 ++++-------- > > 1 file changed, 4 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c > > index 67a30dc39641..165c5e8473f7 100644 > > --- a/drivers/media/i2c/imx219.c > > +++ b/drivers/media/i2c/imx219.c > > @@ -71,10 +71,6 @@ > > > > /* V_TIMING internal */ > > #define IMX219_REG_VTS CCI_REG16(0x0160) > > -#define IMX219_VTS_15FPS 0x0dc6 > > -#define IMX219_VTS_30FPS_1080P 0x06e3 > > -#define IMX219_VTS_30FPS_BINNED 0x06e3 > > -#define IMX219_VTS_30FPS_640x480 0x06e3 > > #define IMX219_VTS_MAX 0xffff > > > > #define IMX219_VBLANK_MIN 4 > > @@ -302,7 +298,7 @@ static const struct imx219_mode supported_modes[] = { > > .width = 3280, > > .height = 2464 > > }, > > - .vts_def = IMX219_VTS_15FPS, > > + .vts_def = 3526, > > }, > > { > > /* 1080P 30fps cropped */ > > @@ -314,7 +310,7 @@ static const struct imx219_mode supported_modes[] = { > > .width = 1920, > > .height = 1080 > > }, > > - .vts_def = IMX219_VTS_30FPS_1080P, > > + .vts_def = 1763, > > }, > > { > > /* 2x2 binned 30fps mode */ > > @@ -326,7 +322,7 @@ static const struct imx219_mode supported_modes[] = { > > .width = 3280, > > .height = 2464 > > }, > > - .vts_def = IMX219_VTS_30FPS_BINNED, > > + .vts_def = 1763, > > }, > > { > > /* 640x480 30fps mode */ > > @@ -338,7 +334,7 @@ static const struct imx219_mode supported_modes[] = { > > .width = 1280, > > .height = 960 > > }, > > - .vts_def = IMX219_VTS_30FPS_640x480, > > + .vts_def = 1763, > > }, > > }; > > > > -- > > Regards, > > > > Laurent Pinchart > >
diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c index 67a30dc39641..165c5e8473f7 100644 --- a/drivers/media/i2c/imx219.c +++ b/drivers/media/i2c/imx219.c @@ -71,10 +71,6 @@ /* V_TIMING internal */ #define IMX219_REG_VTS CCI_REG16(0x0160) -#define IMX219_VTS_15FPS 0x0dc6 -#define IMX219_VTS_30FPS_1080P 0x06e3 -#define IMX219_VTS_30FPS_BINNED 0x06e3 -#define IMX219_VTS_30FPS_640x480 0x06e3 #define IMX219_VTS_MAX 0xffff #define IMX219_VBLANK_MIN 4 @@ -302,7 +298,7 @@ static const struct imx219_mode supported_modes[] = { .width = 3280, .height = 2464 }, - .vts_def = IMX219_VTS_15FPS, + .vts_def = 3526, }, { /* 1080P 30fps cropped */ @@ -314,7 +310,7 @@ static const struct imx219_mode supported_modes[] = { .width = 1920, .height = 1080 }, - .vts_def = IMX219_VTS_30FPS_1080P, + .vts_def = 1763, }, { /* 2x2 binned 30fps mode */ @@ -326,7 +322,7 @@ static const struct imx219_mode supported_modes[] = { .width = 3280, .height = 2464 }, - .vts_def = IMX219_VTS_30FPS_BINNED, + .vts_def = 1763, }, { /* 640x480 30fps mode */ @@ -338,7 +334,7 @@ static const struct imx219_mode supported_modes[] = { .width = 1280, .height = 960 }, - .vts_def = IMX219_VTS_30FPS_640x480, + .vts_def = 1763, }, };
The IMX219_VTS_* macros define default VTS values for the modes supported by the driver. They are used in a single place, and hinder readability compared to using the value directly as a decimal number. Drop them. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/media/i2c/imx219.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)