From: Lee Jones <lee.jones@linaro.org>
To: Alexander Shiyan <shc_work@mail.ru>
Cc: linux-input@vger.kernel.org,
Dmitry Torokhov <dmitry.torokhov@gmail.com>,
Shawn Guo <shawn.guo@linaro.org>,
Sascha Hauer <kernel@pengutronix.de>,
Samuel Ortiz <sameo@linux.intel.com>
Subject: Re: [PATCH 1/4] input: mc13783: Make mc13xxx_buttons_platform_data more flexible
Date: Tue, 11 Feb 2014 09:17:35 +0000 [thread overview]
Message-ID: <20140211091735.GF32042@lee--X1> (raw)
In-Reply-To: <1392101218.989322000@f169.i.mail.ru>
> > > Instead of define each button in separate variable, make an array
> > > for storing all buttons. This change will help to add support for
> > > other types of PMIC and add support for probing with devicetree
> > > in the future.
> > >
> > > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> > > ---
> > > arch/arm/mach-imx/mach-mx31moboard.c | 9 ++++--
> > > drivers/input/misc/mc13783-pwrbutton.c | 56
> > +++++++++++++++++-----------------
> > > include/linux/mfd/mc13xxx.h | 28 +++++++++--------
> >
> > Are all the changes in these files entangled? If there is any way to
> > make them orthogonal, then all the better.
>
> Can you say the same thing in other words, I didn't understand a bit
> of your comment.
Do all of this changes in each of the files listed above depend on
each other?
> ...
> > > +struct mc13xxx_button {
> > > + u16 keycode;
> > > + unsigned int flags;
> > > +#define MC13XXX_BUTTON_DBNC_0MS 0
> > > +#define MC13XXX_BUTTON_DBNC_30MS 1
> > > +#define MC13XXX_BUTTON_DBNC_150MS 2
> > > +#define MC13XXX_BUTTON_DBNC_750MS 3
> > > +#define MC13XXX_BUTTON_ENABLE (1 << 2)
> > > +#define MC13XXX_BUTTON_POL_INVERT (1 << 3)
> > > +#define MC13XXX_BUTTON_RESET_EN (1 << 4)
> > > +};
> > > +
> >
> > Please take the opportunity to remove this slab list of #defines from
> > the centre of the struct definition. Just above will be fine.
>
> I would like to make it in a separate patch for the whole header file later.
You may as well do it in this commit? Just add them outside of the
struct definition instead of inside.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2014-02-11 9:17 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-10 4:22 [PATCH 1/4] input: mc13783: Make mc13xxx_buttons_platform_data more flexible Alexander Shiyan
2014-02-10 11:29 ` Lee Jones
2014-02-11 6:46 ` Alexander Shiyan
2014-02-11 9:17 ` Lee Jones [this message]
2014-02-11 9:42 ` Alexander Shiyan
2014-02-11 10:07 ` Lee Jones
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=20140211091735.GF32042@lee--X1 \
--to=lee.jones@linaro.org \
--cc=dmitry.torokhov@gmail.com \
--cc=kernel@pengutronix.de \
--cc=linux-input@vger.kernel.org \
--cc=sameo@linux.intel.com \
--cc=shawn.guo@linaro.org \
--cc=shc_work@mail.ru \
/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 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.