Hello Muzaffer, 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; > + } > + usleep_range(400, 500); > }
There are two times given in the datasheet. min 400 us between reset deassert and ID register read over CCI. And minimum time between deassert and stream start, which is 12ms. https://xff.cz/dl/tmp/b8efc244280a2b35.png https://xff.cz/dl/tmp/ed9b967a140bd347.png I wonder if this driver satisfied the second constraint. That may perhaps depend on how userspace uses it. Best regards, o. > 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); > + 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, > > --- > base-commit: db2ddb87143519e20a95aa36c60b36107b736a58 > change-id: 20260825-imx258-add-reset-gpio-patch-c438dd35f9cc > > Best regards, > -- > Muzaffer Kadir <[email protected]> > >
