On 2/18/11, Tejun Heo <tj@xxxxxxxxxx> wrote:
> On Fri, Feb 18, 2011 at 05:10:14PM +0530, Viresh Kumar wrote:
>> +static struct scsi_host_template arasan_cf_sht = {
>> + ATA_PIO_SHT(DRIVER_NAME),
>> +};
>
> Hmmm... You probably have different requirements for .sg_tablesize and
> .dma_boundary, likely to be more lax.
>
I will recheck this structure.
>> +static inline void cf_ctrl_reset(struct arasan_cf_dev *acdev)
>> +{
>> + writel(readl(acdev->vbase + OP_MODE) & ~CFHOST_ENB, acdev->vbase +
>> + OP_MODE);
>> + writel(readl(acdev->vbase + OP_MODE) | CFHOST_ENB, acdev->vbase +
>> + OP_MODE);
>> +}
>
> No big deal but I think the following is more preferrable.
>
> writel(readl(acdev->vbase + OP_MODE) & ~CFHOST_ENB,
> acdev->vbase + OP_MODE);
>
> Applies to other similar line breaks too.
>
Ok
>> +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 stat 0x%x",
>> + ATA_DMA_INTR | ATA_DMA_ERR);
>> +}
>
> Those are BMDMA status bits, right? Why not just print out string
> describing what's going on?
>
Ok.
>> +static inline int wait4buf(struct arasan_cf_dev *acdev)
>> +{
>> + if (!wait_for_completion_timeout(&acdev->cf_completion, TIMEOUT)) {
>> + u32 rw = acdev->qc->tf.flags & ATA_TFLAG_WRITE;
>> +
>> + dev_err(acdev->host->dev, "%s TimeOut", rw ? "write" : "read");
>> + return -ETIMEDOUT;
>> + }
>
> Like here.
>
> Hmmm... so, now there's a work item which requeues itself. That can't
> be reliably stopped from freeze() as it's called with host lock held
> without process context. I think you should override
> ->error_handler() which looks like the following,
>
actually i am disabling controller in freeze. After which we will
surely get timeout here and
so finally we will get an error in this work. Wouldn't that be enough?
> arsan_error_handler(...)
> {
> cancel_work_sync(...);
> cancel_delayed_work_sync(...);
>
> return ata_sff_error_handler(xxx);
> }
>
thanks for your time & feedback.
--
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
