On Tue, Feb 22, 2011 at 02:32:39PM +0530, Viresh Kumar wrote:
> The Arasan CompactFlash Device Controller has three basic modes of
> operation: PC card ATA using I/O mode, PC card ATA using memory mode, PC card
> ATA using true IDE modes.
>
> Currently driver supports only True IDE mode.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxx>
Looks mostly good to me now but just few more things.
> +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.
> +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.
> +static int __devinit arasan_cf_probe(struct platform_device *pdev)
> +{
...
> + ap->flags |= ATA_FLAG_PIO_POLLING;
Maybe ATA_FLAG_NO_ATAPI too?
Thanks.
--
tejun
--
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
