From alan@linux.intel.com Mon Oct 26 14:48:19 2009 From: Alan Cox Date: Tue, 06 Oct 2009 15:47:41 +0100 Subject: Staging: et131x: tidy up initpci code To: greg@kroah.com Message-ID: <20091006144734.8604.20671.stgit@localhost.localdomain> Perform some easy tidying so we can see what needs to be done next Signed-off-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/staging/et131x/et131x_initpci.c | 444 +++++++++++++------------------- drivers/staging/et131x/et131x_initpci.h | 2 drivers/staging/et131x/et131x_netdev.c | 6 3 files changed, 196 insertions(+), 256 deletions(-) --- a/drivers/staging/et131x/et131x_initpci.c +++ b/drivers/staging/et131x/et131x_initpci.c @@ -132,16 +132,60 @@ MODULE_PARM_DESC(et131x_speed_set, "Set Link speed and dublex manually (0-5) [0] \n 1 : 10Mb Half-Duplex \n 2 : 10Mb Full-Duplex \n 3 : 100Mb Half-Duplex \n 4 : 100Mb Full-Duplex \n 5 : 1000Mb Full-Duplex \n 0 : Auto Speed Auto Dublex"); /** - * et131x_find_adapter - Find the adapter and get all the assigned resources + * et131x_hwaddr_init - set up the MAC Address on the ET1310 * @adapter: pointer to our private adapter structure + */ +void et131x_hwaddr_init(struct et131x_adapter *adapter) +{ + /* If have our default mac from init and no mac address from + * EEPROM then we need to generate the last octet and set it on the + * device + */ + if (adapter->PermanentAddress[0] == 0x00 && + adapter->PermanentAddress[1] == 0x00 && + adapter->PermanentAddress[2] == 0x00 && + adapter->PermanentAddress[3] == 0x00 && + adapter->PermanentAddress[4] == 0x00 && + adapter->PermanentAddress[5] == 0x00) { + /* + * We need to randomly generate the last octet so we + * decrease our chances of setting the mac address to + * same as another one of our cards in the system + */ + get_random_bytes(&adapter->CurrentAddress[5], 1); + /* + * We have the default value in the register we are + * working with so we need to copy the current + * address into the permanent address + */ + memcpy(adapter->PermanentAddress, + adapter->CurrentAddress, ETH_ALEN); + } else { + /* We do not have an override address, so set the + * current address to the permanent address and add + * it to the device + */ + memcpy(adapter->CurrentAddress, + adapter->PermanentAddress, ETH_ALEN); + } +} + + +/** + * et131x_pci_init - initial PCI setup + * @adapter: pointer to our private adapter structure + * @pdev: our PCI device * - * Returns 0 on success, errno on failure (as defined in errno.h) + * Perform the initial setup of PCI registers and if possible initialise + * the MAC address. At this point the I/O registers have yet to be mapped */ -int et131x_find_adapter(struct et131x_adapter *adapter, struct pci_dev *pdev) + +static int et131x_pci_init(struct et131x_adapter *adapter, + struct pci_dev *pdev) { - int result; - uint8_t maxPayload = 0; - uint8_t read_size_reg; + int i; + u8 max_payload; + u8 read_size_reg; /* Allow disabling of Non-Maskable Interrupts in I/O space, to * support validation. @@ -164,32 +208,28 @@ int et131x_find_adapter(struct et131x_ad /* Let's set up the PORT LOGIC Register. First we need to know what * the max_payload_size is */ - result = pci_read_config_byte(pdev, ET1310_PCI_MAX_PYLD, &maxPayload); - if (result != PCIBIOS_SUCCESSFUL) { + if (pci_read_config_byte(pdev, ET1310_PCI_MAX_PYLD, &max_payload)) { dev_err(&pdev->dev, "Could not read PCI config space for Max Payload Size\n"); return -EIO; } /* Program the Ack/Nak latency and replay timers */ - maxPayload &= 0x07; /* Only the lower 3 bits are valid */ + max_payload &= 0x07; /* Only the lower 3 bits are valid */ - if (maxPayload < 2) { - const uint16_t AckNak[2] = { 0x76, 0xD0 }; - const uint16_t Replay[2] = { 0x1E0, 0x2ED }; - - result = pci_write_config_word(pdev, ET1310_PCI_ACK_NACK, - AckNak[maxPayload]); - if (result != PCIBIOS_SUCCESSFUL) { - dev_err(&pdev->dev, + if (max_payload < 2) { + static const u16 AckNak[2] = { 0x76, 0xD0 }; + static const u16 Replay[2] = { 0x1E0, 0x2ED }; + + if (pci_write_config_word(pdev, ET1310_PCI_ACK_NACK, + AckNak[max_payload])) { + dev_err(&pdev->dev, "Could not write PCI config space for ACK/NAK\n"); return -EIO; } - - result = pci_write_config_word(pdev, ET1310_PCI_REPLAY, - Replay[maxPayload]); - if (result != PCIBIOS_SUCCESSFUL) { - dev_err(&pdev->dev, + if (pci_write_config_word(pdev, ET1310_PCI_REPLAY, + Replay[max_payload])) { + dev_err(&pdev->dev, "Could not write PCI config space for Replay Timer\n"); return -EIO; } @@ -198,16 +238,14 @@ int et131x_find_adapter(struct et131x_ad /* l0s and l1 latency timers. We are using default values. * Representing 001 for L0s and 010 for L1 */ - result = pci_write_config_byte(pdev, ET1310_PCI_L0L1LATENCY, 0x11); - if (result != PCIBIOS_SUCCESSFUL) { - dev_err(&pdev->dev, + if (pci_write_config_byte(pdev, ET1310_PCI_L0L1LATENCY, 0x11)) { + dev_err(&pdev->dev, "Could not write PCI config space for Latency Timers\n"); return -EIO; } /* Change the max read size to 2k */ - result = pci_read_config_byte(pdev, 0x51, &read_size_reg); - if (result != PCIBIOS_SUCCESSFUL) { + if (pci_read_config_byte(pdev, 0x51, &read_size_reg)) { dev_err(&pdev->dev, "Could not read PCI config space for Max read size\n"); return -EIO; @@ -216,8 +254,7 @@ int et131x_find_adapter(struct et131x_ad read_size_reg &= 0x8f; read_size_reg |= 0x40; - result = pci_write_config_byte(pdev, 0x51, read_size_reg); - if (result != PCIBIOS_SUCCESSFUL) { + if (pci_write_config_byte(pdev, 0x51, read_size_reg)) { dev_err(&pdev->dev, "Could not write PCI config space for Max read size\n"); return -EIO; @@ -226,19 +263,19 @@ int et131x_find_adapter(struct et131x_ad /* Get MAC address from config space if an eeprom exists, otherwise * the MAC address there will not be valid */ - if (adapter->has_eeprom) { - int i; - - for (i = 0; i < ETH_ALEN; i++) { - result = pci_read_config_byte( - pdev, ET1310_PCI_MAC_ADDRESS + i, - adapter->PermanentAddress + i); - if (result != PCIBIOS_SUCCESSFUL) { - dev_err(&pdev->dev, ";Could not read PCI config space for MAC address\n"); - return -EIO; - } + if (!adapter->has_eeprom) { + et131x_hwaddr_init(adapter); + return 0; + } + + for (i = 0; i < ETH_ALEN; i++) { + if (pci_read_config_byte(pdev, ET1310_PCI_MAC_ADDRESS + i, + adapter->PermanentAddress + i)) { + dev_err(&pdev->dev, "Could not read PCI config space for MAC address\n"); + return -EIO; } } + memcpy(adapter->CurrentAddress, adapter->PermanentAddress, ETH_ALEN); return 0; } @@ -433,45 +470,6 @@ int et131x_adapter_setup(struct et131x_a } /** - * et131x_setup_hardware_properties - set up the MAC Address on the ET1310 - * @adapter: pointer to our private adapter structure - */ -void et131x_setup_hardware_properties(struct et131x_adapter *adapter) -{ - /* If have our default mac from registry and no mac address from - * EEPROM then we need to generate the last octet and set it on the - * device - */ - if (adapter->PermanentAddress[0] == 0x00 && - adapter->PermanentAddress[1] == 0x00 && - adapter->PermanentAddress[2] == 0x00 && - adapter->PermanentAddress[3] == 0x00 && - adapter->PermanentAddress[4] == 0x00 && - adapter->PermanentAddress[5] == 0x00) { - /* - * We need to randomly generate the last octet so we - * decrease our chances of setting the mac address to - * same as another one of our cards in the system - */ - get_random_bytes(&adapter->CurrentAddress[5], 1); - /* - * We have the default value in the register we are - * working with so we need to copy the current - * address into the permanent address - */ - memcpy(adapter->PermanentAddress, - adapter->CurrentAddress, ETH_ALEN); - } else { - /* We do not have an override address, so set the - * current address to the permanent address and add - * it to the device - */ - memcpy(adapter->CurrentAddress, - adapter->PermanentAddress, ETH_ALEN); - } -} - -/** * et131x_soft_reset - Issue a soft reset to the hardware, complete for ET1310 * @adapter: pointer to our private adapter structure */ @@ -523,36 +521,32 @@ void et131x_align_allocated_memory(struc */ int et131x_adapter_memory_alloc(struct et131x_adapter *adapter) { - int status = 0; - - do { - /* Allocate memory for the Tx Ring */ - status = et131x_tx_dma_memory_alloc(adapter); - if (status != 0) { - dev_err(&adapter->pdev->dev, - "et131x_tx_dma_memory_alloc FAILED\n"); - break; - } + int status; - /* Receive buffer memory allocation */ - status = et131x_rx_dma_memory_alloc(adapter); - if (status != 0) { - dev_err(&adapter->pdev->dev, - "et131x_rx_dma_memory_alloc FAILED\n"); - et131x_tx_dma_memory_free(adapter); - break; - } + /* Allocate memory for the Tx Ring */ + status = et131x_tx_dma_memory_alloc(adapter); + if (status != 0) { + dev_err(&adapter->pdev->dev, + "et131x_tx_dma_memory_alloc FAILED\n"); + return status; + } + /* Receive buffer memory allocation */ + status = et131x_rx_dma_memory_alloc(adapter); + if (status != 0) { + dev_err(&adapter->pdev->dev, + "et131x_rx_dma_memory_alloc FAILED\n"); + et131x_tx_dma_memory_free(adapter); + return status; + } - /* Init receive data structures */ - status = et131x_init_recv(adapter); - if (status != 0) { - dev_err(&adapter->pdev->dev, - "et131x_init_recv FAILED\n"); - et131x_tx_dma_memory_free(adapter); - et131x_rx_dma_memory_free(adapter); - break; - } - } while (0); + /* Init receive data structures */ + status = et131x_init_recv(adapter); + if (status != 0) { + dev_err(&adapter->pdev->dev, + "et131x_init_recv FAILED\n"); + et131x_tx_dma_memory_free(adapter); + et131x_rx_dma_memory_free(adapter); + } return status; } @@ -567,22 +561,51 @@ void et131x_adapter_memory_free(struct e et131x_rx_dma_memory_free(adapter); } + + /** - * et131x_config_parse + * et131x_adapter_init * @etdev: pointer to the private adapter struct + * @pdev: pointer to the PCI device * - * Parses a configuration from some location (module parameters, for example) - * into the private adapter struct. This really has no sensible analogy in - * Linux as sysfs parameters are dynamic. Several things that were hee could - * go into sysfs, but other stuff like speed handling is part of the mii - * interfaces/ethtool. + * Initialize the data structures for the et131x_adapter object and link + * them together with the platform provided device structures. */ -void et131x_config_parse(struct et131x_adapter *etdev) + + +static struct et131x_adapter *et131x_adapter_init(struct net_device *netdev, + struct pci_dev *pdev) { static const u8 default_mac[] = { 0x00, 0x05, 0x3d, 0x00, 0x02, 0x00 }; static const u8 duplex[] = { 0, 1, 2, 1, 2, 2 }; static const u16 speed[] = { 0, 10, 10, 100, 100, 1000 }; + struct et131x_adapter *etdev; + + /* Setup the fundamental net_device and private adapter structure elements */ + SET_NETDEV_DEV(netdev, &pdev->dev); + + /* Allocate private adapter struct and copy in relevant information */ + etdev = netdev_priv(netdev); + etdev->pdev = pci_dev_get(pdev); + etdev->netdev = netdev; + + /* Do the same for the netdev struct */ + netdev->irq = pdev->irq; + netdev->base_addr = pci_resource_start(pdev, 0); + + /* Initialize spinlocks here */ + spin_lock_init(&etdev->Lock); + spin_lock_init(&etdev->TCBSendQLock); + spin_lock_init(&etdev->TCBReadyQLock); + spin_lock_init(&etdev->SendHWLock); + spin_lock_init(&etdev->SendWaitLock); + spin_lock_init(&etdev->RcvLock); + spin_lock_init(&etdev->RcvPendLock); + spin_lock_init(&etdev->FbrLock); + spin_lock_init(&etdev->PHYLock); + + /* Parse configuration parameters into the private adapter struct */ if (et131x_speed_set) dev_info(&etdev->pdev->dev, "Speed set manually to : %d \n", et131x_speed_set); @@ -609,40 +632,10 @@ void et131x_config_parse(struct et131x_a etdev->AiForceSpeed = speed[etdev->SpeedDuplex]; etdev->AiForceDpx = duplex[etdev->SpeedDuplex]; /* Auto FDX */ -} - - -/** - * et131x_pci_remove - * @pdev: a pointer to the device's pci_dev structure - * - * Registered in the pci_driver structure, this function is called when the - * PCI subsystem detects that a PCI device which matches the information - * contained in the pci_device_id table has been removed. - */ - -void __devexit et131x_pci_remove(struct pci_dev *pdev) -{ - struct net_device *netdev; - struct et131x_adapter *adapter; - - /* Retrieve the net_device pointer from the pci_dev struct, as well - * as the private adapter struct - */ - netdev = (struct net_device *) pci_get_drvdata(pdev); - adapter = netdev_priv(netdev); - /* Perform device cleanup */ - unregister_netdev(netdev); - et131x_adapter_memory_free(adapter); - iounmap(adapter->regs); - pci_dev_put(adapter->pdev); - free_netdev(netdev); - pci_release_regions(pdev); - pci_disable_device(pdev); + return etdev; } - /** * et131x_pci_setup - Perform device initialization * @pdev: a pointer to the device's pci_dev structure @@ -656,34 +649,31 @@ void __devexit et131x_pci_remove(struct * a device insertion routine. */ -int __devinit et131x_pci_setup(struct pci_dev *pdev, +static int __devinit et131x_pci_setup(struct pci_dev *pdev, const struct pci_device_id *ent) { - int result = 0; + int result = -EBUSY; int pm_cap; bool pci_using_dac; - struct net_device *netdev = NULL; - struct et131x_adapter *adapter = NULL; + struct net_device *netdev; + struct et131x_adapter *adapter; /* Enable the device via the PCI subsystem */ - result = pci_enable_device(pdev); - if (result != 0) { - dev_err(&adapter->pdev->dev, + if (pci_enable_device(pdev) != 0) { + dev_err(&pdev->dev, "pci_enable_device() failed\n"); - goto out; + return -EIO; } /* Perform some basic PCI checks */ if (!(pci_resource_flags(pdev, 0) & IORESOURCE_MEM)) { - dev_err(&adapter->pdev->dev, + dev_err(&pdev->dev, "Can't find PCI device's base address\n"); - result = -ENODEV; - goto out; + goto err_disable; } - result = pci_request_regions(pdev, DRIVER_NAME); - if (result != 0) { - dev_err(&adapter->pdev->dev, + if (pci_request_regions(pdev, DRIVER_NAME)) { + dev_err(&pdev->dev, "Can't get PCI resources\n"); goto err_disable; } @@ -698,27 +688,26 @@ int __devinit et131x_pci_setup(struct pc */ pm_cap = pci_find_capability(pdev, PCI_CAP_ID_PM); if (pm_cap == 0) { - dev_err(&adapter->pdev->dev, + dev_err(&pdev->dev, "Cannot find Power Management capabilities\n"); result = -EIO; goto err_release_res; } /* Check the DMA addressing support of this device */ - if (!pci_set_dma_mask(pdev, 0xffffffffffffffffULL)) { + if (!pci_set_dma_mask(pdev, DMA_BIT_MASK(64))) { pci_using_dac = true; - result = - pci_set_consistent_dma_mask(pdev, 0xffffffffffffffffULL); + result = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64)); if (result != 0) { dev_err(&pdev->dev, "Unable to obtain 64 bit DMA for consistent allocations\n"); goto err_release_res; } - } else if (!pci_set_dma_mask(pdev, 0xffffffffULL)) { + } else if (!pci_set_dma_mask(pdev, DMA_BIT_MASK(32))) { pci_using_dac = false; } else { - dev_err(&adapter->pdev->dev, + dev_err(&pdev->dev, "No usable DMA addressing method\n"); result = -EIO; goto err_release_res; @@ -727,87 +716,22 @@ int __devinit et131x_pci_setup(struct pc /* Allocate netdev and private adapter structs */ netdev = et131x_device_alloc(); if (netdev == NULL) { - dev_err(&adapter->pdev->dev, - "Couldn't alloc netdev struct\n"); + dev_err(&pdev->dev, "Couldn't alloc netdev struct\n"); result = -ENOMEM; goto err_release_res; } - - /* Setup the fundamental net_device and private adapter structure elements */ - SET_NETDEV_DEV(netdev, &pdev->dev); - /* - if (pci_using_dac) { - netdev->features |= NETIF_F_HIGHDMA; - } - */ - - /* - * NOTE - Turn this on when we're ready to deal with SG-DMA - * - * NOTE: According to "Linux Device Drivers", 3rd ed, Rubini et al, - * if checksumming is not performed in HW, then the kernel will not - * use SG. - * From pp 510-511: - * - * "Note that the kernel does not perform scatter/gather I/O to your - * device if it does not also provide some form of checksumming as - * well. The reason is that, if the kernel has to make a pass over a - * fragmented ("nonlinear") packet to calculate the checksum, it - * might as well copy the data and coalesce the packet at the same - * time." - * - * This has been verified by setting the flags below and still not - * receiving a scattered buffer from the network stack, so leave it - * off until checksums are calculated in HW. - */ - /* netdev->features |= NETIF_F_SG; */ - /* netdev->features |= NETIF_F_NO_CSUM; */ - /* netdev->features |= NETIF_F_LLTX; */ - - /* Allocate private adapter struct and copy in relevant information */ - adapter = netdev_priv(netdev); - adapter->pdev = pci_dev_get(pdev); - adapter->netdev = netdev; - - /* Do the same for the netdev struct */ - netdev->irq = pdev->irq; - netdev->base_addr = pdev->resource[0].start; - - /* Initialize spinlocks here */ - spin_lock_init(&adapter->Lock); - spin_lock_init(&adapter->TCBSendQLock); - spin_lock_init(&adapter->TCBReadyQLock); - spin_lock_init(&adapter->SendHWLock); - spin_lock_init(&adapter->SendWaitLock); - spin_lock_init(&adapter->RcvLock); - spin_lock_init(&adapter->RcvPendLock); - spin_lock_init(&adapter->FbrLock); - spin_lock_init(&adapter->PHYLock); - - /* Parse configuration parameters into the private adapter struct */ - et131x_config_parse(adapter); - - /* Find the physical adapter - * - * NOTE: This is the equivalent of the MpFindAdapter() routine; can we - * lump it's init with the device specific init below into a - * single init function? - */ - /* while (et131x_find_adapter(adapter, pdev) != 0); */ - et131x_find_adapter(adapter, pdev); + adapter = et131x_adapter_init(netdev, pdev); + /* Initialise the PCI setup for the device */ + et131x_pci_init(adapter, pdev); /* Map the bus-relative registers to system virtual memory */ - - adapter->regs = ioremap_nocache(pci_resource_start(pdev, 0), - pci_resource_len(pdev, 0)); + adapter->regs = pci_ioremap_bar(pdev, 0); if (adapter->regs == NULL) { dev_err(&pdev->dev, "Cannot map device registers\n"); result = -ENOMEM; goto err_free_dev; } - /* Perform device-specific initialization here (See code below) */ - /* If Phy COMA mode was enabled when we went down, disable it here. */ writel(ET_PMCSR_INIT, &adapter->regs->global.pm_csr); @@ -827,20 +751,12 @@ int __devinit et131x_pci_setup(struct pc /* Init send data structures */ et131x_init_send(adapter); - /* Register the interrupt - * - * NOTE - This is being done in the open routine, where most other - * Linux drivers setup IRQ handlers. Make sure device - * interrupts are not turned on before the IRQ is registered!! - * - * What we will do here is setup the task structure for the - * ISR's deferred handler + /* + * Set up the task structure for the ISR's deferred handler */ INIT_WORK(&adapter->task, et131x_isr_handler); - /* Determine MAC Address, and copy into the net_device struct */ - et131x_setup_hardware_properties(adapter); - + /* Copy address into the net_device struct */ memcpy(netdev->dev_addr, adapter->CurrentAddress, ETH_ALEN); /* Setup et1310 as per the documentation */ @@ -879,10 +795,7 @@ int __devinit et131x_pci_setup(struct pc * been initialized, just in case it needs to be quickly restored. */ pci_set_drvdata(pdev, netdev); - pci_save_state(adapter->pdev); - -out: return result; err_mem_free: @@ -896,7 +809,37 @@ err_release_res: pci_release_regions(pdev); err_disable: pci_disable_device(pdev); - goto out; + return result; +} + +/** + * et131x_pci_remove + * @pdev: a pointer to the device's pci_dev structure + * + * Registered in the pci_driver structure, this function is called when the + * PCI subsystem detects that a PCI device which matches the information + * contained in the pci_device_id table has been removed. + */ + +static void __devexit et131x_pci_remove(struct pci_dev *pdev) +{ + struct net_device *netdev; + struct et131x_adapter *adapter; + + /* Retrieve the net_device pointer from the pci_dev struct, as well + * as the private adapter struct + */ + netdev = (struct net_device *) pci_get_drvdata(pdev); + adapter = netdev_priv(netdev); + + /* Perform device cleanup */ + unregister_netdev(netdev); + et131x_adapter_memory_free(adapter); + iounmap(adapter->regs); + pci_dev_put(adapter->pdev); + free_netdev(netdev); + pci_release_regions(pdev); + pci_disable_device(pdev); } static struct pci_device_id et131x_pci_table[] __devinitdata = { @@ -945,7 +888,6 @@ static void __exit et131x_cleanup_module module_init(et131x_init_module); module_exit(et131x_cleanup_module); - /* Modinfo parameters (filled out using defines from et131x_version.h) */ MODULE_AUTHOR(DRIVER_AUTHOR); MODULE_DESCRIPTION(DRIVER_INFO); --- a/drivers/staging/et131x/et131x_initpci.h +++ b/drivers/staging/et131x/et131x_initpci.h @@ -67,7 +67,7 @@ void et131x_align_allocated_memory(struc int et131x_adapter_setup(struct et131x_adapter *adapter); int et131x_adapter_memory_alloc(struct et131x_adapter *adapter); void et131x_adapter_memory_free(struct et131x_adapter *adapter); -void et131x_setup_hardware_properties(struct et131x_adapter *adapter); +void et131x_hwaddr_init(struct et131x_adapter *adapter); void et131x_soft_reset(struct et131x_adapter *adapter); #endif /* __ET131X_INITPCI_H__ */ --- a/drivers/staging/et131x/et131x_netdev.c +++ b/drivers/staging/et131x/et131x_netdev.c @@ -622,7 +622,7 @@ int et131x_change_mtu(struct net_device et131x_init_send(adapter); - et131x_setup_hardware_properties(adapter); + et131x_hwaddr_init(adapter); memcpy(netdev->dev_addr, adapter->CurrentAddress, ETH_ALEN); /* Init the device with the new settings */ @@ -709,9 +709,7 @@ int et131x_set_mac_addr(struct net_devic et131x_init_send(adapter); - et131x_setup_hardware_properties(adapter); - /* memcpy( netdev->dev_addr, adapter->CurrentAddress, ETH_ALEN ); */ - /* blux: no, do not override our nice address */ + et131x_hwaddr_init(adapter); /* Init the device with the new settings */ et131x_adapter_setup(adapter);