Idea of this big series is to introduce the concept of PHY package in DT
and generalize the support of it by PHY driver.
The concept of PHY package is nothing new and is already a thing in the
kernel with the API phy_package_join/leave/read/write.
The main idea of those API is to permit the PHY to have a shared global
data and to run probe/config only once for the PHY package. There are
various example of this already in the kernel with the mscc, bcm54140
mediatek ge and micrle driver and they all follow the same pattern.
What is currently lacking is describing this in DT and better reference
the PHY in charge of global configuration of the PHY package. For the
already present PHY, the implementation is simple enough with only one
PHY having the required regs to apply global configuration.
This can be ok for simple PHY package but some Qcom PHY package on
""modern"" SoC have more complex implementation. One example is the PHY
for qca807x where some global regs are present in the so-called "combo"
port and everything about psgmii calibration is placed in a 5th port in
the PHY package.
Given these additional thing, the original phy_package API are extended
with support for multiple global PHY for configuration. Each PHY driver
will have an enum of the ID for the global PHY to reference and is
required to pass to the read/write function.
On top of this, it's added correct DT support for describing PHY
package.
One example is this:
ethernet-phy-package {
compatible = "ethernet-phy-package";
#address-cells = <1>;
#size-cells = <0>;
global-phys = <&phy4>;
global-phy-names = "base";
ethernet-phy@1 {
compatible = "ethernet-phy-ieee802.3-c22";
reg = <1>;
};
phy4: ethernet-phy@4 {
compatible = "ethernet-phy-ieee802.3-c22";
reg = <4>;
};
};
The mdio parse functions are changed to address for this additional
special node, the function is changed to simply detect this node and
search also in this.
If this is detected phy core will join each PHY present in the node and
use (if defined) the additional info in the PHY driver to probe/config
once the PHY package.
Some PHY package also supports a single phy-mode for every PHY in the
PHY package. This is also supported and can be described in DT by
defining the phy-mode in the PHY package node.
I hope this implementation is clean enough as I expect more and more of
these configuration to appear in the future.
Christian Marangi (12):
net: phy: extend PHY package API to support multiple global address
dt-bindings: net: move PHY modes to common PHY mode types definition
dt-bindings: net: document ethernet PHY package nodes
net: phy: add initial support for PHY package in DT
net: phy: add support for named global PHY in DT PHY package
net: phy: add support for shared priv data size for PHY package in DT
net: phy: add support for driver specific PHY package probe/config
net: phy: add support for PHY package interface mode
net: phy: move mmd_phy_indirect to generic header
net: phy: add support for PHY package MMD read/write
dt-bindings: net: Document Qcom QCA807x PHY package
net: phy: qca807x: Add support for configurable LED
Robert Marko (2):
dt-bindings: net: add QCA807x PHY defines
net: phy: add Qualcom QCA807x driver
.../bindings/net/ethernet-controller.yaml | 47 +-
.../bindings/net/ethernet-phy-mode-types.yaml | 132 ++
.../bindings/net/ethernet-phy-package.yaml | 86 ++
.../devicetree/bindings/net/qcom,qca807x.yaml | 143 ++
drivers/net/mdio/of_mdio.c | 60 +-
drivers/net/phy/Kconfig | 7 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/bcm54140.c | 23 +-
drivers/net/phy/mdio_bus.c | 33 +-
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-core.c | 14 -
drivers/net/phy/phy_device.c | 218 ++-
drivers/net/phy/qca807x.c | 1232 +++++++++++++++++
include/dt-bindings/net/qcom-qca807x.h | 45 +
include/linux/phy.h | 178 ++-
18 files changed, 2119 insertions(+), 147 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/ethernet-phy-mode-types.yaml
create mode 100644 Documentation/devicetree/bindings/net/ethernet-phy-package.yaml
create mode 100644 Documentation/devicetree/bindings/net/qcom,qca807x.yaml
create mode 100644 drivers/net/phy/qca807x.c
create mode 100644 include/dt-bindings/net/qcom-qca807x.h
--
2.40.1
Move PHY modes from ethernet-controller schema to dedicated common PHY
mode types definition. This is needed to have a centralized place to
define PHY interface mode and permit usage and reference of these modes
in other schemas.
Signed-off-by: Christian Marangi <[email protected]>
---
.../bindings/net/ethernet-controller.yaml | 47 +------
.../bindings/net/ethernet-phy-mode-types.yaml | 132 ++++++++++++++++++
2 files changed, 133 insertions(+), 46 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/ethernet-phy-mode-types.yaml
diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
index 9f6a5ccbcefe..40c1daff2a48 100644
--- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
@@ -55,55 +55,10 @@ properties:
const: mac-address
phy-connection-type:
+ $ref: /schemas/net/ethernet-phy-mode-types.yaml#definitions/phy-connection-type
description:
Specifies interface type between the Ethernet device and a physical
layer (PHY) device.
- enum:
- # There is not a standard bus between the MAC and the PHY,
- # something proprietary is being used to embed the PHY in the
- # MAC.
- - internal
- - mii
- - gmii
- - sgmii
- - psgmii
- - qsgmii
- - qusgmii
- - tbi
- - rev-mii
- - rmii
- - rev-rmii
- - moca
-
- # RX and TX delays are added by the MAC when required
- - rgmii
-
- # RGMII with internal RX and TX delays provided by the PHY,
- # the MAC should not add the RX or TX delays in this case
- - rgmii-id
-
- # RGMII with internal RX delay provided by the PHY, the MAC
- # should not add an RX delay in this case
- - rgmii-rxid
-
- # RGMII with internal TX delay provided by the PHY, the MAC
- # should not add an TX delay in this case
- - rgmii-txid
- - rtbi
- - smii
- - xgmii
- - trgmii
- - 1000base-x
- - 2500base-x
- - 5gbase-r
- - rxaui
- - xaui
-
- # 10GBASE-KR, XFI, SFI
- - 10gbase-kr
- - usxgmii
- - 10gbase-r
- - 25gbase-r
phy-mode:
$ref: "#/properties/phy-connection-type"
diff --git a/Documentation/devicetree/bindings/net/ethernet-phy-mode-types.yaml b/Documentation/devicetree/bindings/net/ethernet-phy-mode-types.yaml
new file mode 100644
index 000000000000..6d15743b4ffa
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ethernet-phy-mode-types.yaml
@@ -0,0 +1,132 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/ethernet-phy-mode-types.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ethernet PHY Common Mode Types
+
+maintainers:
+ - David S. Miller <[email protected]>
+
+definitions:
+ phy-connection-type:
+ # const: "internal"
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/string
+ - oneOf:
+ # There is not a standard bus between the MAC and the PHY,
+ # something proprietary is being used to embed the PHY in the
+ # MAC.
+ - items:
+ - type: string
+ const: internal
+ - items:
+ - type: string
+ const: mii
+ - items:
+ - type: string
+ const: gmii
+ - items:
+ - type: string
+ const: sgmii
+ - items:
+ - type: string
+ const: psgmii
+ - items:
+ - type: string
+ const: qsgmii
+ - items:
+ - type: string
+ const: qusgmii
+ - items:
+ - type: string
+ const: tbi
+ - items:
+ - type: string
+ const: rev-mii
+ - items:
+ - type: string
+ const: rmii
+ - items:
+ - type: string
+ const: rev-rmii
+ - items:
+ - type: string
+ const: moca
+
+ # RX and TX delays are added by the MAC when required
+ - items:
+ - type: string
+ const: rgmii
+
+ # RGMII with internal RX and TX delays provided by the PHY,
+ # the MAC should not add the RX or TX delays in this case
+ - items:
+ - type: string
+ const: rgmii-id
+
+ # RGMII with internal RX delay provided by the PHY, the MAC
+ # should not add an RX delay in this case
+ - items:
+ - type: string
+ const: rgmii-rxid
+
+ # RGMII with internal TX delay provided by the PHY, the MAC
+ # should not add an TX delay in this case
+ - items:
+ - type: string
+ const: rgmii-txid
+
+ - items:
+ - type: string
+ const: rtbi
+
+ - items:
+ - type: string
+ const: smii
+
+ - items:
+ - type: string
+ const: xgmii
+
+ - items:
+ - type: string
+ const: trgmii
+
+ - items:
+ - type: string
+ const: 1000base-x
+
+ - items:
+ - type: string
+ const: 2500base-x
+
+ - items:
+ - type: string
+ const: 5gbase-r
+
+ - items:
+ - type: string
+ const: rxaui
+
+ - items:
+ - type: string
+ const: xaui
+
+ # 10GBASE-KR, XFI, SFI
+ - items:
+ - type: string
+ const: 10gbase-kr
+
+ - items:
+ - type: string
+ const: usxgmii
+
+ - items:
+ - type: string
+ const: 10gbase-r
+
+ - items:
+ - type: string
+ const: 25gbase-r
--
2.40.1
Add support for defining shared data size for PHY package defined in DT.
A PHY driver has to define the value .phy_package_priv_data_size to make
the generic OF PHY package function alloc priv data in the shared struct
for the PHY package.
Signed-off-by: Christian Marangi <[email protected]>
---
drivers/net/phy/phy_device.c | 7 ++++++-
include/linux/phy.h | 3 +++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 452fd69e8924..91d17129b774 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -3200,6 +3200,7 @@ static int of_phy_package(struct phy_device *phydev)
struct device_node *package_node;
const char *global_phy_name;
int i, global_phys_num, ret;
+ int shared_priv_data_size;
int *global_phy_addrs;
if (!node)
@@ -3252,8 +3253,12 @@ static int of_phy_package(struct phy_device *phydev)
global_phy_addrs[i] = addr;
}
+ shared_priv_data_size = 0;
+ if (phydev->drv->phy_package_priv_data_size)
+ shared_priv_data_size = phydev->drv->phy_package_priv_data_size;
+
ret = devm_phy_package_join(&phydev->mdio.dev, phydev, global_phy_addrs,
- global_phys_num, 0);
+ global_phys_num, shared_priv_data_size);
if (ret)
goto exit;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 5e595a0a43b6..7c47c12cffa0 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -896,6 +896,8 @@ struct phy_led {
* for PHY package global init. If defined, list is compared
* with DT values to match correct PHY phandle order. List
* last element MUST BE an empty string.
+ * @phy_package_priv_data_size: Size of the priv data to alloc
+ * for shared struct of PHY package.
*
* All functions are optional. If config_aneg or read_status
* are not implemented, the phy core uses the genphy versions.
@@ -915,6 +917,7 @@ struct phy_driver {
const void *driver_data;
unsigned int phy_package_global_phy_num;
const char * const *phy_package_global_phy_names;
+ unsigned int phy_package_priv_data_size;
/**
* @soft_reset: Called to issue a PHY software reset
--
2.40.1
Move mmd_phy_indirect function from phy-core to generic phy.h to permit
future usage for PHY package read/write_mmd.
Signed-off-by: Christian Marangi <[email protected]>
---
drivers/net/phy/phy-core.c | 14 --------------
include/linux/phy.h | 14 ++++++++++++++
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 966c93cbe616..b4f80847eefd 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -526,20 +526,6 @@ int phy_speed_down_core(struct phy_device *phydev)
return 0;
}
-static void mmd_phy_indirect(struct mii_bus *bus, int phy_addr, int devad,
- u16 regnum)
-{
- /* Write the desired MMD Devad */
- __mdiobus_write(bus, phy_addr, MII_MMD_CTRL, devad);
-
- /* Write the desired MMD register address */
- __mdiobus_write(bus, phy_addr, MII_MMD_DATA, regnum);
-
- /* Select the Function : DATA with no post increment */
- __mdiobus_write(bus, phy_addr, MII_MMD_CTRL,
- devad | MII_MMD_CTRL_NOINCR);
-}
-
/**
* __phy_read_mmd - Convenience function for reading a register
* from an MMD on a given PHY.
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 8af0a8a72b88..dd2381652dd1 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1361,6 +1361,20 @@ static inline int __phy_modify_changed(struct phy_device *phydev, u32 regnum,
regnum, mask, set);
}
+static inline void mmd_phy_indirect(struct mii_bus *bus, int phy_addr, int devad,
+ u16 regnum)
+{
+ /* Write the desired MMD Devad */
+ __mdiobus_write(bus, phy_addr, MII_MMD_CTRL, devad);
+
+ /* Write the desired MMD register address */
+ __mdiobus_write(bus, phy_addr, MII_MMD_DATA, regnum);
+
+ /* Select the Function : DATA with no post increment */
+ __mdiobus_write(bus, phy_addr, MII_MMD_CTRL,
+ devad | MII_MMD_CTRL_NOINCR);
+}
+
/*
* phy_read_mmd - Convenience function for reading a register
* from an MMD on a given PHY.
--
2.40.1
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 <[email protected]>
---
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
Document ethernet PHY package nodes used to describe PHY shipped in
bundle of 4-5 PHY. These particular PHY require specific PHY in the
package for global onfiguration of the PHY package.
Example are PHY package that have some regs only in one PHY of the
package and will affect every other PHY in the package, for example
related to PHY interface mode calibration or global PHY mode selection.
The PHY package node should use the global-phys property and the
global-phy-names to define PHY in the package required by the PHY driver
for global configuration.
It's also possible to specify the property phy-mode to specify that the
PHY package sets a global PHY interface mode and every PHY of the
package requires to have the same PHY interface mode.
Signed-off-by: Christian Marangi <[email protected]>
---
.../bindings/net/ethernet-phy-package.yaml | 86 +++++++++++++++++++
1 file changed, 86 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/ethernet-phy-package.yaml
diff --git a/Documentation/devicetree/bindings/net/ethernet-phy-package.yaml b/Documentation/devicetree/bindings/net/ethernet-phy-package.yaml
new file mode 100644
index 000000000000..2aa109e155d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ethernet-phy-package.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/ethernet-phy-package.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ethernet PHY Package Common Properties
+
+maintainers:
+ - Christian Marangi <[email protected]
+
+properties:
+ $nodename:
+ pattern: "^ethernet-phy-package(-[0-9]+)?$"
+
+ compatible:
+ const: ethernet-phy-package
+
+ '#address-cells':
+ description: number of address cells for the MDIO bus
+ const: 1
+
+ '#size-cells':
+ description: number of size cells on the MDIO bus
+ const: 0
+
+ global-phys:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ minItems: 1
+ maxItems: 31
+ description:
+ List of phandle to the PHY in the package required and
+ used to configure the PHY package.
+
+ global-phy-names:
+ $ref: /schemas/types.yaml#/definitions/string-array
+ minItems: 1
+ maxItems: 31
+ description:
+ List of names of the PHY defined in global-phys.
+
+ phy-connection-type:
+ $ref: /schemas/net/ethernet-phy-mode-types.yaml#definitions/phy-connection-type
+ description:
+ Specifies global interface type for the PHY package.
+
+ phy-mode:
+ $ref: "#/properties/phy-connection-type"
+
+patternProperties:
+ ^ethernet-phy(@[a-f0-9]+)?$:
+ $ref: /schemas/net/ethernet-phy.yaml#
+
+required:
+ - compatible
+
+dependencies:
+ global-phy-names: [global-phys]
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ ethernet {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethernet-phy-package {
+ compatible = "ethernet-phy-package";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ global-phys = <&phy4>;
+ global-phy-names = "base";
+
+ ethernet-phy@1 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <1>;
+ };
+
+ phy4: ethernet-phy@4 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <4>;
+ };
+ };
+ };
--
2.40.1
Add initial support for PHY package in DT.
Make it easier to define PHY package and describe the global PHY
directly in DT by refereincing them by phandles instead of custom
functions in each PHY driver.
Each PHY in a package needs to be defined in a dedicated node in the
mdio node. This dedicated node needs to have the compatible set to
"ethernet-phy-package" and define "global-phys" and "#global-phy-cells"
respectively to a list of phandle to the global phy to define for the
PHY package and 0 for cells as the phandle won't take any args.
With this defined, the generic PHY probe will join each PHY in this
dedicated node to the package.
PHY driver MUST set the required global PHY count in
.phy_package_global_phy_num to correctly verify that DT define the
correct number of phandle to the required global PHY.
mdio_bus.c and of_mdio.c is updated to now support and parse also
PHY package subnote that have the compatible "phy-package".
Signed-off-by: Christian Marangi <[email protected]>
---
drivers/net/mdio/of_mdio.c | 60 ++++++++++++++++++++++-----------
drivers/net/phy/mdio_bus.c | 33 ++++++++++++++-----
drivers/net/phy/phy_device.c | 64 ++++++++++++++++++++++++++++++++++++
include/linux/phy.h | 5 +++
4 files changed, 135 insertions(+), 27 deletions(-)
diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index 64ebcb6d235c..bb910651118f 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -139,6 +139,44 @@ bool of_mdiobus_child_is_phy(struct device_node *child)
}
EXPORT_SYMBOL(of_mdiobus_child_is_phy);
+static int __of_mdiobus_parse_phys(struct mii_bus *mdio, struct device_node *np,
+ bool *scanphys)
+{
+ struct device_node *child;
+ int addr, rc;
+
+ /* Loop over the child nodes and register a phy_device for each phy */
+ for_each_available_child_of_node(np, child) {
+ if (of_device_is_compatible(child, "ethernet-phy-package")) {
+ rc = __of_mdiobus_parse_phys(mdio, child, scanphys);
+ if (rc && rc != -ENODEV)
+ return rc;
+
+ continue;
+ }
+
+ addr = of_mdio_parse_addr(&mdio->dev, child);
+ if (addr < 0) {
+ *scanphys = true;
+ continue;
+ }
+
+ if (of_mdiobus_child_is_phy(child))
+ rc = of_mdiobus_register_phy(mdio, child, addr);
+ else
+ rc = of_mdiobus_register_device(mdio, child, addr);
+
+ if (rc == -ENODEV)
+ dev_err(&mdio->dev,
+ "MDIO device at address %d is missing.\n",
+ addr);
+ else if (rc)
+ return rc;
+ }
+
+ return 0;
+}
+
/**
* __of_mdiobus_register - Register mii_bus and create PHYs from the device tree
* @mdio: pointer to mii_bus structure
@@ -180,25 +218,9 @@ int __of_mdiobus_register(struct mii_bus *mdio, struct device_node *np,
return rc;
/* Loop over the child nodes and register a phy_device for each phy */
- for_each_available_child_of_node(np, child) {
- addr = of_mdio_parse_addr(&mdio->dev, child);
- if (addr < 0) {
- scanphys = true;
- continue;
- }
-
- if (of_mdiobus_child_is_phy(child))
- rc = of_mdiobus_register_phy(mdio, child, addr);
- else
- rc = of_mdiobus_register_device(mdio, child, addr);
-
- if (rc == -ENODEV)
- dev_err(&mdio->dev,
- "MDIO device at address %d is missing.\n",
- addr);
- else if (rc)
- goto unregister;
- }
+ rc = __of_mdiobus_parse_phys(mdio, np, &scanphys);
+ if (rc)
+ goto unregister;
if (!scanphys)
return 0;
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 25dcaa49ab8b..ecec20fd3fd3 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -455,19 +455,23 @@ EXPORT_SYMBOL(of_mdio_find_bus);
* found, set the of_node pointer for the mdio device. This allows
* auto-probed phy devices to be supplied with information passed in
* via DT.
+ * If a PHY package is found, PHY is searched also there.
*/
-static void of_mdiobus_link_mdiodev(struct mii_bus *bus,
- struct mdio_device *mdiodev)
+static int of_mdiobus_find_phy(struct device *dev, struct mdio_device *mdiodev,
+ struct device_node *np)
{
- struct device *dev = &mdiodev->dev;
struct device_node *child;
- if (dev->of_node || !bus->dev.of_node)
- return;
-
- for_each_available_child_of_node(bus->dev.of_node, child) {
+ for_each_available_child_of_node(np, child) {
int addr;
+ if (of_device_is_compatible(child, "ethernet-phy-package")) {
+ if (!of_mdiobus_find_phy(dev, mdiodev, child))
+ return 0;
+
+ continue;
+ }
+
addr = of_mdio_parse_addr(dev, child);
if (addr < 0)
continue;
@@ -477,9 +481,22 @@ static void of_mdiobus_link_mdiodev(struct mii_bus *bus,
/* The refcount on "child" is passed to the mdio
* device. Do _not_ use of_node_put(child) here.
*/
- return;
+ return 0;
}
}
+
+ return -ENODEV;
+}
+
+static void of_mdiobus_link_mdiodev(struct mii_bus *bus,
+ struct mdio_device *mdiodev)
+{
+ struct device *dev = &mdiodev->dev;
+
+ if (dev->of_node || !bus->dev.of_node)
+ return;
+
+ of_mdiobus_find_phy(dev, mdiodev, bus->dev.of_node);
}
#else /* !IS_ENABLED(CONFIG_OF_MDIO) */
static inline void of_mdiobus_link_mdiodev(struct mii_bus *mdio,
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e016dbfb0d27..9ff76d84dca0 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -3193,6 +3193,65 @@ static int of_phy_leds(struct phy_device *phydev)
return 0;
}
+static int of_phy_package(struct phy_device *phydev)
+{
+ struct device_node *node = phydev->mdio.dev.of_node;
+ struct of_phandle_args phy_phandle;
+ struct device_node *package_node;
+ int i, global_phys_num, ret;
+ int *global_phy_addrs;
+
+ if (!node)
+ return 0;
+
+ package_node = of_get_parent(node);
+ if (!package_node)
+ return 0;
+
+ if (!of_device_is_compatible(package_node, "ethernet-phy-package"))
+ return 0;
+
+ ret = of_count_phandle_with_args(package_node, "global-phys", NULL);
+ if (ret < 0)
+ return 0;
+ global_phys_num = ret;
+
+ if (global_phys_num != phydev->drv->phy_package_global_phy_num)
+ return -EINVAL;
+
+ global_phy_addrs = kmalloc_array(global_phys_num, sizeof(*global_phy_addrs),
+ GFP_KERNEL);
+ if (!global_phy_addrs)
+ return -ENOMEM;
+
+ for (i = 0; i < global_phys_num; i++) {
+ int addr;
+
+ ret = of_parse_phandle_with_args(package_node, "global-phys",
+ NULL, i, &phy_phandle);
+ if (ret)
+ goto exit;
+
+ ret = of_property_read_u32(phy_phandle.np, "reg", &addr);
+ if (ret)
+ goto exit;
+
+ global_phy_addrs[i] = addr;
+ }
+
+ ret = devm_phy_package_join(&phydev->mdio.dev, phydev, global_phy_addrs,
+ global_phys_num, 0);
+ if (ret)
+ goto exit;
+
+ phydev->shared->np = package_node;
+
+exit:
+ kfree(global_phy_addrs);
+
+ return ret;
+}
+
/**
* fwnode_mdio_find_device - Given a fwnode, find the mdio_device
* @fwnode: pointer to the mdio_device's fwnode
@@ -3301,6 +3360,11 @@ static int phy_probe(struct device *dev)
if (phydrv->flags & PHY_IS_INTERNAL)
phydev->is_internal = true;
+ /* Parse DT to detect PHY package and join them */
+ err = of_phy_package(phydev);
+ if (err)
+ goto out;
+
/* Deassert the reset signal */
phy_device_reset(phydev, 0);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index c2bb3f0b9dda..5bf90c49e5bd 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -339,6 +339,8 @@ struct mdio_bus_stats {
* phy_package_leave().
*/
struct phy_package_shared {
+ /* With PHY package defined in DT this points to the PHY package node */
+ struct device_node *np;
/* addrs list pointer */
/* note that this pointer is shared between different phydevs.
* It is allocated and freed automatically by phy_package_join() and
@@ -888,6 +890,8 @@ struct phy_led {
* @flags: A bitfield defining certain other features this PHY
* supports (like interrupts)
* @driver_data: Static driver data
+ * @phy_package_global_phy_num: Num of the required global phy
+ * for PHY package global configuration.
*
* All functions are optional. If config_aneg or read_status
* are not implemented, the phy core uses the genphy versions.
@@ -905,6 +909,7 @@ struct phy_driver {
const unsigned long * const features;
u32 flags;
const void *driver_data;
+ unsigned int phy_package_global_phy_num;
/**
* @soft_reset: Called to issue a PHY software reset
--
2.40.1
Add PHY driver specific function to probe and configure PHY package.
These function are run only once before the PHY probe and config_init.
They are used in conjunction with DT PHY package define for basic PHY
package implementation to setup and probe PHY package with simple
functions directly defined in the PHY driver struct.
Signed-off-by: Christian Marangi <[email protected]>
---
drivers/net/phy/phy_device.c | 14 ++++++++++++++
include/linux/phy.h | 21 +++++++++++++++++++++
2 files changed, 35 insertions(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 91d17129b774..0b7ba6995929 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1246,6 +1246,13 @@ int phy_init_hw(struct phy_device *phydev)
if (ret < 0)
return ret;
+ if (phydev->drv->phy_package_config_init_once &&
+ phy_package_init_once(phydev)) {
+ ret = phydev->drv->phy_package_config_init_once(phydev);
+ if (ret < 0)
+ return ret;
+ }
+
if (phydev->drv->config_init) {
ret = phydev->drv->config_init(phydev);
if (ret < 0)
@@ -3386,6 +3393,13 @@ static int phy_probe(struct device *dev)
/* Deassert the reset signal */
phy_device_reset(phydev, 0);
+ if (phydev->drv->phy_package_probe_once &&
+ phy_package_probe_once(phydev)) {
+ err = phydev->drv->phy_package_probe_once(phydev);
+ if (err)
+ goto out;
+ }
+
if (phydev->drv->probe) {
err = phydev->drv->probe(phydev);
if (err)
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7c47c12cffa0..1849fc637196 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -924,12 +924,33 @@ struct phy_driver {
*/
int (*soft_reset)(struct phy_device *phydev);
+ /**
+ * @phy_package_config_init_once: Driver specific PHY package
+ * config init call
+ * @def: PHY device to use to probe PHY package
+ *
+ * Called to initialize the PHY package, including after
+ * a reset.
+ * Called BEFORE PHY config_init.
+ */
+ int (*phy_package_config_init_once)(struct phy_device *dev);
+
/**
* @config_init: Called to initialize the PHY,
* including after a reset
*/
int (*config_init)(struct phy_device *phydev);
+ /**
+ * @phy_package_probe_once: Driver specific PHY package probe
+ * @def: PHY device to use to probe PHY package
+ *
+ * Called during discovery once per PHY package. Used to set
+ * up device-specific PHY package structures, if any.
+ * Called BEFORE PHY probe.
+ */
+ int (*phy_package_probe_once)(struct phy_device *dev);
+
/**
* @probe: Called during discovery. Used to set
* up device-specific structures, if any
--
2.40.1
Some PHY in PHY package supports running only at a specific mode for
each PHY in the package. Add support for enforcing this special thing a
verify consistency with the requested mode to prevent misconfiguration.
To set the PHY package mode, simply set "phy-mode" in the phy-package
node. Each PHY on init will verify if the requested mode match the one
set for the PHY package and will return -EINVAL if this is not true.
If PHY package doesn't specify any mode, it's assumed that PHY in the
package doesn't have such limitation.
Signed-off-by: Christian Marangi <[email protected]>
---
drivers/net/phy/phy_device.c | 37 ++++++++++++++++++++++++++++++++++++
include/linux/phy.h | 6 ++++++
2 files changed, 43 insertions(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 0b7ba6995929..73af4197a7af 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -25,6 +25,7 @@
#include <linux/mm.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_net.h>
#include <linux/netdevice.h>
#include <linux/phy.h>
#include <linux/phylib_stubs.h>
@@ -1225,8 +1226,17 @@ static int phy_poll_reset(struct phy_device *phydev)
int phy_init_hw(struct phy_device *phydev)
{
+ phy_interface_t package_interface;
int ret = 0;
+ /* Validate we are requesting consistent mode if we
+ * are in a PHY package and the PHY package requires a
+ * specific mode.
+ */
+ ret = phy_package_get_mode(phydev, &package_interface);
+ if (!ret && phydev->interface != package_interface)
+ return -EINVAL;
+
/* Deassert the reset signal */
phy_device_reset(phydev, 0);
@@ -1776,6 +1786,32 @@ void phy_package_leave(struct phy_device *phydev)
}
EXPORT_SYMBOL_GPL(phy_package_leave);
+/**
+ * phy_package_get_mode - get PHY interface mode for PHY package
+ * @phydev: target phy_device struct
+ * @interface: phy_interface_t pointer where to save the PHY package mode
+ *
+ * Gets PHY interface mode for the shared data of the PHY package.
+ * Returns 0 and updates @interface with the PHY package value, or -ENODEV
+ * if PHY is not in PHY package or -EINVAL if a PHY package interface mode
+ * is not set.
+ */
+int phy_package_get_mode(struct phy_device *phydev, phy_interface_t *interface)
+{
+ struct phy_package_shared *shared = phydev->shared;
+
+ if (!shared)
+ return -ENODEV;
+
+ if (shared->package_interface == PHY_INTERFACE_MODE_NA)
+ return -EINVAL;
+
+ *interface = shared->package_interface;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(phy_package_get_mode);
+
static void devm_phy_package_leave(struct device *dev, void *res)
{
phy_package_leave(*(struct phy_device **)res);
@@ -3270,6 +3306,7 @@ static int of_phy_package(struct phy_device *phydev)
goto exit;
phydev->shared->np = package_node;
+ of_get_phy_mode(package_node, &phydev->shared->package_interface);
exit:
kfree(global_phy_addrs);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 1849fc637196..8af0a8a72b88 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -341,6 +341,11 @@ struct mdio_bus_stats {
struct phy_package_shared {
/* With PHY package defined in DT this points to the PHY package node */
struct device_node *np;
+ /* PHY package interface
+ * If defined, each PHY of the package MUST have the interface
+ * set to the PHY package.
+ */
+ phy_interface_t package_interface;
/* addrs list pointer */
/* note that this pointer is shared between different phydevs.
* It is allocated and freed automatically by phy_package_join() and
@@ -2014,6 +2019,7 @@ int phy_ethtool_nway_reset(struct net_device *ndev);
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 phy_package_get_mode(struct phy_device *phydev, phy_interface_t *interface);
int devm_phy_package_join(struct device *dev, struct phy_device *phydev,
int *addrs, size_t addrs_num, size_t priv_size);
--
2.40.1
From: Robert Marko <[email protected]>
Add DT bindings defined for Qualcomm QCA807x PHY series related to
calibration and DAC settings.
Signed-off-by: Robert Marko <[email protected]>
Signed-off-by: Christian Marangi <[email protected]>
---
include/dt-bindings/net/qcom-qca807x.h | 45 ++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
create mode 100644 include/dt-bindings/net/qcom-qca807x.h
diff --git a/include/dt-bindings/net/qcom-qca807x.h b/include/dt-bindings/net/qcom-qca807x.h
new file mode 100644
index 000000000000..42c45c7d5210
--- /dev/null
+++ b/include/dt-bindings/net/qcom-qca807x.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR MIT */
+/*
+ * Device Tree constants for the Qualcomm QCA807X PHYs
+ */
+
+#ifndef _DT_BINDINGS_QCOM_QCA807X_H
+#define _DT_BINDINGS_QCOM_QCA807X_H
+
+#define PSGMII_QSGMII_TX_DRIVER_140MV 0
+#define PSGMII_QSGMII_TX_DRIVER_160MV 1
+#define PSGMII_QSGMII_TX_DRIVER_180MV 2
+#define PSGMII_QSGMII_TX_DRIVER_200MV 3
+#define PSGMII_QSGMII_TX_DRIVER_220MV 4
+#define PSGMII_QSGMII_TX_DRIVER_240MV 5
+#define PSGMII_QSGMII_TX_DRIVER_260MV 6
+#define PSGMII_QSGMII_TX_DRIVER_280MV 7
+#define PSGMII_QSGMII_TX_DRIVER_300MV 8
+#define PSGMII_QSGMII_TX_DRIVER_320MV 9
+#define PSGMII_QSGMII_TX_DRIVER_400MV 10
+#define PSGMII_QSGMII_TX_DRIVER_500MV 11
+/* Default value */
+#define PSGMII_QSGMII_TX_DRIVER_600MV 12
+
+/* Full amplitude, full bias current */
+#define QCA807X_CONTROL_DAC_FULL_VOLT_BIAS 0
+/* Amplitude follow DSP (amplitude is adjusted based on cable length), half bias current */
+#define QCA807X_CONTROL_DAC_DSP_VOLT_HALF_BIAS 1
+/* Full amplitude, bias current follow DSP (bias current is adjusted based on cable length) */
+#define QCA807X_CONTROL_DAC_FULL_VOLT_DSP_BIAS 2
+/* Both amplitude and bias current follow DSP */
+#define QCA807X_CONTROL_DAC_DSP_VOLT_BIAS 3
+/* Full amplitude, half bias current */
+#define QCA807X_CONTROL_DAC_FULL_VOLT_HALF_BIAS 4
+/* Amplitude follow DSP setting; 1/4 bias current when cable<10m,
+ * otherwise half bias current
+ */
+#define QCA807X_CONTROL_DAC_DSP_VOLT_QUARTER_BIAS 5
+/* Full amplitude; same bias current setting with “010” and “011”,
+ * but half more bias is reduced when cable <10m
+ */
+#define QCA807X_CONTROL_DAC_FULL_VOLT_HALF_BIAS_SHORT 6
+/* Amplitude follow DSP; same bias current setting with “110”, default value */
+#define QCA807X_CONTROL_DAC_DSP_VOLT_HALF_BIAS_SHORT 7
+
+#endif
--
2.40.1
Some PHY in PHY package may require to read/write MMD regs to correctly
configure the PHY package.
Add support for these additional required function in both lock and no
lock variant.
Signed-off-by: Christian Marangi <[email protected]>
---
include/linux/phy.h | 72 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 72 insertions(+)
diff --git a/include/linux/phy.h b/include/linux/phy.h
index dd2381652dd1..90bfaa36689f 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -2110,6 +2110,78 @@ static inline int __phy_package_write(struct phy_device *phydev,
return __mdiobus_write(phydev->mdio.bus, addr, regnum, val);
}
+static inline int phy_package_read_mmd(struct phy_device *phydev,
+ int global_phy_index, int devad, u32 regnum)
+{
+ struct phy_package_shared *shared = phydev->shared;
+ struct mii_bus *bus = phydev->mdio.bus;
+ int addr, val;
+
+ if (!shared || global_phy_index > shared->addrs_num - 1)
+ return -EIO;
+
+ addr = shared->addrs[global_phy_index];
+
+ phy_lock_mdio_bus(phydev);
+ mmd_phy_indirect(bus, addr, devad, regnum);
+ val = __mdiobus_read(bus, addr, MII_MMD_DATA);
+ phy_unlock_mdio_bus(phydev);
+
+ return val;
+}
+
+static inline int __phy_package_read_mmd(struct phy_device *phydev,
+ int global_phy_index, int devad, u32 regnum)
+{
+ struct phy_package_shared *shared = phydev->shared;
+ struct mii_bus *bus = phydev->mdio.bus;
+ int addr;
+
+ if (!shared || global_phy_index > shared->addrs_num - 1)
+ return -EIO;
+
+ addr = shared->addrs[global_phy_index];
+ mmd_phy_indirect(bus, addr, devad, regnum);
+ return __mdiobus_read(bus, addr, MII_MMD_DATA);
+}
+
+static inline int phy_package_write_mmd(struct phy_device *phydev,
+ int global_phy_index, int devad,
+ u32 regnum, u16 val)
+{
+ struct phy_package_shared *shared = phydev->shared;
+ struct mii_bus *bus = phydev->mdio.bus;
+ int addr, ret;
+
+ if (!shared || global_phy_index > shared->addrs_num - 1)
+ return -EIO;
+
+ addr = shared->addrs[global_phy_index];
+
+ phy_lock_mdio_bus(phydev);
+ mmd_phy_indirect(bus, addr, devad, regnum);
+ ret = __mdiobus_write(bus, addr, MII_MMD_DATA, val);
+ phy_unlock_mdio_bus(phydev);
+
+ return ret;
+}
+
+static inline int __phy_package_write_mmd(struct phy_device *phydev,
+ int global_phy_index, int devad,
+ u32 regnum, u16 val)
+{
+ struct phy_package_shared *shared = phydev->shared;
+ struct mii_bus *bus = phydev->mdio.bus;
+ int addr;
+
+ if (!shared || global_phy_index > shared->addrs_num - 1)
+ return -EIO;
+
+ addr = shared->addrs[global_phy_index];
+ mmd_phy_indirect(bus, addr, devad, regnum);
+ return __mdiobus_write(bus, addr, MII_MMD_DATA, val);
+}
+
static inline bool __phy_package_set_once(struct phy_device *phydev,
unsigned int b)
{
--
2.40.1
Document Qcom QCA807x PHY package.
Qualcomm QCA807X Ethernet PHY is PHY package of 2 or 5
IEEE 802.3 clause 22 compliant 10BASE-Te, 100BASE-TX and
1000BASE-T PHY-s.
Document the required property to make the PHY package correctly
configure and work.
Signed-off-by: Christian Marangi <[email protected]>
---
.../devicetree/bindings/net/qcom,qca807x.yaml | 143 ++++++++++++++++++
1 file changed, 143 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/qcom,qca807x.yaml
diff --git a/Documentation/devicetree/bindings/net/qcom,qca807x.yaml b/Documentation/devicetree/bindings/net/qcom,qca807x.yaml
new file mode 100644
index 000000000000..c2d59068d015
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/qcom,qca807x.yaml
@@ -0,0 +1,143 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/qcom,qca807x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm QCA807X Ethernet PHY
+
+maintainers:
+ - Christian Marangi <[email protected]>
+ - Robert Marko <[email protected]>
+
+description: |
+ Qualcomm QCA807X Ethernet PHY is PHY package of 2 or 5
+ 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 4 digitally controlled
+ output only pins that natively drive LED-s for up to 2 attached
+ LEDs. Some vendor also use these 4 output for GPIO usage without
+ attaching LEDs.
+
+ Note that output pins can be set to drive LEDs OR GPIO, mixed
+ definition are not accepted.
+
+allOf:
+ - $ref: ethernet-phy-package.yaml#
+
+select:
+ properties:
+ compatible:
+ contains:
+ const: ethernet-phy-package
+ patternProperties:
+ ^ethernet-phy(@[a-f0-9]+)?$:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - ethernet-phy-id004d.d0b2
+ - ethernet-phy-id004d.d0b1
+ required:
+ - compatible
+
+properties:
+ global-phys:
+ maxItems: 2
+
+ global-phy-names:
+ items:
+ - const: combo
+ - const: pqsgmii
+
+ phy-connection-type:
+ enum:
+ - qsgmii
+ - psgmii
+
+patternProperties:
+ ^ethernet-phy(@[a-f0-9]+)?$:
+ $ref: /schemas/net/ethernet-phy.yaml#
+
+ properties:
+ gpio-controller:
+ description: set the output lines as GPIO instead of LEDs
+ type: boolean
+
+ '#gpio-cells':
+ description: number of GPIO cells for the PHY
+ const: 2
+
+ dependencies:
+ gpio-controller: ['#gpio-cells']
+
+ unevaluatedProperties: false
+
+required:
+ - compatible
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/leds/common.h>
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethernet-phy-package {
+ compatible = "ethernet-phy-package";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ global-phys = <&qca8075_4>, <&qca8075_psgmii>;
+ global-phy-names = "combo", "pqsgmii";
+
+ qca8075_1: ethernet-phy@1 {
+ compatible = "ethernet-phy-id004d.d0b2";
+ reg = <1>;
+
+ leds {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led@0 {
+ reg = <0>;
+ color = <LED_COLOR_ID_GREEN>;
+ function = LED_FUNCTION_LAN;
+ default-state = "keep";
+ };
+ };
+ };
+
+ qca8075_2: ethernet-phy@2 {
+ compatible = "ethernet-phy-id004d.d0b2";
+ reg = <2>;
+
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+
+ qca8075_3: ethernet-phy@3 {
+ compatible = "ethernet-phy-id004d.d0b2";
+ reg = <3>;
+ };
+
+ qca8075_4: ethernet-phy@4 {
+ compatible = "ethernet-phy-id004d.d0b2";
+ reg = <4>;
+ };
+
+ qca8075_pqsgmii: ethernet-phy@5 {
+ reg = <5>;
+ };
+ };
+ };
--
2.40.1
Add support for named global PHY in DT PHY package definition.
A phy-package node can provide the property "global-phy-names" to
provide named global PHY to correctly reference the phandle defined in
the global-phys list.
Each PHY driver has to define the .phy_package_global_phy_names list
to verify correct order of named PHY phandle defined in DT with the
expected order in the PHY driver.
If the list is not defined or "global-phy-names" is not defined in
DT, global phy addresses are insered in order defined in the "global-phys"
property.
Signed-off-by: Christian Marangi <[email protected]>
---
drivers/net/phy/phy_device.c | 13 +++++++++++++
include/linux/phy.h | 6 +++++-
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 9ff76d84dca0..452fd69e8924 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -3198,6 +3198,7 @@ static int of_phy_package(struct phy_device *phydev)
struct device_node *node = phydev->mdio.dev.of_node;
struct of_phandle_args phy_phandle;
struct device_node *package_node;
+ const char *global_phy_name;
int i, global_phys_num, ret;
int *global_phy_addrs;
@@ -3236,6 +3237,18 @@ static int of_phy_package(struct phy_device *phydev)
if (ret)
goto exit;
+ ret = of_property_read_string_index(package_node, "global-phy-names",
+ i, &global_phy_name);
+ if (!ret && phydev->drv->phy_package_global_phy_names) {
+ const char *name;
+
+ name = phydev->drv->phy_package_global_phy_names[i];
+ if (strcmp(global_phy_name, name)) {
+ ret = -EINVAL;
+ goto exit;
+ }
+ }
+
global_phy_addrs[i] = addr;
}
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 5bf90c49e5bd..5e595a0a43b6 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -892,6 +892,10 @@ struct phy_led {
* @driver_data: Static driver data
* @phy_package_global_phy_num: Num of the required global phy
* for PHY package global configuration.
+ * @phy_package_global_phy_names: List of global PHY names used
+ * for PHY package global init. If defined, list is compared
+ * with DT values to match correct PHY phandle order. List
+ * last element MUST BE an empty string.
*
* All functions are optional. If config_aneg or read_status
* are not implemented, the phy core uses the genphy versions.
@@ -910,6 +914,7 @@ struct phy_driver {
u32 flags;
const void *driver_data;
unsigned int phy_package_global_phy_num;
+ const char * const *phy_package_global_phy_names;
/**
* @soft_reset: Called to issue a PHY software reset
@@ -1154,7 +1159,6 @@ struct phy_driver {
*/
int (*led_hw_control_get)(struct phy_device *dev, u8 index,
unsigned long *rules);
-
};
#define to_phy_driver(d) container_of(to_mdio_common_driver(d), \
struct phy_driver, mdiodrv)
--
2.40.1
QCA8072/5 have up to 2 LEDs attached for PHY.
LEDs can be configured to be ON/hw blink or be set to HW control.
Hw blink mode is set to blink at 4Hz or 250ms.
PHY can support both copper (TP) or fiber (FIBRE) kind and supports
different HW control modes based on the port type.
HW control modes supported for netdev trigger for copper ports are:
- LINK_10
- LINK_100
- LINK_1000
- TX
- RX
- FULL_DUPLEX
- HALF_DUPLEX
HW control modes supported for netdev trigger for fiber ports are:
- LINK_100
- LINK_1000
- TX
- RX
- FULL_DUPLEX
- HALF_DUPLEX
LED support conflicts with GPIO controller feature and must be disabled
if gpio-controller is used for the PHY.
Signed-off-by: Christian Marangi <[email protected]>
---
drivers/net/phy/qca807x.c | 382 +++++++++++++++++++++++++++++++++++++-
1 file changed, 375 insertions(+), 7 deletions(-)
diff --git a/drivers/net/phy/qca807x.c b/drivers/net/phy/qca807x.c
index 5b82af52778a..a0daf65d872d 100644
--- a/drivers/net/phy/qca807x.c
+++ b/drivers/net/phy/qca807x.c
@@ -80,17 +80,60 @@
#define QCA807X_MMD7_1000BASE_T_POWER_SAVE_PER_CABLE_LENGTH 0x801a
#define QCA807X_CONTROL_DAC_MASK GENMASK(2, 0)
+#define QCA807X_MMD7_LED_GLOBAL 0x8073
+#define QCA807X_LED_BLINK_1 GENMASK(11, 6)
+#define QCA807X_LED_BLINK_2 GENMASK(5, 0)
+/* Values are the same for both BLINK_1 and BLINK_2 */
+#define QCA807X_LED_BLINK_FREQ_MASK GENMASK(5, 3)
+#define QCA807X_LED_BLINK_FREQ_2HZ FIELD_PREP(QCA807X_LED_BLINK_FREQ_MASK, 0x0)
+#define QCA807X_LED_BLINK_FREQ_4HZ FIELD_PREP(QCA807X_LED_BLINK_FREQ_MASK, 0x1)
+#define QCA807X_LED_BLINK_FREQ_8HZ FIELD_PREP(QCA807X_LED_BLINK_FREQ_MASK, 0x2)
+#define QCA807X_LED_BLINK_FREQ_16HZ FIELD_PREP(QCA807X_LED_BLINK_FREQ_MASK, 0x3)
+#define QCA807X_LED_BLINK_FREQ_32HZ FIELD_PREP(QCA807X_LED_BLINK_FREQ_MASK, 0x4)
+#define QCA807X_LED_BLINK_FREQ_64HZ FIELD_PREP(QCA807X_LED_BLINK_FREQ_MASK, 0x5)
+#define QCA807X_LED_BLINK_FREQ_128HZ FIELD_PREP(QCA807X_LED_BLINK_FREQ_MASK, 0x6)
+#define QCA807X_LED_BLINK_FREQ_256HZ FIELD_PREP(QCA807X_LED_BLINK_FREQ_MASK, 0x7)
+#define QCA807X_LED_BLINK_DUTY_MASK GENMASK(2, 0)
+#define QCA807X_LED_BLINK_DUTY_50_50 FIELD_PREP(QCA807X_LED_BLINK_DUTY_MASK, 0x0)
+#define QCA807X_LED_BLINK_DUTY_75_25 FIELD_PREP(QCA807X_LED_BLINK_DUTY_MASK, 0x1)
+#define QCA807X_LED_BLINK_DUTY_25_75 FIELD_PREP(QCA807X_LED_BLINK_DUTY_MASK, 0x2)
+#define QCA807X_LED_BLINK_DUTY_33_67 FIELD_PREP(QCA807X_LED_BLINK_DUTY_MASK, 0x3)
+#define QCA807X_LED_BLINK_DUTY_67_33 FIELD_PREP(QCA807X_LED_BLINK_DUTY_MASK, 0x4)
+#define QCA807X_LED_BLINK_DUTY_17_83 FIELD_PREP(QCA807X_LED_BLINK_DUTY_MASK, 0x5)
+#define QCA807X_LED_BLINK_DUTY_83_17 FIELD_PREP(QCA807X_LED_BLINK_DUTY_MASK, 0x6)
+#define QCA807X_LED_BLINK_DUTY_8_92 FIELD_PREP(QCA807X_LED_BLINK_DUTY_MASK, 0x7)
#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_LED_TXACT_BLK_EN_2 BIT(10)
-#define QCA807X_LED_RXACT_BLK_EN_2 BIT(9)
-#define QCA807X_LED_GT_ON_EN_2 BIT(6)
-#define QCA807X_LED_HT_ON_EN_2 BIT(5)
-#define QCA807X_LED_BT_ON_EN_2 BIT(4)
-#define QCA807X_GPIO_FORCE_EN BIT(15)
-#define QCA807X_GPIO_FORCE_MODE_MASK GENMASK(14, 13)
+/* Values are the same for LED1 and LED2 */
+/* Values for control 1 */
+#define QCA807X_LED_COPPER_ON_BLINK_MASK GENMASK(12, 0)
+#define QCA807X_LED_FDX_ON_EN BIT(12)
+#define QCA807X_LED_HDX_ON_EN BIT(11)
+#define QCA807X_LED_TXACT_BLK_EN BIT(10)
+#define QCA807X_LED_RXACT_BLK_EN BIT(9)
+#define QCA807X_LED_GT_ON_EN BIT(6)
+#define QCA807X_LED_HT_ON_EN BIT(5)
+#define QCA807X_LED_BT_ON_EN BIT(4)
+/* Values for control 2 */
+#define QCA807X_LED_FORCE_EN BIT(15)
+#define QCA807X_LED_FORCE_MODE_MASK GENMASK(14, 13)
+#define QCA807X_LED_FORCE_BLINK_1 FIELD_PREP(QCA807X_LED_FORCE_MODE_MASK, 0x3)
+#define QCA807X_LED_FORCE_BLINK_2 FIELD_PREP(QCA807X_LED_FORCE_MODE_MASK, 0x2)
+#define QCA807X_LED_FORCE_ON FIELD_PREP(QCA807X_LED_FORCE_MODE_MASK, 0x1)
+#define QCA807X_LED_FORCE_OFF FIELD_PREP(QCA807X_LED_FORCE_MODE_MASK, 0x0)
+#define QCA807X_LED_FIBER_ON_BLINK_MASK GENMASK(11, 1)
+#define QCA807X_LED_FIBER_TXACT_BLK_EN BIT(10)
+#define QCA807X_LED_FIBER_RXACT_BLK_EN BIT(9)
+#define QCA807X_LED_FIBER_FDX_ON_EN BIT(6)
+#define QCA807X_LED_FIBER_HDX_ON_EN BIT(5)
+#define QCA807X_LED_FIBER_1000BX_ON_EN BIT(2)
+#define QCA807X_LED_FIBER_100FX_ON_EN BIT(1)
+
+/* Some device repurpose the LED as GPIO out */
+#define QCA807X_GPIO_FORCE_EN QCA807X_LED_FORCE_EN
+#define QCA807X_GPIO_FORCE_MODE_MASK QCA807X_LED_FORCE_MODE_MASK
#define QCA807X_INTR_ENABLE 0x12
#define QCA807X_INTR_STATUS 0x13
@@ -338,6 +381,320 @@ static int qca807x_cable_test_start(struct phy_device *phydev)
return ret;
}
+static int qca807x_led_parse_netdev(struct phy_device *phydev, unsigned long rules,
+ u16 *offload_trigger)
+{
+ /* Parsing specific to netdev trigger */
+ switch (phydev->port) {
+ case PORT_TP:
+ if (test_bit(TRIGGER_NETDEV_TX, &rules))
+ *offload_trigger |= QCA807X_LED_TXACT_BLK_EN;
+ if (test_bit(TRIGGER_NETDEV_RX, &rules))
+ *offload_trigger |= QCA807X_LED_RXACT_BLK_EN;
+ if (test_bit(TRIGGER_NETDEV_LINK_10, &rules))
+ *offload_trigger |= QCA807X_LED_BT_ON_EN;
+ if (test_bit(TRIGGER_NETDEV_LINK_100, &rules))
+ *offload_trigger |= QCA807X_LED_HT_ON_EN;
+ if (test_bit(TRIGGER_NETDEV_LINK_1000, &rules))
+ *offload_trigger |= QCA807X_LED_GT_ON_EN;
+ if (test_bit(TRIGGER_NETDEV_HALF_DUPLEX, &rules))
+ *offload_trigger |= QCA807X_LED_HDX_ON_EN;
+ if (test_bit(TRIGGER_NETDEV_FULL_DUPLEX, &rules))
+ *offload_trigger |= QCA807X_LED_FDX_ON_EN;
+ break;
+ case PORT_FIBRE:
+ if (test_bit(TRIGGER_NETDEV_TX, &rules))
+ *offload_trigger |= QCA807X_LED_FIBER_TXACT_BLK_EN;
+ if (test_bit(TRIGGER_NETDEV_RX, &rules))
+ *offload_trigger |= QCA807X_LED_FIBER_RXACT_BLK_EN;
+ if (test_bit(TRIGGER_NETDEV_LINK_100, &rules))
+ *offload_trigger |= QCA807X_LED_FIBER_100FX_ON_EN;
+ if (test_bit(TRIGGER_NETDEV_LINK_1000, &rules))
+ *offload_trigger |= QCA807X_LED_FIBER_1000BX_ON_EN;
+ if (test_bit(TRIGGER_NETDEV_HALF_DUPLEX, &rules))
+ *offload_trigger |= QCA807X_LED_FIBER_HDX_ON_EN;
+ if (test_bit(TRIGGER_NETDEV_FULL_DUPLEX, &rules))
+ *offload_trigger |= QCA807X_LED_FIBER_FDX_ON_EN;
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ if (rules && !*offload_trigger)
+ return -EOPNOTSUPP;
+
+ return 0;
+}
+
+static int qca807x_led_hw_control_enable(struct phy_device *phydev, u8 index)
+{
+ int val, reg, ret;
+
+ switch (index) {
+ case 0:
+ reg = QCA807X_MMD7_LED_100N_2;
+ break;
+ case 1:
+ reg = QCA807X_MMD7_LED_1000N_2;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, reg);
+ val &= ~QCA807X_LED_FORCE_EN;
+ ret = phy_write_mmd(phydev, MDIO_MMD_AN, reg, val);
+
+ return ret;
+}
+
+static int qca807x_led_hw_is_supported(struct phy_device *phydev, u8 index,
+ unsigned long rules)
+{
+ u16 offload_trigger = 0;
+
+ if (index > 1)
+ return -EINVAL;
+
+ return qca807x_led_parse_netdev(phydev, rules, &offload_trigger);
+}
+
+static int qca807x_led_hw_control_set(struct phy_device *phydev, u8 index,
+ unsigned long rules)
+{
+ int val, ret, copper_reg, fibre_reg;
+ u16 offload_trigger = 0;
+
+ switch (index) {
+ case 0:
+ copper_reg = QCA807X_MMD7_LED_100N_1;
+ fibre_reg = QCA807X_MMD7_LED_100N_2;
+ break;
+ case 1:
+ copper_reg = QCA807X_MMD7_LED_1000N_1;
+ fibre_reg = QCA807X_MMD7_LED_1000N_2;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ret = qca807x_led_parse_netdev(phydev, rules, &offload_trigger);
+ if (ret)
+ return ret;
+
+ ret = qca807x_led_hw_control_enable(phydev, index);
+ if (ret)
+ return ret;
+
+ switch (phydev->port) {
+ case PORT_TP:
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, copper_reg);
+ val &= ~QCA807X_LED_COPPER_ON_BLINK_MASK;
+ val |= offload_trigger;
+ ret = phy_write_mmd(phydev, MDIO_MMD_AN, copper_reg, val);
+ break;
+ case PORT_FIBRE:
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, fibre_reg);
+ val &= ~QCA807X_LED_FIBER_ON_BLINK_MASK;
+ val |= offload_trigger;
+ ret = phy_write_mmd(phydev, MDIO_MMD_AN, fibre_reg, val);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
+static bool qca807x_led_hw_control_status(struct phy_device *phydev, u8 index)
+{
+ int val, reg;
+
+ switch (index) {
+ case 0:
+ reg = QCA807X_MMD7_LED_100N_2;
+ break;
+ case 1:
+ reg = QCA807X_MMD7_LED_1000N_2;
+ break;
+ default:
+ return false;
+ }
+
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, reg);
+
+ return !(val & QCA807X_LED_FORCE_EN);
+}
+
+static int qca807x_led_hw_control_get(struct phy_device *phydev, u8 index,
+ unsigned long *rules)
+{
+ int val, copper_reg, fibre_reg;
+
+ switch (index) {
+ case 0:
+ copper_reg = QCA807X_MMD7_LED_100N_1;
+ fibre_reg = QCA807X_MMD7_LED_100N_2;
+ break;
+ case 1:
+ copper_reg = QCA807X_MMD7_LED_1000N_1;
+ fibre_reg = QCA807X_MMD7_LED_100N_2;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* Check if we have hw control enabled */
+ if (qca807x_led_hw_control_status(phydev, index))
+ return -EINVAL;
+
+ /* Parsing specific to netdev trigger */
+ switch (phydev->port) {
+ case PORT_TP:
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, copper_reg);
+ if (val & QCA807X_LED_TXACT_BLK_EN)
+ set_bit(TRIGGER_NETDEV_TX, rules);
+ if (val & QCA807X_LED_RXACT_BLK_EN)
+ set_bit(TRIGGER_NETDEV_RX, rules);
+ if (val & QCA807X_LED_BT_ON_EN)
+ set_bit(TRIGGER_NETDEV_LINK_10, rules);
+ if (val & QCA807X_LED_HT_ON_EN)
+ set_bit(TRIGGER_NETDEV_LINK_100, rules);
+ if (val & QCA807X_LED_GT_ON_EN)
+ set_bit(TRIGGER_NETDEV_LINK_1000, rules);
+ if (val & QCA807X_LED_HDX_ON_EN)
+ set_bit(TRIGGER_NETDEV_HALF_DUPLEX, rules);
+ if (val & QCA807X_LED_FDX_ON_EN)
+ set_bit(TRIGGER_NETDEV_FULL_DUPLEX, rules);
+ break;
+ case PORT_FIBRE:
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, fibre_reg);
+ if (val & QCA807X_LED_FIBER_TXACT_BLK_EN)
+ set_bit(TRIGGER_NETDEV_TX, rules);
+ if (val & QCA807X_LED_FIBER_RXACT_BLK_EN)
+ set_bit(TRIGGER_NETDEV_RX, rules);
+ if (val & QCA807X_LED_FIBER_100FX_ON_EN)
+ set_bit(TRIGGER_NETDEV_LINK_100, rules);
+ if (val & QCA807X_LED_FIBER_1000BX_ON_EN)
+ set_bit(TRIGGER_NETDEV_LINK_1000, rules);
+ if (val & QCA807X_LED_FIBER_HDX_ON_EN)
+ set_bit(TRIGGER_NETDEV_HALF_DUPLEX, rules);
+ if (val & QCA807X_LED_FIBER_FDX_ON_EN)
+ set_bit(TRIGGER_NETDEV_FULL_DUPLEX, rules);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int qca807x_led_hw_control_reset(struct phy_device *phydev, u8 index)
+{
+ int val, copper_reg, fibre_reg, ret;
+
+ switch (index) {
+ case 0:
+ copper_reg = QCA807X_MMD7_LED_100N_1;
+ fibre_reg = QCA807X_MMD7_LED_100N_2;
+ break;
+ case 1:
+ copper_reg = QCA807X_MMD7_LED_1000N_1;
+ fibre_reg = QCA807X_MMD7_LED_100N_2;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (phydev->port) {
+ case PORT_TP:
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, copper_reg);
+ val &= ~QCA807X_LED_COPPER_ON_BLINK_MASK;
+ ret = phy_write_mmd(phydev, MDIO_MMD_AN, copper_reg, val);
+ break;
+ case PORT_FIBRE:
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, fibre_reg);
+ val &= ~QCA807X_LED_FIBER_ON_BLINK_MASK;
+ ret = phy_write_mmd(phydev, MDIO_MMD_AN, fibre_reg, val);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
+static int qca807x_led_brightness_set(struct phy_device *phydev,
+ u8 index, enum led_brightness value)
+{
+ int val, ret;
+ u16 reg;
+
+ switch (index) {
+ case 0:
+ reg = QCA807X_MMD7_LED_100N_2;
+ break;
+ case 1:
+ reg = QCA807X_MMD7_LED_1000N_2;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* If we are setting off the LED reset any hw control rule */
+ if (!value) {
+ ret = qca807x_led_hw_control_reset(phydev, index);
+ if (ret)
+ return ret;
+ }
+
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, reg);
+ val &= ~(QCA807X_LED_FORCE_EN | QCA807X_LED_FORCE_MODE_MASK);
+ val |= QCA807X_LED_FORCE_EN;
+ if (value)
+ val |= QCA807X_LED_FORCE_ON;
+ ret = phy_write_mmd(phydev, MDIO_MMD_AN, reg, val);
+
+ return ret;
+}
+
+static int qca807x_led_blink_set(struct phy_device *phydev, u8 index,
+ unsigned long *delay_on,
+ unsigned long *delay_off)
+{
+ int val, ret;
+ u16 reg;
+
+ switch (index) {
+ case 0:
+ reg = QCA807X_MMD7_LED_100N_2;
+ break;
+ case 1:
+ reg = QCA807X_MMD7_LED_1000N_2;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* Set blink to 50% off, 50% on at 4Hz by default */
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, QCA807X_MMD7_LED_GLOBAL);
+ val &= ~(QCA807X_LED_BLINK_FREQ_MASK | QCA807X_LED_BLINK_DUTY_MASK);
+ val |= QCA807X_LED_BLINK_FREQ_4HZ | QCA807X_LED_BLINK_DUTY_50_50;
+ ret = phy_write_mmd(phydev, MDIO_MMD_AN, QCA807X_MMD7_LED_GLOBAL, val);
+
+ /* We use BLINK_1 for normal blinking */
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, reg);
+ val &= ~(QCA807X_LED_FORCE_EN | QCA807X_LED_FORCE_MODE_MASK);
+ val |= QCA807X_LED_FORCE_EN | QCA807X_LED_FORCE_BLINK_1;
+ ret = phy_write_mmd(phydev, MDIO_MMD_AN, reg, val);
+
+ /* We set blink to 4Hz, aka 250ms */
+ *delay_on = 250 / 2;
+ *delay_off = 250 / 2;
+
+ return ret;
+}
+
#ifdef CONFIG_GPIOLIB
static int qca807x_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
{
@@ -716,6 +1073,12 @@ static int qca807x_probe(struct phy_device *phydev)
ret = qca807x_gpio(phydev);
if (ret)
return ret;
+
+ phydev->drv->led_brightness_set = NULL;
+ phydev->drv->led_blink_set = NULL;
+ phydev->drv->led_hw_is_supported = NULL;
+ phydev->drv->led_hw_control_set = NULL;
+ phydev->drv->led_hw_control_get = NULL;
}
}
@@ -843,6 +1206,11 @@ static struct phy_driver qca807x_drivers[] = {
.suspend = genphy_suspend,
.cable_test_start = qca807x_cable_test_start,
.cable_test_get_status = qca807x_cable_test_get_status,
+ .led_brightness_set = qca807x_led_brightness_set,
+ .led_blink_set = qca807x_led_blink_set,
+ .led_hw_is_supported = qca807x_led_hw_is_supported,
+ .led_hw_control_set = qca807x_led_hw_control_set,
+ .led_hw_control_get = qca807x_led_hw_control_get,
/* PHY package define */
.phy_package_global_phy_num = ARRAY_SIZE(qca807x_global_phy_names),
.phy_package_global_phy_names = qca807x_global_phy_names,
--
2.40.1
From: Robert Marko <[email protected]>
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 4 digitally controlled
output only pins that natively drive LED-s.
But some vendors used these to driver generic LED-s controlled
by userspace, so lets enable registering each PHY as GPIO
controller and add driver for it.
These are commonly used in Qualcomm IPQ40xx, IPQ60xx and IPQ807x
boards.
Co-developed-by: Christian Marangi <[email protected]>
Signed-off-by: Robert Marko <[email protected]>
Signed-off-by: Christian Marangi <[email protected]>
---
drivers/net/phy/Kconfig | 7 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/qca807x.c | 864 ++++++++++++++++++++++++++++++++++++++
3 files changed, 872 insertions(+)
create mode 100644 drivers/net/phy/qca807x.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 25cfc5ded1da..5ad85bd978a0 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -325,6 +325,13 @@ config AT803X_PHY
Currently supports the AR8030, AR8031, AR8033, AR8035 and internal
QCA8337(Internal qca8k PHY) model
+config QCA807X_PHY
+ tristate "Qualcomm QCA807x PHYs"
+ depends on OF_MDIO
+ help
+ Currently supports the Qualcomm QCA8072, QCA8075 and the PSGMII
+ control PHY.
+
config QSEMI_PHY
tristate "Quality Semiconductor PHYs"
help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index f65e85c91fc1..a4da4f127b23 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_NCN26000_PHY) += ncn26000.o
obj-$(CONFIG_NXP_C45_TJA11XX_PHY) += nxp-c45-tja11xx.o
obj-$(CONFIG_NXP_CBTX_PHY) += nxp-cbtx.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..5b82af52778a
--- /dev/null
+++ b/drivers/net/phy/qca807x.c
@@ -0,0 +1,864 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 Sartura Ltd.
+ *
+ * Author: Robert Marko <[email protected]>
+ * Christian Marangi <[email protected]>
+ *
+ * Qualcomm QCA8072 and QCA8075 PHY driver
+ */
+
+#include <linux/version.h>
+#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
+
+/* 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_BT_BX_REG_SEL_FIBER 0
+#define QCA807X_BT_BX_REG_SEL_COPPER 1
+#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_LED_TXACT_BLK_EN_2 BIT(10)
+#define QCA807X_LED_RXACT_BLK_EN_2 BIT(9)
+#define QCA807X_LED_GT_ON_EN_2 BIT(6)
+#define QCA807X_LED_HT_ON_EN_2 BIT(5)
+#define QCA807X_LED_BT_ON_EN_2 BIT(4)
+#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)
+
+/* PQSGMII Analog PHY specific */
+#define PQSGMII_CTRL_REG 0x0
+#define PQSGMII_ANALOG_SW_RESET BIT(6)
+#define PQSGMII_DRIVE_CONTROL_1 0xb
+#define PQSGMII_TX_DRIVER_MASK GENMASK(7, 4)
+#define PQSGMII_MODE_CTRL 0x6d
+#define PQSGMII_MODE_CTRL_AZ_WORKAROUND_MASK BIT(0)
+#define PQSGMII_MMD3_SERDES_CONTROL 0x805a
+
+#define SERDES_RESET_SLEEP 100
+
+enum qca807x_global_phy {
+ QCA807X_COMBO_ADDR = 0,
+ QCA807X_PQSGMII_ADDR,
+
+ __QCA807X_GLOBAL_PHY_MAX,
+};
+
+const char * const qca807x_global_phy_names[] = {
+ [QCA807X_COMBO_ADDR] = "combo",
+ [QCA807X_PQSGMII_ADDR] = "pqsgmii",
+};
+
+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;
+
+ 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 |= QCA807X_GPIO_FORCE_EN;
+ val |= FIELD_PREP(QCA807X_GPIO_FORCE_MODE_MASK, value);
+
+ phy_write_mmd(priv->phy, MDIO_MMD_AN, qca807x_gpio_get_reg(offset), val);
+}
+
+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)
+{
+ int ss, err, old_link = phydev->link;
+
+ /* 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)
+{
+ int ss, err, lpa, old_link = phydev->link;
+
+ /* 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;
+
+ if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete) {
+ lpa = phy_read(phydev, MII_LPA);
+ if (lpa < 0)
+ return lpa;
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
+ phydev->lp_advertising, lpa & LPA_LPACK);
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
+ phydev->lp_advertising, lpa & LPA_1000XFULL);
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_Pause_BIT,
+ phydev->lp_advertising, lpa & LPA_1000XPAUSE);
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+ phydev->lp_advertising,
+ lpa & LPA_1000XPAUSE_ASYM);
+
+ phy_resolve_aneg_linkmode(phydev);
+ }
+
+ /* 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)
+{
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, phydev->supported)) {
+ switch (phydev->port) {
+ case PORT_FIBRE:
+ return qca807x_read_fiber_status(phydev);
+ case PORT_TP:
+ return qca807x_read_copper_status(phydev);
+ default:
+ return -EINVAL;
+ }
+ }
+
+ return qca807x_read_copper_status(phydev);
+}
+
+static int qca807x_config_intr(struct phy_device *phydev)
+{
+ int ret, val;
+
+ val = phy_read(phydev, QCA807X_INTR_ENABLE);
+
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+ /* 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;
+ }
+ ret = phy_write(phydev, QCA807X_INTR_ENABLE, val);
+ } else {
+ ret = phy_write(phydev, QCA807X_INTR_ENABLE, 0);
+ }
+
+ 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;
+ int ret;
+ DECLARE_PHY_INTERFACE_MASK(interfaces);
+
+ sfp_parse_support(phydev->sfp_bus, id, support, interfaces);
+ iface = sfp_select_interface(phydev->sfp_bus, support);
+
+ dev_info(&phydev->mdio.dev, "%s SFP module inserted\n", phy_modes(iface));
+
+ switch (iface) {
+ case PHY_INTERFACE_MODE_1000BASEX:
+ case PHY_INTERFACE_MODE_100BASEX:
+ /* Set PHY mode to PSGMII combo (1/4 copper + combo ports) mode */
+ ret = phy_modify(phydev,
+ QCA807X_CHIP_CONFIGURATION,
+ QCA807X_CHIP_CONFIGURATION_MODE_CFG_MASK,
+ QCA807X_CHIP_CONFIGURATION_MODE_PSGMII_FIBER);
+ /* Enable fiber mode autodection (1000Base-X or 100Base-FX) */
+ ret = phy_set_bits_mmd(phydev,
+ MDIO_MMD_AN,
+ QCA807X_MMD7_FIBER_MODE_AUTO_DETECTION,
+ QCA807X_MMD7_FIBER_MODE_AUTO_DETECTION_EN);
+ /* Select fiber page */
+ ret = phy_clear_bits(phydev,
+ QCA807X_CHIP_CONFIGURATION,
+ QCA807X_BT_BX_REG_SEL);
+
+ phydev->port = PORT_FIBRE;
+ break;
+ default:
+ dev_err(&phydev->mdio.dev, "Incompatible SFP module inserted\n");
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
+static void qca807x_sfp_remove(void *upstream)
+{
+ struct phy_device *phydev = upstream;
+
+ /* Select copper page */
+ phy_set_bits(phydev,
+ QCA807X_CHIP_CONFIGURATION,
+ QCA807X_BT_BX_REG_SEL);
+
+ phydev->port = PORT_TP;
+}
+
+static const struct sfp_upstream_ops qca807x_sfp_ops = {
+ .attach = phy_sfp_attach,
+ .detach = phy_sfp_detach,
+ .module_insert = qca807x_sfp_insert,
+ .module_remove = qca807x_sfp_remove,
+};
+
+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;
+
+ if (of_property_read_u32(node, "qcom,control-dac", &of_control_dac))
+ of_control_dac = QCA807X_CONTROL_DAC_DSP_VOLT_QUARTER_BIAS;
+
+ 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)) {
+ if (of_find_property(node, "leds") &&
+ of_find_property(node, "gpio-controller")) {
+ phydev_err("Invalid property detected. LEDs and gpio-controller are mutually exclusive.");
+ return -EINVAL;
+ }
+
+ /* Do not register a GPIO controller unless flagged for it */
+ if (of_property_read_bool(node, "gpio-controller")) {
+ ret = qca807x_gpio(phydev);
+ if (ret)
+ return ret;
+ }
+ }
+
+ /* Attach SFP bus on combo port*/
+ if (phy_read(phydev, QCA807X_CHIP_CONFIGURATION)) {
+ ret = phy_sfp_probe(phydev, &qca807x_sfp_ops);
+ linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, phydev->supported);
+ linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, phydev->advertising);
+ }
+
+ return ret;
+}
+
+static int qca807x_phy_package_config_init_once(struct phy_device *phydev)
+{
+ phy_interface_t package_interface = PHY_INTERFACE_MODE_NA;
+ struct phy_package_shared *shared = phydev->shared;
+ u32 tx_driver_strength;
+ int val, ret;
+
+ if (of_property_read_u32(shared->np, "qcom,tx-driver-strength",
+ &tx_driver_strength))
+ tx_driver_strength = PSGMII_QSGMII_TX_DRIVER_600MV;
+
+ phy_lock_mdio_bus(phydev);
+
+ /* Set correct PHY package mode */
+ phy_package_get_mode(phydev, &package_interface);
+ val = __phy_package_read(phydev, QCA807X_COMBO_ADDR,
+ QCA807X_CHIP_CONFIGURATION);
+ val &= ~QCA807X_CHIP_CONFIGURATION_MODE_CFG_MASK;
+ if (package_interface == PHY_INTERFACE_MODE_QSGMII)
+ val |= QCA807X_CHIP_CONFIGURATION_MODE_QSGMII_SGMII;
+ else if (package_interface == PHY_INTERFACE_MODE_PSGMII)
+ val |= QCA807X_CHIP_CONFIGURATION_MODE_PSGMII_ALL_COPPER;
+ ret = __phy_package_write(phydev, QCA807X_COMBO_ADDR,
+ QCA807X_CHIP_CONFIGURATION, val);
+ if (ret)
+ goto exit;
+
+ /* After mode change Serdes reset is required */
+ val = __phy_package_read(phydev, QCA807X_PQSGMII_ADDR,
+ PQSGMII_CTRL_REG);
+ val &= ~PQSGMII_ANALOG_SW_RESET;
+ ret = __phy_package_write(phydev, QCA807X_PQSGMII_ADDR,
+ PQSGMII_CTRL_REG, val);
+ if (ret)
+ goto exit;
+
+ msleep(SERDES_RESET_SLEEP);
+
+ val = __phy_package_read(phydev, QCA807X_PQSGMII_ADDR,
+ PQSGMII_CTRL_REG);
+ val |= PQSGMII_ANALOG_SW_RESET;
+ ret = __phy_package_write(phydev, QCA807X_PQSGMII_ADDR,
+ PQSGMII_CTRL_REG, val);
+ if (ret)
+ goto exit;
+
+ /* Workaround to enable AZ transmitting ability */
+ val = __phy_package_read_mmd(phydev, QCA807X_PQSGMII_ADDR,
+ MDIO_MMD_PMAPMD, PQSGMII_MODE_CTRL);
+ val &= ~PQSGMII_MODE_CTRL_AZ_WORKAROUND_MASK;
+ ret = __phy_package_write_mmd(phydev, QCA807X_PQSGMII_ADDR,
+ MDIO_MMD_PMAPMD, PQSGMII_MODE_CTRL, val);
+ if (ret)
+ goto exit;
+
+ /* Set PQSGMII TX AMP strength */
+ val = __phy_package_read(phydev, QCA807X_PQSGMII_ADDR,
+ PQSGMII_DRIVE_CONTROL_1);
+ val &= ~PQSGMII_TX_DRIVER_MASK;
+ val |= FIELD_PREP(PQSGMII_TX_DRIVER_MASK, tx_driver_strength);
+ ret = __phy_package_write(phydev, QCA807X_PQSGMII_ADDR,
+ PQSGMII_DRIVE_CONTROL_1, val);
+ if (ret)
+ goto exit;
+
+ /* Prevent PSGMII going into hibernation via PSGMII self test */
+ val = __phy_package_read_mmd(phydev, QCA807X_COMBO_ADDR,
+ MDIO_MMD_PCS, PQSGMII_MMD3_SERDES_CONTROL);
+ val &= ~BIT(1);
+ ret = __phy_package_write_mmd(phydev, QCA807X_COMBO_ADDR,
+ MDIO_MMD_PCS, PQSGMII_MMD3_SERDES_CONTROL, val);
+
+exit:
+ phy_unlock_mdio_bus(phydev);
+
+ 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,
+ .resume = genphy_resume,
+ .suspend = genphy_suspend,
+ .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,
+ .resume = genphy_resume,
+ .suspend = genphy_suspend,
+ .cable_test_start = qca807x_cable_test_start,
+ .cable_test_get_status = qca807x_cable_test_get_status,
+ /* PHY package define */
+ .phy_package_global_phy_num = ARRAY_SIZE(qca807x_global_phy_names),
+ .phy_package_global_phy_names = qca807x_global_phy_names,
+ .phy_package_config_init_once = qca807x_phy_package_config_init_once,
+ },
+};
+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) },
+ { }
+};
+
+MODULE_AUTHOR("Robert Marko <[email protected]>");
+MODULE_AUTHOR("Christian Marangi <[email protected]>");
+MODULE_DESCRIPTION("Qualcomm QCA807x PHY driver");
+MODULE_DEVICE_TABLE(mdio, qca807x_tbl);
+MODULE_LICENSE("GPL");
--
2.40.1
Hello Christian,
On Mon, 20 Nov 2023 14:50:27 +0100
Christian Marangi <[email protected]> wrote:
> Idea of this big series is to introduce the concept of PHY package in DT
> and generalize the support of it by PHY driver.
I don't have much to say on the series itself, I looked at it and
didn't find anything obviously wrong, however know that this work can
also be helpful for some other use-cases. I have in mind the Marvell
Alaska 88e1543 PHY, which is a 4-port package that can either be used
as a classic Quad-PHY, or a Dual-PHY, each PHY having 2 ports. Having a
DT description of the package is a good step towards being able to
auto-detect which configuration to use for that package.
Thanks for this work,
Maxime
On Mon, 20 Nov 2023 14:50:29 +0100, Christian Marangi wrote:
> Move PHY modes from ethernet-controller schema to dedicated common PHY
> mode types definition. This is needed to have a centralized place to
> define PHY interface mode and permit usage and reference of these modes
> in other schemas.
>
> Signed-off-by: Christian Marangi <[email protected]>
> ---
> .../bindings/net/ethernet-controller.yaml | 47 +------
> .../bindings/net/ethernet-phy-mode-types.yaml | 132 ++++++++++++++++++
> 2 files changed, 133 insertions(+), 46 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/ethernet-phy-mode-types.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
./Documentation/devicetree/bindings/net/ethernet-phy-mode-types.yaml:21:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/ethernet-phy-mode-types.yaml: 'oneOf' conditional failed, one must be fixed:
'unevaluatedProperties' is a required property
'additionalProperties' is a required property
hint: Either unevaluatedProperties or additionalProperties must be present
from schema $id: http://devicetree.org/meta-schemas/core.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/ethernet-phy-mode-types.yaml: 'anyOf' conditional failed, one must be fixed:
'properties' is a required property
'patternProperties' is a required property
hint: Metaschema for devicetree binding documentation
from schema $id: http://devicetree.org/meta-schemas/core.yaml#
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
On Mon, Nov 20, 2023 at 02:50:29PM +0100, Christian Marangi wrote:
> Move PHY modes from ethernet-controller schema to dedicated common PHY
> mode types definition. This is needed to have a centralized place to
> define PHY interface mode and permit usage and reference of these modes
> in other schemas.
>
> Signed-off-by: Christian Marangi <[email protected]>
> ---
> .../bindings/net/ethernet-controller.yaml | 47 +------
> .../bindings/net/ethernet-phy-mode-types.yaml | 132 ++++++++++++++++++
> 2 files changed, 133 insertions(+), 46 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/ethernet-phy-mode-types.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> index 9f6a5ccbcefe..40c1daff2a48 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> @@ -55,55 +55,10 @@ properties:
> const: mac-address
>
> phy-connection-type:
> + $ref: /schemas/net/ethernet-phy-mode-types.yaml#definitions/phy-connection-type
You need a '/' after '#' or it is not a JSON pointer.
> description:
> Specifies interface type between the Ethernet device and a physical
> layer (PHY) device.
> - enum:
> - # There is not a standard bus between the MAC and the PHY,
> - # something proprietary is being used to embed the PHY in the
> - # MAC.
> - - internal
> - - mii
> - - gmii
> - - sgmii
> - - psgmii
> - - qsgmii
> - - qusgmii
> - - tbi
> - - rev-mii
> - - rmii
> - - rev-rmii
> - - moca
> -
> - # RX and TX delays are added by the MAC when required
> - - rgmii
> -
> - # RGMII with internal RX and TX delays provided by the PHY,
> - # the MAC should not add the RX or TX delays in this case
> - - rgmii-id
> -
> - # RGMII with internal RX delay provided by the PHY, the MAC
> - # should not add an RX delay in this case
> - - rgmii-rxid
> -
> - # RGMII with internal TX delay provided by the PHY, the MAC
> - # should not add an TX delay in this case
> - - rgmii-txid
> - - rtbi
> - - smii
> - - xgmii
> - - trgmii
> - - 1000base-x
> - - 2500base-x
> - - 5gbase-r
> - - rxaui
> - - xaui
> -
> - # 10GBASE-KR, XFI, SFI
> - - 10gbase-kr
> - - usxgmii
> - - 10gbase-r
> - - 25gbase-r
>
> phy-mode:
> $ref: "#/properties/phy-connection-type"
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy-mode-types.yaml b/Documentation/devicetree/bindings/net/ethernet-phy-mode-types.yaml
> new file mode 100644
> index 000000000000..6d15743b4ffa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy-mode-types.yaml
> @@ -0,0 +1,132 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/ethernet-phy-mode-types.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ethernet PHY Common Mode Types
> +
> +maintainers:
> + - David S. Miller <[email protected]>
> +
> +definitions:
Use '$defs'
'definitions' is special in that we don't process it at all. That's
probably why you ended up with this overly complicated schema below.
But really, there's no need to use $defs or definitions at all here.
Just put it under 'properties' And move the references up a level (i.e.
to the node, not a property).
> + phy-connection-type:
> + # const: "internal"
> + allOf:
> + - $ref: /schemas/types.yaml#/definitions/string
> + - oneOf:
> + # There is not a standard bus between the MAC and the PHY,
> + # something proprietary is being used to embed the PHY in the
> + # MAC.
> + - items:
'items' is for arrays, but you just said this is a 'string', not
'string-array'.
> + - type: string
Already covered by the type $ref.
This can all be expressed with just 1 'enum' as it was before.
> + const: internal
> + - items:
> + - type: string
> + const: mii
> + - items:
> + - type: string
> + const: gmii
> + - items:
> + - type: string
> + const: sgmii
> + - items:
> + - type: string
> + const: psgmii
> + - items:
> + - type: string
> + const: qsgmii
> + - items:
> + - type: string
> + const: qusgmii
> + - items:
> + - type: string
> + const: tbi
> + - items:
> + - type: string
> + const: rev-mii
> + - items:
> + - type: string
> + const: rmii
> + - items:
> + - type: string
> + const: rev-rmii
> + - items:
> + - type: string
> + const: moca
> +
> + # RX and TX delays are added by the MAC when required
> + - items:
> + - type: string
> + const: rgmii
> +
> + # RGMII with internal RX and TX delays provided by the PHY,
> + # the MAC should not add the RX or TX delays in this case
> + - items:
> + - type: string
> + const: rgmii-id
> +
> + # RGMII with internal RX delay provided by the PHY, the MAC
> + # should not add an RX delay in this case
> + - items:
> + - type: string
> + const: rgmii-rxid
> +
> + # RGMII with internal TX delay provided by the PHY, the MAC
> + # should not add an TX delay in this case
> + - items:
> + - type: string
> + const: rgmii-txid
> +
> + - items:
> + - type: string
> + const: rtbi
> +
> + - items:
> + - type: string
> + const: smii
> +
> + - items:
> + - type: string
> + const: xgmii
> +
> + - items:
> + - type: string
> + const: trgmii
> +
> + - items:
> + - type: string
> + const: 1000base-x
> +
> + - items:
> + - type: string
> + const: 2500base-x
> +
> + - items:
> + - type: string
> + const: 5gbase-r
> +
> + - items:
> + - type: string
> + const: rxaui
> +
> + - items:
> + - type: string
> + const: xaui
> +
> + # 10GBASE-KR, XFI, SFI
> + - items:
> + - type: string
> + const: 10gbase-kr
> +
> + - items:
> + - type: string
> + const: usxgmii
> +
> + - items:
> + - type: string
> + const: 10gbase-r
> +
> + - items:
> + - type: string
> + const: 25gbase-r
> --
> 2.40.1
>
On Mon, Nov 20, 2023 at 02:50:30PM +0100, Christian Marangi wrote:
> Document ethernet PHY package nodes used to describe PHY shipped in
> bundle of 4-5 PHY. These particular PHY require specific PHY in the
> package for global onfiguration of the PHY package.
>
> Example are PHY package that have some regs only in one PHY of the
> package and will affect every other PHY in the package, for example
> related to PHY interface mode calibration or global PHY mode selection.
>
> The PHY package node should use the global-phys property and the
> global-phy-names to define PHY in the package required by the PHY driver
> for global configuration.
>
> It's also possible to specify the property phy-mode to specify that the
> PHY package sets a global PHY interface mode and every PHY of the
> package requires to have the same PHY interface mode.
>
> Signed-off-by: Christian Marangi <[email protected]>
> ---
> .../bindings/net/ethernet-phy-package.yaml | 86 +++++++++++++++++++
> 1 file changed, 86 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/ethernet-phy-package.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy-package.yaml b/Documentation/devicetree/bindings/net/ethernet-phy-package.yaml
> new file mode 100644
> index 000000000000..2aa109e155d9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy-package.yaml
> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/ethernet-phy-package.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ethernet PHY Package Common Properties
> +
> +maintainers:
> + - Christian Marangi <[email protected]
Missing a '>'
> +
> +properties:
> + $nodename:
> + pattern: "^ethernet-phy-package(-[0-9]+)?$"
> +
> + compatible:
> + const: ethernet-phy-package
> +
> + '#address-cells':
> + description: number of address cells for the MDIO bus
> + const: 1
> +
> + '#size-cells':
> + description: number of size cells on the MDIO bus
> + const: 0
> +
> + global-phys:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + minItems: 1
> + maxItems: 31
> + description:
> + List of phandle to the PHY in the package required and
> + used to configure the PHY package.
> +
> + global-phy-names:
> + $ref: /schemas/types.yaml#/definitions/string-array
> + minItems: 1
> + maxItems: 31
> + description:
> + List of names of the PHY defined in global-phys.
> +
> + phy-connection-type:
> + $ref: /schemas/net/ethernet-phy-mode-types.yaml#definitions/phy-connection-type
> + description:
> + Specifies global interface type for the PHY package.
> +
> + phy-mode:
> + $ref: "#/properties/phy-connection-type"
> +
> +patternProperties:
> + ^ethernet-phy(@[a-f0-9]+)?$:
> + $ref: /schemas/net/ethernet-phy.yaml#
> +
> +required:
> + - compatible
> +
> +dependencies:
> + global-phy-names: [global-phys]
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + ethernet {
You mean 'mdio' here, right?
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ethernet-phy-package {
This doesn't work. Child nodes of MDIO bus must be an MDIO device with
an address. What you need is a node with all the addresses of the
device:
mdio {
...
ethernet-phy@1 {
compatible = "vendor,specifc-compatible-for-device";
reg = <1>, <4>;
...
};
};
There's also some MDIO devices which define a secondary address as a
child device. Maybe those are similar to your situation. I don't recall
which ones offhand.
> + compatible = "ethernet-phy-package";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + global-phys = <&phy4>;
> + global-phy-names = "base";
> +
> + ethernet-phy@1 {
> + compatible = "ethernet-phy-ieee802.3-c22";
> + reg = <1>;
> + };
> +
> + phy4: ethernet-phy@4 {
> + compatible = "ethernet-phy-ieee802.3-c22";
> + reg = <4>;
> + };
> + };
> + };
> --
> 2.40.1
>
On Mon, Nov 20, 2023 at 10:41:33AM -0700, Rob Herring wrote:
> On Mon, Nov 20, 2023 at 02:50:30PM +0100, Christian Marangi wrote:
> > Document ethernet PHY package nodes used to describe PHY shipped in
> > bundle of 4-5 PHY. These particular PHY require specific PHY in the
> > package for global onfiguration of the PHY package.
> >
> > Example are PHY package that have some regs only in one PHY of the
> > package and will affect every other PHY in the package, for example
> > related to PHY interface mode calibration or global PHY mode selection.
> >
> > The PHY package node should use the global-phys property and the
> > global-phy-names to define PHY in the package required by the PHY driver
> > for global configuration.
> >
> > It's also possible to specify the property phy-mode to specify that the
> > PHY package sets a global PHY interface mode and every PHY of the
> > package requires to have the same PHY interface mode.
> >
> > Signed-off-by: Christian Marangi <[email protected]>
> > ---
> > .../bindings/net/ethernet-phy-package.yaml | 86 +++++++++++++++++++
> > 1 file changed, 86 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/net/ethernet-phy-package.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/net/ethernet-phy-package.yaml b/Documentation/devicetree/bindings/net/ethernet-phy-package.yaml
> > new file mode 100644
> > index 000000000000..2aa109e155d9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/ethernet-phy-package.yaml
> > @@ -0,0 +1,86 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/ethernet-phy-package.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Ethernet PHY Package Common Properties
> > +
> > +maintainers:
> > + - Christian Marangi <[email protected]
>
> Missing a '>'
>
> > +
> > +properties:
> > + $nodename:
> > + pattern: "^ethernet-phy-package(-[0-9]+)?$"
> > +
> > + compatible:
> > + const: ethernet-phy-package
> > +
> > + '#address-cells':
> > + description: number of address cells for the MDIO bus
> > + const: 1
> > +
> > + '#size-cells':
> > + description: number of size cells on the MDIO bus
> > + const: 0
> > +
> > + global-phys:
> > + $ref: /schemas/types.yaml#/definitions/phandle-array
> > + minItems: 1
> > + maxItems: 31
> > + description:
> > + List of phandle to the PHY in the package required and
> > + used to configure the PHY package.
> > +
> > + global-phy-names:
> > + $ref: /schemas/types.yaml#/definitions/string-array
> > + minItems: 1
> > + maxItems: 31
> > + description:
> > + List of names of the PHY defined in global-phys.
> > +
> > + phy-connection-type:
> > + $ref: /schemas/net/ethernet-phy-mode-types.yaml#definitions/phy-connection-type
> > + description:
> > + Specifies global interface type for the PHY package.
> > +
> > + phy-mode:
> > + $ref: "#/properties/phy-connection-type"
> > +
> > +patternProperties:
> > + ^ethernet-phy(@[a-f0-9]+)?$:
> > + $ref: /schemas/net/ethernet-phy.yaml#
> > +
> > +required:
> > + - compatible
> > +
> > +dependencies:
> > + global-phy-names: [global-phys]
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + - |
> > + ethernet {
>
> You mean 'mdio' here, right?
>
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + ethernet-phy-package {
>
> This doesn't work. Child nodes of MDIO bus must be an MDIO device with
> an address. What you need is a node with all the addresses of the
> device:
>
> mdio {
> ...
>
> ethernet-phy@1 {
> compatible = "vendor,specifc-compatible-for-device";
> reg = <1>, <4>;
> ...
> };
> };
>
> There's also some MDIO devices which define a secondary address as a
> child device. Maybe those are similar to your situation. I don't recall
> which ones offhand.
>
Ehh this is not really a situation. We really need a way to describe PHY
package. (In the sense of device that expose multiple PHY package, as
they can be treated as single one but they are actually in bulk of 2-4-5
PHY)
qca807x is one example, quickinc is trying to push another PHY with just
a similar implementation and Maxime Chevallier just pointed out that Marvell
Alaska 88e1543 PHY also have this kind of configuration.
I feel defining PHY in subnode is a MUST and using ethernet-phy might be
confusing to describe PHY package (so I think a brand new node name
might be a better solution)
About the reg, I wonder if it would like it more if the PHY package node
would include the reg as the first address of the package and the reg
property as a list of all the reg the PHY package use.
Something like this?
ethernet-phy-package@1 {
compatible = "ethernet-phy-package";
#address-cells = <1>;
#size-cells = <0>;
reg = <1>, <2>, <3>, <4>;
global-phys = <&phy4>;
global-phy-names = "base";
ethernet-phy@1 {
compatible = "ethernet-phy-ieee802.3-c22";
reg = <1>;
};
phy4: ethernet-phy@4 {
compatible = "ethernet-phy-ieee802.3-c22";
reg = <4>;
};
};
Thanks a lot for the review and I hope we can find a good and correct
way to model this. Just hope we don't have to add all kind of proprerty
to describe the idea of PHY package. (I think the current example makes
it very clear that the PHY under the node are all part of a single piece
on the device)
> > + compatible = "ethernet-phy-package";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + global-phys = <&phy4>;
> > + global-phy-names = "base";
> > +
> > + ethernet-phy@1 {
> > + compatible = "ethernet-phy-ieee802.3-c22";
> > + reg = <1>;
> > + };
> > +
> > + phy4: ethernet-phy@4 {
> > + compatible = "ethernet-phy-ieee802.3-c22";
> > + reg = <4>;
> > + };
> > + };
> > + };
> > --
> > 2.40.1
> >
--
Ansuel
On Mon, Nov 20, 2023 at 02:50:30PM +0100, Christian Marangi wrote:
> Document ethernet PHY package nodes used to describe PHY shipped in
> bundle of 4-5 PHY. These particular PHY require specific PHY in the
> package for global onfiguration of the PHY package.
>
> Example are PHY package that have some regs only in one PHY of the
> package and will affect every other PHY in the package, for example
> related to PHY interface mode calibration or global PHY mode selection.
I think you are being overly narrow here. The 'global' registers could
be spread over multiple addresses. Particularly for a C22 PHY. I
suppose they could even be in a N+1 address space, where there is no
PHY at all.
Where the global registers are is specific to a PHY package
vendor/model. The PHY driver should know this. All the PHY driver
needs to know is some sort of base offset. PHY0 in this package is
using address X. It can then use relative addressing from this base to
access the global registers for this package.
> It's also possible to specify the property phy-mode to specify that the
> PHY package sets a global PHY interface mode and every PHY of the
> package requires to have the same PHY interface mode.
I don't think it is what simple. See the QCA8084 for example. 3 of the
4 PHYs must use QXGMII. The fourth PHY can also use QXGMII but it can
be multiplexed to a different PMA and use 1000BaseX, SGMII or
2500BaseX.
I do think we need somewhere to put package properties. But i don't
think phy-mode is such a property. At the moment, i don't have a good
example of a package property.
> +examples:
> + - |
> + ethernet {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ethernet-phy-package {
> + compatible = "ethernet-phy-package";
> + #address-cells = <1>;
> + #size-cells = <0>;
You have the PHYs within the Ethernet node. This is allowed by DT, for
historic reasons. However, i don't remember the last time a patch was
submitted that actually used this method. Now a days, PHYs are on an
MDIO bus, and they are children of that bus in the DT representation.
However you represent the package needs to work with MDIO busses.
Andrew
---
pw-bot: cr
On Mon, Nov 20, 2023 at 09:44:58PM +0100, Andrew Lunn wrote:
> On Mon, Nov 20, 2023 at 02:50:30PM +0100, Christian Marangi wrote:
> > Document ethernet PHY package nodes used to describe PHY shipped in
> > bundle of 4-5 PHY. These particular PHY require specific PHY in the
> > package for global onfiguration of the PHY package.
> >
> > Example are PHY package that have some regs only in one PHY of the
> > package and will affect every other PHY in the package, for example
> > related to PHY interface mode calibration or global PHY mode selection.
>
> I think you are being overly narrow here. The 'global' registers could
> be spread over multiple addresses. Particularly for a C22 PHY. I
> suppose they could even be in a N+1 address space, where there is no
> PHY at all.
>
> Where the global registers are is specific to a PHY package
> vendor/model. The PHY driver should know this. All the PHY driver
> needs to know is some sort of base offset. PHY0 in this package is
> using address X. It can then use relative addressing from this base to
> access the global registers for this package.
Yes that would also work but adds extra fragile code in PHY driver.
An idea might be define PHY package node with a reg that is the base
addr... and if we really want every PHY in the PHY package node is an
offset of the base addr.
>
> > It's also possible to specify the property phy-mode to specify that the
> > PHY package sets a global PHY interface mode and every PHY of the
> > package requires to have the same PHY interface mode.
>
> I don't think it is what simple. See the QCA8084 for example. 3 of the
> 4 PHYs must use QXGMII. The fourth PHY can also use QXGMII but it can
> be multiplexed to a different PMA and use 1000BaseX, SGMII or
> 2500BaseX.
Yes that is totally a problem but I think it can only be handled with
some validation in the PHY driver... I assume probe_once would validate
the modes?
>
> I do think we need somewhere to put package properties. But i don't
> think phy-mode is such a property. At the moment, i don't have a good
> example of a package property.
>
And this is the main problem with this thing... Find a good way to
define them that everyone is OK with.
Another idea might be introduce to each PHY a property that point to the
PHY package node (phandle) with all the info... But where to place
that??? Outside mdio node? That would be confusing... This is why I like
this subnode way.
I know it deviates a bit from the normal way of defining small node in
the mdio node one for each PHY.
> > +examples:
> > + - |
> > + ethernet {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + ethernet-phy-package {
> > + compatible = "ethernet-phy-package";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
>
> You have the PHYs within the Ethernet node. This is allowed by DT, for
> historic reasons. However, i don't remember the last time a patch was
> submitted that actually used this method. Now a days, PHYs are on an
> MDIO bus, and they are children of that bus in the DT representation.
> However you represent the package needs to work with MDIO busses.
>
Using the ethernet node was an oversight and actually this is defined as
a subnode in the mdio node.
A real DT that use this is (ipq807x):
&mdio {
status = "okay";
pinctrl-0 = <&mdio_pins>;
pinctrl-names = "default";
reset-gpios = <&tlmm 37 GPIO_ACTIVE_LOW>;
ethernet-phy-package {
compatible = "ethernet-phy-package";
phy-mode = "psgmii";
global-phys = <&qca8075_4>, <&qca8075_psgmii>;
global-phy-names = "combo", "analog_psgmii";
qca8075_0: ethernet-phy@0 {
compatible = "ethernet-phy-ieee802.3-c22";
reg = <0>;
};
qca8075_1: ethernet-phy@1 {
compatible = "ethernet-phy-ieee802.3-c22";
reg = <1>;
};
qca8075_2: ethernet-phy@2 {
compatible = "ethernet-phy-ieee802.3-c22";
reg = <2>;
};
qca8075_3: ethernet-phy@3 {
compatible = "ethernet-phy-ieee802.3-c22";
reg = <3>;
};
qca8075_4: ethernet-phy@4 {
compatible = "ethernet-phy-ieee802.3-c22";
reg = <4>;
};
qca8075_psgmii: ethernet-phy@5 {
compatible = "ethernet-phy-ieee802.3-c22";
reg = <5>;
};
};
qca8081: ethernet-phy@28 {
compatible = "ethernet-phy-id004d.d101";
reg = <28>;
reset-gpios = <&tlmm 31 GPIO_ACTIVE_LOW>;
};
aqr113c: ethernet-phy@8 {
compatible = "ethernet-phy-ieee802.3-c45";
reg = <8>;
reset-gpios = <&tlmm 63 GPIO_ACTIVE_LOW>;
};
};
--
Ansuel
> A real DT that use this is (ipq807x):
>
> &mdio {
> status = "okay";
> pinctrl-0 = <&mdio_pins>;
> pinctrl-names = "default";
> reset-gpios = <&tlmm 37 GPIO_ACTIVE_LOW>;
>
> ethernet-phy-package {
> compatible = "ethernet-phy-package";
> phy-mode = "psgmii";
>
> global-phys = <&qca8075_4>, <&qca8075_psgmii>;
> global-phy-names = "combo", "analog_psgmii";
>
> qca8075_0: ethernet-phy@0 {
> compatible = "ethernet-phy-ieee802.3-c22";
> reg = <0>;
> };
...
> };
>
> qca8081: ethernet-phy@28 {
> compatible = "ethernet-phy-id004d.d101";
> reg = <28>;
> reset-gpios = <&tlmm 31 GPIO_ACTIVE_LOW>;
> };
I've no idea if DT allows this. The issue is that reg is the same for
both nodes within the ethernet-phy-package container, and
ethernet-phy@28. They are all addresses on the same MDIO bus. We are
parsing this bus structure ourselves in __of_mdiobus_register(), so we
could make it work, but i don't know if we should make it work.
Andrew
On Mon, Nov 20, 2023 at 10:25:10PM +0100, Andrew Lunn wrote:
> > A real DT that use this is (ipq807x):
> >
> > &mdio {
> > status = "okay";
> > pinctrl-0 = <&mdio_pins>;
> > pinctrl-names = "default";
> > reset-gpios = <&tlmm 37 GPIO_ACTIVE_LOW>;
> >
> > ethernet-phy-package {
> > compatible = "ethernet-phy-package";
> > phy-mode = "psgmii";
> >
> > global-phys = <&qca8075_4>, <&qca8075_psgmii>;
> > global-phy-names = "combo", "analog_psgmii";
> >
> > qca8075_0: ethernet-phy@0 {
> > compatible = "ethernet-phy-ieee802.3-c22";
> > reg = <0>;
> > };
>
> ...
>
> > };
> >
> > qca8081: ethernet-phy@28 {
> > compatible = "ethernet-phy-id004d.d101";
> > reg = <28>;
> > reset-gpios = <&tlmm 31 GPIO_ACTIVE_LOW>;
> > };
>
> I've no idea if DT allows this. The issue is that reg is the same for
> both nodes within the ethernet-phy-package container, and
> ethernet-phy@28. They are all addresses on the same MDIO bus. We are
> parsing this bus structure ourselves in __of_mdiobus_register(), so we
> could make it work, but i don't know if we should make it work.
>
And that is why I have some reserve on the idea of defining a reg for
ethernet-phy-package. Adding a reg would create some duplicate. Is it
really a problem to have a node with no reg in the mdio node?
(patch 04 of this series already updates the parsing function to check
one level deeper in the presence of the ethernet-phy-compatible treating
any node found as it was defined in the upper mdio node)
--
Ansuel
On Mon, Nov 20, 2023 at 09:44:58PM +0100, Andrew Lunn wrote:
> On Mon, Nov 20, 2023 at 02:50:30PM +0100, Christian Marangi wrote:
> > Document ethernet PHY package nodes used to describe PHY shipped in
> > bundle of 4-5 PHY. These particular PHY require specific PHY in the
> > package for global onfiguration of the PHY package.
> >
> > Example are PHY package that have some regs only in one PHY of the
> > package and will affect every other PHY in the package, for example
> > related to PHY interface mode calibration or global PHY mode selection.
>
> I think you are being overly narrow here. The 'global' registers could
> be spread over multiple addresses. Particularly for a C22 PHY. I
> suppose they could even be in a N+1 address space, where there is no
> PHY at all.
>
> Where the global registers are is specific to a PHY package
> vendor/model.
For this reason in particular, the package needs a specific compatible.
> The PHY driver should know this. All the PHY driver
> needs to know is some sort of base offset. PHY0 in this package is
> using address X. It can then use relative addressing from this base to
> access the global registers for this package.
>
> > It's also possible to specify the property phy-mode to specify that the
> > PHY package sets a global PHY interface mode and every PHY of the
> > package requires to have the same PHY interface mode.
>
> I don't think it is what simple. See the QCA8084 for example. 3 of the
> 4 PHYs must use QXGMII. The fourth PHY can also use QXGMII but it can
> be multiplexed to a different PMA and use 1000BaseX, SGMII or
> 2500BaseX.
>
> I do think we need somewhere to put package properties. But i don't
> think phy-mode is such a property. At the moment, i don't have a good
> example of a package property.
What about power supplies and reset/enable lines?
Rob
> > I do think we need somewhere to put package properties. But i don't
> > think phy-mode is such a property. At the moment, i don't have a good
> > example of a package property.
>
> What about power supplies and reset/enable lines?
Yes, good point. I can imagine some packages sharing regulators. Reset
might also be shared, but it makes things messy to handle.
Andrew
On Mon, Nov 20, 2023 at 02:50:31PM +0100, Christian Marangi wrote:
> Add initial support for PHY package in DT.
>
> Make it easier to define PHY package and describe the global PHY
> directly in DT by refereincing them by phandles instead of custom
> functions in each PHY driver.
>
> Each PHY in a package needs to be defined in a dedicated node in the
> mdio node. This dedicated node needs to have the compatible set to
> "ethernet-phy-package" and define "global-phys" and "#global-phy-cells"
> respectively to a list of phandle to the global phy to define for the
> PHY package and 0 for cells as the phandle won't take any args.
>
> With this defined, the generic PHY probe will join each PHY in this
> dedicated node to the package.
>
> PHY driver MUST set the required global PHY count in
> .phy_package_global_phy_num to correctly verify that DT define the
> correct number of phandle to the required global PHY.
>
> mdio_bus.c and of_mdio.c is updated to now support and parse also
> PHY package subnote that have the compatible "phy-package".
>
> Signed-off-by: Christian Marangi <[email protected]>
...
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index c2bb3f0b9dda..5bf90c49e5bd 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -339,6 +339,8 @@ struct mdio_bus_stats {
> * phy_package_leave().
> */
> struct phy_package_shared {
> + /* With PHY package defined in DT this points to the PHY package node */
> + struct device_node *np;
> /* addrs list pointer */
> /* note that this pointer is shared between different phydevs.
> * It is allocated and freed automatically by phy_package_join() and
Hi Christian,
a minor nit from my side: please add np to the kernel doc for
struct phy_package_shared.
> @@ -888,6 +890,8 @@ struct phy_led {
> * @flags: A bitfield defining certain other features this PHY
> * supports (like interrupts)
> * @driver_data: Static driver data
> + * @phy_package_global_phy_num: Num of the required global phy
> + * for PHY package global configuration.
> *
> * All functions are optional. If config_aneg or read_status
> * are not implemented, the phy core uses the genphy versions.
...
On Mon, Nov 20, 2023 at 02:50:31PM +0100, Christian Marangi wrote:
> Add initial support for PHY package in DT.
>
> Make it easier to define PHY package and describe the global PHY
> directly in DT by refereincing them by phandles instead of custom
> functions in each PHY driver.
>
> Each PHY in a package needs to be defined in a dedicated node in the
> mdio node. This dedicated node needs to have the compatible set to
> "ethernet-phy-package" and define "global-phys" and "#global-phy-cells"
> respectively to a list of phandle to the global phy to define for the
> PHY package and 0 for cells as the phandle won't take any args.
>
> With this defined, the generic PHY probe will join each PHY in this
> dedicated node to the package.
>
> PHY driver MUST set the required global PHY count in
> .phy_package_global_phy_num to correctly verify that DT define the
> correct number of phandle to the required global PHY.
>
> mdio_bus.c and of_mdio.c is updated to now support and parse also
> PHY package subnote that have the compatible "phy-package".
>
> Signed-off-by: Christian Marangi <[email protected]>
Hi Christian,
I was a little hasty in hitting send on my previous message.
Please find some more minor feedback from my side below.
...
> diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
> index 64ebcb6d235c..bb910651118f 100644
> --- a/drivers/net/mdio/of_mdio.c
> +++ b/drivers/net/mdio/of_mdio.c
> @@ -139,6 +139,44 @@ bool of_mdiobus_child_is_phy(struct device_node *child)
> }
> EXPORT_SYMBOL(of_mdiobus_child_is_phy);
>
> +static int __of_mdiobus_parse_phys(struct mii_bus *mdio, struct device_node *np,
> + bool *scanphys)
> +{
> + struct device_node *child;
> + int addr, rc;
> +
> + /* Loop over the child nodes and register a phy_device for each phy */
> + for_each_available_child_of_node(np, child) {
> + if (of_device_is_compatible(child, "ethernet-phy-package")) {
> + rc = __of_mdiobus_parse_phys(mdio, child, scanphys);
> + if (rc && rc != -ENODEV)
> + return rc;
for_each_available_child_of_node() makes calls to of_node_get() and
of_node_put(), so when jumping out of a loop it is necessary to call
of_node_put(), in this case of_node_put(child).
As flagged by Coccinelle.
Also flagged in of_mdiobus_find_phy() both before and after this patch.
> +
> + continue;
> + }
> +
> + addr = of_mdio_parse_addr(&mdio->dev, child);
> + if (addr < 0) {
> + *scanphys = true;
> + continue;
> + }
> +
> + if (of_mdiobus_child_is_phy(child))
> + rc = of_mdiobus_register_phy(mdio, child, addr);
> + else
> + rc = of_mdiobus_register_device(mdio, child, addr);
> +
> + if (rc == -ENODEV)
> + dev_err(&mdio->dev,
> + "MDIO device at address %d is missing.\n",
> + addr);
> + else if (rc)
> + return rc;
> + }
> +
> + return 0;
> +}
> +
> /**
> * __of_mdiobus_register - Register mii_bus and create PHYs from the device tree
> * @mdio: pointer to mii_bus structure
> @@ -180,25 +218,9 @@ int __of_mdiobus_register(struct mii_bus *mdio, struct device_node *np,
> return rc;
>
> /* Loop over the child nodes and register a phy_device for each phy */
> - for_each_available_child_of_node(np, child) {
> - addr = of_mdio_parse_addr(&mdio->dev, child);
> - if (addr < 0) {
> - scanphys = true;
> - continue;
> - }
> -
> - if (of_mdiobus_child_is_phy(child))
> - rc = of_mdiobus_register_phy(mdio, child, addr);
> - else
> - rc = of_mdiobus_register_device(mdio, child, addr);
> -
> - if (rc == -ENODEV)
> - dev_err(&mdio->dev,
> - "MDIO device at address %d is missing.\n",
> - addr);
> - else if (rc)
> - goto unregister;
> - }
> + rc = __of_mdiobus_parse_phys(mdio, np, &scanphys);
> + if (rc)
> + goto unregister;
Jumping to unregister will call of_node_put(child),
however child appears to be uninitialised here.
Flagged by clang-16 W=1 build, and Smatch.
>
> if (!scanphys)
> return 0;
...
On Wed, Nov 22, 2023 at 10:52:43AM +0000, Simon Horman wrote:
> On Mon, Nov 20, 2023 at 02:50:31PM +0100, Christian Marangi wrote:
> > Add initial support for PHY package in DT.
> >
> > Make it easier to define PHY package and describe the global PHY
> > directly in DT by refereincing them by phandles instead of custom
> > functions in each PHY driver.
> >
> > Each PHY in a package needs to be defined in a dedicated node in the
> > mdio node. This dedicated node needs to have the compatible set to
> > "ethernet-phy-package" and define "global-phys" and "#global-phy-cells"
> > respectively to a list of phandle to the global phy to define for the
> > PHY package and 0 for cells as the phandle won't take any args.
> >
> > With this defined, the generic PHY probe will join each PHY in this
> > dedicated node to the package.
> >
> > PHY driver MUST set the required global PHY count in
> > .phy_package_global_phy_num to correctly verify that DT define the
> > correct number of phandle to the required global PHY.
> >
> > mdio_bus.c and of_mdio.c is updated to now support and parse also
> > PHY package subnote that have the compatible "phy-package".
> >
> > Signed-off-by: Christian Marangi <[email protected]>
>
> Hi Christian,
>
> I was a little hasty in hitting send on my previous message.
> Please find some more minor feedback from my side below.
>
Thanks a lot for the initial review and sorry for the various warning
you had to write about it. I know this was a new concept and that I had
to discuss a lot about the DT structure so I was a bit relaxed in
releasing OF node. Will handle all of them in v2. Again thanks!
> ...
>
> > diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
> > index 64ebcb6d235c..bb910651118f 100644
> > --- a/drivers/net/mdio/of_mdio.c
> > +++ b/drivers/net/mdio/of_mdio.c
> > @@ -139,6 +139,44 @@ bool of_mdiobus_child_is_phy(struct device_node *child)
> > }
> > EXPORT_SYMBOL(of_mdiobus_child_is_phy);
> >
> > +static int __of_mdiobus_parse_phys(struct mii_bus *mdio, struct device_node *np,
> > + bool *scanphys)
> > +{
> > + struct device_node *child;
> > + int addr, rc;
> > +
> > + /* Loop over the child nodes and register a phy_device for each phy */
> > + for_each_available_child_of_node(np, child) {
> > + if (of_device_is_compatible(child, "ethernet-phy-package")) {
> > + rc = __of_mdiobus_parse_phys(mdio, child, scanphys);
> > + if (rc && rc != -ENODEV)
> > + return rc;
>
> for_each_available_child_of_node() makes calls to of_node_get() and
> of_node_put(), so when jumping out of a loop it is necessary to call
> of_node_put(), in this case of_node_put(child).
>
> As flagged by Coccinelle.
>
> Also flagged in of_mdiobus_find_phy() both before and after this patch.
>
> > +
> > + continue;
> > + }
> > +
> > + addr = of_mdio_parse_addr(&mdio->dev, child);
> > + if (addr < 0) {
> > + *scanphys = true;
> > + continue;
> > + }
> > +
> > + if (of_mdiobus_child_is_phy(child))
> > + rc = of_mdiobus_register_phy(mdio, child, addr);
> > + else
> > + rc = of_mdiobus_register_device(mdio, child, addr);
> > +
> > + if (rc == -ENODEV)
> > + dev_err(&mdio->dev,
> > + "MDIO device at address %d is missing.\n",
> > + addr);
> > + else if (rc)
> > + return rc;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > /**
> > * __of_mdiobus_register - Register mii_bus and create PHYs from the device tree
> > * @mdio: pointer to mii_bus structure
> > @@ -180,25 +218,9 @@ int __of_mdiobus_register(struct mii_bus *mdio, struct device_node *np,
> > return rc;
> >
> > /* Loop over the child nodes and register a phy_device for each phy */
> > - for_each_available_child_of_node(np, child) {
> > - addr = of_mdio_parse_addr(&mdio->dev, child);
> > - if (addr < 0) {
> > - scanphys = true;
> > - continue;
> > - }
> > -
> > - if (of_mdiobus_child_is_phy(child))
> > - rc = of_mdiobus_register_phy(mdio, child, addr);
> > - else
> > - rc = of_mdiobus_register_device(mdio, child, addr);
> > -
> > - if (rc == -ENODEV)
> > - dev_err(&mdio->dev,
> > - "MDIO device at address %d is missing.\n",
> > - addr);
> > - else if (rc)
> > - goto unregister;
> > - }
> > + rc = __of_mdiobus_parse_phys(mdio, np, &scanphys);
> > + if (rc)
> > + goto unregister;
>
> Jumping to unregister will call of_node_put(child),
> however child appears to be uninitialised here.
>
> Flagged by clang-16 W=1 build, and Smatch.
>
> >
> > if (!scanphys)
> > return 0;
>
> ...
--
Ansuel
On Tue, Nov 21, 2023 at 03:45:42PM +0100, Andrew Lunn wrote:
> > > I do think we need somewhere to put package properties. But i don't
> > > think phy-mode is such a property. At the moment, i don't have a good
> > > example of a package property.
> >
> > What about power supplies and reset/enable lines?
>
> Yes, good point. I can imagine some packages sharing regulators. Reset
> might also be shared, but it makes things messy to handle.
>
Sooooo.... Sorry if I insist but I would really love to have something
""stable"" to move this further. (the changes are easy enough so it's
really a matter of finding a good DT structure)
Maybe a good idea would be summarize the concern and see what solution
was proposed:
Concern list:
1. ethernet-phy-package MUST be placed in mdio node (not in ethernet,
the example was wrong anyway) and MUST have an addr
Current example doesn't have an addr. I would prefer this way but
no problem in changing this.
Solution:
- Add reg to the ethernet-phy-package node with the base address of
the PHY package (base address = the first PHY address of the
package)
We will have a PHY node with the same address of the PHY package
node. Each PHY node in the PHY package node will have reg set to
the REAL address in the mdio bus.
2. global-phys are redundant and can be dropped.
They are used to facilitate and make it less obscure how the PHY
package is described. Can totally be handled internally by the PHY
driver. Still I would prefer to keep them as is.
Solution:
- Drop the thing and leave the PHY driver handle it with hardcoded
values.
Due to point 1, the shared struct will have the base address of
the PHY package and will be handle to reference the global PHY at
an offset from the base address.
3. phy-mode is problematic.
It's an optional value to enforce a specific mode for each PHY in the
package. For complex configuration the mode won't be defined.
Solution:
- Rename it to package-phy-mode to make it less confusing.
- Add an additional function that PHY package can use to make custom
validation on the mode for every PHY attached (in the PHY package).
Would make it less clear but more flexible for complex
configuration. Maybe both solution can be implemented and the
special function is used if the mode is not defined?
4. Not finding a correct place to put PHY package info.
I'm still convinced the mdio node is the correct place.
- PHY package are PHY in bundle so they are actual PHY
- We already have in the mdio node special handling (every DSA switch
use custom compatible and PHY ID is not used to probe them
normally)
- Node this way won't be treated as PHY as they won't match the PHY
node name pattern and also won't have the compatible pattern for
PHY.
Solution:
- ethernet-phy-package node is OK given a reg is defined.
These are the 4 concern we have currently, hoping I didn't miss any, I
hope we can sort those so I can send a v2 and make some progress on
this.
--
Ansuel
On Wed, Nov 22, 2023 at 07:15:06PM +0100, Christian Marangi wrote:
...
> > Hi Christian,
> >
> > I was a little hasty in hitting send on my previous message.
> > Please find some more minor feedback from my side below.
> >
>
> Thanks a lot for the initial review and sorry for the various warning
> you had to write about it. I know this was a new concept and that I had
> to discuss a lot about the DT structure so I was a bit relaxed in
> releasing OF node. Will handle all of them in v2. Again thanks!
No problem. I'm never sure if this kind of feedback is appropriate for and
RFC or not. And perhaps in this case it would have been better to wait for
at least one more revision. So sorry for the noise at this stage of the
patch-set's development.
...
> + They feature 2 SerDes, one for PSGMII or QSGMII connection with
> + MAC, while second one is SGMII for connection to MAC or fiber.
Can you connect 1/5 of the PSGMII SERDES to the SGMII SERDES? So
making use of the PHY as a media converter to connect to an SFP cage?
I assume the SGMII serdes can also do 1000BaseX?
How do you describe what the SGMII SERDES is connected to?
Andrew
On Mon, Nov 20, 2023 at 02:50:40PM +0100, Christian Marangi wrote:
> From: Robert Marko <[email protected]>
>
> 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 4 digitally controlled
> output only pins that natively drive LED-s.
> But some vendors used these to driver generic LED-s controlled
> by userspace, so lets enable registering each PHY as GPIO
> controller and add driver for it.
>
> These are commonly used in Qualcomm IPQ40xx, IPQ60xx and IPQ807x
> boards.
You need to justification why this is a whole new driver, rather than
just extending at803x.c.
I did a few quick checks. The Downshift code is identical, once you
take into account the different names. Interrupt handling looks very
similar. MDIX looks the same, and Specific status looks similar, etc.
There are clear differences, but i think there is enough identical
code you should be sharing it.
Andrew
On Mon, Nov 20, 2023 at 02:50:38PM +0100, Christian Marangi wrote:
> From: Robert Marko <[email protected]>
>
> Add DT bindings defined for Qualcomm QCA807x PHY series related to
> calibration and DAC settings.
>
> Signed-off-by: Robert Marko <[email protected]>
> Signed-off-by: Christian Marangi <[email protected]>
> ---
> include/dt-bindings/net/qcom-qca807x.h | 45 ++++++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
> create mode 100644 include/dt-bindings/net/qcom-qca807x.h
>
> diff --git a/include/dt-bindings/net/qcom-qca807x.h b/include/dt-bindings/net/qcom-qca807x.h
> new file mode 100644
> index 000000000000..42c45c7d5210
> --- /dev/null
> +++ b/include/dt-bindings/net/qcom-qca807x.h
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR MIT */
> +/*
> + * Device Tree constants for the Qualcomm QCA807X PHYs
> + */
> +
> +#ifndef _DT_BINDINGS_QCOM_QCA807X_H
> +#define _DT_BINDINGS_QCOM_QCA807X_H
> +
> +#define PSGMII_QSGMII_TX_DRIVER_140MV 0
> +#define PSGMII_QSGMII_TX_DRIVER_160MV 1
> +#define PSGMII_QSGMII_TX_DRIVER_180MV 2
> +#define PSGMII_QSGMII_TX_DRIVER_200MV 3
> +#define PSGMII_QSGMII_TX_DRIVER_220MV 4
> +#define PSGMII_QSGMII_TX_DRIVER_240MV 5
> +#define PSGMII_QSGMII_TX_DRIVER_260MV 6
> +#define PSGMII_QSGMII_TX_DRIVER_280MV 7
> +#define PSGMII_QSGMII_TX_DRIVER_300MV 8
> +#define PSGMII_QSGMII_TX_DRIVER_320MV 9
> +#define PSGMII_QSGMII_TX_DRIVER_400MV 10
> +#define PSGMII_QSGMII_TX_DRIVER_500MV 11
> +/* Default value */
> +#define PSGMII_QSGMII_TX_DRIVER_600MV 12
Its not clear what these are used for. I think these are actually
package properties, so having the correctly documented in the binding
would help a lot understand why we need package properties.
Actually using #define like this is probably wrong. You should just
have a u32 tx_driver_mv property, and list [140, 160, 180, 200,
... 600] as valid values. The driver should then convert to whatever
value it should write into the register.
Andrew
On Wed, Nov 22, 2023 at 07:32:22PM +0100, Christian Marangi wrote:
> On Tue, Nov 21, 2023 at 03:45:42PM +0100, Andrew Lunn wrote:
> > > > I do think we need somewhere to put package properties. But i don't
> > > > think phy-mode is such a property. At the moment, i don't have a good
> > > > example of a package property.
> > >
> > > What about power supplies and reset/enable lines?
> >
> > Yes, good point. I can imagine some packages sharing regulators. Reset
> > might also be shared, but it makes things messy to handle.
> >
>
> Sooooo.... Sorry if I insist but I would really love to have something
> ""stable"" to move this further. (the changes are easy enough so it's
> really a matter of finding a good DT structure)
>
> Maybe a good idea would be summarize the concern and see what solution
> was proposed:
>
> Concern list:
> 1. ethernet-phy-package MUST be placed in mdio node (not in ethernet,
> the example was wrong anyway) and MUST have an addr
>
> Current example doesn't have an addr. I would prefer this way but
> no problem in changing this.
>
> Solution:
> - Add reg to the ethernet-phy-package node with the base address of
> the PHY package (base address = the first PHY address of the
> package)
Yes.
> We will have a PHY node with the same address of the PHY package
> node. Each PHY node in the PHY package node will have reg set to
> the REAL address in the mdio bus.
Basically Yes. I actually think the first sentence is not 100%
correct. It could be all the package configuration registers are in
the base address, without an actual PHY. The PHYs then follow at
addresses above it. I can also imagine a case where the first PHY in
the package is not used, so its not listed at all. So i think it
should be "We often have a PHY node with the same address of the PHY
package node."
> 3. phy-mode is problematic.
>
> It's an optional value to enforce a specific mode for each PHY in the
> package. For complex configuration the mode won't be defined.
>
> Solution:
> - Rename it to package-phy-mode to make it less confusing.
>
> - Add an additional function that PHY package can use to make custom
> validation on the mode for every PHY attached (in the PHY package).
>
> Would make it less clear but more flexible for complex
> configuration. Maybe both solution can be implemented and the
> special function is used if the mode is not defined?
The description you give is that there are two SERDES, and both could
be connected to a MAC. What does package-phy-mode represent then?
Luo Jie patch for the qca8084 seems to have the same issue. It has two
SERDES/PMA, and both could be connected to the MACs. So it seems like
QCA devices don't actually fit this model. If we want to describe the
package link mode, we probably need to list each PMA, and have a
property in the PMA node indicating what link mode the PMA is
operating at.
At least for the moment, its not clear we actually need this at
all. It seems like the phy-mode is all we need. The PHY driver should
know what values are valid per port, and so can validate the value.
> 4. Not finding a correct place to put PHY package info.
>
> I'm still convinced the mdio node is the correct place.
> - PHY package are PHY in bundle so they are actual PHY
> - We already have in the mdio node special handling (every DSA switch
> use custom compatible and PHY ID is not used to probe them
> normally)
> - Node this way won't be treated as PHY as they won't match the PHY
> node name pattern and also won't have the compatible pattern for
> PHY.
We do need a compatible for the package. The kernel is unlikely to use
it, but the validation tools will. Each package can have its own DT
properties, so we need a .yaml to describe those properties. So i
would expect to have a "qca807x-package" compatible, which then lists
the tx driver strength etc. The PHYs within the package don't need
compatible, they are just linux PHYs, probed using the same code as
PHYs outside of a package.
Andrew
On Mon, Nov 20, 2023 at 02:50:39PM +0100, Christian Marangi wrote:
> +examples:
> + - |
> + #include <dt-bindings/leds/common.h>
> +
> + mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ethernet-phy-package {
> + compatible = "ethernet-phy-package";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + global-phys = <&qca8075_4>, <&qca8075_psgmii>;
Should the second one be &qca8075_pqsgmii ? (note the addition of 'q').
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Thu, Nov 23, 2023 at 04:30:48AM +0100, Andrew Lunn wrote:
> On Wed, Nov 22, 2023 at 07:32:22PM +0100, Christian Marangi wrote:
> > On Tue, Nov 21, 2023 at 03:45:42PM +0100, Andrew Lunn wrote:
> > > > > I do think we need somewhere to put package properties. But i don't
> > > > > think phy-mode is such a property. At the moment, i don't have a good
> > > > > example of a package property.
> > > >
> > > > What about power supplies and reset/enable lines?
> > >
> > > Yes, good point. I can imagine some packages sharing regulators. Reset
> > > might also be shared, but it makes things messy to handle.
> > >
> >
> > Sooooo.... Sorry if I insist but I would really love to have something
> > ""stable"" to move this further. (the changes are easy enough so it's
> > really a matter of finding a good DT structure)
> >
> > Maybe a good idea would be summarize the concern and see what solution
> > was proposed:
> >
> > Concern list:
> > 1. ethernet-phy-package MUST be placed in mdio node (not in ethernet,
> > the example was wrong anyway) and MUST have an addr
> >
> > Current example doesn't have an addr. I would prefer this way but
> > no problem in changing this.
> >
> > Solution:
> > - Add reg to the ethernet-phy-package node with the base address of
> > the PHY package (base address = the first PHY address of the
> > package)
>
> Yes.
>
Thanks.
> > We will have a PHY node with the same address of the PHY package
> > node. Each PHY node in the PHY package node will have reg set to
> > the REAL address in the mdio bus.
>
> Basically Yes. I actually think the first sentence is not 100%
> correct. It could be all the package configuration registers are in
> the base address, without an actual PHY. The PHYs then follow at
> addresses above it. I can also imagine a case where the first PHY in
> the package is not used, so its not listed at all. So i think it
> should be "We often have a PHY node with the same address of the PHY
> package node."
>
Just to add details, also the opposite can happen. Where the last PHY in
the bundle is used for global configuration but is not defined. This is
another reason I wanted the global-phy proprerty, having to reference
them even if they are not used better describe the actual PHY address
used in the mdio bus. If the PHY is handled internally and omitted in DT
then people might think that address is not used.
> > 3. phy-mode is problematic.
> >
> > It's an optional value to enforce a specific mode for each PHY in the
> > package. For complex configuration the mode won't be defined.
> >
> > Solution:
> > - Rename it to package-phy-mode to make it less confusing.
> >
> > - Add an additional function that PHY package can use to make custom
> > validation on the mode for every PHY attached (in the PHY package).
> >
> > Would make it less clear but more flexible for complex
> > configuration. Maybe both solution can be implemented and the
> > special function is used if the mode is not defined?
>
> The description you give is that there are two SERDES, and both could
> be connected to a MAC. What does package-phy-mode represent then?
>
> Luo Jie patch for the qca8084 seems to have the same issue. It has two
> SERDES/PMA, and both could be connected to the MACs. So it seems like
> QCA devices don't actually fit this model. If we want to describe the
> package link mode, we probably need to list each PMA, and have a
> property in the PMA node indicating what link mode the PMA is
> operating at.
>
> At least for the moment, its not clear we actually need this at
> all. It seems like the phy-mode is all we need. The PHY driver should
> know what values are valid per port, and so can validate the value.
>
Just to be more precise qca807x can operate in 3 different mode:
(this is controlled by the MODE_CFG bits)
- QSGMII: 5 copper port
- PSGMII: 5 copper port
- PSGMII: 4 copper port + 1 combo (that can be both fiber or copper)
When mode is set to QSGMII each PHY needs to have the matching mode or
we have no traffic. It makes it difficult driver side to understand what
mode should be enforced with all kind of parsing or magic in private
struct.
Maybe for this specific case would be ok to introduce a simple specific
proprerty? Like qca,qsgmii_mode ?
> > 4. Not finding a correct place to put PHY package info.
> >
> > I'm still convinced the mdio node is the correct place.
> > - PHY package are PHY in bundle so they are actual PHY
> > - We already have in the mdio node special handling (every DSA switch
> > use custom compatible and PHY ID is not used to probe them
> > normally)
> > - Node this way won't be treated as PHY as they won't match the PHY
> > node name pattern and also won't have the compatible pattern for
> > PHY.
>
> We do need a compatible for the package. The kernel is unlikely to use
> it, but the validation tools will. Each package can have its own DT
> properties, so we need a .yaml to describe those properties. So i
> would expect to have a "qca807x-package" compatible, which then lists
> the tx driver strength etc. The PHYs within the package don't need
> compatible, they are just linux PHYs, probed using the same code as
> PHYs outside of a package.
>
Since the idea is add OF support for the PHY we also need a generic
compatible for it.
Is it ok to have something like
compatible = "ethernet-phy-package", "qca807x-phy-package";
With "ethernet-phy-package" a must and "qca807x-phy-package" used only
if additional property are used.
My current idea was to use select and base everything on the possible
PHY compatible (and it does work, tested by adding bloat in the DT
example and seeing if the schema was rejected). Had this idea since the
compatible would never be used.
--
Ansuel
On Thu, Nov 23, 2023 at 3:15 AM Andrew Lunn <[email protected]> wrote:
>
> > + They feature 2 SerDes, one for PSGMII or QSGMII connection with
> > + MAC, while second one is SGMII for connection to MAC or fiber.
>
> Can you connect 1/5 of the PSGMII SERDES to the SGMII SERDES? So
> making use of the PHY as a media converter to connect to an SFP cage?
> I assume the SGMII serdes can also do 1000BaseX?
>
> How do you describe what the SGMII SERDES is connected to?
Hi Andrew,
I think that the description is confusing.
QCA807x supports 3 different modes:
1. PSGMII (5 copper ports)
2. PSGMII (4 copper ports + 1 combo port)
3. QSGMII+SGMII
So, in case option 2 is selected then the combo port can also be used for
1000Base-X and 100Base-FX modules or copper and it will autodetect the
exact media.
This is supported via the SFP op-s and I have been using it without issues
for a while.
I have not tested option 3 in combination with SFP to the copper
module so I cant
say whether that works.
From what I can gather from the typical usage examples in the
datasheet, this QSMII+SGMII
mode is basically intended as a backward compatibility thing as only
QCA SoC-s have PSGMII
support so that you could still use SoC-s with QSGMII and SGMII support only.
So there is no way to control the SerDes-es individually, only the
global mode can be changed via
the Chip configuration register in the Combo port.
You can see the block diagram of this PHY in this public PDF on page 2[1].
[1] https://content.codico.com/fileadmin/media/download/datasheets/qualcomm/qualcomm_qca8075.pdf
>
> Andrew
>
--
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: [email protected]
Web: http://www.sartura.hr
> Just to be more precise qca807x can operate in 3 different mode:
> (this is controlled by the MODE_CFG bits)
> - QSGMII: 5 copper port
4 slots over QSGMII, plus the second SERDES is connected to the MAC
using SGMII/1000BaseX?
> - PSGMII: 5 copper port
5 slots over QSGMII, the second SERDES is idle?
> - PSGMII: 4 copper port + 1 combo (that can be both fiber or copper)
5 slots over QSGMII, with the second SERDES connected to an SFP cage.
Are ports 1-4 always connected to the P/Q SGMII. Its only port 5 which
can use the second SERDES?
Does changing between QSGMII and PSGMII really change the protocol run
over the multiplex link? The clock rate is slower, there are only 4
multiplexed slots vs five? Or does it keep using PSGMII and leaves one slot
I can see how it is messy to validate, if you only have phy-mode. So
maybe MODE_CFG is a package property. You then can validate the
phy-mode against MODE_CFG.
Andrew
On Thu, Nov 23, 2023 at 03:27:05PM +0100, Andrew Lunn wrote:
> > Just to be more precise qca807x can operate in 3 different mode:
> > (this is controlled by the MODE_CFG bits)
>
> > - QSGMII: 5 copper port
>
> 4 slots over QSGMII, plus the second SERDES is connected to the MAC
> using SGMII/1000BaseX?
>
> > - PSGMII: 5 copper port
>
> 5 slots over QSGMII, the second SERDES is idle?
>
> > - PSGMII: 4 copper port + 1 combo (that can be both fiber or copper)
>
> 5 slots over QSGMII, with the second SERDES connected to an SFP cage.
>
> Are ports 1-4 always connected to the P/Q SGMII. Its only port 5 which
> can use the second SERDES?
I think what would really help here is if there was an ascii table to
describe the configurations, rather than trying to put it into words.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Thu, Nov 23, 2023 at 02:35:31PM +0000, Russell King (Oracle) wrote:
> On Thu, Nov 23, 2023 at 03:27:05PM +0100, Andrew Lunn wrote:
> > > Just to be more precise qca807x can operate in 3 different mode:
> > > (this is controlled by the MODE_CFG bits)
> >
> > > - QSGMII: 5 copper port
> >
> > 4 slots over QSGMII, plus the second SERDES is connected to the MAC
> > using SGMII/1000BaseX?
> >
> > > - PSGMII: 5 copper port
> >
> > 5 slots over QSGMII, the second SERDES is idle?
> >
> > > - PSGMII: 4 copper port + 1 combo (that can be both fiber or copper)
> >
> > 5 slots over QSGMII, with the second SERDES connected to an SFP cage.
> >
> > Are ports 1-4 always connected to the P/Q SGMII. Its only port 5 which
> > can use the second SERDES?
>
> I think what would really help here is if there was an ascii table to
> describe the configurations, rather than trying to put it into words.
Yes.
And also for ipq4019. We need to merge these two threads of
conversation, since in the end they are probably the same driver, same
device tree etc.
Andrew
> compatible = "ethernet-phy-package", "qca807x-phy-package";
>
> With "ethernet-phy-package" a must and "qca807x-phy-package" used only
> if additional property are used.
>
> My current idea was to use select and base everything on the possible
> PHY compatible (and it does work, tested by adding bloat in the DT
> example and seeing if the schema was rejected). Had this idea since the
> compatible would never be used.
The DT people are unhappy with PHYs don't use compatibles, so
validation does not work. It probably too late to add compatibles to
very PHY driver. But this is new development work, we don't have any
history. So we can add a compatible per package to make the validation
tools work.
So for parsing the tree in the kernel we look for
'ethernet-phy-package'. For validating the tree using the yaml tools
we use the 'qca807x-phy-package'.
Andrew
On Thu, Nov 23, 2023 at 03:57:58PM +0100, Andrew Lunn wrote:
> On Thu, Nov 23, 2023 at 02:35:31PM +0000, Russell King (Oracle) wrote:
> > On Thu, Nov 23, 2023 at 03:27:05PM +0100, Andrew Lunn wrote:
> > > > Just to be more precise qca807x can operate in 3 different mode:
> > > > (this is controlled by the MODE_CFG bits)
> > >
> > > > - QSGMII: 5 copper port
> > >
> > > 4 slots over QSGMII, plus the second SERDES is connected to the MAC
> > > using SGMII/1000BaseX?
> > >
> > > > - PSGMII: 5 copper port
> > >
> > > 5 slots over QSGMII, the second SERDES is idle?
> > >
> > > > - PSGMII: 4 copper port + 1 combo (that can be both fiber or copper)
> > >
> > > 5 slots over QSGMII, with the second SERDES connected to an SFP cage.
> > >
> > > Are ports 1-4 always connected to the P/Q SGMII. Its only port 5 which
> > > can use the second SERDES?
> >
> > I think what would really help here is if there was an ascii table to
> > describe the configurations, rather than trying to put it into words.
>
> Yes.
>
> And also for ipq4019. We need to merge these two threads of
> conversation, since in the end they are probably the same driver, same
> device tree etc.
>
For everyone that missed Robert response in patch 12 let me quote him
also here.
"
Hi Andrew,
I think that the description is confusing.
QCA807x supports 3 different modes:
1. PSGMII (5 copper ports)
2. PSGMII (4 copper ports + 1 combo port)
3. QSGMII+SGMII
So, in case option 2 is selected then the combo port can also be used for
1000Base-X and 100Base-FX modules or copper and it will autodetect the
exact media.
This is supported via the SFP op-s and I have been using it without issues
for a while.
I have not tested option 3 in combination with SFP to the copper
module so I cant
say whether that works.
From what I can gather from the typical usage examples in the
datasheet, this QSMII+SGMII
mode is basically intended as a backward compatibility thing as only
QCA SoC-s have PSGMII
support so that you could still use SoC-s with QSGMII and SGMII support only.
So there is no way to control the SerDes-es individually, only the
global mode can be changed via
the Chip configuration register in the Combo port.
You can see the block diagram of this PHY in this public PDF on page 2[1].
[1] https://content.codico.com/fileadmin/media/download/datasheets/qualcomm/qualcomm_qca8075.pdf
"
--
Ansuel
On Thu, Nov 23, 2023 at 04:07:14PM +0100, Andrew Lunn wrote:
> > compatible = "ethernet-phy-package", "qca807x-phy-package";
> >
> > With "ethernet-phy-package" a must and "qca807x-phy-package" used only
> > if additional property are used.
> >
> > My current idea was to use select and base everything on the possible
> > PHY compatible (and it does work, tested by adding bloat in the DT
> > example and seeing if the schema was rejected). Had this idea since the
> > compatible would never be used.
>
> The DT people are unhappy with PHYs don't use compatibles, so
> validation does not work. It probably too late to add compatibles to
> very PHY driver. But this is new development work, we don't have any
> history. So we can add a compatible per package to make the validation
> tools work.
>
> So for parsing the tree in the kernel we look for
> 'ethernet-phy-package'. For validating the tree using the yaml tools
> we use the 'qca807x-phy-package'.
>
Ok clear, what about the generic ethernet-phy-package.yaml?
Idea was to describe common properties there and then specific PHY
package would add every common property with $ref and add their special
ones on top of that. Would that be ok? (similar to the current
implementation with ethernet-phy-package and qcom,qca807x with the only
difference that qcom,qca807x.yaml would also have the compatible set
(currently missing from this RFC)
--
Ansuel
On 11/24/2023 3:33 AM, Christian Marangi wrote:
> On Thu, Nov 23, 2023 at 03:57:58PM +0100, Andrew Lunn wrote:
>> On Thu, Nov 23, 2023 at 02:35:31PM +0000, Russell King (Oracle) wrote:
>>> On Thu, Nov 23, 2023 at 03:27:05PM +0100, Andrew Lunn wrote:
>>>>> Just to be more precise qca807x can operate in 3 different mode:
>>>>> (this is controlled by the MODE_CFG bits)
>>>>
>>>>> - QSGMII: 5 copper port
>>>>
>>>> 4 slots over QSGMII, plus the second SERDES is connected to the MAC
>>>> using SGMII/1000BaseX?
>>>>
>>>>> - PSGMII: 5 copper port
>>>>
>>>> 5 slots over QSGMII, the second SERDES is idle?
>>>>
>>>>> - PSGMII: 4 copper port + 1 combo (that can be both fiber or copper)
>>>>
>>>> 5 slots over QSGMII, with the second SERDES connected to an SFP cage.
>>>>
>>>> Are ports 1-4 always connected to the P/Q SGMII. Its only port 5 which
>>>> can use the second SERDES?
>>>
>>> I think what would really help here is if there was an ascii table to
>>> describe the configurations, rather than trying to put it into words.
>>
>> Yes.
>>
>> And also for ipq4019. We need to merge these two threads of
>> conversation, since in the end they are probably the same driver, same
>> device tree etc.
>>
>
> For everyone that missed Robert response in patch 12 let me quote him
> also here.
>
> "
> Hi Andrew,
> I think that the description is confusing.
> QCA807x supports 3 different modes:
> 1. PSGMII (5 copper ports)
> 2. PSGMII (4 copper ports + 1 combo port)
> 3. QSGMII+SGMII
>
> So, in case option 2 is selected then the combo port can also be used for
> 1000Base-X and 100Base-FX modules or copper and it will autodetect the
> exact media.
> This is supported via the SFP op-s and I have been using it without issues
> for a while.
>
> I have not tested option 3 in combination with SFP to the copper
> module so I cant
> say whether that works.
For the option3, the last PHY works on SGMII mode, then it can't be
used with SFP, only copper is supported in this mode.
> From what I can gather from the typical usage examples in the
> datasheet, this QSMII+SGMII
> mode is basically intended as a backward compatibility thing as only
> QCA SoC-s have PSGMII
> support so that you could still use SoC-s with QSGMII and SGMII support only.
>
> So there is no way to control the SerDes-es individually, only the
> global mode can be changed via
> the Chip configuration register in the Combo port.
>
> You can see the block diagram of this PHY in this public PDF on page 2[1].
>
> [1] https://content.codico.com/fileadmin/media/download/datasheets/qualcomm/qualcomm_qca8075.pdf
> "
>
On Fri, Nov 24, 2023 at 07:49:27PM +0800, Jie Luo wrote:
>
>
> On 11/24/2023 3:33 AM, Christian Marangi wrote:
> > On Thu, Nov 23, 2023 at 03:57:58PM +0100, Andrew Lunn wrote:
> > > On Thu, Nov 23, 2023 at 02:35:31PM +0000, Russell King (Oracle) wrote:
> > > > On Thu, Nov 23, 2023 at 03:27:05PM +0100, Andrew Lunn wrote:
> > > > > > Just to be more precise qca807x can operate in 3 different mode:
> > > > > > (this is controlled by the MODE_CFG bits)
> > > > >
> > > > > > - QSGMII: 5 copper port
> > > > >
> > > > > 4 slots over QSGMII, plus the second SERDES is connected to the MAC
> > > > > using SGMII/1000BaseX?
> > > > >
> > > > > > - PSGMII: 5 copper port
> > > > >
> > > > > 5 slots over QSGMII, the second SERDES is idle?
> > > > >
> > > > > > - PSGMII: 4 copper port + 1 combo (that can be both fiber or copper)
> > > > >
> > > > > 5 slots over QSGMII, with the second SERDES connected to an SFP cage.
> > > > >
> > > > > Are ports 1-4 always connected to the P/Q SGMII. Its only port 5 which
> > > > > can use the second SERDES?
> > > >
> > > > I think what would really help here is if there was an ascii table to
> > > > describe the configurations, rather than trying to put it into words.
> > >
> > > Yes.
> > >
> > > And also for ipq4019. We need to merge these two threads of
> > > conversation, since in the end they are probably the same driver, same
> > > device tree etc.
> > >
> >
> > For everyone that missed Robert response in patch 12 let me quote him
> > also here.
> >
> > "
> > Hi Andrew,
> > I think that the description is confusing.
> > QCA807x supports 3 different modes:
> > 1. PSGMII (5 copper ports)
> > 2. PSGMII (4 copper ports + 1 combo port)
> > 3. QSGMII+SGMII
> >
> > So, in case option 2 is selected then the combo port can also be used for
> > 1000Base-X and 100Base-FX modules or copper and it will autodetect the
> > exact media.
> > This is supported via the SFP op-s and I have been using it without issues
> > for a while.
> >
> > I have not tested option 3 in combination with SFP to the copper
> > module so I cant
> > say whether that works.
>
> For the option3, the last PHY works on SGMII mode, then it can't be
> used with SFP, only copper is supported in this mode.
So, from what you've written, and looking at the PDF for QCA8075:
First Serdes mode Second Serdes mode
Option 1 PSGMII for copper Disabled
ports 0-4
Option 2 PSGMII for copper 1000BASE-X / 100BASE-FX
ports 0-4
Option 3 QSGMII for copper SGMII for
ports 0-3 copper port 4
In all cases, ports 0-4 have a copper (baseT/baseTx/baseTe) port
available.
This is a much easier to understand presentation than writing out a
longwinded description of the three modes. Please include the table
and the statement below the table in the commit description as that
is necessary to describe the hardware setup being addressed in these
patches.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> First Serdes mode Second Serdes mode
> Option 1 PSGMII for copper Disabled
> ports 0-4
> Option 2 PSGMII for copper 1000BASE-X / 100BASE-FX
> ports 0-4
> Option 3 QSGMII for copper SGMII for
> ports 0-3 copper port 4
With option 2, can the second SERDES also do SGMII? You are likely to
need that when a Copper SFP module is inserted into the cage.
Andrew
On Fri, Nov 24, 2023 at 03:44:20PM +0100, Andrew Lunn wrote:
> > First Serdes mode Second Serdes mode
> > Option 1 PSGMII for copper Disabled
> > ports 0-4
> > Option 2 PSGMII for copper 1000BASE-X / 100BASE-FX
> > ports 0-4
> > Option 3 QSGMII for copper SGMII for
> > ports 0-3 copper port 4
>
> With option 2, can the second SERDES also do SGMII? You are likely to
> need that when a Copper SFP module is inserted into the cage.
The document states "The fiber port supports 1000BASE-X/100BASE-FX".
The same is true of Marvell 88x3310's fiber port - it supports only
fiber not SGMII. This is actually something else that - when the
patches for stacked PHYs mature - will need to be addressed. If we
have a 1G copper SFP plugged into an interface that only supports
1000base-X then we need a way to switch the PHY on the SFP module
to 1000base-X if it's in SGMII mode.
Some copper SFPs come up in 1000base-X mode, and we currently rely
on the 88e1111 driver to switch them to SGMII mode. Others do want
SGMII mode (like Mikrotik RJ01 where the PHY is inaccessible and
thus can't be reconfigured.)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Fri, Nov 24, 2023 at 4:16 PM Russell King (Oracle)
<[email protected]> wrote:
>
> On Fri, Nov 24, 2023 at 03:44:20PM +0100, Andrew Lunn wrote:
> > > First Serdes mode Second Serdes mode
> > > Option 1 PSGMII for copper Disabled
> > > ports 0-4
> > > Option 2 PSGMII for copper 1000BASE-X / 100BASE-FX
> > > ports 0-4
> > > Option 3 QSGMII for copper SGMII for
> > > ports 0-3 copper port 4
> >
> > With option 2, can the second SERDES also do SGMII? You are likely to
> > need that when a Copper SFP module is inserted into the cage.
>
> The document states "The fiber port supports 1000BASE-X/100BASE-FX".
>
> The same is true of Marvell 88x3310's fiber port - it supports only
> fiber not SGMII. This is actually something else that - when the
> patches for stacked PHYs mature - will need to be addressed. If we
> have a 1G copper SFP plugged into an interface that only supports
> 1000base-X then we need a way to switch the PHY on the SFP module
> to 1000base-X if it's in SGMII mode.
>
> Some copper SFPs come up in 1000base-X mode, and we currently rely
> on the 88e1111 driver to switch them to SGMII mode. Others do want
> SGMII mode (like Mikrotik RJ01 where the PHY is inaccessible and
> thus can't be reconfigured.)
I can confirm that SGMII mode doesn't work with Option 2, I have tested this
a while ago with Mikrotik RJ01 (I think it has AR803x, but its not accessible as
you pointed out) and it was somewhat working but in half duplex only
and dropping
packets.
Currently, SFP mode is checked and only 1000Base-X and 100Base-FX are
accepted, otherwise the module insert will return and error for unsupported
mode.
Regards,
Robert
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
--
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: [email protected]
Web: http://www.sartura.hr
On Wed, Nov 22, 2023 at 07:32:22PM +0100, Christian Marangi wrote:
> Sooooo.... Sorry if I insist but I would really love to have something
> ""stable"" to move this further. (the changes are easy enough so it's
> really a matter of finding a good DT structure)
>
> Maybe a good idea would be summarize the concern and see what solution
> was proposed:
Sorry, I didn't follow the entire discussion. I hope I'm not too far off
with my understanding of your problems.
I think you are hitting some of the same points I have hit with DSA.
The PHY package could be considered an SoC with lots of peripherals on
it, for which you'd want separate drivers. Just like a DSA switch would.
I don't think it's exactly phylib's place to deal with that, just like
it's not DSA's place to deal with complex SoCs, just with the switching
IP (like the Ethernet PHY IP for phylib).
https://lore.kernel.org/lkml/20221222134844.lbzyx5hz7z5n763n@skbuf/
Why does the ethernet-phy-package DT binding attempt to be so grand and
generic? I would expect the 180 degree opposite. Make it have a _single_
compatible of "qcom,qca807x" (but don't use an "x" wildcard, do specify
the full package name).
Make it have a "reg" property which is the base MDIO address of the package.
Write an mdio_device driver that probes on that. The PHY core already
knows that if a child on the MDIO bus has a compatible string of the
normal form (not like "ethernet-phy-id004d.d0b2"), then it's looking at
an mdio_device.
Make the OF node of the package have an "mdio" child with its own
compatible string, which represents the place where PHYs are. The driver
for the "mdio" child has a very simple implementation of the mii_bus
ops, which just calls into the device parent (it can assume a certain
parent implementation and private data structures).
Lateral to the "mdio" child node of the "qcom,qca807x" package node, you
could put any other device tree properties that you want.
Make the mdio_device driver for "qcom,qca807x" use shared code if you
want - but keep the device tree structure hardware-specific. Look at the
compatible strings that e.g. the drivers/mfd/simple-mfd-i2c.c driver
probes on. You could always change the driver implementation for a
certain compatible string, but you'll be stuck with the ultra-generic
compatible = "ethernet-phy-package", which has the problems that you
mention.
>
> Concern list:
> 1. ethernet-phy-package MUST be placed in mdio node (not in ethernet,
> the example was wrong anyway) and MUST have an addr
>
> Current example doesn't have an addr. I would prefer this way but
> no problem in changing this.
>
> Solution:
> - Add reg to the ethernet-phy-package node with the base address of
> the PHY package (base address = the first PHY address of the
> package)
Correct, what I'm saying is compatible with this.
>
> We will have a PHY node with the same address of the PHY package
> node. Each PHY node in the PHY package node will have reg set to
> the REAL address in the mdio bus.
If the real PHY IPs are children of the package rather than on the same
level with it, I don't think this will be a problem. I wonder if some
address translation could be done with the "ranges" device tree property.
I've never seen this with MDIO though.
> 4. Not finding a correct place to put PHY package info.
>
> I'm still convinced the mdio node is the correct place.
> - PHY package are PHY in bundle so they are actual PHY
> - We already have in the mdio node special handling (every DSA switch
> use custom compatible and PHY ID is not used to probe them
> normally)
> - Node this way won't be treated as PHY as they won't match the PHY
> node name pattern and also won't have the compatible pattern for
> PHY.
>
> Solution:
> - ethernet-phy-package node is OK given a reg is defined.
I agree that it should sit under the MDIO node. I disagree with the idea
of a standardized binding for PHY packages.
On Fri, Nov 24, 2023 at 06:59:23PM +0200, Vladimir Oltean wrote:
> On Wed, Nov 22, 2023 at 07:32:22PM +0100, Christian Marangi wrote:
> > Sooooo.... Sorry if I insist but I would really love to have something
> > ""stable"" to move this further. (the changes are easy enough so it's
> > really a matter of finding a good DT structure)
> >
> > Maybe a good idea would be summarize the concern and see what solution
> > was proposed:
>
> Sorry, I didn't follow the entire discussion. I hope I'm not too far off
> with my understanding of your problems.
>
> I think you are hitting some of the same points I have hit with DSA.
> The PHY package could be considered an SoC with lots of peripherals on
> it, for which you'd want separate drivers. Just like a DSA switch would.
> I don't think it's exactly phylib's place to deal with that, just like
> it's not DSA's place to deal with complex SoCs, just with the switching
> IP (like the Ethernet PHY IP for phylib).
> https://lore.kernel.org/lkml/20221222134844.lbzyx5hz7z5n763n@skbuf/
>
> Why does the ethernet-phy-package DT binding attempt to be so grand and
> generic? I would expect the 180 degree opposite. Make it have a _single_
> compatible of "qcom,qca807x" (but don't use an "x" wildcard, do specify
> the full package name).
>
> Make it have a "reg" property which is the base MDIO address of the package.
>
> Write an mdio_device driver that probes on that. The PHY core already
> knows that if a child on the MDIO bus has a compatible string of the
> normal form (not like "ethernet-phy-id004d.d0b2"), then it's looking at
> an mdio_device.
>
> Make the OF node of the package have an "mdio" child with its own
> compatible string, which represents the place where PHYs are. The driver
> for the "mdio" child has a very simple implementation of the mii_bus
> ops, which just calls into the device parent (it can assume a certain
> parent implementation and private data structures).
>
> Lateral to the "mdio" child node of the "qcom,qca807x" package node, you
> could put any other device tree properties that you want.
>
> Make the mdio_device driver for "qcom,qca807x" use shared code if you
> want - but keep the device tree structure hardware-specific. Look at the
> compatible strings that e.g. the drivers/mfd/simple-mfd-i2c.c driver
> probes on. You could always change the driver implementation for a
> certain compatible string, but you'll be stuck with the ultra-generic
> compatible = "ethernet-phy-package", which has the problems that you
> mention.
>
The main reason is the fact that PHY package are already a thing and API
are already there (phy_package_join/leave...) so we just lack any way to
support this in DT without using specialized code in the PHY driver.
This is really completing the feature.
The only reason for the generic "ethernet-phy-package" compatible is to
have a solid detection of the node in PHY core. (I assume parsing the
node name might be problematic? Or maybe not now that we require adding
a reg to it)
Also I don't expect tons of special properties for PHY package, with the
current probe/config implementation, a PHY driver have lots of
flexibility in all kind of validation.
Consider that the additional global-phys and global-phy-names are
already expected to be dropped.
(we know the PHY package base address and we can calculate the offset of
the global phy from there in the PHY package probe)
And even the phy-mode has been scrapped for more specific solution...
(some assumption can be done on probe by checking the PHY mode and set
stuff accordingly or even do parsing in the PHY package node as we pass
the OF node of the phy package)
The PHY package node would be reduced to a simple compatible (and even
this can be dropped) and a reg.
I feel there is a big chance here to generalize it and prevent any kind
of mess with all kind of similar/equal code that just do the same thing.
(and we already have an example with the PHY package API usage with
every PHY having the same exact pattern for probe/config and nothing
describing that the PHY are actually a package in DT)
Hope it all makes sense to you.
> >
> > Concern list:
> > 1. ethernet-phy-package MUST be placed in mdio node (not in ethernet,
> > the example was wrong anyway) and MUST have an addr
> >
> > Current example doesn't have an addr. I would prefer this way but
> > no problem in changing this.
> >
> > Solution:
> > - Add reg to the ethernet-phy-package node with the base address of
> > the PHY package (base address = the first PHY address of the
> > package)
>
> Correct, what I'm saying is compatible with this.
>
> >
> > We will have a PHY node with the same address of the PHY package
> > node. Each PHY node in the PHY package node will have reg set to
> > the REAL address in the mdio bus.
>
> If the real PHY IPs are children of the package rather than on the same
> level with it, I don't think this will be a problem. I wonder if some
> address translation could be done with the "ranges" device tree property.
> I've never seen this with MDIO though.
>
I can check it, I would love some way to describe the address used by
the PHY package. (since everything will be handled internally with
offsets, would be good to define in DT that (for example) addrs from 0
to 5 are used). Some PHY might be not attached but still used for global
configuration of the PHY package.
> > 4. Not finding a correct place to put PHY package info.
> >
> > I'm still convinced the mdio node is the correct place.
> > - PHY package are PHY in bundle so they are actual PHY
> > - We already have in the mdio node special handling (every DSA switch
> > use custom compatible and PHY ID is not used to probe them
> > normally)
> > - Node this way won't be treated as PHY as they won't match the PHY
> > node name pattern and also won't have the compatible pattern for
> > PHY.
> >
> > Solution:
> > - ethernet-phy-package node is OK given a reg is defined.
>
> I agree that it should sit under the MDIO node. I disagree with the idea
> of a standardized binding for PHY packages.
--
Ansuel
On Fri, Nov 24, 2023 at 05:25:16PM +0100, Christian Marangi wrote:
> The main reason is the fact that PHY package are already a thing and API
> are already there (phy_package_join/leave...) so we just lack any way to
> support this in DT without using specialized code in the PHY driver.
>
> This is really completing the feature.
Hmm, I see struct phy_package_shared as a mechanism to tell phylib that
multiple device structures are actually related with each other, because
the device core, and their parent bus, has no idea. If you're under
control of the parent bus code and you can probe PHY devices in any
order you want and do whatever you want before probing them, I don't see
why you would need struct phy_package_shared any longer? I don't see why
this feature needs to be completed, if that involves changes to the
device tree structure. PHY packages assumed no changes to the device
tree (they rely on a hacky interpretation of the MDIO address AFAIU).
If we change that basic premise, all implementation options are on the
table, I think.
> The only reason for the generic "ethernet-phy-package" compatible is to
> have a solid detection of the node in PHY core. (I assume parsing the
> node name might be problematic? Or maybe not now that we require adding
> a reg to it)
Our opinions seem to differ, but I don't think that the package needs a
solid detection of the node in the PHY core :) I think phy_devices and
mdio_devices already cover everything that's necessary to build a
solution.
> Also I don't expect tons of special properties for PHY package, with the
> current probe/config implementation, a PHY driver have lots of
> flexibility in all kind of validation.
>
> Consider that the additional global-phys and global-phy-names are
> already expected to be dropped.
> (we know the PHY package base address and we can calculate the offset of
> the global phy from there in the PHY package probe)
>
> And even the phy-mode has been scrapped for more specific solution...
> (some assumption can be done on probe by checking the PHY mode and set
> stuff accordingly or even do parsing in the PHY package node as we pass
> the OF node of the phy package)
>
> The PHY package node would be reduced to a simple compatible (and even
> this can be dropped) and a reg.
So why does it need to be described in DT, at this stage? :)
> I feel there is a big chance here to generalize it and prevent any kind
> of mess with all kind of similar/equal code that just do the same thing.
> (and we already have an example with the PHY package API usage with
> every PHY having the same exact pattern for probe/config and nothing
> describing that the PHY are actually a package in DT)
>
> Hope it all makes sense to you.
> I think you are hitting some of the same points I have hit with DSA.
> The PHY package could be considered an SoC with lots of peripherals on
> it, for which you'd want separate drivers.
At least at the moment, this is not true. The package does just
contain PHYs. But it also has some properties which are shared across
those PHYs, e.g. reset.
What you describe might become true in the future. e.g. The LED/GPIO
controller is currently part of the PHY, and each PHY has its own. I
could however imagine that becomes a block of its own, outside of the
PHY address space, and maybe it might want its own class LED
driver. Some PHYs have temperature sensors, which could be a package
sensor, so could in theory be an individual hwmon driver. However,
i've not yet seen such a package.
Do we consider this now? At the moment i don't see an MFD style system
is required. We could crystal ball gaze and come up with some
requirements, but i would prefer to have some real devices and
datasheets. Without them, we will get the requirements wrong.
I also think we are not that far away from it, in terms of DT, if you
consider the later comments. I suggested we need a phy package
specific compatible. At the moment, it will be ignored by the kernel,
the kernel does not need it, it probes the PHYs in the current way,
using the ID registers. But it could in future be used to probe a real
driver, which could be an MFD style driver. We need to see updated DT
binding examples, but i don't see why we cannot slot it in at a later
date.
Andrew
On Fri, Nov 24, 2023 at 07:35:35PM +0100, Andrew Lunn wrote:
> > I think you are hitting some of the same points I have hit with DSA.
> > The PHY package could be considered an SoC with lots of peripherals on
> > it, for which you'd want separate drivers.
>
> At least at the moment, this is not true. The package does just
> contain PHYs. But it also has some properties which are shared across
> those PHYs, e.g. reset.
>
> What you describe might become true in the future. e.g. The LED/GPIO
> controller is currently part of the PHY, and each PHY has its own. I
> could however imagine that becomes a block of its own, outside of the
> PHY address space, and maybe it might want its own class LED
> driver. Some PHYs have temperature sensors, which could be a package
> sensor, so could in theory be an individual hwmon driver. However,
> i've not yet seen such a package.
>
> Do we consider this now? At the moment i don't see an MFD style system
> is required. We could crystal ball gaze and come up with some
> requirements, but i would prefer to have some real devices and
> datasheets. Without them, we will get the requirements wrong.
>
> I also think we are not that far away from it, in terms of DT, if you
> consider the later comments. I suggested we need a phy package
> specific compatible. At the moment, it will be ignored by the kernel,
> the kernel does not need it, it probes the PHYs in the current way,
> using the ID registers. But it could in future be used to probe a real
> driver, which could be an MFD style driver. We need to see updated DT
> binding examples, but i don't see why we cannot slot it in at a later
> date.
I'm not suggesting to go for MFD right away. Just with a structure that
is extensible to possibly cover that. For now, a package node with a
Qualcomm compatible, with the most minimal driver that forwards MDIO
access to PHY children.
I can't speak for the future of PHY drivers, since I don't know enough
about PHYs. I'm just coming from the DSA background where I really wish
we had this sort of infrastructure earlier. Now I have the SJA1110 which
still lacks support for the interrupt controller for its integrated
PHYs, and a bunch of other IP blocks in the package, because it's so
incredibly hard to make the driver support the old-style and the
new-style device trees.