On Fri 2014-02-28 08:37:27, Jenny Tc wrote: > On Thu, Feb 27, 2014 at 09:18:57PM +0100, Linus Walleij wrote: > > On Tue, Feb 4, 2014 at 6:12 AM, Jenny TC <jenny.tc@xxxxxxxxx> wrote: > > > > > +static inline bool __is_battery_full > > > + (long volt, long cur, long iterm, unsigned long cv) > > > > Overall I wonder if you've run checkpatch on these patches, but why > > are you naming this one function with a double __underscore? > > Just is_battery_full_check() or something would work fine I guess? > > Just to convey that is_battery_full is a local function and not generic. You > can find similar usage in power_supply_core.c (__power_supply_changed_work) > and in other drivers. Isn't it advised to have __ prefixes? It is static; everybody sees it is local. __ prefix usually means something else. > > > + u16 capacity; /* mAh */ > > > + u16 voltage_max; /* mV */ > > > + /* charge termination current */ > > > + u16 chrg_term_mA; > > > + /* Low battery level voltage */ > > > + u16 low_batt_mV; > > > + /* upper and lower temperature limits on discharging */ > > > + s8 disch_temp_ul; /* Degree Celsius */ > > > + s8 disch_temp_ll; /* Degree Celsius */ > > > + /* number of temperature monitoring ranges */ > > > + u16 temp_mon_ranges; > > > + struct psy_ps_temp_chg_table temp_mon_range[BATT_TEMP_NR_RNG]; > > > + /* lowest temperature supported */ > > > + s8 temp_low_lim; > > > +} __packed; > > > > Why packed, and convert to kerneldoc for this struct. > > Battery charging profile, may come from EEPROM/emmc which would be packed. Do you need to do endianness conversion, too? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
