Received-SPF: pass (google.com: domain of linux-kernel+bounces-112727-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Sun, 24 Mar 2024 16:32:53 +0100
From: Andrew Lunn <andrew@lunn.ch>
To: Luiz Angelo Daros de Luca <luizluca@gmail.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Alvin =?utf-8?Q?=C5=A0ipraga?= <alsi@bang-olufsen.dk>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Vladimir Oltean <olteanv@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 4/4] net: dsa: realtek: add LED drivers for
 rtl8366rb
Message-ID: <f76754f9-4141-4d48-81e8-f43aa2dfa90c@lunn.ch>
References: <20240310-realtek-led-v1-0-4d9813ce938e@gmail.com>
 <20240310-realtek-led-v1-4-4d9813ce938e@gmail.com>
 <d064b1be-1004-487b-9944-b62d91b671c9@lunn.ch>
 <CAJq09z54+049aPL2LzAqAFigrvpchPhv_YQ6yJ5C9b9J7mngLQ@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAJq09z54+049aPL2LzAqAFigrvpchPhv_YQ6yJ5C9b9J7mngLQ@mail.gmail.com>

> 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?

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.

  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?

      Andrew