On Tue, Sep 06, 2022 at 08:55:59PM +0000, Michalik, Michal wrote:

Greg,

Thanks - answer inline.

As is required, no need to put this on your emails, as top-posting is
not allowed :)

quoted

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

Please fix your email client to not do this.

quoted

quoted

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.

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.

You get the error on the first get or the set?

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.

This should not be needed as I thought the default would be "just ignore
this", but maybe not.  Can you look into this problem please and figure
out why this is required and fix that up?

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. 

Let's fix the root problem here, not paper over it.

thanks,

greg k-h