| Message ID | 20210301165932.62352-2-andriy.shevchenko@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/2] lib/cmdline: Export next_arg() for being used in modules | expand |
On Mon, Mar 1, 2021 at 6:00 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > cmdline library provides next_arg() helper to traverse over parameters > and their values given in command line. Replace custom approach in the driver > by it. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Wow how nice! Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
Hi Andy, On Mon, Mar 1, 2021 at 5:59 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > cmdline library provides next_arg() helper to traverse over parameters > and their values given in command line. Replace custom approach in the driver > by it. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Thanks for your patch! > --- a/drivers/gpio/gpio-aggregator.c > +++ b/drivers/gpio/gpio-aggregator.c > @@ -93,13 +68,9 @@ static int aggr_parse(struct gpio_aggregator *aggr) > if (!bitmap) > return -ENOMEM; > > - for (name = get_arg(&args), offsets = get_arg(&args); name; > - offsets = get_arg(&args)) { > - if (IS_ERR(name)) { > - pr_err("Cannot get GPIO specifier: %pe\n", name); > - error = PTR_ERR(name); > - goto free_bitmap; > - } > + args = next_arg(args, &name, &p); > + while (*args) { > + args = next_arg(args, &offsets, &p); As name and offsets should not contain equal signs (can they?), I guess using next_arg() is fine. Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
On Thu, Mar 04, 2021 at 10:01:46AM +0100, Geert Uytterhoeven wrote: > Hi Andy, > > On Mon, Mar 1, 2021 at 5:59 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > cmdline library provides next_arg() helper to traverse over parameters > > and their values given in command line. Replace custom approach in the driver > > by it. > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Thanks for your patch! > > > --- a/drivers/gpio/gpio-aggregator.c > > +++ b/drivers/gpio/gpio-aggregator.c > > @@ -93,13 +68,9 @@ static int aggr_parse(struct gpio_aggregator *aggr) > > if (!bitmap) > > return -ENOMEM; > > > > - for (name = get_arg(&args), offsets = get_arg(&args); name; > > - offsets = get_arg(&args)) { > > - if (IS_ERR(name)) { > > - pr_err("Cannot get GPIO specifier: %pe\n", name); > > - error = PTR_ERR(name); > > - goto free_bitmap; > > - } > > + args = next_arg(args, &name, &p); > > + while (*args) { > > + args = next_arg(args, &offsets, &p); > > As name and offsets should not contain equal signs (can they?), > I guess using next_arg() is fine. Even though we can use double quotes (AFAICU the next_arg() code). > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Thanks!
diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c index 08171431bb8f..34e35b64dcdc 100644 --- a/drivers/gpio/gpio-aggregator.c +++ b/drivers/gpio/gpio-aggregator.c @@ -37,31 +37,6 @@ struct gpio_aggregator { static DEFINE_MUTEX(gpio_aggregator_lock); /* protects idr */ static DEFINE_IDR(gpio_aggregator_idr); -static char *get_arg(char **args) -{ - char *start, *end; - - start = skip_spaces(*args); - if (!*start) - return NULL; - - if (*start == '"') { - /* Quoted arg */ - end = strchr(++start, '"'); - if (!end) - return ERR_PTR(-EINVAL); - } else { - /* Unquoted arg */ - for (end = start; *end && !isspace(*end); end++) ; - } - - if (*end) - *end++ = '\0'; - - *args = end; - return start; -} - static int aggr_add_gpio(struct gpio_aggregator *aggr, const char *key, int hwnum, unsigned int *n) { @@ -83,8 +58,8 @@ static int aggr_add_gpio(struct gpio_aggregator *aggr, const char *key, static int aggr_parse(struct gpio_aggregator *aggr) { + char *args = skip_spaces(aggr->args); char *name, *offsets, *p; - char *args = aggr->args; unsigned long *bitmap; unsigned int i, n = 0; int error = 0; @@ -93,13 +68,9 @@ static int aggr_parse(struct gpio_aggregator *aggr) if (!bitmap) return -ENOMEM; - for (name = get_arg(&args), offsets = get_arg(&args); name; - offsets = get_arg(&args)) { - if (IS_ERR(name)) { - pr_err("Cannot get GPIO specifier: %pe\n", name); - error = PTR_ERR(name); - goto free_bitmap; - } + args = next_arg(args, &name, &p); + while (*args) { + args = next_arg(args, &offsets, &p); p = get_options(offsets, 0, &error); if (error == 0 || *p) { @@ -125,7 +96,7 @@ static int aggr_parse(struct gpio_aggregator *aggr) goto free_bitmap; } - name = get_arg(&args); + args = next_arg(args, &name, &p); } if (!n) {
cmdline library provides next_arg() helper to traverse over parameters and their values given in command line. Replace custom approach in the driver by it. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/gpio/gpio-aggregator.c | 39 +++++----------------------------- 1 file changed, 5 insertions(+), 34 deletions(-)