On Wed, Jun 7, 2017 at 3:07 PM, Sakari Ailus <sakari.ailus@xxxxxx> wrote: >> +static int ti_tps68470_pmic_get_power(struct regmap *regmap, int reg, >> + int bitmask, u64 *value) >> +{ >> + int data; > > Shouldn't you use unsigned int here? Same in the functions below. +1, regmap_read() returns unsigned int. >> +static acpi_status ti_pmic_common_handler(u32 function, > + acpi_physical_address address, > + u32 bits, u64 *value, > + void *handler_context, > handler_context is unused. >> + int, int, u64 *), >> + int (*update)(struct regmap *, >> + int, int, u64), >> + struct ti_pmic_table *table, >> + int table_size) I would even split this to have separate update() and get() paths instead of having such a monster of parameters. >> +static acpi_status ti_pmic_clk_freq_handler(u32 function, >> + acpi_physical_address address, >> + u32 bits, u64 *value, >> + void *handler_context, >> + void *region_context) >> +{ >> + return ti_pmic_common_handler(function, address, bits, value, >> + handler_context, region_context, >> + ti_tps68470_pmic_get_clk_freq, >> + ti_tps68470_regmap_update_bits, >> + (struct ti_pmic_table *) &clk_freq_table, > > You shouldn't use an explicit cast here. Instead make the function argument > const as well and you're fine. +1. -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html