From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9856110EF94 for ; Wed, 20 Apr 2022 05:05:40 +0000 (UTC) Date: Tue, 19 Apr 2022 22:05:39 -0700 Message-ID: <87zgkgwdi4.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Kamil Konieczny , igt-dev@lists.freedesktop.org, Ashutosh Dixit , Andi Shyti , Tvrtko Ursulin , Sujaritha Sundaresan , Umesh Nerlige Ramappa In-Reply-To: References: MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Subject: Re: [igt-dev] [PATCH i-g-t 1/2] lib/igt_sysfs: Add helpers to iterate over gts List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: On Thu, 14 Apr 2022 08:18:57 -0700, Kamil Konieczny wrote: > > Hi Ashutosh, Hi Kamil, > may you change last word in subject from "gts" into "GTs" ? Done in v2. > > +static const char *i915_attr_name[SYSFS_NUM_TYPES][SYSFS_NUM_ATTR] = { > > + { > > + "gt_act_freq_mhz", > > + "gt_cur_freq_mhz", > > + "gt_min_freq_mhz", > > + "gt_max_freq_mhz", > > + "gt_RP0_freq_mhz", > > + "gt_RP1_freq_mhz", > > + "gt_RPn_freq_mhz", > ------------------- ^ ------ ^ > I suppose it is too late now, but mixing capitals with small > letters is bad, it should either be all small or proper names, > so either rpn and mhz or RPn and MHz (MHz for Mega Hertz). So these are names of sysfs files exposed by the kernel, so these cannot be changed even in the kernel, they are already uapi. > These two functions below have no descriptions in c file. > > > +const char *igt_sysfs_dir_id_to_name(int dir, enum i915_attr_id id); > > +const char *igt_sysfs_path_id_to_name(const char *path, enum i915_attr_id id); I have added these descriptions in v2. Also made some minor changes to these two functions since what we had was not entirely correct. Please review. Also I have separated out the RPS related changes (including the functions above) into a separate patch since the original patch commit message only referred to the per-gt sysfs parsing. So now there are two patches: - lib/igt_sysfs: Add helpers to iterate over GTs - lib/igt_sysfs: Add RPS sysfs helpers Thanks for reviewing! -- Ashutosh