[PATCH v2 09/17] drivers/firmware/sdei: Remove while loop in sdei_event_unregister()

[Gavin Shan]

Hi Jonathan,

On 7/24/20 1:51 AM, Jonathan Cameron wrote:
> On Wed, 22 Jul 2020 19:57:32 +1000
> Gavin Shan <gshan at redhat.com> wrote:
> 
>> This removes the unnecessary while loop in sdei_event_unregister().
>> This shouldn't cause any functional changes.
>>
>> Signed-off-by: Gavin Shan <gshan at redhat.com>
>> ---
>> v2: Derived from "drivers/firmware/sdei: Drop unnecessary while loop"
>> ---
>>   drivers/firmware/arm_sdei.c | 30 ++++++++++++++----------------
>>   1 file changed, 14 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
>> index ab1088fbdd37..d03371161aaf 100644
>> --- a/drivers/firmware/arm_sdei.c
>> +++ b/drivers/firmware/arm_sdei.c
>> @@ -491,26 +491,24 @@ int sdei_event_unregister(u32 event_num)
>>   
>>   	mutex_lock(&sdei_events_lock);
>>   	event = sdei_event_find(event_num);
>> -	do {
>> -		if (!event) {
>> -			pr_warn("Event %u not registered\n", event_num);
>> -			err = -ENOENT;
>> -			break;
>> -		}
>> +	if (!event) {
>> +		pr_warn("Event %u not registered\n", event_num);
>> +		err = -ENOENT;
>> +		goto out;
>> +	}
>>   
>> -		spin_lock(&sdei_list_lock);
>> -		event->reregister = false;
>> -		event->reenable = false;
>> -		spin_unlock(&sdei_list_lock);
>> +	spin_lock(&sdei_list_lock);
>> +	event->reregister = false;
>> +	event->reenable = false;
>> +	spin_unlock(&sdei_list_lock);
>>   
>> -		err = _sdei_event_unregister(event);
>> -		if (err)
>> -			break;
>> +	err = _sdei_event_unregister(event);
>> +	if (err)
>> +		goto out;
>>   
>> -		sdei_event_destroy(event);
>> -	} while (0);
>> +	sdei_event_destroy(event);
>> +out:
> As in previous I'd give the label a name to indicate there is still work
> to be done.
> 

Sure, I will rename this to "unlock" in v3. However, I would hold
v3 for a while until v2 is fully reviewed. Please take a look on
the left patches if you have more time, thanks for your time on
this series.

>>   	mutex_unlock(&sdei_events_lock);
>> -
> That whitespace is arguably slightly useful from a readability point of view.
> I would leave it here.
> 

Ok. I will keep this in v3 then.

>>   	return err;
>>   }
>>   

Thanks,
Gavin