| Message ID | 20220101182806.19311-9-laurent.pinchart+renesas@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | media: i2c: max9286: Small new features | expand |
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
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(-)