Hi
On Fri, Jan 17, 2025 at 03:49:25PM +0100, Lars Pedersen wrote:
On Fri, 17 Jan 2025 at 13:32, Greg KH gregkh@linuxfoundation.org wrote:
On Thu, Jan 16, 2025 at 03:21:13PM +0100, Lars Pedersen wrote:
Hi. We have discovered an SPI regression when upgrading from 6.1.99 to a newer LTS version. Same error on kernel 6.6.71 and 6.12.8.
We have a very similar regression on the Nokia N900, causing everything on the SPI bus (WLAN, screen) to stop working or be unreliable.
I think we have identified the problem down to the reference clock calculation that seems to end up to zero in the spi-omap2-mcspi driver.
Also we think it relates to commit 4c6ac5446d060f0bf435ccc8bc3aa7b7b5f718ad, where OMAP2_MCSPI_MAX_FREQ is used as fallback on error. In our case it seems to hit the else case.
If you revert the offending commit, does that solve the issue?
thanks,
greg k-h
Hi Greg.
No it doesn't solve the issue by reverting the commit. The commit isn't the regression, but it attempts to handle it in the if/else statement. Everything starts to work again if we hard code it to "mcspi->ref_clk_hz = OMAP2_MCSPI_MAX_FREQ;", so it seems like the if else statement isn't 100% foolproof (or we have missed a setting in the device tree).
The commit actually *is* the regression. The subtle issue causing it is that devm_clk_get_optional_enabled() may also return NULL. In the previous code, the NULL was not considered an error condition. The change to the IS_ERR macro causes the NULL condition to be unhandled, hence it takes the `else` path. Using IS_ERR_OR_NULL restores the previous behavior, since changing it was not the intention of the commit in question.
Please check if the attached patch does not fix it for you, and I can submit it, perhaps with your Tested-by. It does resolve the issue on the Nokia N900, for both WLAN and the screen.
Regards Sicelo