Re: [PATCH v3 2/2] usb: serial: xr_serial: Add gpiochip support

[Johan Hovold <johan@kernel.org>]

On Wed, May 27, 2020 at 08:39:59AM +0530, Manivannan Sadhasivam wrote:
> Hi,
> 
> On Tue, May 19, 2020 at 03:07:40PM +0200, Johan Hovold wrote:
> > On Fri, May 01, 2020 at 12:19:24AM +0530, mani@kernel.org wrote:
> > > From: Manivannan Sadhasivam <mani@kernel.org>
> > > 
> > > Add gpiochip support for Maxlinear/Exar USB to serial converter
> > > for controlling the available gpios.

> > > +static int xr21v141x_gpio_init(struct usb_serial_port *port)
> > > +{
> > > +	struct xr_port_private *port_priv = usb_get_serial_port_data(port);
> > > +	int ret;
> > > +	u8 gpio_mode;
> > > +
> > > +	port_priv->gc.ngpio = 6;
> > > +
> > > +	ret = xr_get_reg(port, XR21V141X_UART_REG_BLOCK,
> > > +			 port_priv->regs->gpio_mode, &gpio_mode);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	/* Mark all pins which are not in GPIO mode */
> > > +	if (gpio_mode & UART_MODE_RTS_CTS)
> > > +		port_priv->gpio_altfunc |= (BIT(4) | BIT(5));
> > > +	else if (gpio_mode & UART_MODE_DTR_DSR)
> > > +		port_priv->gpio_altfunc |= (BIT(2) | BIT(3));
> > > +	else if (gpio_mode & UART_MODE_RS485)
> > > +		port_priv->gpio_altfunc |= BIT(5);
> > > +	else if (gpio_mode & UART_MODE_RS485_ADDR)
> > > +		port_priv->gpio_altfunc |= BIT(5);
> > > +	else
> > > +		port_priv->gpio_altfunc = 0; /* All GPIOs are available */
> > 
> > So this clearly isn't sufficient as the serial driver updates the
> > gpio-mode settings at runtime, which means you may have the two drivers
> > interfering with each other.
> 
> Agree. I was not sure earlier on how to handle this.
> 
> > You probably need to reserve at least CTS/RTS (gpio 4 and 5) for use
> > by the serial driver. But suddenly driving the DSR, RI and CD inputs
> > probably isn't a good idea either.
> > 
> > How would you even what know what these pins are used for generally?
> > 
> > Perhaps refusing all gpio requests while the port is open and making
> > sure that the serial driver never touches a requested pin could work
> > (including indirectly through hardware flow control, etc).
> 
> Is there an API or recommended way to check if the port is open? I
> just see a helper for tty...

You're gonna need to add your own flag and locking as you're
coordinating two distinct drivers.

Johan