Date: Tue, 19 Dec 2017 09:54:23 +0100
From: Johan Hovold <johan@kernel.org>
To: =?iso-8859-1?Q?Bj=F8rn?= Mork <bjorn@mork.no>
Cc: SZ Lin =?utf-8?B?KOael+S4iuaZuik=?= <SZ.Lin@moxa.com>,
        Johan Hovold <johan@kernel.org>,
        Taiyi TY Wu =?utf-8?B?KOWQs+azsOavhSk=?= 
        <TaiyiTY.Wu@moxa.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] USB: serial: option: adding support for YUGA CLM920-NC5
Message-ID: <20171219085423.GA8917@localhost>
References: <20171215103203.2579-1-sz.lin@moxa.com>
 <20171218165248.GF3374@localhost>
 <87lghzc1vs.fsf@miraculix.mork.no>
 <5942081F7727964D830FC876E62CA85601194755CD@MHQMBX01.moxa.com>
 <878tdzb5xi.fsf@miraculix.mork.no>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <878tdzb5xi.fsf@miraculix.mork.no>
User-Agent: Mutt/1.7.2 (2016-11-26)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1370
Lines: 34

On Tue, Dec 19, 2017 at 07:14:17AM +0100, Bjørn Mork wrote:
> "SZ Lin (林上智)" <SZ.Lin@moxa.com> writes:
> >> Johan Hovold <johan@kernel.org> writes:
> >> 
> >> >> +static const struct option_blacklist_info yuga_clm920_nc5_blacklist = {
> >> >> +	.reserved = BIT(0) | BIT(1) | BIT(4), };
> >> >
> >> > Do you really need to blacklist the first interface?
> >> 
> >> Good question. Interface #0 does look a lot like a Qualcomm DM/DIAG
> >> function, based on two bulk endpoints, no additional descriptors
> >> and the fact that it is the first interface.  If so, then we do
> >> want a serial driver for it.  There is a basic libqcdm
> >> implementation in ModemManager if you want to test it out.
> >
> >I have confirmed that interface #0 is QCDM/DIAG port in this module,
> >and thus I will remove this from reserved list in next patch.

Thanks to both of you for confirming.

> >Furthermore, interface #1 is ADB port. Should I also remove this from
> >reserved list?
> 
> No. ADB is handled by userspace tools using libusb.  It should not be
> bound to any serial driver, so you will need to blacklist it.  But you
> need to keep the blacklist anyway to include the QCDM/DIAG port
>
> I assume Johan's alternative was to match class/subclass/protocol
> against ff/00/00, which would have worked if you only wanted to include
> interfaces 2 and 3.

Indeed.

Johan