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@xxxxxxxxxxxxxxxx>
> ---
> 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);
>
--
Kind regards,
Sakari Ailus
