Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
Hi Jon, on 31 Jul 10 at 17:53, Jon Smirl wrote: On Sat, Jul 31, 2010 at 2:51 PM, Andy Walls awa...@md.metrocast.net wrote: On Sat, 2010-07-31 at 14:14 -0400, Jon Smirl wrote: On Sat, Jul 31, 2010 at 1:47 PM, Christoph Bartelmus l...@bartelmus.de wrote: Hi Jon, on 31 Jul 10 at 12:25, Jon Smirl wrote: On Sat, Jul 31, 2010 at 11:12 AM, Andy Walls awa...@md.metrocast.net wrote: I think you won't be able to fix the problem conclusively either way.. A lot of how the chip's clocks should be programmed depends on how the GPIOs are used and what crystal is used. I suspect many designers will use some reference design layout from ENE, but it won't be good in every case. The wire-up of the ENE of various motherboards is likely something you'll have to live with as unknowns. This is a case where looser tolerances in the in kernel decoders could reduce this driver's complexity and/or get rid of arbitrary fudge factors in the driver. The tolerances are as loose as they can be. The NEC protocol uses pulses that are 4% longer than JVC. The decoders allow errors up to 2% (50% of 4%). The crystals used in electronics are accurate to 0.0001%+. But the standard IR receivers are far from being accurate enough to allow tolerance windows of only 2%. I'm surprised that this works for you. LIRC uses a standard tolerance of 30% / 100 us and even this is not enough sometimes. For the NEC protocol one signal consists of 22 individual pulses at 38kHz. If the receiver just misses one pulse, you already have an error of 1/22 4%. There are different types of errors. The decoders can take large variations in bit times. The problem is with cumulative errors. In this case the error had accumulated up to 450us in the lead pulse. That's just too big of an error Hi Jon, Hmmm. Leader marks are, by protocol design, there to give time for the receiver's AGC to settle. We should make it OK to miss somewhat large portions of leader marks. I'm not sure what the harm is in accepting too long of a leader mark, but I'm pretty sure a leader mark that is too long will always be due to systematic error and not noise errors. However, if we know we have systematic errors caused by unknowns, we should be designing and implementing a decoding system that reasonably deals with those systematic errors. Again the part of the system almost completely out of our control is the remote controls, and we *have no control* over systematic errors introduced by remotes. We haven't encountered remotes with systematic errors. If remotes had large errors in them they wouldn't be able to reliably control their target devices. Find a remote that won't work with the protocol engines and a reasonably accurate receiver. Obviously we want to reduce or eliminate systematic errors by determining the unknowns and undoing their effects or by compensating for their overall effect. But in the case of the ENE receiver driver, you didn't seem to like the Maxim's software compensation for the systematic receiver errors. I would be happier if we could track down the source of the error instead of sticking a bandaid on at the end of the process. and caused the JVC code to be misclassified as NEC. I still have not heard why we need protocol discrimination/classifcation in the kernel. Doing discrimination between two protocols with such close timings is whose requirement again? If we don't do protocol engines we have to revert back to raw recording and having everyone train the system with their remotes. The goal is to eliminate the training step. We would also have to have large files (LIRC configs) for building the keymaps and a new API to deal with them. With the engines the key presses are identified by short strings. Only 437 of 3486 config files on lirc.org use raw mode (probably what you mean with large files). Many of them are recorded with an very old irrecord version. Current versions of irrecord wouldn't create a raw mode config file for these remotes. A use case: install mythtv, add an IR receiver. Myth UI says to set your universal remote to a Motorola DVR profile. Remote works - no training step needed. + Myth UI reconfigures lircd with an existing Motorola DVR config file. Where's the difference? LIRC has protocol engines too. irrecord first tries to fit the remote into a protocol engine. With the sublte difference to your approach that LIRC does not make any assumptions on any timings in contrast to hardcoded values in the kernel. If it can't it reverts to raw mode. Let's analyze those cases where lirc ends up in raw mode and see if we can figure out what's going wrong. For example I know of two things that will trip up irrecord that are fixed in the kernel system 1) the ene driver. we know now it had a 4% error in the reported periods Wrong. 2) Sony remotes - they mix protocols on a single remote. This is a long known issue. I didn't care to fix it
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
Hi Jon, on 31 Jul 10 at 14:14, Jon Smirl wrote: On Sat, Jul 31, 2010 at 1:47 PM, Christoph Bartelmus l...@bartelmus.de wrote: Hi Jon, on 31 Jul 10 at 12:25, Jon Smirl wrote: On Sat, Jul 31, 2010 at 11:12 AM, Andy Walls awa...@md.metrocast.net wrote: I think you won't be able to fix the problem conclusively either way. A lot of how the chip's clocks should be programmed depends on how the GPIOs are used and what crystal is used. I suspect many designers will use some reference design layout from ENE, but it won't be good in every case. The wire-up of the ENE of various motherboards is likely something you'll have to live with as unknowns. This is a case where looser tolerances in the in kernel decoders could reduce this driver's complexity and/or get rid of arbitrary fudge factors in the driver. The tolerances are as loose as they can be. The NEC protocol uses pulses that are 4% longer than JVC. The decoders allow errors up to 2% (50% of 4%). The crystals used in electronics are accurate to 0.0001%+. But the standard IR receivers are far from being accurate enough to allow tolerance windows of only 2%. I'm surprised that this works for you. LIRC uses a standard tolerance of 30% / 100 us and even this is not enough sometimes. For the NEC protocol one signal consists of 22 individual pulses at 38kHz.. If the receiver just misses one pulse, you already have an error of 1/22 4%. There are different types of errors. The decoders can take large variations in bit times. The problem is with cumulative errors. In this case the error had accumulated up to 450us in the lead pulse. That's just too big of an error and caused the JVC code to be misclassified as NEC. I think he said lirc was misclassifying it too. So we both did the same thing. No way. JVC is a 16 bit code. NEC uses 32 bits. How can you ever confuse JVC with NEC signals? LIRC will work if there is a 4% or 40% or 400% error. Because irrecord generates the config file using your receiver it will compensate for any timing error. It will work with pulses cut down to 50 us like IrDA hardware does and it will work when half of the bits are swollowed like the IgorPlug USB receiver does. But of course the driver should try to generate timings as accurate as possible. Christoph -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
On Sun, Aug 1, 2010 at 5:50 AM, Christoph Bartelmus l...@bartelmus.de wrote: Hi Jon, on 31 Jul 10 at 14:14, Jon Smirl wrote: On Sat, Jul 31, 2010 at 1:47 PM, Christoph Bartelmus l...@bartelmus.de wrote: Hi Jon, on 31 Jul 10 at 12:25, Jon Smirl wrote: On Sat, Jul 31, 2010 at 11:12 AM, Andy Walls awa...@md.metrocast.net wrote: I think you won't be able to fix the problem conclusively either way. A lot of how the chip's clocks should be programmed depends on how the GPIOs are used and what crystal is used. I suspect many designers will use some reference design layout from ENE, but it won't be good in every case. The wire-up of the ENE of various motherboards is likely something you'll have to live with as unknowns. This is a case where looser tolerances in the in kernel decoders could reduce this driver's complexity and/or get rid of arbitrary fudge factors in the driver. The tolerances are as loose as they can be. The NEC protocol uses pulses that are 4% longer than JVC. The decoders allow errors up to 2% (50% of 4%). The crystals used in electronics are accurate to 0.0001%+. But the standard IR receivers are far from being accurate enough to allow tolerance windows of only 2%. I'm surprised that this works for you. LIRC uses a standard tolerance of 30% / 100 us and even this is not enough sometimes. For the NEC protocol one signal consists of 22 individual pulses at 38kHz.. If the receiver just misses one pulse, you already have an error of 1/22 4%. There are different types of errors. The decoders can take large variations in bit times. The problem is with cumulative errors. In this case the error had accumulated up to 450us in the lead pulse. That's just too big of an error and caused the JVC code to be misclassified as NEC. I think he said lirc was misclassifying it too. So we both did the same thing. No way. JVC is a 16 bit code. NEC uses 32 bits. How can you ever confuse JVC with NEC signals? LIRC will work if there is a 4% or 40% or 400% error. Because irrecord generates the config file using your receiver it will compensate for any At the end of the process we can build a record and match raw mode if we have to. timing error. It will work with pulses cut down to 50 us like IrDA hardware does and it will work when half of the bits are swallowed like the IgorPlug USB receiver does. The code for fixing IrDA and IgorPLug should live inside their low level device drivers. The characteristics of the errors produced by this hardware are known so a fix can be written to compensate. The IgorPlug people might find it easier to fix their firmware. You don't have to get the timing exactly right for the protocol engines to work, you just need to get the big systematic errors out. Before doing raw the low level IR drivers should be fixed to provide the most accurate data possible. I don't think it is good design for a low level driver to be injecting systematic errors and then have the next layer of code remove them. The source of the systematic errors should be characterized and fixed in the low level driver. That also allows the fixed to be constrained to fixing data from only a single receiver type. I have been burnt twice in the past from fixing errors in a low level driver higher in the stack. I have learned the hard way that it is a bad thing to do. As the fixes accumulate in the higher layers you will reach a point where they conflict and no solution is possible. Bugs in the low level drivers need to be fixed in that driver. Show me a case that can't be fixed in the low level driver and we can explore the options. But of course the driver should try to generate timings as accurate as possible. Christoph -- Jon Smirl jonsm...@gmail.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
On Sun, Aug 1, 2010 at 10:00 AM, Jon Smirl jonsm...@gmail.com wrote: On Sun, Aug 1, 2010 at 5:50 AM, Christoph Bartelmus l...@bartelmus.de wrote: Hi Jon, on 31 Jul 10 at 14:14, Jon Smirl wrote: On Sat, Jul 31, 2010 at 1:47 PM, Christoph Bartelmus l...@bartelmus.de wrote: Hi Jon, on 31 Jul 10 at 12:25, Jon Smirl wrote: On Sat, Jul 31, 2010 at 11:12 AM, Andy Walls awa...@md.metrocast.net wrote: I think you won't be able to fix the problem conclusively either way. A lot of how the chip's clocks should be programmed depends on how the GPIOs are used and what crystal is used. I suspect many designers will use some reference design layout from ENE, but it won't be good in every case. The wire-up of the ENE of various motherboards is likely something you'll have to live with as unknowns. This is a case where looser tolerances in the in kernel decoders could reduce this driver's complexity and/or get rid of arbitrary fudge factors in the driver. The tolerances are as loose as they can be. The NEC protocol uses pulses that are 4% longer than JVC. The decoders allow errors up to 2% (50% of 4%). The crystals used in electronics are accurate to 0.0001%+. But the standard IR receivers are far from being accurate enough to allow tolerance windows of only 2%. I'm surprised that this works for you. LIRC uses a standard tolerance of 30% / 100 us and even this is not enough sometimes. For the NEC protocol one signal consists of 22 individual pulses at 38kHz.. If the receiver just misses one pulse, you already have an error of 1/22 4%. There are different types of errors. The decoders can take large variations in bit times. The problem is with cumulative errors. In this case the error had accumulated up to 450us in the lead pulse. That's just too big of an error and caused the JVC code to be misclassified as NEC. I think he said lirc was misclassifying it too. So we both did the same thing. No way. JVC is a 16 bit code. NEC uses 32 bits. How can you ever confuse JVC with NEC signals? LIRC will work if there is a 4% or 40% or 400% error. Because irrecord generates the config file using your receiver it will compensate for any At the end of the process we can build a record and match raw mode if we have to. timing error. It will work with pulses cut down to 50 us like IrDA hardware does and it will work when half of the bits are swallowed like the IgorPlug USB receiver does. The code for fixing IrDA and IgorPLug should live inside their low level device drivers. The characteristics of the errors produced by this hardware are known so a fix can be written to compensate. The IgorPlug people might find it easier to fix their firmware. You don't have to get the timing exactly right for the protocol engines to work, you just need to get the big systematic errors out. There is a real benefit to strict protocol engines. If the IgorPlus people had strict protocol engines to test against they would have discovered their firmware bugs during the initial development process. Before doing raw the low level IR drivers should be fixed to provide the most accurate data possible. I don't think it is good design for a low level driver to be injecting systematic errors and then have the next layer of code remove them. The source of the systematic errors should be characterized and fixed in the low level driver. That also allows the fixed to be constrained to fixing data from only a single receiver type. I have been burnt twice in the past from fixing errors in a low level driver higher in the stack. I have learned the hard way that it is a bad thing to do. As the fixes accumulate in the higher layers you will reach a point where they conflict and no solution is possible. Bugs in the low level drivers need to be fixed in that driver. Show me a case that can't be fixed in the low level driver and we can explore the options. But of course the driver should try to generate timings as accurate as possible. Christoph -- Jon Smirl jonsm...@gmail.com -- Jon Smirl jonsm...@gmail.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
Hi! Jon Smirl jonsm...@gmail.com wrote: On Sun, Aug 1, 2010 at 5:50 AM, Christoph Bartelmus l...@bartelmus.de wrote: Hi Jon, on 31 Jul 10 at 14:14, Jon Smirl wrote: On Sat, Jul 31, 2010 at 1:47 PM, Christoph Bartelmus l...@bartelmus.de wrote: Hi Jon, on 31 Jul 10 at 12:25, Jon Smirl wrote: On Sat, Jul 31, 2010 at 11:12 AM, Andy Walls awa...@md.metrocast.net wrote: I think you won't be able to fix the problem conclusively either way. A lot of how the chip's clocks should be programmed depends on how the GPIOs are used and what crystal is used. I suspect many designers will use some reference design layout from ENE, but it won't be good in every case. The wire-up of the ENE of various motherboards is likely something you'll have to live with as unknowns. This is a case where looser tolerances in the in kernel decoders could reduce this driver's complexity and/or get rid of arbitrary fudge factors in the driver. The tolerances are as loose as they can be. The NEC protocol uses pulses that are 4% longer than JVC. The decoders allow errors up to 2% (50% of 4%). The crystals used in electronics are accurate to 0.0001%+. But the standard IR receivers are far from being accurate enough to allow tolerance windows of only 2%. I'm surprised that this works for you. LIRC uses a standard tolerance of 30% / 100 us and even this is not enough sometimes. For the NEC protocol one signal consists of 22 individual pulses at 38kHz.. If the receiver just misses one pulse, you already have an error of 1/22 4%. There are different types of errors. The decoders can take large variations in bit times. The problem is with cumulative errors. In this case the error had accumulated up to 450us in the lead pulse. That's just too big of an error and caused the JVC code to be misclassified as NEC. I think he said lirc was misclassifying it too. So we both did the same thing. No way. JVC is a 16 bit code. NEC uses 32 bits. How can you ever confuse JVC with NEC signals? LIRC will work if there is a 4% or 40% or 400% error. Because irrecord generates the config file using your receiver it will compensate for any At the end of the process we can build a record and match raw mode if we have to. I'm not talking about raw mode here. lircd will happily decode the signals despite of any timing error as long it's consistent. I'm still interested how JVC can be confused with NEC codes. timing error. It will work with pulses cut down to 50 us like IrDA hardware does and it will work when half of the bits are swallowed like the IgorPlug USB receiver does. The code for fixing IrDA and IgorPLug should live inside their low level device drivers. The characteristics of the errors produced by this hardware are known so a fix can be written to compensate. The function f(x) = 50 is not bijective. No way to compensate. Missing bits cannot be magically regenerated by the driver. The IgorPlug people might find it easier to fix their firmware. There is a firmware patch available? Do you have a pointer? Christoph -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
On Fri, 2010-07-30 at 15:45 +0300, Maxim Levitsky wrote: On Fri, 2010-07-30 at 08:07 -0400, Jon Smirl wrote: On Fri, Jul 30, 2010 at 8:02 AM, Jon Smirl jonsm...@gmail.com wrote: On Fri, Jul 30, 2010 at 7:54 AM, Maxim Levitsky maximlevit...@gmail.com wrote: + pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) 4) + + (ene_hw_read_reg(dev, ENE_PLLFRL) 2); I can understand the shift of the high bits, but that shift of the low bits is unlikely. A manual would tell us if it is right. This shift is correct (according to datasheet, which contains mostly useless info, but it does dociment this reg briefly.) The KB3700 series datasheet indicates that the value from ENE_PLLFRL should be shifted by 4 bits, not by 2. Of course, the KB3700 isn't the exact same chip. Regards, Andy -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
On Sat, 2010-07-31 at 09:55 -0400, Andy Walls wrote: On Fri, 2010-07-30 at 15:45 +0300, Maxim Levitsky wrote: On Fri, 2010-07-30 at 08:07 -0400, Jon Smirl wrote: On Fri, Jul 30, 2010 at 8:02 AM, Jon Smirl jonsm...@gmail.com wrote: On Fri, Jul 30, 2010 at 7:54 AM, Maxim Levitsky maximlevit...@gmail.com wrote: + pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) 4) + + (ene_hw_read_reg(dev, ENE_PLLFRL) 2); I can understand the shift of the high bits, but that shift of the low bits is unlikely. A manual would tell us if it is right. This shift is correct (according to datasheet, which contains mostly useless info, but it does dociment this reg briefly.) The KB3700 series datasheet indicates that the value from ENE_PLLFRL should be shifted by 4 bits, not by 2. Of course, the KB3700 isn't the exact same chip. You are right about that, thanks! Best regards, Maxim Levitsky -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
On Sat, Jul 31, 2010 at 10:28 AM, Maxim Levitsky maximlevit...@gmail.com wrote: On Sat, 2010-07-31 at 09:55 -0400, Andy Walls wrote: On Fri, 2010-07-30 at 15:45 +0300, Maxim Levitsky wrote: On Fri, 2010-07-30 at 08:07 -0400, Jon Smirl wrote: On Fri, Jul 30, 2010 at 8:02 AM, Jon Smirl jonsm...@gmail.com wrote: On Fri, Jul 30, 2010 at 7:54 AM, Maxim Levitsky maximlevit...@gmail.com wrote: + pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) 4) + + (ene_hw_read_reg(dev, ENE_PLLFRL) 2); I can understand the shift of the high bits, but that shift of the low bits is unlikely. A manual would tell us if it is right. This shift is correct (according to datasheet, which contains mostly useless info, but it does dociment this reg briefly.) The KB3700 series datasheet indicates that the value from ENE_PLLFRL should be shifted by 4 bits, not by 2. Of course, the KB3700 isn't the exact same chip. You are right about that, thanks! I looked at KB3700 manual. It says it is trying to make a 32Mhz clock by multiplying 32.768Khz * 1000. 32,768 * 1000 = 32.768Mhz is a 2.4% error. When you are computing the timings of the pulses did you assume a 32Mhz clock? It looks like the clock is actuall 32.768Mhz. Best regards, Maxim Levitsky -- Jon Smirl jonsm...@gmail.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
On Sat, 2010-07-31 at 10:37 -0400, Jon Smirl wrote: On Sat, Jul 31, 2010 at 10:28 AM, Maxim Levitsky maximlevit...@gmail.com wrote: On Sat, 2010-07-31 at 09:55 -0400, Andy Walls wrote: On Fri, 2010-07-30 at 15:45 +0300, Maxim Levitsky wrote: On Fri, 2010-07-30 at 08:07 -0400, Jon Smirl wrote: On Fri, Jul 30, 2010 at 8:02 AM, Jon Smirl jonsm...@gmail.com wrote: On Fri, Jul 30, 2010 at 7:54 AM, Maxim Levitsky maximlevit...@gmail.com wrote: + pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) 4) + + (ene_hw_read_reg(dev, ENE_PLLFRL) 2); I can understand the shift of the high bits, but that shift of the low bits is unlikely. A manual would tell us if it is right. This shift is correct (according to datasheet, which contains mostly useless info, but it does dociment this reg briefly.) The KB3700 series datasheet indicates that the value from ENE_PLLFRL should be shifted by 4 bits, not by 2. Of course, the KB3700 isn't the exact same chip. You are right about that, thanks! I looked at KB3700 manual. It says it is trying to make a 32Mhz clock by multiplying 32.768Khz * 1000. 32,768 * 1000 = 32.768Mhz is a 2.4% error. When you are computing the timings of the pulses did you assume a 32Mhz clock? It looks like the clock is actuall 32.768Mhz. No, I just take the samples hardware give me. Lets just leave this as is. Best regards, Maxim Levitsky -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
On Sat, Jul 31, 2010 at 11:12 AM, Andy Walls awa...@md.metrocast.net wrote:
I think you won't be able to fix the problem conclusively either way. A
lot of how the chip's clocks should be programmed depends on how the
GPIOs are used and what crystal is used.
I suspect many designers will use some reference design layout from ENE,
but it won't be good in every case. The wire-up of the ENE of various
motherboards is likely something you'll have to live with as unknowns.
This is a case where looser tolerances in the in kernel decoders could
reduce this driver's complexity and/or get rid of arbitrary fudge
factors in the driver.
The tolerances are as loose as they can be. The NEC protocol uses
pulses that are 4% longer than JVC. The decoders allow errors up to 2%
(50% of 4%). The crystals used in electronics are accurate to
0.0001%+. The 4% error in this driver is because the hardware is not
being programmed accurately. This needs to be fixed in the driver and
not in the upper layers.
How is sample period being computed, where is the complete source to
this driver?
dev-tx_period = 32;
Where is sample_period computed?
@@ -672,13 +583,25 @@ static irqreturn_t ene_isr(int irq, void *data)
pulse = !(hw_value ENE_SAMPLE_SPC_MASK);
hw_value = ENE_SAMPLE_VALUE_MASK;
hw_sample = hw_value * sample_period;
+
+ if (dev-rx_period_adjust) {
+ hw_sample *= (100 - dev-rx_period_adjust);
+ hw_sample /= 100;
+ }
}
I suspect sample_period is set to 32us. For 32.768Mhz the period needs
to be 30.5us. I don't see the code for how it was computed.
You have to be careful with rounding errors when doing this type of
computation. What looks like a minor error can amplify into a large
error. Sometimes I do the math in 64b ints just to keep the round off
errors from accumulating. Instead of doing the math in calculator and
plugging in 32. Use #defines and do the math in the code.
Maybe something like
#define sample_period (1 / (32768 * 1000))
Then don't store this constant in a variable since it will cause a
round off. Just use it directly in the computation.
Regards,
Andy
--
Jon Smirl
jonsm...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
On Sat, 2010-07-31 at 12:25 -0400, Jon Smirl wrote: On Sat, Jul 31, 2010 at 11:12 AM, Andy Walls awa...@md.metrocast.net wrote: I think you won't be able to fix the problem conclusively either way. A lot of how the chip's clocks should be programmed depends on how the GPIOs are used and what crystal is used. I suspect many designers will use some reference design layout from ENE, but it won't be good in every case. The wire-up of the ENE of various motherboards is likely something you'll have to live with as unknowns. This is a case where looser tolerances in the in kernel decoders could reduce this driver's complexity and/or get rid of arbitrary fudge factors in the driver. The tolerances are as loose as they can be. The NEC protocol uses pulses that are 4% longer than JVC. The decoders allow errors up to 2% (50% of 4%). The crystals used in electronics are accurate to 0.0001%+. The 4% error in this driver is because the hardware is not being programmed accurately. This needs to be fixed in the driver and not in the upper layers. Let me explain again. I get samples in 4 byte buffer. each sample is a count of sample periods. Sample period is programmed into hardware, at 'ENE_CIR_SAMPLE_PERIOD' (it is in us) Default sample period is 50 us. The error source isn't 'electronics' fault. The device is microprocessor. I don't read the samples 'directly' from hardware, but rather from ram of that microprocessor. I don't know how it samples the input. A expiration of sample period might just cause a IRQ inside that microprocessor, and it can't process it instantly. That is probably the source of the delay. Or something like that. Best regards, Maxim Levitsky -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
On Sat, 2010-07-31 at 12:25 -0400, Jon Smirl wrote:
On Sat, Jul 31, 2010 at 11:12 AM, Andy Walls awa...@md.metrocast.net wrote:
I think you won't be able to fix the problem conclusively either way. A
lot of how the chip's clocks should be programmed depends on how the
GPIOs are used and what crystal is used.
I suspect many designers will use some reference design layout from ENE,
but it won't be good in every case. The wire-up of the ENE of various
motherboards is likely something you'll have to live with as unknowns.
This is a case where looser tolerances in the in kernel decoders could
reduce this driver's complexity and/or get rid of arbitrary fudge
factors in the driver.
The tolerances are as loose as they can be. The NEC protocol uses
pulses that are 4% longer than JVC. The decoders allow errors up to 2%
(50% of 4%). The crystals used in electronics are accurate to
0.0001%+. The 4% error in this driver is because the hardware is not
being programmed accurately. This needs to be fixed in the driver and
not in the upper layers.
How is sample period being computed, where is the complete source to
this driver?
dev-tx_period = 32;
Where is sample_period computed?
@@ -672,13 +583,25 @@ static irqreturn_t ene_isr(int irq, void *data)
pulse = !(hw_value ENE_SAMPLE_SPC_MASK);
hw_value = ENE_SAMPLE_VALUE_MASK;
hw_sample = hw_value * sample_period;
+
+ if (dev-rx_period_adjust) {
+ hw_sample *= (100 - dev-rx_period_adjust);
+ hw_sample /= 100;
+ }
}
I suspect sample_period is set to 32us. For 32.768Mhz the period needs
to be 30.5us. I don't see the code for how it was computed.
You have to be careful with rounding errors when doing this type of
computation. What looks like a minor error can amplify into a large
error. Sometimes I do the math in 64b ints just to keep the round off
errors from accumulating. Instead of doing the math in calculator and
plugging in 32. Use #defines and do the math in the
There is no reason to worry about rounding here.
hw_sample is maximum of 127 * 50, so when I muliply by 100 I get exact
result.
Then I do one divide.
Best regards,
Maxim Levitsky
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
Hi Jon, on 31 Jul 10 at 12:25, Jon Smirl wrote: On Sat, Jul 31, 2010 at 11:12 AM, Andy Walls awa...@md.metrocast.net wrote: I think you won't be able to fix the problem conclusively either way. A lot of how the chip's clocks should be programmed depends on how the GPIOs are used and what crystal is used. I suspect many designers will use some reference design layout from ENE, but it won't be good in every case. The wire-up of the ENE of various motherboards is likely something you'll have to live with as unknowns. This is a case where looser tolerances in the in kernel decoders could reduce this driver's complexity and/or get rid of arbitrary fudge factors in the driver. The tolerances are as loose as they can be. The NEC protocol uses pulses that are 4% longer than JVC. The decoders allow errors up to 2% (50% of 4%). The crystals used in electronics are accurate to 0.0001%+. But the standard IR receivers are far from being accurate enough to allow tolerance windows of only 2%. I'm surprised that this works for you. LIRC uses a standard tolerance of 30% / 100 us and even this is not enough sometimes. For the NEC protocol one signal consists of 22 individual pulses at 38kHz. If the receiver just misses one pulse, you already have an error of 1/22 4%. Christoph -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
On Sat, Jul 31, 2010 at 1:47 PM, Christoph Bartelmus l...@bartelmus.de wrote: Hi Jon, on 31 Jul 10 at 12:25, Jon Smirl wrote: On Sat, Jul 31, 2010 at 11:12 AM, Andy Walls awa...@md.metrocast.net wrote: I think you won't be able to fix the problem conclusively either way. A lot of how the chip's clocks should be programmed depends on how the GPIOs are used and what crystal is used. I suspect many designers will use some reference design layout from ENE, but it won't be good in every case. The wire-up of the ENE of various motherboards is likely something you'll have to live with as unknowns. This is a case where looser tolerances in the in kernel decoders could reduce this driver's complexity and/or get rid of arbitrary fudge factors in the driver. The tolerances are as loose as they can be. The NEC protocol uses pulses that are 4% longer than JVC. The decoders allow errors up to 2% (50% of 4%). The crystals used in electronics are accurate to 0.0001%+. But the standard IR receivers are far from being accurate enough to allow tolerance windows of only 2%. I'm surprised that this works for you. LIRC uses a standard tolerance of 30% / 100 us and even this is not enough sometimes. For the NEC protocol one signal consists of 22 individual pulses at 38kHz. If the receiver just misses one pulse, you already have an error of 1/22 4%. There are different types of errors. The decoders can take large variations in bit times. The problem is with cumulative errors. In this case the error had accumulated up to 450us in the lead pulse. That's just too big of an error and caused the JVC code to be misclassified as NEC. I think he said lirc was misclassifying it too. So we both did the same thing. Christoph -- Jon Smirl jonsm...@gmail.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
On Sat, 2010-07-31 at 17:53 -0400, Jon Smirl wrote: On Sat, Jul 31, 2010 at 2:51 PM, Andy Walls awa...@md.metrocast.net wrote: On Sat, 2010-07-31 at 14:14 -0400, Jon Smirl wrote: On Sat, Jul 31, 2010 at 1:47 PM, Christoph Bartelmus l...@bartelmus.de wrote: Hi Jon, on 31 Jul 10 at 12:25, Jon Smirl wrote: On Sat, Jul 31, 2010 at 11:12 AM, Andy Walls awa...@md.metrocast.net wrote: I think you won't be able to fix the problem conclusively either way. A lot of how the chip's clocks should be programmed depends on how the GPIOs are used and what crystal is used. I suspect many designers will use some reference design layout from ENE, but it won't be good in every case. The wire-up of the ENE of various motherboards is likely something you'll have to live with as unknowns. This is a case where looser tolerances in the in kernel decoders could reduce this driver's complexity and/or get rid of arbitrary fudge factors in the driver. The tolerances are as loose as they can be. The NEC protocol uses pulses that are 4% longer than JVC. The decoders allow errors up to 2% (50% of 4%). The crystals used in electronics are accurate to 0.0001%+. But the standard IR receivers are far from being accurate enough to allow tolerance windows of only 2%. I'm surprised that this works for you. LIRC uses a standard tolerance of 30% / 100 us and even this is not enough sometimes. For the NEC protocol one signal consists of 22 individual pulses at 38kHz. If the receiver just misses one pulse, you already have an error of 1/22 4%. There are different types of errors. The decoders can take large variations in bit times. The problem is with cumulative errors. In this case the error had accumulated up to 450us in the lead pulse. That's just too big of an error Hi Jon, Hmmm. Leader marks are, by protocol design, there to give time for the receiver's AGC to settle. We should make it OK to miss somewhat large portions of leader marks. I'm not sure what the harm is in accepting too long of a leader mark, but I'm pretty sure a leader mark that is too long will always be due to systematic error and not noise errors. However, if we know we have systematic errors caused by unknowns, we should be designing and implementing a decoding system that reasonably deals with those systematic errors. Again the part of the system almost completely out of our control is the remote controls, and we *have no control* over systematic errors introduced by remotes. We haven't encountered remotes with systematic errors. If remotes had large errors in them they wouldn't be able to reliably control their target devices. Find a remote that won't work with the protocol engines and a reasonably accurate receiver. Obviously we want to reduce or eliminate systematic errors by determining the unknowns and undoing their effects or by compensating for their overall effect. But in the case of the ENE receiver driver, you didn't seem to like the Maxim's software compensation for the systematic receiver errors. I would be happier if we could track down the source of the error instead of sticking a bandaid on at the end of the process. This isn't a bandaid. Windows driver programs the period to 52 but treats it as a 50. (I don't do that because I set period to 75 - otherwise leading pulse of NEC/JVC is almost missing. I know the reason for that, and it isn't important). and caused the JVC code to be misclassified as NEC. I still have not heard why we need protocol discrimination/classifcation in the kernel. Doing discrimination between two protocols with such close timings is whose requirement again? If we don't do protocol engines we have to revert back to raw recording and having everyone train the system with their remotes. The goal is to eliminate the training step. We would also have to have large files (LIRC configs) for building the keymaps and a new API to deal with them. With the engines the key presses are identified by short strings. A use case: install mythtv, add an IR receiver. Myth UI says to set your universal remote to a Motorola DVR profile. Remote works - no training step needed. LIRC has protocol engines too. irrecord first tries to fit the remote into a protocol engine. If it can't it reverts to raw mode. Let's analyze those cases where lirc ends up in raw mode and see if we can figure out what's going wrong. For example I know of two things that will trip up irrecord that are fixed in the kernel system 1) the ene driver. we know now it had a 4% error in the reported periods No it doesn't It even works if leading large pulse is missing. I would never ever think of doing the adjustments, because lircds tolerance is much better. I am tired of this discussion. My ENE receiver does work now, it gives correct samples, it applies same
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
On Thu, 2010-07-29 at 23:46 -0400, Andy Walls wrote: On Thu, 2010-07-29 at 22:39 -0400, Jon Smirl wrote: On Thu, Jul 29, 2010 at 10:17 PM, Maxim Levitsky maximlevit...@gmail.com wrote: note that error_adjustment module option is added. This allows to reduce input samples by a percent. This makes input on my system more correct. Default is 4% as it works best here. Note that only normal input is adjusted. I don't know what adjustments to apply to fan tachometer input. Maybe it is accurate already. Do you have the manual for the ENE chip in English? or do you read Chinese? The datasheet for a similar chip, the KB3700, is out there in English, but it doesn't have CIR. You might find these links mildly interesting: http://www.coreboot.org/Embedded_controller http://wiki.laptop.org/go/Embedded_controller http://lists.laptop.org/pipermail/openec/2008-July/000108.html Nope, I have read that. Regards, Andy Maybe you can figure out why the readings are off by 4%. I suspect that someone has set a clock divider wrong when programming the chip. For example setting the divider for a 25Mhz clock when the clock is actually 26Mhz would cause the error you are seeing. Or they just made a mistake in computing the divisor. It is probably a bug in the BIOS of your laptop. If that's the case you could add a quirk in the system boot code to fix the register setting. I figured out how windows driver compensates for the offset, and do the same in my driver. I think the problem is solved. Best regards, Maxim Levitsky -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
On Fri, Jul 30, 2010 at 7:36 AM, Maxim Levitsky maximlevit...@gmail.com wrote:
On Thu, 2010-07-29 at 23:46 -0400, Andy Walls wrote:
On Thu, 2010-07-29 at 22:39 -0400, Jon Smirl wrote:
On Thu, Jul 29, 2010 at 10:17 PM, Maxim Levitsky
maximlevit...@gmail.com wrote:
note that error_adjustment module option is added.
This allows to reduce input samples by a percent.
This makes input on my system more correct.
Default is 4% as it works best here.
Note that only normal input is adjusted. I don't know
what adjustments to apply to fan tachometer input.
Maybe it is accurate already.
Do you have the manual for the ENE chip in English? or do you read Chinese?
The datasheet for a similar chip, the KB3700, is out there in English,
but it doesn't have CIR.
You might find these links mildly interesting:
http://www.coreboot.org/Embedded_controller
http://wiki.laptop.org/go/Embedded_controller
http://lists.laptop.org/pipermail/openec/2008-July/000108.html
Nope, I have read that.
Regards,
Andy
Maybe you can figure out why the readings are off by 4%. I suspect
that someone has set a clock divider wrong when programming the chip.
For example setting the divider for a 25Mhz clock when the clock is
actually 26Mhz would cause the error you are seeing. Or they just made
a mistake in computing the divisor. It is probably a bug in the BIOS
of your laptop. If that's the case you could add a quirk in the
system boot code to fix the register setting.
I figured out how windows driver compensates for the offset, and do the
same in my driver. I think the problem is solved.
Should that be a = or = instead of !=?
+ if (pll_freq != 1000)
Programming the PLL wrong would cause the 4% error.
hw_revision = ene_hw_read_reg(dev, ENE_HW_VERSION);
old_ver = ene_hw_read_reg(dev, ENE_HW_VER_OLD);
+ pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) 4) +
+ (ene_hw_read_reg(dev, ENE_PLLFRL) 2);
+
+ if (pll_freq != 1000)
+ dev-rx_period_adjust = 4;
+ else
+ dev-rx_period_adjust = 2;
+
+
+ ene_printk(KERN_NOTICE, PLL freq = %d\n, pll_freq);
+
if (hw_revision == 0xFF) {
Best regards,
Maxim Levitsky
--
Jon Smirl
jonsm...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
On Fri, 2010-07-30 at 07:51 -0400, Jon Smirl wrote:
On Fri, Jul 30, 2010 at 7:36 AM, Maxim Levitsky maximlevit...@gmail.com
wrote:
On Thu, 2010-07-29 at 23:46 -0400, Andy Walls wrote:
On Thu, 2010-07-29 at 22:39 -0400, Jon Smirl wrote:
On Thu, Jul 29, 2010 at 10:17 PM, Maxim Levitsky
maximlevit...@gmail.com wrote:
note that error_adjustment module option is added.
This allows to reduce input samples by a percent.
This makes input on my system more correct.
Default is 4% as it works best here.
Note that only normal input is adjusted. I don't know
what adjustments to apply to fan tachometer input.
Maybe it is accurate already.
Do you have the manual for the ENE chip in English? or do you read
Chinese?
The datasheet for a similar chip, the KB3700, is out there in English,
but it doesn't have CIR.
You might find these links mildly interesting:
http://www.coreboot.org/Embedded_controller
http://wiki.laptop.org/go/Embedded_controller
http://lists.laptop.org/pipermail/openec/2008-July/000108.html
Nope, I have read that.
Regards,
Andy
Maybe you can figure out why the readings are off by 4%. I suspect
that someone has set a clock divider wrong when programming the chip.
For example setting the divider for a 25Mhz clock when the clock is
actually 26Mhz would cause the error you are seeing. Or they just made
a mistake in computing the divisor. It is probably a bug in the BIOS
of your laptop. If that's the case you could add a quirk in the
system boot code to fix the register setting.
I figured out how windows driver compensates for the offset, and do the
same in my driver. I think the problem is solved.
Should that be a = or = instead of !=?
+ if (pll_freq != 1000)
This is how its done in windows driver.
Programming the PLL wrong would cause the 4% error.
hw_revision = ene_hw_read_reg(dev, ENE_HW_VERSION);
old_ver = ene_hw_read_reg(dev, ENE_HW_VER_OLD);
+ pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) 4) +
+ (ene_hw_read_reg(dev, ENE_PLLFRL) 2);
+
+ if (pll_freq != 1000)
+ dev-rx_period_adjust = 4;
+ else
+ dev-rx_period_adjust = 2;
+
+
+ ene_printk(KERN_NOTICE, PLL freq = %d\n, pll_freq);
+
if (hw_revision == 0xFF) {
Best regards,
Maxim Levitsky
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
On Fri, Jul 30, 2010 at 7:54 AM, Maxim Levitsky maximlevit...@gmail.com wrote:
On Fri, 2010-07-30 at 07:51 -0400, Jon Smirl wrote:
On Fri, Jul 30, 2010 at 7:36 AM, Maxim Levitsky maximlevit...@gmail.com
wrote:
On Thu, 2010-07-29 at 23:46 -0400, Andy Walls wrote:
On Thu, 2010-07-29 at 22:39 -0400, Jon Smirl wrote:
On Thu, Jul 29, 2010 at 10:17 PM, Maxim Levitsky
maximlevit...@gmail.com wrote:
note that error_adjustment module option is added.
This allows to reduce input samples by a percent.
This makes input on my system more correct.
Default is 4% as it works best here.
Note that only normal input is adjusted. I don't know
what adjustments to apply to fan tachometer input.
Maybe it is accurate already.
Do you have the manual for the ENE chip in English? or do you read
Chinese?
The datasheet for a similar chip, the KB3700, is out there in English,
but it doesn't have CIR.
You might find these links mildly interesting:
http://www.coreboot.org/Embedded_controller
http://wiki.laptop.org/go/Embedded_controller
http://lists.laptop.org/pipermail/openec/2008-July/000108.html
Nope, I have read that.
Regards,
Andy
Maybe you can figure out why the readings are off by 4%. I suspect
that someone has set a clock divider wrong when programming the chip.
For example setting the divider for a 25Mhz clock when the clock is
actually 26Mhz would cause the error you are seeing. Or they just made
a mistake in computing the divisor. It is probably a bug in the BIOS
of your laptop. If that's the case you could add a quirk in the
system boot code to fix the register setting.
I figured out how windows driver compensates for the offset, and do the
same in my driver. I think the problem is solved.
Should that be a = or = instead of !=?
+ if (pll_freq != 1000)
This is how its done in windows driver.
That doesn't mean it is bug free.
Experimenting with changing the PLL frequency register may correct the
error. Try taking 96% of pll_freq and write it back into these
register. This would be easy to fix with a manual. The root problem is
almost certainly a bug in the way the PLLs were programmed.
I don't like putting in fudge factors like the 4% correction. What
happens if a later version of the hardware has fixed firmware? I
normal user is never going to figure out that they need to change the
fudge factor.
+ pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) 4) +
+ (ene_hw_read_reg(dev, ENE_PLLFRL) 2);
+
Programming the PLL wrong would cause the 4% error.
hw_revision = ene_hw_read_reg(dev, ENE_HW_VERSION);
old_ver = ene_hw_read_reg(dev, ENE_HW_VER_OLD);
+ pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) 4) +
+ (ene_hw_read_reg(dev, ENE_PLLFRL) 2);
+
+ if (pll_freq != 1000)
+ dev-rx_period_adjust = 4;
+ else
+ dev-rx_period_adjust = 2;
+
+
+ ene_printk(KERN_NOTICE, PLL freq = %d\n, pll_freq);
+
if (hw_revision == 0xFF) {
Best regards,
Maxim Levitsky
--
Jon Smirl
jonsm...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
On Fri, Jul 30, 2010 at 8:02 AM, Jon Smirl jonsm...@gmail.com wrote:
On Fri, Jul 30, 2010 at 7:54 AM, Maxim Levitsky maximlevit...@gmail.com
wrote:
On Fri, 2010-07-30 at 07:51 -0400, Jon Smirl wrote:
On Fri, Jul 30, 2010 at 7:36 AM, Maxim Levitsky maximlevit...@gmail.com
wrote:
On Thu, 2010-07-29 at 23:46 -0400, Andy Walls wrote:
On Thu, 2010-07-29 at 22:39 -0400, Jon Smirl wrote:
On Thu, Jul 29, 2010 at 10:17 PM, Maxim Levitsky
maximlevit...@gmail.com wrote:
note that error_adjustment module option is added.
This allows to reduce input samples by a percent.
This makes input on my system more correct.
Default is 4% as it works best here.
Note that only normal input is adjusted. I don't know
what adjustments to apply to fan tachometer input.
Maybe it is accurate already.
Do you have the manual for the ENE chip in English? or do you read
Chinese?
The datasheet for a similar chip, the KB3700, is out there in English,
but it doesn't have CIR.
You might find these links mildly interesting:
http://www.coreboot.org/Embedded_controller
http://wiki.laptop.org/go/Embedded_controller
http://lists.laptop.org/pipermail/openec/2008-July/000108.html
Nope, I have read that.
Regards,
Andy
Maybe you can figure out why the readings are off by 4%. I suspect
that someone has set a clock divider wrong when programming the chip.
For example setting the divider for a 25Mhz clock when the clock is
actually 26Mhz would cause the error you are seeing. Or they just made
a mistake in computing the divisor. It is probably a bug in the BIOS
of your laptop. If that's the case you could add a quirk in the
system boot code to fix the register setting.
I figured out how windows driver compensates for the offset, and do the
same in my driver. I think the problem is solved.
Should that be a = or = instead of !=?
+ if (pll_freq != 1000)
This is how its done in windows driver.
That doesn't mean it is bug free.
Experimenting with changing the PLL frequency register may correct the
error. Try taking 96% of pll_freq and write it back into these
register. This would be easy to fix with a manual. The root problem is
almost certainly a bug in the way the PLLs were programmed.
I don't like putting in fudge factors like the 4% correction. What
happens if a later version of the hardware has fixed firmware? I
normal user is never going to figure out that they need to change the
fudge factor.
+ pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) 4) +
+ (ene_hw_read_reg(dev, ENE_PLLFRL) 2);
I can understand the shift of the high bits, but that shift of the low
bits is unlikely. A manual would tell us if it is right.
+
Programming the PLL wrong would cause the 4% error.
hw_revision = ene_hw_read_reg(dev, ENE_HW_VERSION);
old_ver = ene_hw_read_reg(dev, ENE_HW_VER_OLD);
+ pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) 4) +
+ (ene_hw_read_reg(dev, ENE_PLLFRL) 2);
+
+ if (pll_freq != 1000)
+ dev-rx_period_adjust = 4;
+ else
+ dev-rx_period_adjust = 2;
+
+
+ ene_printk(KERN_NOTICE, PLL freq = %d\n, pll_freq);
+
if (hw_revision == 0xFF) {
Best regards,
Maxim Levitsky
--
Jon Smirl
jonsm...@gmail.com
--
Jon Smirl
jonsm...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
On Fri, 2010-07-30 at 08:07 -0400, Jon Smirl wrote: On Fri, Jul 30, 2010 at 8:02 AM, Jon Smirl jonsm...@gmail.com wrote: On Fri, Jul 30, 2010 at 7:54 AM, Maxim Levitsky maximlevit...@gmail.com wrote: On Fri, 2010-07-30 at 07:51 -0400, Jon Smirl wrote: On Fri, Jul 30, 2010 at 7:36 AM, Maxim Levitsky maximlevit...@gmail.com wrote: On Thu, 2010-07-29 at 23:46 -0400, Andy Walls wrote: On Thu, 2010-07-29 at 22:39 -0400, Jon Smirl wrote: On Thu, Jul 29, 2010 at 10:17 PM, Maxim Levitsky maximlevit...@gmail.com wrote: note that error_adjustment module option is added. This allows to reduce input samples by a percent. This makes input on my system more correct. Default is 4% as it works best here. Note that only normal input is adjusted. I don't know what adjustments to apply to fan tachometer input. Maybe it is accurate already. Do you have the manual for the ENE chip in English? or do you read Chinese? The datasheet for a similar chip, the KB3700, is out there in English, but it doesn't have CIR. You might find these links mildly interesting: http://www.coreboot.org/Embedded_controller http://wiki.laptop.org/go/Embedded_controller http://lists.laptop.org/pipermail/openec/2008-July/000108.html Nope, I have read that. Regards, Andy Maybe you can figure out why the readings are off by 4%. I suspect that someone has set a clock divider wrong when programming the chip. For example setting the divider for a 25Mhz clock when the clock is actually 26Mhz would cause the error you are seeing. Or they just made a mistake in computing the divisor. It is probably a bug in the BIOS of your laptop. If that's the case you could add a quirk in the system boot code to fix the register setting. I figured out how windows driver compensates for the offset, and do the same in my driver. I think the problem is solved. Should that be a = or = instead of !=? + if (pll_freq != 1000) This is how its done in windows driver. That doesn't mean it is bug free. This PLL frequency is likely to be chip internal frequency. And windows driver doesn't touch it. Its embedded controller, so I don't want to touch things I am not sure about. Experimenting with changing the PLL frequency register may correct the error. Try taking 96% of pll_freq and write it back into these register. This would be easy to fix with a manual. The root problem is almost certainly a bug in the way the PLLs were programmed. I don't like putting in fudge factors like the 4% correction. What happens if a later version of the hardware has fixed firmware? I normal user is never going to figure out that they need to change the fudge factor. I don't think that is a hardware bug, rather a limitation. Lets leave it as is. I will soon publish the driver on launchpad or something like that and try to contact users I debugged that driver with, and then see what ranges PLL register takes. + pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) 4) + + (ene_hw_read_reg(dev, ENE_PLLFRL) 2); I can understand the shift of the high bits, but that shift of the low bits is unlikely. A manual would tell us if it is right. This shift is correct (according to datasheet, which contains mostly useless info, but it does dociment this reg briefly.) Best regards, Maxim Levitsky -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
On Thu, Jul 29, 2010 at 10:17 PM, Maxim Levitsky maximlevit...@gmail.com wrote: note that error_adjustment module option is added. This allows to reduce input samples by a percent. This makes input on my system more correct. Default is 4% as it works best here. Note that only normal input is adjusted. I don't know what adjustments to apply to fan tachometer input. Maybe it is accurate already. Do you have the manual for the ENE chip in English? or do you read Chinese? Maybe you can figure out why the readings are off by 4%. I suspect that someone has set a clock divider wrong when programming the chip. For example setting the divider for a 25Mhz clock when the clock is actually 26Mhz would cause the error you are seeing. Or they just made a mistake in computing the divisor. It is probably a bug in the BIOS of your laptop. If that's the case you could add a quirk in the system boot code to fix the register setting. -- Jon Smirl jonsm...@gmail.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.
On Thu, 2010-07-29 at 22:39 -0400, Jon Smirl wrote: On Thu, Jul 29, 2010 at 10:17 PM, Maxim Levitsky maximlevit...@gmail.com wrote: note that error_adjustment module option is added. This allows to reduce input samples by a percent. This makes input on my system more correct. Default is 4% as it works best here. Note that only normal input is adjusted. I don't know what adjustments to apply to fan tachometer input. Maybe it is accurate already. Do you have the manual for the ENE chip in English? or do you read Chinese? The datasheet for a similar chip, the KB3700, is out there in English, but it doesn't have CIR. You might find these links mildly interesting: http://www.coreboot.org/Embedded_controller http://wiki.laptop.org/go/Embedded_controller http://lists.laptop.org/pipermail/openec/2008-July/000108.html Regards, Andy Maybe you can figure out why the readings are off by 4%. I suspect that someone has set a clock divider wrong when programming the chip. For example setting the divider for a 25Mhz clock when the clock is actually 26Mhz would cause the error you are seeing. Or they just made a mistake in computing the divisor. It is probably a bug in the BIOS of your laptop. If that's the case you could add a quirk in the system boot code to fix the register setting. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
