Hello.

???????, 11 ??????? 2014, 18:45 +01:00 ?? Richard Genoud [off-list ref]:

This patch add some helpers to control modem lines (CTS/RTS/DSR...) via
GPIO.
This will be useful for many boards which have a serial controller that
only handle CTS/RTS pins (or even just RX/TX).

Signed-off-by: Richard Genoud <redacted>
---

OK, a few comments below.

...

+config SERIAL_MCTRL_GPIO
+	def_bool y
+	depends on GPIOLIB

I suggest to move GPIOLIB dependency to serial_mctrl_gpio.h header, so
unit can be used with or without GPIOLIB, so Kconfig will look something like this:

config SERIAL_MCTRL_GPIO
  tristate

Then, you can select this option for particular UART:
config SERIAL_ATMEL
  ...
  select SERIAL_MCTRL_GPIO

...

...

+static const char *mctrl_gpio_of_names[UART_GPIO_MAX] = {
+	"cts", "dsr", "dcd", "ri", "rts", "dtr"
+};

Make a combined array. This will use cycles for set/get operations.

static struct {
  const char *name;
  unsigned int mctrl;
} mctrl_gpios[] = {
  { "cts", TIOCM_CTS, },
  ...
};

...

+int mctrl_gpio_init(struct device *dev, struct mctrl_gpios *gpios)
+{

I'm not sure whether to make a non-DT support at all ...

+	enum mctrl_gpio_idx i;
+	int err = 0;
+	int ret = 0;
+
+	for (i = UART_GPIO_MIN; i < UART_GPIO_MAX; i++) {
+		gpios->gpio[i] = gpiod_get(dev, mctrl_gpio_of_names[i]);

What a reason to using gpiod_xxx() ?
Why we cannot use standart devm_gpio_request/get/set etc. ?

In addition, I recommend create this patch as separate,
because it will most likely still be comments later.
Thanks.

---