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

Reply via email to