Re: [LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

[Lu, Yinghai]

USB DEBUG port support.

YH

-----Original Message-----
From: [e-mail]
[mailto:[e-mail]] On Behalf Of Stefan Reinauer
Sent: Saturday, February 03, 2007 7:31 AM
To: Uwe Hermann
Cc: [e-mail]
Subject: Re: [LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

Uwe Hermann wrote:
> Add support for the Winbond W83627EHG Super I/O.
>
> Signed-off-by: Yinghai Lu 
> Signed-off-by: Uwe Hermann

applied.
> This is from Yinghai's huge patch. I added missing license headers
> (Yinghai please complain if any of them is incorrect!), fixed some
> coding style issues and added a few comments.

What's still missing from Yinghai's patch now?

[LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

[Stefan Reinauer]

* Lu, Yinghai  [070205 18:09]:
> USB DEBUG port support.

I committed this now, but I am unwilling to let this printk_ram stuff go
in like that. We do not want yet another set of printk functions that
have to be used with ifdefs spread in dozens of files. Can you please
hook this up cleanly and against the CURRENT svn?

There's also smaller stuff that seems or seems not to be missing, like
randomly commented in and out functions in yh_rest_of_patch.patch.

I will not go into further trouble of merging these changes now. If
anyone feels this is needed, please help.

I made a new, somewhat cleaned up version of yh_rest_of_patch.patch
But it is far from ready to go in.
> -----Original Message-----
> From: [e-mail]
> [mailto:[e-mail]] On Behalf Of Stefan Reinauer
> Sent: Saturday, February 03, 2007 7:31 AM
> To: Uwe Hermann
> Cc: [e-mail]
> Subject: Re: [LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O
>
> Uwe Hermann wrote:
> > Add support for the Winbond W83627EHG Super I/O.
> >
> > Signed-off-by: Yinghai Lu 
> > Signed-off-by: Uwe Hermann
>
> applied.
> > This is from Yinghai's huge patch. I added missing license headers
> > (Yinghai please complain if any of them is incorrect!), fixed some
> > coding style issues and added a few comments.
>
> What's still missing from Yinghai's patch now?

[LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

[Carl-Daniel Hailfinger]

Hi,

On 28.02.2007 12:47, Stefan Reinauer wrote:
> Lu, Yinghai  [070205 18:09]:
> > USB DEBUG port support.
>
> I committed this now, but I am unwilling to let this printk_ram stuff go
> in like that. We do not want yet another set of printk functions that
> have to be used with ifdefs spread in dozens of files. Can you please
> hook this up cleanly and against the CURRENT svn?
>
> There's also smaller stuff that seems or seems not to be missing, like
> randomly commented in and out functions in yh_rest_of_patch.patch.
>
> I will not go into further trouble of merging these changes now. If
> anyone feels this is needed, please help.
>
> I made a new, somewhat cleaned up version of yh_rest_of_patch.patch
> But it is far from ready to go in.

The current printk stuff is a mess. Yinghai at least tried to clean
this up, but his tree diverged and so we have an even bigger mess.

Look at current svn and see the horror:
./src/arch/ppc/include/printk.h is a subset of
./src/include/console/console.h is a subset of
./src/arch/i386/lib/console.c
Duplicated code everywhere.

Suggestion: Look at the diff between LB-svn/src/arch/i386/lib/console.c
and LB-yhlu/src/arch/i386/lib/console_printk.c and see that moving code
between files helps readability. I can prepare a patch without functional
changes if anyone agrees.

Regards,
Carl-Daniel

[LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

[Stefan Reinauer]

* Carl-Daniel Hailfinger  [070228 15:19]:
> The current printk stuff is a mess. Yinghai at least tried to clean
> this up, but his tree diverged and so we have an even bigger mess.
>
> Look at current svn and see the horror:
> ./src/arch/ppc/include/printk.h is a subset of
> ./src/include/console/console.h is a subset of
> ./src/arch/i386/lib/console.c
> Duplicated code everywhere.
>
> Suggestion: Look at the diff between LB-svn/src/arch/i386/lib/console.c
> and LB-yhlu/src/arch/i386/lib/console_printk.c and see that moving code
> between files helps readability. I can prepare a patch without functional
> changes if anyone agrees.

Please do!

[LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

[Lu, Yinghai]

Thanks.

After you commit it, I would produce another patch about
1. UNITID shifting support
2. one Typo in console_printk*...

YH

-----Original Message-----
From: Stefan Reinauer [mailto:[e-mail]] 
Sent: Wednesday, February 28, 2007 3:47 AM
To: Lu, Yinghai
Cc: Stefan Reinauer; Uwe Hermann; [e-mail]
Subject: Re: [LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

* Lu, Yinghai  [070205 18:09]:
> USB DEBUG port support.

I committed this now, but I am unwilling to let this printk_ram stuff go
in like that. We do not want yet another set of printk functions that
have to be used with ifdefs spread in dozens of files. Can you please
hook this up cleanly and against the CURRENT svn?

There's also smaller stuff that seems or seems not to be missing, like
randomly commented in and out functions in yh_rest_of_patch.patch.

I will not go into further trouble of merging these changes now. If
anyone feels this is needed, please help.

I made a new, somewhat cleaned up version of yh_rest_of_patch.patch
But it is far from ready to go in.
> -----Original Message-----
> From: [e-mail]
> [mailto:[e-mail]] On Behalf Of Stefan Reinauer
> Sent: Saturday, February 03, 2007 7:31 AM
> To: Uwe Hermann
> Cc: [e-mail]
> Subject: Re: [LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super

I/O
> Uwe Hermann wrote:
> > Add support for the Winbond W83627EHG Super I/O.
> >
> > Signed-off-by: Yinghai Lu 
> > Signed-off-by: Uwe Hermann
>
> applied.
> > This is from Yinghai's huge patch. I added missing license headers
> > (Yinghai please complain if any of them is incorrect!), fixed some
> > coding style issues and added a few comments.
>
> What's still missing from Yinghai's patch now?

[LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

[Lu, Yinghai]

In the V2 you need to everyone happy
1. ROMCC: print
2. CAR: a. print
      b. printk
      c. print when ram is enabled already 
      d. printk when ram is enabled already
c/d are introduced for USBDebug direct, because dbg_info will be in RAM
instead of cache..
dbg_info will contain the BAR info that is needed for dbgp_bulk_write_x

check pc80/usbdebug_dirct_serial.c for more

YH

-----Original Message-----
From: Stefan Reinauer [mailto:[e-mail]] 
Sent: Wednesday, February 28, 2007 6:53 AM
To: Carl-Daniel Hailfinger
Cc: Lu, Yinghai; Stefan Reinauer; [e-mail]
Subject: Re: [LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

* Carl-Daniel Hailfinger  [070228
15:19]:
> The current printk stuff is a mess. Yinghai at least tried to clean
> this up, but his tree diverged and so we have an even bigger mess.
>
> Look at current svn and see the horror:
> ./src/arch/ppc/include/printk.h is a subset of
> ./src/include/console/console.h is a subset of
> ./src/arch/i386/lib/console.c
> Duplicated code everywhere.
>
> Suggestion: Look at the diff between

LB-svn/src/arch/i386/lib/console.c
> and LB-yhlu/src/arch/i386/lib/console_printk.c and see that moving

code
> between files helps readability. I can prepare a patch without

functional
> changes if anyone agrees.

Please do!

[LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

[Carl-Daniel Hailfinger]

On 28.02.2007 18:22, Lu, Yinghai wrote:
> In the V2 you need to everyone happy
> ROMCC: print
> CAR: a. print
>   b. printk
>   c. print when ram is enabled already 
>   d. printk when ram is enabled already
>
> c/d are introduced for USBDebug direct, because dbg_info will be in RAM
> instead of cache..

Can we use a variable to automatically call the right version in CAR?
I know the code below will not work, but to give you an idea...

int do_printk(int msg_level, const char *fmt, ...)
{
    if (ram_enabled)
    	return do_ram_printk()
    else
    	return do_car_printk()
}

Or we do that with function pointer manipulation.

Comments?

Regards,
Carl-Daniel

[LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

[Lu, Yinghai]

Where will be ram_enabled? Cache or Ram?

YH

-----Original Message-----
From: [e-mail]
[mailto:[e-mail]] On Behalf Of Carl-Daniel
Hailfinger
Sent: Wednesday, February 28, 2007 10:12 AM
To: Lu, Yinghai
Cc: Stefan Reinauer; [e-mail]
Subject: Re: [LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

On 28.02.2007 18:22, Lu, Yinghai wrote:
> In the V2 you need to everyone happy
> ROMCC: print
> CAR: a. print
>   b. printk
>   c. print when ram is enabled already 
>   d. printk when ram is enabled already
>
> c/d are introduced for USBDebug direct, because dbg_info will be in

RAM
> instead of cache..

Can we use a variable to automatically call the right version in CAR?
I know the code below will not work, but to give you an idea...

int do_printk(int msg_level, const char *fmt, ...)
{
    if (ram_enabled)
    	return do_ram_printk()
    else
    	return do_car_printk()
}

Or we do that with function pointer manipulation.

Comments?

Regards,
Carl-Daniel

[LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

[Stefan Reinauer]

Lu, Yinghai wrote:
> In the V2 you need to everyone happy
> ROMCC: print
> CAR: a. print
>   b. printk
>   c. print when ram is enabled already 
>   d. printk when ram is enabled already
> c/d are introduced for USBDebug direct, because dbg_info will be in RAM
> instead of cache..
> dbg_info will contain the BAR info that is needed for dbgp_bulk_write_x

Sorry, I still dont get it. Why can't romcc just use print_, CAR could
only use printk, and printk does the right thing, depending on whether
we have ram or not. that's two variations instead of 5.

Stefan

[LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

[Stefan Reinauer]

Lu, Yinghai wrote:
> In the V2 you need to everyone happy
> ROMCC: print
> CAR: a. print
>   b. printk
>   c. print when ram is enabled already 
>   d. printk when ram is enabled already
>
> c/d are introduced for USBDebug direct, because dbg_info will be in RAM
> instead of cache..
> dbg_info will contain the BAR info that is needed for dbgp_bulk_write_x

c and d are not needed. When RAM is enabled, we are not in the CAR path
of the code anymore.

[LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

[Lu, Yinghai]

But you share codes between ROMCC version and CAR version.

YH

-----Original Message-----
From: Stefan Reinauer [mailto:[e-mail]] 
Sent: Thursday, March 01, 2007 7:31 AM
To: Lu, Yinghai
Cc: Stefan Reinauer; Carl-Daniel Hailfinger; [e-mail]
Subject: Re: [LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

Lu, Yinghai wrote:
> In the V2 you need to everyone happy
> ROMCC: print
> CAR: a. print
>   b. printk
>   c. print when ram is enabled already 
>   d. printk when ram is enabled already
> c/d are introduced for USBDebug direct, because dbg_info will be in

RAM
> instead of cache..
> dbg_info will contain the BAR info that is needed for

dbgp_bulk_write_x

Sorry, I still dont get it. Why can't romcc just use print_, CAR could
only use printk, and printk does the right thing, depending on whether
we have ram or not. that's two variations instead of 5.

Stefan

[LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

[Lu, Yinghai]

In the copy_and_run you may need some debug print out. And copy_and_run
is after stack switching.

YH

-----Original Message-----
From: Stefan Reinauer [mailto:[e-mail]] 
Sent: Thursday, March 01, 2007 7:33 AM
To: Lu, Yinghai
Cc: Stefan Reinauer; Carl-Daniel Hailfinger; [e-mail]
Subject: Re: [LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

Lu, Yinghai wrote:
> In the V2 you need to everyone happy
> ROMCC: print
> CAR: a. print
>   b. printk
>   c. print when ram is enabled already 
>   d. printk when ram is enabled already
>
> c/d are introduced for USBDebug direct, because dbg_info will be in

RAM
> instead of cache..
> dbg_info will contain the BAR info that is needed for

dbgp_bulk_write_x

c and d are not needed. When RAM is enabled, we are not in the CAR path
of the code anymore.

[LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

[Carl-Daniel Hailfinger]

On 28.02.2007 19:12, Lu, Yinghai wrote:
> Where will be ram_enabled? Cache or Ram?

If we want to call it from CAR, we must place it in cache.
Is cache content lost after switching off CAR?

Regards,
Carl-Daniel

[LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

[Lu, Yinghai]

The Global variables area is copied from the cache to RAM, but position
is changed.

YH

-----Original Message-----
From: Carl-Daniel Hailfinger [mailto:[e-mail]] 
Sent: Thursday, March 01, 2007 4:17 PM
To: Lu, Yinghai
Cc: Stefan Reinauer; [e-mail]
Subject: Re: [LinuxBIOS] [PATCH] Support for Winbond W83627EHG Super I/O

On 28.02.2007 19:12, Lu, Yinghai wrote:
> Where will be ram_enabled? Cache or Ram?

If we want to call it from CAR, we must place it in cache.
Is cache content lost after switching off CAR?

Regards,
Carl-Daniel