Re: [PATCH v2 net-next 3/4] net: phy: Add Qualcomm QCA807x driver

From: Heiner Kallweit
Date: Wed Feb 10 2021 - 09:03:44 EST


On 10.02.2021 13:55, Robert Marko wrote:
> This adds driver for the Qualcomm QCA8072 and QCA8075 PHY-s.
>
> They are 2 or 5 port IEEE 802.3 clause 22 compliant
> 10BASE-Te, 100BASE-TX and 1000BASE-T PHY-s.
>
> They feature 2 SerDes, one for PSGMII or QSGMII connection with MAC,
> while second one is SGMII for connection to MAC or fiber.
>
> Both models have a combo port that supports 1000BASE-X and 100BASE-FX
> fiber.
>
> Each PHY inside of QCA807x series has 2 digitally controlled output only
> pins that natively drive LED-s.
> But some vendors used these to driver generic LED-s controlled by
> user space, so lets enable registering each PHY as GPIO controller and
> add driver for it.
>
> This also adds the ability to specify DT properties so that 1000 Base-T
> LED will also be lit up for 100 and 10 Base connections.
>
> This is usually done by U-boot, but boards running mainline U-boot are
> not configuring this yet.
>
> These PHY-s are commonly used in Qualcomm IPQ40xx, IPQ60xx and IPQ807x
> boards.
>
> Signed-off-by: Robert Marko <robert.marko@xxxxxxxxxx>
> Cc: Luka Perkov <luka.perkov@xxxxxxxxxx>
> ---
> Changes in v2:
> * Drop LED related code
> * Fix ordering in KConfig and Makefile
> * Add SFP module validation upon instert
> * Rework IRQ code
> * Convert values for PSGMII/QSGMII SerDes driver
> into register values instead of using defines in dt-bindings
> * Fill phydev->port with correct port type
>
> drivers/net/phy/Kconfig | 10 +
> drivers/net/phy/Makefile | 1 +
> drivers/net/phy/qca807x.c | 855 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 866 insertions(+)
> create mode 100644 drivers/net/phy/qca807x.c
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 92164e7b7f60..f0725d8acd4a 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -259,6 +259,16 @@ config AT803X_PHY
> help
> Currently supports the AR8030, AR8031, AR8033 and AR8035 model
>
> +config QCA807X_PHY
> + tristate "Qualcomm QCA807X PHYs"
> + depends on OF_MDIO
> + help
> + Adds support for the Qualcomm QCA807x PHYs.
> + These are 802.3 Clause 22 compliant PHYs supporting gigabit
> + ethernet as well as 100Base-FX and 1000Base-X fibre.
> +
> + Currently supports the QCA8072 and QCA8075 models.
> +
> config QSEMI_PHY
> tristate "Quality Semiconductor PHYs"
> help
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index ca0a313423b9..25530057950e 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -73,6 +73,7 @@ obj-$(CONFIG_MICROCHIP_T1_PHY) += microchip_t1.o
> obj-$(CONFIG_MICROSEMI_PHY) += mscc/
> obj-$(CONFIG_NATIONAL_PHY) += national.o
> obj-$(CONFIG_NXP_TJA11XX_PHY) += nxp-tja11xx.o
> +obj-$(CONFIG_QCA807X_PHY) += qca807x.o
> obj-$(CONFIG_QSEMI_PHY) += qsemi.o
> obj-$(CONFIG_REALTEK_PHY) += realtek.o
> obj-$(CONFIG_RENESAS_PHY) += uPD60620.o
> diff --git a/drivers/net/phy/qca807x.c b/drivers/net/phy/qca807x.c
> new file mode 100644
> index 000000000000..9126035853e4
> --- /dev/null
> +++ b/drivers/net/phy/qca807x.c
> @@ -0,0 +1,855 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2020 Sartura Ltd.
> + *
> + * Author: Robert Marko <robert.marko@xxxxxxxxxx>
> + *
> + * Qualcomm QCA8072 and QCA8075 PHY driver
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/phy.h>
> +#include <linux/bitfield.h>
> +#include <linux/ethtool_netlink.h>
> +#include <linux/gpio.h>
> +#include <linux/sfp.h>
> +
> +#include <dt-bindings/net/qcom-qca807x.h>
> +
> +#define PHY_ID_QCA8072 0x004dd0b2
> +#define PHY_ID_QCA8075 0x004dd0b1
> +#define PHY_ID_QCA807X_PSGMII 0x06820805
> +
> +/* Downshift */
> +#define QCA807X_SMARTSPEED_EN BIT(5)
> +#define QCA807X_SMARTSPEED_RETRY_LIMIT_MASK GENMASK(4, 2)
> +#define QCA807X_SMARTSPEED_RETRY_LIMIT_DEFAULT 5
> +#define QCA807X_SMARTSPEED_RETRY_LIMIT_MIN 2
> +#define QCA807X_SMARTSPEED_RETRY_LIMIT_MAX 9
> +
> +/* Cable diagnostic test (CDT) */
> +#define QCA807X_CDT 0x16
> +#define QCA807X_CDT_ENABLE BIT(15)
> +#define QCA807X_CDT_ENABLE_INTER_PAIR_SHORT BIT(13)
> +#define QCA807X_CDT_STATUS BIT(11)
> +#define QCA807X_CDT_MMD3_STATUS 0x8064
> +#define QCA807X_CDT_MDI0_STATUS_MASK GENMASK(15, 12)
> +#define QCA807X_CDT_MDI1_STATUS_MASK GENMASK(11, 8)
> +#define QCA807X_CDT_MDI2_STATUS_MASK GENMASK(7, 4)
> +#define QCA807X_CDT_MDI3_STATUS_MASK GENMASK(3, 0)
> +#define QCA807X_CDT_RESULTS_INVALID 0x0
> +#define QCA807X_CDT_RESULTS_OK 0x1
> +#define QCA807X_CDT_RESULTS_OPEN 0x2
> +#define QCA807X_CDT_RESULTS_SAME_SHORT 0x3
> +#define QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI1_SAME_OK 0x4
> +#define QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI2_SAME_OK 0x8
> +#define QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI3_SAME_OK 0xc
> +#define QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI1_SAME_OPEN 0x6
> +#define QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI2_SAME_OPEN 0xa
> +#define QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI3_SAME_OPEN 0xe
> +#define QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI1_SAME_SHORT 0x7
> +#define QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI2_SAME_SHORT 0xb
> +#define QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI3_SAME_SHORT 0xf
> +#define QCA807X_CDT_RESULTS_BUSY 0x9
> +#define QCA807X_CDT_MMD3_MDI0_LENGTH 0x8065
> +#define QCA807X_CDT_MMD3_MDI1_LENGTH 0x8066
> +#define QCA807X_CDT_MMD3_MDI2_LENGTH 0x8067
> +#define QCA807X_CDT_MMD3_MDI3_LENGTH 0x8068
> +#define QCA807X_CDT_SAME_SHORT_LENGTH_MASK GENMASK(15, 8)
> +#define QCA807X_CDT_CROSS_SHORT_LENGTH_MASK GENMASK(7, 0)
> +
> +#define QCA807X_CHIP_CONFIGURATION 0x1f
> +#define QCA807X_BT_BX_REG_SEL BIT(15)
> +#define QCA807X_CHIP_CONFIGURATION_MODE_CFG_MASK GENMASK(3, 0)
> +#define QCA807X_CHIP_CONFIGURATION_MODE_QSGMII_SGMII 4
> +#define QCA807X_CHIP_CONFIGURATION_MODE_PSGMII_FIBER 3
> +#define QCA807X_CHIP_CONFIGURATION_MODE_PSGMII_ALL_COPPER 0
> +
> +#define QCA807X_MEDIA_SELECT_STATUS 0x1a
> +#define QCA807X_MEDIA_DETECTED_COPPER BIT(5)
> +#define QCA807X_MEDIA_DETECTED_1000_BASE_X BIT(4)
> +#define QCA807X_MEDIA_DETECTED_100_BASE_FX BIT(3)
> +
> +#define QCA807X_MMD7_FIBER_MODE_AUTO_DETECTION 0x807e
> +#define QCA807X_MMD7_FIBER_MODE_AUTO_DETECTION_EN BIT(0)
> +
> +#define QCA807X_MMD7_1000BASE_T_POWER_SAVE_PER_CABLE_LENGTH 0x801a
> +#define QCA807X_CONTROL_DAC_MASK GENMASK(2, 0)
> +
> +#define QCA807X_MMD7_LED_100N_1 0x8074
> +#define QCA807X_MMD7_LED_100N_2 0x8075
> +#define QCA807X_MMD7_LED_1000N_1 0x8076
> +#define QCA807X_MMD7_LED_1000N_2 0x8077
> +#define QCA807X_GPIO_FORCE_EN BIT(15)
> +#define QCA807X_GPIO_FORCE_MODE_MASK GENMASK(14, 13)
> +
> +#define QCA807X_INTR_ENABLE 0x12
> +#define QCA807X_INTR_STATUS 0x13
> +#define QCA807X_INTR_ENABLE_AUTONEG_ERR BIT(15)
> +#define QCA807X_INTR_ENABLE_SPEED_CHANGED BIT(14)
> +#define QCA807X_INTR_ENABLE_DUPLEX_CHANGED BIT(13)
> +#define QCA807X_INTR_ENABLE_LINK_FAIL BIT(11)
> +#define QCA807X_INTR_ENABLE_LINK_SUCCESS BIT(10)
> +
> +#define QCA807X_FUNCTION_CONTROL 0x10
> +#define QCA807X_FC_MDI_CROSSOVER_MODE_MASK GENMASK(6, 5)
> +#define QCA807X_FC_MDI_CROSSOVER_AUTO 3
> +#define QCA807X_FC_MDI_CROSSOVER_MANUAL_MDIX 1
> +#define QCA807X_FC_MDI_CROSSOVER_MANUAL_MDI 0
> +
> +#define QCA807X_PHY_SPECIFIC_STATUS 0x11
> +#define QCA807X_SS_SPEED_AND_DUPLEX_RESOLVED BIT(11)
> +#define QCA807X_SS_SPEED_MASK GENMASK(15, 14)
> +#define QCA807X_SS_SPEED_1000 2
> +#define QCA807X_SS_SPEED_100 1
> +#define QCA807X_SS_SPEED_10 0
> +#define QCA807X_SS_DUPLEX BIT(13)
> +#define QCA807X_SS_MDIX BIT(6)
> +
> +/* PSGMII PHY specific */
> +#define PSGMII_QSGMII_DRIVE_CONTROL_1 0xb
> +#define PSGMII_QSGMII_TX_DRIVER_MASK GENMASK(7, 4)
> +#define PSGMII_MODE_CTRL 0x6d
> +#define PSGMII_MODE_CTRL_AZ_WORKAROUND_MASK GENMASK(3, 0)
> +#define PSGMII_MMD3_SERDES_CONTROL 0x805a
> +
> +struct qca807x_gpio_priv {
> + struct phy_device *phy;
> +};
> +
> +static int qca807x_get_downshift(struct phy_device *phydev, u8 *data)
> +{
> + int val, cnt, enable;
> +
> + val = phy_read(phydev, MII_NWAYTEST);
> + if (val < 0)
> + return val;
> +
> + enable = FIELD_GET(QCA807X_SMARTSPEED_EN, val);
> + cnt = FIELD_GET(QCA807X_SMARTSPEED_RETRY_LIMIT_MASK, val) + 2;
> +
> + *data = enable ? cnt : DOWNSHIFT_DEV_DISABLE;
> +
> + return 0;
> +}
> +
> +static int qca807x_set_downshift(struct phy_device *phydev, u8 cnt)
> +{
> + int ret, val;
> +
> + if (cnt > QCA807X_SMARTSPEED_RETRY_LIMIT_MAX ||
> + (cnt < QCA807X_SMARTSPEED_RETRY_LIMIT_MIN && cnt != DOWNSHIFT_DEV_DISABLE))
> + return -EINVAL;
> +
> + if (!cnt) {
> + ret = phy_clear_bits(phydev, MII_NWAYTEST, QCA807X_SMARTSPEED_EN);
> + } else {
> + val = QCA807X_SMARTSPEED_EN;
> + val |= FIELD_PREP(QCA807X_SMARTSPEED_RETRY_LIMIT_MASK, cnt - 2);
> +
> + phy_modify(phydev, MII_NWAYTEST,
> + QCA807X_SMARTSPEED_EN |
> + QCA807X_SMARTSPEED_RETRY_LIMIT_MASK,
> + val);
> + }
> +
> + ret = genphy_soft_reset(phydev);
> +
> + return ret;
> +}
> +
> +static int qca807x_get_tunable(struct phy_device *phydev,
> + struct ethtool_tunable *tuna, void *data)
> +{
> + switch (tuna->id) {
> + case ETHTOOL_PHY_DOWNSHIFT:
> + return qca807x_get_downshift(phydev, data);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int qca807x_set_tunable(struct phy_device *phydev,
> + struct ethtool_tunable *tuna, const void *data)
> +{
> + switch (tuna->id) {
> + case ETHTOOL_PHY_DOWNSHIFT:
> + return qca807x_set_downshift(phydev, *(const u8 *)data);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static bool qca807x_distance_valid(int result)
> +{
> + switch (result) {
> + case QCA807X_CDT_RESULTS_OPEN:
> + case QCA807X_CDT_RESULTS_SAME_SHORT:
> + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI1_SAME_OK:
> + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI2_SAME_OK:
> + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI3_SAME_OK:
> + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI1_SAME_OPEN:
> + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI2_SAME_OPEN:
> + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI3_SAME_OPEN:
> + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI1_SAME_SHORT:
> + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI2_SAME_SHORT:
> + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI3_SAME_SHORT:
> + return true;
> + }
> + return false;
> +}
> +
> +static int qca807x_report_length(struct phy_device *phydev,
> + int pair, int result)
> +{
> + int length;
> + int ret;
> +
> + ret = phy_read_mmd(phydev, MDIO_MMD_PCS, QCA807X_CDT_MMD3_MDI0_LENGTH + pair);
> + if (ret < 0)
> + return ret;
> +
> + switch (result) {
> + case ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT:
> + length = (FIELD_GET(QCA807X_CDT_SAME_SHORT_LENGTH_MASK, ret) * 800) / 10;
> + break;
> + case ETHTOOL_A_CABLE_RESULT_CODE_OPEN:
> + case ETHTOOL_A_CABLE_RESULT_CODE_CROSS_SHORT:
> + length = (FIELD_GET(QCA807X_CDT_CROSS_SHORT_LENGTH_MASK, ret) * 800) / 10;
> + break;
> + }
> +
> + ethnl_cable_test_fault_length(phydev, pair, length);
> +
> + return 0;
> +}
> +
> +static int qca807x_cable_test_report_trans(int result)
> +{
> + switch (result) {
> + case QCA807X_CDT_RESULTS_OK:
> + return ETHTOOL_A_CABLE_RESULT_CODE_OK;
> + case QCA807X_CDT_RESULTS_OPEN:
> + return ETHTOOL_A_CABLE_RESULT_CODE_OPEN;
> + case QCA807X_CDT_RESULTS_SAME_SHORT:
> + return ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT;
> + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI1_SAME_OK:
> + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI2_SAME_OK:
> + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI3_SAME_OK:
> + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI1_SAME_OPEN:
> + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI2_SAME_OPEN:
> + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI3_SAME_OPEN:
> + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI1_SAME_SHORT:
> + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI2_SAME_SHORT:
> + case QCA807X_CDT_RESULTS_CROSS_SHORT_WITH_MDI3_SAME_SHORT:
> + return ETHTOOL_A_CABLE_RESULT_CODE_CROSS_SHORT;
> + default:
> + return ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC;
> + }
> +}
> +
> +static int qca807x_cable_test_report(struct phy_device *phydev)
> +{
> + int pair0, pair1, pair2, pair3;
> + int ret;
> +
> + ret = phy_read_mmd(phydev, MDIO_MMD_PCS, QCA807X_CDT_MMD3_STATUS);
> + if (ret < 0)
> + return ret;
> +
> + pair0 = FIELD_GET(QCA807X_CDT_MDI0_STATUS_MASK, ret);
> + pair1 = FIELD_GET(QCA807X_CDT_MDI1_STATUS_MASK, ret);
> + pair2 = FIELD_GET(QCA807X_CDT_MDI2_STATUS_MASK, ret);
> + pair3 = FIELD_GET(QCA807X_CDT_MDI3_STATUS_MASK, ret);
> +
> + ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_A,
> + qca807x_cable_test_report_trans(pair0));
> + ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_B,
> + qca807x_cable_test_report_trans(pair1));
> + ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_C,
> + qca807x_cable_test_report_trans(pair2));
> + ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_D,
> + qca807x_cable_test_report_trans(pair3));
> +
> + if (qca807x_distance_valid(pair0))
> + qca807x_report_length(phydev, 0, qca807x_cable_test_report_trans(pair0));
> + if (qca807x_distance_valid(pair1))
> + qca807x_report_length(phydev, 1, qca807x_cable_test_report_trans(pair1));
> + if (qca807x_distance_valid(pair2))
> + qca807x_report_length(phydev, 2, qca807x_cable_test_report_trans(pair2));
> + if (qca807x_distance_valid(pair3))
> + qca807x_report_length(phydev, 3, qca807x_cable_test_report_trans(pair3));
> +
> + return 0;
> +}
> +
> +static int qca807x_cable_test_get_status(struct phy_device *phydev,
> + bool *finished)
> +{
> + int val;
> +
> + *finished = false;
> +
> + val = phy_read(phydev, QCA807X_CDT);
> + if (!((val & QCA807X_CDT_ENABLE) && (val & QCA807X_CDT_STATUS))) {
> + *finished = true;
> +
> + return qca807x_cable_test_report(phydev);
> + }
> +
> + return 0;
> +}
> +
> +static int qca807x_cable_test_start(struct phy_device *phydev)
> +{
> + int val, ret;
> +
> + /* Check if fiber is being used in the combo port,
> + * as we cant cable test fiber modules.
> + */
> + if (phy_read(phydev, QCA807X_CHIP_CONFIGURATION)) {
> + if (linkmode_test_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, phydev->supported)) {
> + val = phy_read(phydev, QCA807X_MEDIA_SELECT_STATUS);
> + if ((val & QCA807X_MEDIA_DETECTED_1000_BASE_X) ||
> + (val & QCA807X_MEDIA_DETECTED_100_BASE_FX))
> + return -EOPNOTSUPP;
> + }
> + }
> +
> + val = phy_read(phydev, QCA807X_CDT);
> + /* Enable inter-pair short check as well */
> + val &= ~QCA807X_CDT_ENABLE_INTER_PAIR_SHORT;
> + val |= QCA807X_CDT_ENABLE;
> + ret = phy_write(phydev, QCA807X_CDT, val);
> +
> + return ret;
> +}
> +
> +#ifdef CONFIG_GPIOLIB
> +static int qca807x_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
> +{
> + return GPIO_LINE_DIRECTION_OUT;
> +}
> +
> +static int qca807x_gpio_get_reg(unsigned int offset)
> +{
> + return QCA807X_MMD7_LED_100N_2 + (offset % 2) * 2;
> +}
> +
> +static int qca807x_gpio_get(struct gpio_chip *gc, unsigned int offset)
> +{
> + struct qca807x_gpio_priv *priv = gpiochip_get_data(gc);
> + int val;
> +
> + val = phy_read_mmd(priv->phy, MDIO_MMD_AN, qca807x_gpio_get_reg(offset));
> +
> + return FIELD_GET(QCA807X_GPIO_FORCE_MODE_MASK, val);
> +}
> +
> +static void qca807x_gpio_set(struct gpio_chip *gc, unsigned int offset, int value)
> +{
> + struct qca807x_gpio_priv *priv = gpiochip_get_data(gc);
> + int val;
> +
> + val = phy_read_mmd(priv->phy, MDIO_MMD_AN, qca807x_gpio_get_reg(offset));
> + val &= ~QCA807X_GPIO_FORCE_MODE_MASK;
> + val |= FIELD_PREP(QCA807X_GPIO_FORCE_MODE_MASK, value);
> +
> + phy_write_mmd(priv->phy, MDIO_MMD_AN, qca807x_gpio_get_reg(offset), val);

You could use phy_modify_mmd() here.

> +}
> +
> +static int qca807x_gpio_dir_out(struct gpio_chip *gc, unsigned int offset, int value)
> +{
> + qca807x_gpio_set(gc, offset, value);
> +
> + return 0;
> +}
> +
> +static int qca807x_gpio(struct phy_device *phydev)
> +{
> + struct device *dev = &phydev->mdio.dev;
> + struct qca807x_gpio_priv *priv;
> + struct gpio_chip *gc;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->phy = phydev;
> +
> + gc = devm_kzalloc(dev, sizeof(*gc), GFP_KERNEL);
> + if (!gc)
> + return -ENOMEM;
> +
> + gc->label = dev_name(dev);
> + gc->base = -1;
> + gc->ngpio = 2;
> + gc->parent = dev;
> + gc->owner = THIS_MODULE;
> + gc->can_sleep = true;
> + gc->get_direction = qca807x_gpio_get_direction;
> + gc->direction_output = qca807x_gpio_dir_out;
> + gc->get = qca807x_gpio_get;
> + gc->set = qca807x_gpio_set;
> +
> + return devm_gpiochip_add_data(dev, gc, priv);
> +}
> +#endif
> +
> +static int qca807x_read_copper_status(struct phy_device *phydev, bool combo_port)
> +{
> + int ss, err, page, old_link = phydev->link;
> +
> + if (phydev->port != PORT_TP)
> + phydev->port = PORT_TP;
> +
> + /* Only combo port has dual pages */
> + if (combo_port) {
> + /* Check whether copper page is set and set if needed */
> + page = phy_read(phydev, QCA807X_CHIP_CONFIGURATION);
> + if (!(page & QCA807X_BT_BX_REG_SEL)) {
> + page |= QCA807X_BT_BX_REG_SEL;
> + phy_write(phydev, QCA807X_CHIP_CONFIGURATION, page);
> + }
> + }
> +
> + /* Update the link, but return if there was an error */
> + err = genphy_update_link(phydev);
> + if (err)
> + return err;
> +
> + /* why bother the PHY if nothing can have changed */
> + if (phydev->autoneg == AUTONEG_ENABLE && old_link && phydev->link)
> + return 0;
> +
> + phydev->speed = SPEED_UNKNOWN;
> + phydev->duplex = DUPLEX_UNKNOWN;
> + phydev->pause = 0;
> + phydev->asym_pause = 0;
> +
> + err = genphy_read_lpa(phydev);
> + if (err < 0)
> + return err;
> +
> + /* Read the QCA807x PHY-Specific Status register copper page,
> + * which indicates the speed and duplex that the PHY is actually
> + * using, irrespective of whether we are in autoneg mode or not.
> + */
> + ss = phy_read(phydev, QCA807X_PHY_SPECIFIC_STATUS);
> + if (ss < 0)
> + return ss;
> +
> + if (ss & QCA807X_SS_SPEED_AND_DUPLEX_RESOLVED) {
> + int sfc;
> +
> + sfc = phy_read(phydev, QCA807X_FUNCTION_CONTROL);
> + if (sfc < 0)
> + return sfc;
> +
> + switch (FIELD_GET(QCA807X_SS_SPEED_MASK, ss)) {
> + case QCA807X_SS_SPEED_10:
> + phydev->speed = SPEED_10;
> + break;
> + case QCA807X_SS_SPEED_100:
> + phydev->speed = SPEED_100;
> + break;
> + case QCA807X_SS_SPEED_1000:
> + phydev->speed = SPEED_1000;
> + break;
> + }
> + if (ss & QCA807X_SS_DUPLEX)
> + phydev->duplex = DUPLEX_FULL;
> + else
> + phydev->duplex = DUPLEX_HALF;
> +
> + if (ss & QCA807X_SS_MDIX)
> + phydev->mdix = ETH_TP_MDI_X;
> + else
> + phydev->mdix = ETH_TP_MDI;
> +
> + switch (FIELD_GET(QCA807X_FC_MDI_CROSSOVER_MODE_MASK, sfc)) {
> + case QCA807X_FC_MDI_CROSSOVER_MANUAL_MDI:
> + phydev->mdix_ctrl = ETH_TP_MDI;
> + break;
> + case QCA807X_FC_MDI_CROSSOVER_MANUAL_MDIX:
> + phydev->mdix_ctrl = ETH_TP_MDI_X;
> + break;
> + case QCA807X_FC_MDI_CROSSOVER_AUTO:
> + phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
> + break;
> + }
> + }
> +
> + if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete)
> + phy_resolve_aneg_pause(phydev);
> +
> + return 0;
> +}
> +
> +static int qca807x_read_fiber_status(struct phy_device *phydev, bool combo_port)
> +{
> + int ss, err, page;
> +
> + if (phydev->port != PORT_FIBRE)
> + phydev->port = PORT_FIBRE;
> +
> + /* Check whether fiber page is set and set if needed */
> + page = phy_read(phydev, QCA807X_CHIP_CONFIGURATION);
> + if (page & QCA807X_BT_BX_REG_SEL) {
> + page &= ~QCA807X_BT_BX_REG_SEL;
> + phy_write(phydev, QCA807X_CHIP_CONFIGURATION, page);
> + }
> +
> + err = genphy_c37_read_status(phydev);
> + if (err)
> + return err;
> +
> + phydev->speed = SPEED_UNKNOWN;
> + phydev->duplex = DUPLEX_UNKNOWN;
> +
> + /* Read the QCA807x PHY-Specific Status register fiber page,
> + * which indicates the speed and duplex that the PHY is actually
> + * using, irrespective of whether we are in autoneg mode or not.
> + */
> + ss = phy_read(phydev, QCA807X_PHY_SPECIFIC_STATUS);
> + if (ss < 0)
> + return ss;
> +
> + if (ss & QCA807X_SS_SPEED_AND_DUPLEX_RESOLVED) {
> + switch (FIELD_GET(QCA807X_SS_SPEED_MASK, ss)) {
> + case QCA807X_SS_SPEED_100:
> + phydev->speed = SPEED_100;
> + break;
> + case QCA807X_SS_SPEED_1000:
> + phydev->speed = SPEED_1000;
> + break;
> + }
> +
> + if (ss & QCA807X_SS_DUPLEX)
> + phydev->duplex = DUPLEX_FULL;
> + else
> + phydev->duplex = DUPLEX_HALF;
> + }
> +
> + return 0;
> +}
> +
> +static int qca807x_read_status(struct phy_device *phydev)
> +{
> + int val;
> +
> + /* Check for Combo port */
> + if (phy_read(phydev, QCA807X_CHIP_CONFIGURATION)) {
> + /* Check for fiber mode first */
> + if (linkmode_test_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, phydev->supported)) {
> + /* Check for actual detected media */
> + val = phy_read(phydev, QCA807X_MEDIA_SELECT_STATUS);
> + if (val & QCA807X_MEDIA_DETECTED_COPPER) {
> + qca807x_read_copper_status(phydev, true);
> + } else if ((val & QCA807X_MEDIA_DETECTED_1000_BASE_X) ||
> + (val & QCA807X_MEDIA_DETECTED_100_BASE_FX)) {
> + qca807x_read_fiber_status(phydev, true);
> + }
> + } else {
> + qca807x_read_copper_status(phydev, true);
> + }
> + } else {
> + qca807x_read_copper_status(phydev, false);
> + }
> +
> + return 0;
> +}
> +
> +static int qca807x_ack_intr(struct phy_device *phydev)
> +{
> + int ret;
> +
> + ret = phy_read(phydev, QCA807X_INTR_STATUS);
> +
> + return (ret < 0) ? ret : 0;
> +}
> +
> +static int qca807x_config_intr(struct phy_device *phydev)
> +{
> + int ret, val;
> +
> + val = phy_read(phydev, QCA807X_INTR_ENABLE);
> +
No check for val < 0 here?

> + if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
> + /* Clear any pending interrupts */
> + ret = qca807x_ack_intr(phydev);
> + if (ret)
> + return ret;
> + /* Check for combo port as it has fewer interrupts */
> + if (phy_read(phydev, QCA807X_CHIP_CONFIGURATION)) {
> + val |= QCA807X_INTR_ENABLE_SPEED_CHANGED;
> + val |= QCA807X_INTR_ENABLE_LINK_FAIL;
> + val |= QCA807X_INTR_ENABLE_LINK_SUCCESS;
> + } else {
> + val |= QCA807X_INTR_ENABLE_AUTONEG_ERR;
> + val |= QCA807X_INTR_ENABLE_SPEED_CHANGED;
> + val |= QCA807X_INTR_ENABLE_DUPLEX_CHANGED;
> + val |= QCA807X_INTR_ENABLE_LINK_FAIL;
> + val |= QCA807X_INTR_ENABLE_LINK_SUCCESS;
> + }

Your interrupt handler has no extra functionality, therefore it
should be sufficient to get the link change events only.

> + ret = phy_write(phydev, QCA807X_INTR_ENABLE, val);
> + } else {
> + ret = phy_write(phydev, QCA807X_INTR_ENABLE, 0);
> + if (ret)
> + return ret;
> +
> + /* Clear any pending interrupts */
> + ret = qca807x_ack_intr(phydev);
> + }
> +
> + return ret;
> +}
> +
> +static irqreturn_t qca807x_handle_interrupt(struct phy_device *phydev)
> +{
> + int irq_status, int_enabled;
> +
> + irq_status = phy_read(phydev, QCA807X_INTR_STATUS);
> + if (irq_status < 0) {
> + phy_error(phydev);
> + return IRQ_NONE;
> + }
> +
> + /* Read the current enabled interrupts */
> + int_enabled = phy_read(phydev, QCA807X_INTR_ENABLE);
> + if (int_enabled < 0) {
> + phy_error(phydev);
> + return IRQ_NONE;
> + }
> +
> + /* See if this was one of our enabled interrupts */
> + if (!(irq_status & int_enabled))
> + return IRQ_NONE;
> +
> + phy_trigger_machine(phydev);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int qca807x_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
> +{
> + struct phy_device *phydev = upstream;
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(support) = { 0, };
> + phy_interface_t iface;
> +
> + sfp_parse_support(phydev->sfp_bus, id, support);
> + iface = sfp_select_interface(phydev->sfp_bus, support);
> +
> + if (iface != PHY_INTERFACE_MODE_1000BASEX &&
> + iface != PHY_INTERFACE_MODE_SGMII) {
> + dev_err(&phydev->mdio.dev, "incompatible SFP module inserted\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static const struct sfp_upstream_ops qca807x_sfp_ops = {
> + .attach = phy_sfp_attach,
> + .detach = phy_sfp_detach,
> + .module_insert = qca807x_sfp_insert,
> +};
> +
> +static int qca807x_config(struct phy_device *phydev)
> +{
> + struct device_node *node = phydev->mdio.dev.of_node;
> + int control_dac, ret = 0;
> + u32 of_control_dac;
> +
> + /* Check for Combo port */
> + if (phy_read(phydev, QCA807X_CHIP_CONFIGURATION)) {
> + int fiber_mode_autodect;
> + int psgmii_serdes;
> + int chip_config;
> +
> + if (of_property_read_bool(node, "qcom,fiber-enable")) {
> + /* Enable fiber mode autodection (1000Base-X or 100Base-FX) */
> + fiber_mode_autodect = phy_read_mmd(phydev, MDIO_MMD_AN,
> + QCA807X_MMD7_FIBER_MODE_AUTO_DETECTION);
> + fiber_mode_autodect |= QCA807X_MMD7_FIBER_MODE_AUTO_DETECTION_EN;
> + phy_write_mmd(phydev, MDIO_MMD_AN, QCA807X_MMD7_FIBER_MODE_AUTO_DETECTION,
> + fiber_mode_autodect);
> +
> + /* Enable 4 copper + combo port mode */
> + chip_config = phy_read(phydev, QCA807X_CHIP_CONFIGURATION);
> + chip_config &= ~QCA807X_CHIP_CONFIGURATION_MODE_CFG_MASK;
> + chip_config |= FIELD_PREP(QCA807X_CHIP_CONFIGURATION_MODE_CFG_MASK,
> + QCA807X_CHIP_CONFIGURATION_MODE_PSGMII_FIBER);
> + phy_write(phydev, QCA807X_CHIP_CONFIGURATION, chip_config);
> +
> + linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, phydev->supported);
> + linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, phydev->advertising);
> + }
> +
> + /* Prevent PSGMII going into hibernation via PSGMII self test */
> + psgmii_serdes = phy_read_mmd(phydev, MDIO_MMD_PCS, PSGMII_MMD3_SERDES_CONTROL);
> + psgmii_serdes &= ~BIT(1);
> + ret = phy_write_mmd(phydev, MDIO_MMD_PCS,
> + PSGMII_MMD3_SERDES_CONTROL,
> + psgmii_serdes);

Here phy_modify_mmd() could be used too, same a few lines later.

> + }
> +
> + if (!of_property_read_u32(node, "qcom,control-dac", &of_control_dac)) {
> + control_dac = phy_read_mmd(phydev, MDIO_MMD_AN,
> + QCA807X_MMD7_1000BASE_T_POWER_SAVE_PER_CABLE_LENGTH);
> + control_dac &= ~QCA807X_CONTROL_DAC_MASK;
> + control_dac |= FIELD_PREP(QCA807X_CONTROL_DAC_MASK, of_control_dac);
> + ret = phy_write_mmd(phydev, MDIO_MMD_AN,
> + QCA807X_MMD7_1000BASE_T_POWER_SAVE_PER_CABLE_LENGTH,
> + control_dac);
> + }
> +
> + return ret;
> +}
> +
> +static int qca807x_probe(struct phy_device *phydev)
> +{
> + struct device_node *node = phydev->mdio.dev.of_node;
> + int ret = 0;
> +
> + if (IS_ENABLED(CONFIG_GPIOLIB)) {
> + /* Do not register a GPIO controller unless flagged for it */
> + if (of_property_read_bool(node, "gpio-controller"))
> + ret = qca807x_gpio(phydev);

The return value is ignored if fiber is enabled. I think the function
can return -EPROBE_DEFER, and that would be something you want to
propagate to the caller.

> + }
> +
> + /* Attach SFP bus on combo port*/
> + if (of_property_read_bool(node, "qcom,fiber-enable")) {
> + if (phy_read(phydev, QCA807X_CHIP_CONFIGURATION))
> + ret = phy_sfp_probe(phydev, &qca807x_sfp_ops);
> + }
> +
> + return ret;
> +}
> +
> +static int qca807x_psgmii_config(struct phy_device *phydev)
> +{
> + struct device_node *node = phydev->mdio.dev.of_node;
> + int psgmii_az, tx_amp, ret = 0;
> + u32 tx_driver_strength_dt;
> +
> + /* Workaround to enable AZ transmitting ability */
> + if (of_property_read_bool(node, "qcom,psgmii-az")) {
> + psgmii_az = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, PSGMII_MODE_CTRL);
> + psgmii_az &= ~PSGMII_MODE_CTRL_AZ_WORKAROUND_MASK;
> + psgmii_az |= FIELD_PREP(PSGMII_MODE_CTRL_AZ_WORKAROUND_MASK, 0xc);
> + ret = phy_write_mmd(phydev, MDIO_MMD_PMAPMD, PSGMII_MODE_CTRL, psgmii_az);
> + psgmii_az = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, PSGMII_MODE_CTRL);
> + }
> +
> + /* PSGMII/QSGMII TX amp set to DT defined value instead of default 600mV */
> + if (!of_property_read_u32(node, "qcom,tx-driver-strength", &tx_driver_strength_dt)) {
> + int tx_driver_strength;
> +
> + switch (tx_driver_strength_dt) {
> + case 140:
> + tx_driver_strength = 0;
> + break;
> + case 160:
> + tx_driver_strength = 1;
> + break;
> + case 180:
> + tx_driver_strength = 2;
> + break;
> + case 200:
> + tx_driver_strength = 3;
> + break;
> + case 220:
> + tx_driver_strength = 4;
> + break;
> + case 240:
> + tx_driver_strength = 5;
> + break;
> + case 260:
> + tx_driver_strength = 6;
> + break;
> + case 280:
> + tx_driver_strength = 7;
> + break;
> + case 300:
> + tx_driver_strength = 8;
> + break;
> + case 320:
> + tx_driver_strength = 9;
> + break;
> + case 400:
> + tx_driver_strength = 10;
> + break;
> + case 500:
> + tx_driver_strength = 11;
> + break;
> + case 600:
> + tx_driver_strength = 12;
> + break;
> + default:
> + tx_driver_strength = 12;
> + break;
> + }
> +
> + tx_amp = phy_read(phydev, PSGMII_QSGMII_DRIVE_CONTROL_1);
> + tx_amp &= ~PSGMII_QSGMII_TX_DRIVER_MASK;
> + tx_amp |= FIELD_PREP(PSGMII_QSGMII_TX_DRIVER_MASK, tx_driver_strength);
> + ret = phy_write(phydev, PSGMII_QSGMII_DRIVE_CONTROL_1, tx_amp);
> + }

phy_modify() ?

> +
> + return ret;
> +}
> +
> +static struct phy_driver qca807x_drivers[] = {
> + {
> + PHY_ID_MATCH_EXACT(PHY_ID_QCA8072),
> + .name = "Qualcomm QCA8072",
> + .flags = PHY_POLL_CABLE_TEST,
> + /* PHY_GBIT_FEATURES */
> + .probe = qca807x_probe,
> + .config_init = qca807x_config,
> + .read_status = qca807x_read_status,
> + .config_intr = qca807x_config_intr,
> + .handle_interrupt = qca807x_handle_interrupt,
> + .soft_reset = genphy_soft_reset,
> + .get_tunable = qca807x_get_tunable,
> + .set_tunable = qca807x_set_tunable,
> + .cable_test_start = qca807x_cable_test_start,
> + .cable_test_get_status = qca807x_cable_test_get_status,
> + },
> + {
> + PHY_ID_MATCH_EXACT(PHY_ID_QCA8075),
> + .name = "Qualcomm QCA8075",
> + .flags = PHY_POLL_CABLE_TEST,
> + /* PHY_GBIT_FEATURES */
> + .probe = qca807x_probe,
> + .config_init = qca807x_config,
> + .read_status = qca807x_read_status,
> + .config_intr = qca807x_config_intr,
> + .handle_interrupt = qca807x_handle_interrupt,
> + .soft_reset = genphy_soft_reset,
> + .get_tunable = qca807x_get_tunable,
> + .set_tunable = qca807x_set_tunable,
> + .cable_test_start = qca807x_cable_test_start,
> + .cable_test_get_status = qca807x_cable_test_get_status,
> + },
> + {
> + PHY_ID_MATCH_EXACT(PHY_ID_QCA807X_PSGMII),
> + .name = "Qualcomm QCA807x PSGMII",
> + .probe = qca807x_psgmii_config,
> + },
> +};
> +module_phy_driver(qca807x_drivers);
> +
> +static struct mdio_device_id __maybe_unused qca807x_tbl[] = {
> + { PHY_ID_MATCH_EXACT(PHY_ID_QCA8072) },
> + { PHY_ID_MATCH_EXACT(PHY_ID_QCA8075) },
> + { PHY_ID_MATCH_MODEL(PHY_ID_QCA807X_PSGMII) },
> + { }
> +};
> +
> +MODULE_AUTHOR("Robert Marko");
> +MODULE_DESCRIPTION("Qualcomm QCA807x PHY driver");
> +MODULE_DEVICE_TABLE(mdio, qca807x_tbl);
> +MODULE_LICENSE("GPL");
>