Re: [RFC PATCH 0/1] input/touchscreen: Synaptics Touchscreen Driver

Previous thread: Re: [PATCH] Add intel drm blacklist to intel_opregion_present detect by Joey Lee on Wednesday, August 25, 2010 - 2:52 am. (1 message)

Next thread: [MeeGo-Dev][PATCH] Topcliff: Update PCH_PHUB driver to 2.6.35 by Masayuki Ohtak on Wednesday, August 25, 2010 - 3:16 am. (1 message)
From: Naveen Kumar GADDIPATI
Date: Wednesday, August 25, 2010 - 2:29 am

Hi Christopher,

Some generic review comments for this patch

1. Normally, defconfig should be a separate patch. You need not
post the entire defconfig, but only the changes to the Makefiles and
the Kconfig files.

2. All RMI specific files can be moved to a separate driver folder
in input/ like drivers/input/rmi/*.


We are also using the Synaptics RMI4 touch pad on our U8500 platform. 
We had to make some changes to these posted drivers to make it work on 
our platform. Also, we modified the touch screen driver to be compliant 
to the kernel coding guidelines, but as a stand alone driver though.

I will soon post out an RFC for our patch which we got working on our board. 
Please do have a look and we would like you guys to incorporate these changes 
into your final patch sets as well, so that we can avoid any re-works later on 
when your patch set gets merged into the mainline kernel.


Cheers!
Naveen
--

From: Christopher Heiny
Date: Wednesday, August 25, 2010 - 1:59 pm

Hi Naveen,

Thanks very much for your input!

We'll be changing the patch structure with our next submission, as the 
current patch file is getting kind of large and clumsy.  We'll split the 
defconfig out into its own patch at that point.

We'll also be eliminating the Hungarian notation in the next submission.

Your suggestion regarding the location of the RMI specific files is 
interesting.  We'll defer to Dmitry on where he'd like to see those 
files placed, though.

Unfortunately, the patch you submitted appears to be based on the older 
monolithic RMI3/Android driver, which is limited in functionality and 
maintainability.  Also, your submitted patch appears to be specific to 
the U8500 platform.

The aim of the current work is to develop a platform independent modular 
driver that will support all RMI4 functionality in a generic fashion. 
We'll be more than happy to work with your team to develop support for 
the U8500 under the modular structure.  If we proceed in that direction, 
it eliminates redundant effort and the need for any rework when our 
driver gets merged into the mainline.

I think the first step there should be for your team to resubmit your 
work as modifications of the current development effort, rather than 
backporting the current effort onto the obsolete framework.  Although 
that involves upfront work for your team in the near term, it provides a 
reduction in effort all around going forward:
     - your team doesn't need to keep back-patching changes onto
       the obsolete driver structure
     - our team doesn't need to keep forward-patching U8500
       related changes into the current driver structure

In a recent email conversation with me, Dmitry indicated that he will be 
creating a development branch for this work in the near future.  That 
will greatly facilitate this effort.

					Thanks,
						Chris
--

Previous thread: Re: [PATCH] Add intel drm blacklist to intel_opregion_present detect by Joey Lee on Wednesday, August 25, 2010 - 2:52 am. (1 message)

Next thread: [MeeGo-Dev][PATCH] Topcliff: Update PCH_PHUB driver to 2.6.35 by Masayuki Ohtak on Wednesday, August 25, 2010 - 3:16 am. (1 message)