Re: [RFC/context] add serdev interfaces to n_gsm

[Johan Hovold <johan@kernel.org>]

[-- 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 --]