Messages in this thread | | | Date | Thu, 16 Nov 2023 14:16:47 +0200 (EET) | From | Ilpo Järvinen <> | Subject | Re: [PATCH v2 3/5] tty: serial: amba-pl011: cleanup driver |
| |
On Thu, 16 Nov 2023, Théo Lebrun wrote:
> Follow recommandations from: > > $ ./scripts/checkpatch.pl --strict --file \ > drivers/tty/serial/amba-pl011.c > > It does NOT fix alerts relative to TIOCMBIT which will be dealt with in > another patch. Fixes following alerts: > > CHECK: Alignment should match open parenthesis > CHECK: Blank lines aren't necessary after an open brace '{' > CHECK: Comparison to NULL could be written [...] > CHECK: Lines should not end with a '(' > CHECK: Please don't use multiple blank lines > CHECK: Please use a blank line after function/struct/union/enum declarations > CHECK: Prefer using the BIT macro > CHECK: Unbalanced braces around else statement > CHECK: Unnecessary parentheses around [...] > CHECK: braces {} should be used on all arms of this statement > CHECK: spaces preferred around that '/' (ctx:VxV) > CHECK: spaces preferred around that '|' (ctx:VxV) > ERROR: do not initialise statics to false > WARNING: Comparisons should place the constant on the right side of the test > WARNING: Possible unnecessary 'out of memory' message > WARNING: Prefer 'unsigned int' to bare use of 'unsigned' > WARNING: Prefer [subsystem eg: netdev]_info([subsystem]dev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ... > WARNING: quoted string split across lines > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> > ---
> @@ -2916,9 +2911,12 @@ static int sbsa_uart_probe(struct platform_device *pdev) > if (qdf2400_e44_present) { > dev_info(&pdev->dev, "working around QDF2400 SoC erratum 44\n"); > uap->vendor = &vendor_qdt_qdf2400_e44; > - } else > -#endif > + } else { > uap->vendor = &vendor_sbsa; > + } > +#else > + uap->vendor = &vendor_sbsa; > +#endif
IMO, this would look a lot nicer if you make a function out of it (but perhaps it can be changed in a follow-up patch):
#ifdef CONFIG_ACPI_SPCR_TABLE static void qpdf2400_erratum44_workaround(struct platform_device *pdev, struct uart_amba_port *uap) { if (!qdf2400_e44_present) return;
dev_info(&pdev->dev, "working around QDF2400 SoC erratum 44\n"); uap->vendor = &vendor_qdt_qdf2400_e44; } #else static void qpdf2400_erratum44_workaround(struct platform_device *pdev, struct uart_amba_port *uap) { } #endif
static int sbsa_uart_probe(struct platform_device *pdev) { ... uap->vendor = &vendor_sbsa; qpdf2400_erratum44_workaround(pdev, uap); ... }
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
-- i.
| |