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
