Re: [PATCH v2] i2c: omap: Fix standard mode false ACK readings

[H. Nikolaus Schaller]

Hi Andreas,

> Am 13.09.2024 um 15:32 schrieb Andreas Kemnade <andreas@xxxxxxxxxxxx>:
> 
>>> I had a patch to disable 1Ghz on that
>>> device in my tree. Do you have anything strange in your
>>> tree?  
>> 
>> No, and the omap3 is running with 800 MHz only.
>> 
> So you have a patch disabling 1Ghz OPP in there?

I think the speed is binned to be 800 MHz only. So the OPP is ignored.

> The error messages
> look like things I got when 1Ghz was enabled, so better double check.

Well, it turns out to be difficult to check since with 6.11-rc7
cpufreq-info seems to be broken... I have not yet installed the fixed 4.19.283 again.

But indeed I have found a potential issue. We have a patch [1] for the gta04a5 (only) that adds

&cpu0_opp_table {
	/* is unreliable on gta04a5 - enable by echo 1 >/sys/devices/system/cpu/cpufreq/boost */
	opp1g-1000000000 {
		turbo-mode;
	};
};

so that 1 GHz must be explicitly enabled by user-space.

But some time ago the 1GHz node was apparently renamed to opp-1000000000 (5821d766932cc8)
and this patch was not adjusted.

After fixing it I can ask again for cpufreq-info and the 1GHz OPP is not activated:

root@letux:~# cpufreq-info 
cpufrequtils 008: cpufreq-info (C) Dominik Brodowski 2004-2009
Report errors and bugs to cpufreq@xxxxxxxxxxxxxxx, please.
analyzing CPU 0:
  driver: cpufreq-dt
  CPUs which run at the same hardware frequency: 0
  CPUs which need to have their frequency coordinated by software: 0
  maximum transition latency: 300 us.
  hardware limits: 300 MHz - 800 MHz
  available frequency steps: 300 MHz, 600 MHz, 800 MHz
  available cpufreq governors: conservative, ondemand, userspace, powersave, performance
  current policy: frequency should be within 300 MHz and 800 MHz.
                  The governor "ondemand" may decide which speed to use
                  within this range.
  current CPU frequency is 800 MHz (asserted by call to hardware).
  cpufreq stats: 300 MHz:37.38%, 600 MHz:10.11%, 800 MHz:52.51%, 1000 MHz:0.00%  (1740)
root@letux:~# 

Anyways, this bug was introduced some months after this i2c patch we
are discussing here. So i2c broke first before the 800MHz limitation was accidentially
removed. Therefore I am quite sure that the failing 4.19.283 did run at 800 MHz.

And in the v4.19.282 and v4.19.283 based kernels we have simply commented out the 1GHz
option (since 2018) or there is no 1GHz OPP at all.

Thanks for the hint to take a second and closer look at it, but it doesn't seem to
be a factor here.

> if it is letux, then there is e.g. the interrupt reversal in there.
> Maybe it unveils some problem which should be fixed, maybe it is
> harmful, it was never well reviewed...

I know what you refer to but I could not find it any more. But I may not have
searched correctly.

> 
>> I haven't tested on another board but the bug is very reproducible
>> and I was able to bisect it to this patch, which makes the difference.
>> 
> the error messages, esp. regarding rcu do not look so related to this.
> Maybe having this patch or not triggers some other bug. Maybe we trigger
> some race conditions. Or i2c error checking regarding OPP setting...

That is what I suspect as well. I2C is used to switch the twl4030 for different OPPs...

> 
>> So there may be boards which happily run with the patch and some
>> don't. Maybe a race condition with hardware.
>> 
> I am not ruling out that this patch has nasty side effects but I think
> there is more in the game.

Yes, that is why I think just reverting this patch may only hide a
symptom and does not solve it.

But it may as well have introduced a bug as Tony apparently was thinking
of when asking.

BR and thanks,
Nikolaus

[1]: https://git.goldelico.com/?p=letux-kernel.git;a=commit;h=e824f0c9513cf1d57eba0c9a2ce5fe264fafc8d5