linux-kernel - Re: [PATCH v3 5/5] tty/serial: Add Spreadtrum sc9836-uart driver support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 25 Nov 2014 12:03:53 -0800
From:	Greg KH <gregkh@...uxfoundation.org>
To:	Chunyan Zhang <chunyan.zhang@...eadtrum.com>
Cc:	grant.likely@...aro.org, robh+dt@...nel.org,
	catalin.marinas@....com, ijc+devicetree@...lion.org.uk,
	jslaby@...e.cz, galak@...eaurora.org, broonie@...aro.org,
	mark.rutland@....com, m-karicheri2@...com, pawel.moll@....com,
	artagnon@...il.com, rrichter@...ium.com, will.deacon@....com,
	arnd@...db.de, gnomes@...rguk.ukuu.org.uk, corbet@....net,
	jason@...edaemon.net, broonie@...nel.org, heiko@...ech.de,
	shawn.guo@...escale.com, florian.vaussard@...l.ch, andrew@...n.ch,
	hytszk@...il.com, orsonzhai@...il.com, geng.ren@...eadtrum.com,
	zhizhou.zhang@...eadtrum.com, lanqing.liu@...eadtrum.com,
	zhang.lyra@...il.com, wei.qiao@...eadtrum.com,
	devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, sprdlinux@...elists.org,
	linux-doc@...r.kernel.org, linux-serial@...r.kernel.org,
	linux-api@...r.kernel.org
Subject: Re: [PATCH v3 5/5] tty/serial: Add Spreadtrum sc9836-uart driver
 support

On Tue, Nov 25, 2014 at 08:16:58PM +0800, Chunyan Zhang wrote:
> Add a full sc9836-uart driver for SC9836 SoC which is based on the
> spreadtrum sharkl64 platform.
> This driver also support earlycon.
> 
> Signed-off-by: Chunyan Zhang <chunyan.zhang@...eadtrum.com>
> Signed-off-by: Orson Zhai <orson.zhai@...eadtrum.com>
> Originally-by: Lanqing Liu <lanqing.liu@...eadtrum.com>

Some objections:

> +static struct platform_driver sprd_platform_driver = {
> +	.probe = sprd_probe,
> +	.remove = sprd_remove,
> +	.suspend = sprd_suspend,
> +	.resume = sprd_resume,
> +	.driver = {
> +		   .name = "sprd_serial",
> +		   .owner = THIS_MODULE,

platform drivers don't need .owner in them anymore, it's handled by the
driver core automatically.


> +		   .of_match_table = of_match_ptr(serial_ids),
> +		   },
> +};
> +
> +static int __init sprd_serial_init(void)
> +{
> +	int ret = 0;
> +
> +	ret = uart_register_driver(&sprd_uart_driver);
> +	if (unlikely(ret != 0))
> +		return ret;

NEVER use unlikely unless you can measure the difference without it.
And even then, you better be able to justify it.  For something as dumb
as this type of check, youare working against the complier and cpu which
already knows that 0 is the usual response.

So please remove all usages of likely/unlikely in this patch series.

> +
> +	ret = platform_driver_register(&sprd_platform_driver);
> +	if (unlikely(ret != 0))
> +		uart_unregister_driver(&sprd_uart_driver);
> +
> +	return ret;
> +}
> +
> +static void __exit sprd_serial_exit(void)
> +{
> +	platform_driver_unregister(&sprd_platform_driver);
> +	uart_unregister_driver(&sprd_uart_driver);
> +}
> +
> +module_init(sprd_serial_init);
> +module_exit(sprd_serial_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Spreadtrum SoC serial driver series");
> diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
> index 16ad852..d9a8c88 100644
> --- a/include/uapi/linux/serial_core.h
> +++ b/include/uapi/linux/serial_core.h
> @@ -247,4 +247,7 @@
>  /* MESON */
>  #define PORT_MESON	109
>  
> +/* SPRD SERIAL  */
> +#define PORT_SPRD   110

Please use a tab character here.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/