On 04/11/2016 11:00 AM, Eric Nelson wrote:
Many drivers use a common form of offset + flags for device
tree nodes. e.g.:
        <&gpio1 2 GPIO_ACTIVE_LOW>

This patch adds a common implementation of this type of parsing
and calls it when a gpio driver doesn't supply its' own xlate
routine.

This will allow removal of the driver-specific versions in a
handful of drivers and simplify the addition of new drivers.

The series,
Acked-by: Stephen Warren <swar...@nvidia.com>

Although one nit below.

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c

+int gpio_xlate_offs_flags(struct udevice *dev,
+                                        struct gpio_desc *desc,
+                                        struct fdtdec_phandle_args *args)
+{
+       int ret = -EINVAL;
+       if (args->args_count > 1) {
+               desc->offset = args->args[0];
+               if (args->args[1] & GPIO_ACTIVE_LOW)
+                       desc->flags = GPIOD_ACTIVE_LOW;
+               ret = 0;
+       }
+       return ret;
+}

Why not return the error first, and avoid the need for an extra indentation level for the rest of the function, and make it quite a bit more readable in my opinion. The default could also be made to cover more situations (e.g. bindings that define a single cell for the GPIO but not cell at all for any flags):

if (args->args_count < 1)
    return -EINVAL;

desc->offset = args->args[0];

if (args->args_count < 2)
    return 0;

if (args->args[1] & GPIO_ACTIVE_LOW)
    desc->flags = GPIOD_ACTIVE_LOW;

return 0;
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to