Hi

Attached is a new patch which takes into account the comments on the earlier
patch.
I have changed the patch as per the comments below :

--------------------------------------------------------------------------------------------------
a) Comments from Andrew Morton :

>> +++ b/drivers/ide/pci/sgiioc4.c       Thu Oct  2 16:53:34 2003
>> +
>>+extern int dma_timer_expiry(ide_drive_t * drive);

>This is unused.  Just as well, as it is static to a different file.

This line has been removed.

>> +static struct pci_device_id sgiioc4_pci_tbl[] __devinitdata = {

>This cannot be __devinitdata because the PCI table walking will look at it
>even after __init code has been dropped.  We've had oopses from this.

The __devinitdata attribute has been removed from this declaration.


>> --- /dev/null Wed Dec 31 16:00:00 1969
>> +++ b/drivers/ide/pci/sgiioc4.h       Thu Oct  2 16:53:34 2003

>hrm, why does this file exist?  It has only one include site, and should
>not be included by other .c files anyway because it defines static storage.
>
>It looks like the whole file should just be pasted into sgiioc4.c?

This file has been merged with sgiioc4.c as was suggested.

>> +typedef volatile struct {
>> +     u32 timing_reg0;
>> +     u32 timing_reg1;
>> +     u32 low_mem_ptr;
>> +     u32 high_mem_ptr;
>> +     u32 low_mem_addr;
>> +     u32 high_mem_addr;
>> +     u32 dev_byte_count;
>> +     u32 mem_byte_count;
>> +     u32 status;
>> +} ioc4_dma_regs_t;
>
>Does this actually need to be volatile?

This structure is no longer volatile in the new patch.

--------------------------------------------------------------------------------------------------
b) Comments from Bartlomiej Zolnierkiewicz

>Please follow Documentation/CodingStyle and respect 80-columns limit.

Lindent has been used to format the original file. Some parts have been
reformatted by hand.
The 80 column limit is respected for a majority of the patch.

+ *
+ *     This code is identical to ide_raw_build_sglist in ide-dma.c
+ *     however that it not exported and even if it were would create
+ *     dependancy problems for modular drivers.
+ */
>What problems?
>BTW during coping tabs were replaced by spaces in these functions.

Actually what I meant by problems was that, when this driver is compiled as a
module and the earlier functions are not exported, the driver fails to find
them. I have removed the earlier line from the new patch.


+       return p - buffer;
+}

>Do you really need /proc/ide/sgiioc4?
>You can print revision number during init.

It has been helpful to be able to see the firmware revision num anytime during
system operation. 
So the new patch still creates the above entry.

+                                           int ddir);
+static unsigned int __init pci_init_sgiioc4(struct pci_dev
*dev,ide_pci_device_t *d);

>Most of this declarations are not needed as sgiioc4.h is only included from shiioc4.c.

The sgiioc4.h file has been removed in the new patch.
--------------------------------------------------------------------------------------------------

Please merge this patch if there are no other issues.

Thanks
Aniket