| 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: <416d75fd-40d0-45d7-9590-0322abb480ca@linaro.org>
Date: Wed, 12 Feb 2025 21:11:58 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Sebastian LaVine <slavine@...mbedded.com>, 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 12/02/2025 20:56, Sebastian LaVine wrote:
> +static int imx728_detect(struct imx728 *imx728)
> +{
> + int ret = 0;
> + u64 minor, major;
> +
> + cci_read(imx728->regmap, IMX728_REG_VMAJOR, &major, &ret);
> + if (ret != 0) {
> + dev_err(imx728->dev, "Could not read PARAM_MAJOR_VER!");
> + return ret;
> + }
> + cci_read(imx728->regmap, IMX728_REG_VMINOR, &minor, &ret);
> + if (ret != 0) {
> + dev_err(imx728->dev, "Could not read PARAM_MINOR_VER!");
> + return ret;
> + }
> + dev_dbg(imx728->dev, "Got version: %lld.%lld", major, minor);
> +
> + return 0;
> +}
> +
> +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);
> + usleep_range(1000, 10000);
> + gpiod_set_value_cansleep(imx728->reset_gpio, 0);
> + msleep(100);
> + } else {
> + // Software reset.
> + cci_write(imx728->regmap, IMX728_REG_RESET_0, 0xc3, &ret);
> + cci_update_bits(imx728->regmap, IMX728_REG_RESET_1, 0x1, 0x1, &ret);
> + msleep(100);
> + }
> +
> + return ret;
> +}
> +
> +static int imx728_power_on(struct imx728 *imx728)
> +{
> + int ret;
> +
> + ret = clk_prepare_enable(imx728->clk);
> + if (ret < 0)
> + return ret;
> +
> + imx728_reset(imx728);
> + return 0;
> +}
> +
> +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_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.
> + 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;
> + }
> +
> + ret = imx728_start_stream(imx728);
> + if (ret < 0)
> + goto err_runtime_put;
> + } else {
> + ret = imx728_stop_stream(imx728);
> + if (ret < 0)
> + goto err_runtime_put;
> + pm_runtime_mark_last_busy(imx728->dev);
> + pm_runtime_put_autosuspend(imx728->dev);
> + }
> +
> + imx728->streaming = 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);
> + dev_err(imx728->dev,
> + "%s: failed to setup streaming %d\n", __func__, ret);
> + return ret;
> +}
> +
> +static const struct v4l2_subdev_core_ops imx728_core_ops = {
> + .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
> + .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> +};
> +
> +static const struct v4l2_subdev_video_ops imx728_subdev_video_ops = {
> + .s_stream = imx728_set_stream,
> +};
> +
> +static const struct v4l2_subdev_pad_ops imx728_subdev_pad_ops = {
> + .enum_frame_size = imx728_enum_frame_sizes,
> + .enum_mbus_code = imx728_enum_mbus_code,
> + .get_fmt = v4l2_subdev_get_fmt,
> + .get_frame_desc = imx728_get_frame_desc,
> + .get_frame_interval = imx728_get_frame_interval,
> + .get_selection = imx728_get_selection,
> + .set_fmt = imx728_set_fmt,
> + .set_frame_interval = imx728_set_frame_interval,
> + .set_routing = imx728_set_routing,
> +};
> +
> +static const struct v4l2_subdev_ops imx728_subdev_ops = {
> + .core = &imx728_core_ops,
> + .video = &imx728_subdev_video_ops,
> + .pad = &imx728_subdev_pad_ops,
> +};
> +
> +static const struct v4l2_subdev_internal_ops imx728_internal_ops = {
> + .init_state = imx728_init_state,
> +};
> +
> +
> +static const struct v4l2_ctrl_ops imx728_ctrl_ops = {
> + .s_ctrl = imx728_set_ctrl,
> +};
> +
> +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.
Best regards,
Krzysztof
Powered by blists - more mailing lists