Dear Stefan Althoefer,
In message <4938471d.ipiaro7pyltl2jna%[email protected]> 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: [email protected]
Success in marriage is not so much finding the right person as it is
being the right person.
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot
