Hi Stephen, On 04/11/2016 10:16 AM, Stephen Warren wrote: > 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;
I like it. V3 of that one patch forthcoming. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot