Re: [PATCH net 3/3] ice: Add set_termios tty operations handle to GNSS

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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

> > -----Original Message-----
> > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
> > Sent: Tuesday, September 6, 2022 8:16 AM
> > To: Michalik, Michal <michal.michalik@intel.com>
> > 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 <gurucharanx.g@intel.com>; 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.

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