Re: [PATCH 09/11] qla4xxx: Added support for ISP82XX

[Mike Christie <michaelc@cs.wisc.edu>]

On 06/11/2010 03:44 AM, Vikas Chaudhary wrote:
>>> +/*
>>> +Address and length are byte address
>>> +*/
>>> +uint8_t *
>>> +qla82xx_read_optrom_data(struct scsi_qla_host *ha, uint8_t *buf,
>>> +            uint32_t offset, uint32_t length)
>>> +{
>>> +    scsi_block_requests(ha->host);
>>> +
>>> +    qla82xx_read_flash_data(ha, (uint32_t *)buf, offset, length);
>>> +
>>> +    scsi_unblock_requests(ha->host);
>>
>>
>>
>> What is the block/unblock for?  Looks like it could be racey.
>>
>>
>
> We want to block I/O while doing flash operations.
>

I was more wondering if there is also a test below that is also hit when 
you are doing flash operations? Is it the dpc reset bit check?

And what happens to scsi commands that are running when the flash 
operation runs? Does your firmware fail them and if it does what host 
byte error code are you using (or could you point out the chunk of 
driver code that handles this)?



> if (test_bit(DPC_RESET_HA_INTR,&ha->dpc_flags) ||
>              test_bit(DPC_RESET_ACTIVE,&ha->dpc_flags) ||
>              test_bit(DPC_RESET_HA,&ha->dpc_flags) ||
>              test_bit(DPC_HA_UNRECOVERABLE,&ha->dpc_flags) ||
>              test_bit(DPC_HA_NEED_QUIESCENT,&ha->dpc_flags) ||
>              !test_bit(AF_ONLINE,&ha->flags) ||
>              test_bit(DPC_RESET_HA_FW_CONTEXT,&ha->dpc_flags))
>                  goto qc_host_busy;
>

I missed that chunk when I first reviewed the patch. I think that is 
where I got confused in the other comment.