[PATCH] pl2303: Remove antiquated support for I330 phone

[PATCH] pl2303: Remove antiquated support for I330 phone

[Jason A. Donenfeld]

Samsung has just released a portable USB3 SSD, coming in a very small
and nice form factor. It's USB ID is 04e8:8001, which unfortunately is
already used by the pl2303 USB serial driver for the Samsung I330 phone
cradle. This phone was manufactured in 2001, and I seriously doubt it is
still in use on any contemporary networks. Having pl2303 pick up this
device ID results in conflicts with the usb-storage driver, which
handles the newly released portable USB3 SSD. In this patch I remove the
device ID of the old antiquated telephone from the pl2303 driver, so
that usb-storage can handle it.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/usb/serial/pl2303.c | 1 -
 drivers/usb/serial/pl2303.h | 4 ----
 2 files changed, 5 deletions(-)

diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index 829604d..f5257af 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -61,7 +61,6 @@ static const struct usb_device_id id_table[] = {
 	{ USB_DEVICE(DCU10_VENDOR_ID, DCU10_PRODUCT_ID) },
 	{ USB_DEVICE(SITECOM_VENDOR_ID, SITECOM_PRODUCT_ID) },
 	{ USB_DEVICE(ALCATEL_VENDOR_ID, ALCATEL_PRODUCT_ID) },
-	{ USB_DEVICE(SAMSUNG_VENDOR_ID, SAMSUNG_PRODUCT_ID) },
 	{ USB_DEVICE(SIEMENS_VENDOR_ID, SIEMENS_PRODUCT_ID_SX1),
 		.driver_info = PL2303_QUIRK_UART_STATE_IDX0 },
 	{ USB_DEVICE(SIEMENS_VENDOR_ID, SIEMENS_PRODUCT_ID_X65),
diff --git a/drivers/usb/serial/pl2303.h b/drivers/usb/serial/pl2303.h
index 71fd9da..e3b7af8 100644
--- a/drivers/usb/serial/pl2303.h
+++ b/drivers/usb/serial/pl2303.h
@@ -62,10 +62,6 @@
 #define ALCATEL_VENDOR_ID	0x11f7
 #define ALCATEL_PRODUCT_ID	0x02df
 
-/* Samsung I330 phone cradle */
-#define SAMSUNG_VENDOR_ID	0x04e8
-#define SAMSUNG_PRODUCT_ID	0x8001
-
 #define SIEMENS_VENDOR_ID	0x11f5
 #define SIEMENS_PRODUCT_ID_SX1	0x0001
 #define SIEMENS_PRODUCT_ID_X65	0x0003
-- 
2.3.5

Re: [PATCH] pl2303: Remove antiquated support for I330 phone

[Greg Kroah-Hartman]

On Tue, Apr 21, 2015 at 05:07:05PM +0200, Jason A. Donenfeld wrote:
> Samsung has just released a portable USB3 SSD, coming in a very small
> and nice form factor. It's USB ID is 04e8:8001, which unfortunately is
> already used by the pl2303 USB serial driver for the Samsung I330 phone
> cradle. This phone was manufactured in 2001, and I seriously doubt it is
> still in use on any contemporary networks. Having pl2303 pick up this
> device ID results in conflicts with the usb-storage driver, which
> handles the newly released portable USB3 SSD. In this patch I remove the
> device ID of the old antiquated telephone from the pl2303 driver, so
> that usb-storage can handle it.
> 
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  drivers/usb/serial/pl2303.c | 1 -
>  drivers/usb/serial/pl2303.h | 4 ----
>  2 files changed, 5 deletions(-)
> 
> diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
> index 829604d..f5257af 100644
> --- a/drivers/usb/serial/pl2303.c
> +++ b/drivers/usb/serial/pl2303.c
> @@ -61,7 +61,6 @@ static const struct usb_device_id id_table[] = {
>  	{ USB_DEVICE(DCU10_VENDOR_ID, DCU10_PRODUCT_ID) },
>  	{ USB_DEVICE(SITECOM_VENDOR_ID, SITECOM_PRODUCT_ID) },
>  	{ USB_DEVICE(ALCATEL_VENDOR_ID, ALCATEL_PRODUCT_ID) },
> -	{ USB_DEVICE(SAMSUNG_VENDOR_ID, SAMSUNG_PRODUCT_ID) },

How about adding a .driver_info quirk, that checks the device to see if
it matches a mass storage device?  If so, don't accept the probe, which
will keep the driver from working on the SSD device, and still keep
working on the old docking station.

thanks,

greg k-h

[PATCH] pl2303, visor: Match I330 phone more precisely

[Jason A. Donenfeld]

Samsung has just released a portable USB3 SSD, coming in a very small
and nice form factor. It's USB ID is 04e8:8001, which unfortunately is
already used by the pl2303 USB serial driver and the Palm Visor driver
for the Samsung I330 phone cradle. Having pl2303 or visor pick up this
device ID results in conflicts with the usb-storage driver, which
handles the newly released portable USB3 SSD.

To work around this conflict, I've dug up a mailing list post [1] from a
long time ago, in which a user posts the full USB descriptor
information. The most specific value in this appears to be the interface
class, which has value 255 (0xff). Since usb-storage requires an
interface class of 0x8, I believe it's correct to disambiguate the two
devices by matching on 0xff inside pl2303 and visor.

[1] http://permalink.gmane.org/gmane.linux.usb.user/4264

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/usb/serial/pl2303.c | 2 +-
 drivers/usb/serial/pl2303.h | 1 +
 drivers/usb/serial/visor.c  | 2 +-
 drivers/usb/serial/visor.h  | 1 +
 4 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index 829604d..45e0c5a 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -61,7 +61,7 @@ static const struct usb_device_id id_table[] = {
 	{ USB_DEVICE(DCU10_VENDOR_ID, DCU10_PRODUCT_ID) },
 	{ USB_DEVICE(SITECOM_VENDOR_ID, SITECOM_PRODUCT_ID) },
 	{ USB_DEVICE(ALCATEL_VENDOR_ID, ALCATEL_PRODUCT_ID) },
-	{ USB_DEVICE(SAMSUNG_VENDOR_ID, SAMSUNG_PRODUCT_ID) },
+	{ USB_DEVICE_INTERFACE_CLASS(SAMSUNG_VENDOR_ID, SAMSUNG_PRODUCT_ID, SAMSUNG_IF_CLASS) },
 	{ USB_DEVICE(SIEMENS_VENDOR_ID, SIEMENS_PRODUCT_ID_SX1),
 		.driver_info = PL2303_QUIRK_UART_STATE_IDX0 },
 	{ USB_DEVICE(SIEMENS_VENDOR_ID, SIEMENS_PRODUCT_ID_X65),
diff --git a/drivers/usb/serial/pl2303.h b/drivers/usb/serial/pl2303.h
index 71fd9da..9c29fe8 100644
--- a/drivers/usb/serial/pl2303.h
+++ b/drivers/usb/serial/pl2303.h
@@ -65,6 +65,7 @@
 /* Samsung I330 phone cradle */
 #define SAMSUNG_VENDOR_ID	0x04e8
 #define SAMSUNG_PRODUCT_ID	0x8001
+#define SAMSUNG_IF_CLASS	0xff
 
 #define SIEMENS_VENDOR_ID	0x11f5
 #define SIEMENS_PRODUCT_ID_SX1	0x0001
diff --git a/drivers/usb/serial/visor.c b/drivers/usb/serial/visor.c
index bf2bd40..4e439ef 100644
--- a/drivers/usb/serial/visor.c
+++ b/drivers/usb/serial/visor.c
@@ -95,7 +95,7 @@ static const struct usb_device_id id_table[] = {
 		.driver_info = (kernel_ulong_t)&palm_os_4_probe },
 	{ USB_DEVICE(ACER_VENDOR_ID, ACER_S10_ID),
 		.driver_info = (kernel_ulong_t)&palm_os_4_probe },
-	{ USB_DEVICE(SAMSUNG_VENDOR_ID, SAMSUNG_SCH_I330_ID),
+	{ USB_DEVICE_INTERFACE_CLASS(SAMSUNG_VENDOR_ID, SAMSUNG_SCH_I330_ID, SAMSUNG_SCH_I330_IFCLASS),
 		.driver_info = (kernel_ulong_t)&palm_os_4_probe },
 	{ USB_DEVICE(SAMSUNG_VENDOR_ID, SAMSUNG_SPH_I500_ID),
 		.driver_info = (kernel_ulong_t)&palm_os_4_probe },
diff --git a/drivers/usb/serial/visor.h b/drivers/usb/serial/visor.h
index 4c456dd..ffe93a2 100644
--- a/drivers/usb/serial/visor.h
+++ b/drivers/usb/serial/visor.h
@@ -54,6 +54,7 @@
 
 #define SAMSUNG_VENDOR_ID		0x04E8
 #define SAMSUNG_SCH_I330_ID		0x8001
+#define SAMSUNG_SCH_I330_IFCLASS	0xFF
 #define SAMSUNG_SPH_I500_ID		0x6601
 
 #define TAPWAVE_VENDOR_ID		0x12EF
-- 
2.3.5

Re: [PATCH] pl2303, visor: Match I330 phone more precisely

[Sergei Shtylyov]

Hello.

On 4/22/2015 1:14 PM, Jason A. Donenfeld wrote:

> Samsung has just released a portable USB3 SSD, coming in a very small
> and nice form factor. It's USB ID is 04e8:8001, which unfortunately is
> already used by the pl2303 USB serial driver and the Palm Visor driver
> for the Samsung I330 phone cradle. Having pl2303 or visor pick up this
> device ID results in conflicts with the usb-storage driver, which
> handles the newly released portable USB3 SSD.

> To work around this conflict, I've dug up a mailing list post [1] from a
> long time ago, in which a user posts the full USB descriptor
> information. The most specific value in this appears to be the interface
> class, which has value 255 (0xff). Since usb-storage requires an
> interface class of 0x8, I believe it's correct to disambiguate the two
> devices by matching on 0xff inside pl2303 and visor.

> [1] http://permalink.gmane.org/gmane.linux.usb.user/4264

> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
[...]
> diff --git a/drivers/usb/serial/pl2303.h b/drivers/usb/serial/pl2303.h
> index 71fd9da..9c29fe8 100644
> --- a/drivers/usb/serial/pl2303.h
> +++ b/drivers/usb/serial/pl2303.h
> @@ -65,6 +65,7 @@
>   /* Samsung I330 phone cradle */
>   #define SAMSUNG_VENDOR_ID	0x04e8
>   #define SAMSUNG_PRODUCT_ID	0x8001
> +#define SAMSUNG_IF_CLASS	0xff

    No need for this #define, especially here. 0xff just means vendor-specific 
AFAIK.

>
>   #define SIEMENS_VENDOR_ID	0x11f5
>   #define SIEMENS_PRODUCT_ID_SX1	0x0001
[...]
> diff --git a/drivers/usb/serial/visor.h b/drivers/usb/serial/visor.h
> index 4c456dd..ffe93a2 100644
> --- a/drivers/usb/serial/visor.h
> +++ b/drivers/usb/serial/visor.h
> @@ -54,6 +54,7 @@
>
>   #define SAMSUNG_VENDOR_ID		0x04E8
>   #define SAMSUNG_SCH_I330_ID		0x8001
> +#define SAMSUNG_SCH_I330_IFCLASS	0xFF

    No need for this #define, especially here. 0xff just means vendor-specific 
AFAIK.

>   #define SAMSUNG_SPH_I500_ID		0x6601
>
>   #define TAPWAVE_VENDOR_ID		0x12EF

WBR, Sergei

Re: [PATCH] pl2303, visor: Match I330 phone more precisely

[Johan Hovold]

On Wed, Apr 22, 2015 at 12:14:05PM +0200, Jason A. Donenfeld wrote:
> Samsung has just released a portable USB3 SSD, coming in a very small
> and nice form factor. It's USB ID is 04e8:8001, which unfortunately is
> already used by the pl2303 USB serial driver and the Palm Visor driver
> for the Samsung I330 phone cradle. Having pl2303 or visor pick up this
> device ID results in conflicts with the usb-storage driver, which
> handles the newly released portable USB3 SSD.

First of all, the device should not be claimed by both pl2303 and visor.
This predates the git, but it looks like the device id should simply be
removed from pl2303. Care to do that as a preparatory patch?

> To work around this conflict, I've dug up a mailing list post [1] from a
> long time ago, in which a user posts the full USB descriptor
> information. The most specific value in this appears to be the interface
> class, which has value 255 (0xff). Since usb-storage requires an
> interface class of 0x8, I believe it's correct to disambiguate the two
> devices by matching on 0xff inside pl2303 and visor.
> 
> [1] http://permalink.gmane.org/gmane.linux.usb.user/4264

That seems like the way to go. As Sergei already suggested you can use
the interface class 0xff directly in the USB_DEVICE_INTERFACE_CLASS
macro.

Thanks,
Johan

[PATCH 1/2] pl2303: Remove support for Samsung I330

[Jason A. Donenfeld]

This phone is already supported by the visor driver.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/usb/serial/pl2303.c | 1 -
 drivers/usb/serial/pl2303.h | 4 ----
 2 files changed, 5 deletions(-)

diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index 829604d..f5257af 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -61,7 +61,6 @@ static const struct usb_device_id id_table[] = {
 	{ USB_DEVICE(DCU10_VENDOR_ID, DCU10_PRODUCT_ID) },
 	{ USB_DEVICE(SITECOM_VENDOR_ID, SITECOM_PRODUCT_ID) },
 	{ USB_DEVICE(ALCATEL_VENDOR_ID, ALCATEL_PRODUCT_ID) },
-	{ USB_DEVICE(SAMSUNG_VENDOR_ID, SAMSUNG_PRODUCT_ID) },
 	{ USB_DEVICE(SIEMENS_VENDOR_ID, SIEMENS_PRODUCT_ID_SX1),
 		.driver_info = PL2303_QUIRK_UART_STATE_IDX0 },
 	{ USB_DEVICE(SIEMENS_VENDOR_ID, SIEMENS_PRODUCT_ID_X65),
diff --git a/drivers/usb/serial/pl2303.h b/drivers/usb/serial/pl2303.h
index 71fd9da..e3b7af8 100644
--- a/drivers/usb/serial/pl2303.h
+++ b/drivers/usb/serial/pl2303.h
@@ -62,10 +62,6 @@
 #define ALCATEL_VENDOR_ID	0x11f7
 #define ALCATEL_PRODUCT_ID	0x02df
 
-/* Samsung I330 phone cradle */
-#define SAMSUNG_VENDOR_ID	0x04e8
-#define SAMSUNG_PRODUCT_ID	0x8001
-
 #define SIEMENS_VENDOR_ID	0x11f5
 #define SIEMENS_PRODUCT_ID_SX1	0x0001
 #define SIEMENS_PRODUCT_ID_X65	0x0003
-- 
2.3.5

[PATCH 2/2] visor: Match I330 phone more precisely

[Jason A. Donenfeld]

Samsung has just released a portable USB3 SSD, coming in a very small
and nice form factor. It's USB ID is 04e8:8001, which unfortunately is
already used by the Palm Visor driver for the Samsung I330 phone cradle.
Having pl2303 or visor pick up this device ID results in conflicts with
the usb-storage driver, which handles the newly released portable USB3
SSD.

To work around this conflict, I've dug up a mailing list post [1] from a
long time ago, in which a user posts the full USB descriptor
information. The most specific value in this appears to be the interface
class, which has value 255 (0xff). Since usb-storage requires an
interface class of 0x8, I believe it's correct to disambiguate the two
devices by matching on 0xff inside visor.

[1] http://permalink.gmane.org/gmane.linux.usb.user/4264

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/usb/serial/visor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/serial/visor.c b/drivers/usb/serial/visor.c
index bf2bd40..60afb39 100644
--- a/drivers/usb/serial/visor.c
+++ b/drivers/usb/serial/visor.c
@@ -95,7 +95,7 @@ static const struct usb_device_id id_table[] = {
 		.driver_info = (kernel_ulong_t)&palm_os_4_probe },
 	{ USB_DEVICE(ACER_VENDOR_ID, ACER_S10_ID),
 		.driver_info = (kernel_ulong_t)&palm_os_4_probe },
-	{ USB_DEVICE(SAMSUNG_VENDOR_ID, SAMSUNG_SCH_I330_ID),
+	{ USB_DEVICE_INTERFACE_CLASS(SAMSUNG_VENDOR_ID, SAMSUNG_SCH_I330_ID, 0xff),
 		.driver_info = (kernel_ulong_t)&palm_os_4_probe },
 	{ USB_DEVICE(SAMSUNG_VENDOR_ID, SAMSUNG_SPH_I500_ID),
 		.driver_info = (kernel_ulong_t)&palm_os_4_probe },
-- 
2.3.5

Re: [PATCH] pl2303, visor: Match I330 phone more precisely

[Greg Kroah-Hartman]

On Wed, Apr 22, 2015 at 01:20:38PM +0200, Johan Hovold wrote:
> On Wed, Apr 22, 2015 at 12:14:05PM +0200, Jason A. Donenfeld wrote:
> > Samsung has just released a portable USB3 SSD, coming in a very small
> > and nice form factor. It's USB ID is 04e8:8001, which unfortunately is
> > already used by the pl2303 USB serial driver and the Palm Visor driver
> > for the Samsung I330 phone cradle. Having pl2303 or visor pick up this
> > device ID results in conflicts with the usb-storage driver, which
> > handles the newly released portable USB3 SSD.
> 
> First of all, the device should not be claimed by both pl2303 and visor.
> This predates the git, but it looks like the device id should simply be
> removed from pl2303. Care to do that as a preparatory patch?

It was added back in 2004 in the 2.4.26 kernel release.  Petr Slansky
asked me to add it, and sent a patch as he had the device.

But before that, back in 2003 Adam emailed saying that the visor driver
worked with the device, and sent me a patch for it.  I didn't notice the
duplicate ids for well over a decade now :(

I'd bet this is really a pl2303 device, given that the visor driver was
just a "dumb" pipe to the device and the pilot sync tools never cared
about baud rates and the like, so odds are the visor entry should be
removed.

Petr, do you still have this device?  Is it still really a pl2303
device?

thanks,

greg k-h

Re: [PATCH] pl2303, visor: Match I330 phone more precisely

[Jason A. Donenfeld]

On Wed, Apr 22, 2015 at 2:38 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> I'd bet this is really a pl2303 device, given that the visor driver was
> just a "dumb" pipe to the device and the pilot sync tools never cared
> about baud rates and the like, so odds are the visor entry should be
> removed.

In the latest patch set I sent a minute ago, I removed the pl2303
entry, not the visor one. I guess I should resubmit with this the
other way around?

[PATCH 1/2] visor: Remove support for Samsung I330

[Jason A. Donenfeld]

This phone is actually a pl2303 device, and is already supported there.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/usb/serial/visor.c | 2 --
 drivers/usb/serial/visor.h | 1 -
 2 files changed, 3 deletions(-)

diff --git a/drivers/usb/serial/visor.c b/drivers/usb/serial/visor.c
index bf2bd40..2fe4afb 100644
--- a/drivers/usb/serial/visor.c
+++ b/drivers/usb/serial/visor.c
@@ -95,8 +95,6 @@ static const struct usb_device_id id_table[] = {
 		.driver_info = (kernel_ulong_t)&palm_os_4_probe },
 	{ USB_DEVICE(ACER_VENDOR_ID, ACER_S10_ID),
 		.driver_info = (kernel_ulong_t)&palm_os_4_probe },
-	{ USB_DEVICE(SAMSUNG_VENDOR_ID, SAMSUNG_SCH_I330_ID),
-		.driver_info = (kernel_ulong_t)&palm_os_4_probe },
 	{ USB_DEVICE(SAMSUNG_VENDOR_ID, SAMSUNG_SPH_I500_ID),
 		.driver_info = (kernel_ulong_t)&palm_os_4_probe },
 	{ USB_DEVICE(TAPWAVE_VENDOR_ID, TAPWAVE_ZODIAC_ID),
diff --git a/drivers/usb/serial/visor.h b/drivers/usb/serial/visor.h
index 4c456dd..3a2c25f 100644
--- a/drivers/usb/serial/visor.h
+++ b/drivers/usb/serial/visor.h
@@ -53,7 +53,6 @@
 #define ACER_S10_ID			0x0001
 
 #define SAMSUNG_VENDOR_ID		0x04E8
-#define SAMSUNG_SCH_I330_ID		0x8001
 #define SAMSUNG_SPH_I500_ID		0x6601
 
 #define TAPWAVE_VENDOR_ID		0x12EF
-- 
2.3.5

[PATCH 2/2] pl2303: Match I330 phone more precisely

[Jason A. Donenfeld]

Samsung has just released a portable USB3 SSD, coming in a very small
and nice form factor. Its USB ID is 04e8:8001, which unfortunately is
already used by the pl2303 driver for the Samsung I330 phone cradle.
Having pl2303 pick up this device ID results in conflicts with the
usb-storage driver, which handles the newly released portable USB3 SSD.

To work around this conflict, I've dug up a mailing list post [1] from a
long time ago, in which a user posts the full USB descriptor
information. The most specific value in this appears to be the interface
class, which has value 255 (0xff). Since usb-storage requires an
interface class of 0x8, I believe it's correct to disambiguate the two
devices by matching on 0xff inside pl2303.

[1] http://permalink.gmane.org/gmane.linux.usb.user/4264

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/usb/serial/pl2303.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index 829604d..8bb929a 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -61,7 +61,7 @@ static const struct usb_device_id id_table[] = {
 	{ USB_DEVICE(DCU10_VENDOR_ID, DCU10_PRODUCT_ID) },
 	{ USB_DEVICE(SITECOM_VENDOR_ID, SITECOM_PRODUCT_ID) },
 	{ USB_DEVICE(ALCATEL_VENDOR_ID, ALCATEL_PRODUCT_ID) },
-	{ USB_DEVICE(SAMSUNG_VENDOR_ID, SAMSUNG_PRODUCT_ID) },
+	{ USB_DEVICE_INTERFACE_CLASS(SAMSUNG_VENDOR_ID, SAMSUNG_PRODUCT_ID, 0xff) },
 	{ USB_DEVICE(SIEMENS_VENDOR_ID, SIEMENS_PRODUCT_ID_SX1),
 		.driver_info = PL2303_QUIRK_UART_STATE_IDX0 },
 	{ USB_DEVICE(SIEMENS_VENDOR_ID, SIEMENS_PRODUCT_ID_X65),
-- 
2.3.5

Re: [PATCH] pl2303, visor: Match I330 phone more precisely

[Greg Kroah-Hartman]

On Wed, Apr 22, 2015 at 02:42:20PM +0200, Jason A. Donenfeld wrote:
> On Wed, Apr 22, 2015 at 2:38 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > I'd bet this is really a pl2303 device, given that the visor driver was
> > just a "dumb" pipe to the device and the pilot sync tools never cared
> > about baud rates and the like, so odds are the visor entry should be
> > removed.
> 
> In the latest patch set I sent a minute ago, I removed the pl2303
> entry, not the visor one. I guess I should resubmit with this the
> other way around?

Johan is the maintainer of this subsystem now, I'll defer to him as to
what he wants to accept :)

thanks,

greg k-h

Re: [PATCH] pl2303, visor: Match I330 phone more precisely

[Johan Hovold]

On Wed, Apr 22, 2015 at 02:38:00PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Apr 22, 2015 at 01:20:38PM +0200, Johan Hovold wrote:
> > On Wed, Apr 22, 2015 at 12:14:05PM +0200, Jason A. Donenfeld wrote:
> > > Samsung has just released a portable USB3 SSD, coming in a very small
> > > and nice form factor. It's USB ID is 04e8:8001, which unfortunately is
> > > already used by the pl2303 USB serial driver and the Palm Visor driver
> > > for the Samsung I330 phone cradle. Having pl2303 or visor pick up this
> > > device ID results in conflicts with the usb-storage driver, which
> > > handles the newly released portable USB3 SSD.
> > 
> > First of all, the device should not be claimed by both pl2303 and visor.
> > This predates the git, but it looks like the device id should simply be
> > removed from pl2303. Care to do that as a preparatory patch?
> 
> It was added back in 2004 in the 2.4.26 kernel release.  Petr Slansky
> asked me to add it, and sent a patch as he had the device.
> 
> But before that, back in 2003 Adam emailed saying that the visor driver
> worked with the device, and sent me a patch for it.  I didn't notice the
> duplicate ids for well over a decade now :(
>
> I'd bet this is really a pl2303 device, given that the visor driver was
> just a "dumb" pipe to the device and the pilot sync tools never cared
> about baud rates and the like, so odds are the visor entry should be
> removed.

My thinking was the other way around; that a dumb, generic device
(visor) would never work with a more specialised driver (pl2303).

The opposite could work though as the pl2303 setup commands would simply
fail, but the bulk in/out layout appears to be the same and should work
with a dumb device.

I double checked just now with a pl2303 HX device, and I can't seem to
get anything through with the default settings (visor or generic
driver) in loopback.

Note that there's another Samsung SPH-I500 Palm OS phone handled by
visor as well.

> Petr, do you still have this device?  Is it still really a pl2303
> device?

Let's see if we get some feedback first. But unless someone thinks my
reasoning above is flawed we could try removing it from pl2303 a little
later either way.

Thanks,
Johan

Re: [PATCH 2/2] visor: Match I330 phone more precisely

[Johan Hovold]

On Wed, Apr 22, 2015 at 02:35:09PM +0200, Jason A. Donenfeld wrote:
> Samsung has just released a portable USB3 SSD, coming in a very small
> and nice form factor. It's USB ID is 04e8:8001, which unfortunately is
> already used by the Palm Visor driver for the Samsung I330 phone cradle.
> Having pl2303 or visor pick up this device ID results in conflicts with
> the usb-storage driver, which handles the newly released portable USB3
> SSD.
> 
> To work around this conflict, I've dug up a mailing list post [1] from a
> long time ago, in which a user posts the full USB descriptor
> information. The most specific value in this appears to be the interface
> class, which has value 255 (0xff). Since usb-storage requires an
> interface class of 0x8, I believe it's correct to disambiguate the two
> devices by matching on 0xff inside visor.
> 
> [1] http://permalink.gmane.org/gmane.linux.usb.user/4264
> 
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

Both patches look good, but next time remember to include a patch
revision in the subject of all patches in a series (this would be
[PATCH v3]). Please use a "USB: " prefix as well.

I'll fix it up before applying this time.

Thanks,
Johan

Re: [PATCH] pl2303, visor: Match I330 phone more precisely

[Greg Kroah-Hartman]

On Wed, Apr 22, 2015 at 04:20:39PM +0200, Johan Hovold wrote:
> On Wed, Apr 22, 2015 at 02:38:00PM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Apr 22, 2015 at 01:20:38PM +0200, Johan Hovold wrote:
> > > On Wed, Apr 22, 2015 at 12:14:05PM +0200, Jason A. Donenfeld wrote:
> > > > Samsung has just released a portable USB3 SSD, coming in a very small
> > > > and nice form factor. It's USB ID is 04e8:8001, which unfortunately is
> > > > already used by the pl2303 USB serial driver and the Palm Visor driver
> > > > for the Samsung I330 phone cradle. Having pl2303 or visor pick up this
> > > > device ID results in conflicts with the usb-storage driver, which
> > > > handles the newly released portable USB3 SSD.
> > > 
> > > First of all, the device should not be claimed by both pl2303 and visor.
> > > This predates the git, but it looks like the device id should simply be
> > > removed from pl2303. Care to do that as a preparatory patch?
> > 
> > It was added back in 2004 in the 2.4.26 kernel release.  Petr Slansky
> > asked me to add it, and sent a patch as he had the device.
> > 
> > But before that, back in 2003 Adam emailed saying that the visor driver
> > worked with the device, and sent me a patch for it.  I didn't notice the
> > duplicate ids for well over a decade now :(
> >
> > I'd bet this is really a pl2303 device, given that the visor driver was
> > just a "dumb" pipe to the device and the pilot sync tools never cared
> > about baud rates and the like, so odds are the visor entry should be
> > removed.
> 
> My thinking was the other way around; that a dumb, generic device
> (visor) would never work with a more specialised driver (pl2303).
> 
> The opposite could work though as the pl2303 setup commands would simply
> fail, but the bulk in/out layout appears to be the same and should work
> with a dumb device.
> 
> I double checked just now with a pl2303 HX device, and I can't seem to
> get anything through with the default settings (visor or generic
> driver) in loopback.

Ok, fair enough, I wonder how Petr got it to work.  Given that there
were reports of it working just fine for the visor driver a full year
before Petr's patch, odds are he forgot to build that driver into his
kernel.

thanks,

greg k-h

Re: [PATCH] pl2303, visor: Match I330 phone more precisely

[Johan Hovold]

On Wed, Apr 22, 2015 at 04:58:44PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Apr 22, 2015 at 04:20:39PM +0200, Johan Hovold wrote:
> > On Wed, Apr 22, 2015 at 02:38:00PM +0200, Greg Kroah-Hartman wrote:
> > > On Wed, Apr 22, 2015 at 01:20:38PM +0200, Johan Hovold wrote:
> > > > On Wed, Apr 22, 2015 at 12:14:05PM +0200, Jason A. Donenfeld wrote:
> > > > > Samsung has just released a portable USB3 SSD, coming in a very small
> > > > > and nice form factor. It's USB ID is 04e8:8001, which unfortunately is
> > > > > already used by the pl2303 USB serial driver and the Palm Visor driver
> > > > > for the Samsung I330 phone cradle. Having pl2303 or visor pick up this
> > > > > device ID results in conflicts with the usb-storage driver, which
> > > > > handles the newly released portable USB3 SSD.
> > > > 
> > > > First of all, the device should not be claimed by both pl2303 and visor.
> > > > This predates the git, but it looks like the device id should simply be
> > > > removed from pl2303. Care to do that as a preparatory patch?
> > > 
> > > It was added back in 2004 in the 2.4.26 kernel release.  Petr Slansky
> > > asked me to add it, and sent a patch as he had the device.
> > > 
> > > But before that, back in 2003 Adam emailed saying that the visor driver
> > > worked with the device, and sent me a patch for it.  I didn't notice the
> > > duplicate ids for well over a decade now :(
> > >
> > > I'd bet this is really a pl2303 device, given that the visor driver was
> > > just a "dumb" pipe to the device and the pilot sync tools never cared
> > > about baud rates and the like, so odds are the visor entry should be
> > > removed.
> > 
> > My thinking was the other way around; that a dumb, generic device
> > (visor) would never work with a more specialised driver (pl2303).
> > 
> > The opposite could work though as the pl2303 setup commands would simply
> > fail, but the bulk in/out layout appears to be the same and should work
> > with a dumb device.
> > 
> > I double checked just now with a pl2303 HX device, and I can't seem to
> > get anything through with the default settings (visor or generic
> > driver) in loopback.
> 
> Ok, fair enough, I wonder how Petr got it to work.  Given that there
> were reports of it working just fine for the visor driver a full year
> before Petr's patch, odds are he forgot to build that driver into his
> kernel.

Sounds like a plausible explanation.

Johan

Re: [PATCH 2/2] visor: Match I330 phone more precisely

[Johan Hovold]

On Wed, Apr 22, 2015 at 04:30:12PM +0200, Johan Hovold wrote:
> On Wed, Apr 22, 2015 at 02:35:09PM +0200, Jason A. Donenfeld wrote:
> > Samsung has just released a portable USB3 SSD, coming in a very small
> > and nice form factor. It's USB ID is 04e8:8001, which unfortunately is
> > already used by the Palm Visor driver for the Samsung I330 phone cradle.
> > Having pl2303 or visor pick up this device ID results in conflicts with
> > the usb-storage driver, which handles the newly released portable USB3
> > SSD.
> > 
> > To work around this conflict, I've dug up a mailing list post [1] from a
> > long time ago, in which a user posts the full USB descriptor
> > information. The most specific value in this appears to be the interface
> > class, which has value 255 (0xff). Since usb-storage requires an
> > interface class of 0x8, I believe it's correct to disambiguate the two
> > devices by matching on 0xff inside visor.
> > 
> > [1] http://permalink.gmane.org/gmane.linux.usb.user/4264
> > 
> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> 
> Both patches look good, but next time remember to include a patch
> revision in the subject of all patches in a series (this would be
> [PATCH v3]). Please use a "USB: " prefix as well.
> 
> I'll fix it up before applying this time.

Both patches now applied with stable tags.

Thanks,
Johan