lkml.org 
[lkml]   [2012]   [Aug]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [REGRESION] Suspend hangs with 3.6-rc1 on Lenovo T60 notebook
On Thu, 16 Aug 2012, Hans de Goede wrote:

> Ah right, these are bus_driver operations. That explains some things, so I've
> done some more research asking myself: "Why does generic_ide_suspend(), which
> is a *bus* op, call dev_get_drvdata?", the answer to that seems to be that
> the ide subsystem is abusing (IMHO) drvdata to store per device bus_driver
> data. Which I believe is not how drvdata is intended to be used.
>
> With that said, the above knowledge has allowed me to write an (ugly) fix for
> the regression Miklos is seeing. Miklos can you give the attached patch a
> try please?
>
> > It clearly should check if drive is not NULL before using that pointer.
>
> I assume you mean drive*r*, yes I agree that generic_ide_remove should
> check for that. So who is going to write a patch for that?

The existing code could certainly be improved. Your patch does:

> + /*
> + * device_register() will have cleared drvdata on
> + * device_attach failure, but we use drvdata to store per
> + * device bus info, rather then for driver info, so restore it.
> + */
> + dev_set_drvdata(dev, drive);

But at this point, dev is defined by:

struct device *dev = &drive->gendev;

So why bother setting anything? It seems to me that
generic_ide_suspend() and generic_ide_resume() could easily replace

ide_drive_t *drive = dev_get_drvdata(dev);

with

ide_drive_t *drive = dev_to_ide_drive(dev);

where dev_to_ide_drive is defined as "container_of(dev, ide_drive_t,
gendev)" (if this isn't defined already).

Alan Stern



\
 
 \ /
  Last update: 2012-08-17 01:01    [W:0.746 / U:0.256 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site