archive mirror
 help / color / mirror / Atom feed
From: Lukas Wunner <>
To: Andy Shevchenko <>
Cc: Greg Kroah-Hartman <>,
	Jiri Slaby <>, Rob Herring <>,
	"Matwey V. Kornilov" <>,
	Giulio Benetti <>,
	Heiko Stuebner <>,
	Christoph Muellner <>,
	Jan Kiszka <>,,
Subject: Re: [PATCH 4/4] serial: 8250: Support rs485 bus termination GPIO
Date: Wed, 6 May 2020 08:29:43 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Tue, May 05, 2020 at 07:10:35PM +0300, Andy Shevchenko wrote:
> On Tue, May 05, 2020 at 04:42:04PM +0200, Lukas Wunner wrote:
> > Commit e8759ad17d41 ("serial: uapi: Add support for bus termination")
> > introduced the ability to enable rs485 bus termination from user space.
> > So far the feature is only used by a single driver, 8250_exar.c, using a
> > hardcoded GPIO pin specific to Siemens IOT2040 products.
> > 
> > Provide for a more generic solution by allowing specification of an
> > rs485 bus termination GPIO pin in the device tree:  Amend the serial
> > core to retrieve the GPIO from the device tree (or ACPI table) and amend
> > the default ->rs485_config() callback for 8250 drivers to change the
> > GPIO on request from user space.
> ...
> > @@ -3331,6 +3332,29 @@ int uart_get_rs485_mode(struct uart_port *port)
> > +		devm_gpiod_put(dev, port->rs485_term_gpio);
> > +	port->rs485_term_gpio = devm_gpiod_get_optional(dev, "rs485-term",
> Using devm_*() in uart_get_rs485_mode() seems not right.
> Why do you need this?

uart_get_rs485_mode() is called from a driver's ->probe() hook and we
do not have a corresponding function that is called from a ->remove()
hook where we'd be able to relinquish rs485 resources we've acquired
on probe.

Of course I could add that but it would be more heavy-weight compared
to simply using devm_*().  Do you disagree?

devm_gpiod_put() isn't strictly necessary here.  It is only necessary
if one of the drivers would invoke uart_get_rs485_mode() multiple
times, which none of them does AFAICS.  It's just a safety measure.
I can drop it if that is preferred.

> Parameter has a specific macro GPIOD_OUT_HIGH.

Good point.  It's also occurred to me now that reading the GPIO's
value after changing its direction to output is nonsense.  If anything
it ought to be read *before* changing the direction to output.
That would make sense in case the board has a pullup or pulldown on
the Termination Enable pin.  In other cases the pin may just float
and the value will be unpredictable.  However if I do not read the
pin, I'd have to choose either high or low as initial state.  Hm.
Let me check back with our hardware engineers today and see what they



  reply	other threads:[~2020-05-06  6:29 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-05 14:42 [PATCH 0/4] rs485 bus termination GPIO Lukas Wunner
2020-05-05 14:42 ` [PATCH 1/4] serial: 8250: Avoid error message on reprobe Lukas Wunner
2020-05-05 16:01   ` Andy Shevchenko
2020-05-06  6:06     ` Lukas Wunner
2020-05-06 10:01       ` Andy Shevchenko
2020-05-06 11:54         ` Lukas Wunner
2020-05-05 14:42 ` [PATCH 2/4] serial: Allow uart_get_rs485_mode() to return errno Lukas Wunner
2020-05-05 14:42 ` [PATCH 3/4] dt-bindings: serial: Add binding for rs485 bus termination GPIO Lukas Wunner
2020-05-05 14:42 ` [PATCH 4/4] serial: 8250: Support " Lukas Wunner
2020-05-05 16:10   ` Andy Shevchenko
2020-05-06  6:29     ` Lukas Wunner [this message]
2020-05-06 10:06       ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).