Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1561102

Re: [PATCH] net: ethernet: stmmac: add ARP management

From Andy Shevchenko <andy.shevchenko@gmail.com>
Newsgroups linux.kernel
Subject Re: [PATCH] net: ethernet: stmmac: add ARP management
Date 2017-01-18 00:20 +0100
Message-ID <t0LcL-4RA-33@gated-at.bofh.it> (permalink)
References <t0Fh0-12w-13@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Tue, Jan 17, 2017 at 6:56 PM, Christophe Roullier
<christophe.roullier@st.com> wrote:

> +static int dwmac4_arp_enable(struct mac_device_info *hw)
> +{
> +       void __iomem *ioaddr = hw->pcsr;

__iomem *config = hw->pcsr + GMAC_CONFIG;

> +       u32 value = readl(ioaddr + GMAC_CONFIG);
> +
> +       value |= GMAC_CONFIG_ARPEN;
> +
> +       writel(value, ioaddr + GMAC_CONFIG);

u32 value;

value = readl();
writel(value | ...);

?

> +
> +       value = readl(ioaddr + GMAC_CONFIG);
> +
> +       return !!(value & GMAC_CONFIG_ARPEN);
> +}

> +/* Set ARP Address */
> +static void dwmac4_set_arp_addr(void __iomem *ioaddr, bool set, u32 addr)
> +{

__iomem *arp_addr = ioaddr + GMAC_ARP_ADDR;

> +       u32 value;
> +
> +       value = readl(ioaddr + GMAC_ARP_ADDR);

Care to explain why do you need dummy readl() here?

> +
> +       if (set) {
> +               /* set arp address */
> +               value = addr;
> +       } else {
> +               /* unset arp address */
> +               value = 0;
> +       }

value = set ? addr : 0;


> +
> +       writel(value, ioaddr + GMAC_ARP_ADDR);
> +       value = readl(ioaddr + GMAC_ARP_ADDR);
> +}


> +       if ((priv->plat->arp_en) && (priv->dma_cap.arpoffsel)) {
> +               ret = priv->hw->mac->arp_en(priv->hw);
> +               if (!ret) {

Hmm... Most would expect

if (ret) {
 doing something
} else {
 doing something else
}

> +                       pr_warn(" ARP feature disabled\n");
> +               } else {

> +                       pr_info(" ARP feature enabled\n");

Wouldn't be too noisy?

pr_* -> dev_*

> +                       /* Copy MAC addr into MAC_ARP_ADDRESS register*/
> +                       priv->hw->dma->set_arp_addr(priv->ioaddr, 1,
> +                                                   priv->dev->dev_addr);
> +               }
> +       }

-- 
With Best Regards,
Andy Shevchenko

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] net: ethernet: stmmac: add ARP management Christophe Roullier <christophe.roullier@st.com> - 2017-01-17 18:00 +0100
  Re: [PATCH] net: ethernet: stmmac: add ARP management David Miller <davem@davemloft.net> - 2017-01-17 18:10 +0100
  Re: [PATCH] net: ethernet: stmmac: add ARP management Andrew Lunn <andrew@lunn.ch> - 2017-01-17 18:30 +0100
    Re: [PATCH] net: ethernet: stmmac: add ARP management Florian Fainelli <f.fainelli@gmail.com> - 2017-01-17 18:50 +0100
  Re: [PATCH] net: ethernet: stmmac: add ARP management Florian Fainelli <f.fainelli@gmail.com> - 2017-01-17 18:30 +0100
  Re: [PATCH] net: ethernet: stmmac: add ARP management Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-01-18 00:20 +0100
  Re: [PATCH] net: ethernet: stmmac: add ARP management kbuild test robot <lkp@intel.com> - 2017-01-18 04:00 +0100
  Re: [PATCH] net: ethernet: stmmac: add ARP management Rayagond Kokatanur <rayagond@vayavyalabs.com> - 2017-01-18 11:20 +0100

csiph-web