2014-11-26 4:03 GMT+08:00 Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>:
> 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@xxxxxxxxxxxxxx>
>> Signed-off-by: Orson Zhai <orson.zhai@xxxxxxxxxxxxxx>
>> Originally-by: Lanqing Liu <lanqing.liu@xxxxxxxxxxxxxx>
>
> 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 devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
