[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZHIMF8k+bYGosakh@corigine.com>
Date: Sat, 27 May 2023 15:56:39 +0200
From: Simon Horman <simon.horman@...igine.com>
To: Tristram.Ha@...rochip.com
Cc: "David S. Miller" <davem@...emloft.net>, Andrew Lunn <andrew@...n.ch>,
Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org,
UNGLinuxDriver@...rochip.com
Subject: Re: [PATCH net-next] net: phy: smsc: add WoL support to
LAN8740/LAN8742 PHYs.
On Fri, May 26, 2023 at 06:39:34PM -0700, Tristram.Ha@...rochip.com wrote:
> From: Tristram Ha <Tristram.Ha@...rochip.com>
>
> Microchip LAN8740/LAN8742 PHYs support basic unicast, broadcast, and
> Magic Packet WoL. They have one pattern filter matching up to 128 bytes
> of frame data, which can be used to implement ARP or multicast WoL.
>
> ARP WoL matches ARP request for IPv4 address of the net device using the
> PHY.
>
> Multicast WoL matches IPv6 Neighbor Solicitation which is sent when
> somebody wants to talk to the net device using IPv6. This
> implementation may not be appropriate and can be changed by users later.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@...rochip.com>
...
> +static int lan874x_set_wol(struct phy_device *phydev,
> + struct ethtool_wolinfo *wol)
> +{
> + struct net_device *ndev = phydev->attached_dev;
> + struct smsc_phy_priv *priv = phydev->priv;
> + u16 val, val_wucsr;
> + u8 data[128];
> + u8 datalen;
> + int rc;
> +
> + if (wol->wolopts & WAKE_PHY)
> + return -EOPNOTSUPP;
> +
> + /* lan874x has only one WoL filter pattern */
> + if ((wol->wolopts & (WAKE_ARP | WAKE_MCAST)) ==
> + (WAKE_ARP | WAKE_MCAST)) {
> + phydev_info(phydev,
> + "lan874x WoL supports one of ARP|MCAST at a time\n");
> + return -EOPNOTSUPP;
> + }
> +
> + rc = phy_read_mmd(phydev, MDIO_MMD_PCS, MII_LAN874X_PHY_MMD_WOL_WUCSR);
> + if (rc < 0)
> + return rc;
> +
> + val_wucsr = rc;
> +
> + if (wol->wolopts & WAKE_UCAST)
> + val_wucsr |= MII_LAN874X_PHY_WOL_PFDAEN;
> + else
> + val_wucsr &= ~MII_LAN874X_PHY_WOL_PFDAEN;
> +
> + if (wol->wolopts & WAKE_BCAST)
> + val_wucsr |= MII_LAN874X_PHY_WOL_BCSTEN;
> + else
> + val_wucsr &= ~MII_LAN874X_PHY_WOL_BCSTEN;
> +
> + if (wol->wolopts & WAKE_MAGIC)
> + val_wucsr |= MII_LAN874X_PHY_WOL_MPEN;
> + else
> + val_wucsr &= ~MII_LAN874X_PHY_WOL_MPEN;
> +
> + /* Need to use pattern matching */
> + if (wol->wolopts & (WAKE_ARP | WAKE_MCAST))
> + val_wucsr |= MII_LAN874X_PHY_WOL_WUEN;
> + else
> + val_wucsr &= ~MII_LAN874X_PHY_WOL_WUEN;
> +
> + if (wol->wolopts & WAKE_ARP) {
> + const u8 *ip_addr =
> + ((const u8 *)&((ndev->ip_ptr)->ifa_list)->ifa_address);
Hi Tristram,
Sparse seems unhappy about this:
.../smsc.c:449:27: warning: cast removes address space '__rcu' of expression
> + const u16 mask[3] = { 0xF03F, 0x003F, 0x03C0 };
> + u8 pattern[42] = {
> + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x08, 0x06,
> + 0x00, 0x01, 0x08, 0x00, 0x06, 0x04, 0x00, 0x01,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00 };
> + u8 len = 42;
> +
> + memcpy(&pattern[38], ip_addr, 4);
> + rc = lan874x_chk_wol_pattern(pattern, mask, len,
> + data, &datalen);
> + if (rc)
> + phydev_dbg(phydev, "pattern not valid at %d\n", rc);
> +
> + /* Need to match broadcast destination address. */
> + val = MII_LAN874X_PHY_WOL_FILTER_BCSTEN;
> + rc = lan874x_set_wol_pattern(phydev, val, data, datalen, mask,
> + len);
> + if (rc < 0)
> + return rc;
> + priv->wol_arp = true;
> + }
> +
> + if (wol->wolopts & WAKE_MCAST) {
> + u8 pattern[6] = { 0x33, 0x33, 0xFF, 0x00, 0x00, 0x00 };
> + u16 mask[1] = { 0x0007 };
> + u8 len = 3;
> +
> + /* Try to match IPv6 Neighbor Solicitation. */
> + if (ndev->ip6_ptr) {
> + struct list_head *addr_list =
> + &ndev->ip6_ptr->addr_list;
And this:
.../smsc.c:485:38: warning: incorrect type in initializer (different address spaces)
.../smsc.c:485:38: expected struct list_head *addr_list
.../smsc.c:485:38: got struct list_head [noderef] __rcu *
.../smsc.c:449:45: warning: dereference of noderef expression
Please make sure that patches don't intoduce new warnings with W=1 C=1 builds.
> + struct inet6_ifaddr *ifa;
> +
> + list_for_each_entry(ifa, addr_list, if_list) {
> + if (ifa->scope == IFA_LINK) {
> + memcpy(&pattern[3],
> + &ifa->addr.in6_u.u6_addr8[13],
> + 3);
> + mask[0] = 0x003F;
> + len = 6;
> + break;
> + }
> + }
> + }
> + rc = lan874x_chk_wol_pattern(pattern, mask, len,
> + data, &datalen);
> + if (rc)
> + phydev_dbg(phydev, "pattern not valid at %d\n", rc);
> +
> + /* Need to match multicast destination address. */
> + val = MII_LAN874X_PHY_WOL_FILTER_MCASTTEN;
> + rc = lan874x_set_wol_pattern(phydev, val, data, datalen, mask,
> + len);
> + if (rc < 0)
> + return rc;
> + priv->wol_arp = false;
> + }
...
> diff --git a/include/linux/smscphy.h b/include/linux/smscphy.h
> index e1c88627755a..b876333257bf 100644
> --- a/include/linux/smscphy.h
> +++ b/include/linux/smscphy.h
> @@ -38,4 +38,38 @@ int smsc_phy_set_tunable(struct phy_device *phydev,
> struct ethtool_tunable *tuna, const void *data);
> int smsc_phy_probe(struct phy_device *phydev);
>
> +#define MII_LAN874X_PHY_MMD_WOL_WUCSR 0x8010
> +#define MII_LAN874X_PHY_MMD_WOL_WUF_CFGA 0x8011
> +#define MII_LAN874X_PHY_MMD_WOL_WUF_CFGB 0x8012
> +#define MII_LAN874X_PHY_MMD_WOL_WUF_MASK0 0x8021
> +#define MII_LAN874X_PHY_MMD_WOL_WUF_MASK1 0x8022
> +#define MII_LAN874X_PHY_MMD_WOL_WUF_MASK2 0x8023
> +#define MII_LAN874X_PHY_MMD_WOL_WUF_MASK3 0x8024
> +#define MII_LAN874X_PHY_MMD_WOL_WUF_MASK4 0x8025
> +#define MII_LAN874X_PHY_MMD_WOL_WUF_MASK5 0x8026
> +#define MII_LAN874X_PHY_MMD_WOL_WUF_MASK6 0x8027
> +#define MII_LAN874X_PHY_MMD_WOL_WUF_MASK7 0x8028
> +#define MII_LAN874X_PHY_MMD_WOL_RX_ADDRA 0x8061
> +#define MII_LAN874X_PHY_MMD_WOL_RX_ADDRB 0x8062
> +#define MII_LAN874X_PHY_MMD_WOL_RX_ADDRC 0x8063
> +#define MII_LAN874X_PHY_MMD_MCFGR 0x8064
> +
> +#define MII_LAN874X_PHY_PME1_SET (2<<13)
> +#define MII_LAN874X_PHY_PME2_SET (2<<11)
nit: Maybe GENMASK is appropriate here.
If not, please consider spaces around '<<'
> +#define MII_LAN874X_PHY_PME_SELF_CLEAR BIT(9)
> +#define MII_LAN874X_PHY_WOL_PFDA_FR BIT(7)
> +#define MII_LAN874X_PHY_WOL_WUFR BIT(6)
> +#define MII_LAN874X_PHY_WOL_MPR BIT(5)
> +#define MII_LAN874X_PHY_WOL_BCAST_FR BIT(4)
> +#define MII_LAN874X_PHY_WOL_PFDAEN BIT(3)
> +#define MII_LAN874X_PHY_WOL_WUEN BIT(2)
> +#define MII_LAN874X_PHY_WOL_MPEN BIT(1)
> +#define MII_LAN874X_PHY_WOL_BCSTEN BIT(0)
> +
> +#define MII_LAN874X_PHY_WOL_FILTER_EN BIT(15)
> +#define MII_LAN874X_PHY_WOL_FILTER_MCASTTEN BIT(9)
> +#define MII_LAN874X_PHY_WOL_FILTER_BCSTEN BIT(8)
> +
> +#define MII_LAN874X_PHY_PME_SELF_CLEAR_DELAY 0x1000 /* 81 milliseconds */
> +
> #endif /* __LINUX_SMSCPHY_H__ */
--
pw-bot: cr
Powered by blists - more mailing lists