linux-kernel - Re: [linux-sunxi] [PATCH v4 09/15] media: sunxi: Add support for the A31 MIPI CSI-2 controller

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAAEAJfAJYCE2z662hPderJ-5Qv3WBA8K5ZQaZ1JuZbZN+KfFig@mail.gmail.com>
Date:   Mon, 11 Jan 2021 15:21:19 -0300
From:   Ezequiel Garcia <ezequiel@...guardiasur.com.ar>
To:     Paul Kocialkowski <paul.kocialkowski@...tlin.com>
Cc:     linux-media <linux-media@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-doc@...r.kernel.org, devel@...verdev.osuosl.org,
        linux-sunxi@...glegroups.com, Yong Deng <yong.deng@...ewell.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Maxime Ripard <mripard@...nel.org>,
        Chen-Yu Tsai <wens@...e.org>,
        Jernej Skrabec <jernej.skrabec@...l.net>,
        Jonathan Corbet <corbet@....net>,
        Kishon Vijay Abraham I <kishon@...com>,
        Vinod Koul <vkoul@...nel.org>,
        Helen Koike <helen.koike@...labora.com>,
        Dafna Hirschfeld <dafna.hirschfeld@...labora.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Hans Verkuil <hans.verkuil@...co.com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        kevin.lhopital@...mail.com
Subject: Re: [linux-sunxi] [PATCH v4 09/15] media: sunxi: Add support for the
 A31 MIPI CSI-2 controller

Salut Paul,

Just a minor comment about the v4l2 async API.

On Thu, 31 Dec 2020 at 11:30, Paul Kocialkowski
<paul.kocialkowski@...tlin.com> wrote:
>
> The A31 MIPI CSI-2 controller is a dedicated MIPI CSI-2 bridge
> found on Allwinner SoCs such as the A31 and V3/V3s.
>
> It is a standalone block, connected to the CSI controller on one side
> and to the MIPI D-PHY block on the other. It has a dedicated address
> space, interrupt line and clock.
>
> It is represented as a V4L2 subdev to the CSI controller and takes a
> MIPI CSI-2 sensor as its own subdev, all using the fwnode graph and
> media controller API.
>
> Only 8-bit and 10-bit Bayer formats are currently supported.
> While up to 4 internal channels to the CSI controller exist, only one
> is currently supported by this implementation.
>
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@...tlin.com>
> ---
>  drivers/media/platform/sunxi/Kconfig          |   1 +
>  drivers/media/platform/sunxi/Makefile         |   1 +
>  .../platform/sunxi/sun6i-mipi-csi2/Kconfig    |  12 +
>  .../platform/sunxi/sun6i-mipi-csi2/Makefile   |   4 +
>  .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c   | 590 ++++++++++++++++++
>  .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h   | 117 ++++
>  6 files changed, 725 insertions(+)
>  create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/Kconfig
>  create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/Makefile
>  create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c
>  create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h
>
[..]
> +static int sun6i_mipi_csi2_v4l2_setup(struct sun6i_mipi_csi2_dev *cdev)
> +{
> +       struct sun6i_mipi_csi2_video *video = &cdev->video;
> +       struct v4l2_subdev *subdev = &video->subdev;
> +       struct v4l2_async_notifier *notifier = &video->notifier;
> +       struct fwnode_handle *handle;
> +       struct v4l2_fwnode_endpoint *endpoint;
> +       struct v4l2_async_subdev *subdev_async;
> +       int ret;
> +
> +       /* Subdev */
> +
> +       v4l2_subdev_init(subdev, &sun6i_mipi_csi2_subdev_ops);
> +       subdev->dev = cdev->dev;
> +       subdev->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> +       strscpy(subdev->name, MODULE_NAME, sizeof(subdev->name));
> +       v4l2_set_subdevdata(subdev, cdev);
> +
> +       /* Entity */
> +
> +       subdev->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> +       subdev->entity.ops = &sun6i_mipi_csi2_entity_ops;
> +
> +       /* Pads */
> +
> +       video->pads[0].flags = MEDIA_PAD_FL_SINK;
> +       video->pads[1].flags = MEDIA_PAD_FL_SOURCE;
> +
> +       ret = media_entity_pads_init(&subdev->entity, 2, video->pads);
> +       if (ret)
> +               return ret;
> +
> +       /* Endpoint */
> +
> +       handle = fwnode_graph_get_endpoint_by_id(dev_fwnode(cdev->dev), 0, 0,
> +                                                FWNODE_GRAPH_ENDPOINT_NEXT);
> +       if (!handle) {
> +               ret = -ENODEV;
> +               goto error_media_entity;
> +       }
> +
> +       endpoint = &video->endpoint;
> +       endpoint->bus_type = V4L2_MBUS_CSI2_DPHY;
> +
> +       ret = v4l2_fwnode_endpoint_parse(handle, endpoint);
> +       fwnode_handle_put(handle);

I think the _put should be...

> +       if (ret)
> +               goto error_media_entity;
> +
> +       /* Notifier */
> +
> +       v4l2_async_notifier_init(notifier);
> +
> +       subdev_async = &video->subdev_async;
> +       ret = v4l2_async_notifier_add_fwnode_remote_subdev(notifier, handle,
> +                                                          subdev_async);

... here. See for instance drivers/media/platform/rcar-vin/rcar-csi2.c.

(Unless I've missed something, of course).

Cheers,
Ezequiel