* [PATCH] input:Misc changes against kxtj9 v5 patch
@ 2011-07-05 19:55 chris.hudson.comp.eng
2011-07-05 20:05 ` Dmitry Torokhov
0 siblings, 1 reply; 5+ messages in thread
From: chris.hudson.comp.eng @ 2011-07-05 19:55 UTC (permalink / raw)
To: linux-input; +Cc: dmitry.torokhov, jic23, Chris Hudson
From: Chris Hudson <chudson@kionix.com>
As requested, miscellaneous changes originally suggested by Jonathan Cameron,
patched against the original kxtj9 v5 patch.
Signed-off-by: Chris Hudson <chudson@kionix.com>
---
drivers/input/misc/kxtj9.c | 38 +++++++++++++-------------------------
include/linux/input/kxtj9.h | 10 +---------
2 files changed, 14 insertions(+), 34 deletions(-)
diff --git a/drivers/input/misc/kxtj9.c b/drivers/input/misc/kxtj9.c
index 7faf155..b2da841 100644
--- a/drivers/input/misc/kxtj9.c
+++ b/drivers/input/misc/kxtj9.c
@@ -71,6 +71,15 @@ static const struct {
{ 0, ODR12_5F},
};
+static const struct {
+ u8 g_range;
+ u8 shift;
+} kxtj9_range[] = {
+ { 0, 4 }, /* +/-2g */
+ { (1 << 3), 3 }, /* +/-4g */
+ { (1 << 4), 2 }, /* +/-8g */
+};
+
struct kxtj9_data {
struct i2c_client *client;
struct kxtj9_platform_data pdata;
@@ -129,25 +138,9 @@ static void kxtj9_report_acceleration_data(struct kxtj9_data *tj9)
static int kxtj9_update_g_range(struct kxtj9_data *tj9, u8 new_g_range)
{
- switch (new_g_range) {
- case KXTJ9_G_2G:
- tj9->shift = SHIFT_ADJ_2G;
- break;
-
- case KXTJ9_G_4G:
- tj9->shift = SHIFT_ADJ_4G;
- break;
-
- case KXTJ9_G_8G:
- tj9->shift = SHIFT_ADJ_8G;
- break;
-
- default:
- return -EINVAL;
- }
-
+ tj9->shift = kxtj9_range[new_g_range].shift;
tj9->ctrl_reg1 &= 0xe7;
- tj9->ctrl_reg1 |= new_g_range;
+ tj9->ctrl_reg1 |= kxtj9_range[new_g_range].g_range;
return 0;
}
@@ -181,13 +174,8 @@ static int kxtj9_update_odr(struct kxtj9_data *tj9, int poll_interval)
static int kxtj9_device_power_on(struct kxtj9_data *tj9)
{
- int err;
-
- if (tj9->pdata.power_on) {
- err = tj9->pdata.power_on();
- if (err < 0)
- return err;
- }
+ if (tj9->pdata.power_on)
+ return tj9->pdata.power_on();
return 0;
}
diff --git a/include/linux/input/kxtj9.h b/include/linux/input/kxtj9.h
index cc5928b..6064d45 100644
--- a/include/linux/input/kxtj9.h
+++ b/include/linux/input/kxtj9.h
@@ -22,11 +22,6 @@
#define KXTJ9_I2C_ADDR 0x0F
-/* These shift values are used to provide consistent output data */
-#define SHIFT_ADJ_2G 4
-#define SHIFT_ADJ_4G 3
-#define SHIFT_ADJ_8G 2
-
#ifdef __KERNEL__
struct kxtj9_platform_data {
int poll_interval; /* current poll interval (in milli-seconds) */
@@ -49,10 +44,7 @@ struct kxtj9_platform_data {
#define RES_8BIT 0
#define RES_12BIT (1 << 6)
u8 res_12bit;
- /* Output g-range: +/-2g, 4g, or 8g */
- #define KXTJ9_G_2G 0
- #define KXTJ9_G_4G (1 << 3)
- #define KXTJ9_G_8G (1 << 4)
+ /* Output g-range: 0 = +/-2g, 1 = 4g, or 2 = 8g */
u8 g_range;
/* DATA_CTRL_REG: controls the output data rate of the part */
--
1.5.4.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] input:Misc changes against kxtj9 v5 patch
2011-07-05 19:55 [PATCH] input:Misc changes against kxtj9 v5 patch chris.hudson.comp.eng
@ 2011-07-05 20:05 ` Dmitry Torokhov
[not found] ` <CAB7Re7XDB6Rmckxidgr+90Br5LhYk_5Pi_JQXTLd9QS2V+Vm0A@mail.gmail.com>
0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Torokhov @ 2011-07-05 20:05 UTC (permalink / raw)
To: chris.hudson.comp.eng; +Cc: linux-input, jic23, Chris Hudson
On Tue, Jul 05, 2011 at 03:55:54PM -0400, chris.hudson.comp.eng@gmail.com wrote:
> From: Chris Hudson <chudson@kionix.com>
>
> As requested, miscellaneous changes originally suggested by Jonathan Cameron,
> patched against the original kxtj9 v5 patch.
>
> Signed-off-by: Chris Hudson <chudson@kionix.com>
> ---
> drivers/input/misc/kxtj9.c | 38 +++++++++++++-------------------------
> include/linux/input/kxtj9.h | 10 +---------
> 2 files changed, 14 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/input/misc/kxtj9.c b/drivers/input/misc/kxtj9.c
> index 7faf155..b2da841 100644
> --- a/drivers/input/misc/kxtj9.c
> +++ b/drivers/input/misc/kxtj9.c
> @@ -71,6 +71,15 @@ static const struct {
> { 0, ODR12_5F},
> };
>
> +static const struct {
> + u8 g_range;
> + u8 shift;
> +} kxtj9_range[] = {
> + { 0, 4 }, /* +/-2g */
> + { (1 << 3), 3 }, /* +/-4g */
> + { (1 << 4), 2 }, /* +/-8g */
> +};
> +
> struct kxtj9_data {
> struct i2c_client *client;
> struct kxtj9_platform_data pdata;
> @@ -129,25 +138,9 @@ static void kxtj9_report_acceleration_data(struct kxtj9_data *tj9)
>
> static int kxtj9_update_g_range(struct kxtj9_data *tj9, u8 new_g_range)
> {
> - switch (new_g_range) {
> - case KXTJ9_G_2G:
> - tj9->shift = SHIFT_ADJ_2G;
> - break;
> -
> - case KXTJ9_G_4G:
> - tj9->shift = SHIFT_ADJ_4G;
> - break;
> -
> - case KXTJ9_G_8G:
> - tj9->shift = SHIFT_ADJ_8G;
> - break;
> -
> - default:
> - return -EINVAL;
> - }
> -
> + tj9->shift = kxtj9_range[new_g_range].shift;
This is not safe, you still need to sanitize input to make sure you do
not go beyond array boundaries.
--
Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] input:Misc changes against kxtj9 v5 patch
[not found] ` <20110705203502.GC15876@core.coreip.homeip.net>
@ 2011-07-06 15:42 ` Christopher Hudson
2011-07-06 15:58 ` Christopher Hudson
0 siblings, 1 reply; 5+ messages in thread
From: Christopher Hudson @ 2011-07-06 15:42 UTC (permalink / raw)
To: Dmitry Torokhov, Jonathan Cameron, linux-input
On Tue, Jul 5, 2011 at 4:35 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Tue, Jul 05, 2011 at 04:19:32PM -0400, Christopher Hudson wrote:
>> On Tue, Jul 5, 2011 at 4:05 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > On Tue, Jul 05, 2011 at 03:55:54PM -0400, chris.hudson.comp.eng@gmail.com wrote:
>> >> From: Chris Hudson <chudson@kionix.com>
>> >>
>> >> As requested, miscellaneous changes originally suggested by Jonathan Cameron,
>> >> patched against the original kxtj9 v5 patch.
>> >>
>> >> Signed-off-by: Chris Hudson <chudson@kionix.com>
>> >> ---
>> >> drivers/input/misc/kxtj9.c | 38 +++++++++++++-------------------------
>> >> include/linux/input/kxtj9.h | 10 +---------
>> >> 2 files changed, 14 insertions(+), 34 deletions(-)
>> >>
>> >> diff --git a/drivers/input/misc/kxtj9.c b/drivers/input/misc/kxtj9.c
>> >> index 7faf155..b2da841 100644
>> >> --- a/drivers/input/misc/kxtj9.c
>> >> +++ b/drivers/input/misc/kxtj9.c
>> >> @@ -71,6 +71,15 @@ static const struct {
>> >> { 0, ODR12_5F},
>> >> };
>> >>
>> >> +static const struct {
>> >> + u8 g_range;
>> >> + u8 shift;
>> >> +} kxtj9_range[] = {
>> >> + { 0, 4 }, /* +/-2g */
>> >> + { (1 << 3), 3 }, /* +/-4g */
>> >> + { (1 << 4), 2 }, /* +/-8g */
>> >> +};
>> >> +
>> >> struct kxtj9_data {
>> >> struct i2c_client *client;
>> >> struct kxtj9_platform_data pdata;
>> >> @@ -129,25 +138,9 @@ static void kxtj9_report_acceleration_data(struct kxtj9_data *tj9)
>> >>
>> >> static int kxtj9_update_g_range(struct kxtj9_data *tj9, u8 new_g_range)
>> >> {
>> >> - switch (new_g_range) {
>> >> - case KXTJ9_G_2G:
>> >> - tj9->shift = SHIFT_ADJ_2G;
>> >> - break;
>> >> -
>> >> - case KXTJ9_G_4G:
>> >> - tj9->shift = SHIFT_ADJ_4G;
>> >> - break;
>> >> -
>> >> - case KXTJ9_G_8G:
>> >> - tj9->shift = SHIFT_ADJ_8G;
>> >> - break;
>> >> -
>> >> - default:
>> >> - return -EINVAL;
>> >> - }
>> >> -
>> >> + tj9->shift = kxtj9_range[new_g_range].shift;
>> >
>> > This is not safe, you still need to sanitize input to make sure you do
>> > not go beyond array boundaries.
>>
>> Oops...would a simple
>> if (new_g_range > 2)
>> return -EINVAL;
>> do the trick?
>
> Or "if (new_g_range >= ARRAY_SIZE(kxtj9_range)) ..."
>
> However, why do we use indices instead of values? I.e. why don't we
> allow users to specify 2, 4 and 8 in platform data and use array to
> validate the supplied value and return -EINVAL if this value is not
> found?
Before we were allowing the user to specify KXTJ9_G_2G, etc, which
were bitmasks that represented each available g-range. I think
Jonathan's intent was to optimize/clean up the update_g_range
function, but I just couldn't figure out the most efficient way to do
this. It seems that just replacing the globally defined shift
variables with their numerical counterparts within the update_g_range
function probably gives us the best combination of efficiency and
ease-of-use.
>
> --
> Dmitry
>
--
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] input:Misc changes against kxtj9 v5 patch
2011-07-06 15:42 ` Christopher Hudson
@ 2011-07-06 15:58 ` Christopher Hudson
2011-07-07 5:19 ` Dmitry Torokhov
0 siblings, 1 reply; 5+ messages in thread
From: Christopher Hudson @ 2011-07-06 15:58 UTC (permalink / raw)
To: Dmitry Torokhov, Jonathan Cameron, linux-input
On Wed, Jul 6, 2011 at 11:42 AM, Christopher Hudson
<chris.hudson.comp.eng@gmail.com> wrote:
> On Tue, Jul 5, 2011 at 4:35 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>> On Tue, Jul 05, 2011 at 04:19:32PM -0400, Christopher Hudson wrote:
>>> On Tue, Jul 5, 2011 at 4:05 PM, Dmitry Torokhov
>>> <dmitry.torokhov@gmail.com> wrote:
>>> > On Tue, Jul 05, 2011 at 03:55:54PM -0400, chris.hudson.comp.eng@gmail.com wrote:
>>> >> From: Chris Hudson <chudson@kionix.com>
>>> >>
>>> >> As requested, miscellaneous changes originally suggested by Jonathan Cameron,
>>> >> patched against the original kxtj9 v5 patch.
>>> >>
>>> >> Signed-off-by: Chris Hudson <chudson@kionix.com>
>>> >> ---
>>> >> drivers/input/misc/kxtj9.c | 38 +++++++++++++-------------------------
>>> >> include/linux/input/kxtj9.h | 10 +---------
>>> >> 2 files changed, 14 insertions(+), 34 deletions(-)
>>> >>
>>> >> diff --git a/drivers/input/misc/kxtj9.c b/drivers/input/misc/kxtj9.c
>>> >> index 7faf155..b2da841 100644
>>> >> --- a/drivers/input/misc/kxtj9.c
>>> >> +++ b/drivers/input/misc/kxtj9.c
>>> >> @@ -71,6 +71,15 @@ static const struct {
>>> >> { 0, ODR12_5F},
>>> >> };
>>> >>
>>> >> +static const struct {
>>> >> + u8 g_range;
>>> >> + u8 shift;
>>> >> +} kxtj9_range[] = {
>>> >> + { 0, 4 }, /* +/-2g */
>>> >> + { (1 << 3), 3 }, /* +/-4g */
>>> >> + { (1 << 4), 2 }, /* +/-8g */
>>> >> +};
>>> >> +
>>> >> struct kxtj9_data {
>>> >> struct i2c_client *client;
>>> >> struct kxtj9_platform_data pdata;
>>> >> @@ -129,25 +138,9 @@ static void kxtj9_report_acceleration_data(struct kxtj9_data *tj9)
>>> >>
>>> >> static int kxtj9_update_g_range(struct kxtj9_data *tj9, u8 new_g_range)
>>> >> {
>>> >> - switch (new_g_range) {
>>> >> - case KXTJ9_G_2G:
>>> >> - tj9->shift = SHIFT_ADJ_2G;
>>> >> - break;
>>> >> -
>>> >> - case KXTJ9_G_4G:
>>> >> - tj9->shift = SHIFT_ADJ_4G;
>>> >> - break;
>>> >> -
>>> >> - case KXTJ9_G_8G:
>>> >> - tj9->shift = SHIFT_ADJ_8G;
>>> >> - break;
>>> >> -
>>> >> - default:
>>> >> - return -EINVAL;
>>> >> - }
>>> >> -
>>> >> + tj9->shift = kxtj9_range[new_g_range].shift;
>>> >
>>> > This is not safe, you still need to sanitize input to make sure you do
>>> > not go beyond array boundaries.
>>>
>>> Oops...would a simple
>>> if (new_g_range > 2)
>>> return -EINVAL;
>>> do the trick?
>>
>> Or "if (new_g_range >= ARRAY_SIZE(kxtj9_range)) ..."
>>
>> However, why do we use indices instead of values? I.e. why don't we
>> allow users to specify 2, 4 and 8 in platform data and use array to
>> validate the supplied value and return -EINVAL if this value is not
>> found?
>
> Before we were allowing the user to specify KXTJ9_G_2G, etc, which
> were bitmasks that represented each available g-range. I think
> Jonathan's intent was to optimize/clean up the update_g_range
> function, but I just couldn't figure out the most efficient way to do
> this. It seems that just replacing the globally defined shift
> variables with their numerical counterparts within the update_g_range
> function probably gives us the best combination of efficiency and
> ease-of-use.
My apologies I meant to include the following revised version:
Signed-off-by: Chris Hudson <chudson@kionix.com>
---
drivers/input/misc/kxtj9.c | 18 +++++-------------
include/linux/input/kxtj9.h | 5 -----
2 files changed, 5 insertions(+), 18 deletions(-)
diff --git a/drivers/input/misc/kxtj9.c b/drivers/input/misc/kxtj9.c
index 7faf155..4cfce82 100644
--- a/drivers/input/misc/kxtj9.c
+++ b/drivers/input/misc/kxtj9.c
@@ -131,17 +131,14 @@ static int kxtj9_update_g_range(struct
kxtj9_data *tj9, u8 new_g_range)
{
switch (new_g_range) {
case KXTJ9_G_2G:
- tj9->shift = SHIFT_ADJ_2G;
+ tj9->shift = 4;
break;
-
case KXTJ9_G_4G:
- tj9->shift = SHIFT_ADJ_4G;
+ tj9->shift = 3;
break;
-
case KXTJ9_G_8G:
- tj9->shift = SHIFT_ADJ_8G;
+ tj9->shift = 2;
break;
-
default:
return -EINVAL;
}
@@ -181,13 +178,8 @@ static int kxtj9_update_odr(struct kxtj9_data
*tj9, int poll_interval)
static int kxtj9_device_power_on(struct kxtj9_data *tj9)
{
- int err;
-
- if (tj9->pdata.power_on) {
- err = tj9->pdata.power_on();
- if (err < 0)
- return err;
- }
+ if (tj9->pdata.power_on)
+ return tj9->pdata.power_on();
return 0;
}
diff --git a/include/linux/input/kxtj9.h b/include/linux/input/kxtj9.h
index cc5928b..2575205 100644
--- a/include/linux/input/kxtj9.h
+++ b/include/linux/input/kxtj9.h
@@ -22,11 +22,6 @@
#define KXTJ9_I2C_ADDR 0x0F
-/* These shift values are used to provide consistent output data */
-#define SHIFT_ADJ_2G 4
-#define SHIFT_ADJ_4G 3
-#define SHIFT_ADJ_8G 2
-
#ifdef __KERNEL__
struct kxtj9_platform_data {
int poll_interval; /* current poll interval (in milli-seconds) */
--
1.5.4.3
>
>>
>> --
>> Dmitry
>>
>
--
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] input:Misc changes against kxtj9 v5 patch
2011-07-06 15:58 ` Christopher Hudson
@ 2011-07-07 5:19 ` Dmitry Torokhov
0 siblings, 0 replies; 5+ messages in thread
From: Dmitry Torokhov @ 2011-07-07 5:19 UTC (permalink / raw)
To: Christopher Hudson; +Cc: Jonathan Cameron, linux-input
On Wed, Jul 06, 2011 at 11:58:41AM -0400, Christopher Hudson wrote:
> On Wed, Jul 6, 2011 at 11:42 AM, Christopher Hudson
> <chris.hudson.comp.eng@gmail.com> wrote:
> > On Tue, Jul 5, 2011 at 4:35 PM, Dmitry Torokhov
> > <dmitry.torokhov@gmail.com> wrote:
> >> On Tue, Jul 05, 2011 at 04:19:32PM -0400, Christopher Hudson wrote:
> >>> On Tue, Jul 5, 2011 at 4:05 PM, Dmitry Torokhov
> >>> <dmitry.torokhov@gmail.com> wrote:
> >>> > On Tue, Jul 05, 2011 at 03:55:54PM -0400, chris.hudson.comp.eng@gmail.com wrote:
> >>> >> From: Chris Hudson <chudson@kionix.com>
> >>> >>
> >>> >> As requested, miscellaneous changes originally suggested by Jonathan Cameron,
> >>> >> patched against the original kxtj9 v5 patch.
> >>> >>
> >>> >> Signed-off-by: Chris Hudson <chudson@kionix.com>
> >>> >> ---
> >>> >> drivers/input/misc/kxtj9.c | 38 +++++++++++++-------------------------
> >>> >> include/linux/input/kxtj9.h | 10 +---------
> >>> >> 2 files changed, 14 insertions(+), 34 deletions(-)
> >>> >>
> >>> >> diff --git a/drivers/input/misc/kxtj9.c b/drivers/input/misc/kxtj9.c
> >>> >> index 7faf155..b2da841 100644
> >>> >> --- a/drivers/input/misc/kxtj9.c
> >>> >> +++ b/drivers/input/misc/kxtj9.c
> >>> >> @@ -71,6 +71,15 @@ static const struct {
> >>> >> { 0, ODR12_5F},
> >>> >> };
> >>> >>
> >>> >> +static const struct {
> >>> >> + u8 g_range;
> >>> >> + u8 shift;
> >>> >> +} kxtj9_range[] = {
> >>> >> + { 0, 4 }, /* +/-2g */
> >>> >> + { (1 << 3), 3 }, /* +/-4g */
> >>> >> + { (1 << 4), 2 }, /* +/-8g */
> >>> >> +};
> >>> >> +
> >>> >> struct kxtj9_data {
> >>> >> struct i2c_client *client;
> >>> >> struct kxtj9_platform_data pdata;
> >>> >> @@ -129,25 +138,9 @@ static void kxtj9_report_acceleration_data(struct kxtj9_data *tj9)
> >>> >>
> >>> >> static int kxtj9_update_g_range(struct kxtj9_data *tj9, u8 new_g_range)
> >>> >> {
> >>> >> - switch (new_g_range) {
> >>> >> - case KXTJ9_G_2G:
> >>> >> - tj9->shift = SHIFT_ADJ_2G;
> >>> >> - break;
> >>> >> -
> >>> >> - case KXTJ9_G_4G:
> >>> >> - tj9->shift = SHIFT_ADJ_4G;
> >>> >> - break;
> >>> >> -
> >>> >> - case KXTJ9_G_8G:
> >>> >> - tj9->shift = SHIFT_ADJ_8G;
> >>> >> - break;
> >>> >> -
> >>> >> - default:
> >>> >> - return -EINVAL;
> >>> >> - }
> >>> >> -
> >>> >> + tj9->shift = kxtj9_range[new_g_range].shift;
> >>> >
> >>> > This is not safe, you still need to sanitize input to make sure you do
> >>> > not go beyond array boundaries.
> >>>
> >>> Oops...would a simple
> >>> if (new_g_range > 2)
> >>> return -EINVAL;
> >>> do the trick?
> >>
> >> Or "if (new_g_range >= ARRAY_SIZE(kxtj9_range)) ..."
> >>
> >> However, why do we use indices instead of values? I.e. why don't we
> >> allow users to specify 2, 4 and 8 in platform data and use array to
> >> validate the supplied value and return -EINVAL if this value is not
> >> found?
> >
> > Before we were allowing the user to specify KXTJ9_G_2G, etc, which
> > were bitmasks that represented each available g-range. I think
> > Jonathan's intent was to optimize/clean up the update_g_range
> > function, but I just couldn't figure out the most efficient way to do
> > this. It seems that just replacing the globally defined shift
> > variables with their numerical counterparts within the update_g_range
> > function probably gives us the best combination of efficiency and
> > ease-of-use.
>
> My apologies I meant to include the following revised version:
>
> Signed-off-by: Chris Hudson <chudson@kionix.com>
All folded and applied. Thanks Chris.
--
Dmitry
--
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-07-07 5:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-05 19:55 [PATCH] input:Misc changes against kxtj9 v5 patch chris.hudson.comp.eng
2011-07-05 20:05 ` Dmitry Torokhov
[not found] ` <CAB7Re7XDB6Rmckxidgr+90Br5LhYk_5Pi_JQXTLd9QS2V+Vm0A@mail.gmail.com>
[not found] ` <20110705203502.GC15876@core.coreip.homeip.net>
2011-07-06 15:42 ` Christopher Hudson
2011-07-06 15:58 ` Christopher Hudson
2011-07-07 5:19 ` Dmitry Torokhov
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).