Hi, On 11-Mar-25 12:40 PM, Hans de Goede wrote: > From: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx> > > The ACPI version of this driver "just works" on dts based systems with a > few extensions to facilitate. > > - Add support for DT based probing > - Add support for taking the part out of reset via a GPIO reset pin > - Add in regulator bulk on/off logic for the power rails. > > Once done this sensor works nicely on a Qualcomm X1E80100 CRD. > > Tested-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx> # x1e80100-crd > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> > Signed-off-by: Hans Verkuil <hverkuil@xxxxxxxxx> Sorry about sending out this old patch from Bryan, clearly that was not my intention. Please ignore. Regards, Hans > --- > drivers/media/i2c/ov08x40.c | 144 +++++++++++++++++++++++++++++++----- > 1 file changed, 127 insertions(+), 17 deletions(-) > > diff --git a/drivers/media/i2c/ov08x40.c b/drivers/media/i2c/ov08x40.c > index 3ab8b51df157..ff17e09a1f96 100644 > --- a/drivers/media/i2c/ov08x40.c > +++ b/drivers/media/i2c/ov08x40.c > @@ -3,10 +3,13 @@ > > #include <asm-generic/unaligned.h> > #include <linux/acpi.h> > +#include <linux/clk.h> > #include <linux/i2c.h> > +#include <linux/gpio/consumer.h> > #include <linux/module.h> > #include <linux/delay.h> > #include <linux/pm_runtime.h> > +#include <linux/regulator/consumer.h> > #include <media/v4l2-ctrls.h> > #include <media/v4l2-device.h> > #include <media/v4l2-fwnode.h> > @@ -1279,6 +1282,12 @@ static const struct ov08x40_mode supported_modes[] = { > }, > }; > > +static const char * const ov08x40_supply_names[] = { > + "dovdd", /* Digital I/O power */ > + "avdd", /* Analog power */ > + "dvdd", /* Digital core power */ > +}; > + > struct ov08x40 { > struct v4l2_subdev sd; > struct media_pad pad; > @@ -1291,6 +1300,10 @@ struct ov08x40 { > struct v4l2_ctrl *hblank; > struct v4l2_ctrl *exposure; > > + struct clk *xvclk; > + struct gpio_desc *reset_gpio; > + struct regulator_bulk_data supplies[ARRAY_SIZE(ov08x40_supply_names)]; > + > /* Current mode */ > const struct ov08x40_mode *cur_mode; > > @@ -1303,6 +1316,61 @@ struct ov08x40 { > > #define to_ov08x40(_sd) container_of(_sd, struct ov08x40, sd) > > +static int ov08x40_power_on(struct device *dev) > +{ > + struct v4l2_subdev *sd = dev_get_drvdata(dev); > + struct ov08x40 *ov08x = to_ov08x40(sd); > + int ret; > + > + if (is_acpi_node(dev_fwnode(dev))) > + return 0; > + > + ret = clk_prepare_enable(ov08x->xvclk); > + if (ret < 0) { > + dev_err(dev, "failed to enable xvclk\n"); > + return ret; > + } > + > + if (ov08x->reset_gpio) { > + gpiod_set_value_cansleep(ov08x->reset_gpio, 1); > + usleep_range(1000, 2000); > + } > + > + ret = regulator_bulk_enable(ARRAY_SIZE(ov08x40_supply_names), > + ov08x->supplies); > + if (ret < 0) { > + dev_err(dev, "failed to enable regulators\n"); > + goto disable_clk; > + } > + > + gpiod_set_value_cansleep(ov08x->reset_gpio, 0); > + usleep_range(1500, 1800); > + > + return 0; > + > +disable_clk: > + gpiod_set_value_cansleep(ov08x->reset_gpio, 1); > + clk_disable_unprepare(ov08x->xvclk); > + > + return ret; > +} > + > +static int ov08x40_power_off(struct device *dev) > +{ > + struct v4l2_subdev *sd = dev_get_drvdata(dev); > + struct ov08x40 *ov08x = to_ov08x40(sd); > + > + if (is_acpi_node(dev_fwnode(dev))) > + return 0; > + > + gpiod_set_value_cansleep(ov08x->reset_gpio, 1); > + regulator_bulk_disable(ARRAY_SIZE(ov08x40_supply_names), > + ov08x->supplies); > + clk_disable_unprepare(ov08x->xvclk); > + > + return 0; > +} > + > /* Read registers up to 4 at a time */ > static int ov08x40_read_reg(struct ov08x40 *ov08x, > u16 reg, u32 len, u32 *val) > @@ -2072,7 +2140,7 @@ static void ov08x40_free_controls(struct ov08x40 *ov08x) > mutex_destroy(&ov08x->mutex); > } > > -static int ov08x40_check_hwcfg(struct device *dev) > +static int ov08x40_check_hwcfg(struct ov08x40 *ov08x, struct device *dev) > { > struct v4l2_fwnode_endpoint bus_cfg = { > .bus_type = V4L2_MBUS_CSI2_DPHY > @@ -2086,11 +2154,36 @@ static int ov08x40_check_hwcfg(struct device *dev) > if (!fwnode) > return -ENXIO; > > - ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency", > - &xvclk_rate); > - if (ret) { > - dev_err(dev, "can't get clock frequency"); > - return ret; > + if (!is_acpi_node(fwnode)) { > + ov08x->xvclk = devm_clk_get(dev, NULL); > + if (IS_ERR(ov08x->xvclk)) { > + dev_err(dev, "could not get xvclk clock (%pe)\n", > + ov08x->xvclk); > + return PTR_ERR(ov08x->xvclk); > + } > + > + xvclk_rate = clk_get_rate(ov08x->xvclk); > + > + ov08x->reset_gpio = devm_gpiod_get_optional(dev, "reset", > + GPIOD_OUT_LOW); > + if (IS_ERR(ov08x->reset_gpio)) > + return PTR_ERR(ov08x->reset_gpio); > + > + for (i = 0; i < ARRAY_SIZE(ov08x40_supply_names); i++) > + ov08x->supplies[i].supply = ov08x40_supply_names[i]; > + > + ret = devm_regulator_bulk_get(dev, > + ARRAY_SIZE(ov08x40_supply_names), > + ov08x->supplies); > + if (ret) > + return ret; > + } else { > + ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency", > + &xvclk_rate); > + if (ret) { > + dev_err(dev, "can't get clock frequency"); > + return ret; > + } > } > > if (xvclk_rate != OV08X40_XVCLK) { > @@ -2143,32 +2236,37 @@ static int ov08x40_check_hwcfg(struct device *dev) > } > > static int ov08x40_probe(struct i2c_client *client) > -{ > - struct ov08x40 *ov08x; > +{ struct ov08x40 *ov08x; > int ret; > bool full_power; > > - /* Check HW config */ > - ret = ov08x40_check_hwcfg(&client->dev); > - if (ret) { > - dev_err(&client->dev, "failed to check hwcfg: %d", ret); > - return ret; > - } > - > ov08x = devm_kzalloc(&client->dev, sizeof(*ov08x), GFP_KERNEL); > if (!ov08x) > return -ENOMEM; > > + /* Check HW config */ > + ret = ov08x40_check_hwcfg(ov08x, &client->dev); > + if (ret) { > + dev_err(&client->dev, "failed to check hwcfg: %d", ret); > + return ret; > + } > + > /* Initialize subdev */ > v4l2_i2c_subdev_init(&ov08x->sd, client, &ov08x40_subdev_ops); > > full_power = acpi_dev_state_d0(&client->dev); > if (full_power) { > + ret = ov08x40_power_on(&client->dev); > + if (ret) { > + dev_err(&client->dev, "failed to power on\n"); > + return ret; > + } > + > /* Check module identity */ > ret = ov08x40_identify_module(ov08x); > if (ret) { > dev_err(&client->dev, "failed to find sensor: %d\n", ret); > - return ret; > + goto probe_power_off; > } > } > > @@ -2177,7 +2275,7 @@ static int ov08x40_probe(struct i2c_client *client) > > ret = ov08x40_init_controls(ov08x); > if (ret) > - return ret; > + goto probe_power_off; > > /* Initialize subdev */ > ov08x->sd.internal_ops = &ov08x40_internal_ops; > @@ -2210,6 +2308,9 @@ static int ov08x40_probe(struct i2c_client *client) > error_handler_free: > ov08x40_free_controls(ov08x); > > +probe_power_off: > + ov08x40_power_off(&client->dev); > + > return ret; > } > > @@ -2224,6 +2325,8 @@ static void ov08x40_remove(struct i2c_client *client) > > pm_runtime_disable(&client->dev); > pm_runtime_set_suspended(&client->dev); > + > + ov08x40_power_off(&client->dev); > } > > #ifdef CONFIG_ACPI > @@ -2235,10 +2338,17 @@ static const struct acpi_device_id ov08x40_acpi_ids[] = { > MODULE_DEVICE_TABLE(acpi, ov08x40_acpi_ids); > #endif > > +static const struct of_device_id ov08x40_of_match[] = { > + { .compatible = "ovti,ov08x40" }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, ov08x40_of_match); > + > static struct i2c_driver ov08x40_i2c_driver = { > .driver = { > .name = "ov08x40", > .acpi_match_table = ACPI_PTR(ov08x40_acpi_ids), > + .of_match_table = ov08x40_of_match, > }, > .probe = ov08x40_probe, > .remove = ov08x40_remove,