* [PATCH] option: added support for Thales Cinterion EXS82 option port.
@ 2020-11-24 9:41 Giacinto Cifelli
2020-11-25 9:40 ` Johan Hovold
0 siblings, 1 reply; 4+ messages in thread
From: Giacinto Cifelli @ 2020-11-24 9:41 UTC (permalink / raw)
To: linux-usb; +Cc: Giacinto Cifelli
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(+)
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),
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] option: added support for Thales Cinterion EXS82 option port.
2020-11-24 9:41 [PATCH] option: added support for Thales Cinterion EXS82 option port Giacinto Cifelli
@ 2020-11-25 9:40 ` Johan Hovold
2020-11-25 10:00 ` Giacinto Cifelli
0 siblings, 1 reply; 4+ messages in thread
From: Johan Hovold @ 2020-11-25 9:40 UTC (permalink / raw)
To: Giacinto Cifelli; +Cc: linux-usb, Lars Melin
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] option: added support for Thales Cinterion EXS82 option port.
2020-11-25 9:40 ` Johan Hovold
@ 2020-11-25 10:00 ` Giacinto Cifelli
2020-11-25 10:08 ` Johan Hovold
0 siblings, 1 reply; 4+ messages in thread
From: Giacinto Cifelli @ 2020-11-25 10:00 UTC (permalink / raw)
To: Johan Hovold; +Cc: linux-usb, Lars Melin
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] option: added support for Thales Cinterion EXS82 option port.
2020-11-25 10:00 ` Giacinto Cifelli
@ 2020-11-25 10:08 ` Johan Hovold
0 siblings, 0 replies; 4+ messages in thread
From: Johan Hovold @ 2020-11-25 10:08 UTC (permalink / raw)
To: Giacinto Cifelli; +Cc: Johan Hovold, linux-usb, Lars Melin
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-11-25 10:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-24 9:41 [PATCH] option: added support for Thales Cinterion EXS82 option port Giacinto Cifelli
2020-11-25 9:40 ` Johan Hovold
2020-11-25 10:00 ` Giacinto Cifelli
2020-11-25 10:08 ` Johan Hovold
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox