On 02/22/2011 02:44 PM, Tejun Heo wrote:
> On Tue, Feb 22, 2011 at 02:32:39PM +0530, Viresh Kumar wrote:
>> +static inline void dma_complete(struct arasan_cf_dev *acdev)
>> +{
>> + struct ata_queued_cmd *qc = acdev->qc;
>> +
>> + acdev->qc = NULL;
>> + ata_sff_interrupt(acdev->irq, acdev->host);
>> +
>> + if (unlikely(qc->err_mask) && ata_is_dma(qc->tf.protocol))
>> + ata_ehi_push_desc(&qc->ap->link.eh_info, "DMA Failed: Timeout");
>> +}
>
> ata_ehi_push_desc() needs to be called under host lock. In case I
> missed other places, all functions which modify port/host states need
> to be called under host lock. Please make sure they are.
>
Ok. Will do that. I have checked other places, lock was missing only at this place.
>> +void arasan_cf_error_handler(struct ata_port *ap)
>> +{
>> + struct arasan_cf_dev *acdev = ap->host->private_data;
>> +
>> + cancel_work_sync(&acdev->work);
>> + cancel_delayed_work_sync(&acdev->dwork);
>> + return ata_sff_error_handler(ap);
>> +}
>
> Please add some comments explaining what's going on with DMA
> processing as this driver is quite unusual. Here and in the issue
> path.
>
Will do that.
>> +static int __devinit arasan_cf_probe(struct platform_device *pdev)
>> +{
> ...
>> + ap->flags |= ATA_FLAG_PIO_POLLING;
>
> Maybe ATA_FLAG_NO_ATAPI too?
>
Yes.
--
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
