Subject: Re: [patch 00/18] PS3 patches for 2.6.23

From: Milton Miller <[email-masked]>
Date: Mon, 11 Jun 2007 02:08:58 -0500

On Wed Jun 6 12:59:06 EST 2007, Geoff Levand wrote:
> Here is a series I have for 2.6.23.
>
> Patches 1-8 are more or less independent of each other.
> Patches 9-18 are a dependent series of my system bus
> rework patches.

I'm repling to the summary but will pull a few points from the series.

The dependent series is currently split per file.   While this makes it 
easier to see what a given area looks like after all the patches, it 
means that the kernel compile will break during the series.  Breaking 
git bisect is frowned upon, so it will need to be re-split for merge.  
(Or disable the ps3 system bus during the merge of the series).

> [09/18] PS3: System-bus rework
> [10/18] PS3: System-bus uevent
> [11/18] PS3: System-bus modinfo attribute

If you define a module device table and add it to 
scripts/bin/file2alias.c you would not need to create the MOD_ALIAS_xxx 
driver defines.

Should your module alias and matching include the device_type?

Rather than setting core.owner = THIS_MODULE, take the 
pci_driver_register approach where __pci_driver_register takes the 
module name and owner id as arguments that are automatically passed by 
a macro.

> [12/18] PS3: Repository probe cleanups
> [13/18] PS3: USB system-bus rework
> [14/18] PS3: vuart rework

This changes vuart_device->core from struct_device to ps3 system 
device, but doesn't update the drivers.

> [15/18] PS3: sys-manager re-work
> [16/18] PS3: Rework AV settings driver

There are several dev_debug() that still use vuart_device.core not 
.core.core.
Maybe vuart_device.core should be renamed?

The probe routine assigns to the global ps3av without checking if its 
already set.  If a second device caused probe to be called the 
references to the first one would be lost.

> [17/18] PS3: frame buffer system-bus rework
> [18/18] PS3: Device registration routines.

ps3_start_probe_thread should add at least the bus_type to the thread 
name (the thread name is a sprintf va_list format arg; you don't need 
to create a temporary string buffer).

milton