linux-kernel - Re: [PATCHv2] gnss: motmdm: Add support for Motorola Mapphone MDM6600 modem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20210407105255.GA15534@amd>
Date:   Wed, 7 Apr 2021 12:52:55 +0200
From:   Pavel Machek <pavel@....cz>
To:     Johan Hovold <johan@...nel.org>
Cc:     kernel list <linux-kernel@...r.kernel.org>,
        phone-devel@...r.kernel.org, tony@...mide.com
Subject: Re: [PATCHv2] gnss: motmdm: Add support for Motorola Mapphone
 MDM6600 modem

Hi!

> > Could I get some comments on the gnss patch? It is fairly simple, and
> > I believe it is ready for merge.
> 
> Let's get the mux driver in shape first.

Well, gnss driver is now completely independent of that in the new
versions.

> > Here's new version of the serdev multiplexing patch for reference. And
> > yes, I'd like you to review the design here, too. Yes,
> > gsm_serdev_register_tty_port() needs to be cleaned up, but with ifdefs
> > you can see alternative approach I tried to work around "controller
> > busy" problem.
> 
> As I said before, if you're trying to work around the one
> client-per-port assumption of serdev, you're doing it wrong. You still
> have one client per *virtual* port.

Yes, I have one client per virtual port. But those virtual ports need
their parents.

We can have

serial@ -- motorola,mapphone-mdm6600-serial
(serdev here)
modem -- ts27010-mux
(here is the problem!)
serial@4 --                                      serial@1 -- 
(serdev here)                                    (serdev here)
gnss -- motorola,mapphone-mdm6600-gnss           modem voice -- something

and I believe we agree on that. But serial@1 and serial@4 still need
some kind of parent, and that's where I'm getting the controller busy
problem.

> > +++ b/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml
> 
> > +  ttymask:
> > +    $ref: /schemas/types.yaml#/definitions/uint64
> > +    description: Mask of the TS 27.010 channel TTY interfaces to start (64 bit)
> 
> How is this intended to be used? Looks too Linux-specific for a binding.

The mask says which virtual ports contain "something". We have kernel
clients for some, and we want to expose the rest of the used ones to
the userspace.

> So is this is probably the issue: you're skipping one level of the
> topology and do not describe the virtual ports.

Well, docs need to be updated.

> You should probably just replace the ttymask property with explicit
> nodes for the virtual ports to enable. With no client devices described
> in devicetree, these will then show up as regular tty devices under
> Linux.

Yes, that can be done.

> If you add a compatible property for the virtual ports (e.g.
> "ts27010-port") you might be able to just use of_platform_populate()
> instead of walking the child nodes in gsm_serdev_register_tty_port().

I have "gsmmux,port" for that. And yes, I can take a look at
of_platform_populate.

But as I explained above, there's still problem with the parent
devices.

Best regards,
								Pavel
-- 
http://www.livejournal.com/~pavelmachek

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)