Re: [PATCH v2 6/6] serial: 8250: make port operations work on struct uart_8250_port

On 13. 10. 26, 11:09, Andy Shevchenko wrote:

On Tue, Oct 13, 2026 at 09:53:26AM +0200, Jiri Slaby wrote:

quoted

On 13. 10. 26, 9:33, Andy Shevchenko wrote:

quoted

On Tue, Oct 13, 2026 at 08:48:51AM +0200, Jiri Slaby (SUSE) wrote:

...

quoted

quoted

quoted

-	rc = serial8250_register_8250_port(&port);
-	if (rc < 0)
+	struct uart_8250_port *uport = serial8250_register_8250_port(&port);
+	if (IS_ERR(uport)) {
+		rc = PTR_ERR(uport);
   		goto err_sysfs_remove;
+	}

quoted

Also do not define
variables in the middle of the code (it's only allowed for auto cleanup
variables and for-loop iterators).

Why, actually?

Because it's the style.

There is no hard style though. It varies across subsystems.

Linus also told it many times

Which is neither an argument. Linus is wrong at times ("Link:" use as an example).

All in all, I do not insist on this at all. It's easier to read and follow the code for me. Not necessarily the case for others...

quoted

First, it is used in the kernel quite a few times already
[1]. Second, while it is sort of c++ism (and I like to use modern c++), it
makes the life of the variable a lot more obvious.

[1] eg. see output of: rg -U -C 5 '}\n+\s+struct .*\*.*=' drivers/

(you made me to install something I haven't heard about, it's better to use
  `git grep` though)

rg is order of magnitude faster (searches in parallel). More importantly in this case, supports multiline matches. (And respects .git ignores.)

Note excluded __free() if I get regexp correctly

$ rg -U -l '}\n+\s+struct .*\*[^)]*=' drivers/ | less
drivers/ufs/host/ufs-qcom.c

...

drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c

So, less than dozen of drivers doing wrong is not an argument, try something
better. But thanks, I will check and address this in some of the above.

Well, it's only an example of a declaration after a previous opening brace "{" -- this one turned out to be easy to search for. There is a lot of more, of course. kernel/ included.

thanks,
-- 
js
suse labs