Patchwork [U-Boot] biosemu: include <asm/io.h> header

login
register
mail settings
Submitter Linus Walleij
Date April 2, 2013, 8:14 a.m.
Message ID <1364890454-5659-1-git-send-email-linus.walleij@linaro.org>
Download mbox | patch
Permalink /patch/232913/
State Accepted
Delegated to: Tom Rini
Headers show

Comments

Linus Walleij - April 2, 2013, 8:14 a.m.
This makes sure we have inline functions such as inb/outb that
are used in these two files by including the arch-specific
<asm/io.h> header. However the ARM version does not provide the
accessors unless the config symbol __io is also defined so add
that in front of the include.

After this the bios emulator will compile on ARM systems.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/bios_emulator/besys.c | 2 ++
 drivers/bios_emulator/bios.c  | 2 ++
 2 files changed, 4 insertions(+)
Albert ARIBAUD - April 2, 2013, 8:56 a.m.
Hi Linus,

On Tue,  2 Apr 2013 10:14:14 +0200, Linus Walleij
<linus.walleij@linaro.org> wrote:

> This makes sure we have inline functions such as inb/outb that
> are used in these two files by including the arch-specific
> <asm/io.h> header. However the ARM version does not provide the
> accessors unless the config symbol __io is also defined so add
> that in front of the include.
> 
> After this the bios emulator will compile on ARM systems.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/bios_emulator/besys.c | 2 ++
>  drivers/bios_emulator/bios.c  | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/bios_emulator/besys.c b/drivers/bios_emulator/besys.c
> index 84724b7..ad88a53 100644
> --- a/drivers/bios_emulator/besys.c
> +++ b/drivers/bios_emulator/besys.c
> @@ -47,6 +47,8 @@
>  *
>  ****************************************************************************/
>  
> +#define __io
> +#include <asm/io.h>
>  #include <common.h>
>  #include "biosemui.h"
>  
> diff --git a/drivers/bios_emulator/bios.c b/drivers/bios_emulator/bios.c
> index 7cf4879..bcc192f 100644
> --- a/drivers/bios_emulator/bios.c
> +++ b/drivers/bios_emulator/bios.c
> @@ -41,6 +41,8 @@
>  *
>  ****************************************************************************/
>  
> +#define __io
> +#include <asm/io.h>
>  #include <common.h>
>  #include "biosemui.h"

NAK -- no ARM target needs bios emulation, so basing the #define on ARM
requirements is incorrect.

Actually, ARM targets build drivers/bios_emulator/libatibiosemu.o as
the result of an overlook in ./Makefile where this object is compiled 
unconditionally.

A git grep CONFIG_BIOSEMU seems to indicate only a handful of PowerPC
targets need bios emulation; I suggest doing a V2 of this patch
where the object is built only for PowerPC, and the #define is removed.

Amicalement,
Linus Walleij - April 2, 2013, 10:09 a.m.
On Tue, Apr 2, 2013 at 10:56 AM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:

> NAK -- no ARM target needs bios emulation, so basing the #define on ARM
> requirements is incorrect.
>
> Actually, ARM targets build drivers/bios_emulator/libatibiosemu.o as
> the result of an overlook in ./Makefile where this object is compiled
> unconditionally.
>
> A git grep CONFIG_BIOSEMU seems to indicate only a handful of PowerPC
> targets need bios emulation; I suggest doing a V2 of this patch
> where the object is built only for PowerPC, and the #define is removed.

So the patch was initiated by the discussion here:
http://marc.info/?l=linux-arm-kernel&m=136399798826572&w=2

I am looking into getting biosemu up on ARM for the PCI-equipped
ARM boards, but admittedly I have no idea where it'll go.

But this can surely wait until I have a series that can be run as well.

Yours,
Linus Walleij
Albert ARIBAUD - April 2, 2013, 10:32 a.m.
Hi Linus,

On Tue, 2 Apr 2013 12:09:21 +0200, Linus Walleij
<linus.walleij@linaro.org> wrote:

> On Tue, Apr 2, 2013 at 10:56 AM, Albert ARIBAUD
> <albert.u.boot@aribaud.net> wrote:
> 
> > NAK -- no ARM target needs bios emulation, so basing the #define on ARM
> > requirements is incorrect.
> >
> > Actually, ARM targets build drivers/bios_emulator/libatibiosemu.o as
> > the result of an overlook in ./Makefile where this object is compiled
> > unconditionally.
> >
> > A git grep CONFIG_BIOSEMU seems to indicate only a handful of PowerPC
> > targets need bios emulation; I suggest doing a V2 of this patch
> > where the object is built only for PowerPC, and the #define is removed.
> 
> So the patch was initiated by the discussion here:
> http://marc.info/?l=linux-arm-kernel&m=136399798826572&w=2
> 
> I am looking into getting biosemu up on ARM for the PCI-equipped
> ARM boards, but admittedly I have no idea where it'll go.
> 
> But this can surely wait until I have a series that can be run as well.

Indeed: when some real ARM target needs biosemu, then yes, this patch
will make sense as part of the series that introduces this target.

> Yours,
> Linus Walleij

Amicalement,
Tom Rini - April 3, 2013, 3:30 p.m.
On Mon, Apr 01, 2013 at 10:14:14PM -0000, Linus Walleij wrote:

> This makes sure we have inline functions such as inb/outb that
> are used in these two files by including the arch-specific
> <asm/io.h> header. However the ARM version does not provide the
> accessors unless the config symbol __io is also defined so add
> that in front of the include.
> 
> After this the bios emulator will compile on ARM systems.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Applied to u-boot/master, thanks!
Tom Rini - April 3, 2013, 4:02 p.m.
On Tue, Apr 02, 2013 at 12:32:40PM +0200, Albert ARIBAUD wrote:

> Hi Linus,
> 
> On Tue, 2 Apr 2013 12:09:21 +0200, Linus Walleij
> <linus.walleij@linaro.org> wrote:
> 
> > On Tue, Apr 2, 2013 at 10:56 AM, Albert ARIBAUD
> > <albert.u.boot@aribaud.net> wrote:
> > 
> > > NAK -- no ARM target needs bios emulation, so basing the #define on ARM
> > > requirements is incorrect.
> > >
> > > Actually, ARM targets build drivers/bios_emulator/libatibiosemu.o as
> > > the result of an overlook in ./Makefile where this object is compiled
> > > unconditionally.
> > >
> > > A git grep CONFIG_BIOSEMU seems to indicate only a handful of PowerPC
> > > targets need bios emulation; I suggest doing a V2 of this patch
> > > where the object is built only for PowerPC, and the #define is removed.
> > 
> > So the patch was initiated by the discussion here:
> > http://marc.info/?l=linux-arm-kernel&m=136399798826572&w=2
> > 
> > I am looking into getting biosemu up on ARM for the PCI-equipped
> > ARM boards, but admittedly I have no idea where it'll go.
> > 
> > But this can surely wait until I have a series that can be run as well.
> 
> Indeed: when some real ARM target needs biosemu, then yes, this patch
> will make sense as part of the series that introduces this target.

Grr, I missed this discussion, sorry.  But I don't feel this falls into
the "no dead code" rule, but does fall into the "lets not make life too
hard for others" rule I have at least.  There's at least one board in
u-boot-arm now (ti814x) which can get PCI turned on, with further work,
and another that was posted recently (ti816x), and more coming down the
pipe.  And also what Linus has access to.
Linus Walleij - April 3, 2013, 5:07 p.m.
On Wed, Apr 3, 2013 at 6:02 PM, Tom Rini <trini@ti.com> wrote:

> Grr, I missed this discussion, sorry.  But I don't feel this falls into
> the "no dead code" rule, but does fall into the "lets not make life too
> hard for others" rule I have at least.  There's at least one board in
> u-boot-arm now (ti814x) which can get PCI turned on, with further work,
> and another that was posted recently (ti816x), and more coming down the
> pipe.  And also what Linus has access to.

Yes the Integrator has PCI support since ages, and I do have a VGA
card mounted in one of the slots ... the only issue is that I need
biosemu to be working to initialize it. More people will run into the
same issue definately, especially with ARM servers and such things
using PCI starting to appear.

Yours,
Linus Walleij
Tom Rini - April 3, 2013, 10:05 p.m.
On Wed, Apr 03, 2013 at 07:07:00PM +0200, Linus Walleij wrote:

> On Wed, Apr 3, 2013 at 6:02 PM, Tom Rini <trini@ti.com> wrote:
> 
> > Grr, I missed this discussion, sorry.  But I don't feel this falls into
> > the "no dead code" rule, but does fall into the "lets not make life too
> > hard for others" rule I have at least.  There's at least one board in
> > u-boot-arm now (ti814x) which can get PCI turned on, with further work,
> > and another that was posted recently (ti816x), and more coming down the
> > pipe.  And also what Linus has access to.
> 
> Yes the Integrator has PCI support since ages, and I do have a VGA
> card mounted in one of the slots ... the only issue is that I need
> biosemu to be working to initialize it. More people will run into the
> same issue definately, especially with ARM servers and such things
> using PCI starting to appear.

Can you pass along a patch to enable this stuff for integrator then
please?  That should keep everyone happy then, thanks!
Linus Walleij - April 4, 2013, 5:59 a.m.
On Thu, Apr 4, 2013 at 12:05 AM, Tom Rini <trini@ti.com> wrote:
> On Wed, Apr 03, 2013 at 07:07:00PM +0200, Linus Walleij wrote:
>>
>> Yes the Integrator has PCI support since ages, and I do have a VGA
>> card mounted in one of the slots ... the only issue is that I need
>> biosemu to be working to initialize it. More people will run into the
>> same issue definately, especially with ARM servers and such things
>> using PCI starting to appear.
>
> Can you pass along a patch to enable this stuff for integrator then
> please?  That should keep everyone happy then, thanks!

Well, it can be enabled for example like this:

+#define CONFIG_VIDEO
+#define VIDEO_IO_OFFSET 0x60000000
+#define CONFIG_SYS_ISA_IO_BASE_ADDRESS 0x60000000
+#define CONFIG_BIOSEMU
+#define CONFIG_CFB_CONSOLE
+#define CONFIG_VIDEO_SW_CURSOR
+#define CONFIG_VGA_AS_SINGLE_DEVICE
+#define CONFIG_ATI_RADEON_FB
+#define CONFIG_VIDEO_LOGO

The thing is that since it's more or less a PC form-factor thing
with PCI slots, enabling VGA emulation and the Radeon driver
is just overhead for everyone without such a card in their machine.

I was more thinking about making it possible to do this, not
necessarily enabling it by default and increasing the overhead
for everyone...

Yours,
Linus Walleij
Tom Rini - April 4, 2013, 8:14 p.m.
On Thu, Apr 04, 2013 at 07:59:35AM +0200, Linus Walleij wrote:

> On Thu, Apr 4, 2013 at 12:05 AM, Tom Rini <trini@ti.com> wrote:
> > On Wed, Apr 03, 2013 at 07:07:00PM +0200, Linus Walleij wrote:
> >>
> >> Yes the Integrator has PCI support since ages, and I do have a VGA
> >> card mounted in one of the slots ... the only issue is that I need
> >> biosemu to be working to initialize it. More people will run into the
> >> same issue definately, especially with ARM servers and such things
> >> using PCI starting to appear.
> >
> > Can you pass along a patch to enable this stuff for integrator then
> > please?  That should keep everyone happy then, thanks!
> 
> Well, it can be enabled for example like this:
> 
> +#define CONFIG_VIDEO
> +#define VIDEO_IO_OFFSET 0x60000000
> +#define CONFIG_SYS_ISA_IO_BASE_ADDRESS 0x60000000
> +#define CONFIG_BIOSEMU
> +#define CONFIG_CFB_CONSOLE
> +#define CONFIG_VIDEO_SW_CURSOR
> +#define CONFIG_VGA_AS_SINGLE_DEVICE
> +#define CONFIG_ATI_RADEON_FB
> +#define CONFIG_VIDEO_LOGO
> 
> The thing is that since it's more or less a PC form-factor thing
> with PCI slots, enabling VGA emulation and the Radeon driver
> is just overhead for everyone without such a card in their machine.
> 
> I was more thinking about making it possible to do this, not
> necessarily enabling it by default and increasing the overhead
> for everyone...

Fair enough, thanks!

Patch

diff --git a/drivers/bios_emulator/besys.c b/drivers/bios_emulator/besys.c
index 84724b7..ad88a53 100644
--- a/drivers/bios_emulator/besys.c
+++ b/drivers/bios_emulator/besys.c
@@ -47,6 +47,8 @@ 
 *
 ****************************************************************************/
 
+#define __io
+#include <asm/io.h>
 #include <common.h>
 #include "biosemui.h"
 
diff --git a/drivers/bios_emulator/bios.c b/drivers/bios_emulator/bios.c
index 7cf4879..bcc192f 100644
--- a/drivers/bios_emulator/bios.c
+++ b/drivers/bios_emulator/bios.c
@@ -41,6 +41,8 @@ 
 *
 ****************************************************************************/
 
+#define __io
+#include <asm/io.h>
 #include <common.h>
 #include "biosemui.h"