Re: [PATCH] aha1542: convert to DMA mapping API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Friday 09 November 2018 16:45:54 Christoph Hellwig wrote:
> aha1542 is one of the last users of the legacy isa_*_to_bus APIs, which
> also isn't portable enough.  Convert it to the proper DMA mapping API.
> 
> Thanks to Ondrej Zary for testing and finding and fixing a crucial
> bug.
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> Tested-by: Ondrej Zary <linux@xxxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/scsi/aha1542.c | 126 +++++++++++++++++++++++++++++------------
>  1 file changed, 91 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/scsi/aha1542.c b/drivers/scsi/aha1542.c
> index 41add33e3f1f..398fcdc6f4c9 100644
> --- a/drivers/scsi/aha1542.c
> +++ b/drivers/scsi/aha1542.c
> @@ -58,8 +58,15 @@ struct aha1542_hostdata {
>  	int aha1542_last_mbi_used;
>  	int aha1542_last_mbo_used;
>  	struct scsi_cmnd *int_cmds[AHA1542_MAILBOXES];
> -	struct mailbox mb[2 * AHA1542_MAILBOXES];
> -	struct ccb ccb[AHA1542_MAILBOXES];
> +	struct mailbox *mb;
> +	dma_addr_t mb_handle;
> +	struct ccb *ccb;
> +	dma_addr_t ccb_handle;
> +};
> +
> +struct aha1542_cmd {
> +	struct chain *chain;
> +	dma_addr_t chain_handle;
>  };
>  
>  static inline void aha1542_intr_reset(u16 base)
> @@ -233,6 +240,21 @@ static int aha1542_test_port(struct Scsi_Host *sh)
>  	return 1;
>  }
>  
> +static void aha1542_free_cmd(struct scsi_cmnd *cmd)
> +{
> +	struct aha1542_cmd *acmd = scsi_cmd_priv(cmd);
> +	struct device *dev = cmd->device->host->dma_dev;
> +	size_t len = scsi_sg_count(cmd) * sizeof(struct chain);
> +
> +	if (acmd->chain) {
> +		dma_unmap_single(dev, acmd->chain_handle, len, DMA_TO_DEVICE);
> +		kfree(acmd->chain);
> +	}
> +
> +	acmd->chain = NULL;
> +	scsi_dma_unmap(cmd);
> +}
> +
>  static irqreturn_t aha1542_interrupt(int irq, void *dev_id)
>  {
>  	struct Scsi_Host *sh = dev_id;
> @@ -303,7 +325,7 @@ static irqreturn_t aha1542_interrupt(int irq, void *dev_id)
>  			return IRQ_HANDLED;
>  		};
>  
> -		mbo = (scsi2int(mb[mbi].ccbptr) - (isa_virt_to_bus(&ccb[0]))) / sizeof(struct ccb);
> +		mbo = (scsi2int(mb[mbi].ccbptr) - aha1542->ccb_handle) / sizeof(struct ccb);
>  		mbistatus = mb[mbi].status;
>  		mb[mbi].status = 0;
>  		aha1542->aha1542_last_mbi_used = mbi;
> @@ -331,8 +353,7 @@ static irqreturn_t aha1542_interrupt(int irq, void *dev_id)
>  			return IRQ_HANDLED;
>  		}
>  		my_done = tmp_cmd->scsi_done;
> -		kfree(tmp_cmd->host_scribble);
> -		tmp_cmd->host_scribble = NULL;
> +		aha1542_free_cmd(tmp_cmd);
>  		/* Fetch the sense data, and tuck it away, in the required slot.  The
>  		   Adaptec automatically fetches it, and there is no guarantee that
>  		   we will still have it in the cdb when we come back */
> @@ -369,6 +390,7 @@ static irqreturn_t aha1542_interrupt(int irq, void *dev_id)
>  
>  static int aha1542_queuecommand(struct Scsi_Host *sh, struct scsi_cmnd *cmd)
>  {
> +	struct aha1542_cmd *acmd = scsi_cmd_priv(cmd);
>  	struct aha1542_hostdata *aha1542 = shost_priv(sh);
>  	u8 direction;
>  	u8 target = cmd->device->id;
> @@ -378,7 +400,6 @@ static int aha1542_queuecommand(struct Scsi_Host *sh, struct scsi_cmnd *cmd)
>  	int mbo, sg_count;
>  	struct mailbox *mb = aha1542->mb;
>  	struct ccb *ccb = aha1542->ccb;
> -	struct chain *cptr;
>  
>  	if (*cmd->cmnd == REQUEST_SENSE) {
>  		/* Don't do the command - we have the sense data already */
> @@ -398,15 +419,17 @@ static int aha1542_queuecommand(struct Scsi_Host *sh, struct scsi_cmnd *cmd)
>  		print_hex_dump_bytes("command: ", DUMP_PREFIX_NONE, cmd->cmnd, cmd->cmd_len);
>  	}
>  #endif
> -	if (bufflen) {	/* allocate memory before taking host_lock */
> -		sg_count = scsi_sg_count(cmd);
> -		cptr = kmalloc_array(sg_count, sizeof(*cptr),
> -				     GFP_KERNEL | GFP_DMA);
> -		if (!cptr)
> -			return SCSI_MLQUEUE_HOST_BUSY;
> -	} else {
> -		sg_count = 0;
> -		cptr = NULL;
> +	sg_count = scsi_dma_map(cmd);
> +	if (sg_count) {
> +		size_t len = sg_count * sizeof(struct chain);
> +
> +		acmd->chain = kmalloc(len, GFP_DMA);
> +		if (!acmd->chain)
> +			goto out_unmap;
> +		acmd->chain_handle = dma_map_single(sh->dma_dev, acmd->chain,
> +				len, DMA_TO_DEVICE);
> +		if (dma_mapping_error(sh->dma_dev, acmd->chain_handle))
> +			goto out_free_chain;
>  	}
>  
>  	/* Use the outgoing mailboxes in a round-robin fashion, because this
> @@ -437,7 +460,8 @@ static int aha1542_queuecommand(struct Scsi_Host *sh, struct scsi_cmnd *cmd)
>  	shost_printk(KERN_DEBUG, sh, "Sending command (%d %p)...", mbo, cmd->scsi_done);
>  #endif
>  
> -	any2scsi(mb[mbo].ccbptr, isa_virt_to_bus(&ccb[mbo]));	/* This gets trashed for some reason */
> +	/* This gets trashed for some reason */
> +	any2scsi(mb[mbo].ccbptr, aha1542->ccb_handle + mbo * sizeof(*ccb));
>  
>  	memset(&ccb[mbo], 0, sizeof(struct ccb));
>  
> @@ -456,21 +480,18 @@ static int aha1542_queuecommand(struct Scsi_Host *sh, struct scsi_cmnd *cmd)
>  		int i;
>  
>  		ccb[mbo].op = 2;	/* SCSI Initiator Command  w/scatter-gather */
> -		cmd->host_scribble = (void *)cptr;
>  		scsi_for_each_sg(cmd, sg, sg_count, i) {
> -			any2scsi(cptr[i].dataptr, isa_page_to_bus(sg_page(sg))
> -								+ sg->offset);
> -			any2scsi(cptr[i].datalen, sg->length);
> +			any2scsi(acmd->chain[i].dataptr, sg_dma_address(sg));
> +			any2scsi(acmd->chain[i].datalen, sg_dma_len(sg));
>  		};
>  		any2scsi(ccb[mbo].datalen, sg_count * sizeof(struct chain));
> -		any2scsi(ccb[mbo].dataptr, isa_virt_to_bus(cptr));
> +		any2scsi(ccb[mbo].dataptr, acmd->chain_handle);
>  #ifdef DEBUG
> -		shost_printk(KERN_DEBUG, sh, "cptr %p: ", cptr);
> -		print_hex_dump_bytes("cptr: ", DUMP_PREFIX_NONE, cptr, 18);
> +		shost_printk(KERN_DEBUG, sh, "cptr %p: ", acmd->chain);
> +		print_hex_dump_bytes("cptr: ", DUMP_PREFIX_NONE, acmd->chain, 18);
>  #endif
>  	} else {
>  		ccb[mbo].op = 0;	/* SCSI Initiator Command */
> -		cmd->host_scribble = NULL;
>  		any2scsi(ccb[mbo].datalen, 0);
>  		any2scsi(ccb[mbo].dataptr, 0);
>  	};
> @@ -488,24 +509,29 @@ static int aha1542_queuecommand(struct Scsi_Host *sh, struct scsi_cmnd *cmd)
>  	spin_unlock_irqrestore(sh->host_lock, flags);
>  
>  	return 0;
> +out_free_chain:
> +	kfree(acmd->chain);
> +	acmd->chain = NULL;
> +out_unmap:
> +	scsi_dma_unmap(cmd);
> +	return SCSI_MLQUEUE_HOST_BUSY;
>  }
>  
>  /* Initialize mailboxes */
>  static void setup_mailboxes(struct Scsi_Host *sh)
>  {
>  	struct aha1542_hostdata *aha1542 = shost_priv(sh);
> -	int i;
> -	struct mailbox *mb = aha1542->mb;
> -	struct ccb *ccb = aha1542->ccb;
> -
>  	u8 mb_cmd[5] = { CMD_MBINIT, AHA1542_MAILBOXES, 0, 0, 0};
> +	int i;
>  
>  	for (i = 0; i < AHA1542_MAILBOXES; i++) {
> -		mb[i].status = mb[AHA1542_MAILBOXES + i].status = 0;
> -		any2scsi(mb[i].ccbptr, isa_virt_to_bus(&ccb[i]));
> +		aha1542->mb[i].status = 0;
> +		any2scsi(aha1542->mb[i].ccbptr,
> +			 aha1542->ccb_handle + i * sizeof(struct ccb));
> +		aha1542->mb[AHA1542_MAILBOXES + i].status = 0;
>  	};
>  	aha1542_intr_reset(sh->io_port);	/* reset interrupts, so they don't block */
> -	any2scsi((mb_cmd + 2), isa_virt_to_bus(mb));
> +	any2scsi(mb_cmd + 2, aha1542->mb_handle);
>  	if (aha1542_out(sh->io_port, mb_cmd, 5))
>  		shost_printk(KERN_ERR, sh, "failed setting up mailboxes\n");
>  	aha1542_intr_reset(sh->io_port);
> @@ -739,11 +765,26 @@ static struct Scsi_Host *aha1542_hw_init(struct scsi_host_template *tpnt, struct
>  	if (aha1542->bios_translation == BIOS_TRANSLATION_25563)
>  		shost_printk(KERN_INFO, sh, "Using extended bios translation\n");
>  
> +	if (dma_set_mask_and_coherent(pdev, DMA_BIT_MASK(24)) < 0)
> +		goto unregister;
> +
> +	aha1542->mb = dma_alloc_coherent(pdev,
> +			AHA1542_MAILBOXES * 2 * sizeof(struct mailbox),
> +			&aha1542->mb_handle, GFP_KERNEL);
> +	if (!aha1542->mb)
> +		goto unregister;
> +
> +	aha1542->ccb = dma_alloc_coherent(pdev,
> +			AHA1542_MAILBOXES * sizeof(struct ccb),
> +			&aha1542->ccb_handle, GFP_KERNEL);
> +	if (!aha1542->ccb)
> +		goto free_mb;
> +
>  	setup_mailboxes(sh);
>  
>  	if (request_irq(sh->irq, aha1542_interrupt, 0, "aha1542", sh)) {
>  		shost_printk(KERN_ERR, sh, "Unable to allocate IRQ.\n");
> -		goto unregister;
> +		goto free_ccb;
>  	}
>  	if (sh->dma_channel != 0xFF) {
>  		if (request_dma(sh->dma_channel, "aha1542")) {
> @@ -762,11 +803,18 @@ static struct Scsi_Host *aha1542_hw_init(struct scsi_host_template *tpnt, struct
>  	scsi_scan_host(sh);
>  
>  	return sh;
> +
>  free_dma:
>  	if (sh->dma_channel != 0xff)
>  		free_dma(sh->dma_channel);
>  free_irq:
>  	free_irq(sh->irq, sh);
> +free_ccb:
> +	dma_free_coherent(pdev, AHA1542_MAILBOXES * sizeof(struct ccb),
> +			  aha1542->ccb, aha1542->ccb_handle);
> +free_mb:
> +	dma_free_coherent(pdev, AHA1542_MAILBOXES * 2 * sizeof(struct mailbox),
> +			  aha1542->mb, aha1542->mb_handle);
>  unregister:
>  	scsi_host_put(sh);
>  release:
> @@ -777,9 +825,16 @@ static struct Scsi_Host *aha1542_hw_init(struct scsi_host_template *tpnt, struct
>  
>  static int aha1542_release(struct Scsi_Host *sh)
>  {
> +	struct aha1542_hostdata *aha1542 = shost_priv(sh);
> +	struct device *dev = sh->dma_dev;
> +
>  	scsi_remove_host(sh);
>  	if (sh->dma_channel != 0xff)
>  		free_dma(sh->dma_channel);
> +	dma_free_coherent(dev, AHA1542_MAILBOXES * sizeof(struct ccb),
> +			  aha1542->ccb, aha1542->ccb_handle);
> +	dma_free_coherent(dev, AHA1542_MAILBOXES * 2 * sizeof(struct mailbox),
> +			  aha1542->mb, aha1542->mb_handle);
>  	if (sh->irq)
>  		free_irq(sh->irq, sh);
>  	if (sh->io_port && sh->n_io_port)
> @@ -826,7 +881,8 @@ static int aha1542_dev_reset(struct scsi_cmnd *cmd)
>  
>  	aha1542->aha1542_last_mbo_used = mbo;
>  
> -	any2scsi(mb[mbo].ccbptr, isa_virt_to_bus(&ccb[mbo]));	/* This gets trashed for some reason */
> +	/* This gets trashed for some reason */
> +	any2scsi(mb[mbo].ccbptr, aha1542->ccb_handle + mbo * sizeof(ccb));
                                                                    ^^^
This looks wrong too. It's the same code as in aha1542_queuecommand.

>  
>  	memset(&ccb[mbo], 0, sizeof(struct ccb));
>  
> @@ -901,8 +957,7 @@ static int aha1542_reset(struct scsi_cmnd *cmd, u8 reset_cmd)
>  				 */
>  				continue;
>  			}
> -			kfree(tmp_cmd->host_scribble);
> -			tmp_cmd->host_scribble = NULL;
> +			aha1542_free_cmd(tmp_cmd);
>  			aha1542->int_cmds[i] = NULL;
>  			aha1542->mb[i].status = 0;
>  		}
> @@ -946,6 +1001,7 @@ static struct scsi_host_template driver_template = {
>  	.module			= THIS_MODULE,
>  	.proc_name		= "aha1542",
>  	.name			= "Adaptec 1542",
> +	.cmd_size		= sizeof(struct aha1542_cmd),
>  	.queuecommand		= aha1542_queuecommand,
>  	.eh_device_reset_handler= aha1542_dev_reset,
>  	.eh_bus_reset_handler	= aha1542_bus_reset,


-- 
Ondrej Zary



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux