Linux Input Archive on lore.kernel.org
 help / color / Atom feed
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Roy Im <roy.im.opensource@diasemi.com>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	Brian Masney <masneyb@onstation.org>,
	Greg KH <gregkh@linuxfoundation.org>,
	Lee Jones <lee.jones@linaro.org>, Luca Weiss <luca@z3ntu.xyz>,
	Maximilian Luz <luzmaximilian@gmail.com>,
	Pascal PAILLET-LME <p.paillet@st.com>,
	Rob Herring <robh@kernel.org>,
	Samuel Ortiz <sameo@linux.intel.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Support Opensource <support.opensource@diasemi.com>,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-pwm@vger.kernel.org, Petr Mladek <pmladek@suse.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>
Subject: Re: [PATCH v18 3/3] Input: new da7280 haptic driver
Date: Wed, 29 Jul 2020 23:34:04 -0700
Message-ID: <20200730063404.GH1665100@dtor-ws> (raw)
In-Reply-To: <20200730061631.y4r4s6v3xepktj54@pengutronix.de>

On Thu, Jul 30, 2020 at 08:16:31AM +0200, Uwe Kleine-König wrote:
> [Adding vsprintf maintainers to Cc:]
> 
> Hello,
> 
> On Wed, Jul 29, 2020 at 10:06:53PM -0700, Dmitry Torokhov wrote:
> > On Wed, Jul 29, 2020 at 09:21:45AM +0200, Uwe Kleine-König wrote:
> > > Hello,
> > > 
> > > On Tue, Jul 28, 2020 at 11:36:38PM -0700, Dmitry Torokhov wrote:
> > > > > v9: 
> > > > > 	- Removed the header file and put the definitions into the c file.
> > > > > 	- Updated the pwm code and error logs with %pE
> > > > 
> > > > I believe the %pE is to format an escaped buffer, you probably want to
> > > > %pe (lowercase) to print errors. I am also not quite sure if we want to
> > > > use it in cases when we have non-pointer error, or we should stick with
> > > > %d as most of the kernel does.
> > > 
> > > compared with %d %pe is easier to understand as it emits "-ETIMEOUT"
> > > instead of "-110". And yes, %pE is wrong.
> > 
> > While I can see that symbolic name instead of a numeric constant might
> > be appealing, I do not believe that we want fragments like this with
> > endless conversions between integer and pointer errors:
> > 
> > 	if (haptics->const_op_mode == DA7280_PWM_MODE) {
> > 		haptics->pwm_dev = devm_pwm_get(dev, NULL);
> > 		if (IS_ERR(haptics->pwm_dev)) {
> > 			error = PTR_ERR(haptics->pwm_dev);
> > 			if (error != -EPROBE_DEFER)
> > 				dev_err(dev, "unable to request PWM: %pE\n",
> > 					ERR_PTR(error));
> > 			return error;
> > 		}
> > 
> > Maybe we should introduce something like '%de' for the integer error
> > case?
> 
> I suggested that some time ago with limited success, see
> https://lore.kernel.org/lkml/20200129115516.zsvxu56e6h7gheiw@pathway.suse.cz/
> .
> 
> > In the meantime I would prefer using %d when we have integer
> > error. We should not see these error messages anyway ;)
> 
> I don't agree. Error messages are supposed to be helpful and I prefer
> some casting between error pointer and error int over emitting bare
> numbers to the kernel log. (And additionally the uglyness might help to
> convince the vsprintf guys that %de might be a good idea after all :-)

Sorry, but I do not think that I want to introduce this pointless
casting just to sway printk folks. Either we have proper way of
formatting error codes, or we do not and we stick with what was working
since forever.

Thanks.

-- 
Dmitry

  reply index

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-29  2:59 [PATCH v18 0/3] da7280: haptic driver submission Roy Im
2020-07-29  2:59 ` [PATCH v18 3/3] Input: new da7280 haptic driver Roy Im
2020-07-29  6:36   ` Dmitry Torokhov
2020-07-29  7:21     ` Uwe Kleine-König
2020-07-30  5:06       ` Dmitry Torokhov
2020-07-30  6:16         ` Uwe Kleine-König
2020-07-30  6:34           ` Dmitry Torokhov [this message]
2020-07-30  7:00             ` Uwe Kleine-König
2020-07-30  8:30           ` Andy Shevchenko
2020-07-30 16:21             ` Dmitry Torokhov
2020-07-29 14:09     ` Roy Im
2020-07-30  5:10       ` Dmitry Torokhov
2020-07-31 13:26         ` Roy Im
2020-08-02 11:54     ` Pavel Machek
2020-08-03  1:13       ` Roy Im
2020-07-29  2:59 ` [PATCH v18 2/3] dt-bindings: input: Add document bindings for DA7280 Roy Im

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=20200730063404.GH1665100@dtor-ws \
    --to=dmitry.torokhov@gmail.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=lee.jones@linaro.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=luca@z3ntu.xyz \
    --cc=luzmaximilian@gmail.com \
    --cc=masneyb@onstation.org \
    --cc=p.paillet@st.com \
    --cc=pmladek@suse.com \
    --cc=robh@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=roy.im.opensource@diasemi.com \
    --cc=sameo@linux.intel.com \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=support.opensource@diasemi.com \
    --cc=tglx@linutronix.de \
    --cc=thierry.reding@gmail.com \
    --cc=u.kleine-koenig@pengutronix.de \
    /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

Linux Input Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-input/0 linux-input/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-input linux-input/ https://lore.kernel.org/linux-input \
		linux-input@vger.kernel.org
	public-inbox-index linux-input

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-input


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git