* [PATCH] nfit: remove redundant list_for_each_entry
@ 2023-07-19 8:05 Shiyang Ruan
2023-07-24 22:47 ` Alison Schofield
0 siblings, 1 reply; 4+ messages in thread
From: Shiyang Ruan @ 2023-07-19 8:05 UTC (permalink / raw)
To: dan.j.williams, vishal.l.verma, nvdimm, linux-acpi, lenb; +Cc: ruansy.fnst
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.
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] nfit: remove redundant list_for_each_entry
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
0 siblings, 1 reply; 4+ messages in thread
From: Alison Schofield @ 2023-07-24 22:47 UTC (permalink / raw)
To: Shiyang Ruan; +Cc: dan.j.williams, vishal.l.verma, nvdimm, linux-acpi, lenb
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.
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] nfit: remove redundant list_for_each_entry
2023-07-24 22:47 ` Alison Schofield
@ 2023-07-25 5:33 ` Shiyang Ruan
2023-07-26 3:05 ` Alison Schofield
0 siblings, 1 reply; 4+ messages in thread
From: Shiyang Ruan @ 2023-07-25 5:33 UTC (permalink / raw)
To: Alison Schofield; +Cc: dan.j.williams, vishal.l.verma, nvdimm, linux-acpi, lenb
在 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;
}
--
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
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] nfit: remove redundant list_for_each_entry
2023-07-25 5:33 ` Shiyang Ruan
@ 2023-07-26 3:05 ` Alison Schofield
0 siblings, 0 replies; 4+ messages in thread
From: Alison Schofield @ 2023-07-26 3:05 UTC (permalink / raw)
To: Shiyang Ruan; +Cc: dan.j.williams, vishal.l.verma, nvdimm, linux-acpi, lenb
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
> > >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-07-26 3:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox