Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Pavel Machek]

Hi!

> > No, we need some kind of at least rudimentary gps framework even if we
> > allow for a raw (NMEA) interface for the time being (possibly
> > indefinitely).
> 
> Ok, that would be fine if we can get that!
> 
> For a minimal set of API I think something like this (following hci_dev) 
> would suffice:
> 
> struct gps_dev {
>   ...
>   int (*open)(struct gps_dev *gdev);
>   int (*close)(struct gps_dev *gdev);
>   int (*send)(struct gps_dev *gdev, char *data, int length);
> };
> 
> int gps_register_dev(struct gps_dev *gdev);
> void gps_unregister_dev(struct gps_dev *gdev);
> int gps_recv_nmea_chars(struct gps_dev *gdev, char *data, int length);
> 
> If that would wrap all creation of some /dev/ttyGPS0 (or however it is 
> called),
> it would fit our needs for a driver and user-space for our system.
> 
> And I would be happy to get rid of creating and registering a /dev/ttyGPS0
> in the w2sg0004 driver.

Sounds like a good start.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Pavel Machek]

Hi!

> > No, we need some kind of at least rudimentary gps framework even if we
> > allow for a raw (NMEA) interface for the time being (possibly
> > indefinitely).
> 
> Ok, that would be fine if we can get that!
> 
> For a minimal set of API I think something like this (following hci_dev) 
> would suffice:
> 
> struct gps_dev {
>   ...
>   int (*open)(struct gps_dev *gdev);
>   int (*close)(struct gps_dev *gdev);
>   int (*send)(struct gps_dev *gdev, char *data, int length);
> };
> 
> int gps_register_dev(struct gps_dev *gdev);
> void gps_unregister_dev(struct gps_dev *gdev);
> int gps_recv_nmea_chars(struct gps_dev *gdev, char *data, int length);
> 
> If that would wrap all creation of some /dev/ttyGPS0 (or however it is 
> called),
> it would fit our needs for a driver and user-space for our system.
> 
> And I would be happy to get rid of creating and registering a /dev/ttyGPS0
> in the w2sg0004 driver.

Sounds like a good start.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[H. Nikolaus Schaller]

Hi Johan,

> Am 19.03.2018 um 14:54 schrieb Johan Hovold :
> 
> On Tue, Feb 27, 2018 at 08:32:50AM +0100, H. Nikolaus Schaller wrote:
>> Hi Johan,
>> 
>>> Am 27.02.2018 um 08:04 schrieb Johan Hovold :
>>> 
>>> On Mon, Feb 12, 2018 at 04:26:18PM +0100, Pavel Machek wrote:
 Hi!
 
>> Let's restart this discussion and focus on the main roadblock (others
>> are minor details which can be sorted out later).
>> 
>> If it feels like a hack, the key issue seems to me to be the choice of
>> the API to present the GPS data to user space. Right?
> 
> Or even more fundamentally, does this belong in the kernel at all?
 
 Yes, it does.
>> 
>> Thanks, Pavel for supporting our view.
>> 
>>> 
>>> But not necessarily in its current form.
>> 
>> Is this a "yes after some code fixes"?
> 
> No, we need some kind of at least rudimentary gps framework even if we
> allow for a raw (NMEA) interface for the time being (possibly
> indefinitely).

Ok, that would be fine if we can get that!

For a minimal set of API I think something like this (following hci_dev) would 
suffice:

struct gps_dev {
...
int (*open)(struct gps_dev *gdev);
int (*close)(struct gps_dev *gdev);
int (*send)(struct gps_dev *gdev, char *data, int length);
};

int gps_register_dev(struct gps_dev *gdev);
void gps_unregister_dev(struct gps_dev *gdev);
int gps_recv_nmea_chars(struct gps_dev *gdev, char *data, int length);

If that would wrap all creation of some /dev/ttyGPS0 (or however it is called),
it would fit our needs for a driver and user-space for our system.

And I would be happy to get rid of creating and registering a /dev/ttyGPS0
in the w2sg0004 driver.

Then, the driver will not need to be touched if the GPS framework is improved
in some far future (e.g. to provide some additional ioctl for getting 
kalman-filtered
position+heading by doing sensor fusion with some iio-based 
accelerometer/gyro). 

So I am looking forward to some framework for review and integration testing.

BR and thanks,
Nikolaus

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[H. Nikolaus Schaller]

Hi Johan,

> Am 19.03.2018 um 14:54 schrieb Johan Hovold :
> 
> On Tue, Feb 27, 2018 at 08:32:50AM +0100, H. Nikolaus Schaller wrote:
>> Hi Johan,
>> 
>>> Am 27.02.2018 um 08:04 schrieb Johan Hovold :
>>> 
>>> On Mon, Feb 12, 2018 at 04:26:18PM +0100, Pavel Machek wrote:
 Hi!
 
>> Let's restart this discussion and focus on the main roadblock (others
>> are minor details which can be sorted out later).
>> 
>> If it feels like a hack, the key issue seems to me to be the choice of
>> the API to present the GPS data to user space. Right?
> 
> Or even more fundamentally, does this belong in the kernel at all?
 
 Yes, it does.
>> 
>> Thanks, Pavel for supporting our view.
>> 
>>> 
>>> But not necessarily in its current form.
>> 
>> Is this a "yes after some code fixes"?
> 
> No, we need some kind of at least rudimentary gps framework even if we
> allow for a raw (NMEA) interface for the time being (possibly
> indefinitely).

Ok, that would be fine if we can get that!

For a minimal set of API I think something like this (following hci_dev) would 
suffice:

struct gps_dev {
...
int (*open)(struct gps_dev *gdev);
int (*close)(struct gps_dev *gdev);
int (*send)(struct gps_dev *gdev, char *data, int length);
};

int gps_register_dev(struct gps_dev *gdev);
void gps_unregister_dev(struct gps_dev *gdev);
int gps_recv_nmea_chars(struct gps_dev *gdev, char *data, int length);

If that would wrap all creation of some /dev/ttyGPS0 (or however it is called),
it would fit our needs for a driver and user-space for our system.

And I would be happy to get rid of creating and registering a /dev/ttyGPS0
in the w2sg0004 driver.

Then, the driver will not need to be touched if the GPS framework is improved
in some far future (e.g. to provide some additional ioctl for getting 
kalman-filtered
position+heading by doing sensor fusion with some iio-based 
accelerometer/gyro). 

So I am looking forward to some framework for review and integration testing.

BR and thanks,
Nikolaus

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Johan Hovold]

On Thu, Mar 08, 2018 at 07:16:44AM +0100, Andreas Kemnade wrote:
> Hi,
> 
> On Thu, 18 Jan 2018 17:47:36 +1100
> Johan Hovold  wrote:
> 
> [...]
> > > 
> > > So to avoid having hardware information spread all over the table at least
> > > these information would need to be in devicetree. But that also all feels
> > > like a hack and hard to maintain.  
> > 
> > Having the device described in the device tree is certainly desirable,
> > not least for chip identification. And with a GPS framework in the
> > kernel with a well-defined interface, implementing power management
> > would be straight forward.
> > 
> Hmm, devicetree without in-kernel drivers, do we have anything like that
> somewhere? I thought that was a big no-go. But maybe I am wrong.

No, that's probably not a good idea, even if it may be possible (what
about ACPI then, for example?).

> > I'm just not convinced that the proposed tty interface is the right
> > interface for this. User space would still rely on gpsd for the GPS
> > protocols, and would also ultimately be managing power by killing gpsd
> > or whatever daemon that would otherwise be holding the port open.
> > 
> > Something like the generic power sequences that has been discussed
> > elsewhere might be a better fit for this if all you want to do is power
> > on and off on port open and close (and on suspend/resume). There really
> > isn't anything GPS-specific in the current proposal (besides the
> > suggested tty-device name).
> 
> So a bit like that mmc-powerseq stuff we already have?

Yeah, the generic power sequence patches were inspired by that and
intended to generalise it (e.g. to be used by the USB bus to power on
devices so that they could be enumerated). There were some issues with
that work though (which also precludes it from being used for something
like this), and it still wouldn't be sufficient to deal with the gps
device in question (which needs to monitor the incoming data).

> > But sure, that wouldn't be sufficient to deal with the
> > unknown-power-state problem with the device in question.
>
> Maybe there could be a kind of active flag set by the tty if
> there is traffic, so that active flag could be used in these
> power sequence stuff? But then again the tty layer has to be extended
> which would probably also cause a lot of ruffled feathers.

Yeah, I think this is a dead end. We need some kind of gps framework
with drivers that can implement the device specific bits.

I may have some time to look at little closer at it this week.

Johan

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Johan Hovold]

On Thu, Mar 08, 2018 at 07:16:44AM +0100, Andreas Kemnade wrote:
> Hi,
> 
> On Thu, 18 Jan 2018 17:47:36 +1100
> Johan Hovold  wrote:
> 
> [...]
> > > 
> > > So to avoid having hardware information spread all over the table at least
> > > these information would need to be in devicetree. But that also all feels
> > > like a hack and hard to maintain.  
> > 
> > Having the device described in the device tree is certainly desirable,
> > not least for chip identification. And with a GPS framework in the
> > kernel with a well-defined interface, implementing power management
> > would be straight forward.
> > 
> Hmm, devicetree without in-kernel drivers, do we have anything like that
> somewhere? I thought that was a big no-go. But maybe I am wrong.

No, that's probably not a good idea, even if it may be possible (what
about ACPI then, for example?).

> > I'm just not convinced that the proposed tty interface is the right
> > interface for this. User space would still rely on gpsd for the GPS
> > protocols, and would also ultimately be managing power by killing gpsd
> > or whatever daemon that would otherwise be holding the port open.
> > 
> > Something like the generic power sequences that has been discussed
> > elsewhere might be a better fit for this if all you want to do is power
> > on and off on port open and close (and on suspend/resume). There really
> > isn't anything GPS-specific in the current proposal (besides the
> > suggested tty-device name).
> 
> So a bit like that mmc-powerseq stuff we already have?

Yeah, the generic power sequence patches were inspired by that and
intended to generalise it (e.g. to be used by the USB bus to power on
devices so that they could be enumerated). There were some issues with
that work though (which also precludes it from being used for something
like this), and it still wouldn't be sufficient to deal with the gps
device in question (which needs to monitor the incoming data).

> > But sure, that wouldn't be sufficient to deal with the
> > unknown-power-state problem with the device in question.
>
> Maybe there could be a kind of active flag set by the tty if
> there is traffic, so that active flag could be used in these
> power sequence stuff? But then again the tty layer has to be extended
> which would probably also cause a lot of ruffled feathers.

Yeah, I think this is a dead end. We need some kind of gps framework
with drivers that can implement the device specific bits.

I may have some time to look at little closer at it this week.

Johan

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Johan Hovold]

On Tue, Feb 27, 2018 at 08:32:50AM +0100, H. Nikolaus Schaller wrote:
> Hi Johan,
> 
> > Am 27.02.2018 um 08:04 schrieb Johan Hovold :
> > 
> > On Mon, Feb 12, 2018 at 04:26:18PM +0100, Pavel Machek wrote:
> >> Hi!
> >> 
>  Let's restart this discussion and focus on the main roadblock (others
>  are minor details which can be sorted out later).
>  
>  If it feels like a hack, the key issue seems to me to be the choice of
>  the API to present the GPS data to user space. Right?
> >>> 
> >>> Or even more fundamentally, does this belong in the kernel at all?
> >> 
> >> Yes, it does.
> 
> Thanks, Pavel for supporting our view.
> 
> > 
> > But not necessarily in its current form.
> 
> Is this a "yes after some code fixes"?

No, we need some kind of at least rudimentary gps framework even if we
allow for a raw (NMEA) interface for the time being (possibly
indefinitely).

> Pavel mentioned an example where such an evolutionary approach was taken.
> > 
> >>> Now, if we'd ever have a proper GPS framework that handled everything in
> >>> kernel space (i.e. no more gpsd) then we would be able to write kernel
> >>> drivers that also take care of PM. But perhaps that's unlikely to ever
> >>> be realised given the state of things (proprietary protocols, numerous
> >>> quirky implementations, etc).
> >> 
> >> That is what needs to happen.
> >> 
> >>> The kernel is probably not the place to be working around issues like
> >>> that, even if serdev at least allows for such hacks to be fairly
> >>> isolated in drivers (unlike some of the earlier proposals touching core
> >>> code).
> >> 
> >> Oh, kernel is indeed right place to provide hardware abstraction --
> >> and that includes bug workarounds.
> > 
> > Right, at least when such hacks can be confined to a driver and not be
> > spread all over the place.
> 
> It seems that you forgot that the driver we propose is not spread all over
> the place. It *is* confined to a single driver thanks to the serdev api.

I believe that's what I wrote above.

Johan

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Johan Hovold]

On Tue, Feb 27, 2018 at 08:32:50AM +0100, H. Nikolaus Schaller wrote:
> Hi Johan,
> 
> > Am 27.02.2018 um 08:04 schrieb Johan Hovold :
> > 
> > On Mon, Feb 12, 2018 at 04:26:18PM +0100, Pavel Machek wrote:
> >> Hi!
> >> 
>  Let's restart this discussion and focus on the main roadblock (others
>  are minor details which can be sorted out later).
>  
>  If it feels like a hack, the key issue seems to me to be the choice of
>  the API to present the GPS data to user space. Right?
> >>> 
> >>> Or even more fundamentally, does this belong in the kernel at all?
> >> 
> >> Yes, it does.
> 
> Thanks, Pavel for supporting our view.
> 
> > 
> > But not necessarily in its current form.
> 
> Is this a "yes after some code fixes"?

No, we need some kind of at least rudimentary gps framework even if we
allow for a raw (NMEA) interface for the time being (possibly
indefinitely).

> Pavel mentioned an example where such an evolutionary approach was taken.
> > 
> >>> Now, if we'd ever have a proper GPS framework that handled everything in
> >>> kernel space (i.e. no more gpsd) then we would be able to write kernel
> >>> drivers that also take care of PM. But perhaps that's unlikely to ever
> >>> be realised given the state of things (proprietary protocols, numerous
> >>> quirky implementations, etc).
> >> 
> >> That is what needs to happen.
> >> 
> >>> The kernel is probably not the place to be working around issues like
> >>> that, even if serdev at least allows for such hacks to be fairly
> >>> isolated in drivers (unlike some of the earlier proposals touching core
> >>> code).
> >> 
> >> Oh, kernel is indeed right place to provide hardware abstraction --
> >> and that includes bug workarounds.
> > 
> > Right, at least when such hacks can be confined to a driver and not be
> > spread all over the place.
> 
> It seems that you forgot that the driver we propose is not spread all over
> the place. It *is* confined to a single driver thanks to the serdev api.

I believe that's what I wrote above.

Johan

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Andreas Kemnade]

Hi,

On Thu, 18 Jan 2018 17:47:36 +1100
Johan Hovold  wrote:

[...]
> > 
> > So to avoid having hardware information spread all over the table at least
> > these information would need to be in devicetree. But that also all feels
> > like a hack and hard to maintain.  
> 
> Having the device described in the device tree is certainly desirable,
> not least for chip identification. And with a GPS framework in the
> kernel with a well-defined interface, implementing power management
> would be straight forward.
> 
Hmm, devicetree without in-kernel drivers, do we have anything like that
somewhere? I thought that was a big no-go. But maybe I am wrong.

> I'm just not convinced that the proposed tty interface is the right
> interface for this. User space would still rely on gpsd for the GPS
> protocols, and would also ultimately be managing power by killing gpsd
> or whatever daemon that would otherwise be holding the port open.
> 
> Something like the generic power sequences that has been discussed
> elsewhere might be a better fit for this if all you want to do is power
> on and off on port open and close (and on suspend/resume). There really
> isn't anything GPS-specific in the current proposal (besides the
> suggested tty-device name).

So a bit like that mmc-powerseq stuff we already have?
> 
> But sure, that wouldn't be sufficient to deal with the
> unknown-power-state problem with the device in question.
> 
Maybe there could be a kind of active flag set by the tty if
there is traffic, so that active flag could be used in these
power sequence stuff? But then again the tty layer has to be extended
which would probably also cause a lot of ruffled feathers.

Regards,
Andreas


pgpq27ubjUuCY.pgp
Description: OpenPGP digital signature

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Andreas Kemnade]

Hi,

On Thu, 18 Jan 2018 17:47:36 +1100
Johan Hovold  wrote:

[...]
> > 
> > So to avoid having hardware information spread all over the table at least
> > these information would need to be in devicetree. But that also all feels
> > like a hack and hard to maintain.  
> 
> Having the device described in the device tree is certainly desirable,
> not least for chip identification. And with a GPS framework in the
> kernel with a well-defined interface, implementing power management
> would be straight forward.
> 
Hmm, devicetree without in-kernel drivers, do we have anything like that
somewhere? I thought that was a big no-go. But maybe I am wrong.

> I'm just not convinced that the proposed tty interface is the right
> interface for this. User space would still rely on gpsd for the GPS
> protocols, and would also ultimately be managing power by killing gpsd
> or whatever daemon that would otherwise be holding the port open.
> 
> Something like the generic power sequences that has been discussed
> elsewhere might be a better fit for this if all you want to do is power
> on and off on port open and close (and on suspend/resume). There really
> isn't anything GPS-specific in the current proposal (besides the
> suggested tty-device name).

So a bit like that mmc-powerseq stuff we already have?
> 
> But sure, that wouldn't be sufficient to deal with the
> unknown-power-state problem with the device in question.
> 
Maybe there could be a kind of active flag set by the tty if
there is traffic, so that active flag could be used in these
power sequence stuff? But then again the tty layer has to be extended
which would probably also cause a lot of ruffled feathers.

Regards,
Andreas


pgpq27ubjUuCY.pgp
Description: OpenPGP digital signature

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Pavel Machek]

Hi!

> > > > Let's restart this discussion and focus on the main roadblock (others
> > > > are minor details which can be sorted out later).
> > > > 
> > > > If it feels like a hack, the key issue seems to me to be the choice of
> > > > the API to present the GPS data to user space. Right?
> > > 
> > > Or even more fundamentally, does this belong in the kernel at all?
> > 
> > Yes, it does.
> 
> But not necessarily in its current form.

Not necessarily, no. OTOH this hack was not too bad IIRC and we
usually do present NMEA to userspace at the moment. We'll need proper
GPS subsystem one day, but I'm afraid that will take some time...

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Pavel Machek]

Hi!

> > > > Let's restart this discussion and focus on the main roadblock (others
> > > > are minor details which can be sorted out later).
> > > > 
> > > > If it feels like a hack, the key issue seems to me to be the choice of
> > > > the API to present the GPS data to user space. Right?
> > > 
> > > Or even more fundamentally, does this belong in the kernel at all?
> > 
> > Yes, it does.
> 
> But not necessarily in its current form.

Not necessarily, no. OTOH this hack was not too bad IIRC and we
usually do present NMEA to userspace at the moment. We'll need proper
GPS subsystem one day, but I'm afraid that will take some time...

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[H. Nikolaus Schaller]

Hi Johan,

> Am 27.02.2018 um 08:04 schrieb Johan Hovold :
> 
> On Mon, Feb 12, 2018 at 04:26:18PM +0100, Pavel Machek wrote:
>> Hi!
>> 
 Let's restart this discussion and focus on the main roadblock (others
 are minor details which can be sorted out later).
 
 If it feels like a hack, the key issue seems to me to be the choice of
 the API to present the GPS data to user space. Right?
>>> 
>>> Or even more fundamentally, does this belong in the kernel at all?
>> 
>> Yes, it does.

Thanks, Pavel for supporting our view.

> 
> But not necessarily in its current form.

Is this a "yes after some code fixes"?

Pavel mentioned an example where such an evolutionary approach was taken.

> 
>>> Now, if we'd ever have a proper GPS framework that handled everything in
>>> kernel space (i.e. no more gpsd) then we would be able to write kernel
>>> drivers that also take care of PM. But perhaps that's unlikely to ever
>>> be realised given the state of things (proprietary protocols, numerous
>>> quirky implementations, etc).
>> 
>> That is what needs to happen.
>> 
>>> The kernel is probably not the place to be working around issues like
>>> that, even if serdev at least allows for such hacks to be fairly
>>> isolated in drivers (unlike some of the earlier proposals touching core
>>> code).
>> 
>> Oh, kernel is indeed right place to provide hardware abstraction --
>> and that includes bug workarounds.
> 
> Right, at least when such hacks can be confined to a driver and not be
> spread all over the place.

It seems that you forgot that the driver we propose is not spread all over
the place. It *is* confined to a single driver thanks to the serdev api.

BR and thanks,
Nikolaus

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[H. Nikolaus Schaller]

Hi Johan,

> Am 27.02.2018 um 08:04 schrieb Johan Hovold :
> 
> On Mon, Feb 12, 2018 at 04:26:18PM +0100, Pavel Machek wrote:
>> Hi!
>> 
 Let's restart this discussion and focus on the main roadblock (others
 are minor details which can be sorted out later).
 
 If it feels like a hack, the key issue seems to me to be the choice of
 the API to present the GPS data to user space. Right?
>>> 
>>> Or even more fundamentally, does this belong in the kernel at all?
>> 
>> Yes, it does.

Thanks, Pavel for supporting our view.

> 
> But not necessarily in its current form.

Is this a "yes after some code fixes"?

Pavel mentioned an example where such an evolutionary approach was taken.

> 
>>> Now, if we'd ever have a proper GPS framework that handled everything in
>>> kernel space (i.e. no more gpsd) then we would be able to write kernel
>>> drivers that also take care of PM. But perhaps that's unlikely to ever
>>> be realised given the state of things (proprietary protocols, numerous
>>> quirky implementations, etc).
>> 
>> That is what needs to happen.
>> 
>>> The kernel is probably not the place to be working around issues like
>>> that, even if serdev at least allows for such hacks to be fairly
>>> isolated in drivers (unlike some of the earlier proposals touching core
>>> code).
>> 
>> Oh, kernel is indeed right place to provide hardware abstraction --
>> and that includes bug workarounds.
> 
> Right, at least when such hacks can be confined to a driver and not be
> spread all over the place.

It seems that you forgot that the driver we propose is not spread all over
the place. It *is* confined to a single driver thanks to the serdev api.

BR and thanks,
Nikolaus

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Johan Hovold]

On Mon, Feb 12, 2018 at 04:26:18PM +0100, Pavel Machek wrote:
> Hi!
> 
> > > Let's restart this discussion and focus on the main roadblock (others
> > > are minor details which can be sorted out later).
> > > 
> > > If it feels like a hack, the key issue seems to me to be the choice of
> > > the API to present the GPS data to user space. Right?
> > 
> > Or even more fundamentally, does this belong in the kernel at all?
> 
> Yes, it does.

But not necessarily in its current form.

> > Now, if we'd ever have a proper GPS framework that handled everything in
> > kernel space (i.e. no more gpsd) then we would be able to write kernel
> > drivers that also take care of PM. But perhaps that's unlikely to ever
> > be realised given the state of things (proprietary protocols, numerous
> > quirky implementations, etc).
> 
> That is what needs to happen.
> 
> > The kernel is probably not the place to be working around issues like
> > that, even if serdev at least allows for such hacks to be fairly
> > isolated in drivers (unlike some of the earlier proposals touching core
> > code).
> 
> Oh, kernel is indeed right place to provide hardware abstraction --
> and that includes bug workarounds.

Right, at least when such hacks can be confined to a driver and not be
spread all over the place.

Johan

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Johan Hovold]

On Mon, Feb 12, 2018 at 04:26:18PM +0100, Pavel Machek wrote:
> Hi!
> 
> > > Let's restart this discussion and focus on the main roadblock (others
> > > are minor details which can be sorted out later).
> > > 
> > > If it feels like a hack, the key issue seems to me to be the choice of
> > > the API to present the GPS data to user space. Right?
> > 
> > Or even more fundamentally, does this belong in the kernel at all?
> 
> Yes, it does.

But not necessarily in its current form.

> > Now, if we'd ever have a proper GPS framework that handled everything in
> > kernel space (i.e. no more gpsd) then we would be able to write kernel
> > drivers that also take care of PM. But perhaps that's unlikely to ever
> > be realised given the state of things (proprietary protocols, numerous
> > quirky implementations, etc).
> 
> That is what needs to happen.
> 
> > The kernel is probably not the place to be working around issues like
> > that, even if serdev at least allows for such hacks to be fairly
> > isolated in drivers (unlike some of the earlier proposals touching core
> > code).
> 
> Oh, kernel is indeed right place to provide hardware abstraction --
> and that includes bug workarounds.

Right, at least when such hacks can be confined to a driver and not be
spread all over the place.

Johan

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Pavel Machek]

Hi!

> > Let's restart this discussion and focus on the main roadblock (others
> > are minor details which can be sorted out later).
> > 
> > If it feels like a hack, the key issue seems to me to be the choice of
> > the API to present the GPS data to user space. Right?
> 
> Or even more fundamentally, does this belong in the kernel at all?

Yes, it does.

> Given that we'd still depend on gpsd and other, proprietary, daemons to
> actually parse and use (also for control) the plethora of GPS protocols
> available, it may even be best to just keep it all in user space.

No. We'd want to move away from gpsd in the long
term. (/dev/input/mice was in similar situation.)

> Now, if we'd ever have a proper GPS framework that handled everything in
> kernel space (i.e. no more gpsd) then we would be able to write kernel
> drivers that also take care of PM. But perhaps that's unlikely to ever
> be realised given the state of things (proprietary protocols, numerous
> quirky implementations, etc).

That is what needs to happen.

> The kernel is probably not the place to be working around issues like
> that, even if serdev at least allows for such hacks to be fairly
> isolated in drivers (unlike some of the earlier proposals touching core
> code).

Oh, kernel is indeed right place to provide hardware abstraction --
and that includes bug workarounds.

We'd like unmodified userspace to run on any supported hardware,
remember?

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Pavel Machek]

Hi!

> >> I'm sorry (and I know this discussion has been going on for a long
> >> time),but this still feels like too much of a hack.
> 
> Happy new year ... Happy new attempt...
> 
> Let's restart this discussion and focus on the main roadblock (others are 
> minor
> details which can be sorted out later).
> 
> If it feels like a hack, the key issue seems to me to be the choice of
> the API to present the GPS data to user space. Right?
> 
> I see three reasonable options how this presentation can be done:
> 
> 1. char device
> 2. tty device
> 3. some new gps interface API (similar to network, bluetooth interfaces)
> 4. no driver and use the UART tty directly

> 3. some new gps interface API
> + could become very elegant and general
> - does not exist (AFAIK not even a plan but I am not aware of everything)
> - no user-space daemons and applications exist which use it

Yes, that is what needs to be done. It is very similar problem to
serial mice we used to have long time ago. (And it has pretty much
same solution; exporting NMEA for gpsd, then slowly moving to system
with no gpsd).

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Pavel Machek]

Hi!

> > Let's restart this discussion and focus on the main roadblock (others
> > are minor details which can be sorted out later).
> > 
> > If it feels like a hack, the key issue seems to me to be the choice of
> > the API to present the GPS data to user space. Right?
> 
> Or even more fundamentally, does this belong in the kernel at all?

Yes, it does.

> Given that we'd still depend on gpsd and other, proprietary, daemons to
> actually parse and use (also for control) the plethora of GPS protocols
> available, it may even be best to just keep it all in user space.

No. We'd want to move away from gpsd in the long
term. (/dev/input/mice was in similar situation.)

> Now, if we'd ever have a proper GPS framework that handled everything in
> kernel space (i.e. no more gpsd) then we would be able to write kernel
> drivers that also take care of PM. But perhaps that's unlikely to ever
> be realised given the state of things (proprietary protocols, numerous
> quirky implementations, etc).

That is what needs to happen.

> The kernel is probably not the place to be working around issues like
> that, even if serdev at least allows for such hacks to be fairly
> isolated in drivers (unlike some of the earlier proposals touching core
> code).

Oh, kernel is indeed right place to provide hardware abstraction --
and that includes bug workarounds.

We'd like unmodified userspace to run on any supported hardware,
remember?

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Pavel Machek]

Hi!

> >> I'm sorry (and I know this discussion has been going on for a long
> >> time),but this still feels like too much of a hack.
> 
> Happy new year ... Happy new attempt...
> 
> Let's restart this discussion and focus on the main roadblock (others are 
> minor
> details which can be sorted out later).
> 
> If it feels like a hack, the key issue seems to me to be the choice of
> the API to present the GPS data to user space. Right?
> 
> I see three reasonable options how this presentation can be done:
> 
> 1. char device
> 2. tty device
> 3. some new gps interface API (similar to network, bluetooth interfaces)
> 4. no driver and use the UART tty directly

> 3. some new gps interface API
> + could become very elegant and general
> - does not exist (AFAIK not even a plan but I am not aware of everything)
> - no user-space daemons and applications exist which use it

Yes, that is what needs to be done. It is very similar problem to
serial mice we used to have long time ago. (And it has pretty much
same solution; exporting NMEA for gpsd, then slowly moving to system
with no gpsd).

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Johan Hovold]

On Fri, Jan 12, 2018 at 07:40:35PM +0100, Andreas Kemnade wrote:
> On Fri, 12 Jan 2018 15:46:47 +0100
> Johan Hovold  wrote:
> 
> > On Tue, Jan 09, 2018 at 06:43:47PM +0100, Andreas Kemnade wrote:
> > > On Fri, 22 Dec 2017 13:44:27 +0100
> > > Johan Hovold  wrote:
> > > 
> > > [...]  
> > > > I'd suggest reiterating the problem you're trying to solve and
> > > > enumerating the previously discussed potential solutions in order to
> > > > find a proper abstraction level for this (before getting lost in
> > > > implementation details).
> > > >   
> > > The main point here is in short words: Having a device powered on or off
> > > when the uart it is attached to, is used or not used anymore,
> > > so the already available userspace applications do not need to be 
> > > changed.  
> > 
> > So we'd end up with something in-between a kernel driver and a
> > user-space solution. What about devices that need to be (partially)
> > powered also when the port isn't open? A pure user-space solution would
> > be able to handle all variants.
> > 
> Well partly powered devices are at many places, And they hide that problem
> from userspace, just get the open()/get() and close()/put() from there and 
> power the
> device accordingly. 
> 
> So the question still remains why should the kernel hide some things and some
> it should not.
> If it all is in userspace, then there is still something needed in the 
> devicetree
> (if I understand correctly, every information about hardware which cannot be
> auto-probed belongs into device tree) so that the userspace knows what kind of
> device is at that port. So there can be a daemon powering on and off devices.
> But that would break existing applications which just expect that they just 
> need
> to open/close the device. 
> 
> Or you need to have some inotify handler in userspace and attach it there to
> react on close() and open() of that device.
> But this thing needs to have two kind of information:
> 
> 1. the type of chip available to do the right powerup sequence. 
> 
> 2. how the chip is wired up to the cpu. 
> 
> So to avoid having hardware information spread all over the table at least
> these information would need to be in devicetree. But that also all feels
> like a hack and hard to maintain.

Having the device described in the device tree is certainly desirable,
not least for chip identification. And with a GPS framework in the
kernel with a well-defined interface, implementing power management
would be straight forward.

I'm just not convinced that the proposed tty interface is the right
interface for this. User space would still rely on gpsd for the GPS
protocols, and would also ultimately be managing power by killing gpsd
or whatever daemon that would otherwise be holding the port open.

Something like the generic power sequences that has been discussed
elsewhere might be a better fit for this if all you want to do is power
on and off on port open and close (and on suspend/resume). There really
isn't anything GPS-specific in the current proposal (besides the
suggested tty-device name).

But sure, that wouldn't be sufficient to deal with the
unknown-power-state problem with the device in question.

Johan

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Johan Hovold]

On Fri, Jan 12, 2018 at 07:40:35PM +0100, Andreas Kemnade wrote:
> On Fri, 12 Jan 2018 15:46:47 +0100
> Johan Hovold  wrote:
> 
> > On Tue, Jan 09, 2018 at 06:43:47PM +0100, Andreas Kemnade wrote:
> > > On Fri, 22 Dec 2017 13:44:27 +0100
> > > Johan Hovold  wrote:
> > > 
> > > [...]  
> > > > I'd suggest reiterating the problem you're trying to solve and
> > > > enumerating the previously discussed potential solutions in order to
> > > > find a proper abstraction level for this (before getting lost in
> > > > implementation details).
> > > >   
> > > The main point here is in short words: Having a device powered on or off
> > > when the uart it is attached to, is used or not used anymore,
> > > so the already available userspace applications do not need to be 
> > > changed.  
> > 
> > So we'd end up with something in-between a kernel driver and a
> > user-space solution. What about devices that need to be (partially)
> > powered also when the port isn't open? A pure user-space solution would
> > be able to handle all variants.
> > 
> Well partly powered devices are at many places, And they hide that problem
> from userspace, just get the open()/get() and close()/put() from there and 
> power the
> device accordingly. 
> 
> So the question still remains why should the kernel hide some things and some
> it should not.
> If it all is in userspace, then there is still something needed in the 
> devicetree
> (if I understand correctly, every information about hardware which cannot be
> auto-probed belongs into device tree) so that the userspace knows what kind of
> device is at that port. So there can be a daemon powering on and off devices.
> But that would break existing applications which just expect that they just 
> need
> to open/close the device. 
> 
> Or you need to have some inotify handler in userspace and attach it there to
> react on close() and open() of that device.
> But this thing needs to have two kind of information:
> 
> 1. the type of chip available to do the right powerup sequence. 
> 
> 2. how the chip is wired up to the cpu. 
> 
> So to avoid having hardware information spread all over the table at least
> these information would need to be in devicetree. But that also all feels
> like a hack and hard to maintain.

Having the device described in the device tree is certainly desirable,
not least for chip identification. And with a GPS framework in the
kernel with a well-defined interface, implementing power management
would be straight forward.

I'm just not convinced that the proposed tty interface is the right
interface for this. User space would still rely on gpsd for the GPS
protocols, and would also ultimately be managing power by killing gpsd
or whatever daemon that would otherwise be holding the port open.

Something like the generic power sequences that has been discussed
elsewhere might be a better fit for this if all you want to do is power
on and off on port open and close (and on suspend/resume). There really
isn't anything GPS-specific in the current proposal (besides the
suggested tty-device name).

But sure, that wouldn't be sufficient to deal with the
unknown-power-state problem with the device in question.

Johan

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Andreas Kemnade]

On Fri, 12 Jan 2018 15:46:47 +0100
Johan Hovold  wrote:

> On Tue, Jan 09, 2018 at 06:43:47PM +0100, Andreas Kemnade wrote:
> > On Fri, 22 Dec 2017 13:44:27 +0100
> > Johan Hovold  wrote:
> > 
> > [...]  
> > > I'd suggest reiterating the problem you're trying to solve and
> > > enumerating the previously discussed potential solutions in order to
> > > find a proper abstraction level for this (before getting lost in
> > > implementation details).
> > >   
> > The main point here is in short words: Having a device powered on or off
> > when the uart it is attached to, is used or not used anymore,
> > so the already available userspace applications do not need to be changed.  
> 
> So we'd end up with something in-between a kernel driver and a
> user-space solution. What about devices that need to be (partially)
> powered also when the port isn't open? A pure user-space solution would
> be able to handle all variants.
> 
Well partly powered devices are at many places, And they hide that problem
from userspace, just get the open()/get() and close()/put() from there and 
power the
device accordingly. 

So the question still remains why should the kernel hide some things and some
it should not.
If it all is in userspace, then there is still something needed in the 
devicetree
(if I understand correctly, every information about hardware which cannot be
auto-probed belongs into device tree) so that the userspace knows what kind of
device is at that port. So there can be a daemon powering on and off devices.
But that would break existing applications which just expect that they just need
to open/close the device. 

Or you need to have some inotify handler in userspace and attach it there to
react on close() and open() of that device.
But this thing needs to have two kind of information:

1. the type of chip available to do the right powerup sequence. 

2. how the chip is wired up to the cpu. 

So to avoid having hardware information spread all over the table at least
these information would need to be in devicetree. But that also all feels
like a hack and hard to maintain.

Regards,
Andreas


pgpUTBsCJ8UuT.pgp
Description: OpenPGP digital signature

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Andreas Kemnade]

On Fri, 12 Jan 2018 15:46:47 +0100
Johan Hovold  wrote:

> On Tue, Jan 09, 2018 at 06:43:47PM +0100, Andreas Kemnade wrote:
> > On Fri, 22 Dec 2017 13:44:27 +0100
> > Johan Hovold  wrote:
> > 
> > [...]  
> > > I'd suggest reiterating the problem you're trying to solve and
> > > enumerating the previously discussed potential solutions in order to
> > > find a proper abstraction level for this (before getting lost in
> > > implementation details).
> > >   
> > The main point here is in short words: Having a device powered on or off
> > when the uart it is attached to, is used or not used anymore,
> > so the already available userspace applications do not need to be changed.  
> 
> So we'd end up with something in-between a kernel driver and a
> user-space solution. What about devices that need to be (partially)
> powered also when the port isn't open? A pure user-space solution would
> be able to handle all variants.
> 
Well partly powered devices are at many places, And they hide that problem
from userspace, just get the open()/get() and close()/put() from there and 
power the
device accordingly. 

So the question still remains why should the kernel hide some things and some
it should not.
If it all is in userspace, then there is still something needed in the 
devicetree
(if I understand correctly, every information about hardware which cannot be
auto-probed belongs into device tree) so that the userspace knows what kind of
device is at that port. So there can be a daemon powering on and off devices.
But that would break existing applications which just expect that they just need
to open/close the device. 

Or you need to have some inotify handler in userspace and attach it there to
react on close() and open() of that device.
But this thing needs to have two kind of information:

1. the type of chip available to do the right powerup sequence. 

2. how the chip is wired up to the cpu. 

So to avoid having hardware information spread all over the table at least
these information would need to be in devicetree. But that also all feels
like a hack and hard to maintain.

Regards,
Andreas


pgpUTBsCJ8UuT.pgp
Description: OpenPGP digital signature

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Johan Hovold]

On Tue, Jan 09, 2018 at 12:55:11PM +0100, H. Nikolaus Schaller wrote:
> Hi Johan,
> 
> > Am 22.12.2017 um 15:40 schrieb H. Nikolaus Schaller :
> > 
> > Hi Johan,
> > 
> >> Am 22.12.2017 um 13:44 schrieb Johan Hovold :
> >> 
> >> On Fri, Dec 01, 2017 at 08:49:36AM +0100, H. Nikolaus Schaller wrote:
> >>> Add driver for Wi2Wi W2SG0004/84 GPS module connected to some SoC UART.
> >>> 
> >>> It uses serdev API hooks to monitor and forward the UART traffic to 
> >>> /dev/ttyGPSn
> >>> and turn on/off the module. It also detects if the module is turned on 
> >>> (sends data)
> >>> but should be off, e.g. if it was already turned on during boot or 
> >>> power-on-reset.
> >>> 
> >>> Additionally, rfkill block/unblock can be used to control an external LNA
> >>> (and power down the module if not needed).
> >>> 
> >>> The driver concept is based on code developed by Neil Brown 
> >>> 
> >>> but simplified and adapted to use the new serdev API introduced in v4.11.
> >> 
> >> I'm sorry (and I know this discussion has been going on for a long
> >> time),but this still feels like too much of a hack.
> 
> Happy new year ... Happy new attempt...

Same to you.

> Let's restart this discussion and focus on the main roadblock (others
> are minor details which can be sorted out later).
> 
> If it feels like a hack, the key issue seems to me to be the choice of
> the API to present the GPS data to user space. Right?

Or even more fundamentally, does this belong in the kernel at all?

Given that we'd still depend on gpsd and other, proprietary, daemons to
actually parse and use (also for control) the plethora of GPS protocols
available, it may even be best to just keep it all in user space.

The next user may want to keep the GPS powered also when the port is
closed; this all seems like policy that should remain in user space.

Now, if we'd ever have a proper GPS framework that handled everything in
kernel space (i.e. no more gpsd) then we would be able to write kernel
drivers that also take care of PM. But perhaps that's unlikely to ever
be realised given the state of things (proprietary protocols, numerous
quirky implementations, etc).

Also it seems at least part of your specific problem is that you have
failed to wire up the WAKEUP pin of the W2SG0004/84 properly, which then
forces you to look at the data stream to determine the power state of
the chip. Judging from a quick look at the GTA04 schematics it seems
you've even connected the WAKEUP output to the 1V8_OUT output?!

The kernel is probably not the place to be working around issues like
that, even if serdev at least allows for such hacks to be fairly
isolated in drivers (unlike some of the earlier proposals touching core
code).

> I see three reasonable options how this presentation can be done:
> 
> 1. char device
> 2. tty device
> 3. some new gps interface API (similar to network, bluetooth interfaces)
> 4. no driver and use the UART tty directly
> 
> Pros and cons:
 
> 4. no driver and use UART directly
> + a non-solution seems to be attractive
> - must turn on/off chip by gpio hacks from user-space

I'm not sure that would amount to more of hack then doing it in the
kernel would.

> - can not guarantee (!) to power off the chip if the last user-space
> process using it is killed (which is essential for power-management of
> a handheld, battery operated device)

That depends on how you implement things (extending gpsd, wrapper
script, pty daemon, ...).

> I would clearly prefer 3 over 2 over 1 over 4.
> 
> So do you see a chance that the kernel core team provides something useable
> (not perfect) for variant 3 in reasonable time (let's say 3-6 months)?

No, I'm afraid not. At least not if we're talking about a framework
that would replace gpsd.

> If not, I want to suggest to accept the second-best choice 2. for now and we
> will update the driver as soon as 3. appears. IMHO it would be a good test 
> case
> for a new subsystem.

Getting the interface right from the start is quite important, as
otherwise we may end up having to support a superseded one for a long
time.

Johan

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Johan Hovold]

On Tue, Jan 09, 2018 at 12:55:11PM +0100, H. Nikolaus Schaller wrote:
> Hi Johan,
> 
> > Am 22.12.2017 um 15:40 schrieb H. Nikolaus Schaller :
> > 
> > Hi Johan,
> > 
> >> Am 22.12.2017 um 13:44 schrieb Johan Hovold :
> >> 
> >> On Fri, Dec 01, 2017 at 08:49:36AM +0100, H. Nikolaus Schaller wrote:
> >>> Add driver for Wi2Wi W2SG0004/84 GPS module connected to some SoC UART.
> >>> 
> >>> It uses serdev API hooks to monitor and forward the UART traffic to 
> >>> /dev/ttyGPSn
> >>> and turn on/off the module. It also detects if the module is turned on 
> >>> (sends data)
> >>> but should be off, e.g. if it was already turned on during boot or 
> >>> power-on-reset.
> >>> 
> >>> Additionally, rfkill block/unblock can be used to control an external LNA
> >>> (and power down the module if not needed).
> >>> 
> >>> The driver concept is based on code developed by Neil Brown 
> >>> 
> >>> but simplified and adapted to use the new serdev API introduced in v4.11.
> >> 
> >> I'm sorry (and I know this discussion has been going on for a long
> >> time),but this still feels like too much of a hack.
> 
> Happy new year ... Happy new attempt...

Same to you.

> Let's restart this discussion and focus on the main roadblock (others
> are minor details which can be sorted out later).
> 
> If it feels like a hack, the key issue seems to me to be the choice of
> the API to present the GPS data to user space. Right?

Or even more fundamentally, does this belong in the kernel at all?

Given that we'd still depend on gpsd and other, proprietary, daemons to
actually parse and use (also for control) the plethora of GPS protocols
available, it may even be best to just keep it all in user space.

The next user may want to keep the GPS powered also when the port is
closed; this all seems like policy that should remain in user space.

Now, if we'd ever have a proper GPS framework that handled everything in
kernel space (i.e. no more gpsd) then we would be able to write kernel
drivers that also take care of PM. But perhaps that's unlikely to ever
be realised given the state of things (proprietary protocols, numerous
quirky implementations, etc).

Also it seems at least part of your specific problem is that you have
failed to wire up the WAKEUP pin of the W2SG0004/84 properly, which then
forces you to look at the data stream to determine the power state of
the chip. Judging from a quick look at the GTA04 schematics it seems
you've even connected the WAKEUP output to the 1V8_OUT output?!

The kernel is probably not the place to be working around issues like
that, even if serdev at least allows for such hacks to be fairly
isolated in drivers (unlike some of the earlier proposals touching core
code).

> I see three reasonable options how this presentation can be done:
> 
> 1. char device
> 2. tty device
> 3. some new gps interface API (similar to network, bluetooth interfaces)
> 4. no driver and use the UART tty directly
> 
> Pros and cons:
 
> 4. no driver and use UART directly
> + a non-solution seems to be attractive
> - must turn on/off chip by gpio hacks from user-space

I'm not sure that would amount to more of hack then doing it in the
kernel would.

> - can not guarantee (!) to power off the chip if the last user-space
> process using it is killed (which is essential for power-management of
> a handheld, battery operated device)

That depends on how you implement things (extending gpsd, wrapper
script, pty daemon, ...).

> I would clearly prefer 3 over 2 over 1 over 4.
> 
> So do you see a chance that the kernel core team provides something useable
> (not perfect) for variant 3 in reasonable time (let's say 3-6 months)?

No, I'm afraid not. At least not if we're talking about a framework
that would replace gpsd.

> If not, I want to suggest to accept the second-best choice 2. for now and we
> will update the driver as soon as 3. appears. IMHO it would be a good test 
> case
> for a new subsystem.

Getting the interface right from the start is quite important, as
otherwise we may end up having to support a superseded one for a long
time.

Johan

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Johan Hovold]

On Tue, Jan 09, 2018 at 06:43:47PM +0100, Andreas Kemnade wrote:
> On Fri, 22 Dec 2017 13:44:27 +0100
> Johan Hovold  wrote:
> 
> [...]
> > I'd suggest reiterating the problem you're trying to solve and
> > enumerating the previously discussed potential solutions in order to
> > find a proper abstraction level for this (before getting lost in
> > implementation details).
> > 
> The main point here is in short words: Having a device powered on or off
> when the uart it is attached to, is used or not used anymore,
> so the already available userspace applications do not need to be changed.

So we'd end up with something in-between a kernel driver and a
user-space solution. What about devices that need to be (partially)
powered also when the port isn't open? A pure user-space solution would
be able to handle all variants.

> I digged out a bit around:
> alternative aproaches were:
> adding hooks to the uart/tty layer:
> https://marc.info/?l=linux-kernel=14222014616=2
> https://marc.info/?l=devicetree=143130955414580=2

Thanks for the pointers, I remember those threads...

> I do not find it right now in my archive:
> adding a virtual gpio for dtr to the omap_serial driver.
> The driver behind the virtual io would then handle pm. One reason it was
> rejected was that the devicetree should only contain real hardware and
> not virtual stuff.

Oh, yeah, I think something like that made it in briefly before getting
reverted again.

I'll respond to Nikolaus mail as well.

Johan

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Johan Hovold]

On Tue, Jan 09, 2018 at 06:43:47PM +0100, Andreas Kemnade wrote:
> On Fri, 22 Dec 2017 13:44:27 +0100
> Johan Hovold  wrote:
> 
> [...]
> > I'd suggest reiterating the problem you're trying to solve and
> > enumerating the previously discussed potential solutions in order to
> > find a proper abstraction level for this (before getting lost in
> > implementation details).
> > 
> The main point here is in short words: Having a device powered on or off
> when the uart it is attached to, is used or not used anymore,
> so the already available userspace applications do not need to be changed.

So we'd end up with something in-between a kernel driver and a
user-space solution. What about devices that need to be (partially)
powered also when the port isn't open? A pure user-space solution would
be able to handle all variants.

> I digged out a bit around:
> alternative aproaches were:
> adding hooks to the uart/tty layer:
> https://marc.info/?l=linux-kernel=14222014616=2
> https://marc.info/?l=devicetree=143130955414580=2

Thanks for the pointers, I remember those threads...

> I do not find it right now in my archive:
> adding a virtual gpio for dtr to the omap_serial driver.
> The driver behind the virtual io would then handle pm. One reason it was
> rejected was that the devicetree should only contain real hardware and
> not virtual stuff.

Oh, yeah, I think something like that made it in briefly before getting
reverted again.

I'll respond to Nikolaus mail as well.

Johan

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Andreas Kemnade]

On Fri, 22 Dec 2017 13:44:27 +0100
Johan Hovold  wrote:

[...]
> I'd suggest reiterating the problem you're trying to solve and
> enumerating the previously discussed potential solutions in order to
> find a proper abstraction level for this (before getting lost in
> implementation details).
> 
The main point here is in short words: Having a device powered on or off
when the uart it is attached to, is used or not used anymore,
so the already available userspace applications do not need to be changed.

I digged out a bit around:
alternative aproaches were:
adding hooks to the uart/tty layer:
https://marc.info/?l=linux-kernel=14222014616=2
https://marc.info/?l=devicetree=143130955414580=2

I do not find it right now in my archive:
adding a virtual gpio for dtr to the omap_serial driver.
The driver behind the virtual io would then handle pm. One reason it was
rejected was that the devicetree should only contain real hardware and
not virtual stuff.

Regards,
Andreas


pgpa7BZ0bIL2w.pgp
Description: OpenPGP digital signature

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Andreas Kemnade]

On Fri, 22 Dec 2017 13:44:27 +0100
Johan Hovold  wrote:

[...]
> I'd suggest reiterating the problem you're trying to solve and
> enumerating the previously discussed potential solutions in order to
> find a proper abstraction level for this (before getting lost in
> implementation details).
> 
The main point here is in short words: Having a device powered on or off
when the uart it is attached to, is used or not used anymore,
so the already available userspace applications do not need to be changed.

I digged out a bit around:
alternative aproaches were:
adding hooks to the uart/tty layer:
https://marc.info/?l=linux-kernel=14222014616=2
https://marc.info/?l=devicetree=143130955414580=2

I do not find it right now in my archive:
adding a virtual gpio for dtr to the omap_serial driver.
The driver behind the virtual io would then handle pm. One reason it was
rejected was that the devicetree should only contain real hardware and
not virtual stuff.

Regards,
Andreas


pgpa7BZ0bIL2w.pgp
Description: OpenPGP digital signature

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[H. Nikolaus Schaller]

Hi Johan,

> Am 22.12.2017 um 15:40 schrieb H. Nikolaus Schaller :
> 
> Hi Johan,
> 
>> Am 22.12.2017 um 13:44 schrieb Johan Hovold :
>> 
>> On Fri, Dec 01, 2017 at 08:49:36AM +0100, H. Nikolaus Schaller wrote:
>>> Add driver for Wi2Wi W2SG0004/84 GPS module connected to some SoC UART.
>>> 
>>> It uses serdev API hooks to monitor and forward the UART traffic to 
>>> /dev/ttyGPSn
>>> and turn on/off the module. It also detects if the module is turned on 
>>> (sends data)
>>> but should be off, e.g. if it was already turned on during boot or 
>>> power-on-reset.
>>> 
>>> Additionally, rfkill block/unblock can be used to control an external LNA
>>> (and power down the module if not needed).
>>> 
>>> The driver concept is based on code developed by Neil Brown 
>>> but simplified and adapted to use the new serdev API introduced in v4.11.
>> 
>> I'm sorry (and I know this discussion has been going on for a long
>> time),but this still feels like too much of a hack.

Happy new year ... Happy new attempt...

Let's restart this discussion and focus on the main roadblock (others are minor
details which can be sorted out later).

If it feels like a hack, the key issue seems to me to be the choice of
the API to present the GPS data to user space. Right?

I see three reasonable options how this presentation can be done:

1. char device
2. tty device
3. some new gps interface API (similar to network, bluetooth interfaces)
4. no driver and use the UART tty directly

Pros and cons:

1. char device
+ seems to save resources (but IMHO doesn't if we look deeper to handle select, 
blocking, buffer overflow)
- the standard function of buffering a character stream has to be done by this 
driver again, although tty subsystem already has proper buffering
- no line disciplines (e.g. if some gps client wants to translate CR and NL or 
use canonical/noncanonical mode)
- capabilities of the interface change if same chip is connected through USB or 
Bluetooth serial interface

2. tty device
+ full tty port like USB, Bluetooth or UART connection (w/o driver)
+ handles tcsetattr like USB, Bluetooth or UART
+ buffering and line disciplines come for free (at least wrt. driver code)
+ tested
- seems to appear to be complex and overkill and a hack (but IMHO is neither)

3. some new gps interface API
+ could become very elegant and general
- does not exist (AFAIK not even a plan but I am not aware of everything)
- no user-space daemons and applications exist which use it

4. no driver and use UART directly
+ a non-solution seems to be attractive
- must turn on/off chip by gpio hacks from user-space
- can not guarantee (!) to power off the chip if the last user-space process 
using it is killed
  (which is essential for power-management of a handheld, battery operated 
device)

I would clearly prefer 3 over 2 over 1 over 4.

So do you see a chance that the kernel core team provides something useable
(not perfect) for variant 3 in reasonable time (let's say 3-6 months)?

If not, I want to suggest to accept the second-best choice 2. for now and we
will update the driver as soon as 3. appears. IMHO it would be a good test case
for a new subsystem.

Please advise how you want to proceed.

BR and thanks,
Nikolaus

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[H. Nikolaus Schaller]

Hi Johan,

> Am 22.12.2017 um 15:40 schrieb H. Nikolaus Schaller :
> 
> Hi Johan,
> 
>> Am 22.12.2017 um 13:44 schrieb Johan Hovold :
>> 
>> On Fri, Dec 01, 2017 at 08:49:36AM +0100, H. Nikolaus Schaller wrote:
>>> Add driver for Wi2Wi W2SG0004/84 GPS module connected to some SoC UART.
>>> 
>>> It uses serdev API hooks to monitor and forward the UART traffic to 
>>> /dev/ttyGPSn
>>> and turn on/off the module. It also detects if the module is turned on 
>>> (sends data)
>>> but should be off, e.g. if it was already turned on during boot or 
>>> power-on-reset.
>>> 
>>> Additionally, rfkill block/unblock can be used to control an external LNA
>>> (and power down the module if not needed).
>>> 
>>> The driver concept is based on code developed by Neil Brown 
>>> but simplified and adapted to use the new serdev API introduced in v4.11.
>> 
>> I'm sorry (and I know this discussion has been going on for a long
>> time),but this still feels like too much of a hack.

Happy new year ... Happy new attempt...

Let's restart this discussion and focus on the main roadblock (others are minor
details which can be sorted out later).

If it feels like a hack, the key issue seems to me to be the choice of
the API to present the GPS data to user space. Right?

I see three reasonable options how this presentation can be done:

1. char device
2. tty device
3. some new gps interface API (similar to network, bluetooth interfaces)
4. no driver and use the UART tty directly

Pros and cons:

1. char device
+ seems to save resources (but IMHO doesn't if we look deeper to handle select, 
blocking, buffer overflow)
- the standard function of buffering a character stream has to be done by this 
driver again, although tty subsystem already has proper buffering
- no line disciplines (e.g. if some gps client wants to translate CR and NL or 
use canonical/noncanonical mode)
- capabilities of the interface change if same chip is connected through USB or 
Bluetooth serial interface

2. tty device
+ full tty port like USB, Bluetooth or UART connection (w/o driver)
+ handles tcsetattr like USB, Bluetooth or UART
+ buffering and line disciplines come for free (at least wrt. driver code)
+ tested
- seems to appear to be complex and overkill and a hack (but IMHO is neither)

3. some new gps interface API
+ could become very elegant and general
- does not exist (AFAIK not even a plan but I am not aware of everything)
- no user-space daemons and applications exist which use it

4. no driver and use UART directly
+ a non-solution seems to be attractive
- must turn on/off chip by gpio hacks from user-space
- can not guarantee (!) to power off the chip if the last user-space process 
using it is killed
  (which is essential for power-management of a handheld, battery operated 
device)

I would clearly prefer 3 over 2 over 1 over 4.

So do you see a chance that the kernel core team provides something useable
(not perfect) for variant 3 in reasonable time (let's say 3-6 months)?

If not, I want to suggest to accept the second-best choice 2. for now and we
will update the driver as soon as 3. appears. IMHO it would be a good test case
for a new subsystem.

Please advise how you want to proceed.

BR and thanks,
Nikolaus