Re: [PATCH net-next v2 4/5] net: dsa: microchip: add WoL support for KSZ87xx family

[<Arun.Ramadoss@microchip.com>]

Hi Pieter, 


> 
> +/**
> + * ksz8_ind_write8 - EEE/ACL/PME indirect register write
> + * @dev: The device structure.
> + * @table: Function & table select, register 110.
> + * @addr: Indirect access control, register 111.
> + * @data: The data to be written.
> + *
> + * This function performs an indirect register write for EEE, ACL or
> + * PME switch functionalities. Both 8-bit registers 110 and 111 are
> + * written at once with ksz_write16, using the serial multiple write
> + * functionality.
> + *
> + * Return: 0 on success, or an error code on failure.
> + */
>  static int ksz8_ind_write8(struct ksz_device *dev, u8 table, u16
> addr, u8 data)
>  {
>         const u16 *regs;
> @@ -58,6 +72,59 @@ static int ksz8_ind_write8(struct ksz_device *dev,
> u8 table, u16 addr, u8 data)
>         return ret;
>  }
> 
> 
>  int ksz8_reset_switch(struct ksz_device *dev)
>  {
>         if (ksz_is_ksz88x3(dev)) {
> @@ -1545,6 +1612,7 @@ static void ksz8795_cpu_interface_select(struct
> ksz_device *dev, int port)
> 
>  void ksz8_port_setup(struct ksz_device *dev, int port, bool
> cpu_port)
>  {
> +       const u16 *regs = dev->info->regs;
>         struct dsa_switch *ds = dev->ds;
>         const u32 *masks;
>         int queues;
> @@ -1575,6 +1643,13 @@ void ksz8_port_setup(struct ksz_device *dev,
> int port, bool cpu_port)
>                 member = BIT(dsa_upstream_port(ds, port));
> 
>         ksz8_cfg_port_member(dev, port, member);
> +
> +       /* Disable all WoL options by default. Otherwise
> +        * ksz_switch_macaddr_get/put logic will not work properly.
> +        * CPU port 4 has no WoL functionality.
> +        */
> +       if (ksz_is_ksz87xx(dev) && !cpu_port)
> +               ksz8_pme_pwrite8(dev, port, regs[REG_PORT_PME_CTRL], 

check the return value of register write.

> 0);
>  }
> 
>  static void ksz88x3_config_rmii_clk(struct ksz_device *dev)
> @@ -1790,6 +1865,7 @@ int ksz8_enable_stp_addr(struct ksz_device
> *dev)
>  int ksz8_setup(struct dsa_switch *ds)
>  {
>         struct ksz_device *dev = ds->priv;
> +       const u16 *regs = dev->info->regs;
>         int i;
> 
>         ds->mtu_enforcement_ingress = true;
> @@ -1829,6 +1905,16 @@ int ksz8_setup(struct dsa_switch *ds)
>         for (i = 0; i < (dev->info->num_vlans / 4); i++)
>                 ksz8_r_vlan_entries(dev, i);
> 
> +       /* Make sure PME (WoL) is not enabled. If requested, it will
> +        * be enabled by ksz_wol_pre_shutdown(). Otherwise, some
> PMICs
> +        * do not like PME events changes before shutdown. PME only
> +        * available on KSZ87xx family.
> +        */
> +       if (ksz_is_ksz87xx(dev)) {
> +               ksz8_pme_write8(dev, regs[REG_SW_PME_CTRL], 0);
> +               ksz_rmw8(dev, REG_INT_ENABLE, INT_PME, 0);

Here also.
> +       }
> +
>         return ksz8_handle_global_errata(ds);
>  }
> 
> 
> 
>  static const u32 ksz8795_masks[] = {
> @@ -3752,7 +3758,7 @@ static void ksz_get_wol(struct dsa_switch *ds,
> int port,
>         u8 pme_ctrl;
>         int ret;
> 
> -       if (!is_ksz9477(dev))
> +       if (!is_ksz9477(dev) && !ksz_is_ksz87xx(dev))
>                 return;
> 
>         if (!dev->wakeup_source)
> @@ -3805,7 +3811,7 @@ static int ksz_set_wol(struct dsa_switch *ds,
> int port,
>         if (wol->wolopts & ~(WAKE_PHY | WAKE_MAGIC))
>                 return -EINVAL;
> 
> -       if (!is_ksz9477(dev))
> +       if (!is_ksz9477(dev) && !ksz_is_ksz87xx(dev))
>                 return -EOPNOTSUPP;
> 
>         if (!dev->wakeup_source)
> @@ -3905,13 +3911,15 @@ int ksz_handle_wake_reason(struct ksz_device
> *dev, int port)
>   */
>  static void ksz_wol_pre_shutdown(struct ksz_device *dev, bool
> *wol_enabled)
>  {
> +       const struct ksz_dev_ops *ops = dev->dev_ops;

This change could be introduced in previous patch itself. 

>         const u16 *regs = dev->info->regs;
> +       u8 pme_pin_en = PME_ENABLE;
>         struct dsa_port *dp;
>         int ret;
> 
>         *wol_enabled = false;
> 
> -       if (!is_ksz9477(dev))
> +       if (!is_ksz9477(dev) && !ksz_is_ksz87xx(dev))
>                 return;
> 
>         if (!dev->wakeup_source)
> @@ -3920,8 +3928,8 @@ static void ksz_wol_pre_shutdown(struct
> ksz_device *dev, bool *wol_enabled)
>         dsa_switch_for_each_user_port(dp, dev->ds) {
>                 u8 pme_ctrl = 0;
> 
> -               ret = dev->dev_ops->pme_pread8(dev, dp->index,
> -                                              regs[REG_PORT_PME_CTRL
> ], &pme_ctrl);
> +               ret = ops->pme_pread8(dev, dp->index,
> +                                     regs[REG_PORT_PME_CTRL],
> &pme_ctrl);
>                 if (!ret && pme_ctrl)
>                         *wol_enabled = true;
> 
> @@ -3932,8 +3940,13 @@ static void ksz_wol_pre_shutdown(struct
> ksz_device *dev, bool *wol_enabled)
>         }
> 
>         /* Now we are save to enable PME pin. */
> -       if (*wol_enabled)
> -               dev->dev_ops->pme_write8(dev, regs[REG_SW_PME_CTRL],
> PME_ENABLE);
> +       if (*wol_enabled) {
> +               if (dev->pme_active_high)
> +                       pme_pin_en |= PME_POLARITY;
> +               ops->pme_write8(dev, regs[REG_SW_PME_CTRL],
> pme_pin_en);
> +               if (ksz_is_ksz87xx(dev))
> +                       ksz_write8(dev, KSZ8795_REG_INT_EN,
> KSZ8795_INT_PME_MASK);

check the return values. 
> +       }
>  }
> 
> 
>         ret = dsa_register_switch(dev->ds);
> diff --git a/drivers/net/dsa/microchip/ksz_common.h
> b/drivers/net/dsa/microchip/ksz_common.h
> index c60c218afa64..c0b93825726d 100644
> --- a/drivers/net/dsa/microchip/ksz_common.h
> +++ b/drivers/net/dsa/microchip/ksz_common.h
> @@ -174,6 +174,7 @@ struct ksz_device {
>         bool synclko_125;
>         bool synclko_disable;
>         bool wakeup_source;
> +       bool pme_active_high;
> 
>         struct vlan_table *vlan_cache;
> 
> @@ -712,6 +713,9 @@ static inline bool is_lan937x_tx_phy(struct
> ksz_device *dev, int port)
>  #define PME_ENABLE                     BIT(1)
>  #define PME_POLARITY                   BIT(0)
> 
> +#define KSZ8795_REG_INT_EN             0x7D
> +#define KSZ8795_INT_PME_MASK           BIT(4)
> +
>  /* Interrupt */
>  #define REG_SW_PORT_INT_STATUS__1      0x001B
>  #define REG_SW_PORT_INT_MASK__1                0x001F
> --
> 2.43.0
>