From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <igt-dev-bounces@lists.freedesktop.org>
Received: from mga03.intel.com (mga03.intel.com [134.134.136.65])
 by gabe.freedesktop.org (Postfix) with ESMTPS id 2BD5210E34F
 for <igt-dev@lists.freedesktop.org>; Mon, 25 Apr 2022 23:15:40 +0000 (UTC)
Date: Mon, 25 Apr 2022 16:15:39 -0700
Message-ID: <874k2g7o10.wl-ashutosh.dixit@intel.com>
From: "Dixit, Ashutosh" <ashutosh.dixit@intel.com>
To: Kamil Konieczny <kamil.konieczny@linux.intel.com>,
 igt-dev@lists.freedesktop.org,
 Ashutosh Dixit <ashutosh.dixit@intel.com>, Andi Shyti <andi.shyti@intel.com>,
 Tvrtko Ursulin <tvrtko.ursulin@intel.com>,
 Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>,
 Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
In-Reply-To: <YmF9EXfaWmG4vPin@kamilkon-DESK1>
References: <85c51c99da76e3fe286ed28e80fa964e148f6020.1650435058.git.ashutosh.dixit@intel.com>	<d38e666fe8159eed5defb4d30045fa1154410e4f.1650435058.git.ashutosh.dixit@intel.com>	<YmF9EXfaWmG4vPin@kamilkon-DESK1>
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 2/3] lib/igt_sysfs: Add RPS sysfs helpers
List-Unsubscribe: <https://lists.freedesktop.org/mailman/options/igt-dev>,
 <mailto:igt-dev-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <https://lists.freedesktop.org/archives/igt-dev>
List-Post: <mailto:igt-dev@lists.freedesktop.org>
List-Help: <mailto:igt-dev-request@lists.freedesktop.org?subject=help>
List-Subscribe: <https://lists.freedesktop.org/mailman/listinfo/igt-dev>,
 <mailto:igt-dev-request@lists.freedesktop.org?subject=subscribe>
Errors-To: igt-dev-bounces@lists.freedesktop.org
Sender: "igt-dev" <igt-dev-bounces@lists.freedesktop.org>
List-ID: <igt-dev@lists.freedesktop.org>

On Thu, 21 Apr 2022 08:49:37 -0700, Kamil Konieczny wrote:
>
> Hi Ashutosh,

Hi Kamil,

> > +/**
> > + * igt_sysfs_dir_id_to_name:
> > + * @dir: sysfs directory fd
> > + * @id: sysfs attribute id
> > + *
> > + * Returns attribute name corresponding to attribute id in either the
> > + * per-gt or legacy per-device sysfs
> > + *
> > + * Returns:
> > + * Attribute name in sysfs
> > + */
> > +const char *igt_sysfs_dir_id_to_name(int dir, enum i915_attr_id id)
> > +{
> > +	igt_assert(id < SYSFS_NUM_ATTR);
>
> What about id < 0 ?

Changed to "igt_assert((uint32_t) id < SYSFS_NUM_ATTR);

> > +
> > +	if (igt_sysfs_has_attr(dir, i915_attr_name[GT][id]))
> > +		return i915_attr_name[GT][id];
> > +	else if (igt_sysfs_has_attr(dir, i915_attr_name[RPS][id]))
> ------- ^
> s/else //

Done.

> > +		return i915_attr_name[RPS][id];
> > +	else
> ------- ^
> drop this else here, checkpatch warn:
> WARNING: else is not generally useful after a break or return
> #102: FILE: lib/igt_sysfs.c:118:
> +               return i915_attr_name[RPS][id];
> +       else
>
> > +		igt_assert_f(0, "Invalid sysfs dir\n");
>
> and align this assert.

Done.

>
> Rest looks good, with that fixed you can add my r-b tag.

Thanks, merged with the above changes.