lkml.org 
[lkml]   [2009]   [Jul]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 05/12] SFI: add core support

    * Len Brown <lenb@kernel.org> wrote:

    > From: Feng Tang <feng.tang@intel.com>
    >
    > drivers/sfi/sfi_core.c contains the generic SFI implementation.
    > It has a private header, sfi_core.h, for its own use and the
    > private use of future files in drivers/sfi/
    >
    > Signed-off-by: Feng Tang <feng.tang@intel.com>
    > Signed-off-by: Len Brown <len.brown@intel.com>
    > ---
    > drivers/Makefile | 1 +
    > drivers/sfi/Makefile | 1 +
    > drivers/sfi/sfi_core.c | 385 ++++++++++++++++++++++++++++++++++++++++++++++++
    > drivers/sfi/sfi_core.h | 44 ++++++
    > 4 files changed, 431 insertions(+), 0 deletions(-)
    > create mode 100644 drivers/sfi/Makefile
    > create mode 100644 drivers/sfi/sfi_core.c
    > create mode 100644 drivers/sfi/sfi_core.h
    >
    > diff --git a/drivers/Makefile b/drivers/Makefile
    > index bc4205d..ccfa259 100644
    > --- a/drivers/Makefile
    > +++ b/drivers/Makefile
    > @@ -11,6 +11,7 @@ obj-$(CONFIG_PARISC) += parisc/
    > obj-$(CONFIG_RAPIDIO) += rapidio/
    > obj-y += video/
    > obj-$(CONFIG_ACPI) += acpi/
    > +obj-$(CONFIG_SFI) += sfi/
    > # PnP must come after ACPI since it will eventually need to check if acpi
    > # was used and do nothing if so
    > obj-$(CONFIG_PNP) += pnp/
    > diff --git a/drivers/sfi/Makefile b/drivers/sfi/Makefile
    > new file mode 100644
    > index 0000000..f176470
    > --- /dev/null
    > +++ b/drivers/sfi/Makefile
    > @@ -0,0 +1 @@
    > +obj-y += sfi_core.o
    > diff --git a/drivers/sfi/sfi_core.c b/drivers/sfi/sfi_core.c
    > new file mode 100644
    > index 0000000..f9a9849
    > --- /dev/null
    > +++ b/drivers/sfi/sfi_core.c
    > @@ -0,0 +1,385 @@
    > +/* sfi_core.c Simple Firmware Interface - core internals */
    > +
    > +/*
    > + * Copyright (C) 2009, Intel Corp.
    > + * All rights reserved.
    > + *
    > + * Redistribution and use in source and binary forms, with or without
    > + * modification, are permitted provided that the following conditions
    > + * are met:
    > + * 1. Redistributions of source code must retain the above copyright
    > + * notice, this list of conditions, and the following disclaimer,
    > + * without modification.
    > + * 2. Redistributions in binary form must reproduce at minimum a disclaimer
    > + * substantially similar to the "NO WARRANTY" disclaimer below
    > + * ("Disclaimer") and any redistribution must be conditioned upon
    > + * including a substantially similar Disclaimer requirement for further
    > + * binary redistribution.
    > + * 3. Neither the names of the above-listed copyright holders nor the names
    > + * of any contributors may be used to endorse or promote products derived
    > + * from this software without specific prior written permission.
    > + *
    > + * Alternatively, this software may be distributed under the terms of the
    > + * GNU General Public License ("GPL") version 2 as published by the Free
    > + * Software Foundation.
    > + *
    > + * NO WARRANTY
    > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
    > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
    > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR
    > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
    > + * HOLDERS OR CONTRIBUTORS BE LIABLE FOR SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
    > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
    > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
    > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
    > + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING
    > + * IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
    > + * POSSIBILITY OF SUCH DAMAGES.
    > + */
    > +
    > +#define KMSG_COMPONENT "SFI"
    > +#define pr_fmt(fmt) KMSG_COMPONENT ": " fmt
    > +
    > +#include <linux/bootmem.h>
    > +#include <linux/kernel.h>
    > +#include <linux/module.h>
    > +#include <linux/string.h>
    > +#include <linux/errno.h>
    > +#include <linux/types.h>
    > +#include <linux/acpi.h>
    > +#include <linux/init.h>
    > +#include <linux/irq.h>
    > +#include <linux/smp.h>
    > +#include <linux/sfi.h>
    > +
    > +#include <asm/pgtable.h>
    > +
    > +#include "sfi_core.h"
    > +
    > +#define ON_SAME_PAGE(addr1, addr2) \
    > + (((unsigned long)(addr1) & PAGE_MASK) == \
    > + ((unsigned long)(addr2) & PAGE_MASK))
    > +#define TABLE_ON_PAGE(page, table, size) (ON_SAME_PAGE(page, table) && \
    > + ON_SAME_PAGE(page, table + size))

    these two should be written in C, not CPP.

    > +int sfi_disabled __read_mostly;
    > +EXPORT_SYMBOL(sfi_disabled);
    > +
    > +static u64 syst_pa __read_mostly;
    > +static struct sfi_table_simple *syst_va __read_mostly;
    > +
    > +/*
    > + * FW creates and saves the SFI tables in memory. When these tables get
    > + * used, they may need to be mapped to virtual address space, and the mapping
    > + * can happen before or after the ioremap() is ready, so a flag is needed
    > + * to indicating this
    > + */
    > +static u32 sfi_use_ioremap __read_mostly;
    > +
    > +static void __iomem *sfi_map_memory(u64 phys, u32 size)
    > +{
    > + if (!phys || !size)
    > + return NULL;
    > +
    > + if (sfi_use_ioremap)
    > + return ioremap(phys, size);
    > + else
    > + return early_ioremap(phys, size);
    > +}
    > +
    > +static void sfi_unmap_memory(void __iomem *virt, u32 size)
    > +{
    > + if (!virt || !size)
    > + return;
    > +
    > + if (sfi_use_ioremap)
    > + iounmap(virt);
    > + else
    > + early_iounmap(virt, size);
    > +}
    > +
    > +static void sfi_print_table_header(unsigned long long pa,
    > + struct sfi_table_header *header)
    > +{
    > + pr_info("%4.4s %llX, %04X (v%d %6.6s %8.8s)\n",
    > + header->signature, pa,
    > + header->length, header->revision, header->oem_id,
    > + header->oem_table_id);
    > +}
    > +
    > +/*
    > + * sfi_verify_table()
    > + * sanity check table lengh, calculate checksum
    > + */
    > +static __init int sfi_verify_table(struct sfi_table_header *table)
    > +{
    > +
    > + u8 checksum = 0;
    > + u8 *puchar = (u8 *)table;
    > + u32 length = table->length;
    > +
    > + /* Sanity check table length against arbitrary 1MB limit */
    > + if (length > 0x100000) {
    > + pr_err("Invalid table length 0x%x\n", length);
    > + return -1;
    > + }
    > +
    > + while (length--)
    > + checksum += *puchar++;
    > +
    > + if (checksum) {
    > + pr_err("Checksum %2.2X should be %2.2X\n",
    > + table->checksum, table->checksum - checksum);
    > + return -1;
    > + }
    > + return 0;
    > +}
    > +
    > +/*
    > + * sfi_map_table()
    > + *
    > + * Return address of mapped table
    > + * Check for common case that we can re-use mapping to SYST,
    > + * which requires syst_pa, syst_va to be initialized.
    > + */
    > +struct sfi_table_header *sfi_map_table(u64 pa)
    > +{
    > + struct sfi_table_header *th;
    > + u32 length;
    > +
    > + if (!TABLE_ON_PAGE(syst_pa, pa, sizeof(struct sfi_table_header)))
    > + th = sfi_map_memory(pa, sizeof(struct sfi_table_header));
    > + else
    > + th = (void *)syst_va - (syst_pa - pa);

    The '- (syst_pa - pa)' arithmetics is correct but rather unreadable
    - it should be something like:

    th = (void *)syst_va + (pa - syst_pa);

    To show that we return an offset into the SYST table - instead of
    the double negation.

    > +
    > + /* If table fits on same page as its header, we are done */
    > + if (TABLE_ON_PAGE(th, th, th->length))
    > + return th;
    > +
    > + /* entire table does not fit on same page as SYST */

    Nit: please capitalize comments consistently, by starting them with
    a capital letter.

    > + length = th->length;
    > + if (!TABLE_ON_PAGE(syst_pa, pa, sizeof(struct sfi_table_header)))
    > + sfi_unmap_memory(th, sizeof(struct sfi_table_header));
    > +
    > + return sfi_map_memory(pa, length);

    tricky.

    > +}
    > +
    > +/*
    > + * sfi_unmap_table()
    > + *
    > + * undoes effect of sfi_map_table() by unmapping table
    > + * if it did not completely fit on same page as SYST.
    > + */
    > +void sfi_unmap_table(struct sfi_table_header *th)
    > +{
    > + if (!TABLE_ON_PAGE(syst_va, th, th->length))
    > + sfi_unmap_memory(th, TABLE_ON_PAGE(th, th, th->length) ?
    > + sizeof(*th) : th->length);
    > +}
    > +
    > +/*
    > + * sfi_get_table()
    > + *
    > + * Search SYST for the specified table.
    > + * return the mapped table
    > + */
    > +static struct sfi_table_header *sfi_get_table(char *signature, char *oem_id,
    > + char *oem_table_id, unsigned int flags)
    > +{
    > + struct sfi_table_header *th;
    > + u32 tbl_cnt, i;
    > +
    > + tbl_cnt = SFI_GET_NUM_ENTRIES(syst_va, u64);
    > +
    > + for (i = 0; i < tbl_cnt; i++) {
    > + th = sfi_map_table(syst_va->pentry[i]);
    > + if (!th)
    > + return NULL;
    > +
    > + if (strncmp(th->signature, signature, SFI_SIGNATURE_SIZE))
    > + goto loop_continue;
    > +
    > + if (oem_id && strncmp(th->oem_id, oem_id, SFI_OEM_ID_SIZE))
    > + goto loop_continue;
    > +
    > + if (oem_table_id && strncmp(th->oem_table_id, oem_table_id,
    > + SFI_OEM_TABLE_ID_SIZE))
    > + goto loop_continue;
    > +
    > + return th; /* success */
    > +loop_continue:
    > + sfi_unmap_table(th);
    > + }
    > +
    > + return NULL;
    > +}

    This loop should be cleaned up in the same fashion as i suggested
    for sfi_acpi_get_table().

    > +
    > +void sfi_put_table(struct sfi_table_header *table)
    > +{
    > + if (!ON_SAME_PAGE(((void *)table + table->length),
    > + (void *)syst_va + syst_va->header.length)
    > + && !ON_SAME_PAGE(table, syst_va))
    > + sfi_unmap_memory(table, table->length);

    syst_va should probably be a void * to begin with - that avoids the
    second cast.

    I'm quite uneasy about the many open-coded ON_SAME_PAGE()
    conditions. That logic should perhaps be in sfi_map/unmap_memory()
    instead? (with an additional 'finegrained_len' argument to it or
    so.)

    > +}
    > +
    > +/* find table with signature, run handler on it */
    > +int sfi_table_parse(char *signature, char *oem_id, char *oem_table_id,
    > + unsigned int flags, sfi_table_handler handler)
    > +{
    > + int ret = 0;
    > + struct sfi_table_header *table = NULL;
    > +
    > + if (sfi_disabled || !handler || !signature)
    > + return -EINVAL;
    > +
    > + table = sfi_get_table(signature, oem_id, oem_table_id, flags);
    > + if (!table)
    > + return -EINVAL;
    > +
    > + ret = handler(table);
    > + sfi_put_table(table);
    > + return ret;
    > +}
    > +EXPORT_SYMBOL_GPL(sfi_table_parse);

    the error conditions could be written cleaner and shorter as:

    int sfi_table_parse(char *signature, char *oem_id, char *oem_table_id,
    unsigned int flags, sfi_table_handler handler)
    {
    struct sfi_table_header *table = NULL;
    int ret = -EINVAL;

    if (sfi_disabled || !handler || !signature)
    goto err;

    table = sfi_get_table(signature, oem_id, oem_table_id, flags);
    if (!table)
    goto err;

    ret = handler(table);
    sfi_put_table(table);
    err:
    return ret;
    }

    Thanks,

    Ingo


    \
     
     \ /
      Last update: 2009-07-10 09:43    [W:3.133 / U:0.136 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site