[v5,3/9] media: i2c: ov5670: Probe clocks with OF
Commit Message
Add support for probing the main system clock using the common clock
framework and its OF bindings.
Maintain ACPI compatibility by falling back to parse 'clock-frequency'.
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
drivers/media/i2c/ov5670.c | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
Comments
Hi Jacopo,
On Thu, Jan 26, 2023 at 01:46:26PM +0100, Jacopo Mondi wrote:
> Add support for probing the main system clock using the common clock
> framework and its OF bindings.
The OF bindings are actually not related to this patch directly. Only
indirectly. It's all about CCF.
>
> Maintain ACPI compatibility by falling back to parse 'clock-frequency'.
>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
> drivers/media/i2c/ov5670.c | 25 +++++++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
> index 07a396c8ab68..c8beb2bc3d0f 100644
> --- a/drivers/media/i2c/ov5670.c
> +++ b/drivers/media/i2c/ov5670.c
> @@ -2,6 +2,7 @@
> // Copyright (c) 2017 Intel Corporation.
>
> #include <linux/acpi.h>
> +#include <linux/clk.h>
> #include <linux/i2c.h>
> #include <linux/mod_devicetable.h>
> #include <linux/module.h>
> @@ -2475,13 +2476,10 @@ static int ov5670_probe(struct i2c_client *client)
> struct ov5670 *ov5670;
> const char *err_msg;
> u32 input_clk = 0;
> + struct clk *clk;
> bool full_power;
> int ret;
>
> - device_property_read_u32(&client->dev, "clock-frequency", &input_clk);
> - if (input_clk != 19200000)
> - return -EINVAL;
> -
> ov5670 = devm_kzalloc(&client->dev, sizeof(*ov5670), GFP_KERNEL);
> if (!ov5670) {
> ret = -ENOMEM;
> @@ -2489,6 +2487,25 @@ static int ov5670_probe(struct i2c_client *client)
> goto error_print;
> }
>
> + /* OF uses the common clock framework, ACPI uses "clock-frequency". */
> + if (is_of_node(dev_fwnode(&client->dev))) {
This is not a great way to test whether you're expecting a clock: you might
have one in an ACPI system as well.
See e.g. what CCS does: for -ENOENT (clock not found) it relies on
clock-frequency property. Could you do the same here?
> + clk = devm_clk_get(&client->dev, NULL);
> + if (IS_ERR(clk))
> + return dev_err_probe(&client->dev, PTR_ERR(clk),
> + "error getting clock\n");
> +
> + input_clk = clk_get_rate(clk);
> + } else {
> + device_property_read_u32(&client->dev, "clock-frequency",
> + &input_clk);
> + }
> +
> + if (input_clk != 19200000) {
> + dev_err(&client->dev,
> + "Unsupported clock frequency %u\n", input_clk);
> + return -EINVAL;
> + }
> +
> /* Initialize subdev */
> v4l2_i2c_subdev_init(&ov5670->sd, client, &ov5670_subdev_ops);
>
@@ -2,6 +2,7 @@
// Copyright (c) 2017 Intel Corporation.
#include <linux/acpi.h>
+#include <linux/clk.h>
#include <linux/i2c.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
@@ -2475,13 +2476,10 @@ static int ov5670_probe(struct i2c_client *client)
struct ov5670 *ov5670;
const char *err_msg;
u32 input_clk = 0;
+ struct clk *clk;
bool full_power;
int ret;
- device_property_read_u32(&client->dev, "clock-frequency", &input_clk);
- if (input_clk != 19200000)
- return -EINVAL;
-
ov5670 = devm_kzalloc(&client->dev, sizeof(*ov5670), GFP_KERNEL);
if (!ov5670) {
ret = -ENOMEM;
@@ -2489,6 +2487,25 @@ static int ov5670_probe(struct i2c_client *client)
goto error_print;
}
+ /* OF uses the common clock framework, ACPI uses "clock-frequency". */
+ if (is_of_node(dev_fwnode(&client->dev))) {
+ clk = devm_clk_get(&client->dev, NULL);
+ if (IS_ERR(clk))
+ return dev_err_probe(&client->dev, PTR_ERR(clk),
+ "error getting clock\n");
+
+ input_clk = clk_get_rate(clk);
+ } else {
+ device_property_read_u32(&client->dev, "clock-frequency",
+ &input_clk);
+ }
+
+ if (input_clk != 19200000) {
+ dev_err(&client->dev,
+ "Unsupported clock frequency %u\n", input_clk);
+ return -EINVAL;
+ }
+
/* Initialize subdev */
v4l2_i2c_subdev_init(&ov5670->sd, client, &ov5670_subdev_ops);