On Wed, 28 Mar 2007, Tony Lindgren wrote:

> OK, attached is take #3 with ifndef CONFIG_HIGHMEM. Anybody have other
> limitations in mind?

> +             /*
> +              * Some systems need both transfer_buffer and transfer_dma
> +              * to revert to PIO if DMA is temporarily unavailable.
> +              * Note that transfer_buffer may not be accessible on some
> +              * systems. In that case there is no need to use it either.
> +              */

It would be nice if this comment were more grammatical.  Consider this 
instead:

                /*
                 * Some systems can revert to PIO if DMA is temporarily
                 * unavailable; for their sake both transfer_buffer and
                 * transfer_dma must be set properly.  However this is
                 * possible only on systems with no high memory, since
                 * DMA buffers located in high memory are not directly
                 * addressable by the kernel for PIO.  When high memory
                 * is available, we NULL out transfer_buffer to avoid
                 * keeping a stale pointer and to help spot possible
                 * addressing bugs.
                 */

>               if (dma) {
> -                     /* hc may use _only_ transfer_dma */
>                       io->urbs [i]->transfer_dma = sg_dma_address (sg + i);
>                       len = sg_dma_len (sg + i);
> -             } else {
> -                     /* hc may use _only_ transfer_buffer */
> +#ifndef CONFIG_HIGHMEM
>                       io->urbs [i]->transfer_buffer =
>                               page_address (sg [i].page) + sg [i].offset;

Insert here:

+#else
+                       io->urbs[i]->transfer_buffer = NULL;

> +#endif
> +             } else {
>                       len = sg [i].length;
> +                     io->urbs [i]->transfer_buffer =
> +                             page_address (sg [i].page) + sg [i].offset;
>               }

Don't duplicate the old non-approved spacing style in new code.  This
should say

+                       io->urbs[i]->transfer_buffer =
+                               page_address(sg[i].page) + sg[i].offset;

If you like, you can change the other occurrence of this to match.

Alan Stern


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to