Quoting Maulik Shah (2020-03-09 03:58:27) > > On 3/7/2020 12:12 PM, Bjorn Andersson wrote: > > On Thu 05 Mar 23:23 PST 2020, Maulik Shah wrote: > >> diff --git a/drivers/soc/qcom/soc_sleep_stats.c b/drivers/soc/qcom/soc_sleep_stats.c > >> new file mode 100644 > >> index 00000000..79a14d2 > >> --- /dev/null > >> +++ b/drivers/soc/qcom/soc_sleep_stats.c > >> @@ -0,0 +1,248 @@ [...] > >> + u32 pid; > >> +}; > >> + > >> +static struct subsystem_data subsystems[] = { > >> + { "modem", MODEM_ITEM_ID, PID_MODEM }, > >> + { "adsp", ADSP_ITEM_ID, PID_ADSP }, > >> + { "cdsp", CDSP_ITEM_ID, PID_CDSP }, > >> + { "slpi", SLPI_ITEM_ID, PID_SLPI }, > >> + { "gpu", GPU_ITEM_ID, PID_APSS }, > >> + { "display", DISPLAY_ITEM_ID, PID_APSS }, > >> +}; > >> + > >> +struct stats_config { > >> + unsigned int offset_addr; > >> + unsigned int num_records; > >> + bool appended_stats_avail; > >> +}; > >> + > >> +static const struct stats_config *config; > > Add this to soc_sleep_stats_data and pass that as s->private instead of > > just the reg, to avoid the global variable. > > No, this is required to keep global. we are not passing just reg as s->private, > we are passing "reg + offset" which differs for each stat. Can you please stop sending these review comment replies and then immediately turning around and sending the next revision of the patch series. I doubt that the changes take less than an hour to write and it would be helpful for everyone involved to have constructive discussions about the code if there's ever a response besides "done" or "ok". In this case it should be possible to get rid of the global 'config'. Make a new container struct to hold the config and offset or figure out what actually needs to be passed into the functions instead and do that when the files are created.