Re: [PATCH] 2.5.32 port PnP BIOS to the driver model RESEND #1

[Adam Belay <ambx1@netscape.net>]

[-- Attachment #1: Type: text/plain, Size: 2783 bytes --]



greg@kroah.com wrote:

>
>Hi,
>
>I don't have a box with a PnP BIOS (well, I don't think I do...), so
>could you send the relevant portions of the driverfs tree, showing the
>new devices that you add for this bus?
>
Just out of curiosity what architecture are you using, is it one that 
doesn't support PnP BIOS?

Here they are:
/driverfs/device/pnp
/driverfs/device/pnp/01
/driverfs/device/pnp/02
/driverfs/device/pnp/03
etc.
/driverfs/bus/pnp
/driverfs/bus/pnp/devices
etc.
/driverfs/bus/pnp/drivers
etc.

>
>
>Also a few minor comments on the patch:
>    - pnpbios_bus_type should probably be made static, along with
>      alloc_pnpbios_root().
>
alloc_pnpbios_root is now static.  I'm going to leave bus_type as it is 
because I want it open to other files at least for now.

>
>    - You don't check for out of memory in alloc_pnpbios_root() when
>      you call kmalloc().
>
Thanks for pointing it out, fixed it.

>
>    - why are you modifying the set_limit() parameters at the top of
>      your patch?  That doesn't seem relevant to the driverfs
>      changes.
>
The pnpbios driver will not compile without these changes, it was broken 
a few versions back.  I also made some improvements to insert_device.

>
>    - in pnpbios_bus_match(), don't you have to check the value of
>      the call to match_device() to make sure you have a match?
>      That would keep pnpbios_device_probe() from being called for
>      every device like it looks your patch causes.
>
I did some serious restructuring here and in pnpbios_device_probe.  Also 
I made it a bit more like the one used by pci.  Hopefully it's all right 
now.

>
>    - the pnpbios_device_probe() call should return a negative error
>      number if the device does not match, or some error happens.
>      Returning 1 does not mean success.  You also need to save off
>      the device specific info somehow in your structure, so that
>      the pnpbios_device_remove() can remove it.  Or am I just
>      missing something here?
>
pnpbios_device_probe now returns a negative number on failure.  I'm 
creating a more flexible pnpbios specific device data structure that can 
be used instead of pci_dev in my next patch.  I should be able to clean 
some of this up once I do that.  I'll take care of the device specific 
info then.

>
>
>And is there some way you can inline the patch?  It wasn't that big...
>
Yep, I'm getting a new isp and email address soon, then I'll be able to 
use Mutt instead of Mozzila.  It turns out netscape mail doesn't really 
use imap like it appeared to, so it's incompatible.  For now this is the 
only safe way I can send patches.

>
>
>thanks,
>
>greg k-h
>
Thank you for reviewing my patch.  I appreciate it.  If there are any 
other issues please let me know.

thanks,
Adam

[-- Attachment #2: pnpbios1-rev1.patch.gz --]
[-- Type: application/octet-stream, Size: 2708 bytes --]