Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 29 May 2020 11:00:43 +0200
From:   Johan Hovold <johan@kernel.org>
To:     Manivannan Sadhasivam <mani@kernel.org>
Cc:     Johan Hovold <johan@kernel.org>, gregkh@linuxfoundation.org,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
        patong.mxl@gmail.com, Linus Walleij <linus.walleij@linaro.org>,
        linux-gpio@vger.kernel.org
Subject: Re: [PATCH v3 2/2] usb: serial: xr_serial: Add gpiochip support
Message-ID: <20200529090043.GB19480@localhost>
References: <20200430184924.31690-1-mani@kernel.org>
 <20200430184924.31690-3-mani@kernel.org>
 <20200519130740.GE27787@localhost>
 <20200527030959.GA6129@Mani-XPS-13-9360>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200527030959.GA6129@Mani-XPS-13-9360>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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