Re: [PATCH 14/28] media: ov2680: Add support for more clk setups

[Andy Shevchenko <andy@kernel.org>]

On Wed, Jun 07, 2023 at 06:46:58PM +0200, Hans de Goede wrote:
> On ACPI systems the following 2 scenarios are possible:
> 
> 1. The xvclk is fully controlled by ACPI powermanagement, so there
>    is no "xvclk" for the driver to get (since it is abstracted away).
>    In this case there will be a "clock-frequency" device property
>    to tell the driver the xvclk rate.
> 
> 2. There is a xvclk modelled in the clk framework for the driver,
>    but the clk-generator may not be set to the right frequency
>    yet. In this case there will also be a "clock-frequency" device
>    property and the driver is expected to set the rate of the xvclk
>    through this frequency through the clk framework.
> 
> Handle both these scenarios by switching to devm_clk_get_optional()
> and checking for a "clock-frequency" device property.
> 
> This is modelled after how this same issues was fixed for the ov8865 in

this --> the

> commit 73dcffeb2ff9 ("media: i2c: Support 19.2MHz input clock in ov8865").

...

> +	/*
> +	 * We could have either a 24MHz or 19.2MHz clock rate from either dt or

DT

> +	 * ACPI...but we also need to support the weird IPU3 case which will

ACPI... but

> +	 * have an external clock AND a clock-frequency property. Check for the
> +	 * clock-frequency property and if found, set that rate if we managed
> +	 * to acquire a clock. This should cover the ACPI case. If the system
> +	 * uses devicetree then the configured rate should already be set, so
> +	 * we can just read it.
> +	 */
> +	ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency", &rate);

	if (ret && !sensor->xvclk)
		return dev_err_probe(dev, ret, "invalid clock config\n");

('else' is redundant)

> +	if (!ret && sensor->xvclk) {
> +		ret = clk_set_rate(sensor->xvclk, rate);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "failed to set clock rate\n");
> +	} else if (ret && !sensor->xvclk) {
> +		return dev_err_probe(dev, ret, "invalid clock config\n");
> +	}

...

> +	sensor->xvclk_freq = rate ? rate : clk_get_rate(sensor->xvclk);

Elvis can be used:

	sensor->xvclk_freq = rate ?: clk_get_rate(sensor->xvclk);

-- 
With Best Regards,
Andy Shevchenko