Re: devicefs & sleep support for IDE

[Pavel Machek]

Hi!

> > New patch, rediffed against 2.5.36.
> > 
> > More patches will be needed to support IDE properly (like DVD burners
> > you mentioned), but this is known to fix data corruption. It has zero
> > impact on actual I/O. It affects initialization and suspend only.
> > Please apply this time.
> 
> Basic driver model support for IDE is in 2.5.38. This just involves 
> creating an IDE bus type, and registering drives as devices. I.e. there is 
> no driver set for any of the drives. 

Yep, I saw the support and it confused me a lot.

Questions: is it possible that in hwif_register you don't need to
initialize parent?

Where is device_put of hwif->gendev? I miss it.

> I do have a couple of comments though.
> 
> > +static struct device_driver idedisk_devdrv = {
> > +	.lock = RW_LOCK_UNLOCKED,
> > +	.suspend = idedisk_suspend,
> > +	.resume = idedisk_resume,
> > +};
> 
> You don't need to initialize .lock. But, you do need a .name and .bus. The 
> driver won't even be registered unless .bus is set. 
> 
> > @@ -835,6 +837,7 @@
> >  	int		crc_count;	/* crc counter to reduce drive speed */
> >  	struct list_head list;
> >  	struct gendisk *disk;
> > +	struct device	device;		/* for driverfs */
> >  } ide_drive_t;
> 
> There is a struct device in struct gendisk; that should suffice. But note 
> that you may have to do an extra conversion in order to access it in the 
> driver callbacks. 

Thanx for info.

Ouch. There are actually two devices in struct gendisk. I choosed
disk_dev. Was it right?

> > +	struct device	device;		/* for devicefs */
> 
> Please: it's driver model support, not driverfs. And devicefs does not 
> even exist. :)

Okay, okay ;-).
								Pavel
-- 
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.