On Mon, 2008-05-05 at 15:47 +0800, Jason Jin wrote: > This MSI driver can be used on 83xx/85xx/86xx board. > In this driver, virtual interrupt host and chip were > setup. There are 256 MSI interrupts in this host, Every 32 > MSI interrupts cascaded to one IPIC/MPIC interrupt. > The chip was treated as edge sensitive and some necessary > functions were setup for this chip. > > Before using the MSI interrupt, PCI/PCIE device need to > ask for a MSI interrupt in the 256 MSI interrupts. A 256bit > bitmap show which MSI interrupt was used, reserve bit in > the bitmap can be used to force the device use some designate > MSI interrupt in the 256 MSI interrupts. Sometimes this is useful > for testing the all the MSI interrupts. The msi-available-ranges > property in the dts file was used for this purpose. > > Signed-off-by: Jason Jin <[EMAIL PROTECTED]>
Hi Jason,
Just a couple of comments below:
> diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
> new file mode 100644
> index 0000000..c53f716
> --- /dev/null
> +++ b/arch/powerpc/sysdev/fsl_msi.c
> @@ -0,0 +1,442 @@
> +/*
> + * Copyright (C) 2007-2008 Freescale Semiconductor, Inc. All rights reserved.
> + *
> + * Author: Tony Li <[EMAIL PROTECTED]>
> + * Jason Jin <[EMAIL PROTECTED]>
> + *
> + * 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; version 2 of the
> + * License.
> + *
> + */
> +#include <linux/irq.h>
> +#include <linux/bootmem.h>
> +#include <linux/bitmap.h>
> +#include <linux/msi.h>
> +#include <linux/pci.h>
> +#include <linux/of_platform.h>
> +#include <sysdev/fsl_soc.h>
> +#include <asm/prom.h>
> +#include <asm/hw_irq.h>
> +#include <asm/ppc-pci.h>
> +#include "fsl_msi.h"
> +
> +struct fsl_msi_feature {
> + u32 fsl_pic_ip;
> + u32 msiir_offset;
> +};
> +
> +/* A bit ugly, can we get this from the pci_dev somehow? */
This comment is old (from my code) and should go.
> +static struct fsl_msi *fsl_msi;
> +
> +static inline u32 fsl_msi_read(u32 __iomem *base, unsigned int reg)
> +{
> + return in_be32(base + (reg >> 2));
> +}
> +
> +static inline void fsl_msi_write(u32 __iomem *base,
> + unsigned int reg, u32 value)
> +{
> + out_be32(base + (reg >> 2), value);
> +}
> +
> +/*
> + * We do not need this actually. The MSIR register has been read once
> + * in the cascade interrupt. So, this MSI interrupt has been acked
> +*/
> +static void fsl_msi_end_irq(unsigned int virq)
> +{
> +}
> +
> +static struct irq_chip fsl_msi_chip = {
> + .mask = mask_msi_irq,
> + .unmask = unmask_msi_irq,
> + .ack = fsl_msi_end_irq,
> + .typename = " FSL-MSI ",
> +};
> +
> +static int fsl_msi_host_match(struct irq_host *h, struct device_node *node)
> +{
> + /* Exact match, unless node is NULL */
> + return h->of_node == NULL || h->of_node == node;
> +}
Are you sure you want this as your match routine? It looks wrong to me.
You won't ever create a fsl_msi in your probe routine unless you have a
device node, so AFAICT the of_node == NULL is only ever going to match
some _other_ irqhost.
> diff --git a/arch/powerpc/sysdev/fsl_msi.h b/arch/powerpc/sysdev/fsl_msi.h
> new file mode 100644
> index 0000000..0449c96
> --- /dev/null
> +++ b/arch/powerpc/sysdev/fsl_msi.h
> @@ -0,0 +1,31 @@
> +#ifndef _POWERPC_SYSDEV_FSL_MSI_H
> +#define _POWERPC_SYSDEV_FSL_MSI_H
This should have a copyright header.
> +#define NR_MSI_REG 8
> +#define IRQS_PER_MSI_REG 32
> +#define NR_MSI_IRQS (NR_MSI_REG * IRQS_PER_MSI_REG)
> +
> +#define FSL_PIC_IP_MASK 0x0000000F
> +#define FSL_PIC_IP_MPIC 0x00000001
> +#define FSL_PIC_IP_IPIC 0x00000002
> +
> +struct fsl_msi {
> + /* Device node of the MSI interrupt*/
> + struct device_node *of_node;
> +
> + struct irq_host *irqhost;
> +
> + unsigned long cascade_irq;
> +
> + u32 msi_addr_lo;
> + u32 msi_addr_hi;
> + void __iomem *msi_regs;
> + u32 feature;
> +
> + unsigned long *fsl_msi_bitmap;
> + spinlock_t bitmap_lock;
> + const char *name;
I don't see where this is used?
cheers
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
