On 7/18/25 10:07, Krzysztof Kozlowski wrote:

On 17/07/2025 14:40, Vladimir Zapolskiy wrote:

quoted

+
+static int ov6211_power_on(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct ov6211 *ov6211 = to_ov6211(sd);
+	int ret;
+
+	if (ov6211->avdd) {
+		ret = regulator_enable(ov6211->avdd);

You should just use bulk API and simplify all the regulator handling code.

There is no bulk API for optional regulators.

<snip>

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;

syntax is always: return dev_err_probe

Ack.

quoted

+	}
+
+	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);
+
+	ret = ov6211_check_hwcfg(ov6211);
+	if (ret) {
+		dev_err(&client->dev, "failed to check HW configuration: %d",
+			ret);
+		return ret;
+	}
+
+	ov6211->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset",
+						     GPIOD_OUT_HIGH);
+	if (IS_ERR(ov6211->reset_gpio)) {
+		dev_err(&client->dev, "cannot get reset GPIO\n");
+		return PTR_ERR(ov6211->reset_gpio);

syntax is always: return dev_err_probe

Ack.

Thanks for the review.

-- 
Best wishes,
Vladimir