linux-kernel - Re: [PATCH] gpio: fixed some coding style issues in gpiolib files

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 21 Jul 2020 13:55:39 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Abanoub Sameh <abanoubsameh8@...il.com>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Abanoub Sameh <abanoubsameh@...tonmail.com>
Subject: Re: [PATCH] gpio: fixed some coding style issues in gpiolib files

Thanks, my comments below.


On Tue, Jul 21, 2020 at 1:41 PM Abanoub Sameh <abanoubsameh8@...il.com> wrote:

Commit message is missed on why you did all these changes.

...

>         if (pin <= 255) {
>                 char ev_name[5];
> +
>                 sprintf(ev_name, "_%c%02hhX",
>                         agpio->triggering == ACPI_EDGE_SENSITIVE ? 'E' : 'L',
>                         pin);

Should be a separate patch.

...

>                 dev_err(chip->parent,
> -                       "Failed to install GPIO OpRegion handler\n");
> +                       "Failed to install GPIO OpRegion handler\n");
Make it simple in one line.

Also a separate patch.

...

> @@ -316,9 +316,8 @@ struct gpio_desc *gpiod_get_from_of_node(struct device_node *node,
>         desc = of_get_named_gpiod_flags(node, propname,
>                                         index, &flags);
>
> -       if (!desc || IS_ERR(desc)) {
> +       if (!desc || IS_ERR(desc))
>                 return desc;
> -       }

Also a separate patch.

...

> @@ -974,6 +974,7 @@ static irqreturn_t lineevent_irq_thread(int irq, void *p)
>         if (le->eflags & GPIOEVENT_REQUEST_RISING_EDGE
>             && le->eflags & GPIOEVENT_REQUEST_FALLING_EDGE) {
>                 int level = gpiod_get_value_cansleep(le->desc);
> +
>                 if (level)
>                         /* Emit low-to-high event */
>                         ge.id = GPIOEVENT_EVENT_RISING_EDGE;

This can be unified with a patch against gpiolib-acpi.

...

> -       if (is_of_node(fwspec->fwnode) && fwspec->param_count == 2) {
> +       if (is_of_node(fwspec->fwnode) && fwspec->param_count == 2)
>                 return irq_domain_translate_twocell(d, fwspec, hwirq, type);
> -       }

Unify with the patch above removing {}.

>         /* If a parent irqdomain is provided, let's build a hierarchy */
>         if (gpiochip_hierarchy_is_hierarchical(gc)) {
>                 int ret = gpiochip_hierarchy_add_domain(gc);
> +
>                 if (ret)
>                         return ret;

Similar as above.

And so on...

-- 
With Best Regards,
Andy Shevchenko