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

[Andrew Jeffery]

On Thu, 2017-10-05 at 21:03 -0500, Eddie James wrote:
> 
> 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.

Sure. I'm not terribly fussed either way, was just interested.

Reviewed-by: Andrew Jeffery <andrew at aj.id.au>

> 
> 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);
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20171006/ce688280/attachment.sig>