RE: [PATCH v2 -mm 4/7] I/OAT: Split PCI startup from DMA handling code

["Nelson, Shannon" <shannon.nelson@intel.com>]

Randy Dunlap [mailto:randy.dunlap@oracle.com] 
>
>On Thu, 23 Aug 2007 17:15:12 -0700 Shannon Nelson wrote:
>
>> Split the general PCI startup from the DMA handling code in order to
>> prepare for adding support for DCA services and future 
>versions of the
>> ioatdma device.
>> 
>> Signed-off-by: Shannon Nelson <shannon.nelson@intel.com>
>> Acked-by: David S. Miller <davem@davemloft.net>
>> ---
>> 
>>  drivers/dma/Makefile     |    2 
>>  drivers/dma/ioat.c       |  186 
>++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/dma/ioat_dma.c   |  196 
>+++++++++++-----------------------------------
>>  drivers/dma/ioatdma.h    |   16 +++-
>>  drivers/dma/ioatdma_hw.h |    2 
>>  5 files changed, 245 insertions(+), 157 deletions(-)
>
>> diff --git a/drivers/dma/ioat_dma.c b/drivers/dma/ioat_dma.c
>> index 9a4d154..9012176 100644
>> --- a/drivers/dma/ioat_dma.c
>> +++ b/drivers/dma/ioat_dma.c
>
>> -	printk(KERN_INFO " "
>> -		"ioatdma: Intel(R) I/OAT DMA Engine 
>initialization failed\n");
>> -
>> -	return err;
>> +	iounmap(iobase);
>> +	printk(KERN_ERR " "
>> +	       "ioatdma: Intel(R) I/OAT DMA Engine 
>initialization failed\n");
>
>Drop the " " after KERN_ERR...

Yes, is dumb, but is addressed in the next patch in the series.

>
>> +	return NULL;
>>  }
>
>> diff --git a/drivers/dma/ioatdma.h b/drivers/dma/ioatdma.h
>> index bf4dad7..26aff08 100644
>> --- a/drivers/dma/ioatdma.h
>> +++ b/drivers/dma/ioatdma.h
>> @@ -31,7 +31,7 @@
>>  #define IOAT_LOW_COMPLETION_MASK	0xffffffc0
>>  
>>  /**
>> - * struct ioat_device - internal representation of a IOAT device
>> + * struct ioatdma_device - internal representation of a IOAT device
>>   * @pdev: PCI-Express device
>>   * @reg_base: MMIO register space base address
>>   * @dma_pool: for allocating DMA descriptors
>> @@ -39,14 +39,14 @@
>>   * @msi: Message Signaled Interrupt number
>>   */
>>  
>> -struct ioat_device {
>> +struct ioatdma_device {
>>  	struct pci_dev *pdev;
>>  	void __iomem *reg_base;
>>  	struct pci_pool *dma_pool;
>>  	struct pci_pool *completion_pool;
>>  
>>  	struct dma_device common;
>> -	u8 msi;
>> +	u8 version;
>
>This field name change needs a corresponding change in the
>struct's kernel-doc above here.

Oops, missed that one - thanks.

>
>>  };
>>  
>>  /**
>
>> @@ -117,4 +117,12 @@ struct ioat_desc_sw {
>>  	struct dma_async_tx_descriptor async_tx;
>>  };
>>  
>> +#if defined(CONFIG_INTEL_IOATDMA) || 
>defined(CONFIG_INTEL_IOATDMA_MODULE)
>> +struct ioatdma_device *ioat_dma_probe(struct pci_dev *, 
>void __iomem *);
>
>Please use parameter variable names in function prototypes (above;
>most places already have them).

Yep, I'll get it.

Again, thanks for your comments,
sln
--
======================================================================
Mr. Shannon Nelson                 LAN Access Division, Intel Corp.
Shannon.Nelson@intel.com                I don't speak for Intel
(503) 712-7659                    Parents can't afford to be squeamish.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/