diff mbox series

[09/10] media: ar0521: Rework startup sequence

Message ID 20221005190613.394277-10-jacopo@jmondi.org (mailing list archive)
State New, archived
Headers show
Series media: ar0521: Add analog gain, rework clock tree | expand

Commit Message

Jacopo Mondi Oct. 5, 2022, 7:06 p.m. UTC 
The ar0521_write_mode() function explicitly programs the exposure time
register and the test pattern register, which are now setup by the call
to __v4l2_ctrl_handler_setup() in ar0521_set_stream().

Removing those register writes from ar0521_write_mode() reduces the
function to two operations: geometry configuration and pll
configuration.

Move those operations in the ar0521_set_stream() caller and remove
ar0521_write_mode(). However maintain the ar0521_calc_pll() function
separated as it is used during pad format configuration to update the
PIXEL_RATE control value.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ar0521.c | 50 ++++++++++++--------------------------
 1 file changed, 16 insertions(+), 34 deletions(-)

--
2.37.3

Comments

Dave Stevenson Oct. 6, 2022, 3:45 p.m. UTC | #1 
On Wed, 5 Oct 2022 at 20:07, Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> The ar0521_write_mode() function explicitly programs the exposure time
> register and the test pattern register, which are now setup by the call
> to __v4l2_ctrl_handler_setup() in ar0521_set_stream().
>
> Removing those register writes from ar0521_write_mode() reduces the
> function to two operations: geometry configuration and pll
> configuration.
>
> Move those operations in the ar0521_set_stream() caller and remove
> ar0521_write_mode(). However maintain the ar0521_calc_pll() function
> separated as it is used during pad format configuration to update the
> PIXEL_RATE control value.
>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Looks good.

Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

> ---
>  drivers/media/i2c/ar0521.c | 50 ++++++++++++--------------------------
>  1 file changed, 16 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/media/i2c/ar0521.c b/drivers/media/i2c/ar0521.c
> index d46a51332964..670fa33acc6f 100644
> --- a/drivers/media/i2c/ar0521.c
> +++ b/drivers/media/i2c/ar0521.c
> @@ -329,7 +329,7 @@ static u32 calc_pll(struct ar0521_dev *sensor, u32 freq, u16 *pre_ptr, u16 *mult
>         return pll;
>  }
>
> -static void ar0521_calc_mode(struct ar0521_dev *sensor)
> +static void ar0521_calc_pll(struct ar0521_dev *sensor)
>  {
>         unsigned int pixel_clock;
>         unsigned int link_freq;
> @@ -403,7 +403,7 @@ static void ar0521_calc_mode(struct ar0521_dev *sensor)
>         sensor->pll.mult = sensor->pll.mult2 = mult;
>  }
>
> -static int ar0521_write_mode(struct ar0521_dev *sensor)
> +static int ar0521_pll_config(struct ar0521_dev *sensor)
>  {
>         __be16 pll_regs[] = {
>                 be(AR0521_REG_VT_PIX_CLK_DIV),
> @@ -414,36 +414,9 @@ static int ar0521_write_mode(struct ar0521_dev *sensor)
>                 /* 0x308 */ be(sensor->pll.vt_pix * 2), /* op_pix_clk_div = 2 * vt_pix_clk_div */
>                 /* 0x30A */ be(1)  /* op_sys_clk_div */
>         };
> -       int ret;
> -
> -       /* Stop streaming for just a moment */
> -       ret = ar0521_write_reg(sensor, AR0521_REG_RESET,
> -                              AR0521_REG_RESET_DEFAULTS);
> -       if (ret)
> -               return ret;
> -
> -       ret = ar0521_set_geometry(sensor);
> -       if (ret)
> -               return ret;
> -
> -       ret = ar0521_write_regs(sensor, pll_regs, ARRAY_SIZE(pll_regs));
> -       if (ret)
> -               return ret;
> -
> -       ret = ar0521_write_reg(sensor, AR0521_REG_COARSE_INTEGRATION_TIME,
> -                              sensor->ctrls.exposure->val);
> -       if (ret)
> -               return ret;
> -
> -       ret = ar0521_write_reg(sensor, AR0521_REG_RESET,
> -                              AR0521_REG_RESET_DEFAULTS |
> -                              AR0521_REG_RESET_STREAM);
> -       if (ret)
> -               return ret;
>
> -       ret = ar0521_write_reg(sensor, AR0521_REG_TEST_PATTERN_MODE,
> -                              sensor->ctrls.test_pattern->val);
> -       return ret;
> +       ar0521_calc_pll(sensor);
> +       return ar0521_write_regs(sensor, pll_regs, ARRAY_SIZE(pll_regs));
>  }
>
>  static int ar0521_set_stream(struct ar0521_dev *sensor, bool on)
> @@ -455,8 +428,17 @@ static int ar0521_set_stream(struct ar0521_dev *sensor, bool on)
>                 if (ret < 0)
>                         return ret;
>
> -               ar0521_calc_mode(sensor);
> -               ret = ar0521_write_mode(sensor);
> +               /* Stop streaming for just a moment */
> +               ret = ar0521_write_reg(sensor, AR0521_REG_RESET,
> +                                      AR0521_REG_RESET_DEFAULTS);
> +               if (ret)
> +                       return ret;
> +
> +               ret = ar0521_set_geometry(sensor);
> +               if (ret)
> +                       return ret;
> +
> +               ret = ar0521_pll_config(sensor);
>                 if (ret)
>                         goto err;
>
> @@ -562,7 +544,7 @@ static int ar0521_set_fmt(struct v4l2_subdev *sd,
>         }
>
>         sensor->fmt = format->format;
> -       ar0521_calc_mode(sensor);
> +       ar0521_calc_pll(sensor);
>
>         /*
>          * Update the exposure and blankings limits. Blankings are also reset
> --
> 2.37.3
>
diff mbox series

Patch

diff --git a/drivers/media/i2c/ar0521.c b/drivers/media/i2c/ar0521.c
index d46a51332964..670fa33acc6f 100644
--- a/drivers/media/i2c/ar0521.c
+++ b/drivers/media/i2c/ar0521.c
@@ -329,7 +329,7 @@  static u32 calc_pll(struct ar0521_dev *sensor, u32 freq, u16 *pre_ptr, u16 *mult
 	return pll;
 }

-static void ar0521_calc_mode(struct ar0521_dev *sensor)
+static void ar0521_calc_pll(struct ar0521_dev *sensor)
 {
 	unsigned int pixel_clock;
 	unsigned int link_freq;
@@ -403,7 +403,7 @@  static void ar0521_calc_mode(struct ar0521_dev *sensor)
 	sensor->pll.mult = sensor->pll.mult2 = mult;
 }

-static int ar0521_write_mode(struct ar0521_dev *sensor)
+static int ar0521_pll_config(struct ar0521_dev *sensor)
 {
 	__be16 pll_regs[] = {
 		be(AR0521_REG_VT_PIX_CLK_DIV),
@@ -414,36 +414,9 @@  static int ar0521_write_mode(struct ar0521_dev *sensor)
 		/* 0x308 */ be(sensor->pll.vt_pix * 2), /* op_pix_clk_div = 2 * vt_pix_clk_div */
 		/* 0x30A */ be(1)  /* op_sys_clk_div */
 	};
-	int ret;
-
-	/* Stop streaming for just a moment */
-	ret = ar0521_write_reg(sensor, AR0521_REG_RESET,
-			       AR0521_REG_RESET_DEFAULTS);
-	if (ret)
-		return ret;
-
-	ret = ar0521_set_geometry(sensor);
-	if (ret)
-		return ret;
-
-	ret = ar0521_write_regs(sensor, pll_regs, ARRAY_SIZE(pll_regs));
-	if (ret)
-		return ret;
-
-	ret = ar0521_write_reg(sensor, AR0521_REG_COARSE_INTEGRATION_TIME,
-			       sensor->ctrls.exposure->val);
-	if (ret)
-		return ret;
-
-	ret = ar0521_write_reg(sensor, AR0521_REG_RESET,
-			       AR0521_REG_RESET_DEFAULTS |
-			       AR0521_REG_RESET_STREAM);
-	if (ret)
-		return ret;

-	ret = ar0521_write_reg(sensor, AR0521_REG_TEST_PATTERN_MODE,
-			       sensor->ctrls.test_pattern->val);
-	return ret;
+	ar0521_calc_pll(sensor);
+	return ar0521_write_regs(sensor, pll_regs, ARRAY_SIZE(pll_regs));
 }

 static int ar0521_set_stream(struct ar0521_dev *sensor, bool on)
@@ -455,8 +428,17 @@  static int ar0521_set_stream(struct ar0521_dev *sensor, bool on)
 		if (ret < 0)
 			return ret;

-		ar0521_calc_mode(sensor);
-		ret = ar0521_write_mode(sensor);
+		/* Stop streaming for just a moment */
+		ret = ar0521_write_reg(sensor, AR0521_REG_RESET,
+				       AR0521_REG_RESET_DEFAULTS);
+		if (ret)
+			return ret;
+
+		ret = ar0521_set_geometry(sensor);
+		if (ret)
+			return ret;
+
+		ret = ar0521_pll_config(sensor);
 		if (ret)
 			goto err;

@@ -562,7 +544,7 @@  static int ar0521_set_fmt(struct v4l2_subdev *sd,
 	}

 	sensor->fmt = format->format;
-	ar0521_calc_mode(sensor);
+	ar0521_calc_pll(sensor);

 	/*
 	 * Update the exposure and blankings limits. Blankings are also reset