[igt-dev] [PATCH i-g-t 3/6] lib/sysfs: helper for number of gts

[Kamil Konieczny]

Hi Priyanka,

On 2022-04-21 at 20:22:08 +0530, priyanka.dandamudi at intel.com wrote:
> From: Marcin Bernatowicz <marcin.bernatowicz at linux.intel.com>
> 
> Added convenience function returning number of gts.
> It is calculated as number of gt sysfs entries,
> if no entries are found it returns 1 as number of gts.
> 
Please change gts in subject and commit message to GTs
s/gts/GTs/

> Signed-off-by: Marcin Bernatowicz <marcin.bernatowicz at linux.intel.com>
> Signed-off-by: Priyanka Dandamudi <priyanka.dandamudi at intel.com>
> ---
>  lib/igt_sysfs.c | 21 +++++++++++++++++++++
>  lib/igt_sysfs.h |  1 +
>  2 files changed, 22 insertions(+)
> 
> diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
> index b167c050..aebc6eb7 100644
> --- a/lib/igt_sysfs.c
> +++ b/lib/igt_sysfs.c
> @@ -175,6 +175,27 @@ int igt_sysfs_gt_open(int device, int gt)
>  	return open(path, O_RDONLY);
>  }
>  
> +/**
> + * igt_sysfs_get_num_gts:
------------------------- ^
imho this function name should be igt_sysfs_get_num_gt

> + * @device: fd of the device
> + *
> + * Reads number of gt sysfs entries.
> + * If no gt sysfs entries are found returns 1

Why not zero for no entries ? This '1' do not match with previous
description one line above. Maybe there should be additional
function

igt_get_num_gt

and that one will return >= 1 if i915 driver (or other GPU
driver) is loaded ?

> + * (see igt_sysfs_gt_path).
> + *
> + * Returns: Number of gts (at least one).

s/gts/GTs/

> + */
> +int igt_sysfs_get_num_gts(int device)

s/gts/gt/

> +{
> +	int num_gts = 0;
> +	char path[96];
> +
> +	while (igt_sysfs_gt_path(device, num_gts, path, sizeof(path)))
> +		++num_gts;

If while fails at start then num_gts will be 0.
imho here should be igt_assert(num_gts > 0);

> +
> +	return num_gts;
> +}
> +
>  /**
>   * igt_sysfs_write:
>   * @dir: directory for the device from igt_sysfs_open()
> diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
> index 33317a96..6c8b92a7 100644
> --- a/lib/igt_sysfs.h
> +++ b/lib/igt_sysfs.h
> @@ -42,6 +42,7 @@ char *igt_sysfs_path(int device, char *path, int pathlen);
>  int igt_sysfs_open(int device);
>  char *igt_sysfs_gt_path(int device, int gt, char *path, int pathlen);
>  int igt_sysfs_gt_open(int device, int gt);
> +int igt_sysfs_get_num_gts(int device);

s/gts/gt/

>  bool igt_sysfs_has_attr(int dir, const char *attr);
>  
>  int igt_sysfs_read(int dir, const char *attr, void *data, int len);
> -- 
> 2.25.1
>
Regards,
Kamil