diff mbox series

[v5,3/9] media: i2c: ov5670: Probe clocks with OF

Message ID 20230126124632.45842-4-jacopo.mondi@ideasonboard.com (mailing list archive)
State New, archived
Headers show
Series media: i2c: ov5670: OF support, runtime_pm, regulators | expand

Commit Message

Jacopo Mondi Jan. 26, 2023, 12:46 p.m. UTC
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

Sakari Ailus Jan. 26, 2023, 2:21 p.m. UTC | #1
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);
>
diff mbox series

Patch

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))) {
+		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);