Re: SPI regression seen on ARM am335x in kernel 6.12.8 and 6.6.71

[Sicelo <absicsz@xxxxxxxxx>]

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@xxxxxxxxxxxxxxxxxxx> 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