On Wed, Jan 24, 2018 at 4:20 PM, Marek Vasut <marek.va...@gmail.com> wrote: > Split sh_eth_recv into two functions, one which checks whether > a packet was received and one which handles the received packet. > This is done in preparation for DM support, which handles these > two parts separately. > > Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com> > Cc: Nobuhiro Iwamatsu <iwama...@nigauri.org> > Cc: Joe Hershberger <joe.hershber...@ni.com> > --- > drivers/net/sh_eth.c | 73 > +++++++++++++++++++++++++++++++++------------------- > 1 file changed, 46 insertions(+), 27 deletions(-) > > diff --git a/drivers/net/sh_eth.c b/drivers/net/sh_eth.c > index 7b11a5a0d3..99eab4c688 100644 > --- a/drivers/net/sh_eth.c > +++ b/drivers/net/sh_eth.c > @@ -118,39 +118,58 @@ static int sh_eth_send_legacy(struct eth_device *dev, > void *packet, int len) > return sh_eth_send_common(eth, packet, len); > } > > -static int sh_eth_recv_common(struct sh_eth_dev *eth) > +static int sh_eth_recv_start(struct sh_eth_dev *eth) > { > int port = eth->port, len = 0;
This "port" local variable seems pretty useless. > struct sh_eth_info *port_info = ð->port_info[port]; > - uchar *packet; > > /* Check if the rx descriptor is ready */ > invalidate_cache(port_info->rx_desc_cur, sizeof(struct rx_desc_s)); > - if (!(port_info->rx_desc_cur->rd0 & RD_RACT)) { > - /* Check for errors */ > - if (!(port_info->rx_desc_cur->rd0 & RD_RFE)) { > - len = port_info->rx_desc_cur->rd1 & 0xffff; > - packet = (uchar *) > - ADDR_TO_P2(port_info->rx_desc_cur->rd2); > - invalidate_cache(packet, len); > - net_process_received_packet(packet, len); > - } > - > - /* Make current descriptor available again */ > - if (port_info->rx_desc_cur->rd0 & RD_RDLE) > - port_info->rx_desc_cur->rd0 = RD_RACT | RD_RDLE; > - else > - port_info->rx_desc_cur->rd0 = RD_RACT; > - > - flush_cache_wback(port_info->rx_desc_cur, > - sizeof(struct rx_desc_s)); > - > - /* Point to the next descriptor */ > - port_info->rx_desc_cur++; > - if (port_info->rx_desc_cur >= > - port_info->rx_desc_base + NUM_RX_DESC) > - port_info->rx_desc_cur = port_info->rx_desc_base; > - } > + if (port_info->rx_desc_cur->rd0 & RD_RACT) > + return -EINVAL; > + > + /* Check for errors */ > + if (port_info->rx_desc_cur->rd0 & RD_RFE) > + return -EINVAL; > + > + len = port_info->rx_desc_cur->rd1 & 0xffff; Just return here directly and delete the "len" variable. > + > + return len; > +} > + > +static void sh_eth_recv_finish(struct sh_eth_dev *eth) > +{ > + struct sh_eth_info *port_info = ð->port_info[eth->port]; > + > + /* Make current descriptor available again */ > + if (port_info->rx_desc_cur->rd0 & RD_RDLE) > + port_info->rx_desc_cur->rd0 = RD_RACT | RD_RDLE; > + else > + port_info->rx_desc_cur->rd0 = RD_RACT; > + > + flush_cache_wback(port_info->rx_desc_cur, > + sizeof(struct rx_desc_s)); > + > + /* Point to the next descriptor */ > + port_info->rx_desc_cur++; > + if (port_info->rx_desc_cur >= > + port_info->rx_desc_base + NUM_RX_DESC) > + port_info->rx_desc_cur = port_info->rx_desc_base; > +} > + > +static int sh_eth_recv_common(struct sh_eth_dev *eth) > +{ > + int port = eth->port, len = 0; I would also remove "port" here. > + struct sh_eth_info *port_info = ð->port_info[port]; > + uchar *packet = (uchar *)ADDR_TO_P2(port_info->rx_desc_cur->rd2); > + > + len = sh_eth_recv_start(eth); > + if (len > 0) { > + invalidate_cache(packet, len); > + net_process_received_packet(packet, len); > + sh_eth_recv_finish(eth); > + } else I thought checkpatch would want you to have matching braces here. I think it looks cleaner at least. Probably a moot point though since... > + len = 0; Are you expecting "len" to be negative? If not, it will already be 0. From the function above, it looks like it will always be >= 0. > > /* Restart the receiver if disabled */ > if (!(sh_eth_read(port_info, EDRRR) & EDRRR_R)) > -- > 2.15.1 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot