From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
Sent: Wednesday, September 7, 2022 7:46 AM

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

quoted

Greg,

Thanks - answer inline.

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

Thanks - lesson learned.

quoted

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.

Thanks, I will no longer send it. 

quoted

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?

On "set" - I'm doing get only to prove interface is working and read current
attributes to change only one of them.

quoted

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?

quoted

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.

Ok - now I understand what you mean. If I get it correctly in your opinion the
kernel should not return error 22 if this handler is not set. I thought it is
an expected behavior, but I will take a look if that might be a bug.

Thanks,
M^2

thanks,

greg k-h