> + sg = (struct scatterlist *)orb->cmd->request_buffer;
> + count = dma_map_sg(device->card->device, sg, orb->cmd->use_sg,
> + orb->cmd->sc_data_direction);
you need to handle the error case (count == 0)
> + /* Convert the scatterlist to an sbp2 page table. If any
> + * scatterlist entries are too big for sbp2 we split the as we go. */
Please set the max_sectors value in your host template so that the
block layer doesn't build sg entries too big for you.
> + orb->page_table_bus =
> + dma_map_single(device->card->device, orb->page_table,
> + size, DMA_TO_DEVICE);
This needs handling of mapping errors (dma_mapping_error())
> + orb = kzalloc(sizeof *orb, GFP_ATOMIC);
Normal kernel style is sizeof(*orb)
> + if (cmd->use_sg) {
> + sbp2_command_orb_map_scatterlist(orb);
> + } else if (cmd->request_bufflen > SBP2_MAX_SG_ELEMENT_LENGTH) {
> + /* FIXME: Need to split this into a sg list... but
> + * could we get the scsi or blk layer to do that by
> + * reporting our max supported block size? */
> + fw_error("command > 64k\n");
> + goto fail_bufflen;
> + } else if (cmd->request_bufflen > 0) {
> + sbp2_command_orb_map_buffer(orb);
> + }
The use_sg == 0, request_bufflen != 0 case can't happen anymore.
> + fail_mapping:
> + kfree(orb);
> + fail_alloc:
> + cmd->result = DID_ERROR << 16;
> + done(cmd);
Failure due to ressource shortage should not complete the command
but return SCSI_MLQUEUE_HOST_BUSY/SCSI_MLQUEUE_DEVICE_BUSY.
> + return 0;
> +}
> +static struct scsi_host_template scsi_driver_template = {
> + .module = THIS_MODULE,
> + .name = "SBP-2 IEEE-1394",
> + .proc_name = (char *)sbp2_driver_name,
Please don't use casrs here. Either fix up the definition so it
accepts const strings or pass a non-const one.
> +static int add_scsi_devices(struct fw_unit *unit)
> +{
> + struct sbp2_device *sd = unit->device.driver_data;
> + int retval, lun;
> +
> + if (sd->scsi_host != NULL)
> + return 0;
> +
> + sd->scsi_host = scsi_host_alloc(&scsi_driver_template,
> + sizeof(unsigned long));
> + if (sd->scsi_host == NULL) {
> + fw_error("failed to register scsi host\n");
> + return -1;
> + }
> +
> + sd->scsi_host->hostdata[0] = (unsigned long)unit;
Please take a look ar ther other scsi drivers how this is supposed
to be used.
> + retval = scsi_add_host(sd->scsi_host, &unit->device);
> + if (retval < 0) {
> + fw_error("failed to add scsi host\n");
> + scsi_host_put(sd->scsi_host);
> + sd->scsi_host = NULL;
> + return retval;
> + }
> +
> + /* FIXME: Loop over luns here. */
> + lun = 0;
> + retval = scsi_add_device(sd->scsi_host, 0, 0, lun);
> + if (retval < 0) {
> + fw_error("failed to add scsi device\n");
> + scsi_remove_host(sd->scsi_host);
> + scsi_host_put(sd->scsi_host);
> + sd->scsi_host = NULL;
> + return retval;
> + }
> +
> + return 0;
> +}
Do we really need another scanning algorithm? Can't you use
scsi_scan_target instead and let the core scsi code handle the
scanning?
> +
> +static void remove_scsi_devices(struct fw_unit *unit)
> +{
> + struct sbp2_device *sd = unit->device.driver_data;
> +
> + if (sd->scsi_host != NULL) {
> + scsi_remove_host(sd->scsi_host);
> + scsi_host_put(sd->scsi_host);
> + }
> + sd->scsi_host = NULL;
> +}
This function seems rather oddly named. And the checking and
setting of scsi_host looks like you have some lifetime rule
problems.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]