Linux ACPI
 help / color / mirror / Atom feed
From: Alison Schofield <alison.schofield@intel.com>
To: Shiyang Ruan <ruansy.fnst@fujitsu.com>
Cc: dan.j.williams@intel.com, vishal.l.verma@intel.com,
	nvdimm@lists.linux.dev, linux-acpi@vger.kernel.org,
	lenb@kernel.org
Subject: Re: [PATCH] nfit: remove redundant list_for_each_entry
Date: Tue, 25 Jul 2023 20:05:49 -0700	[thread overview]
Message-ID: <ZMCNja5S3tnNBm79@aschofie-mobl2> (raw)
In-Reply-To: <32cb262a-8ae6-60ba-2032-f02973f44a1e@fujitsu.com>

On Tue, Jul 25, 2023 at 01:33:18PM +0800, Shiyang Ruan wrote:
> 
> 
> 在 2023/7/25 6:47, Alison Schofield 写道:
> > On Wed, Jul 19, 2023 at 04:05:26PM +0800, Shiyang Ruan wrote:
> > > The first for_each only do acpi_nfit_init_ars() for NFIT_SPA_VOLATILE
> > > and NFIT_SPA_PM, which can be moved to next one.
> > 
> > Can the result of nfit_spa_type(nfit_spa->spa) change as a result of
> > the first switch statement? That would be a reason why they are separate.
> 
> nfit_spa_type() just gets the type of *spa by querying a type-uuid table.
> Also, according to the code shown below, we can find that it doesn't change
> anything.
> 
> int nfit_spa_type(struct acpi_nfit_system_address *spa)
> {
> 	guid_t guid;
> 	int i;
> 
> 	import_guid(&guid, spa->range_guid);
> 	for (i = 0; i < NFIT_UUID_MAX; i++)
> 		if (guid_equal(to_nfit_uuid(i), &guid))
> 			return i;
> 	return -1;
> }
>

Hi Ruan,

I see that. I was questioning if the type change as a *result* of the
first switch statement, which does that acpi_nfi_init_ars().

I don't think it does. I'm only asking if you proved the correctness
of the change because I'm guessing this change is tested by inspection
only. Maybe not.

Thanks,
Alison

> --
> Thanks,
> Ruan.
> 
> > 
> > Alison
> > 
> > > 
> > > Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com>
> > > ---
> > >   drivers/acpi/nfit/core.c | 8 --------
> > >   1 file changed, 8 deletions(-)
> > > 
> > > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> > > index 07204d482968..4090a0a0505c 100644
> > > --- a/drivers/acpi/nfit/core.c
> > > +++ b/drivers/acpi/nfit/core.c
> > > @@ -2971,14 +2971,6 @@ static int acpi_nfit_register_regions(struct acpi_nfit_desc *acpi_desc)
> > >   		case NFIT_SPA_VOLATILE:
> > >   		case NFIT_SPA_PM:
> > >   			acpi_nfit_init_ars(acpi_desc, nfit_spa);
> > > -			break;
> > > -		}
> > > -	}
> > > -
> > > -	list_for_each_entry(nfit_spa, &acpi_desc->spas, list) {
> > > -		switch (nfit_spa_type(nfit_spa->spa)) {
> > > -		case NFIT_SPA_VOLATILE:
> > > -		case NFIT_SPA_PM:
> > >   			/* register regions and kick off initial ARS run */
> > >   			rc = ars_register(acpi_desc, nfit_spa);
> > >   			if (rc)
> > > -- 
> > > 2.41.0
> > > 

      reply	other threads:[~2023-07-26  3:06 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-19  8:05 [PATCH] nfit: remove redundant list_for_each_entry Shiyang Ruan
2023-07-24 22:47 ` Alison Schofield
2023-07-25  5:33   ` Shiyang Ruan
2023-07-26  3:05     ` Alison Schofield [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZMCNja5S3tnNBm79@aschofie-mobl2 \
    --to=alison.schofield@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=nvdimm@lists.linux.dev \
    --cc=ruansy.fnst@fujitsu.com \
    --cc=vishal.l.verma@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox