| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <D82NHO05EBSQ.3AZF1VYBU17V5@d3embedded.com>
Date: Wed, 26 Feb 2025 15:13:27 -0500
From: "Sebastian LaVine" <slavine@...mbedded.com>
To: "Krzysztof Kozlowski" <krzysztof.kozlowski@...aro.org>,
<devicetree@...r.kernel.org>, <imx@...ts.linux.dev>,
<linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
<linux-media@...r.kernel.org>
Cc: Nícolas F. R. A. Prado <nfraprado@...labora.com>,
"Abel Vesa" <abel.vesa@...aro.org>, "Achath Vaishnav" <vaishnav.a@...com>,
"AngeloGioacchino Del Regno" <angelogioacchino.delregno@...labora.com>,
"Ard Biesheuvel" <ardb@...nel.org>, "Benjamin Mugnier"
<benjamin.mugnier@...s.st.com>, "Biju Das" <biju.das.jz@...renesas.com>,
"Bjorn Andersson" <quic_bjorande@...cinc.com>, "Catalin Marinas"
<catalin.marinas@....com>, "Conor Dooley" <conor+dt@...nel.org>, "Dmitry
Baryshkov" <dmitry.baryshkov@...aro.org>, "Elinor Montmasson"
<elinor.montmasson@...oirfairelinux.com>, "Fabio Estevam"
<festevam@...il.com>, "Geert Uytterhoeven" <geert+renesas@...der.be>, "Hans
Verkuil" <hverkuil@...all.nl>, "Javier Carrasco"
<javier.carrasco@...fvision.net>, "Jianzhong Xu" <xuj@...com>, "Julien
Massot" <julien.massot@...labora.com>, "Kieran Bingham"
<kieran.bingham@...asonboard.com>, "Kory Maincent"
<kory.maincent@...tlin.com>, "Laurent Pinchart"
<laurent.pinchart@...asonboard.com>, "Mauro Carvalho Chehab"
<mchehab@...nel.org>, "Mikhail Rudenko" <mike.rudenko@...il.com>, "Nishanth
Menon" <nm@...com>, "Pengutronix Kernel Team" <kernel@...gutronix.de>, "Rob
Herring" <robh@...nel.org>, "Sakari Ailus" <sakari.ailus@...ux.intel.com>,
"Sascha Hauer" <s.hauer@...gutronix.de>, "Shawn Guo" <shawnguo@...nel.org>,
"Stuart Burtner" <sburtner@...mbedded.com>, "Tero Kristo"
<kristo@...nel.org>, "Thakkar Devarsh" <devarsht@...com>, "Tomi Valkeinen"
<tomi.valkeinen@...asonboard.com>, "Umang Jain"
<umang.jain@...asonboard.com>, "Vignesh Raghavendra" <vigneshr@...com>,
"Will Deacon" <will@...nel.org>, "Zhi Mao" <zhi.mao@...iatek.com>
Subject: Re: [PATCH 2/4] media: i2c: Add driver for Sony IMX728
On Wed Feb 12, 2025 at 3:11 PM EST, Krzysztof Kozlowski wrote:
> On 12/02/2025 20:56, Sebastian LaVine wrote:
>>
>> ...
>>
>> +static int imx728_reset(struct imx728 *imx728)
>> +{
>> +
>> + int ret = 0;
>> +
>> + // Prefer hardware reset if available.
>> + if (!IS_ERR_OR_NULL(imx728->reset_gpio)) {
>
> Here can be ERR (although why?) but...
>
>> + gpiod_set_value_cansleep(imx728->reset_gpio, 1);
>>
>> ...
>>
>> +static int imx728_power_off(struct imx728 *imx728)
>> +{
>> +
>> + if (imx728->reset_gpio) {
>
> Here cannot.
>
>> + gpiod_set_value_cansleep(imx728->reset_gpio, 1);
>> +
>> + usleep_range(1, 10);
>> + }
>> + clk_disable_unprepare(imx728->clk);
>> + return 0;
>> +}
>> +
>>
>> ...
>>
>> +static int imx728_probe(struct i2c_client *client)
>> +{
>> + struct imx728 *imx728;
>> + struct v4l2_subdev *sd;
>> + struct v4l2_ctrl_handler *ctrl_hdr;
>> + int ret;
>> +
>> + imx728 = devm_kzalloc(&client->dev, sizeof(*imx728), GFP_KERNEL);
>> + if (!imx728)
>> + return -ENOMEM;
>> +
>> + imx728->dev = &client->dev;
>> +
>> + imx728->regmap = devm_cci_regmap_init_i2c(client, 16);
>> + if (IS_ERR(imx728->regmap))
>> + return PTR_ERR(imx728->regmap);
>> +
>> + imx728->reset_gpio = devm_gpiod_get_optional(imx728->dev,
>> + "reset", GPIOD_OUT_LOW);
>> + if (IS_ERR(imx728->reset_gpio))
>> + return PTR_ERR(imx728->reset_gpio);
>
> So can it be ERR after that point? Looks like not.
>
I see what you mean -- I'll change the check in imx728_reset to a simple
null check in v4. Thanks.
> (Jumping back up to previous inline feedback)
>
> ...
>
>> +
>> +static int imx728_set_stream(struct v4l2_subdev *sd, int enable)
>> +{
>> + struct imx728 *imx728 = to_imx728(sd);
>> + int ret;
>> +
>> + mutex_lock(&imx728->lock);
>
> Just use guard. That's actually perfect candidate.
>
Okay -- I'll include this change in v4. I'm not so familiar with using
this interface, so please let me know if this is incorrect:
diff --git a/drivers/media/i2c/imx728.c b/drivers/media/i2c/imx728.c
index 4a6dfa0a6c58..d7d62e9917a4 100644
--- a/drivers/media/i2c/imx728.c
+++ b/drivers/media/i2c/imx728.c
@@ -9320,17 +9320,16 @@ static int imx728_set_stream(struct v4l2_subdev *sd, int enable)
struct imx728 *imx728 = to_imx728(sd);
int ret;
- mutex_lock(&imx728->lock);
+ guard(mutex)(&imx728->lock);
+
- if (imx728->streaming == enable) {
+ if (imx728->streaming == enable)
- mutex_unlock(&imx728->lock);
return 0;
- }
if (enable) {
ret = pm_runtime_get_sync(imx728->dev);
if (ret < 0) {
pm_runtime_put_noidle(imx728->dev);
- goto err_unlock;
+ goto err;
}
ret = imx728_start_stream(imx728);
@@ -9349,15 +9348,12 @@ static int imx728_set_stream(struct v4l2_subdev *sd, int enable)
__v4l2_ctrl_grab(imx728->ctrl.h_flip, enable);
__v4l2_ctrl_grab(imx728->ctrl.v_flip, enable);
- mutex_unlock(&imx728->lock);
-
return 0;
err_runtime_put:
pm_runtime_put(imx728->dev);
-err_unlock:
- mutex_unlock(&imx728->lock);
+err:
dev_err(imx728->dev,
"%s: failed to setup streaming %d\n", __func__, ret);
return ret;
Thanks,
--
Sebastian
Please be aware that this email includes email addresses outside of the organization.
Powered by blists - more mailing lists