Re: Prolific 2501 link cable

You should distribute this in the form of a patch ... and
name it something other than "plusb", which is the name
used by the (legacy-compatible) PL230x driver.

I'm glad to see some more code that interops with vendor
drivers on Win32, but you need to do it differently if
you want to submit this upstream.  You'd also need to
submit it to netdev, not the USB list...


On Friday 05 October 2007, Tobias wrote:
> /*
>  * PL-2301/2302/2501 USB host-to-host link cables
>  * Copyright (C) 2007 by Tobias Arp ([email protected])
>  *
>  *  This driver does now work with Windows driver from Prolific:
>  *    http://www.prolific.com.tw/support/files/sp_pl2502NW_v20044.zip
>  *
>  * Copyright (C) 2000-2005 by David Brownell

You did the wrong thing by modifying the existing pl230x code,
since your changes are incompatible with the existing driver.


>  *
>  * This program is free software; you can redistribute it and/or modify
>  * it under the terms of the GNU General Public License as published by
>  * the Free Software Foundation; either version 2 of the License, or
>  * (at your option) any later version.
>  *
>  * This program is distributed in the hope that it will be useful,
>  * but WITHOUT ANY WARRANTY; without even the implied warranty of
>  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>  * GNU General Public License for more details.
>  *
>  * You should have received a copy of the GNU General Public License
>  * along with this program; if not, write to the Free Software
>  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>  */
> 
> // #define      DEBUG                   // error path messages, extra info
> // #define      VERBOSE                 // more; success messages
> 
> #include <linux/config.h>
> #include <linux/module.h>
> #include <linux/sched.h>
> #include <linux/init.h>
> #include <linux/netdevice.h>
> #include <linux/etherdevice.h>
> #include <linux/ethtool.h>
> #include <linux/workqueue.h>
> #include <linux/mii.h>
> #include <linux/usb.h>
> 
> #include "usbnet.h"
> 
> /*
>  * Prolific PL-2301/PL-2302 driver ... http://www.prolifictech.com

Before you said "pl2501".  Here you say "pl2301".

>  *
>  * The protocol and handshaking used here should be bug-compatible
>  * with the Linux 2.2 "plusb" driver, by Deti Fliegl.

Clearly impossible if it interops with the PL2501 windows driver.
Which makes me think the comment below is wrong too...

>  *
>  * HEADS UP:  this handshaking isn't all that robust.  This driver
>  * gets confused easily if you unplug one end of the cable then
>  * try to connect it again; you'll need to restart both ends. The
>  * "naplink" software (used by some PlayStation/2 deveopers) does
>  * the handshaking much better!   Also, sometimes this hardware
>  * seems to get wedged under load.  Prolific docs are weak, and
>  * don't identify differences between PL2301 and PL2302, much less
>  * anything to explain the different PL2302 versions observed.
>  */
> 
> // max packet length
> #define PL_MAX_PACKET_LEN                       1520

Is this a hardware limitation?  If not, how is this computed?
I'd guess maybe some fuunction of ETH_FRAME_LEN + VLAN_HLEN...

> 
> /*
>  * Bits 0-4 can be used for software handshaking; they're set from
>  * one end, cleared from the other, "read" with the interrupt byte.
>  */
> #define PL_S_EN         (1<<7)          /* (feature only) suspend enable */
> /* reserved bit -- rx ready (6) ? */
> #define PL_TX_READY     (1<<5)          /* (interrupt only) transmit ready */
> #define PL_RESET_OUT    (1<<4)          /* reset output pipe */
> #define PL_RESET_IN     (1<<3)          /* reset input pipe */
> #define PL_TX_C         (1<<2)          /* transmission complete */
> #define PL_TX_REQ       (1<<1)          /* transmission received */
> #define PL_PEER_E       (1<<0)          /* peer exists */
> 
> static inline int
> pl_vendor_req(struct usbnet *dev, u8 req, u8 val, u8 index)
> {
>         return usb_control_msg(dev->udev,
>                 usb_rcvctrlpipe(dev->udev, 0),
>                 req,
>                 USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_INTERFACE,
>                 val, index,
>                 NULL, 0,
>                 USB_CTRL_SET_TIMEOUT);
> }
> 
> static inline int
> pl_clear_QuickLink_features(struct usbnet *dev, int val)
> {
>         return pl_vendor_req(dev, USB_REQ_CLEAR_FEATURE, (u8) val, 0);
> }
> 
> static inline int
> pl_set_QuickLink_features(struct usbnet *dev, int val)
> {
>         return pl_vendor_req(dev, USB_REQ_SET_FEATURE, (u8) val, 0);
> }
> 
> static int pl_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
> {
>         u32     iLen;

dontUseCamelCaseInLinux

>         iLen = le32_to_cpu(*((u32 *)skb->data));

This should probably be get_unaligned((u32*)...) so it
works on platforms that don't have the relevant hardware.

> 
>         if (iLen > PL_MAX_PACKET_LEN)
>         {

If you provided this as a patch, scripts/checkpatch.pl would
have told you this is wrong.  (Use "if (...) {" on one line.)

>                 dbg("pl: invalid received packet size: %d",iLen);

Use dev_dbg instead.

>                 return 0;
>         }
> 
>         skb_pull(skb,4);
> 
>         return 1;
> }
> 
> static struct sk_buff *pl_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
> {
>         int     padlen;
>         int     length = skb->len;
>         int     headroom = skb_headroom(skb);
>         int     tailroom = skb_tailroom(skb);
>         u32     *packet_len;
> 
>         // FIXME:  magic numbers, bleech

Why didn't you "fix" that then?

>         padlen = (skb->len + (4  % 64)) ? 0 : 1;

... which you changed later ...

And use "length" not "skb->len"; in case the compiler is less than
smart you'll save a memory access.


> 
>         if ((!skb_cloned(skb))
>                         && ((headroom + tailroom) >= (padlen + 4))) {
>                 if ((headroom < 4) || (tailroom < padlen)) {
>                         skb->data = memmove(skb->head + 4,
>                                              skb->data, skb->len);
>                         skb->tail = skb->data + skb->len;
>                 }
>         } else {
>                 struct sk_buff  *skb2;
>                 skb2 = skb_copy_expand(skb,4,padlen,flags);
>                 dev_kfree_skb_any(skb);
>                 skb = skb2;
>                 if (!skb)
>                         return NULL;
>         }
> 
>         // attach the packet count to the header
>         packet_len = (u32 *) skb_push(skb,4);
> 
>         *packet_len = cpu_to_le32(length);
> 
>         return skb;
> }
> 
> static int pl_reset(struct usbnet *dev)
> {
>         (void) pl_set_QuickLink_features(dev,PL_TX_REQ);
>         (void) pl_set_QuickLink_features(dev,PL_RESET_IN);
>         (void) pl_set_QuickLink_features(dev,PL_RESET_OUT);
> 
>         mdelay(50);     
> 
>         (void) pl_set_QuickLink_features(dev,PL_PEER_E);
> 
>         return 0;
> }
> 
> static const struct driver_info prolific_info = {
>         .description =  "Prolific PL-2301/PL-2302",
>         .flags =        FLAG_NO_SETINT,
>                 /* some PL-2302 versions seem to fail usb_set_interface() */


You said "PL-2501" in $SUBJECT ... 

>         .reset =        pl_reset,
>         .rx_fixup =     pl_rx_fixup,
>         .tx_fixup =     pl_tx_fixup,
> };
> 
> 
> /*-------------------------------------------------------------------------*/
> 
> /*
>  * Proilific's name won't normally be on the cables, and
>  * may not be on the device.
>  */
> 
> static const struct usb_device_id       products [] = {
> 
> {
>         USB_DEVICE(0x067b, 0x0000),     // PL-2301
>         .driver_info =  (unsigned long) &prolific_info,
> }, {
>         USB_DEVICE(0x067b, 0x0001),     // PL-2302
>         .driver_info =  (unsigned long) &prolific_info,

You shouldn't claim the 230x chips in this code, since you've
sacrificed compatibility with the original "plusb" driver.

Or if you do have a mode where you talk to 230x chips, that
would need to be optional, not the default.


> }, {
>         USB_DEVICE(0x067b, 0x2501),     // PL-2501
>         .driver_info =  (unsigned long) &prolific_info,
> },
>         { },            // END
> };
> MODULE_DEVICE_TABLE(usb, products);
> 
> static struct usb_driver plusb_driver = {
>         .name =         "plusb",
>         .id_table =     products,
>         .probe =        usbnet_probe,
>         .disconnect =   usbnet_disconnect,
>         .suspend =      usbnet_suspend,
>         .resume =       usbnet_resume,
> };
> 
> static int __init plusb_init(void)
> {
>         return usb_register(&plusb_driver);
> }
> module_init(plusb_init);
> 
> static void __exit plusb_exit(void)
> {
>         usb_deregister(&plusb_driver);
> }
> module_exit(plusb_exit);
> 
> MODULE_AUTHOR("Tobias Arp");
> MODULE_DESCRIPTION("Prolific PL-2301/2302/2501 USB Host to Host Link Driver");
> MODULE_LICENSE("GPL");



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel