+ 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.