Re: linux 4.2-rc1 broken Nokia N900

Pali Rohár
2015-07-22T14:27:18Z

On Wednesday 22 July 2015 15:33:18 Sebastian Reichel wrote:
> Hi,
>
> On Wed, Jul 22, 2015 at 09:27:32AM +0200, Pali Rohár wrote:
> > > > diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
> > > > index 5867384..f7d9ffd 100644
> > > > --- a/drivers/spi/spi-omap2-mcspi.c
> > > > +++ b/drivers/spi/spi-omap2-mcspi.c
> > > > @@ -245,6 +245,7 @@ static void omap2_mcspi_set_enable(const struct spi_device *spi, int enable)
> > > >
> > > > static void omap2_mcspi_set_cs(struct spi_device *spi, bool enable)
> > > > {
> > > > + struct omap2_mcspi *mcspi = spi_master_get_devdata(spi->master);
> > > > u32 l;
> > > >
> > > > /* The controller handles the inverted chip selects
> > > > @@ -255,6 +256,8 @@ static void omap2_mcspi_set_cs(struct spi_device *spi, bool enable)
> > > > enable = !enable;
> > > >
> > > > if (spi->controller_state) {
> > > > + pm_runtime_get_sync(mcspi->dev);
> > > > +
> > >
> > > Should the return code should be checked here as with the other
> > > instances of pm_runtime_get_sync?
>
> Yes. If it fails, the line fetch error would be triggered again.
>
> Probably the set_cs callback should also be changed, so that it can
> return an error code. This is not something for 4.2-rc, though.
>
> > Anyway, what do you think about adding gcc macro for pm_runtime_get_sync
> > function which report compiler warning when return value is not checked?
>
> Adding __must_check to pm_runtime_get_sync() should be discussed
> separately with the relevant people in Cc. A quick grep in drivers/
> reveals a huge amount of code not handling pm_runtime_get_sync's return
> code.
>
> -- Sebastian

Two times, missing check for pm_runtime_get_sync() cased kernel crash on
Nokia N900. See that crypto commits. I think all drivers which do not
check should be fixed. Otherwise we can see another mysterious problems.

Sebastian, can you CC relevant people? Or maybe start new thread?

--
Pali Rohár
pali.rohar@gmail.com