Re: [PATCH net-next v2] net: phy: realtek: Add support for WOL magic packet on RTL8211F

[Jon Hunter]

Hi Daniel,

On 29/04/2025 12:33, Daniel Braunwarth via B4 Relay wrote:
> From: Daniel Braunwarth <daniel.braunwarth@xxxxxxxx>
>
> The RTL8211F supports multiple WOL modes. This patch adds support for
> magic packets.
>
> The PHY notifies the system via the INTB/PMEB pin when a WOL event
> occurs.
>
> Signed-off-by: Daniel Braunwarth <daniel.braunwarth@xxxxxxxx>
> ---
> Changes in v2:
> - Read current WOL configuration from PHY
> - Replace magic numbers with defines
> - Link to v1: https://lore.kernel.org/r/20250428-realtek_wol-v1-1-15de3139d488@xxxxxxxx
> ---
>   drivers/net/phy/realtek/realtek_main.c | 69 ++++++++++++++++++++++++++++++++++
>   1 file changed, 69 insertions(+)
>
> diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
> index 893c824796715a905bab99646a474c3bea95ec11..05c4f4d394a5ff32b43dd51f0bff08f437ad0494 100644
> --- a/drivers/net/phy/realtek/realtek_main.c
> +++ b/drivers/net/phy/realtek/realtek_main.c
> @@ -10,6 +10,7 @@
>   #include <linux/bitops.h>
>   #include <linux/of.h>
>   #include <linux/phy.h>
> +#include <linux/netdevice.h>
>   #include <linux/module.h>
>   #include <linux/delay.h>
>   #include <linux/clk.h>
> @@ -38,6 +39,24 @@
>   
>   #define RTL8211F_INSR				0x1d
>   
> +/* RTL8211F WOL interrupt configuration */
> +#define RTL8211F_INTBCR_PAGE			0xd40
> +#define RTL8211F_INTBCR				0x16
> +#define RTL8211F_INTBCR_INTB_PMEB		BIT(5)
> +
> +/* RTL8211F WOL settings */
> +#define RTL8211F_WOL_SETTINGS_PAGE		0xd8a
> +#define RTL8211F_WOL_SETTINGS_EVENTS		16
> +#define RTL8211F_WOL_EVENT_MAGIC		BIT(12)
> +#define RTL8211F_WOL_SETTINGS_STATUS		17
> +#define RTL8211F_WOL_STATUS_RESET		(BIT(15) | 0x1fff)
> +
> +/* RTL8211F Unique phyiscal and multicast address (WOL) */
> +#define RTL8211F_PHYSICAL_ADDR_PAGE		0xd8c
> +#define RTL8211F_PHYSICAL_ADDR_WORD0		16
> +#define RTL8211F_PHYSICAL_ADDR_WORD1		17
> +#define RTL8211F_PHYSICAL_ADDR_WORD2		18
> +
>   #define RTL8211F_LEDCR				0x10
>   #define RTL8211F_LEDCR_MODE			BIT(15)
>   #define RTL8211F_LEDCR_ACT_TXRX			BIT(4)
> @@ -123,6 +142,7 @@ struct rtl821x_priv {
>   	u16 phycr2;
>   	bool has_phycr2;
>   	struct clk *clk;
> +	u32 saved_wolopts;
>   };
>   
>   static int rtl821x_read_page(struct phy_device *phydev)
> @@ -354,6 +374,53 @@ static irqreturn_t rtl8211f_handle_interrupt(struct phy_device *phydev)
>   	return IRQ_HANDLED;
>   }
>   
> +static void rtl8211f_get_wol(struct phy_device *dev, struct ethtool_wolinfo *wol)
> +{
> +	wol->supported = WAKE_MAGIC;
> +	if (phy_read_paged(dev, RTL8211F_WOL_SETTINGS_PAGE, RTL8211F_WOL_SETTINGS_EVENTS)
> +	    & RTL8211F_WOL_EVENT_MAGIC)

Given that phy_read_paged() can return an error, should we not check the 
value return is greater than 0 before ANDing with RTL8211F_WOL_EVENT_MAGIC?

Thanks
Jon

--
nvpublic