Re: [PATCH 13/13] IR: Port ene driver to new IR subsystem and enable it.

Maxim Levitsky
2010-07-31T16:51:14Z

On Sat, 2010-07-31 at 12:25 -0400, Jon Smirl wrote:
> On Sat, Jul 31, 2010 at 11:12 AM, Andy Walls <awalls@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