Hi Sakari
On Tue, May 02, 2023 at 03:51:50PM +0300, Sakari Ailus wrote:
> Leftovers from the earlier fix. Fix also the conditions for reading the
> clock-frequency property as well as accessing the clock.
>
> Fixes: 8df08ba4a331 ("media: ov5670: Fix probe on ACPI")
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> ---
> drivers/media/i2c/ov5670.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
> index c5e783a06f06c..5437cf32a7b3a 100644
> --- a/drivers/media/i2c/ov5670.c
> +++ b/drivers/media/i2c/ov5670.c
> @@ -2661,9 +2661,9 @@ static int ov5670_probe(struct i2c_client *client)
> }
>
> ov5670->xvclk = devm_clk_get_optional(&client->dev, NULL);
> - if (!IS_ERR_OR_NULL(ov5670->xvclk))
> + if (!IS_ERR(ov5670->xvclk))
Can't clk_get_optional() return NULL in the case the clock source is not
available ?
Also, if !CONFIG_HAVE_CLK devm_clk_get_optional() returns 0.
I would then keep the _OR_NULL() part
> input_clk = clk_get_rate(ov5670->xvclk);
> - else if (PTR_ERR(ov5670->xvclk) == -ENOENT)
> + else if (!ov5670->xvclk)
While here it's probably correct, we only want to read
"clock-frequency" if xvclk == NULL (when either the clock providere is
not there, or !CONFIG_HAVE_CLK)
> device_property_read_u32(&client->dev, "clock-frequency",
> &input_clk);
> else
Everything else is an error (in example, -EPROBE_DEFER)
> --
> 2.30.2
>
