Hello Bastien,
On Tue, 27 Feb 2024 10:39:43 +0100
Bastien Curutchet <bastien.curutchet@xxxxxxxxxxx> wrote:
> The driver does not support enabling/disabling Energy Detect Power Down
> (EDPD).
> The PHY itself support EDPD.
>
> Add missing part in the driver in order to have this support based on
> ethtool {set,get}_phy_tunable functions.
>
> Signed-off-by: Bastien Curutchet <bastien.curutchet@xxxxxxxxxxx>
> ---
> drivers/net/phy/dp83640.c | 62 +++++++++++++++++++++++++++++++++++
> drivers/net/phy/dp83640_reg.h | 4 +++
> 2 files changed, 66 insertions(+)
>
> diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
> index c46c81ef0ad0..16c9fda50b19 100644
> --- a/drivers/net/phy/dp83640.c
> +++ b/drivers/net/phy/dp83640.c
> @@ -1531,6 +1531,66 @@ static void dp83640_remove(struct phy_device *phydev)
> kfree(dp83640);
> }
>
> +static int dp83640_get_edpd(struct phy_device *phydev, u16 *edpd)
> +{
> + int val;
> +
> + phy_write(phydev, PAGESEL, 0);
Same comment as the previous patch on paged accesses
> + val = phy_read(phydev, EDCR);
> + if (val & ED_EN)
> + *edpd = 2000; /* 2 seconds */
> + else
> + *edpd = ETHTOOL_PHY_EDPD_DISABLE;
> +
> + return 0;
> +}
> +
> +static int dp83640_set_edpd(struct phy_device *phydev, u16 edpd)
> +{
> + int val;
> +
> + phy_write(phydev, PAGESEL, 0);
ditto
> + val = phy_read(phydev, EDCR);
> +
> + switch (edpd) {
> + case ETHTOOL_PHY_EDPD_DFLT_TX_MSECS:
> + case 2000: /* 2 seconds */
> + val |= ED_EN;
> + break;
> + case ETHTOOL_PHY_EDPD_DISABLE:
> + val &= ~ED_EN;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + phy_write(phydev, EDCR, val);
Here you could use phy_clear_bits / phy_set_bits, which are flavors of
the phy_modify accessors.
> +
> + return 0;
> +}
> +
> +static int dp83640_get_tunable(struct phy_device *phydev,
> + struct ethtool_tunable *tuna, void *data)
> +{
> + switch (tuna->id) {
> + case ETHTOOL_PHY_EDPD:
> + return dp83640_get_edpd(phydev, data);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int dp83640_set_tunable(struct phy_device *phydev,
> + struct ethtool_tunable *tuna, const void *data)
> +{
> + switch (tuna->id) {
> + case ETHTOOL_PHY_EDPD:
> + return dp83640_set_edpd(phydev, *(u16 *)data);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> static int dp83640_led_brightness_set(struct phy_device *phydev, u8 index,
> enum led_brightness brightness)
> {
> @@ -1692,6 +1752,8 @@ static struct phy_driver dp83640_driver = {
> .name = "NatSemi DP83640",
> /* PHY_BASIC_FEATURES */
> .probe = dp83640_probe,
> + .get_tunable = dp83640_get_tunable,
> + .set_tunable = dp83640_set_tunable,
> .remove = dp83640_remove,
> .soft_reset = dp83640_soft_reset,
> .config_init = dp83640_config_init,
> diff --git a/drivers/net/phy/dp83640_reg.h b/drivers/net/phy/dp83640_reg.h
> index 09dd2d2527c7..bf34d422d91e 100644
> --- a/drivers/net/phy/dp83640_reg.h
> +++ b/drivers/net/phy/dp83640_reg.h
> @@ -9,6 +9,7 @@
> #define LEDCR 0x0018 /* PHY Control Register */
> #define PHYCR 0x0019 /* PHY Control Register */
> #define PHYCR2 0x001c /* PHY Control Register 2 */
> +#define EDCR 0x001D /* Energy Detect Control Register */
>
> #define PAGE4 0x0004
> #define PTP_CTL 0x0014 /* PTP Control Register */
> @@ -64,6 +65,9 @@
> /* Bit definitions for the PHYCR2 register */
> #define BC_WRITE (1<<11) /* Broadcast Write Enable */
>
> +/* Bit definitions for the EDCR register */
> +#define ED_EN BIT(15) /* Enable Energy Detect Mode */
> +
> /* Bit definitions for the PTP_CTL register */
> #define TRIG_SEL_SHIFT (10) /* PTP Trigger Select */
> #define TRIG_SEL_MASK (0x7)
Thanks,
Maxime
