Mailing List Archive

[Xen-merge] [PATCH] merge i386's agp.h
Subject says it all.
Re: [Xen-merge] [PATCH] merge i386's agp.h [ In reply to ]
On Tue, Dec 20, 2005 at 03:15:00PM +0100, Jan Beulich wrote:
> Subject says it all.

I'm not sure that is the right approch. Adding ifdef all over [1]
has never been well accepted by kernel people ...

But it look backwards from Chris' approch to split files into mach and
generic (especially floppy.h)
(which is, imho not nicer on a certain POV.)

Maybe a mix of the two would be the `cleanest' [2] solution.

[1] there's not that much ifdef so "all over" is a bit strong here ...
[2] I'm afraid there's no such thing here ..
--
Vincent Hanquez

_______________________________________________
Xen-merge mailing list
Xen-merge@lists.xensource.com
http://lists.xensource.com/xen-merge
Re: [Xen-merge] [PATCH] merge i386's agp.h [ In reply to ]
>>> Vincent Hanquez <vincent.hanquez@cl.cam.ac.uk> 20.12.05 19:01 >>>
>On Tue, Dec 20, 2005 at 03:15:00PM +0100, Jan Beulich wrote:
> Subject says it all.
>
>I'm not sure that is the right approch. Adding ifdef all over [1]
>has never been well accepted by kernel people ...

I know.

>But it look backwards from Chris' approch to split files into mach
and
>generic (especially floppy.h)
>(which is, imho not nicer on a certain POV.)
>
>Maybe a mix of the two would be the `cleanest' [2] solution.

That is what I'm aiming towards. If more splitting is needed, this can
always be done in a second round. But we should get started somehow...

>[1] there's not that much ifdef so "all over" is a bit strong here
...
>[2] I'm afraid there's no such thing here ..

Jan

_______________________________________________
Xen-merge mailing list
Xen-merge@lists.xensource.com
http://lists.xensource.com/xen-merge
Re: [Xen-merge] [PATCH] merge i386's agp.h [ In reply to ]
On Tue, Dec 20, 2005 at 03:15:00PM +0100, Jan Beulich wrote:
> Subject says it all.

How about having a mach-{default,xen}/mach_agp.h file included at the top
of agp.h with:

#define phys_to_gart(x) phys_to_machine(x)
#define gart_to_phys(x) machine_to_phys(x)
#define HAVE_MACH_PHYS_GART_MACROS

and then in asm-i386/agp.h:
/* Convert a physical address to an address suitable for the GART. */
#ifndef HAVE_MACH_PHYS_GART_MACROS
#define phys_to_gart(x) (x)
#define gart_to_phys(x) (x)
#endif

Same for the {alloc,free}_gatt_pages macros.

That would be my preference since it preserves the default definitions
in the regular file moving the overrides into machine specific files.

christian


_______________________________________________
Xen-merge mailing list
Xen-merge@lists.xensource.com
http://lists.xensource.com/xen-merge
Re: [Xen-merge] [PATCH] merge i386's agp.h [ In reply to ]
Sure, it can be done that way, but for small blocks of conditionals I
really wanted to avoid introducing endless new header files. Of course,
if you had a clear direction from Linus and/or Andrew that you should
reduce the conditionals to a minimum, then I'd even go further and say
let's not have a conditional at all and move the default definitions to
mach-default/mach_agp.h. What you recommend is sitting in the middle of
the two, and hence doesn't seem a really clean approach to me. Jan

>>> Christian Limpach <Christian.Limpach@cl.cam.ac.uk> 22.12.05
16:08:20 >>>
On Tue, Dec 20, 2005 at 03:15:00PM +0100, Jan Beulich wrote:
> Subject says it all.

How about having a mach-{default,xen}/mach_agp.h file included at the
top
of agp.h with:

#define phys_to_gart(x) phys_to_machine(x)
#define gart_to_phys(x) machine_to_phys(x)
#define HAVE_MACH_PHYS_GART_MACROS

and then in asm-i386/agp.h:
/* Convert a physical address to an address suitable for the GART. */
#ifndef HAVE_MACH_PHYS_GART_MACROS
#define phys_to_gart(x) (x)
#define gart_to_phys(x) (x)
#endif

Same for the {alloc,free}_gatt_pages macros.

That would be my preference since it preserves the default definitions
in the regular file moving the overrides into machine specific files.

christian


_______________________________________________
Xen-merge mailing list
Xen-merge@lists.xensource.com
http://lists.xensource.com/xen-merge