Hello,
On Mon, May 23, 2016 at 10:08:26AM -0700, Peter Hurley wrote:
> On 05/23/2016 01:59 AM, Uwe Kleine-König wrote:
> > Hello Yegor,
> >
> > On Mon, May 23, 2016 at 09:30:56AM +0200, yegorslists@xxxxxxxxxxxxxx wrote:
> >> From: Yegor Yefremov <yegorslists@xxxxxxxxxxxxxx>
> >>
> >> mctrl_gpio_get_outputs() returns the state of following signals:
> >>
> >> RTS, DTR
> >>
> >> Signed-off-by: Yegor Yefremov <yegorslists@xxxxxxxxxxxxxx>
> >> ---
> >> drivers/tty/serial/serial_mctrl_gpio.c | 18 ++++++++++++++++++
> >> drivers/tty/serial/serial_mctrl_gpio.h | 15 ++++++++++++++-
> >> 2 files changed, 32 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
> >> index e8dd509..a868595 100644
> >> --- a/drivers/tty/serial/serial_mctrl_gpio.c
> >> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
> >> @@ -86,6 +86,24 @@ unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
> >> }
> >> EXPORT_SYMBOL_GPL(mctrl_gpio_get);
> >>
> >> +unsigned int
> >> +mctrl_gpio_get_outputs(struct mctrl_gpios *gpios, unsigned int *mctrl)
> >> +{
> >> + enum mctrl_gpio_idx i;
> >> +
> >> + for (i = 0; i < UART_GPIO_MAX; i++) {
> >> + if (gpios->gpio[i] && mctrl_gpios_desc[i].dir_out) {
> >> + if (gpiod_get_value(gpios->gpio[i]))
> >> + *mctrl |= mctrl_gpios_desc[i].mctrl;
> >> + else
> >> + *mctrl &= ~mctrl_gpios_desc[i].mctrl;
> >
> > Given that get_value for outputs isn't well defined I'd prefer if there
> > were no usecase for this. This is intended for probe-time, right?
> > Otherwise the state of these lines should be known.
>
> It should be known to both gpio and this emulation layer already,
> and it's an oversight if it doesn't.
So in which situations is this function supposed to be called?
(OK, I could check how it is used in patch 4, but I think explaining it
here in a comment to that function would be nice.)
> Why create a write-only interface? As if it's not hard enough when
> hardware registers are write-only...
There are different possible semantics and so the different hardware
implementations differ. Some return what the gpio was set to, some
return the real level of the pin, some return always 0 and I'm sure if I
ask a few hardware engineers they can come up with at least two further
creative ideas.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
