Received-SPF: pass (google.com: domain of linux-kernel+bounces-44709-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <5770175.DvuYhMxLoT@kreacher> <2192653.irdbgypaU6@kreacher> <Zbie+bepNv1xob3J@linux.intel.com>
In-Reply-To: <Zbie+bepNv1xob3J@linux.intel.com>
From: "Rafael J. Wysocki" <rafael@kernel.org>
Date: Tue, 30 Jan 2024 14:42:10 +0100
Message-ID: <CAJZ5v0hwXbtYBLx8EJo3N_SY9RU-63syf=6Q-dCaOh7eXkg-0w@mail.gmail.com>
Subject: Re: [PATCH v2 02/10] PM: sleep: stats: Use an array of step failure counters
To: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>, Linux PM <linux-pm@vger.kernel.org>, 
	LKML <linux-kernel@vger.kernel.org>, Ulf Hansson <ulf.hansson@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Jan 30, 2024 at 8:02=E2=80=AFAM Stanislaw Gruszka
<stanislaw.gruszka@linux.intel.com> wrote:
>
> On Mon, Jan 29, 2024 at 05:11:57PM +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Instead of using a set of individual struct suspend_stats fields
> > representing suspend step failure counters, use an array of counters
> > indexed by enum suspend_stat_step for this purpose, which allows
> > dpm_save_failed_step() to increment the appropriate counter
> > automatically, so that its callers don't need to do that directly.
> >
> > It also allows suspend_stats_show() to carry out a loop over the
> > counters array to print their values.
> >
> > Because the counters cannot become negative, use unsigned int for
> > representing them.
> >
> > The only user-observable impact of this change is a different
> > ordering of entries in the suspend_stats debugfs file which is not
> > expected to matter.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >
> > v1 -> v2:
> >    * Use one cell less in suspend_stats.step_failures[] to avoid
> >      introducing an unused array cell (Stanislaw).
> >
> > @Stanislaw: This is different from setting SUSPEND_FREEZE to 0, because
> > that would complicate printing in the sysfs attributes and the debugfs
> > code, so I've not added the R-by.
>
> LGTM.
>
> Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
>
> > +     for (step =3D SUSPEND_FREEZE; step <=3D SUSPEND_NR_STEPS; step++)
> > +             seq_printf(s, "failed_%s: %u\n", suspend_step_names[step]=
,
> > +                        suspend_stats.step_failures[step-1]);
>
> Consider (in separate patch) removing SUSPEND_NONE from suspend_step_name=
s[]
> and use step-1 for it as well.

SUSPEND_NONE is handy for printing an empty string when there are no
suspend-resume errors.