On Mon, Jan 19, 2026 at 10:50:41PM -0500, Richard Acayan wrote:
> On Sat, Jan 17, 2026 at 02:03:02PM +0200, Vladimir Zapolskiy wrote:
> > On 1/17/26 06:06, Richard Acayan wrote:
> (snip)
> > > +static int imx355_power_on(struct device *dev)
> > > +{
> > > + struct i2c_client *client = container_of(dev, struct i2c_client, dev);
> > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > + struct imx355 *imx355 = to_imx355(sd);
> > > + int ret;
> > > +
> > > + ret = clk_prepare_enable(imx355->clk);
> > > + if (ret) {
> > > + dev_err(dev, "failed to enable clocks: %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + ret = regulator_bulk_enable(ARRAY_SIZE(imx355_supplies),
> > > + imx355->supplies);
> > > + if (ret) {
> > > + dev_err(dev, "failed to enable regulators: %d\n", ret);
> > > + goto error_disable_clocks;
> > > + }
> > > +
> > > + gpiod_set_value_cansleep(imx355->reset_gpio, 1);
> > > + usleep_range(1000, 2000);
> >
> > The deassert above is not needed IMO, anyway.
>
> This assert is for clarity, otherwise it isn't obvious that the GPIO is
> asserted low when the function is called. It should stay.
I'd also remove it: it's redundant.
--
Sakari Ailus
