Messages in this thread | | | From | Andy Shevchenko <> | Date | Wed, 7 Jun 2017 16:37:12 +0300 | Subject | Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver |
| |
On Wed, Jun 7, 2017 at 3:07 PM, Sakari Ailus <sakari.ailus@iki.fi> 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
| |