From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 38B1B32572F; Wed, 7 Jan 2026 13:31:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767792678; cv=none; b=i1s2kyO+bHuiNHujoXpeT+oiEOepufjtovLmreWm976MoUU7wEAV2Yl9ZRhL9vaFkstd0m/krtCKb41c7IGSDXgUTAaMK+hOnZgEF9G3V2VwyTAlB9vZkKtVgjvpMCbydeRUUiYalKYaYEg2JkqvXLfWRbtakuEBfk3wwLZ19AU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767792678; c=relaxed/simple; bh=YsZ/INo/RnjBgpxGjV/4i7ySEyRNPPlX6zQEkR9Xeo4=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=EcToBwKR01316mG8tu1l//iYaZBBLwz8s8eGz0Vl+c/MDLRqBnssLaDlljT7yYZvGnm3iJqLk4WN5BF4959t7WfGiXFp01yZUMEvY0DmFKxpGvLM11g8YUfcNCrg5WxgK6nqAGiIATCgFwTkWX4iqNFixvVHkBvUqx8rAA8Ifls= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=quIQbV1m; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="quIQbV1m" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 82AD3C4CEF7; Wed, 7 Jan 2026 13:31:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1767792676; bh=YsZ/INo/RnjBgpxGjV/4i7ySEyRNPPlX6zQEkR9Xeo4=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=quIQbV1m5BOuHOoI2ZXUp+Y4fpJV4TY+L3q5tUbk3xpHPo8XOWNe4rOhxGke/DuF5 h9EYrZ0NpK/HLVf+RHOz58AanQ0mdZ/drgrGWdDByXGcov53F+QVeCTVa45jZmiWfY iQIuaucndOKeRBe44MD5xgRoKSusWLApjC9aKMsa20SEMq86vJREYL2wqVx1+hnKpD Swuc+HP2Z2BSwMCH/cbKB4i3bN/cgK1/vSFcZ7yN1Ga/yahpkjdweLtOmPwf+AsMp/ 147s1DQqN5F/nXaQKD6jxzaP4Ilw2E6XdER9ZFpbRb+gg8g1r0hpJGJMAGBRFwy+kn arHwxvfkJJuCQ== Message-ID: <89bebe89-4bbd-42cb-b759-64562d763fc1@kernel.org> Date: Wed, 7 Jan 2026 13:31:11 +0000 Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v6 2/5] media: i2c: imx355: Support devicetree and power management To: Richard Acayan , Mauro Carvalho Chehab , Rob Herring , Conor Dooley , Bjorn Andersson , Konrad Dybcio , Sakari Ailus , Tianshu Qiu , linux-media@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-msm@vger.kernel.org Cc: Krzysztof Kozlowski , Robert Mader , Vladimir Zapolskiy , David Heidelberg , phone-devel@vger.kernel.org References: <20260107043044.92485-1-mailingradian@gmail.com> <20260107043044.92485-3-mailingradian@gmail.com> From: Bryan O'Donoghue Content-Language: en-US In-Reply-To: <20260107043044.92485-3-mailingradian@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 07/01/2026 04:30, Richard Acayan wrote: > A device tree compatible makes it possible for this driver to be used on > Open Firmware devices. Initialization of power-managed resources such as > the reset GPIO and voltage regulators can be specified in the device > tree and handled by the driver. Add support for this so the Pixel 3a can > use the driver. > > Signed-off-by: Richard Acayan > Nacked-by: Krzysztof Kozlowski > --- > drivers/media/i2c/imx355.c | 116 ++++++++++++++++++++++++++++++++++--- > 1 file changed, 108 insertions(+), 8 deletions(-) > > diff --git a/drivers/media/i2c/imx355.c b/drivers/media/i2c/imx355.c > index 776107efe386..4ac8495d1a3d 100644 > --- a/drivers/media/i2c/imx355.c > +++ b/drivers/media/i2c/imx355.c > @@ -3,9 +3,13 @@ > > #include > #include > +#include > +#include > #include > #include > +#include > #include > +#include > #include > > #include > @@ -125,6 +129,15 @@ struct imx355 { > * Protect access to sensor v4l2 controls. > */ > struct mutex mutex; > + > + struct gpio_desc *reset_gpio; > + struct regulator_bulk_data *supplies; > +}; > + > +static const struct regulator_bulk_data imx355_supplies[] = { > + { .supply = "avdd" }, > + { .supply = "dvdd" }, > + { .supply = "dovdd" }, > }; > > static const struct imx355_reg imx355_global_regs[] = { > @@ -1515,6 +1528,55 @@ static const struct v4l2_subdev_internal_ops imx355_internal_ops = { > .open = imx355_open, > }; > > +static int imx355_power_off(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); > + > + gpiod_set_value_cansleep(imx355->reset_gpio, 1); > + > + regulator_bulk_disable(ARRAY_SIZE(imx355_supplies), imx355->supplies); > + clk_disable_unprepare(imx355->clk); > + > + return 0; > +} > + > +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(5000, 5100); > + gpiod_set_value_cansleep(imx355->reset_gpio, 0); > + usleep_range(8000, 8100); > + > + return 0; > + > +error_disable_clocks: > + clk_disable_unprepare(imx355->clk); > + return ret; > +} > + > +static DEFINE_RUNTIME_DEV_PM_OPS(imx355_pm_ops, imx355_power_off, > + imx355_power_on, NULL); > + > /* Initialize control handlers */ > static int imx355_init_controls(struct imx355 *imx355) > { > @@ -1689,16 +1751,26 @@ static int imx355_probe(struct i2c_client *client) > "external clock %lu is not supported\n", > freq); > > - /* Initialize subdev */ > - v4l2_i2c_subdev_init(&imx355->sd, client, &imx355_subdev_ops); > - > - /* Check module identity */ > - ret = imx355_identify_module(imx355); > + ret = devm_regulator_bulk_get_const(imx355->dev, > + ARRAY_SIZE(imx355_supplies), > + imx355_supplies, > + &imx355->supplies); > if (ret) { > - dev_err(imx355->dev, "failed to find sensor: %d", ret); > + dev_err_probe(imx355->dev, ret, "could not get regulators"); > goto error_probe; > } > > + imx355->reset_gpio = devm_gpiod_get_optional(imx355->dev, "reset", > + GPIOD_OUT_LOW); > + if (IS_ERR(imx355->reset_gpio)) { > + ret = dev_err_probe(imx355->dev, PTR_ERR(imx355->reset_gpio), > + "failed to get gpios"); > + goto error_probe; > + } > + > + /* Initialize subdev */ > + v4l2_i2c_subdev_init(&imx355->sd, client, &imx355_subdev_ops); > + > imx355->hwcfg = imx355_get_hwcfg(imx355->dev); > if (!imx355->hwcfg) { > dev_err(imx355->dev, "failed to get hwcfg"); > @@ -1706,13 +1778,26 @@ static int imx355_probe(struct i2c_client *client) > goto error_probe; > } > > + ret = imx355_power_on(imx355->dev); > + if (ret) { > + dev_err(imx355->dev, "failed to power on sensor: %d", ret); > + goto error_probe; > + } > + > + /* Check module identity */ > + ret = imx355_identify_module(imx355); > + if (ret) { > + dev_err(imx355->dev, "failed to find sensor: %d", ret); > + goto error_power_off; > + } > + > /* Set default mode to max resolution */ > imx355->cur_mode = &supported_modes[0]; > > ret = imx355_init_controls(imx355); > if (ret) { > dev_err(imx355->dev, "failed to init controls: %d", ret); > - goto error_probe; > + goto error_power_off; > } > > /* Initialize subdev */ > @@ -1752,6 +1837,9 @@ static int imx355_probe(struct i2c_client *client) > error_handler_free: > v4l2_ctrl_handler_free(imx355->sd.ctrl_handler); > > +error_power_off: > + imx355_power_off(imx355->dev); > + > error_probe: > mutex_destroy(&imx355->mutex); > > @@ -1768,7 +1856,11 @@ static void imx355_remove(struct i2c_client *client) > v4l2_ctrl_handler_free(sd->ctrl_handler); > > pm_runtime_disable(imx355->dev); > - pm_runtime_set_suspended(imx355->dev); > + > + if (!pm_runtime_status_suspended(imx355->dev)) { > + imx355_power_off(imx355->dev); > + pm_runtime_set_suspended(imx355->dev); > + } > > mutex_destroy(&imx355->mutex); > } > @@ -1779,10 +1871,18 @@ static const struct acpi_device_id imx355_acpi_ids[] __maybe_unused = { > }; > MODULE_DEVICE_TABLE(acpi, imx355_acpi_ids); > > +static const struct of_device_id imx355_match_table[] __maybe_unused = { > + { .compatible = "sony,imx355", }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, imx355_match_table); > + > static struct i2c_driver imx355_i2c_driver = { > .driver = { > .name = "imx355", > .acpi_match_table = ACPI_PTR(imx355_acpi_ids), > + .of_match_table = imx355_match_table, > + .pm = &imx355_pm_ops, > }, > .probe = imx355_probe, > .remove = imx355_remove, > -- > 2.52.0 > Power-on sequence looks better now. Reviewed-by: Bryan O'Donoghue