Hello.
Вторник, 11 февраля 2014, 18:45 +01:00 от Richard Genoud <richard.genoud@xxxxxxxxx>:
> 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@xxxxxxxxx>
> ---
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
...
> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
...
> +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.
---
��.n��������+%������w��{.n�����{��ǫ����{ay� ʇڙ���f���h������_�(�階�ݢj"��������G����?���&��
