Hi Robert,

On Fri, Mar 27, 2020 at 11:32:29AM +0100, Robert Foss wrote:

On Thu, 26 Mar 2020 at 15:47, Sakari Ailus [off-list ref] wrote:

quoted

Hi Robert,

On Thu, Mar 26, 2020 at 12:56:37PM +0100, Robert Foss wrote:
...

quoted

quoted

quoted

+static int __ov8856_power_on(struct ov8856 *ov8856)
+{
+     struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd);
+     int ret;
+
+     ret = clk_prepare_enable(ov8856->xvclk);
+     if (ret < 0) {
+             dev_err(&client->dev, "failed to enable xvclk\n");
+             return ret;
+     }
+
+     gpiod_set_value_cansleep(ov8856->reset_gpio, GPIOD_OUT_HIGH);
+
+     ret = regulator_bulk_enable(ARRAY_SIZE(ov8856_supply_names),
+                                 ov8856->supplies);
+     if (ret < 0) {
+             dev_err(&client->dev, "failed to enable regulators\n");
+             goto disable_clk;
+     }
+
+     gpiod_set_value_cansleep(ov8856->reset_gpio, GPIOD_OUT_LOW);
+
+     usleep_range(1500, 1800);

I think you could omit the delay on ACPI based systems. Or just bail out
early in that case.

I'll add a check for reset_gpio being NULL, and skip the sleep for that case.

There could also be a regulator but no GPIO.

I think if you don't have either, then certainly there's no need for a
delay.

Removing the delay if no action is taken makes sense, but I'm not sure
how best to do it.
If there are no regulators dummy ones are created automatically, which
makes distinguishing between a little bit cumbersome. The regulator
structs could of course all be inspected, and if all are dummy ones,
the delay could be skipped. But is there a neater way of doing this?
Manually inspecting the regs strikes me as a bit inelegant.

I guess the cleanest, easy way to make this right, albeit slightly
unoptimal in very rare cases where you have none of the above resources in
a DT system, is to bail out if you're running on an ACPI based system.

I.e. checking for e.g. is_acpi_node(dev->fwnode).

-- 
Sakari Ailus

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel