Shouldn't we use pci_alloc_consistent (or indirectly via pci_pool_alloc) instead of kmalloc?
-ch > -----Original Message----- > From: [EMAIL PROTECTED] > [mailto:[EMAIL PROTECTED]] On > Behalf Of Roland Dreier > Sent: Friday, June 07, 2002 4:57 PM > To: [EMAIL PROTECTED]; > [EMAIL PROTECTED] > Subject: [linux-usb-devel] USB on PPC440GP (cache incoherent) > Importance: High > > > I just got USB working on my PPC440GP (IBM Ebony eval board). > I'm using an Opti 82C861 OHCI controller, and so far I've > tried an Alcor SD card reader (mass storage) and a Belkin > usbnet device. My kernel is 2.4.19-pre10 from the > bk://ppc.bkbits.net/linuxppc_2_4_devel > BitKeeper tree. > > I had to make some changes to the USB driver to get this > working as there are still some places where structures on > the stack are being used for DMA. This causes problems on > processors like the 440GP, which are not cache coherent (you > can only invalidate a whole cache line, which can corrupt the > stack). I just changed all of these places to use kmalloc to > get memory instead. > > Note that this might not work perfectly on all > cache-incoherent processors, since kmalloc could potentially > allocate a chunk of memory that is smaller than the > processor's cache line size. However it is safe on the 440GP > since the 440GP's cache line size is 32 bytes. > > I've included a patch with my changes in this email. I'm > sending it to the linuxppc-embedded list in case someone else > is trying to do USB on a 440 or 405 or similar processor. > I'm also sending it to linux-usb-devel in the hope that these > changes make it into the mainline kernel. > > Best, > Roland > > # This is a BitKeeper generated patch for the following > project: # Project Name: Linux 2.4 for PowerPC development > tree # This patch format is intended for GNU patch command > version 2.5 or higher. # This patch includes the following deltas: > # ChangeSet 1.1077 -> 1.1078 > # drivers/usb/usb.c 1.19 -> 1.20 > # drivers/usb/hub.c 1.14 -> 1.15 > # drivers/usb/hub.h 1.7 -> 1.8 > # drivers/usb/storage/transport.c 1.10 -> 1.11 > # > # The following is the BitKeeper ChangeSet Log > # -------------------------------------------- > # 02/06/07 [EMAIL PROTECTED] 1.1078 > # Fix problems in USB when running on cache-incoherent cpus > like PPC440GP # (Don't do DMAs into memory allocated on > stack) # -------------------------------------------- > # > diff -Nru a/drivers/usb/hub.c b/drivers/usb/hub.c > --- a/drivers/usb/hub.c Fri Jun 7 16:35:31 2002 > +++ b/drivers/usb/hub.c Fri Jun 7 16:35:31 2002 > @@ -155,7 +155,7 @@ > static int usb_hub_configure(struct usb_hub *hub, struct > usb_endpoint_descriptor *endpoint) { > struct usb_device *dev = hub->dev; > - struct usb_hub_status hubstatus; > + struct usb_hub_status *hubstatus; > char portstr[USB_MAXCHILDREN + 1]; > unsigned int pipe; > int i, maxp, ret; > @@ -258,27 +258,36 @@ > > dbg("port removable status: %s", portstr); > > - ret = usb_get_hub_status(dev, &hubstatus); > + hubstatus = kmalloc(sizeof *hubstatus, GFP_KERNEL); > + if (!hubstatus) { > + err("Unable to allocate hubstatus"); > + kfree(hub->descriptor); > + return -1; > + } > + ret = usb_get_hub_status(dev, hubstatus); > if (ret < 0) { > err("Unable to get hub status (err = %d)", ret); > + kfree(hubstatus); > kfree(hub->descriptor); > return -1; > } > > - le16_to_cpus(&hubstatus.wHubStatus); > + le16_to_cpus(&hubstatus->wHubStatus); > > dbg("local power source is %s", > - (hubstatus.wHubStatus & HUB_STATUS_LOCAL_POWER) > ? "lost (inactive)" : "good"); > + (hubstatus->wHubStatus & > HUB_STATUS_LOCAL_POWER) ? "lost (inactive)" > +: "good"); > > dbg("%sover-current condition exists", > - (hubstatus.wHubStatus & HUB_STATUS_OVERCURRENT) > ? "" : "no "); > + (hubstatus->wHubStatus & > HUB_STATUS_OVERCURRENT) ? "" : "no "); > + > + kfree(hubstatus); > > /* Start the interrupt endpoint */ > pipe = usb_rcvintpipe(dev, endpoint->bEndpointAddress); > maxp = usb_maxpacket(dev, pipe, usb_pipeout(pipe)); > > - if (maxp > sizeof(hub->buffer)) > - maxp = sizeof(hub->buffer); > + if (maxp > USB_HUB_BUFFER_SIZE) > + maxp = USB_HUB_BUFFER_SIZE; > > hub->urb = usb_alloc_urb(0); > if (!hub->urb) { > @@ -357,6 +366,13 @@ > > memset(hub, 0, sizeof(*hub)); > > + hub->buffer = kmalloc(USB_HUB_BUFFER_SIZE, GFP_KERNEL); > + if (!hub->buffer) { > + err("couldn't kmalloc hub->buffer"); > + kfree(hub); > + return NULL; > + } > + > INIT_LIST_HEAD(&hub->event_list); > hub->dev = dev; > init_MUTEX(&hub->khubd_sem); > @@ -383,6 +399,7 @@ > > spin_unlock_irqrestore(&hub_event_lock, flags); > > + kfree(hub->buffer); > kfree(hub); > > return NULL; > @@ -417,6 +434,11 @@ > hub->descriptor = NULL; > } > > + if (hub->buffer) { > + kfree(hub->buffer); > + hub->buffer = NULL; > + } > + > /* Free the memory */ > kfree(hub); > } > @@ -782,7 +804,7 @@ > struct list_head *tmp; > struct usb_device *dev; > struct usb_hub *hub; > - struct usb_hub_status hubsts; > + struct usb_hub_status *hubsts; > u16 hubstatus; > u16 hubchange; > u16 portstatus; > @@ -872,22 +894,28 @@ > } /* end for i */ > > /* deal with hub status changes */ > - if (usb_get_hub_status(dev, &hubsts) < 0) > - err("get_hub_status failed"); > - else { > - hubstatus = le16_to_cpup(&hubsts.wHubStatus); > - hubchange = le16_to_cpup(&hubsts.wHubChange); > - if (hubchange & HUB_CHANGE_LOCAL_POWER) { > - dbg("hub power change"); > - usb_clear_hub_feature(dev, > C_HUB_LOCAL_POWER); > - } > - if (hubchange & HUB_CHANGE_OVERCURRENT) { > - dbg("hub overcurrent change"); > - wait_ms(500); /* Cool down */ > - usb_clear_hub_feature(dev, > C_HUB_OVER_CURRENT); > - usb_hub_power_on(hub); > - } > + hubsts = kmalloc(sizeof *hubsts, GFP_KERNEL); > + if (!hubsts) { > + err("couldn't allocate hubsts"); > + } else { > + if (usb_get_hub_status(dev, hubsts) < 0) > + err("get_hub_status failed"); > + else { > + hubstatus = > le16_to_cpup(&hubsts->wHubStatus); > + hubchange = > le16_to_cpup(&hubsts->wHubChange); > + if (hubchange & > HUB_CHANGE_LOCAL_POWER) { > + dbg("hub power change"); > + > usb_clear_hub_feature(dev, C_HUB_LOCAL_POWER); > + } > + if (hubchange & > HUB_CHANGE_OVERCURRENT) { > + dbg("hub overcurrent > change"); > + wait_ms(500); > /* Cool down */ > + > usb_clear_hub_feature(dev, C_HUB_OVER_CURRENT); > + usb_hub_power_on(hub); > + } > + } > } > + kfree(hubsts); > up(&hub->khubd_sem); > } /* end while (1) */ > > @@ -995,7 +1023,7 @@ > int usb_reset_device(struct usb_device *dev) > { > struct usb_device *parent = dev->parent; > - struct usb_device_descriptor descriptor; > + struct usb_device_descriptor *descriptor; > int i, ret, port = -1; > > if (!parent) { > @@ -1044,17 +1072,19 @@ > * If nothing changed, we reprogram the configuration and then > * the alternate settings. > */ > - ret = usb_get_descriptor(dev, USB_DT_DEVICE, 0, &descriptor, > - sizeof(descriptor)); > + descriptor = kmalloc(sizeof *descriptor, GFP_KERNEL); > + ret = usb_get_descriptor(dev, USB_DT_DEVICE, 0, descriptor, > + sizeof *descriptor); > if (ret < 0) > return ret; > > - le16_to_cpus(&descriptor.bcdUSB); > - le16_to_cpus(&descriptor.idVendor); > - le16_to_cpus(&descriptor.idProduct); > - le16_to_cpus(&descriptor.bcdDevice); > + le16_to_cpus(&descriptor->bcdUSB); > + le16_to_cpus(&descriptor->idVendor); > + le16_to_cpus(&descriptor->idProduct); > + le16_to_cpus(&descriptor->bcdDevice); > > - if (memcmp(&dev->descriptor, &descriptor, sizeof(descriptor))) { > + if (memcmp(&dev->descriptor, descriptor, sizeof *descriptor)) { > + kfree(descriptor); > usb_destroy_configuration(dev); > > ret = usb_get_device_descriptor(dev); > @@ -1083,6 +1113,7 @@ > > return 1; > } > + kfree(descriptor); > > ret = usb_set_configuration(dev, > dev->actconfig->bConfigurationValue); > if (ret < 0) { > diff -Nru a/drivers/usb/hub.h b/drivers/usb/hub.h > --- a/drivers/usb/hub.h Fri Jun 7 16:35:31 2002 > +++ b/drivers/usb/hub.h Fri Jun 7 16:35:31 2002 > @@ -120,13 +120,15 @@ > > struct usb_device; > > +/* add 1 bit for hub status change and add 7 bits to round > up to byte > +boundary */ #define USB_HUB_BUFFER_SIZE ((USB_MAXCHILDREN + > 1 + 7) / 8) > + > struct usb_hub { > struct usb_device *dev; > > struct urb *urb; /* Interrupt polling pipe */ > > - char buffer[(USB_MAXCHILDREN + 1 + 7) / 8]; /* add 1 > bit for hub status change */ > - /* and add 7 bits to > round up to byte boundary */ > + char *buffer; > int error; > int nerrors; > > diff -Nru a/drivers/usb/storage/transport.c > b/drivers/usb/storage/transport.c > --- a/drivers/usb/storage/transport.c Fri Jun 7 16:35:31 2002 > +++ b/drivers/usb/storage/transport.c Fri Jun 7 16:35:31 2002 > @@ -723,7 +723,7 @@ > > /* use the new buffer we have */ > old_request_buffer = srb->request_buffer; > - srb->request_buffer = srb->sense_buffer; > + srb->request_buffer = kmalloc(18, > in_interrupt() ? GFP_ATOMIC : > +GFP_KERNEL); > > /* set the buffer length for transfer */ > old_request_bufflen = srb->request_bufflen; > @@ -733,8 +733,14 @@ > old_sg = srb->use_sg; > srb->use_sg = 0; > > - /* issue the auto-sense command */ > - temp_result = us->transport(us->srb, us); > + if (srb->request_buffer) { > + /* issue the auto-sense command */ > + temp_result = us->transport(us->srb, us); > + memcpy(srb->sense_buffer, > srb->request_buffer, 18); > + kfree(srb->request_buffer); > + } else { > + temp_result = USB_STOR_TRANSPORT_ERROR; > + } > > /* let's clean up right away */ > srb->request_buffer = old_request_buffer; > @@ -1041,9 +1047,12 @@ > int usb_stor_Bulk_max_lun(struct us_data *us) > { > unsigned char data; > + unsigned char *buffer; > int result; > int pipe; > > + buffer = kmalloc(sizeof data, GFP_KERNEL); > + > /* issue the command -- use usb_control_msg() because > * the state machine is not yet alive */ > pipe = usb_rcvctrlpipe(us->pusb_dev, 0); > @@ -1051,7 +1060,9 @@ > US_BULK_GET_MAX_LUN, > USB_DIR_IN | USB_TYPE_CLASS | > USB_RECIP_INTERFACE, > - 0, us->ifnum, &data, sizeof(data), HZ); > + 0, us->ifnum, buffer, > sizeof(data), HZ); > + data = *buffer; > + kfree(buffer); > > US_DEBUGP("GetMaxLUN command result is %d, data is %d\n", > result, data); > @@ -1077,41 +1088,54 @@ > > int usb_stor_Bulk_transport(Scsi_Cmnd *srb, struct us_data *us) { > - struct bulk_cb_wrap bcb; > - struct bulk_cs_wrap bcs; > + struct bulk_cb_wrap *bcb; > + struct bulk_cs_wrap *bcs; > int result; > int pipe; > int partial; > + int ret = USB_STOR_TRANSPORT_ERROR; > + > + bcb = kmalloc(sizeof *bcb, in_interrupt() ? > GFP_ATOMIC : GFP_KERNEL); > + if (!bcb) { > + return USB_STOR_TRANSPORT_ERROR; > + } > + bcs = kmalloc(sizeof *bcs, in_interrupt() ? > GFP_ATOMIC : GFP_KERNEL); > + if (!bcs) { > + kfree(bcb); > + return USB_STOR_TRANSPORT_ERROR; > + } > > /* set up the command wrapper */ > - bcb.Signature = cpu_to_le32(US_BULK_CB_SIGN); > - bcb.DataTransferLength = > cpu_to_le32(usb_stor_transfer_length(srb)); > - bcb.Flags = srb->sc_data_direction == SCSI_DATA_READ ? > 1 << 7 : 0; > - bcb.Tag = srb->serial_number; > - bcb.Lun = srb->cmnd[1] >> 5; > + bcb->Signature = cpu_to_le32(US_BULK_CB_SIGN); > + bcb->DataTransferLength = > cpu_to_le32(usb_stor_transfer_length(srb)); > + bcb->Flags = srb->sc_data_direction == SCSI_DATA_READ ? > 1 << 7 : 0; > + bcb->Tag = srb->serial_number; > + bcb->Lun = srb->cmnd[1] >> 5; > if (us->flags & US_FL_SCM_MULT_TARG) > - bcb.Lun |= srb->target << 4; > - bcb.Length = srb->cmd_len; > + bcb->Lun |= srb->target << 4; > + bcb->Length = srb->cmd_len; > > /* construct the pipe handle */ > pipe = usb_sndbulkpipe(us->pusb_dev, us->ep_out); > > /* copy the command payload */ > - memset(bcb.CDB, 0, sizeof(bcb.CDB)); > - memcpy(bcb.CDB, srb->cmnd, bcb.Length); > + memset(bcb->CDB, 0, sizeof(bcb->CDB)); > + memcpy(bcb->CDB, srb->cmnd, bcb->Length); > > /* send it to out endpoint */ > US_DEBUGP("Bulk command S 0x%x T 0x%x Trg %d LUN %d L > %d F %d CL %d\n", > - le32_to_cpu(bcb.Signature), bcb.Tag, > - (bcb.Lun >> 4), (bcb.Lun & 0x0F), > - bcb.DataTransferLength, bcb.Flags, bcb.Length); > - result = usb_stor_bulk_msg(us, &bcb, pipe, US_BULK_CB_WRAP_LEN, > + le32_to_cpu(bcb->Signature), bcb->Tag, > + (bcb->Lun >> 4), (bcb->Lun & 0x0F), > + bcb->DataTransferLength, bcb->Flags, bcb->Length); > + result = usb_stor_bulk_msg(us, bcb, pipe, US_BULK_CB_WRAP_LEN, > &partial); > US_DEBUGP("Bulk command transfer result=%d\n", result); > > /* if the command was aborted, indicate that */ > - if (result == -ENOENT) > - return USB_STOR_TRANSPORT_ABORTED; > + if (result == -ENOENT) { > + ret = USB_STOR_TRANSPORT_ABORTED; > + goto out; > + } > > /* if we stall, we need to clear it before we go on */ > if (result == -EPIPE) { > @@ -1119,25 +1143,30 @@ > result = usb_stor_clear_halt(us, pipe); > > /* if the command was aborted, indicate that */ > - if (result == -ENOENT) > - return USB_STOR_TRANSPORT_ABORTED; > + if (result == -ENOENT) { > + ret = USB_STOR_TRANSPORT_ABORTED; > + goto out; > + } > result = -EPIPE; > } else if (result) { > /* unknown error -- we've got a problem */ > - return USB_STOR_TRANSPORT_ERROR; > + ret = USB_STOR_TRANSPORT_ERROR; > + goto out; > } > > /* if the command transfered well, then we go to the > data stage */ > if (result == 0) { > /* send/receive data payload, if there is any */ > - if (bcb.DataTransferLength) { > + if (bcb->DataTransferLength) { > usb_stor_transfer(srb, us); > result = srb->result; > US_DEBUGP("Bulk data transfer result > 0x%x\n", result); > > /* if it was aborted, we need to > indicate that */ > - if (result == US_BULK_TRANSFER_ABORTED) > - return USB_STOR_TRANSPORT_ABORTED; > + if (result == US_BULK_TRANSFER_ABORTED) { > + ret = USB_STOR_TRANSPORT_ABORTED; > + goto out; > + } > } > } > > @@ -1150,12 +1179,14 @@ > > /* get CSW for device status */ > US_DEBUGP("Attempting to get CSW...\n"); > - result = usb_stor_bulk_msg(us, &bcs, pipe, US_BULK_CS_WRAP_LEN, > + result = usb_stor_bulk_msg(us, bcs, pipe, US_BULK_CS_WRAP_LEN, > &partial); > > /* if the command was aborted, indicate that */ > - if (result == -ENOENT) > - return USB_STOR_TRANSPORT_ABORTED; > + if (result == -ENOENT) { > + ret = USB_STOR_TRANSPORT_ABORTED; > + goto out; > + } > > /* did the attempt to read the CSW fail? */ > if (result == -EPIPE) { > @@ -1163,17 +1194,21 @@ > result = usb_stor_clear_halt(us, pipe); > > /* if the command was aborted, indicate that */ > - if (result == -ENOENT) > - return USB_STOR_TRANSPORT_ABORTED; > + if (result == -ENOENT) { > + ret = USB_STOR_TRANSPORT_ABORTED; > + goto out; > + } > > /* get the status again */ > US_DEBUGP("Attempting to get CSW (2nd try)...\n"); > - result = usb_stor_bulk_msg(us, &bcs, pipe, > + result = usb_stor_bulk_msg(us, bcs, pipe, > US_BULK_CS_WRAP_LEN, > &partial); > > /* if the command was aborted, indicate that */ > - if (result == -ENOENT) > - return USB_STOR_TRANSPORT_ABORTED; > + if (result == -ENOENT) { > + ret = USB_STOR_TRANSPORT_ABORTED; > + goto out; > + } > > /* if it fails again, we need a reset and > return an error*/ > if (result == -EPIPE) { > @@ -1181,48 +1216,60 @@ > result = usb_stor_clear_halt(us, pipe); > > /* if the command was aborted, indicate that */ > - if (result == -ENOENT) > - return USB_STOR_TRANSPORT_ABORTED; > - return USB_STOR_TRANSPORT_ERROR; > + if (result == -ENOENT) { > + ret = USB_STOR_TRANSPORT_ABORTED; > + goto out; > + } > + ret = USB_STOR_TRANSPORT_ERROR; > + goto out; > } > } > > /* if we still have a failure at this point, we're in trouble */ > US_DEBUGP("Bulk status result = %d\n", result); > if (result) { > - return USB_STOR_TRANSPORT_ERROR; > + ret = USB_STOR_TRANSPORT_ERROR; > + goto out; > } > > /* check bulk status */ > US_DEBUGP("Bulk status Sig 0x%x T 0x%x R %d Stat 0x%x\n", > - le32_to_cpu(bcs.Signature), bcs.Tag, > - bcs.Residue, bcs.Status); > - if (bcs.Signature != cpu_to_le32(US_BULK_CS_SIGN) || > - bcs.Tag != bcb.Tag || > - bcs.Status > US_BULK_STAT_PHASE || partial != 13) { > + le32_to_cpu(bcs->Signature), bcs->Tag, > + bcs->Residue, bcs->Status); > + if (bcs->Signature != cpu_to_le32(US_BULK_CS_SIGN) || > + bcs->Tag != bcb->Tag || > + bcs->Status > US_BULK_STAT_PHASE || partial != 13) { > US_DEBUGP("Bulk logical error\n"); > - return USB_STOR_TRANSPORT_ERROR; > + ret = USB_STOR_TRANSPORT_ERROR; > + goto out; > } > > /* based on the status code, we report good or bad */ > - switch (bcs.Status) { > + switch (bcs->Status) { > case US_BULK_STAT_OK: > /* command good -- note that data could > be short */ > - return USB_STOR_TRANSPORT_GOOD; > + ret = USB_STOR_TRANSPORT_GOOD; > + goto out; > > case US_BULK_STAT_FAIL: > /* command failed */ > - return USB_STOR_TRANSPORT_FAILED; > + ret = USB_STOR_TRANSPORT_FAILED; > + goto out; > > case US_BULK_STAT_PHASE: > /* phase error -- note that a transport > reset will be > * invoked by the invoke_transport() function > */ > - return USB_STOR_TRANSPORT_ERROR; > + ret = USB_STOR_TRANSPORT_ERROR; > + goto out; > } > > /* we should never get here, but if we do, we're in trouble */ > - return USB_STOR_TRANSPORT_ERROR; > + > + out: > + kfree(bcb); > + kfree(bcs); > + return ret; > } > > > /************************************************************* > ********** > diff -Nru a/drivers/usb/usb.c b/drivers/usb/usb.c > --- a/drivers/usb/usb.c Fri Jun 7 16:35:31 2002 > +++ b/drivers/usb/usb.c Fri Jun 7 16:35:31 2002 > @@ -1787,16 +1787,23 @@ > { > int i = 5; > int result; > + void *tmp_buf; > > - memset(buf,0,size); // Make sure we parse really > received data > + tmp_buf = kmalloc(size, in_interrupt() ? GFP_ATOMIC > : GFP_KERNEL); > + if (!tmp_buf) { > + return -ENOMEM; > + } > + memset(tmp_buf,0,size); // Make sure we parse really > received data > > while (i--) { > if ((result = usb_control_msg(dev, > usb_rcvctrlpipe(dev, 0), > USB_REQ_GET_DESCRIPTOR, USB_DIR_IN, > - (type << 8) + index, 0, buf, size, HZ * > GET_TIMEOUT)) > 0 || > + (type << 8) + index, 0, tmp_buf, size, > HZ * GET_TIMEOUT)) > 0 || > result == -EPIPE) > break; /* retry if the returned length > was 0; flaky device */ > } > + memcpy(buf, tmp_buf, size); > + kfree(tmp_buf); > return result; > } > > > _______________________________________________________________ > > Don't miss the 2002 Sprint PCS Application Developer's > Conference August 25-28 in Las Vegas - > http://devcon.sprintpcs.com/adp/index.cfm?> source=osdntextlink > > > _______________________________________________ > [EMAIL PROTECTED] > To unsubscribe, use the last form field at: > https://lists.sourceforge.net/lists/listinfo/l> inux-usb-devel > _______________________________________________________________ Don't miss the 2002 Sprint PCS Application Developer's Conference August 25-28 in Las Vegas - http://devcon.sprintpcs.com/adp/index.cfm?source=osdntextlink _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
