From: André Apitzsch <git@apitzsch.eu>
Replace the hard coded external clock frequency by the one read from
device tree.
Signed-off-by: André Apitzsch <git@apitzsch.eu>
---
drivers/media/i2c/imx214.c | 99 ++++++++++++++++++++++++++++++++++++----------
1 file changed, 79 insertions(+), 20 deletions(-)
diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
index 53b6b427f263a8ad7e3a0d1f711ece234601100e..c3d55259d6fd1c4ca96f52833864bdfe6bedf13a 100644
--- a/drivers/media/i2c/imx214.c
+++ b/drivers/media/i2c/imx214.c
@@ -30,11 +30,10 @@
#define IMX214_REG_FAST_STANDBY_CTRL CCI_REG8(0x0106)
-#define IMX214_DEFAULT_CLK_FREQ 24000000
-#define IMX214_DEFAULT_LINK_FREQ 600000000
+#define IMX214_CLK_FREQ_24000KHZ 24000000
+#define IMX214_LINK_FREQ_600MHZ 600000000
/* Keep wrong link frequency for backward compatibility */
#define IMX214_DEFAULT_LINK_FREQ_LEGACY 480000000
-#define IMX214_DEFAULT_PIXEL_RATE ((IMX214_DEFAULT_LINK_FREQ * 8LL) / 10)
#define IMX214_FPS 30
/* V-TIMING internal */
@@ -233,6 +232,22 @@ static const char * const imx214_supply_name[] = {
#define IMX214_NUM_SUPPLIES ARRAY_SIZE(imx214_supply_name)
+static const s64 link_freq[] = {
+ IMX214_LINK_FREQ_600MHZ,
+};
+
+struct imx214_clk_params {
+ u32 clk_freq;
+ u64 link_freq;
+};
+
+static const struct imx214_clk_params imx214_clk_params[] = {
+ {
+ .clk_freq = IMX214_CLK_FREQ_24000KHZ,
+ .link_freq = IMX214_LINK_FREQ_600MHZ,
+ },
+};
+
/*
* The supported formats.
* This table MUST contain 4 entries per format, to cover the various flip
@@ -270,6 +285,8 @@ struct imx214 {
struct clk *xclk;
struct regmap *regmap;
+ const struct imx214_clk_params *clk_params;
+
struct v4l2_subdev sd;
struct media_pad pad;
@@ -794,7 +811,7 @@ static int imx214_set_clock(struct imx214 *imx214)
cci_write(imx214->regmap, IMX214_REG_PLL_MULT_DRIV,
IMX214_PLL_SINGLE, &ret);
cci_write(imx214->regmap, IMX214_REG_EXCK_FREQ,
- IMX214_EXCK_FREQ(IMX214_DEFAULT_CLK_FREQ / 1000000), &ret);
+ IMX214_EXCK_FREQ(imx214->clk_params->clk_freq / 1000000), &ret);
return ret;
}
@@ -899,9 +916,6 @@ static const struct v4l2_ctrl_ops imx214_ctrl_ops = {
static int imx214_ctrls_init(struct imx214 *imx214)
{
- static const s64 link_freq[] = {
- IMX214_DEFAULT_LINK_FREQ
- };
static const struct v4l2_area unit_size = {
.width = 1120,
.height = 1120,
@@ -910,6 +924,7 @@ static int imx214_ctrls_init(struct imx214 *imx214)
struct v4l2_fwnode_device_properties props;
struct v4l2_ctrl_handler *ctrl_hdlr;
int exposure_max, exposure_def;
+ int pixel_rate;
int hblank;
int i, ret;
@@ -922,15 +937,25 @@ static int imx214_ctrls_init(struct imx214 *imx214)
if (ret)
return ret;
+ pixel_rate = imx214->clk_params->link_freq * 8 / 10;
imx214->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, NULL,
V4L2_CID_PIXEL_RATE, 0,
- IMX214_DEFAULT_PIXEL_RATE, 1,
- IMX214_DEFAULT_PIXEL_RATE);
+ pixel_rate, 1, pixel_rate);
+
+ for (i = 0; i < ARRAY_SIZE(link_freq); ++i) {
+ if (imx214->clk_params->link_freq == link_freq[i])
+ break;
+ }
+ if (i == ARRAY_SIZE(link_freq)) {
+ dev_err(imx214->dev, "link frequency %lld not supported\n",
+ imx214->clk_params->link_freq);
+ return -EINVAL;
+ }
imx214->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr, NULL,
V4L2_CID_LINK_FREQ,
ARRAY_SIZE(link_freq) - 1,
- 0, link_freq);
+ i, link_freq);
if (imx214->link_freq)
imx214->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
@@ -1047,7 +1072,7 @@ static int imx214_start_streaming(struct imx214 *imx214)
return ret;
}
- link_bit_rate = IMX214_LINK_BIT_RATE(IMX214_DEFAULT_CLK_FREQ);
+ link_bit_rate = IMX214_LINK_BIT_RATE(imx214->clk_params->clk_freq);
ret = cci_write(imx214->regmap, IMX214_REG_REQ_LINK_BIT_RATE,
IMX214_LINK_BIT_RATE_MBPS(link_bit_rate), NULL);
if (ret) {
@@ -1238,7 +1263,20 @@ static int imx214_identify_module(struct imx214 *imx214)
return 0;
}
-static int imx214_parse_fwnode(struct device *dev)
+static int imx214_has_link_freq(struct imx214 *imx214, u64 link_frequency)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(imx214_clk_params); ++i) {
+ if (imx214_clk_params[i].link_freq == link_frequency) {
+ imx214->clk_params = &imx214_clk_params[i];
+ break;
+ }
+ }
+ return (i < ARRAY_SIZE(imx214_clk_params));
+}
+
+static int imx214_parse_fwnode(struct device *dev, struct imx214 *imx214)
{
struct fwnode_handle *endpoint;
struct v4l2_fwnode_endpoint bus_cfg = {
@@ -1268,13 +1306,14 @@ static int imx214_parse_fwnode(struct device *dev)
dev_warn(dev, "Only one link-frequency supported, please review your DT. Continuing anyway\n");
for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) {
- if (bus_cfg.link_frequencies[i] == IMX214_DEFAULT_LINK_FREQ)
+ if (imx214_has_link_freq(imx214, bus_cfg.link_frequencies[i]))
break;
if (bus_cfg.link_frequencies[i] ==
IMX214_DEFAULT_LINK_FREQ_LEGACY) {
dev_warn(dev,
"link-frequencies %d not supported, please review your DT. Continuing anyway\n",
- IMX214_DEFAULT_LINK_FREQ);
+ IMX214_LINK_FREQ_600MHZ);
+ imx214->clk_params = &imx214_clk_params[1];
break;
}
}
@@ -1282,7 +1321,7 @@ static int imx214_parse_fwnode(struct device *dev)
if (i == bus_cfg.nr_of_link_frequencies)
ret = dev_err_probe(dev, -EINVAL,
"link-frequencies %d not supported, please review your DT\n",
- IMX214_DEFAULT_LINK_FREQ);
+ IMX214_LINK_FREQ_600MHZ);
done:
v4l2_fwnode_endpoint_free(&bus_cfg);
@@ -1294,16 +1333,17 @@ static int imx214_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
struct imx214 *imx214;
+ u32 xclk_freq;
int ret;
- ret = imx214_parse_fwnode(dev);
- if (ret)
- return ret;
-
imx214 = devm_kzalloc(dev, sizeof(*imx214), GFP_KERNEL);
if (!imx214)
return -ENOMEM;
+ ret = imx214_parse_fwnode(dev, imx214);
+ if (ret)
+ return ret;
+
imx214->dev = dev;
imx214->xclk = devm_clk_get(dev, NULL);
@@ -1311,7 +1351,26 @@ static int imx214_probe(struct i2c_client *client)
return dev_err_probe(dev, PTR_ERR(imx214->xclk),
"failed to get xclk\n");
- ret = clk_set_rate(imx214->xclk, IMX214_DEFAULT_CLK_FREQ);
+ ret = device_property_read_u32(dev, "clock-frequency", &xclk_freq);
+ if (ret) {
+ dev_warn(dev,
+ "clock-frequency not set, please review your DT. Fallback to default\n");
+ xclk_freq = IMX214_CLK_FREQ_24000KHZ;
+ }
+
+ switch (xclk_freq) {
+ case IMX214_CLK_FREQ_24000KHZ:
+ if (imx214->clk_params->clk_freq != xclk_freq)
+ return dev_err_probe(imx214->dev, -EINVAL,
+ "combination of clock and link frequency is not supported\n");
+ break;
+ default:
+ return dev_err_probe(imx214->dev, -EINVAL,
+ "external clock frequency %u is not supported\n",
+ xclk_freq);
+ }
+
+ ret = clk_set_rate(imx214->xclk, xclk_freq);
if (ret)
return dev_err_probe(dev, ret,
"failed to set xclk frequency\n");
--
2.48.1
On Sat, Mar 8, 2025 at 10:48 PM André Apitzsch via B4 Relay
<devnull+git.apitzsch.eu@kernel.org> wrote:
>
> From: André Apitzsch <git@apitzsch.eu>
>
> Replace the hard coded external clock frequency by the one read from
> device tree.
>
> Signed-off-by: André Apitzsch <git@apitzsch.eu>
> ---
> drivers/media/i2c/imx214.c | 99 ++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 79 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
> index 53b6b427f263a8ad7e3a0d1f711ece234601100e..c3d55259d6fd1c4ca96f52833864bdfe6bedf13a 100644
> --- a/drivers/media/i2c/imx214.c
> +++ b/drivers/media/i2c/imx214.c
> @@ -30,11 +30,10 @@
>
> #define IMX214_REG_FAST_STANDBY_CTRL CCI_REG8(0x0106)
>
> -#define IMX214_DEFAULT_CLK_FREQ 24000000
> -#define IMX214_DEFAULT_LINK_FREQ 600000000
> +#define IMX214_CLK_FREQ_24000KHZ 24000000
> +#define IMX214_LINK_FREQ_600MHZ 600000000
> /* Keep wrong link frequency for backward compatibility */
> #define IMX214_DEFAULT_LINK_FREQ_LEGACY 480000000
> -#define IMX214_DEFAULT_PIXEL_RATE ((IMX214_DEFAULT_LINK_FREQ * 8LL) / 10)
> #define IMX214_FPS 30
>
> /* V-TIMING internal */
> @@ -233,6 +232,22 @@ static const char * const imx214_supply_name[] = {
>
> #define IMX214_NUM_SUPPLIES ARRAY_SIZE(imx214_supply_name)
>
> +static const s64 link_freq[] = {
> + IMX214_LINK_FREQ_600MHZ,
> +};
Do we need to move this?
> +
> +struct imx214_clk_params {
> + u32 clk_freq;
> + u64 link_freq;
> +};
> +
> +static const struct imx214_clk_params imx214_clk_params[] = {
> + {
> + .clk_freq = IMX214_CLK_FREQ_24000KHZ,
> + .link_freq = IMX214_LINK_FREQ_600MHZ,
> + },
> +};
> +
> /*
> * The supported formats.
> * This table MUST contain 4 entries per format, to cover the various flip
> @@ -270,6 +285,8 @@ struct imx214 {
> struct clk *xclk;
> struct regmap *regmap;
>
> + const struct imx214_clk_params *clk_params;
> +
> struct v4l2_subdev sd;
> struct media_pad pad;
>
> @@ -794,7 +811,7 @@ static int imx214_set_clock(struct imx214 *imx214)
> cci_write(imx214->regmap, IMX214_REG_PLL_MULT_DRIV,
> IMX214_PLL_SINGLE, &ret);
> cci_write(imx214->regmap, IMX214_REG_EXCK_FREQ,
> - IMX214_EXCK_FREQ(IMX214_DEFAULT_CLK_FREQ / 1000000), &ret);
> + IMX214_EXCK_FREQ(imx214->clk_params->clk_freq / 1000000), &ret);
>
> return ret;
> }
> @@ -899,9 +916,6 @@ static const struct v4l2_ctrl_ops imx214_ctrl_ops = {
>
> static int imx214_ctrls_init(struct imx214 *imx214)
> {
> - static const s64 link_freq[] = {
> - IMX214_DEFAULT_LINK_FREQ
> - };
> static const struct v4l2_area unit_size = {
> .width = 1120,
> .height = 1120,
> @@ -910,6 +924,7 @@ static int imx214_ctrls_init(struct imx214 *imx214)
> struct v4l2_fwnode_device_properties props;
> struct v4l2_ctrl_handler *ctrl_hdlr;
> int exposure_max, exposure_def;
> + int pixel_rate;
> int hblank;
> int i, ret;
>
> @@ -922,15 +937,25 @@ static int imx214_ctrls_init(struct imx214 *imx214)
> if (ret)
> return ret;
>
> + pixel_rate = imx214->clk_params->link_freq * 8 / 10;
> imx214->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, NULL,
> V4L2_CID_PIXEL_RATE, 0,
> - IMX214_DEFAULT_PIXEL_RATE, 1,
> - IMX214_DEFAULT_PIXEL_RATE);
> + pixel_rate, 1, pixel_rate);
> +
> + for (i = 0; i < ARRAY_SIZE(link_freq); ++i) {
> + if (imx214->clk_params->link_freq == link_freq[i])
> + break;
> + }
> + if (i == ARRAY_SIZE(link_freq)) {
> + dev_err(imx214->dev, "link frequency %lld not supported\n",
> + imx214->clk_params->link_freq);
> + return -EINVAL;
> + }
>
> imx214->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr, NULL,
> V4L2_CID_LINK_FREQ,
> ARRAY_SIZE(link_freq) - 1,
> - 0, link_freq);
> + i, link_freq);
> if (imx214->link_freq)
> imx214->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>
> @@ -1047,7 +1072,7 @@ static int imx214_start_streaming(struct imx214 *imx214)
> return ret;
> }
>
> - link_bit_rate = IMX214_LINK_BIT_RATE(IMX214_DEFAULT_CLK_FREQ);
> + link_bit_rate = IMX214_LINK_BIT_RATE(imx214->clk_params->clk_freq);
> ret = cci_write(imx214->regmap, IMX214_REG_REQ_LINK_BIT_RATE,
> IMX214_LINK_BIT_RATE_MBPS(link_bit_rate), NULL);
> if (ret) {
> @@ -1238,7 +1263,20 @@ static int imx214_identify_module(struct imx214 *imx214)
> return 0;
> }
>
> -static int imx214_parse_fwnode(struct device *dev)
> +static int imx214_has_link_freq(struct imx214 *imx214, u64 link_frequency)
static bool imx214_set_clk_params(struct imx214 *imx214, u64 link_frequency)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(imx214_clk_params); ++i) {
> + if (imx214_clk_params[i].link_freq == link_frequency) {
> + imx214->clk_params = &imx214_clk_params[i];
> + break;
return true;
> + }
> + }
> + return (i < ARRAY_SIZE(imx214_clk_params));
return false;
> +}
> +
> +static int imx214_parse_fwnode(struct device *dev, struct imx214 *imx214)
> {
> struct fwnode_handle *endpoint;
> struct v4l2_fwnode_endpoint bus_cfg = {
> @@ -1268,13 +1306,14 @@ static int imx214_parse_fwnode(struct device *dev)
> dev_warn(dev, "Only one link-frequency supported, please review your DT. Continuing anyway\n");
>
> for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) {
> - if (bus_cfg.link_frequencies[i] == IMX214_DEFAULT_LINK_FREQ)
> + if (imx214_has_link_freq(imx214, bus_cfg.link_frequencies[i]))
> break;
> if (bus_cfg.link_frequencies[i] ==
> IMX214_DEFAULT_LINK_FREQ_LEGACY) {
> dev_warn(dev,
> "link-frequencies %d not supported, please review your DT. Continuing anyway\n",
> - IMX214_DEFAULT_LINK_FREQ);
> + IMX214_LINK_FREQ_600MHZ);
> + imx214->clk_params = &imx214_clk_params[1];
I guess you mean:
imx214->clk_params = &imx214_clk_params[0];
> break;
> }
> }
> @@ -1282,7 +1321,7 @@ static int imx214_parse_fwnode(struct device *dev)
> if (i == bus_cfg.nr_of_link_frequencies)
> ret = dev_err_probe(dev, -EINVAL,
> "link-frequencies %d not supported, please review your DT\n",
> - IMX214_DEFAULT_LINK_FREQ);
> + IMX214_LINK_FREQ_600MHZ);
I think this was wrong on the original driver.
it should be
bus_cfg.nr_of_link_frequencies ? bus_cfg.link_frequencies[0] : 0;
>
> done:
> v4l2_fwnode_endpoint_free(&bus_cfg);
> @@ -1294,16 +1333,17 @@ static int imx214_probe(struct i2c_client *client)
> {
> struct device *dev = &client->dev;
> struct imx214 *imx214;
> + u32 xclk_freq;
> int ret;
>
> - ret = imx214_parse_fwnode(dev);
> - if (ret)
> - return ret;
> -
> imx214 = devm_kzalloc(dev, sizeof(*imx214), GFP_KERNEL);
> if (!imx214)
> return -ENOMEM;
>
> + ret = imx214_parse_fwnode(dev, imx214);
> + if (ret)
> + return ret;
> +
> imx214->dev = dev;
>
> imx214->xclk = devm_clk_get(dev, NULL);
> @@ -1311,7 +1351,26 @@ static int imx214_probe(struct i2c_client *client)
> return dev_err_probe(dev, PTR_ERR(imx214->xclk),
> "failed to get xclk\n");
>
> - ret = clk_set_rate(imx214->xclk, IMX214_DEFAULT_CLK_FREQ);
> + ret = device_property_read_u32(dev, "clock-frequency", &xclk_freq);
> + if (ret) {
> + dev_warn(dev,
> + "clock-frequency not set, please review your DT. Fallback to default\n");
> + xclk_freq = IMX214_CLK_FREQ_24000KHZ;
> + }
> +
> + switch (xclk_freq) {
> + case IMX214_CLK_FREQ_24000KHZ:
> + if (imx214->clk_params->clk_freq != xclk_freq)
> + return dev_err_probe(imx214->dev, -EINVAL,
> + "combination of clock and link frequency is not supported\n");
> + break;
> + default:
> + return dev_err_probe(imx214->dev, -EINVAL,
> + "external clock frequency %u is not supported\n",
> + xclk_freq);
> + }
> +
> + ret = clk_set_rate(imx214->xclk, xclk_freq);
> if (ret)
> return dev_err_probe(dev, ret,
> "failed to set xclk frequency\n");
>
> --
> 2.48.1
>
>
© 2016 - 2025 Red Hat, Inc.