Re: [PATCH 07/12] scsi: add 'am53c974' driver

[Hannes Reinecke <hare@suse.de>]

On 11/24/2014 02:50 PM, Christoph Hellwig wrote:
>> +/*
>> + * AMD am53c974 driver.
>> + * Copyright (c) 2014 Hannes Reinecke, SUSE Linux GmbH
>> + */
> 
> Is there anything left from the old driver that should get credit here?
> 
Hmm. I copied the EEprom handling more-or-less verbatim from the
tmscsim driver; not sure if that warrants a credit.

>> +#define PCI_ESP_GET_PRIV(esp) ((struct pci_esp_priv *) \
>> +			       pci_get_drvdata((struct pci_dev *) \
>> +					       (esp)->dev))
> 
> I think an inline function would be a lot cleaner, especially as that
> should avoid all these casts.
> 
Ok.

>> +#ifdef ESP_DMA_DEBUG
>> +	shost_printk(KERN_INFO, esp->host, "start dma addr[%x] count[%d:%d]\n",
>> +		     addr, esp_count, dma_count);
>> +#endif
> 
> Can you add a esp_dma_printk or similar instead of all these ifdefs?
> 
Yep, I can.

>> +	u8 carryFlag = 1, j = 0x80, bval;
> 
> carry_flag.
> 
As said, copied verbatim. Will be fixing it up.

>> +	if (pci_request_regions(pdev, DRV_MODULE_NAME)) {
> 
>> +	if (request_irq(pdev->irq, scsi_esp_intr, IRQF_SHARED,
>> +			DRV_MODULE_NAME, esp)) {
> 
> Please propagate the return values from these two.
> 
Ok.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html