From: "H. Nikolaus Schaller" <hns@goldelico.com> To: Johan Hovold <johan@kernel.org> Cc: "Rob Herring" <robh+dt@kernel.org>, "Mark Rutland" <mark.rutland@arm.com>, "Benoît Cousson" <bcousson@baylibre.com>, "Tony Lindgren" <tony@atomide.com>, "Russell King" <linux@armlinux.org.uk>, "Arnd Bergmann" <arnd@arndb.de>, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, "Kevin Hilman" <khilman@baylibre.com>, "Andreas Färber" <afaerber@suse.de>, "Thierry Reding" <treding@nvidia.com>, "Jonathan Cameron" <jic23@kernel.org>, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org, letux-kernel@openphoenux.org, kernel@pyra-handheld.com, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v5 5/5] misc serdev: w2sg0004: add debugging code and Kconfig Date: Fri, 22 Dec 2017 15:41:24 +0100 [thread overview] Message-ID: <A2720858-5172-4627-8133-C677E4B4DC72@goldelico.com> (raw) In-Reply-To: <20171222125150.GJ3374@localhost> > Am 22.12.2017 um 13:51 schrieb Johan Hovold <johan@kernel.org>: > > On Fri, Dec 01, 2017 at 08:49:38AM +0100, H. Nikolaus Schaller wrote: >> This allows to set CONFIG_W2SG0004_DEBUG which will >> make the driver report more activities and it will turn on the >> GPS module during boot while the driver assumes that it >> is off. This can be used to debug the correct functioning of >> the hardware. Therefore we add it as an option to the driver >> because we think it is of general use (and a little tricky to >> add by system testers). >> >> Normally it should be off. >> >> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> >> --- >> drivers/misc/Kconfig | 8 ++++++++ >> drivers/misc/w2sg0004.c | 37 +++++++++++++++++++++++++++++++++++++ >> 2 files changed, 45 insertions(+) > > I'd say say this does not belong in the kernel at all. There are other examples of such DEBUG options for drivers. E.g. CONFIG_LIBERTAS_DEBUG. It helps the hardware developer to test things and should of course be disabled in a production kernel. During hardware bringup I am always happy if someone else has shared such tools instead of omitting them. So that we do not have to invent them (again) and hack into our own kernel. Of course, this should be discussed and you are open to take it or leave it. > And even if the > power-state test were to be allowed, most of the pr_debugs would > need to go. I see. "/* If you are writing a driver, please use dev_dbg instead */" Well, nobody actively reads this comment in the header file... Maybe checkpatch could be teached to warn? > You really should be using dev_dbg and friends, which can > already be enabled selectively at run time using dynamic debugging. Well, in this case it is not needed to dynamically turn it on/off. BR and thanks, Nikolaus Schaller
WARNING: multiple messages have this Message-ID (diff)
From: hns@goldelico.com (H. Nikolaus Schaller) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v5 5/5] misc serdev: w2sg0004: add debugging code and Kconfig Date: Fri, 22 Dec 2017 15:41:24 +0100 [thread overview] Message-ID: <A2720858-5172-4627-8133-C677E4B4DC72@goldelico.com> (raw) In-Reply-To: <20171222125150.GJ3374@localhost> > Am 22.12.2017 um 13:51 schrieb Johan Hovold <johan@kernel.org>: > > On Fri, Dec 01, 2017 at 08:49:38AM +0100, H. Nikolaus Schaller wrote: >> This allows to set CONFIG_W2SG0004_DEBUG which will >> make the driver report more activities and it will turn on the >> GPS module during boot while the driver assumes that it >> is off. This can be used to debug the correct functioning of >> the hardware. Therefore we add it as an option to the driver >> because we think it is of general use (and a little tricky to >> add by system testers). >> >> Normally it should be off. >> >> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> >> --- >> drivers/misc/Kconfig | 8 ++++++++ >> drivers/misc/w2sg0004.c | 37 +++++++++++++++++++++++++++++++++++++ >> 2 files changed, 45 insertions(+) > > I'd say say this does not belong in the kernel at all. There are other examples of such DEBUG options for drivers. E.g. CONFIG_LIBERTAS_DEBUG. It helps the hardware developer to test things and should of course be disabled in a production kernel. During hardware bringup I am always happy if someone else has shared such tools instead of omitting them. So that we do not have to invent them (again) and hack into our own kernel. Of course, this should be discussed and you are open to take it or leave it. > And even if the > power-state test were to be allowed, most of the pr_debugs would > need to go. I see. "/* If you are writing a driver, please use dev_dbg instead */" Well, nobody actively reads this comment in the header file... Maybe checkpatch could be teached to warn? > You really should be using dev_dbg and friends, which can > already be enabled selectively at run time using dynamic debugging. Well, in this case it is not needed to dynamically turn it on/off. BR and thanks, Nikolaus Schaller
next prev parent reply other threads:[~2017-12-22 14:41 UTC|newest] Thread overview: 100+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-12-01 7:49 [PATCH v5 0/5] misc serdev: new serdev based driver for Wi2Wi w2sg00x4 GPS module H. Nikolaus Schaller 2017-12-01 7:49 ` H. Nikolaus Schaller 2017-12-01 7:49 ` [PATCH v5 1/5] dt-bindings: define vendor prefix for Wi2Wi, Inc H. Nikolaus Schaller 2017-12-01 7:49 ` H. Nikolaus Schaller 2017-12-01 14:44 ` Andreas Färber 2017-12-01 14:44 ` Andreas Färber 2017-12-01 7:49 ` [PATCH v5 2/5] dt-bindings: gps: add w2sg00x4 bindings documentation (GPS module with UART)) H. Nikolaus Schaller 2017-12-01 7:49 ` H. Nikolaus Schaller 2017-12-01 7:49 ` H. Nikolaus Schaller 2017-12-01 7:49 ` [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver H. Nikolaus Schaller 2017-12-01 7:49 ` H. Nikolaus Schaller 2017-12-01 7:49 ` H. Nikolaus Schaller 2017-12-22 12:44 ` Johan Hovold 2017-12-22 12:44 ` Johan Hovold 2017-12-22 12:44 ` Johan Hovold 2017-12-22 14:40 ` H. Nikolaus Schaller 2017-12-22 14:40 ` H. Nikolaus Schaller 2017-12-22 14:40 ` H. Nikolaus Schaller 2018-01-09 11:55 ` [Letux-kernel] " H. Nikolaus Schaller 2018-01-09 11:55 ` H. Nikolaus Schaller 2018-01-09 11:55 ` H. Nikolaus Schaller 2018-01-12 15:39 ` Johan Hovold 2018-01-12 15:39 ` Johan Hovold 2018-01-12 15:39 ` Johan Hovold 2018-01-12 17:59 ` H. Nikolaus Schaller 2018-01-12 17:59 ` H. Nikolaus Schaller 2018-01-12 17:59 ` H. Nikolaus Schaller 2018-01-18 6:13 ` Johan Hovold 2018-01-18 6:13 ` Johan Hovold 2018-01-18 6:13 ` Johan Hovold 2018-01-18 13:43 ` H. Nikolaus Schaller 2018-01-18 13:43 ` H. Nikolaus Schaller 2018-01-18 13:43 ` H. Nikolaus Schaller 2018-03-07 15:53 ` H. Nikolaus Schaller 2018-03-07 15:53 ` H. Nikolaus Schaller 2018-03-07 15:53 ` H. Nikolaus Schaller 2018-03-19 14:05 ` Johan Hovold 2018-03-19 14:05 ` Johan Hovold 2018-03-19 14:05 ` Johan Hovold 2018-02-12 15:26 ` [Letux-kernel] " Pavel Machek 2018-02-12 15:26 ` Pavel Machek 2018-02-12 15:26 ` Pavel Machek 2018-02-27 7:04 ` Johan Hovold 2018-02-27 7:04 ` Johan Hovold 2018-02-27 7:04 ` Johan Hovold 2018-02-27 7:32 ` H. Nikolaus Schaller 2018-02-27 7:32 ` H. Nikolaus Schaller 2018-02-27 7:32 ` H. Nikolaus Schaller 2018-03-19 13:54 ` Johan Hovold 2018-03-19 13:54 ` Johan Hovold 2018-03-19 13:54 ` Johan Hovold 2018-03-20 14:49 ` H. Nikolaus Schaller 2018-03-20 14:49 ` H. Nikolaus Schaller 2018-03-20 14:49 ` H. Nikolaus Schaller 2018-03-26 13:59 ` Pavel Machek 2018-03-26 13:59 ` Pavel Machek 2018-02-27 18:38 ` Pavel Machek 2018-02-27 18:38 ` Pavel Machek 2018-02-27 18:38 ` Pavel Machek 2018-02-12 15:25 ` Pavel Machek 2018-02-12 15:25 ` Pavel Machek 2018-01-09 17:43 ` Andreas Kemnade 2018-01-09 17:43 ` Andreas Kemnade 2018-01-09 17:43 ` Andreas Kemnade 2018-01-12 14:46 ` Johan Hovold 2018-01-12 14:46 ` Johan Hovold 2018-01-12 18:40 ` Andreas Kemnade 2018-01-12 18:40 ` Andreas Kemnade 2018-01-12 18:40 ` Andreas Kemnade 2018-01-18 6:47 ` Johan Hovold 2018-01-18 6:47 ` Johan Hovold 2018-01-18 6:47 ` Johan Hovold 2018-03-08 6:16 ` Andreas Kemnade 2018-03-08 6:16 ` Andreas Kemnade 2018-03-19 14:02 ` Johan Hovold 2018-03-19 14:02 ` Johan Hovold 2017-12-01 7:49 ` [PATCH v5 4/5] DTS: gta04: add uart2 child node for w2sg00x4 H. Nikolaus Schaller 2017-12-01 7:49 ` H. Nikolaus Schaller 2017-12-21 14:53 ` Tony Lindgren 2017-12-21 14:53 ` Tony Lindgren 2017-12-21 14:53 ` Tony Lindgren 2017-12-01 7:49 ` [PATCH v5 5/5] misc serdev: w2sg0004: add debugging code and Kconfig H. Nikolaus Schaller 2017-12-01 7:49 ` H. Nikolaus Schaller 2017-12-01 7:49 ` H. Nikolaus Schaller 2017-12-22 12:51 ` Johan Hovold 2017-12-22 12:51 ` Johan Hovold 2017-12-22 12:51 ` Johan Hovold 2017-12-22 14:41 ` H. Nikolaus Schaller [this message] 2017-12-22 14:41 ` H. Nikolaus Schaller 2017-12-22 14:58 ` Greg Kroah-Hartman 2017-12-22 14:58 ` Greg Kroah-Hartman 2017-12-22 14:58 ` Greg Kroah-Hartman 2017-12-18 8:52 ` [PATCH v5 0/5] misc serdev: new serdev based driver for Wi2Wi w2sg00x4 GPS module H. Nikolaus Schaller 2017-12-18 8:52 ` H. Nikolaus Schaller 2017-12-18 8:52 ` H. Nikolaus Schaller 2017-12-18 14:48 ` Greg Kroah-Hartman 2017-12-18 14:48 ` Greg Kroah-Hartman 2017-12-18 14:52 ` H. Nikolaus Schaller 2017-12-18 14:52 ` H. Nikolaus Schaller 2017-12-18 14:52 ` H. Nikolaus Schaller
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=A2720858-5172-4627-8133-C677E4B4DC72@goldelico.com \ --to=hns@goldelico.com \ --cc=afaerber@suse.de \ --cc=arnd@arndb.de \ --cc=bcousson@baylibre.com \ --cc=devicetree@vger.kernel.org \ --cc=gregkh@linuxfoundation.org \ --cc=jic23@kernel.org \ --cc=johan@kernel.org \ --cc=kernel@pyra-handheld.com \ --cc=khilman@baylibre.com \ --cc=letux-kernel@openphoenux.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-omap@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=mark.rutland@arm.com \ --cc=robh+dt@kernel.org \ --cc=tony@atomide.com \ --cc=treding@nvidia.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: 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.