Some serious stuff going on the lists.. The Linux Kernel really is scrutinized awesomely.
http://thread.gmane.org/gmane.linux.kernel.iio/8861
Highlights:
- Quirks in error handling paths for freeing the device due to devm_iio_alloc
- error handling technique for trigger alloc in the probe. should be in trigger alloc function.
- missing lines for removing trigger in remove function.
- TSC/ADC IRQ sharing is confusing and messed up. Explanations.
Missing entry in Kconfig.
Refix and resend. And then the wait begins again.
Although, the main guy reviewing Sebastian is leaving for 3 weeks.. Hope Jonathan gives the driver time and accepts it for the next cycle.
Wednesday, 28 August 2013
Sunday, 25 August 2013
Round 6. Continuous mode the way iio wants
IIO didn't like my usage of their trigger oscilloscope style. Messed their ABI.
Soo. there is an internal trigger in the driver that occurs for every threshold.
After the changes and some cleanup.
I'm back online with round 6. hope iio likes the driver this time.
http://thread.gmane.org/gmane.linux.kernel.iio/8861
Major changes in the way driver works.
Previously i would use a sysfs trigger to trigger the start of sampling after enabling the buffer.
IIO didn't like that. And suggested that I create a driver trigger that would automatically trigger upon the FIFO threshold being reached every time.
So. I did that. It took a while to restructure the driver.
I do note one thing. Previously I would get the illusion that the adc driver is working alongside the touchscreen events.
I think I was wrong. I looked at the code, thought, and checked the irq status again and again. Only one side is active at one instant.
Which makes sense sorta. Cause the way I understand the hardware. The SW steps can happen (adc) or the HW steps can happen (TSC). So one of em has to give in.
Although, at the moment, the ADC is at max sampling frequency. Perhaps configuring the adc with a slower sampling rate might allow the TSC steps to be configured. I'm not going deep into the sampling frequency yet. it has to do with the step delay register. the way i see it. i think all the channels can have individual sampling frequencies as well.
IIO was designed with loads of flexibility in mind. but i think it was made with external adc chips in mind. not internal powerful adcs like this am335x one.
Soo. there is an internal trigger in the driver that occurs for every threshold.
After the changes and some cleanup.
I'm back online with round 6. hope iio likes the driver this time.
http://thread.gmane.org/gmane.linux.kernel.iio/8861
Major changes in the way driver works.
Previously i would use a sysfs trigger to trigger the start of sampling after enabling the buffer.
IIO didn't like that. And suggested that I create a driver trigger that would automatically trigger upon the FIFO threshold being reached every time.
So. I did that. It took a while to restructure the driver.
I do note one thing. Previously I would get the illusion that the adc driver is working alongside the touchscreen events.
I think I was wrong. I looked at the code, thought, and checked the irq status again and again. Only one side is active at one instant.
Which makes sense sorta. Cause the way I understand the hardware. The SW steps can happen (adc) or the HW steps can happen (TSC). So one of em has to give in.
Although, at the moment, the ADC is at max sampling frequency. Perhaps configuring the adc with a slower sampling rate might allow the TSC steps to be configured. I'm not going deep into the sampling frequency yet. it has to do with the step delay register. the way i see it. i think all the channels can have individual sampling frequencies as well.
IIO was designed with loads of flexibility in mind. but i think it was made with external adc chips in mind. not internal powerful adcs like this am335x one.
Sunday, 18 August 2013
IIO list feedback Update + Short Hiatus
Feedback from the IIO lists has been great.
https://lkml.org/lkml/2013/8/13/553
and
https://lkml.org/lkml/2013/8/13/666
I use the IIO ABI in a slightly non-standard way to get things done.
The trigger in IIO is for triggering a single sample of data. I guess its for ADCs using the DRDY style signals etc. Thats not useful for me to to have continuous sampling.
I used trigger to start the trigger oscilloscope style. To trigger a large buffer of samples.
Jonathan actually suggested that such a feature could be great in the IIO core and was open to a proposal for it. Even gave a couple of paragraphs for the interface.
Given the nature of the am335x sampling and fifos, the IIO trigger model doesn't fit well to trigger every sample in continuous sampling mode.
Jonathan suggested to use internal triggers that occurred every time the FIFO thres handler is called. Just so that the driver fits IIO ABI a bit more/closer.
I don't see any problems with implementing the driver the way IIO wants.
But I'm taking a short break to finalize my MSc. thesis report this week.
GregKH is great and cool with it.
https://lkml.org/lkml/2013/8/13/553
and
https://lkml.org/lkml/2013/8/13/666
I use the IIO ABI in a slightly non-standard way to get things done.
The trigger in IIO is for triggering a single sample of data. I guess its for ADCs using the DRDY style signals etc. Thats not useful for me to to have continuous sampling.
I used trigger to start the trigger oscilloscope style. To trigger a large buffer of samples.
Jonathan actually suggested that such a feature could be great in the IIO core and was open to a proposal for it. Even gave a couple of paragraphs for the interface.
Given the nature of the am335x sampling and fifos, the IIO trigger model doesn't fit well to trigger every sample in continuous sampling mode.
Jonathan suggested to use internal triggers that occurred every time the FIFO thres handler is called. Just so that the driver fits IIO ABI a bit more/closer.
I don't see any problems with implementing the driver the way IIO wants.
But I'm taking a short break to finalize my MSc. thesis report this week.
GregKH is great and cool with it.
Tuesday, 13 August 2013
Round 4
More patchesssss
Small fix to MFD here
http://thread.gmane.org/gmane.linux.kernel/1543722
TSC fixes and Continuous mode here.
http://thread.gmane.org/gmane.linux.kernel.input/31427
I left out a fix for rc1 of the new one. Because I sent around various patches sporadically. One fix in iio. One fix in mfd.
I cant send the last fix because if plays around with everywhere.
But no issues.
Small fix to MFD here
http://thread.gmane.org/gmane.linux.kernel/1543722
TSC fixes and Continuous mode here.
http://thread.gmane.org/gmane.linux.kernel.input/31427
I left out a fix for rc1 of the new one. Because I sent around various patches sporadically. One fix in iio. One fix in mfd.
I cant send the last fix because if plays around with everywhere.
But no issues.
Monday, 12 August 2013
And fixes
lots of tiny fixes here n there
It turned into a chain of fixes. I'd fix one and then find another in the process.
Fixes would become bugs and then fixes for that lol ;)
Lets see.
HW preempt enabled.
Before, ADC was given priority. Not TSC. TSC event couldn't mess with ADC sampling. By enabling HW pre-emption, TSC event is given priority. This was done in an attempt to fix the TSC not registering a pen UP event even if the finger was lifted. (during ADC continuous mode sampling in the background)
There is a small delay in the TSC IRQ which lets the TSC sequencer settle before checking for pen UP events. Had to increase that delay slightly to allow a pen UP to be registered.
Locks in mfd.
One fix is already in the staging. And I forgot that the function comes in pairs. I sent a fix for the set function. But forgot the clr function lol. While fixing the continuous mode driver, i found the need for the clr function and realized that the fix is mising here..
TSC steps enable issue
TSC steps would disable if I mess with the ADC. Because the ADC would also play with the step enable register. the common mfd core locks for set/clr help. But not completely.
In some cases (during the reg_SE read, update, write, some bit would go bad), the TSC steps wouldn't be enabled properly. And that would caused the wrong configuration to hang. So had to add a mask to the mfd core.
TSCADC disable in ADC code.
The continuous mode code was littered in dozens of places with areas that would enable/disable the entire module. I think this was the real culprit causing the entire thing to hang. Put checks around to not disable the entire module if the TSC is alive.
Reg_SE written with 0x00 on ADC side.
Previously, channels for continuous sampling would enable and disable in iio_trigger_preenable. That would mess step enb register. There was even a write 0x00 for the SE register in there.
Now they update the SE register via the common mfd core.
I guess I was looking only at the ADC before and I didn't realize.
During the bug fix phase, I'm looking at both TSC and ADC and making sure they work.
Now I have a working continuous mode with TSC events that can occur simultaneously and TSC doesn't hang.
But I have a different issue lol. Asking greg about it.
Fix in mfd tree.
Fix in iio tree.
My patch series depends on both trees. lol.
and splitting them can be tricky.
lets see. greg said something about split. most of em can be split except for one fix which can go later.
lets see.
have to sort out the code and send it to the subsystems.
It turned into a chain of fixes. I'd fix one and then find another in the process.
Fixes would become bugs and then fixes for that lol ;)
Lets see.
HW preempt enabled.
Before, ADC was given priority. Not TSC. TSC event couldn't mess with ADC sampling. By enabling HW pre-emption, TSC event is given priority. This was done in an attempt to fix the TSC not registering a pen UP event even if the finger was lifted. (during ADC continuous mode sampling in the background)
There is a small delay in the TSC IRQ which lets the TSC sequencer settle before checking for pen UP events. Had to increase that delay slightly to allow a pen UP to be registered.
Locks in mfd.
One fix is already in the staging. And I forgot that the function comes in pairs. I sent a fix for the set function. But forgot the clr function lol. While fixing the continuous mode driver, i found the need for the clr function and realized that the fix is mising here..
TSC steps enable issue
TSC steps would disable if I mess with the ADC. Because the ADC would also play with the step enable register. the common mfd core locks for set/clr help. But not completely.
In some cases (during the reg_SE read, update, write, some bit would go bad), the TSC steps wouldn't be enabled properly. And that would caused the wrong configuration to hang. So had to add a mask to the mfd core.
TSCADC disable in ADC code.
The continuous mode code was littered in dozens of places with areas that would enable/disable the entire module. I think this was the real culprit causing the entire thing to hang. Put checks around to not disable the entire module if the TSC is alive.
Reg_SE written with 0x00 on ADC side.
Previously, channels for continuous sampling would enable and disable in iio_trigger_preenable. That would mess step enb register. There was even a write 0x00 for the SE register in there.
Now they update the SE register via the common mfd core.
I guess I was looking only at the ADC before and I didn't realize.
During the bug fix phase, I'm looking at both TSC and ADC and making sure they work.
Now I have a working continuous mode with TSC events that can occur simultaneously and TSC doesn't hang.
But I have a different issue lol. Asking greg about it.
Fix in mfd tree.
Fix in iio tree.
My patch series depends on both trees. lol.
and splitting them can be tricky.
lets see. greg said something about split. most of em can be split except for one fix which can go later.
lets see.
have to sort out the code and send it to the subsystems.
Monday, 5 August 2013
BUGGGGGGGGS
I have noticed three(lol. Update. 6 now) bugs in the TSC ADC driver.
Subtle ones. Quite possible won't occur in use cases at all.
Bug 1. (resolved)
When I am waiting for a trigger, the touchscreen stops working..
This one is cause the following line on the ADC side disables the TSC steps.
tiadc_writel(adc_dev, REG_SE, 0x00);
the tsc is ok after trigger.
But if i do an event during waiting for trigger.
tsc works. but adc doesnt. meaning tsc is disabling all configuration i did for continuous mode!
Managed to move around the bit masks to the poll handler function.
This one is ok.
Heard back from iio list with some comments.
I'll resend the patches with this bug fix squashed in.
Bug 2. (sudo resolved)
If I do a
while(true)
do
cat in_voltage*
done
to continuously read samples using one-shot adc mode.
The touchscreen registers clicks randomly!!
Although this is a wild test case, it shouldn't happen
Update:
This is weird. Shouldn't waste time on this bug.
while (true)
do
cat in_voltage4_raw;
cat in_voltage5_raw;
cat in_voltage6_raw;
cat in_voltage7_raw;
done
wouldn't make the tsc register events or go haywire.
but
while (true)
do
cat in*;
done
would register events on TSC. Strange. I quadruple checked all the ADC reg configs even! everything is ok. The TSC shouldn't receive interrupts.
What happens is that HW steps which aren't supposed to run unless a touch is registered actually do seem to run 'once'.
This fills FIFO0 which threshold interrupts.
Thus, a random click and PEN UP somewhere. Uncool.
But anyways. Seems random. Especially since the expanded while loop didn't register the same phenomena.
Useless. No more time wasting on this bug. managed to catch another unknown one during this process lol. bug 4!.
I have to say. The more bugs I encounter. The more I understand the driver and TSADC module. The more I realize I know nothing!
Bug 3. (left for later)
This one is even more subtle.
The old 3.8 touchscreen driver was very smooth and registered clicks properly.
The one after sebastians patches seems as if the mouse is going slightly haywire sometimes.
Sometimes click takes the mouse somewhere random. This is a really subtle one.
I'll spend time on this if I get the chance. My focus is more ADC. But obviously. Community is community. And I'm in a good position to fix this.
Basically have to compare the previous driver and the one after sebastians patches in mainline. Somewhere some calculation(i guess) is different. Although sebastians work is based on the same arago. But something feels off..
Bug 4. (Resolved. Patch upstreamed :) )
This time its the MFD driver. the MFD driver has a small function to update the Step enable register.
Problem is. It used a reg_cache. Which isn't updated anywhere. So if I use a TSC and ADC. The reg_cache gets loaded with FFFF and all steps are enabled no matter what. ADC steps disable themselves after one sample. Which makes it ok.
But sorta. The FIFO1 still gets filled up with samples.
The current read_raw code was ok in the sense that it flushed the entire fifo.
But too many TSC touches and the ADC FIFO will fill up with useless samples. That'll make a mess most probably.
So yea. good thing I caught this bug. I'll send it as a fix to the MFD list..
(OOO. Just realized. Steps updation might be why continuous Bug 6 isn't working. Damn. I'll have to check again..) (Update again lol. Dimitry from input pointed out as well. IRQs might be hanging..)
Fix was simplish.
added a read in there which checks current steps.
Bug 5 (Resolved!)
This is a fine example of a bug you add for no reason and then have to fix!
I was aiming to optimize the read_raw function. But indirectly added some redundant samples that wouldn't flush. This would make a mess.
If I read channel 5, I'd return early thinking I was efficient. Didn't realize Channel 6 and 7 would be in FIFO and those needed flushing.
So if i read channel 7 after 10 sec. I'll get the old value. And more old data would be in the fifo. "Unoptimized the bug again". I'll squash this fix in the next series I send to IIO.
Although I personally don't like the idea of sampling all channels and then flushing them out. the continuous mode code can be used to enable one channel only. Its a simple mask.
But alas. I'll see if i want to clean up things. it works as it is anyways..
Bug 6 (WIP)
I dont know if this is a viable test case or not.
A simple delay in a userspace application might make this go away. But yes. its bad.
While waiting for trigger.
If i'm pressing down on TSC,
and trigger the adc.
The entire TSCADC module hangs. Nothing works.
I think the IRQs hang somehow. Cause cat /proc/interrupts stops registering anything.
Maybe both handlers return none or something. And the IRQs of the TSCADC module don't occur again.
Tried moving around stuff to end. But didn't work.
Added ovverun handling but nope.
Russ tried pointing it out on a patch. But Greg said level interrupts are ok.
(Update, Dimitry from the input list pointed out the same for the patch I sent. What will happen if both IRQs go off.. Thats why system hangs maybe..)
I guess this might be something really quirky.
Its an awful usecase anyways..
Maybe both overrun and stop the TSCADC module. and the IRQ handlers don't go into ovverun. This is quirky to debug.
If I'm making a GUI running on TSC acquiring samples. I'll add a delay between the button press and the ADC sampling to avoid this race condition.
But yea. something like a GUI oscilloscope on a BBB might seem far of. I don't think the module can handle such complexity for real time sampling and display.
But sorta periodic trigger buffer sampling display might work. Anyways.
Oscilloscope on BBB might be a fun idea but I need to work on the GSoC project for now!
Subtle ones. Quite possible won't occur in use cases at all.
Bug 1. (resolved)
When I am waiting for a trigger, the touchscreen stops working..
This one is cause the following line on the ADC side disables the TSC steps.
tiadc_writel(adc_dev, REG_SE, 0x00);
the tsc is ok after trigger.
But if i do an event during waiting for trigger.
tsc works. but adc doesnt. meaning tsc is disabling all configuration i did for continuous mode!
Managed to move around the bit masks to the poll handler function.
This one is ok.
Heard back from iio list with some comments.
I'll resend the patches with this bug fix squashed in.
Bug 2. (sudo resolved)
If I do a
while(true)
do
cat in_voltage*
done
to continuously read samples using one-shot adc mode.
The touchscreen registers clicks randomly!!
Although this is a wild test case, it shouldn't happen
Update:
This is weird. Shouldn't waste time on this bug.
while (true)
do
cat in_voltage4_raw;
cat in_voltage5_raw;
cat in_voltage6_raw;
cat in_voltage7_raw;
done
wouldn't make the tsc register events or go haywire.
but
while (true)
do
cat in*;
done
would register events on TSC. Strange. I quadruple checked all the ADC reg configs even! everything is ok. The TSC shouldn't receive interrupts.
What happens is that HW steps which aren't supposed to run unless a touch is registered actually do seem to run 'once'.
This fills FIFO0 which threshold interrupts.
Thus, a random click and PEN UP somewhere. Uncool.
But anyways. Seems random. Especially since the expanded while loop didn't register the same phenomena.
Useless. No more time wasting on this bug. managed to catch another unknown one during this process lol. bug 4!.
I have to say. The more bugs I encounter. The more I understand the driver and TSADC module. The more I realize I know nothing!
Bug 3. (left for later)
This one is even more subtle.
The old 3.8 touchscreen driver was very smooth and registered clicks properly.
The one after sebastians patches seems as if the mouse is going slightly haywire sometimes.
Sometimes click takes the mouse somewhere random. This is a really subtle one.
I'll spend time on this if I get the chance. My focus is more ADC. But obviously. Community is community. And I'm in a good position to fix this.
Basically have to compare the previous driver and the one after sebastians patches in mainline. Somewhere some calculation(i guess) is different. Although sebastians work is based on the same arago. But something feels off..
Bug 4. (Resolved. Patch upstreamed :) )
This time its the MFD driver. the MFD driver has a small function to update the Step enable register.
Problem is. It used a reg_cache. Which isn't updated anywhere. So if I use a TSC and ADC. The reg_cache gets loaded with FFFF and all steps are enabled no matter what. ADC steps disable themselves after one sample. Which makes it ok.
But sorta. The FIFO1 still gets filled up with samples.
The current read_raw code was ok in the sense that it flushed the entire fifo.
But too many TSC touches and the ADC FIFO will fill up with useless samples. That'll make a mess most probably.
So yea. good thing I caught this bug. I'll send it as a fix to the MFD list..
(OOO. Just realized. Steps updation might be why continuous Bug 6 isn't working. Damn. I'll have to check again..) (Update again lol. Dimitry from input pointed out as well. IRQs might be hanging..)
Fix was simplish.
added a read in there which checks current steps.
Bug 5 (Resolved!)
This is a fine example of a bug you add for no reason and then have to fix!
I was aiming to optimize the read_raw function. But indirectly added some redundant samples that wouldn't flush. This would make a mess.
If I read channel 5, I'd return early thinking I was efficient. Didn't realize Channel 6 and 7 would be in FIFO and those needed flushing.
So if i read channel 7 after 10 sec. I'll get the old value. And more old data would be in the fifo. "Unoptimized the bug again". I'll squash this fix in the next series I send to IIO.
Although I personally don't like the idea of sampling all channels and then flushing them out. the continuous mode code can be used to enable one channel only. Its a simple mask.
But alas. I'll see if i want to clean up things. it works as it is anyways..
Bug 6 (WIP)
I dont know if this is a viable test case or not.
A simple delay in a userspace application might make this go away. But yes. its bad.
While waiting for trigger.
If i'm pressing down on TSC,
and trigger the adc.
The entire TSCADC module hangs. Nothing works.
I think the IRQs hang somehow. Cause cat /proc/interrupts stops registering anything.
Maybe both handlers return none or something. And the IRQs of the TSCADC module don't occur again.
Tried moving around stuff to end. But didn't work.
Added ovverun handling but nope.
Russ tried pointing it out on a patch. But Greg said level interrupts are ok.
(Update, Dimitry from the input list pointed out the same for the patch I sent. What will happen if both IRQs go off.. Thats why system hangs maybe..)
I guess this might be something really quirky.
Its an awful usecase anyways..
Maybe both overrun and stop the TSCADC module. and the IRQ handlers don't go into ovverun. This is quirky to debug.
If I'm making a GUI running on TSC acquiring samples. I'll add a delay between the button press and the ADC sampling to avoid this race condition.
But yea. something like a GUI oscilloscope on a BBB might seem far of. I don't think the module can handle such complexity for real time sampling and display.
But sorta periodic trigger buffer sampling display might work. Anyways.
Oscilloscope on BBB might be a fun idea but I need to work on the GSoC project for now!
Thursday, 1 August 2013
Triggering the IIO ADC driver using a GPIO survey
In continuous mode, the ADC driver can be triggered using a GPIO.
(For details on continuous mode: http://beagleboard-gsoc13.blogspot.co.uk/2013/07/sampling-analogue-signals-using-adc-on.html)
GPIO numbering on the BBB can be a bit tricky due to the multiple banks.
If you don't know what I'm talking about, see
http://www.armhf.com/index.php/using-beaglebone-black-gpios/
Now that we know which GPIO to use for triggering your ADC, we try to look at how to connect the gpio to the IIO driver.
As usual, the IIO subsystem is cryptic. Diving deep into IIO code is very tricky. I'm not that good yet I guess.
I've asked on the iio list. Could be something really simple i'm missing..
Have tried googling for a couple of hours.
(For details on continuous mode: http://beagleboard-gsoc13.blogspot.co.uk/2013/07/sampling-analogue-signals-using-adc-on.html)
GPIO numbering on the BBB can be a bit tricky due to the multiple banks.
If you don't know what I'm talking about, see
http://www.armhf.com/index.php/using-beaglebone-black-gpios/
Now that we know which GPIO to use for triggering your ADC, we try to look at how to connect the gpio to the IIO driver.
As usual, the IIO subsystem is cryptic. Diving deep into IIO code is very tricky. I'm not that good yet I guess.
I've asked on the iio list. Could be something really simple i'm missing..
Have tried googling for a couple of hours.
Subscribe to:
Posts (Atom)