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 <[email protected]>
>>> ---
>>> 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 <[email protected]>
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 <[email protected]>
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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
