[igt-dev] [PATCH i-g-t 6/7] igt/params: use igt_params_set_save for igt_set_module_param*

[Arkadiusz Hiler]

On Tue, Apr 28, 2020 at 11:22:54PM +0300, Juha-Pekka Heikkila wrote:
> Unify parameter access. Here is also decided 'default' device
> if -1 is specified as device. Derault being that device
> which is first to match requirements, starting search
> from zero upwards.
> 
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> ---
>  lib/igt_params.c | 134 +++++++++++++++++++++++++++++++++++------------
>  1 file changed, 100 insertions(+), 34 deletions(-)
> 
> diff --git a/lib/igt_params.c b/lib/igt_params.c
> index d9cf986c..dc35f233 100644
> --- a/lib/igt_params.c
> +++ b/lib/igt_params.c
> @@ -27,12 +27,14 @@
>  #include <stdbool.h>
>  #include <stdio.h>
>  #include <sys/ioctl.h>
> +#include <sys/stat.h>
>  
>  #include <i915_drm.h>
>  
>  #include "igt_core.h"
>  #include "igt_params.h"
>  #include "igt_sysfs.h"
> +#include "igt_debugfs.h"
>  
>  struct module_param_data {
>  	char *path;
> @@ -107,15 +109,59 @@ static void igt_params_save(int dir, const char *path, const char *name)
>  	module_params = data;
>  }
>  
> -static int __igt_params_open(int device, char **outpath)
> +static int __igt_params_open(int device, char **outpath, const char *param)

Please add a function documentation block on top of this. Deciphering
the role of the new "param" argument in the context of this function
from the code alone is really hard. You should also mention how it being
set vs it being NULL affect the outcome.

I would appreciate a bit more contet on this change in the commit
message, especially about the sysfs -> debugfs switch.

If I got your intention right, the code LGTM.

-- 
Cheers,
Arek