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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html