Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 24 Jul 2022 16:46:59 +0200
From:   Johan Hovold <johan@kernel.org>
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     Yan Xinyu <sdlyyxy@bupt.edu.cn>, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] USB: serial: usb_wwan: replace DTR/RTS magic numbers
 with macros
Message-ID: <Yt1bY74PirDyuYcu@hovoldconsulting.com>
References: <20220722085040.704885-1-sdlyyxy@bupt.edu.cn>
 <Yt0MfqQQTwe4ztuN@hovoldconsulting.com>
 <Yt1OPKNlWZuMrZv4@kroah.com>
 <Yt1SFXlwbW1JCohE@hovoldconsulting.com>
 <Yt1UG058Qjsngps6@kroah.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Yt1UG058Qjsngps6@kroah.com>
Precedence: bulk

On Sun, Jul 24, 2022 at 04:15:55PM +0200, Greg Kroah-Hartman wrote:
> On Sun, Jul 24, 2022 at 04:07:17PM +0200, Johan Hovold wrote:
> > On Sun, Jul 24, 2022 at 03:50:52PM +0200, Greg Kroah-Hartman wrote:

> > > I think Yan should write a patch series to unify these and make it
> > > right, instead of just papering over it all.
> > 
> > Ok, I just fear it will be more work for us since that involves
> > decisions like whether it should be added to the uapi header, and then
> > we get into naming, etc. But we're in no rush.
> > 
> > > Also this "../" stuff in a
> > > #include directive is not ok, I wouldn't recommend this change be taken
> > > as-is.
> > 
> > That was never an option, but I'd be ok with taking the v2 which added
> > defines for the constants directly in the driver.
> 
> These are global defines, in a public spec, and they should all just be
> in one place in the kernel.  What's wrong with include/uapi/linux/cdc.h
> which is where the other ACM defined values are at?

Nothing. We'd just need to figure out how best to name them if they're
going to become UAPI, that's all (e.g. at least use a USB_CDC_ prefix to
match the other defines).

And the in-tree users would need to be updated to match.

And it's not just the control lines. We have the state notification bits
as well. Someone would need to dig out the spec.

So we go from accepting a small clean-up patch to some non-trivial
tree-wide work and review. But sure, bring it on.

Johan