From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: kernel test robot <lkp@intel.com>,
devicetree@vger.kernel.org, linux-acpi@vger.kernel.org,
netdev@vger.kernel.org
Cc: llvm@lists.linux.dev, oe-kbuild-all@lists.linux.dev
Subject: Re: [PATCH net-next v2] net: sfp: clean up i2c-bus property parsing
Date: Sat, 3 Dec 2022 17:24:31 +0000 [thread overview]
Message-ID: <Y4uGT19d1Euz75Vd@shell.armlinux.org.uk> (raw)
In-Reply-To: <202212040026.WN9NQzqq-lkp@intel.com>
On Sun, Dec 04, 2022 at 12:40:25AM +0800, kernel test robot wrote:
> Thank you for the patch! Perhaps something to improve:
Sigh... this is another stupidity.
> 73970055450eeb Russell King 2017-07-25 2701 if (pdev->dev.of_node) {
> 73970055450eeb Russell King 2017-07-25 @2702 struct device_node *node = pdev->dev.of_node;
"node" declared here...
> 259c8618b0099b Russell King 2017-12-14 2703 const struct of_device_id *id;
> 73970055450eeb Russell King 2017-07-25 2704
> 259c8618b0099b Russell King 2017-12-14 2705 id = of_match_node(sfp_of_match, node);
... and clearly used here, so the code looks to be correct.
However, when CONFIG_OF is not set, of_match_node() does not make use
of this argument:
#define of_match_node(_matches, _node) NULL
which results in otherwise correct code issuing a warning when
CONFIG_OF is disabled... and sure enough, your configuration has:
> # CONFIG_OF is not set
This illustrates just how bad an idea it is to use compiler macros for
this stuff - it actively hurts compile testing, because you have to
test every damn combination of configuration options to get proper
coverage, which is totally and utterly rediculous.
of_match_node() and ACPI_HANDLE_FWNODE() should *both* be inline
functions when the subsystem is disabled, so that incorrect arguments
can be detected, and warnings about unused variables such as the one
you're reporting here doesn't happen.
While the issue lies firmly in the realms of the DT (and ACPI) headers,
I will yet again respin this patch to sort this out - but really the
correct solution is to fix the bloody headers so compile coverage
actually works.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
parent reply other threads:[~2022-12-03 17:24 UTC|newest]
Thread overview: expand[flat|nested] mbox.gz Atom feed
[parent not found: <202212040026.WN9NQzqq-lkp@intel.com>]
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=Y4uGT19d1Euz75Vd@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=devicetree@vger.kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=lkp@intel.com \
--cc=llvm@lists.linux.dev \
--cc=netdev@vger.kernel.org \
--cc=oe-kbuild-all@lists.linux.dev \
/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;
as well as URLs for NNTP newsgroup(s).