From: Russell King - ARM Linux <linux@arm.linux.org.uk> To: Olof Johansson <olof@lixom.net> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Arnd Bergmann <arnd@arndb.de>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Emilio Lopez <emilio@elopez.com.ar>, torvalds@linux-foundation.org Subject: Re: [PATCH 0/2] ARM: sunxi: Convert DTSI to new CPU bindings Date: Sun, 30 Jun 2013 00:14:26 +0100 [thread overview] Message-ID: <20130629231425.GG3353@n2100.arm.linux.org.uk> (raw) In-Reply-To: <20130629225426.GA12221@quad.lixom.net> On Sat, Jun 29, 2013 at 03:54:26PM -0700, Olof Johansson wrote: > Most of this ruffle seems to be about the fact that booting a kernel > with a device tree that doesn't conform to the brand spanking new, > and never previously enforced, binding for the cpu nodes will produce > a WARN_ON(). Lots of our in-tree device trees fall into this category. > > And while I think it was a bad idea for Lorenzo to ask for this to be > merged as a fix this late (and most in particular for stable), as far > as I can tell nothing (new) is broken by it -- just the alarming warning > is being printed. > > I think it probably makes sense to downgrade the WARN to just a printk, and > people will be a lot less worried. How about the below? > > If you're OK with it, Russell, can we get your ack so Linus can apply > directly given the imminence of final 3.10? Or, if you prefer, you can of > course apply and send it on instead. You can have my usual rmk+kernel ack for it with one change... > + if (!bootcpu_valid) { > + pr_warn("DT missing boot CPU MPIDR[23:0], fall back to " > + "default cpu_logical_map\n"); Don't wrap messages kernel messages inspite of what checkpatch says. Always keep messages like that on a single line so they're greppable. Checkpatch is far from perfect and does get stuff wrong, and this is one of its common mistakes. Incidentally, here's a few of fun ones I found today which illustrates why checkpatch can be bad news if everything it spits out is believed by the user: WARNING: simple_strtoul is obsolete, use kstrtoul instead #1424: FILE: drivers/gpu/drm/armada/armada_debugfs.c:90: + reg = simple_strtoul(buf, &p, 16); Umm yes, and to use kstrtoul(), I'd have to: - copy the string _safely_ to avoid any buffer overflow - find the first non-value character - terminate the string with a \0 or a \n\0 - remember where in the string I'd got to to parse the next argument And pushing that complexity into drivers, which if it's wrong causes security problems, is better than using simple_strtoul() because ...? ERROR: Macros with multiple statements should be enclosed in a do - while loop #2122: FILE: drivers/gpu/drm/armada/armada_fb.c:45: +#define FMT(drm, fmt, mod) \ + case DRM_FORMAT_##drm: \ + format = CFG_##fmt; \ + config = mod; \ + break Oh yea, that's really going to work for that isn't it! WARNING: externs should be avoided in .c files #2126: FILE: drivers/gpu/drm/armada/armada_fb.c:49: + break Err, what extern? :)
WARNING: multiple messages have this Message-ID (diff)
From: linux@arm.linux.org.uk (Russell King - ARM Linux) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 0/2] ARM: sunxi: Convert DTSI to new CPU bindings Date: Sun, 30 Jun 2013 00:14:26 +0100 [thread overview] Message-ID: <20130629231425.GG3353@n2100.arm.linux.org.uk> (raw) In-Reply-To: <20130629225426.GA12221@quad.lixom.net> On Sat, Jun 29, 2013 at 03:54:26PM -0700, Olof Johansson wrote: > Most of this ruffle seems to be about the fact that booting a kernel > with a device tree that doesn't conform to the brand spanking new, > and never previously enforced, binding for the cpu nodes will produce > a WARN_ON(). Lots of our in-tree device trees fall into this category. > > And while I think it was a bad idea for Lorenzo to ask for this to be > merged as a fix this late (and most in particular for stable), as far > as I can tell nothing (new) is broken by it -- just the alarming warning > is being printed. > > I think it probably makes sense to downgrade the WARN to just a printk, and > people will be a lot less worried. How about the below? > > If you're OK with it, Russell, can we get your ack so Linus can apply > directly given the imminence of final 3.10? Or, if you prefer, you can of > course apply and send it on instead. You can have my usual rmk+kernel ack for it with one change... > + if (!bootcpu_valid) { > + pr_warn("DT missing boot CPU MPIDR[23:0], fall back to " > + "default cpu_logical_map\n"); Don't wrap messages kernel messages inspite of what checkpatch says. Always keep messages like that on a single line so they're greppable. Checkpatch is far from perfect and does get stuff wrong, and this is one of its common mistakes. Incidentally, here's a few of fun ones I found today which illustrates why checkpatch can be bad news if everything it spits out is believed by the user: WARNING: simple_strtoul is obsolete, use kstrtoul instead #1424: FILE: drivers/gpu/drm/armada/armada_debugfs.c:90: + reg = simple_strtoul(buf, &p, 16); Umm yes, and to use kstrtoul(), I'd have to: - copy the string _safely_ to avoid any buffer overflow - find the first non-value character - terminate the string with a \0 or a \n\0 - remember where in the string I'd got to to parse the next argument And pushing that complexity into drivers, which if it's wrong causes security problems, is better than using simple_strtoul() because ...? ERROR: Macros with multiple statements should be enclosed in a do - while loop #2122: FILE: drivers/gpu/drm/armada/armada_fb.c:45: +#define FMT(drm, fmt, mod) \ + case DRM_FORMAT_##drm: \ + format = CFG_##fmt; \ + config = mod; \ + break Oh yea, that's really going to work for that isn't it! WARNING: externs should be avoided in .c files #2126: FILE: drivers/gpu/drm/armada/armada_fb.c:49: + break Err, what extern? :)
next prev parent reply other threads:[~2013-06-29 23:15 UTC|newest] Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-06-28 16:44 [PATCH 0/2] ARM: sunxi: Convert DTSI to new CPU bindings Maxime Ripard 2013-06-28 16:44 ` Maxime Ripard 2013-06-28 16:44 ` [PATCH 1/2] ARM: dts: sunxi: cpus/cpu nodes dts updates Maxime Ripard 2013-06-28 16:44 ` Maxime Ripard 2013-06-28 16:44 ` [PATCH 2/2] sunxi: a10s: dtsi: Convert cpu node to the new cpu bindings Maxime Ripard 2013-06-28 16:44 ` Maxime Ripard 2013-06-28 17:15 ` [PATCH 0/2] ARM: sunxi: Convert DTSI to new CPU bindings Lorenzo Pieralisi 2013-06-28 17:15 ` Lorenzo Pieralisi 2013-06-28 20:03 ` Maxime Ripard 2013-06-28 20:03 ` Maxime Ripard 2013-06-28 20:05 ` Olof Johansson 2013-06-28 20:05 ` Olof Johansson 2013-06-28 21:45 ` Lorenzo Pieralisi 2013-06-28 21:45 ` Lorenzo Pieralisi 2013-06-29 18:07 ` Maxime Ripard 2013-06-29 18:07 ` Maxime Ripard 2013-06-29 19:03 ` Lorenzo Pieralisi 2013-06-29 19:03 ` Lorenzo Pieralisi 2013-06-29 19:38 ` Russell King - ARM Linux 2013-06-29 19:38 ` Russell King - ARM Linux 2013-06-29 20:11 ` Lorenzo Pieralisi 2013-06-29 20:11 ` Lorenzo Pieralisi 2013-06-29 22:54 ` Olof Johansson 2013-06-29 22:54 ` Olof Johansson 2013-06-29 23:14 ` Russell King - ARM Linux [this message] 2013-06-29 23:14 ` Russell King - ARM Linux 2013-06-29 23:20 ` Olof Johansson 2013-06-29 23:20 ` Olof Johansson 2013-06-30 9:48 ` Lorenzo Pieralisi 2013-06-30 9:48 ` Lorenzo Pieralisi 2013-07-05 10:19 ` Lorenzo Pieralisi 2013-07-05 10:19 ` Lorenzo Pieralisi 2013-07-05 10:19 ` Lorenzo Pieralisi 2013-07-12 9:37 ` Maxime Ripard 2013-07-12 9:37 ` Maxime Ripard 2013-07-12 9:37 ` Maxime Ripard 2013-07-02 12:12 ` Nicolas Ferre 2013-07-02 12:12 ` Nicolas Ferre 2013-07-02 12:56 ` Lorenzo Pieralisi 2013-07-02 12:56 ` Lorenzo Pieralisi
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=20130629231425.GG3353@n2100.arm.linux.org.uk \ --to=linux@arm.linux.org.uk \ --cc=arnd@arndb.de \ --cc=emilio@elopez.com.ar \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=maxime.ripard@free-electrons.com \ --cc=olof@lixom.net \ --cc=torvalds@linux-foundation.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.