From: Vijay R. Iyengar on

> -----Original Message-----
> From: J.I. Cameron [mailto:jic23(a)hermes.cam.ac.uk] On Behalf
> Of Jonathan
> Cameron
> Sent: 19 May 2010 12:22
> To: David Dajun Chen
> Cc: linux-kernel(a)vger.kernel.org; sameo(a)linux.intel.com
> Subject: Re: [PATCH] MFD of DA9052 Linux device drivers (1/9)
>
> Hi David,
>
> Below is an initial review. Please take a look at more code
> for similar
> drivers
> in the various subsystems as it will give you an idea how how much of
> this should
> be done.
>
> I've run out of time, so I'm afraid I didn't get quite to the
> bottom of
> this patch.
> Still plenty of stuff still to do in the bit I did look at!
>
> Jonathan
> > +
> > +/**
> > + * da9052_eh_unregister_nb: Function to de-register
> a notifier block
> > + *
> > + * Caller must not pass any non-existing notifier block or
> notifiler
> block
> > + * that has already been de-registered. Its caller's responsibility
> to
> > + * allocate/deallocate memory associated with notifier
> block. EH will
> only
> > + * Link/De-Link passed notifier blocks.
> > + *
> > + * @param nb : Notifier block to unregister
> > + * @return s32 : Status of unregistration
> > + */
> > +s32 da9052_eh_unregister_nb(da9052_eh_nb* nb){
> > +
> > + if(nb == NULL)
> > + return (INVALID_NB);
> > +
> > + DA9052_DEBUG("%s() called for event %d \n", __FUNCTION__,
> nb->eve_type);
> > +
> > + /* Acqurie nb array lock */
> > + if(down_interruptible(&eh_info.eve_nb_array_lock))
> > + return (FAILURE);
> > +
> > + /* Remove passed NB from list */
> > + list_del_init(&(nb->nb_list));
> > +
> > + /* Release NB array lock */
> > + up(&eh_info.eve_nb_array_lock);
> > +
> > + return (SUCCESS);
> > +}
> > +
>
> These are looking pretyt touch screen specific. What are
> they doing in
> this patch?

We have designed the EH (interrupt handler driver) and the TSI (touch screen driver) in a manner that the EH reads raw co-ordinates from the touch control registers of the DA9052 hardware in response to sampling ready interrupt requests and stores them in a FIFO for TSI further processing.

We are implementing a common Event handler module for processing all the interrupt events from the DA9052 devices like battery, regulator, RTC etc. Fetching the TSI samples on touch events by EH rather than notifying the TSI driver through a call-back routine helps us to get a good amount of filtered touch samples from the TSI driver.

Hence we intend to keep the TSI raw co-ordinate reading in the event handling module, which is a part of the MFD driver.

> > +/**
> > + * da9052_tsi_start_sampling: Called by TSI driver to indicate
> start of data
> > + * sampling
> > + * @param void
> > + * @return int status
> > + */
> > +s32 da9052_tsi_start_sampling(void){
> > +
> > + /* This request is only valid when we are waiting for one */
> > + if(tsi_info.tsi_state == WAIT_FOR_SAMPLING){
> > + DA9052_DEBUG("__%s__ \n", __FUNCTION__);
> > + tsi_info.tsi_state = SAMPLING_ACTIVE;
> > + return (SUCCESS);
> > + }
> > + else{
> > + DA9052_DEBUG("Invalid request for
> da9052_tsi_start_sampling() \n");
> > + return (FAILURE);
> > + }
> > +}
> > +
> > +/**
> > + * da9052_tsi_stop_sampling: Called by TSI driver to
> indicate end of
> data
> > + * sampling
> > + * @param void
> > + * @return int status
> > + */
> > +s32 da9052_tsi_stop_sampling(void){
> > +
> > + /* Irrespective of current state, move to initial state */
> > + DA9052_DEBUG("__%s__ \n", __FUNCTION__);
> > + tsi_info.tsi_state = WAIT_FOR_PEN_DOWN;
> > +
> > + return (SUCCESS);
> > +}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Mark Brown on
On Wed, Jun 09, 2010 at 05:04:09PM +0530, Vijay R. Iyengar wrote:

[Please fix your MUA to word wrap lines at ~80 columns. I've reflowed
the text here]

> > These are looking pretyt touch screen specific. What are
> > they doing in
> > this patch?

> We have designed the EH (interrupt handler driver) and the TSI (touch
> screen driver) in a manner that the EH reads raw co-ordinates from the
> touch control registers of the DA9052 hardware in response to sampling
> ready interrupt requests and stores them in a FIFO for TSI further
> processing.

> We are implementing a common Event handler module for processing all
> the interrupt events from the DA9052 devices like battery, regulator,
> RTC etc. Fetching the TSI samples on touch events by EH rather than
> notifying the TSI driver through a call-back routine helps us to get a
> good amount of filtered touch samples from the TSI driver.

> Hence we intend to keep the TSI raw co-ordinate reading in the event
> handling module, which is a part of the MFD driver.

The additional effort involved in pushing the interrupt notification to
the touchscreen driver is so small that it's not going to make a
meaningful difference to the performance of your driver. I'd guess
you're already doing an equivalent amount of work managing the data
queues so I'd anticipate that the performance would be at worst
equivalent and likely better using something idiomatic (given that you
could inject the touchscreen data into the input subsystem immediately
from within interrupt context rather than having threads sitting around
in the background which need to at least be scheduled before the data is
delivered).

Obviously, if you've got benchmarking that demonstrates different
performance then I'm sure people would be interested in improving the
standard Linux stack for this.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/