linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Paul Bolle <pebolle@tiscali.nl>
To: Valentin Rothberg <valentinrothberg@gmail.com>
Cc: smohammed@nvidia.com, treding@nvidia.com,
	Michal Marek <mmarek@suse.cz>,
	linux-kernel@vger.kernel.org
Subject: Re: i2c: undefined option I2C_ALGO_BUSCLEAR
Date: Thu, 17 Nov 2016 09:58:31 +0100	[thread overview]
Message-ID: <1479373111.19539.24.camel@tiscali.nl> (raw)
In-Reply-To: <CAD3Xx4+ieNPqsumoXuiTtpwxct-66ipJB8tb-Kfpe1njBc+OZQ@mail.gmail.com>

[Added Michal]

Hi Valentin,

On Wed, 2016-11-16 at 08:31 +0100, Valentin Rothberg wrote:
> your commit c3ca951fe41a ("i2c: Add Tegra BPMP I2C proxy driver")
> popped up in today's linux-next tree, adding Kconfig option
> I2C_TEGRA_BPMP, which further selects I2C_ALGO_BUSCLEAR, which is
> nowhere defined in Kconfig.
> 
> Is there a patch queued somewhere to add I2C_ALGO_BUSCLEAR to Kconfig?
>  I could not find anything on the lkml; only some older repositories
> on github, where the options is defined in drivers/i2c/busses/Kconfig.

Attached is a v2 of a patch I submitted way to long ago. It adds a warning for
exactly this issue. That should help others to spot them. (It's a bit smarter
than your check kconfig script as it also warns if you select symbols that are
outside of you arch's scope. Ie, not only for entirely unknown symbols.)

Please play with it. Unless you spot some issues I really should be properly
resubmitting it.

Thanks,


Paul Bolle

-------- >8 >8 >8 >8 -------
From: Paul Bolle <pebolle@tiscali.nl>
Subject: [PATCH] kconfig: warn if an unknown symbol is selected

A select of an unknown symbol is basically treated as a nop and is
silently skipped. This is annoying if the selected symbol contains a
typo. It can also hide the fact that a treewide symbol cleanup was only
done partially.

There are also a few cases were this might have been done on purpose.
But that anti-pattern should be discouraged. Almost all select
statements point to a known and reachable symbol. So people will likely
assume that any selected symbol is actually set. Selects that violate
this assumption can only be spotted by checking multiple Kconfig files,
often across architectures. It is unlikely that people will do this
regularily.

So let's warn when we notice a select of a symbol that is not known in
the configuration we're creating.

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
Acked-by: Michal Marek <mmarek@suse.cz> (v1)
---
 scripts/kconfig/conf.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 866369f10ff8..55a4dacb196d 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -447,6 +447,46 @@ static void check_conf(struct menu *menu)
 		check_conf(child);
 }
 
+static void check_selects(struct menu *menu)
+{
+	struct symbol *sym, *sel;
+	struct property *prop;
+
+	while (menu) {
+		sym = menu->sym;
+
+		if (sym && !sym_is_choice(sym) &&
+		    sym->type != S_UNKNOWN &&
+		    sym_get_tristate_value(sym) != no &&
+		    sym->flags & SYMBOL_WRITE) {
+			for_all_properties(sym, prop, P_SELECT) {
+				sel = prop->expr->left.sym;
+				if (sel->type == S_UNKNOWN &&
+				    expr_calc_value(prop->visible.expr) != no) {
+					fprintf(stderr, "%s:%d:warning: ",
+						prop->file->name,
+						prop->lineno);
+					fprintf(stderr,
+						"'%s' selects unknown symbol '%s'\n",
+						sym->name,
+						sel->name);
+				}
+			}
+		}
+
+		if (menu->list) {
+			menu = menu->list;
+		} else if (menu->next) {
+			menu = menu->next;
+		} else while ((menu = menu->parent)) {
+			if (menu->next) {
+				menu = menu->next;
+				break;
+			}
+		}
+	}
+}
+
 static struct option long_opts[] = {
 	{"oldaskconfig",    no_argument,       NULL, oldaskconfig},
 	{"oldconfig",       no_argument,       NULL, oldconfig},
@@ -686,6 +726,8 @@ int main(int ac, char **av)
 		break;
 	}
 
+	check_selects(rootmenu.list);
+
 	if (sync_kconfig) {
 		/* silentoldconfig is used during the build so we shall update autoconf.
 		 * All other commands are only used to generate a config.
-- 
2.7.4

  parent reply	other threads:[~2016-11-17  8:58 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-16  7:31 i2c: undefined option I2C_ALGO_BUSCLEAR Valentin Rothberg
2016-11-16 15:39 ` Thierry Reding
2016-11-16 15:43   ` Thierry Reding
2016-11-17  8:40     ` Shardar Mohammed
2016-11-17  8:58 ` Paul Bolle [this message]
2016-11-17 11:33   ` Valentin Rothberg
2016-11-17 11:55     ` Paul Bolle
2016-11-17 12:44       ` Valentin Rothberg

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=1479373111.19539.24.camel@tiscali.nl \
    --to=pebolle@tiscali.nl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mmarek@suse.cz \
    --cc=smohammed@nvidia.com \
    --cc=treding@nvidia.com \
    --cc=valentinrothberg@gmail.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;
as well as URLs for NNTP newsgroup(s).