From: Alexander Stein <alexander.stein@ew.tq-group.com>
To: Manivannan Sadhasivam <mani@kernel.org>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
linux-media@vger.kernel.org,
Dave Stevenson <dave.stevenson@raspberrypi.com>
Cc: Dave Stevenson <dave.stevenson@raspberrypi.com>
Subject: Re: [PATCH 10/11] media: i2c: imx290: Add support for 74.25MHz external clock
Date: Fri, 03 Feb 2023 08:44:08 +0100 [thread overview]
Message-ID: <1890472.taCxCBeP46@steina-w> (raw)
In-Reply-To: <20230131192016.3476937-11-dave.stevenson@raspberrypi.com>
Hi Dave,
thanks for the patch.
Am Dienstag, 31. Januar 2023, 20:20:15 CET schrieb Dave Stevenson:
> The sensor supports either a 37.125 or 74.25MHz external, but
> the driver only supported 37.125MHz.
>
> Add the relevant register configuration for either clock
> frequency option.
>
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> ---
> drivers/media/i2c/imx290.c | 120 +++++++++++++++++++++++++++++++------
> 1 file changed, 103 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> index 5202ef3cc3e6..7f6746f74040 100644
> --- a/drivers/media/i2c/imx290.c
> +++ b/drivers/media/i2c/imx290.c
> @@ -102,6 +102,7 @@
> #define IMX290_TCLKPREPARE
IMX290_REG_16BIT(0x3452)
> #define IMX290_TLPX
IMX290_REG_16BIT(0x3454)
> #define IMX290_X_OUT_SIZE
IMX290_REG_16BIT(0x3472)
> +#define IMX290_INCKSEL7
IMX290_REG_8BIT(0x3480)
Please add also defines for the clock setting, e.g.
#define IMX290_INCKSEL7_INCK_37_125 0x49
#define IMX290_INCKSEL7_INCK_74_25 0x92
>
> #define IMX290_PGCTRL_REGEN BIT(0)
> #define IMX290_PGCTRL_THRU BIT(1)
> @@ -159,11 +160,27 @@
>
> #define IMX290_NUM_SUPPLIES 3
>
> +#define CLK_37_125 0
> +#define CLK_74_25 1
> +#define NUM_CLK 2
> +
How about using an enum?
enum imx290_clk_freq {
CLK_37_125 = 0,
CLK_74_25 = 1,
NUM_CLK
};
> struct imx290_regval {
> u32 reg;
> u32 val;
> };
>
> +/*
> + * Clock configuration for registers INCKSEL1 to INCKSEL6.
> + */
> +struct imx290_clk_cfg {
> + u8 incksel1;
> + u8 incksel2;
> + u8 incksel3;
> + u8 incksel4;
> + u8 incksel5;
> + u8 incksel6;
> +};
> +
> struct imx290_mode {
> u32 width;
> u32 height;
> @@ -173,6 +190,8 @@ struct imx290_mode {
>
> const struct imx290_regval *data;
> u32 data_size;
> +
> + const struct imx290_clk_cfg *clk_cfg;
> };
>
> struct imx290_csi_cfg {
> @@ -191,6 +210,7 @@ struct imx290 {
> struct device *dev;
> struct clk *xclk;
> struct regmap *regmap;
> + u32 xclk_freq;
> u8 nlanes;
> u8 mono;
>
> @@ -219,7 +239,6 @@ static inline struct imx290 *to_imx290(struct
> v4l2_subdev *_sd) */
>
> static const struct imx290_regval imx290_global_init_settings[] = {
> - { IMX290_EXTCK_FREQ, 0x2520 },
> { IMX290_WINWV_OB, 12 },
> { IMX290_WINPH, 0 },
> { IMX290_WINPV, 0 },
> @@ -269,7 +288,16 @@ static const struct imx290_regval
> imx290_global_init_settings[] = { { IMX290_REG_8BIT(0x33b0), 0x50 },
> { IMX290_REG_8BIT(0x33b2), 0x1a },
> { IMX290_REG_8BIT(0x33b3), 0x04 },
> - { IMX290_REG_8BIT(0x3480), 0x49 },
> +};
> +
> +static const struct imx290_regval imx290_37_125mhz_clock[] = {
> + { IMX290_EXTCK_FREQ, 0x2520 },
Please also add defines for these magic numbers.
Best regards,
Alexander
> + { IMX290_INCKSEL7, 0x49 },
> +};
> +
> +static const struct imx290_regval imx290_74_25mhz_clock[] = {
> + { IMX290_EXTCK_FREQ, 0x4a40 },
> + { IMX290_INCKSEL7, 0x92 },
> };
>
> static const struct imx290_regval imx290_1080p_settings[] = {
> @@ -279,12 +307,6 @@ static const struct imx290_regval
> imx290_1080p_settings[] = { { IMX290_OPB_SIZE_V, 10 },
> { IMX290_X_OUT_SIZE, 1920 },
> { IMX290_Y_OUT_SIZE, 1080 },
> - { IMX290_INCKSEL1, 0x18 },
> - { IMX290_INCKSEL2, 0x03 },
> - { IMX290_INCKSEL3, 0x20 },
> - { IMX290_INCKSEL4, 0x01 },
> - { IMX290_INCKSEL5, 0x1a },
> - { IMX290_INCKSEL6, 0x1a },
> };
>
> static const struct imx290_regval imx290_720p_settings[] = {
> @@ -294,12 +316,6 @@ static const struct imx290_regval
> imx290_720p_settings[] = { { IMX290_OPB_SIZE_V, 4 },
> { IMX290_X_OUT_SIZE, 1280 },
> { IMX290_Y_OUT_SIZE, 720 },
> - { IMX290_INCKSEL1, 0x20 },
> - { IMX290_INCKSEL2, 0x00 },
> - { IMX290_INCKSEL3, 0x20 },
> - { IMX290_INCKSEL4, 0x01 },
> - { IMX290_INCKSEL5, 0x1a },
> - { IMX290_INCKSEL6, 0x1a },
> };
>
> static const struct imx290_regval imx290_10bit_settings[] = {
> @@ -405,6 +421,48 @@ static inline int imx290_link_freqs_num(const struct
> imx290 *imx290) return ARRAY_SIZE(imx290_link_freq_4lanes);
> }
>
> +static const struct imx290_clk_cfg imx290_1080p_clock_config[NUM_CLK] = {
> + [CLK_37_125] = {
> + /* 37.125MHz clock config */
> + .incksel1 = 0x18,
> + .incksel2 = 0x03,
> + .incksel3 = 0x20,
> + .incksel4 = 0x01,
> + .incksel5 = 0x1a,
> + .incksel6 = 0x1a,
> + },
> + [CLK_74_25] = {
> + /* 74.25MHz clock config */
> + .incksel1 = 0x0c,
> + .incksel2 = 0x03,
> + .incksel3 = 0x10,
> + .incksel4 = 0x01,
> + .incksel5 = 0x1b,
> + .incksel6 = 0x1b,
> + },
> +};
> +
> +static const struct imx290_clk_cfg imx290_720p_clock_config[NUM_CLK] = {
> + [CLK_37_125] = {
> + /* 37.125MHz clock config */
> + .incksel1 = 0x20,
> + .incksel2 = 0x00,
> + .incksel3 = 0x20,
> + .incksel4 = 0x01,
> + .incksel5 = 0x1a,
> + .incksel6 = 0x1a,
> + },
> + [CLK_74_25] = {
> + /* 74.25MHz clock config */
> + .incksel1 = 0x10,
> + .incksel2 = 0x00,
> + .incksel3 = 0x10,
> + .incksel4 = 0x01,
> + .incksel5 = 0x1b,
> + .incksel6 = 0x1b,
> + },
> +};
> +
> /* Mode configs */
> static const struct imx290_mode imx290_modes_2lanes[] = {
> {
> @@ -415,6 +473,7 @@ static const struct imx290_mode imx290_modes_2lanes[] =
> { .link_freq_index = FREQ_INDEX_1080P,
> .data = imx290_1080p_settings,
> .data_size = ARRAY_SIZE(imx290_1080p_settings),
> + .clk_cfg = imx290_1080p_clock_config,
> },
> {
> .width = 1280,
> @@ -424,6 +483,7 @@ static const struct imx290_mode imx290_modes_2lanes[] =
> { .link_freq_index = FREQ_INDEX_720P,
> .data = imx290_720p_settings,
> .data_size = ARRAY_SIZE(imx290_720p_settings),
> + .clk_cfg = imx290_720p_clock_config,
> },
> };
>
> @@ -436,6 +496,7 @@ static const struct imx290_mode imx290_modes_4lanes[] =
> { .link_freq_index = FREQ_INDEX_1080P,
> .data = imx290_1080p_settings,
> .data_size = ARRAY_SIZE(imx290_1080p_settings),
> + .clk_cfg = imx290_1080p_clock_config,
> },
> {
> .width = 1280,
> @@ -445,6 +506,7 @@ static const struct imx290_mode imx290_modes_4lanes[] =
> { .link_freq_index = FREQ_INDEX_720P,
> .data = imx290_720p_settings,
> .data_size = ARRAY_SIZE(imx290_720p_settings),
> + .clk_cfg = imx290_720p_clock_config,
> },
> };
>
> @@ -563,6 +625,23 @@ static int imx290_set_register_array(struct imx290
> *imx290, return 0;
> }
>
> +static int imx290_set_clock(struct imx290 *imx290)
> +{
> + int clk_idx = (imx290->xclk_freq == 37125000) ? 0 : 1;
> + const struct imx290_mode *mode = imx290->current_mode;
> + const struct imx290_clk_cfg *clk_cfg = &mode->clk_cfg[clk_idx];
> + int ret = 0;
> +
> + imx290_write(imx290, IMX290_INCKSEL1, clk_cfg->incksel1, &ret);
> + imx290_write(imx290, IMX290_INCKSEL2, clk_cfg->incksel2, &ret);
> + imx290_write(imx290, IMX290_INCKSEL3, clk_cfg->incksel3, &ret);
> + imx290_write(imx290, IMX290_INCKSEL4, clk_cfg->incksel4, &ret);
> + imx290_write(imx290, IMX290_INCKSEL5, clk_cfg->incksel5, &ret);
> + imx290_write(imx290, IMX290_INCKSEL6, clk_cfg->incksel6, &ret);
> +
> + return ret;
> +}
> +
> static int imx290_set_data_lanes(struct imx290 *imx290)
> {
> int ret = 0;
> @@ -863,6 +942,13 @@ static int imx290_start_streaming(struct imx290
> *imx290, return ret;
> }
>
> + /* Set clock parameters based on mode and xclk */
> + ret = imx290_set_clock(imx290);
> + if (ret < 0) {
> + dev_err(imx290->dev, "Could not set clocks\n");
> + return ret;
> + }
> +
> /* Set data lane count */
> ret = imx290_set_data_lanes(imx290);
> if (ret < 0) {
> @@ -1259,14 +1345,14 @@ static int imx290_init_clk(struct imx290 *imx290)
> int ret;
>
> ret = fwnode_property_read_u32(dev_fwnode(imx290->dev),
> - "clock-frequency", &xclk_freq);
> + "clock-frequency", &imx290-
>xclk_freq);
> if (ret) {
> dev_err(imx290->dev, "Could not get xclk frequency\n");
> return ret;
> }
>
> - /* external clock must be 37.125 MHz */
> - if (xclk_freq != 37125000) {
> + /* external clock must be 37.125 MHz or 74.25MHz */
> + if (imx290->xclk_freq != 37125000 && imx290->xclk_freq != 74250000)
{
> dev_err(imx290->dev, "External clock frequency %u is not
supported\n",
> xclk_freq);
> return -EINVAL;
next prev parent reply other threads:[~2023-02-03 7:44 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-31 19:20 [PATCH 00/11] imx290: Minor fixes, support for alternate INCK, and more ctrls Dave Stevenson
2023-01-31 19:20 ` [PATCH 01/11] media: i2c: imx290: Match kernel coding style on whitespace Dave Stevenson
2023-02-02 0:21 ` Laurent Pinchart
2023-01-31 19:20 ` [PATCH 02/11] media: i2c: imx290: Set the colorspace fields in the format Dave Stevenson
2023-02-02 0:29 ` Laurent Pinchart
2023-01-31 19:20 ` [PATCH 03/11] media: i2c: imx290: Add V4L2_SUBDEV_FL_HAS_EVENTS and subscribe hooks Dave Stevenson
2023-02-02 0:29 ` Laurent Pinchart
2023-02-03 8:54 ` Alexander Stein
2023-01-31 19:20 ` [PATCH 04/11] media: i2c: imx290: Fix the pixel rate at 148.5Mpix/s Dave Stevenson
2023-02-02 8:13 ` Laurent Pinchart
2023-01-31 19:20 ` [PATCH 05/11] media: i2c: imx290: Support 60fps in 2 lane operation Dave Stevenson
2023-02-02 10:50 ` Laurent Pinchart
2023-02-03 8:50 ` Alexander Stein
2023-01-31 19:20 ` [PATCH 06/11] media: i2c: imx290: Use CSI timings as per datasheet Dave Stevenson
2023-02-02 14:36 ` Laurent Pinchart
2023-02-03 8:04 ` Alexander Stein
2023-02-03 9:09 ` Dave Stevenson
2023-01-31 19:20 ` [PATCH 07/11] media: i2c: imx290: Convert V4L2_CID_HBLANK to read/write Dave Stevenson
2023-02-02 14:58 ` Laurent Pinchart
2023-02-02 15:48 ` Dave Stevenson
2023-02-02 22:14 ` Laurent Pinchart
2023-02-03 7:19 ` Alexander Stein
2023-02-03 8:05 ` Dave Stevenson
2023-02-03 8:39 ` Alexander Stein
2023-01-31 19:20 ` [PATCH 08/11] media: i2c: imx290: Convert V4L2_CID_VBLANK " Dave Stevenson
2023-02-02 15:40 ` Alexander Stein
2023-02-02 16:04 ` Dave Stevenson
2023-02-02 17:42 ` Dave Stevenson
2023-02-03 7:16 ` Alexander Stein
2023-02-02 16:01 ` Laurent Pinchart
2023-01-31 19:20 ` [PATCH 09/11] media: i2c: imx290: Remove duplicated write to IMX290_CTRL_07 Dave Stevenson
2023-02-02 16:04 ` Laurent Pinchart
2023-02-03 7:45 ` Alexander Stein
2023-01-31 19:20 ` [PATCH 10/11] media: i2c: imx290: Add support for 74.25MHz external clock Dave Stevenson
2023-02-02 22:03 ` Laurent Pinchart
2023-02-03 17:04 ` Dave Stevenson
2023-02-03 18:04 ` Laurent Pinchart
2023-02-03 7:44 ` Alexander Stein [this message]
2023-02-03 8:59 ` Laurent Pinchart
2023-02-03 9:30 ` Dave Stevenson
2023-01-31 19:20 ` [PATCH 11/11] media: i2c: imx290: Add support for H & V Flips Dave Stevenson
2023-02-02 22:10 ` Laurent Pinchart
2023-02-03 10:21 ` Dave Stevenson
2023-02-03 18:01 ` Laurent Pinchart
2023-02-03 7:35 ` Alexander Stein
2023-02-03 7:57 ` Dave Stevenson
2023-02-03 8:33 ` Alexander Stein
2023-02-03 9:47 ` Dave Stevenson
2023-02-02 22:16 ` [PATCH 00/11] imx290: Minor fixes, support for alternate INCK, and more ctrls Laurent Pinchart
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1890472.taCxCBeP46@steina-w \
--to=alexander.stein@ew.tq-group.com \
--cc=dave.stevenson@raspberrypi.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=mani@kernel.org \
--cc=mchehab@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox