* [PATCH 01/10] esp_scsi: spellcheck 'driver'
2014-11-21 9:27 [PATCH 00/10] Re-implement am53c974 driver Hannes Reinecke
@ 2014-11-21 9:27 ` Hannes Reinecke
2014-11-21 9:35 ` Paolo Bonzini
2014-11-21 9:27 ` [PATCH 02/10] esp_scsi: make number of tags configurable Hannes Reinecke
` (9 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Hannes Reinecke @ 2014-11-21 9:27 UTC (permalink / raw)
To: James Bottomley
Cc: Christoph Hellwig, Paolo Bonzini, linux-scsi, Hannes Reinecke
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/esp_scsi.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/esp_scsi.h b/drivers/scsi/esp_scsi.h
index cd68805..b5862e4 100644
--- a/drivers/scsi/esp_scsi.h
+++ b/drivers/scsi/esp_scsi.h
@@ -1,4 +1,4 @@
-/* esp_scsi.h: Defines and structures for the ESP drier.
+/* esp_scsi.h: Defines and structures for the ESP driver.
*
* Copyright (C) 2007 David S. Miller (davem@davemloft.net)
*/
--
1.8.5.2
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 01/10] esp_scsi: spellcheck 'driver'
2014-11-21 9:27 ` [PATCH 01/10] esp_scsi: spellcheck 'driver' Hannes Reinecke
@ 2014-11-21 9:35 ` Paolo Bonzini
0 siblings, 0 replies; 31+ messages in thread
From: Paolo Bonzini @ 2014-11-21 9:35 UTC (permalink / raw)
To: Hannes Reinecke, James Bottomley; +Cc: Christoph Hellwig, linux-scsi
On 21/11/2014 10:27, Hannes Reinecke wrote:
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> drivers/scsi/esp_scsi.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/esp_scsi.h b/drivers/scsi/esp_scsi.h
> index cd68805..b5862e4 100644
> --- a/drivers/scsi/esp_scsi.h
> +++ b/drivers/scsi/esp_scsi.h
> @@ -1,4 +1,4 @@
> -/* esp_scsi.h: Defines and structures for the ESP drier.
> +/* esp_scsi.h: Defines and structures for the ESP driver.
> *
> * Copyright (C) 2007 David S. Miller (davem@davemloft.net)
> */
>
A good start. :)
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 02/10] esp_scsi: make number of tags configurable
2014-11-21 9:27 [PATCH 00/10] Re-implement am53c974 driver Hannes Reinecke
2014-11-21 9:27 ` [PATCH 01/10] esp_scsi: spellcheck 'driver' Hannes Reinecke
@ 2014-11-21 9:27 ` Hannes Reinecke
2014-11-21 9:37 ` Paolo Bonzini
2014-11-21 9:27 ` [PATCH 03/10] esp_scsi: convert to dev_printk Hannes Reinecke
` (8 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Hannes Reinecke @ 2014-11-21 9:27 UTC (permalink / raw)
To: James Bottomley
Cc: Christoph Hellwig, Paolo Bonzini, linux-scsi, Hannes Reinecke
Add a field 'num_tags' to the esp structure to allow drivers
to overwrite the number of avialable tags if required.
Default is ESP_DEFAULT_TAGS.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/esp_scsi.c | 10 ++++------
drivers/scsi/esp_scsi.h | 3 +--
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
index 38c23e0..99cd42f 100644
--- a/drivers/scsi/esp_scsi.c
+++ b/drivers/scsi/esp_scsi.c
@@ -2317,6 +2317,8 @@ int scsi_esp_register(struct esp *esp, struct device *dev)
static int instance;
int err;
+ if (!esp->num_tags)
+ esp->num_tags = ESP_DEFAULT_TAGS;
esp->host->transportt = esp_transport_template;
esp->host->max_lun = ESP_MAX_LUN;
esp->host->cmd_per_lun = 2;
@@ -2403,12 +2405,8 @@ static int esp_slave_configure(struct scsi_device *dev)
struct esp *esp = shost_priv(dev->host);
struct esp_target_data *tp = &esp->target[dev->id];
- if (dev->tagged_supported) {
- /* XXX make this configurable somehow XXX */
- int goal_tags = min(ESP_DEFAULT_TAGS, ESP_MAX_TAG);
-
- scsi_adjust_queue_depth(dev, goal_tags);
- }
+ if (dev->tagged_supported)
+ scsi_adjust_queue_depth(dev, esp->num_tags);
tp->flags |= ESP_TGT_DISCONNECT;
diff --git a/drivers/scsi/esp_scsi.h b/drivers/scsi/esp_scsi.h
index b5862e4..975d293 100644
--- a/drivers/scsi/esp_scsi.h
+++ b/drivers/scsi/esp_scsi.h
@@ -283,7 +283,6 @@ struct esp_cmd_entry {
struct completion *eh_done;
};
-/* XXX make this configurable somehow XXX */
#define ESP_DEFAULT_TAGS 16
#define ESP_MAX_TARGET 16
@@ -445,7 +444,7 @@ struct esp {
u8 prev_soff;
u8 prev_stp;
u8 prev_cfg3;
- u8 __pad;
+ u8 num_tags;
struct list_head esp_cmd_pool;
--
1.8.5.2
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 02/10] esp_scsi: make number of tags configurable
2014-11-21 9:27 ` [PATCH 02/10] esp_scsi: make number of tags configurable Hannes Reinecke
@ 2014-11-21 9:37 ` Paolo Bonzini
0 siblings, 0 replies; 31+ messages in thread
From: Paolo Bonzini @ 2014-11-21 9:37 UTC (permalink / raw)
To: Hannes Reinecke, James Bottomley; +Cc: Christoph Hellwig, linux-scsi
On 21/11/2014 10:27, Hannes Reinecke wrote:
> Add a field 'num_tags' to the esp structure to allow drivers
> to overwrite the number of avialable tags if required.
> Default is ESP_DEFAULT_TAGS.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> drivers/scsi/esp_scsi.c | 10 ++++------
> drivers/scsi/esp_scsi.h | 3 +--
> 2 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
> index 38c23e0..99cd42f 100644
> --- a/drivers/scsi/esp_scsi.c
> +++ b/drivers/scsi/esp_scsi.c
> @@ -2317,6 +2317,8 @@ int scsi_esp_register(struct esp *esp, struct device *dev)
> static int instance;
> int err;
>
> + if (!esp->num_tags)
> + esp->num_tags = ESP_DEFAULT_TAGS;
> esp->host->transportt = esp_transport_template;
> esp->host->max_lun = ESP_MAX_LUN;
> esp->host->cmd_per_lun = 2;
> @@ -2403,12 +2405,8 @@ static int esp_slave_configure(struct scsi_device *dev)
> struct esp *esp = shost_priv(dev->host);
> struct esp_target_data *tp = &esp->target[dev->id];
>
> - if (dev->tagged_supported) {
> - /* XXX make this configurable somehow XXX */
> - int goal_tags = min(ESP_DEFAULT_TAGS, ESP_MAX_TAG);
min(16, 256) = 16
Might add a WARN_ON(esp->num_tags > ESP_MAX_TAG) after you assign
esp->num_tags.
> - scsi_adjust_queue_depth(dev, goal_tags);
> - }
> + if (dev->tagged_supported)
> + scsi_adjust_queue_depth(dev, esp->num_tags);
>
> tp->flags |= ESP_TGT_DISCONNECT;
>
> diff --git a/drivers/scsi/esp_scsi.h b/drivers/scsi/esp_scsi.h
> index b5862e4..975d293 100644
> --- a/drivers/scsi/esp_scsi.h
> +++ b/drivers/scsi/esp_scsi.h
> @@ -283,7 +283,6 @@ struct esp_cmd_entry {
> struct completion *eh_done;
> };
>
> -/* XXX make this configurable somehow XXX */
> #define ESP_DEFAULT_TAGS 16
>
> #define ESP_MAX_TARGET 16
> @@ -445,7 +444,7 @@ struct esp {
> u8 prev_soff;
> u8 prev_stp;
> u8 prev_cfg3;
> - u8 __pad;
> + u8 num_tags;
>
> struct list_head esp_cmd_pool;
>
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 03/10] esp_scsi: convert to dev_printk
2014-11-21 9:27 [PATCH 00/10] Re-implement am53c974 driver Hannes Reinecke
2014-11-21 9:27 ` [PATCH 01/10] esp_scsi: spellcheck 'driver' Hannes Reinecke
2014-11-21 9:27 ` [PATCH 02/10] esp_scsi: make number of tags configurable Hannes Reinecke
@ 2014-11-21 9:27 ` Hannes Reinecke
2014-11-21 9:37 ` Paolo Bonzini
2014-11-21 9:27 ` [PATCH 04/10] esp_scsi: debug event and command Hannes Reinecke
` (7 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Hannes Reinecke @ 2014-11-21 9:27 UTC (permalink / raw)
To: James Bottomley
Cc: Christoph Hellwig, Paolo Bonzini, linux-scsi, Hannes Reinecke
Use dev_printk functions for correct device annotations.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/esp_scsi.c | 212 ++++++++++++++++++++++++------------------------
1 file changed, 106 insertions(+), 106 deletions(-)
diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
index 99cd42f..492c51b 100644
--- a/drivers/scsi/esp_scsi.c
+++ b/drivers/scsi/esp_scsi.c
@@ -52,52 +52,52 @@ static u32 esp_debug;
#define esp_log_intr(f, a...) \
do { if (esp_debug & ESP_DEBUG_INTR) \
- printk(f, ## a); \
+ shost_printk(KERN_DEBUG, esp->host, f, ## a); \
} while (0)
#define esp_log_reset(f, a...) \
do { if (esp_debug & ESP_DEBUG_RESET) \
- printk(f, ## a); \
+ shost_printk(KERN_DEBUG, esp->host, f, ## a); \
} while (0)
#define esp_log_msgin(f, a...) \
do { if (esp_debug & ESP_DEBUG_MSGIN) \
- printk(f, ## a); \
+ shost_printk(KERN_DEBUG, esp->host, f, ## a); \
} while (0)
#define esp_log_msgout(f, a...) \
do { if (esp_debug & ESP_DEBUG_MSGOUT) \
- printk(f, ## a); \
+ shost_printk(KERN_DEBUG, esp->host, f, ## a); \
} while (0)
#define esp_log_cmddone(f, a...) \
do { if (esp_debug & ESP_DEBUG_CMDDONE) \
- printk(f, ## a); \
+ shost_printk(KERN_DEBUG, esp->host, f, ## a); \
} while (0)
#define esp_log_disconnect(f, a...) \
do { if (esp_debug & ESP_DEBUG_DISCONNECT) \
- printk(f, ## a); \
+ shost_printk(KERN_DEBUG, esp->host, f, ## a); \
} while (0)
#define esp_log_datastart(f, a...) \
do { if (esp_debug & ESP_DEBUG_DATASTART) \
- printk(f, ## a); \
+ shost_printk(KERN_DEBUG, esp->host, f, ## a); \
} while (0)
#define esp_log_datadone(f, a...) \
do { if (esp_debug & ESP_DEBUG_DATADONE) \
- printk(f, ## a); \
+ shost_printk(KERN_DEBUG, esp->host, f, ## a); \
} while (0)
#define esp_log_reconnect(f, a...) \
do { if (esp_debug & ESP_DEBUG_RECONNECT) \
- printk(f, ## a); \
+ shost_printk(KERN_DEBUG, esp->host, f, ## a); \
} while (0)
#define esp_log_autosense(f, a...) \
do { if (esp_debug & ESP_DEBUG_AUTOSENSE) \
- printk(f, ## a); \
+ shost_printk(KERN_DEBUG, esp->host, f, ## a); \
} while (0)
#define esp_read8(REG) esp->ops->esp_read8(esp, REG)
@@ -150,19 +150,17 @@ static void esp_dump_cmd_log(struct esp *esp)
int idx = esp->esp_event_cur;
int stop = idx;
- printk(KERN_INFO PFX "esp%d: Dumping command log\n",
- esp->host->unique_id);
+ shost_printk(KERN_INFO, esp->host, "Dumping command log\n");
do {
struct esp_event_ent *p = &esp->esp_event_log[idx];
- printk(KERN_INFO PFX "esp%d: ent[%d] %s ",
- esp->host->unique_id, idx,
- p->type == ESP_EVENT_TYPE_CMD ? "CMD" : "EVENT");
-
- printk("val[%02x] sreg[%02x] seqreg[%02x] "
- "sreg2[%02x] ireg[%02x] ss[%02x] event[%02x]\n",
- p->val, p->sreg, p->seqreg,
- p->sreg2, p->ireg, p->select_state, p->event);
+ shost_printk(KERN_INFO, esp->host,
+ "ent[%d] %s val[%02x] sreg[%02x] seqreg[%02x] "
+ "sreg2[%02x] ireg[%02x] ss[%02x] event[%02x]\n",
+ idx,
+ p->type == ESP_EVENT_TYPE_CMD ? "CMD" : "EVENT",
+ p->val, p->sreg, p->seqreg,
+ p->sreg2, p->ireg, p->select_state, p->event);
idx = (idx + 1) & (ESP_EVENT_LOG_SZ - 1);
} while (idx != stop);
@@ -176,9 +174,8 @@ static void esp_flush_fifo(struct esp *esp)
while (esp_read8(ESP_FFLAGS) & ESP_FF_FBYTES) {
if (--lim == 0) {
- printk(KERN_ALERT PFX "esp%d: ESP_FF_BYTES "
- "will not clear!\n",
- esp->host->unique_id);
+ shost_printk(KERN_ALERT, esp->host,
+ "ESP_FF_BYTES will not clear!\n");
break;
}
udelay(1);
@@ -383,12 +380,11 @@ static void esp_advance_dma(struct esp *esp, struct esp_cmd_entry *ent,
p->cur_residue -= len;
p->tot_residue -= len;
if (p->cur_residue < 0 || p->tot_residue < 0) {
- printk(KERN_ERR PFX "esp%d: Data transfer overflow.\n",
- esp->host->unique_id);
- printk(KERN_ERR PFX "esp%d: cur_residue[%d] tot_residue[%d] "
- "len[%u]\n",
- esp->host->unique_id,
- p->cur_residue, p->tot_residue, len);
+ shost_printk(KERN_ERR, esp->host,
+ "Data transfer overflow.\n");
+ shost_printk(KERN_ERR, esp->host,
+ "cur_residue[%d] tot_residue[%d] len[%u]\n",
+ p->cur_residue, p->tot_residue, len);
p->cur_residue = 0;
p->tot_residue = 0;
}
@@ -604,9 +600,8 @@ static void esp_autosense(struct esp *esp, struct esp_cmd_entry *ent)
if (!ent->sense_ptr) {
- esp_log_autosense("esp%d: Doing auto-sense for "
- "tgt[%d] lun[%d]\n",
- esp->host->unique_id, tgt, lun);
+ esp_log_autosense("Doing auto-sense for tgt[%d] lun[%d]\n",
+ tgt, lun);
ent->sense_ptr = cmd->sense_buffer;
ent->sense_dma = esp->ops->map_single(esp,
@@ -953,8 +948,8 @@ static int esp_check_gross_error(struct esp *esp)
* - DMA programmed with wrong direction
* - improper phase change
*/
- printk(KERN_ERR PFX "esp%d: Gross error sreg[%02x]\n",
- esp->host->unique_id, esp->sreg);
+ shost_printk(KERN_ERR, esp->host,
+ "Gross error sreg[%02x]\n", esp->sreg);
/* XXX Reset the chip. XXX */
return 1;
}
@@ -982,14 +977,13 @@ static int esp_check_spur_intr(struct esp *esp)
* ESP is not, the only possibility is a DMA error.
*/
if (!esp->ops->dma_error(esp)) {
- printk(KERN_ERR PFX "esp%d: Spurious irq, "
- "sreg=%02x.\n",
- esp->host->unique_id, esp->sreg);
+ shost_printk(KERN_ERR, esp->host,
+ "Spurious irq, sreg=%02x.\n",
+ esp->sreg);
return -1;
}
- printk(KERN_ERR PFX "esp%d: DMA error\n",
- esp->host->unique_id);
+ shost_printk(KERN_ERR, esp->host, "DMA error\n");
/* XXX Reset the chip. XXX */
return -1;
@@ -1002,7 +996,7 @@ static int esp_check_spur_intr(struct esp *esp)
static void esp_schedule_reset(struct esp *esp)
{
- esp_log_reset("ESP: esp_schedule_reset() from %pf\n",
+ esp_log_reset("esp_schedule_reset() from %pf\n",
__builtin_return_address(0));
esp->flags |= ESP_FLAG_RESETTING;
esp_event(esp, ESP_EVENT_RESET);
@@ -1019,20 +1013,20 @@ static struct esp_cmd_entry *esp_reconnect_with_tag(struct esp *esp,
int i;
if (!lp->num_tagged) {
- printk(KERN_ERR PFX "esp%d: Reconnect w/num_tagged==0\n",
- esp->host->unique_id);
+ shost_printk(KERN_ERR, esp->host,
+ "Reconnect w/num_tagged==0\n");
return NULL;
}
- esp_log_reconnect("ESP: reconnect tag, ");
+ esp_log_reconnect("reconnect tag, ");
for (i = 0; i < ESP_QUICKIRQ_LIMIT; i++) {
if (esp->ops->irq_pending(esp))
break;
}
if (i == ESP_QUICKIRQ_LIMIT) {
- printk(KERN_ERR PFX "esp%d: Reconnect IRQ1 timeout\n",
- esp->host->unique_id);
+ shost_printk(KERN_ERR, esp->host,
+ "Reconnect IRQ1 timeout\n");
return NULL;
}
@@ -1043,14 +1037,14 @@ static struct esp_cmd_entry *esp_reconnect_with_tag(struct esp *esp,
i, esp->ireg, esp->sreg);
if (esp->ireg & ESP_INTR_DC) {
- printk(KERN_ERR PFX "esp%d: Reconnect, got disconnect.\n",
- esp->host->unique_id);
+ shost_printk(KERN_ERR, esp->host,
+ "Reconnect, got disconnect.\n");
return NULL;
}
if ((esp->sreg & ESP_STAT_PMASK) != ESP_MIP) {
- printk(KERN_ERR PFX "esp%d: Reconnect, not MIP sreg[%02x].\n",
- esp->host->unique_id, esp->sreg);
+ shost_printk(KERN_ERR, esp->host,
+ "Reconnect, not MIP sreg[%02x].\n", esp->sreg);
return NULL;
}
@@ -1073,8 +1067,7 @@ static struct esp_cmd_entry *esp_reconnect_with_tag(struct esp *esp,
udelay(1);
}
if (i == ESP_RESELECT_TAG_LIMIT) {
- printk(KERN_ERR PFX "esp%d: Reconnect IRQ2 timeout\n",
- esp->host->unique_id);
+ shost_printk(KERN_ERR, esp->host, "Reconnect IRQ2 timeout\n");
return NULL;
}
esp->ops->dma_drain(esp);
@@ -1087,17 +1080,17 @@ static struct esp_cmd_entry *esp_reconnect_with_tag(struct esp *esp,
if (esp->command_block[0] < SIMPLE_QUEUE_TAG ||
esp->command_block[0] > ORDERED_QUEUE_TAG) {
- printk(KERN_ERR PFX "esp%d: Reconnect, bad tag "
- "type %02x.\n",
- esp->host->unique_id, esp->command_block[0]);
+ shost_printk(KERN_ERR, esp->host,
+ "Reconnect, bad tag type %02x.\n",
+ esp->command_block[0]);
return NULL;
}
ent = lp->tagged_cmds[esp->command_block[1]];
if (!ent) {
- printk(KERN_ERR PFX "esp%d: Reconnect, no entry for "
- "tag %02x.\n",
- esp->host->unique_id, esp->command_block[1]);
+ shost_printk(KERN_ERR, esp->host,
+ "Reconnect, no entry for tag %02x.\n",
+ esp->command_block[1]);
return NULL;
}
@@ -1163,9 +1156,9 @@ static int esp_reconnect(struct esp *esp)
tp = &esp->target[target];
dev = __scsi_device_lookup_by_target(tp->starget, lun);
if (!dev) {
- printk(KERN_ERR PFX "esp%d: Reconnect, no lp "
- "tgt[%u] lun[%u]\n",
- esp->host->unique_id, target, lun);
+ shost_printk(KERN_ERR, esp->host,
+ "Reconnect, no lp tgt[%u] lun[%u]\n",
+ target, lun);
goto do_reset;
}
lp = dev->hostdata;
@@ -1291,8 +1284,8 @@ static int esp_finish_select(struct esp *esp)
return 0;
}
- printk("ESP: Unexpected selection completion ireg[%x].\n",
- esp->ireg);
+ shost_printk(KERN_INFO, esp->host,
+ "Unexpected selection completion ireg[%x]\n", esp->ireg);
esp_schedule_reset(esp);
return 0;
}
@@ -1556,8 +1549,8 @@ static void esp_msgin_extended(struct esp *esp)
return;
}
- printk("ESP: Unexpected extended msg type %x\n",
- esp->msg_in[2]);
+ shost_printk(KERN_INFO, esp->host,
+ "Unexpected extended msg type %x\n", esp->msg_in[2]);
esp->msg_out[0] = ABORT_TASK_SET;
esp->msg_out_len = 1;
@@ -1574,7 +1567,8 @@ static int esp_msgin_process(struct esp *esp)
if (msg0 & 0x80) {
/* Identify */
- printk("ESP: Unexpected msgin identify\n");
+ shost_printk(KERN_INFO, esp->host,
+ "Unexpected msgin identify\n");
return 0;
}
@@ -1673,8 +1667,9 @@ again:
break;
default:
- printk("ESP: Unexpected phase, sreg=%02x\n",
- esp->sreg);
+ shost_printk(KERN_INFO, esp->host,
+ "Unexpected phase, sreg=%02x\n",
+ esp->sreg);
esp_schedule_reset(esp);
return 0;
}
@@ -1708,18 +1703,17 @@ again:
esp->data_dma_len = dma_len;
if (!dma_len) {
- printk(KERN_ERR PFX "esp%d: DMA length is zero!\n",
- esp->host->unique_id);
- printk(KERN_ERR PFX "esp%d: cur adr[%08llx] len[%08x]\n",
- esp->host->unique_id,
- (unsigned long long)esp_cur_dma_addr(ent, cmd),
- esp_cur_dma_len(ent, cmd));
+ shost_printk(KERN_ERR, esp->host,
+ "DMA length is zero!\n");
+ shost_printk(KERN_ERR, esp->host,
+ "cur adr[%08llx] len[%08x]\n",
+ (unsigned long long)esp_cur_dma_addr(ent, cmd),
+ esp_cur_dma_len(ent, cmd));
esp_schedule_reset(esp);
return 0;
}
- esp_log_datastart("ESP: start data addr[%08llx] len[%u] "
- "write(%d)\n",
+ esp_log_datastart("start data addr[%08llx] len[%u] write(%d)\n",
(unsigned long long)dma_addr, dma_len, write);
esp->ops->send_dma_cmd(esp, dma_addr, dma_len, dma_len,
@@ -1733,7 +1727,8 @@ again:
int bytes_sent;
if (esp->ops->dma_error(esp)) {
- printk("ESP: data done, DMA error, resetting\n");
+ shost_printk(KERN_INFO, esp->host,
+ "data done, DMA error, resetting\n");
esp_schedule_reset(esp);
return 0;
}
@@ -1749,14 +1744,15 @@ again:
/* We should always see exactly a bus-service
* interrupt at the end of a successful transfer.
*/
- printk("ESP: data done, not BSERV, resetting\n");
+ shost_printk(KERN_INFO, esp->host,
+ "data done, not BSERV, resetting\n");
esp_schedule_reset(esp);
return 0;
}
bytes_sent = esp_data_bytes_sent(esp, ent, cmd);
- esp_log_datadone("ESP: data done flgs[%x] sent[%d]\n",
+ esp_log_datadone("data done flgs[%x] sent[%d]\n",
ent->flags, bytes_sent);
if (bytes_sent < 0) {
@@ -1785,8 +1781,9 @@ again:
}
if (ent->message != COMMAND_COMPLETE) {
- printk("ESP: Unexpected message %x in status\n",
- ent->message);
+ shost_printk(KERN_INFO, esp->host,
+ "Unexpected message %x in status\n",
+ ent->message);
esp_schedule_reset(esp);
return 0;
}
@@ -1804,8 +1801,7 @@ again:
scsi_esp_cmd(esp, ESP_CMD_ESEL);
if (ent->message == COMMAND_COMPLETE) {
- esp_log_cmddone("ESP: Command done status[%x] "
- "message[%x]\n",
+ esp_log_cmddone("Command done status[%x] message[%x]\n",
ent->status, ent->message);
if (ent->status == SAM_STAT_TASK_SET_FULL)
esp_event_queue_full(esp, ent);
@@ -1821,16 +1817,16 @@ again:
DID_OK));
}
} else if (ent->message == DISCONNECT) {
- esp_log_disconnect("ESP: Disconnecting tgt[%d] "
- "tag[%x:%x]\n",
+ esp_log_disconnect("Disconnecting tgt[%d] tag[%x:%x]\n",
cmd->device->id,
ent->tag[0], ent->tag[1]);
esp->active_cmd = NULL;
esp_maybe_execute_command(esp);
} else {
- printk("ESP: Unexpected message %x in freebus\n",
- ent->message);
+ shost_printk(KERN_INFO, esp->host,
+ "Unexpected message %x in freebus\n",
+ ent->message);
esp_schedule_reset(esp);
return 0;
}
@@ -1917,7 +1913,7 @@ again:
val = esp_read8(ESP_FDATA);
esp->msg_in[esp->msg_in_len++] = val;
- esp_log_msgin("ESP: Got msgin byte %x\n", val);
+ esp_log_msgin("Got msgin byte %x\n", val);
if (!esp_msgin_process(esp))
esp->msg_in_len = 0;
@@ -1930,7 +1926,8 @@ again:
if (esp->event != ESP_EVENT_FREE_BUS)
esp_event(esp, ESP_EVENT_CHECK_PHASE);
} else {
- printk("ESP: MSGIN neither BSERV not FDON, resetting");
+ shost_printk(KERN_INFO, esp->host,
+ "MSGIN neither BSERV not FDON, resetting");
esp_schedule_reset(esp);
return 0;
}
@@ -1961,8 +1958,8 @@ again:
break;
default:
- printk("ESP: Unexpected event %x, resetting\n",
- esp->event);
+ shost_printk(KERN_INFO, esp->host,
+ "Unexpected event %x, resetting\n", esp->event);
esp_schedule_reset(esp);
return 0;
break;
@@ -2085,14 +2082,15 @@ static void __esp_interrupt(struct esp *esp)
}
}
- esp_log_intr("ESP: intr sreg[%02x] seqreg[%02x] "
+ esp_log_intr("intr sreg[%02x] seqreg[%02x] "
"sreg2[%02x] ireg[%02x]\n",
esp->sreg, esp->seqreg, esp->sreg2, esp->ireg);
intr_done = 0;
if (esp->ireg & (ESP_INTR_S | ESP_INTR_SATN | ESP_INTR_IC)) {
- printk("ESP: unexpected IREG %02x\n", esp->ireg);
+ shost_printk(KERN_INFO, esp->host,
+ "unexpected IREG %02x\n", esp->ireg);
if (esp->ireg & ESP_INTR_IC)
esp_dump_cmd_log(esp);
@@ -2332,12 +2330,13 @@ int scsi_esp_register(struct esp *esp, struct device *dev)
esp_bootup_reset(esp);
- printk(KERN_INFO PFX "esp%u, regs[%1p:%1p] irq[%u]\n",
- esp->host->unique_id, esp->regs, esp->dma_regs,
- esp->host->irq);
- printk(KERN_INFO PFX "esp%u is a %s, %u MHz (ccf=%u), SCSI ID %u\n",
- esp->host->unique_id, esp_chip_names[esp->rev],
- esp->cfreq / 1000000, esp->cfact, esp->scsi_id);
+ dev_printk(KERN_INFO, dev, "esp%u: regs[%1p:%1p] irq[%u]\n",
+ esp->host->unique_id, esp->regs, esp->dma_regs,
+ esp->host->irq);
+ dev_printk(KERN_INFO, dev,
+ "esp%u: is a %s, %u MHz (ccf=%u), SCSI ID %u\n",
+ esp->host->unique_id, esp_chip_names[esp->rev],
+ esp->cfreq / 1000000, esp->cfact, esp->scsi_id);
/* Let the SCSI bus reset settle. */
ssleep(esp_bus_reset_settle);
@@ -2435,19 +2434,20 @@ static int esp_eh_abort_handler(struct scsi_cmnd *cmd)
* XXX much for the final driver.
*/
spin_lock_irqsave(esp->host->host_lock, flags);
- printk(KERN_ERR PFX "esp%d: Aborting command [%p:%02x]\n",
- esp->host->unique_id, cmd, cmd->cmnd[0]);
+ shost_printk(KERN_ERR, esp->host, "Aborting command [%p:%02x]\n",
+ cmd, cmd->cmnd[0]);
ent = esp->active_cmd;
if (ent)
- printk(KERN_ERR PFX "esp%d: Current command [%p:%02x]\n",
- esp->host->unique_id, ent->cmd, ent->cmd->cmnd[0]);
+ shost_printk(KERN_ERR, esp->host,
+ "Current command [%p:%02x]\n",
+ ent->cmd, ent->cmd->cmnd[0]);
list_for_each_entry(ent, &esp->queued_cmds, list) {
- printk(KERN_ERR PFX "esp%d: Queued command [%p:%02x]\n",
- esp->host->unique_id, ent->cmd, ent->cmd->cmnd[0]);
+ shost_printk(KERN_ERR, esp->host, "Queued command [%p:%02x]\n",
+ ent->cmd, ent->cmd->cmnd[0]);
}
list_for_each_entry(ent, &esp->active_cmds, list) {
- printk(KERN_ERR PFX "esp%d: Active command [%p:%02x]\n",
- esp->host->unique_id, ent->cmd, ent->cmd->cmnd[0]);
+ shost_printk(KERN_ERR, esp->host, " Active command [%p:%02x]\n",
+ ent->cmd, ent->cmd->cmnd[0]);
}
esp_dump_cmd_log(esp);
spin_unlock_irqrestore(esp->host->host_lock, flags);
--
1.8.5.2
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 03/10] esp_scsi: convert to dev_printk
2014-11-21 9:27 ` [PATCH 03/10] esp_scsi: convert to dev_printk Hannes Reinecke
@ 2014-11-21 9:37 ` Paolo Bonzini
0 siblings, 0 replies; 31+ messages in thread
From: Paolo Bonzini @ 2014-11-21 9:37 UTC (permalink / raw)
To: Hannes Reinecke, James Bottomley; +Cc: Christoph Hellwig, linux-scsi
On 21/11/2014 10:27, Hannes Reinecke wrote:
> Use dev_printk functions for correct device annotations.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> drivers/scsi/esp_scsi.c | 212 ++++++++++++++++++++++++------------------------
> 1 file changed, 106 insertions(+), 106 deletions(-)
>
> diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
> index 99cd42f..492c51b 100644
> --- a/drivers/scsi/esp_scsi.c
> +++ b/drivers/scsi/esp_scsi.c
> @@ -52,52 +52,52 @@ static u32 esp_debug;
>
> #define esp_log_intr(f, a...) \
> do { if (esp_debug & ESP_DEBUG_INTR) \
> - printk(f, ## a); \
> + shost_printk(KERN_DEBUG, esp->host, f, ## a); \
> } while (0)
>
> #define esp_log_reset(f, a...) \
> do { if (esp_debug & ESP_DEBUG_RESET) \
> - printk(f, ## a); \
> + shost_printk(KERN_DEBUG, esp->host, f, ## a); \
> } while (0)
>
> #define esp_log_msgin(f, a...) \
> do { if (esp_debug & ESP_DEBUG_MSGIN) \
> - printk(f, ## a); \
> + shost_printk(KERN_DEBUG, esp->host, f, ## a); \
> } while (0)
>
> #define esp_log_msgout(f, a...) \
> do { if (esp_debug & ESP_DEBUG_MSGOUT) \
> - printk(f, ## a); \
> + shost_printk(KERN_DEBUG, esp->host, f, ## a); \
> } while (0)
>
> #define esp_log_cmddone(f, a...) \
> do { if (esp_debug & ESP_DEBUG_CMDDONE) \
> - printk(f, ## a); \
> + shost_printk(KERN_DEBUG, esp->host, f, ## a); \
> } while (0)
>
> #define esp_log_disconnect(f, a...) \
> do { if (esp_debug & ESP_DEBUG_DISCONNECT) \
> - printk(f, ## a); \
> + shost_printk(KERN_DEBUG, esp->host, f, ## a); \
> } while (0)
>
> #define esp_log_datastart(f, a...) \
> do { if (esp_debug & ESP_DEBUG_DATASTART) \
> - printk(f, ## a); \
> + shost_printk(KERN_DEBUG, esp->host, f, ## a); \
> } while (0)
>
> #define esp_log_datadone(f, a...) \
> do { if (esp_debug & ESP_DEBUG_DATADONE) \
> - printk(f, ## a); \
> + shost_printk(KERN_DEBUG, esp->host, f, ## a); \
> } while (0)
>
> #define esp_log_reconnect(f, a...) \
> do { if (esp_debug & ESP_DEBUG_RECONNECT) \
> - printk(f, ## a); \
> + shost_printk(KERN_DEBUG, esp->host, f, ## a); \
> } while (0)
>
> #define esp_log_autosense(f, a...) \
> do { if (esp_debug & ESP_DEBUG_AUTOSENSE) \
> - printk(f, ## a); \
> + shost_printk(KERN_DEBUG, esp->host, f, ## a); \
> } while (0)
>
> #define esp_read8(REG) esp->ops->esp_read8(esp, REG)
> @@ -150,19 +150,17 @@ static void esp_dump_cmd_log(struct esp *esp)
> int idx = esp->esp_event_cur;
> int stop = idx;
>
> - printk(KERN_INFO PFX "esp%d: Dumping command log\n",
> - esp->host->unique_id);
> + shost_printk(KERN_INFO, esp->host, "Dumping command log\n");
> do {
> struct esp_event_ent *p = &esp->esp_event_log[idx];
>
> - printk(KERN_INFO PFX "esp%d: ent[%d] %s ",
> - esp->host->unique_id, idx,
> - p->type == ESP_EVENT_TYPE_CMD ? "CMD" : "EVENT");
> -
> - printk("val[%02x] sreg[%02x] seqreg[%02x] "
> - "sreg2[%02x] ireg[%02x] ss[%02x] event[%02x]\n",
> - p->val, p->sreg, p->seqreg,
> - p->sreg2, p->ireg, p->select_state, p->event);
> + shost_printk(KERN_INFO, esp->host,
> + "ent[%d] %s val[%02x] sreg[%02x] seqreg[%02x] "
> + "sreg2[%02x] ireg[%02x] ss[%02x] event[%02x]\n",
> + idx,
> + p->type == ESP_EVENT_TYPE_CMD ? "CMD" : "EVENT",
> + p->val, p->sreg, p->seqreg,
> + p->sreg2, p->ireg, p->select_state, p->event);
>
> idx = (idx + 1) & (ESP_EVENT_LOG_SZ - 1);
> } while (idx != stop);
> @@ -176,9 +174,8 @@ static void esp_flush_fifo(struct esp *esp)
>
> while (esp_read8(ESP_FFLAGS) & ESP_FF_FBYTES) {
> if (--lim == 0) {
> - printk(KERN_ALERT PFX "esp%d: ESP_FF_BYTES "
> - "will not clear!\n",
> - esp->host->unique_id);
> + shost_printk(KERN_ALERT, esp->host,
> + "ESP_FF_BYTES will not clear!\n");
> break;
> }
> udelay(1);
> @@ -383,12 +380,11 @@ static void esp_advance_dma(struct esp *esp, struct esp_cmd_entry *ent,
> p->cur_residue -= len;
> p->tot_residue -= len;
> if (p->cur_residue < 0 || p->tot_residue < 0) {
> - printk(KERN_ERR PFX "esp%d: Data transfer overflow.\n",
> - esp->host->unique_id);
> - printk(KERN_ERR PFX "esp%d: cur_residue[%d] tot_residue[%d] "
> - "len[%u]\n",
> - esp->host->unique_id,
> - p->cur_residue, p->tot_residue, len);
> + shost_printk(KERN_ERR, esp->host,
> + "Data transfer overflow.\n");
> + shost_printk(KERN_ERR, esp->host,
> + "cur_residue[%d] tot_residue[%d] len[%u]\n",
> + p->cur_residue, p->tot_residue, len);
> p->cur_residue = 0;
> p->tot_residue = 0;
> }
> @@ -604,9 +600,8 @@ static void esp_autosense(struct esp *esp, struct esp_cmd_entry *ent)
>
>
> if (!ent->sense_ptr) {
> - esp_log_autosense("esp%d: Doing auto-sense for "
> - "tgt[%d] lun[%d]\n",
> - esp->host->unique_id, tgt, lun);
> + esp_log_autosense("Doing auto-sense for tgt[%d] lun[%d]\n",
> + tgt, lun);
>
> ent->sense_ptr = cmd->sense_buffer;
> ent->sense_dma = esp->ops->map_single(esp,
> @@ -953,8 +948,8 @@ static int esp_check_gross_error(struct esp *esp)
> * - DMA programmed with wrong direction
> * - improper phase change
> */
> - printk(KERN_ERR PFX "esp%d: Gross error sreg[%02x]\n",
> - esp->host->unique_id, esp->sreg);
> + shost_printk(KERN_ERR, esp->host,
> + "Gross error sreg[%02x]\n", esp->sreg);
> /* XXX Reset the chip. XXX */
> return 1;
> }
> @@ -982,14 +977,13 @@ static int esp_check_spur_intr(struct esp *esp)
> * ESP is not, the only possibility is a DMA error.
> */
> if (!esp->ops->dma_error(esp)) {
> - printk(KERN_ERR PFX "esp%d: Spurious irq, "
> - "sreg=%02x.\n",
> - esp->host->unique_id, esp->sreg);
> + shost_printk(KERN_ERR, esp->host,
> + "Spurious irq, sreg=%02x.\n",
> + esp->sreg);
> return -1;
> }
>
> - printk(KERN_ERR PFX "esp%d: DMA error\n",
> - esp->host->unique_id);
> + shost_printk(KERN_ERR, esp->host, "DMA error\n");
>
> /* XXX Reset the chip. XXX */
> return -1;
> @@ -1002,7 +996,7 @@ static int esp_check_spur_intr(struct esp *esp)
>
> static void esp_schedule_reset(struct esp *esp)
> {
> - esp_log_reset("ESP: esp_schedule_reset() from %pf\n",
> + esp_log_reset("esp_schedule_reset() from %pf\n",
> __builtin_return_address(0));
> esp->flags |= ESP_FLAG_RESETTING;
> esp_event(esp, ESP_EVENT_RESET);
> @@ -1019,20 +1013,20 @@ static struct esp_cmd_entry *esp_reconnect_with_tag(struct esp *esp,
> int i;
>
> if (!lp->num_tagged) {
> - printk(KERN_ERR PFX "esp%d: Reconnect w/num_tagged==0\n",
> - esp->host->unique_id);
> + shost_printk(KERN_ERR, esp->host,
> + "Reconnect w/num_tagged==0\n");
> return NULL;
> }
>
> - esp_log_reconnect("ESP: reconnect tag, ");
> + esp_log_reconnect("reconnect tag, ");
>
> for (i = 0; i < ESP_QUICKIRQ_LIMIT; i++) {
> if (esp->ops->irq_pending(esp))
> break;
> }
> if (i == ESP_QUICKIRQ_LIMIT) {
> - printk(KERN_ERR PFX "esp%d: Reconnect IRQ1 timeout\n",
> - esp->host->unique_id);
> + shost_printk(KERN_ERR, esp->host,
> + "Reconnect IRQ1 timeout\n");
> return NULL;
> }
>
> @@ -1043,14 +1037,14 @@ static struct esp_cmd_entry *esp_reconnect_with_tag(struct esp *esp,
> i, esp->ireg, esp->sreg);
>
> if (esp->ireg & ESP_INTR_DC) {
> - printk(KERN_ERR PFX "esp%d: Reconnect, got disconnect.\n",
> - esp->host->unique_id);
> + shost_printk(KERN_ERR, esp->host,
> + "Reconnect, got disconnect.\n");
> return NULL;
> }
>
> if ((esp->sreg & ESP_STAT_PMASK) != ESP_MIP) {
> - printk(KERN_ERR PFX "esp%d: Reconnect, not MIP sreg[%02x].\n",
> - esp->host->unique_id, esp->sreg);
> + shost_printk(KERN_ERR, esp->host,
> + "Reconnect, not MIP sreg[%02x].\n", esp->sreg);
> return NULL;
> }
>
> @@ -1073,8 +1067,7 @@ static struct esp_cmd_entry *esp_reconnect_with_tag(struct esp *esp,
> udelay(1);
> }
> if (i == ESP_RESELECT_TAG_LIMIT) {
> - printk(KERN_ERR PFX "esp%d: Reconnect IRQ2 timeout\n",
> - esp->host->unique_id);
> + shost_printk(KERN_ERR, esp->host, "Reconnect IRQ2 timeout\n");
> return NULL;
> }
> esp->ops->dma_drain(esp);
> @@ -1087,17 +1080,17 @@ static struct esp_cmd_entry *esp_reconnect_with_tag(struct esp *esp,
>
> if (esp->command_block[0] < SIMPLE_QUEUE_TAG ||
> esp->command_block[0] > ORDERED_QUEUE_TAG) {
> - printk(KERN_ERR PFX "esp%d: Reconnect, bad tag "
> - "type %02x.\n",
> - esp->host->unique_id, esp->command_block[0]);
> + shost_printk(KERN_ERR, esp->host,
> + "Reconnect, bad tag type %02x.\n",
> + esp->command_block[0]);
> return NULL;
> }
>
> ent = lp->tagged_cmds[esp->command_block[1]];
> if (!ent) {
> - printk(KERN_ERR PFX "esp%d: Reconnect, no entry for "
> - "tag %02x.\n",
> - esp->host->unique_id, esp->command_block[1]);
> + shost_printk(KERN_ERR, esp->host,
> + "Reconnect, no entry for tag %02x.\n",
> + esp->command_block[1]);
> return NULL;
> }
>
> @@ -1163,9 +1156,9 @@ static int esp_reconnect(struct esp *esp)
> tp = &esp->target[target];
> dev = __scsi_device_lookup_by_target(tp->starget, lun);
> if (!dev) {
> - printk(KERN_ERR PFX "esp%d: Reconnect, no lp "
> - "tgt[%u] lun[%u]\n",
> - esp->host->unique_id, target, lun);
> + shost_printk(KERN_ERR, esp->host,
> + "Reconnect, no lp tgt[%u] lun[%u]\n",
> + target, lun);
> goto do_reset;
> }
> lp = dev->hostdata;
> @@ -1291,8 +1284,8 @@ static int esp_finish_select(struct esp *esp)
> return 0;
> }
>
> - printk("ESP: Unexpected selection completion ireg[%x].\n",
> - esp->ireg);
> + shost_printk(KERN_INFO, esp->host,
> + "Unexpected selection completion ireg[%x]\n", esp->ireg);
> esp_schedule_reset(esp);
> return 0;
> }
> @@ -1556,8 +1549,8 @@ static void esp_msgin_extended(struct esp *esp)
> return;
> }
>
> - printk("ESP: Unexpected extended msg type %x\n",
> - esp->msg_in[2]);
> + shost_printk(KERN_INFO, esp->host,
> + "Unexpected extended msg type %x\n", esp->msg_in[2]);
>
> esp->msg_out[0] = ABORT_TASK_SET;
> esp->msg_out_len = 1;
> @@ -1574,7 +1567,8 @@ static int esp_msgin_process(struct esp *esp)
>
> if (msg0 & 0x80) {
> /* Identify */
> - printk("ESP: Unexpected msgin identify\n");
> + shost_printk(KERN_INFO, esp->host,
> + "Unexpected msgin identify\n");
> return 0;
> }
>
> @@ -1673,8 +1667,9 @@ again:
> break;
>
> default:
> - printk("ESP: Unexpected phase, sreg=%02x\n",
> - esp->sreg);
> + shost_printk(KERN_INFO, esp->host,
> + "Unexpected phase, sreg=%02x\n",
> + esp->sreg);
> esp_schedule_reset(esp);
> return 0;
> }
> @@ -1708,18 +1703,17 @@ again:
> esp->data_dma_len = dma_len;
>
> if (!dma_len) {
> - printk(KERN_ERR PFX "esp%d: DMA length is zero!\n",
> - esp->host->unique_id);
> - printk(KERN_ERR PFX "esp%d: cur adr[%08llx] len[%08x]\n",
> - esp->host->unique_id,
> - (unsigned long long)esp_cur_dma_addr(ent, cmd),
> - esp_cur_dma_len(ent, cmd));
> + shost_printk(KERN_ERR, esp->host,
> + "DMA length is zero!\n");
> + shost_printk(KERN_ERR, esp->host,
> + "cur adr[%08llx] len[%08x]\n",
> + (unsigned long long)esp_cur_dma_addr(ent, cmd),
> + esp_cur_dma_len(ent, cmd));
> esp_schedule_reset(esp);
> return 0;
> }
>
> - esp_log_datastart("ESP: start data addr[%08llx] len[%u] "
> - "write(%d)\n",
> + esp_log_datastart("start data addr[%08llx] len[%u] write(%d)\n",
> (unsigned long long)dma_addr, dma_len, write);
>
> esp->ops->send_dma_cmd(esp, dma_addr, dma_len, dma_len,
> @@ -1733,7 +1727,8 @@ again:
> int bytes_sent;
>
> if (esp->ops->dma_error(esp)) {
> - printk("ESP: data done, DMA error, resetting\n");
> + shost_printk(KERN_INFO, esp->host,
> + "data done, DMA error, resetting\n");
> esp_schedule_reset(esp);
> return 0;
> }
> @@ -1749,14 +1744,15 @@ again:
> /* We should always see exactly a bus-service
> * interrupt at the end of a successful transfer.
> */
> - printk("ESP: data done, not BSERV, resetting\n");
> + shost_printk(KERN_INFO, esp->host,
> + "data done, not BSERV, resetting\n");
> esp_schedule_reset(esp);
> return 0;
> }
>
> bytes_sent = esp_data_bytes_sent(esp, ent, cmd);
>
> - esp_log_datadone("ESP: data done flgs[%x] sent[%d]\n",
> + esp_log_datadone("data done flgs[%x] sent[%d]\n",
> ent->flags, bytes_sent);
>
> if (bytes_sent < 0) {
> @@ -1785,8 +1781,9 @@ again:
> }
>
> if (ent->message != COMMAND_COMPLETE) {
> - printk("ESP: Unexpected message %x in status\n",
> - ent->message);
> + shost_printk(KERN_INFO, esp->host,
> + "Unexpected message %x in status\n",
> + ent->message);
> esp_schedule_reset(esp);
> return 0;
> }
> @@ -1804,8 +1801,7 @@ again:
> scsi_esp_cmd(esp, ESP_CMD_ESEL);
>
> if (ent->message == COMMAND_COMPLETE) {
> - esp_log_cmddone("ESP: Command done status[%x] "
> - "message[%x]\n",
> + esp_log_cmddone("Command done status[%x] message[%x]\n",
> ent->status, ent->message);
> if (ent->status == SAM_STAT_TASK_SET_FULL)
> esp_event_queue_full(esp, ent);
> @@ -1821,16 +1817,16 @@ again:
> DID_OK));
> }
> } else if (ent->message == DISCONNECT) {
> - esp_log_disconnect("ESP: Disconnecting tgt[%d] "
> - "tag[%x:%x]\n",
> + esp_log_disconnect("Disconnecting tgt[%d] tag[%x:%x]\n",
> cmd->device->id,
> ent->tag[0], ent->tag[1]);
>
> esp->active_cmd = NULL;
> esp_maybe_execute_command(esp);
> } else {
> - printk("ESP: Unexpected message %x in freebus\n",
> - ent->message);
> + shost_printk(KERN_INFO, esp->host,
> + "Unexpected message %x in freebus\n",
> + ent->message);
> esp_schedule_reset(esp);
> return 0;
> }
> @@ -1917,7 +1913,7 @@ again:
> val = esp_read8(ESP_FDATA);
> esp->msg_in[esp->msg_in_len++] = val;
>
> - esp_log_msgin("ESP: Got msgin byte %x\n", val);
> + esp_log_msgin("Got msgin byte %x\n", val);
>
> if (!esp_msgin_process(esp))
> esp->msg_in_len = 0;
> @@ -1930,7 +1926,8 @@ again:
> if (esp->event != ESP_EVENT_FREE_BUS)
> esp_event(esp, ESP_EVENT_CHECK_PHASE);
> } else {
> - printk("ESP: MSGIN neither BSERV not FDON, resetting");
> + shost_printk(KERN_INFO, esp->host,
> + "MSGIN neither BSERV not FDON, resetting");
> esp_schedule_reset(esp);
> return 0;
> }
> @@ -1961,8 +1958,8 @@ again:
> break;
>
> default:
> - printk("ESP: Unexpected event %x, resetting\n",
> - esp->event);
> + shost_printk(KERN_INFO, esp->host,
> + "Unexpected event %x, resetting\n", esp->event);
> esp_schedule_reset(esp);
> return 0;
> break;
> @@ -2085,14 +2082,15 @@ static void __esp_interrupt(struct esp *esp)
> }
> }
>
> - esp_log_intr("ESP: intr sreg[%02x] seqreg[%02x] "
> + esp_log_intr("intr sreg[%02x] seqreg[%02x] "
> "sreg2[%02x] ireg[%02x]\n",
> esp->sreg, esp->seqreg, esp->sreg2, esp->ireg);
>
> intr_done = 0;
>
> if (esp->ireg & (ESP_INTR_S | ESP_INTR_SATN | ESP_INTR_IC)) {
> - printk("ESP: unexpected IREG %02x\n", esp->ireg);
> + shost_printk(KERN_INFO, esp->host,
> + "unexpected IREG %02x\n", esp->ireg);
> if (esp->ireg & ESP_INTR_IC)
> esp_dump_cmd_log(esp);
>
> @@ -2332,12 +2330,13 @@ int scsi_esp_register(struct esp *esp, struct device *dev)
>
> esp_bootup_reset(esp);
>
> - printk(KERN_INFO PFX "esp%u, regs[%1p:%1p] irq[%u]\n",
> - esp->host->unique_id, esp->regs, esp->dma_regs,
> - esp->host->irq);
> - printk(KERN_INFO PFX "esp%u is a %s, %u MHz (ccf=%u), SCSI ID %u\n",
> - esp->host->unique_id, esp_chip_names[esp->rev],
> - esp->cfreq / 1000000, esp->cfact, esp->scsi_id);
> + dev_printk(KERN_INFO, dev, "esp%u: regs[%1p:%1p] irq[%u]\n",
> + esp->host->unique_id, esp->regs, esp->dma_regs,
> + esp->host->irq);
> + dev_printk(KERN_INFO, dev,
> + "esp%u: is a %s, %u MHz (ccf=%u), SCSI ID %u\n",
> + esp->host->unique_id, esp_chip_names[esp->rev],
> + esp->cfreq / 1000000, esp->cfact, esp->scsi_id);
>
> /* Let the SCSI bus reset settle. */
> ssleep(esp_bus_reset_settle);
> @@ -2435,19 +2434,20 @@ static int esp_eh_abort_handler(struct scsi_cmnd *cmd)
> * XXX much for the final driver.
> */
> spin_lock_irqsave(esp->host->host_lock, flags);
> - printk(KERN_ERR PFX "esp%d: Aborting command [%p:%02x]\n",
> - esp->host->unique_id, cmd, cmd->cmnd[0]);
> + shost_printk(KERN_ERR, esp->host, "Aborting command [%p:%02x]\n",
> + cmd, cmd->cmnd[0]);
> ent = esp->active_cmd;
> if (ent)
> - printk(KERN_ERR PFX "esp%d: Current command [%p:%02x]\n",
> - esp->host->unique_id, ent->cmd, ent->cmd->cmnd[0]);
> + shost_printk(KERN_ERR, esp->host,
> + "Current command [%p:%02x]\n",
> + ent->cmd, ent->cmd->cmnd[0]);
> list_for_each_entry(ent, &esp->queued_cmds, list) {
> - printk(KERN_ERR PFX "esp%d: Queued command [%p:%02x]\n",
> - esp->host->unique_id, ent->cmd, ent->cmd->cmnd[0]);
> + shost_printk(KERN_ERR, esp->host, "Queued command [%p:%02x]\n",
> + ent->cmd, ent->cmd->cmnd[0]);
> }
> list_for_each_entry(ent, &esp->active_cmds, list) {
> - printk(KERN_ERR PFX "esp%d: Active command [%p:%02x]\n",
> - esp->host->unique_id, ent->cmd, ent->cmd->cmnd[0]);
> + shost_printk(KERN_ERR, esp->host, " Active command [%p:%02x]\n",
> + ent->cmd, ent->cmd->cmnd[0]);
> }
> esp_dump_cmd_log(esp);
> spin_unlock_irqrestore(esp->host->host_lock, flags);
>
Looks good.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 04/10] esp_scsi: debug event and command
2014-11-21 9:27 [PATCH 00/10] Re-implement am53c974 driver Hannes Reinecke
` (2 preceding siblings ...)
2014-11-21 9:27 ` [PATCH 03/10] esp_scsi: convert to dev_printk Hannes Reinecke
@ 2014-11-21 9:27 ` Hannes Reinecke
2014-11-21 9:38 ` Paolo Bonzini
2014-11-21 9:27 ` [PATCH 05/10] esp_scsi: read status registers Hannes Reinecke
` (6 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Hannes Reinecke @ 2014-11-21 9:27 UTC (permalink / raw)
To: James Bottomley
Cc: Christoph Hellwig, Paolo Bonzini, linux-scsi, Hannes Reinecke
Add new debug definitions for event and command logging.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/esp_scsi.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
index 492c51b..fe3278e 100644
--- a/drivers/scsi/esp_scsi.c
+++ b/drivers/scsi/esp_scsi.c
@@ -49,6 +49,8 @@ static u32 esp_debug;
#define ESP_DEBUG_DATADONE 0x00000100
#define ESP_DEBUG_RECONNECT 0x00000200
#define ESP_DEBUG_AUTOSENSE 0x00000400
+#define ESP_DEBUG_EVENT 0x00000800
+#define ESP_DEBUG_COMMAND 0x00001000
#define esp_log_intr(f, a...) \
do { if (esp_debug & ESP_DEBUG_INTR) \
@@ -100,6 +102,16 @@ do { if (esp_debug & ESP_DEBUG_AUTOSENSE) \
shost_printk(KERN_DEBUG, esp->host, f, ## a); \
} while (0)
+#define esp_log_event(f, a...) \
+do { if (esp_debug & ESP_DEBUG_EVENT) \
+ shost_printk(KERN_DEBUG, esp->host, f, ## a); \
+} while (0)
+
+#define esp_log_command(f, a...) \
+do { if (esp_debug & ESP_DEBUG_COMMAND) \
+ shost_printk(KERN_DEBUG, esp->host, f, ## a); \
+} while (0)
+
#define esp_read8(REG) esp->ops->esp_read8(esp, REG)
#define esp_write8(VAL,REG) esp->ops->esp_write8(esp, VAL, REG)
@@ -126,6 +138,7 @@ void scsi_esp_cmd(struct esp *esp, u8 val)
esp->esp_event_cur = (idx + 1) & (ESP_EVENT_LOG_SZ - 1);
+ esp_log_command("cmd[%02x]\n", val);
esp_write8(val, ESP_CMD);
}
EXPORT_SYMBOL(scsi_esp_cmd);
@@ -1638,6 +1651,8 @@ static int esp_process_event(struct esp *esp)
again:
write = 0;
+ esp_log_event("process event %d phase %x\n",
+ esp->event, esp->sreg & ESP_STAT_PMASK);
switch (esp->event) {
case ESP_EVENT_CHECK_PHASE:
switch (esp->sreg & ESP_STAT_PMASK) {
--
1.8.5.2
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 04/10] esp_scsi: debug event and command
2014-11-21 9:27 ` [PATCH 04/10] esp_scsi: debug event and command Hannes Reinecke
@ 2014-11-21 9:38 ` Paolo Bonzini
0 siblings, 0 replies; 31+ messages in thread
From: Paolo Bonzini @ 2014-11-21 9:38 UTC (permalink / raw)
To: Hannes Reinecke, James Bottomley; +Cc: Christoph Hellwig, linux-scsi
On 21/11/2014 10:27, Hannes Reinecke wrote:
> Add new debug definitions for event and command logging.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> drivers/scsi/esp_scsi.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
> index 492c51b..fe3278e 100644
> --- a/drivers/scsi/esp_scsi.c
> +++ b/drivers/scsi/esp_scsi.c
> @@ -49,6 +49,8 @@ static u32 esp_debug;
> #define ESP_DEBUG_DATADONE 0x00000100
> #define ESP_DEBUG_RECONNECT 0x00000200
> #define ESP_DEBUG_AUTOSENSE 0x00000400
> +#define ESP_DEBUG_EVENT 0x00000800
> +#define ESP_DEBUG_COMMAND 0x00001000
>
> #define esp_log_intr(f, a...) \
> do { if (esp_debug & ESP_DEBUG_INTR) \
> @@ -100,6 +102,16 @@ do { if (esp_debug & ESP_DEBUG_AUTOSENSE) \
> shost_printk(KERN_DEBUG, esp->host, f, ## a); \
> } while (0)
>
> +#define esp_log_event(f, a...) \
> +do { if (esp_debug & ESP_DEBUG_EVENT) \
> + shost_printk(KERN_DEBUG, esp->host, f, ## a); \
> +} while (0)
> +
> +#define esp_log_command(f, a...) \
> +do { if (esp_debug & ESP_DEBUG_COMMAND) \
> + shost_printk(KERN_DEBUG, esp->host, f, ## a); \
> +} while (0)
> +
> #define esp_read8(REG) esp->ops->esp_read8(esp, REG)
> #define esp_write8(VAL,REG) esp->ops->esp_write8(esp, VAL, REG)
>
> @@ -126,6 +138,7 @@ void scsi_esp_cmd(struct esp *esp, u8 val)
>
> esp->esp_event_cur = (idx + 1) & (ESP_EVENT_LOG_SZ - 1);
>
> + esp_log_command("cmd[%02x]\n", val);
> esp_write8(val, ESP_CMD);
> }
> EXPORT_SYMBOL(scsi_esp_cmd);
> @@ -1638,6 +1651,8 @@ static int esp_process_event(struct esp *esp)
>
> again:
> write = 0;
> + esp_log_event("process event %d phase %x\n",
> + esp->event, esp->sreg & ESP_STAT_PMASK);
> switch (esp->event) {
> case ESP_EVENT_CHECK_PHASE:
> switch (esp->sreg & ESP_STAT_PMASK) {
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 05/10] esp_scsi: read status registers
2014-11-21 9:27 [PATCH 00/10] Re-implement am53c974 driver Hannes Reinecke
` (3 preceding siblings ...)
2014-11-21 9:27 ` [PATCH 04/10] esp_scsi: debug event and command Hannes Reinecke
@ 2014-11-21 9:27 ` Hannes Reinecke
2014-11-21 9:43 ` Paolo Bonzini
2014-11-21 9:27 ` [PATCH 06/10] scsi: add 'am53c974' driver Hannes Reinecke
` (5 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Hannes Reinecke @ 2014-11-21 9:27 UTC (permalink / raw)
To: James Bottomley
Cc: Christoph Hellwig, Paolo Bonzini, linux-scsi, Hannes Reinecke
A read to ESP_INTRPT will clear ESP_STATUS and ESP_SSTEP. So read
all status registers in one go to avoid losing information.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/esp_scsi.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
index fe3278e..92ab921 100644
--- a/drivers/scsi/esp_scsi.c
+++ b/drivers/scsi/esp_scsi.c
@@ -982,7 +982,6 @@ static int esp_check_spur_intr(struct esp *esp)
default:
if (!(esp->sreg & ESP_STAT_INTR)) {
- esp->ireg = esp_read8(ESP_INTRPT);
if (esp->ireg & ESP_INTR_SR)
return 1;
@@ -2056,7 +2055,12 @@ static void __esp_interrupt(struct esp *esp)
int finish_reset, intr_done;
u8 phase;
+ /*
+ * Once INTRPT is read STATUS and SSTEP are cleared.
+ */
esp->sreg = esp_read8(ESP_STATUS);
+ esp->seqreg = esp_read8(ESP_SSTEP);
+ esp->ireg = esp_read8(ESP_INTRPT);
if (esp->flags & ESP_FLAG_RESETTING) {
finish_reset = 1;
@@ -2069,8 +2073,6 @@ static void __esp_interrupt(struct esp *esp)
return;
}
- esp->ireg = esp_read8(ESP_INTRPT);
-
if (esp->ireg & ESP_INTR_SR)
finish_reset = 1;
--
1.8.5.2
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 05/10] esp_scsi: read status registers
2014-11-21 9:27 ` [PATCH 05/10] esp_scsi: read status registers Hannes Reinecke
@ 2014-11-21 9:43 ` Paolo Bonzini
0 siblings, 0 replies; 31+ messages in thread
From: Paolo Bonzini @ 2014-11-21 9:43 UTC (permalink / raw)
To: Hannes Reinecke, James Bottomley; +Cc: Christoph Hellwig, linux-scsi
On 21/11/2014 10:27, Hannes Reinecke wrote:
> A read to ESP_INTRPT will clear ESP_STATUS and ESP_SSTEP. So read
> all status registers in one go to avoid losing information.
(ESP_STAT_TCNT is actually kept in the status register, it is cleared by
writing TCLO/MID/HI).
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> drivers/scsi/esp_scsi.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
> index fe3278e..92ab921 100644
> --- a/drivers/scsi/esp_scsi.c
> +++ b/drivers/scsi/esp_scsi.c
> @@ -982,7 +982,6 @@ static int esp_check_spur_intr(struct esp *esp)
>
> default:
> if (!(esp->sreg & ESP_STAT_INTR)) {
> - esp->ireg = esp_read8(ESP_INTRPT);
> if (esp->ireg & ESP_INTR_SR)
> return 1;
>
> @@ -2056,7 +2055,12 @@ static void __esp_interrupt(struct esp *esp)
> int finish_reset, intr_done;
> u8 phase;
>
> + /*
> + * Once INTRPT is read STATUS and SSTEP are cleared.
> + */
> esp->sreg = esp_read8(ESP_STATUS);
> + esp->seqreg = esp_read8(ESP_SSTEP);
> + esp->ireg = esp_read8(ESP_INTRPT);
>
> if (esp->flags & ESP_FLAG_RESETTING) {
> finish_reset = 1;
> @@ -2069,8 +2073,6 @@ static void __esp_interrupt(struct esp *esp)
> return;
> }
>
> - esp->ireg = esp_read8(ESP_INTRPT);
> -
> if (esp->ireg & ESP_INTR_SR)
> finish_reset = 1;
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 06/10] scsi: add 'am53c974' driver
2014-11-21 9:27 [PATCH 00/10] Re-implement am53c974 driver Hannes Reinecke
` (4 preceding siblings ...)
2014-11-21 9:27 ` [PATCH 05/10] esp_scsi: read status registers Hannes Reinecke
@ 2014-11-21 9:27 ` Hannes Reinecke
2014-11-21 10:14 ` Paolo Bonzini
2014-11-21 10:20 ` Paolo Bonzini
2014-11-21 9:27 ` [PATCH 07/10] esp: Use FIFO for command submission Hannes Reinecke
` (4 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Hannes Reinecke @ 2014-11-21 9:27 UTC (permalink / raw)
To: James Bottomley
Cc: Christoph Hellwig, Paolo Bonzini, linux-scsi, Hannes Reinecke
This patch adds a new implementation for the Tekram DC-390T /
AMD AM53c974 SCSI controller, based on the generic
esp_scsi infrastructure.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/Kconfig | 18 ++
drivers/scsi/Makefile | 1 +
drivers/scsi/am53c974.c | 523 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 542 insertions(+)
create mode 100644 drivers/scsi/am53c974.c
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 21bc674..497e7d5 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -1357,6 +1357,24 @@ config SCSI_DC390T
To compile this driver as a module, choose M here: the
module will be called tmscsim.
+config SCSI_AM53C974
+ tristate "Tekram DC390(T) and Am53/79C974 SCSI support (new driver)"
+ depends on PCI && SCSI
+ select SCSI_SPI_ATTRS
+ ---help---
+ This driver supports PCI SCSI host adapters based on the Am53C974A
+ chip, e.g. Tekram DC390(T), DawiControl 2974 and some onboard
+ PCscsi/PCnet (Am53/79C974) solutions.
+ This is a new implementation base on the generic esp_scsi driver.
+
+ Documentation can be found in <file:Documentation/scsi/tmscsim.txt>.
+
+ Note that this driver does NOT support Tekram DC390W/U/F, which are
+ based on NCR/Symbios chips. Use "NCR53C8XX SCSI support" for those.
+
+ To compile this driver as a module, choose M here: the
+ module will be called am53c974.
+
config SCSI_T128
tristate "Trantor T128/T128F/T228 SCSI support"
depends on ISA && SCSI
diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
index 76f8932..7f974fc 100644
--- a/drivers/scsi/Makefile
+++ b/drivers/scsi/Makefile
@@ -101,6 +101,7 @@ obj-$(CONFIG_SCSI_7000FASST) += wd7000.o
obj-$(CONFIG_SCSI_EATA) += eata.o
obj-$(CONFIG_SCSI_DC395x) += dc395x.o
obj-$(CONFIG_SCSI_DC390T) += tmscsim.o
+obj-$(CONFIG_SCSI_AM53C974) += esp_scsi.o am53c974.o
obj-$(CONFIG_MEGARAID_LEGACY) += megaraid.o
obj-$(CONFIG_MEGARAID_NEWGEN) += megaraid/
obj-$(CONFIG_MEGARAID_SAS) += megaraid/
diff --git a/drivers/scsi/am53c974.c b/drivers/scsi/am53c974.c
new file mode 100644
index 0000000..b9af8b0
--- /dev/null
+++ b/drivers/scsi/am53c974.c
@@ -0,0 +1,523 @@
+/*
+ * AMD am53c974 driver.
+ * Copyright (c) 2014 Hannes Reinecke, SUSE Linux GmbH
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/pci.h>
+#include <linux/interrupt.h>
+
+#include <scsi/scsi_host.h>
+
+#include "esp_scsi.h"
+
+#define DRV_MODULE_NAME "am53c974"
+#define DRV_MODULE_VERSION "1.00"
+
+// #define ESP_DMA_DEBUG
+
+#define ESP_DMA_CMD 0x10
+#define ESP_DMA_STC 0x11
+#define ESP_DMA_SPA 0x12
+#define ESP_DMA_WBC 0x13
+#define ESP_DMA_WAC 0x14
+#define ESP_DMA_STATUS 0x15
+#define ESP_DMA_SMDLA 0x16
+#define ESP_DMA_WMAC 0x17
+
+#define ESP_DMA_CMD_IDLE 0x00
+#define ESP_DMA_CMD_BLAST 0x01
+#define ESP_DMA_CMD_ABORT 0x02
+#define ESP_DMA_CMD_START 0x03
+#define ESP_DMA_CMD_MASK 0x03
+#define ESP_DMA_CMD_DIAG 0x04
+#define ESP_DMA_CMD_MDL 0x10
+#define ESP_DMA_CMD_INTE_P 0x20
+#define ESP_DMA_CMD_INTE_D 0x40
+#define ESP_DMA_CMD_DIR 0x80
+
+#define ESP_DMA_STAT_PWDN 0x01
+#define ESP_DMA_STAT_ERROR 0x02
+#define ESP_DMA_STAT_ABORT 0x04
+#define ESP_DMA_STAT_DONE 0x08
+#define ESP_DMA_STAT_SCSIINT 0x10
+#define ESP_DMA_STAT_BCMPLT 0x20
+
+/* EEPROM is accessed with 16-bit values */
+#define DC390_EEPROM_READ 0x80
+#define DC390_EEPROM_LEN 0x40
+
+/*
+ * DC390 EEPROM
+ *
+ * 8 * 4 bytes of per-device options
+ * followed by HBA specific options
+ */
+
+/* Per-device options */
+#define DC390_EE_MODE1 0x00
+#define DC390_EE_SPEED 0x01
+
+/* HBA-specific options */
+#define DC390_EE_ADAPT_SCSI_ID 0x40
+#define DC390_EE_MODE2 0x41
+#define DC390_EE_DELAY 0x42
+#define DC390_EE_TAG_CMD_NUM 0x43
+
+#define DC390_EE_MODE1_PARITY_CHK 0x01
+#define DC390_EE_MODE1_SYNC_NEGO 0x02
+#define DC390_EE_MODE1_EN_DISC 0x04
+#define DC390_EE_MODE1_SEND_START 0x08
+#define DC390_EE_MODE1_TCQ 0x10
+
+#define DC390_EE_MODE2_MORE_2DRV 0x01
+#define DC390_EE_MODE2_GREATER_1G 0x02
+#define DC390_EE_MODE2_RST_SCSI_BUS 0x04
+#define DC390_EE_MODE2_ACTIVE_NEGATION 0x08
+#define DC390_EE_MODE2_NO_SEEK 0x10
+#define DC390_EE_MODE2_LUN_CHECK 0x20
+
+struct pci_esp_priv {
+ struct esp *esp;
+ u8 dma_status;
+};
+
+#define PCI_ESP_GET_PRIV(esp) ((struct pci_esp_priv *) \
+ pci_get_drvdata((struct pci_dev *) \
+ (esp)->dev))
+
+static void pci_esp_dma_drain(struct esp *esp);
+
+static void pci_esp_write8(struct esp *esp, u8 val, unsigned long reg)
+{
+ iowrite8(val, esp->regs + (reg * 4UL));
+}
+
+static u8 pci_esp_read8(struct esp *esp, unsigned long reg)
+{
+ return ioread8(esp->regs + (reg * 4UL));
+}
+
+static void pci_esp_write32(struct esp *esp, u32 val, unsigned long reg)
+{
+ return iowrite32(val, esp->regs + (reg * 4UL));
+}
+
+static dma_addr_t pci_esp_map_single(struct esp *esp, void *buf,
+ size_t sz, int dir)
+{
+ return pci_map_single(esp->dev, buf, sz, dir);
+}
+
+static int pci_esp_map_sg(struct esp *esp, struct scatterlist *sg,
+ int num_sg, int dir)
+{
+ return pci_map_sg(esp->dev, sg, num_sg, dir);
+}
+
+static void pci_esp_unmap_single(struct esp *esp, dma_addr_t addr,
+ size_t sz, int dir)
+{
+ pci_unmap_single(esp->dev, addr, sz, dir);
+}
+
+static void pci_esp_unmap_sg(struct esp *esp, struct scatterlist *sg,
+ int num_sg, int dir)
+{
+ pci_unmap_sg(esp->dev, sg, num_sg, dir);
+}
+
+static int pci_esp_irq_pending(struct esp *esp)
+{
+ struct pci_esp_priv *pep = PCI_ESP_GET_PRIV(esp);
+
+ pep->dma_status = pci_esp_read8(esp, ESP_DMA_STATUS);
+#ifdef ESP_DMA_DEBUG
+ if (pep->dma_status)
+ shost_printk(KERN_INFO, esp->host, "dma intr dreg[%02x]\n",
+ pep->dma_status);
+#endif
+ if (pep->dma_status & (ESP_DMA_STAT_ERROR |
+ ESP_DMA_STAT_ABORT |
+ ESP_DMA_STAT_DONE |
+ ESP_DMA_STAT_SCSIINT))
+ return 1;
+
+ return 0;
+}
+
+static void pci_esp_reset_dma(struct esp *esp)
+{
+ /* Nothing to do ? */
+}
+
+static void pci_esp_dma_drain(struct esp *esp)
+{
+ u8 resid;
+ int lim = 1000;
+
+
+ if ((esp->sreg & ESP_STAT_PMASK) == ESP_DOP ||
+ (esp->sreg & ESP_STAT_PMASK) == ESP_DIP)
+ /* Data-In or Data-Out, nothing to be done */
+ return;
+
+ while (--lim > 0) {
+ resid = pci_esp_read8(esp, ESP_FFLAGS) & ESP_FF_FBYTES;
+ if (resid <= 1)
+ break;
+ }
+ if (resid > 1) {
+ /* FIFO not cleared */
+ shost_printk(KERN_INFO, esp->host,
+ "FIFO not cleared, %d bytes left\n",
+ resid);
+ }
+
+ /*
+ * When there is a residual BCMPLT will never be set
+ * (obviously). But we still have to issue the BLAST
+ * command, otherwise the data will not being transferred.
+ * But we'll never know when the BLAST operation is
+ * finished. So check for some time and give up eventually.
+ */
+ lim = 1000;
+ pci_esp_write8(esp, ESP_DMA_CMD_DIR | ESP_DMA_CMD_BLAST, ESP_DMA_CMD);
+ while (pci_esp_read8(esp, ESP_DMA_STATUS) & ESP_DMA_STAT_BCMPLT) {
+ if (--lim == 0)
+ break;
+ }
+ pci_esp_write8(esp, ESP_DMA_CMD_DIR | ESP_DMA_CMD_IDLE, ESP_DMA_CMD);
+#ifdef ESP_DMA_DEBUG
+ shost_printk(KERN_INFO, esp->host,
+ "DMA blast done (%d tries, %d bytes left)\n", lim, resid);
+#endif
+}
+
+static void pci_esp_dma_invalidate(struct esp *esp)
+{
+ struct pci_esp_priv *pep = PCI_ESP_GET_PRIV(esp);
+
+#ifdef ESP_DMA_DEBUG
+ shost_printk(KERN_INFO, esp->host, "invalidate DMA\n");
+#endif
+ pci_esp_write8(esp, ESP_DMA_CMD_IDLE, ESP_DMA_CMD);
+ pep->dma_status = 0;
+}
+
+static int pci_esp_dma_error(struct esp *esp)
+{
+ struct pci_esp_priv *pep = PCI_ESP_GET_PRIV(esp);
+
+ if (pep->dma_status & ESP_DMA_STAT_ERROR) {
+ u8 dma_cmd = pci_esp_read8(esp, ESP_DMA_CMD);
+
+ if ((dma_cmd & ESP_DMA_CMD_MASK) == ESP_DMA_CMD_START)
+ pci_esp_write8(esp, ESP_DMA_CMD_ABORT, ESP_DMA_CMD);
+
+ return 1;
+ }
+ if (pep->dma_status & ESP_DMA_STAT_ABORT) {
+ pci_esp_write8(esp, ESP_DMA_CMD_IDLE, ESP_DMA_CMD);
+ pep->dma_status = pci_esp_read8(esp, ESP_DMA_CMD);
+ return 1;
+ }
+ return 0;
+}
+
+static void pci_esp_send_dma_cmd(struct esp *esp, u32 addr, u32 esp_count,
+ u32 dma_count, int write, u8 cmd)
+{
+ struct pci_esp_priv *pep = PCI_ESP_GET_PRIV(esp);
+ u32 val = 0;
+
+ BUG_ON(!(cmd & ESP_CMD_DMA));
+
+ pep->dma_status = 0;
+
+ /* Set DMA engine to IDLE */
+ if (write)
+ /* DMA write direction logic is inverted */
+ val |= ESP_DMA_CMD_DIR;
+ pci_esp_write8(esp, ESP_DMA_CMD_IDLE | val, ESP_DMA_CMD);
+
+ pci_esp_write8(esp, (esp_count >> 0) & 0xff, ESP_TCLOW);
+ pci_esp_write8(esp, (esp_count >> 8) & 0xff, ESP_TCMED);
+
+ pci_esp_write32(esp, esp_count, ESP_DMA_STC);
+ pci_esp_write32(esp, addr, ESP_DMA_SPA);
+
+#ifdef ESP_DMA_DEBUG
+ shost_printk(KERN_INFO, esp->host, "start dma addr[%x] count[%d:%d]\n",
+ addr, esp_count, dma_count);
+#endif
+ scsi_esp_cmd(esp, cmd);
+ /* Send DMA Start command */
+ pci_esp_write8(esp, ESP_DMA_CMD_START | val, ESP_DMA_CMD);
+}
+
+static const struct esp_driver_ops pci_esp_ops = {
+ .esp_write8 = pci_esp_write8,
+ .esp_read8 = pci_esp_read8,
+ .map_single = pci_esp_map_single,
+ .map_sg = pci_esp_map_sg,
+ .unmap_single = pci_esp_unmap_single,
+ .unmap_sg = pci_esp_unmap_sg,
+ .irq_pending = pci_esp_irq_pending,
+ .reset_dma = pci_esp_reset_dma,
+ .dma_drain = pci_esp_dma_drain,
+ .dma_invalidate = pci_esp_dma_invalidate,
+ .send_dma_cmd = pci_esp_send_dma_cmd,
+ .dma_error = pci_esp_dma_error,
+};
+
+/*
+ * Read DC-390 eeprom
+ */
+static void dc390_eeprom_prepare_read(struct pci_dev *pdev, u8 cmd)
+{
+ u8 carryFlag = 1, j = 0x80, bval;
+ int i;
+
+ for (i = 0; i < 9; i++) {
+ if (carryFlag) {
+ pci_write_config_byte(pdev, 0x80, 0x40);
+ bval = 0xc0;
+ } else
+ bval = 0x80;
+
+ udelay(160);
+ pci_write_config_byte(pdev, 0x80, bval);
+ udelay(160);
+ pci_write_config_byte(pdev, 0x80, 0);
+ udelay(160);
+
+ carryFlag = (cmd & j) ? 1 : 0;
+ j >>= 1;
+ }
+}
+
+static u16 dc390_eeprom_get_data(struct pci_dev *pdev)
+{
+ int i;
+ u16 wval = 0;
+ u8 bval;
+
+ for (i = 0; i < 16; i++) {
+ wval <<= 1;
+
+ pci_write_config_byte(pdev, 0x80, 0x80);
+ udelay(160);
+ pci_write_config_byte(pdev, 0x80, 0x40);
+ udelay(160);
+ pci_read_config_byte(pdev, 0x00, &bval);
+
+ if (bval == 0x22)
+ wval |= 1;
+ }
+
+ return wval;
+}
+
+static void dc390_read_eeprom(struct pci_dev *pdev, u16 *ptr)
+{
+ u8 cmd = DC390_EEPROM_READ, i;
+
+ for (i = 0; i < DC390_EEPROM_LEN; i++) {
+ pci_write_config_byte(pdev, 0xc0, 0);
+ udelay(160);
+
+ dc390_eeprom_prepare_read(pdev, cmd++);
+ *ptr++ = dc390_eeprom_get_data(pdev);
+
+ pci_write_config_byte(pdev, 0x80, 0);
+ pci_write_config_byte(pdev, 0x80, 0);
+ udelay(160);
+ }
+}
+
+static void dc390_check_eeprom(struct esp *esp)
+{
+ u8 EEbuf[128];
+ u16 *ptr = (u16 *)EEbuf, wval = 0;
+ int i;
+
+ dc390_read_eeprom((struct pci_dev *)esp->dev, ptr);
+
+ for (i = 0; i < DC390_EEPROM_LEN; i++, ptr++)
+ wval += *ptr;
+
+ /* no Tekram EEprom found */
+ if (wval != 0x1234) {
+ struct pci_dev *pdev = esp->dev;
+ dev_printk(KERN_INFO, &pdev->dev,
+ "No valid Tekram EEprom found\n");
+ return;
+ }
+ esp->scsi_id = EEbuf[DC390_EE_ADAPT_SCSI_ID];
+ esp->num_tags = 2 << EEbuf[DC390_EE_TAG_CMD_NUM];
+}
+
+static int pci_esp_probe_one(struct pci_dev *pdev,
+ const struct pci_device_id *id)
+{
+ struct scsi_host_template *hostt = &scsi_esp_template;
+ int err = -ENODEV;
+ struct Scsi_Host *shost;
+ struct esp *esp;
+ struct pci_esp_priv *pep;
+
+ if (pci_enable_device(pdev)) {
+ dev_printk(KERN_INFO, &pdev->dev, "cannot enable device\n");
+ return -ENODEV;
+ }
+
+ if (pci_set_dma_mask(pdev, DMA_BIT_MASK(32))) {
+ dev_printk(KERN_INFO, &pdev->dev,
+ "failed to set 32bit DMA mask\n");
+ goto fail_disable_device;
+ }
+
+ shost = scsi_host_alloc(hostt, sizeof(struct esp));
+ if (!shost) {
+ dev_printk(KERN_INFO, &pdev->dev,
+ "failed to allocate scsi host\n");
+ err = -ENOMEM;
+ goto fail_disable_device;
+ }
+
+ pep = kzalloc(sizeof(struct pci_esp_priv), GFP_KERNEL);
+ if (!pep) {
+ dev_printk(KERN_INFO, &pdev->dev,
+ "failed to allocate esp_priv\n");
+ err = -ENOMEM;
+ goto fail_host_alloc;
+ }
+
+ esp = shost_priv(shost);
+ esp->host = shost;
+ esp->dev = pdev;
+ esp->ops = &pci_esp_ops;
+ pep->esp = esp;
+
+ if (pci_request_regions(pdev, DRV_MODULE_NAME)) {
+ dev_printk(KERN_ERR, &pdev->dev,
+ "pci memory selection failed\n");
+ goto fail_priv_alloc;
+ }
+
+ esp->regs = pci_iomap(pdev, 0, pci_resource_len(pdev, 0));
+ if (!esp->regs) {
+ dev_printk(KERN_ERR, &pdev->dev, "pci I/O map failed\n");
+ goto fail_release_regions;
+ }
+ esp->dma_regs = esp->regs;
+
+ pci_set_master(pdev);
+
+ esp->command_block = pci_alloc_consistent(pdev, 16,
+ &esp->command_block_dma);
+ if (!esp->command_block) {
+ dev_printk(KERN_ERR, &pdev->dev,
+ "failed to allocate command block\n");
+ err = -ENOMEM;
+ goto fail_unmap_regs;
+ }
+
+ if (request_irq(pdev->irq, scsi_esp_intr, IRQF_SHARED,
+ DRV_MODULE_NAME, esp)) {
+ dev_printk(KERN_ERR, &pdev->dev, "failed to register IRQ\n");
+ goto fail_unmap_command_block;
+ }
+
+ esp->scsi_id = 7;
+ dc390_check_eeprom(esp);
+
+ shost->this_id = esp->scsi_id;
+ shost->max_id = 8;
+ shost->irq = pdev->irq;
+ shost->io_port = pci_resource_start(pdev, 0);
+ shost->n_io_port = pci_resource_len(pdev, 0);
+ shost->unique_id = shost->io_port;
+ esp->scsi_id_mask = (1 << esp->scsi_id);
+ /* Assume 40MHz clock */
+ esp->cfreq = 40000000;
+
+ pci_set_drvdata(pdev, pep);
+
+ err = scsi_esp_register(esp, &pdev->dev);
+ if (err)
+ goto fail_free_irq;
+
+ return 0;
+
+fail_free_irq:
+ free_irq(pdev->irq, esp);
+fail_unmap_command_block:
+ pci_free_consistent(pdev, 16, esp->command_block,
+ esp->command_block_dma);
+fail_unmap_regs:
+ pci_iounmap(pdev, esp->regs);
+fail_release_regions:
+ pci_release_regions(pdev);
+fail_priv_alloc:
+ kfree(pep);
+fail_host_alloc:
+ scsi_host_put(shost);
+fail_disable_device:
+ pci_disable_device(pdev);
+
+ return err;
+}
+
+static void pci_esp_remove_one(struct pci_dev *pdev)
+{
+ struct pci_esp_priv *pep = pci_get_drvdata(pdev);
+ struct esp *esp = pep->esp;
+
+ scsi_esp_unregister(esp);
+ free_irq(pdev->irq, esp);
+ pci_free_consistent(pdev, 16, esp->command_block,
+ esp->command_block_dma);
+ pci_iounmap(pdev, esp->regs);
+ pci_release_regions(pdev);
+ pci_disable_device(pdev);
+ kfree(pep);
+
+ scsi_host_put(esp->host);
+}
+
+static struct pci_device_id am53c974_pci_tbl[] = {
+ { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_SCSI,
+ PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(pci, am53c974_pci_tbl);
+
+static struct pci_driver am53c974_driver = {
+ .name = DRV_MODULE_NAME,
+ .id_table = am53c974_pci_tbl,
+ .probe = pci_esp_probe_one,
+ .remove = pci_esp_remove_one,
+};
+
+static int __init am53c974_module_init(void)
+{
+ return pci_register_driver(&am53c974_driver);
+}
+
+static void __exit am53c974_module_exit(void)
+{
+ pci_unregister_driver(&am53c974_driver);
+}
+
+MODULE_DESCRIPTION("AM53C974 SCSI driver");
+MODULE_AUTHOR("Hannes Reinecke <hare@suse.de>");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(DRV_MODULE_VERSION);
+
+module_init(am53c974_module_init);
+module_exit(am53c974_module_exit);
--
1.8.5.2
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 06/10] scsi: add 'am53c974' driver
2014-11-21 9:27 ` [PATCH 06/10] scsi: add 'am53c974' driver Hannes Reinecke
@ 2014-11-21 10:14 ` Paolo Bonzini
2014-11-21 10:34 ` Hannes Reinecke
2014-11-21 10:20 ` Paolo Bonzini
1 sibling, 1 reply; 31+ messages in thread
From: Paolo Bonzini @ 2014-11-21 10:14 UTC (permalink / raw)
To: Hannes Reinecke, James Bottomley; +Cc: Christoph Hellwig, linux-scsi
On 21/11/2014 10:27, Hannes Reinecke wrote:
> This patch adds a new implementation for the Tekram DC-390T /
> AMD AM53c974 SCSI controller, based on the generic
> esp_scsi infrastructure.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> drivers/scsi/Kconfig | 18 ++
> drivers/scsi/Makefile | 1 +
> drivers/scsi/am53c974.c | 523 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 542 insertions(+)
> create mode 100644 drivers/scsi/am53c974.c
>
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index 21bc674..497e7d5 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -1357,6 +1357,24 @@ config SCSI_DC390T
> To compile this driver as a module, choose M here: the
> module will be called tmscsim.
>
> +config SCSI_AM53C974
> + tristate "Tekram DC390(T) and Am53/79C974 SCSI support (new driver)"
> + depends on PCI && SCSI
Perhaps make it a choice with SCSI_DC390, since they match on the same
PCI vendor/device IDs?
Paolo
> + select SCSI_SPI_ATTRS
> + ---help---
> + This driver supports PCI SCSI host adapters based on the Am53C974A
> + chip, e.g. Tekram DC390(T), DawiControl 2974 and some onboard
> + PCscsi/PCnet (Am53/79C974) solutions.
> + This is a new implementation base on the generic esp_scsi driver.
> +
> + Documentation can be found in <file:Documentation/scsi/tmscsim.txt>.
> +
> + Note that this driver does NOT support Tekram DC390W/U/F, which are
> + based on NCR/Symbios chips. Use "NCR53C8XX SCSI support" for those.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called am53c974.
> +
> config SCSI_T128
> tristate "Trantor T128/T128F/T228 SCSI support"
> depends on ISA && SCSI
> diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
> index 76f8932..7f974fc 100644
> --- a/drivers/scsi/Makefile
> +++ b/drivers/scsi/Makefile
> @@ -101,6 +101,7 @@ obj-$(CONFIG_SCSI_7000FASST) += wd7000.o
> obj-$(CONFIG_SCSI_EATA) += eata.o
> obj-$(CONFIG_SCSI_DC395x) += dc395x.o
> obj-$(CONFIG_SCSI_DC390T) += tmscsim.o
> +obj-$(CONFIG_SCSI_AM53C974) += esp_scsi.o am53c974.o
> obj-$(CONFIG_MEGARAID_LEGACY) += megaraid.o
> obj-$(CONFIG_MEGARAID_NEWGEN) += megaraid/
> obj-$(CONFIG_MEGARAID_SAS) += megaraid/
> diff --git a/drivers/scsi/am53c974.c b/drivers/scsi/am53c974.c
> new file mode 100644
> index 0000000..b9af8b0
> --- /dev/null
> +++ b/drivers/scsi/am53c974.c
> @@ -0,0 +1,523 @@
> +/*
> + * AMD am53c974 driver.
> + * Copyright (c) 2014 Hannes Reinecke, SUSE Linux GmbH
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/delay.h>
> +#include <linux/pci.h>
> +#include <linux/interrupt.h>
> +
> +#include <scsi/scsi_host.h>
> +
> +#include "esp_scsi.h"
> +
> +#define DRV_MODULE_NAME "am53c974"
> +#define DRV_MODULE_VERSION "1.00"
> +
> +// #define ESP_DMA_DEBUG
> +
> +#define ESP_DMA_CMD 0x10
> +#define ESP_DMA_STC 0x11
> +#define ESP_DMA_SPA 0x12
> +#define ESP_DMA_WBC 0x13
> +#define ESP_DMA_WAC 0x14
> +#define ESP_DMA_STATUS 0x15
> +#define ESP_DMA_SMDLA 0x16
> +#define ESP_DMA_WMAC 0x17
> +
> +#define ESP_DMA_CMD_IDLE 0x00
> +#define ESP_DMA_CMD_BLAST 0x01
> +#define ESP_DMA_CMD_ABORT 0x02
> +#define ESP_DMA_CMD_START 0x03
> +#define ESP_DMA_CMD_MASK 0x03
> +#define ESP_DMA_CMD_DIAG 0x04
> +#define ESP_DMA_CMD_MDL 0x10
> +#define ESP_DMA_CMD_INTE_P 0x20
> +#define ESP_DMA_CMD_INTE_D 0x40
> +#define ESP_DMA_CMD_DIR 0x80
> +
> +#define ESP_DMA_STAT_PWDN 0x01
> +#define ESP_DMA_STAT_ERROR 0x02
> +#define ESP_DMA_STAT_ABORT 0x04
> +#define ESP_DMA_STAT_DONE 0x08
> +#define ESP_DMA_STAT_SCSIINT 0x10
> +#define ESP_DMA_STAT_BCMPLT 0x20
> +
> +/* EEPROM is accessed with 16-bit values */
> +#define DC390_EEPROM_READ 0x80
> +#define DC390_EEPROM_LEN 0x40
> +
> +/*
> + * DC390 EEPROM
> + *
> + * 8 * 4 bytes of per-device options
> + * followed by HBA specific options
> + */
> +
> +/* Per-device options */
> +#define DC390_EE_MODE1 0x00
> +#define DC390_EE_SPEED 0x01
> +
> +/* HBA-specific options */
> +#define DC390_EE_ADAPT_SCSI_ID 0x40
> +#define DC390_EE_MODE2 0x41
> +#define DC390_EE_DELAY 0x42
> +#define DC390_EE_TAG_CMD_NUM 0x43
> +
> +#define DC390_EE_MODE1_PARITY_CHK 0x01
> +#define DC390_EE_MODE1_SYNC_NEGO 0x02
> +#define DC390_EE_MODE1_EN_DISC 0x04
> +#define DC390_EE_MODE1_SEND_START 0x08
> +#define DC390_EE_MODE1_TCQ 0x10
> +
> +#define DC390_EE_MODE2_MORE_2DRV 0x01
> +#define DC390_EE_MODE2_GREATER_1G 0x02
> +#define DC390_EE_MODE2_RST_SCSI_BUS 0x04
> +#define DC390_EE_MODE2_ACTIVE_NEGATION 0x08
> +#define DC390_EE_MODE2_NO_SEEK 0x10
> +#define DC390_EE_MODE2_LUN_CHECK 0x20
> +
> +struct pci_esp_priv {
> + struct esp *esp;
> + u8 dma_status;
> +};
> +
> +#define PCI_ESP_GET_PRIV(esp) ((struct pci_esp_priv *) \
> + pci_get_drvdata((struct pci_dev *) \
> + (esp)->dev))
> +
> +static void pci_esp_dma_drain(struct esp *esp);
> +
> +static void pci_esp_write8(struct esp *esp, u8 val, unsigned long reg)
> +{
> + iowrite8(val, esp->regs + (reg * 4UL));
> +}
> +
> +static u8 pci_esp_read8(struct esp *esp, unsigned long reg)
> +{
> + return ioread8(esp->regs + (reg * 4UL));
> +}
> +
> +static void pci_esp_write32(struct esp *esp, u32 val, unsigned long reg)
> +{
> + return iowrite32(val, esp->regs + (reg * 4UL));
> +}
> +
> +static dma_addr_t pci_esp_map_single(struct esp *esp, void *buf,
> + size_t sz, int dir)
> +{
> + return pci_map_single(esp->dev, buf, sz, dir);
> +}
> +
> +static int pci_esp_map_sg(struct esp *esp, struct scatterlist *sg,
> + int num_sg, int dir)
> +{
> + return pci_map_sg(esp->dev, sg, num_sg, dir);
> +}
> +
> +static void pci_esp_unmap_single(struct esp *esp, dma_addr_t addr,
> + size_t sz, int dir)
> +{
> + pci_unmap_single(esp->dev, addr, sz, dir);
> +}
> +
> +static void pci_esp_unmap_sg(struct esp *esp, struct scatterlist *sg,
> + int num_sg, int dir)
> +{
> + pci_unmap_sg(esp->dev, sg, num_sg, dir);
> +}
> +
> +static int pci_esp_irq_pending(struct esp *esp)
> +{
> + struct pci_esp_priv *pep = PCI_ESP_GET_PRIV(esp);
> +
> + pep->dma_status = pci_esp_read8(esp, ESP_DMA_STATUS);
> +#ifdef ESP_DMA_DEBUG
> + if (pep->dma_status)
> + shost_printk(KERN_INFO, esp->host, "dma intr dreg[%02x]\n",
> + pep->dma_status);
> +#endif
> + if (pep->dma_status & (ESP_DMA_STAT_ERROR |
> + ESP_DMA_STAT_ABORT |
> + ESP_DMA_STAT_DONE |
> + ESP_DMA_STAT_SCSIINT))
> + return 1;
> +
> + return 0;
> +}
> +
> +static void pci_esp_reset_dma(struct esp *esp)
> +{
> + /* Nothing to do ? */
> +}
> +
> +static void pci_esp_dma_drain(struct esp *esp)
> +{
> + u8 resid;
> + int lim = 1000;
> +
> +
> + if ((esp->sreg & ESP_STAT_PMASK) == ESP_DOP ||
> + (esp->sreg & ESP_STAT_PMASK) == ESP_DIP)
> + /* Data-In or Data-Out, nothing to be done */
> + return;
> +
> + while (--lim > 0) {
> + resid = pci_esp_read8(esp, ESP_FFLAGS) & ESP_FF_FBYTES;
> + if (resid <= 1)
> + break;
> + }
> + if (resid > 1) {
> + /* FIFO not cleared */
> + shost_printk(KERN_INFO, esp->host,
> + "FIFO not cleared, %d bytes left\n",
> + resid);
> + }
> +
> + /*
> + * When there is a residual BCMPLT will never be set
> + * (obviously). But we still have to issue the BLAST
> + * command, otherwise the data will not being transferred.
> + * But we'll never know when the BLAST operation is
> + * finished. So check for some time and give up eventually.
> + */
> + lim = 1000;
> + pci_esp_write8(esp, ESP_DMA_CMD_DIR | ESP_DMA_CMD_BLAST, ESP_DMA_CMD);
> + while (pci_esp_read8(esp, ESP_DMA_STATUS) & ESP_DMA_STAT_BCMPLT) {
> + if (--lim == 0)
> + break;
> + }
> + pci_esp_write8(esp, ESP_DMA_CMD_DIR | ESP_DMA_CMD_IDLE, ESP_DMA_CMD);
> +#ifdef ESP_DMA_DEBUG
> + shost_printk(KERN_INFO, esp->host,
> + "DMA blast done (%d tries, %d bytes left)\n", lim, resid);
> +#endif
> +}
> +
> +static void pci_esp_dma_invalidate(struct esp *esp)
> +{
> + struct pci_esp_priv *pep = PCI_ESP_GET_PRIV(esp);
> +
> +#ifdef ESP_DMA_DEBUG
> + shost_printk(KERN_INFO, esp->host, "invalidate DMA\n");
> +#endif
> + pci_esp_write8(esp, ESP_DMA_CMD_IDLE, ESP_DMA_CMD);
> + pep->dma_status = 0;
> +}
> +
> +static int pci_esp_dma_error(struct esp *esp)
> +{
> + struct pci_esp_priv *pep = PCI_ESP_GET_PRIV(esp);
> +
> + if (pep->dma_status & ESP_DMA_STAT_ERROR) {
> + u8 dma_cmd = pci_esp_read8(esp, ESP_DMA_CMD);
> +
> + if ((dma_cmd & ESP_DMA_CMD_MASK) == ESP_DMA_CMD_START)
> + pci_esp_write8(esp, ESP_DMA_CMD_ABORT, ESP_DMA_CMD);
> +
> + return 1;
> + }
> + if (pep->dma_status & ESP_DMA_STAT_ABORT) {
> + pci_esp_write8(esp, ESP_DMA_CMD_IDLE, ESP_DMA_CMD);
> + pep->dma_status = pci_esp_read8(esp, ESP_DMA_CMD);
> + return 1;
> + }
> + return 0;
> +}
> +
> +static void pci_esp_send_dma_cmd(struct esp *esp, u32 addr, u32 esp_count,
> + u32 dma_count, int write, u8 cmd)
> +{
> + struct pci_esp_priv *pep = PCI_ESP_GET_PRIV(esp);
> + u32 val = 0;
> +
> + BUG_ON(!(cmd & ESP_CMD_DMA));
> +
> + pep->dma_status = 0;
> +
> + /* Set DMA engine to IDLE */
> + if (write)
> + /* DMA write direction logic is inverted */
> + val |= ESP_DMA_CMD_DIR;
> + pci_esp_write8(esp, ESP_DMA_CMD_IDLE | val, ESP_DMA_CMD);
> +
> + pci_esp_write8(esp, (esp_count >> 0) & 0xff, ESP_TCLOW);
> + pci_esp_write8(esp, (esp_count >> 8) & 0xff, ESP_TCMED);
> +
> + pci_esp_write32(esp, esp_count, ESP_DMA_STC);
> + pci_esp_write32(esp, addr, ESP_DMA_SPA);
> +
> +#ifdef ESP_DMA_DEBUG
> + shost_printk(KERN_INFO, esp->host, "start dma addr[%x] count[%d:%d]\n",
> + addr, esp_count, dma_count);
> +#endif
> + scsi_esp_cmd(esp, cmd);
> + /* Send DMA Start command */
> + pci_esp_write8(esp, ESP_DMA_CMD_START | val, ESP_DMA_CMD);
> +}
> +
> +static const struct esp_driver_ops pci_esp_ops = {
> + .esp_write8 = pci_esp_write8,
> + .esp_read8 = pci_esp_read8,
> + .map_single = pci_esp_map_single,
> + .map_sg = pci_esp_map_sg,
> + .unmap_single = pci_esp_unmap_single,
> + .unmap_sg = pci_esp_unmap_sg,
> + .irq_pending = pci_esp_irq_pending,
> + .reset_dma = pci_esp_reset_dma,
> + .dma_drain = pci_esp_dma_drain,
> + .dma_invalidate = pci_esp_dma_invalidate,
> + .send_dma_cmd = pci_esp_send_dma_cmd,
> + .dma_error = pci_esp_dma_error,
> +};
> +
> +/*
> + * Read DC-390 eeprom
> + */
> +static void dc390_eeprom_prepare_read(struct pci_dev *pdev, u8 cmd)
> +{
> + u8 carryFlag = 1, j = 0x80, bval;
> + int i;
> +
> + for (i = 0; i < 9; i++) {
> + if (carryFlag) {
> + pci_write_config_byte(pdev, 0x80, 0x40);
> + bval = 0xc0;
> + } else
> + bval = 0x80;
> +
> + udelay(160);
> + pci_write_config_byte(pdev, 0x80, bval);
> + udelay(160);
> + pci_write_config_byte(pdev, 0x80, 0);
> + udelay(160);
> +
> + carryFlag = (cmd & j) ? 1 : 0;
> + j >>= 1;
> + }
> +}
> +
> +static u16 dc390_eeprom_get_data(struct pci_dev *pdev)
> +{
> + int i;
> + u16 wval = 0;
> + u8 bval;
> +
> + for (i = 0; i < 16; i++) {
> + wval <<= 1;
> +
> + pci_write_config_byte(pdev, 0x80, 0x80);
> + udelay(160);
> + pci_write_config_byte(pdev, 0x80, 0x40);
> + udelay(160);
> + pci_read_config_byte(pdev, 0x00, &bval);
> +
> + if (bval == 0x22)
> + wval |= 1;
> + }
> +
> + return wval;
> +}
> +
> +static void dc390_read_eeprom(struct pci_dev *pdev, u16 *ptr)
> +{
> + u8 cmd = DC390_EEPROM_READ, i;
> +
> + for (i = 0; i < DC390_EEPROM_LEN; i++) {
> + pci_write_config_byte(pdev, 0xc0, 0);
> + udelay(160);
> +
> + dc390_eeprom_prepare_read(pdev, cmd++);
> + *ptr++ = dc390_eeprom_get_data(pdev);
> +
> + pci_write_config_byte(pdev, 0x80, 0);
> + pci_write_config_byte(pdev, 0x80, 0);
> + udelay(160);
> + }
> +}
> +
> +static void dc390_check_eeprom(struct esp *esp)
> +{
> + u8 EEbuf[128];
> + u16 *ptr = (u16 *)EEbuf, wval = 0;
> + int i;
> +
> + dc390_read_eeprom((struct pci_dev *)esp->dev, ptr);
> +
> + for (i = 0; i < DC390_EEPROM_LEN; i++, ptr++)
> + wval += *ptr;
> +
> + /* no Tekram EEprom found */
> + if (wval != 0x1234) {
> + struct pci_dev *pdev = esp->dev;
> + dev_printk(KERN_INFO, &pdev->dev,
> + "No valid Tekram EEprom found\n");
> + return;
> + }
> + esp->scsi_id = EEbuf[DC390_EE_ADAPT_SCSI_ID];
> + esp->num_tags = 2 << EEbuf[DC390_EE_TAG_CMD_NUM];
> +}
> +
> +static int pci_esp_probe_one(struct pci_dev *pdev,
> + const struct pci_device_id *id)
> +{
> + struct scsi_host_template *hostt = &scsi_esp_template;
> + int err = -ENODEV;
> + struct Scsi_Host *shost;
> + struct esp *esp;
> + struct pci_esp_priv *pep;
> +
> + if (pci_enable_device(pdev)) {
> + dev_printk(KERN_INFO, &pdev->dev, "cannot enable device\n");
> + return -ENODEV;
> + }
> +
> + if (pci_set_dma_mask(pdev, DMA_BIT_MASK(32))) {
> + dev_printk(KERN_INFO, &pdev->dev,
> + "failed to set 32bit DMA mask\n");
> + goto fail_disable_device;
> + }
> +
> + shost = scsi_host_alloc(hostt, sizeof(struct esp));
> + if (!shost) {
> + dev_printk(KERN_INFO, &pdev->dev,
> + "failed to allocate scsi host\n");
> + err = -ENOMEM;
> + goto fail_disable_device;
> + }
> +
> + pep = kzalloc(sizeof(struct pci_esp_priv), GFP_KERNEL);
> + if (!pep) {
> + dev_printk(KERN_INFO, &pdev->dev,
> + "failed to allocate esp_priv\n");
> + err = -ENOMEM;
> + goto fail_host_alloc;
> + }
> +
> + esp = shost_priv(shost);
> + esp->host = shost;
> + esp->dev = pdev;
> + esp->ops = &pci_esp_ops;
> + pep->esp = esp;
> +
> + if (pci_request_regions(pdev, DRV_MODULE_NAME)) {
> + dev_printk(KERN_ERR, &pdev->dev,
> + "pci memory selection failed\n");
> + goto fail_priv_alloc;
> + }
> +
> + esp->regs = pci_iomap(pdev, 0, pci_resource_len(pdev, 0));
> + if (!esp->regs) {
> + dev_printk(KERN_ERR, &pdev->dev, "pci I/O map failed\n");
> + goto fail_release_regions;
> + }
> + esp->dma_regs = esp->regs;
> +
> + pci_set_master(pdev);
> +
> + esp->command_block = pci_alloc_consistent(pdev, 16,
> + &esp->command_block_dma);
> + if (!esp->command_block) {
> + dev_printk(KERN_ERR, &pdev->dev,
> + "failed to allocate command block\n");
> + err = -ENOMEM;
> + goto fail_unmap_regs;
> + }
> +
> + if (request_irq(pdev->irq, scsi_esp_intr, IRQF_SHARED,
> + DRV_MODULE_NAME, esp)) {
> + dev_printk(KERN_ERR, &pdev->dev, "failed to register IRQ\n");
> + goto fail_unmap_command_block;
> + }
> +
> + esp->scsi_id = 7;
> + dc390_check_eeprom(esp);
> +
> + shost->this_id = esp->scsi_id;
> + shost->max_id = 8;
> + shost->irq = pdev->irq;
> + shost->io_port = pci_resource_start(pdev, 0);
> + shost->n_io_port = pci_resource_len(pdev, 0);
> + shost->unique_id = shost->io_port;
> + esp->scsi_id_mask = (1 << esp->scsi_id);
> + /* Assume 40MHz clock */
> + esp->cfreq = 40000000;
> +
> + pci_set_drvdata(pdev, pep);
> +
> + err = scsi_esp_register(esp, &pdev->dev);
> + if (err)
> + goto fail_free_irq;
> +
> + return 0;
> +
> +fail_free_irq:
> + free_irq(pdev->irq, esp);
> +fail_unmap_command_block:
> + pci_free_consistent(pdev, 16, esp->command_block,
> + esp->command_block_dma);
> +fail_unmap_regs:
> + pci_iounmap(pdev, esp->regs);
> +fail_release_regions:
> + pci_release_regions(pdev);
> +fail_priv_alloc:
> + kfree(pep);
> +fail_host_alloc:
> + scsi_host_put(shost);
> +fail_disable_device:
> + pci_disable_device(pdev);
> +
> + return err;
> +}
> +
> +static void pci_esp_remove_one(struct pci_dev *pdev)
> +{
> + struct pci_esp_priv *pep = pci_get_drvdata(pdev);
> + struct esp *esp = pep->esp;
> +
> + scsi_esp_unregister(esp);
> + free_irq(pdev->irq, esp);
> + pci_free_consistent(pdev, 16, esp->command_block,
> + esp->command_block_dma);
> + pci_iounmap(pdev, esp->regs);
> + pci_release_regions(pdev);
> + pci_disable_device(pdev);
> + kfree(pep);
> +
> + scsi_host_put(esp->host);
> +}
> +
> +static struct pci_device_id am53c974_pci_tbl[] = {
> + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_SCSI,
> + PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(pci, am53c974_pci_tbl);
> +
> +static struct pci_driver am53c974_driver = {
> + .name = DRV_MODULE_NAME,
> + .id_table = am53c974_pci_tbl,
> + .probe = pci_esp_probe_one,
> + .remove = pci_esp_remove_one,
> +};
> +
> +static int __init am53c974_module_init(void)
> +{
> + return pci_register_driver(&am53c974_driver);
> +}
> +
> +static void __exit am53c974_module_exit(void)
> +{
> + pci_unregister_driver(&am53c974_driver);
> +}
> +
> +MODULE_DESCRIPTION("AM53C974 SCSI driver");
> +MODULE_AUTHOR("Hannes Reinecke <hare@suse.de>");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION(DRV_MODULE_VERSION);
> +
> +module_init(am53c974_module_init);
> +module_exit(am53c974_module_exit);
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 06/10] scsi: add 'am53c974' driver
2014-11-21 10:14 ` Paolo Bonzini
@ 2014-11-21 10:34 ` Hannes Reinecke
0 siblings, 0 replies; 31+ messages in thread
From: Hannes Reinecke @ 2014-11-21 10:34 UTC (permalink / raw)
To: Paolo Bonzini, James Bottomley; +Cc: Christoph Hellwig, linux-scsi
On 11/21/2014 11:14 AM, Paolo Bonzini wrote:
>
>
> On 21/11/2014 10:27, Hannes Reinecke wrote:
>> This patch adds a new implementation for the Tekram DC-390T /
>> AMD AM53c974 SCSI controller, based on the generic
>> esp_scsi infrastructure.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>> drivers/scsi/Kconfig | 18 ++
>> drivers/scsi/Makefile | 1 +
>> drivers/scsi/am53c974.c | 523 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 542 insertions(+)
>> create mode 100644 drivers/scsi/am53c974.c
>>
>> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
>> index 21bc674..497e7d5 100644
>> --- a/drivers/scsi/Kconfig
>> +++ b/drivers/scsi/Kconfig
>> @@ -1357,6 +1357,24 @@ config SCSI_DC390T
>> To compile this driver as a module, choose M here: the
>> module will be called tmscsim.
>>
>> +config SCSI_AM53C974
>> + tristate "Tekram DC390(T) and Am53/79C974 SCSI support (new driver)"
>> + depends on PCI && SCSI
>
> Perhaps make it a choice with SCSI_DC390, since they match on the same
> PCI vendor/device IDs?
>
Cf my response to Christoph. The idea was to have another patch
removing tmscsim altogether :-)
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 06/10] scsi: add 'am53c974' driver
2014-11-21 9:27 ` [PATCH 06/10] scsi: add 'am53c974' driver Hannes Reinecke
2014-11-21 10:14 ` Paolo Bonzini
@ 2014-11-21 10:20 ` Paolo Bonzini
1 sibling, 0 replies; 31+ messages in thread
From: Paolo Bonzini @ 2014-11-21 10:20 UTC (permalink / raw)
To: Hannes Reinecke, James Bottomley; +Cc: Christoph Hellwig, linux-scsi
Oops, hit send too early. A small nit:
On 21/11/2014 10:27, Hannes Reinecke wrote:
> +static void pci_esp_dma_drain(struct esp *esp)
> +{
> + u8 resid;
> + int lim = 1000;
> +
> +
> + if ((esp->sreg & ESP_STAT_PMASK) == ESP_DOP ||
> + (esp->sreg & ESP_STAT_PMASK) == ESP_DIP)
> + /* Data-In or Data-Out, nothing to be done */
> + return;
> +
> + while (--lim > 0) {
> + resid = pci_esp_read8(esp, ESP_FFLAGS) & ESP_FF_FBYTES;
> + if (resid <= 1)
> + break;
cpu_relax()?
> + }
> + if (resid > 1) {
> + /* FIFO not cleared */
> + shost_printk(KERN_INFO, esp->host,
> + "FIFO not cleared, %d bytes left\n",
> + resid);
> + }
> +
> + /*
> + * When there is a residual BCMPLT will never be set
> + * (obviously). But we still have to issue the BLAST
> + * command, otherwise the data will not being transferred.
> + * But we'll never know when the BLAST operation is
> + * finished. So check for some time and give up eventually.
> + */
> + lim = 1000;
> + pci_esp_write8(esp, ESP_DMA_CMD_DIR | ESP_DMA_CMD_BLAST, ESP_DMA_CMD);
> + while (pci_esp_read8(esp, ESP_DMA_STATUS) & ESP_DMA_STAT_BCMPLT) {
> + if (--lim == 0)
> + break;
cpu_relax()?
Otherwise looks sane.
Paolo
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 07/10] esp: Use FIFO for command submission
2014-11-21 9:27 [PATCH 00/10] Re-implement am53c974 driver Hannes Reinecke
` (5 preceding siblings ...)
2014-11-21 9:27 ` [PATCH 06/10] scsi: add 'am53c974' driver Hannes Reinecke
@ 2014-11-21 9:27 ` Hannes Reinecke
2014-11-21 10:04 ` Paolo Bonzini
2014-11-21 9:27 ` [PATCH 08/10] am53c974: BLAST residual handling Hannes Reinecke
` (3 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Hannes Reinecke @ 2014-11-21 9:27 UTC (permalink / raw)
To: James Bottomley
Cc: Christoph Hellwig, Paolo Bonzini, linux-scsi, Hannes Reinecke
The am53c974 has a design flaw causing it to throw an
DMA interrupt whenever a DMA transmission completed,
even though DMA interrupt reporting is disabled.
This confuses the esp routines as it would register
a DMA interrupt whenever a cdb has been transmitted
to the drive.
So implement an option to use the FIFO for command
submission and enable it for am53c974.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/am53c974.c | 1 +
drivers/scsi/esp_scsi.c | 83 ++++++++++++++++++++++++++++++++++++++-----------
drivers/scsi/esp_scsi.h | 1 +
3 files changed, 66 insertions(+), 19 deletions(-)
diff --git a/drivers/scsi/am53c974.c b/drivers/scsi/am53c974.c
index b9af8b0..652762e 100644
--- a/drivers/scsi/am53c974.c
+++ b/drivers/scsi/am53c974.c
@@ -401,6 +401,7 @@ static int pci_esp_probe_one(struct pci_dev *pdev,
esp->host = shost;
esp->dev = pdev;
esp->ops = &pci_esp_ops;
+ esp->flags |= ESP_FLAG_USE_FIFO;
pep->esp = esp;
if (pci_request_regions(pdev, DRV_MODULE_NAME)) {
diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
index 92ab921..ab7d2bc 100644
--- a/drivers/scsi/esp_scsi.c
+++ b/drivers/scsi/esp_scsi.c
@@ -605,7 +605,7 @@ static void esp_autosense(struct esp *esp, struct esp_cmd_entry *ent)
{
struct scsi_cmnd *cmd = ent->cmd;
struct scsi_device *dev = cmd->device;
- int tgt, lun;
+ int tgt, lun, i;
u8 *p, val;
tgt = dev->id;
@@ -626,7 +626,10 @@ static void esp_autosense(struct esp *esp, struct esp_cmd_entry *ent)
esp->active_cmd = ent;
- p = esp->command_block;
+ if (esp->flags & ESP_FLAG_USE_FIFO)
+ p = esp->fifo;
+ else
+ p = esp->command_block;
esp->msg_out_len = 0;
*p++ = IDENTIFY(0, lun);
@@ -648,12 +651,21 @@ static void esp_autosense(struct esp *esp, struct esp_cmd_entry *ent)
esp_write_tgt_sync(esp, tgt);
esp_write_tgt_config3(esp, tgt);
- val = (p - esp->command_block);
+ if (esp->flags & ESP_FLAG_USE_FIFO)
+ val = (p - esp->fifo);
+ else
+ val = (p - esp->command_block);
if (esp->rev == FASHME)
scsi_esp_cmd(esp, ESP_CMD_FLUSH);
- esp->ops->send_dma_cmd(esp, esp->command_block_dma,
- val, 16, 0, ESP_CMD_DMA | ESP_CMD_SELA);
+ if (esp->flags & ESP_FLAG_USE_FIFO) {
+ scsi_esp_cmd(esp, ESP_CMD_FLUSH);
+ for (i = 0; i < val; i++)
+ esp_write8(esp->fifo[i], ESP_FDATA);
+ scsi_esp_cmd(esp, ESP_CMD_SELA);
+ } else
+ esp->ops->send_dma_cmd(esp, esp->command_block_dma,
+ val, 16, 0, ESP_CMD_DMA | ESP_CMD_SELA);
}
static struct esp_cmd_entry *find_and_prep_issuable_command(struct esp *esp)
@@ -727,7 +739,10 @@ static void esp_maybe_execute_command(struct esp *esp)
esp_check_command_len(esp, cmd);
- p = esp->command_block;
+ if (esp->flags & ESP_FLAG_USE_FIFO)
+ p = esp->fifo;
+ else
+ p = esp->command_block;
esp->msg_out_len = 0;
if (tp->flags & ESP_TGT_CHECK_NEGO) {
@@ -789,13 +804,15 @@ build_identify:
}
if (!(esp->flags & ESP_FLAG_DOING_SLOWCMD)) {
- start_cmd = ESP_CMD_DMA | ESP_CMD_SELA;
+ start_cmd = ESP_CMD_SELA;
if (ent->tag[0]) {
*p++ = ent->tag[0];
*p++ = ent->tag[1];
- start_cmd = ESP_CMD_DMA | ESP_CMD_SA3;
+ start_cmd = ESP_CMD_SA3;
}
+ if (!(esp->flags & ESP_FLAG_USE_FIFO))
+ start_cmd |= ESP_CMD_DMA;
for (i = 0; i < cmd->cmd_len; i++)
*p++ = cmd->cmnd[i];
@@ -814,7 +831,9 @@ build_identify:
esp->msg_out_len += 2;
}
- start_cmd = ESP_CMD_DMA | ESP_CMD_SELAS;
+ start_cmd = ESP_CMD_SELAS;
+ if (!(esp->flags & ESP_FLAG_USE_FIFO))
+ start_cmd |= ESP_CMD_DMA;
esp->select_state = ESP_SELECT_MSGOUT;
}
val = tgt;
@@ -825,7 +844,10 @@ build_identify:
esp_write_tgt_sync(esp, tgt);
esp_write_tgt_config3(esp, tgt);
- val = (p - esp->command_block);
+ if (esp->flags & ESP_FLAG_USE_FIFO)
+ val = (p - esp->fifo);
+ else
+ val = (p - esp->command_block);
if (esp_debug & ESP_DEBUG_SCSICMD) {
printk("ESP: tgt[%d] lun[%d] scsi_cmd [ ", tgt, lun);
@@ -836,8 +858,17 @@ build_identify:
if (esp->rev == FASHME)
scsi_esp_cmd(esp, ESP_CMD_FLUSH);
- esp->ops->send_dma_cmd(esp, esp->command_block_dma,
- val, 16, 0, start_cmd);
+
+ if (esp->flags & ESP_FLAG_USE_FIFO) {
+ scsi_esp_cmd(esp, ESP_CMD_FLUSH);
+ for (i = 0; i < val; i++)
+ esp_write8(esp->fifo[i], ESP_FDATA);
+
+ scsi_esp_cmd(esp, start_cmd);
+ } else {
+ esp->ops->send_dma_cmd(esp, esp->command_block_dma,
+ val, 16, 0, start_cmd);
+ }
}
static struct esp_cmd_entry *esp_get_ent(struct esp *esp)
@@ -1646,7 +1677,7 @@ static int esp_msgin_process(struct esp *esp)
static int esp_process_event(struct esp *esp)
{
- int write;
+ int write, i;
again:
write = 0;
@@ -1872,6 +1903,10 @@ again:
if (esp->msg_out_len == 1) {
esp_write8(esp->msg_out[0], ESP_FDATA);
scsi_esp_cmd(esp, ESP_CMD_TI);
+ } else if (esp->flags & ESP_FLAG_USE_FIFO) {
+ for (i = 0; i < esp->msg_out_len; i++)
+ esp_write8(esp->msg_out[i], ESP_FDATA);
+ scsi_esp_cmd(esp, ESP_CMD_TI);
} else {
/* Use DMA. */
memcpy(esp->command_block,
@@ -1947,13 +1982,23 @@ again:
}
break;
case ESP_EVENT_CMD_START:
- memcpy(esp->command_block, esp->cmd_bytes_ptr,
- esp->cmd_bytes_left);
- if (esp->rev == FASHME)
+ if (esp->flags & ESP_FLAG_USE_FIFO) {
scsi_esp_cmd(esp, ESP_CMD_FLUSH);
- esp->ops->send_dma_cmd(esp, esp->command_block_dma,
- esp->cmd_bytes_left, 16, 0,
- ESP_CMD_DMA | ESP_CMD_TI);
+ while (esp->cmd_bytes_left) {
+ esp_write8(esp->cmd_bytes_ptr[0], ESP_FDATA);
+ esp->cmd_bytes_ptr++;
+ esp->cmd_bytes_left--;
+ }
+ scsi_esp_cmd(esp, ESP_CMD_TI);
+ } else {
+ memcpy(esp->command_block, esp->cmd_bytes_ptr,
+ esp->cmd_bytes_left);
+ if (esp->rev == FASHME)
+ scsi_esp_cmd(esp, ESP_CMD_FLUSH);
+ esp->ops->send_dma_cmd(esp, esp->command_block_dma,
+ esp->cmd_bytes_left, 16, 0,
+ ESP_CMD_DMA | ESP_CMD_TI);
+ }
esp_event(esp, ESP_EVENT_CMD_DONE);
esp->flags |= ESP_FLAG_QUICKIRQ_CHECK;
break;
diff --git a/drivers/scsi/esp_scsi.h b/drivers/scsi/esp_scsi.h
index 975d293..27dcaf8 100644
--- a/drivers/scsi/esp_scsi.h
+++ b/drivers/scsi/esp_scsi.h
@@ -478,6 +478,7 @@ struct esp {
#define ESP_FLAG_WIDE_CAPABLE 0x00000008
#define ESP_FLAG_QUICKIRQ_CHECK 0x00000010
#define ESP_FLAG_DISABLE_SYNC 0x00000020
+#define ESP_FLAG_USE_FIFO 0x00000040
u8 select_state;
#define ESP_SELECT_NONE 0x00 /* Not selecting */
--
1.8.5.2
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 07/10] esp: Use FIFO for command submission
2014-11-21 9:27 ` [PATCH 07/10] esp: Use FIFO for command submission Hannes Reinecke
@ 2014-11-21 10:04 ` Paolo Bonzini
2014-11-21 10:38 ` Hannes Reinecke
0 siblings, 1 reply; 31+ messages in thread
From: Paolo Bonzini @ 2014-11-21 10:04 UTC (permalink / raw)
To: Hannes Reinecke, James Bottomley; +Cc: Christoph Hellwig, linux-scsi
On 21/11/2014 10:27, Hannes Reinecke wrote:
> The am53c974 has a design flaw causing it to throw an
> DMA interrupt whenever a DMA transmission completed,
> even though DMA interrupt reporting is disabled.
> This confuses the esp routines as it would register
> a DMA interrupt whenever a cdb has been transmitted
> to the drive.
> So implement an option to use the FIFO for command
> submission and enable it for am53c974.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> drivers/scsi/am53c974.c | 1 +
> drivers/scsi/esp_scsi.c | 83 ++++++++++++++++++++++++++++++++++++++-----------
> drivers/scsi/esp_scsi.h | 1 +
> 3 files changed, 66 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/scsi/am53c974.c b/drivers/scsi/am53c974.c
> index b9af8b0..652762e 100644
> --- a/drivers/scsi/am53c974.c
> +++ b/drivers/scsi/am53c974.c
> @@ -401,6 +401,7 @@ static int pci_esp_probe_one(struct pci_dev *pdev,
> esp->host = shost;
> esp->dev = pdev;
> esp->ops = &pci_esp_ops;
> + esp->flags |= ESP_FLAG_USE_FIFO;
Why not invert this patch and the previous one, and add this line
directly when the am53c974 driver is born?
> pep->esp = esp;
>
> if (pci_request_regions(pdev, DRV_MODULE_NAME)) {
> diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
> index 92ab921..ab7d2bc 100644
> --- a/drivers/scsi/esp_scsi.c
> +++ b/drivers/scsi/esp_scsi.c
> @@ -605,7 +605,7 @@ static void esp_autosense(struct esp *esp, struct esp_cmd_entry *ent)
> {
> struct scsi_cmnd *cmd = ent->cmd;
> struct scsi_device *dev = cmd->device;
> - int tgt, lun;
> + int tgt, lun, i;
> u8 *p, val;
>
> tgt = dev->id;
> @@ -626,7 +626,10 @@ static void esp_autosense(struct esp *esp, struct esp_cmd_entry *ent)
>
> esp->active_cmd = ent;
>
> - p = esp->command_block;
> + if (esp->flags & ESP_FLAG_USE_FIFO)
> + p = esp->fifo;
> + else
> + p = esp->command_block;
Any reason not to use esp->command_block unconditionally?
> esp->msg_out_len = 0;
>
> *p++ = IDENTIFY(0, lun);
> @@ -648,12 +651,21 @@ static void esp_autosense(struct esp *esp, struct esp_cmd_entry *ent)
> esp_write_tgt_sync(esp, tgt);
> esp_write_tgt_config3(esp, tgt);
>
> - val = (p - esp->command_block);
> + if (esp->flags & ESP_FLAG_USE_FIFO)
> + val = (p - esp->fifo);
> + else
> + val = (p - esp->command_block);
>
> if (esp->rev == FASHME)
> scsi_esp_cmd(esp, ESP_CMD_FLUSH);
For consistency with what you do elsewhere, please move this in the "else".
> - esp->ops->send_dma_cmd(esp, esp->command_block_dma,
> - val, 16, 0, ESP_CMD_DMA | ESP_CMD_SELA);
> + if (esp->flags & ESP_FLAG_USE_FIFO) {
> + scsi_esp_cmd(esp, ESP_CMD_FLUSH);
> + for (i = 0; i < val; i++)
> + esp_write8(esp->fifo[i], ESP_FDATA);
> + scsi_esp_cmd(esp, ESP_CMD_SELA);
> + } else
> + esp->ops->send_dma_cmd(esp, esp->command_block_dma,
> + val, 16, 0, ESP_CMD_DMA | ESP_CMD_SELA);
> }
Hmm, what about a wrapper
__send_cmd(esp, data, esp_count, dma_count, cmd, force_flush,
force_fifo)
{
use_fifo =
force_fifo || (esp->flags & ESP_FLAG_USE_FIFO) ||
esp_count == 1;
if (force_flush || use_fifo || esp->rev == FASHME)
scsi_esp_cmd(esp, ESP_CMD_FLUSH);
if (use_fifo) {
for (i = 0; i < val; i++)
esp_write8(esp->fifo[i], ESP_FDATA);
scsi_esp_cmd(esp, cmd);
} else {
if (data != esp->command_block)
memcpy(esp->command_block, data, length)
esp->ops->send_dma_cmd(esp,
esp->command_block_dma,
esp_count, dma_count, 0,
cmd | ESP_CMD_DMA);
}
}
send_cmd(esp, data, esp_count, dma_count, cmd)
{
__send_cmd(esp, data, esp_count, dma_count, cmd, false, false);
}
This would be:
send_cmd(esp, esp->command_block, val, 16, ESP_CMD_SELA);
> static struct esp_cmd_entry *find_and_prep_issuable_command(struct esp *esp)
> @@ -727,7 +739,10 @@ static void esp_maybe_execute_command(struct esp *esp)
>
> esp_check_command_len(esp, cmd);
>
> - p = esp->command_block;
> + if (esp->flags & ESP_FLAG_USE_FIFO)
> + p = esp->fifo;
Any reason not to use esp->command_block unconditionally?
> + else
> + p = esp->command_block;
>
> esp->msg_out_len = 0;
> if (tp->flags & ESP_TGT_CHECK_NEGO) {
> @@ -789,13 +804,15 @@ build_identify:
> }
>
> if (!(esp->flags & ESP_FLAG_DOING_SLOWCMD)) {
> - start_cmd = ESP_CMD_DMA | ESP_CMD_SELA;
> + start_cmd = ESP_CMD_SELA;
> if (ent->tag[0]) {
> *p++ = ent->tag[0];
> *p++ = ent->tag[1];
>
> - start_cmd = ESP_CMD_DMA | ESP_CMD_SA3;
> + start_cmd = ESP_CMD_SA3;
> }
> + if (!(esp->flags & ESP_FLAG_USE_FIFO))
> + start_cmd |= ESP_CMD_DMA;
>
> for (i = 0; i < cmd->cmd_len; i++)
> *p++ = cmd->cmnd[i];
> @@ -814,7 +831,9 @@ build_identify:
> esp->msg_out_len += 2;
> }
>
> - start_cmd = ESP_CMD_DMA | ESP_CMD_SELAS;
> + start_cmd = ESP_CMD_SELAS;
> + if (!(esp->flags & ESP_FLAG_USE_FIFO))
> + start_cmd |= ESP_CMD_DMA;
> esp->select_state = ESP_SELECT_MSGOUT;
> }
> val = tgt;
> @@ -825,7 +844,10 @@ build_identify:
> esp_write_tgt_sync(esp, tgt);
> esp_write_tgt_config3(esp, tgt);
>
> - val = (p - esp->command_block);
> + if (esp->flags & ESP_FLAG_USE_FIFO)
> + val = (p - esp->fifo);
> + else
> + val = (p - esp->command_block);
>
> if (esp_debug & ESP_DEBUG_SCSICMD) {
> printk("ESP: tgt[%d] lun[%d] scsi_cmd [ ", tgt, lun);
> @@ -836,8 +858,17 @@ build_identify:
>
> if (esp->rev == FASHME)
> scsi_esp_cmd(esp, ESP_CMD_FLUSH);
> - esp->ops->send_dma_cmd(esp, esp->command_block_dma,
> - val, 16, 0, start_cmd);
> +
> + if (esp->flags & ESP_FLAG_USE_FIFO) {
> + scsi_esp_cmd(esp, ESP_CMD_FLUSH);
> + for (i = 0; i < val; i++)
> + esp_write8(esp->fifo[i], ESP_FDATA);
> +
> + scsi_esp_cmd(esp, start_cmd);
> + } else {
> + esp->ops->send_dma_cmd(esp, esp->command_block_dma,
> + val, 16, 0, start_cmd);
send_cmd(esp, esp->command_block, val, 16, start_cmd);
> + }
> }
>
> static struct esp_cmd_entry *esp_get_ent(struct esp *esp)
> @@ -1646,7 +1677,7 @@ static int esp_msgin_process(struct esp *esp)
>
> static int esp_process_event(struct esp *esp)
> {
> - int write;
> + int write, i;
>
> again:
> write = 0;
> @@ -1872,6 +1903,10 @@ again:
> if (esp->msg_out_len == 1) {
> esp_write8(esp->msg_out[0], ESP_FDATA);
> scsi_esp_cmd(esp, ESP_CMD_TI);
> + } else if (esp->flags & ESP_FLAG_USE_FIFO) {
> + for (i = 0; i < esp->msg_out_len; i++)
> + esp_write8(esp->msg_out[i], ESP_FDATA);
> + scsi_esp_cmd(esp, ESP_CMD_TI);
> } else {
> /* Use DMA. */
> memcpy(esp->command_block,
__send_cmd(esp, esp->msg_out, esp->msg_out_len, esp->msg_out_len,
ESP_CMD_TI, true, esp->rev == FASHME);
> @@ -1947,13 +1982,23 @@ again:
> }
> break;
> case ESP_EVENT_CMD_START:
> - memcpy(esp->command_block, esp->cmd_bytes_ptr,
> - esp->cmd_bytes_left);
> - if (esp->rev == FASHME)
> + if (esp->flags & ESP_FLAG_USE_FIFO) {
> scsi_esp_cmd(esp, ESP_CMD_FLUSH);
> - esp->ops->send_dma_cmd(esp, esp->command_block_dma,
> - esp->cmd_bytes_left, 16, 0,
> - ESP_CMD_DMA | ESP_CMD_TI);
> + while (esp->cmd_bytes_left) {
> + esp_write8(esp->cmd_bytes_ptr[0], ESP_FDATA);
> + esp->cmd_bytes_ptr++;
> + esp->cmd_bytes_left--;
> + }
> + scsi_esp_cmd(esp, ESP_CMD_TI);
> + } else {
> + memcpy(esp->command_block, esp->cmd_bytes_ptr,
> + esp->cmd_bytes_left);
> + if (esp->rev == FASHME)
> + scsi_esp_cmd(esp, ESP_CMD_FLUSH);
> + esp->ops->send_dma_cmd(esp, esp->command_block_dma,
> + esp->cmd_bytes_left, 16, 0,
> + ESP_CMD_DMA | ESP_CMD_TI);
> + }
send_cmd(esp, esp->cmd_bytes_ptr, esp->cmd_bytes_left, 16,
ESP_CMD_TI);
> esp_event(esp, ESP_EVENT_CMD_DONE);
> esp->flags |= ESP_FLAG_QUICKIRQ_CHECK;
> break;
> diff --git a/drivers/scsi/esp_scsi.h b/drivers/scsi/esp_scsi.h
> index 975d293..27dcaf8 100644
> --- a/drivers/scsi/esp_scsi.h
> +++ b/drivers/scsi/esp_scsi.h
> @@ -478,6 +478,7 @@ struct esp {
> #define ESP_FLAG_WIDE_CAPABLE 0x00000008
> #define ESP_FLAG_QUICKIRQ_CHECK 0x00000010
> #define ESP_FLAG_DISABLE_SYNC 0x00000020
> +#define ESP_FLAG_USE_FIFO 0x00000040
>
> u8 select_state;
> #define ESP_SELECT_NONE 0x00 /* Not selecting */
>
Paolo
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 07/10] esp: Use FIFO for command submission
2014-11-21 10:04 ` Paolo Bonzini
@ 2014-11-21 10:38 ` Hannes Reinecke
2014-11-21 10:50 ` Paolo Bonzini
0 siblings, 1 reply; 31+ messages in thread
From: Hannes Reinecke @ 2014-11-21 10:38 UTC (permalink / raw)
To: Paolo Bonzini, James Bottomley; +Cc: Christoph Hellwig, linux-scsi
On 11/21/2014 11:04 AM, Paolo Bonzini wrote:
>
>
> On 21/11/2014 10:27, Hannes Reinecke wrote:
>> The am53c974 has a design flaw causing it to throw an
>> DMA interrupt whenever a DMA transmission completed,
>> even though DMA interrupt reporting is disabled.
>> This confuses the esp routines as it would register
>> a DMA interrupt whenever a cdb has been transmitted
>> to the drive.
>> So implement an option to use the FIFO for command
>> submission and enable it for am53c974.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>> drivers/scsi/am53c974.c | 1 +
>> drivers/scsi/esp_scsi.c | 83 ++++++++++++++++++++++++++++++++++++++-----------
>> drivers/scsi/esp_scsi.h | 1 +
>> 3 files changed, 66 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/scsi/am53c974.c b/drivers/scsi/am53c974.c
>> index b9af8b0..652762e 100644
>> --- a/drivers/scsi/am53c974.c
>> +++ b/drivers/scsi/am53c974.c
>> @@ -401,6 +401,7 @@ static int pci_esp_probe_one(struct pci_dev *pdev,
>> esp->host = shost;
>> esp->dev = pdev;
>> esp->ops = &pci_esp_ops;
>> + esp->flags |= ESP_FLAG_USE_FIFO;
>
> Why not invert this patch and the previous one, and add this line
> directly when the am53c974 driver is born?
>
I've added it as separate patch as the original esp driver would
always be using DMA for CDB transfer.
And you could actually use the am53c974 driver with that, only you'd
see lots of 'spurious IRQ' messages.
But for the order I don't have any preference; sure I can rearrange
those.
>> pep->esp = esp;
>>
>> if (pci_request_regions(pdev, DRV_MODULE_NAME)) {
>> diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
>> index 92ab921..ab7d2bc 100644
>> --- a/drivers/scsi/esp_scsi.c
>> +++ b/drivers/scsi/esp_scsi.c
>> @@ -605,7 +605,7 @@ static void esp_autosense(struct esp *esp, struct esp_cmd_entry *ent)
>> {
>> struct scsi_cmnd *cmd = ent->cmd;
>> struct scsi_device *dev = cmd->device;
>> - int tgt, lun;
>> + int tgt, lun, i;
>> u8 *p, val;
>>
>> tgt = dev->id;
>> @@ -626,7 +626,10 @@ static void esp_autosense(struct esp *esp, struct esp_cmd_entry *ent)
>>
>> esp->active_cmd = ent;
>>
>> - p = esp->command_block;
>> + if (esp->flags & ESP_FLAG_USE_FIFO)
>> + p = esp->fifo;
>> + else
>> + p = esp->command_block;
>
> Any reason not to use esp->command_block unconditionally?
>
'command_block' is actually mapped onto PCI DMA space, the FIFO is
not. Plus I'd thought to use the 'fifo' array here to make things
more obvious about what's going on.
For FIFO submission both are pretty close, so, yeah, I could be
using command_block here.
>> esp->msg_out_len = 0;
>>
>> *p++ = IDENTIFY(0, lun);
>> @@ -648,12 +651,21 @@ static void esp_autosense(struct esp *esp, struct esp_cmd_entry *ent)
>> esp_write_tgt_sync(esp, tgt);
>> esp_write_tgt_config3(esp, tgt);
>>
>> - val = (p - esp->command_block);
>> + if (esp->flags & ESP_FLAG_USE_FIFO)
>> + val = (p - esp->fifo);
>> + else
>> + val = (p - esp->command_block);
>>
>> if (esp->rev == FASHME)
>> scsi_esp_cmd(esp, ESP_CMD_FLUSH);
>
> For consistency with what you do elsewhere, please move this in the "else".
>
No. The 'USE_FIFO' mechanism is a general feature which _should_
work on all chips. The above 'if' condition is a chipset-specific
feature which should be treated separately.
>> - esp->ops->send_dma_cmd(esp, esp->command_block_dma,
>> - val, 16, 0, ESP_CMD_DMA | ESP_CMD_SELA);
>> + if (esp->flags & ESP_FLAG_USE_FIFO) {
>> + scsi_esp_cmd(esp, ESP_CMD_FLUSH);
>> + for (i = 0; i < val; i++)
>> + esp_write8(esp->fifo[i], ESP_FDATA);
>> + scsi_esp_cmd(esp, ESP_CMD_SELA);
>> + } else
>> + esp->ops->send_dma_cmd(esp, esp->command_block_dma,
>> + val, 16, 0, ESP_CMD_DMA | ESP_CMD_SELA);
>> }
>
> Hmm, what about a wrapper
>
> __send_cmd(esp, data, esp_count, dma_count, cmd, force_flush,
> force_fifo)
> {
> use_fifo =
> force_fifo || (esp->flags & ESP_FLAG_USE_FIFO) ||
> esp_count == 1;
> if (force_flush || use_fifo || esp->rev == FASHME)
> scsi_esp_cmd(esp, ESP_CMD_FLUSH);
> if (use_fifo) {
> for (i = 0; i < val; i++)
> esp_write8(esp->fifo[i], ESP_FDATA);
> scsi_esp_cmd(esp, cmd);
> } else {
> if (data != esp->command_block)
> memcpy(esp->command_block, data, length)
> esp->ops->send_dma_cmd(esp,
> esp->command_block_dma,
> esp_count, dma_count, 0,
> cmd | ESP_CMD_DMA);
> }
> }
>
> send_cmd(esp, data, esp_count, dma_count, cmd)
> {
> __send_cmd(esp, data, esp_count, dma_count, cmd, false, false);
> }
>
> This would be:
>
> send_cmd(esp, esp->command_block, val, 16, ESP_CMD_SELA);
>
Good point.
Will be updating the patch.
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
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 07/10] esp: Use FIFO for command submission
2014-11-21 10:38 ` Hannes Reinecke
@ 2014-11-21 10:50 ` Paolo Bonzini
0 siblings, 0 replies; 31+ messages in thread
From: Paolo Bonzini @ 2014-11-21 10:50 UTC (permalink / raw)
To: Hannes Reinecke, James Bottomley; +Cc: Christoph Hellwig, linux-scsi
On 21/11/2014 11:38, Hannes Reinecke wrote:
>>> esp->msg_out_len = 0;
>>>
>>> *p++ = IDENTIFY(0, lun);
>>> @@ -648,12 +651,21 @@ static void esp_autosense(struct esp *esp, struct esp_cmd_entry *ent)
>>> esp_write_tgt_sync(esp, tgt);
>>> esp_write_tgt_config3(esp, tgt);
>>>
>>> - val = (p - esp->command_block);
>>> + if (esp->flags & ESP_FLAG_USE_FIFO)
>>> + val = (p - esp->fifo);
>>> + else
>>> + val = (p - esp->command_block);
>>>
>>> if (esp->rev == FASHME)
>>> scsi_esp_cmd(esp, ESP_CMD_FLUSH);
>>
>> For consistency with what you do elsewhere, please move this in the "else".
>>
> No. The 'USE_FIFO' mechanism is a general feature which _should_
> work on all chips. The above 'if' condition is a chipset-specific
> feature which should be treated separately.
Yup, but USE_FIFO always sends a flush anyway. Sending two is useless.
>>> - esp->ops->send_dma_cmd(esp, esp->command_block_dma,
>>> - val, 16, 0, ESP_CMD_DMA | ESP_CMD_SELA);
>>> + if (esp->flags & ESP_FLAG_USE_FIFO) {
>>> + scsi_esp_cmd(esp, ESP_CMD_FLUSH);
>>> + for (i = 0; i < val; i++)
>>> + esp_write8(esp->fifo[i], ESP_FDATA);
>>> + scsi_esp_cmd(esp, ESP_CMD_SELA);
>>> + } else
>>> + esp->ops->send_dma_cmd(esp, esp->command_block_dma,
>>> + val, 16, 0, ESP_CMD_DMA | ESP_CMD_SELA);
>>> }
>>
>> Hmm, what about a wrapper
>>
>> __send_cmd(esp, data, esp_count, dma_count, cmd, force_flush,
>> force_fifo)
>> {
>> use_fifo =
>> force_fifo || (esp->flags & ESP_FLAG_USE_FIFO) ||
>> esp_count == 1;
>> if (force_flush || use_fifo || esp->rev == FASHME)
>> scsi_esp_cmd(esp, ESP_CMD_FLUSH);
>> if (use_fifo) {
>> for (i = 0; i < val; i++)
>> esp_write8(esp->fifo[i], ESP_FDATA);
>> scsi_esp_cmd(esp, cmd);
>> } else {
>> if (data != esp->command_block)
>> memcpy(esp->command_block, data, length)
>> esp->ops->send_dma_cmd(esp,
>> esp->command_block_dma,
>> esp_count, dma_count, 0,
>> cmd | ESP_CMD_DMA);
>> }
>> }
>>
>> send_cmd(esp, data, esp_count, dma_count, cmd)
>> {
>> __send_cmd(esp, data, esp_count, dma_count, cmd, false, false);
>> }
>>
>> This would be:
>>
>> send_cmd(esp, esp->command_block, val, 16, ESP_CMD_SELA);
>>
> Good point.
(Note this was also why I was suggesting just using esp->command_block
unconditionally).
Paolo
> Will be updating the patch.
>
> Cheers,
>
> Hannes
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 08/10] am53c974: BLAST residual handling
2014-11-21 9:27 [PATCH 00/10] Re-implement am53c974 driver Hannes Reinecke
` (6 preceding siblings ...)
2014-11-21 9:27 ` [PATCH 07/10] esp: Use FIFO for command submission Hannes Reinecke
@ 2014-11-21 9:27 ` Hannes Reinecke
2014-11-21 10:10 ` Paolo Bonzini
2014-11-21 9:27 ` [PATCH 09/10] esp: correctly detect am53c974 Hannes Reinecke
` (2 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Hannes Reinecke @ 2014-11-21 9:27 UTC (permalink / raw)
To: James Bottomley
Cc: Christoph Hellwig, Paolo Bonzini, linux-scsi, Hannes Reinecke
The am53c974 has an design issue where a single byte might be
left in the SCSI FIFO after a DMA transfer.
As the handling code is currently untested add a WARN_ON()
statement here.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/am53c974.c | 6 ++++++
drivers/scsi/esp_scsi.c | 29 +++++++++++++++++++++++++++++
drivers/scsi/esp_scsi.h | 1 +
3 files changed, 36 insertions(+)
diff --git a/drivers/scsi/am53c974.c b/drivers/scsi/am53c974.c
index 652762e..0b7643e 100644
--- a/drivers/scsi/am53c974.c
+++ b/drivers/scsi/am53c974.c
@@ -195,6 +195,12 @@ static void pci_esp_dma_drain(struct esp *esp)
shost_printk(KERN_INFO, esp->host,
"DMA blast done (%d tries, %d bytes left)\n", lim, resid);
#endif
+ /* BLAST residual handling is currently untested */
+ if (WARN_ON(resid == 1)) {
+ struct esp_cmd_entry *ent = esp->active_cmd;
+
+ ent->flags |= ESP_CMD_FLAG_RESIDUAL;
+ }
}
static void pci_esp_dma_invalidate(struct esp *esp)
diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
index ab7d2bc..88272bb 100644
--- a/drivers/scsi/esp_scsi.c
+++ b/drivers/scsi/esp_scsi.c
@@ -1353,6 +1353,35 @@ static int esp_data_bytes_sent(struct esp *esp, struct esp_cmd_entry *ent,
bytes_sent = esp->data_dma_len;
bytes_sent -= ecount;
+ /*
+ * The am53c974 has a DMA 'pecularity'. The doc states:
+ * In some odd byte conditions, one residual byte will
+ * be left in the SCSI FIFO, and the FIFO Flags will
+ * never count to '0 '. When this happens, the residual
+ * byte should be retrieved via PIO following completion
+ * of the BLAST operation.
+ */
+ if (fifo_cnt == 1 && ent->flags & ESP_CMD_FLAG_RESIDUAL) {
+ size_t count = 1;
+ size_t offset = bytes_sent;
+ u8 bval = esp_read8(ESP_FDATA);
+
+ if (ent->flags & ESP_CMD_FLAG_AUTOSENSE)
+ ent->sense_ptr[bytes_sent] = bval;
+ else {
+ struct esp_cmd_priv *p = ESP_CMD_PRIV(cmd);
+ u8 *ptr;
+
+ ptr = scsi_kmap_atomic_sg(p->cur_sg, p->u.num_sg,
+ &offset, &count);
+ if (likely(ptr)) {
+ *(ptr + offset) = bval;
+ scsi_kunmap_atomic_sg(ptr);
+ }
+ }
+ bytes_sent += fifo_cnt;
+ ent->flags &= ~ESP_CMD_FLAG_RESIDUAL;
+ }
if (!(ent->flags & ESP_CMD_FLAG_WRITE))
bytes_sent -= fifo_cnt;
diff --git a/drivers/scsi/esp_scsi.h b/drivers/scsi/esp_scsi.h
index 27dcaf8..5fa456c 100644
--- a/drivers/scsi/esp_scsi.h
+++ b/drivers/scsi/esp_scsi.h
@@ -269,6 +269,7 @@ struct esp_cmd_entry {
#define ESP_CMD_FLAG_WRITE 0x01 /* DMA is a write */
#define ESP_CMD_FLAG_ABORT 0x02 /* being aborted */
#define ESP_CMD_FLAG_AUTOSENSE 0x04 /* Doing automatic REQUEST_SENSE */
+#define ESP_CMD_FLAG_RESIDUAL 0x08 /* AM53c974 BLAST residual */
u8 tag[2];
u8 orig_tag[2];
--
1.8.5.2
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 08/10] am53c974: BLAST residual handling
2014-11-21 9:27 ` [PATCH 08/10] am53c974: BLAST residual handling Hannes Reinecke
@ 2014-11-21 10:10 ` Paolo Bonzini
2014-11-21 10:29 ` Hannes Reinecke
0 siblings, 1 reply; 31+ messages in thread
From: Paolo Bonzini @ 2014-11-21 10:10 UTC (permalink / raw)
To: Hannes Reinecke, James Bottomley; +Cc: Christoph Hellwig, linux-scsi
On 21/11/2014 10:27, Hannes Reinecke wrote:
> The am53c974 has an design issue where a single byte might be
> left in the SCSI FIFO after a DMA transfer.
> As the handling code is currently untested add a WARN_ON()
> statement here.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> drivers/scsi/am53c974.c | 6 ++++++
> drivers/scsi/esp_scsi.c | 29 +++++++++++++++++++++++++++++
> drivers/scsi/esp_scsi.h | 1 +
> 3 files changed, 36 insertions(+)
>
> diff --git a/drivers/scsi/am53c974.c b/drivers/scsi/am53c974.c
> index 652762e..0b7643e 100644
> --- a/drivers/scsi/am53c974.c
> +++ b/drivers/scsi/am53c974.c
> @@ -195,6 +195,12 @@ static void pci_esp_dma_drain(struct esp *esp)
> shost_printk(KERN_INFO, esp->host,
> "DMA blast done (%d tries, %d bytes left)\n", lim, resid);
> #endif
> + /* BLAST residual handling is currently untested */
> + if (WARN_ON(resid == 1)) {
WARN_ON_ONCE?
Paolo
> + struct esp_cmd_entry *ent = esp->active_cmd;
> +
> + ent->flags |= ESP_CMD_FLAG_RESIDUAL;
> + }
> }
>
> static void pci_esp_dma_invalidate(struct esp *esp)
> diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
> index ab7d2bc..88272bb 100644
> --- a/drivers/scsi/esp_scsi.c
> +++ b/drivers/scsi/esp_scsi.c
> @@ -1353,6 +1353,35 @@ static int esp_data_bytes_sent(struct esp *esp, struct esp_cmd_entry *ent,
> bytes_sent = esp->data_dma_len;
> bytes_sent -= ecount;
>
> + /*
> + * The am53c974 has a DMA 'pecularity'. The doc states:
> + * In some odd byte conditions, one residual byte will
> + * be left in the SCSI FIFO, and the FIFO Flags will
> + * never count to '0 '. When this happens, the residual
> + * byte should be retrieved via PIO following completion
> + * of the BLAST operation.
> + */
> + if (fifo_cnt == 1 && ent->flags & ESP_CMD_FLAG_RESIDUAL) {
> + size_t count = 1;
> + size_t offset = bytes_sent;
> + u8 bval = esp_read8(ESP_FDATA);
> +
> + if (ent->flags & ESP_CMD_FLAG_AUTOSENSE)
> + ent->sense_ptr[bytes_sent] = bval;
> + else {
> + struct esp_cmd_priv *p = ESP_CMD_PRIV(cmd);
> + u8 *ptr;
> +
> + ptr = scsi_kmap_atomic_sg(p->cur_sg, p->u.num_sg,
> + &offset, &count);
> + if (likely(ptr)) {
> + *(ptr + offset) = bval;
> + scsi_kunmap_atomic_sg(ptr);
> + }
> + }
> + bytes_sent += fifo_cnt;
> + ent->flags &= ~ESP_CMD_FLAG_RESIDUAL;
> + }
> if (!(ent->flags & ESP_CMD_FLAG_WRITE))
> bytes_sent -= fifo_cnt;
>
> diff --git a/drivers/scsi/esp_scsi.h b/drivers/scsi/esp_scsi.h
> index 27dcaf8..5fa456c 100644
> --- a/drivers/scsi/esp_scsi.h
> +++ b/drivers/scsi/esp_scsi.h
> @@ -269,6 +269,7 @@ struct esp_cmd_entry {
> #define ESP_CMD_FLAG_WRITE 0x01 /* DMA is a write */
> #define ESP_CMD_FLAG_ABORT 0x02 /* being aborted */
> #define ESP_CMD_FLAG_AUTOSENSE 0x04 /* Doing automatic REQUEST_SENSE */
> +#define ESP_CMD_FLAG_RESIDUAL 0x08 /* AM53c974 BLAST residual */
>
> u8 tag[2];
> u8 orig_tag[2];
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 08/10] am53c974: BLAST residual handling
2014-11-21 10:10 ` Paolo Bonzini
@ 2014-11-21 10:29 ` Hannes Reinecke
0 siblings, 0 replies; 31+ messages in thread
From: Hannes Reinecke @ 2014-11-21 10:29 UTC (permalink / raw)
To: Paolo Bonzini, James Bottomley; +Cc: Christoph Hellwig, linux-scsi
On 11/21/2014 11:10 AM, Paolo Bonzini wrote:
>
>
> On 21/11/2014 10:27, Hannes Reinecke wrote:
>> The am53c974 has an design issue where a single byte might be
>> left in the SCSI FIFO after a DMA transfer.
>> As the handling code is currently untested add a WARN_ON()
>> statement here.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>> drivers/scsi/am53c974.c | 6 ++++++
>> drivers/scsi/esp_scsi.c | 29 +++++++++++++++++++++++++++++
>> drivers/scsi/esp_scsi.h | 1 +
>> 3 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/scsi/am53c974.c b/drivers/scsi/am53c974.c
>> index 652762e..0b7643e 100644
>> --- a/drivers/scsi/am53c974.c
>> +++ b/drivers/scsi/am53c974.c
>> @@ -195,6 +195,12 @@ static void pci_esp_dma_drain(struct esp *esp)
>> shost_printk(KERN_INFO, esp->host,
>> "DMA blast done (%d tries, %d bytes left)\n", lim, resid);
>> #endif
>> + /* BLAST residual handling is currently untested */
>> + if (WARN_ON(resid == 1)) {
>
> WARN_ON_ONCE?
>
I have not seen it once so far, so I doubt it's a common
case. But yeah, WARN_ON_ONCE is probably better.
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 09/10] esp: correctly detect am53c974
2014-11-21 9:27 [PATCH 00/10] Re-implement am53c974 driver Hannes Reinecke
` (7 preceding siblings ...)
2014-11-21 9:27 ` [PATCH 08/10] am53c974: BLAST residual handling Hannes Reinecke
@ 2014-11-21 9:27 ` Hannes Reinecke
2014-11-21 10:11 ` Paolo Bonzini
2014-11-21 9:27 ` [PATCH 10/10] esp: enable CONFIG2_FENAB for am53c974 Hannes Reinecke
2014-11-21 10:26 ` [PATCH 00/10] Re-implement am53c974 driver Christoph Hellwig
10 siblings, 1 reply; 31+ messages in thread
From: Hannes Reinecke @ 2014-11-21 9:27 UTC (permalink / raw)
To: James Bottomley
Cc: Christoph Hellwig, Paolo Bonzini, linux-scsi, Hannes Reinecke
The am53c974 returns the same ID as the FAS236, but implements
things slightly differently. So detect the am53c974 by checking
for ESP_CONFIG4 register.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/am53c974.c | 2 ++
drivers/scsi/esp_scsi.c | 17 ++++++++++++++++-
drivers/scsi/esp_scsi.h | 15 +++++++++++++++
3 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/am53c974.c b/drivers/scsi/am53c974.c
index 0b7643e..0452ed1 100644
--- a/drivers/scsi/am53c974.c
+++ b/drivers/scsi/am53c974.c
@@ -365,6 +365,8 @@ static void dc390_check_eeprom(struct esp *esp)
}
esp->scsi_id = EEbuf[DC390_EE_ADAPT_SCSI_ID];
esp->num_tags = 2 << EEbuf[DC390_EE_TAG_CMD_NUM];
+ if (EEbuf[DC390_EE_MODE2] & DC390_EE_MODE2_ACTIVE_NEGATION)
+ esp->config4 |= ESP_CONFIG4_RADE | ESP_CONFIG4_RAE;
}
static int pci_esp_probe_one(struct pci_dev *pdev,
diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
index 88272bb..01753f5 100644
--- a/drivers/scsi/esp_scsi.c
+++ b/drivers/scsi/esp_scsi.c
@@ -250,6 +250,19 @@ static void esp_reset_esp(struct esp *esp)
} else {
esp->min_period = ((5 * esp->ccycle) / 1000);
}
+ if (esp->rev == FAS236) {
+ /*
+ * The AM53c974 chip returns the same ID as FAS236;
+ * try to configure glitch eater.
+ */
+ u8 config4 = ESP_CONFIG4_GE1;
+ esp_write8(config4, ESP_CFG4);
+ config4 = esp_read8(ESP_CFG4);
+ if ((config4 & ESP_CONFIG4_GE1) == ESP_CONFIG4_GE1) {
+ esp->rev = PCSCSI;
+ esp_write8(esp->config4, ESP_CFG4);
+ }
+ }
esp->max_period = (esp->max_period + 3)>>2;
esp->min_period = (esp->min_period + 3)>>2;
@@ -275,7 +288,8 @@ static void esp_reset_esp(struct esp *esp)
/* fallthrough... */
case FAS236:
- /* Fast 236 or HME */
+ case PCSCSI:
+ /* Fast 236, AM53c974 or HME */
esp_write8(esp->config2, ESP_CFG2);
if (esp->rev == FASHME) {
u8 cfg3 = esp->target[0].esp_config3;
@@ -2397,6 +2411,7 @@ static const char *esp_chip_names[] = {
"FAS100A",
"FAST",
"FASHME",
+ "AM53C974",
};
static struct scsi_transport_template *esp_transport_template;
diff --git a/drivers/scsi/esp_scsi.h b/drivers/scsi/esp_scsi.h
index 5fa456c..84dcbe4 100644
--- a/drivers/scsi/esp_scsi.h
+++ b/drivers/scsi/esp_scsi.h
@@ -25,6 +25,7 @@
#define ESP_CTEST 0x0aUL /* wo Chip test register 0x28 */
#define ESP_CFG2 0x0bUL /* rw Second cfg register 0x2c */
#define ESP_CFG3 0x0cUL /* rw Third cfg register 0x30 */
+#define ESP_CFG4 0x0dUL /* rw Fourth cfg register 0x34 */
#define ESP_TCHI 0x0eUL /* rw High bits transf count 0x38 */
#define ESP_UID ESP_TCHI /* ro Unique ID code 0x38 */
#define FAS_RLO ESP_TCHI /* rw HME extended counter 0x38 */
@@ -76,6 +77,18 @@
#define ESP_CONFIG3_IMS 0x80 /* ID msg chk'ng (esp/fas236) */
#define ESP_CONFIG3_OBPUSH 0x80 /* Push odd-byte to dma (hme) */
+/* ESP config register 4 read-write, found only on am53c974 chips */
+#define ESP_CONFIG4_RADE 0x04 /* Active negation */
+#define ESP_CONFIG4_RAE 0x08 /* Active negation on REQ and ACK */
+#define ESP_CONFIG4_PWD 0x20 /* Reduced power feature */
+#define ESP_CONFIG4_GE0 0x40 /* Glitch eater bit 0 */
+#define ESP_CONFIG4_GE1 0x80 /* Glitch eater bit 1 */
+
+#define ESP_CONFIG_GE_12NS (0)
+#define ESP_CONFIG_GE_25NS (ESP_CONFIG_GE1)
+#define ESP_CONFIG_GE_35NS (ESP_CONFIG_GE0)
+#define ESP_CONFIG_GE_0NS (ESP_CONFIG_GE0 | ESP_CONFIG_GE1)
+
/* ESP command register read-write */
/* Group 1 commands: These may be sent at any point in time to the ESP
* chip. None of them can generate interrupts 'cept
@@ -254,6 +267,7 @@ enum esp_rev {
FAS100A = 0x04,
FAST = 0x05,
FASHME = 0x06,
+ PCSCSI = 0x07, /* AM53c974 */
};
struct esp_cmd_entry {
@@ -466,6 +480,7 @@ struct esp {
u8 bursts;
u8 config1;
u8 config2;
+ u8 config4;
u8 scsi_id;
u32 scsi_id_mask;
--
1.8.5.2
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 09/10] esp: correctly detect am53c974
2014-11-21 9:27 ` [PATCH 09/10] esp: correctly detect am53c974 Hannes Reinecke
@ 2014-11-21 10:11 ` Paolo Bonzini
0 siblings, 0 replies; 31+ messages in thread
From: Paolo Bonzini @ 2014-11-21 10:11 UTC (permalink / raw)
To: Hannes Reinecke, James Bottomley; +Cc: Christoph Hellwig, linux-scsi
On 21/11/2014 10:27, Hannes Reinecke wrote:
> The am53c974 returns the same ID as the FAS236, but implements
> things slightly differently. So detect the am53c974 by checking
> for ESP_CONFIG4 register.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> drivers/scsi/am53c974.c | 2 ++
> drivers/scsi/esp_scsi.c | 17 ++++++++++++++++-
> drivers/scsi/esp_scsi.h | 15 +++++++++++++++
> 3 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/am53c974.c b/drivers/scsi/am53c974.c
> index 0b7643e..0452ed1 100644
> --- a/drivers/scsi/am53c974.c
> +++ b/drivers/scsi/am53c974.c
> @@ -365,6 +365,8 @@ static void dc390_check_eeprom(struct esp *esp)
> }
> esp->scsi_id = EEbuf[DC390_EE_ADAPT_SCSI_ID];
> esp->num_tags = 2 << EEbuf[DC390_EE_TAG_CMD_NUM];
> + if (EEbuf[DC390_EE_MODE2] & DC390_EE_MODE2_ACTIVE_NEGATION)
> + esp->config4 |= ESP_CONFIG4_RADE | ESP_CONFIG4_RAE;
> }
>
> static int pci_esp_probe_one(struct pci_dev *pdev,
> diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
> index 88272bb..01753f5 100644
> --- a/drivers/scsi/esp_scsi.c
> +++ b/drivers/scsi/esp_scsi.c
> @@ -250,6 +250,19 @@ static void esp_reset_esp(struct esp *esp)
> } else {
> esp->min_period = ((5 * esp->ccycle) / 1000);
> }
> + if (esp->rev == FAS236) {
> + /*
> + * The AM53c974 chip returns the same ID as FAS236;
> + * try to configure glitch eater.
> + */
> + u8 config4 = ESP_CONFIG4_GE1;
> + esp_write8(config4, ESP_CFG4);
> + config4 = esp_read8(ESP_CFG4);
> + if ((config4 & ESP_CONFIG4_GE1) == ESP_CONFIG4_GE1) {
> + esp->rev = PCSCSI;
> + esp_write8(esp->config4, ESP_CFG4);
> + }
> + }
> esp->max_period = (esp->max_period + 3)>>2;
> esp->min_period = (esp->min_period + 3)>>2;
>
> @@ -275,7 +288,8 @@ static void esp_reset_esp(struct esp *esp)
> /* fallthrough... */
>
> case FAS236:
> - /* Fast 236 or HME */
> + case PCSCSI:
> + /* Fast 236, AM53c974 or HME */
> esp_write8(esp->config2, ESP_CFG2);
> if (esp->rev == FASHME) {
> u8 cfg3 = esp->target[0].esp_config3;
> @@ -2397,6 +2411,7 @@ static const char *esp_chip_names[] = {
> "FAS100A",
> "FAST",
> "FASHME",
> + "AM53C974",
> };
>
> static struct scsi_transport_template *esp_transport_template;
> diff --git a/drivers/scsi/esp_scsi.h b/drivers/scsi/esp_scsi.h
> index 5fa456c..84dcbe4 100644
> --- a/drivers/scsi/esp_scsi.h
> +++ b/drivers/scsi/esp_scsi.h
> @@ -25,6 +25,7 @@
> #define ESP_CTEST 0x0aUL /* wo Chip test register 0x28 */
> #define ESP_CFG2 0x0bUL /* rw Second cfg register 0x2c */
> #define ESP_CFG3 0x0cUL /* rw Third cfg register 0x30 */
> +#define ESP_CFG4 0x0dUL /* rw Fourth cfg register 0x34 */
> #define ESP_TCHI 0x0eUL /* rw High bits transf count 0x38 */
> #define ESP_UID ESP_TCHI /* ro Unique ID code 0x38 */
> #define FAS_RLO ESP_TCHI /* rw HME extended counter 0x38 */
> @@ -76,6 +77,18 @@
> #define ESP_CONFIG3_IMS 0x80 /* ID msg chk'ng (esp/fas236) */
> #define ESP_CONFIG3_OBPUSH 0x80 /* Push odd-byte to dma (hme) */
>
> +/* ESP config register 4 read-write, found only on am53c974 chips */
> +#define ESP_CONFIG4_RADE 0x04 /* Active negation */
> +#define ESP_CONFIG4_RAE 0x08 /* Active negation on REQ and ACK */
> +#define ESP_CONFIG4_PWD 0x20 /* Reduced power feature */
> +#define ESP_CONFIG4_GE0 0x40 /* Glitch eater bit 0 */
> +#define ESP_CONFIG4_GE1 0x80 /* Glitch eater bit 1 */
> +
> +#define ESP_CONFIG_GE_12NS (0)
> +#define ESP_CONFIG_GE_25NS (ESP_CONFIG_GE1)
> +#define ESP_CONFIG_GE_35NS (ESP_CONFIG_GE0)
> +#define ESP_CONFIG_GE_0NS (ESP_CONFIG_GE0 | ESP_CONFIG_GE1)
> +
> /* ESP command register read-write */
> /* Group 1 commands: These may be sent at any point in time to the ESP
> * chip. None of them can generate interrupts 'cept
> @@ -254,6 +267,7 @@ enum esp_rev {
> FAS100A = 0x04,
> FAST = 0x05,
> FASHME = 0x06,
> + PCSCSI = 0x07, /* AM53c974 */
> };
>
> struct esp_cmd_entry {
> @@ -466,6 +480,7 @@ struct esp {
> u8 bursts;
> u8 config1;
> u8 config2;
> + u8 config4;
>
> u8 scsi_id;
> u32 scsi_id_mask;
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 10/10] esp: enable CONFIG2_FENAB for am53c974
2014-11-21 9:27 [PATCH 00/10] Re-implement am53c974 driver Hannes Reinecke
` (8 preceding siblings ...)
2014-11-21 9:27 ` [PATCH 09/10] esp: correctly detect am53c974 Hannes Reinecke
@ 2014-11-21 9:27 ` Hannes Reinecke
2014-11-21 10:08 ` Paolo Bonzini
2014-11-21 10:26 ` [PATCH 00/10] Re-implement am53c974 driver Christoph Hellwig
10 siblings, 1 reply; 31+ messages in thread
From: Hannes Reinecke @ 2014-11-21 9:27 UTC (permalink / raw)
To: James Bottomley
Cc: Christoph Hellwig, Paolo Bonzini, linux-scsi, Hannes Reinecke
CONFIG2_FENAB ('feature enable') changed definition between chip
revisions, from 'Latch SCSI Phase' to 'Latch SCSI Phase, display
chip ID upon reset, and enable 24 bit addresses'.
So only enable it for am53c974 where we know what it's doing.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/am53c974.c | 30 ++++++++++++++++++++++++++++++
drivers/scsi/esp_scsi.c | 4 ++++
2 files changed, 34 insertions(+)
diff --git a/drivers/scsi/am53c974.c b/drivers/scsi/am53c974.c
index 0452ed1..722e781 100644
--- a/drivers/scsi/am53c974.c
+++ b/drivers/scsi/am53c974.c
@@ -252,6 +252,8 @@ static void pci_esp_send_dma_cmd(struct esp *esp, u32 addr, u32 esp_count,
pci_esp_write8(esp, (esp_count >> 0) & 0xff, ESP_TCLOW);
pci_esp_write8(esp, (esp_count >> 8) & 0xff, ESP_TCMED);
+ if (esp->config2 & ESP_CONFIG2_FENAB)
+ pci_esp_write8(esp, (esp_count >> 16) & 0xff, ESP_TCHI);
pci_esp_write32(esp, esp_count, ESP_DMA_STC);
pci_esp_write32(esp, addr, ESP_DMA_SPA);
@@ -265,6 +267,33 @@ static void pci_esp_send_dma_cmd(struct esp *esp, u32 addr, u32 esp_count,
pci_esp_write8(esp, ESP_DMA_CMD_START | val, ESP_DMA_CMD);
}
+static u32 pci_esp_dma_length_limit(struct esp *esp, u32 dma_addr, u32 dma_len)
+{
+ int dma_limit = 16;
+ u32 base, end;
+
+ /*
+ * If CONFIG2_FENAB is set we can
+ * handle up to 24 bit addresses
+ */
+ if (esp->config2 & ESP_CONFIG2_FENAB)
+ dma_limit = 24;
+
+ if (dma_len > (1U << dma_limit))
+ dma_len = (1U << dma_limit);
+
+ /*
+ * Prevent crossing a 24-bit address boundary.
+ */
+ base = dma_addr & ((1U << 24) - 1U);
+ end = base + dma_len;
+ if (end > (1U << 24))
+ end = (1U <<24);
+ dma_len = end - base;
+
+ return dma_len;
+}
+
static const struct esp_driver_ops pci_esp_ops = {
.esp_write8 = pci_esp_write8,
.esp_read8 = pci_esp_read8,
@@ -278,6 +307,7 @@ static const struct esp_driver_ops pci_esp_ops = {
.dma_invalidate = pci_esp_dma_invalidate,
.send_dma_cmd = pci_esp_send_dma_cmd,
.dma_error = pci_esp_dma_error,
+ .dma_length_limit = pci_esp_dma_length_limit,
};
/*
diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
index 01753f5..d0b7b32 100644
--- a/drivers/scsi/esp_scsi.c
+++ b/drivers/scsi/esp_scsi.c
@@ -289,6 +289,8 @@ static void esp_reset_esp(struct esp *esp)
case FAS236:
case PCSCSI:
+ if (esp->rev == PCSCSI)
+ esp->config2 |= ESP_CONFIG2_FENAB;
/* Fast 236, AM53c974 or HME */
esp_write8(esp->config2, ESP_CFG2);
if (esp->rev == FASHME) {
@@ -1362,6 +1364,8 @@ static int esp_data_bytes_sent(struct esp *esp, struct esp_cmd_entry *ent,
(((unsigned int)esp_read8(ESP_TCMED)) << 8));
if (esp->rev == FASHME)
ecount |= ((unsigned int)esp_read8(FAS_RLO)) << 16;
+ if (esp->rev == PCSCSI && (esp->config2 & ESP_CONFIG2_FENAB))
+ ecount |= ((unsigned int)esp_read8(ESP_TCHI)) << 16;
}
bytes_sent = esp->data_dma_len;
--
1.8.5.2
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 10/10] esp: enable CONFIG2_FENAB for am53c974
2014-11-21 9:27 ` [PATCH 10/10] esp: enable CONFIG2_FENAB for am53c974 Hannes Reinecke
@ 2014-11-21 10:08 ` Paolo Bonzini
2014-11-21 10:22 ` Hannes Reinecke
0 siblings, 1 reply; 31+ messages in thread
From: Paolo Bonzini @ 2014-11-21 10:08 UTC (permalink / raw)
To: Hannes Reinecke, James Bottomley; +Cc: Christoph Hellwig, linux-scsi
On 21/11/2014 10:27, Hannes Reinecke wrote:
> CONFIG2_FENAB ('feature enable') changed definition between chip
> revisions, from 'Latch SCSI Phase' to 'Latch SCSI Phase, display
> chip ID upon reset, and enable 24 bit addresses'.
> So only enable it for am53c974 where we know what it's doing.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> drivers/scsi/am53c974.c | 30 ++++++++++++++++++++++++++++++
> drivers/scsi/esp_scsi.c | 4 ++++
> 2 files changed, 34 insertions(+)
>
> diff --git a/drivers/scsi/am53c974.c b/drivers/scsi/am53c974.c
> index 0452ed1..722e781 100644
> --- a/drivers/scsi/am53c974.c
> +++ b/drivers/scsi/am53c974.c
> @@ -252,6 +252,8 @@ static void pci_esp_send_dma_cmd(struct esp *esp, u32 addr, u32 esp_count,
>
> pci_esp_write8(esp, (esp_count >> 0) & 0xff, ESP_TCLOW);
> pci_esp_write8(esp, (esp_count >> 8) & 0xff, ESP_TCMED);
> + if (esp->config2 & ESP_CONFIG2_FENAB)
> + pci_esp_write8(esp, (esp_count >> 16) & 0xff, ESP_TCHI);
Why do this conditionally? We know that FENAB is true here, don't we?
(Maybe I'm missing something obvious though).
Paolo
>
> pci_esp_write32(esp, esp_count, ESP_DMA_STC);
> pci_esp_write32(esp, addr, ESP_DMA_SPA);
> @@ -265,6 +267,33 @@ static void pci_esp_send_dma_cmd(struct esp *esp, u32 addr, u32 esp_count,
> pci_esp_write8(esp, ESP_DMA_CMD_START | val, ESP_DMA_CMD);
> }
>
> +static u32 pci_esp_dma_length_limit(struct esp *esp, u32 dma_addr, u32 dma_len)
> +{
> + int dma_limit = 16;
> + u32 base, end;
> +
> + /*
> + * If CONFIG2_FENAB is set we can
> + * handle up to 24 bit addresses
> + */
> + if (esp->config2 & ESP_CONFIG2_FENAB)
> + dma_limit = 24;
> +
> + if (dma_len > (1U << dma_limit))
> + dma_len = (1U << dma_limit);
> +
> + /*
> + * Prevent crossing a 24-bit address boundary.
> + */
> + base = dma_addr & ((1U << 24) - 1U);
> + end = base + dma_len;
> + if (end > (1U << 24))
> + end = (1U <<24);
> + dma_len = end - base;
> +
> + return dma_len;
> +}
> +
> static const struct esp_driver_ops pci_esp_ops = {
> .esp_write8 = pci_esp_write8,
> .esp_read8 = pci_esp_read8,
> @@ -278,6 +307,7 @@ static const struct esp_driver_ops pci_esp_ops = {
> .dma_invalidate = pci_esp_dma_invalidate,
> .send_dma_cmd = pci_esp_send_dma_cmd,
> .dma_error = pci_esp_dma_error,
> + .dma_length_limit = pci_esp_dma_length_limit,
> };
>
> /*
> diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
> index 01753f5..d0b7b32 100644
> --- a/drivers/scsi/esp_scsi.c
> +++ b/drivers/scsi/esp_scsi.c
> @@ -289,6 +289,8 @@ static void esp_reset_esp(struct esp *esp)
>
> case FAS236:
> case PCSCSI:
> + if (esp->rev == PCSCSI)
> + esp->config2 |= ESP_CONFIG2_FENAB;
> /* Fast 236, AM53c974 or HME */
> esp_write8(esp->config2, ESP_CFG2);
> if (esp->rev == FASHME) {
> @@ -1362,6 +1364,8 @@ static int esp_data_bytes_sent(struct esp *esp, struct esp_cmd_entry *ent,
> (((unsigned int)esp_read8(ESP_TCMED)) << 8));
> if (esp->rev == FASHME)
> ecount |= ((unsigned int)esp_read8(FAS_RLO)) << 16;
> + if (esp->rev == PCSCSI && (esp->config2 & ESP_CONFIG2_FENAB))
> + ecount |= ((unsigned int)esp_read8(ESP_TCHI)) << 16;
> }
>
> bytes_sent = esp->data_dma_len;
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 10/10] esp: enable CONFIG2_FENAB for am53c974
2014-11-21 10:08 ` Paolo Bonzini
@ 2014-11-21 10:22 ` Hannes Reinecke
2014-11-21 10:52 ` Paolo Bonzini
0 siblings, 1 reply; 31+ messages in thread
From: Hannes Reinecke @ 2014-11-21 10:22 UTC (permalink / raw)
To: Paolo Bonzini, James Bottomley; +Cc: Christoph Hellwig, linux-scsi
On 11/21/2014 11:08 AM, Paolo Bonzini wrote:
>
>
> On 21/11/2014 10:27, Hannes Reinecke wrote:
>> CONFIG2_FENAB ('feature enable') changed definition between chip
>> revisions, from 'Latch SCSI Phase' to 'Latch SCSI Phase, display
>> chip ID upon reset, and enable 24 bit addresses'.
>> So only enable it for am53c974 where we know what it's doing.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>> drivers/scsi/am53c974.c | 30 ++++++++++++++++++++++++++++++
>> drivers/scsi/esp_scsi.c | 4 ++++
>> 2 files changed, 34 insertions(+)
>>
>> diff --git a/drivers/scsi/am53c974.c b/drivers/scsi/am53c974.c
>> index 0452ed1..722e781 100644
>> --- a/drivers/scsi/am53c974.c
>> +++ b/drivers/scsi/am53c974.c
>> @@ -252,6 +252,8 @@ static void pci_esp_send_dma_cmd(struct esp *esp, u32 addr, u32 esp_count,
>>
>> pci_esp_write8(esp, (esp_count >> 0) & 0xff, ESP_TCLOW);
>> pci_esp_write8(esp, (esp_count >> 8) & 0xff, ESP_TCMED);
>> + if (esp->config2 & ESP_CONFIG2_FENAB)
>> + pci_esp_write8(esp, (esp_count >> 16) & 0xff, ESP_TCHI);
>
> Why do this conditionally? We know that FENAB is true here, don't we?
>
> (Maybe I'm missing something obvious though).
>
Not really. Point is that 'FENAB' does actually three things:
- Enable TCHI for 24-bit DMA transfer lengths
- Provide Chip ID in TCHI after reset
- Latch SCSI phase after completion in SCSI STATUS
So we _might_ run into timing issues due to the last point, so I've
made it conditional in case we'd have to disable it.
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
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 10/10] esp: enable CONFIG2_FENAB for am53c974
2014-11-21 10:22 ` Hannes Reinecke
@ 2014-11-21 10:52 ` Paolo Bonzini
0 siblings, 0 replies; 31+ messages in thread
From: Paolo Bonzini @ 2014-11-21 10:52 UTC (permalink / raw)
To: Hannes Reinecke, James Bottomley; +Cc: Christoph Hellwig, linux-scsi
On 21/11/2014 11:22, Hannes Reinecke wrote:
> On 11/21/2014 11:08 AM, Paolo Bonzini wrote:
>>
>>
>> On 21/11/2014 10:27, Hannes Reinecke wrote:
>>> CONFIG2_FENAB ('feature enable') changed definition between chip
>>> revisions, from 'Latch SCSI Phase' to 'Latch SCSI Phase, display
>>> chip ID upon reset, and enable 24 bit addresses'.
>>> So only enable it for am53c974 where we know what it's doing.
>>>
>>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>>> ---
>>> drivers/scsi/am53c974.c | 30 ++++++++++++++++++++++++++++++
>>> drivers/scsi/esp_scsi.c | 4 ++++
>>> 2 files changed, 34 insertions(+)
>>>
>>> diff --git a/drivers/scsi/am53c974.c b/drivers/scsi/am53c974.c
>>> index 0452ed1..722e781 100644
>>> --- a/drivers/scsi/am53c974.c
>>> +++ b/drivers/scsi/am53c974.c
>>> @@ -252,6 +252,8 @@ static void pci_esp_send_dma_cmd(struct esp *esp, u32 addr, u32 esp_count,
>>>
>>> pci_esp_write8(esp, (esp_count >> 0) & 0xff, ESP_TCLOW);
>>> pci_esp_write8(esp, (esp_count >> 8) & 0xff, ESP_TCMED);
>>> + if (esp->config2 & ESP_CONFIG2_FENAB)
>>> + pci_esp_write8(esp, (esp_count >> 16) & 0xff, ESP_TCHI);
>>
>> Why do this conditionally? We know that FENAB is true here, don't we?
>>
>> (Maybe I'm missing something obvious though).
>>
> Not really. Point is that 'FENAB' does actually three things:
> - Enable TCHI for 24-bit DMA transfer lengths
> - Provide Chip ID in TCHI after reset
> - Latch SCSI phase after completion in SCSI STATUS
> So we _might_ run into timing issues due to the last point, so I've
> made it conditional in case we'd have to disable it.
Maybe make it a module parameter maybe?
Something like this lets you set esp->config2 from the driver. Look at
it with -b to avoid insanity, it changes half a dozen line modulo the
reindendation.
-------------- 8< --------------------
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: [PATCH] esp_scsi: let DMA driver provide a config2 value
On PCscsi, the FENAB configuration also enables 24-bit DMA transfer lengths
(and provides the chip id in TCHI after reset). We want this to be available
as a module parameter.
Check if the caller of scsi_esp_register provided a value for esp->config2.
If this is the case, assume this is not an ESP100, skip the detection
phase and leave esp->config2 untouched. It will be used in esp_reset_esp.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
index 55548dc5cec3..123edcf439c0 100644
--- a/drivers/scsi/esp_scsi.c
+++ b/drivers/scsi/esp_scsi.c
@@ -2149,47 +2149,50 @@ static void esp_get_revision(struct esp *esp)
u8 val;
esp->config1 = (ESP_CONFIG1_PENABLE | (esp->scsi_id & 7));
- esp->config2 = (ESP_CONFIG2_SCSI2ENAB | ESP_CONFIG2_REGPARITY);
+ if (esp->config2 == 0) {
+ esp->config2 = (ESP_CONFIG2_SCSI2ENAB | ESP_CONFIG2_REGPARITY);
+ esp_write8(esp->config2, ESP_CFG2);
+
+ val = esp_read8(ESP_CFG2);
+ val &= ~ESP_CONFIG2_MAGIC;
+
+ esp->config2 = 0;
+ if (val != (ESP_CONFIG2_SCSI2ENAB | ESP_CONFIG2_REGPARITY)) {
+ /* If what we write to cfg2 does not come back, cfg2 is not
+ * implemented, therefore this must be a plain esp100.
+ */
+ esp->rev = ESP100;
+ return;
+ }
+ }
+
+ esp_set_all_config3(esp, 5);
+ esp->prev_cfg3 = 5;
esp_write8(esp->config2, ESP_CFG2);
+ esp_write8(0, ESP_CFG3);
+ esp_write8(esp->prev_cfg3, ESP_CFG3);
- val = esp_read8(ESP_CFG2);
- val &= ~ESP_CONFIG2_MAGIC;
- if (val != (ESP_CONFIG2_SCSI2ENAB | ESP_CONFIG2_REGPARITY)) {
- /* If what we write to cfg2 does not come back, cfg2 is not
- * implemented, therefore this must be a plain esp100.
+ val = esp_read8(ESP_CFG3);
+ if (val != 5) {
+ /* The cfg2 register is implemented, however
+ * cfg3 is not, must be esp100a.
*/
- esp->rev = ESP100;
+ esp->rev = ESP100A;
} else {
- esp->config2 = 0;
- esp_set_all_config3(esp, 5);
- esp->prev_cfg3 = 5;
- esp_write8(esp->config2, ESP_CFG2);
- esp_write8(0, ESP_CFG3);
+ esp_set_all_config3(esp, 0);
+ esp->prev_cfg3 = 0;
esp_write8(esp->prev_cfg3, ESP_CFG3);
- val = esp_read8(ESP_CFG3);
- if (val != 5) {
- /* The cfg2 register is implemented, however
- * cfg3 is not, must be esp100a.
- */
- esp->rev = ESP100A;
+ /* All of cfg{1,2,3} implemented, must be one of
+ * the fas variants, figure out which one.
+ */
+ if (esp->cfact == 0 || esp->cfact > ESP_CCF_F5) {
+ esp->rev = FAST;
+ esp->sync_defp = SYNC_DEFP_FAST;
} else {
- esp_set_all_config3(esp, 0);
- esp->prev_cfg3 = 0;
- esp_write8(esp->prev_cfg3, ESP_CFG3);
-
- /* All of cfg{1,2,3} implemented, must be one of
- * the fas variants, figure out which one.
- */
- if (esp->cfact == 0 || esp->cfact > ESP_CCF_F5) {
- esp->rev = FAST;
- esp->sync_defp = SYNC_DEFP_FAST;
- } else {
- esp->rev = ESP236;
- }
- esp->config2 = 0;
- esp_write8(esp->config2, ESP_CFG2);
+ esp->rev = ESP236;
}
+ esp_write8(esp->config2, ESP_CFG2);
}
}
Paolo
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 00/10] Re-implement am53c974 driver
2014-11-21 9:27 [PATCH 00/10] Re-implement am53c974 driver Hannes Reinecke
` (9 preceding siblings ...)
2014-11-21 9:27 ` [PATCH 10/10] esp: enable CONFIG2_FENAB for am53c974 Hannes Reinecke
@ 2014-11-21 10:26 ` Christoph Hellwig
2014-11-21 10:33 ` Hannes Reinecke
10 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2014-11-21 10:26 UTC (permalink / raw)
To: Hannes Reinecke
Cc: James Bottomley, Paolo Bonzini, linux-scsi, davem,
Guennadi Liakhovetski
On Fri, Nov 21, 2014 at 10:27:47AM +0100, Hannes Reinecke wrote:
> Hi all,
>
> having found some issues with the current tmscsim driver
> I looked at the code and found is really awkward to clean
> up.
> Seeing that the am53c974 chip is actually based on the
> 'ESP' chip design I've re-implemented it based on the
> common esp_scsi routines. This new driver
> (cunningly named 'am53c974') provides all features
> found on the tmscsim driver, with the goal of obsoleting
> the old tmscsim driver.
> Tested with Tekram DC390 and qemu esp-scsi.
This looks very nice!
Three high level comments:
- please Cc Dave for changes to esp_scsi
- please Cc Guennadi as the tmscsim maintainer
- this probably should deprecated and/or remove the tmscsim driver
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 00/10] Re-implement am53c974 driver
2014-11-21 10:26 ` [PATCH 00/10] Re-implement am53c974 driver Christoph Hellwig
@ 2014-11-21 10:33 ` Hannes Reinecke
2014-11-21 10:47 ` Guennadi Liakhovetski
0 siblings, 1 reply; 31+ messages in thread
From: Hannes Reinecke @ 2014-11-21 10:33 UTC (permalink / raw)
To: Christoph Hellwig
Cc: James Bottomley, Paolo Bonzini, linux-scsi, davem,
Guennadi Liakhovetski
On 11/21/2014 11:26 AM, Christoph Hellwig wrote:
> On Fri, Nov 21, 2014 at 10:27:47AM +0100, Hannes Reinecke wrote:
>> Hi all,
>>
>> having found some issues with the current tmscsim driver
>> I looked at the code and found is really awkward to clean
>> up.
>> Seeing that the am53c974 chip is actually based on the
>> 'ESP' chip design I've re-implemented it based on the
>> common esp_scsi routines. This new driver
>> (cunningly named 'am53c974') provides all features
>> found on the tmscsim driver, with the goal of obsoleting
>> the old tmscsim driver.
>> Tested with Tekram DC390 and qemu esp-scsi.
>
> This looks very nice!
>
> Three high level comments:
>
> - please Cc Dave for changes to esp_scsi
That's what I thought, too, but 'get_maintainers.pl' doesn't
know about him. Maybe we should fix this up ...
> - please Cc Guennadi as the tmscsim maintainer
> - this probably should deprecated and/or remove the tmscsim driver
>
That was the general idea. I just thought to be conservative and
add a new driver. But yeah, the idea is to sent another patch
to remove tmscsim altogether.
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 00/10] Re-implement am53c974 driver
2014-11-21 10:33 ` Hannes Reinecke
@ 2014-11-21 10:47 ` Guennadi Liakhovetski
0 siblings, 0 replies; 31+ messages in thread
From: Guennadi Liakhovetski @ 2014-11-21 10:47 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Christoph Hellwig, James Bottomley, Paolo Bonzini, linux-scsi,
davem
Hi,
On Fri, 21 Nov 2014, Hannes Reinecke wrote:
> On 11/21/2014 11:26 AM, Christoph Hellwig wrote:
> > On Fri, Nov 21, 2014 at 10:27:47AM +0100, Hannes Reinecke wrote:
> >> Hi all,
> >>
> >> having found some issues with the current tmscsim driver
> >> I looked at the code and found is really awkward to clean
> >> up.
> >> Seeing that the am53c974 chip is actually based on the
> >> 'ESP' chip design I've re-implemented it based on the
> >> common esp_scsi routines. This new driver
> >> (cunningly named 'am53c974') provides all features
> >> found on the tmscsim driver, with the goal of obsoleting
> >> the old tmscsim driver.
> >> Tested with Tekram DC390 and qemu esp-scsi.
> >
> > This looks very nice!
> >
> > Three high level comments:
> >
> > - please Cc Dave for changes to esp_scsi
> That's what I thought, too, but 'get_maintainers.pl' doesn't
> know about him. Maybe we should fix this up ...
>
> > - please Cc Guennadi as the tmscsim maintainer
> > - this probably should deprecated and/or remove the tmscsim driver
> >
> That was the general idea. I just thought to be conservative and
> add a new driver. But yeah, the idea is to sent another patch
> to remove tmscsim altogether.
Sure, the driver is rather old and... well... its style is somewhat
foreign to the modern Linux kernel coding style :) I actually also
thought, that the hardware is really legacy and hardly anyone is still
using it, so the tmscsim driver was in a bug-fix only mode, but if you
want to replace it with a better written one - certainly go for it! I've
hardly had any work with this my maintainer task in the last few years,
except for a recent SCSI Tag rehash, but that was committed before I had
enough time to sufficiently understand them :) And I'm not using that
hardware actively any more these days... So, I'd have nothing against
removal of tmscsim and, respectively, from its entry in MAINTAINERS :)
Thanks
Guennadi
^ permalink raw reply [flat|nested] 31+ messages in thread