Re: [PATCH v2 5/5] media: i2c: lm3560: Add support for PM features

Svyatoslav Ryhel
2026-04-21T17:32:29Z

вт, 21 квіт. 2026 р. о 19:44 Sakari Ailus <sakari.ailus@linux.intel.com> пише:
>
> Hi Shyvatoslav,
>
> On Sun, Apr 19, 2026 at 12:34:12PM +0300, Svyatoslav Ryhel wrote:
> > @@ -403,6 +423,60 @@ static int lm3560_init_device(struct lm3560_flash *flash)
> > return rval;
> > }
> >
> > +static void lm3560_power_off(struct lm3560_flash *flash)
> > +{
> > + gpiod_set_value_cansleep(flash->hwen_gpio, 0);
> > + regulator_disable(flash->vin_supply);
> > +}
> > +
> > +static int lm3560_power_on(struct lm3560_flash *flash)
> > +{
> > + int rval;
> > +
> > + rval = regulator_enable(flash->vin_supply);
> > + if (rval < 0) {
> > + dev_err(flash->dev, "failed to enable vin power supply\n");
> > + return rval;
> > + }
> > +
> > + gpiod_set_value_cansleep(flash->hwen_gpio, 1);
> > +
> > + rval = lm3560_init_device(flash);
> > + if (rval < 0) {
> > + lm3560_power_off(flash);
> > + return rval;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int __maybe_unused lm3560_pm_runtime_resume(struct device *dev)
> > +{
> > + struct lm3560_flash *flash = dev_get_drvdata(dev);
> > +
> > + return lm3560_power_on(flash);
> > +}
> > +
> > +static int __maybe_unused lm3560_pm_runtime_suspend(struct device *dev)
>
> Could you change lm3560_power_o{n,ff}() take struct device pointer as the
> argument?
>

What benefit it will bring? Unless you propose to use on/off functions
in pm directly, I don't see any benefit in this change.

> > +{
> > + struct lm3560_flash *flash = dev_get_drvdata(dev);
> > +
> > + lm3560_power_off(flash);
> > +
> > + return 0;
> > +}
> > +
> > +static void lm3560_subdev_cleanup(struct lm3560_flash *flash)
> > +{
> > + unsigned int i;
> > +
> > + for (i = LM3560_LED0; i < LM3560_LED_MAX; i++) {
>
> You could define i here.
>

You mean:
for (unsigned int i = LM3560_LED0; i < LM3560_LED_MAX; i++)?

If yes, I would like to keep it as is, it looks cleaner to me, and
this part was transferred from remove as is.

> > + v4l2_device_unregister_subdev(&flash->subdev_led[i]);
> > + v4l2_ctrl_handler_free(&flash->ctrls_led[i]);
> > + media_entity_cleanup(&flash->subdev_led[i].entity);
> > + }
> > +}
> > +
> > static int lm3560_probe(struct i2c_client *client)
> > {
> > struct lm3560_flash *flash;
>
> --
> Kind regards,
>
> Sakari Ailus