Re: [Xen-devel] [PATCH v6 03/22] arm/acpi: Add __acpi_map_table function for ARM

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 29 Feb 2016, Jan Beulich wrote:
> >>> On 29.02.16 at 12:43, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> > On Mon, 29 Feb 2016, Jan Beulich wrote:
> >> >>> On 29.02.16 at 11:54, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> >> > On Sat, 27 Feb 2016, Shannon Zhao wrote:
> >> >> From: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
> >> >> 
> >> >> Implement __acpi_map_table function for ARM. Move FIX_ACPI_PAGES to
> >> >> common place and rename it to NUM_FIXMAP_ACPI_PAGES.
> >> >> 
> >> >> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> >> >> Signed-off-by: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
> >> > 
> >> > Reviewed-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> >> 
> >> Are you sure, particularly with ...
> >> 
> >> >> --- /dev/null
> >> >> +++ b/xen/arch/arm/acpi/lib.c
> >> >> @@ -0,0 +1,52 @@
> >> >> +/*
> >> >> + *  lib.c - Architecture-Specific Low-Level ACPI Support
> >> >> + *
> >> >> + *  Copyright (C) 2015, Shannon Zhao <shannon.zhao@xxxxxxxxxx>
> >> >> + *
> >> >> + * 
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> >> + *
> >> >> + *  This program is free software; you can redistribute it and/or 
> >> >> modify
> >> >> + *  it under the terms of the GNU General Public License as published 
> >> >> by
> >> >> + *  the Free Software Foundation; either version 2 of the License, or
> >> >> + *  (at your option) any later version.
> >> >> + *
> >> >> + *  This program is distributed in the hope that it will be useful,
> >> >> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> >> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >> >> + *  GNU General Public License for more details.
> >> >> + *
> >> >> + *  You should have received a copy of the GNU General Public License
> >> >> + *  along with this program; if not, write to the Free Software
> >> >> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  
> >> >> 02111-1307  
> > USA
> >> >> + *
> >> >> + * 
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> >> + */
> >> >> +
> >> >> +#include <xen/acpi.h>
> >> >> +#include <xen/mm.h>
> >> >> +#include <asm/config.h>
> >> >> +
> >> >> +char *__acpi_map_table(paddr_t phys, unsigned long size)
> >> >> +{
> >> >> +       unsigned long base, offset, mapped_size;
> >> >> +       int idx;
> >> >> +
> >> >> +       offset = phys & (PAGE_SIZE - 1);
> >> >> +       mapped_size = PAGE_SIZE - offset;
> >> >> +       set_fixmap(FIXMAP_ACPI_BEGIN, phys >> PAGE_SHIFT, 
> >> >> PAGE_HYPERVISOR);
> >> >> +       base = FIXMAP_ADDR(FIXMAP_ACPI_BEGIN);
> >> >> +
> >> >> +       /*
> >> >> +        * Most cases can be covered by the below.
> >> >> +        */
> >> >> +       idx = FIXMAP_ACPI_BEGIN;
> >> >> +       while (mapped_size < size) {
> >> >> +               if (++idx > FIXMAP_ACPI_END)
> >> >> +                       return NULL;    /* cannot handle this */
> >> >> +               phys += PAGE_SIZE;
> >> >> +               set_fixmap(idx, phys >> PAGE_SHIFT, PAGE_HYPERVISOR);
> >> >> +               mapped_size += PAGE_SIZE;
> >> >> +       }
> >> >> +
> >> >> +       return ((char *) base + offset);
> >> >> +}
> >> 
> >> ... this new file using Linux instead of Xen coding style?
> > 
> > That's true, but it is clearly taken from
> > xen/arch/x86/acpi/lib.c:__acpi_map_table, which is also Linux coding
> > style.
> 
> The main difference being that the x86 file is a clone of the Linux
> original, whereas the way the new ARM one gets introduced it
> doesn't look to be quite that way.

Fair enough, you are right, it would be nice if __acpi_map_table was
following the Xen coding style.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel