Differences
This shows you the differences between two versions of the page.
| Both sides previous revision Previous revision Next revision | Previous revision | ||
|
soc:2008:mdeck:journal:week5 [2008/06/26 12:56] mdeck |
soc:2008:mdeck:journal:week5 [2008/07/02 09:50] (current) mdeck |
||
|---|---|---|---|
| Line 67: | Line 67: | ||
| outw ( intr_status, ioaddr + SCBStatus ); | outw ( intr_status, ioaddr + SCBStatus ); | ||
| inw ( ioaddr + SCBStatus ); | inw ( ioaddr + SCBStatus ); | ||
| + | |||
| + | |||
| + | |||
| + | |||
| + | |||
| + | |||
| + | |||
| + | |||
| + | |||
| Line 94: | Line 103: | ||
| * [[http://git.etherboot.org/?p=people/mdeck/gpxe.git;a=commit;h=2c2322662e619ac6817f1822d6f185c7d39f8612|[Drivers-eepro100] Free iob in ifec_net_close bug removed. More comments.]] | * [[http://git.etherboot.org/?p=people/mdeck/gpxe.git;a=commit;h=2c2322662e619ac6817f1822d6f185c7d39f8612|[Drivers-eepro100] Free iob in ifec_net_close bug removed. More comments.]] | ||
| More on the way. 8-) | More on the way. 8-) | ||
| + | * [[http://git.etherboot.org/?p=people/mdeck/gpxe.git;a=commitdiff;h=27c70247fdc7d8d250731cc7c077cee19b96044a|[Drivers-eepro100] Moved rx data into struct ifec_active]] | ||
| + | * [[http://git.etherboot.org/?p=people/mdeck/gpxe.git;a=commitdiff;h=fac57320f5b6a0df8e9064c02e03333c0f7d2f19|[Drivers-eepro100] Fixed ias bug introduced when moving rx to ifec_active]] | ||
| + | All data members of ''ifec_private'' supporting the rx ring were moved into ''ifec_active''. ''ifec_active'' is allocated in ''ifec_net_open()'' and freed in ''ifec_net_close()'', so the driver's footprint is minimized while not open. | ||
| + | |||
| + | I also converted the IAS command block allocation in ''ifec_net_open()'' to use ''malloc_dma()'' instead of ''malloc()''. The card requires command blocks to be aligned on even physical addresses. Any alignment provided via gcc will affect virtual addresses. Although the functionality of page-mapping hardware dictates the lower bits of a virtual address will correspond to a physical address, I'm unsure of how much of an assumption the driver should make as to the underlying hardware. ''malloc_dma()'' provides an interface that guarantees aligned physical addresses, so this is the best method of allocation. | ||
| + | |||
| + | The second commit corrects a small typo in ''ifec_net_open()'', caused by some temporary changes to the ias allocation. The destination of the ''link'' field shouldn't be followed, since the card is ordered to suspend, and is later followed by the issuance of a start command. However, the IAS command is a little touchy in how it is performed, so to be safe it links to itself. | ||
| + | |||
| + | * [[http://git.etherboot.org/?p=people/mdeck/gpxe.git;a=commitdiff;h=c2b105c2be6313fea5305aa925f97fbe74e09f82|[Drivers-eepro100] Moved EEPROM caching into fn, made dynamic allocation.]] | ||
| + | The EEPROM caching sequence in ''ifec_pci_probe()'' was moved into a separate routine, ''ifec_eeprom_cache()''. Additionally ''eeprom'' was changed from a static array member of ''ifec_private'' to a pointer. The cache is allocated in ''ifec_eeprom_cache()'' and is later freed after its usage in ''ifec_net_open()'''s call to ''ifec_mdio_setup()''. If the pointer is not freed, e.g. an error occurs or ''ifec_net_open()'' is never called, then ''ifec_pci_remove()'' will free the pointer. | ||
| + | |||
| + | I also changed ''ifec_scb_cmd_wait()'' to //not// be inline. No reason for it. | ||
| + | |||
| + | * [[http://git.etherboot.org/?p=people/mdeck/gpxe.git;a=commitdiff;h=ee2c4b1990d204daeaa3a9dff40558bb20f01451|[Drivers-eepro100] Converted all fns to take net_device rather than ioaddr.]] | ||
| + | Previously, most of the non-API functions took ''ioaddr'' as a parameter. Marty suggested I change them to take a ''net_device *netdev'' as did the API functions, so all the function prototypes will be consistent. This should make it easier for newbies browsing the code to figure things out. | ||
| + | Only one function, ''ifec_rfd_init()'', doesn't take a netdev. This function only encapsulates a data structure filling sequence. | ||
| + | |||
| + | Tomorrow we shall see what's next! | ||
| + | |||
| + | === 27 June === | ||
| + | |||
| + | Had the weekly meeting today. Marty suggested reworking my error handling technique. He also pointed me to this [[http://lxr.linux.no/linux/Documentation/CodingStyle|Linux coding style guide]]. So the code will be getting a style upgrade ;) | ||
| + | |||
| + | === 28 & 29 June === | ||
| + | |||
| + | * [[http://git.etherboot.org/?p=people/mdeck/gpxe.git;a=commitdiff;h=feb5f34d4c1d83e175aba47603e9842acbe2cf98|[Drivers-eepro100] Formatting, code modularizing, etc.]] | ||
| + | |||
| + | Formatting changes were made throughout the code to conform with the aforementioned style guide. Multiple variable declarations on a single line were separated. Multiple assignments on a line were separated. if-statements on a single line were split. Large comments were moved to function comments or to the header. | ||
| + | |||
| + | In ''ifec_net_open()'', the ''struct ifec_cfg'' code was redundant. The struct's data declaration and the run-time modification were not necessary. I moved most of the assignments into the data declaration for the struct. Also, Individual Address Setup (ias) command references were coalesced into one sequence for code clarity. | ||
| + | |||
| + | In ''ifec_net_poll()'', tx processing and rx processing were separated into ''ifec_tx_process()'' and ''ifec_rx_process()''. In ''ifec_net_transmit()'', the switch statement responsible for issuing resume commands as necessary was moved into ''ifec_tx_wake()''. | ||
| + | |||
| + | Also, a pointer check was added in ''ifec_net_close()'' and error handline in ''ifec_net_open()'' was cleaned up. | ||
| + | |||
| + | The code segment at the beginning of ''ifec_net_open()'' is currently commented out. I don't believe this is needed, as it was added while trying to debug earlier. I will do more thorough testing before removing it completely. | ||
