| Message ID | 20231115123817.196252-3-hdegoede@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | media: ov2740: Add support for reset GPIO and external clock | expand |
Hi Bingbu, On 11/20/23 05:06, Bingbu Cao wrote: > > Hans, > > On 11/15/23 8:38 PM, Hans de Goede wrote: >> On some ACPI platforms, such as Chromebooks the ACPI methods to >> change the power-state (_PS0 and _PS3) fully take care of powering >> on/off the sensor. >> >> On other ACPI platforms, such as e.g. various ThinkPad models with >> IPU6 + ov2740 sensor, the sensor driver must control the reset GPIO >> and the sensor's clock itself. >> >> Add support for having the driver control an optional clock. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/media/i2c/ov2740.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c >> index e5f9569a229d..0a87d0920eb8 100644 >> --- a/drivers/media/i2c/ov2740.c >> +++ b/drivers/media/i2c/ov2740.c >> @@ -3,6 +3,7 @@ >> >> #include <asm/unaligned.h> >> #include <linux/acpi.h> >> +#include <linux/clk.h> >> #include <linux/delay.h> >> #include <linux/gpio/consumer.h> >> #include <linux/i2c.h> >> @@ -336,6 +337,7 @@ struct ov2740 { >> >> /* GPIOs, clocks */ >> struct gpio_desc *reset_gpio; >> + struct clk *clk; >> >> /* Current mode */ >> const struct ov2740_mode *cur_mode; >> @@ -1068,6 +1070,7 @@ static int ov2740_suspend(struct device *dev) >> struct ov2740 *ov2740 = to_ov2740(sd); >> >> gpiod_set_value_cansleep(ov2740->reset_gpio, 1); >> + clk_disable_unprepare(ov2740->clk); >> return 0; >> } >> >> @@ -1075,6 +1078,11 @@ static int ov2740_resume(struct device *dev) >> { >> struct v4l2_subdev *sd = dev_get_drvdata(dev); >> struct ov2740 *ov2740 = to_ov2740(sd); >> + int ret; >> + >> + ret = clk_prepare_enable(ov2740->clk); >> + if (ret) >> + return ret; >> >> gpiod_set_value_cansleep(ov2740->reset_gpio, 0); >> msleep(20); >> @@ -1102,6 +1110,10 @@ static int ov2740_probe(struct i2c_client *client) >> return dev_err_probe(dev, PTR_ERR(ov2740->reset_gpio), >> "failed to get reset GPIO\n"); >> >> + ov2740->clk = devm_clk_get_optional(dev, "clk"); >> + if (IS_ERR(ov2740->clk)) >> + return dev_err_probe(dev, PTR_ERR(ov2740->clk), "failed to get clock\n"); >> + > > I am not very sure that the 80-char rule is still valid for checkpatch.pl. checkpatch.pl defaults to allowing longer lines (<100 chars) now, but you are right that the linux-media maintainers prefer 80. Still there is an exception to not split strings running over the limit and this line ends with a string, so I think that this is fine. Regards, Hans
diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c index e5f9569a229d..0a87d0920eb8 100644 --- a/drivers/media/i2c/ov2740.c +++ b/drivers/media/i2c/ov2740.c @@ -3,6 +3,7 @@ #include <asm/unaligned.h> #include <linux/acpi.h> +#include <linux/clk.h> #include <linux/delay.h> #include <linux/gpio/consumer.h> #include <linux/i2c.h> @@ -336,6 +337,7 @@ struct ov2740 { /* GPIOs, clocks */ struct gpio_desc *reset_gpio; + struct clk *clk; /* Current mode */ const struct ov2740_mode *cur_mode; @@ -1068,6 +1070,7 @@ static int ov2740_suspend(struct device *dev) struct ov2740 *ov2740 = to_ov2740(sd); gpiod_set_value_cansleep(ov2740->reset_gpio, 1); + clk_disable_unprepare(ov2740->clk); return 0; } @@ -1075,6 +1078,11 @@ static int ov2740_resume(struct device *dev) { struct v4l2_subdev *sd = dev_get_drvdata(dev); struct ov2740 *ov2740 = to_ov2740(sd); + int ret; + + ret = clk_prepare_enable(ov2740->clk); + if (ret) + return ret; gpiod_set_value_cansleep(ov2740->reset_gpio, 0); msleep(20); @@ -1102,6 +1110,10 @@ static int ov2740_probe(struct i2c_client *client) return dev_err_probe(dev, PTR_ERR(ov2740->reset_gpio), "failed to get reset GPIO\n"); + ov2740->clk = devm_clk_get_optional(dev, "clk"); + if (IS_ERR(ov2740->clk)) + return dev_err_probe(dev, PTR_ERR(ov2740->clk), "failed to get clock\n"); + v4l2_i2c_subdev_init(&ov2740->sd, client, &ov2740_subdev_ops); full_power = acpi_dev_state_d0(&client->dev); if (full_power) {
On some ACPI platforms, such as Chromebooks the ACPI methods to change the power-state (_PS0 and _PS3) fully take care of powering on/off the sensor. On other ACPI platforms, such as e.g. various ThinkPad models with IPU6 + ov2740 sensor, the sensor driver must control the reset GPIO and the sensor's clock itself. Add support for having the driver control an optional clock. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/media/i2c/ov2740.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)