Re: [PATCHv8 0/6] n_gsm serdev support and GNSS driver for droid4

From: Johan Hovold <johan@kernel.org>
To: Tony Lindgren <tony@atomide.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Johan Hovold <johan@kernel.org>, Rob Herring <robh@kernel.org>,
	Alan Cox <gnomes@lxorguk.ukuu.org.uk>,
	Lee Jones <lee.jones@linaro.org>, Jiri Slaby <jslaby@suse.cz>,
	Merlijn Wajer <merlijn@wizzup.org>, Pavel Machek <pavel@ucw.cz>,
	Peter Hurley <peter@hurleysoftware.com>,
	Sebastian Reichel <sre@kernel.org>,
	linux-serial@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org
Subject: Re: [PATCHv8 0/6] n_gsm serdev support and GNSS driver for droid4
Date: Thu, 28 May 2020 10:39:18 +0200	[thread overview]
Message-ID: <20200528083918.GB10358@localhost> (raw)
In-Reply-To: <20200512214713.40501-1-tony@atomide.com>

On Tue, May 12, 2020 at 02:47:07PM -0700, Tony Lindgren wrote:
> Hi all,
> 
> Here's the updated set of these patches fixed up for Johan's and
> Pavel's earlier comments.
> 
> This series does the following:
> 
> 1. Adds functions to n_gsm.c for serdev-ngsm.c driver to use
> 
> 2. Adds a generic serdev-ngsm.c driver that brings up the TS 27.010
>    TTY ports configured in devicetree with help of n_gsm.c
> 
> 3. Allows the use of standard Linux device drivers for dedicated
>    TS 27.010 channels for devices like GNSS and ALSA found on some
>    modems for example

Unfortunately that does not seem to be the case just yet. Your gnss
driver is still aware that it's using n_gsm for the transport and calls
into the "parent" serdev-ngsm driver instead of using the serdev
interface (e.g. as if this was still and MFD driver).

If you model this right, the GNSS driver should work equally well
regardless of whether you use the serial interface (with n_gsm) or USB
(e.g. cdc-acm or usb-serial).

> 4. Adds a gnss-motmdm consumer driver for the GNSS device found on
>    the Motorola Mapphone MDM6600 modem on devices like droid4
> 
> I've placed the serdev-ngsm.c under drivers/tty/serdev as it still
> seems to make most sense with no better places available. It's no
> longer an MFD driver as it really does not need to care what channel
> specific consumer drivers might be configured for the generic driver.
> Now serdev-ngsm just uses of_platform_populate() to probe whatever
> child nodes it might find.
>
> I'm not attached having the driver in drivers/tty/serdev. I just
> don't have any better locations in mind. So using Johan's earlier
> i2c example, the drivers/tty/serdev/serdev-ngsm.c driver is now a
> generic protocol and bus driver, so it's getting closer to the
> the drivers/i2c/busses analogy maybe :) Please do suggest better
> locations other than MFD and misc if you have better ideas.

Please move it up one level to drivers/tty where the n_gsm line
discipline lives. This is (supposed to be) a tty driver exposing tty
devices.

> Now without the chardev support, the /dev/gsmtty* using apps need
> to use "U1234AT+CFUN?" format for the packets. The advantage is
> less kernel code, and we keep the existing /dev/gsmtty* interface.

Would it not be possible to deal with this in a plugin sort of way,
again, similar to how the hci ldisc work with an additional "protocol"
ioctl?

Johan