diff mbox series

nfit: remove redundant list_for_each_entry

Message ID 20230719080526.2436951-1-ruansy.fnst@fujitsu.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series nfit: remove redundant list_for_each_entry | expand

Commit Message

Shiyang Ruan July 19, 2023, 8:05 a.m. UTC
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(-)

Comments

Alison Schofield July 24, 2023, 10:47 p.m. UTC | #1
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
>
Shiyang Ruan July 25, 2023, 5:33 a.m. UTC | #2
在 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
>>
Alison Schofield July 26, 2023, 3:05 a.m. UTC | #3
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
> > >
diff mbox series

Patch

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)