Re: [PATCH net-next 4/4] net: dsa: realtek: add LED drivers for rtl8366rb

[Luiz Angelo Daros de Luca]

> > OK, I'll add _group_/_GROUP_ both to the enum name and macros. Led
> > blink rate, for example, is global, used by all groups. However, it
> > will be difficult to respect the 80 columns limit passing
> > RTL8366RB_LED_GROUP_OFF to a rb8366rb_set_ledgroup_mode function with
> > only two levels of indentation. Do you have any recommendations?

Hi Andrew,

> https://www.kernel.org/doc/html/v4.10/process/coding-style.html
>
>   Now, some people will claim that having 8-character indentations
>   makes the code move too far to the right, and makes it hard to read
>   on a 80-character terminal screen. The answer to that is that if you
>   need more than 3 levels of indentation, you’re screwed anyway, and
>   should fix your program.

I need 3, not more than 3.

>   Functions should be short and sweet, and do just one thing. They
>   should fit on one or two screenfuls of text (the ISO/ANSI screen
>   size is 80x24, as we all know), and do one thing and do that well.
>
> Maybe you need to use more helper functions?

The call that violates (by 1) the limit is to
rb8366rb_set_ledgroup_mode(). With its name (a little long), the now
5-char longer macro/enum and 3 tabs, it has 81 columns when I align
the argument to the opening parenthesis.

static int rtl8366rb_setup(struct dsa_switch *ds)
{
       (...)
       if (priv->leds_disabled) {
               /* Turn everything off */
               regmap_update_bits(priv->map,
                                  RTL8366RB_INTERRUPT_CONTROL_REG,
                                  RTL8366RB_P4_RGMII_LED,
                                  0);

               for (i = 0; i < RTL8366RB_NUM_LEDGROUPS; i++) {
                       ret = rb8366rb_set_ledgroup_mode(priv, i,
                                                        RTL8366RB_LEDGROUP_OFF);
                       if (ret)
                               return ret;
               }
        }
}

Should I rename the rb8366rb_set_ledgroup_mode function,
RTL8366RB_LEDGROUP_OFF or is the violation here acceptable?

Can I use the double tab indentation here like it appears in
https://elixir.bootlin.com/linux/latest/source/net/8021q/vlanproc.c#L120?

Regards,

Luiz