2014-03-03 12:27 GMT+01:00 Alexander Shiyan <shc_work@xxxxxxx>:
> Hello.
>
> Понедельник, 3 марта 2014, 12:11 +01:00 от Richard Genoud <noreply.rgenoud@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>
> ...
>> +struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
>> +{
>> + struct mctrl_gpios *gpios;
>> + enum mctrl_gpio_idx i;
>> + int err;
>> +
>> + gpios = devm_kzalloc(dev, sizeof(*gpios), GFP_KERNEL);
>> + if (!gpios)
>> + goto out;
>
> if (!gpios)
> return ERR_PTR(-ENOMEM);
I think we are looping here...
If I returns a ERR_PTR(-ENOMEM), I'll have to change back all tests
from if (!gpios) to if (IS_ERR_OR_NULL(gpios))
So, I'll just return NULL here, and change the test in atmel_serial.c
(that was wrong anyway, as you pointed out)
>
>> + for (i = 0; i < UART_GPIO_MAX; i++) {
>> + gpios->gpio[i] = devm_gpiod_get_index(dev,
>> + mctrl_gpios_desc[i].name,
>> + idx);
>> +
>> + /*
>> + * The GPIOs are maybe not all filled,
>> + * this is not an error.
>> + */
>> + if (IS_ERR_OR_NULL(gpios->gpio[i]))
>> + continue;
>> +
>> + if (mctrl_gpios_desc[i].dir_out)
>> + err = gpiod_direction_output(gpios->gpio[i], 0);
>> + else
>> + err = gpiod_direction_input(gpios->gpio[i]);
>> + if (err) {
>> + dev_warn(dev, "Unable to set direction for %s GPIO",
>> + mctrl_gpios_desc[i].name);
>> + devm_gpiod_put(dev, gpios->gpio[i]);
>> + gpios->gpio[i] = NULL;
>> + }
>> + }
>> +
>> +out:
>> + return gpios;
>> +}
>> +EXPORT_SYMBOL_GPL(mctrl_gpio_init);
>> +
>> +void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
>> +{
>> + enum mctrl_gpio_idx i;
>> +
>> + if (!gpios)
>> + return;
>> +
>> + for (i = 0; i < UART_GPIO_MAX; i++)
>> + if (!IS_ERR_OR_NULL(gpios->gpio[i])) {
>> + devm_gpiod_put(dev, gpios->gpio[i]);
>> + gpios->gpio[i] = NULL;
>
> No need to NULL this variable. Instead, you should NULL
> "gpios" entirely at the end of this function.
that's right, setting to NULL here is now useless.
>> + }
>> + devm_kfree(dev, gpios);
>
> gpios = NULL;
> So this will indicate that its no more valid.
well, this won't help...
to do that, we'll have to pass a struct mctrl_gpios** instead of a
struct mctrl_gpios *
>> +}
> ...
>
> Thanks.
> ---
thanks !
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
