Phone-Devel Archive on lore.kernel.org.
 help / color / Atom feed
From: Johan Hovold <johan@kernel.org>
To: Pavel Machek <pavel@ucw.cz>
Cc: Greg KH <greg@kroah.com>,
	kernel list <linux-kernel@vger.kernel.org>,
	phone-devel@vger.kernel.org, tony@atomide.com
Subject: Re: [RFC/context] add serdev interfaces to n_gsm
Date: Thu, 11 Feb 2021 10:04:42 +0100
Message-ID: <YCTzKm+70jwqkdLK@hovoldconsulting.com> (raw)
In-Reply-To: <20210210212836.GA18497@duo.ucw.cz>


[-- Attachment #1: Type: text/plain, Size: 1770 bytes --]

On Wed, Feb 10, 2021 at 10:28:36PM +0100, Pavel Machek wrote:
> Hi!
> 
> > > > +config GNSS_MOTMDM
> > > > +	tristate "Motorola Modem TS 27.010 serdev GNSS receiver support"
> > > > +	depends on SERIAL_DEV_N_GSM
> > > 
> > > You need to post this driver together with the serdev-ngsm driver. This
> > > one cannot currently even be built without it, but we also need to see
> > > the greater picture here.
> > 
> > Well, here it is, for greater picture. But it is not ready. Current
> > problem I have is gsm_serdev_register_tty_port(). The way I do
> > platform device registration causes oops on module unload. Help with
> > that would be welcome
> 
> I would not mind comments on parent patch and some help here.
> 
> Basically I tried to work around limitation in 
> 
> int serdev_device_add(struct serdev_device *serdev)
> {
> ...
>        /* Only a single slave device is currently supported. */
>        if (ctrl->serdev) {
> ...

I haven't really had time to look at the code yet, but trying to work
around the single-client (slave) assumption seems wrong. You still have
only one client per port even if the mux driver provides multiple
(virtual) ports.

But judging from a quick look it appears that you are indeed registering
one tty device per mux channel in gsm_serdev_register_tty_port() (as you
should) so perhaps that's not the issue here.

Do you have a stack trace from the oops? Are the client drivers holding
the ports open while you unload the parent driver? That sounds like
something which could go boom unless you pin the parent for example
(serdev doesn't support hangups).

Also, did you forget to post the gsm_tty_driver implementation? I don't
see a definition of that symbol in the patch.

Johan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

  reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-07 22:45 [PATCH] gnss: motmdm: Add support for Motorola Mapphone MDM6600 modem Pavel Machek
2021-01-28 18:34 ` Pavel Machek
     [not found] ` <YBQvvUitX4MtRrh+@hovoldconsulting.com>
2021-01-29 22:34   ` Pavel Machek
     [not found]   ` <20210131170639.GA21067@duo.ucw.cz>
2021-02-10 21:28     ` [RFC/context] add serdev interfaces to n_gsm Pavel Machek
2021-02-11  9:04       ` Johan Hovold [this message]
2021-02-19 22:06         ` Pavel Machek
2021-01-29 22:42 ` [PATCHv2] gnss: motmdm: Add support for Motorola Mapphone MDM6600 modem Pavel Machek
2021-02-28 20:46   ` Pavel Machek
2021-04-01  9:39     ` Johan Hovold
2021-04-07 10:52       ` Pavel Machek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YCTzKm+70jwqkdLK@hovoldconsulting.com \
    --to=johan@kernel.org \
    --cc=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pavel@ucw.cz \
    --cc=phone-devel@vger.kernel.org \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Phone-Devel Archive on lore.kernel.org.

Archives are clonable:
	git clone --mirror https://lore.kernel.org/phone-devel/0 phone-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 phone-devel phone-devel/ https://lore.kernel.org/phone-devel \
		phone-devel@vger.kernel.org
	public-inbox-index phone-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.phone-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git