Skip to contentSkip to conversation

PS3 patches for 2.6.23

PATCH In review Geoff Levand 4 participants 5 messages Jun 6, 2007 – Jun 12, 2007

lore
Share & tools
lore
#0 lore raw

cover PS3 patches for 2.6.23

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

#1 lore raw
Quoted 5 lines · Geoff Levand

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

Quoted 3 lines

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

Quoted 3 lines

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

Quoted 2 lines

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

Quoted 2 lines

[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

#2 lore raw
Quoted 1 line · Milton Miller

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

#3 lore raw
In reply to Milton Miller
Quoted 7 lines · Milton Miller
Quoted 5 lines · Geoff Levand

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.

Quoted 7 lines

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.

Quoted 8 lines
Quoted 3 lines

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

Quoted 6 lines

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.

Quoted 6 lines
Quoted 3 lines

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

Quoted 6 lines
Quoted 2 lines

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

Quoted 3 lines

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.

Quoted 6 lines
Quoted 2 lines

[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

#4 lore raw
Quoted 2 lines · Geoff Levand

=20

Quoted 1 line

The dependent series is currently split per file. =A0 While this makes =

it=20

Quoted 2 lines
Quoted 2 lines

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

Quoted 1 line
Quoted 1 line

git bisect is frowned upon, so it will need to be re-split for merge. =

=A0

Quoted 6 lines
Quoted 1 line

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