From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Mon, 11 Apr 2016 11:16:33 -0600 Subject: [U-Boot] [PATCH V2 1/6] dm: gpio: add a default gpio xlate routine In-Reply-To: <1460394019-3393-2-git-send-email-eric@nelint.com> References: <1459525662-28032-1-git-send-email-eric@nelint.com> <1460394019-3393-1-git-send-email-eric@nelint.com> <1460394019-3393-2-git-send-email-eric@nelint.com> Message-ID: <570BDBF1.1010602@wwwdotorg.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 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;