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

[Dandamudi, Priyanka]

> -----Original Message-----
> From: Dandamudi, Priyanka
> Sent: 26 April 2022 09:04 PM
> To: Kamil Konieczny <kamil.konieczny at linux.intel.com>; igt-
> dev at lists.freedesktop.org
> Cc: Marcin Bernatowicz <marcin.bernatowicz at linux.intel.com>
> Subject: RE: [igt-dev] [PATCH i-g-t 3/6] lib/sysfs: helper for number of gts
> 
> 
> 
> > -----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.
[Dandamudi, Priyanka] 
Kamil you are right it will return 0 gt if while fails in first iteration.
Will modify it accordingly and send it.
> >
> > > +
> > > +	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
> >