On Tue, Dec 26, 2017 at 11:26:00AM +0800, Jiaxun Yang wrote: > diff --git a/drivers/platform/mips/yeeloong_laptop.c > b/drivers/platform/mips/yeeloong_laptop.c > new file mode 100755
Checkpatch complains about having the execute permission set on this
file.
> +static int __init yeeloong_init(void)
> +{
> + int ret;
> +
> + if (mips_machtype != MACH_LEMOTE_YL2F89) {
> + pr_err("Unsupported system.\n");
> + return -ENODEV;
> + }
> +
> + pr_info("Load YeeLoong Laptop Platform Specific Driver.\n");
> +
> + /* Register platform stuff */
> + ret = platform_driver_register(&platform_driver);
> + if (ret) {
> + pr_err("Fail to register yeeloong platform driver.\n");
> + return ret;
> + }
> +
> + ret = yeeloong_backlight_init();
> + if (ret) {
> + pr_err("Fail to register yeeloong backlight driver.\n");
> + yeeloong_backlight_exit();
> + return ret;
> + }
> +
> + ret = yeeloong_bat_init();
> + if (ret) {
> + pr_err("Fail to register yeeloong battery driver.\n");
> + yeeloong_bat_exit();
> + return ret;
> + }
> +
> + ret = yeeloong_hwmon_init();
> + if (ret) {
> + pr_err("Fail to register yeeloong hwmon driver.\n");
> + yeeloong_hwmon_exit();
> + return ret;
> + }
> +
> + ret = yeeloong_hotkey_init();
> + if (ret) {
> + pr_err("Fail to register yeeloong hotkey driver.\n");
> + yeeloong_hotkey_exit();
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void __exit yeeloong_exit(void)
> +{
> + yeeloong_hotkey_exit();
> + yeeloong_hwmon_exit();
> + yeeloong_bat_exit();
> + yeeloong_backlight_exit();
I can't help thinking it would be better to separate this into separate
drivers for each part (backlight, power supply etc), and move them into
the appropriate driver directories (drivers/power/supply,
drivers/video/backlight etc). That way each part would get proper review
from the appropriate maintainers (or at least they should be Cc'd).
Is there a particular reason for it to be a single driver?
Cheers
James
signature.asc
Description: Digital signature
