Re: [PATCH] radeon: Make PM info available to all, not just debug users

[Lauri Kasanen <cand@gmx.com>]

On Sun, 03 Jun 2012 12:54:30 +0200
Christian König <deathsimple@vodafone.de> wrote:

> > This moves the pm_info file from debugfs to next to the other two power files.
> >
> > Requested by several users at Phoronix.
> >
> > PS: Please CC me. Also please be gentle, it's my first step in kernel-land ;)

> Hui? What should this be good for?
> 
> Sysfs files are for setting driver parameters, like the power management 
> method or profile currently in use. One major advantage of sysfs is the 
> strict rules for a permanent and machine usable interface, for example 
> it is mandatory to only specify one parameter per sysfs file.
> 
> Debugfs on the other hand should be used for human readable 
> informations, e.g. the printing the current clocks in a human readable 
> form. Also you don't need a debug build or turn on any other debugging 
> facility to get those information, just take a look under 
> "sys/kernel/debug/dri/*".

I have no such dir, /sys/kernel/debug. The fact you have it means you have CONFIG_DEBUGFS enabled and mounted.

> So the code is actually quite valid as it is.

First, the current location is illogical, and several users have complained about it. This info should be right next to where it is tweaked, ie right next to power_profile and power_method. That is where it's expected to be by users.

Secondly, checking the clocks is absolutely not a debug operation. Therefore requiring a debug option (CONFIG_DEBUGFS) to see this info is plain wrong. This info needs to be available to all users, including those on production kernels without such debug options.

--

So the issue is the location of the info, not the format. I'd be more than happy to split it into six files (default_core_clock, current_core_clock...) that each offer just a kHz number, just like the cpufreq scaling_cur_freq do. Would that be preferable?

- Lauri