Re: [PATCH] i2c: mxs: fix broken timing calculation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Lothar,

> Hi Mark,
> 
> Marek Vasut writes:
> > Dear Lothar Waßmann,
> > 
> > > Hi,
> > > 
> > > Marek Vasut writes:
> > > > Hi Lothar,
> > > > 
> > > > > The timing calculation is rather bogus and gives extremely wrong
> > > > > results for higher frequencies (on an i.MX28). E.g. instead of 400
> > > > > kHz I measured 770 kHz.
> > > > > 
> > > > > Implement a calculation that adheres to the I2C spec and gives
> > > > > exact results for I2C frequencies from 12.56 kHz to 960 kHz.
> > > > > 
> > > > > Also the bus_free and leadin parameters are programmed according to
> > > > > the I2C spec for standard and fast mode.
> > > > 
> > > > I suspect the resulting speed is heavily dependent on hardware
> > > > properties of the bus. Did you have a chance to check it with a
> > > > scope? I will try to recheck on other boards next week.
> > > 
> > > Of course I did! I found the DS1339 RTC not working on our hardware
> > > with the I2C clock frequency set to 400kHz and then checked the bus
> > > timing. I found the SCL frequency to be 770kHz instead of 400kHz and
> > > 113kHz instead of 100kHz.
> > > On what hardware did you do your measurements?
> > 
> > MX28EVK and M28EVK.
> > 
> > > The fancy constants -2 and -7 in the calculation were derived from
> > > measuring the clock low and high time with low_count and high_count
> > > set to 1 and measuring the actual timing of the signal.
> > > The clock frequeny in this setup is 2.18 MHz corresponding to 11 clock
> > > cycles of the 24MHz clock. The LOW time was about 140ns and the HIGH
> > > time 318ns corresponding to 3 and 8 (instead of 1) clock cycles.
> > > 
> > > These constants could be different for different SoCs (i.MX23).
> > > But I don't have any hardware to verify that.
> > 
> > I can check it for you on MX23 next week, I have two boards with that
> > chip with well accessible I2C. I am not in the office now, so this will
> > have to wait until next week.
> 
> Did you have time to look into this? And also recheck the timing on
> the i.MX28? Because there obviously is a fundamental discrepancy
> between your and my measurements.

I did not, sorry. I am catching a train to Prague only today, my vacation took a 
tag longer.

> > btw offtopic, I will at least try to fix the PIO in the meantime.
> 
> Did you succeed at this? Because this is the real problem for the
> DS1339 failing on our board. With DMA only transfers it works, but
> other chips (TSC2007, PCA9554, SGTL5000) fail.

Is that correct to assume that even DMA fails? So far I got to a patch [1], 
which is almost an RFC, but please give it a go. I suspect I didn't CC you, I 
will CC you on V2.

> I also tried to get the PIO transfers working but didn't have any
> success. Interestingly all the chips mentioned above worked flawlessly
> with the 3.3 kernel. That should exclude any HW deficiencies of our
> modules or the I2C interface of the i.MX28 in general.

At that time, the i2c used the PIOQUEUE mode, but that is not compatible with 
MX23. Think of it like this:

        PIO    PIOQUEUE  DMA
MX23  broken      N/A    OK
MX28    OK        OK     OK

So MX23 is really a poor device :(

> What disturbs me most is the sequence of accessing the I2C DATA
> register and clearing the DMAREQ bit. Normally you should clear the
> DMAREQ _before_ accessing the DATA reg, because the access will
> trigger a new assertion of DMAREQ.

Fully agreed. I spent _days_ on the patch [1] to fully understand the hardware 
and fix all the fine details of this operation. I also tried to document it in 
the code for future adventurers who dare to enter this insanity. The DMAREQ bit 
is nonsense it seems.

> Thus clearing that bit _after_
> the access (like suggested in the i.MX28 Ref Manual and currently
> implemented in the driver) is racy and potentially leads to
> inadvertently clearing a fresh DMAREQ condition.

Yes, indeed.

> But handling it 'the right way' in the driver doesn't improve
> anything.
> 
> 
> Lothar Waßmann

[1] http://permalink.gmane.org/gmane.linux.drivers.i2c/15787

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux