first output: dbe62 and v3

[ron minnich]

attached. It dies and I'm sure it is bad ram settings, but it's a start.

One thing I plan to change pretty soon: power button control can move
from stage1 to stage2, and power button control will be managed in dts
(another dts setting we would NOT want visible in Kconfig; you could
render a board totally unbootable and unrepairable with the wrong
setting. Kconfig should be for settings that are safe to change).

There are other things in stage1 for the lx that we should try to move
to stage 2:
        cs5536_setup_extmsr();
        cs5536_setup_cis_mode();

msr = rdmsr(GLCP_SYS_RSTPLL);
        if (msr.lo & (0x3f << 26)) {
                /* PLL is already set and we are reboot from PLL reset. */
                return;
        }

cs5536_setup_idsel();
        cs5536_usb_swapsif();
        cs5536_setup_iobase();
        cs5536_setup_smbus_gpio();
        /* cs5536_enable_smbus(); -- Leave this out for now. */
//      cs5536_setup_power_button();

how much of this do we want managed in stage1? How much is unchanging,
board to board? How much MUST be there? Really, a lot is going on;
what can we leave to stage 2. Power button for sure; what else?

And how do we parameterize this? Just pass in a set of booleans for
those things we wish to control (e.g. smbus).

At this rate, carl-daniel, we may soon have another board for your list.

thanks

ron
p.s. thanks to artecgroup for such great product design and support.

[PATCH] more fake SPD debugging

[Carl-Daniel Hailfinger]

On 05.03.2008 07:23, ron minnich wrote:
> attached. It dies and I'm sure it is bad ram settings, but it's a start.

Yes. Try the following patch.

Add debugging to fake SPD read functions. We want to know when we try to 
read from a nonexisting member of the fake SPD.

Signed-off-by: Carl-Daniel Hailfinger

Index: LinuxBIOSv3-spd_debug/mainboard/artecgroup/dbe61/initram.c
===================================================================
--- LinuxBIOSv3-spd_debug/mainboard/artecgroup/dbe61/initram.c	(Revision 631)
+++ LinuxBIOSv3-spd_debug/mainboard/artecgroup/dbe61/initram.c	(Arbeitskopie)
@@ -97,7 +97,7 @@
    /* returns 0xFF on any failures */
    u8 ret = 0xff;

-	printk(BIOS_DEBUG, "spd_read_byte dev %04x\n", device);
+	printk(BIOS_DEBUG, "spd_read_byte dev %04x", device);
    if (device == DIMM0) {
    	for (i = 0; i < ARRAY_SIZE(spd_table); i++) {
    		if (spd_table[i].address == address) {
@@ -105,6 +105,9 @@
    		}
    	}
    }
+	if (i == ARRAY_SIZE(spd_table))
+		printk(BIOS_DEBUG, " addr %02x does not exist in SPD table",
+			address);

printk(BIOS_DEBUG, " addr %02x returns %02x\n", address, ret);
    return ret;
Index: LinuxBIOSv3-spd_debug/mainboard/artecgroup/dbe62/initram.c
===================================================================
--- LinuxBIOSv3-spd_debug/mainboard/artecgroup/dbe62/initram.c	(Revision 631)
+++ LinuxBIOSv3-spd_debug/mainboard/artecgroup/dbe62/initram.c	(Arbeitskopie)
@@ -87,7 +87,7 @@
    /* returns 0xFF on any failures */
    u8 ret = 0xff;

-	printk(BIOS_DEBUG, "spd_read_byte dev %04x\n", device);
+	printk(BIOS_DEBUG, "spd_read_byte dev %04x", device);
    if (device == DIMM0) {
    	for (i = 0; i < ARRAY_SIZE(spd_table); i++) {
    		if (spd_table[i].address == address) {
@@ -95,6 +95,9 @@
    		}
    	}
    }
+	if (i == ARRAY_SIZE(spd_table))
+		printk(BIOS_DEBUG, " addr %02x does not exist in SPD table",
+			address);

printk(BIOS_DEBUG, " addr %02x returns %02x\n", address, ret);
    return ret;
Index: LinuxBIOSv3-spd_debug/mainboard/pcengines/alix1c/initram.c
===================================================================
--- LinuxBIOSv3-spd_debug/mainboard/pcengines/alix1c/initram.c	(Revision 631)
+++ LinuxBIOSv3-spd_debug/mainboard/pcengines/alix1c/initram.c	(Arbeitskopie)
@@ -97,7 +97,7 @@
    /* returns 0xFF on any failures */
    u8 ret = 0xff;

-	printk(BIOS_DEBUG, "spd_read_byte dev %04x\n", device);
+	printk(BIOS_DEBUG, "spd_read_byte dev %04x", device);
    if (device == DIMM0) {
    	for (i = 0; i < ARRAY_SIZE(spd_table); i++) {
    		if (spd_table[i].address == address) {
@@ -105,6 +105,9 @@
    		}
    	}
    }
+	if (i == ARRAY_SIZE(spd_table))
+		printk(BIOS_DEBUG, " addr %02x does not exist in SPD table",
+			address);

printk(BIOS_DEBUG, " addr %02x returns %02x\n", address, ret);
    return ret;

[PATCH] more fake SPD debugging

[ron minnich]

Tested and working on both dbe62 and alix1c. I can't commit as I have
other changes pending.

Acked-by: Ronald G. Minnich

On Wed, Mar 5, 2008 at 5:38 AM, Carl-Daniel Hailfinger
 wrote:
> On 05.03.2008 07:23, ron minnich wrote:
> > attached. It dies and I'm sure it is bad ram settings, but it's a start.
>
> Yes. Try the following patch.
>
> Add debugging to fake SPD read functions. We want to know when we try to
>  read from a nonexisting member of the fake SPD.
>
> Signed-off-by: Carl-Daniel Hailfinger
> Index: LinuxBIOSv3-spd_debug/mainboard/artecgroup/dbe61/initram.c
>
> --- LinuxBIOSv3-spd_debug/mainboard/artecgroup/dbe61/initram.c  (Revision 631)
>  +++ LinuxBIOSv3-spd_debug/mainboard/artecgroup/dbe61/initram.c  (Arbeitskopie)
>  @@ -97,7 +97,7 @@
>         /* returns 0xFF on any failures */
>         u8 ret = 0xff;
>   printk(BIOS_DEBUG, "spd_read_byte dev %04x\n", device);
>   printk(BIOS_DEBUG, "spd_read_byte dev %04x", device);
>  if (device == DIMM0) {
>          for (i = 0; i < ARRAY_SIZE(spd_table); i++) {
>                  if (spd_table[i].address == address) {
>
> @@ -105,6 +105,9 @@
>                         }
>                 }
>         }
>   if (i == ARRAY_SIZE(spd_table))
>           printk(BIOS_DEBUG, " addr %02x does not exist in SPD table",
>                   address);
>
>  printk(BIOS_DEBUG, " addr %02x returns %02x\n", address, ret);
>  return ret;
> Index: LinuxBIOSv3-spd_debug/mainboard/artecgroup/dbe62/initram.c
>
> --- LinuxBIOSv3-spd_debug/mainboard/artecgroup/dbe62/initram.c  (Revision 631)
>  +++ LinuxBIOSv3-spd_debug/mainboard/artecgroup/dbe62/initram.c  (Arbeitskopie)
>  @@ -87,7 +87,7 @@
>         /* returns 0xFF on any failures */
>         u8 ret = 0xff;
>   printk(BIOS_DEBUG, "spd_read_byte dev %04x\n", device);
>   printk(BIOS_DEBUG, "spd_read_byte dev %04x", device);
>  if (device == DIMM0) {
>          for (i = 0; i < ARRAY_SIZE(spd_table); i++) {
>                  if (spd_table[i].address == address) {
>
> @@ -95,6 +95,9 @@
>                         }
>                 }
>         }
>   if (i == ARRAY_SIZE(spd_table))
>           printk(BIOS_DEBUG, " addr %02x does not exist in SPD table",
>                   address);
>
>  printk(BIOS_DEBUG, " addr %02x returns %02x\n", address, ret);
>  return ret;
> Index: LinuxBIOSv3-spd_debug/mainboard/pcengines/alix1c/initram.c
>
> --- LinuxBIOSv3-spd_debug/mainboard/pcengines/alix1c/initram.c  (Revision 631)
>  +++ LinuxBIOSv3-spd_debug/mainboard/pcengines/alix1c/initram.c  (Arbeitskopie)
>  @@ -97,7 +97,7 @@
>         /* returns 0xFF on any failures */
>         u8 ret = 0xff;
>   printk(BIOS_DEBUG, "spd_read_byte dev %04x\n", device);
>   printk(BIOS_DEBUG, "spd_read_byte dev %04x", device);
>  if (device == DIMM0) {
>          for (i = 0; i < ARRAY_SIZE(spd_table); i++) {
>                  if (spd_table[i].address == address) {
>
> @@ -105,6 +105,9 @@
>                         }
>                 }
>         }
>   if (i == ARRAY_SIZE(spd_table))
>           printk(BIOS_DEBUG, " addr %02x does not exist in SPD table",
>                   address);
>
>  printk(BIOS_DEBUG, " addr %02x returns %02x\n", address, ret);
>  return ret;

[PATCH] more fake SPD debugging

[ron minnich]

On Wed, Mar 5, 2008 at 7:07 PM, ron minnich  wrote:
> Tested and working on both dbe62 and alix1c. I can't commit as I have
>  other changes pending.
>
> Acked-by: Ronald G. Minnich

Let me retract this ack until I fix the code. Don't commit it.

Thanks

ron

[Carl-Daniel Hailfinger]

On 05.03.2008 07:23, ron minnich wrote:
> One thing I plan to change pretty soon: power button control can move
> from stage1 to stage2,

That depends on your goals. We once discussed how to fail over to a 
initram/stage2/payload downloaded via serial. That can take a lot of 
time. I seem to remember that some power button control on some Geode 
boards has to be done in the first 4 seconds after poweron. That timing 
constraint will bite us when downloading over serial. Of course, if I'm 
wrong (or we can perform failsafe powerbutton control in stage 1), 
discard my objection.
> and power button control will be managed in dts
> (another dts setting we would NOT want visible in Kconfig; you could
> render a board totally unbootable and unrepairable with the wrong
> setting. Kconfig should be for settings that are safe to change).

We currently abuse Kconfig for settings which should be available 
before/outside stage2, iow when the device tree is not available. 
Depending on our design stategy, we may want to generate a subset of the 
device tree as #defines suitable for inclusion outside stage2.
> There are other things in stage1 for the lx that we should try to move
> to stage 2:
>         cs5536_setup_extmsr();
>         cs5536_setup_cis_mode();
>     msr = rdmsr(GLCP_SYS_RSTPLL);
>     if (msr.lo & (0x3f << 26)) {
>             /* PLL is already set and we are reboot from PLL reset. */
>             return;
>     }
>
>     cs5536_setup_idsel();
>     cs5536_usb_swapsif();
>     cs5536_setup_iobase();
>     cs5536_setup_smbus_gpio();
>     /* cs5536_enable_smbus(); -- Leave this out for now. */
>
> //      cs5536_setup_power_button();
>
> how much of this do we want managed in stage1? How much is unchanging,
> board to board? How much MUST be there? Really, a lot is going on;
> what can we leave to stage 2. Power button for sure; what else?

I'd say anything which is neither time-critical (in the sense of time 
after poweron) nor essential for emergency initram+stage2 downloads over 
serial/usbdebug nor a ROM access/execution speedup can move to stage2 
phase 1.
> And how do we parameterize this? Just pass in a set of booleans for
> those things we wish to control (e.g. smbus).

See my device tree subset generation idea above.
> At this rate, carl-daniel, we may soon have another board for your list.

Nice.

Regards,
Carl-Daniel

[ron minnich]

On Wed, Mar 5, 2008 at 5:49 AM, Carl-Daniel Hailfinger
 wrote:
> That depends on your goals. We once discussed how to fail over to a
>  initram/stage2/payload downloaded via serial. That can take a lot of
>  time. I seem to remember that some power button control on some Geode
>  boards has to be done in the first 4 seconds after poweron. That timing
>  constraint will bite us when downloading over serial. Of course, if I'm
>  wrong (or we can perform failsafe powerbutton control in stage 1),
>  discard my objection.

right, but the enable ENABLES the timeout. Safest thing to do is wait
until stage 2 at least to enable such things. I.e. later.

Although I see your point now. Sigh. mainboard_pre_payload?

We're finding that the device tree needs to span stage1 to
mainboard_pre_payload, now. We had not planned for this. We're going
to have to rethink it a bit.

If we do this it will clean a lot of things up.

Good work for denver.

ron

[Carl-Daniel Hailfinger]

On 05.03.2008 16:58, ron minnich wrote:
> On Wed, Mar 5, 2008 at 5:49 AM, Carl-Daniel Hailfinger
>  wrote:
> > That depends on your goals. We once discussed how to fail over to a
> >  initram/stage2/payload downloaded via serial. That can take a lot of
> >  time. I seem to remember that some power button control on some Geode
> >  boards has to be done in the first 4 seconds after poweron. That timing
> >  constraint will bite us when downloading over serial. Of course, if I'm
> >  wrong (or we can perform failsafe powerbutton control in stage 1),
> >  discard my objection.
>
> right, but the enable ENABLES the timeout. Safest thing to do is wait
> until stage 2 at least to enable such things. I.e. later.

OK, then I'm totally fine doing all the powerbutton handling in stage2.
> Although I see your point now. Sigh. mainboard_pre_payload?

I'd simply decide on a phase in stage2 where all this happens (except 
time-critical stuff).
> We're finding that the device tree needs to span stage1 to
> mainboard_pre_payload, now. We had not planned for this. We're going
> to have to rethink it a bit.

See my other mail from 5 minutes ago for a possible solution for that 
problem.
> If we do this it will clean a lot of things up.

And the bootblock will grow a lot. I'd still like to avoid device tree 
handling in stage 1 and initram.
> Good work for denver.

Speaking of Denver, how many people have confirmed to come?

Regards,
Carl-Daniel

[ron minnich]

On Wed, Mar 5, 2008 at 8:06 AM, Carl-Daniel Hailfinger
 wrote:
> Speaking of Denver, how many people have confirmed to come?

I will have to look. The checkbox for "linuxbios summit" was not
obvious to many people, so I don't have a good count.

It's amazing how hard it is to write a good web page. It makes me even
happier that the coreboot web page is so good.

ron

[Marc Jones]

ron minnich wrote:
> attached. It dies and I'm sure it is bad ram settings, but it's a start.
>
> One thing I plan to change pretty soon: power button control can move
> from stage1 to stage2, and power button control will be managed in dts
> (another dts setting we would NOT want visible in Kconfig; you could
> render a board totally unbootable and unrepairable with the wrong
> setting. Kconfig should be for settings that are safe to change).

This would be great. It should be easy to add a dts item and call 
cs5536_setup_power_button or not. Moving it to stage 2 should be fine 
BUT until the power button is setup it won't work. Some people might not 
like that if you hang or delay somewhere prior to it being setup.

You might be able to move a few of the following but most are needed to 
do the ram init and the others are a chicken/egg problem.
> There are other things in stage1 for the lx that we should try to move
> to stage 2:
>         cs5536_setup_extmsr();

cs5536_setup_extmsr() can't be moved. It is required to write the MSR in 
the 5536 which is neeeded to setup the SMBus for RAM init.
>     cs5536_setup_cis_mode();

These are the sideband signals from the 5536 to the LX. These include 
int, smi, susp, and some others. You could move it BUT if you setup the 
IRQ in the uart the 5536 could do an int and then you would miss it. 
Doing it early avoids issues.
>     msr = rdmsr(GLCP_SYS_RSTPLL);
>     if (msr.lo & (0x3f << 26)) {
>             /* PLL is already set and we are reboot from PLL reset. */
>             return;
>     }
>
>     cs5536_setup_idsel();

Need this to do stage2. Without it the 5536 won't decode PCI.
>     cs5536_usb_swapsif();

This can move but needs to be done before any the other usbsetup.
>     cs5536_setup_iobase();

This is required before raminit. Specificly the smbus io bar but if you 
set one you should probably just set them all. Once VSA loads these 
settings are reflected in the BAR and then are changed during PCI 
enumeration.
>     cs5536_setup_smbus_gpio();

This is required to set SMBUS sda/scl lines. There is a question how 
common this is in the function header. I have never seen these gpios 
used for anything else. Their purpose is to be the smbus lines.
>     /* cs5536_enable_smbus(); -- Leave this out for now. */

I just noticed that this code moved to smbus_initram which isn't really 
correct. SMBus is important for reading the SPD but it isn't the only 
device on the SMBus. Even if you don't have an SPD you still might like 
SMBus setup (thermal, gpio expander, etc). If you need SMBus for RAM 
init it needs to be done early, if not it can be done in stage2 but I 
think that it being in two places would be confusing.
> //      cs5536_setup_power_button();

Discussed it above.

Marc

[Marc Jones]

Marc Jones wrote:
> ron minnich wrote:
> > attached. It dies and I'm sure it is bad ram settings, but it's a start.
> >
> > One thing I plan to change pretty soon: power button control can move
> > from stage1 to stage2, and power button control will be managed in dts
> > (another dts setting we would NOT want visible in Kconfig; you could
> > render a board totally unbootable and unrepairable with the wrong
> > setting. Kconfig should be for settings that are safe to change).
>
> This would be great. It should be easy to add a dts item and call 
> cs5536_setup_power_button or not. Moving it to stage 2 should be fine 
> BUT until the power button is setup it won't work. Some people might not 
> like that if you hang or delay somewhere prior to it being setup.
>
> You might be able to move a few of the following but most are needed to 
> do the ram init and the others are a chicken/egg problem.

Thinking about this further... isn't this a problem for the dts since it 
is pre static tree?  The simple way to handle this is to move the 
cs5536_setup_power_button call from 5536\stage1.c to the mainboard 
hardware_stage1().

Yes, the dts and kconfig are great for setting things up but sometimes 
platform code is easier.

Marc

[Carl-Daniel Hailfinger]

On 05.03.2008 18:13, Marc Jones wrote:
> ron minnich wrote:
> > One thing I plan to change pretty soon: power button control can move
> > from stage1 to stage2, and power button control will be managed in dts
> > (another dts setting we would NOT want visible in Kconfig; you could
> > render a board totally unbootable and unrepairable with the wrong
> > setting. Kconfig should be for settings that are safe to change).
>
> This would be great. It should be easy to add a dts item and call 
> cs5536_setup_power_button or not. Moving it to stage 2 should be fine 
> BUT until the power button is setup it won't work. Some people might not 
> like that if you hang or delay somewhere prior to it being setup.

Can we set up the power button in a way that makes it work always, then 
change the setup in stage2?
> >     msr = rdmsr(GLCP_SYS_RSTPLL);
> >     if (msr.lo & (0x3f << 26)) {
> >             /* PLL is already set and we are reboot from PLL reset. */
> >             return;
> >     }
> >
> >     cs5536_setup_idsel();
>
> Need this to do stage2. Without it the 5536 won't decode PCI.

Did you mean stage1?

Regards,
Carl-Daniel

[ron minnich]

On Wed, Mar 5, 2008 at 9:48 AM, Carl-Daniel Hailfinger
 wrote:
> Can we set up the power button in a way that makes it work always, then
>  change the setup in stage2?

you really don't want that button working pre stage2, because, if the
button is hardwired on, the machine turns off in 4 seconds :-)

So you don't want to "default enable" it. You want to "don't do
ANYTHING until you are sure". right now code is default enable, and I
had to comment that out to get dbe62 up.
> > Need this to do stage2. Without it the 5536 won't decode PCI.
>
> Did you mean stage1?

I don't think the 5536 is visible until you do this, i.e. is not
addressable? Is that what you meant Marc?

I am going to take this slowly. I am going to move power button to
stage 2, and make it a standard part of cs5536 bringup in stage 2. No
harm is done by delaying the enable of the power button, and I already
know lots of harm is done by enabling the power button too early.
Patch later tonight.

ron

[Peter Stuge]

On Wed, Mar 05, 2008 at 10:28:57AM -0700, Marc Jones wrote:
> > This would be great. It should be easy to add a dts item and call
> > cs5536_setup_power_button or not.

Yes.
> The simple way to handle this is to move the
> cs5536_setup_power_button call from 5536\stage1.c to the mainboard
> hardware_stage1().

Better to eliminate mainboard code than add more.
> Yes, the dts and kconfig are great for setting things up but
> sometimes platform code is easier.

Then dts and Kconfig are not working well enough.

They (dts really) should be used for all hardware parameters and
behavior description. DisTilled netliSt. :p

The actual code should be in nobr/amd/geodelx or sobr/amd/cs5536
whichever is closer to the truth.

The code should use information entered into the dts.

This is exactly what the dts is for - describing how two mainboards
with the same devices are configured differently.

On Wed, Mar 05, 2008 at 09:57:42AM -0800, ron minnich wrote:
> you really don't want that button working pre stage2, because, if
> the button is hardwired on, the machine turns off in 4 seconds :-)

Agreed.
> So you don't want to "default enable" it. You want to "don't do
> ANYTHING until you are sure". right now code is default enable,
> and I had to comment that out to get dbe62 up.

It used to bite on the alix1c too.

//Peter

[Marc Jones]

Carl-Daniel Hailfinger wrote:
> On 05.03.2008 18:13, Marc Jones wrote:
> > ron minnich wrote:
> > > One thing I plan to change pretty soon: power button control can move
> > > from stage1 to stage2, and power button control will be managed in dts
> > > (another dts setting we would NOT want visible in Kconfig; you could
> > > render a board totally unbootable and unrepairable with the wrong
> > > setting. Kconfig should be for settings that are safe to change).
> >
> > This would be great. It should be easy to add a dts item and call 
> > cs5536_setup_power_button or not. Moving it to stage 2 should be fine 
> > BUT until the power button is setup it won't work. Some people might not 
> > like that if you hang or delay somewhere prior to it being setup.
>
> Can we set up the power button in a way that makes it work always, then 
> change the setup in stage2?

That is the problem. For systems that don't have a powerbutton, enabling 
the button logic will cause a reset in 4 seconds which would be fine if 
you could gaurnettee that it would be reprogrammed by then but that is 
just the oposite problem described above. What if there is a delay prior 
to rprograming it?
> > >     msr = rdmsr(GLCP_SYS_RSTPLL);
> > >     if (msr.lo & (0x3f << 26)) {
> > >             /* PLL is already set and we are reboot from PLL reset. */
> > >             return;
> > >     }
> > >
> > >     cs5536_setup_idsel();
> >
> > Need this to do stage2. Without it the 5536 won't decode PCI.
>
> Did you mean stage1?

You need to do it in stage1 so you can do stage2 (stage 2 needs to find 
the device before it can program it).

Marc

[ron minnich]

On Wed, Mar 5, 2008 at 9:59 AM, Marc Jones  wrote:
> > > Need this to do stage2. Without it the 5536 won't decode PCI.
> >
> > Did you mean stage1?
>
> You need to do it in stage1 so you can do stage2 (stage 2 needs to find
>  the device before it can program it).

so we don't need this in stage 1? then we can do it in stage  1 phase
2, which is designed specifically for this type of operation: doing
something before you try to scan pci.

Thanks
ron

[ron minnich]

On Wed, Mar 5, 2008 at 11:33 AM, ron minnich  wrote:
> On Wed, Mar 5, 2008 at 9:59 AM, Marc Jones  wrote:
> > > > Need this to do stage2. Without it the 5536 won't decode PCI.
> > >
> > > Did you mean stage1?
> >
> > You need to do it in stage1 so you can do stage2 (stage 2 needs to find
> >  the device before it can program it).
>
> so we don't need this in stage 1? then we can do it in stage  1 phase
>  2, which is designed specifically for this type of operation: doing
>  something before you try to scan pci.

Sorry! Stage 2 phase  1!

ron

[Marc Jones]

ron minnich wrote:
> On Wed, Mar 5, 2008 at 11:33 AM, ron minnich  wrote:
> > On Wed, Mar 5, 2008 at 9:59 AM, Marc Jones  wrote:
> > > > > Need this to do stage2. Without it the 5536 won't decode PCI.
> > > >
> > > > Did you mean stage1?
> > >
> > > You need to do it in stage1 so you can do stage2 (stage 2 needs to find
> > >  the device before it can program it).
> >
> > so we don't need this in stage 1? then we can do it in stage  1 phase
> >  2, which is designed specifically for this type of operation: doing
> >  something before you try to scan pci.
>
> Sorry! Stage 2 phase  1!
>
> ron

I can't think of anything in stage1 that specifically needs it but it 
does seem risky. It would need to be the first thing ins stage2 phase1. 
I suppose you can always move it back if you find a problem.
Marc

[Carl-Daniel Hailfinger]

On 05.03.2008 18:59, Marc Jones wrote:
> Carl-Daniel Hailfinger wrote:
> > On 05.03.2008 18:13, Marc Jones wrote:
> > > ron minnich wrote:
> > > > One thing I plan to change pretty soon: power button control can move
> > > > from stage1 to stage2, and power button control will be managed in dts
> > > > (another dts setting we would NOT want visible in Kconfig; you could
> > > > render a board totally unbootable and unrepairable with the wrong
> > > > setting. Kconfig should be for settings that are safe to change).
> > >
> > > This would be great. It should be easy to add a dts item and call 
> > > cs5536_setup_power_button or not. Moving it to stage 2 should be 
> > > fine BUT until the power button is setup it won't work. Some people 
> > > might not like that if you hang or delay somewhere prior to it being 
> > > setup.
> >
> > Can we set up the power button in a way that makes it work always, 
> > then change the setup in stage2?
>
> That is the problem. For systems that don't have a powerbutton, 
> enabling the button logic will cause a reset in 4 seconds which would 
> be fine if you could gaurnettee that it would be reprogrammed by then 
> but that is just the oposite problem described above. What if there is 
> a delay prior to rprograming it?

Now that problem makes sense. Thanks for explaining it!
> > > >     msr = rdmsr(GLCP_SYS_RSTPLL);
> > > >     if (msr.lo & (0x3f << 26)) {
> > > >             /* PLL is already set and we are reboot from PLL 
> > > >
> > > > reset. */
> > > >                 return;
> > > >         }
> > > >     cs5536_setup_idsel();
> > >
> > > Need this to do stage2. Without it the 5536 won't decode PCI.
> >
> > Did you mean stage1?
>
> You need to do it in stage1 so you can do stage2 (stage 2 needs to 
> find the device before it can program it).

Sorry, I had misread your sentence as "Need to do this in stage2". 
That's why I was confused. Rereading it revealed my misunderstanding.

Regards,
Carl-Daniel