Hemant Pedanekar <hema...@ti.com> writes:

> The driver code is organized in two parts, DM646x specific part and 'generic'
> part which could be easily adopted on other platform/SoCs having same PCI core
> as in DM6467.

First of all, sorry for the *huge* delay in reviewing this series.
It's been > 2 months since this original posting.  Feel free to harass
me more often if I let things slip more than a couple weeks.

> Also overrides io accessor functions to support  PCI IO.
>
> Currently PCI interrupt mapping (INTA to D) is not supported as the EVM has no
> PCI interrupt pins connected to interript controller.

A link to the PCI doc (spruer2) would be helpful here too.

> Signed-off-by: Hemant Pedanekar <hema...@ti.com>

Overall, this looks pretty good.  After some general comments, this
should probably be posted to linux-arm-kernel for some broader review
by folks that may have some more PCI background.  After that, I'll
pull into davinci git.

Some general comments:

- Documentation should follow kerneldoc style
- see preferred multi-line comment sytle in Documentation/CodingStyle
- do you want to enable DEBUG for the pci-* files when CONFIG_PCI_DEBUG=y?

> ---
>  arch/arm/mach-davinci/include/mach/dm646x.h |    5 +
>  arch/arm/mach-davinci/include/mach/io.h     |  222 ++++++++++++
>  arch/arm/mach-davinci/include/mach/pci.h    |   44 +++
>  arch/arm/mach-davinci/pci-dm646x.c          |  139 ++++++++
>  arch/arm/mach-davinci/pci-generic.c         |  493 
> +++++++++++++++++++++++++++

not a big deal, but I think pci-common.c is probably a better name.

>  arch/arm/mach-davinci/pci.h                 |   74 ++++
>  6 files changed, 977 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-davinci/include/mach/pci.h
>  create mode 100644 arch/arm/mach-davinci/pci-dm646x.c
>  create mode 100644 arch/arm/mach-davinci/pci-generic.c
>  create mode 100644 arch/arm/mach-davinci/pci.h
>
> diff --git a/arch/arm/mach-davinci/include/mach/dm646x.h 
> b/arch/arm/mach-davinci/include/mach/dm646x.h
> index feb1e02..d6d713b 100644
> --- a/arch/arm/mach-davinci/include/mach/dm646x.h
> +++ b/arch/arm/mach-davinci/include/mach/dm646x.h
> @@ -24,6 +24,11 @@
>  
>  #define DM646X_ATA_REG_BASE          (0x01C66000)
>  
> +/* System module register offsets */
> +#define PINMUX0                              0x00
> +#define PINMUX1                              0x04
> +#define BOOTCFG                              0x14
> +
>  void __init dm646x_init(void);
>  void __init dm646x_init_ide(void);
>  void __init dm646x_init_mcasp0(struct snd_platform_data *pdata);
> diff --git a/arch/arm/mach-davinci/include/mach/io.h 
> b/arch/arm/mach-davinci/include/mach/io.h
> index 62b0a90..67b98d0 100644
> --- a/arch/arm/mach-davinci/include/mach/io.h
> +++ b/arch/arm/mach-davinci/include/mach/io.h
> @@ -17,7 +17,229 @@
>   * We don't actually have real ISA nor PCI buses, but there is so many
>   * drivers out there that might just work if we fake them...
>   */
> +#if !defined CONFIG_PCI || defined __ASSEMBLER__
>  #define __io(a)                      __typesafe_io(a)
> +#else
> +#include <mach/pci.h>
> +/*
> + * Here we provide DMSoC to PCI I/O space access as PCI I/O is performed
> + * indirectly by accessing PCI IO Register.
> + *
> + * We will still use the default I/O (memory mapped for ARM)  for any access
> + * which is outside the allotted PCI I/O window. Note: In this case we 
> replicate
> + * the default inx/outx behavior as found in include/asm-arm/io.h (except of
> + * course, addition of sequence point, which is not needed in our case).
> + */
> +#define CFG_PCIM_IO_START               (PCIBIOS_MIN_IO)
> +#define CFG_PCIM_IO_END                 0x4BFFFFFF
> +
> +#define      outb(v, p)                      _davinci_outb(v, p)
> +#define      outw(v, p)                      _davinci_outw(v, p)
> +#define      outl(v, p)                      _davinci_outl(v, p)
> +
> +#define      outsb(p, d, l)                  _davinci_outsb(p, d, l)
> +#define      outsw(p, d, l)                  _davinci_outsw(p, d, l)
> +#define      outsl(p, d, l)                  _davinci_outsl(p, d, l)
> +
> +#define      inb(p)                          _davinci_inb(p)
> +#define      inw(p)                          _davinci_inw(p)
> +#define      inl(p)                          _davinci_inl(p)
> +
> +#define      insb(p, d, l)                   _davinci_insb(p, d, l)
> +#define      insw(p, d, l)                   _davinci_insw(p, d, l)
> +#define      insl(p, d, l)                   _davinci_insl(p, d, l)
> +
> +#define IS_PCI_IO(p)                 (((p) >= PCIBIOS_MIN_IO) \
> +                                             && ((p) <= PCIBIOS_MAX_IO))

This is just a minor optimization, but I wonder if this would be a
little faster without having to do two compares for every access.

Since "normal" users of the accessors will all be using virtual
addresses and since the PCI addresses will always be physical, the
access functions could be slightly faster if they just did a raw
access for any virtual address and a PCI access otherwise.  Something
like:

      if (addr >= VMALLOC_START)
              __raw_<access>
              return;

      /* then do PCI function here */

> +/*
> + * I/O Functions
> + */
> +#define PCI_ACCESS_TYPE_CFG             0
> +#define PCI_ACCESS_TYPE_IO              1
> +extern int ti_pci_io_write(u32 addr, int size, u32 value);
> +extern int ti_pci_io_read(u32 addr, int size, u32 *value);
> +
> +static inline void _davinci_outb(u8 value, u32 addr)
> +{
> +     if (IS_PCI_IO(addr))
> +             ti_pci_io_write(addr, 1, (u32)value);
> +     else
> +             __raw_writeb(value, __typesafe_io(addr));
> +}
> +
> +static inline void _davinci_outsb(u32 addr, const void *data, u32 blen)
> +{
> +     while (blen--)
> +             _davinci_outb(*(const u8 *)data++, (u32)((u8 *)addr++));
> +}
> +
> +static inline void _davinci_outw(u16 value, u32 addr)
> +{
> +     if (IS_PCI_IO(addr))
> +             ti_pci_io_write(addr, 2, (u32)value);
> +     else
> +             __raw_writew(cpu_to_le16(value), __typesafe_io(addr));
> +}
> +
> +static inline void _davinci_outsw(u32 addr, const void *data, u32 wlen)
> +{
> +     while (wlen--)
> +             _davinci_outw(*(const u16 *)data++, (u32)((u16 *)addr++));
> +}
> +
> +static inline void _davinci_outl(u32 value, u32 addr)
> +{
> +     if (IS_PCI_IO(addr))
> +             ti_pci_io_write(addr, 4, (u32)value);
> +     else
> +             __raw_writel(cpu_to_le32(value), __typesafe_io(addr));
> +}
> +
> +static inline void _davinci_outsl(u32 addr, const void *data, u32 llen)
> +{
> +     while (llen--)
> +             _davinci_outl(*(const u32 *)data++, (u32)((u32 *)addr++));
> +}
> +
> +static inline u8 _davinci_inb(u32 addr)
> +{
> +     if (IS_PCI_IO(addr)) {
> +             u32 value;
> +             ti_pci_io_read(addr, 1, &value);
> +             return (u8)value;
> +     } else {
> +             return __raw_readb(__typesafe_io(addr));
> +     }

extra braces not needed here, or several places below

> +}
> +
> +static inline void _davinci_insb(u32 addr, void *data, u32 blen)
> +{
> +     while (blen--)
> +             *(u8 *)data++ = _davinci_inb((u32)((u8 *)addr++));
> +}
> +
> +static inline u16 _davinci_inw(u32 addr)
> +{
> +     if (IS_PCI_IO(addr)) {
> +             u32 value;
> +             ti_pci_io_read(addr, 2, &value);
> +             return (u16)value;
> +     } else {
> +             return le16_to_cpu(__raw_readw(__typesafe_io(addr)));
> +     }
> +}
> +
> +static inline void _davinci_insw(u32 addr, void *data, u32 wlen)
> +{
> +     while (wlen--)
> +             *(u16 *)data++ = _davinci_inw((u32)((u16 *)addr++));
> +}
> +
> +static inline u32 _davinci_inl(u32 addr)
> +{
> +     if (IS_PCI_IO(addr)) {
> +             u32 value;
> +             ti_pci_io_read(addr, 4, &value);
> +             return value;
> +     } else {
> +             return le32_to_cpu(__raw_readl(__typesafe_io(addr)));
> +     }
> +}
> +
> +static inline void _davinci_insl(u32 addr, void *data, u32 llen)
> +{
> +     while (llen--)
> +             *(u32 *)data++ = _davinci_inl((u32)((u32 *)addr++));
> +}
> +
> +static inline unsigned int _davinci_ioread8(void __iomem *port)
> +{
> +     return (unsigned int)_davinci_inb((unsigned long __force)port);
> +}
> +
> +static inline void _davinci_ioread8_rep(void __iomem *port, void *dst,
> +                                             unsigned long count)
> +{
> +     _davinci_insb((unsigned long __force)port, dst, count);
> +}
> +
> +static inline unsigned int _davinci_ioread16(void __iomem *port)
> +{
> +     return (unsigned int)_davinci_inw((unsigned long __force)port);
> +}
> +
> +static inline void _davinci_ioread16_rep(void __iomem *port, void *dst,
> +                                             unsigned long count)
> +{
> +     _davinci_insw((unsigned long __force)port, dst, count);
> +}
> +
> +static inline unsigned int _davinci_ioread32(void __iomem *port)
> +{
> +     return (unsigned int)_davinci_inl((unsigned long __force)port);
> +}
> +
> +static inline void _davinci_ioread32_rep(void __iomem *port, void *dst,
> +                                             unsigned long count)
> +{
> +     _davinci_insl((unsigned long __force)port, dst, count);
> +}
> +
> +static inline void _davinci_iowrite8(u8 value, void __iomem *port)
> +{
> +     _davinci_outb(value, (unsigned long __force)port);
> +}
> +
> +static inline void _davinci_iowrite8_rep(void __iomem *port, const void *src,
> +                                     unsigned long count)
> +{
> +     _davinci_outsb((unsigned long __force)port, src, count);
> +}
> +
> +static inline void _davinci_iowrite16(u16 value, void __iomem *port)
> +{
> +     _davinci_outw(value, (unsigned long __force)port);
> +}
> +
> +static inline void _davinci_iowrite16_rep(void __iomem *port, const void 
> *src,
> +                                     unsigned long count)
> +{
> +     _davinci_outsw((unsigned long __force)port, src, count);
> +}
> +
> +static inline void _davinci_iowrite32(u32 value, void __iomem *port)
> +{
> +     _davinci_outl(value, (unsigned long __force)port);
> +}
> +
> +static inline void _davinci_iowrite32_rep(void __iomem *port, const void 
> *src,
> +                                     unsigned long count)
> +{
> +     _davinci_outsl((unsigned long __force)port, src, count);
> +}
> +

A minor readability issue, but instead of blen, wlen and llen, can you
just use count for all of them.

> +#define ioread8(p)                   _davinci_ioread8(p)
> +#define ioread16(p)                  _davinci_ioread16(p)
> +#define ioread32(p)                  _davinci_ioread32(p)
> +
> +#define iowrite8(v, p)                       _davinci_iowrite8(v, p)
> +#define iowrite16(v, p)                      _davinci_iowrite16(v, p)
> +#define iowrite32(v, p)                      _davinci_iowrite32(v, p)
> +
> +#define ioread8_rep(p, d, c)         _davinci_ioread8_rep(p, d, c)
> +#define ioread16_rep(p, d, c)                _davinci_ioread8_rep(p, d, c)
> +#define ioread32_rep(p, d, c)                _davinci_ioread8_rep(p, d, c)
> +
> +#define iowrite8_rep(p, s, c)                _davinci_iowrite8_rep(p, s, c)
> +#define iowrite16_rep(p, s, c)               _davinci_iowrite16_rep(p, s, c)
> +#define iowrite32_rep(p, s, c)               _davinci_iowrite32_rep(p, s, c)
> +
> +#define      ioport_map(port, nr)            ((void __iomem *)port)
> +#define      ioport_unmap(addr)
> +
> +#endif /* CONFIG_PCI */
> +
>  #define __mem_pci(a)         (a)
>  #define __mem_isa(a)         (a)
>  
> diff --git a/arch/arm/mach-davinci/include/mach/pci.h 
> b/arch/arm/mach-davinci/include/mach/pci.h
> new file mode 100644
> index 0000000..46de6b2
> --- /dev/null
> +++ b/arch/arm/mach-davinci/include/mach/pci.h
> @@ -0,0 +1,44 @@
> +/*
> + * pci.h
> + *  Description:
> + *  Resource data for PCI host. Currently only has DM6467 specific 
> information
> + *  since other DaVinci family SoCs do not have PCI Host support.
> + *
> + * Copyright (C) 2009 Texas Instruments Incorporated - http://www.ti.com/
> + *
> + * 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.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#ifndef __MACH_PCI_H
> +#define __MACH_PCI_H
> +
> +/* Enumeration flag: Checked during PCI Enumeration of Bridges */
> +#define pcibios_assign_all_busses()     1
> +
> +/*
> + * PCI Resource allocation
> + */
> +
> +/* PCI IO window.
> + * This could be set anywhere in the 4G space by adjusting PCIBIOS_MIN_IO and
> + * PCIBIOS_MAX_IO, which, in turn are used by in/out macros to distinguish
> + * between PCI IO and normal MMIO.
> + */
> +
> +/* Using 32M reserved window from DM6467 memory map as PCI IO region */
> +#define PCIBIOS_MIN_IO               (0x4A000000)
> +#define PCIBIOS_MAX_IO               (0x4BFFFFFF)
> +
> +/* PCI Memory window base */
> +#define PCIBIOS_MIN_MEM              (0x30000000)
> +
> +/* PCI Control register base (backend) */
> +#define PCICTL_REG_BASE                      (0x01C1A000)
> +
> +#endif /* !__MACH_PCI_H */
> diff --git a/arch/arm/mach-davinci/pci-dm646x.c 
> b/arch/arm/mach-davinci/pci-dm646x.c
> new file mode 100644
> index 0000000..636cc4c
> --- /dev/null
> +++ b/arch/arm/mach-davinci/pci-dm646x.c
> @@ -0,0 +1,139 @@
> +/*
> + * pci-dm646x.c
> + *  Description:
> + *  DM6467 (and in some parts, EVM) specific PCI initialization.
> + *
> + * Copyright (C) 2009 Texas Instruments Incorporated - http://www.ti.com/
> + *
> + * 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.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#undef DEBUG
> +
> +#include <linux/kernel.h>       /* pr_ wrappers */
> +#include <linux/pci.h>               /* PCI data structures */
> +#include <asm/mach/pci.h>    /* hw_pci */
>
> +#include <linux/types.h>
> +
> +#include "pci.h"             /* Platform spec */
> +
> +#include <mach/cputype.h>
> +#include <mach/dm646x.h>     /* SoC reg  defs, __REG, IO info etc */
> +#include <linux/clk.h>               /* For clk_enable interface */

The extra comments after the #includes aren't needed there and tend to
get stale.  Just drop them.

> +
> +/* NOTE:
> + * Most of the code in this file assumes that it is runnig as PCI Host and
> + * configures the system accordingly. The only place where it is checked if 
> we
> + * are booted through PCI Boot, is the function dm646x_pci_init (below) 
> before
> + * it registers with the bios. Thus, if we were not hosting the PCI bus the
> + * other functions wouldn;t get called anyway as the pci bios will not be
> + * invoked.
> + */

FYI... I tried this series on top of current davinci git, enabled
CONFIG_PCI and tested on my standalone dm646x EVM.  The kernel reboots
immediately on startup.  Is there a way to check for neither PCI host
or target and abort the init phase?

> +/* Forward declarations */
> +static void __init dm646x_pci_preinit(void);
> +static int __init dm646x_pci_map_irq(struct pci_dev *dev, u8 slot, u8 pin);
> +
> +/* Specify the physical address and size information for Host windows to be
> + * configured in BAR0-5 registers. Note that _ALL_ of the 6 entries must be
> + * filled. For entries with no real window to be mapped, set 
> bar_window_size=0.
> + */
> +struct pcihost_bar_cfg bar_cfg[6] = {
> +     { PHYS_OFFSET /* bar_window_phys */ , 0x8000000 /* bar_window_size */},
> +     { 0                                 , 0                              },
> +     { 0                                 , 0                              },
> +     { 0                                 , 0                              },
> +     { 0                                 , 0                              },
> +     { 0                                 , 0                              }
> +};

Hmm, do you really want to expose the entire memory map to the host by
default?  Seems like something that a board file should be allowed to
configure and override.

> +/* Plug into Linux PCI BIOS Interface */
> +static struct hw_pci davinci_pci __initdata = {
> +     .nr_controllers     = 1,
> +     .setup              = ti_pci_setup,
> +     .scan               = ti_pci_scan,
> +     .preinit            = dm646x_pci_preinit,
> +     .postinit           = NULL,
> +     .swizzle            = pci_std_swizzle,       /* Using default Linux
> +                                                     ARM-BIOS algorithm */
> +     .map_irq            = dm646x_pci_map_irq
> +};
> +
> +/* dm646x_pci_preinit
> + *  This function handles initializations to do before accessing PCI bus. 
> This
> + *  function is optional and most of the things done here could be handled in
> + *  board/SoC level init.
> + */
> +void dm646x_pci_preinit(void)
> +{
> +     /*
> +      * Use clk_enable interface to do the PSC init as well as take care of
> +      * pin muxing for PCI  module domain
> +      */
> +     struct clk *pci_clk;
> +
> +     pr_info("PCI: Enabling Clock...\n");
> +     pci_clk = clk_get(NULL, "pci");
> +     if (IS_ERR(pci_clk))
> +             return;
> +
> +     clk_enable(pci_clk);
> +
> +     /* At tis point, we assume here that the board level code has taken care
> +      * of driving RST# over PCI Bus (or, if not, then already taken care in
> +      * H/W - this is default behavior of the EVM)
> +      */
> +}
> +
> +int dm646x_pci_map_irq(struct pci_dev *dev, u8 slot, u8 pin)
> +{
> +     /*
> +      * Standard EVM doesn't have any dedicated lines connected to PCI
> +      * Interrupts.
> +      */
> +
> +     return -1;
> +}
> +
> +/* dm646x_pci_init
> + *  PCI system initialization. Fills in required information for PCI BIOS to
> + *  perrform enumeratios and invokes pci_common_init.
> + */
> +static int __init dm646x_pci_init(void)
> +{
> +     if (cpu_is_davinci_dm646x()) {

Is this cpu_is* necessary?  we're in dm646x specific code already, right?

> +             /* Since DM646x can act either as PCI host or target, we skip
> +              * setting up PCI BIOS for enumeration if we are 'target'.
> +              * This is determined by checking BOOTCFG BOOTMODE[0:3] and
> +              * PCIEN values.
> +              * Note : BOOTCFG values are latched across soft resets and thus
> +              * the check below cannot detect any change in actual boot mode.
> +              */
> +             u32 bootcfg = __raw_readl(
> +                             (IO_ADDRESS(DAVINCI_SYSTEM_MODULE_BASE)
> +                              + BOOTCFG));
> +             u32 bootmode = bootcfg & 0xf;
> +
> +             pr_info("PCI: bootcfg = %#x, bootmode = %#x\n",
> +                     bootcfg, bootmode);
> +
> +             if (!((bootcfg & BIT(16))
> +                     && ((bootmode == 2) || (bootmode == 3)))) {
> +                     pr_info("PCI: Invoking PCI BIOS...\n");
> +                     pci_common_init(&davinci_pci);
> +             } else {
> +                     pr_info("PCI: Skipping PCI Host setup...\n");
> +             }
> +     }
> +
> +     return 0;
> +}
> +
> +subsys_initcall(dm646x_pci_init);
> diff --git a/arch/arm/mach-davinci/pci-generic.c 
> b/arch/arm/mach-davinci/pci-generic.c
> new file mode 100644
> index 0000000..016b83d
> --- /dev/null
> +++ b/arch/arm/mach-davinci/pci-generic.c
> @@ -0,0 +1,493 @@
> +/*
> + * pci-generic.c
> + *  Description:
> + *  Generic part of PCI Host Driver for TI PCI Module. Note that though code 
> in
> + *  this file is supposed to be 'generic' it might still require some 
> tweaking
> + *  when porting to other boards/platform using same TI PCI Module
> + *
> + * Copyright (C) 2009 Texas Instruments Incorporated - http://www.ti.com/
> + *
> + * 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.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#undef DEBUG
> +
> +#include <linux/kernel.h>               /* pr_ wrappers */
> +#include <linux/pci.h>                  /* PCI data structures */
> +#include <asm/mach/pci.h>               /* hw_pci */
> +#include <linux/types.h>
> +#include <linux/interrupt.h>
> +#include <asm/irq.h>
> +
> +#include "pci.h"                             /* Configuration data */
> +
> +/*
> + *  PCI Register Offsets
> + */
> +#define PCISTATSET                      0x010
> +#define PCISTATCLR                      0x014
> +#define PCIHINTSET                      0x020
> +#define PCIHINTCLR                      0x024
> +#define PCIBINTSET                      0x030
> +#define PCIBINTCLR                      0x034
> +#define PCIVENDEVMIR                    0x100
> +#define PCICSRMIR                       0x104
> +#define PCICLREVMIR                     0x108
> +#define PCICLINEMIR                     0x10C
> +#define PCIBARMSK(n)                    (0x110 + (4 * n))     /* 6 Registers 
> */
> +#define PCISUBIDMIR                     0x12C
> +#define PCICPBPTRMIR                    0x134
> +#define PCILGINTMIR                     0x13C
> +#define PCISLVCNTL                      0x180
> +#define PCIBARTRL(n)                    (0x1C0 + (4 * n))     /* 6 Registers 
> */
> +#define PCIBARMIR(n)                    (0x1E0 + (4 * n))     /* 6 Registers 
> */
> +#define PCIMCFGDAT                      0x300
> +#define PCIMCFGADR                      0x304
> +#define PCIMCFGCMD                      0x308
> +#define PCIMSTCFG                       0x310
> +#define PCIADDSUB(n)                    (0x314 + (4 * n))     /* 32 
> Registers */
> +#define PCIVENDEVPRG                    0x394
> +#define PCICLREVPRG                     0x39C
> +#define PCISUBIDPRG                     0x3A0
> +#define PCIMAXLGPRG                     0x3A4
> +#define PCICFGDONE                      0x3AC
> +
> +#define PCI_WAIT_FOR_RDY()              pci_wait_for_rdy()
> +
> +#define CFG_PCIM_CMD_IO                 BIT(2)
> +#define CFG_PCIM_CMD_CONFIG             (0)
> +#define CFG_PCIM_CMD_READ               BIT(0)
> +#define CFG_PCIM_CMD_WRITE              (0)
> +
> +/* Command Register Values */
> +#define PCIM_INTA_DIS        BIT(10)    /* Don't pass interrupt on INTA */
> +#define PCIM_SERR_EN         BIT(8)     /* Propagate error on SERR# */
> +#define PCIM_PERR_EN         BIT(6)     /* Consider parity error */
> +#define PCIM_MEMWRINV_EN     BIT(4)     /* Invalidate memory writes */
> +#define PCIM_BUS_MASTER      BIT(2)     /* Set as PCI master */
> +#define PCIM_MEMACC_EN       BIT(1)     /* Host as memory slave */
> +#define PCIM_IOACC_EN        BIT(0)     /* Host as IO device */
> +#define PCIM_SPLCYC_EN       BIT(3)     /* Could be used for sideband
> +                                        signalling (non-critical) */
> +/* Default value used for CSMIR register */
> +#define CFG_PCIM_CSR_VAL         (PCIM_SERR_EN | PCIM_MEMWRINV_EN    \
> +                                     | PCIM_BUS_MASTER               \
> +                                     | PCIM_MEMACC_EN                \
> +                                     | PCIM_INTA_DIS)
> +
> +#define PCIM_MAX_LAT_SHIFT        (24)
> +#define PCIM_MAX_LAT_MASK         (0xFF << PCIM_MAX_LAT_SHIFT)
> +#define PCIM_MAX_LAT_VAL          (CFG_PCIM_MAX_LAT << PCIM_MAX_LAT_SHIFT)
> +
> +#define PCIM_MIN_GRANT_SHIFT      (16)
> +#define PCIM_MIN_GRANT_MASK       (0xFF << PCIM_MIN_GRANT_SHIFT)
> +#define PCIM_MIN_GRANT_VAL        (CFG_PCIM_MIN_GRANT \
> +                                     << PCIM_MIN_GRANT_SHIFT)
> +
> +/* Error mask for errors to check on CFG/IO */
> +#define CFG_PCI_ERR_MASK       ((0xf << 28) | (1 < 24))
> +
> +/* Protect PCI accesses (config/io read/writes) */
> +static DEFINE_SPINLOCK(ti_pci_lock);
> +
> +static inline void dump_pci_status(u32 status)
> +{
> +     pr_debug("PCI: Error Status:\nData Parity Error: %d, "
> +             "System Error: %d\nMaster Abort: %d, "
> +             "Target Abort: %d\nParity Report: %d\n",
> +             ((status & BIT(31)) != 0), ((status & BIT(30)) != 0),
> +             ((status & BIT(29)) != 0), ((status & BIT(28)) != 0),
> +             ((status & BIT(24)) != 0));
> +}
> +
> +/* ti_pci_setup
> + *  This function does Host controller configuration before bios starts PCI 
> bus
> + *  enumeration (scan).
> + */
> +int ti_pci_setup(int nr, struct pci_sys_data *sys)
> +{
> +     int i, bar_en = 0;
> +     struct resource *res;
> +
> +     pr_info("PCI: Setting up Host Controller...\n");
> +     pr_debug("PCI: PCIADDSUB[0] @%#x\n", (u32)PCI_REGV(PCIADDSUB(0)));
> +
> +     if (nr != 0)
> +             return 0;
> +
> +     /* Make specified Host windows visible over PCI bus. This will enable
> +      * PCI Masters to perform DMA to Host System RAM (1:1 mapping of
> +      * course!).
> +      */
> +     for (i = 0; i < 6; i++) {
> +             u32 size = bar_cfg[i].bar_window_size;
> +             u32 addr = bar_cfg[i].bar_window_phys;
> +
> +             if (size) {
> +                     /* Ensure size in 16 B to 2 GB range */
> +                     if (size < 16)
> +                             size = 16;
> +                     else if (size > 0x80000000)
> +                             size = 0x80000000;
> +
> +                     if (size & (size - 1))
> +                             size = BIT(fls(size));
> +
> +                     bar_en |= BIT(i);
> +
> +                     /* 1:1 from PCI -> Host */
> +                     __raw_writel(addr, PCI_REGV(PCIBARTRL(i)));
> +
> +                     /* Setup PCI BAR Mirror register to bus address (= phys)
> +                      */
> +                     __raw_writel(addr, PCI_REGV(PCIBARMIR(i)));
> +             }
> +
> +             /* Set writable bits - masking for size */
> +             __raw_writel(~(size-1), PCI_REGV(PCIBARMSK(i)));
> +     }
> +
> +     /* Enable applicable  BAR windows in Slave control register and Set
> +      * CFG_DONE to enable PCI access
> +      */
> +     __raw_writel(((bar_en << 16) | 1), PCI_REGV(PCISLVCNTL));
> +
> +     /* IMPORTANT *** Change our class code from
> +      * PCI_BASE_CLASS_SIGNAL_PROCESSING(PCI_CLASS_SP_OTHER = 0x1180) to
> +      * PCI_CLASS_BRIDGE_HOST (0x0600) to prevent resource (BAR) assignment
> +      * to us. The PCI probe will be happy with whatever original BAR
> +      * settings we have!
> +      * If at all we want to restore the default class-subclass values, the
> +      * best place would be to set mirror register after returning from
> +      * pci_common_init () [in dm6467_pci_init()].
> +      */
> +     __raw_writel(((__raw_readl(PCI_REGV(PCICLREVMIR)) & 0xffff)
> +                     | (PCI_CLASS_BRIDGE_HOST << 16)),
> +                     PCI_REGV(PCICLREVMIR));
> +
> +     /*
> +      * Set PHYADDR <-> BUSADDR mapping for accessing 256MB PCI space
> +      */
> +
> +     /* Using Direct 1:1 mapping of DMSoC <-> PCI memory space */
> +     for (i = 0; i < CFG_PCIM_WINDOW_CNT; i++)
> +             __raw_writel((CFG_PCIM_MEM_START + (CFG_PCIM_WINDOW_SZ*i)),
> +                             PCI_REGV(PCIADDSUB(i)));
> +
> +
> +     /* Setup as PCI master */
> +     pr_debug("PCI: Default PCICSRMIR = %x\n",
> +                     __raw_readl(PCI_REGV(PCICSRMIR)));
> +
> +     __raw_writel((__raw_readl(PCI_REGV(PCICSRMIR)) | CFG_PCIM_CSR_VAL),
> +                     PCI_REGV(PCICSRMIR));
> +
> +     pr_debug("PCI: New PCICSRMIR = %x\n",
> +                     __raw_readl(PCI_REGV(PCICSRMIR)));
> +
> +     __raw_writel(((__raw_readl(PCI_REGV(PCILGINTMIR))
> +                             & ~PCIM_MAX_LAT_MASK & ~PCIM_MIN_GRANT_MASK)
> +                             | PCIM_MAX_LAT_VAL | PCIM_MIN_GRANT_VAL),
> +                     PCI_REGV(PCILGINTMIR));
> +
> +     /*
> +      * Setup resources which will be used assign BARs to targets during
> +      * scanning
> +      */
> +
> +     /* Overwrite the resources which were set up by default by
> +      * pcibios_init_hw
> +      */
> +     res = kzalloc(sizeof(*res) * 2, GFP_KERNEL);
> +     if (res == NULL) {
> +             panic("PCI: resource structure allocation failed.\n");
> +             /* Not reached */
> +             return 0;
> +     }

You shouldn't halt the whole kernel here on an alloc failure.  You
should use a WARN instead of panic and fail gracefully.


> +     res[0].start = PCIBIOS_MIN_IO;
> +     res[0].end = PCIBIOS_MAX_IO;
> +     res[0].name = "PCI I/O";
> +     res[0].flags = IORESOURCE_IO;
> +
> +     res[1].start = CFG_PCIM_MEM_START;
> +     res[1].end = CFG_PCIM_MEM_END;
> +     res[1].name = "PCI Memory";
> +     res[1].flags = IORESOURCE_MEM;
> +
> +     request_resource(&ioport_resource, &res[0]);
> +     request_resource(&iomem_resource, &res[1]);
> +
> +     sys->resource[0] = &res[0];
> +     sys->resource[1] = &res[1];
> +     sys->resource[2] = NULL;
> +
> +     return 1;
> +}
> +
> +static inline u32 get_config_addr(u8 bus, u16 devfn, int where)
> +{
> +     u32 addr;
> +
> +     /* Determine configuration cycle type depending upon the bus:
> +      * - TYPE 0 for PCI bus directly connected to host
> +      * - TYPE 1 for across bridge(s)
> +      */
> +     if (bus == 0) {
> +             /* TYPE 0 */
> +             addr = BIT(11 + PCI_SLOT(devfn)) | (PCI_FUNC(devfn) << 8)
> +                     | (where & ~3);
> +     } else {
> +             /* TYPE 1 */
> +             addr = (bus << 16) | (PCI_SLOT(devfn) << 11)
> +                     | (PCI_FUNC(devfn) << 8) | (where & ~3) | 1;
> +     }
> +
> +     return addr;
> +}
> +
> +static inline int get_byte_enables(u32 addr, u32 size)
> +{
> +     /*
> +      * Byte Enables (BE#[3:0]) will be generated taking into account AD[1:0]
> +      * and size of transaction (1-4 Byte). Refer following table:
> +      *  PCI_ADDR[1:0]       Starting Byte           BE#[3:0]
> +      * -----------------------------------------------------------------
> +      *  00                  Byte 0                  xxx0 (size = 1->4)
> +      *  01                  Byte 1                  xx01 (size = 1->3)
> +      *  10                  Byte 2                  x011 (size = 1->2)
> +      *  11                  Byte 3                  0111 (size = 1)
> +      *
> +      *  (Here 'x' can either be '0' (byte enabled) or '1' as required by
> +      *  'size')
> +      *
> +      *  As mentioned above, BE values are function of 'size' and AD[1:0] as
> +      *  well as size so we start by doing sanity check on combination of
> +      *  supplied parameters. We won't return any error as such, but any
> +      *  wrong combination will result into either all BE de-asserted or
> +      *  only/all possible BEs asserted. E.g.,
> +      *  size=0 -> All BEs de-asserted
> +      *  size>4 -> Only possible BEs de-asserted
> +      */
> +
> +     addr &= 3;
> +
> +     pr_debug("PCI: AD[1:0]:size = %01x:%01x, Byte enables = %#lx\n",
> +                     addr&3, size, (((BIT(size) - 1) << addr) & 0xf));
> +
> +     /* BE values are inverted for BE# */
> +     return ((BIT(size) - 1) << addr) & 0xf;
> +}
> +
> +/*
> + * Returns:
> + *  0   - When READY bit is not set within timeout, else non-zero.
> + */
> +static inline int pci_wait_for_rdy(void)
> +{
> +     int wait_cnt = CFG_PCI_READY_WAITCNT;
> +
> +     /* Note : This function doesn't check aborts since the READY bit it set
> +      * when bus aborts happen. Need to verify this is true for all kinds of
> +      * PCI access errors.
> +      */
> +     while (wait_cnt && !(__raw_readl(PCI_REGV(PCIMCFGCMD)) & BIT(31)))
> +             wait_cnt--;
> +
> +     return (wait_cnt != 0);
> +}
> +
> +static inline int pci_err(void)
> +{
> +     u32 status = __raw_readl(PCI_REGV(PCICSRMIR));
> +     if (status & CFG_PCI_ERR_MASK) {
> +             dump_pci_status(status);
> +             return status >> 24;
> +     } else {
> +             return 0;
> +     }
> +}
> +
> +static inline int pci_cfgio_read(u32 addr, int size, u32 *value,
> +                                     int be, int access_type)
> +{
> +     unsigned long flags;
> +     int ret_val = 0;
> +
> +     spin_lock_irqsave(&ti_pci_lock, flags);

Can taking the lock come after the check for READY?

> +     /* Check of READY. Continue even if not READY as we will catch the error
> +      * after operation. Same logic is applied for 'write' operation below...
> +      */
> +     if (!PCI_WAIT_FOR_RDY())
> +             pr_warning("PCI: CFG/IO not ready."
> +                     "Might need tuning CFG_PCI_READY_WAITCNT value.\n");
> +
> +     /* Clear aborts */
> +     __raw_writel((__raw_readl(PCI_REGV(PCICSRMIR)) | (0xFF << 24)),
> +                     PCI_REGV(PCICSRMIR));
> +
> +     __raw_writel(addr, PCI_REGV(PCIMCFGADR));
> +
> +     __raw_writel((BIT(31) | ((access_type == PCI_ACCESS_TYPE_CFG)
> +                             ? CFG_PCIM_CMD_CONFIG : CFG_PCIM_CMD_IO)
> +                     | CFG_PCIM_CMD_READ
> +                     | (be << 4)), PCI_REGV(PCIMCFGCMD));
> +
> +     /* Check for READY. */
> +     ret_val = !PCI_WAIT_FOR_RDY();
> +
> +     if (pci_err()) {
> +             pr_debug("PCI: CFG/IO Read @%#x Failed.\n", addr);
> +             *value = ~0;
> +             /* Clear aborts */
> +             __raw_writel((__raw_readl(PCI_REGV(PCICSRMIR)) | (0xFF << 24)),
> +                             PCI_REGV(PCICSRMIR));
> +             ret_val = -1;
> +     } else {
> +             *value = __raw_readl(PCI_REGV(PCIMCFGDAT));
> +             pr_debug("PCI: Config/IO read done, value = %x\n", *value);
> +     }
> +
> +     spin_unlock_irqrestore(&ti_pci_lock, flags);
> +     return ret_val;
> +}
> +
> +/*
> + * This is just a wrapper (exported) to provide BE value for I/O
> + */
> +int ti_pci_io_read(u32 addr, int size, u32 *value)
> +{
> +     int ret_val = pci_cfgio_read(addr, size, value,
> +                                     get_byte_enables(addr, size),
> +                                     PCI_ACCESS_TYPE_IO);
> +     *value >>= ((addr & 3)*8);
> +     pr_debug("PCI: IO read @%#x = %#x\n", addr, *value);
> +     return ret_val;
> +}
> +EXPORT_SYMBOL(ti_pci_io_read);
>
> +static int ti_pci_read_config(struct pci_bus *bus, unsigned int devfn,
> +                             int where, int size, u32 *value)
> +{
> +     u8 bus_num = bus->number;
> +
> +     /* Sanity checks */
> +     if ((bus_num == 0) && (PCI_SLOT(devfn) >= PCI_MAX_SLOTS))
> +             return PCIBIOS_DEVICE_NOT_FOUND;
> +
> +     pr_debug("PCI: Reading config[%x] for device %04x:%02x:%02x ...",
> +                     where, bus_num, PCI_SLOT(devfn), PCI_FUNC(devfn));
> +
> +     if (pci_cfgio_read(get_config_addr(bus_num, devfn, where), size,
> +                             value, get_byte_enables(where, size),
> +                             PCI_ACCESS_TYPE_CFG)) {
> +             /* Ignoring other error codes (> 0) */
> +             pr_debug("failed.");
> +             return PCIBIOS_DEVICE_NOT_FOUND;
> +     } else {
> +             *value >>= ((where & 3)*8);
> +             pr_debug("done, value = %x\n", *value);
> +             return PCIBIOS_SUCCESSFUL;
> +     }
> +}
> +
> +static int pci_cfgio_write(u32 addr, int size, u32 value,
> +                             int be, int access_type)
> +{
> +     unsigned long flags;
> +     int ret_val = 0;
> +
> +     spin_lock_irqsave(&ti_pci_lock, flags);
> +
> +     /* Check of READY */
> +     if (!PCI_WAIT_FOR_RDY())
> +             pr_warning("PCI: CFG/IO not ready."
> +                     "Might need tuning CFG_PCI_READY_WAITCNT value.\n");
> +
> +     /* Clear aborts */
> +     __raw_writel((__raw_readl(PCI_REGV(PCICSRMIR)) | (0xFF << 24)),
> +                     PCI_REGV(PCICSRMIR));
> +
> +     __raw_writel((value << ((addr & 3)*8)), PCI_REGV(PCIMCFGDAT));
> +     __raw_writel(addr, PCI_REGV(PCIMCFGADR));
> +
> +     __raw_writel((0x80000000 | ((access_type == PCI_ACCESS_TYPE_CFG)
> +                                     ? CFG_PCIM_CMD_CONFIG : CFG_PCIM_CMD_IO)
> +                     | CFG_PCIM_CMD_WRITE | (be << 4)),
> +                     PCI_REGV(PCIMCFGCMD));
> +
> +     /* Check for READY. */
> +     ret_val = !PCI_WAIT_FOR_RDY();
> +
> +     if (pci_err()) {
> +             pr_debug("PCI: CFG/IO Write @%#x Failed\n", addr);
> +             /* Clear aborts */
> +             __raw_writel((__raw_readl(PCI_REGV(PCICSRMIR)) | (0xFF << 24)),
> +                             PCI_REGV(PCICSRMIR));
> +             ret_val = -1;
> +     } else {
> +             pr_debug("PCI: Config/IO write done.\n");
> +     }
> +
> +     spin_unlock_irqrestore(&ti_pci_lock, flags);
> +     return ret_val;
> +}
> +
> +/*
> + * This is just a wrapper (exported) to provide BE value for I/O
> + */
> +int ti_pci_io_write(u32 addr, int size, u32 value)
> +{
> +     pr_debug("PCI: IO write @%#x = %#x\n", addr, value);
> +     return pci_cfgio_write(addr, size, value,
> +                             get_byte_enables(addr, size),
> +                             PCI_ACCESS_TYPE_IO);
> +}
> +EXPORT_SYMBOL(ti_pci_io_write);
>
> +static int ti_pci_write_config(struct pci_bus *bus, unsigned int devfn,
> +                             int where, int size, u32 value)
> +{
> +     u8 bus_num = bus->number;
> +
> +     /* Sanity checks */
> +     if ((bus_num == 0) && (PCI_SLOT(devfn) >= PCI_MAX_SLOTS))
> +             return PCIBIOS_DEVICE_NOT_FOUND;
> +
> +     pr_debug("PCI: Writing config[%x] = %x "
> +                     "for device %04x:%02x:%02x ...", where, value,
> +                     bus_num, PCI_SLOT(devfn), PCI_FUNC(devfn));
> +
> +     if (pci_cfgio_write(get_config_addr(bus_num, devfn, where),
> +                             size, value, get_byte_enables(where, size),
> +                             PCI_ACCESS_TYPE_CFG)) {
> +             /* Ignoring other error codes (> 0) */
> +             pr_debug("failed.\n");
> +             return PCIBIOS_DEVICE_NOT_FOUND;
> +     } else {
> +             pr_debug("done.\n");
> +             return PCIBIOS_SUCCESSFUL;
> +     }
> +}
> +
> +static struct pci_ops ti_pci_ops = {
> +     .read   = ti_pci_read_config,
> +     .write  = ti_pci_write_config,
> +};
> +
> +struct pci_bus *ti_pci_scan(int nr, struct pci_sys_data *sys)
> +{
> +     pr_info("PCI: Starting PCI scan...\n");
> +     if (nr == 0)
> +             return pci_scan_bus(0, &ti_pci_ops, sys);
> +
> +     return NULL;
> +}
> diff --git a/arch/arm/mach-davinci/pci.h b/arch/arm/mach-davinci/pci.h
> new file mode 100644
> index 0000000..7a07d2d
> --- /dev/null
> +++ b/arch/arm/mach-davinci/pci.h
> @@ -0,0 +1,74 @@
> +/*
> + * pci.h
> + *  Description:
> + *  PCI module specific types/configurations and definitions.
> + *
> + * Copyright (C) 2009 Texas Instruments Incorporated - http://www.ti.com/
> + *
> + * 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.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#ifndef __ARCH_PCI_H
> +#define __ARCH_PCI_H
> +
> +#include <mach/pci.h>
> +
> +/*
> + * Configuration options
> + */
> +
> +/* Could be updated with optimum PCILGINTMIR values */
> +#define CFG_PCIM_MAX_LAT             0xF
> +#define CFG_PCIM_MIN_GRANT           0xF
> +
> +#define CFG_PCI_INTSET_MASK          ((0x3 << 1) | (0x3 << 5))
> +#define CFG_PCI_INTCLR_MASK          (CFG_PCI_INTSET_MASK)
> +
> +#define PCI_MAX_SLOTS                        (21)        /* Only checked for 
> bus=0 as
> +                                                     IDSEL can be provided
> +                                                     only within AD[31:11] */
> +
> +#define CFG_PCIM_WINDOW_SZ   (0x00800000)        /* Master window size */

Use SZ_8M.

> +#define CFG_PCIM_WINDOW_CNT  (32)                /* Number of windows */
> +#define CFG_PCIM_MEM_START   (PCIBIOS_MIN_MEM)   /* PCI master memory map
> +                                                     base NOTE: We are using
> +                                                     1:1 mapping, i.e.,
> +                                                     0x30000000 from DMSoC
> +                                                     side is translated as
> +                                                     0x30000000 on PCI Bus */
> +#define CFG_PCIM_MEM_END     (CFG_PCIM_MEM_START                    \
> +                                     + (CFG_PCIM_WINDOW_SZ          \
> +                                             * CFG_PCIM_WINDOW_CNT) \
> +                                     - 1)    /* PCI master memory map size */

Since the WINDOW_CNT could be changed, for extra sanity, you should
probably check that this never goes past 256M.  Something like this
should work (untested):

#if CFG_PCIM_MEM_END >= SZ_256M
#error PCI memory size must be <= 256M
#endif

> +
> +
> +/* The default value below can be substituted with proper timeout value which
> + * taking into account various PCI system parameters (buses, devices/slots,
> + * average latency to gain PCI bus access etc) to have sufficiant time till 
> the
> + * master completes previous transaction.
> + */
> +#define CFG_PCI_READY_WAITCNT        0xffff
> +
> +/*
> + * Provide the virtual address of PCI Backend register at specified offset. 
> Uses
> + * SoC specific PCI register base for address calculation.
> + */
> +#define PCI_REGV(reg)           (IO_ADDRESS(PCICTL_REG_BASE) + reg)

Rather than use IO_ADDRESS() here, please use ioremap during init.

> +struct pcihost_bar_cfg {
> +    u32 bar_window_phys;
> +    u32 bar_window_size;
> +};
> +
> +/* Forward declarations */
> +extern int __init ti_pci_setup(int nr, struct pci_sys_data *);
> +extern struct  pci_bus * __init ti_pci_scan(int nr, struct pci_sys_data *);
> +extern struct pcihost_bar_cfg bar_cfg[6];
> +
> +#endif /* !__ARCH_PCI_H */

Kevin

_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to