On Fri, Mar 13, 2020 at 12:03:49PM +0100, Robert Foss wrote:
> Add devicetree match table, and enable ov8856_probe()
> to initialize power, clocks and reset pins.
Thanks for an update.
My comments below.
...
> + ov8856->xvclk = devm_clk_get(&client->dev, "xvclk");
In many frameworks we have '_optional' variants of API. Please use it instead
of open coded approach.
> + if (PTR_ERR(ov8856->xvclk) == -ENOENT) {
> + dev_info(&client->dev, "xvclk clock not defined, continuing...\n");
> + ov8856->xvclk = NULL;
> + } else if (IS_ERR(ov8856->xvclk)) {
> + dev_err(&client->dev, "could not get xvclk clock (%ld)\n",
> + PTR_ERR(ov8856->xvclk));
> + return PTR_ERR(ov8856->xvclk);
> + }
> +
> + ret = clk_set_rate(ov8856->xvclk, OV8856_XVCLK_24);
> + if (ret < 0) {
> + dev_err(&client->dev, "failed to set xvclk rate (24MHz)\n");
> + return ret;
> + }
> +
> + ov8856->reset_gpio = devm_gpiod_get(&client->dev, "reset",
> + GPIOD_OUT_HIGH);
Same here.
> + if (IS_ERR(ov8856->reset_gpio)) {
> + dev_err(&client->dev, "failed to get reset-gpios\n");
> + return PTR_ERR(ov8856->reset_gpio);
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(ov8856_supply_names); i++)
> + ov8856->supplies[i].supply = ov8856_supply_names[i];
> +
> + ret = devm_regulator_bulk_get(&client->dev,
> + ARRAY_SIZE(ov8856_supply_names),
> + ov8856->supplies);
Luckily regulator framework will create dummy ones if there is none found.
> + if (ret) {
> + dev_warn(&client->dev, "failed to get regulators\n");
> + return ret;
> + }
--
With Best Regards,
Andy Shevchenko
