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

[Dandamudi, Priyanka]

> -----Original Message-----
> From: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> Sent: 26 April 2022 02:03 PM
> To: igt-dev at lists.freedesktop.org
> Cc: Marcin Bernatowicz <marcin.bernatowicz at linux.intel.com>; Dandamudi,
> Priyanka <priyanka.dandamudi at intel.com>
> Subject: Re: [igt-dev] [PATCH i-g-t 3/6] lib/sysfs: helper for number of gts
> 
> 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);
[Dandamudi, Priyanka] 
Imho if while loop passes then it will return minimum of 1 gt 
else it will not go further it will either return null or the path.
In that case no need to put assert condition after while loop.
> 
> > +
> > +	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
>