[U-Boot] [PATCH] IXP425: Fixing PCI access Part 1/1

[Wolfgang Denk]

Dear Stefan Althoefer,

In message <4938471d.iPiarO7PylTL2jNa%stefan.althoefer at web.de> you wrote:
> [PATCH] IXP425: Fixing PCI access
> 
> This patch fixes the PCI handling routines of the IXP port.
> It seems that this hasn't been touch for quite a while and
> u-boot PCI handling has changed since then (but nobody
> update IXP). Not even access to configuration space
> did work.
> 
> It was tested with Janz emPC-A400.

Coding style, and comments below "---", please.

> +static u32 ixp4xx_config_addr(u8 bus_num, pci_dev_t devfn, int where)
> +{
> +	u32 addr;

Empty line after declarations, please.

> +	if (!bus_num) {
> +		/* type 0 */
> +		addr = BIT(32-PCI_DEV(devfn)) | ((PCI_FUNC(devfn)) << 8) |
> +		    (where & ~3);

Indentation by TAB, please.

...
> +	addr = ixp4xx_config_addr(PCI_BUS(dev),dev, where & ~3);
> +	if( non_prefetch_read (addr, NP_CMD_CONFIGREAD, &retval) != OK ){

Space between '0' and '{', please.

> +	//addr = BIT ((31 - dev)) | (where & ~3);

No C++ comments, please.

And don't add dead code.

> +	if( isr & PCI_ISR_PFE ){

Write this as:

	if (isr & PCI_ISR_PFE) {

> @@ -304,9 +355,7 @@ void pci_ixp_init (struct pci_controller
>  	pci_write_config_word (0, PCI_CFG_COMMAND, INITIAL_PCI_CMD);
>  	REG_WRITE (PCI_CSR_BASE, PCI_ISR_OFFSET, PCI_ISR_PSE
>  		   | PCI_ISR_PFE | PCI_ISR_PPE | PCI_ISR_AHBE);
> -#ifdef CONFIG_PCI_SCAN_SHOW
> -	printf ("Device  bus  dev  func  deviceID  vendorID \n");
> -#endif
> +
>  	pci_bus_scan ();

Why do you remove the message?

> +	for(busno=0; busno<2; busno++){
> +	  for(devno=0; devno<PCI_MAX_PCI_DEVICES; devno++){
> +	    for(funcno=0; funcno<PCI_MAX_PCI_FUNCTIONS; funcno++) {
> +		dev = PCI_BDF(busno,devno,funcno);
> +
> +		if( pci_read_config_dword (dev, PCI_CFG_VENDOR_ID, &vendorId) == ERROR ){
> +		    funcno=PCI_MAX_PCI_FUNCTIONS;
> +		    continue;
> +		}

Indentation by TAB, please.

> -			/*devices[nDevices].irq = ixp425PciIntTranslate[dev][intPin-1]; */
> -			devices[nDevices].irq = intPin;
> +			//devices[nDevices].irq = ixp425PciIntTranslate[devno][intPin-1];
> +			devices[nDevices].irq = pciTranslateIrq(dev,intPin);
> +			//devices[nDevices].irq = intPin;

No C++ comments, no dead code.

> -#ifdef CONFIG_PCI_SCAN_SHOW
> -		printf ("%06d    %03d %03d %04d  %08d      %08x\n", nDevices,
> -			devices[nDevices].vendor_id);
> -#endif

???


and so on and on...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Success in marriage is not so much finding the right person as it  is
being the right person.