Hi!
> > Motorola is using a custom TS 27.010 based serial port line discipline
>
> s/serial port line discipline/multiplexer protocol/
> > diff --git a/drivers/gnss/Kconfig b/drivers/gnss/Kconfig
> > index bd12e3d57baa..a7c449d8428c 100644
> > --- a/drivers/gnss/Kconfig
> > +++ b/drivers/gnss/Kconfig
> > @@ -13,6 +13,14 @@ menuconfig GNSS
> >
> > if GNSS
> >
> > +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.
>
> Does this even still need to be a build-time dependency?
Not any more, it is now normal serdev driver, that's why I posted it
separately.
> > + Say Y here if you have a Motorola modem using TS 27.010 line
>
> s/line discipline/multiplexer protocol/
Ok.
> > +#define MOTMDM_GNSS_HEADER_LEN 5 /*
> > U1234 */
> > +#define MOTMDM_GNSS_RESP_LEN (MOTMDM_GNSS_HEADER_LEN + 4) /*
> > U1234+MPD */
> > +#define MOTMDM_GNSS_DATA_LEN (MOTMDM_GNSS_RESP_LEN + 1) /*
> > U1234~+MPD */
> > +#define MOTMDM_GNSS_STATUS_LEN (MOTMDM_GNSS_DATA_LEN + 7) /*
> > STATUS= */
> > +#define MOTMDM_GNSS_NMEA_LEN (MOTMDM_GNSS_DATA_LEN + 8) /*
> > NMEA=NN, */
>
> The comments are inconsistent; does the latter two have a "U1234"
> prefix?
It is U1234~+MPDSTATUS= and U1234~+MPDNMEA=NN, -- will fix. Hopefully
85 columns is okay with you here.
> > + /*
> > + * Firmware bug: Strip out extra data based on an
> > + * earlier line break in the data
> > + */
> > + if (msg[msglen - 5 - 1] == 0x0a)
> > + msglen -= 5;
> > +
> > + error = gnss_insert_raw(gdev, msg, msglen);
> > + WARN_ON(error);
>
> The return value is not an "error" but the number of queued bytes.
>
> So that WARN_ON(error) makes it look like you never even tested this?
Well, I did test it and it works. Unfortunately Droid 4 produces lot
of output during normal operation, including periodic WARNs, so I
missed that. Sorry about that.
> > + error = serdev_device_open(ddata->serdev);
> > + if (error) {
> > + return error;
> > + }
>
> Nit: drop the brackets.
Ok.
> > + error = motmdm_gnss_init(gdev);
> > + if (error) {
>
> You must close the port before returning.
Ok.
> > +static int motmdm_gnss_write_raw(struct gnss_device *gdev,
> > + const unsigned char *buf,
> > + size_t count)
> > +{
> > + struct motmdm_gnss_data *ddata = gnss_get_drvdata(gdev);
> > +
> > + return serdev_device_write(ddata->serdev, buf, count,
> > MAX_SCHEDULE_TIMEOUT);
> > + serdev_device_wait_until_sent(ddata->serdev, 0);
>
> This code is never reached.
Fixed.
I'll get new version out. I'll also get serdev/ngsm patch out for
context, but that one needs more work.
Best regards,
Pavel
--
http://www.livejournal.com/~pavelmachek
signature.asc
Description: PGP signature
