Greg,

Thanks - answer inline.

BR,
M^2

-----Original Message-----
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
Sent: Tuesday, September 6, 2022 8:16 AM
To: Michalik, Michal <redacted>
Cc: Jakub Kicinski <kuba@kernel.org>; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; davem@davemloft.net; pabeni@redhat.com; edumazet@google.com; netdev@vger.kernel.org; richardcochran@gmail.com; G, GurucharanX <redacted>; Jiri Slaby <jirislaby@kernel.org>; Johan Hovold <johan@kernel.org>
Subject: Re: [PATCH net 3/3] ice: Add set_termios tty operations handle to GNSS

On Mon, Sep 05, 2022 at 07:32:44PM +0000, Michalik, Michal wrote:

quoted

Hello Greg,

Much thanks for a feedback. Please excuse me for delayed answer, we tried to collect all
the required information before returning to you - but we are still working on it.

Best regards,
M^2

quoted

-----Original Message-----
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
Sent: Thursday, September 1, 2022 7:46 AM
To: Jakub Kicinski <kuba@kernel.org>
Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; davem@davemloft.net; pabeni@redhat.com; edumazet@google.com; Michalik, Michal <redacted>; netdev@vger.kernel.org; richardcochran@gmail.com; G, GurucharanX <redacted>; Jiri Slaby <jirislaby@kernel.org>; Johan Hovold <johan@kernel.org>
Subject: Re: [PATCH net 3/3] ice: Add set_termios tty operations handle to GNSS

On Wed, Aug 31, 2022 at 02:54:39PM -0700, Jakub Kicinski wrote:

quoted

On Mon, 29 Aug 2022 15:00:49 -0700 Tony Nguyen wrote:

quoted

From: Michal Michalik <redacted>

Some third party tools (ex. ubxtool) try to change GNSS TTY parameters
(ex. speed). While being optional implementation, without set_termios
handle this operation fails and prevents those third party tools from
working.

What tools are "blocked" by this?  And what is the problem they have
with just the default happening here?  You are now doing nothing, while
if you do not have the callback, at least a basic "yes, we accepted
these values" happens which was intended for userspace to not know that
there was a problem here.

As I stated in the commit message, the example tool is ubxtool - while trying to
connect to the GPS module the error appreared:
Traceback (most recent call last):

	  File "/usr/local/bin/ubxtool", line 378, in <module>
		io_handle = gps.gps_io(
	  File "/usr/local/lib/python3.9/site-packages/gps/gps.py", line 309, in __init__
		self.ser = Serial.Serial(
	  File "/usr/local/lib/python3.9/site-packages/serial/serialutil.py", line 244, in __init__
		self.open()
	  File "/usr/local/lib/python3.9/site-packages/serial/serialposix.py", line 332, in open
		self._reconfigure_port(force_update=True)
	  File "/usr/local/lib/python3.9/site-packages/serial/serialposix.py", line 517, in _reconfigure_port
		termios.tcsetattr(
	termios.error: (22, 'Invalid argument')
	
Adding this empty function solved the problem.

That seems very wrong, please work to fix this by NOT having an empty
function like this as it should not be required.

Thanks for sharing the feedback - I love the possibility to learn from you and
other community members, because I'm really new in here.

I don't get one thing, though. You are saying, it "seem" wrong and that
"should not" be required but I observe different behavior. I have prepared
a very simple code to reproduce the issue:
	#include <termios.h>
	#include <unistd.h>
	#include <stdio.h>
	#include <fcntl.h>
	#include <errno.h>

	int main()
	{
		struct termios tty;
		int fd;
		
		fd = open("/dev/ttyGNSS_0300", O_RDWR | O_NOCTTY | O_SYNC);

		if (fd < 0) {
				printf("Error - TTY not open.\n");
				return -1;
		}
				
		if (tcgetattr (fd, &tty) != 0) {
			printf("Error on get - errno=%i\n", errno);
			return -1;
		}
		tty.c_cflag |= CS8; // try to set 8 data bits 
		if (tcsetattr(fd, TCSANOW, &tty) != 0) {
			printf("Error on set - errno=%i\n", errno);
			return -1;
		}

		close(fd);
		printf("Done.\n");
	}

In this case, when I don't satisfy this API, I get an errno 22. If add this
empty function and therefore implement the full API it works as expected (no
error). In our case no action is needed, therefore we have an empty function.
At the moment, I'm not sure how I should fix it other way - since no action
on HW is neccessary.

Of course in the meantime we are working on investigating if we can easily
align to existing GNSS interface accroding to community suggestions. Still,
we believe that this fix is solving the problem at the moment. 

thanks,

greg k-h