| Message ID | 1447799871-56374-7-git-send-email-lina.iyer@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 17 November 2015 at 23:37, Lina Iyer <lina.iyer@linaro.org> wrote: > From: Marc Titinger <mtitinger@baylibre.com> > > This purpose of these debug seq-files is to help investigate > generic power domain state transitions, based on device constraints. > requires the "multiple states" patches from Axel Haslam. This last sentence doesn't belong in the change-log, please remove it. > > also rename 'summary' from 'pm_genpd_summary' > > sample output for 'states' > ========================== > > Domain State name Eval = 2200nter + Exit = Min_off_on (ns) > ----------------------------------------------------------------------- > a53_pd cluster-sleep-0 1500000+800000=2300000 > a57_pd d1-retention 1000000+800000=1800000 > a57_pd cluster-sleep-0 1500000+800000=2300000 > > sample output for 'timings' > =========================== > > Domain Devices, Timings in ns > Stop/Start Save/Restore, Effective > ---------------------------------------------------- --- > a53_pd > /cpus/cpu@100 1060 /660 1580 /1940 ,0 (cached stop) > /cpus/cpu@101 1060 /740 1520 /1600 ,0 (cached stop) > /cpus/cpu@102 880 /620 1380 /1780 ,0 (cached stop) > /cpus/cpu@103 1080 /640 1340 /1600 ,0 (cached stop) > a57_pd > /cpus/cpu@0 1160 /740 3280 /1800 ,0 (cached stop) > /cpus/cpu@1 780 /1400 1440 /2080 ,0 (cached stop) > /D1 600 /540 7140 /6420 ,2199460 (cached stop) > > Signed-off-by: Marc Titinger <mtitinger@baylibre.com> A general comment. Static functions in genpd shall start with one of the following prefix. genpd_* _genpd_* __genpd_* Please change accordingly. > --- > drivers/base/power/domain.c | 115 +++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 107 insertions(+), 8 deletions(-) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index c300293..9a0df09 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -2169,21 +2169,120 @@ static const struct file_operations pm_genpd_summary_fops = { > .release = single_release, > }; > > +static int pm_genpd_states_show(struct seq_file *s, void *data) > +{ > + struct generic_pm_domain *genpd; > + > + seq_puts(s, > + "\n Domain State name Enter + Exit = Min_off_on (ns)\n"); > + seq_puts(s, > + "-----------------------------------------------------------------------\n"); > + You must hold the gpd_list_lock while traversing the gpd list. > + list_for_each_entry(genpd, &gpd_list, gpd_list_node) { > + > + int i; > + > + for (i = 0; i < genpd->state_count; i++) { To be sure you have valid data, you should hold the genpd lock here. > + seq_printf(s, "%-20s %-20s %lld+%lld=%lld\n", > + genpd->name, > + genpd->states[i].name, > + genpd->states[i].power_on_latency_ns, > + genpd->states[i].power_off_latency_ns, > + genpd->states[i].power_off_latency_ns > + + genpd->states[i].power_on_latency_ns); > + } > + > + } > + > + seq_puts(s, "\n"); > + > + return 0; > +} > + > +static int pm_genpd_states_open(struct inode *inode, struct file *file) > +{ > + return single_open(file, pm_genpd_states_show, NULL); > +} > + > +static const struct file_operations pm_genpd_states_fops = { > + .open = pm_genpd_states_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = single_release, > +}; > + > +static int pm_genpd_timing_show(struct seq_file *s, void *data) > +{ > + struct generic_pm_domain *genpd; > + > + seq_puts(s, "\n Domain Devices, Timings in ns\n"); > + seq_puts(s, > + " Stop/Start Save/Restore, Effective\n"); > + seq_puts(s, > + "---------------------------------------------------- ---\n"); > + You must hold the gpd_list_lock while traversing the gpd list. > + list_for_each_entry(genpd, &gpd_list, gpd_list_node) { > + struct pm_domain_data *pm_data; > + > + seq_printf(s, "%-30s", genpd->name); > + You must hold the genpd lock while traversing the device list. > + list_for_each_entry(pm_data, &genpd->dev_list, list_node) { > + struct gpd_timing_data *td = &to_gpd_data(pm_data)->td; > + > + if (!pm_data->dev->of_node) > + continue; > + > + seq_printf(s, > + "\n %-20s %-6lld/%-6lld %-6lld/%-6lld,%lld %s%s", > + pm_data->dev->of_node->full_name, > + td->stop_latency_ns, td->start_latency_ns, > + td->save_state_latency_ns, > + td->restore_state_latency_ns, This needs a re-base as these values have been merged. > + td->effective_constraint_ns, > + td->cached_stop_ok ? "(cached stop) " : "", > + td->constraint_changed ? "(changed)" : ""); > + } > + seq_puts(s, "\n"); > + } > + return 0; > +} > + > +static int pm_genpd_timing_open(struct inode *inode, struct file *file) > +{ > + return single_open(file, pm_genpd_timing_show, NULL); > +} > + > +static const struct file_operations pm_genpd_timing_fops = { > + .open = pm_genpd_timing_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = single_release, > +}; > + > static int __init pm_genpd_debug_init(void) > { > - struct dentry *d; > + struct dentry *d = NULL; > > pm_genpd_debugfs_dir = debugfs_create_dir("pm_genpd", NULL); > > - if (!pm_genpd_debugfs_dir) > - return -ENOMEM; > + if (pm_genpd_debugfs_dir) In case when CONFIG_DEBUG_FS is unset, this doesn't work very well, as pm_genpd_debugfs_dir will contain an error code. Since you are anyway make quite big change to the debugfs support for genpd, perhaps you can try to fix that up as well!? > + d = debugfs_create_file("summary", S_IRUGO, > + pm_genpd_debugfs_dir, NULL, > + &pm_genpd_summary_fops); > + if (d) > + d = debugfs_create_file("states", S_IRUGO, > + pm_genpd_debugfs_dir, NULL, > + &pm_genpd_states_fops); > + if (d) > + d = debugfs_create_file("timings", S_IRUGO, > + pm_genpd_debugfs_dir, NULL, > + &pm_genpd_timing_fops); > + if (d) > + return 0; > > - d = debugfs_create_file("pm_genpd_summary", S_IRUGO, > - pm_genpd_debugfs_dir, NULL, &pm_genpd_summary_fops); > - if (!d) > - return -ENOMEM; > + debugfs_remove_recursive(pm_genpd_debugfs_dir /*can be null*/); > > - return 0; > + return -ENOMEM; > } > late_initcall(pm_genpd_debug_init); > Kind regards Uffe
On 11/12/2015 12:46, Ulf Hansson wrote: > On 17 November 2015 at 23:37, Lina Iyer <lina.iyer@linaro.org> wrote: Hi Ulf, thanks for the review, comments and questions below. Best Regards, Marc. >> From: Marc Titinger <mtitinger@baylibre.com> >> >> This purpose of these debug seq-files is to help investigate >> generic power domain state transitions, based on device constraints. >> requires the "multiple states" patches from Axel Haslam. > > This last sentence doesn't belong in the change-log, please remove it. > >> >> also rename 'summary' from 'pm_genpd_summary' >> >> sample output for 'states' >> ========================== >> >> Domain State name Eval = 2200nter + Exit = Min_off_on (ns) >> ----------------------------------------------------------------------- >> a53_pd cluster-sleep-0 1500000+800000=2300000 >> a57_pd d1-retention 1000000+800000=1800000 >> a57_pd cluster-sleep-0 1500000+800000=2300000 >> >> sample output for 'timings' >> =========================== >> >> Domain Devices, Timings in ns >> Stop/Start Save/Restore, Effective >> ---------------------------------------------------- --- >> a53_pd >> /cpus/cpu@100 1060 /660 1580 /1940 ,0 (cached stop) >> /cpus/cpu@101 1060 /740 1520 /1600 ,0 (cached stop) >> /cpus/cpu@102 880 /620 1380 /1780 ,0 (cached stop) >> /cpus/cpu@103 1080 /640 1340 /1600 ,0 (cached stop) >> a57_pd >> /cpus/cpu@0 1160 /740 3280 /1800 ,0 (cached stop) >> /cpus/cpu@1 780 /1400 1440 /2080 ,0 (cached stop) >> /D1 600 /540 7140 /6420 ,2199460 (cached stop) >> >> Signed-off-by: Marc Titinger <mtitinger@baylibre.com> > > A general comment. Static functions in genpd shall start with one of > the following prefix. > > genpd_* > _genpd_* > __genpd_* > > Please change accordingly. Many static routines were already prefixed like the exported functions with "pm_", shall I make a separate patch for this renaming ? > >> --- >> drivers/base/power/domain.c | 115 +++++++++++++++++++++++++++++++++++++++++--- >> 1 file changed, 107 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c >> index c300293..9a0df09 100644 >> --- a/drivers/base/power/domain.c >> +++ b/drivers/base/power/domain.c >> @@ -2169,21 +2169,120 @@ static const struct file_operations pm_genpd_summary_fops = { >> .release = single_release, >> }; >> >> +static int pm_genpd_states_show(struct seq_file *s, void *data) >> +{ >> + struct generic_pm_domain *genpd; >> + >> + seq_puts(s, >> + "\n Domain State name Enter + Exit = Min_off_on (ns)\n"); >> + seq_puts(s, >> + "-----------------------------------------------------------------------\n"); >> + > > You must hold the gpd_list_lock while traversing the gpd list. > >> + list_for_each_entry(genpd, &gpd_list, gpd_list_node) { >> + >> + int i; >> + >> + for (i = 0; i < genpd->state_count; i++) { > > To be sure you have valid data, you should hold the genpd lock here. > At some point while testing with calling suspend from the power_off handler, the cpu would go to sleep with the lock held, hence using this seq-file would not work. while I agree, I think it is not super likely that a domain/child/devices/states are added or removed at this point (the DT is parsed already), would using list_for_each_entry_safe be safe enough ? >> + seq_printf(s, "%-20s %-20s %lld+%lld=%lld\n", >> + genpd->name, >> + genpd->states[i].name, >> + genpd->states[i].power_on_latency_ns, >> + genpd->states[i].power_off_latency_ns, >> + genpd->states[i].power_off_latency_ns >> + + genpd->states[i].power_on_latency_ns); >> + } >> + >> + } >> + >> + seq_puts(s, "\n"); >> + >> + return 0; >> +} >> + >> +static int pm_genpd_states_open(struct inode *inode, struct file *file) >> +{ >> + return single_open(file, pm_genpd_states_show, NULL); >> +} >> + >> +static const struct file_operations pm_genpd_states_fops = { >> + .open = pm_genpd_states_open, >> + .read = seq_read, >> + .llseek = seq_lseek, >> + .release = single_release, >> +}; >> + >> +static int pm_genpd_timing_show(struct seq_file *s, void *data) >> +{ >> + struct generic_pm_domain *genpd; >> + >> + seq_puts(s, "\n Domain Devices, Timings in ns\n"); >> + seq_puts(s, >> + " Stop/Start Save/Restore, Effective\n"); >> + seq_puts(s, >> + "---------------------------------------------------- ---\n"); >> + > > You must hold the gpd_list_lock while traversing the gpd list. > > >> + list_for_each_entry(genpd, &gpd_list, gpd_list_node) { >> + struct pm_domain_data *pm_data; >> + >> + seq_printf(s, "%-30s", genpd->name); >> + > > You must hold the genpd lock while traversing the device list. > >> + list_for_each_entry(pm_data, &genpd->dev_list, list_node) { >> + struct gpd_timing_data *td = &to_gpd_data(pm_data)->td; >> + >> + if (!pm_data->dev->of_node) >> + continue; >> + >> + seq_printf(s, >> + "\n %-20s %-6lld/%-6lld %-6lld/%-6lld,%lld %s%s", >> + pm_data->dev->of_node->full_name, >> + td->stop_latency_ns, td->start_latency_ns, >> + td->save_state_latency_ns, >> + td->restore_state_latency_ns, > > This needs a re-base as these values have been merged. > >> + td->effective_constraint_ns, >> + td->cached_stop_ok ? "(cached stop) " : "", >> + td->constraint_changed ? "(changed)" : ""); >> + } >> + seq_puts(s, "\n"); >> + } >> + return 0; >> +} >> + >> +static int pm_genpd_timing_open(struct inode *inode, struct file *file) >> +{ >> + return single_open(file, pm_genpd_timing_show, NULL); >> +} >> + >> +static const struct file_operations pm_genpd_timing_fops = { >> + .open = pm_genpd_timing_open, >> + .read = seq_read, >> + .llseek = seq_lseek, >> + .release = single_release, >> +}; >> + >> static int __init pm_genpd_debug_init(void) >> { >> - struct dentry *d; >> + struct dentry *d = NULL; >> >> pm_genpd_debugfs_dir = debugfs_create_dir("pm_genpd", NULL); >> >> - if (!pm_genpd_debugfs_dir) >> - return -ENOMEM; >> + if (pm_genpd_debugfs_dir) > > In case when CONFIG_DEBUG_FS is unset, this doesn't work very well, as > pm_genpd_debugfs_dir will contain an error code. > > Since you are anyway make quite big change to the debugfs support for > genpd, perhaps you can try to fix that up as well!? Ah yes ok, will do. > >> + d = debugfs_create_file("summary", S_IRUGO, >> + pm_genpd_debugfs_dir, NULL, >> + &pm_genpd_summary_fops); >> + if (d) >> + d = debugfs_create_file("states", S_IRUGO, >> + pm_genpd_debugfs_dir, NULL, >> + &pm_genpd_states_fops); >> + if (d) >> + d = debugfs_create_file("timings", S_IRUGO, >> + pm_genpd_debugfs_dir, NULL, >> + &pm_genpd_timing_fops); >> + if (d) >> + return 0; >> >> - d = debugfs_create_file("pm_genpd_summary", S_IRUGO, >> - pm_genpd_debugfs_dir, NULL, &pm_genpd_summary_fops); >> - if (!d) >> - return -ENOMEM; >> + debugfs_remove_recursive(pm_genpd_debugfs_dir /*can be null*/); >> >> - return 0; >> + return -ENOMEM; >> } >> late_initcall(pm_genpd_debug_init); >> > > Kind regards > Uffe >
[...] >> >> A general comment. Static functions in genpd shall start with one of >> the following prefix. >> >> genpd_* >> _genpd_* >> __genpd_* >> >> Please change accordingly. > > > Many static routines were already prefixed like the exported functions with > "pm_", shall I make a separate patch for this renaming ? My point is that I don't want it to becomes worse. If you follow the above rule for new changes, I am happy. Whether you want to send a separate patch fixing the other existing name to be consistent with above rule, I would also be happy. :-) > > >> >>> --- >>> drivers/base/power/domain.c | 115 >>> +++++++++++++++++++++++++++++++++++++++++--- >>> 1 file changed, 107 insertions(+), 8 deletions(-) >>> >>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c >>> index c300293..9a0df09 100644 >>> --- a/drivers/base/power/domain.c >>> +++ b/drivers/base/power/domain.c >>> @@ -2169,21 +2169,120 @@ static const struct file_operations >>> pm_genpd_summary_fops = { >>> .release = single_release, >>> }; >>> >>> +static int pm_genpd_states_show(struct seq_file *s, void *data) >>> +{ >>> + struct generic_pm_domain *genpd; >>> + >>> + seq_puts(s, >>> + "\n Domain State name Enter + Exit = >>> Min_off_on (ns)\n"); >>> + seq_puts(s, >>> + >>> "-----------------------------------------------------------------------\n"); >>> + >> >> >> You must hold the gpd_list_lock while traversing the gpd list. >> >>> + list_for_each_entry(genpd, &gpd_list, gpd_list_node) { >>> + >>> + int i; >>> + >>> + for (i = 0; i < genpd->state_count; i++) { >> >> >> To be sure you have valid data, you should hold the genpd lock here. >> > > At some point while testing with calling suspend from the power_off handler, > the cpu would go to sleep with the lock held, hence using this seq-file > would not work. That seems like a bad behaviour during suspend. Why does it hold the lock? On the other it shouldn't matter as userspace can't access the debugfs nodes, since its frozen at those times, right!? > > while I agree, I think it is not super likely that a > domain/child/devices/states are added or removed at this point (the DT is > parsed already), would using list_for_each_entry_safe be safe enough ? No it's not. The gpd_list is protected with the gdp_list_lock, which is needed because new genpds can be added at any point. You also need the genpd lock here, as otherwise you may print the latency-values in the middle of when these are being updated. > > >>> + seq_printf(s, "%-20s %-20s %lld+%lld=%lld\n", >>> + genpd->name, >>> + genpd->states[i].name, >>> + genpd->states[i].power_on_latency_ns, >>> + genpd->states[i].power_off_latency_ns, >>> + genpd->states[i].power_off_latency_ns >>> + + >>> genpd->states[i].power_on_latency_ns); >>> + } >>> + >>> + } >>> + >>> + seq_puts(s, "\n"); >>> + >>> + return 0; >>> +} >>> + [...] Kind regards Uffe
On 16/12/2015 13:48, Ulf Hansson wrote: > [...] > >>> >>> A general comment. Static functions in genpd shall start with one of >>> the following prefix. >>> >>> genpd_* >>> _genpd_* >>> __genpd_* >>> >>> Please change accordingly. >> >> >> Many static routines were already prefixed like the exported functions with >> "pm_", shall I make a separate patch for this renaming ? > > My point is that I don't want it to becomes worse. > > If you follow the above rule for new changes, I am happy. > > Whether you want to send a separate patch fixing the other existing > name to be consistent with above rule, I would also be happy. :-) Fair enough, I'll do a renaming patch :) > >> >> >>> >>>> --- >>>> drivers/base/power/domain.c | 115 >>>> +++++++++++++++++++++++++++++++++++++++++--- >>>> 1 file changed, 107 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c >>>> index c300293..9a0df09 100644 >>>> --- a/drivers/base/power/domain.c >>>> +++ b/drivers/base/power/domain.c >>>> @@ -2169,21 +2169,120 @@ static const struct file_operations >>>> pm_genpd_summary_fops = { >>>> .release = single_release, >>>> }; >>>> >>>> +static int pm_genpd_states_show(struct seq_file *s, void *data) >>>> +{ >>>> + struct generic_pm_domain *genpd; >>>> + >>>> + seq_puts(s, >>>> + "\n Domain State name Enter + Exit = >>>> Min_off_on (ns)\n"); >>>> + seq_puts(s, >>>> + >>>> "-----------------------------------------------------------------------\n"); >>>> + >>> >>> >>> You must hold the gpd_list_lock while traversing the gpd list. >>> >>>> + list_for_each_entry(genpd, &gpd_list, gpd_list_node) { >>>> + >>>> + int i; >>>> + >>>> + for (i = 0; i < genpd->state_count; i++) { >>> >>> >>> To be sure you have valid data, you should hold the genpd lock here. >>> >> >> At some point while testing with calling suspend from the power_off handler, >> the cpu would go to sleep with the lock held, hence using this seq-file >> would not work. > > That seems like a bad behaviour during suspend. Why does it hold the lock? that was in the scenario where 2 or more cpus are devices of the same power domain. IIRC you would have something like: arm_enter_idle_state(cpu) pm_runtime_put_sync rpm_suspend __rpm_get_callback pm_genpd_runtime_suspend [Take lock] ->platform code to enter sleep... and race condition with another cpu of the same cluster trying to suspend or resume at the same time. it is a bad behaviour, I cannot test the os-initiated mode here but I assume this is done differently and is no longer an issue (sorry for not being more specific). > > On the other it shouldn't matter as userspace can't access the debugfs > nodes, since its frozen at those times, right!? you can have cpu_j off, and cpu_i running the shell, in the scenario above. But since this was while hacking calling psci from genpd.power_off, I'm not sure it's worth mentioning... > >> >> while I agree, I think it is not super likely that a >> domain/child/devices/states are added or removed at this point (the DT is >> parsed already), would using list_for_each_entry_safe be safe enough ? > > No it's not. > > The gpd_list is protected with the gdp_list_lock, which is needed > because new genpds can be added at any point. > > You also need the genpd lock here, as otherwise you may print the > latency-values in the middle of when these are being updated. Understood, I'll put the lock back. > >> >> >>>> + seq_printf(s, "%-20s %-20s %lld+%lld=%lld\n", >>>> + genpd->name, >>>> + genpd->states[i].name, >>>> + genpd->states[i].power_on_latency_ns, >>>> + genpd->states[i].power_off_latency_ns, >>>> + genpd->states[i].power_off_latency_ns >>>> + + >>>> genpd->states[i].power_on_latency_ns); >>>> + } >>>> + >>>> + } >>>> + >>>> + seq_puts(s, "\n"); >>>> + >>>> + return 0; >>>> +} >>>> + > > [...] > > Kind regards > Uffe >
========================== Domain State name Eval = 2200nter + Exit = Min_off_on (ns) ----------------------------------------------------------------------- a53_pd cluster-sleep-0 1500000+800000=2300000 a57_pd d1-retention 1000000+800000=1800000 a57_pd cluster-sleep-0 1500000+800000=2300000 sample output for 'timings' =========================== Domain Devices, Timings in ns Stop/Start Save/Restore, Effective ---------------------------------------------------- --- a53_pd /cpus/cpu@100 1060 /660 1580 /1940 ,0 (cached stop) /cpus/cpu@101 1060 /740 1520 /1600 ,0 (cached stop) /cpus/cpu@102 880 /620 1380 /1780 ,0 (cached stop) /cpus/cpu@103 1080 /640 1340 /1600 ,0 (cached stop) a57_pd /cpus/cpu@0 1160 /740 3280 /1800 ,0 (cached stop) /cpus/cpu@1 780 /1400 1440 /2080 ,0 (cached stop) /D1 600 /540 7140 /6420 ,2199460 (cached stop) Signed-off-by: Marc Titinger <mtitinger@baylibre.com> --- drivers/base/power/domain.c | 115 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 107 insertions(+), 8 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index c300293..9a0df09 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -2169,21 +2169,120 @@ static const struct file_operations pm_genpd_summary_fops = { .release = single_release, }; +static int pm_genpd_states_show(struct seq_file *s, void *data) +{ + struct generic_pm_domain *genpd; + + seq_puts(s, + "\n Domain State name Enter + Exit = Min_off_on (ns)\n"); + seq_puts(s, + "-----------------------------------------------------------------------\n"); + + list_for_each_entry(genpd, &gpd_list, gpd_list_node) { + + int i; + + for (i = 0; i < genpd->state_count; i++) { + seq_printf(s, "%-20s %-20s %lld+%lld=%lld\n", + genpd->name, + genpd->states[i].name, + genpd->states[i].power_on_latency_ns, + genpd->states[i].power_off_latency_ns, + genpd->states[i].power_off_latency_ns + + genpd->states[i].power_on_latency_ns); + } + + } + + seq_puts(s, "\n"); + + return 0; +} + +static int pm_genpd_states_open(struct inode *inode, struct file *file) +{ + return single_open(file, pm_genpd_states_show, NULL); +} + +static const struct file_operations pm_genpd_states_fops = { + .open = pm_genpd_states_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, +}; + +static int pm_genpd_timing_show(struct seq_file *s, void *data) +{ + struct generic_pm_domain *genpd; + + seq_puts(s, "\n Domain Devices, Timings in ns\n"); + seq_puts(s, + " Stop/Start Save/Restore, Effective\n"); + seq_puts(s, + "---------------------------------------------------- ---\n"); + + list_for_each_entry(genpd, &gpd_list, gpd_list_node) { + struct pm_domain_data *pm_data; + + seq_printf(s, "%-30s", genpd->name); + + list_for_each_entry(pm_data, &genpd->dev_list, list_node) { + struct gpd_timing_data *td = &to_gpd_data(pm_data)->td; + + if (!pm_data->dev->of_node) + continue; + + seq_printf(s, + "\n %-20s %-6lld/%-6lld %-6lld/%-6lld,%lld %s%s", + pm_data->dev->of_node->full_name, + td->stop_latency_ns, td->start_latency_ns, + td->save_state_latency_ns, + td->restore_state_latency_ns, + td->effective_constraint_ns, + td->cached_stop_ok ? "(cached stop) " : "", + td->constraint_changed ? "(changed)" : ""); + } + seq_puts(s, "\n"); + } + return 0; +} + +static int pm_genpd_timing_open(struct inode *inode, struct file *file) +{ + return single_open(file, pm_genpd_timing_show, NULL); +} + +static const struct file_operations pm_genpd_timing_fops = { + .open = pm_genpd_timing_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, +}; + static int __init pm_genpd_debug_init(void) { - struct dentry *d; + struct dentry *d = NULL; pm_genpd_debugfs_dir = debugfs_create_dir("pm_genpd", NULL); - if (!pm_genpd_debugfs_dir) - return -ENOMEM; + if (pm_genpd_debugfs_dir) + d = debugfs_create_file("summary", S_IRUGO, + pm_genpd_debugfs_dir, NULL, + &pm_genpd_summary_fops); + if (d) + d = debugfs_create_file("states", S_IRUGO, + pm_genpd_debugfs_dir, NULL, + &pm_genpd_states_fops); + if (d) + d = debugfs_create_file("timings", S_IRUGO, + pm_genpd_debugfs_dir, NULL, + &pm_genpd_timing_fops); + if (d) + return 0; - d = debugfs_create_file("pm_genpd_summary", S_IRUGO, - pm_genpd_debugfs_dir, NULL, &pm_genpd_summary_fops); - if (!d) - return -ENOMEM; + debugfs_remove_recursive(pm_genpd_debugfs_dir /*can be null*/); - return 0; + return -ENOMEM; } late_initcall(pm_genpd_debug_init);
From: Marc Titinger <mtitinger@baylibre.com> This purpose of these debug seq-files is to help investigate generic power domain state transitions, based on device constraints. requires the "multiple states" patches from Axel Haslam. also rename 'summary' from 'pm_genpd_summary' sample output for 'states'