diff mbox series

[10/12] net: smc911x: Clean up the status handling in smc911x_recv()

Message ID 20200315165843.81753-11-marek.vasut+renesas@gmail.com
State Superseded
Headers show
Series net: smc911x: Convert to DM | expand

Commit Message

Marek Vasut March 15, 2020, 4:58 p.m. UTC 
Invest the status handling logic in smc911x_recv(), to make the
function easier to read, no functional change.

Signed-off-by: Marek Vasut <marek.vasut+renesas at gmail.com>
Cc: Joe Hershberger <joe.hershberger at ni.com>
Cc: Masahiro Yamada <yamada.masahiro at socionext.com>
---
 drivers/net/smc911x.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

Comments

Masahiro Yamada March 17, 2020, 6:30 a.m. UTC | #1 
On Mon, Mar 16, 2020 at 2:01 AM Marek Vasut <marek.vasut at gmail.com> wrote:
>
> Invest the status handling logic in smc911x_recv(), to make the
> function easier to read, no functional change.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas at gmail.com>
> Cc: Joe Hershberger <joe.hershberger at ni.com>
> Cc: Masahiro Yamada <yamada.masahiro at socionext.com>
> ---
>  drivers/net/smc911x.c | 31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
> index fc874e8d2a..a6da586448 100644
> --- a/drivers/net/smc911x.c
> +++ b/drivers/net/smc911x.c
> @@ -352,23 +352,26 @@ static int smc911x_recv(struct eth_device *dev)
>         u32 pktlen, tmplen;
>         u32 status;
>
> -       if ((smc911x_reg_read(priv, RX_FIFO_INF) & RX_FIFO_INF_RXSUSED) >> 16) {
> -               status = smc911x_reg_read(priv, RX_STATUS_FIFO);
> -               pktlen = (status & RX_STS_PKT_LEN) >> 16;
> +       status = smc911x_reg_read(priv, RX_FIFO_INF);
> +       status = (status & RX_FIFO_INF_RXSUSED) >> 16;
> +       if (!status)
> +               return 0;


If you are only interested in whether the
RX_FIFO_INF_RXSUSED field is zero or not,
">> 16" seems unneeded.


    status = smc911x_reg_read(priv, RX_FIFO_INF);
    if (!(status & RX_FIFO_INF_RXSUSED))
            return 0;

is simpler.



> -               smc911x_reg_write(priv, RX_CFG, 0);
> +       status = smc911x_reg_read(priv, RX_STATUS_FIFO);
> +       pktlen = (status & RX_STS_PKT_LEN) >> 16;
>
> -               tmplen = (pktlen + 3) / 4;
> -               while (tmplen--)
> -                       *data++ = smc911x_reg_read(priv, RX_DATA_FIFO);
> +       smc911x_reg_write(priv, RX_CFG, 0);
>
> -               if (status & RX_STS_ES)
> -                       printf(DRIVERNAME
> -                               ": dropped bad packet. Status: 0x%08x\n",
> -                               status);
> -               else
> -                       net_process_received_packet(net_rx_packets[0], pktlen);
> -       }
> +       tmplen = (pktlen + 3) / 4;
> +       while (tmplen--)
> +               *data++ = smc911x_reg_read(priv, RX_DATA_FIFO);
> +
> +       if (status & RX_STS_ES)
> +               printf(DRIVERNAME
> +                       ": dropped bad packet. Status: 0x%08x\n",
> +                       status);
> +       else
> +               net_process_received_packet(net_rx_packets[0], pktlen);
>
>         return 0;
>  }
> --
> 2.25.0
>
Joe Hershberger March 17, 2020, 5:07 p.m. UTC | #2 
On Sun, Mar 15, 2020 at 12:01 PM Marek Vasut <marek.vasut at gmail.com> wrote:
>
> Invest the status handling logic in smc911x_recv(), to make the
> function easier to read, no functional change.

Invest -> Invert

> Signed-off-by: Marek Vasut <marek.vasut+renesas at gmail.com>
> Cc: Joe Hershberger <joe.hershberger at ni.com>
> Cc: Masahiro Yamada <yamada.masahiro at socionext.com>
> ---
>  drivers/net/smc911x.c | 31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
> index fc874e8d2a..a6da586448 100644
> --- a/drivers/net/smc911x.c
> +++ b/drivers/net/smc911x.c
> @@ -352,23 +352,26 @@ static int smc911x_recv(struct eth_device *dev)
>         u32 pktlen, tmplen;
>         u32 status;
>
> -       if ((smc911x_reg_read(priv, RX_FIFO_INF) & RX_FIFO_INF_RXSUSED) >> 16) {
> -               status = smc911x_reg_read(priv, RX_STATUS_FIFO);
> -               pktlen = (status & RX_STS_PKT_LEN) >> 16;
> +       status = smc911x_reg_read(priv, RX_FIFO_INF);
> +       status = (status & RX_FIFO_INF_RXSUSED) >> 16;
> +       if (!status)
> +               return 0;
>
> -               smc911x_reg_write(priv, RX_CFG, 0);
> +       status = smc911x_reg_read(priv, RX_STATUS_FIFO);
> +       pktlen = (status & RX_STS_PKT_LEN) >> 16;
>
> -               tmplen = (pktlen + 3) / 4;
> -               while (tmplen--)
> -                       *data++ = smc911x_reg_read(priv, RX_DATA_FIFO);
> +       smc911x_reg_write(priv, RX_CFG, 0);
>
> -               if (status & RX_STS_ES)
> -                       printf(DRIVERNAME
> -                               ": dropped bad packet. Status: 0x%08x\n",
> -                               status);
> -               else
> -                       net_process_received_packet(net_rx_packets[0], pktlen);
> -       }
> +       tmplen = (pktlen + 3) / 4;
> +       while (tmplen--)
> +               *data++ = smc911x_reg_read(priv, RX_DATA_FIFO);
> +
> +       if (status & RX_STS_ES)
> +               printf(DRIVERNAME
> +                       ": dropped bad packet. Status: 0x%08x\n",
> +                       status);
> +       else
> +               net_process_received_packet(net_rx_packets[0], pktlen);
>
>         return 0;
>  }
> --
> 2.25.0
>
diff mbox series

Patch

diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
index fc874e8d2a..a6da586448 100644
--- a/drivers/net/smc911x.c
+++ b/drivers/net/smc911x.c
@@ -352,23 +352,26 @@  static int smc911x_recv(struct eth_device *dev)
 	u32 pktlen, tmplen;
 	u32 status;
 
-	if ((smc911x_reg_read(priv, RX_FIFO_INF) & RX_FIFO_INF_RXSUSED) >> 16) {
-		status = smc911x_reg_read(priv, RX_STATUS_FIFO);
-		pktlen = (status & RX_STS_PKT_LEN) >> 16;
+	status = smc911x_reg_read(priv, RX_FIFO_INF);
+	status = (status & RX_FIFO_INF_RXSUSED) >> 16;
+	if (!status)
+		return 0;
 
-		smc911x_reg_write(priv, RX_CFG, 0);
+	status = smc911x_reg_read(priv, RX_STATUS_FIFO);
+	pktlen = (status & RX_STS_PKT_LEN) >> 16;
 
-		tmplen = (pktlen + 3) / 4;
-		while (tmplen--)
-			*data++ = smc911x_reg_read(priv, RX_DATA_FIFO);
+	smc911x_reg_write(priv, RX_CFG, 0);
 
-		if (status & RX_STS_ES)
-			printf(DRIVERNAME
-				": dropped bad packet. Status: 0x%08x\n",
-				status);
-		else
-			net_process_received_packet(net_rx_packets[0], pktlen);
-	}
+	tmplen = (pktlen + 3) / 4;
+	while (tmplen--)
+		*data++ = smc911x_reg_read(priv, RX_DATA_FIFO);
+
+	if (status & RX_STS_ES)
+		printf(DRIVERNAME
+			": dropped bad packet. Status: 0x%08x\n",
+			status);
+	else
+		net_process_received_packet(net_rx_packets[0], pktlen);
 
 	return 0;
 }