On Thu, May 27, 2021 at 01:41:57PM +0300, Dan Carpenter wrote: > On Thu, May 27, 2021 at 12:19:51PM +0200, Oleksij Rempel wrote: > > Hi, > > > > On Thu, May 27, 2021 at 12:30:00PM +0300, Dan Carpenter wrote: > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next > > > head: 6cf3b3abbf0b3b778138c0f8936aa7820af62cfc > > > commit: fb082cd59afa7fbd3a610e9835050675040b1b99 [2732/2735] Input: resistive-adc-touch - add support for z1 and z2 channels > > > config: x86_64-randconfig-m001-20210526 (attached as .config) > > > compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 > > > > > > If you fix the issue, kindly add following tag as appropriate > > > Reported-by: kernel test robot <lkp@xxxxxxxxx> > > > Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > > > > > New smatch warnings: > > > drivers/input/touchscreen/resistive-adc-touch.c:84 grts_cb() error: uninitialized symbol 'z2'. > > > drivers/input/touchscreen/resistive-adc-touch.c:88 grts_cb() error: uninitialized symbol 'x'. > > > drivers/input/touchscreen/resistive-adc-touch.c:101 grts_cb() error: uninitialized symbol 'y'. > > > > > > Old smatch warnings: > > > drivers/input/touchscreen/resistive-adc-touch.c:101 grts_cb() error: uninitialized symbol 'x'. > > > drivers/input/touchscreen/resistive-adc-touch.c:109 grts_cb() error: uninitialized symbol 'y'. > > > > > > vim +/z2 +84 drivers/input/touchscreen/resistive-adc-touch.c > > > > > > aa132ffb6b0a18 Eugen Hristev 2018-05-22 56 static int grts_cb(const void *data, void *private) > > > aa132ffb6b0a18 Eugen Hristev 2018-05-22 57 { > > > aa132ffb6b0a18 Eugen Hristev 2018-05-22 58 const u16 *touch_info = data; > > > aa132ffb6b0a18 Eugen Hristev 2018-05-22 59 struct grts_state *st = private; > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 60 unsigned int x, y, press = 0, z1 = 0, z2; > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 61 unsigned int Rt, i; > > > aa132ffb6b0a18 Eugen Hristev 2018-05-22 62 > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 63 for (i = 0; i < ARRAY_SIZE(st->ch) && st->ch[i] != GRTS_CH_NONE; i++) { > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 64 switch (st->ch[i]) { > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 65 case GRTS_CH_X: > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 66 x = touch_info[i]; > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 67 break; > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 68 case GRTS_CH_Y: > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 69 y = touch_info[i]; > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 70 break; > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 71 case GRTS_CH_PRESSURE: > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 72 press = touch_info[i]; > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 73 break; > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 74 case GRTS_CH_Z1: > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 75 z1 = touch_info[i]; > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 76 break; > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 77 case GRTS_CH_Z2: > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 78 z2 = touch_info[i]; > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 79 break; > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 80 } > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 81 } > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 82 > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 83 if (z1) { > > > fb082cd59afa7f Oleksij Rempel 2021-05-25 @84 Rt = z2; > > > > > > It's likely this is a false positive, but we can see why the static > > > checker would complain. > > > > The presence of this values was validated on probe. Do we really need > > extra set them to 0? > > You don't need to do anything. ;) These are a one time email. I just > look it over and hit forward. > > Currently GCC's uninitialized variable checking is disabled in the > kernel otherwise there would be more presure to silence the checker > warning. I find "ununitialzied" warnings to be quite useful. How about we change a bit how we map the channels, and this will help with uninitialized warnings as well. Absolutely not tested, so... Thanks. -- Dmitry Input: resistive-adc-touch - rework mapping of channels From: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> Instead of iterating over channels establish and use channel map to retrieve data. As a side effect this will silence "uninitialized variable" warnings. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> --- drivers/input/touchscreen/resistive-adc-touch.c | 114 ++++++++++------------- 1 file changed, 51 insertions(+), 63 deletions(-) diff --git a/drivers/input/touchscreen/resistive-adc-touch.c b/drivers/input/touchscreen/resistive-adc-touch.c index ea1884fb49a1..2102916a37ea 100644 --- a/drivers/input/touchscreen/resistive-adc-touch.c +++ b/drivers/input/touchscreen/resistive-adc-touch.c @@ -25,12 +25,12 @@ #define GRTS_MAX_CHANNELS 4 enum grts_ch_type { - GRTS_CH_NONE = 0, GRTS_CH_X, GRTS_CH_Y, GRTS_CH_PRESSURE, GRTS_CH_Z1, GRTS_CH_Z2, + GRTS_CH_MAX = GRTS_CH_Z2 + 1 }; /** @@ -42,7 +42,7 @@ enum grts_ch_type { * @iio_cb: iio_callback buffer for the data * @input: the input device structure that we register * @prop: touchscreen properties struct - * @ch: channels that are defined for the touchscreen + * @ch_map: map of channels that are defined for the touchscreen */ struct grts_state { u32 x_plate_ohms; @@ -52,37 +52,25 @@ struct grts_state { struct iio_cb_buffer *iio_cb; struct input_dev *input; struct touchscreen_properties prop; - u8 ch[GRTS_MAX_CHANNELS]; + u8 ch_map[GRTS_CH_MAX]; }; static int grts_cb(const void *data, void *private) { const u16 *touch_info = data; struct grts_state *st = private; - unsigned int x, y, press = 0, z1 = 0, z2; - unsigned int Rt, i; - - for (i = 0; i < ARRAY_SIZE(st->ch) && st->ch[i] != GRTS_CH_NONE; i++) { - switch (st->ch[i]) { - case GRTS_CH_X: - x = touch_info[i]; - break; - case GRTS_CH_Y: - y = touch_info[i]; - break; - case GRTS_CH_PRESSURE: - press = touch_info[i]; - break; - case GRTS_CH_Z1: - z1 = touch_info[i]; - break; - case GRTS_CH_Z2: - z2 = touch_info[i]; - break; - } - } + unsigned int x, y, press; + + x = touch_info[st->ch_map[GRTS_CH_X]]; + y = touch_info[st->ch_map[GRTS_CH_X]]; + + if (st->ch_map[GRTS_CH_PRESSURE] < GRTS_MAX_CHANNELS) { + press = touch_info[st->ch_map[GRTS_CH_PRESSURE]]; + } else if (st->ch_map[GRTS_CH_Z1] < GRTS_MAX_CHANNELS) { + unsigned int z1 = touch_info[st->ch_map[GRTS_CH_Z1]]; + unsigned int z2 = touch_info[st->ch_map[GRTS_CH_Z2]]; + unsigned int Rt; - if (z1) { Rt = z2; Rt -= z1; Rt *= st->x_plate_ohms; @@ -142,60 +130,59 @@ static void grts_disable(void *data) iio_channel_release_all_cb(data); } -static int grts_get_properties(struct grts_state *st, struct device *dev) +static int grts_map_channel(struct grts_state *st, struct device *dev, + enum grts_ch_type type, const char *name, + bool optional) { - int idx, error; - - idx = device_property_match_string(dev, "io-channel-names", "x"); - if (idx < 0) - return idx; - - if (idx >= ARRAY_SIZE(st->ch)) + int idx; + + idx = device_property_match_string(dev, "io-channel-names", name); + if (idx < 0) { + if (!optional) + return idx; + idx = GRTS_MAX_CHANNELS; + } else if (idx >= GRTS_MAX_CHANNELS) { return -EOVERFLOW; + } - st->ch[idx] = GRTS_CH_X; + st->ch_map[type] = idx; + return 0; +} - idx = device_property_match_string(dev, "io-channel-names", "y"); - if (idx < 0) - return idx; +static int grts_get_properties(struct grts_state *st, struct device *dev) +{ + int error; - if (idx >= ARRAY_SIZE(st->ch)) - return -EOVERFLOW; + error = grts_map_channel(st, dev, GRTS_CH_X, "x", false); + if (error) + return error; - st->ch[idx] = GRTS_CH_Y; + error = grts_map_channel(st, dev, GRTS_CH_Y, "y", false); + if (error) + return error; /* pressure is optional */ - idx = device_property_match_string(dev, "io-channel-names", "pressure"); - if (idx >= 0) { - if (idx >= ARRAY_SIZE(st->ch)) - return -EOVERFLOW; + error = grts_map_channel(st, dev, GRTS_CH_PRESSURE, "pressure", true); + if (error) + return error; - st->ch[idx] = GRTS_CH_PRESSURE; + if (st->ch_map[GRTS_CH_PRESSURE] < GRTS_MAX_CHANNELS) { st->pressure = true; - return 0; } /* if no pressure is defined, try optional z1 + z2 */ - idx = device_property_match_string(dev, "io-channel-names", "z1"); - if (idx < 0) - return 0; - - if (idx >= ARRAY_SIZE(st->ch)) - return -EOVERFLOW; + error = grts_map_channel(st, dev, GRTS_CH_Z1, "z1", true); + if (error) + return error; - st->ch[idx] = GRTS_CH_Z1; + if (st->ch_map[GRTS_CH_Z1] >= GRTS_MAX_CHANNELS) + return 0; /* if z1 is provided z2 is not optional */ - idx = device_property_match_string(dev, "io-channel-names", "z2"); - if (idx < 0) - return idx; - - if (idx >= ARRAY_SIZE(st->ch)) - return -EOVERFLOW; - - st->ch[idx] = GRTS_CH_Z2; - st->pressure = true; + error = grts_map_channel(st, dev, GRTS_CH_Z2, "z2", true); + if (error) + return error; error = device_property_read_u32(dev, "touchscreen-x-plate-ohms", @@ -205,6 +192,7 @@ static int grts_get_properties(struct grts_state *st, struct device *dev) return error; } + st->pressure = true; return 0; }
