Hi Muzaffer, Thanks for the set.
On Fri, Aug 28, 2026 at 01:51:47PM +0300, Muzaffer Kadir via B4 Relay wrote: > From: Muzaffer Kadir <[email protected]> > > reset-gpio is already documented in dt-bindings but never implemented > in the driver. > Reset deassert delay comes from Luis Garcia's and Ondrej Jirman's patch. > > Link: https://lore.kernel.org/all/[email protected] > Signed-off-by: Muzaffer Kadir <[email protected]> > --- > I have a device that is not upstreamed yet (General Mobile Shamrock) > whose camera needs reset gpio to probe, it is documented for > dts check but not implemented for some reason. > With adding it rear camera on the device probes correctly. > > I created this patch without knowing the older one that submitted > before: https://lore.kernel.org/all/[email protected]/ > > I don't fully know the correct reset timing so I was using a random wait > before, > after I discovered existing patch I reused previous work for delay time after > reset. > --- > drivers/media/i2c/imx258.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c > index bc9ee449a87c..af3f12c7452a 100644 > --- a/drivers/media/i2c/imx258.c > +++ b/drivers/media/i2c/imx258.c > @@ -9,6 +9,7 @@ > #include <linux/pm_runtime.h> > #include <linux/regulator/consumer.h> > #include <linux/unaligned.h> > +#include <linux/gpio/consumer.h> > > #include <media/v4l2-cci.h> > #include <media/v4l2-ctrls.h> > @@ -681,6 +682,7 @@ struct imx258 { > > struct clk *clk; > struct regulator_bulk_data supplies[IMX258_NUM_SUPPLIES]; > + struct gpio_desc *reset_gpio; > }; > > static inline struct imx258 *to_imx258(struct v4l2_subdev *_sd) > @@ -1128,6 +1130,18 @@ static int imx258_power_on(struct device *dev) > if (ret) { > dev_err(dev, "failed to enable clock\n"); > regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies); > + return ret; > + } > + > + if (imx258->reset_gpio) { > + ret = gpiod_set_value_cansleep(imx258->reset_gpio, 0); > + if (ret) { > + dev_err(dev, "failed to deassert reset\n"); > + clk_disable_unprepare(imx258->clk); > + regulator_bulk_disable(IMX258_NUM_SUPPLIES, > imx258->supplies); > + return ret; This warrants reworking error handling; please use gotos and move it to the end of the function. Same for clock error handling. > + } > + usleep_range(400, 500); The delay seems right. Can you use fsleep()? In fact the delay should always have been there so this is a bugfix. It should go to a separate patch. > } > > return ret; > @@ -1138,6 +1152,7 @@ static int imx258_power_off(struct device *dev) > struct v4l2_subdev *sd = dev_get_drvdata(dev); > struct imx258 *imx258 = to_imx258(sd); > > + gpiod_set_value_cansleep(imx258->reset_gpio, 1); > clk_disable_unprepare(imx258->clk); > regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies); > > @@ -1382,6 +1397,11 @@ static int imx258_probe(struct i2c_client *client) > return ret; > } > > + imx258->reset_gpio = devm_gpiod_get_optional(imx258->dev, "reset", > GPIOD_OUT_HIGH); Over 80, please wrap (there's another earlier, too). > + if (IS_ERR(imx258->reset_gpio)) > + return dev_err_probe(imx258->dev, PTR_ERR(imx258->reset_gpio), > + "Failed to get reset-gpios\n"); > + > ret = imx258_get_regulators(imx258); > if (ret) > return dev_err_probe(imx258->dev, ret, > -- Kind regards, Sakari Ailus
