Re: [PATCH v4 1/5] tty/serial: Add GPIOLIB helpers for controlling modem lines

[Alexander Shiyan]

Hello Richard.

Среда, 26 февраля 2014, 17:19 +01:00 от Richard Genoud <richard.genoud at gmail.com>:
> 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 <richard.genoud at gmail.com>

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

...
> +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.

...
> +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.


> +		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().

Thanks.
---