diff mbox series

option: added support for Thales Cinterion EXS82 option port.

Message ID 20201124094155.10510-1-gciofono@gmail.com
State New
Headers show
Series option: added support for Thales Cinterion EXS82 option port. | expand

Commit Message

Giacinto Cifelli Nov. 24, 2020, 9:41 a.m. UTC
There is a single option port in this modem, and it is used as debug port

Signed-off-by: Giacinto Cifelli <gciofono@gmail.com>
---
 drivers/usb/serial/option.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Johan Hovold Nov. 25, 2020, 9:40 a.m. UTC | #1
On Tue, Nov 24, 2020 at 10:41:55AM +0100, Giacinto Cifelli wrote:
> There is a single option port in this modem, and it is used as debug port

> 

> Signed-off-by: Giacinto Cifelli <gciofono@gmail.com>


Thanks for the update (and thanks for the review, Lars).

Using the option driver for just a debug port seems like overkill, but
ok.

Some form issues:

 - When updating a patch always include a patch revision number in the
   Subject prefix (e.g. "[PATCH v2] USB: serial: option: add ...").

 - Include a short changelog below the "---" line so we know what
   changed since the previous version.

 - Try to follow the convention used by the subsystem for the Subject
   prefix (i.e. "USB: serial: option: add ...").

> ---


And please keep the lsusb -v info here below the cut-off line when
resending.

>  drivers/usb/serial/option.c | 2 ++

>  1 file changed, 2 insertions(+)

> 

> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c

> index 54ca85cc920d..fda291e68e4b 100644

> --- a/drivers/usb/serial/option.c

> +++ b/drivers/usb/serial/option.c

> @@ -424,6 +424,7 @@ static void option_instat_callback(struct urb *urb);

>  #define CINTERION_PRODUCT_AHXX_2RMNET		0x0084

>  #define CINTERION_PRODUCT_AHXX_AUDIO		0x0085

>  #define CINTERION_PRODUCT_CLS8			0x00b0

> +#define CINTERION_PRODUCT_EXS82			0x006c


Please keep the entries sorted by PID.

>  

>  /* Olivetti products */

>  #define OLIVETTI_VENDOR_ID			0x0b3c

> @@ -1908,6 +1909,7 @@ static const struct usb_device_id option_ids[] = {

>  	{ USB_DEVICE(SIEMENS_VENDOR_ID, CINTERION_PRODUCT_HC25_MDMNET) },

>  	{ USB_DEVICE(SIEMENS_VENDOR_ID, CINTERION_PRODUCT_HC28_MDM) }, /* HC28 enumerates with Siemens or Cinterion VID depending on FW revision */

>  	{ USB_DEVICE(SIEMENS_VENDOR_ID, CINTERION_PRODUCT_HC28_MDMNET) },

> +	{ USB_DEVICE_INTERFACE_CLASS(CINTERION_VENDOR_ID, CINTERION_PRODUCT_EXS82, 0xff) },


And keep these sorted alphabetically if possible (at least keep the
CINTERION_VENDOR_ID entries together).

>  	{ USB_DEVICE(OLIVETTI_VENDOR_ID, OLIVETTI_PRODUCT_OLICARD100),

>  	  .driver_info = RSVD(4) },

>  	{ USB_DEVICE(OLIVETTI_VENDOR_ID, OLIVETTI_PRODUCT_OLICARD120),


Care to fix that up in a v3?

Johan
Giacinto Cifelli Nov. 25, 2020, 10 a.m. UTC | #2
Hi Johan,

On Wed, Nov 25, 2020 at 10:40 AM Johan Hovold <johan@kernel.org> wrote:
>

> On Tue, Nov 24, 2020 at 10:41:55AM +0100, Giacinto Cifelli wrote:

> > There is a single option port in this modem, and it is used as debug port

> >

> > Signed-off-by: Giacinto Cifelli <gciofono@gmail.com>

>

> Thanks for the update (and thanks for the review, Lars).

>

> Using the option driver for just a debug port seems like overkill, but

> ok.

>

> Some form issues:

>

>  - When updating a patch always include a patch revision number in the

>    Subject prefix (e.g. "[PATCH v2] USB: serial: option: add ...").

>


technically this is a new patch, as I have changed the name, but I was
reserving the v2 for the actual MV31 patch that I will send in the
near future.

>  - Include a short changelog below the "---" line so we know what

>    changed since the previous version.


ok

>

>  - Try to follow the convention used by the subsystem for the Subject

>    prefix (i.e. "USB: serial: option: add ...").


ok. Should I re-submit this patch?  Do you prefer v2 or v3 for a new submit?

thank you,
Giacinto
Johan Hovold Nov. 25, 2020, 10:08 a.m. UTC | #3
On Wed, Nov 25, 2020 at 11:00:26AM +0100, Giacinto Cifelli wrote:
> Hi Johan,

> 

> On Wed, Nov 25, 2020 at 10:40 AM Johan Hovold <johan@kernel.org> wrote:

> >

> > On Tue, Nov 24, 2020 at 10:41:55AM +0100, Giacinto Cifelli wrote:

> > > There is a single option port in this modem, and it is used as debug port

> > >

> > > Signed-off-by: Giacinto Cifelli <gciofono@gmail.com>

> >

> > Thanks for the update (and thanks for the review, Lars).

> >

> > Using the option driver for just a debug port seems like overkill, but

> > ok.

> >

> > Some form issues:

> >

> >  - When updating a patch always include a patch revision number in the

> >    Subject prefix (e.g. "[PATCH v2] USB: serial: option: add ...").

> >

> 

> technically this is a new patch, as I have changed the name, but I was

> reserving the v2 for the actual MV31 patch that I will send in the

> near future.


Since it's the same PID you're adding I'd still consider this a v2 even
if you changed the product name, but I see your point.

> >  - Try to follow the convention used by the subsystem for the Subject

> >    prefix (i.e. "USB: serial: option: add ...").

> 

> ok. Should I re-submit this patch?  Do you prefer v2 or v3 for a new submit?


Yes, please do send a v3.

Johan
diff mbox series

Patch

diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 54ca85cc920d..fda291e68e4b 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -424,6 +424,7 @@  static void option_instat_callback(struct urb *urb);
 #define CINTERION_PRODUCT_AHXX_2RMNET		0x0084
 #define CINTERION_PRODUCT_AHXX_AUDIO		0x0085
 #define CINTERION_PRODUCT_CLS8			0x00b0
+#define CINTERION_PRODUCT_EXS82			0x006c
 
 /* Olivetti products */
 #define OLIVETTI_VENDOR_ID			0x0b3c
@@ -1908,6 +1909,7 @@  static const struct usb_device_id option_ids[] = {
 	{ USB_DEVICE(SIEMENS_VENDOR_ID, CINTERION_PRODUCT_HC25_MDMNET) },
 	{ USB_DEVICE(SIEMENS_VENDOR_ID, CINTERION_PRODUCT_HC28_MDM) }, /* HC28 enumerates with Siemens or Cinterion VID depending on FW revision */
 	{ USB_DEVICE(SIEMENS_VENDOR_ID, CINTERION_PRODUCT_HC28_MDMNET) },
+	{ USB_DEVICE_INTERFACE_CLASS(CINTERION_VENDOR_ID, CINTERION_PRODUCT_EXS82, 0xff) },
 	{ USB_DEVICE(OLIVETTI_VENDOR_ID, OLIVETTI_PRODUCT_OLICARD100),
 	  .driver_info = RSVD(4) },
 	{ USB_DEVICE(OLIVETTI_VENDOR_ID, OLIVETTI_PRODUCT_OLICARD120),