Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   =?iso-8859-1?Q?M=E5ns_Rullg=E5rd?= <mans@mansr.com>
To:     Johan Hovold <johan@kernel.org>
Cc:     =?iso-8859-1?Q?Bj=F8rn?= Mork <bjorn@mork.no>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] USB: serial: option: set driver_info for SIM5218 and compatibles
References: <20190226170710.12709-1-mans@mansr.com>
        <20190227083342.GJ4747@localhost> <yw1xpnrdv3z7.fsf@mansr.com>
        <20190227131315.GO4747@localhost> <yw1xlg21uwrp.fsf@mansr.com>
Date:   Mon, 18 Mar 2019 14:32:14 +0000
In-Reply-To: <yw1xlg21uwrp.fsf@mansr.com> (=?iso-8859-1?Q?=22M=E5ns_Rullg?=
 =?iso-8859-1?Q?=E5rd=22's?= message of "Wed,
        27 Feb 2019 14:32:58 +0000")
Message-ID: <yw1xsgvk1cep.fsf@mansr.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

M?ns Rullg?rd <mans@mansr.com> writes:

> Johan Hovold <johan@kernel.org> writes:
>
>> Adding Bj?rn.
>>
>> On Wed, Feb 27, 2019 at 11:57:16AM +0000, M?ns Rullg?rd wrote:
>>> Johan Hovold <johan@kernel.org> writes:
>>> 
>>> > On Tue, Feb 26, 2019 at 05:07:10PM +0000, Mans Rullgard wrote:
>>> >> The SIMCom SIM5218 and compatible devices have 5 USB interfaces, only 4
>>> >> of which are serial ports.  The fifth is a network interface supported
>>> >> by the qmi-wwan driver.  Furthermore, the serial ports do not support
>>> >> modem control signals.  Add driver_info flags to reflect this.
>>> >> 
>>> >> Signed-off-by: Mans Rullgard <mans@mansr.com>
>>> >> ---
>>> >>  drivers/usb/serial/option.c | 3 ++-
>>> >>  1 file changed, 2 insertions(+), 1 deletion(-)
>>> >> 
>>> >> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
>>> >> index fb544340888b..af4cbfecc3ff 100644
>>> >> --- a/drivers/usb/serial/option.c
>>> >> +++ b/drivers/usb/serial/option.c
>>> >> @@ -1066,7 +1066,8 @@ static const struct usb_device_id option_ids[] = {
>>> >>  	  .driver_info = RSVD(3) },
>>> >>  	{ USB_DEVICE(QUALCOMM_VENDOR_ID, 0x6613)}, /* Onda H600/ZTE MF330 */
>>> >>  	{ USB_DEVICE(QUALCOMM_VENDOR_ID, 0x0023)}, /* ONYX 3G device */
>>> >> -	{ USB_DEVICE(QUALCOMM_VENDOR_ID, 0x9000)}, /* SIMCom SIM5218 */
>>> >> +	{ USB_DEVICE(QUALCOMM_VENDOR_ID, 0x9000), /* SIMCom SIM5218 */
>>> >> +	  .driver_info = NCTRL(0) | NCTRL(1) | NCTRL(2) | NCTRL(3) | RSVD(4) },
>>> >>  	/* Quectel products using Qualcomm vendor ID */
>>> >>  	{ USB_DEVICE(QUALCOMM_VENDOR_ID, QUECTEL_PRODUCT_UC15)},
>>> >>  	{ USB_DEVICE(QUALCOMM_VENDOR_ID, QUECTEL_PRODUCT_UC20),
>>> >
>>> > Could you please provide the output of usb-devices (or lsusb -v) for
>>> > this device?
>>> 
>>> lsusb -v:
>>> [...]
>
>> So the patch looks fine to me. The fifth interface is QMI, but hasn't
>> been available for use until now then, and this seems to have been the
>> vendors idea from the start:
>>
>> 	http://www.microchip.ua/simcom/WCDMA/APPNOTES/SIMCom_3G_Linux_driver_Application%20Note_V1.00.pdf
>
> That document predates the qmi-wwan driver in the kernel.  Note that
> this driver has an ID table entry for interface 4 of this device.  Right
> now, whichever driver is probed first claims that interface.  I haven't
> actually tried using the QMI interface, though.
>
>> And you're seeing errors when opening ports 0-3 due to the DTR calls
>> which I guess no one noticed or cared about before?
>
> Right, some userspace tools complain about this.
>
>> Before you sent me the lsusb I searched for it and came across the below
>> thread where Bj?rn's having a go at SIMCom. In it there's output from a
>> second device using the same id but with entirely different descriptors.
>>
>> 	https://forum.openwrt.org/t/lte-wireless-module-support-by-openwrt-led-on-tplink/13586?page=3
>>
>> If this is a common theme with this vendor we may need to be extra
>> careful when making changes.
>
> Isn't this a common theme with most USB vendors, especially wireless things?
>
> Regardless, setting the NCTRL flag should be harmless.

Any further comments on this?

-- 
M?ns Rullg?rd