[PATCH linux dev-4.10 v3 06/31] drivers: fsi: sbefifo: remove redundant function

[Eddie James]

On 10/05/2017 04:48 PM, Andrew Jeffery wrote:
> On Thu, 2017-10-05 at 14:23 -0500, Eddie James wrote:
>> From: "Edward A. James" <eajames at us.ibm.com>
>>   
>> sbefifo_client_next_xfr function just does what list_first_entry_or_null
>> does.
> Any reason you didn't implement sbefifo_client_next_xfr() in terms of
> list_first_entry_or_null() rather than open-code it?

I prefer simplicity of using the kernel helper. With a custom function, 
it's not clear if it's doing some additional work unless you go look at 
it. With the list function, it's clear it's just returning the top entry.

Thanks for the review!
Eddie

>
>>   
>> Signed-off-by: Edward A. James <eajames at us.ibm.com>
>> ---
>>   drivers/fsi/fsi-sbefifo.c | 28 +++++++++++++++-------------
>>   1 file changed, 15 insertions(+), 13 deletions(-)
>>   
>> diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c
>> index dbac0c3..173cd03 100644
>> --- a/drivers/fsi/fsi-sbefifo.c
>> +++ b/drivers/fsi/fsi-sbefifo.c
>> @@ -279,20 +279,12 @@ static struct sbefifo_xfr *sbefifo_enq_xfr(struct sbefifo_client *client)
>>   	return xfr;
>>   }
>>   
>> -static struct sbefifo_xfr *sbefifo_client_next_xfr(
>> -		struct sbefifo_client *client)
>> -{
>> -	if (list_empty(&client->xfrs))
>> -		return NULL;
>> -
>> -	return container_of(client->xfrs.next, struct sbefifo_xfr, client);
>> -}
>> -
>>   static bool sbefifo_xfr_rsp_pending(struct sbefifo_client *client)
>>   {
>> -	struct sbefifo_xfr *xfr;
>> +	struct sbefifo_xfr *xfr = list_first_entry_or_null(&client->xfrs,
>> +							   struct sbefifo_xfr,
>> +							   client);
>>   
>> -	xfr = sbefifo_client_next_xfr(client);
>>   	if (xfr && test_bit(SBEFIFO_XFR_RESP_PENDING, &xfr->flags))
>>   		return true;
>>   
>> @@ -595,7 +587,15 @@ static ssize_t sbefifo_read_common(struct sbefifo_client *client,
>>   	}
>>   
>>   	if (sbefifo_buf_readnb(&client->rbuf, n)) {
>> -		xfr = sbefifo_client_next_xfr(client);
>> +		xfr = list_first_entry_or_null(&client->xfrs,
>> +					       struct sbefifo_xfr, client);
>> +		if (!xfr) {
>> +			/* should be impossible to not have an xfr here */
>> +			WARN_ONCE(1, "no xfr in queue");
>> +			sbefifo_put_client(client);
>> +			return -EPROTO;
>> +		}
>> +
>>   		if (!test_bit(SBEFIFO_XFR_COMPLETE, &xfr->flags)) {
>>   			/*
>>   			 * Fill the read buffer back up.
>> @@ -632,7 +632,9 @@ static bool sbefifo_write_ready(struct sbefifo *sbefifo,
>>   				struct sbefifo_xfr *xfr,
>>   				struct sbefifo_client *client, size_t *n)
>>   {
>> -	struct sbefifo_xfr *next = sbefifo_client_next_xfr(client);
>> +	struct sbefifo_xfr *next = list_first_entry_or_null(&client->xfrs,
>> +							    struct sbefifo_xfr,
>> +							    client);
>>   
>>   	*n = sbefifo_buf_nbwriteable(&client->wbuf);
>>   	return READ_ONCE(sbefifo->rc) || (next == xfr && *n);