From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: "Linux Media Mailing List" <linux-media@vger.kernel.org>,
"Mauro Carvalho Chehab" <mchehab@infradead.org>,
"Hyun Kwon" <hyun.kwon@xilinx.com>,
"Michal Simek" <michal.simek@xilinx.com>,
"Sören Brinkmann" <soren.brinkmann@xilinx.com>,
linux-arm-kernel@lists.infradead.org,
"Rob Herring" <robh@kernel.org>,
"Sakari Ailus" <sakari.ailus@linux.intel.com>
Subject: Re: [PATCH v2 14/26] media: xilinx: fix a debug printk
Date: Thu, 02 Nov 2017 04:43:51 +0200 [thread overview]
Message-ID: <2117711.dO2rQLXOup@avalon> (raw)
In-Reply-To: <be86653c5e5641582f65f43780b1fe255e92cdc0.1509569763.git.mchehab@s-opensource.com>
Hi Mauro,
(CC'ing Rob and Sakari)
Thank you for the patch.
On Wednesday, 1 November 2017 23:05:51 EET Mauro Carvalho Chehab wrote:
> Two orthogonal changesets caused a breakage at several printk
> messages inside xilinx. Changeset 859969b38e2e
> ("[media] v4l: Switch from V4L2 OF not V4L2 fwnode API")
> made davinci to use struct fwnode_handle instead of
> struct device_node. Changeset 68d9c47b1679
> ("media: Convert to using %pOF instead of full_name")
> changed the printk to not use ->full_name, but, instead,
> to rely on %pOF.
>
> With both patches applied, the Kernel will do the wrong
> thing, as warned by smatch:
> drivers/media/platform/xilinx/xilinx-vipp.c:108 xvip_graph_build_one()
> error: '%pOF' expects argument of type 'struct device_node*', argument 4
> has type 'void*'
> drivers/media/platform/xilinx/xilinx-vipp.c:117 xvip_graph_build_one()
> error: '%pOF' expects argument of type 'struct device_node*', argument 4 has
> type 'void*'
> drivers/media/platform/xilinx/xilinx-vipp.c:126 xvip_graph_build_one()
> error: '%pOF' expects argument of type 'struct device_node*', argument 4
> has type 'void*'
> drivers/media/platform/xilinx/xilinx-vipp.c:138 xvip_graph_build_one()
> error: '%pOF' expects argument of type 'struct device_node*', argument 3 has
> type 'void*'
> drivers/media/platform/xilinx/xilinx-vipp.c:148 xvip_graph_build_one()
> error: '%pOF' expects argument of type 'struct device_node*', argument 4
> has type 'void*'
> drivers/media/platform/xilinx/xilinx-vipp.c:245 xvip_graph_build_dma()
> error: '%pOF' expects argument of type 'struct device_node*', argument 3 has
> type 'void*'
> drivers/media/platform/xilinx/xilinx-vipp.c:254 xvip_graph_build_dma()
> error: '%pOF' expects argument of type 'struct device_node*', argument 4
> has type 'void*'
>
> So, change the logic to actually print the device name
> that was obtained before the print logic.
This doesn't seem like a good idea to me. The main point of commit
68d9c47b1679 is to avoid accessing full_name directly to prepare for removal
of that field.
to_of_node() is defined as
#define to_of_node(__fwnode) \
({ \
typeof(__fwnode) __to_of_node_fwnode = (__fwnode); \
\
is_of_node(__to_of_node_fwnode) ? \
container_of(__to_of_node_fwnode, \
struct device_node, fwnode) : \
NULL; \
})
when CONFIG_OF is set, and as
static inline struct device_node *to_of_node(const struct fwnode_handle
*fwnode)
{
return NULL;
}
otherwise. I wonder why smatch believes the value is a void *, but in any case
that should be fixed either in smatch (if it's a smatch bug) or in the
definition of the to_of_node() macro.
> Fixes: 68d9c47b1679 ("media: Convert to using %pOF instead of full_name")
> Fixes: 859969b38e2e ("[media] v4l: Switch from V4L2 OF not V4L2 fwnode API")
When submitting fixes you should CC the author of the original commits. They
should have more information about the context, and in this case I believe Rob
would have pointed out that adding back references to full_name would break
the refactoring he's working on.
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
> drivers/media/platform/xilinx/xilinx-vipp.c | 31 +++++++++++++-------------
> 1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c
> b/drivers/media/platform/xilinx/xilinx-vipp.c index
> d881cf09876d..dd777c834c43 100644
> --- a/drivers/media/platform/xilinx/xilinx-vipp.c
> +++ b/drivers/media/platform/xilinx/xilinx-vipp.c
> @@ -65,6 +65,9 @@ xvip_graph_find_entity(struct xvip_composite_device *xdev,
> return NULL;
> }
>
> +#define LOCAL_NAME(link) to_of_node(link.local_node)->full_name
> +#define REMOTE_NAME(link) to_of_node(link.remote_node)->full_name
> +
> static int xvip_graph_build_one(struct xvip_composite_device *xdev,
> struct xvip_graph_entity *entity)
> {
> @@ -103,9 +106,9 @@ static int xvip_graph_build_one(struct
> xvip_composite_device *xdev,
> * the link.
> */
> if (link.local_port >= local->num_pads) {
> - dev_err(xdev->dev, "invalid port number %u for %pOF\n",
> + dev_err(xdev->dev, "invalid port number %u for %s\n",
> link.local_port,
> - to_of_node(link.local_node));
> + LOCAL_NAME(link));
> v4l2_fwnode_put_link(&link);
> ret = -EINVAL;
> break;
> @@ -114,8 +117,8 @@ static int xvip_graph_build_one(struct
> xvip_composite_device *xdev, local_pad = &local->pads[link.local_port];
>
> if (local_pad->flags & MEDIA_PAD_FL_SINK) {
> - dev_dbg(xdev->dev, "skipping sink port %pOF:%u\n",
> - to_of_node(link.local_node),
> + dev_dbg(xdev->dev, "skipping sink port %s:%u\n",
> + LOCAL_NAME(link),
> link.local_port);
> v4l2_fwnode_put_link(&link);
> continue;
> @@ -123,8 +126,8 @@ static int xvip_graph_build_one(struct
> xvip_composite_device *xdev,
>
> /* Skip DMA engines, they will be processed separately. */
> if (link.remote_node == of_fwnode_handle(xdev->dev->of_node)) {
> - dev_dbg(xdev->dev, "skipping DMA port %pOF:%u\n",
> - to_of_node(link.local_node),
> + dev_dbg(xdev->dev, "skipping DMA port %s:%u\n",
> + REMOTE_NAME(link),
> link.local_port);
> v4l2_fwnode_put_link(&link);
> continue;
> @@ -134,8 +137,8 @@ static int xvip_graph_build_one(struct
> xvip_composite_device *xdev, ent = xvip_graph_find_entity(xdev,
> to_of_node(link.remote_node));
> if (ent == NULL) {
> - dev_err(xdev->dev, "no entity found for %pOF\n",
> - to_of_node(link.remote_node));
> + dev_err(xdev->dev, "no entity found for %s\n",
> + REMOTE_NAME(link));
> v4l2_fwnode_put_link(&link);
> ret = -ENODEV;
> break;
> @@ -144,8 +147,8 @@ static int xvip_graph_build_one(struct
> xvip_composite_device *xdev, remote = ent->entity;
>
> if (link.remote_port >= remote->num_pads) {
> - dev_err(xdev->dev, "invalid port number %u on %pOF\n",
> - link.remote_port, to_of_node(link.remote_node));
> + dev_err(xdev->dev, "invalid port number %u on %s\n",
> + link.remote_port, REMOTE_NAME(link));
> v4l2_fwnode_put_link(&link);
> ret = -EINVAL;
> break;
> @@ -241,17 +244,17 @@ static int xvip_graph_build_dma(struct
> xvip_composite_device *xdev) ent = xvip_graph_find_entity(xdev,
> to_of_node(link.remote_node));
> if (ent == NULL) {
> - dev_err(xdev->dev, "no entity found for %pOF\n",
> - to_of_node(link.remote_node));
> + dev_err(xdev->dev, "no entity found for %s\n",
> + REMOTE_NAME(link));
> v4l2_fwnode_put_link(&link);
> ret = -ENODEV;
> break;
> }
>
> if (link.remote_port >= ent->entity->num_pads) {
> - dev_err(xdev->dev, "invalid port number %u on %pOF\n",
> + dev_err(xdev->dev, "invalid port number %u on %s\n",
> link.remote_port,
> - to_of_node(link.remote_node));
> + REMOTE_NAME(link));
> v4l2_fwnode_put_link(&link);
> ret = -EINVAL;
> break;
--
Regards,
Laurent Pinchart
next prev parent reply other threads:[~2017-11-02 2:43 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-01 20:56 [PATCH v2 01/26] media: atmel-isc: avoid returning a random value at isc_parse_dt() Mauro Carvalho Chehab
2017-11-01 20:59 ` Mauro Carvalho Chehab
2017-11-01 21:03 ` Mauro Carvalho Chehab
2017-11-01 21:07 ` Mauro Carvalho Chehab
2017-11-01 21:05 ` [PATCH v2 02/26] media: dvb_frontend: be sure to init dvb_frontend_handle_ioctl() return code Mauro Carvalho Chehab
2017-11-07 18:29 ` Daniel Scheller
2017-11-01 21:05 ` [PATCH v2 03/26] media: led-class-flash: better handle NULL flash struct Mauro Carvalho Chehab
2017-11-02 9:08 ` Sakari Ailus
2017-11-01 21:05 ` [PATCH v2 04/26] media: tda8290: initialize agc gain Mauro Carvalho Chehab
2017-11-01 21:05 ` [PATCH v2 05/26] media: s5c73m3-core: fix logic on a timeout condition Mauro Carvalho Chehab
2017-11-02 7:12 ` Andrzej Hajda
2017-11-01 21:05 ` [PATCH v2 06/26] media: xc5000: better handle I2C error messages Mauro Carvalho Chehab
2017-11-01 21:05 ` [PATCH v2 07/26] media: radio-si476x: fix behavior when seek->range* are defined Mauro Carvalho Chehab
2017-11-01 21:05 ` [PATCH v2 08/26] media: v4l2-async: shut up an unitialized symbol warning Mauro Carvalho Chehab
2017-11-02 2:51 ` Laurent Pinchart
2017-11-02 8:49 ` Sakari Ailus
2017-12-11 18:10 ` Mauro Carvalho Chehab
2017-12-11 22:13 ` Laurent Pinchart
2017-12-14 12:12 ` Sakari Ailus
2017-11-01 21:05 ` [PATCH v2 09/26] media: cx25821-alsa: fix usage of a pointer printk Mauro Carvalho Chehab
2017-11-01 21:05 ` [PATCH v2 10/26] media: xc4000: don't ignore error if hwmodel fails Mauro Carvalho Chehab
2017-11-01 21:05 ` [PATCH v2 11/26] media: qt1010: fix bogus warnings Mauro Carvalho Chehab
2017-11-01 21:05 ` [PATCH v2 12/26] media: davinci: fix a debug printk Mauro Carvalho Chehab
2017-11-09 21:33 ` Lad, Prabhakar
2017-11-01 21:05 ` [PATCH v2 13/26] media: rcar: " Mauro Carvalho Chehab
2017-11-02 8:15 ` Niklas Söderlund
2017-11-01 21:05 ` [PATCH v2 14/26] media: xilinx: " Mauro Carvalho Chehab
2017-11-02 2:43 ` Laurent Pinchart [this message]
2017-11-02 9:20 ` Sakari Ailus
2017-11-02 9:57 ` [PATCH 1/1] of: Make return types of to_of_node and of_fwnode_handle macros consistent Sakari Ailus
2017-11-02 9:59 ` [RESEND PATCH " Sakari Ailus
2017-11-02 17:37 ` Laurent Pinchart
2017-11-06 21:45 ` Rob Herring
2017-11-01 21:05 ` [PATCH v2 15/26] media: pt1: fix logic when pt1_nr_tables is zero or negative Mauro Carvalho Chehab
2017-11-01 21:05 ` [PATCH v2 16/26] media: drxd_hard: better handle I2C errors Mauro Carvalho Chehab
2017-11-01 21:05 ` [PATCH v2 17/26] media: mxl111sf: improve error handling logic Mauro Carvalho Chehab
2017-11-01 21:05 ` [PATCH v2 18/26] media: dvbsky: shut up a bogus warning Mauro Carvalho Chehab
2017-11-01 21:05 ` [PATCH v2 19/26] media: ov9650: fix bogus warnings Mauro Carvalho Chehab
2017-11-02 10:06 ` Nicholas Mc Guire
2017-11-02 10:22 ` Nicholas Mc Guire
2017-11-01 21:05 ` [PATCH v2 20/26] media: imx274: don't randomly return if range_count is zero Mauro Carvalho Chehab
2017-11-01 21:05 ` [PATCH v2 21/26] media: m88rs2000: handle the case where tuner doesn't have get_frequency Mauro Carvalho Chehab
2017-11-01 21:05 ` [PATCH v2 22/26] [RFC] media: cxd2841er: ensure that status will always be available Mauro Carvalho Chehab
2017-11-01 21:06 ` [PATCH v2 23/26] media: drxj: better handle errors Mauro Carvalho Chehab
2017-11-01 21:06 ` [PATCH v2 24/26] media: stv090x: Only print tuner lock if get_status is available Mauro Carvalho Chehab
2017-11-01 21:06 ` [PATCH v2 25/26] media: mb86a16: be more resilient if I2C fails on sync Mauro Carvalho Chehab
2017-11-01 21:06 ` [PATCH v2 26/26] media: mb86a16: avoid division by zero Mauro Carvalho Chehab
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2117711.dO2rQLXOup@avalon \
--to=laurent.pinchart@ideasonboard.com \
--cc=hyun.kwon@xilinx.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@infradead.org \
--cc=mchehab@s-opensource.com \
--cc=michal.simek@xilinx.com \
--cc=robh@kernel.org \
--cc=sakari.ailus@linux.intel.com \
--cc=soren.brinkmann@xilinx.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox