[PATCH v2] USB: Serial: cypress_M8: Enable Simply Automated UPB PIM

[PATCH v2] USB: Serial: cypress_M8: Enable Simply Automated UPB PIM

[James Hilliard]

This is a UPB(Universal Powerline Bus) PIM(Powerline Interface Module)
which allows for controlling multiple UPB compatible devices from
Linux using the standard serial interface.

Based on vendor application source code there are two different models
of USB based PIM devices in addition to a number of RS232 based PIM's.

The vendor UPB application source contains the following USB ID's:
#define USB_PCS_VENDOR_ID 0x04b4
#define USB_PCS_PIM_PRODUCT_ID 0x5500

#define USB_SAI_VENDOR_ID 0x17dd
#define USB_SAI_PIM_PRODUCT_ID 0x5500

The first set of ID's correspond to the PIM variant sold by Powerline
Control Systems while the second corresponds to the Simply Automated
Incorporated PIM. As the product ID for both of these match the default
cypress HID->COM RS232 product ID it assumed that they both use an
internal variant of this HID->COM RS232 converter hardware. However
as the vendor ID for the Simply Automated variant is different we need
to also add it to the cypress_M8 driver so that it is properly
detected.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
Changes v1 -> v2:
  - Add more detailed commit message.
---
 drivers/usb/serial/cypress_m8.c | 2 ++
 drivers/usb/serial/cypress_m8.h | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/usb/serial/cypress_m8.c b/drivers/usb/serial/cypress_m8.c
index 216edd5826ca..ecda82198798 100644
--- a/drivers/usb/serial/cypress_m8.c
+++ b/drivers/usb/serial/cypress_m8.c
@@ -59,6 +59,7 @@ static const struct usb_device_id id_table_earthmate[] = {
 
 static const struct usb_device_id id_table_cyphidcomrs232[] = {
 	{ USB_DEVICE(VENDOR_ID_CYPRESS, PRODUCT_ID_CYPHIDCOM) },
+	{ USB_DEVICE(VENDOR_ID_SAI, PRODUCT_ID_CYPHIDCOM) },
 	{ USB_DEVICE(VENDOR_ID_POWERCOM, PRODUCT_ID_UPS) },
 	{ USB_DEVICE(VENDOR_ID_FRWD, PRODUCT_ID_CYPHIDCOM_FRWD) },
 	{ }						/* Terminating entry */
@@ -73,6 +74,7 @@ static const struct usb_device_id id_table_combined[] = {
 	{ USB_DEVICE(VENDOR_ID_DELORME, PRODUCT_ID_EARTHMATEUSB) },
 	{ USB_DEVICE(VENDOR_ID_DELORME, PRODUCT_ID_EARTHMATEUSB_LT20) },
 	{ USB_DEVICE(VENDOR_ID_CYPRESS, PRODUCT_ID_CYPHIDCOM) },
+	{ USB_DEVICE(VENDOR_ID_SAI, PRODUCT_ID_CYPHIDCOM) },
 	{ USB_DEVICE(VENDOR_ID_POWERCOM, PRODUCT_ID_UPS) },
 	{ USB_DEVICE(VENDOR_ID_FRWD, PRODUCT_ID_CYPHIDCOM_FRWD) },
 	{ USB_DEVICE(VENDOR_ID_DAZZLE, PRODUCT_ID_CA42) },
diff --git a/drivers/usb/serial/cypress_m8.h b/drivers/usb/serial/cypress_m8.h
index 35e223751c0e..ca2d951ee238 100644
--- a/drivers/usb/serial/cypress_m8.h
+++ b/drivers/usb/serial/cypress_m8.h
@@ -25,6 +25,9 @@
 #define VENDOR_ID_CYPRESS		0x04b4
 #define PRODUCT_ID_CYPHIDCOM		0x5500
 
+/* Simply Automated HID->COM UPB PIM */
+#define VENDOR_ID_SAI		0x17dd
+
 /* FRWD Dongle - a GPS sports watch */
 #define VENDOR_ID_FRWD			0x6737
 #define PRODUCT_ID_CYPHIDCOM_FRWD	0x0001
-- 
2.25.1

Re: [PATCH v2] USB: Serial: cypress_M8: Enable Simply Automated UPB PIM

[Johan Hovold]

On Tue, Jun 16, 2020 at 04:04:03PM -0600, James Hilliard wrote:
> This is a UPB(Universal Powerline Bus) PIM(Powerline Interface Module)
> which allows for controlling multiple UPB compatible devices from
> Linux using the standard serial interface.
> 
> Based on vendor application source code there are two different models
> of USB based PIM devices in addition to a number of RS232 based PIM's.
> 
> The vendor UPB application source contains the following USB ID's:
> #define USB_PCS_VENDOR_ID 0x04b4
> #define USB_PCS_PIM_PRODUCT_ID 0x5500
> 
> #define USB_SAI_VENDOR_ID 0x17dd
> #define USB_SAI_PIM_PRODUCT_ID 0x5500
> 
> The first set of ID's correspond to the PIM variant sold by Powerline
> Control Systems while the second corresponds to the Simply Automated
> Incorporated PIM. As the product ID for both of these match the default
> cypress HID->COM RS232 product ID it assumed that they both use an
> internal variant of this HID->COM RS232 converter hardware. However
> as the vendor ID for the Simply Automated variant is different we need
> to also add it to the cypress_M8 driver so that it is properly
> detected.
> 
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
> Changes v1 -> v2:
>   - Add more detailed commit message.

Now applied, thanks.

Would you mind posting the output of "lsusb -v" for this device for
completeness?

Johan

Re: [PATCH v2] USB: Serial: cypress_M8: Enable Simply Automated UPB PIM

[James Hilliard]

On Mon, Jun 22, 2020 at 2:53 AM Johan Hovold <johan@kernel.org> wrote:
>
> On Tue, Jun 16, 2020 at 04:04:03PM -0600, James Hilliard wrote:
> > This is a UPB(Universal Powerline Bus) PIM(Powerline Interface Module)
> > which allows for controlling multiple UPB compatible devices from
> > Linux using the standard serial interface.
> >
> > Based on vendor application source code there are two different models
> > of USB based PIM devices in addition to a number of RS232 based PIM's.
> >
> > The vendor UPB application source contains the following USB ID's:
> > #define USB_PCS_VENDOR_ID 0x04b4
> > #define USB_PCS_PIM_PRODUCT_ID 0x5500
> >
> > #define USB_SAI_VENDOR_ID 0x17dd
> > #define USB_SAI_PIM_PRODUCT_ID 0x5500
> >
> > The first set of ID's correspond to the PIM variant sold by Powerline
> > Control Systems while the second corresponds to the Simply Automated
> > Incorporated PIM. As the product ID for both of these match the default
> > cypress HID->COM RS232 product ID it assumed that they both use an
> > internal variant of this HID->COM RS232 converter hardware. However
> > as the vendor ID for the Simply Automated variant is different we need
> > to also add it to the cypress_M8 driver so that it is properly
> > detected.
> >
> > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > ---
> > Changes v1 -> v2:
> >   - Add more detailed commit message.
>
> Now applied, thanks.
>
> Would you mind posting the output of "lsusb -v" for this device for
> completeness?
Bus 001 Device 004: ID 17dd:5500
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               1.00
  bDeviceClass            0
  bDeviceSubClass         0
  bDeviceProtocol         0
  bMaxPacketSize0         8
  idVendor           0x17dd
  idProduct          0x5500
  bcdDevice            0.00
  iManufacturer           1 Simply Automated Inc.
  iProduct                2 USB to Serial
  iSerial                 0
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength       0x0029
    bNumInterfaces          1
    bConfigurationValue     1
    iConfiguration          4 Sample HID
    bmAttributes         0x80
      (Bus Powered)
    MaxPower              100mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass         3
      bInterfaceSubClass      0
      bInterfaceProtocol      0
      iInterface              0
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.00
          bCountryCode            0 Not supported
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength      37
         Report Descriptors:
           ** UNAVAILABLE **
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0008  1x 8 bytes
        bInterval              10
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x02  EP 2 OUT
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0008  1x 8 bytes
        bInterval              10
can't get debug descriptor: Resource temporarily unavailable
Device Status:     0x0000
  (Bus Powered)
>
> Johan

Re: [PATCH v2] USB: Serial: cypress_M8: Enable Simply Automated UPB PIM

[James Hilliard]

On Mon, Jun 22, 2020 at 2:53 AM Johan Hovold <johan@kernel.org> wrote:
>
> On Tue, Jun 16, 2020 at 04:04:03PM -0600, James Hilliard wrote:
> > This is a UPB(Universal Powerline Bus) PIM(Powerline Interface Module)
> > which allows for controlling multiple UPB compatible devices from
> > Linux using the standard serial interface.
> >
> > Based on vendor application source code there are two different models
> > of USB based PIM devices in addition to a number of RS232 based PIM's.
> >
> > The vendor UPB application source contains the following USB ID's:
> > #define USB_PCS_VENDOR_ID 0x04b4
> > #define USB_PCS_PIM_PRODUCT_ID 0x5500
> >
> > #define USB_SAI_VENDOR_ID 0x17dd
> > #define USB_SAI_PIM_PRODUCT_ID 0x5500
> >
> > The first set of ID's correspond to the PIM variant sold by Powerline
> > Control Systems while the second corresponds to the Simply Automated
> > Incorporated PIM. As the product ID for both of these match the default
> > cypress HID->COM RS232 product ID it assumed that they both use an
> > internal variant of this HID->COM RS232 converter hardware. However
> > as the vendor ID for the Simply Automated variant is different we need
> > to also add it to the cypress_M8 driver so that it is properly
> > detected.
> >
> > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > ---
> > Changes v1 -> v2:
> >   - Add more detailed commit message.
>
> Now applied, thanks.
Oh, FYI I think part of the comment got dropped when you amended the patch
I don't see the defines in the comment here:
https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/commit/?h=usb-linus&id=7527d963dff544b0ddfba4319824c50f2a892aeb

I think I had to temporarily change my git config with this to make it not drop
the defines:
git config core.commentchar "*"
>
> Would you mind posting the output of "lsusb -v" for this device for
> completeness?
>
> Johan

Re: [PATCH v2] USB: Serial: cypress_M8: Enable Simply Automated UPB PIM

[Johan Hovold]

On Mon, Jun 22, 2020 at 11:04:09AM -0600, James Hilliard wrote:
> On Mon, Jun 22, 2020 at 2:53 AM Johan Hovold <johan@kernel.org> wrote:
> >
> > On Tue, Jun 16, 2020 at 04:04:03PM -0600, James Hilliard wrote:
> > > This is a UPB(Universal Powerline Bus) PIM(Powerline Interface Module)
> > > which allows for controlling multiple UPB compatible devices from
> > > Linux using the standard serial interface.
> > >
> > > Based on vendor application source code there are two different models
> > > of USB based PIM devices in addition to a number of RS232 based PIM's.
> > >
> > > The vendor UPB application source contains the following USB ID's:
> > > #define USB_PCS_VENDOR_ID 0x04b4
> > > #define USB_PCS_PIM_PRODUCT_ID 0x5500
> > >
> > > #define USB_SAI_VENDOR_ID 0x17dd
> > > #define USB_SAI_PIM_PRODUCT_ID 0x5500
> > >
> > > The first set of ID's correspond to the PIM variant sold by Powerline
> > > Control Systems while the second corresponds to the Simply Automated
> > > Incorporated PIM. As the product ID for both of these match the default
> > > cypress HID->COM RS232 product ID it assumed that they both use an
> > > internal variant of this HID->COM RS232 converter hardware. However
> > > as the vendor ID for the Simply Automated variant is different we need
> > > to also add it to the cypress_M8 driver so that it is properly
> > > detected.
> > >
> > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > > ---
> > > Changes v1 -> v2:
> > >   - Add more detailed commit message.
> >
> > Now applied, thanks.
> >
> > Would you mind posting the output of "lsusb -v" for this device for
> > completeness?
> Bus 001 Device 004: ID 17dd:5500
> Device Descriptor:
>   bLength                18
>   bDescriptorType         1
>   bcdUSB               1.00
>   bDeviceClass            0
>   bDeviceSubClass         0
>   bDeviceProtocol         0
>   bMaxPacketSize0         8
>   idVendor           0x17dd
>   idProduct          0x5500
>   bcdDevice            0.00
>   iManufacturer           1 Simply Automated Inc.
>   iProduct                2 USB to Serial
>   iSerial                 0
>   bNumConfigurations      1
>   Configuration Descriptor:
>     bLength                 9
>     bDescriptorType         2
>     wTotalLength       0x0029
>     bNumInterfaces          1
>     bConfigurationValue     1
>     iConfiguration          4 Sample HID
>     bmAttributes         0x80
>       (Bus Powered)
>     MaxPower              100mA
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        0
>       bAlternateSetting       0
>       bNumEndpoints           2
>       bInterfaceClass         3
>       bInterfaceSubClass      0
>       bInterfaceProtocol      0
>       iInterface              0
>         HID Device Descriptor:
>           bLength                 9
>           bDescriptorType        33
>           bcdHID               1.00
>           bCountryCode            0 Not supported
>           bNumDescriptors         1
>           bDescriptorType        34 Report
>           wDescriptorLength      37
>          Report Descriptors:
>            ** UNAVAILABLE **

Thanks for posting this.

Don't you need to add this device to the HID driver's ignore list
to prevent it from claiming the device (see hid_ignore_list) just like
for the Cypress VID?

Johan

Re: [PATCH v2] USB: Serial: cypress_M8: Enable Simply Automated UPB PIM

[Johan Hovold]

On Mon, Jun 22, 2020 at 11:09:51AM -0600, James Hilliard wrote:
> On Mon, Jun 22, 2020 at 2:53 AM Johan Hovold <johan@kernel.org> wrote:
> >
> > On Tue, Jun 16, 2020 at 04:04:03PM -0600, James Hilliard wrote:
> > > This is a UPB(Universal Powerline Bus) PIM(Powerline Interface Module)
> > > which allows for controlling multiple UPB compatible devices from
> > > Linux using the standard serial interface.
> > >
> > > Based on vendor application source code there are two different models
> > > of USB based PIM devices in addition to a number of RS232 based PIM's.
> > >
> > > The vendor UPB application source contains the following USB ID's:
> > > #define USB_PCS_VENDOR_ID 0x04b4
> > > #define USB_PCS_PIM_PRODUCT_ID 0x5500
> > >
> > > #define USB_SAI_VENDOR_ID 0x17dd
> > > #define USB_SAI_PIM_PRODUCT_ID 0x5500
> > >
> > > The first set of ID's correspond to the PIM variant sold by Powerline
> > > Control Systems while the second corresponds to the Simply Automated
> > > Incorporated PIM. As the product ID for both of these match the default
> > > cypress HID->COM RS232 product ID it assumed that they both use an
> > > internal variant of this HID->COM RS232 converter hardware. However
> > > as the vendor ID for the Simply Automated variant is different we need
> > > to also add it to the cypress_M8 driver so that it is properly
> > > detected.
> > >
> > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > > ---
> > > Changes v1 -> v2:
> > >   - Add more detailed commit message.
> >
> > Now applied, thanks.
>
> Oh, FYI I think part of the comment got dropped when you amended the
> patch
> I don't see the defines in the comment here:
> https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/commit/?h=usb-linus&id=7527d963dff544b0ddfba4319824c50f2a892aeb
> 
> I think I had to temporarily change my git config with this to make it not drop
> the defines:
> git config core.commentchar "*"

Thanks for catching that! I added a leading tab to prevent those lines
from being discarded.

Johan

Re: [PATCH v2] USB: Serial: cypress_M8: Enable Simply Automated UPB PIM

[James Hilliard]

On Tue, Jun 23, 2020 at 1:53 AM Johan Hovold <johan@kernel.org> wrote:
>
> On Mon, Jun 22, 2020 at 11:04:09AM -0600, James Hilliard wrote:
> > On Mon, Jun 22, 2020 at 2:53 AM Johan Hovold <johan@kernel.org> wrote:
> > >
> > > On Tue, Jun 16, 2020 at 04:04:03PM -0600, James Hilliard wrote:
> > > > This is a UPB(Universal Powerline Bus) PIM(Powerline Interface Module)
> > > > which allows for controlling multiple UPB compatible devices from
> > > > Linux using the standard serial interface.
> > > >
> > > > Based on vendor application source code there are two different models
> > > > of USB based PIM devices in addition to a number of RS232 based PIM's.
> > > >
> > > > The vendor UPB application source contains the following USB ID's:
> > > > #define USB_PCS_VENDOR_ID 0x04b4
> > > > #define USB_PCS_PIM_PRODUCT_ID 0x5500
> > > >
> > > > #define USB_SAI_VENDOR_ID 0x17dd
> > > > #define USB_SAI_PIM_PRODUCT_ID 0x5500
> > > >
> > > > The first set of ID's correspond to the PIM variant sold by Powerline
> > > > Control Systems while the second corresponds to the Simply Automated
> > > > Incorporated PIM. As the product ID for both of these match the default
> > > > cypress HID->COM RS232 product ID it assumed that they both use an
> > > > internal variant of this HID->COM RS232 converter hardware. However
> > > > as the vendor ID for the Simply Automated variant is different we need
> > > > to also add it to the cypress_M8 driver so that it is properly
> > > > detected.
> > > >
> > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > > > ---
> > > > Changes v1 -> v2:
> > > >   - Add more detailed commit message.
> > >
> > > Now applied, thanks.
> > >
> > > Would you mind posting the output of "lsusb -v" for this device for
> > > completeness?
> > Bus 001 Device 004: ID 17dd:5500
> > Device Descriptor:
> >   bLength                18
> >   bDescriptorType         1
> >   bcdUSB               1.00
> >   bDeviceClass            0
> >   bDeviceSubClass         0
> >   bDeviceProtocol         0
> >   bMaxPacketSize0         8
> >   idVendor           0x17dd
> >   idProduct          0x5500
> >   bcdDevice            0.00
> >   iManufacturer           1 Simply Automated Inc.
> >   iProduct                2 USB to Serial
> >   iSerial                 0
> >   bNumConfigurations      1
> >   Configuration Descriptor:
> >     bLength                 9
> >     bDescriptorType         2
> >     wTotalLength       0x0029
> >     bNumInterfaces          1
> >     bConfigurationValue     1
> >     iConfiguration          4 Sample HID
> >     bmAttributes         0x80
> >       (Bus Powered)
> >     MaxPower              100mA
> >     Interface Descriptor:
> >       bLength                 9
> >       bDescriptorType         4
> >       bInterfaceNumber        0
> >       bAlternateSetting       0
> >       bNumEndpoints           2
> >       bInterfaceClass         3
> >       bInterfaceSubClass      0
> >       bInterfaceProtocol      0
> >       iInterface              0
> >         HID Device Descriptor:
> >           bLength                 9
> >           bDescriptorType        33
> >           bcdHID               1.00
> >           bCountryCode            0 Not supported
> >           bNumDescriptors         1
> >           bDescriptorType        34 Report
> >           wDescriptorLength      37
> >          Report Descriptors:
> >            ** UNAVAILABLE **
>
> Thanks for posting this.
>
> Don't you need to add this device to the HID driver's ignore list
> to prevent it from claiming the device (see hid_ignore_list) just like
> for the Cypress VID?
Ah, yeah, that does look to be needed, do you want to edit my patch or
should I send a follow up patch with that added?
>
> Johan

Re: [PATCH v2] USB: Serial: cypress_M8: Enable Simply Automated UPB PIM

[Johan Hovold]

On Tue, Jun 23, 2020 at 02:08:57AM -0600, James Hilliard wrote:
> On Tue, Jun 23, 2020 at 1:53 AM Johan Hovold <johan@kernel.org> wrote:

> > Don't you need to add this device to the HID driver's ignore list
> > to prevent it from claiming the device (see hid_ignore_list) just like
> > for the Cypress VID?
>
> Ah, yeah, that does look to be needed, do you want to edit my patch or
> should I send a follow up patch with that added?

A follow-up patch is probably best.

Send it to the HID maintainer and include a CC-stable tag so that it
gets backported along with the new cypress_m8 entry.

Johan