Hi Hans,
On Mon, Nov 20, 2023 at 11:00:14AM +0100, Hans de Goede wrote:
> 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@xxxxxxxxxx>
> >> ---
> >> 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.
The rule is not to split strings in order to satisfy alignment rules. IOW
the line should be wrapped before the string. :-)
--
Regards,
Sakari Ailus
