From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anton Vorontsov Date: Fri, 16 Nov 2012 07:26:43 +0000 Subject: Re: [PATCH v8 1/3] Runtime Interpreted Power Sequences Message-Id: <20121116072642.GA22291@lizard> List-Id: References: <1353047903-14363-1-git-send-email-acourbot@nvidia.com> <1353047903-14363-2-git-send-email-acourbot@nvidia.com> In-Reply-To: <1353047903-14363-2-git-send-email-acourbot@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Alexandre Courbot Cc: Stephen Warren , Thierry Reding , Mark Zhang , Grant Likely , Rob Herring , Mark Brown , David Woodhouse , Arnd Bergmann , Leela Krishna Amudala , linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org, linux-pm@vger.kernel.org, Alexandre Courbot Hi Alexandre, The code looks neat, thanks for you work! Just a couple of comments... On Fri, Nov 16, 2012 at 03:38:21PM +0900, Alexandre Courbot wrote: [...] > + > +#include "power_seq_delay.c" > +#include "power_seq_regulator.c" > +#include "power_seq_pwm.c" > +#include "power_seq_gpio.c" This is odd, although I remember you already explained why you have to include the .c files, instead of linking them separately. But I forgot the reason. :) I think this deserves a comment in the code. > +static int of_power_seq_parse_step(struct device *dev, > + struct device_node *node, > + struct power_seq *seq, > + unsigned int step_nbr, > + struct list_head *resources) > +{ > + struct power_seq_step *step = &seq->steps[step_nbr]; > + struct power_seq_resource res, *res2; > + const char *type; > + int i, err; nit: one variable declaration per line. Thanks, Anton.