Hi Vladimir,

Thank you for the patch,

On Fri, Jul 18, 2025 at 06:27:54PM +0300, Vladimir Zapolskiy wrote:

[..]

quoted

quoted

quoted

+
+       ov6211->xvclk = devm_clk_get_optional(&client->dev, NULL);
+       if (IS_ERR(ov6211->xvclk)) {
+               ret = PTR_ERR(ov6211->xvclk);
+               dev_err(&client->dev, "failed to get XVCLK clock: %d\n", ret);
+               return ret;
+       }
+
+       freq = clk_get_rate(ov6211->xvclk);
+       if (freq && freq != OV6211_MCLK_FREQ_24MHZ)
+               return dev_err_probe(&client->dev, -EINVAL,
+                               "XVCLK clock frequency %lu is not supported\n",
+                               freq);

This would be nicer to make use of the cleanups that have just been
implemented in
https://lore.kernel.org/linux-media/cover.1750942967.git.mehdi.djait@linux.intel.com/ (local)
and
https://lore.kernel.org/linux-media/20250710174808.5361-1-laurent.pinchart@ideasonboard.com/T/ (local)

Actually I've already checked it before publishing the code, as a summary:

1. to my understanding the introduced API is still under review, I didn't
find it in media/master or linux-next,

It has already been reviewed but yes still not in the media tree.
Too late for 6.17 but it will be in the media tree soon.

2. the only needed change to get support of the new helper is to replace
the single line of devm_clk_get_optional() with devm_v4l2_sensor_clk_get(),
no more than that,

Correct.

3. the internal complexity of devm_v4l2_sensor_clk_get() seems excessive
right over here, what's worse I can not test devm_v4l2_sensor_clk_get()
in this driver on any ACPI platform...

You don't need to test it on a ACPI-based platform to use the helper, if
it works for your DT-based platform that's enough.

To sum up and to minimize the overall complexity, I'd rather prefer to
stick to devm_clk_get_optional() at the moment, the switch to the new
proposed API can be done, when it's ready.

-- 
Best wishes,
Vladimir

--
Kind Regards
Mehdi Djait