| Message ID | 20220101182806.19311-9-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | cdcb186e364452cbe309db5a94ff5a510be234e2 |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | media: i2c: max9286: Small new features | expand |
On Sat, Jan 01, 2022 at 08:28:03PM +0200, Laurent Pinchart wrote: > Macros are easier to read than numerical values. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > --- > drivers/media/i2c/max9286.c | 27 ++++++++++++++++++--------- > 1 file changed, 18 insertions(+), 9 deletions(-) > > diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c > index 24c2bf4fda53..4b69bd036ca6 100644 > --- a/drivers/media/i2c/max9286.c > +++ b/drivers/media/i2c/max9286.c > @@ -80,10 +80,13 @@ > #define MAX9286_DATATYPE_RGB565 (1 << 0) > #define MAX9286_DATATYPE_RGB888 (0 << 0) > /* Register 0x15 */ > +#define MAX9286_CSI_IMAGE_TYP BIT(7) > #define MAX9286_VC(n) ((n) << 5) > #define MAX9286_VCTYPE BIT(4) > #define MAX9286_CSIOUTEN BIT(3) > -#define MAX9286_0X15_RESV (3 << 0) > +#define MAX9286_SWP_ENDIAN BIT(2) > +#define MAX9286_EN_CCBSYB_CLK_STR BIT(1) > +#define MAX9286_EN_GPI_CCBSYB BIT(0) > /* Register 0x1b */ > #define MAX9286_SWITCHIN(n) (1 << ((n) + 4)) > #define MAX9286_ENEQ(n) (1 << (n)) > @@ -525,10 +528,12 @@ static void max9286_set_video_format(struct max9286_priv *priv, > return; > > /* > - * Video format setup: > - * Disable CSI output, VC is set according to Link number. > + * Video format setup: disable CSI output, set VC according to Link > + * number, enable I2C clock stretching when CCBSY is low, enable CCBSY > + * in external GPI-to-GPO mode. > */ > - max9286_write(priv, 0x15, MAX9286_VCTYPE | MAX9286_0X15_RESV); > + max9286_write(priv, 0x15, MAX9286_VCTYPE | MAX9286_EN_CCBSYB_CLK_STR | > + MAX9286_EN_GPI_CCBSYB); > > /* Enable CSI-2 Lane D0-D3 only, DBL mode. */ > max9286_write(priv, 0x12, MAX9286_CSIDBL | MAX9286_DBL | > @@ -810,13 +815,17 @@ static int max9286_s_stream(struct v4l2_subdev *sd, int enable) > } > > /* > - * Enable CSI output, VC set according to link number. > - * Bit 7 must be set (chip manual says it's 0 and reserved). > + * Configure the CSI-2 output to line interleaved mode (W x (N > + * x H), as opposed to the (N x W) x H mode that outputs the > + * images stitched side-by-side) and enable it. > */ > - max9286_write(priv, 0x15, 0x80 | MAX9286_VCTYPE | > - MAX9286_CSIOUTEN | MAX9286_0X15_RESV); > + max9286_write(priv, 0x15, MAX9286_CSI_IMAGE_TYP | MAX9286_VCTYPE | > + MAX9286_CSIOUTEN | MAX9286_EN_CCBSYB_CLK_STR | > + MAX9286_EN_GPI_CCBSYB); > } else { > - max9286_write(priv, 0x15, MAX9286_VCTYPE | MAX9286_0X15_RESV); > + max9286_write(priv, 0x15, MAX9286_VCTYPE | > + MAX9286_EN_CCBSYB_CLK_STR | > + MAX9286_EN_GPI_CCBSYB); Probably fits better on two lines only. Trusting the bits definitions: Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Thanks j > > /* Stop all cameras. */ > for_each_source(priv, source) > -- > Regards, > > Laurent Pinchart >
Hi Jacopo, On Sun, Jan 09, 2022 at 11:37:38AM +0100, Jacopo Mondi wrote: > On Sat, Jan 01, 2022 at 08:28:03PM +0200, Laurent Pinchart wrote: > > Macros are easier to read than numerical values. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > --- > > drivers/media/i2c/max9286.c | 27 ++++++++++++++++++--------- > > 1 file changed, 18 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c > > index 24c2bf4fda53..4b69bd036ca6 100644 > > --- a/drivers/media/i2c/max9286.c > > +++ b/drivers/media/i2c/max9286.c > > @@ -80,10 +80,13 @@ > > #define MAX9286_DATATYPE_RGB565 (1 << 0) > > #define MAX9286_DATATYPE_RGB888 (0 << 0) > > /* Register 0x15 */ > > +#define MAX9286_CSI_IMAGE_TYP BIT(7) > > #define MAX9286_VC(n) ((n) << 5) > > #define MAX9286_VCTYPE BIT(4) > > #define MAX9286_CSIOUTEN BIT(3) > > -#define MAX9286_0X15_RESV (3 << 0) > > +#define MAX9286_SWP_ENDIAN BIT(2) > > +#define MAX9286_EN_CCBSYB_CLK_STR BIT(1) > > +#define MAX9286_EN_GPI_CCBSYB BIT(0) > > /* Register 0x1b */ > > #define MAX9286_SWITCHIN(n) (1 << ((n) + 4)) > > #define MAX9286_ENEQ(n) (1 << (n)) > > @@ -525,10 +528,12 @@ static void max9286_set_video_format(struct max9286_priv *priv, > > return; > > > > /* > > - * Video format setup: > > - * Disable CSI output, VC is set according to Link number. > > + * Video format setup: disable CSI output, set VC according to Link > > + * number, enable I2C clock stretching when CCBSY is low, enable CCBSY > > + * in external GPI-to-GPO mode. > > */ > > - max9286_write(priv, 0x15, MAX9286_VCTYPE | MAX9286_0X15_RESV); > > + max9286_write(priv, 0x15, MAX9286_VCTYPE | MAX9286_EN_CCBSYB_CLK_STR | > > + MAX9286_EN_GPI_CCBSYB); > > > > /* Enable CSI-2 Lane D0-D3 only, DBL mode. */ > > max9286_write(priv, 0x12, MAX9286_CSIDBL | MAX9286_DBL | > > @@ -810,13 +815,17 @@ static int max9286_s_stream(struct v4l2_subdev *sd, int enable) > > } > > > > /* > > - * Enable CSI output, VC set according to link number. > > - * Bit 7 must be set (chip manual says it's 0 and reserved). > > + * Configure the CSI-2 output to line interleaved mode (W x (N > > + * x H), as opposed to the (N x W) x H mode that outputs the > > + * images stitched side-by-side) and enable it. > > */ > > - max9286_write(priv, 0x15, 0x80 | MAX9286_VCTYPE | > > - MAX9286_CSIOUTEN | MAX9286_0X15_RESV); > > + max9286_write(priv, 0x15, MAX9286_CSI_IMAGE_TYP | MAX9286_VCTYPE | > > + MAX9286_CSIOUTEN | MAX9286_EN_CCBSYB_CLK_STR | > > + MAX9286_EN_GPI_CCBSYB); > > } else { > > - max9286_write(priv, 0x15, MAX9286_VCTYPE | MAX9286_0X15_RESV); > > + max9286_write(priv, 0x15, MAX9286_VCTYPE | > > + MAX9286_EN_CCBSYB_CLK_STR | > > + MAX9286_EN_GPI_CCBSYB); > > Probably fits better on two lines only. That would be over the 80 columns limit, which is a soft limit now, but still often requested by reviewers (including myself in quite a few cases :-)). > Trusting the bits definitions: > Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > > > > /* Stop all cameras. */ > > for_each_source(priv, source)
Hello! On 1/10/22 2:21 AM, Laurent Pinchart wrote: >>> Macros are easier to read than numerical values. >>> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> >>> --- >>> drivers/media/i2c/max9286.c | 27 ++++++++++++++++++--------- >>> 1 file changed, 18 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c >>> index 24c2bf4fda53..4b69bd036ca6 100644 >>> --- a/drivers/media/i2c/max9286.c >>> +++ b/drivers/media/i2c/max9286.c [...] >>> @@ -810,13 +815,17 @@ static int max9286_s_stream(struct v4l2_subdev *sd, int enable) >>> } >>> >>> /* >>> - * Enable CSI output, VC set according to link number. >>> - * Bit 7 must be set (chip manual says it's 0 and reserved). >>> + * Configure the CSI-2 output to line interleaved mode (W x (N >>> + * x H), as opposed to the (N x W) x H mode that outputs the >>> + * images stitched side-by-side) and enable it. >>> */ >>> - max9286_write(priv, 0x15, 0x80 | MAX9286_VCTYPE | >>> - MAX9286_CSIOUTEN | MAX9286_0X15_RESV); >>> + max9286_write(priv, 0x15, MAX9286_CSI_IMAGE_TYP | MAX9286_VCTYPE | >>> + MAX9286_CSIOUTEN | MAX9286_EN_CCBSYB_CLK_STR | >>> + MAX9286_EN_GPI_CCBSYB); >>> } else { >>> - max9286_write(priv, 0x15, MAX9286_VCTYPE | MAX9286_0X15_RESV); >>> + max9286_write(priv, 0x15, MAX9286_VCTYPE | >>> + MAX9286_EN_CCBSYB_CLK_STR | >>> + MAX9286_EN_GPI_CCBSYB); >> >> Probably fits better on two lines only. > > That would be over the 80 columns limit, which is a soft limit now, but > still often requested by reviewers (including myself in quite a few > cases :-)). The new limit is 100 columns, not 80. :-) [...] MBR, Sergey
Hi Sergey, On Mon, Jan 10, 2022 at 01:37:51PM +0300, Sergey Shtylyov wrote: > Hello! > > On 1/10/22 2:21 AM, Laurent Pinchart wrote: > > >>> Macros are easier to read than numerical values. > >>> > >>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > >>> --- > >>> drivers/media/i2c/max9286.c | 27 ++++++++++++++++++--------- > >>> 1 file changed, 18 insertions(+), 9 deletions(-) > >>> > >>> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c > >>> index 24c2bf4fda53..4b69bd036ca6 100644 > >>> --- a/drivers/media/i2c/max9286.c > >>> +++ b/drivers/media/i2c/max9286.c > [...] > >>> @@ -810,13 +815,17 @@ static int max9286_s_stream(struct v4l2_subdev *sd, int enable) > >>> } > >>> > >>> /* > >>> - * Enable CSI output, VC set according to link number. > >>> - * Bit 7 must be set (chip manual says it's 0 and reserved). > >>> + * Configure the CSI-2 output to line interleaved mode (W x (N > >>> + * x H), as opposed to the (N x W) x H mode that outputs the > >>> + * images stitched side-by-side) and enable it. > >>> */ > >>> - max9286_write(priv, 0x15, 0x80 | MAX9286_VCTYPE | > >>> - MAX9286_CSIOUTEN | MAX9286_0X15_RESV); > >>> + max9286_write(priv, 0x15, MAX9286_CSI_IMAGE_TYP | MAX9286_VCTYPE | > >>> + MAX9286_CSIOUTEN | MAX9286_EN_CCBSYB_CLK_STR | > >>> + MAX9286_EN_GPI_CCBSYB); > >>> } else { > >>> - max9286_write(priv, 0x15, MAX9286_VCTYPE | MAX9286_0X15_RESV); > >>> + max9286_write(priv, 0x15, MAX9286_VCTYPE | > >>> + MAX9286_EN_CCBSYB_CLK_STR | > >>> + MAX9286_EN_GPI_CCBSYB); > >> > >> Probably fits better on two lines only. > > > > That would be over the 80 columns limit, which is a soft limit now, but > > still often requested by reviewers (including myself in quite a few > > cases :-)). > > The new limit is 100 columns, not 80. :-) That's the new hard limit, yes :-) I do occasionally write lines wider than 80 columns and am often asked to change that. In this specific case it doesn't matter much to me, I'll happily pick whatever option reviewers will want to give me a Reviewed-by as both are equally readable for me. > [...]
diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c index 24c2bf4fda53..4b69bd036ca6 100644 --- a/drivers/media/i2c/max9286.c +++ b/drivers/media/i2c/max9286.c @@ -80,10 +80,13 @@ #define MAX9286_DATATYPE_RGB565 (1 << 0) #define MAX9286_DATATYPE_RGB888 (0 << 0) /* Register 0x15 */ +#define MAX9286_CSI_IMAGE_TYP BIT(7) #define MAX9286_VC(n) ((n) << 5) #define MAX9286_VCTYPE BIT(4) #define MAX9286_CSIOUTEN BIT(3) -#define MAX9286_0X15_RESV (3 << 0) +#define MAX9286_SWP_ENDIAN BIT(2) +#define MAX9286_EN_CCBSYB_CLK_STR BIT(1) +#define MAX9286_EN_GPI_CCBSYB BIT(0) /* Register 0x1b */ #define MAX9286_SWITCHIN(n) (1 << ((n) + 4)) #define MAX9286_ENEQ(n) (1 << (n)) @@ -525,10 +528,12 @@ static void max9286_set_video_format(struct max9286_priv *priv, return; /* - * Video format setup: - * Disable CSI output, VC is set according to Link number. + * Video format setup: disable CSI output, set VC according to Link + * number, enable I2C clock stretching when CCBSY is low, enable CCBSY + * in external GPI-to-GPO mode. */ - max9286_write(priv, 0x15, MAX9286_VCTYPE | MAX9286_0X15_RESV); + max9286_write(priv, 0x15, MAX9286_VCTYPE | MAX9286_EN_CCBSYB_CLK_STR | + MAX9286_EN_GPI_CCBSYB); /* Enable CSI-2 Lane D0-D3 only, DBL mode. */ max9286_write(priv, 0x12, MAX9286_CSIDBL | MAX9286_DBL | @@ -810,13 +815,17 @@ static int max9286_s_stream(struct v4l2_subdev *sd, int enable) } /* - * Enable CSI output, VC set according to link number. - * Bit 7 must be set (chip manual says it's 0 and reserved). + * Configure the CSI-2 output to line interleaved mode (W x (N + * x H), as opposed to the (N x W) x H mode that outputs the + * images stitched side-by-side) and enable it. */ - max9286_write(priv, 0x15, 0x80 | MAX9286_VCTYPE | - MAX9286_CSIOUTEN | MAX9286_0X15_RESV); + max9286_write(priv, 0x15, MAX9286_CSI_IMAGE_TYP | MAX9286_VCTYPE | + MAX9286_CSIOUTEN | MAX9286_EN_CCBSYB_CLK_STR | + MAX9286_EN_GPI_CCBSYB); } else { - max9286_write(priv, 0x15, MAX9286_VCTYPE | MAX9286_0X15_RESV); + max9286_write(priv, 0x15, MAX9286_VCTYPE | + MAX9286_EN_CCBSYB_CLK_STR | + MAX9286_EN_GPI_CCBSYB); /* Stop all cameras. */ for_each_source(priv, source)
Macros are easier to read than numerical values. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- drivers/media/i2c/max9286.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-)