Re: [coreboot] [PATCH] Proposal for dealing with superio *.c includes

[Keith Hui]

> Option C with GPLv2+ licensing.

A few quick things:

SUPERIO_FINTEK_F81865F_HAS_EARLY_SERIAL seems long-winded for a name.

I prefer to define CONFIG_SUPERIO_HAS_EARLY_SERIAL (or even just
CONFIG_HAS_EARLY_SERIAL) within the superio Kconfig and not in
mainboard Kconfig. This I think would be a per-superio setting. While
we're at it, have we seen a board with 2+ superios?

Having said that though, isn't early serial support sort of mandatory anyway?

Secretly I'm hoping that some of my ideas end up in here. :-)

Cheers
Keith

[PATCH] Proposal for dealing with superio *.c includes

[Alex G.]

On 03/03/2011 01:01 AM, Keith Hui wrote:
> > Option C with GPLv2+ licensing.
>
> A few quick things:
>
> SUPERIO_FINTEK_F81865F_HAS_EARLY_SERIAL seems long-winded for a name.
>
> I prefer to define CONFIG_SUPERIO_HAS_EARLY_SERIAL (or even just
> CONFIG_HAS_EARLY_SERIAL) within the superio Kconfig and not in
> mainboard Kconfig. This I think would be a per-superio setting. While
> we're at it, have we seen a board with 2+ superios?

The idea was that, if a board has more than one superio, we would have
to select which of those carries the early serial. Let's say board
PETER40S has two superios: an KEITH875H and an ALEX666G. Both have early
serial capability, but you have to specify which of those will be used
for serial purpose.

So you have:
    select SUPERIO_KEITH875H
    select SUPERIO_ALEX666G
    select SUPERIO_ALEX666G_HAS_EARLY_SERIAL
> Having said that though, isn't early serial support sort of mandatory anyway?

The point is not to specify if a superio has early serial. It is to
specify which of the different superio's early serial will get compiled
in, and used.

Alex

[PATCH] Proposal for dealing with superio *.c includes

[Georgi, Patrick]

Am Donnerstag, den 03.03.2011, 01:06 +0200 schrieb Alex G.:
> So you have:
>    select SUPERIO_KEITH875H
>    select SUPERIO_ALEX666G
>    select SUPERIO_ALEX666G_HAS_EARLY_SERIAL
> > Having said that though, isn't early serial support sort of mandatory anyway?
>
> The point is not to specify if a superio has early serial. It is to
> specify which of the different superio's early serial will get compiled
> in, and used.

The chipset components in Kconfig could be derived from the
devicetree.cb (statically, on config time or before) - this would
simplify board config a bit by reducing duplication.

Early serial could be managed with a new keyword there
("chip superio/foo/bar early")

Thoughts?

Patrick

[PATCH] Proposal for dealing with superio *.c includes

[Sven Schnelle]

Hi Keith,

Keith Hui  writes:
> > Option C with GPLv2+ licensing.
>
> A few quick things:
>
> SUPERIO_FINTEK_F81865F_HAS_EARLY_SERIAL seems long-winded for a name.
>
> I prefer to define CONFIG_SUPERIO_HAS_EARLY_SERIAL (or even just
> CONFIG_HAS_EARLY_SERIAL) within the superio Kconfig and not in
> mainboard Kconfig. This I think would be a per-superio setting. While
> we're at it, have we seen a board with 2+ superios?

Depends on what the definition of 'one Board' is. :) One example of
having more than one superio is the Lenovo X60/T60 - both have one
superio in the Laptop, and one in the Docking station. You can't
use the serial Port in the Laptop for an early console without
soldering, but that's one practically example of having more
than one superio.

I would prefer to add such settings to devicetree.cb - Kconfig isn't the
right right place for it IMHO, and isn't flexible enough.

Sven.

[PATCH] Proposal for dealing with superio *.c includes

[Alex G.]

On 03/03/2011 09:35 AM, Georgi, Patrick wrote:
> The chipset components in Kconfig could be derived from the
> devicetree.cb (statically, on config time or before) - this would
> simplify board config a bit by reducing duplication.
>
> Early serial could be managed with a new keyword there
> ("chip superio/foo/bar early")
>
> Thoughts?

I like the idea. It's long term, and cleaner.

chip superio/foo/bar		# Super I/O
              device pnp 2e.1 on		# Com1
                io 0x60 = 0x3f8
                irq 0x70 = 4
                serial_console
              end

So not only would we specify that the superio hosts the serial console,
but also at which address that is, eliminating the need for
#define SERIAL_DEV PNP_DEV(0x2e, FOOBAR_SP1)
in romstage.c

+1

Alex

[PATCH] Proposal for dealing with superio *.c includes

[Keith Hui]

I like this solution too. Just that it requires hacking sconfig, and
I'm not even close to qualified to actually do it. :)

And this requires sconfig to produce some other output for romstage as
well. The hardware tree it produces is currently only used during
ramstage.

While we're on sconfig, devicetree, superio and romstage, can we add a
facility where I can declare some known, fixed initialization
sequences that needs to be programmed into certain devices?

Suppose my new board requires 0x00 to be written into pnp device 2e.5,
registers 0xf0-0xf5. Can we make it so that it will accept this
declaration, and do it during romstage?

If I need to do it now, I'll need to hijack either of io and drq for
it and can only do it when pnp is initialized in ramstage.

chip superio/foo/bar                # Super I/O
              device pnp 2e.5 on                # PME
                reg 0xf0 = 0x00
                reg 0xf1 = 0x00
                reg 0xf2 = 0x00
                reg 0xf3 = 0x00
                reg 0xf4 = 0x00
                reg 0xf5 = 0x00
              end

Thanks

On Thu, Mar 3, 2011 at 3:40 AM, Alex G.  wrote:
> On 03/03/2011 09:35 AM, Georgi, Patrick wrote:
> > The chipset components in Kconfig could be derived from the
> > devicetree.cb (statically, on config time or before) - this would
> > simplify board config a bit by reducing duplication.
> >
> > Early serial could be managed with a new keyword there
> > ("chip superio/foo/bar early")
> >
> > Thoughts?
>
> I like the idea. It's long term, and cleaner.
>
> chip superio/foo/bar                # Super I/O
>              device pnp 2e.1 on                # Com1
>                io 0x60 = 0x3f8
>                irq 0x70 = 4
>                serial_console
>              end
>
> So not only would we specify that the superio hosts the serial console,
> but also at which address that is, eliminating the need for
> #define SERIAL_DEV PNP_DEV(0x2e, FOOBAR_SP1)
> in romstage.c
>
> +1
>
> Alex

[PATCH] Proposal for dealing with superio *.c includes

[Alex G.]

On 03/03/2011 04:49 PM, Keith Hui wrote:
> I like this solution too. Just that it requires hacking sconfig, and
> I'm not even close to qualified to actually do it. :)
>
> And this requires sconfig to produce some other output for romstage as
> well. The hardware tree it produces is currently only used during
> ramstage.
>
> While we're on sconfig, devicetree, superio and romstage, can we add a
> facility where I can declare some known, fixed initialization
> sequences that needs to be programmed into certain devices?

And have the same sequence in every devicetree.cb that uses that device?
That's code duplication, which is exactly what we are trying to avoid.
Imagine if we find a problem with a superio used in 30 boards. Instead
of changing one file, we have 30 devicetree.cb .
> Suppose my new board requires 0x00 to be written into pnp device 2e.5,
> registers 0xf0-0xf5. Can we make it so that it will accept this
> declaration, and do it during romstage?

On second thought, it might be a good idea if and only if your board
requires a _different_ sequence, and anything in devicetree.cb would
override the default one.
> If I need to do it now, I'll need to hijack either of io and drq for
> it and can only do it when pnp is initialized in ramstage.
>         chip superio/foo/bar                # Super I/O
>           device pnp 2e.5 on                # PME
>             reg 0xf0 = 0x00
>             reg 0xf1 = 0x00
>             reg 0xf2 = 0x00
>             reg 0xf3 = 0x00
>             reg 0xf4 = 0x00
>             reg 0xf5 = 0x00
>           end

I'm not seeing any generic "reg" infrastructure. It doesn't look that
hard to extend though.

However, devicetree.cb _seems_ to deal exclusively with ramstage. Not
too sure how we can extend that to romstage.

Alex