[patch 00/18] PS3 patches for 2.6.23

[patch 00/18] PS3 patches for 2.6.23

[Geoff Levand]

Hi.

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. 

[01/18] Cell: Add spu shutdown method
[02/18] PS3: Rename IPI symbols
[03/18] PS3: Use __maybe_unused
[04/18] PS3: Compare firmware version
[05/18] PS3: Fix sparse warnings
[06/18] PS3: Add support for HDMI RGB Full Range mode
[07/18] PS3: Make ps3av.h usable from user space
[08/18] PS3: Kexec support
[09/18] PS3: System-bus rework
[10/18] PS3: System-bus uevent
[11/18] PS3: System-bus modinfo attribute
[12/18] PS3: Repository probe cleanups
[13/18] PS3: USB system-bus rework
[14/18] PS3: vuart rework
[15/18] PS3: sys-manager re-work
[16/18] PS3: Rework AV settings driver
[17/18] PS3: frame buffer system-bus rework
[18/18] PS3: Device registration routines.

-Geoff
 

-- 

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

[Milton Miller]

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

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

[Benjamin Herrenschmidt]

On Mon, 2007-06-11 at 02:08 -0500, Milton Miller wrote:

> (Or disable the ps3 system bus during the merge of the series).

Considering that

 - It breaks only ps3
 - ps3, as of today, doesn't quite work anyway without those patches

I'd be tempted to say bisect doesn't matter. If you're bisecting
something else, then just disable ps3 support.

Ben.

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

[Geoff Levand]

Milton Miller wrote:
> 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).


Yes, I realize the state of the patches.  I will try to rework them for
the next round.  As the existing PS3 support is incomplete, I don't
think there will be many doing a git-bisect with PS3 support enabled.


>> [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.


I will consider this for 2.6.24.  We don't have time to rework it
now.


>> [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.


I think this is just the same as your first comment about change
isolation, or is it something else?


>> [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?


I wonder if you are not looking at the definition of
ps3_vuart_port_driver or struct ps3av, but the usage of a
ps3_system_bus_device?

The core of a ps3_system_bus_device is a struct device, and struct
device has no member named core.  vuart devices are instances of
ps3_system_bus_device, and so have no member named .core.core.

Anyway, I would think it would not build properly if an incorrect
type was used.


> 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.


There is just a single AV setting device, so a second registration would
indicate a code bug.  It won't hurt to add a check and Geert added
that in.


>> [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).


A good idea if we want to run more than one probe in the future.  We
are working on an interrupt driven registration though, and hopefully
that will eliminate this polling thread.


-Geoff

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

[Arnd Bergmann]

On Tuesday 12 June 2007, Geoff Levand wrote:
>=20
> > The dependent series is currently split per file. =A0 While this makes =
it=20
> > easier to see what a given area looks like after all the patches, it=20
> > means that the kernel compile will break during the series. =A0Breaking=
=20
> > git bisect is frowned upon, so it will need to be re-split for merge. =
=A0
> > (Or disable the ps3 system bus during the merge of the series).
>=20
>=20
> Yes, I realize the state of the patches. =A0I will try to rework them for
> the next round. =A0As the existing PS3 support is incomplete, I don't
> think there will be many doing a git-bisect with PS3 support enabled.

I think it's ok to split the patches like this for review purposes.

=46or the upstream merge, it's probably best if Geoff commits all
patches with cross-dependencies as a single git checkin, and asks
Paul (or me) to pull from there. That will take care of the git-bisect
problem, and give a nicer changelog at the same time.

Of course, patches that only have directed dependencies should go
in as separate changesets whereever that makes sense.

	Arnd <><