Re: [PATCH v2 2/2] pxa/hx4700: Add Synaptics NavPoint (PXA27x SSP/SPI) driver

From
Paul Parsons <hidden>
Date
2011-08-16 11:54:45
Also in
linux-arm-kernel
Reply to
Igor Grinberg, 2011-08-16
quoted
MFP_CFG_OUT(GPIO102, ...) had already moved from the
patch v1 platform file to patch v2 mfp-pxa27x.h because it
was suggested that MFP macros should not be used directly.
quoted
Changing the direction in the platform file would
surely require using the MFP macros again, so how to keep
everyone happy? Maybe I should just define a more generic
name such as GPIO102_GPIO_OUT?

No, you will not need to use the MFP macros, just
gpio_direction_*() calls.
Eric has already answered this and I agree with him.
Use the MFP to configure the alternate function and other
MFP related stuff
and use the gpio_direction_*() calls later to set the
direction of the GPIO.
OK, will do.
quoted
Because the underlying hardware is a touchpad
controller and most of the touchpad drivers live in the
mouse directory. I could have added a mouse interface to
this driver but chose not to (at least for now) because the
hx4700 platform already has a working touchscreen
controller; a second mouse device was not needed. If future
platforms require a mouse interface then one could be added
relatively easily; surely this would be preferable to
replicating the whole driver.

So the device is a full touchpad? and can be used for mouse
pointer?
Yes, that's correct.
Is the same device used as mouse pointer by some other
driver?
Not as far as I can see. The word "NavPoint" appears only once in the current kernel source: within the hx4700 platform file. It's possible that other PDA models of similar 2004 vintage used the device, but I don't know whether any of them are supported in the kernel.
quoted
The suspend and resume functions check that the gpio
is valid (which is taken to be non-zero) before using it.
The platform file has already configured the gpio for
output; it's the GPIO102 discussed earlier.

What I'm suggesting here is check that the gpio is valid
here and only once
and not in the suspend/resume functions. That gpio is not
going to change,
so there is no point is checking it each time in
suspend/resume.
Also the check for non-zero is not correct. 0 is a valid
gpio number on most platforms.
You need to use the gpio_is_valid() call for this.
I understand. My feeling was that other platforms wanting to use this driver would not necessarily use a gpio for power on/off; indeed the gpio field was originally a callback to provide full flexibility. Hence suspend / resume needed to use the gpio conditionally anyway. Nevertheless I'm happy to make the change and leave it for future platform developers to revisit suspend / resume as necessary.

Regards,
Paul
Thread (6 messages) 6 messages, 3 authors, last reply 2011-08-16
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help