Re: [PATCH 1/6] serial: sa1100: add support for mctrl gpios

[Russell King - ARM Linux admin <linux@armlinux.org.uk>]

On Fri, May 31, 2019 at 03:56:58PM +0200, Uwe Kleine-König wrote:
> On Fri, May 31, 2019 at 02:23:40PM +0100, Russell King - ARM Linux admin wrote:
> > On Fri, May 31, 2019 at 02:50:13PM +0200, Uwe Kleine-König wrote:
> > > On Fri, May 31, 2019 at 12:13:47PM +0100, Russell King wrote:
> > > > +static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
> > > > +{
> > > > +	sport->port.dev = &dev->dev;
> > > > +	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
> > > 
> > > the _noauto function was only introduced to ease a transition. I think
> > > the driver would benefit to use mctrl_gpio_init() instead.
> > 
> > In what way would the driver benefit?  mctrl_gpio_init() requires that
> > there are IRQs for each input GPIO.  This is not the case with most
> > SA11x0 platforms, where the GPIO controls are implemented using simple
> > latches, hence that interface is entirely unsuitable.
> 
> Ah, but then you can only use the outputs reliably here as an edge on
> (say) CTS stays unnoticed with both mctrl_gpio_init() and
> mctrl_gpio_init_noauto().

Right that is a risk with a polled approach, but that is the approach
that the SA1100 serial driver has taken ever since it was written
almost twenty years ago, and no one has raised any concerns about
that until now.

> Unless I miss something (which is quite possible given that it's quite
> some time ago I looked into mctrl_gpio) with mctrl_gpio_init_noauto()
> having a CTS-gpio is just ignored unless the modem ctrl lines are
> explicitely requestet while with mctrl_gpio_init() it results in an
> error. Isn't the error the better alternative?

Unless the serial driver polls the modem control line status, which
the SA1100 driver continues to do in exactly the same way after this
conversion.

Do you suggest that we just regress the driver by ripping out this
support that no one has had any problems with, and that is known to
work sufficiently in its day, just because we now don't like it?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up