[PATCH 1/7] docs: driver-api: parport-lowlevel: clarify purpose of PARPORT_MODE_PCSPP

[PATCH 1/7] docs: driver-api: parport-lowlevel: clarify purpose of PARPORT_MODE_PCSPP

[Alex Henrie]

Link: http://lists.infradead.org/pipermail/linux-parport/2024-February/001237.html
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 Documentation/driver-api/parport-lowlevel.rst | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/driver-api/parport-lowlevel.rst b/Documentation/driver-api/parport-lowlevel.rst
index 0633d70ffda7..aaf02cf62363 100644
--- a/Documentation/driver-api/parport-lowlevel.rst
+++ b/Documentation/driver-api/parport-lowlevel.rst
@@ -155,10 +155,11 @@ hardware.  It consists of flags which may be bitwise-ored together:
 
   ============================= ===============================================
   PARPORT_MODE_PCSPP		IBM PC registers are available,
-				i.e. functions that act on data,
-				control and status registers are
-				probably writing directly to the
-				hardware.
+				i.e. ``base`` (and ``base_hi`` if ECP)
+				in ``struct parport`` is valid.
+				Functions that act on data, control
+				and status registers are probably
+				writing directly to the hardware.
   PARPORT_MODE_TRISTATE		The data drivers may be turned off.
 				This allows the data lines to be used
 				for reverse (peripheral to host)
-- 
2.44.0

[PATCH 2/7] usb: serial: mos7720: don't advertise PARPORT_MODE_PCSPP

[Alex Henrie]

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 drivers/usb/serial/mos7720.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c
index 23544074eb1c..0e97def80e19 100644
--- a/drivers/usb/serial/mos7720.c
+++ b/drivers/usb/serial/mos7720.c
@@ -589,7 +589,7 @@ static int mos7715_parport_init(struct usb_serial *serial)
 		return -EIO;
 	}
 	mos_parport->pp->private_data = mos_parport;
-	mos_parport->pp->modes = PARPORT_MODE_COMPAT | PARPORT_MODE_PCSPP;
+	mos_parport->pp->modes = PARPORT_MODE_COMPAT;
 	mos_parport->pp->dev = &serial->interface->dev;
 	parport_announce_port(mos_parport->pp);
 
-- 
2.44.0

[PATCH 3/7] usb: misc: uss720: don't advertise PARPORT_MODE_PCSPP

[Alex Henrie]

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 drivers/usb/misc/uss720.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/misc/uss720.c b/drivers/usb/misc/uss720.c
index b00d92db5dfd..fadceb7ab34d 100644
--- a/drivers/usb/misc/uss720.c
+++ b/drivers/usb/misc/uss720.c
@@ -719,7 +719,7 @@ static int uss720_probe(struct usb_interface *intf,
 
 	priv->pp = pp;
 	pp->private_data = priv;
-	pp->modes = PARPORT_MODE_PCSPP | PARPORT_MODE_TRISTATE | PARPORT_MODE_EPP | PARPORT_MODE_ECP | PARPORT_MODE_COMPAT;
+	pp->modes = PARPORT_MODE_TRISTATE | PARPORT_MODE_EPP | PARPORT_MODE_ECP | PARPORT_MODE_COMPAT;
 
 	/* set the USS720 control register to manual mode, no ECP compression, enable all ints */
 	set_1284_register(pp, 7, 0x00, GFP_KERNEL);
-- 
2.44.0

[PATCH 4/7] usb: misc: uss720: point pp->dev to usbdev->dev

[Alex Henrie]

This avoids a "fix this legacy no-device port driver" warning.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 drivers/usb/misc/uss720.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/misc/uss720.c b/drivers/usb/misc/uss720.c
index fadceb7ab34d..be7d3fa4c150 100644
--- a/drivers/usb/misc/uss720.c
+++ b/drivers/usb/misc/uss720.c
@@ -720,6 +720,7 @@ static int uss720_probe(struct usb_interface *intf,
 	priv->pp = pp;
 	pp->private_data = priv;
 	pp->modes = PARPORT_MODE_TRISTATE | PARPORT_MODE_EPP | PARPORT_MODE_ECP | PARPORT_MODE_COMPAT;
+	pp->dev = &usbdev->dev;
 
 	/* set the USS720 control register to manual mode, no ECP compression, enable all ints */
 	set_1284_register(pp, 7, 0x00, GFP_KERNEL);
-- 
2.44.0

[PATCH 5/7] usb: misc: uss720: document the names of the compatible devices

[Alex Henrie]

Information about 04b8:0002 and 05ab:0002 is from commit e3cb7bde9a6a
("USB: misc: uss720: more vendor/product ID's", 2018-03-20). The rest
are from <http://reboots.g-cipher.net/lcd/>.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 drivers/usb/misc/uss720.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/misc/uss720.c b/drivers/usb/misc/uss720.c
index be7d3fa4c150..15cafc7dfd22 100644
--- a/drivers/usb/misc/uss720.c
+++ b/drivers/usb/misc/uss720.c
@@ -767,14 +767,14 @@ static void uss720_disconnect(struct usb_interface *intf)
 
 /* table of cables that work through this driver */
 static const struct usb_device_id uss720_table[] = {
-	{ USB_DEVICE(0x047e, 0x1001) },
-	{ USB_DEVICE(0x04b8, 0x0002) },
-	{ USB_DEVICE(0x04b8, 0x0003) },
+	{ USB_DEVICE(0x047e, 0x1001) }, /* Infowave 901-0030 */
+	{ USB_DEVICE(0x04b8, 0x0002) }, /* Epson CAEUL0002 ISD-103 */
+	{ USB_DEVICE(0x04b8, 0x0003) }, /* Epson ISD-101 */
 	{ USB_DEVICE(0x050d, 0x0002) },
-	{ USB_DEVICE(0x050d, 0x1202) },
+	{ USB_DEVICE(0x050d, 0x1202) }, /* Belkin F5U120-PC */
 	{ USB_DEVICE(0x0557, 0x2001) },
-	{ USB_DEVICE(0x05ab, 0x0002) },
-	{ USB_DEVICE(0x06c6, 0x0100) },
+	{ USB_DEVICE(0x05ab, 0x0002) }, /* Belkin F5U002 ISD-101 */
+	{ USB_DEVICE(0x06c6, 0x0100) }, /* Infowave ISD-103 */
 	{ USB_DEVICE(0x0729, 0x1284) },
 	{ USB_DEVICE(0x1293, 0x0002) },
 	{ }						/* Terminating entry */
-- 
2.44.0

[PATCH 6/7] usb: misc: uss720: add support for another variant of the Belkin F5U002

[Alex Henrie]

This device is a gray USB parallel port adapter with "F5U002" imprinted
on the plastic and a sticker that says both "F5U002" and "P80453-A".
It's identified in lsusb as "05ab:1001 In-System Design BAYI Printer
Class Support". It's subtly different from the other gray cable I have
that has "F5U002 Rev 2" and "P80453-B" on its sticker and device ID
050d:0002.

The uss720 driver appears to work flawlessly with this device, although
the device evidently does not support ECP.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 drivers/usb/misc/uss720.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/misc/uss720.c b/drivers/usb/misc/uss720.c
index 15cafc7dfd22..5803919d7cc4 100644
--- a/drivers/usb/misc/uss720.c
+++ b/drivers/usb/misc/uss720.c
@@ -693,7 +693,7 @@ static int uss720_probe(struct usb_interface *intf,
 
 	interface = intf->cur_altsetting;
 
-	if (interface->desc.bNumEndpoints < 3) {
+	if (interface->desc.bNumEndpoints < 2) {
 		usb_put_dev(usbdev);
 		return -ENODEV;
 	}
@@ -719,7 +719,9 @@ static int uss720_probe(struct usb_interface *intf,
 
 	priv->pp = pp;
 	pp->private_data = priv;
-	pp->modes = PARPORT_MODE_TRISTATE | PARPORT_MODE_EPP | PARPORT_MODE_ECP | PARPORT_MODE_COMPAT;
+	pp->modes = PARPORT_MODE_TRISTATE | PARPORT_MODE_EPP | PARPORT_MODE_COMPAT;
+	if (interface->desc.bNumEndpoints >= 3)
+		pp->modes |= PARPORT_MODE_ECP;
 	pp->dev = &usbdev->dev;
 
 	/* set the USS720 control register to manual mode, no ECP compression, enable all ints */
@@ -774,6 +776,7 @@ static const struct usb_device_id uss720_table[] = {
 	{ USB_DEVICE(0x050d, 0x1202) }, /* Belkin F5U120-PC */
 	{ USB_DEVICE(0x0557, 0x2001) },
 	{ USB_DEVICE(0x05ab, 0x0002) }, /* Belkin F5U002 ISD-101 */
+	{ USB_DEVICE(0x05ab, 0x1001) }, /* Belkin F5U002 P80453-A */
 	{ USB_DEVICE(0x06c6, 0x0100) }, /* Infowave ISD-103 */
 	{ USB_DEVICE(0x0729, 0x1284) },
 	{ USB_DEVICE(0x1293, 0x0002) },
-- 
2.44.0

[PATCH 7/7] usb: misc: uss720: check for incompatible versions of the Belkin F5U002

[Alex Henrie]

The incompatible device in my possession has a sticker that says
"F5U002 Rev 2" and "P80453-B", and lsusb identifies it as
"050d:0002 Belkin Components IEEE-1284 Controller". There is a bug
report from 2007 from Michael Trausch who was seeing the exact same
errors that I saw in 2024 trying to use this cable.

Link: https://lore.kernel.org/all/46DE5830.9060401@trausch.us/
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 drivers/usb/misc/uss720.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/misc/uss720.c b/drivers/usb/misc/uss720.c
index 5803919d7cc4..0ad2bfc6af78 100644
--- a/drivers/usb/misc/uss720.c
+++ b/drivers/usb/misc/uss720.c
@@ -677,7 +677,7 @@ static int uss720_probe(struct usb_interface *intf,
 	struct parport_uss720_private *priv;
 	struct parport *pp;
 	unsigned char reg;
-	int i;
+	int ret;
 
 	dev_dbg(&intf->dev, "probe: vendor id 0x%x, device id 0x%x\n",
 		le16_to_cpu(usbdev->descriptor.idVendor),
@@ -688,8 +688,8 @@ static int uss720_probe(struct usb_interface *intf,
 		usb_put_dev(usbdev);
 		return -ENODEV;
 	}
-	i = usb_set_interface(usbdev, intf->altsetting->desc.bInterfaceNumber, 2);
-	dev_dbg(&intf->dev, "set interface result %d\n", i);
+	ret = usb_set_interface(usbdev, intf->altsetting->desc.bInterfaceNumber, 2);
+	dev_dbg(&intf->dev, "set interface result %d\n", ret);
 
 	interface = intf->cur_altsetting;
 
@@ -728,12 +728,18 @@ static int uss720_probe(struct usb_interface *intf,
 	set_1284_register(pp, 7, 0x00, GFP_KERNEL);
 	set_1284_register(pp, 6, 0x30, GFP_KERNEL);  /* PS/2 mode */
 	set_1284_register(pp, 2, 0x0c, GFP_KERNEL);
-	/* debugging */
-	get_1284_register(pp, 0, &reg, GFP_KERNEL);
+
+	/* The Belkin F5U002 Rev 2 P80453-B USB parallel port adapter shares the
+	 * device ID 050d:0002 with some other device that works with this
+	 * driver, but it itself does not. Detect and handle the bad cable
+	 * here. */
+	ret = get_1284_register(pp, 0, &reg, GFP_KERNEL);
 	dev_dbg(&intf->dev, "reg: %7ph\n", priv->reg);
+	if (ret < 0)
+		return ret;
 
-	i = usb_find_last_int_in_endpoint(interface, &epd);
-	if (!i) {
+	ret = usb_find_last_int_in_endpoint(interface, &epd);
+	if (!ret) {
 		dev_dbg(&intf->dev, "epaddr %d interval %d\n",
 				epd->bEndpointAddress, epd->bInterval);
 	}
-- 
2.44.0

Re: [PATCH 1/7] docs: driver-api: parport-lowlevel: clarify purpose of PARPORT_MODE_PCSPP

[Johan Hovold]

On Mon, Mar 11, 2024 at 11:50:26PM -0600, Alex Henrie wrote:

Please write a proper commit message here explaining why this patch is
needed. You can keep the link if it's relevant, but you still need to
make the patch self-contained.

> Link: http://lists.infradead.org/pipermail/linux-parport/2024-February/001237.html
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>

Johan

Re: [PATCH 2/7] usb: serial: mos7720: don't advertise PARPORT_MODE_PCSPP

[Johan Hovold]

On Mon, Mar 11, 2024 at 11:50:27PM -0600, Alex Henrie wrote:

This one and at least one of the later ones are also missing commit
messages. Please fix in a v2.

> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>

Johan

Re: [PATCH 4/7] usb: misc: uss720: point pp->dev to usbdev->dev

[Johan Hovold]

On Mon, Mar 11, 2024 at 11:50:29PM -0600, Alex Henrie wrote:
> This avoids a "fix this legacy no-device port driver" warning.

Please be more specific.

Johan

Re: [PATCH 2/7] usb: serial: mos7720: don't advertise PARPORT_MODE_PCSPP

[Sudip Mukherjee]

On Tue, 12 Mar 2024 at 07:39, Johan Hovold <johan@kernel.org> wrote:
>
> On Mon, Mar 11, 2024 at 11:50:27PM -0600, Alex Henrie wrote:
>
> This one and at least one of the later ones are also missing commit
> messages. Please fix in a v2.

It will be a NACK from me.

This patch and other patches removing PARPORT_MODE_PCSPP will break
userspace code.
On my system, I have:
$ cat /proc/sys/dev/parport/parport0/modes
PCSPP,TRISTATE

If the flag is removed then I will only get "TRISTATE", and if there
is any userspace code which checks for the mode then those
applications will stop working with an error.


-- 
Regards
Sudip

Re: [PATCH 1/7] docs: driver-api: parport-lowlevel: clarify purpose of PARPORT_MODE_PCSPP

[Daniel Gimpelevich]

That link is to a question that the submitter asked, and nobody
responded to it. It seems that this patch stems from an incomplete
reading of the kernel documentation. Those docs say:

> SPP (Standard Parallel Port) functions modify so-called SPP registers:
> data, status, and control. The hardware may not actually have
> registers exactly like that, but the PC does and this interface is
> modelled after common PC implementations. Other low-level drivers may
> be able to emulate most of the functionality.

So, the PARPORT_MODE_PCSPP flag denotes the availability of the SPP port
functions, not any fields in a struct.

On Tue, 2024-03-12 at 08:38 +0100, Johan Hovold wrote:
> On Mon, Mar 11, 2024 at 11:50:26PM -0600, Alex Henrie wrote:
> 
> Please write a proper commit message here explaining why this patch is
> needed. You can keep the link if it's relevant, but you still need to
> make the patch self-contained.
> 
> > Link: http://lists.infradead.org/pipermail/linux-parport/2024-February/001237.html
> > Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> 
> Johan

Re: [PATCH 2/7] usb: serial: mos7720: don't advertise PARPORT_MODE_PCSPP

[Daniel Gimpelevich]

On Tue, 2024-03-12 at 09:53 +0000, Sudip Mukherjee wrote:
> On Tue, 12 Mar 2024 at 07:39, Johan Hovold <johan@kernel.org> wrote:
> >
> > On Mon, Mar 11, 2024 at 11:50:27PM -0600, Alex Henrie wrote:
> >
> > This one and at least one of the later ones are also missing commit
> > messages. Please fix in a v2.
> 
> It will be a NACK from me.
> 
> This patch and other patches removing PARPORT_MODE_PCSPP will break
> userspace code.
> On my system, I have:
> $ cat /proc/sys/dev/parport/parport0/modes
> PCSPP,TRISTATE
> 
> If the flag is removed then I will only get "TRISTATE", and if there
> is any userspace code which checks for the mode then those
> applications will stop working with an error.

This patch seems like some sort of backscatter from
https://en.wikipedia.org/wiki/XY_problem issues.

Re: [PATCH 6/7] usb: misc: uss720: add support for another variant of the Belkin F5U002

[Daniel Gimpelevich]

You didn't add the "P80453-B" label in this patch nor in PATCH 5/7…

On Mon, 2024-03-11 at 23:50 -0600, Alex Henrie wrote:
> This device is a gray USB parallel port adapter with "F5U002" imprinted
> on the plastic and a sticker that says both "F5U002" and "P80453-A".
> It's identified in lsusb as "05ab:1001 In-System Design BAYI Printer
> Class Support". It's subtly different from the other gray cable I have
> that has "F5U002 Rev 2" and "P80453-B" on its sticker and device ID
> 050d:0002.
> 
> The uss720 driver appears to work flawlessly with this device, although
> the device evidently does not support ECP.
> 
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
>  drivers/usb/misc/uss720.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/misc/uss720.c b/drivers/usb/misc/uss720.c
> index 15cafc7dfd22..5803919d7cc4 100644
> --- a/drivers/usb/misc/uss720.c
> +++ b/drivers/usb/misc/uss720.c
> @@ -693,7 +693,7 @@ static int uss720_probe(struct usb_interface *intf,
>  
>  	interface = intf->cur_altsetting;
>  
> -	if (interface->desc.bNumEndpoints < 3) {
> +	if (interface->desc.bNumEndpoints < 2) {
>  		usb_put_dev(usbdev);
>  		return -ENODEV;
>  	}
> @@ -719,7 +719,9 @@ static int uss720_probe(struct usb_interface *intf,
>  
>  	priv->pp = pp;
>  	pp->private_data = priv;
> -	pp->modes = PARPORT_MODE_TRISTATE | PARPORT_MODE_EPP | PARPORT_MODE_ECP | PARPORT_MODE_COMPAT;
> +	pp->modes = PARPORT_MODE_TRISTATE | PARPORT_MODE_EPP | PARPORT_MODE_COMPAT;
> +	if (interface->desc.bNumEndpoints >= 3)
> +		pp->modes |= PARPORT_MODE_ECP;
>  	pp->dev = &usbdev->dev;
>  
>  	/* set the USS720 control register to manual mode, no ECP compression, enable all ints */
> @@ -774,6 +776,7 @@ static const struct usb_device_id uss720_table[] = {
>  	{ USB_DEVICE(0x050d, 0x1202) }, /* Belkin F5U120-PC */
>  	{ USB_DEVICE(0x0557, 0x2001) },
>  	{ USB_DEVICE(0x05ab, 0x0002) }, /* Belkin F5U002 ISD-101 */
> +	{ USB_DEVICE(0x05ab, 0x1001) }, /* Belkin F5U002 P80453-A */
>  	{ USB_DEVICE(0x06c6, 0x0100) }, /* Infowave ISD-103 */
>  	{ USB_DEVICE(0x0729, 0x1284) },
>  	{ USB_DEVICE(0x1293, 0x0002) },

Re: [PATCH 5/7] usb: misc: uss720: document the names of the compatible devices

[Daniel Gimpelevich]

On Mon, 2024-03-11 at 23:50 -0600, Alex Henrie wrote:
> Information about 04b8:0002 and 05ab:0002 is from commit e3cb7bde9a6a
> ("USB: misc: uss720: more vendor/product ID's", 2018-03-20). The rest
> are from <http://reboots.g-cipher.net/lcd/>.

FYI:
Device: ID 04b8:0002 Seiko Epson Corp. ISD Smart Cable for Mac
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               1.00
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0        64
  idVendor           0x04b8 Seiko Epson Corp.
  idProduct          0x0002 ISD Smart Cable for Mac
  bcdDevice            2.00
  iManufacturer           9 EPSON
  iProduct               12 USB-Parallel Adapter
  iSerial                23 066200241zic9htt
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           71
    bNumInterfaces          1
    bConfigurationValue     1
    iConfiguration          0 
    bmAttributes         0x80
      (Bus Powered)
    MaxPower               96mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         7 Printer
      bInterfaceSubClass      1 Printer
      bInterfaceProtocol      1 Unidirectional
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x01  EP 1 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       1
      bNumEndpoints           2
      bInterfaceClass         7 Printer
      bInterfaceSubClass      1 Printer
      bInterfaceProtocol      2 Bidirectional
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x01  EP 1 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       2
      bNumEndpoints           2
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass      0 
      bInterfaceProtocol    255 
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x01  EP 1 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
Device Status:     0x0000
  (Bus Powered)
Device: ID 05ab:0002 In-System Design Parallel Port
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               1.00
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0         8
  idVendor           0x05ab In-System Design
  idProduct          0x0002 Parallel Port
  bcdDevice            1.04
  iManufacturer           0 
  iProduct                0 
  iSerial                 0 
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           78
    bNumInterfaces          1
    bConfigurationValue     1
    iConfiguration          0 
    bmAttributes         0x80
      (Bus Powered)
    MaxPower               98mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         7 Printer
      bInterfaceSubClass      1 Printer
      bInterfaceProtocol      1 Unidirectional
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x01  EP 1 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       1
      bNumEndpoints           2
      bInterfaceClass         7 Printer
      bInterfaceSubClass      1 Printer
      bInterfaceProtocol      2 Bidirectional
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x01  EP 1 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       2
      bNumEndpoints           3
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass      0 
      bInterfaceProtocol    255 
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x01  EP 1 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0004  1x 4 bytes
        bInterval               1
Device Status:     0x0000
  (Bus Powered)

Re: [PATCH 1/7] docs: driver-api: parport-lowlevel: clarify purpose of PARPORT_MODE_PCSPP

[Alex Henrie]

On Tue, Mar 12, 2024 at 9:24 AM Daniel Gimpelevich
<daniel@gimpelevich.san-francisco.ca.us> wrote:
>
> That link is to a question that the submitter asked, and nobody
> responded to it. It seems that this patch stems from an incomplete
> reading of the kernel documentation. Those docs say:
>
> > SPP (Standard Parallel Port) functions modify so-called SPP registers:
> > data, status, and control. The hardware may not actually have
> > registers exactly like that, but the PC does and this interface is
> > modelled after common PC implementations. Other low-level drivers may
> > be able to emulate most of the functionality.
>
> So, the PARPORT_MODE_PCSPP flag denotes the availability of the SPP port
> functions, not any fields in a struct.

Hello Daniel, thanks for your reply. I still don't quite understand
what it would mean for a driver to lack PARPORT_MODE_PCSPP. Is the
idea that a parallel port could support EPP or ECP without supporting
SPP? If that's the case then in my opinion the documentation should
still be rewritten, but in a way to clarify that the SPP functions [1]
are not available without the flag, and the flag does not imply low
latency.

-Alex

[1] Namely: read_data, write_data, read_status, read_control,
write_control, frob_control, enable_irq, disable_irq, data_forward,
and data_reverse. See
https://docs.kernel.org/driver-api/parport-lowlevel.html

Re: [PATCH 6/7] usb: misc: uss720: add support for another variant of the Belkin F5U002

[Alex Henrie]

On Tue, Mar 12, 2024 at 9:31 AM Daniel Gimpelevich
<daniel@gimpelevich.san-francisco.ca.us> wrote:
>
> You didn't add the "P80453-B" label in this patch nor in PATCH 5/7…

Patch 7 adds it to a comment in uss720_probe. The reason I didn't put
the comment into uss720_table is because the F5U002 Rev 2 P80453-B
doesn't work with this driver. I would love to know which device with
ID 05ab:1001 was previously tested and found to work so that we can
document it.

-Alex

Re: [PATCH 4/7] usb: misc: uss720: point pp->dev to usbdev->dev

[Alex Henrie]

On Tue, Mar 12, 2024 at 1:39 AM Johan Hovold <johan@kernel.org> wrote:
>
> On Mon, Mar 11, 2024 at 11:50:29PM -0600, Alex Henrie wrote:
> > This avoids a "fix this legacy no-device port driver" warning.
>
> Please be more specific.

Hello Johan, thanks for taking a look at these patches.

The warning comes from parport_announce_port in
drivers/parport/share.c. include/linux/parport.h says that dev is the
"Physical device associated with IO/DMA." Commit 4edb38695d9a
("parisc: parport0: fix this legacy no-device port driver!",
2013-05-30) fixed a similar issue and says only "Fix the above kernel
error from parport_announce_port() on 32bit GSC machines (e.g. B160L).
The parport driver requires now a pointer to the device struct."

Do I just need to include "The parport driver now requires a pointer
to the device struct" in the commit message? If not, where can I learn
more about what the dev field is for, to be able to write a better
description of why it's necessary to fill it in?

Thanks,

-Alex

Re: [PATCH 2/7] usb: serial: mos7720: don't advertise PARPORT_MODE_PCSPP

[Alex Henrie]

On Tue, Mar 12, 2024 at 1:39 AM Johan Hovold <johan@kernel.org> wrote:
>
> On Mon, Mar 11, 2024 at 11:50:27PM -0600, Alex Henrie wrote:
>
> This one and at least one of the later ones are also missing commit
> messages. Please fix in a v2.

Thanks for the feedback. I will send a v2 after the 6.9-rc1 release
and if we decide to keep the first three patches, I will add commit
messages to them.

-Alex

Re: [PATCH 4/7] usb: misc: uss720: point pp->dev to usbdev->dev

[Johan Hovold]

On Tue, Mar 12, 2024 at 07:30:21PM -0600, Alex Henrie wrote:
> On Tue, Mar 12, 2024 at 1:39 AM Johan Hovold <johan@kernel.org> wrote:
> >
> > On Mon, Mar 11, 2024 at 11:50:29PM -0600, Alex Henrie wrote:
> > > This avoids a "fix this legacy no-device port driver" warning.
> >
> > Please be more specific.
> 
> Hello Johan, thanks for taking a look at these patches.
> 
> The warning comes from parport_announce_port in
> drivers/parport/share.c. include/linux/parport.h says that dev is the
> "Physical device associated with IO/DMA." Commit 4edb38695d9a
> ("parisc: parport0: fix this legacy no-device port driver!",
> 2013-05-30) fixed a similar issue and says only "Fix the above kernel
> error from parport_announce_port() on 32bit GSC machines (e.g. B160L).
> The parport driver requires now a pointer to the device struct."
> 
> Do I just need to include "The parport driver now requires a pointer
> to the device struct" in the commit message? If not, where can I learn
> more about what the dev field is for, to be able to write a better
> description of why it's necessary to fill it in?

Your commit messages need to be self-contained and explain *why* you
think your proposed change is needed, and in enough detail that a
reviewer can make a judgement as to whether the patch is correct or not.

Basically all my comments were just pointing this out.

Johan

Re: [PATCH 2/7] usb: serial: mos7720: don't advertise PARPORT_MODE_PCSPP

[Greg KH]

On Mon, Mar 11, 2024 at 11:50:27PM -0600, Alex Henrie wrote:
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
>  drivers/usb/serial/mos7720.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

For obvious reasons I can't take patches without any changelog text,
sorry.

greg k-h

[PATCH v2 0/4] usb: misc: uss720: improve support for Belkin F5U002 devices

[Alex Henrie]

Changes in v2:
- Keep PARPORT_MODE_PCSPP
- Expand commit message of first patch

Alex Henrie (4):
  usb: misc: uss720: point pp->dev to usbdev->dev
  usb: misc: uss720: document the names of the compatible devices
  usb: misc: uss720: add support for another variant of the Belkin
    F5U002
  usb: misc: uss720: check for incompatible versions of the Belkin
    F5U002

 drivers/usb/misc/uss720.c | 40 ++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)

-- 
2.44.0

[PATCH v2 1/4] usb: misc: uss720: point pp->dev to usbdev->dev

[Alex Henrie]

This avoids a "fix this legacy no-device port driver" warning from
parport_announce_port in drivers/parport/share.c. The parport driver now
requires a pointer to the device struct.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 drivers/usb/misc/uss720.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/misc/uss720.c b/drivers/usb/misc/uss720.c
index b00d92db5dfd..1c940608deb5 100644
--- a/drivers/usb/misc/uss720.c
+++ b/drivers/usb/misc/uss720.c
@@ -720,6 +720,7 @@ static int uss720_probe(struct usb_interface *intf,
 	priv->pp = pp;
 	pp->private_data = priv;
 	pp->modes = PARPORT_MODE_PCSPP | PARPORT_MODE_TRISTATE | PARPORT_MODE_EPP | PARPORT_MODE_ECP | PARPORT_MODE_COMPAT;
+	pp->dev = &usbdev->dev;
 
 	/* set the USS720 control register to manual mode, no ECP compression, enable all ints */
 	set_1284_register(pp, 7, 0x00, GFP_KERNEL);
-- 
2.44.0

[PATCH v2 2/4] usb: misc: uss720: document the names of the compatible devices

[Alex Henrie]

Information about 04b8:0002 and 05ab:0002 is from commit e3cb7bde9a6a
("USB: misc: uss720: more vendor/product ID's", 2018-03-20). The rest
are from <http://reboots.g-cipher.net/lcd/>.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 drivers/usb/misc/uss720.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/misc/uss720.c b/drivers/usb/misc/uss720.c
index 1c940608deb5..32caf3ca300f 100644
--- a/drivers/usb/misc/uss720.c
+++ b/drivers/usb/misc/uss720.c
@@ -767,14 +767,14 @@ static void uss720_disconnect(struct usb_interface *intf)
 
 /* table of cables that work through this driver */
 static const struct usb_device_id uss720_table[] = {
-	{ USB_DEVICE(0x047e, 0x1001) },
-	{ USB_DEVICE(0x04b8, 0x0002) },
-	{ USB_DEVICE(0x04b8, 0x0003) },
+	{ USB_DEVICE(0x047e, 0x1001) }, /* Infowave 901-0030 */
+	{ USB_DEVICE(0x04b8, 0x0002) }, /* Epson CAEUL0002 ISD-103 */
+	{ USB_DEVICE(0x04b8, 0x0003) }, /* Epson ISD-101 */
 	{ USB_DEVICE(0x050d, 0x0002) },
-	{ USB_DEVICE(0x050d, 0x1202) },
+	{ USB_DEVICE(0x050d, 0x1202) }, /* Belkin F5U120-PC */
 	{ USB_DEVICE(0x0557, 0x2001) },
-	{ USB_DEVICE(0x05ab, 0x0002) },
-	{ USB_DEVICE(0x06c6, 0x0100) },
+	{ USB_DEVICE(0x05ab, 0x0002) }, /* Belkin F5U002 ISD-101 */
+	{ USB_DEVICE(0x06c6, 0x0100) }, /* Infowave ISD-103 */
 	{ USB_DEVICE(0x0729, 0x1284) },
 	{ USB_DEVICE(0x1293, 0x0002) },
 	{ }						/* Terminating entry */
-- 
2.44.0

[PATCH v2 3/4] usb: misc: uss720: add support for another variant of the Belkin F5U002

[Alex Henrie]

This device is a gray USB parallel port adapter with "F5U002" imprinted
on the plastic and a sticker that says both "F5U002" and "P80453-A".
It's identified in lsusb as "05ab:1001 In-System Design BAYI Printer
Class Support". It's subtly different from the other gray cable I have
that has "F5U002 Rev 2" and "P80453-B" on its sticker and device ID
050d:0002.

The uss720 driver appears to work flawlessly with this device, although
the device evidently does not support ECP.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 drivers/usb/misc/uss720.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/misc/uss720.c b/drivers/usb/misc/uss720.c
index 32caf3ca300f..5423da08a467 100644
--- a/drivers/usb/misc/uss720.c
+++ b/drivers/usb/misc/uss720.c
@@ -693,7 +693,7 @@ static int uss720_probe(struct usb_interface *intf,
 
 	interface = intf->cur_altsetting;
 
-	if (interface->desc.bNumEndpoints < 3) {
+	if (interface->desc.bNumEndpoints < 2) {
 		usb_put_dev(usbdev);
 		return -ENODEV;
 	}
@@ -719,7 +719,9 @@ static int uss720_probe(struct usb_interface *intf,
 
 	priv->pp = pp;
 	pp->private_data = priv;
-	pp->modes = PARPORT_MODE_PCSPP | PARPORT_MODE_TRISTATE | PARPORT_MODE_EPP | PARPORT_MODE_ECP | PARPORT_MODE_COMPAT;
+	pp->modes = PARPORT_MODE_PCSPP | PARPORT_MODE_TRISTATE | PARPORT_MODE_EPP | PARPORT_MODE_COMPAT;
+	if (interface->desc.bNumEndpoints >= 3)
+		pp->modes |= PARPORT_MODE_ECP;
 	pp->dev = &usbdev->dev;
 
 	/* set the USS720 control register to manual mode, no ECP compression, enable all ints */
@@ -774,6 +776,7 @@ static const struct usb_device_id uss720_table[] = {
 	{ USB_DEVICE(0x050d, 0x1202) }, /* Belkin F5U120-PC */
 	{ USB_DEVICE(0x0557, 0x2001) },
 	{ USB_DEVICE(0x05ab, 0x0002) }, /* Belkin F5U002 ISD-101 */
+	{ USB_DEVICE(0x05ab, 0x1001) }, /* Belkin F5U002 P80453-A */
 	{ USB_DEVICE(0x06c6, 0x0100) }, /* Infowave ISD-103 */
 	{ USB_DEVICE(0x0729, 0x1284) },
 	{ USB_DEVICE(0x1293, 0x0002) },
-- 
2.44.0

[PATCH v2 4/4] usb: misc: uss720: check for incompatible versions of the Belkin F5U002

[Alex Henrie]

The incompatible device in my possession has a sticker that says
"F5U002 Rev 2" and "P80453-B", and lsusb identifies it as
"050d:0002 Belkin Components IEEE-1284 Controller". There is a bug
report from 2007 from Michael Trausch who was seeing the exact same
errors that I saw in 2024 trying to use this cable.

Link: https://lore.kernel.org/all/46DE5830.9060401@trausch.us/
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 drivers/usb/misc/uss720.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/misc/uss720.c b/drivers/usb/misc/uss720.c
index 5423da08a467..b26c1d382d59 100644
--- a/drivers/usb/misc/uss720.c
+++ b/drivers/usb/misc/uss720.c
@@ -677,7 +677,7 @@ static int uss720_probe(struct usb_interface *intf,
 	struct parport_uss720_private *priv;
 	struct parport *pp;
 	unsigned char reg;
-	int i;
+	int ret;
 
 	dev_dbg(&intf->dev, "probe: vendor id 0x%x, device id 0x%x\n",
 		le16_to_cpu(usbdev->descriptor.idVendor),
@@ -688,8 +688,8 @@ static int uss720_probe(struct usb_interface *intf,
 		usb_put_dev(usbdev);
 		return -ENODEV;
 	}
-	i = usb_set_interface(usbdev, intf->altsetting->desc.bInterfaceNumber, 2);
-	dev_dbg(&intf->dev, "set interface result %d\n", i);
+	ret = usb_set_interface(usbdev, intf->altsetting->desc.bInterfaceNumber, 2);
+	dev_dbg(&intf->dev, "set interface result %d\n", ret);
 
 	interface = intf->cur_altsetting;
 
@@ -728,12 +728,18 @@ static int uss720_probe(struct usb_interface *intf,
 	set_1284_register(pp, 7, 0x00, GFP_KERNEL);
 	set_1284_register(pp, 6, 0x30, GFP_KERNEL);  /* PS/2 mode */
 	set_1284_register(pp, 2, 0x0c, GFP_KERNEL);
-	/* debugging */
-	get_1284_register(pp, 0, &reg, GFP_KERNEL);
+
+	/* The Belkin F5U002 Rev 2 P80453-B USB parallel port adapter shares the
+	 * device ID 050d:0002 with some other device that works with this
+	 * driver, but it itself does not. Detect and handle the bad cable
+	 * here. */
+	ret = get_1284_register(pp, 0, &reg, GFP_KERNEL);
 	dev_dbg(&intf->dev, "reg: %7ph\n", priv->reg);
+	if (ret < 0)
+		return ret;
 
-	i = usb_find_last_int_in_endpoint(interface, &epd);
-	if (!i) {
+	ret = usb_find_last_int_in_endpoint(interface, &epd);
+	if (!ret) {
 		dev_dbg(&intf->dev, "epaddr %d interval %d\n",
 				epd->bEndpointAddress, epd->bInterval);
 	}
-- 
2.44.0

Re: [PATCH v2 1/4] usb: misc: uss720: point pp->dev to usbdev->dev

[Sudip Mukherjee]

On Tue, 26 Mar 2024 at 15:16, Alex Henrie <alexhenrie24@gmail.com> wrote:
>
> This avoids a "fix this legacy no-device port driver" warning from
> parport_announce_port in drivers/parport/share.c. The parport driver now
> requires a pointer to the device struct.
>
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>

Acked-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>


-- 
Regards
Sudip