On Fri, Feb 28, 2014 at 11:07 AM, Jenny Tc <jenny.tc@xxxxxxxxx> 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? The preference is different, usually __ is for compiler things, but while I dislike it (disturbs my perception) I can sure live with it. >> Why are you packing these structs? If no real reason, remove it. >> The compiler will pack what it thinks is appropriate anyway. > > The structure is part of the battery charging profile which can be read directly > from an EEPROM or from secondary storage (emmc). So it should be packed to keep > it align with the stored format. OK I buy that. Make sure this is noted somewhere (or maybe I missed it). Yours, Linus Walleij -- 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
