[PATCH v2 6/6] serial: 8250: make port operations work on struct uart_8250_port
Andy Shevchenko
andriy.shevchenko at linux.intel.com
Tue Oct 13 03:00:16 PDT 2026
On Tue, Oct 13, 2026 at 11:39:00AM +0200, Jiri Slaby wrote:
> On 13. 10. 26, 11:09, Andy Shevchenko wrote:
> > On Tue, Oct 13, 2026 at 09:53:26AM +0200, Jiri Slaby wrote:
> > > On 13. 10. 26, 9:33, Andy Shevchenko wrote:
> > > > On Tue, Oct 13, 2026 at 08:48:51AM +0200, Jiri Slaby (SUSE) wrote:
...
> > > > > - 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;
> > > > > + }
> > >
> > > > 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.
The keyword is *slightly*. I do not see much of deviation in style in the
subsystem maintainer-XXX.rst. Do you?
> > Linus also told it many times
>
> Which is neither an argument. Linus is wrong at times ("Link:" use as an
> example).
Link is different, it wasn't about the code. So, for coding style we have stricter
rules.
> 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...
> > > 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.)
Isn't `git grep` fast enough due to looking into index and not into the files?
`rg` may not look at the Git index, right? So I may not to use an arbitrary
hash without checking it out first.
> > 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.
Please, provide a better search line, I will look at that. For now it's just
a hypothetical assumption, I am afraid.
--
With Best Regards,
Andy Shevchenko
More information about the linux-arm-kernel
mailing list