[U-Boot] [PATCH v2] pci_ftpci100: Implementation FTPCI100 PCI driver

[Mike Frysinger <vapier@gentoo.org>]

On Thursday 20 October 2011 04:16:25 Macpaul Lin wrote:
> --- /dev/null
> +++ b/drivers/pci/pci_ftpci100.c
>
> +#ifndef __io
> +#define __io
> +#endif

this is most likely wrong.  drop it.

> +#include <asm/io.h>
> +#include <asm/types.h> /* u32, u16.... used by pci.h */
> +#include <common.h>
> +#include <malloc.h>
> +#include <pci.h>

the asm/ includes should come after the non-asm includes

> +struct pci_config devs[FTPCI100_MAX_FUNCTIONS];
> +struct pci_controller hose;

need to be static

> +void setup_pci_bar(unsigned int bus, unsigned int dev, unsigned func,
> +		unsigned char header, struct ftpci100_data *priv)

should be static

> +#ifdef CONFIG_FTPCI100_DEBUG
> +			printf("Allocated IO address 0x%X-" \
> +				"0x%X for Bus %d, Device %d, Function %d\n",
> +				priv->io_base,
> +				priv->io_base + size_mask, bus, dev, func);
> +#endif

use debug() and drop the CONFIG_FTPCI100_DEBUG, and fix globally in this file

> +		} else {
> +		/* Memory space */
> +			unsigned int is_64bit = ((tmp32 & 0x6) == 0x4);

indent that comment

> +void pci_bus_scan(struct ftpci100_data *priv)

should be static

> +#ifdef CONFIG_PCI_SCAN_SHOW
> +				printf("%06d  %03d  %03d  " \
> +					"%04d  %08x  %08x  " \
> +					"%03d  %08x  %06d  %08x\n",
> +					priv->ndevs, devs[priv->ndevs].bus,
> +					devs[priv->ndevs].dev,
> +					devs[priv->ndevs].func,
> +					devs[priv->ndevs].d_id,
> +					devs[priv->ndevs].v_id,
> +					devs[priv->ndevs].pin,
> +					devs[priv->ndevs].bar[0].addr,
> +					devs[priv->ndevs].bar[0].size,
> +					data32 >> 8);
> +				priv->ndevs++;
> +#endif

should that ndevs inc really be inside this ifdef ?  and would this be better 
as debug() ?

> +static int ftpci_probe(unsigned int addr_p)
> +{
> +	int ftpci_probed;
> +	unsigned int *addr = (unsigned int *) addr_p;
> +
> +	*(unsigned int *) addr = 0x80000000;
> +
> +	if (*(unsigned int *) addr == 0x80000000) {
> +		printf("Faraday ftpci100 PCI bridge probed ok\n");
> +		ftpci_probed = 1;
> +	} else {
> +		ftpci_probed = 0;
> +	}
> +
> +	*(unsigned int *) addr = 0x0;
> +	return ftpci_probed;
> +}

the LHS casts on addr make no sense.  drop them.

> +void ftpci_preinit(struct ftpci100_data *priv)

static

> +void pci_ftpci_init(void)

static

> +#ifdef CONFIG_PCI_SCAN_SHOW
> +	printf("Device  bus  dev  func  deviceID  vendorID  pin  address" \
> +		"   size    class\n");
> +#endif

debug()

> +	pci_hose_write_config_dword(&hose, bridge_num, PCI_MEM_BASE_SIZE1,
> +					FTPCI100_BASE_ADR_SIZE(1024));
> +	return;
> +}

drop the useless "return;"

> --- /dev/null
> +++ b/drivers/pci/pci_ftpci100.h
>
> +#define PCI_INT_MASK			0x4c
> +#define PCI_MEM_BASE_SIZE1		0x50
> +#define PCI_MEM_BASE_SIZE2		0x54
> +#define PCI_MEM_BASE_SIZE3		0x58
> ...
> +#define PCI_INTA_ENABLE			(1 << 22)
> +#define PCI_INTB_ENABLE			(1 << 23)
> +#define PCI_INTC_ENABLE			(1 << 24)
> +#define PCI_INTD_ENABLE			(1 << 25)
> ...
> +#define PCI_IRQ_LINES			4
> +
> +#define MAX_BUS_NUM			256
> +#define MAX_DEV_NUM			32
> +#define MAX_FUN_NUM			8
> +
> +#define PCI_MAX_BAR_PER_FUNC		6
> ...
> +struct pcibar {
> +	unsigned int size;
> +	unsigned int addr;
> +};
> +
> +struct pci_config {
> +	unsigned int bus;
> +	unsigned int dev;				/* device */
> +	unsigned int func;
> +	unsigned int pin;
> +	unsigned short v_id;				/* vendor id */
> +	unsigned short d_id;				/* device id */
> +	struct pcibar bar[PCI_MAX_BAR_PER_FUNC + 1];
> +};

these should be namespaced to your driver, or moved to a common pci include
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111020/417a7bd3/attachment.pgp