From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0692D421A14 for ; Tue, 20 Jan 2026 12:44:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.42 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768913072; cv=none; b=Dk2je8vw3wknBC8KCE0hVZi/UD31Rse/ftRjgsc7VT39ePAm/gIix2mUmq7sugjxsDFJG0KS8oMU7JtEr/KnqcvfGZhHrTz5DcuoUkLrqPg19TIDLdciBAsrdKAqvaqmV9T7r0KiYOrfMlE3M2epdKfPzGpvDqdlFwodNUMDZ8E= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768913072; c=relaxed/simple; bh=IS8rUR5sEdGEUkR4PIDgTLZLVHe9z6U/zU7WLBOIono=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=MjWbIffvctpxuWGBrl7K4XnhUmkCKQUmzVLsQQHR8Yv50y/vQ+B79piNsRpVxR175V1a4w9rrW9c49Qjsq6vEqv/B3ceteXRRCEI+VHb3u9DLjw5lJ5sj7FGHpAztc0tsidR/bVkS0Nz2Xg4Vd9zCcH1hMJR43O9hOVgP83Ch2c= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=kE738k1u; arc=none smtp.client-ip=209.85.128.42 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="kE738k1u" Received: by mail-wm1-f42.google.com with SMTP id 5b1f17b1804b1-4801d7c72a5so27822315e9.0 for ; Tue, 20 Jan 2026 04:44:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1768913068; x=1769517868; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:content-language:from :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=XERyzWRrypJfozC0Mdo9IWYRT9E2luhxzdFITS/kknw=; b=kE738k1ub1mpy/W5SCbw8O5Siha0VdEPKVCQ76ehtdrSlLTs+4NvA2ngAs9mhfQsqx umF0eSuuCVpTI9s+DxvRVuBpEaJ6rk1kT510cL8bqSNV6jDcTnPBalyVn9c1pajRYpIB ukwFN1QblcxC6B0v3m64hCE/rGjzXb2PvKBV5rXvNzEEm7tu/63yD2xz6XCkCo6VYGrr YO68j1pvUVN/ZLZ3gLzomFqtYr4eEFaogeWnFaO7WGODKr2KfynhimAalTfKMjMbBZBr NoTo6WWif74g8usY9clfLxWaQalVkUBsoTmHDUDaO1SSxmQ4JvW9FSSs1A+eytgGpuE9 DXIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1768913068; x=1769517868; h=content-transfer-encoding:in-reply-to:content-language:from :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=XERyzWRrypJfozC0Mdo9IWYRT9E2luhxzdFITS/kknw=; b=G959Q9C+PM8nSDxYrkxduL2ybU+zF/t+5C0wJoOlUfyDxLSflpdTzwoGBHOTb9NaJA kKbNrRHwwqwBqABPZYx6CAc/ocIJyKNnuhE7epkD6NHTjkhYS4kbZs6lLBozs1etJvdX 1J8RDbN6vUTIc+Vb9CWjeGTrosz4/7QpIGCrI58h/1hc5hbpchT5V1Q8xDW/bXoqOaX4 OCf4HcuMR/Ivp7/oq7AFNA8aW7YfeHKmhA1rBUz8V5uvUXuHUTWXgc9pXB6/ktJYvbZ4 3lp0sZuq7xN+PFyzS3kSFUHJJLz0PA8TKbEvKB7aTfQwsHzD8r2+ZO7PbQ4/Sp2N2vVS OqAQ== X-Forwarded-Encrypted: i=1; AJvYcCXLWVA3BGOYUEXvv29FSgt2nYbfmTTo2UmCG4aiYKQDoIB5XRUrwtp559SHYP+oqKkPPnWrW/2Dlypv@vger.kernel.org X-Gm-Message-State: AOJu0Yw7YgidA3sha/eNB3gDqv86LdADNj0o+k3gFYmgvpyWFymiuwCS laUuXjggE1gK4Xw8Zltitcx2d2D5VfvXZpNIbSid7623Va0SgR/AKnjE/myS6ZGRzkI= X-Gm-Gg: AY/fxX40mE7nK50E+ii0Zwsn9pcbtQ7fUX6XNpcpBfg5UEDmio2pbYA4S9lAzt4w9n+ fksRS4oMLM1N1N+grruou2ZMl+Sdx9Tt2e304JPZGBxOzFUgPaWJdxg0PggkI3YgjIvdqC++BIH xtoATO+JDeFUXc0kSy+yxkhTb322mUYZUGp7Trd1icVBWgFMKWOe4w8wmYQ2l0ruTZrgvXN43lQ IIRhZtB6XybYdcUKbA5bmeCnH91GSjvrWY+5fqIoJw0XVMzN/yvSSH7aAziA9jWoFpxJnd6lKW3 C4/3tKTAtkrDVa1q5gJINYJxJTHkGRyWCEs4aiiTsqFsd3O2MP0oqbPo+8REjrgkoYamCWzhB6i 71EhPCi8MwbsLdU7Ftc5Cw5dNV6F3eyHjdN7E5I+JeoVCbDK/FZnDO1+nwYzC4E7pZDVQ48FHIN 500CdaAhHvXS7U5XIaVwQY/jDq3xvff7rfN1eQr/8HUolih9SYV9wD X-Received: by 2002:a05:600c:474d:b0:477:7b16:5f77 with SMTP id 5b1f17b1804b1-4801eab5193mr147622475e9.3.1768913068176; Tue, 20 Jan 2026 04:44:28 -0800 (PST) Received: from [192.168.0.40] (188-141-3-146.dynamic.upc.ie. [188.141.3.146]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4801e8c0499sm251053925e9.9.2026.01.20.04.44.25 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 20 Jan 2026 04:44:27 -0800 (PST) Message-ID: <578668b0-cba2-4550-b676-26ed0b447bf2@linaro.org> Date: Tue, 20 Jan 2026 12:44:24 +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 v7 2/5] media: i2c: imx355: Support devicetree and power management To: Richard Acayan , Mauro Carvalho Chehab , Rob Herring , Krzysztof Kozlowski , 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: Robert Mader , Vladimir Zapolskiy , David Heidelberg , phone-devel@vger.kernel.org References: <20260117040657.27043-1-mailingradian@gmail.com> <20260117040657.27043-3-mailingradian@gmail.com> From: Bryan O'Donoghue Content-Language: en-US In-Reply-To: <20260117040657.27043-3-mailingradian@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 17/01/2026 04:06, 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. > > Reviewed-by: Bryan O'Donoghue > Signed-off-by: Richard Acayan > --- > 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..5a8da035ba5f 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(1000, 2000); > + gpiod_set_value_cansleep(imx355->reset_gpio, 0); > + usleep_range(10000, 11000); > + > + 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_HIGH); > + 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[] = { > + { .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, I think reset should be asserted before regulators and power are switched on. i.e. before you try to switch the chip on, you should establish that the reset pin is in the state that the timing diagram calls for. If we look at imx214 which someone has posted on the internet https://www.v-visiontech.com/web/userfiles/download/IMX214-0AQH5-C_2.0.0ExcellenceCommsen-281-29_mASEw.pdf Pages 28 and 29 See timing value T4 - you need to provide power and clock for T4 before de-asserting reset and then wait for T7 before starting to stream. We don't have the imx315 spec but likely imx355 has a similar start-up state machine. 1. Assert reset 2. Power up 3. Start clock 4. t4 5. De-assert reset 6. T6 - wait time to first i2c transaction 7. T7 - wait time to first stream on You should definitely set the reset to logical on in power_on() as you don't necessarily know the state of the reset pin each time you power on. --- bod