Re: input:rohm based bu21013 touch panel controller driver support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Naveen,

On Mon, Sep 06, 2010 at 06:59:42AM +0200, Naveen Kumar GADDIPATI wrote:
> Hi Dmitry,
> 
> Thanks for your comments.
> 
> I have some questions about the following comments.
> 
> > -----Original Message-----
> > From: Dmitry Torokhov [mailto:dmitry.torokhov@xxxxxxxxx]
> > Sent: Wednesday, September 01, 2010 7:34 AM
> > To: Naveen Kumar GADDIPATI
> 
> > > +       input_report_abs(data->in_dev, ABS_X, pt0x);
> > > +       input_report_abs(data->in_dev, ABS_Y, pt0y);
> > > +       input_report_abs(data->in_dev, ABS_PRESSURE, 1);
> > > +       input_report_abs(data->in_dev, ABS_TOOL_WIDTH, 1);
> > 
> > The device apparently does not provide pressure and contact size
> > readings, please do not supply fake data.
> 
> 	These values are required for android framework. If I remove the report of these values, 
> 	Android is not able to recognize the touch.
> 

In this case I recommend working with Androis folks to fix their
framework.

> > > +       input_report_abs(data->in_dev, ABS_MT_TOUCH_MAJOR, 1);
> > > +       input_report_abs(data->in_dev, ABS_MT_TOUCH_MINOR, 1);
> > 
> > Your device obviously does not provide contact size data so please do
> > not fake it.
> 
> 	These values are required for android framework. If I remove the report of these values, 
> 	Android is not able to recognize the touch.

See above.

> 
> > > +static void bu21013_touch_calc
> > > +       (struct bu21013_ts_data *data, int x, int y, int count)
> > > +{
> > > +       data->x_pos[count] = x * data->factor_x / SCALE_FACTOR;
> > > +       data->y_pos[count] = y * data->factor_y / SCALE_FACTOR;
> > 
> > Scaling should be done in userspace.
> 
> 	In the controller configuration, we are using the 384 X 704 touch panel resolution.
> 	If we don't do the scaling in driver to display resolution, android framework is not getting exact co-ordinates.
> 

See above.

> > > +static bool bu21013_verify_delta(int x1, int y1, int x2, int y2)
> > > +{
> > > +       int delta_x, delta_y;
> > > +       if ((x1 != 0) && (y1 != 0)) {
> > > +               delta_x = x2 - x1;
> > > +               if (x1 > x2)
> > > +                       delta_x = x1 - x2;
> > > +               delta_y = y2 - y1;
> > > +               if (y1 > y2)
> > > +                       delta_y = y1 - y2;
> > > +               if ((delta_x < DELTA_MIN) || (delta_y < DELTA_MIN))
> > > +                       return false;
> > 
> > If you specify "fuzz" for given axis I think input core will do this
> > for
> > you.
> 	What is fuzz?


It is one of the parameters in input_set_abs_params() calls that
instructs input core to perform filtering on input events.

> 
> > > +/**
> > > + * bu21013_timer_callback() - callback handler for timer
> > > + * @dev_data:touch screen data
> > > + *
> > > + * This timer callback handler used to schedule the work during
> > > + * polling of pen down interrupts and returns none
> > > + */
> > > +static void bu21013_timer_callback(unsigned long dev_data)
> > > +{
> > > +       struct bu21013_ts_data *data = (struct bu21013_ts_data
> > *)dev_data;
> > > +       schedule_work(&data->timer_wq_handler);
> > 
> > This is called delayed_work().
> > 
> > However please consider switching to threaded IRQ - you can sleep in
> > your IRQ handler until you are done and get "pen up" condition. You
> > just
> > need to make sure you exit the handler promptly when unbinding the
> > device from the driver. See drivers/input/keyboard/samsung-keypad.c
> > 
> 	This work queue is called during polling of co-ordinates
> 	when user is doing some dragging on touch panel for every certain
> 	amount of timeout. I think its ok.

I understand the porpose of the work you are using, AT least use
delayed_work structure (which is timer + work) instead of implementing
your own; the best case look into using threaded IRQ as it avoids
headaches when you need to safely shut off IRQ handler and cancel
outstanding work when unbinding device from the driver.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux