[PATCH 0/5] Alvium improvements

[PATCH 0/5] Alvium improvements

[Tommaso Merciai]

Hello,

This series containing improvements for the Alvium camera driver.
Specifically:

Patches:
 - 1 Fix fix alvium_get_fw_version()
 - 2 Rename acquisition frame rate enable define into a short define
 - 3 Alvium camera by default is in free running mode. Datasheet say that
     acquisition frame rate reg can only be used if frame start trigger
     mode is set to off. Enable r/w aquisition frame rate and turn off trigger mode.
 - 4 Implement enum_frame_size
 - 5 Use the right V4L2_CID for the analogue gain

Thanks & Regards,
Tommaso

Tommaso Merciai (5):
  media: i2c: alvium: fix alvium_get_fw_version()
  media: i2c: alvium: rename acquisition frame rate enable reg
  media: i2c: alvium: enable acquisition frame rate
  media: i2c: alvium: implement enum_frame_size
  media: i2c: alvium: Move V4L2_CID_GAIN to V4L2_CID_ANALOG_GAIN

 drivers/media/i2c/alvium-csi2.c | 62 +++++++++++++++++++++++++--------
 drivers/media/i2c/alvium-csi2.h | 17 ++++++---
 2 files changed, 59 insertions(+), 20 deletions(-)

-- 
2.34.1

[PATCH 1/5] media: i2c: alvium: fix alvium_get_fw_version()

[Tommaso Merciai]

Instead of reading device_fw reg as multiple regs let's read the entire
64bit reg using one i2c read and store this info into alvium_fw_version
union fixing the dev_info formatting output.

Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>
---
 drivers/media/i2c/alvium-csi2.c | 20 ++++++++------------
 drivers/media/i2c/alvium-csi2.h | 15 +++++++++++----
 2 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/media/i2c/alvium-csi2.c b/drivers/media/i2c/alvium-csi2.c
index e65702e3f73e..991b3bcc8b80 100644
--- a/drivers/media/i2c/alvium-csi2.c
+++ b/drivers/media/i2c/alvium-csi2.c
@@ -403,21 +403,17 @@ static int alvium_get_bcrm_vers(struct alvium_dev *alvium)
 static int alvium_get_fw_version(struct alvium_dev *alvium)
 {
 	struct device *dev = &alvium->i2c_client->dev;
-	u64 spec, maj, min, pat;
+	union alvium_fw_version v;
 	int ret = 0;
 
-	ret = alvium_read(alvium, REG_BCRM_DEVICE_FW_SPEC_VERSION_R,
-			  &spec, &ret);
-	ret = alvium_read(alvium, REG_BCRM_DEVICE_FW_MAJOR_VERSION_R,
-			  &maj, &ret);
-	ret = alvium_read(alvium, REG_BCRM_DEVICE_FW_MINOR_VERSION_R,
-			  &min, &ret);
-	ret = alvium_read(alvium, REG_BCRM_DEVICE_FW_PATCH_VERSION_R,
-			  &pat, &ret);
-	if (ret)
-		return ret;
+	ret = alvium_read(alvium, REG_BCRM_DEVICE_FW,
+			  &v.value, &ret);
 
-	dev_info(dev, "fw version: %llu.%llu.%llu.%llu\n", spec, maj, min, pat);
+	dev_info(dev, "fw version: %u.%u.%08x special: %u\n",
+		 (u32)v.alvium_fw_ver.major,
+		 (u32)v.alvium_fw_ver.minor,
+		 v.alvium_fw_ver.patch,
+		 (u32)v.alvium_fw_ver.special);
 
 	return 0;
 }
diff --git a/drivers/media/i2c/alvium-csi2.h b/drivers/media/i2c/alvium-csi2.h
index 9463f8604fbc..9c4cfb35de8e 100644
--- a/drivers/media/i2c/alvium-csi2.h
+++ b/drivers/media/i2c/alvium-csi2.h
@@ -31,10 +31,7 @@
 #define REG_BCRM_REG_ADDR_R				CCI_REG16(0x0014)
 
 #define REG_BCRM_FEATURE_INQUIRY_R			REG_BCRM_V4L2_64BIT(0x0008)
-#define REG_BCRM_DEVICE_FW_SPEC_VERSION_R		REG_BCRM_V4L2_8BIT(0x0010)
-#define REG_BCRM_DEVICE_FW_MAJOR_VERSION_R		REG_BCRM_V4L2_8BIT(0x0011)
-#define REG_BCRM_DEVICE_FW_MINOR_VERSION_R		REG_BCRM_V4L2_16BIT(0x0012)
-#define REG_BCRM_DEVICE_FW_PATCH_VERSION_R		REG_BCRM_V4L2_32BIT(0x0014)
+#define REG_BCRM_DEVICE_FW				REG_BCRM_V4L2_64BIT(0x0010)
 #define REG_BCRM_WRITE_HANDSHAKE_RW			REG_BCRM_V4L2_8BIT(0x0018)
 
 /* Streaming Control Registers */
@@ -276,6 +273,16 @@ enum alvium_av_mipi_bit {
 	ALVIUM_NUM_SUPP_MIPI_DATA_BIT
 };
 
+union alvium_fw_version {
+	struct {
+		u8 special;
+		u8 major;
+		u16 minor;
+		u32 patch;
+	} alvium_fw_ver;
+	u64 value;
+};
+
 struct alvium_avail_feat {
 	u64 rev_x:1;
 	u64 rev_y:1;
-- 
2.34.1

Re: [PATCH 1/5] media: i2c: alvium: fix alvium_get_fw_version()

[Sakari Ailus]

Hi Tommaso,

On Tue, Apr 16, 2024 at 04:19:01PM +0200, Tommaso Merciai wrote:
> Instead of reading device_fw reg as multiple regs let's read the entire
> 64bit reg using one i2c read and store this info into alvium_fw_version
> union fixing the dev_info formatting output.
> 
> Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>
> ---
>  drivers/media/i2c/alvium-csi2.c | 20 ++++++++------------
>  drivers/media/i2c/alvium-csi2.h | 15 +++++++++++----
>  2 files changed, 19 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/media/i2c/alvium-csi2.c b/drivers/media/i2c/alvium-csi2.c
> index e65702e3f73e..991b3bcc8b80 100644
> --- a/drivers/media/i2c/alvium-csi2.c
> +++ b/drivers/media/i2c/alvium-csi2.c
> @@ -403,21 +403,17 @@ static int alvium_get_bcrm_vers(struct alvium_dev *alvium)
>  static int alvium_get_fw_version(struct alvium_dev *alvium)
>  {
>  	struct device *dev = &alvium->i2c_client->dev;
> -	u64 spec, maj, min, pat;
> +	union alvium_fw_version v;
>  	int ret = 0;
>  
> -	ret = alvium_read(alvium, REG_BCRM_DEVICE_FW_SPEC_VERSION_R,
> -			  &spec, &ret);
> -	ret = alvium_read(alvium, REG_BCRM_DEVICE_FW_MAJOR_VERSION_R,
> -			  &maj, &ret);
> -	ret = alvium_read(alvium, REG_BCRM_DEVICE_FW_MINOR_VERSION_R,
> -			  &min, &ret);
> -	ret = alvium_read(alvium, REG_BCRM_DEVICE_FW_PATCH_VERSION_R,
> -			  &pat, &ret);
> -	if (ret)
> -		return ret;
> +	ret = alvium_read(alvium, REG_BCRM_DEVICE_FW,
> +			  &v.value, &ret);

Doesn't this have the same endianness issues that earlier were resolved by
doing the reads separately?

>  
> -	dev_info(dev, "fw version: %llu.%llu.%llu.%llu\n", spec, maj, min, pat);
> +	dev_info(dev, "fw version: %u.%u.%08x special: %u\n",
> +		 (u32)v.alvium_fw_ver.major,
> +		 (u32)v.alvium_fw_ver.minor,
> +		 v.alvium_fw_ver.patch,
> +		 (u32)v.alvium_fw_ver.special);
>  
>  	return 0;
>  }
> diff --git a/drivers/media/i2c/alvium-csi2.h b/drivers/media/i2c/alvium-csi2.h
> index 9463f8604fbc..9c4cfb35de8e 100644
> --- a/drivers/media/i2c/alvium-csi2.h
> +++ b/drivers/media/i2c/alvium-csi2.h
> @@ -31,10 +31,7 @@
>  #define REG_BCRM_REG_ADDR_R				CCI_REG16(0x0014)
>  
>  #define REG_BCRM_FEATURE_INQUIRY_R			REG_BCRM_V4L2_64BIT(0x0008)
> -#define REG_BCRM_DEVICE_FW_SPEC_VERSION_R		REG_BCRM_V4L2_8BIT(0x0010)
> -#define REG_BCRM_DEVICE_FW_MAJOR_VERSION_R		REG_BCRM_V4L2_8BIT(0x0011)
> -#define REG_BCRM_DEVICE_FW_MINOR_VERSION_R		REG_BCRM_V4L2_16BIT(0x0012)
> -#define REG_BCRM_DEVICE_FW_PATCH_VERSION_R		REG_BCRM_V4L2_32BIT(0x0014)
> +#define REG_BCRM_DEVICE_FW				REG_BCRM_V4L2_64BIT(0x0010)
>  #define REG_BCRM_WRITE_HANDSHAKE_RW			REG_BCRM_V4L2_8BIT(0x0018)
>  
>  /* Streaming Control Registers */
> @@ -276,6 +273,16 @@ enum alvium_av_mipi_bit {
>  	ALVIUM_NUM_SUPP_MIPI_DATA_BIT
>  };
>  
> +union alvium_fw_version {
> +	struct {
> +		u8 special;
> +		u8 major;
> +		u16 minor;
> +		u32 patch;
> +	} alvium_fw_ver;
> +	u64 value;
> +};
> +
>  struct alvium_avail_feat {
>  	u64 rev_x:1;
>  	u64 rev_y:1;

-- 
Kind regards,

Sakari Ailus

[PATCH 2/5] media: i2c: alvium: rename acquisition frame rate enable reg

[Tommaso Merciai]

Aquisition frame rate enable reg have a very long name let's reduce this
with an abbreviation.

Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>
---
 drivers/media/i2c/alvium-csi2.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/alvium-csi2.h b/drivers/media/i2c/alvium-csi2.h
index 9c4cfb35de8e..c1453ba885cf 100644
--- a/drivers/media/i2c/alvium-csi2.h
+++ b/drivers/media/i2c/alvium-csi2.h
@@ -63,7 +63,7 @@
 #define REG_BCRM_ACQUISITION_FRAME_RATE_MIN_R		REG_BCRM_V4L2_64BIT(0x0098)
 #define REG_BCRM_ACQUISITION_FRAME_RATE_MAX_R		REG_BCRM_V4L2_64BIT(0x00a0)
 #define REG_BCRM_ACQUISITION_FRAME_RATE_INC_R		REG_BCRM_V4L2_64BIT(0x00a8)
-#define REG_BCRM_ACQUISITION_FRAME_RATE_ENABLE_RW	REG_BCRM_V4L2_8BIT(0x00b0)
+#define REG_BCRM_ACQUISITION_FRAME_RATE_EN_RW		REG_BCRM_V4L2_8BIT(0x00b0)
 
 #define REG_BCRM_FRAME_START_TRIGGER_MODE_RW		REG_BCRM_V4L2_8BIT(0x00b4)
 #define REG_BCRM_FRAME_START_TRIGGER_SOURCE_RW		REG_BCRM_V4L2_8BIT(0x00b8)
-- 
2.34.1

[PATCH 3/5] media: i2c: alvium: enable acquisition frame rate

[Tommaso Merciai]

Alvium camera by default is in free running mode. Datasheet say that
acquisition frame rate reg can only be used if frame start trigger
mode is set to off.
Enable r/w aquisition frame rate and turn off trigger mode.

Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>
---
 drivers/media/i2c/alvium-csi2.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/media/i2c/alvium-csi2.c b/drivers/media/i2c/alvium-csi2.c
index 991b3bcc8b80..2ab4c7e563d2 100644
--- a/drivers/media/i2c/alvium-csi2.c
+++ b/drivers/media/i2c/alvium-csi2.c
@@ -1184,6 +1184,20 @@ static int alvium_set_frame_rate(struct alvium_dev *alvium, u64 fr)
 	struct device *dev = &alvium->i2c_client->dev;
 	int ret;
 
+	ret = alvium_write_hshake(alvium, REG_BCRM_ACQUISITION_FRAME_RATE_EN_RW,
+				  1);
+	if (ret) {
+		dev_err(dev, "Fail to set acquisition frame rate enable reg\n");
+		return ret;
+	}
+
+	ret = alvium_write_hshake(alvium, REG_BCRM_FRAME_START_TRIGGER_MODE_RW,
+				  0);
+	if (ret) {
+		dev_err(dev, "Fail to set frame start trigger mode reg\n");
+		return ret;
+	}
+
 	ret = alvium_write_hshake(alvium, REG_BCRM_ACQUISITION_FRAME_RATE_RW,
 				  fr);
 	if (ret) {
-- 
2.34.1

[PATCH 4/5] media: i2c: alvium: implement enum_frame_size

[Tommaso Merciai]

Implement the enum_frame_size pad operation.
The sensor supports a continuous size range of resolutions.

Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>
---
 drivers/media/i2c/alvium-csi2.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/media/i2c/alvium-csi2.c b/drivers/media/i2c/alvium-csi2.c
index 2ab4c7e563d2..30ef9b905211 100644
--- a/drivers/media/i2c/alvium-csi2.c
+++ b/drivers/media/i2c/alvium-csi2.c
@@ -1717,6 +1717,27 @@ alvium_code_to_pixfmt(struct alvium_dev *alvium, u32 code)
 	return &alvium->alvium_csi2_fmt[0];
 }
 
+static int alvium_enum_frame_size(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_state *state,
+				  struct v4l2_subdev_frame_size_enum *fse)
+{
+	struct alvium_dev *alvium = sd_to_alvium(sd);
+	const struct alvium_pixfmt *alvium_csi2_fmt;
+
+	if (fse->index)
+		return -EINVAL;
+
+	alvium_csi2_fmt = alvium_code_to_pixfmt(alvium, fse->code);
+	if (fse->code != alvium_csi2_fmt->code)
+		return -EINVAL;
+
+	fse->min_width = alvium->img_min_width;
+	fse->max_width = alvium->img_max_width;
+	fse->min_height = alvium->img_min_height;
+	fse->max_height = alvium->img_max_height;
+	return 0;
+}
+
 static int alvium_set_mode(struct alvium_dev *alvium,
 			   struct v4l2_subdev_state *state)
 {
@@ -2224,6 +2245,7 @@ static const struct v4l2_subdev_video_ops alvium_video_ops = {
 
 static const struct v4l2_subdev_pad_ops alvium_pad_ops = {
 	.enum_mbus_code = alvium_enum_mbus_code,
+	.enum_frame_size = alvium_enum_frame_size,
 	.get_fmt = v4l2_subdev_get_fmt,
 	.set_fmt = alvium_set_fmt,
 	.get_selection = alvium_get_selection,
-- 
2.34.1

[PATCH 5/5] media: i2c: alvium: Move V4L2_CID_GAIN to V4L2_CID_ANALOG_GAIN

[Tommaso Merciai]

Into alvium cameras REG_BCRM_GAIN_RW control the analog gain.
Let's use the right V4L2_CID_ANALOGUE_GAIN ctrl.

Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>
---
 drivers/media/i2c/alvium-csi2.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/alvium-csi2.c b/drivers/media/i2c/alvium-csi2.c
index 30ef9b905211..56d64f27df72 100644
--- a/drivers/media/i2c/alvium-csi2.c
+++ b/drivers/media/i2c/alvium-csi2.c
@@ -1993,7 +1993,7 @@ static int alvium_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
 	int val;
 
 	switch (ctrl->id) {
-	case V4L2_CID_GAIN:
+	case V4L2_CID_ANALOGUE_GAIN:
 		val = alvium_get_gain(alvium);
 		if (val < 0)
 			return val;
@@ -2025,7 +2025,7 @@ static int alvium_s_ctrl(struct v4l2_ctrl *ctrl)
 		return 0;
 
 	switch (ctrl->id) {
-	case V4L2_CID_GAIN:
+	case V4L2_CID_ANALOGUE_GAIN:
 		ret = alvium_set_ctrl_gain(alvium, ctrl->val);
 		break;
 	case V4L2_CID_AUTOGAIN:
@@ -2154,7 +2154,7 @@ static int alvium_ctrl_init(struct alvium_dev *alvium)
 
 	if (alvium->avail_ft.gain) {
 		ctrls->gain = v4l2_ctrl_new_std(hdl, ops,
-						V4L2_CID_GAIN,
+						V4L2_CID_ANALOGUE_GAIN,
 						alvium->min_gain,
 						alvium->max_gain,
 						alvium->inc_gain,
-- 
2.34.1

Re: [PATCH 5/5] media: i2c: alvium: Move V4L2_CID_GAIN to V4L2_CID_ANALOG_GAIN

[Sakari Ailus]

Hi Tommaso,

On Tue, Apr 16, 2024 at 04:19:05PM +0200, Tommaso Merciai wrote:
> Into alvium cameras REG_BCRM_GAIN_RW control the analog gain.
> Let's use the right V4L2_CID_ANALOGUE_GAIN ctrl.
> 
> Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>
> ---
>  drivers/media/i2c/alvium-csi2.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/i2c/alvium-csi2.c b/drivers/media/i2c/alvium-csi2.c
> index 30ef9b905211..56d64f27df72 100644
> --- a/drivers/media/i2c/alvium-csi2.c
> +++ b/drivers/media/i2c/alvium-csi2.c
> @@ -1993,7 +1993,7 @@ static int alvium_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
>  	int val;
>  
>  	switch (ctrl->id) {
> -	case V4L2_CID_GAIN:
> +	case V4L2_CID_ANALOGUE_GAIN:
>  		val = alvium_get_gain(alvium);
>  		if (val < 0)
>  			return val;
> @@ -2025,7 +2025,7 @@ static int alvium_s_ctrl(struct v4l2_ctrl *ctrl)
>  		return 0;
>  
>  	switch (ctrl->id) {
> -	case V4L2_CID_GAIN:
> +	case V4L2_CID_ANALOGUE_GAIN:
>  		ret = alvium_set_ctrl_gain(alvium, ctrl->val);
>  		break;
>  	case V4L2_CID_AUTOGAIN:
> @@ -2154,7 +2154,7 @@ static int alvium_ctrl_init(struct alvium_dev *alvium)
>  
>  	if (alvium->avail_ft.gain) {
>  		ctrls->gain = v4l2_ctrl_new_std(hdl, ops,
> -						V4L2_CID_GAIN,
> +						V4L2_CID_ANALOGUE_GAIN,
>  						alvium->min_gain,
>  						alvium->max_gain,
>  						alvium->inc_gain,

This looks like a bugfix. Shouldn't it be cc'd to stable as well? A Fixes:
tag would be nice, too.

-- 
Regards,

Sakari Ailus

Re: [PATCH 5/5] media: i2c: alvium: Move V4L2_CID_GAIN to V4L2_CID_ANALOG_GAIN

[Tommaso Merciai]

Hi Sakari,
Thanks for your review.

On Wed, Apr 24, 2024 at 06:11:26PM +0000, Sakari Ailus wrote:
> Hi Tommaso,
> 
> On Tue, Apr 16, 2024 at 04:19:05PM +0200, Tommaso Merciai wrote:
> > Into alvium cameras REG_BCRM_GAIN_RW control the analog gain.
> > Let's use the right V4L2_CID_ANALOGUE_GAIN ctrl.
> > 
> > Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>
> > ---
> >  drivers/media/i2c/alvium-csi2.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/media/i2c/alvium-csi2.c b/drivers/media/i2c/alvium-csi2.c
> > index 30ef9b905211..56d64f27df72 100644
> > --- a/drivers/media/i2c/alvium-csi2.c
> > +++ b/drivers/media/i2c/alvium-csi2.c
> > @@ -1993,7 +1993,7 @@ static int alvium_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
> >  	int val;
> >  
> >  	switch (ctrl->id) {
> > -	case V4L2_CID_GAIN:
> > +	case V4L2_CID_ANALOGUE_GAIN:
> >  		val = alvium_get_gain(alvium);
> >  		if (val < 0)
> >  			return val;
> > @@ -2025,7 +2025,7 @@ static int alvium_s_ctrl(struct v4l2_ctrl *ctrl)
> >  		return 0;
> >  
> >  	switch (ctrl->id) {
> > -	case V4L2_CID_GAIN:
> > +	case V4L2_CID_ANALOGUE_GAIN:
> >  		ret = alvium_set_ctrl_gain(alvium, ctrl->val);
> >  		break;
> >  	case V4L2_CID_AUTOGAIN:
> > @@ -2154,7 +2154,7 @@ static int alvium_ctrl_init(struct alvium_dev *alvium)
> >  
> >  	if (alvium->avail_ft.gain) {
> >  		ctrls->gain = v4l2_ctrl_new_std(hdl, ops,
> > -						V4L2_CID_GAIN,
> > +						V4L2_CID_ANALOGUE_GAIN,
> >  						alvium->min_gain,
> >  						alvium->max_gain,
> >  						alvium->inc_gain,
> 
> This looks like a bugfix. Shouldn't it be cc'd to stable as well? A Fixes:
> tag would be nice, too.

Fully agree.
Plan is to add in v2 Fixes: 0a7af872915e ("media: i2c: Add support for alvium camera")
like you suggest and stable@vger.kernel.org in CC.

Thanks & Regards,
Tommaso

> 
> -- 
> Regards,
> 
> Sakari Ailus

Re: [PATCH 5/5] media: i2c: alvium: Move V4L2_CID_GAIN to V4L2_CID_ANALOG_GAIN

[Sakari Ailus]

Hi Tommaso,

On Fri, Apr 26, 2024 at 09:09:35AM +0200, Tommaso Merciai wrote:
> Hi Sakari,
> Thanks for your review.
> 
> On Wed, Apr 24, 2024 at 06:11:26PM +0000, Sakari Ailus wrote:
> > Hi Tommaso,
> > 
> > On Tue, Apr 16, 2024 at 04:19:05PM +0200, Tommaso Merciai wrote:
> > > Into alvium cameras REG_BCRM_GAIN_RW control the analog gain.
> > > Let's use the right V4L2_CID_ANALOGUE_GAIN ctrl.
> > > 
> > > Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>
> > > ---
> > >  drivers/media/i2c/alvium-csi2.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/media/i2c/alvium-csi2.c b/drivers/media/i2c/alvium-csi2.c
> > > index 30ef9b905211..56d64f27df72 100644
> > > --- a/drivers/media/i2c/alvium-csi2.c
> > > +++ b/drivers/media/i2c/alvium-csi2.c
> > > @@ -1993,7 +1993,7 @@ static int alvium_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
> > >  	int val;
> > >  
> > >  	switch (ctrl->id) {
> > > -	case V4L2_CID_GAIN:
> > > +	case V4L2_CID_ANALOGUE_GAIN:
> > >  		val = alvium_get_gain(alvium);
> > >  		if (val < 0)
> > >  			return val;
> > > @@ -2025,7 +2025,7 @@ static int alvium_s_ctrl(struct v4l2_ctrl *ctrl)
> > >  		return 0;
> > >  
> > >  	switch (ctrl->id) {
> > > -	case V4L2_CID_GAIN:
> > > +	case V4L2_CID_ANALOGUE_GAIN:
> > >  		ret = alvium_set_ctrl_gain(alvium, ctrl->val);
> > >  		break;
> > >  	case V4L2_CID_AUTOGAIN:
> > > @@ -2154,7 +2154,7 @@ static int alvium_ctrl_init(struct alvium_dev *alvium)
> > >  
> > >  	if (alvium->avail_ft.gain) {
> > >  		ctrls->gain = v4l2_ctrl_new_std(hdl, ops,
> > > -						V4L2_CID_GAIN,
> > > +						V4L2_CID_ANALOGUE_GAIN,
> > >  						alvium->min_gain,
> > >  						alvium->max_gain,
> > >  						alvium->inc_gain,
> > 
> > This looks like a bugfix. Shouldn't it be cc'd to stable as well? A Fixes:
> > tag would be nice, too.
> 
> Fully agree.
> Plan is to add in v2 Fixes: 0a7af872915e ("media: i2c: Add support for alvium camera")
> like you suggest and stable@vger.kernel.org in CC.

Just make sure git send-email won't actually cc the patch to the stable
e-mail address. Cc: tag is enough. The git config option is
sendemail.suppresscc.

-- 
Regards,

Sakari Ailus

Re: [PATCH 5/5] media: i2c: alvium: Move V4L2_CID_GAIN to V4L2_CID_ANALOG_GAIN

[Tommaso Merciai]

Hi Sakari,

On Fri, Apr 26, 2024 at 07:13:06AM +0000, Sakari Ailus wrote:
> Hi Tommaso,
> 
> On Fri, Apr 26, 2024 at 09:09:35AM +0200, Tommaso Merciai wrote:
> > Hi Sakari,
> > Thanks for your review.
> > 
> > On Wed, Apr 24, 2024 at 06:11:26PM +0000, Sakari Ailus wrote:
> > > Hi Tommaso,
> > > 
> > > On Tue, Apr 16, 2024 at 04:19:05PM +0200, Tommaso Merciai wrote:
> > > > Into alvium cameras REG_BCRM_GAIN_RW control the analog gain.
> > > > Let's use the right V4L2_CID_ANALOGUE_GAIN ctrl.
> > > > 
> > > > Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>
> > > > ---
> > > >  drivers/media/i2c/alvium-csi2.c | 6 +++---
> > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/media/i2c/alvium-csi2.c b/drivers/media/i2c/alvium-csi2.c
> > > > index 30ef9b905211..56d64f27df72 100644
> > > > --- a/drivers/media/i2c/alvium-csi2.c
> > > > +++ b/drivers/media/i2c/alvium-csi2.c
> > > > @@ -1993,7 +1993,7 @@ static int alvium_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
> > > >  	int val;
> > > >  
> > > >  	switch (ctrl->id) {
> > > > -	case V4L2_CID_GAIN:
> > > > +	case V4L2_CID_ANALOGUE_GAIN:
> > > >  		val = alvium_get_gain(alvium);
> > > >  		if (val < 0)
> > > >  			return val;
> > > > @@ -2025,7 +2025,7 @@ static int alvium_s_ctrl(struct v4l2_ctrl *ctrl)
> > > >  		return 0;
> > > >  
> > > >  	switch (ctrl->id) {
> > > > -	case V4L2_CID_GAIN:
> > > > +	case V4L2_CID_ANALOGUE_GAIN:
> > > >  		ret = alvium_set_ctrl_gain(alvium, ctrl->val);
> > > >  		break;
> > > >  	case V4L2_CID_AUTOGAIN:
> > > > @@ -2154,7 +2154,7 @@ static int alvium_ctrl_init(struct alvium_dev *alvium)
> > > >  
> > > >  	if (alvium->avail_ft.gain) {
> > > >  		ctrls->gain = v4l2_ctrl_new_std(hdl, ops,
> > > > -						V4L2_CID_GAIN,
> > > > +						V4L2_CID_ANALOGUE_GAIN,
> > > >  						alvium->min_gain,
> > > >  						alvium->max_gain,
> > > >  						alvium->inc_gain,
> > > 
> > > This looks like a bugfix. Shouldn't it be cc'd to stable as well? A Fixes:
> > > tag would be nice, too.
> > 
> > Fully agree.
> > Plan is to add in v2 Fixes: 0a7af872915e ("media: i2c: Add support for alvium camera")
> > like you suggest and stable@vger.kernel.org in CC.
> 
> Just make sure git send-email won't actually cc the patch to the stable
> e-mail address. Cc: tag is enough. The git config option is
> sendemail.suppresscc.

Sorry, just to clarify.
You are suggesting just to add sendemail.suppresscc. option right?
No cc stable@vger.kernel.org

Thanks & Regards,
Tommaso

> 
> -- 
> Regards,
> 
> Sakari Ailus

Re: [PATCH 5/5] media: i2c: alvium: Move V4L2_CID_GAIN to V4L2_CID_ANALOG_GAIN

[Sakari Ailus]

Hi Tommaso,

On Fri, Apr 26, 2024 at 10:41:36AM +0200, Tommaso Merciai wrote:
> Hi Sakari,
> 
> On Fri, Apr 26, 2024 at 07:13:06AM +0000, Sakari Ailus wrote:
> > Hi Tommaso,
> > 
> > On Fri, Apr 26, 2024 at 09:09:35AM +0200, Tommaso Merciai wrote:
> > > Hi Sakari,
> > > Thanks for your review.
> > > 
> > > On Wed, Apr 24, 2024 at 06:11:26PM +0000, Sakari Ailus wrote:
> > > > Hi Tommaso,
> > > > 
> > > > On Tue, Apr 16, 2024 at 04:19:05PM +0200, Tommaso Merciai wrote:
> > > > > Into alvium cameras REG_BCRM_GAIN_RW control the analog gain.
> > > > > Let's use the right V4L2_CID_ANALOGUE_GAIN ctrl.
> > > > > 
> > > > > Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>
> > > > > ---
> > > > >  drivers/media/i2c/alvium-csi2.c | 6 +++---
> > > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/media/i2c/alvium-csi2.c b/drivers/media/i2c/alvium-csi2.c
> > > > > index 30ef9b905211..56d64f27df72 100644
> > > > > --- a/drivers/media/i2c/alvium-csi2.c
> > > > > +++ b/drivers/media/i2c/alvium-csi2.c
> > > > > @@ -1993,7 +1993,7 @@ static int alvium_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
> > > > >  	int val;
> > > > >  
> > > > >  	switch (ctrl->id) {
> > > > > -	case V4L2_CID_GAIN:
> > > > > +	case V4L2_CID_ANALOGUE_GAIN:
> > > > >  		val = alvium_get_gain(alvium);
> > > > >  		if (val < 0)
> > > > >  			return val;
> > > > > @@ -2025,7 +2025,7 @@ static int alvium_s_ctrl(struct v4l2_ctrl *ctrl)
> > > > >  		return 0;
> > > > >  
> > > > >  	switch (ctrl->id) {
> > > > > -	case V4L2_CID_GAIN:
> > > > > +	case V4L2_CID_ANALOGUE_GAIN:
> > > > >  		ret = alvium_set_ctrl_gain(alvium, ctrl->val);
> > > > >  		break;
> > > > >  	case V4L2_CID_AUTOGAIN:
> > > > > @@ -2154,7 +2154,7 @@ static int alvium_ctrl_init(struct alvium_dev *alvium)
> > > > >  
> > > > >  	if (alvium->avail_ft.gain) {
> > > > >  		ctrls->gain = v4l2_ctrl_new_std(hdl, ops,
> > > > > -						V4L2_CID_GAIN,
> > > > > +						V4L2_CID_ANALOGUE_GAIN,
> > > > >  						alvium->min_gain,
> > > > >  						alvium->max_gain,
> > > > >  						alvium->inc_gain,
> > > > 
> > > > This looks like a bugfix. Shouldn't it be cc'd to stable as well? A Fixes:
> > > > tag would be nice, too.
> > > 
> > > Fully agree.
> > > Plan is to add in v2 Fixes: 0a7af872915e ("media: i2c: Add support for alvium camera")
> > > like you suggest and stable@vger.kernel.org in CC.
> > 
> > Just make sure git send-email won't actually cc the patch to the stable
> > e-mail address. Cc: tag is enough. The git config option is
> > sendemail.suppresscc.
> 
> Sorry, just to clarify.
> You are suggesting just to add sendemail.suppresscc. option right?
> No cc stable@vger.kernel.org

Yes, and adding a Cc: stable@vger.kernel.org tag.

-- 
Regards,

Sakari Ailus