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

[Maxim Levitsky <maximlevitsky@gmail.com>]

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