Messages in this thread | | | Date | Mon, 7 Oct 2019 11:44:29 +0300 | From | Andy Shevchenko <> | Subject | Re: [PATCH v16 11/14] thermal: intel: intel_soc_dts_iosf: Utilize for_each_set_clump8 macro |
| |
On Sun, Oct 06, 2019 at 11:11:08AM -0400, William Breathitt Gray wrote: > Utilize for_each_set_clump8 macro, and the bitmap_set_value8 and > bitmap_get_value8 functions, where appropriate. In addition, remove the > now unnecessary temp_mask and temp_shift members of the > intel_soc_dts_sensor_entry structure.
Since it perhaps will be next version, I have few style comments here (ignore them if you are not going to send a new version by some other reasons).
> int status; > u32 temp_out;
> + unsigned long update_ptps;
I think it's better to put it one line below.
> u32 out; > u32 store_ptps; > u32 store_ptmc;
> - out = (store_ptps & ~(0xFF << (thres_index * 8))); > - out |= (temp_out & 0xFF) << (thres_index * 8); > + update_ptps = store_ptps; > + bitmap_set_value8(&update_ptps, temp_out & 0xFF, thres_index * 8); > + out = update_ptps;
+ blank line?
After this change it seems we may drop temp_out and use out instead.
> - out = (out & dts->temp_mask) >> dts->temp_shift; > + temp_raw = out; > + out = bitmap_get_value8(&temp_raw, dts->id * 8);
> out -= SOC_DTS_TJMAX_ENCODING; > *temp = sensors->tj_max - out * 1000;
We may also join these together, though it's up to you.
> char name[10]; > int trip_count = 0;
> + int writable_trip_count = 0;
Perhaps move it after next line, or before previous one.
> int trip_mask = 0; > u32 store_ptps; > int ret;
> - int i; > + unsigned long i;
We may skip this change, but if we go with it, better to place before 'int ret;' line.
> + unsigned long trip; > + unsigned long ptps;
I would group each of these with relative group of definitions above.
> if (notification_support) { > trip_count = min(SOC_MAX_DTS_TRIPS, trip_cnt); > + writable_trip_count = trip_count - read_only_trip_cnt;
Maybe writable_trip_count -> writable_trip_cnt? (in align with r/o one).
> + trip_mask = GENMASK(writable_trip_count - 1, 0); > }
-- With Best Regards, Andy Shevchenko
| |