On Tue, Feb 7, 2017 at 3:09 AM, Wojciech Ziemba <wo.ziemba@xxxxxxxxx> wrote: > There is interest in adding a Linux driver for TI BQ2416X battery > charger. The driver supports BQ24160 chip, thus can be easily extended > for other BQ2416X family chargers. > Device exposes 'POWER_SUPPLY_PROP_*' properties and a number of knobs > for controlling the charging process as well as sends power supply change > notification via power-supply subsystem. Some style related comments 0. Less lines of code -> better! (but not be too eager) 1. Use GENMASK() 2. int ret = -EINVAL; if (a) ret = x; else if (b) ret = y; >> else >> ret = -EINVAL; Those are redundant. if (ret) return ret; 3. #ifdef:s are ugly. For CONFIG_PM_SLEEP functions just use __maybe_unused attribute. 4. Try to avoid #ifdef CONFIG_OF (this will limit driver for OF case when it might be used elsewhere, e.g. ACPI case) 5. Check headers and library for existing helpers. I believe some of your bit operations and such already have nice helpers in kernel. -- With Best Regards, Andy Shevchenko -- 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
