Message ID | 20231228093926.748001-1-hidenorik@chromium.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-12538-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7301:6f82:b0:100:9c79:88ff with SMTP id tb2csp1893473dyb; Thu, 28 Dec 2023 01:40:09 -0800 (PST) X-Google-Smtp-Source: AGHT+IGRUgOilMVV0uYIXEFzq51mnI+C9ffqD5ha9ioNAl+YGJWK1c/ErWOAlDNp+5yGT2HxNUjk X-Received: by 2002:a05:6808:1707:b0:3ba:aa7:bd2f with SMTP id bc7-20020a056808170700b003ba0aa7bd2fmr11854242oib.92.1703756409704; Thu, 28 Dec 2023 01:40:09 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1703756409; cv=none; d=google.com; s=arc-20160816; b=Gr8Txoq/EKcfBL3QChpDwFaeIuUZPRUJQH9Y+IF2NeMpMlpT7hccDN7DtNS/LX3WnF uSltxz1uUoj/ewnJh5nL+pHrCHLd5t9IQ2h1LnaQRmBRLQsjmNTIbUE5oakaLVlu6D6V ERTZi4CjHKcXdeaT/PJToIwWnXH2+sUnapHa6iTMrRWkXs08FjjHPBvgRj4nyAlwIV5O jRRituHjr1iAAfYGA3itkWZPVhZyq5owa7jFhLdWdCGOIiGRSdpXIfjWGltQI5CVqQ7O 3K8kXzXUnLpd40l71NMnOu35ipnm+kPjxlvGtAdX7xGzII7oeJGCeka9xCL5NAsTgbxc fo/A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from:dkim-signature; bh=cE0PHPIX9V+Td6mS/CWtPhPTpSAbOD3GNR/J2y9uxsE=; fh=jIO5vXxyawZwCZqkWwwpRws2eigcS5WhitJ+uDvAXxM=; b=sC4tlH5efLVHdnUnzqeSHMBRm5wQzfhWF6pil9MNIuTragitaPRh9nV3hHbpSjEhw1 AbkcwPP5w0a22vfqJNgJ6Iuramz9V0hAE1dh1wPCsUEO0CgAEBcPTMWM5M4wUzpjZYW1 mftSc8EuBt4WQVgGM8Rr/0Ye2ThXC8fcywkkoh9Bg9ORRoz+6uMudUqj5u2DTROClLgg XDDGVVGwNqSR7Ye1WRUwrVbjEYTmUDqjmn3TWKoXFWSvTtSBWinEGYy59V0+Am7hf6IW b5RNi8u1oQ9oO4QidKK1HfaBWygMsri2o075Iv3DcmOdPCxzbTg4/zbosmiV2VKyVNup 4T2A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=HGtaXCTW; spf=pass (google.com: domain of linux-kernel+bounces-12538-ouuuleilei=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) smtp.mailfrom="linux-kernel+bounces-12538-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [139.178.88.99]) by mx.google.com with ESMTPS id q14-20020a631f4e000000b005cdd3a5543bsi7829559pgm.438.2023.12.28.01.40.09 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Dec 2023 01:40:09 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-12538-ouuuleilei=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99; Authentication-Results: mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=HGtaXCTW; spf=pass (google.com: domain of linux-kernel+bounces-12538-ouuuleilei=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) smtp.mailfrom="linux-kernel+bounces-12538-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id 739DD282E41 for <ouuuleilei@gmail.com>; Thu, 28 Dec 2023 09:40:09 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 6C56C6FA4; Thu, 28 Dec 2023 09:39:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="HGtaXCTW" X-Original-To: linux-kernel@vger.kernel.org Received: from mail-oi1-f173.google.com (mail-oi1-f173.google.com [209.85.167.173]) (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 5C50A63AD for <linux-kernel@vger.kernel.org>; Thu, 28 Dec 2023 09:39:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=chromium.org Received: by mail-oi1-f173.google.com with SMTP id 5614622812f47-3bb53e20a43so4742282b6e.1 for <linux-kernel@vger.kernel.org>; Thu, 28 Dec 2023 01:39:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1703756388; x=1704361188; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=cE0PHPIX9V+Td6mS/CWtPhPTpSAbOD3GNR/J2y9uxsE=; b=HGtaXCTWSc5xmYeGmFWcNgiFhRBXHHeg4yGCp0vs0GOy4mpHthhwmQ7ejvF3FBs3+r UDEnutDOzcUCoytdQtiTg1EsQmjM7+AT9Ti4HPreWU/UIe1cdmmNtiHkBUoa/GE7kIlK fZtczkp3dDgOm0KScLJspmINHrYqVYpV49ykE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1703756388; x=1704361188; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=cE0PHPIX9V+Td6mS/CWtPhPTpSAbOD3GNR/J2y9uxsE=; b=hhjblNQIar//V12AuTwLNgQQ2NkmBPkl8mTtARNBn90UfJMnNLtdBXWaxJtoM/T51v SUgsyBtRESCGyuRF54+xj855U6Kif2uFYvExGbT8/w2DztQEMEiF/Rmy5H4z9tTpSK9s KoyoXv/h6OUextg8yz13dl81JIdUAkOjx7n/njYJ4aWMGYjVRDfCzoGXjrEA9H2bLjwN u87uqLJvmVuvBSMprgib9E2KHpa0WMSfc16T5+w7N2+AeH5pAw6IJXjAuR6n4ONIJ5UL 6AIh49g9eXqBsmuVjWsPWliNxJ4AaOMn9hOLkqf6OSekLMLSzO4dlZpqWDkDRJ84BJ0j o/mA== X-Gm-Message-State: AOJu0YxcCZZ3yCAf3CQwBAv8yq4bOSmcSu92A386hv9xTarJyOaaG+5P eKYxLELWGzeg2ZQ8DdtfpK1liBzmSpP+ X-Received: by 2002:a05:6358:340f:b0:174:dc11:4827 with SMTP id h15-20020a056358340f00b00174dc114827mr5349757rwd.51.1703756388166; Thu, 28 Dec 2023 01:39:48 -0800 (PST) Received: from localhost ([2401:fa00:8f:203:5411:cc6a:8ad4:c73c]) by smtp.gmail.com with ESMTPSA id rs16-20020a17090b2b9000b00286da7407f2sm17910570pjb.7.2023.12.28.01.39.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Dec 2023 01:39:47 -0800 (PST) From: Hidenori Kobayashi <hidenorik@chromium.org> To: Sakari Ailus <sakari.ailus@linux.intel.com>, Bingbu Cao <bingbu.cao@intel.com>, Tianshu Qiu <tian.shu.qiu@intel.com>, Mauro Carvalho Chehab <mchehab@kernel.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Hidenori Kobayashi <hidenorik@chromium.org>, linux-media@vger.kernel.org, linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Subject: [PATCH] media: staging: ipu3-imgu: Set fields before media_entity_pads_init() Date: Thu, 28 Dec 2023 18:39:25 +0900 Message-ID: <20231228093926.748001-1-hidenorik@chromium.org> X-Mailer: git-send-email 2.43.0.472.g3155946c3a-goog Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1786518080726162855 X-GMAIL-MSGID: 1786518080726162855 |
Series |
media: staging: ipu3-imgu: Set fields before media_entity_pads_init()
|
|
Commit Message
Hidenori Kobayashi
Dec. 28, 2023, 9:39 a.m. UTC
The pad's flags is checked in media_entity_pads_init(), so it has to be
initialized beforehand. The ops initialization is also moved together
for readability.
Signed-off-by: Hidenori Kobayashi <hidenorik@chromium.org>
---
drivers/staging/media/ipu3/ipu3-v4l2.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
Comments
On Thu, Dec 28, 2023 at 06:39:25PM +0900, Hidenori Kobayashi wrote: > The pad's flags is checked in media_entity_pads_init(), so it has to be > initialized beforehand. The ops initialization is also moved together > for readability. > How does this bug look like to a user? What is the Fixes tag? Does this need to be backported to stable? regards, dan carpenter
On Thu, Jan 04, 2024 at 01:04:27PM +0300, Dan Carpenter wrote: > On Thu, Dec 28, 2023 at 06:39:25PM +0900, Hidenori Kobayashi wrote: > > The pad's flags is checked in media_entity_pads_init(), so it has to be > > initialized beforehand. The ops initialization is also moved together > > for readability. > > > > How does this bug look like to a user? What is the Fixes tag? Does > this need to be backported to stable? I suppose I should have included those in the commit message. 1) To a user, the imgu driver fails to probe with the following message: [ 14.596315] ipu3-imgu 0000:00:05.0: failed initialize subdev media entity (-22) [ 14.596322] ipu3-imgu 0000:00:05.0: failed to register subdev0 ret (-22) [ 14.596327] ipu3-imgu 0000:00:05.0: failed to register pipes (-22) [ 14.596331] ipu3-imgu 0000:00:05.0: failed to create V4L2 devices (-22) 2) Re Fixes tag, I see that the first commit of imgu driver already initializes the flags after media_entity_pads_init(). The documentation of this API ( "Drivers must set the direction of every pad ... before calling media_entity_pads_init") predates the first commit. So, I guess Fixes: a0ca1627b450 ("media: staging/intel-ipu3: Add v4l2 driver based on media framework") 3) Re stable, I was not sure. The probe failure only appears after a check was added by Commit deb866f9e3a45ae058b21765feeffae6aea6a193. That check is not in linux-6.6.y branch. So I was not sure if this counts as "a real bug that bothers people" mentioned in the document. With the above, how does the following sounds to you? The imgu driver fails to probe because it does not set the pad's flags before calling media_entity_pads_init(). Fix the initialization order so that the driver probe succeeds. The ops initialization is also moved together for readability. Fixes: a0ca1627b450 ("media: staging/intel-ipu3: Add v4l2 driver based on media framework") Thanks for the guidance, Hidenori
On Fri, Jan 05, 2024 at 11:19:23AM +0900, Hidenori Kobayashi wrote: > On Thu, Jan 04, 2024 at 01:04:27PM +0300, Dan Carpenter wrote: > > On Thu, Dec 28, 2023 at 06:39:25PM +0900, Hidenori Kobayashi wrote: > > > The pad's flags is checked in media_entity_pads_init(), so it has to be > > > initialized beforehand. The ops initialization is also moved together > > > for readability. > > > > > > > How does this bug look like to a user? What is the Fixes tag? Does > > this need to be backported to stable? > > I suppose I should have included those in the commit message. > > 1) To a user, the imgu driver fails to probe with the following message: > > [ 14.596315] ipu3-imgu 0000:00:05.0: failed initialize subdev media entity (-22) > [ 14.596322] ipu3-imgu 0000:00:05.0: failed to register subdev0 ret (-22) > [ 14.596327] ipu3-imgu 0000:00:05.0: failed to register pipes (-22) > [ 14.596331] ipu3-imgu 0000:00:05.0: failed to create V4L2 devices (-22) > Yeah. This is super useful information. > 2) Re Fixes tag, I see that the first commit of imgu driver already > initializes the flags after media_entity_pads_init(). The documentation > of this API ( "Drivers must set the direction of every pad ... before > calling media_entity_pads_init") predates the first commit. So, I guess > > Fixes: a0ca1627b450 ("media: staging/intel-ipu3: Add v4l2 driver based on media framework") > > 3) Re stable, I was not sure. The probe failure only appears after a > check was added by Commit deb866f9e3a45ae058b21765feeffae6aea6a193. That > check is not in linux-6.6.y branch. So I was not sure if this counts as > "a real bug that bothers people" mentioned in the document. Hm... I don't know either. Wait for a day and see if anyone else has an opinion then listen to your gut and resend with whatever your gut says? regards, dan carpenter
Hi Dan, Hidenori, On Fri, Jan 05, 2024 at 10:34:20AM +0300, Dan Carpenter wrote: > On Fri, Jan 05, 2024 at 11:19:23AM +0900, Hidenori Kobayashi wrote: > > On Thu, Jan 04, 2024 at 01:04:27PM +0300, Dan Carpenter wrote: > > > On Thu, Dec 28, 2023 at 06:39:25PM +0900, Hidenori Kobayashi wrote: > > > > The pad's flags is checked in media_entity_pads_init(), so it has to be > > > > initialized beforehand. The ops initialization is also moved together > > > > for readability. > > > > > > > > > > How does this bug look like to a user? What is the Fixes tag? Does > > > this need to be backported to stable? > > > > I suppose I should have included those in the commit message. > > > > 1) To a user, the imgu driver fails to probe with the following message: > > > > [ 14.596315] ipu3-imgu 0000:00:05.0: failed initialize subdev media entity (-22) > > [ 14.596322] ipu3-imgu 0000:00:05.0: failed to register subdev0 ret (-22) > > [ 14.596327] ipu3-imgu 0000:00:05.0: failed to register pipes (-22) > > [ 14.596331] ipu3-imgu 0000:00:05.0: failed to create V4L2 devices (-22) > > > > Yeah. This is super useful information. > > > 2) Re Fixes tag, I see that the first commit of imgu driver already > > initializes the flags after media_entity_pads_init(). The documentation > > of this API ( "Drivers must set the direction of every pad ... before > > calling media_entity_pads_init") predates the first commit. So, I guess > > > > Fixes: a0ca1627b450 ("media: staging/intel-ipu3: Add v4l2 driver based on media framework") > > > > 3) Re stable, I was not sure. The probe failure only appears after a > > check was added by Commit deb866f9e3a45ae058b21765feeffae6aea6a193. That > > check is not in linux-6.6.y branch. So I was not sure if this counts as > > "a real bug that bothers people" mentioned in the document. > > Hm... I don't know either. Wait for a day and see if anyone else has > an opinion then listen to your gut and resend with whatever your gut > says? The suggested commit message seems good to me, the Fixes: line is important. Thanks for digging this up! The patch should go to v6.7, too, so please add Cc: stable (for v6.7).
Hi Dan, Sakari, On Mon, Jan 08, 2024 at 08:59:37AM +0000, Sakari Ailus wrote: > Hi Dan, Hidenori, > > On Fri, Jan 05, 2024 at 10:34:20AM +0300, Dan Carpenter wrote: > > Hm... I don't know either. Wait for a day and see if anyone else has > > an opinion then listen to your gut and resend with whatever your gut > > says? > > The suggested commit message seems good to me, the Fixes: line is > important. Thanks for digging this up! The patch should go to v6.7, too, so > please add Cc: stable (for v6.7). Thanks for the comments! I'll send v2 with the revised commit message, with Cc: stable. Hidenori
diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c b/drivers/staging/media/ipu3/ipu3-v4l2.c index a66f034380c0..3df58eb3e882 100644 --- a/drivers/staging/media/ipu3/ipu3-v4l2.c +++ b/drivers/staging/media/ipu3/ipu3-v4l2.c @@ -1069,6 +1069,11 @@ static int imgu_v4l2_subdev_register(struct imgu_device *imgu, struct imgu_media_pipe *imgu_pipe = &imgu->imgu_pipe[pipe]; /* Initialize subdev media entity */ + imgu_sd->subdev.entity.ops = &imgu_media_ops; + for (i = 0; i < IMGU_NODE_NUM; i++) { + imgu_sd->subdev_pads[i].flags = imgu_pipe->nodes[i].output ? + MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE; + } r = media_entity_pads_init(&imgu_sd->subdev.entity, IMGU_NODE_NUM, imgu_sd->subdev_pads); if (r) { @@ -1076,11 +1081,6 @@ static int imgu_v4l2_subdev_register(struct imgu_device *imgu, "failed initialize subdev media entity (%d)\n", r); return r; } - imgu_sd->subdev.entity.ops = &imgu_media_ops; - for (i = 0; i < IMGU_NODE_NUM; i++) { - imgu_sd->subdev_pads[i].flags = imgu_pipe->nodes[i].output ? - MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE; - } /* Initialize subdev */ v4l2_subdev_init(&imgu_sd->subdev, &imgu_subdev_ops); @@ -1177,15 +1177,15 @@ static int imgu_v4l2_node_setup(struct imgu_device *imgu, unsigned int pipe, } /* Initialize media entities */ + node->vdev_pad.flags = node->output ? + MEDIA_PAD_FL_SOURCE : MEDIA_PAD_FL_SINK; + vdev->entity.ops = NULL; r = media_entity_pads_init(&vdev->entity, 1, &node->vdev_pad); if (r) { dev_err(dev, "failed initialize media entity (%d)\n", r); mutex_destroy(&node->lock); return r; } - node->vdev_pad.flags = node->output ? - MEDIA_PAD_FL_SOURCE : MEDIA_PAD_FL_SINK; - vdev->entity.ops = NULL; /* Initialize vbq */ vbq->type = node->vdev_fmt.type;