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:	Thu, 27 Nov 2014 19:05:05 +0800
From:	Lyra Zhang <zhang.lyra@...il.com>
To:	Greg KH <gregkh@...uxfoundation.org>
Cc:	Chunyan Zhang <chunyan.zhang@...eadtrum.com>,
	Grant Likely <grant.likely@...aro.org>,
	"robh+dt@...nel.org" <robh+dt@...nel.org>,
	Catalin Marinas <catalin.marinas@....com>,
	"ijc+devicetree@...lion.org.uk" <ijc+devicetree@...lion.org.uk>,
	"jslaby@...e.cz" <jslaby@...e.cz>,
	Kumar Gala <galak@...eaurora.org>,
	Mark Brown <broonie@...aro.org>,
	Mark Rutland <mark.rutland@....com>,
	"m-karicheri2@...com" <m-karicheri2@...com>,
	Pawel Moll <pawel.moll@....com>,
	Ramkumar Ramachandra <artagnon@...il.com>,
	"rrichter@...ium.com" <rrichter@...ium.com>,
	Will Deacon <will.deacon@....com>,
	Arnd Bergmann <arnd@...db.de>, gnomes@...rguk.ukuu.org.uk,
	Jonathan Corbet <corbet@....net>, jason@...edaemon.net,
	Mark Brown <broonie@...nel.org>,
	Heiko Stübner <heiko@...ech.de>,
	shawn.guo@...escale.com, florian.vaussard@...l.ch, andrew@...n.ch,
	Hayato Suzuki <hytszk@...il.com>,
	Orson Zhai <orsonzhai@...il.com>,
	"geng.ren@...eadtrum.com" <geng.ren@...eadtrum.com>,
	"zhizhou.zhang" <zhizhou.zhang@...eadtrum.com>,
	lanqing.liu@...eadtrum.com,
	Wei Qiao (乔伟) <wei.qiao@...eadtrum.com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"sprdlinux@...elists.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

2014-11-26 4:03 GMT+08:00 Greg KH <gregkh@...uxfoundation.org>:
> 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.
>
OK, will remove it in v4.
>
>> +                .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.
>
OK, will remove it.
>> +
>> +     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.
OK.


Thanks for your comments!
Chunyan
--
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/