lkml.org 
[lkml]   [2020]   [Jul]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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@gmail.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

\
 
 \ /
  Last update: 2020-07-21 12:56    [W:0.031 / U:0.452 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site