| 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: <CAGb2v65vSRs-OzR91VWG=LMk2z0y=f9CSSJm_3-U_ywyMidgaw@mail.gmail.com>
Date: Tue, 19 Dec 2017 18:35:49 +0800
From: Chen-Yu Tsai <wens@...e.org>
To: Yong Deng <yong.deng@...ewell.com>
Cc: Maxime Ripard <maxime.ripard@...e-electrons.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Chen-Yu Tsai <wens@...e.org>,
"David S. Miller" <davem@...emloft.net>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Hans Verkuil <hans.verkuil@...co.com>,
Randy Dunlap <rdunlap@...radead.org>,
Benoit Parrot <bparrot@...com>,
Stanimir Varbanov <stanimir.varbanov@...aro.org>,
Arnd Bergmann <arnd@...db.de>,
Hugues Fruchet <hugues.fruchet@...com>,
Philipp Zabel <p.zabel@...gutronix.de>,
Benjamin Gaignard <benjamin.gaignard@...aro.org>,
Ramesh Shanmugasundaram <ramesh.shanmugasundaram@...renesas.com>,
Yannick Fertre <yannick.fertre@...com>,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
Rick Chang <rick.chang@...iatek.com>,
Linux Media Mailing List <linux-media@...r.kernel.org>,
devicetree <devicetree@...r.kernel.org>,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
linux-sunxi <linux-sunxi@...glegroups.com>,
Ondřej Jirman <megous@...ous.com>
Subject: Re: [linux-sunxi] [PATCH v3 1/3] media: V3s: Add support for
Allwinner CSI.
On Mon, Nov 13, 2017 at 3:30 PM, Yong Deng <yong.deng@...ewell.com> wrote:
> Allwinner V3s SoC have two CSI module. CSI0 is used for MIPI interface
> and CSI1 is used for parallel interface. This is not documented in
> datasheet but by testing and guess.
>
> This patch implement a v4l2 framework driver for it.
>
> Currently, the driver only support the parallel interface. MIPI-CSI2,
> ISP's support are not included in this patch.
>
> Signed-off-by: Yong Deng <yong.deng@...ewell.com>
> ---
> drivers/media/platform/Kconfig | 1 +
> drivers/media/platform/Makefile | 2 +
> drivers/media/platform/sunxi/sun6i-csi/Kconfig | 9 +
> drivers/media/platform/sunxi/sun6i-csi/Makefile | 3 +
> drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c | 918 +++++++++++++++++++++
> drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h | 146 ++++
> .../media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h | 203 +++++
> .../media/platform/sunxi/sun6i-csi/sun6i_video.c | 722 ++++++++++++++++
> .../media/platform/sunxi/sun6i-csi/sun6i_video.h | 61 ++
> 9 files changed, 2065 insertions(+)
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/Kconfig
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/Makefile
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.h
>
[...]
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
> new file mode 100644
> index 0000000..0cebcbd
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
[...]
> +/* -----------------------------------------------------------------------------
> + * Media Operations
> + */
> +static int sun6i_video_formats_init(struct sun6i_video *video)
> +{
> + struct v4l2_subdev_mbus_code_enum mbus_code = { 0 };
> + struct sun6i_csi *csi = video->csi;
> + struct v4l2_format format;
> + struct v4l2_subdev *subdev;
> + u32 pad;
> + const u32 *pixformats;
> + int pixformat_count = 0;
> + u32 subdev_codes[32]; /* subdev format codes, 32 should be enough */
> + int codes_count = 0;
> + int num_fmts = 0;
> + int i, j;
> +
> + subdev = sun6i_video_remote_subdev(video, &pad);
> + if (subdev == NULL)
> + return -ENXIO;
> +
> + /* Get supported pixformats of CSI */
> + pixformat_count = sun6i_csi_get_supported_pixformats(csi, &pixformats);
> + if (pixformat_count <= 0)
> + return -ENXIO;
> +
> + /* Get subdev formats codes */
> + mbus_code.pad = pad;
> + mbus_code.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> + while (!v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL,
> + &mbus_code)) {
> + if (codes_count >= ARRAY_SIZE(subdev_codes)) {
> + dev_warn(video->csi->dev,
> + "subdev_codes array is full!\n");
> + break;
> + }
> + subdev_codes[codes_count] = mbus_code.code;
> + codes_count++;
> + mbus_code.index++;
> + }
> +
> + if (!codes_count)
> + return -ENXIO;
> +
> + /* Get supported formats count */
> + for (i = 0; i < codes_count; i++) {
> + for (j = 0; j < pixformat_count; j++) {
> + if (!sun6i_csi_is_format_support(csi, pixformats[j],
> + mbus_code.code)) {
Bug here. You are testing against mbus_code.code, which would be whatever
was leftover from the previous section. Instead you should be testing
against subdev_codes[i], the list you just built.
Without it, it won't get past this part of the code if the last enumerated
media bus format isn't supported.
> + continue;
> + }
> + num_fmts++;
> + }
> + }
> +
> + if (!num_fmts)
> + return -ENXIO;
> +
> + video->num_formats = num_fmts;
> + video->formats = devm_kcalloc(video->csi->dev, num_fmts,
> + sizeof(struct sun6i_csi_format), GFP_KERNEL);
> + if (!video->formats)
> + return -ENOMEM;
> +
> + /* Get supported formats */
> + num_fmts = 0;
> + for (i = 0; i < codes_count; i++) {
> + for (j = 0; j < pixformat_count; j++) {
> + if (!sun6i_csi_is_format_support(csi, pixformats[j],
> + mbus_code.code)) {
Same here.
This gets me past the enumeration part of things...
> + continue;
> + }
> +
> + video->formats[num_fmts].fourcc = pixformats[j];
> + video->formats[num_fmts].mbus_code =
> + mbus_code.code;
> + video->formats[num_fmts].bpp =
> + v4l2_pixformat_get_bpp(pixformats[j]);
> + num_fmts++;
> + }
> + }
> +
> + /* setup default format */
> + format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> + format.fmt.pix.width = 1280;
> + format.fmt.pix.height = 720;
> + format.fmt.pix.pixelformat = video->formats[0].fourcc;
> + sun6i_video_set_fmt(video, &format);
But my system crashes here within the OV5640 driver.
So no tests about the actual functionality. This was on the Bananapi M3,
which has an A83T SoC.
In general I think you should make your driver much more noisy than it
currently is. I spent the whole afternoon adding error messages and
debug traces to narrow down the issue.
ChenYu
Powered by blists - more mailing lists