Changes for removing IDs from dts, etc. etc.

[ron minnich]

This turned into a pretty big cleanup. However, it reduced (by a small
amount) the amount of code, and complexity, and I like it.

Comments welcome. This one is signed off, though there is a bit of
todo left as noted in the patch.

boots on qemu and alix1c.

thanks

ron

[Corey Osgood]

On Fri, Feb 15, 2008 at 1:23 PM, ron minnich  wrote:
> This turned into a pretty big cleanup. However, it reduced (by a small
> amount) the amount of code, and complexity, and I like it.
>
> Comments welcome. This one is signed off, though there is a bit of
> todo left as noted in the patch.
>
> boots on qemu and alix1c.
>
> thanks
>
> ron

Well, since everyone else seems to be asleep, I'll have a peak.

--- southbridge/intel/i82371eb/i82371eb.c    (revision 600)
> +++ southbridge/intel/i82371eb/i82371eb.c    (working copy)
> @@ -83,7 +83,9 @@
>  }
>
> /* You can override or extend each operation as needed for the device. */
> -static struct device_operations i82371eb_isa_ops_dev = {
> +struct device_operations i82371eb_isa = {
> .id = {.type = DEVICE_ID_PCI,
>    .u = {.pci = {.vendor = 0x8086,.device = 0x7000}}},

sweet, but why no longer static?

Other than that, this looks good, but I'd hold off on committing a little
bit, to give someone a chance to nack.

Acked-by: Corey Osgood

[ron minnich]

On Feb 15, 2008 3:41 PM, Corey Osgood  wrote:
> > --- southbridge/intel/i82371eb/i82371eb.c    (revision 600)
> > +++ southbridge/intel/i82371eb/i82371eb.c    (working copy)
> > @@ -83,7 +83,9 @@
> >  }
> >
> > /* You can override or extend each operation as needed for the device. */
> > -static struct device_operations i82371eb_isa_ops_dev = {
> > +struct device_operations i82371eb_isa = {
> > .id = {.type = DEVICE_ID_PCI,
> >    .u = {.pci = {.vendor = 0x8086,.device = 0x7000}}},

because it has to be visible in statictree.c. Before, the constructor
struct was visible, but that is gone. So these are visible instead.

ron

[Carl-Daniel Hailfinger]

On 15.02.2008 19:23, ron minnich wrote:
> This turned into a pretty big cleanup. However, it reduced (by a small
> amount) the amount of code, and complexity, and I like it.
>
> Comments welcome. This one is signed off, though there is a bit of
> todo left as noted in the patch.
>
> boots on qemu and alix1c.

Very nice.
> TODO:
> final removal of support for/use of 'constructor' property, replace with
>
> 'device_operations' property

Your decision whether to do this now or later.
> change lpc back to superio

Please do so before commit if at all possible. I'm not sure the
"superio" keyword is the right one, but keeping that change out of the
commit helps reduce code churn.
> Change limitation in dtc that makes it hard to use hex in pci@ notation.

Can wait for a later patch.
> whatever else you tell me to do :-)

Two questions about superio/lpc specification in the dts, though.
- If a southbridge requires special setup before the superio is
accessible, where do we handle that?
- A superio is attached via LPC, but there is no way to be sure that
future superios will be addressed via I/O Ports. Do we want to change
the superio part of the dts to something like this:

===================================================================
--- mainboard/pcengines/alix1c/dts	(revision 600)
+++ mainboard/pcengines/alix1c/dts	(working copy)
@@ -54,7 +45,7 @@
    		 * See virtual PIC spec. */
    		enable_gpio_int_route = "0x0D0C0700";
    	};
-		superio {
+		ioport@46 {
    		/config/("superio/winbond/w83627hf/dts");
    		com1enable = "1";
    	};
> Signed-off-by: Ronald G. Minnich

With point 2 from the TODO fixed or an answer for the superio/lpc/ioport
problem, the patch is
Acked-by: Carl-Daniel Hailfinger

Thanks for doing that work! The code/dts structure is much cleaner and
more obvious now.

Regards,
Carl-Daniel

[ron minnich]

OK, Commit R603.

Bad news is something broke! But it broke BEFORE my commit. Some bit
of the cleanup that happened today broke alix1c -- nothing else, just
alix1c.

I am going to try to work my way back, to a working version. I know
R593 was clean -- well, I think it was ...

If anyone else can take a look at the geode commits and see what might
have broken I/O, I'd be grateful.

I would not have done this commit, save for one thing: I tried a clean
checkout of r602 and it failed REALLY badly. It exploded, in fact,
before getting done stage2. That's why I suspect today's cleanup.

Once we get this target fixed, we're going to have to start being
super careful. We have a working target now :-)

Failed log attached.

ron

[Stefan Reinauer]

* ron minnich  [080215 19:23]:
> Signed-off-by: Ronald G. Minnich
> --- include/device/device.h	(revision 600)
> +++ include/device/device.h	(working copy)
> @@ -195,6 +195,9 @@
>    struct device *	next;		/* chain of all devices */
>
> struct device_path path;
> /* note there is a device id maintained here. This covers the special case
> * of default_device_operations, which has an id of zero. 
> */

Uh, when exactly is that special case used?
> @@ -268,7 +271,7 @@
>  struct device * dev_find_class (unsigned int class, struct device * from);
>  struct device * dev_find_slot (unsigned int bus, unsigned int devfn);
>  struct device * dev_find_slot_on_smbus (unsigned int bus, unsigned int addr);
> -void default_device_constructor(struct device *dev, struct constructor *constructor);
> +void default_device_constructor(struct device *dev, struct device_operations *constructor);

default_device_operations?

I think this is the way to go.

Acked-by: Stefan Reinauer