mbox series

[PATCHv6,0/6] n_gsm serdev support and GNSS driver for droid4

Message ID 20200430174615.41185-1-tony@atomide.com (mailing list archive)
Headers show
Series n_gsm serdev support and GNSS driver for droid4 | expand

Message

Tony Lindgren April 30, 2020, 5:46 p.m. UTC
Hi all,

Here's v6 set of these patches fixed up for Johan's earlier comments.

Getting rid of the custom chardev interface, and making things more
generic also simplified things quite a bit. Thanks a lot Johan :)

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

4. Adds a serdev-ngsm 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. So it now 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 jsing 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
maybe the drivers/i2c/busses analogy :) Please do suggest better
locations other than MFD and misc if you have better ideas.

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.

If we still really need the custom chardev support, that can now
be added as needed with the channel specific consumer driver(s).

My guess is that at least with the pending ofono patches, we just
want to use the raw interface for /dev/gsmtty* interface and stop
pretending we have a modem that is AT compatible.

Regards,

Tony


Changes since v5:
- Based on comments from Johan, moved back to using the existing
  TS 27.010 TTYs created by n_gsm.c instaed of adding custom chardev
  support to deal with the Motorola custom protocol

- Based on comments from Johan, made the serdev-ngsm driver generic
  with just minimal quirk handling for the Motorola modem

- Dropped the Motorola custom protocol on top of TS 27.010 handling
  from serdev-ngsm.c as this can now be easily handled by the channel
  specific drivers as needed

- Added few more helpers to n_gsm.c for serdev-ngsm.c to use

- Added the channel specific GNSS driver for the Motorola modem

Changes since v4:
- Use drivers/tty/serdev/protocol directory for the driver instead of
  drivers/mfd as discussed on the lists for v3 set of patches
- Fix remove to call kfree only after removing device from the list

Changes since v3:
- Update list of folks in Cc, looks like I sent v3 only to Lee and lkml
- Init privdata before motmdm_register_dlci calls gsm_serdev_register_dlci
- Update binding based on Rob's comments for license and "allOf"

Changes since v2:
- Drop useless send_command indirection, use static motmdm_send_command

Changes since v1:

- Simplified usage and got rid of few pointless inline functions
- Added consumer MFD driver, devicetree binding, and dts changes


Tony Lindgren (6):
  tty: n_gsm: Add support for serdev drivers
  dt-bindings: serdev: ngsm: Add binding for serdev-ngsm
  serdev: ngsm: Add generic serdev-ngsm driver
  dt-bindings: gnss: Add binding for Motorola Mapphone MDM6600 GNSS
  gnss: motmdm: Add support for Motorola Mapphone MDM6600 modem
  ARM: dts: omap4-droid4: Configure modem for serdev-ngsm

 .../devicetree/bindings/gnss/motmdm.yaml      |  29 ++
 .../bindings/serdev/serdev-ngsm.yaml          |  64 +++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 .../boot/dts/motorola-mapphone-common.dtsi    |  14 +
 drivers/gnss/Kconfig                          |   8 +
 drivers/gnss/Makefile                         |   3 +
 drivers/gnss/motmdm.c                         | 419 ++++++++++++++++
 drivers/tty/n_gsm.c                           | 428 +++++++++++++++++
 drivers/tty/serdev/Kconfig                    |  10 +
 drivers/tty/serdev/Makefile                   |   1 +
 drivers/tty/serdev/serdev-ngsm.c              | 449 ++++++++++++++++++
 include/linux/serdev-gsm.h                    | 163 +++++++
 12 files changed, 1590 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gnss/motmdm.yaml
 create mode 100644 Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml
 create mode 100644 drivers/gnss/motmdm.c
 create mode 100644 drivers/tty/serdev/serdev-ngsm.c
 create mode 100644 include/linux/serdev-gsm.h

Comments

Pavel Machek April 30, 2020, 10:26 p.m. UTC | #1
> My guess is that at least with the pending ofono patches, we just
> want to use the raw interface for /dev/gsmtty* interface and stop
> pretending we have a modem that is AT compatible.

I tried to get it to work... it was not fun and I did not get far.

I pushed my results...

user@devuan:/my/ofono$ git push
Counting objects: 10, done.
Delta compression using up to 2 threads.
Compressing objects: 100% (10/10), done.
Writing objects: 100% (10/10), 1.17 KiB | 0 bytes/s, done.
Total 10 (delta 8), reused 0 (delta 0)
remote: Resolving deltas: 100% (8/8), completed with 8 local objects.
To github.com:pavelmachek/ofono.git
   fd34ca20..9042014b  mux-v1.29-1 -> mux-v1.29-1

Best regards,
								Pavel

ofonod[12922]: drivers/atmodem/network-registration.c:at_netreg_probe() Probing creg
Have command of length 9 (AT+CREG=?)
 or ^z 
ofonod[12922]: Voice: > U0000AT+FOO\r
ofonod[12922]: InSMS: > U0000AT+FOO\r
ofonod[12922]: OutSMS: > U0000AT+FOO\r
ofonod[12922]: Voice: < U0000+FOO:ERROR=9\n
new bytes 18
Have line: U0000+FOO:ERROR=9
Last character is 13
command response: U0000+FOO:ERROR=9
ofonod[12922]: plugins/motmdm.c:foo_cb() 
handle command response
ofonod[12922]: Voice: > U0000AT+SCRN=0\r
ofonod[12922]: OutSMS: < U0000+FOO:ERROR=9\n
new bytes 18
Have line: U0000+FOO:ERROR=9
Last character is 13
command response: U0000+FOO:ERROR=9
ofonod[12922]: plugins/motmdm.c:foo_cb() 
handle command response
ofonod[12922]: InSMS: < U0000+FOO:ERROR=9\n
new bytes 18
Have line: U0000+FOO:ERROR=9
Last character is 13
command response: U0000+FOO:ERROR=9
ofonod[12922]: plugins/motmdm.c:foo_cb() 
ofonod[12922]: plugins/motmdm.c:foo_cb() All channels working
handle command response
ofonod[12922]: Voice: < U0000+SCRN:OK\n
new bytes 14
Have line: U0000+SCRN:OK
Last character is 13
command response: U0000+SCRN:OK
ofonod[12922]: plugins/motmdm.c:scrn_cb() 
handle command response
ofonod[12922]: Voice: > U0000AT+CFUN=1\r
ofonod[12922]: Voice: < U0000+CFUN:OK\n
new bytes 14
Have line: U0000+CFUN:OK
Last character is 13
command response: U0000+CFUN:OK
ofonod[12922]: plugins/motmdm.c:cfun_cb() 
handle command response
ofonod[12922]: Voice: > U0000AT+CLIP=1\r
ofonod[12922]: Voice: < U0000+CLIP:OK\n
new bytes 14
Have line: U0000+CLIP:OK
Last character is 13
command response: U0000+CLIP:OK
handle command response
ofonod[12922]: Voice: > U0000AT+CCWA=1\r
ofonod[12922]: Voice: < U0000+CCWA:OK\n
new bytes 14
Have line: U0000+CCWA:OK
Last character is 13
command response: U0000+CCWA:OK
ofonod[12922]: drivers/motorolamodem/voicecall.c:motorola_voicecall_initialized() voicecall_init: registering to notifications
Pavel Machek May 1, 2020, 8:21 a.m. UTC | #2
Hi!

> 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.

Actually... yes, this works. But no, this is not "existing" tty
interface.

ttys work per character, and this interface definitely does not... it
is "packet" based, write() syscalls need exactly right lengths. You
can't just open minicom, and type "U1234...". You can't paste it,
either (I tried). tty controls like start/stop bits and baud rate are
useless here. CR/LF conversions are unwanted/dangerous because it is
confusing hard to debug if you get them wrong.

Now, I don't see reason why this could not be made to work, and it may
be more important to have something in mainline and work with that. So
if you can make this into -next, I'll not complain too loudly. But it
is... still wrong and I liked motmdm* more :-).

Best regards,

									Pavel
Tony Lindgren May 1, 2020, 2:20 p.m. UTC | #3
* Pavel Machek <pavel@denx.de> [200501 08:22]:
> Hi!
> 
> > 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.
> 
> Actually... yes, this works. But no, this is not "existing" tty
> interface.
> 
> ttys work per character, and this interface definitely does not... it
> is "packet" based, write() syscalls need exactly right lengths. You
> can't just open minicom, and type "U1234...". You can't paste it,
> either (I tried). tty controls like start/stop bits and baud rate are
> useless here. CR/LF conversions are unwanted/dangerous because it is
> confusing hard to debug if you get them wrong.

Yes.. That's what n_gsm spins up.

> Now, I don't see reason why this could not be made to work, and it may
> be more important to have something in mainline and work with that. So
> if you can make this into -next, I'll not complain too loudly. But it
> is... still wrong and I liked motmdm* more :-).

Yes.. There are issues too with the motmdm* char dev interface too.

I don't think it would work as is for devices with network interfaces
on some channel, and continuation packets could need more handling
possibly.

But let's try to get the basics sorted out first and use the "raw"
gsmtty* interface. More stuff can always be added later as needed.

Regards,

Tony
Tony Lindgren May 1, 2020, 2:52 p.m. UTC | #4
* Pavel Machek <pavel@denx.de> [200430 22:27]:
> 
> > My guess is that at least with the pending ofono patches, we just
> > want to use the raw interface for /dev/gsmtty* interface and stop
> > pretending we have a modem that is AT compatible.
> 
> I tried to get it to work... it was not fun and I did not get far.

OK. Yeah it's now 2020 and still dealing with serial port stuff :)

> I pushed my results...
> 
> user@devuan:/my/ofono$ git push
> Counting objects: 10, done.
> Delta compression using up to 2 threads.
> Compressing objects: 100% (10/10), done.
> Writing objects: 100% (10/10), 1.17 KiB | 0 bytes/s, done.
> Total 10 (delta 8), reused 0 (delta 0)
> remote: Resolving deltas: 100% (8/8), completed with 8 local objects.
> To github.com:pavelmachek/ofono.git
>    fd34ca20..9042014b  mux-v1.29-1 -> mux-v1.29-1

OK :) I still need to update the ALSA related patches on top
of this $subject series.

Also what I've noticed is that modprobe n_gsm debug=0xff hex output is
currently broken since commit 091cb0994edd ("lib/hexdump: make
print_hex_dump_bytes() a nop on !DEBUG builds"). Reverting the commit
fixes it.

Stephen, any ideas what should be changed to fix it?

Regards,

Tony
Pavel Machek May 1, 2020, 8:19 p.m. UTC | #5
Hi!

> My guess is that at least with the pending ofono patches, we just
> want to use the raw interface for /dev/gsmtty* interface and stop
> pretending we have a modem that is AT compatible.

Ok, so I got ofono back to work. ... I believe. It was not that
bad. SMS send/receive and outgoing call start/hangup worked at some
point (I did not play with mixers).

To github.com:pavelmachek/ofono.git
   61d3d727..195760e9  mux-v1.29-1 -> mux-v1.29-1

Best regards,
									Pavel
Pavel Machek May 1, 2020, 8:23 p.m. UTC | #6
Hi!

> 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.
> 
> If we still really need the custom chardev support, that can now
> be added as needed with the channel specific consumer driver(s).
> 
> My guess is that at least with the pending ofono patches, we just
> want to use the raw interface for /dev/gsmtty* interface and stop
> pretending we have a modem that is AT compatible.

The series:

Tested-by: Pavel Machek <pavel@ucw.cz>
Tony Lindgren May 1, 2020, 8:32 p.m. UTC | #7
* Pavel Machek <pavel@denx.de> [200501 20:20]:
> Hi!
> 
> > My guess is that at least with the pending ofono patches, we just
> > want to use the raw interface for /dev/gsmtty* interface and stop
> > pretending we have a modem that is AT compatible.
> 
> Ok, so I got ofono back to work. ... I believe. It was not that
> bad. SMS send/receive and outgoing call start/hangup worked at some
> point (I did not play with mixers).
> 
> To github.com:pavelmachek/ofono.git
>    61d3d727..195760e9  mux-v1.29-1 -> mux-v1.29-1

OK good to hear and thanks for doing it.

Regards,

Tony
Pavel Machek May 1, 2020, 10:06 p.m. UTC | #8
On Fri 2020-05-01 07:52:52, Tony Lindgren wrote:
> * Pavel Machek <pavel@denx.de> [200430 22:27]:
> > 
> > > My guess is that at least with the pending ofono patches, we just
> > > want to use the raw interface for /dev/gsmtty* interface and stop
> > > pretending we have a modem that is AT compatible.
> > 
> > I tried to get it to work... it was not fun and I did not get far.
> 
> OK. Yeah it's now 2020 and still dealing with serial port stuff :)

Yeah, and scary thing is... it is 2020 and serial port is _still_
complex and hard to understand and debug :-).

> OK :) I still need to update the ALSA related patches on top
> of this $subject series.

Let me know when you have these.

								Pavel