Hi linux-input devs,
There is an ancient Smatch warning from before the invention of git:
drivers/input/serio/hp_sdc_mlc.c:168 hp_sdc_mlc_in()
error: double unlock 'sem:&mlc->isem'
drivers/input/serio/hp_sdc_mlc.c
143 static int hp_sdc_mlc_in(hil_mlc *mlc, suseconds_t timeout)
144 {
145 struct hp_sdc_mlc_priv_s *priv;
146 int rc = 2;
147
148 priv = mlc->priv;
149
150 /* Try to down the semaphore */
151 if (down_trylock(&mlc->isem)) {
This function seems totally messed up. This test is reversed so it's
probably a no-op most of the time.
152 if (priv->emtestmode) {
153 mlc->ipacket[0] =
154 HIL_ERR_INT | (mlc->opacket &
155 (HIL_PKT_CMD |
156 HIL_PKT_ADDR_MASK |
157 HIL_PKT_DATA_MASK));
158 mlc->icount = 14;
159 /* printk(KERN_DEBUG PREFIX ">[%x]\n", mlc->ipacket[0]); */
160 goto wasup;
161 }
162 if (time_after(jiffies, mlc->instart + mlc->intimeout)) {
163 /* printk("!%i %i",
164 tv.tv_usec - mlc->instart.tv_usec,
165 mlc->intimeout);
166 */
167 rc = 1;
--> 168 up(&mlc->isem);
169 }
170 goto done;
171 }
172 wasup:
173 up(&mlc->isem);
Then the other problem is that we release the lock whether or not we
have succeeded. The one path where we don't release the lock is if
we're not in priv->emtestmode and we haven't timed out. Should we
release the lock there?
174 rc = 0;
175 done:
176 return rc;
The returns are confusing. This is called from hilse_donode(). Right
now if we can't take the lock then it returns zero. It feels like it
should return two but who knows?
177 }
This driver calls BUG_ON() quite a bit when we can't take a lock. Why
do we even need locking if we know we're the only thread? The Kconfig
for HP_SDC says it's new and experimental and that people should
probably just stick with the old stable driver. Is this driver still
required?
regards,
dan carpenter
