AW: [PATCH v3] Input: add bu21029 touch driver

["Jonas Mark (BT-FIR/ENG1)" <Mark.Jonas@de.bosch.com>]

Hello Dmitry,

> > > > +static void bu21029_stop_chip(struct input_dev *dev)
> > > > +{
> > > > +       struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);
> > > > +
> > > > +       disable_irq(bu21029->client->irq);
> > > > +       del_timer_sync(&bu21029->timer);
> > > > +
> > > > +       /* put chip into reset */
> > > > +       gpiod_set_value_cansleep(bu21029->reset_gpios, 1);
> > >
> > > > +       udelay(STOP_DELAY_US);
> > >
> > > udelay() ?!
> > >
> > > > +}
> >
> > According to the datasheet disabling the chip will take 30 microseconds.
> > In the defines we added a buffer of 20 microseconds and thus
> > STOP_DELAY_US is 50. The function guarantees that the chip is stopped
> > before it returns.
> >
> > We think that it is ok to use udelay() here because in normal operation
> > the chip is not stopped. It is only stopped when loading or unloading
> > the driver, or when the system suspends.
> >
> > We would like to keep it like it is.
> 
> The issue is not with having delay here, but the kind of delay you are
> using: udelay makes CPU spin for given amount of time; you really want
> msleep() or usleep_range() here.

Understood and changed.

> > > > +static int bu21029_start_chip(struct input_dev *dev)
> > > > +{
> > >
> > > > +       u16 hwid;
> > > > +
> > > > +       /* take chip out of reset */
> > > > +       gpiod_set_value_cansleep(bu21029->reset_gpios, 0);
> > >
> > > > +       mdelay(START_DELAY_MS);
> > >
> > > mdelay()?!
> 
> Same here - replace with msleep().

Replaced.

> > > Instead...
> > >
> > > > +static int bu21029_suspend(struct device *dev)
> > >
> > > ...use __maby_unused annotation.
> > >
> > > > +static int bu21029_resume(struct device *dev)
> > >
> > > Ditto.
> >
> > OK, added.
> 
> You also need to drop #ifdef CONFIG_SLEEP. That's the point: we want to
> reduce amount of conditionally compiled code and use __maybe_unused to
> shut off compiler warning about some functions not used in certain
> configurations. We rely on linker to eliminate dead functions from
> executable.

Done.

Greetings,
Mark

 Mark Jonas

Building Technologies, Panel Software Fire (BT-FIR/ENG1) 
Bosch Sicherheitssysteme GmbH | Postfach 11 11 | 85626 Grasbrunn | GERMANY | www.boschsecurity.com

Sitz: Stuttgart, Registergericht: Amtsgericht Stuttgart HRB 23118 
Aufsichtsratsvorsitzender: Stefan Hartung; Geschäftsführung: Gert van Iperen, Andreas Bartz, Thomas Quante, Bernhard Schuster