2014-02-26 17:37 GMT+01:00 Alexander Shiyan [off-list ref]:

Hello Richard.

?????, 26 ??????? 2014, 17:19 +01:00 ?? Richard Genoud [off-list ref]:

quoted

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>

Better, but I found few bugs... :)
...

quoted

+Modem control lines via GPIO
+----------------------------
+
+Some helpers are provided in order to set/get modem control lines via GPIO.
+
+mctrl_gpio_init(dev, idx):

I think we should indicate variable types used in these functions.

In the whole file, the variable type are not indicated. So, for
consistency, I think it's better like that.

 ...

quoted

+void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
+{
+     enum mctrl_gpio_idx i;
+
+     if (IS_ERR_OR_NULL(gpios))
+             return;

No. IS_ERR_OR_NULL() is not valid here. This is allocated by kzalloc(),
so you should check valid pointer only, ie (!gpios).
Same in other places.

You're right !

quoted

+             if (err) {
+                     dev_err(dev, "Unable to set direction for %s GPIO",
+                             mctrl_gpios_desc[i].name);
+                     devm_gpiod_put(dev, gpios->gpio[i]);
+                     gpios->gpio[i] = NULL;
+             }
+     }

Let's use dev_dbg().

Why dbg ? I agree that _err may be a bit strong, we could use
dev_warn() instead.
But as it really should not happen, I'm a bit reluctant to set it as debug.
Or have you something else in mind ?

Thanks.
---

Thanks for reviewing !