[commit] r5319 - trunk/src/arch/i386/include/arch

[repository service]

Author: stepan
Date: Tue Mar 30 00:05:26 2010
New Revision: 5319
URL: https://tracker.coreboot.org/trac/coreboot/changeset/5319

Log:
- drop GCC 2.x workaround, GCC 2.x is not supported anymore.
- warn if we didn't specify IRQ_SLOT_COUNT in Kconfig
Signed-off-by: Stefan Reinauer <[e-mail]>
Acked-by: Stefan Reinauer <[e-mail]>

Modified:
   trunk/src/arch/i386/include/arch/pirq_routing.h

Modified: trunk/src/arch/i386/include/arch/pirq_routing.h
==============================================================================
--- trunk/src/arch/i386/include/arch/pirq_routing.h	Mon Mar 29 23:56:26 2010	(r5318)
+++ trunk/src/arch/i386/include/arch/pirq_routing.h	Tue Mar 30 00:05:26 2010	(r5319)
@@ -18,9 +18,8 @@
 
 #if defined(CONFIG_IRQ_SLOT_COUNT)
 #define IRQ_SLOTS_COUNT CONFIG_IRQ_SLOT_COUNT
-#elif (__GNUC__ < 3)
-#define IRQ_SLOTS_COUNT 1
 #else
+#warning "No IRQ_SLOT_COUNT in Kconfig."
 #define IRQ_SLOTS_COUNT
 #endif

[Myles Watson]

On Mon, Mar 29, 2010 at 4:05 PM, repository service <[e-mail]>wrote:
> Author: stepan
> Date: Tue Mar 30 00:05:26 2010
> New Revision: 5319
> URL: https://tracker.coreboot.org/trac/coreboot/changeset/5319
>
> Log:
> - drop GCC 2.x workaround, GCC 2.x is not supported anymore.
> - warn if we didn't specify IRQ_SLOT_COUNT in Kconfig
> Signed-off-by: Stefan Reinauer <[e-mail]>
> Acked-by: Stefan Reinauer <[e-mail]>
>
> Modified:
>   trunk/src/arch/i386/include/arch/pirq_routing.h
>
> Modified: trunk/src/arch/i386/include/arch/pirq_routing.h
>
> ==============================================================================
> --- trunk/src/arch/i386/include/arch/pirq_routing.h     Mon Mar 29 23:56:26
> 2010        (r5318)
> +++ trunk/src/arch/i386/include/arch/pirq_routing.h     Tue Mar 30 00:05:26
> 2010        (r5319)
> @@ -18,9 +18,8 @@
>
> #if defined(CONFIG_IRQ_SLOT_COUNT)
>  #define IRQ_SLOTS_COUNT CONFIG_IRQ_SLOT_COUNT
> -#elif (__GNUC__ < 3)
> -#define IRQ_SLOTS_COUNT 1
>  #else
> +#warning "No IRQ_SLOT_COUNT in Kconfig."
>  #define IRQ_SLOTS_COUNT
>  #endif
>
> I think we should get rid of IRQ_SLOTS_COUNT and move the error into
> Kconfig.  The added warning is very similar to one in pirq_routing.c

Thanks,
Myles

[Stefan Reinauer]

On 3/30/10 12:15 AM, Myles Watson wrote:
> Modified: trunk/src/arch/i386/include/arch/pirq_routing.h
>     ==============================================================================
>     --- trunk/src/arch/i386/include/arch/pirq_routing.h     Mon Mar 29
>     23:56:26 2010        (r5318)
>     +++ trunk/src/arch/i386/include/arch/pirq_routing.h     Tue Mar 30
>     00:05:26 2010        (r5319)
>     @@ -18,9 +18,8 @@
>
> #if defined(CONFIG_IRQ_SLOT_COUNT)
>      #define IRQ_SLOTS_COUNT CONFIG_IRQ_SLOT_COUNT
>     -#elif (__GNUC__ < 3)
>     -#define IRQ_SLOTS_COUNT 1
>      #else
>     +#warning "No IRQ_SLOT_COUNT in Kconfig."
>      #define IRQ_SLOTS_COUNT
>      #endif
>
> I think we should get rid of IRQ_SLOTS_COUNT and move the error into
> Kconfig.  The added warning is very similar to one in pirq_routing.c

I like that much better... do you have a patch in mind?

-- 
coresystems GmbH . Brahmsstr. 16 . D-79104 Freiburg i. Br.
      Tel.: [phone] . Fax: [phone]
Email: [e-mail]  . http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg . HRB 7656
Geschäftsführer: Stefan Reinauer . Ust-IdNr.: DE245674866

[Myles Watson]

On Mon, Mar 29, 2010 at 4:35 PM, Stefan Reinauer <[e-mail]>wrote:
> On 3/30/10 12:15 AM, Myles Watson wrote:
>
> Modified: trunk/src/arch/i386/include/arch/pirq_routing.h
> > ==============================================================================
> > --- trunk/src/arch/i386/include/arch/pirq_routing.h     Mon Mar 29
> > 23:56:26 2010        (r5318)
> > +++ trunk/src/arch/i386/include/arch/pirq_routing.h     Tue Mar 30
> > 00:05:26 2010        (r5319)
> > @@ -18,9 +18,8 @@
> >
> > #if defined(CONFIG_IRQ_SLOT_COUNT)
> >  #define IRQ_SLOTS_COUNT CONFIG_IRQ_SLOT_COUNT
> > -#elif (__GNUC__ < 3)
> > -#define IRQ_SLOTS_COUNT 1
> >  #else
> > +#warning "No IRQ_SLOT_COUNT in Kconfig."
> >  #define IRQ_SLOTS_COUNT
> >  #endif
> >
> > I think we should get rid of IRQ_SLOTS_COUNT and move the error into
>
> Kconfig.  The added warning is very similar to one in pirq_routing.c
>
> I like that much better... do you have a patch in mind?

Very fair question.  Not yet.  I sent the mail to remind myself.  I'll send
one soon if you don't beat me to it.

Thanks,
Myles

[Stefan Reinauer]

On 3/30/10 12:35 AM, Stefan Reinauer wrote:
> On 3/30/10 12:15 AM, Myles Watson wrote:
> > Modified: trunk/src/arch/i386/include/arch/pirq_routing.h
> >     ==============================================================================
> >     --- trunk/src/arch/i386/include/arch/pirq_routing.h     Mon Mar
> >     29 23:56:26 2010        (r5318)
> >     +++ trunk/src/arch/i386/include/arch/pirq_routing.h     Tue Mar
> >     30 00:05:26 2010        (r5319)
> >     @@ -18,9 +18,8 @@
> >
> > #if defined(CONFIG_IRQ_SLOT_COUNT)
> >      #define IRQ_SLOTS_COUNT CONFIG_IRQ_SLOT_COUNT
> >     -#elif (__GNUC__ < 3)
> >     -#define IRQ_SLOTS_COUNT 1
> >      #else
> >     +#warning "No IRQ_SLOT_COUNT in Kconfig."
> >      #define IRQ_SLOTS_COUNT
> >      #endif
> >
> > I think we should get rid of IRQ_SLOTS_COUNT and move the error into
> > Kconfig.  The added warning is very similar to one in pirq_routing.c
>
> I like that much better... do you have a patch in mind?

Hm.. how do we error in Kconfig? Is there a way to do that at all? I
think we didn't use this before...

Should we drop the whole block of code in pirq_routing.h and make it an
#error in pirq_routing.c?


-- 
coresystems GmbH . Brahmsstr. 16 . D-79104 Freiburg i. Br.
      Tel.: [phone] . Fax: [phone]
Email: [e-mail]  . http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg . HRB 7656
Geschäftsführer: Stefan Reinauer . Ust-IdNr.: DE245674866

[Myles Watson]

On Mon, Mar 29, 2010 at 4:40 PM, Stefan Reinauer <[e-mail]>wrote:
> On 3/30/10 12:35 AM, Stefan Reinauer wrote:
>
> On 3/30/10 12:15 AM, Myles Watson wrote:
>
> Modified: trunk/src/arch/i386/include/arch/pirq_routing.h
> > ==============================================================================
> > --- trunk/src/arch/i386/include/arch/pirq_routing.h     Mon Mar 29
> > 23:56:26 2010        (r5318)
> > +++ trunk/src/arch/i386/include/arch/pirq_routing.h     Tue Mar 30
> > 00:05:26 2010        (r5319)
> > @@ -18,9 +18,8 @@
> >
> > #if defined(CONFIG_IRQ_SLOT_COUNT)
> >  #define IRQ_SLOTS_COUNT CONFIG_IRQ_SLOT_COUNT
> > -#elif (__GNUC__ < 3)
> > -#define IRQ_SLOTS_COUNT 1
> >  #else
> > +#warning "No IRQ_SLOT_COUNT in Kconfig."
> >  #define IRQ_SLOTS_COUNT
> >  #endif
> >
> > I think we should get rid of IRQ_SLOTS_COUNT and move the error into
>
> Kconfig.  The added warning is very similar to one in pirq_routing.c
>
> I like that much better... do you have a patch in mind?
>
> Hm.. how do we error in Kconfig? Is there a way to do that at all? I think
> we didn't use this before...

I was thinking about making HAVE_PIRQ_TABLE depend on CONFIG_IRQ_SLOT_COUNT

Then when someone ported Coreboot to a board they'd set it correctly.  We
could tell from abuild results how many boards have HAVE_PIRQ_TABLE set and
not CONFIG_IRQ_SLOT_COUNT.
> Should we drop the whole block of code in pirq_routing.h and make it an
> #error in pirq_routing.c?

I guess it depends on how many boards it breaks.

Thanks,
Myles

[Stefan Reinauer]

On 3/30/10 12:43 AM, Myles Watson wrote:
> On Mon, Mar 29, 2010 at 4:40 PM, Stefan Reinauer
> <[e-mail] <mailto:[e-mail]>> wrote:
>
> On 3/30/10 12:35 AM, Stefan Reinauer wrote:
> > On 3/30/10 12:15 AM, Myles Watson wrote:
> > > Modified: trunk/src/arch/i386/include/arch/pirq_routing.h
> > >         ==============================================================================
> > >         --- trunk/src/arch/i386/include/arch/pirq_routing.h     Mon
> > >         Mar 29 23:56:26 2010        (r5318)
> > >         +++ trunk/src/arch/i386/include/arch/pirq_routing.h     Tue
> > >         Mar 30 00:05:26 2010        (r5319)
> > >         @@ -18,9 +18,8 @@
> > >
> > > #if defined(CONFIG_IRQ_SLOT_COUNT)
> > >          #define IRQ_SLOTS_COUNT CONFIG_IRQ_SLOT_COUNT
> > >         -#elif (__GNUC__ < 3)
> > >         -#define IRQ_SLOTS_COUNT 1
> > >          #else
> > >         +#warning "No IRQ_SLOT_COUNT in Kconfig."
> > >          #define IRQ_SLOTS_COUNT
> > >          #endif
> > >
> > > I think we should get rid of IRQ_SLOTS_COUNT and move the error
> > >     into Kconfig.  The added warning is very similar to one in
> > >     pirq_routing.c
> >
> > I like that much better... do you have a patch in mind?
>
> Hm.. how do we error in Kconfig? Is there a way to do that at all?
>     I think we didn't use this before...
>
> I was thinking about making HAVE_PIRQ_TABLE depend on
> CONFIG_IRQ_SLOT_COUNT
>
> Then when someone ported Coreboot to a board they'd set it correctly. 
> We could tell from abuild results how many boards have HAVE_PIRQ_TABLE
> set and not CONFIG_IRQ_SLOT_COUNT.

I'd prefer the build failing if someone sets HAVE_PIRQ_TABLE without
CONFIG_IRQ_SLOT_COUNT (unless we can create CONFIG_IRQ_SLOT_COUNT on the
fly which would be much, much better)

Silently disable HAVE_PIRQ_TABLE if CONFIG_IRQ_SLOT_COUNT is not there
would maybe lead to people enabling HAVE_PIRQ_TABLE and wondering or not
(more likely not) why there is no PIRQ table. At least on ACPI systems,
PIRQ is the untested path anyways..
 
Stefan

-- 
coresystems GmbH . Brahmsstr. 16 . D-79104 Freiburg i. Br.
      Tel.: [phone] . Fax: [phone]
Email: [e-mail]  . http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg . HRB 7656
Geschäftsführer: Stefan Reinauer . Ust-IdNr.: DE245674866

[ron minnich]

The IRQ_SLOTS_COUNT was ONLY needed because of very early bug in gcc 3.

The bug was with things like this:

struct blah {
   int x;
};

struct bee {
   struct blah[0];
};

The idea here is you can initialize the bee struct and the number of
blah structs depends on the initializer, not the declaration.

In the early days of v3 gcc would not handle the initialization
correctly at all. Really, IRQ_SLOTS_COUNT should be something we can
get along without. (at least that's what memory tells me).

Note that on gcc2 it had to be 1, that was the convention; then it
became 0 later.

ron

[Stefan Reinauer]

On 3/30/10 12:44 AM, ron minnich wrote:
> The IRQ_SLOTS_COUNT was ONLY needed because of very early bug in gcc 3.
>
> The bug was with things like this:
>
> struct blah {
>    int x;
> };
>
> struct bee {
>    struct blah[0];
> };
>
> The idea here is you can initialize the bee struct and the number of
> blah structs depends on the initializer, not the declaration.
>
> In the early days of v3 gcc would not handle the initialization
> correctly at all. Really, IRQ_SLOTS_COUNT should be something we can
> get along without. (at least that's what memory tells me).

It became a bit harder when we started to be smart and did this:

...
        32+16*CONFIG_IRQ_SLOT_COUNT,     /* There can be total
CONFIG_IRQ_SLOT_COUNT devices on the bus */
...

in the struct. So without CONFIG_IRQ_SLOT_COUNT, the struct is invalid,
at least on those boards I checked.
> Note that on gcc2 it had to be 1, that was the convention; then it
> became 0 later.

I wonder, how can we completely create pirq_tables dynamically... Is it
that hard?

*                {0x00,(0x01<<3)|0x0, {{0x60, 0xdcf8}, {0x61, 0xdcf8},
{0x62, 0xdcf8}, {0x63, 0x0dcd8}}, 0x0, 0x0}, // PCIe?
                {0x00,(0x02<<3)|0x0, {{0x60, 0xdcf8}, {0x00, 0x0000},
{0x00, 0x0000}, {0x00, 0x00000}}, 0x0, 0x0}, // VGA
                {0x00,(0x1e<<3)|0x0, {{0x61, 0xdcf8}, {0x68, 0xdcf8},
{0x00, 0x0000}, {0x00, 0x00000}}, 0x0, 0x0}, // PCI bridge
                {0x00,(0x1f<<3)|0x0, {{0x62, 0xdcf8}, {0x63, 0xdcd8},
{0x00, 0x0000}, {0x00, 0x00000}}, 0x0, 0x0}, // LPC
                {0x00,(0x1d<<3)|0x0, {{0x6b, 0xdcf8}, {0x63, 0xdcd8},
{0x62, 0xdcf8}, {0x60, 0x0dcf8}}, 0x0, 0x0}, // USB#1
*
How can we decide which interrupts to allow, i.e dcf8, dcd8, ...

I saw some clean bios implementations that just created one PIRQ entry
for every PCI device in the system... I think that should work for us, too.
But how can we find out the mapping...
If we could do this on ACPI boards only, that would be fine, ... ACPI
knows the information, too, but in another format..





-- 
coresystems GmbH . Brahmsstr. 16 . D-79104 Freiburg i. Br.
      Tel.: [phone] . Fax: [phone]
Email: [e-mail]  . http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg . HRB 7656
Geschäftsführer: Stefan Reinauer . Ust-IdNr.: DE245674866

[Patrick Georgi]

Am 30.03.2010 01:05, schrieb Stefan Reinauer:
> I wonder, how can we completely create pirq_tables dynamically... Is it
> that hard?
>
> *                {0x00,(0x01<<3)|0x0, {{0x60, 0xdcf8}, {0x61, 0xdcf8},
> {0x62, 0xdcf8}, {0x63, 0x0dcd8}}, 0x0, 0x0}, // PCIe?
>                 {0x00,(0x02<<3)|0x0, {{0x60, 0xdcf8}, {0x00, 0x0000},
> {0x00, 0x0000}, {0x00, 0x00000}}, 0x0, 0x0}, // VGA
>                 {0x00,(0x1e<<3)|0x0, {{0x61, 0xdcf8}, {0x68, 0xdcf8},
> {0x00, 0x0000}, {0x00, 0x00000}}, 0x0, 0x0}, // PCI bridge
>                 {0x00,(0x1f<<3)|0x0, {{0x62, 0xdcf8}, {0x63, 0xdcd8},
> {0x00, 0x0000}, {0x00, 0x00000}}, 0x0, 0x0}, // LPC
>                 {0x00,(0x1d<<3)|0x0, {{0x6b, 0xdcf8}, {0x63, 0xdcd8},
> {0x62, 0xdcf8}, {0x60, 0x0dcf8}}, 0x0, 0x0}, // USB#1
> *
> How can we decide which interrupts to allow, i.e dcf8, dcd8, ...
>
> I saw some clean bios implementations that just created one PIRQ entry
> for every PCI device in the system... I think that should work for us, too.
> But how can we find out the mapping...
> If we could do this on ACPI boards only, that would be fine, ... ACPI
> knows the information, too, but in another format..
> http://www.coreboot.org/GSoC#Board_config_infrastructure ;-)

Some of the entries have to be scrapped from chipset docs, I think. Once
we're on the PCI bus (which we don't automatically cover at all at this
time), it can be derived from the root bridge's irq routing.

So basically, we need to store some irq data, some chipset dependent,
some mainboard dependent. Is the device tree the right place for the
mainboard dependent parts?


Patrick