Re: ad714x driver help and possible bug

Re: ad714x driver help and possible bug

[Jonathan Cameron]

Cc'd input, and analog devices driver list...

On 04/28/11 19:17, Jean-Francois Dagenais wrote:
> Hi all,
> 
> I am having trouble getting the ad714x (i2c) driver to work in my
> test setup. I am using the VGA i2c bus to talk to the ad7147 I have.
> I used INTA of a PCI ethernet slot in my PC. I enabled the PCI device
> without the driver module loaded.  I then give the interrupt number
> to ad714x through the struct i2c_board_info. I actually tried the
> same setup on two PCs, one intel graphics, the other nvidia to
> eliminate the i2c master as a possible cause of my problem.
> 
> The device is successfully loaded and I can see the interrupts going.
> The eventN device created under /dev/input never spit out anything
> and so I added printks in the threaded ISR handler to see what is
> going on.
> 
> I only have a wheel with 8 stages. In ad714x_wheel_state_machine() I
> see that upon the first interrupt, the state goes from IDLE to
> JITTER. After this the JITTER case checks that c_state == mask (with
> mask being 0xff in our case). This condition is never met and the
> driver stays indefinitely in this state. After lifting my finger from
> the wheel, the chip settles down to scanning every so many
> milliseconds.
> 
> The STAGE_COMPLETE_INT_STATUS is always 0 when my finger is off, but
> varies a lot while my finger is on (while interrupt frequency is
> high). Looking at the value of STAGE_COMPLETE_INT_STATUS in binary
> reveals that the set bits are always in groups, e.g. 0x0007 or 0x001C
> or 0x0081(I imagine a roll-over of our start_stage-end_stage (0-7)).
> There seems to be a timing aspect here. I added a spin counter in the
> threaded ISR to delay reading the 3 registers and that seemed to make
> the c_state change a little.
> 
> I modified the code that reads the 3 registers right after the
> mutex_lock in ad714x_interrupt_thread so that the
> STAGE_COMPLETE_INT_STATUS is read before the other two (LOW and HIGH
> regs). The result was surprising. The COMPLETE reg did read 0xff now
> and the JITTER case went past the "if(c_state == mask)" but later
> crashed (divide by 0) in ad714x_wheel_cal_abs_pos() called from the
> JITTER case.
> 
> 
> Here's the initial configuration I give the driver:
> 
> static struct ad714x_wheel_plat wheel_platform_data = {
> 	.start_stage = 0, // int start_stage;
> 	.end_stage = 7, // int end_stage;
> 	.max_coord = 128,  // int max_coord;
> };
> 
> static struct ad714x_platform_data wheel_dev_platform_data = {
> 	.slider_num = 0,
> 	.wheel_num = 1,
> 	.touchpad_num = 0,
> 	.button_num = 0,
> 	.slider = 0,
> 	.wheel = &wheel_platform_data, // struct ad714x_wheel_plat *wheel;
> 	.touchpad = 0, // struct ad714x_touchpad_plat *touchpad;
> 	.button = 0, // struct ad714x_button_plat *button;
> 	.stage_cfg_reg =  { /* unsigned short stage_cfg_reg[STAGE_NUM][STAGE_CFGREG_NUM] */
> 		{0xfffe, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
> 		{0xfffb, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
> 		{0xffef, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
> 		{0xffbf, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
> 		{0xfeff, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
> 		{0xfbff, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
> 		{0xefff, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
> 		{0xffff, 0x3ffe, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
> 
> 		{0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
> 		{0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
> 		{0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
> 		{0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
> 	},
> 	.sys_cfg_reg = {0x027e, 0x00ff, 0x3233, 0x0819, 0x0832, 0x0000, 0x00ff, 0}, /* unsigned short sys_cfg_reg[SYS_CFGREG_NUM] */
> 	//.sys_cfg_reg = {0x2b2, 0xfff, 0x3233, 0x819, 0x832, 0xcff, 0xcff, 0x0}, /* unsigned short sys_cfg_reg[SYS_CFGREG_NUM] */
> };
> 

> 
> I also had to change the request_threaded_irq flags to specify
> IRQF_ONESHOT so the kernel keeps the interrupt masked while we are
> running ad714x_interrupt_thread(). Otherwise we were getting storms
> of interrupts each time only one was requested. I am wondering if
> this should be pulled back to the mainline kernel?
> 
> Thanks for pointers and clues!
> 

Re: ad714x driver help and possible bug

[Michael Hennerich]

On 04/29/2011 04:52 PM, Michael Hennerich wrote:
> On 04/29/2011 11:55 AM, Jonathan Cameron wrote:
>> Cc'd input, and analog devices driver list...
>>
>> On 04/28/11 19:17, Jean-Francois Dagenais wrote:
>>   
>>
>>> I also had to change the request_threaded_irq flags to specify
>>> IRQF_ONESHOT so the kernel keeps the interrupt masked while we are
>>> running ad714x_interrupt_thread(). Otherwise we were getting storms
>>> of interrupts each time only one was requested. I am wondering if
>>> this should be pulled back to the mainline kernel?
>>>
>>> Thanks for pointers and clues!
>>>
>>>     
> I assume this is due to the fact that the interrupt on the host is
> level sensitive.
> In this case you need to use the IRQF_ONESHOT flag.
> The unmodified driver requests the IRQ as IRQF_TRIGGER_FALLING, so
> this is not necessary.
>
> To my knowledge we regularly test the driver with the sliders, wheels
> and buttons found on the
> official evaluation board, and so far we haven't found oddities.
> For an example platform file see: arch/blackfin/mach-bf537/boards/stamp.c
> From top of my head I don't know whether we're testing with the SPI or
> I2C interface.
>
> http://wiki.analog.com/software/driver/linux/ad714x
>
-- 

Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: ad714x driver help and possible bug

[Jean-Francois Dagenais]

On Apr 29, 2011, at 10:58, Michael Hennerich wrote:

> On 04/29/2011 04:52 PM, Michael Hennerich wrote:
>> On 04/29/2011 11:55 AM, Jonathan Cameron wrote:
>>> Cc'd input, and analog devices driver list...
>>> 
>>> On 04/28/11 19:17, Jean-Francois Dagenais wrote:
>>> 
>>> 
>>>> I also had to change the request_threaded_irq flags to specify
>>>> IRQF_ONESHOT so the kernel keeps the interrupt masked while we are
>>>> running ad714x_interrupt_thread(). Otherwise we were getting storms
>>>> of interrupts each time only one was requested. I am wondering if
>>>> this should be pulled back to the mainline kernel?
>>>> 
>>>> Thanks for pointers and clues!
>>>> 
>>>> 
>> I assume this is due to the fact that the interrupt on the host is
>> level sensitive.
>> In this case you need to use the IRQF_ONESHOT flag.
>> The unmodified driver requests the IRQ as IRQF_TRIGGER_FALLING, so
>> this is not necessary.
I succeeded in making the ISR occur only once per real interrupt, the problem I explained earlier remains.
>> 
>> To my knowledge we regularly test the driver with the sliders, wheels
>> and buttons found on the
>> official evaluation board, and so far we haven't found oddities.
>> For an example platform file see: arch/blackfin/mach-bf537/boards/stamp.c
>> From top of my head I don't know whether we're testing with the SPI or
>> I2C interface.
>> 
>> http://wiki.analog.com/software/driver/linux/ad714x
>> 
> -- 
> 
> Greetings,
> Michael
> 
> --
> Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
> Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
> Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
> Margaret Seif
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: ad714x driver help and possible bug

[Jean-Francois Dagenais]

On Apr 29, 2011, at 5:55, Jonathan Cameron wrote:

> Cc'd input, and analog devices driver list...
> 
> On 04/28/11 19:17, Jean-Francois Dagenais wrote:
>> Hi all,
>> 
>> I am having trouble getting the ad714x (i2c) driver to work in my
>> test setup. I am using the VGA i2c bus to talk to the ad7147 I have.
>> I used INTA of a PCI ethernet slot in my PC. I enabled the PCI device
>> without the driver module loaded.  I then give the interrupt number
>> to ad714x through the struct i2c_board_info. I actually tried the
>> same setup on two PCs, one intel graphics, the other nvidia to
>> eliminate the i2c master as a possible cause of my problem.
>> 
>> The device is successfully loaded and I can see the interrupts going.
>> The eventN device created under /dev/input never spit out anything
>> and so I added printks in the threaded ISR handler to see what is
>> going on.
>> 
>> I only have a wheel with 8 stages. In ad714x_wheel_state_machine() I
>> see that upon the first interrupt, the state goes from IDLE to
>> JITTER. After this the JITTER case checks that c_state == mask (with
>> mask being 0xff in our case). This condition is never met and the
>> driver stays indefinitely in this state. After lifting my finger from
>> the wheel, the chip settles down to scanning every so many
>> milliseconds.
>> 
>> The STAGE_COMPLETE_INT_STATUS is always 0 when my finger is off, but
>> varies a lot while my finger is on (while interrupt frequency is
>> high). Looking at the value of STAGE_COMPLETE_INT_STATUS in binary
>> reveals that the set bits are always in groups, e.g. 0x0007 or 0x001C
>> or 0x0081(I imagine a roll-over of our start_stage-end_stage (0-7)).
>> There seems to be a timing aspect here. I added a spin counter in the
>> threaded ISR to delay reading the 3 registers and that seemed to make
>> the c_state change a little.
>> 
>> I modified the code that reads the 3 registers right after the
>> mutex_lock in ad714x_interrupt_thread so that the
>> STAGE_COMPLETE_INT_STATUS is read before the other two (LOW and HIGH
>> regs). The result was surprising. The COMPLETE reg did read 0xff now
>> and the JITTER case went past the "if(c_state == mask)" but later
>> crashed (divide by 0) in ad714x_wheel_cal_abs_pos() called from the
>> JITTER case.
>> 
>> 
>> Here's the initial configuration I give the driver:
>> 
>> static struct ad714x_wheel_plat wheel_platform_data = {
>> 	.start_stage = 0, // int start_stage;
>> 	.end_stage = 7, // int end_stage;
>> 	.max_coord = 128,  // int max_coord;
>> };
>> 
>> static struct ad714x_platform_data wheel_dev_platform_data = {
>> 	.slider_num = 0,
>> 	.wheel_num = 1,
>> 	.touchpad_num = 0,
>> 	.button_num = 0,
>> 	.slider = 0,
>> 	.wheel = &wheel_platform_data, // struct ad714x_wheel_plat *wheel;
>> 	.touchpad = 0, // struct ad714x_touchpad_plat *touchpad;
>> 	.button = 0, // struct ad714x_button_plat *button;
>> 	.stage_cfg_reg =  { /* unsigned short stage_cfg_reg[STAGE_NUM][STAGE_CFGREG_NUM] */
>> 		{0xfffe, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>> 		{0xfffb, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>> 		{0xffef, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>> 		{0xffbf, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>> 		{0xfeff, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>> 		{0xfbff, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>> 		{0xefff, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>> 		{0xffff, 0x3ffe, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>> 
>> 		{0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
>> 		{0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
>> 		{0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
>> 		{0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
>> 	},
>> 	.sys_cfg_reg = {0x027e, 0x00ff, 0x3233, 0x0819, 0x0832, 0x0000, 0x00ff, 0}, /* unsigned short sys_cfg_reg[SYS_CFGREG_NUM] */
>> 	//.sys_cfg_reg = {0x2b2, 0xfff, 0x3233, 0x819, 0x832, 0xcff, 0xcff, 0x0}, /* unsigned short sys_cfg_reg[SYS_CFGREG_NUM] */
>> };
>> 
> 
>> 
>> I also had to change the request_threaded_irq flags to specify
>> IRQF_ONESHOT so the kernel keeps the interrupt masked while we are
>> running ad714x_interrupt_thread(). Otherwise we were getting storms
>> of interrupts each time only one was requested. I am wondering if
>> this should be pulled back to the mainline kernel?
>> 
>> Thanks for pointers and clues!
>> 
> 
here's the printk I added to ad714x_wheel_state_machine()

	mask = ((1 << (hw->end_stage + 1)) - 1) - ((1 << hw->start_stage) - 1);

	h_state = ad714x->h_state & mask;
	c_state = ad714x->c_state & mask;
	dev_dbg(ad714x->dev, "interrupt state:%d mask:0x%x l:0x%x h:0x%x c:0x%x\n", 
		sw->state, 
		(u32)mask, 
		(u32)ad714x->l_state, 
		(u32)ad714x->h_state, 
		(u32)ad714x->c_state);

Here what it looks like upon loading a module which does the i2c_new_device of the AD7147:

<7>[58302.186886] my-pci-stub 0000:01:04.0: claimed by my platform module PCI stub
<6>[58302.186903] my-pci-stub 0000:01:04.0: PCI INT A -> GSI 17 (level, low) -> IRQ 17
<6>[58302.189815] ad714x_captouch 9-002c: found AD7147(A) captouch, rev:1
<6>[58302.427237] input: Unspecified device as /devices/virtual/input/input8

[ pause here while my hand goes from my mouse and keyboard to the wheel on the AD7147 ]

<7>[58311.646183] ad714x_captouch 9-002c: interrupt state:0 mask:0xff l:0x0 h:0x18 c:0x1c
<7>[58311.646192] ad714x_captouch 9-002c: case IDLE in if
<7>[58311.655436] ad714x_captouch 9-002c: wheel 0 touched
<7>[58311.663087] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x7
<7>[58311.674562] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
<7>[58311.686803] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
<7>[58311.699147] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
<7>[58311.711430] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
<7>[58311.723585] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
<7>[58311.736017] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
<7>[58311.748298] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
<7>[58311.760581] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
<7>[58311.772800] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
<7>[58311.785176] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
<7>[58311.797473] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
<7>[58311.809651] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x10 c:0x81

[ here I lift my finger ]

<7>[58311.822059] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
<7>[58311.834345] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
<7>[58311.846582] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
<7>[58311.858800] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
<7>[58311.871212] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
<7>[58311.883517] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
<7>[58311.895802] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
<7>[58311.908099] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
<7>[58311.920381] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
<7>[58311.932585] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81

[...]

<7>[58313.432218] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
<7>[58314.157343] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81

[... after 2 seconds or so, the rhythm slows down to 2 interrupts per second or so ]

<7>[58314.169629] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x80

<7>[58314.976518] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x80

<7>[58315.783342] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x80


There is a clue in what I did next. I added a wait time in the isr thread function like so:

static irqreturn_t ad714x_interrupt_thread(int irq, void *data)
{
	struct ad714x_chip *ad714x = data;
	volatile int i;

	mutex_lock(&ad714x->mutex);

	i=0xffffff;
	while(i)
		--i;

	ad714x->read(ad714x->dev, STG_LOW_INT_STA_REG, &ad714x->l_state);
	ad714x->read(ad714x->dev, STG_HIGH_INT_STA_REG, &ad714x->h_state);
	ad714x->read(ad714x->dev, STG_COM_INT_STA_REG, &ad714x->c_state);
[ ... ]

this changes the above trace to these values:

while touching the wheel:
<7>[63085.414268] ad714x_captouch 9-002c: interrupt state:0 mask:0xff l:0x0 h:0x70 c:0x60
<7>[63085.414277] ad714x_captouch 9-002c: case IDLE in if
<7>[63085.423519] ad714x_captouch 9-002c: wheel 0 touched
<7>[63085.578931] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x70 c:0x18
<7>[63085.736079] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x70 c:0x3
<7>[63085.832304] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x70 c:0x60
<7>[63086.938334] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x30 c:0x60
<7>[63087.030835] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
<7>[63087.122909] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
<7>[63087.215014] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
<7>[63087.307071] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
<7>[63087.399367] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
<7>[63087.739386] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x83

Again, notice the state going from 0 (IDLE) to 1(JITTER), and never entering the "if" in the JITTER case (needs mask == c_state).  The HIGH register varies a lot while I move my finger around, but the COMPLETE looks like its always going and being cleared. I mentioned before that reading the COMPLETE status reg before the LOW and HIGH produces completely different results.

I am suspecting the register configuration we use is off somehow, I will review them thoroughly. Aside from this we are running out of leads here, anyone has input on this?

Thanks in advance!
JFD

Re: ad714x driver help and possible bug

[Michael Hennerich]

On 05/03/2011 04:13 PM, Jean-Francois Dagenais wrote:
> On Apr 29, 2011, at 5:55, Jonathan Cameron wrote:
>
>   
>> Cc'd input, and analog devices driver list...
>>
>> On 04/28/11 19:17, Jean-Francois Dagenais wrote:
>>     
>>> Hi all,
>>>
>>> I am having trouble getting the ad714x (i2c) driver to work in my
>>> test setup. I am using the VGA i2c bus to talk to the ad7147 I have.
>>> I used INTA of a PCI ethernet slot in my PC. I enabled the PCI device
>>> without the driver module loaded.  I then give the interrupt number
>>> to ad714x through the struct i2c_board_info. I actually tried the
>>> same setup on two PCs, one intel graphics, the other nvidia to
>>> eliminate the i2c master as a possible cause of my problem.
>>>
>>> The device is successfully loaded and I can see the interrupts going.
>>> The eventN device created under /dev/input never spit out anything
>>> and so I added printks in the threaded ISR handler to see what is
>>> going on.
>>>
>>> I only have a wheel with 8 stages. In ad714x_wheel_state_machine() I
>>> see that upon the first interrupt, the state goes from IDLE to
>>> JITTER. After this the JITTER case checks that c_state == mask (with
>>> mask being 0xff in our case). This condition is never met and the
>>> driver stays indefinitely in this state. After lifting my finger from
>>> the wheel, the chip settles down to scanning every so many
>>> milliseconds.
>>>
>>> The STAGE_COMPLETE_INT_STATUS is always 0 when my finger is off, but
>>> varies a lot while my finger is on (while interrupt frequency is
>>> high). Looking at the value of STAGE_COMPLETE_INT_STATUS in binary
>>> reveals that the set bits are always in groups, e.g. 0x0007 or 0x001C
>>> or 0x0081(I imagine a roll-over of our start_stage-end_stage (0-7)).
>>> There seems to be a timing aspect here. I added a spin counter in the
>>> threaded ISR to delay reading the 3 registers and that seemed to make
>>> the c_state change a little.
>>>
>>> I modified the code that reads the 3 registers right after the
>>> mutex_lock in ad714x_interrupt_thread so that the
>>> STAGE_COMPLETE_INT_STATUS is read before the other two (LOW and HIGH
>>> regs). The result was surprising. The COMPLETE reg did read 0xff now
>>> and the JITTER case went past the "if(c_state == mask)" but later
>>> crashed (divide by 0) in ad714x_wheel_cal_abs_pos() called from the
>>> JITTER case.
>>>
>>>
>>> Here's the initial configuration I give the driver:
>>>
>>> static struct ad714x_wheel_plat wheel_platform_data = {
>>>      .start_stage = 0, // int start_stage;
>>>      .end_stage = 7, // int end_stage;
>>>      .max_coord = 128,  // int max_coord;
>>> };
>>>
>>> static struct ad714x_platform_data wheel_dev_platform_data = {
>>>      .slider_num = 0,
>>>      .wheel_num = 1,
>>>      .touchpad_num = 0,
>>>      .button_num = 0,
>>>      .slider = 0,
>>>      .wheel = &wheel_platform_data, // struct ad714x_wheel_plat *wheel;
>>>      .touchpad = 0, // struct ad714x_touchpad_plat *touchpad;
>>>      .button = 0, // struct ad714x_button_plat *button;
>>>      .stage_cfg_reg =  { /* unsigned short stage_cfg_reg[STAGE_NUM][STAGE_CFGREG_NUM] */
>>>              {0xfffe, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>              {0xfffb, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>              {0xffef, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>              {0xffbf, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>              {0xfeff, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>              {0xfbff, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>              {0xefff, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>              {0xffff, 0x3ffe, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>
>>>              {0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
>>>              {0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
>>>              {0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
>>>              {0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
>>>      },
>>>      .sys_cfg_reg = {0x027e, 0x00ff, 0x3233, 0x0819, 0x0832, 0x0000, 0x00ff, 0}, /* unsigned short sys_cfg_reg[SYS_CFGREG_NUM] */
>>>      //.sys_cfg_reg = {0x2b2, 0xfff, 0x3233, 0x819, 0x832, 0xcff, 0xcff, 0x0}, /* unsigned short sys_cfg_reg[SYS_CFGREG_NUM] */
>>> };
>>>
>>>       
>>     
>>> I also had to change the request_threaded_irq flags to specify
>>> IRQF_ONESHOT so the kernel keeps the interrupt masked while we are
>>> running ad714x_interrupt_thread(). Otherwise we were getting storms
>>> of interrupts each time only one was requested. I am wondering if
>>> this should be pulled back to the mainline kernel?
>>>
>>> Thanks for pointers and clues!
>>>
>>>       
>>     
> here's the printk I added to ad714x_wheel_state_machine()
>
>         mask = ((1 << (hw->end_stage + 1)) - 1) - ((1 << hw->start_stage) - 1);
>
>         h_state = ad714x->h_state & mask;
>         c_state = ad714x->c_state & mask;
>         dev_dbg(ad714x->dev, "interrupt state:%d mask:0x%x l:0x%x h:0x%x c:0x%x\n",
>                 sw->state,
>                 (u32)mask,
>                 (u32)ad714x->l_state,
>                 (u32)ad714x->h_state,
>                 (u32)ad714x->c_state);
>
> Here what it looks like upon loading a module which does the i2c_new_device of the AD7147:
>
> <7>[58302.186886] my-pci-stub 0000:01:04.0: claimed by my platform module PCI stub
> <6>[58302.186903] my-pci-stub 0000:01:04.0: PCI INT A -> GSI 17 (level, low) -> IRQ 17
> <6>[58302.189815] ad714x_captouch 9-002c: found AD7147(A) captouch, rev:1
> <6>[58302.427237] input: Unspecified device as /devices/virtual/input/input8
>
> [ pause here while my hand goes from my mouse and keyboard to the wheel on the AD7147 ]
>
> <7>[58311.646183] ad714x_captouch 9-002c: interrupt state:0 mask:0xff l:0x0 h:0x18 c:0x1c
> <7>[58311.646192] ad714x_captouch 9-002c: case IDLE in if
> <7>[58311.655436] ad714x_captouch 9-002c: wheel 0 touched
> <7>[58311.663087] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x7
> <7>[58311.674562] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
> <7>[58311.686803] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
> <7>[58311.699147] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
> <7>[58311.711430] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
> <7>[58311.723585] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
> <7>[58311.736017] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
> <7>[58311.748298] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
> <7>[58311.760581] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
> <7>[58311.772800] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
> <7>[58311.785176] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
> <7>[58311.797473] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
> <7>[58311.809651] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x10 c:0x81
>
> [ here I lift my finger ]
>
> <7>[58311.822059] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
> <7>[58311.834345] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
> <7>[58311.846582] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
> <7>[58311.858800] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
> <7>[58311.871212] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
> <7>[58311.883517] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
> <7>[58311.895802] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
> <7>[58311.908099] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
> <7>[58311.920381] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
> <7>[58311.932585] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>
> [...]
>
> <7>[58313.432218] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
> <7>[58314.157343] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>
> [... after 2 seconds or so, the rhythm slows down to 2 interrupts per second or so ]
>
> <7>[58314.169629] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x80
>
> <7>[58314.976518] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x80
>
> <7>[58315.783342] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x80
>
>
> There is a clue in what I did next. I added a wait time in the isr thread function like so:
>
> static irqreturn_t ad714x_interrupt_thread(int irq, void *data)
> {
>         struct ad714x_chip *ad714x = data;
>         volatile int i;
>
>         mutex_lock(&ad714x->mutex);
>
>         i=0xffffff;
>         while(i)
>                 --i;
>
>         ad714x->read(ad714x->dev, STG_LOW_INT_STA_REG, &ad714x->l_state);
>         ad714x->read(ad714x->dev, STG_HIGH_INT_STA_REG, &ad714x->h_state);
>         ad714x->read(ad714x->dev, STG_COM_INT_STA_REG, &ad714x->c_state);
> [ ... ]
>
> this changes the above trace to these values:
>
> while touching the wheel:
> <7>[63085.414268] ad714x_captouch 9-002c: interrupt state:0 mask:0xff l:0x0 h:0x70 c:0x60
> <7>[63085.414277] ad714x_captouch 9-002c: case IDLE in if
> <7>[63085.423519] ad714x_captouch 9-002c: wheel 0 touched
> <7>[63085.578931] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x70 c:0x18
> <7>[63085.736079] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x70 c:0x3
> <7>[63085.832304] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x70 c:0x60
> <7>[63086.938334] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x30 c:0x60
> <7>[63087.030835] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
> <7>[63087.122909] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
> <7>[63087.215014] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
> <7>[63087.307071] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
> <7>[63087.399367] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
> <7>[63087.739386] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x83
>
> Again, notice the state going from 0 (IDLE) to 1(JITTER), and never entering the "if" in the JITTER case (needs mask == c_state).  The HIGH register varies a lot while I move my finger around, but the COMPLETE looks like its always going and being cleared. I mentioned before that reading the COMPLETE status reg before the LOW and HIGH produces completely different results.
>
> I am suspecting the register configuration we use is off somehow, I will review them thoroughly. Aside from this we are running out of leads here, anyone has input on this?
>
> Thanks in advance!
> JFD
>   
Hi Jean-Francois,

Barry Song, the driver author left ADI quite some time ago
I don't have hardware to test things at the moment.

The EVAL-AD7147 only has sliders and buttons, so I don't know how useful
it will be here.
Need to check if I can find one having a  wheel.

-- 
Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: ad714x driver help and possible bug

[Michael Hennerich]

On 05/03/2011 04:33 PM, Michael Hennerich wrote:
> On 05/03/2011 04:13 PM, Jean-Francois Dagenais wrote:
>   
>> On Apr 29, 2011, at 5:55, Jonathan Cameron wrote:
>>
>>   
>>     
>>> Cc'd input, and analog devices driver list...
>>>
>>> On 04/28/11 19:17, Jean-Francois Dagenais wrote:
>>>     
>>>       
>>>> Hi all,
>>>>
>>>> I am having trouble getting the ad714x (i2c) driver to work in my
>>>> test setup. I am using the VGA i2c bus to talk to the ad7147 I have.
>>>> I used INTA of a PCI ethernet slot in my PC. I enabled the PCI device
>>>> without the driver module loaded.  I then give the interrupt number
>>>> to ad714x through the struct i2c_board_info. I actually tried the
>>>> same setup on two PCs, one intel graphics, the other nvidia to
>>>> eliminate the i2c master as a possible cause of my problem.
>>>>
>>>> The device is successfully loaded and I can see the interrupts going.
>>>> The eventN device created under /dev/input never spit out anything
>>>> and so I added printks in the threaded ISR handler to see what is
>>>> going on.
>>>>
>>>> I only have a wheel with 8 stages. In ad714x_wheel_state_machine() I
>>>> see that upon the first interrupt, the state goes from IDLE to
>>>> JITTER. After this the JITTER case checks that c_state == mask (with
>>>> mask being 0xff in our case). This condition is never met and the
>>>> driver stays indefinitely in this state. After lifting my finger from
>>>> the wheel, the chip settles down to scanning every so many
>>>> milliseconds.
>>>>
>>>> The STAGE_COMPLETE_INT_STATUS is always 0 when my finger is off, but
>>>> varies a lot while my finger is on (while interrupt frequency is
>>>> high). Looking at the value of STAGE_COMPLETE_INT_STATUS in binary
>>>> reveals that the set bits are always in groups, e.g. 0x0007 or 0x001C
>>>> or 0x0081(I imagine a roll-over of our start_stage-end_stage (0-7)).
>>>> There seems to be a timing aspect here. I added a spin counter in the
>>>> threaded ISR to delay reading the 3 registers and that seemed to make
>>>> the c_state change a little.
>>>>
>>>> I modified the code that reads the 3 registers right after the
>>>> mutex_lock in ad714x_interrupt_thread so that the
>>>> STAGE_COMPLETE_INT_STATUS is read before the other two (LOW and HIGH
>>>> regs). The result was surprising. The COMPLETE reg did read 0xff now
>>>> and the JITTER case went past the "if(c_state == mask)" but later
>>>> crashed (divide by 0) in ad714x_wheel_cal_abs_pos() called from the
>>>> JITTER case.
>>>>
>>>>
>>>> Here's the initial configuration I give the driver:
>>>>
>>>> static struct ad714x_wheel_plat wheel_platform_data = {
>>>>      .start_stage = 0, // int start_stage;
>>>>      .end_stage = 7, // int end_stage;
>>>>      .max_coord = 128,  // int max_coord;
>>>> };
>>>>
>>>> static struct ad714x_platform_data wheel_dev_platform_data = {
>>>>      .slider_num = 0,
>>>>      .wheel_num = 1,
>>>>      .touchpad_num = 0,
>>>>      .button_num = 0,
>>>>      .slider = 0,
>>>>      .wheel = &wheel_platform_data, // struct ad714x_wheel_plat *wheel;
>>>>      .touchpad = 0, // struct ad714x_touchpad_plat *touchpad;
>>>>      .button = 0, // struct ad714x_button_plat *button;
>>>>      .stage_cfg_reg =  { /* unsigned short stage_cfg_reg[STAGE_NUM][STAGE_CFGREG_NUM] */
>>>>              {0xfffe, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>>              {0xfffb, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>>              {0xffef, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>>              {0xffbf, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>>              {0xfeff, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>>              {0xfbff, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>>              {0xefff, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>>              {0xffff, 0x3ffe, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>>
>>>>              {0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
>>>>              {0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
>>>>              {0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
>>>>              {0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
>>>>      },
>>>>      .sys_cfg_reg = {0x027e, 0x00ff, 0x3233, 0x0819, 0x0832, 0x0000, 0x00ff, 0}, /* unsigned short sys_cfg_reg[SYS_CFGREG_NUM] */
>>>>      //.sys_cfg_reg = {0x2b2, 0xfff, 0x3233, 0x819, 0x832, 0xcff, 0xcff, 0x0}, /* unsigned short sys_cfg_reg[SYS_CFGREG_NUM] */
>>>> };
>>>>
>>>>       
>>>>         
>>>     
>>>       
>>>> I also had to change the request_threaded_irq flags to specify
>>>> IRQF_ONESHOT so the kernel keeps the interrupt masked while we are
>>>> running ad714x_interrupt_thread(). Otherwise we were getting storms
>>>> of interrupts each time only one was requested. I am wondering if
>>>> this should be pulled back to the mainline kernel?
>>>>
>>>> Thanks for pointers and clues!
>>>>
>>>>       
>>>>         
>>>     
>>>       
>> here's the printk I added to ad714x_wheel_state_machine()
>>
>>         mask = ((1 << (hw->end_stage + 1)) - 1) - ((1 << hw->start_stage) - 1);
>>
>>         h_state = ad714x->h_state & mask;
>>         c_state = ad714x->c_state & mask;
>>         dev_dbg(ad714x->dev, "interrupt state:%d mask:0x%x l:0x%x h:0x%x c:0x%x\n",
>>                 sw->state,
>>                 (u32)mask,
>>                 (u32)ad714x->l_state,
>>                 (u32)ad714x->h_state,
>>                 (u32)ad714x->c_state);
>>
>> Here what it looks like upon loading a module which does the i2c_new_device of the AD7147:
>>
>> <7>[58302.186886] my-pci-stub 0000:01:04.0: claimed by my platform module PCI stub
>> <6>[58302.186903] my-pci-stub 0000:01:04.0: PCI INT A -> GSI 17 (level, low) -> IRQ 17
>> <6>[58302.189815] ad714x_captouch 9-002c: found AD7147(A) captouch, rev:1
>> <6>[58302.427237] input: Unspecified device as /devices/virtual/input/input8
>>
>> [ pause here while my hand goes from my mouse and keyboard to the wheel on the AD7147 ]
>>
>> <7>[58311.646183] ad714x_captouch 9-002c: interrupt state:0 mask:0xff l:0x0 h:0x18 c:0x1c
>> <7>[58311.646192] ad714x_captouch 9-002c: case IDLE in if
>> <7>[58311.655436] ad714x_captouch 9-002c: wheel 0 touched
>> <7>[58311.663087] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x7
>> <7>[58311.674562] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
>> <7>[58311.686803] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
>> <7>[58311.699147] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
>> <7>[58311.711430] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
>> <7>[58311.723585] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
>> <7>[58311.736017] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
>> <7>[58311.748298] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
>> <7>[58311.760581] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
>> <7>[58311.772800] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
>> <7>[58311.785176] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
>> <7>[58311.797473] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
>> <7>[58311.809651] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x10 c:0x81
>>
>> [ here I lift my finger ]
>>
>> <7>[58311.822059] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>> <7>[58311.834345] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>> <7>[58311.846582] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>> <7>[58311.858800] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>> <7>[58311.871212] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>> <7>[58311.883517] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>> <7>[58311.895802] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>> <7>[58311.908099] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>> <7>[58311.920381] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>> <7>[58311.932585] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>>
>> [...]
>>
>> <7>[58313.432218] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>> <7>[58314.157343] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>>
>> [... after 2 seconds or so, the rhythm slows down to 2 interrupts per second or so ]
>>
>> <7>[58314.169629] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x80
>>
>> <7>[58314.976518] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x80
>>
>> <7>[58315.783342] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x80
>>
>>
>> There is a clue in what I did next. I added a wait time in the isr thread function like so:
>>
>> static irqreturn_t ad714x_interrupt_thread(int irq, void *data)
>> {
>>         struct ad714x_chip *ad714x = data;
>>         volatile int i;
>>
>>         mutex_lock(&ad714x->mutex);
>>
>>         i=0xffffff;
>>         while(i)
>>                 --i;
>>
>>         ad714x->read(ad714x->dev, STG_LOW_INT_STA_REG, &ad714x->l_state);
>>         ad714x->read(ad714x->dev, STG_HIGH_INT_STA_REG, &ad714x->h_state);
>>         ad714x->read(ad714x->dev, STG_COM_INT_STA_REG, &ad714x->c_state);
>> [ ... ]
>>
>> this changes the above trace to these values:
>>
>> while touching the wheel:
>> <7>[63085.414268] ad714x_captouch 9-002c: interrupt state:0 mask:0xff l:0x0 h:0x70 c:0x60
>> <7>[63085.414277] ad714x_captouch 9-002c: case IDLE in if
>> <7>[63085.423519] ad714x_captouch 9-002c: wheel 0 touched
>> <7>[63085.578931] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x70 c:0x18
>> <7>[63085.736079] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x70 c:0x3
>> <7>[63085.832304] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x70 c:0x60
>> <7>[63086.938334] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x30 c:0x60
>> <7>[63087.030835] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
>> <7>[63087.122909] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
>> <7>[63087.215014] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
>> <7>[63087.307071] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
>> <7>[63087.399367] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
>> <7>[63087.739386] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x83
>>
>> Again, notice the state going from 0 (IDLE) to 1(JITTER), and never entering the "if" in the JITTER case (needs mask == c_state).  The HIGH register varies a lot while I move my finger around, but the COMPLETE looks like its always going and being cleared. I mentioned before that reading the COMPLETE status reg before the LOW and HIGH produces completely different results.
>>
>> I am suspecting the register configuration we use is off somehow, I will review them thoroughly. Aside from this we are running out of leads here, anyone has input on this?
>>
>> Thanks in advance!
>> JFD
>>   
>>     
> Hi Jean-Francois,
>
> Barry Song, the driver author left ADI quite some time ago
> I don't have hardware to test things at the moment.
>
> The EVAL-AD7147 only has sliders and buttons, so I don't know how useful
> it will be here.
> Need to check if I can find one having a  wheel.
>
>   
Hi Jean-Francois,

The ADZS-BFLLCD-EZEXT features a AD7147-1 + wheel.
I'll take a look and get back to you later this week.

-- 
Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: ad714x driver help and possible bug

[Jean-François Dagenais]

On May 3, 2011, at 11:16 AM, Michael Hennerich wrote:

> On 05/03/2011 04:33 PM, Michael Hennerich wrote:
>> On 05/03/2011 04:13 PM, Jean-Francois Dagenais wrote:
>> 
>>> On Apr 29, 2011, at 5:55, Jonathan Cameron wrote:
>>> 
>>> 
>>> 
>>>> Cc'd input, and analog devices driver list...
>>>> 
>>>> On 04/28/11 19:17, Jean-Francois Dagenais wrote:
>>>> 
>>>> 
>>>>> Hi all,
>>>>> 
>>>>> I am having trouble getting the ad714x (i2c) driver to work in my
>>>>> test setup. I am using the VGA i2c bus to talk to the ad7147 I have.
>>>>> I used INTA of a PCI ethernet slot in my PC. I enabled the PCI device
>>>>> without the driver module loaded.  I then give the interrupt number
>>>>> to ad714x through the struct i2c_board_info. I actually tried the
>>>>> same setup on two PCs, one intel graphics, the other nvidia to
>>>>> eliminate the i2c master as a possible cause of my problem.
>>>>> 
>>>>> The device is successfully loaded and I can see the interrupts going.
>>>>> The eventN device created under /dev/input never spit out anything
>>>>> and so I added printks in the threaded ISR handler to see what is
>>>>> going on.
>>>>> 
>>>>> I only have a wheel with 8 stages. In ad714x_wheel_state_machine() I
>>>>> see that upon the first interrupt, the state goes from IDLE to
>>>>> JITTER. After this the JITTER case checks that c_state == mask (with
>>>>> mask being 0xff in our case). This condition is never met and the
>>>>> driver stays indefinitely in this state. After lifting my finger from
>>>>> the wheel, the chip settles down to scanning every so many
>>>>> milliseconds.
>>>>> 
>>>>> The STAGE_COMPLETE_INT_STATUS is always 0 when my finger is off, but
>>>>> varies a lot while my finger is on (while interrupt frequency is
>>>>> high). Looking at the value of STAGE_COMPLETE_INT_STATUS in binary
>>>>> reveals that the set bits are always in groups, e.g. 0x0007 or 0x001C
>>>>> or 0x0081(I imagine a roll-over of our start_stage-end_stage (0-7)).
>>>>> There seems to be a timing aspect here. I added a spin counter in the
>>>>> threaded ISR to delay reading the 3 registers and that seemed to make
>>>>> the c_state change a little.
>>>>> 
>>>>> I modified the code that reads the 3 registers right after the
>>>>> mutex_lock in ad714x_interrupt_thread so that the
>>>>> STAGE_COMPLETE_INT_STATUS is read before the other two (LOW and HIGH
>>>>> regs). The result was surprising. The COMPLETE reg did read 0xff now
>>>>> and the JITTER case went past the "if(c_state == mask)" but later
>>>>> crashed (divide by 0) in ad714x_wheel_cal_abs_pos() called from the
>>>>> JITTER case.
>>>>> 
>>>>> 
>>>>> Here's the initial configuration I give the driver:
>>>>> 
>>>>> static struct ad714x_wheel_plat wheel_platform_data = {
>>>>>     .start_stage = 0, // int start_stage;
>>>>>     .end_stage = 7, // int end_stage;
>>>>>     .max_coord = 128,  // int max_coord;
>>>>> };
>>>>> 
>>>>> static struct ad714x_platform_data wheel_dev_platform_data = {
>>>>>     .slider_num = 0,
>>>>>     .wheel_num = 1,
>>>>>     .touchpad_num = 0,
>>>>>     .button_num = 0,
>>>>>     .slider = 0,
>>>>>     .wheel = &wheel_platform_data, // struct ad714x_wheel_plat *wheel;
>>>>>     .touchpad = 0, // struct ad714x_touchpad_plat *touchpad;
>>>>>     .button = 0, // struct ad714x_button_plat *button;
>>>>>     .stage_cfg_reg =  { /* unsigned short stage_cfg_reg[STAGE_NUM][STAGE_CFGREG_NUM] */
>>>>>             {0xfffe, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>>>             {0xfffb, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>>>             {0xffef, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>>>             {0xffbf, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>>>             {0xfeff, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>>>             {0xfbff, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>>>             {0xefff, 0x3fff, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>>>             {0xffff, 0x3ffe, 0, 0x2626, 0x3e8, 0x3e8, 0x1388, 0x1388 },
>>>>> 
>>>>>             {0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
>>>>>             {0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
>>>>>             {0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
>>>>>             {0xffff, 0x3fff, 0, 0x0606, 0x01f4, 0x01f4, 0x0320, 0x0320},
>>>>>     },
>>>>>     .sys_cfg_reg = {0x027e, 0x00ff, 0x3233, 0x0819, 0x0832, 0x0000, 0x00ff, 0}, /* unsigned short sys_cfg_reg[SYS_CFGREG_NUM] */
>>>>>     //.sys_cfg_reg = {0x2b2, 0xfff, 0x3233, 0x819, 0x832, 0xcff, 0xcff, 0x0}, /* unsigned short sys_cfg_reg[SYS_CFGREG_NUM] */
>>>>> };
>>>>> 
>>>>> 
>>>>> 
>>>> 
>>>> 
>>>>> I also had to change the request_threaded_irq flags to specify
>>>>> IRQF_ONESHOT so the kernel keeps the interrupt masked while we are
>>>>> running ad714x_interrupt_thread(). Otherwise we were getting storms
>>>>> of interrupts each time only one was requested. I am wondering if
>>>>> this should be pulled back to the mainline kernel?
>>>>> 
>>>>> Thanks for pointers and clues!
>>>>> 
>>>>> 
>>>>> 
>>>> 
>>>> 
>>> here's the printk I added to ad714x_wheel_state_machine()
>>> 
>>>        mask = ((1 << (hw->end_stage + 1)) - 1) - ((1 << hw->start_stage) - 1);
>>> 
>>>        h_state = ad714x->h_state & mask;
>>>        c_state = ad714x->c_state & mask;
>>>        dev_dbg(ad714x->dev, "interrupt state:%d mask:0x%x l:0x%x h:0x%x c:0x%x\n",
>>>                sw->state,
>>>                (u32)mask,
>>>                (u32)ad714x->l_state,
>>>                (u32)ad714x->h_state,
>>>                (u32)ad714x->c_state);
>>> 
>>> Here what it looks like upon loading a module which does the i2c_new_device of the AD7147:
>>> 
>>> <7>[58302.186886] my-pci-stub 0000:01:04.0: claimed by my platform module PCI stub
>>> <6>[58302.186903] my-pci-stub 0000:01:04.0: PCI INT A -> GSI 17 (level, low) -> IRQ 17
>>> <6>[58302.189815] ad714x_captouch 9-002c: found AD7147(A) captouch, rev:1
>>> <6>[58302.427237] input: Unspecified device as /devices/virtual/input/input8
>>> 
>>> [ pause here while my hand goes from my mouse and keyboard to the wheel on the AD7147 ]
>>> 
>>> <7>[58311.646183] ad714x_captouch 9-002c: interrupt state:0 mask:0xff l:0x0 h:0x18 c:0x1c
>>> <7>[58311.646192] ad714x_captouch 9-002c: case IDLE in if
>>> <7>[58311.655436] ad714x_captouch 9-002c: wheel 0 touched
>>> <7>[58311.663087] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x7
>>> <7>[58311.674562] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
>>> <7>[58311.686803] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
>>> <7>[58311.699147] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
>>> <7>[58311.711430] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
>>> <7>[58311.723585] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x18 c:0x81
>>> <7>[58311.736017] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
>>> <7>[58311.748298] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
>>> <7>[58311.760581] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
>>> <7>[58311.772800] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
>>> <7>[58311.785176] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
>>> <7>[58311.797473] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x81
>>> <7>[58311.809651] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x10 c:0x81
>>> 
>>> [ here I lift my finger ]
>>> 
>>> <7>[58311.822059] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>>> <7>[58311.834345] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>>> <7>[58311.846582] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>>> <7>[58311.858800] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>>> <7>[58311.871212] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>>> <7>[58311.883517] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>>> <7>[58311.895802] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>>> <7>[58311.908099] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>>> <7>[58311.920381] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>>> <7>[58311.932585] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>>> 
>>> [...]
>>> 
>>> <7>[58313.432218] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>>> <7>[58314.157343] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x81
>>> 
>>> [... after 2 seconds or so, the rhythm slows down to 2 interrupts per second or so ]
>>> 
>>> <7>[58314.169629] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x80
>>> 
>>> <7>[58314.976518] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x80
>>> 
>>> <7>[58315.783342] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x0 c:0x80
>>> 
>>> 
>>> There is a clue in what I did next. I added a wait time in the isr thread function like so:
>>> 
>>> static irqreturn_t ad714x_interrupt_thread(int irq, void *data)
>>> {
>>>        struct ad714x_chip *ad714x = data;
>>>        volatile int i;
>>> 
>>>        mutex_lock(&ad714x->mutex);
>>> 
>>>        i=0xffffff;
>>>        while(i)
>>>                --i;
>>> 
>>>        ad714x->read(ad714x->dev, STG_LOW_INT_STA_REG, &ad714x->l_state);
>>>        ad714x->read(ad714x->dev, STG_HIGH_INT_STA_REG, &ad714x->h_state);
>>>        ad714x->read(ad714x->dev, STG_COM_INT_STA_REG, &ad714x->c_state);
>>> [ ... ]
>>> 
>>> this changes the above trace to these values:
>>> 
>>> while touching the wheel:
>>> <7>[63085.414268] ad714x_captouch 9-002c: interrupt state:0 mask:0xff l:0x0 h:0x70 c:0x60
>>> <7>[63085.414277] ad714x_captouch 9-002c: case IDLE in if
>>> <7>[63085.423519] ad714x_captouch 9-002c: wheel 0 touched
>>> <7>[63085.578931] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x70 c:0x18
>>> <7>[63085.736079] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x70 c:0x3
>>> <7>[63085.832304] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x70 c:0x60
>>> <7>[63086.938334] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x30 c:0x60
>>> <7>[63087.030835] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
>>> <7>[63087.122909] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
>>> <7>[63087.215014] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
>>> <7>[63087.307071] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
>>> <7>[63087.399367] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x60
>>> <7>[63087.739386] ad714x_captouch 9-002c: interrupt state:1 mask:0xff l:0x0 h:0x38 c:0x83
>>> 
>>> Again, notice the state going from 0 (IDLE) to 1(JITTER), and never entering the "if" in the JITTER case (needs mask == c_state).  The HIGH register varies a lot while I move my finger around, but the COMPLETE looks like its always going and being cleared. I mentioned before that reading the COMPLETE status reg before the LOW and HIGH produces completely different results.
>>> 
>>> I am suspecting the register configuration we use is off somehow, I will review them thoroughly. Aside from this we are running out of leads here, anyone has input on this?
>>> 
>>> Thanks in advance!
>>> JFD
>>> 
>>> 
>> Hi Jean-Francois,
>> 
>> Barry Song, the driver author left ADI quite some time ago
>> I don't have hardware to test things at the moment.
>> 
>> The EVAL-AD7147 only has sliders and buttons, so I don't know how useful
>> it will be here.
>> Need to check if I can find one having a  wheel.
>> 
>> 
> Hi Jean-Francois,
> 
> The ADZS-BFLLCD-EZEXT features a AD7147-1 + wheel.
> I'll take a look and get back to you later this week.
> 
Hi again Michael,

I forgot to mention something that may be useful. We wired the 8 stages exactly like the wheel example of figure 26, page 15, of the AD7147 datasheet (Rev B, 07/2009). The register configuration I pasted above in the init structures should match this.

Thanks again for your support.

> -- 
> Greetings,
> Michael
> 
> --
> Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
> Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
> Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
> Margaret Seif
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/