[PATCH] don't print too early on mcp55-based boards

[Ward Vandewege]

See attached. Perhaps we should also print a post code if the SMBus
controller can't be found - suggestions for a value?

Thanks,
Ward.

[Peter Stuge]

Ward Vandewege wrote:
> See attached. Perhaps we should also print a post code if the SMBus
> controller can't be found - suggestions for a value?

0x5B ?
> We can't print this early.
>
> This patch fixes a hang on
>
> supermicro/h8dme
>   supermicro/h8dmr
>   supermicro/h8dmr_fam10
>
> and possibly on other mcp55-based boards.
>
> Signed-off-by: Ward Vandewege

Acked-by: Peter Stuge

[Uwe Hermann]

On Mon, Nov 01, 2010 at 11:01:41PM +0100, Peter Stuge wrote:
> Ward Vandewege wrote:
> > See attached. Perhaps we should also print a post code if the SMBus
> > controller can't be found - suggestions for a value?
>
> 0x5B ?
> > We can't print this early.
> >
> > This patch fixes a hang on
> >
> > supermicro/h8dme
> >   supermicro/h8dmr
> >   supermicro/h8dmr_fam10
> >
> > and possibly on other mcp55-based boards.
> >
> > Signed-off-by: Ward Vandewege
>
> Acked-by: Peter Stuge

I don't object to the patch, and we should probably fix this in all
other southbridges, I think the same problem applies there.

But: the die() call itself also does a printk(), so that'll still hang
if the error path is chosen (at that point it probably doesn't matter
much, though, as we die anyway).

I also agree that die() should have a POST code, preferrably something
easy to remember. It already has a commented-out "//post_code(0xff);".
Not sure why it's disabled, but I think it should be something other
than 0xff, that's a bit too "special" for my taste.

We have "0xee: Not supposed to get here" as per documentation/POSTCODES,
so maybe we can use 0xdd ("d" as in die), if that's not already used elsewhere.

Uwe.

[Ward Vandewege]

On Tue, Nov 02, 2010 at 11:20:43PM +0100, Uwe Hermann wrote:
> I don't object to the patch, and we should probably fix this in all
> other southbridges, I think the same problem applies there.
>
> But: the die() call itself also does a printk(), so that'll still hang
> if the error path is chosen (at that point it probably doesn't matter
> much, though, as we die anyway).

Right, I think it does not matter. If the die happens when printk is already
functional, great, if not it will hang there which is fine.
> I also agree that die() should have a POST code, preferrably something
> easy to remember. It already has a commented-out "//post_code(0xff);".
> Not sure why it's disabled, but I think it should be something other
> than 0xff, that's a bit too "special" for my taste.
>
> We have "0xee: Not supposed to get here" as per documentation/POSTCODES,
> so maybe we can use 0xdd ("d" as in die), if that's not already used elsewhere.

So, thinking about this a little more, I'm not sure adding a post code to
'die' is a good idea. The problem with doing that is that it would clobber
any previous post codes, which might be a better indicator for what's going
wrong.

Perhaps a good way to deal with fatal runtime error conditions would be

a) set a unique post code
  b) call die

in the assumption that die does not clobber the post code.

What do you think?

Thanks,
Ward.

[Stefan Reinauer]

Sent from my mobile phone

On 03.11.2010, at 09:56, Ward Vandewege  wrote:
> On Tue, Nov 02, 2010 at 11:20:43PM +0100, Uwe Hermann wrote:
> > I don't object to the patch, and we should probably fix this in all
> > other southbridges, I think the same problem applies there.
> >
> > But: the die() call itself also does a printk(), so that'll still hang
> > if the error path is chosen (at that point it probably doesn't matter
> > much, though, as we die anyway).
>
> Right, I think it does not matter. If the die happens when printk is already
> functional, great, if not it will hang there which is fine.
> > I also agree that die() should have a POST code, preferrably something
> > easy to remember. It already has a commented-out "//post_code(0xff);".
> > Not sure why it's disabled, but I think it should be something other
> > than 0xff, that's a bit too "special" for my taste.
> >
> > We have "0xee: Not supposed to get here" as per documentation/POSTCODES,
> > so maybe we can use 0xdd ("d" as in die), if that's not already used elsewhere.
>
> So, thinking about this a little more, I'm not sure adding a post code to
> 'die' is a good idea. The problem with doing that is that it would clobber
> any previous post codes, which might be a better indicator for what's going
> wrong.
>
> Perhaps a good way to deal with fatal runtime error conditions would be
>
> a) set a unique post code
>  b) call die
>
> in the assumption that die does not clobber the post code.

We could add a post code to the parameters of the die() function.
> What do you think?
>
> Thanks,
> Ward.

[Peter Stuge]

Ward Vandewege wrote:
> I'm not sure adding a post code to 'die' is a good idea. The
> problem with doing that is that it would clobber any previous post
> codes, which might be a better indicator for what's going wrong.

I think it is important to have a special POST code for use in die()
to positively identify that that is really where the code has hung.

Stefan Reinauer wrote:
> We could add a post code to the parameters of the die() function.

I think this is a good idea, and I would like to suggest that die()
does the following:

void die(u8 post) {
  while(1) {
    post_code(0xdd);
    mdelay(700);
    post_code(post);
    mdelay(700);
  }
}

There is no printk() which is sad. I think it's better to only use
POST codes if that is the only reliable output method, but I think
it's a lot better if the print functions can just be made to not
hang if called before they can actually output something. That would
need a bit more work though, so for now I'd like to suggest the
above.

//Peter

[Ward Vandewege]

On Mon, Nov 01, 2010 at 11:01:41PM +0100, Peter Stuge wrote:
> Ward Vandewege wrote:
> > See attached. Perhaps we should also print a post code if the SMBus
> > controller can't be found - suggestions for a value?
>
> 0x5B ?

Let's do that as part of the die modification.
> > We can't print this early.
> >
> > This patch fixes a hang on
> >
> > supermicro/h8dme
> >   supermicro/h8dmr
> >   supermicro/h8dmr_fam10
> >
> > and possibly on other mcp55-based boards.
> >
> > Signed-off-by: Ward Vandewege
>
> Acked-by: Peter Stuge

r6048.

Thanks,
Ward.