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

[Andy Shevchenko]

Thanks, my comments below.


On Tue, Jul 21, 2020 at 1:41 PM Abanoub Sameh <abanoubsameh8@xxxxxxxxx> 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