[net-next RFC PATCH v2 01/11] net: phy: extend PHY package API to support multiple global address

From: Christian Marangi
Date: Fri Nov 24 2023 - 19:35:35 EST


Current API for PHY package are limited to single address to configure
global settings for the PHY package.

It was found that some PHY package (for example the qca807x, a PHY
package that is shipped with a bundle of 5 PHY) require multiple PHY
address to configure global settings. An example scenario is a PHY that
have a dedicated PHY for PSGMII/serdes calibrarion and have a specific
PHY in the package where the global PHY mode is set and affects every
other PHY in the package.

Change the API in the following way:
- Make phy_package_join() require a list of address to be passed and the
number of address in the list
- On shared data init, each address is the list is checked and added to
the shared struct.
- Make __/phy_package_write/read() require an additional arg that
select what global PHY address to use in the provided list.

Each user of this API is updated to follow this new implementation
following a pattern where an enum is defined to declare the index of the
addr and the addr list is passed.

Signed-off-by: Christian Marangi <ansuelsmth@xxxxxxxxx>
---
drivers/net/phy/bcm54140.c | 23 +++++++--
drivers/net/phy/mediatek-ge-soc.c | 11 +++-
drivers/net/phy/micrel.c | 13 +++--
drivers/net/phy/mscc/mscc.h | 7 +++
drivers/net/phy/mscc/mscc_main.c | 16 ++++--
drivers/net/phy/phy_device.c | 85 ++++++++++++++++++++-----------
include/linux/phy.h | 51 +++++++++++++------
7 files changed, 147 insertions(+), 59 deletions(-)

diff --git a/drivers/net/phy/bcm54140.c b/drivers/net/phy/bcm54140.c
index d43076592f81..89c735b386d3 100644
--- a/drivers/net/phy/bcm54140.c
+++ b/drivers/net/phy/bcm54140.c
@@ -128,6 +128,12 @@
#define BCM54140_DEFAULT_DOWNSHIFT 5
#define BCM54140_MAX_DOWNSHIFT 9

+enum bcm54140_global_phy {
+ BCM54140_BASE_ADDR = 0,
+
+ __BCM54140_GLOBAL_PHY_MAX,
+};
+
struct bcm54140_priv {
int port;
int base_addr;
@@ -429,11 +435,13 @@ static int bcm54140_base_read_rdb(struct phy_device *phydev, u16 rdb)
int ret;

phy_lock_mdio_bus(phydev);
- ret = __phy_package_write(phydev, MII_BCM54XX_RDB_ADDR, rdb);
+ ret = __phy_package_write(phydev, BCM54140_BASE_ADDR,
+ MII_BCM54XX_RDB_ADDR, rdb);
if (ret < 0)
goto out;

- ret = __phy_package_read(phydev, MII_BCM54XX_RDB_DATA);
+ ret = __phy_package_read(phydev, BCM54140_BASE_ADDR,
+ MII_BCM54XX_RDB_DATA);

out:
phy_unlock_mdio_bus(phydev);
@@ -446,11 +454,13 @@ static int bcm54140_base_write_rdb(struct phy_device *phydev,
int ret;

phy_lock_mdio_bus(phydev);
- ret = __phy_package_write(phydev, MII_BCM54XX_RDB_ADDR, rdb);
+ ret = __phy_package_write(phydev, BCM54140_BASE_ADDR,
+ MII_BCM54XX_RDB_ADDR, rdb);
if (ret < 0)
goto out;

- ret = __phy_package_write(phydev, MII_BCM54XX_RDB_DATA, val);
+ ret = __phy_package_write(phydev, BCM54140_BASE_ADDR,
+ MII_BCM54XX_RDB_DATA, val);

out:
phy_unlock_mdio_bus(phydev);
@@ -570,6 +580,7 @@ static int bcm54140_get_base_addr_and_port(struct phy_device *phydev)

static int bcm54140_probe(struct phy_device *phydev)
{
+ int addrs[__BCM54140_GLOBAL_PHY_MAX];
struct bcm54140_priv *priv;
int ret;

@@ -583,7 +594,9 @@ static int bcm54140_probe(struct phy_device *phydev)
if (ret)
return ret;

- devm_phy_package_join(&phydev->mdio.dev, phydev, priv->base_addr, 0);
+ addrs[BCM54140_BASE_ADDR] = priv->base_addr;
+ devm_phy_package_join(&phydev->mdio.dev, phydev, addrs,
+ ARRAY_SIZE(addrs), 0);

#if IS_ENABLED(CONFIG_HWMON)
mutex_init(&priv->alarm_lock);
diff --git a/drivers/net/phy/mediatek-ge-soc.c b/drivers/net/phy/mediatek-ge-soc.c
index 8a20d9889f10..3f2043fe05ed 100644
--- a/drivers/net/phy/mediatek-ge-soc.c
+++ b/drivers/net/phy/mediatek-ge-soc.c
@@ -298,6 +298,12 @@ struct mtk_socphy_priv {
unsigned long led_state;
};

+enum mtk_global_phy {
+ MTK_BASE_ADDR = 0,
+
+ __MTK_GLOBAL_PHY_MAX
+};
+
struct mtk_socphy_shared {
u32 boottrap;
struct mtk_socphy_priv priv[4];
@@ -1431,13 +1437,16 @@ static void mt798x_phy_leds_state_init(struct phy_device *phydev)
static int mt7988_phy_probe(struct phy_device *phydev)
{
struct mtk_socphy_shared *shared;
+ int addrs[__MTK_GLOBAL_PHY_MAX];
struct mtk_socphy_priv *priv;
int err;

if (phydev->mdio.addr > 3)
return -EINVAL;

- err = devm_phy_package_join(&phydev->mdio.dev, phydev, 0,
+ addrs[MTK_BASE_ADDR] = 0;
+ err = devm_phy_package_join(&phydev->mdio.dev, phydev,
+ addrs, ARRAY_SIZE(addrs),
sizeof(struct mtk_socphy_shared));
if (err)
return err;
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 08e3915001c3..94a4c7d9ae9c 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -328,6 +328,12 @@ struct kszphy_ptp_priv {
spinlock_t seconds_lock;
};

+enum ksz_global_phy {
+ KSZ_BASE_ADDR = 0,
+
+ __KZS_GLOBAL_PHY_MAX,
+};
+
struct kszphy_priv {
struct kszphy_ptp_priv ptp_priv;
const struct kszphy_type *type;
@@ -3274,8 +3280,8 @@ static int lan8814_release_coma_mode(struct phy_device *phydev)
static int lan8814_probe(struct phy_device *phydev)
{
const struct kszphy_type *type = phydev->drv->driver_data;
+ int addrs[__KZS_GLOBAL_PHY_MAX];
struct kszphy_priv *priv;
- u16 addr;
int err;

priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
@@ -3291,9 +3297,10 @@ static int lan8814_probe(struct phy_device *phydev)
/* Strap-in value for PHY address, below register read gives starting
* phy address value
*/
- addr = lanphy_read_page_reg(phydev, 4, 0) & 0x1F;
+ addrs[KSZ_BASE_ADDR] = lanphy_read_page_reg(phydev, 4, 0) & 0x1F;
devm_phy_package_join(&phydev->mdio.dev, phydev,
- addr, sizeof(struct lan8814_shared_priv));
+ addrs, ARRAY_SIZE(addrs),
+ sizeof(struct lan8814_shared_priv));

if (phy_package_init_once(phydev)) {
err = lan8814_release_coma_mode(phydev);
diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h
index 7a962050a4d4..88da8eca2b94 100644
--- a/drivers/net/phy/mscc/mscc.h
+++ b/drivers/net/phy/mscc/mscc.h
@@ -416,6 +416,13 @@ struct vsc8531_private {
* gpio_lock: used for PHC operations. Common for all PHYs as the load/save GPIO
* is shared.
*/
+
+enum vsc85xx_global_phy {
+ VSC88XX_BASE_ADDR = 0,
+
+ __VSC8XX_GLOBAL_PHY_MAX,
+};
+
struct vsc85xx_shared_private {
struct mutex gpio_lock;
};
diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index 4171f01d34e5..749d4a6be60c 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -711,7 +711,7 @@ int phy_base_write(struct phy_device *phydev, u32 regnum, u16 val)
dump_stack();
}

- return __phy_package_write(phydev, regnum, val);
+ return __phy_package_write(phydev, VSC88XX_BASE_ADDR, regnum, val);
}

/* phydev->bus->mdio_lock should be locked when using this function */
@@ -722,7 +722,7 @@ int phy_base_read(struct phy_device *phydev, u32 regnum)
dump_stack();
}

- return __phy_package_read(phydev, regnum);
+ return __phy_package_read(phydev, VSC88XX_BASE_ADDR, regnum);
}

u32 vsc85xx_csr_read(struct phy_device *phydev,
@@ -2204,6 +2204,7 @@ static int vsc85xx_read_status(struct phy_device *phydev)

static int vsc8514_probe(struct phy_device *phydev)
{
+ int addrs[__VSC8XX_GLOBAL_PHY_MAX];
struct vsc8531_private *vsc8531;
u32 default_mode[4] = {VSC8531_LINK_1000_ACTIVITY,
VSC8531_LINK_100_ACTIVITY, VSC8531_LINK_ACTIVITY,
@@ -2216,8 +2217,9 @@ static int vsc8514_probe(struct phy_device *phydev)
phydev->priv = vsc8531;

vsc8584_get_base_addr(phydev);
+ addrs[VSC88XX_BASE_ADDR] = vsc8531->base_addr;
devm_phy_package_join(&phydev->mdio.dev, phydev,
- vsc8531->base_addr, 0);
+ addrs, ARRAY_SIZE(addrs), 0);

vsc8531->nleds = 4;
vsc8531->supp_led_modes = VSC85XX_SUPP_LED_MODES;
@@ -2233,6 +2235,7 @@ static int vsc8514_probe(struct phy_device *phydev)

static int vsc8574_probe(struct phy_device *phydev)
{
+ int addrs[__VSC8XX_GLOBAL_PHY_MAX];
struct vsc8531_private *vsc8531;
u32 default_mode[4] = {VSC8531_LINK_1000_ACTIVITY,
VSC8531_LINK_100_ACTIVITY, VSC8531_LINK_ACTIVITY,
@@ -2245,8 +2248,9 @@ static int vsc8574_probe(struct phy_device *phydev)
phydev->priv = vsc8531;

vsc8584_get_base_addr(phydev);
+ addrs[VSC88XX_BASE_ADDR] = vsc8531->base_addr;
devm_phy_package_join(&phydev->mdio.dev, phydev,
- vsc8531->base_addr, 0);
+ addrs, ARRAY_SIZE(addrs), 0);

vsc8531->nleds = 4;
vsc8531->supp_led_modes = VSC8584_SUPP_LED_MODES;
@@ -2262,6 +2266,7 @@ static int vsc8574_probe(struct phy_device *phydev)

static int vsc8584_probe(struct phy_device *phydev)
{
+ int addrs[__VSC8XX_GLOBAL_PHY_MAX];
struct vsc8531_private *vsc8531;
u32 default_mode[4] = {VSC8531_LINK_1000_ACTIVITY,
VSC8531_LINK_100_ACTIVITY, VSC8531_LINK_ACTIVITY,
@@ -2280,7 +2285,8 @@ static int vsc8584_probe(struct phy_device *phydev)
phydev->priv = vsc8531;

vsc8584_get_base_addr(phydev);
- devm_phy_package_join(&phydev->mdio.dev, phydev, vsc8531->base_addr,
+ addrs[VSC88XX_BASE_ADDR] = vsc8531->base_addr;
+ devm_phy_package_join(&phydev->mdio.dev, phydev, addrs, ARRAY_SIZE(addrs),
sizeof(struct vsc85xx_shared_private));

vsc8531->nleds = 4;
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 478126f6b5bc..e016dbfb0d27 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1648,59 +1648,80 @@ EXPORT_SYMBOL_GPL(phy_driver_is_genphy_10g);
/**
* phy_package_join - join a common PHY group
* @phydev: target phy_device struct
- * @addr: cookie and PHY address for global register access
+ * @addrs: list of cookies and PHY addresses for global register access
+ * @addrs_num: num of cookies and PHY address in addrs list
* @priv_size: if non-zero allocate this amount of bytes for private data
*
* This joins a PHY group and provides a shared storage for all phydevs in
* this group. This is intended to be used for packages which contain
* more than one PHY, for example a quad PHY transceiver.
*
- * The addr parameter serves as a cookie which has to have the same value
+ * The addrs parameters serves as cookies which has to have the same values
* for all members of one group and as a PHY address to access generic
* registers of a PHY package. Usually, one of the PHY addresses of the
* different PHYs in the package provides access to these global registers.
- * The address which is given here, will be used in the phy_package_read()
+ * The addresses which is given here, will be used in the phy_package_read()
* and phy_package_write() convenience functions. If your PHY doesn't have
* global registers you can just pick any of the PHY addresses.
+ * In some special PHY package, multiple PHY are used for global init of
+ * the entire PHY package. In the scenario, multiple address are defined.
+ * phy_package_read() and phy_package_write() requires an index to be passed
+ * to communicate which PHY to use for global init on read/write.
*
* This will set the shared pointer of the phydev to the shared storage.
* If this is the first call for a this cookie the shared storage will be
* allocated. If priv_size is non-zero, the given amount of bytes are
* allocated for the priv member.
+ * A list is allocated based on the addrs_num value and the passed list in
+ * addrs is copied to the just allocated list.
*
* Returns < 1 on error, 0 on success. Esp. calling phy_package_join()
* with the same cookie but a different priv_size is an error.
*/
-int phy_package_join(struct phy_device *phydev, int addr, size_t priv_size)
+int phy_package_join(struct phy_device *phydev, int *addrs, size_t addrs_num,
+ size_t priv_size)
{
struct mii_bus *bus = phydev->mdio.bus;
struct phy_package_shared *shared;
- int ret;
+ int *shared_addrs;
+ int i, addr, ret;

- if (addr < 0 || addr >= PHY_MAX_ADDR)
+ if (!addrs || !addrs_num)
return -EINVAL;

+ for (i = 0; i < addrs_num; i++)
+ if (addrs[i] < 0 || addrs[i] >= PHY_MAX_ADDR)
+ return -EINVAL;
+
mutex_lock(&bus->shared_lock);
- shared = bus->shared[addr];
- if (!shared) {
- ret = -ENOMEM;
- shared = kzalloc(sizeof(*shared), GFP_KERNEL);
- if (!shared)
- goto err_unlock;
- if (priv_size) {
- shared->priv = kzalloc(priv_size, GFP_KERNEL);
- if (!shared->priv)
- goto err_free;
- shared->priv_size = priv_size;
+ for (i = 0; i < addrs_num; i++) {
+ addr = addrs[i];
+ shared = bus->shared[addr];
+ if (!shared) {
+ ret = -ENOMEM;
+ shared = kzalloc(sizeof(*shared), GFP_KERNEL);
+ if (!shared)
+ goto err_unlock;
+ if (priv_size) {
+ shared->priv = kzalloc(priv_size, GFP_KERNEL);
+ if (!shared->priv)
+ goto err_free;
+ shared->priv_size = priv_size;
+ }
+ shared_addrs = kmalloc_array(addrs_num, sizeof(*addrs), GFP_KERNEL);
+ if (!shared_addrs)
+ goto err_free_priv;
+ memcpy(shared_addrs, addrs, sizeof(*addrs) * addrs_num);
+ shared->addrs = shared_addrs;
+ shared->addrs_num = addrs_num;
+ refcount_set(&shared->refcnt, 1);
+ bus->shared[addr] = shared;
+ } else {
+ ret = -EINVAL;
+ if (priv_size && priv_size != shared->priv_size)
+ goto err_unlock;
+ refcount_inc(&shared->refcnt);
}
- shared->addr = addr;
- refcount_set(&shared->refcnt, 1);
- bus->shared[addr] = shared;
- } else {
- ret = -EINVAL;
- if (priv_size && priv_size != shared->priv_size)
- goto err_unlock;
- refcount_inc(&shared->refcnt);
}
mutex_unlock(&bus->shared_lock);

@@ -1708,6 +1729,8 @@ int phy_package_join(struct phy_device *phydev, int addr, size_t priv_size)

return 0;

+err_free_priv:
+ kfree(shared->priv);
err_free:
kfree(shared);
err_unlock:
@@ -1728,13 +1751,16 @@ void phy_package_leave(struct phy_device *phydev)
{
struct phy_package_shared *shared = phydev->shared;
struct mii_bus *bus = phydev->mdio.bus;
+ int i;

if (!shared)
return;

if (refcount_dec_and_mutex_lock(&shared->refcnt, &bus->shared_lock)) {
- bus->shared[shared->addr] = NULL;
+ for (i = 0; i < shared->addrs_num; i++)
+ bus->shared[shared->addrs[i]] = NULL;
mutex_unlock(&bus->shared_lock);
+ kfree(shared->addrs);
kfree(shared->priv);
kfree(shared);
}
@@ -1752,7 +1778,8 @@ static void devm_phy_package_leave(struct device *dev, void *res)
* devm_phy_package_join - resource managed phy_package_join()
* @dev: device that is registering this PHY package
* @phydev: target phy_device struct
- * @addr: cookie and PHY address for global register access
+ * @addrs: list of cookies and PHY addresses for global register access
+ * @addrs_num: num of cookies and PHY address in addrs list
* @priv_size: if non-zero allocate this amount of bytes for private data
*
* Managed phy_package_join(). Shared storage fetched by this function,
@@ -1760,7 +1787,7 @@ static void devm_phy_package_leave(struct device *dev, void *res)
* phy_package_join() for more information.
*/
int devm_phy_package_join(struct device *dev, struct phy_device *phydev,
- int addr, size_t priv_size)
+ int *addrs, size_t addrs_num, size_t priv_size)
{
struct phy_device **ptr;
int ret;
@@ -1770,7 +1797,7 @@ int devm_phy_package_join(struct device *dev, struct phy_device *phydev,
if (!ptr)
return -ENOMEM;

- ret = phy_package_join(phydev, addr, priv_size);
+ ret = phy_package_join(phydev, addrs, addrs_num, priv_size);

if (!ret) {
*ptr = phydev;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 3cc52826f18e..c2bb3f0b9dda 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -327,7 +327,8 @@ struct mdio_bus_stats {

/**
* struct phy_package_shared - Shared information in PHY packages
- * @addr: Common PHY address used to combine PHYs in one package
+ * @addrs: List of common PHY addresses used to combine PHYs in one package
+ * @addrs_num: Number of common PHY addresses in addrs list
* @refcnt: Number of PHYs connected to this shared data
* @flags: Initialization of PHY package
* @priv_size: Size of the shared private data @priv
@@ -338,7 +339,14 @@ struct mdio_bus_stats {
* phy_package_leave().
*/
struct phy_package_shared {
- int addr;
+ /* addrs list pointer */
+ /* note that this pointer is shared between different phydevs.
+ * It is allocated and freed automatically by phy_package_join() and
+ * phy_package_leave(), the list passed to phy_package_join() is copied
+ * to the new allocated list.
+ */
+ int *addrs;
+ size_t addrs_num;
refcount_t refcnt;
unsigned long flags;
size_t priv_size;
@@ -1970,10 +1978,11 @@ int phy_ethtool_get_link_ksettings(struct net_device *ndev,
int phy_ethtool_set_link_ksettings(struct net_device *ndev,
const struct ethtool_link_ksettings *cmd);
int phy_ethtool_nway_reset(struct net_device *ndev);
-int phy_package_join(struct phy_device *phydev, int addr, size_t priv_size);
+int phy_package_join(struct phy_device *phydev, int *addrs, size_t addrs_num,
+ size_t priv_size);
void phy_package_leave(struct phy_device *phydev);
int devm_phy_package_join(struct device *dev, struct phy_device *phydev,
- int addr, size_t priv_size);
+ int *addrs, size_t addrs_num, size_t priv_size);

int __init mdio_bus_init(void);
void mdio_bus_exit(void);
@@ -1996,46 +2005,56 @@ int __phy_hwtstamp_set(struct phy_device *phydev,
struct kernel_hwtstamp_config *config,
struct netlink_ext_ack *extack);

-static inline int phy_package_read(struct phy_device *phydev, u32 regnum)
+static inline int phy_package_read(struct phy_device *phydev,
+ int global_phy_index, u32 regnum)
{
struct phy_package_shared *shared = phydev->shared;
+ int addr;

- if (!shared)
+ if (!shared || global_phy_index > shared->addrs_num - 1)
return -EIO;

- return mdiobus_read(phydev->mdio.bus, shared->addr, regnum);
+ addr = shared->addrs[global_phy_index];
+ return mdiobus_read(phydev->mdio.bus, addr, regnum);
}

-static inline int __phy_package_read(struct phy_device *phydev, u32 regnum)
+static inline int __phy_package_read(struct phy_device *phydev,
+ int global_phy_index, u32 regnum)
{
struct phy_package_shared *shared = phydev->shared;
+ int addr;

- if (!shared)
+ if (!shared || global_phy_index > shared->addrs_num - 1)
return -EIO;

- return __mdiobus_read(phydev->mdio.bus, shared->addr, regnum);
+ addr = shared->addrs[global_phy_index];
+ return __mdiobus_read(phydev->mdio.bus, addr, regnum);
}

static inline int phy_package_write(struct phy_device *phydev,
- u32 regnum, u16 val)
+ int global_phy_index, u32 regnum, u16 val)
{
struct phy_package_shared *shared = phydev->shared;
+ int addr;

- if (!shared)
+ if (!shared || global_phy_index > shared->addrs_num - 1)
return -EIO;

- return mdiobus_write(phydev->mdio.bus, shared->addr, regnum, val);
+ addr = shared->addrs[global_phy_index];
+ return mdiobus_write(phydev->mdio.bus, addr, regnum, val);
}

static inline int __phy_package_write(struct phy_device *phydev,
- u32 regnum, u16 val)
+ int global_phy_index, u32 regnum, u16 val)
{
struct phy_package_shared *shared = phydev->shared;
+ int addr;

- if (!shared)
+ if (!shared || global_phy_index > shared->addrs_num - 1)
return -EIO;

- return __mdiobus_write(phydev->mdio.bus, shared->addr, regnum, val);
+ addr = shared->addrs[global_phy_index];
+ return __mdiobus_write(phydev->mdio.bus, addr, regnum, val);
}

static inline bool __phy_package_set_once(struct phy_device *phydev,
--
2.40.1