| Message ID | 20221108163001.18454-3-davide.tronchin.94@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4,1/3] USB: serial: option: remove old LARA-R6 PID | expand |
On 11/8/2022 23:30, Davide Tronchin wrote: > + { USB_DEVICE(UBLOX_VENDOR_ID, UBLOX_PRODUCT_LARA_L6), > + .driver_info = RSVD(4) }, Why do you blacklist interface 4 in the configuration that according to your list doesn't even have an interface 4 ? /Lars
On Tue, Nov 08, 2022 at 05:30:01PM +0100, Davide Tronchin wrote: > Add LARA-L6 PIDs for three different USB compositions. > > LARA-L6 module can be configured (by AT interface) in three different > USB modes: > * Default mode (Vendor ID: 0x1546 Product ID: 0x1341) with 4 serial > interfaces > * RmNet mode (Vendor ID: 0x1546 Product ID: 0x1342) with 4 serial > interfaces and 1 RmNet virtual network interface > * CDC-ECM mode (Vendor ID: 0x1546 Product ID: 0x1343) with 4 serial > interface and 1 CDC-ECM virtual network interface > > In default mode LARA-L6 exposes the following interfaces: > If 0: Diagnostic > If 1: AT parser > If 2: AT parser > If 3: AT parser/alternative functions > > In RmNet mode LARA-L6 exposes the following interfaces: > If 0: Diagnostic > If 1: AT parser > If 2: AT parser > If 3: AT parset/alternative functions > If 4: RMNET interface > > In CDC-ECM mode LARA-L6 exposes the following interfaces: > If 0: Diagnostic > If 1: AT parser > If 2: AT parser > If 3: AT parset/alternative functions > If 4: CDC-ECM interface > > Signed-off-by: Davide Tronchin <davide.tronchin.94@gmail.com> > --- > > V3 -> V4: as requested, the patch has been split to 3 sub-patches. > Fix comment format. > > V2 -> V3: added this section to tracking changes with previous versions. > Added some explanations about the RSVD(4) in the description session. > Added reservation to port 4 of VID:PID 0x05C6:0x908B to meet other > companies QMI net interface implementation. > > V1 -> V2: define UBLOX_PRODUCT_LARA_R6 0x908b has been deleted together > with the previosly provided definition of USB_DEVICE since the PID > is used by another vendor. > The LARA-L6 patch part is the same of the previosly provided one. > @@ -1125,6 +1130,13 @@ static const struct usb_device_id option_ids[] = { > .driver_info = RSVD(1) | RSVD(3) }, > { USB_DEVICE(QUALCOMM_VENDOR_ID, 0x908b), /* u-blox LARA-R6 00B */ > .driver_info = RSVD(4) }, > + /* u-blox products using u-blox vendor ID */ > + { USB_DEVICE(UBLOX_VENDOR_ID, UBLOX_PRODUCT_LARA_L6), > + .driver_info = RSVD(4) }, Why are you reserving interface 4 here? This should be removed or explained in the commit message. > + { USB_DEVICE(UBLOX_VENDOR_ID, UBLOX_PRODUCT_LARA_L6_RMNET), > + .driver_info = RSVD(4) }, > + { USB_DEVICE(UBLOX_VENDOR_ID, UBLOX_PRODUCT_LARA_L6_ECM), > + .driver_info = RSVD(4) }, > /* Quectel products using Quectel vendor ID */ > { USB_DEVICE_AND_INTERFACE_INFO(QUECTEL_VENDOR_ID, QUECTEL_PRODUCT_EC21, 0xff, 0xff, 0xff), > .driver_info = NUMEP2 }, Johan
On Tue, Nov 15, 2022 at 11:18:17AM +0100, Johan Hovold wrote: > On Tue, Nov 08, 2022 at 05:30:01PM +0100, Davide Tronchin wrote: > > Add LARA-L6 PIDs for three different USB compositions. > > > > LARA-L6 module can be configured (by AT interface) in three different > > USB modes: > > * Default mode (Vendor ID: 0x1546 Product ID: 0x1341) with 4 serial > > interfaces > > * RmNet mode (Vendor ID: 0x1546 Product ID: 0x1342) with 4 serial > > interfaces and 1 RmNet virtual network interface > > * CDC-ECM mode (Vendor ID: 0x1546 Product ID: 0x1343) with 4 serial > > interface and 1 CDC-ECM virtual network interface > > > > In default mode LARA-L6 exposes the following interfaces: > > If 0: Diagnostic > > If 1: AT parser > > If 2: AT parser > > If 3: AT parser/alternative functions > > @@ -1125,6 +1130,13 @@ static const struct usb_device_id option_ids[] = { > > .driver_info = RSVD(1) | RSVD(3) }, > > { USB_DEVICE(QUALCOMM_VENDOR_ID, 0x908b), /* u-blox LARA-R6 00B */ > > .driver_info = RSVD(4) }, > > + /* u-blox products using u-blox vendor ID */ > > + { USB_DEVICE(UBLOX_VENDOR_ID, UBLOX_PRODUCT_LARA_L6), > > + .driver_info = RSVD(4) }, > > Why are you reserving interface 4 here? This should be removed or > explained in the commit message. Apparently Lars already pointed this out, but again you ignored review feedback without any comment and sent a v5. Mistakes happen, but this is starting to look like more than that. > > + { USB_DEVICE(UBLOX_VENDOR_ID, UBLOX_PRODUCT_LARA_L6_RMNET), > > + .driver_info = RSVD(4) }, > > + { USB_DEVICE(UBLOX_VENDOR_ID, UBLOX_PRODUCT_LARA_L6_ECM), > > + .driver_info = RSVD(4) }, > > /* Quectel products using Quectel vendor ID */ > > { USB_DEVICE_AND_INTERFACE_INFO(QUECTEL_VENDOR_ID, QUECTEL_PRODUCT_EC21, 0xff, 0xff, 0xff), > > .driver_info = NUMEP2 }, Johan
Sorry i have not commented because i tought the latest v5 patch version was sufficient to understand. RSVD(4) has been removed because in normal mode interface 4 is not used. Davide
On Tue, Nov 15, 2022 at 05:50:00PM +0100, Davide Tronchin wrote: > Sorry i have not commented because i tought the latest v5 patch version > was sufficient to understand. > RSVD(4) has been removed because in normal mode interface 4 is not used. My apologies. I thought I was reviewing v5 and that it was still there. All good then. Just amend the commit message in 1/3, and wrap all the commit messages at 72 column or so. Johan
diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c index 5ff1b8e1e453..cbc2ba27f74f 100644 --- a/drivers/usb/serial/option.c +++ b/drivers/usb/serial/option.c @@ -580,6 +580,11 @@ static void option_instat_callback(struct urb *urb); #define OPPO_VENDOR_ID 0x22d9 #define OPPO_PRODUCT_R11 0x276c +/* These u-blox products use u-blox's vendor ID */ +#define UBLOX_VENDOR_ID 0x1546 +#define UBLOX_PRODUCT_LARA_L6 0x1341 +#define UBLOX_PRODUCT_LARA_L6_RMNET 0x1342 +#define UBLOX_PRODUCT_LARA_L6_ECM 0x1343 /* Device flags */ @@ -1125,6 +1130,13 @@ static const struct usb_device_id option_ids[] = { .driver_info = RSVD(1) | RSVD(3) }, { USB_DEVICE(QUALCOMM_VENDOR_ID, 0x908b), /* u-blox LARA-R6 00B */ .driver_info = RSVD(4) }, + /* u-blox products using u-blox vendor ID */ + { USB_DEVICE(UBLOX_VENDOR_ID, UBLOX_PRODUCT_LARA_L6), + .driver_info = RSVD(4) }, + { USB_DEVICE(UBLOX_VENDOR_ID, UBLOX_PRODUCT_LARA_L6_RMNET), + .driver_info = RSVD(4) }, + { USB_DEVICE(UBLOX_VENDOR_ID, UBLOX_PRODUCT_LARA_L6_ECM), + .driver_info = RSVD(4) }, /* Quectel products using Quectel vendor ID */ { USB_DEVICE_AND_INTERFACE_INFO(QUECTEL_VENDOR_ID, QUECTEL_PRODUCT_EC21, 0xff, 0xff, 0xff), .driver_info = NUMEP2 },
Add LARA-L6 PIDs for three different USB compositions. LARA-L6 module can be configured (by AT interface) in three different USB modes: * Default mode (Vendor ID: 0x1546 Product ID: 0x1341) with 4 serial interfaces * RmNet mode (Vendor ID: 0x1546 Product ID: 0x1342) with 4 serial interfaces and 1 RmNet virtual network interface * CDC-ECM mode (Vendor ID: 0x1546 Product ID: 0x1343) with 4 serial interface and 1 CDC-ECM virtual network interface In default mode LARA-L6 exposes the following interfaces: If 0: Diagnostic If 1: AT parser If 2: AT parser If 3: AT parser/alternative functions In RmNet mode LARA-L6 exposes the following interfaces: If 0: Diagnostic If 1: AT parser If 2: AT parser If 3: AT parset/alternative functions If 4: RMNET interface In CDC-ECM mode LARA-L6 exposes the following interfaces: If 0: Diagnostic If 1: AT parser If 2: AT parser If 3: AT parset/alternative functions If 4: CDC-ECM interface Signed-off-by: Davide Tronchin <davide.tronchin.94@gmail.com> --- V3 -> V4: as requested, the patch has been split to 3 sub-patches. Fix comment format. V2 -> V3: added this section to tracking changes with previous versions. Added some explanations about the RSVD(4) in the description session. Added reservation to port 4 of VID:PID 0x05C6:0x908B to meet other companies QMI net interface implementation. V1 -> V2: define UBLOX_PRODUCT_LARA_R6 0x908b has been deleted together with the previosly provided definition of USB_DEVICE since the PID is used by another vendor. The LARA-L6 patch part is the same of the previosly provided one. drivers/usb/serial/option.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)