EyeQ5 embeds a system-controller called OLB. It features many unrelated
registers, and some of those are registers used to configure the
integration of the RGMII/SGMII Cadence PHY used by MACB/GEM instances.
Wrap in a neat generic PHY provider, exposing two PHYs with standard
phy_init() / phy_set_mode() / phy_power_on() operations.
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
MAINTAINERS | 1 +
drivers/phy/Kconfig | 13 +++
drivers/phy/Makefile | 1 +
drivers/phy/phy-eyeq5-eth.c | 249 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 264 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 5b11839cba9d..2f67ec9fad57 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17605,6 +17605,7 @@ F: arch/mips/boot/dts/mobileye/
F: arch/mips/configs/eyeq5_defconfig
F: arch/mips/mobileye/board-epm5.its.S
F: drivers/clk/clk-eyeq.c
+F: drivers/phy/phy-eyeq5-eth.c
F: drivers/pinctrl/pinctrl-eyeq5.c
F: drivers/reset/reset-eyeq.c
F: include/dt-bindings/clock/mobileye,eyeq5-clk.h
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 678dd0452f0a..1aa6eff12dbc 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -101,6 +101,19 @@ config PHY_NXP_PTN3222
schemes. It supports all three USB 2.0 data rates: Low Speed, Full
Speed and High Speed.
+config PHY_EYEQ5_ETH
+ tristate "Ethernet PHY Driver on EyeQ5"
+ depends on OF
+ depends on MACH_EYEQ5 || COMPILE_TEST
+ select AUXILIARY_BUS
+ select GENERIC_PHY
+ default MACH_EYEQ5
+ help
+ Enable this to support the Ethernet PHY integrated on EyeQ5.
+ It supports both RGMII and SGMII. Registers are located in a
+ shared register region called OLB. If M is selected, the
+ module will be called phy-eyeq5-eth.
+
source "drivers/phy/allwinner/Kconfig"
source "drivers/phy/amlogic/Kconfig"
source "drivers/phy/broadcom/Kconfig"
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index bfb27fb5a494..8289497ece55 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_PHY_SNPS_EUSB2) += phy-snps-eusb2.o
obj-$(CONFIG_USB_LGM_PHY) += phy-lgm-usb.o
obj-$(CONFIG_PHY_AIROHA_PCIE) += phy-airoha-pcie.o
obj-$(CONFIG_PHY_NXP_PTN3222) += phy-nxp-ptn3222.o
+obj-$(CONFIG_PHY_EYEQ5_ETH) += phy-eyeq5-eth.o
obj-y += allwinner/ \
amlogic/ \
broadcom/ \
diff --git a/drivers/phy/phy-eyeq5-eth.c b/drivers/phy/phy-eyeq5-eth.c
new file mode 100644
index 000000000000..6e28f7e24835
--- /dev/null
+++ b/drivers/phy/phy-eyeq5-eth.c
@@ -0,0 +1,249 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/auxiliary_bus.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/gfp_types.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy.h>
+#include <linux/phy/phy.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#define EQ5_PHY_COUNT 2
+
+#define EQ5_PHY0_GP 0x128
+#define EQ5_PHY1_GP 0x12c
+#define EQ5_PHY0_SGMII 0x134
+#define EQ5_PHY1_SGMII 0x138
+
+#define EQ5_GP_TX_SWRST_DIS BIT(0) // Tx SW reset
+#define EQ5_GP_TX_M_CLKE BIT(1) // Tx M clock enable
+#define EQ5_GP_SYS_SWRST_DIS BIT(2) // Sys SW reset
+#define EQ5_GP_SYS_M_CLKE BIT(3) // Sys clock enable
+#define EQ5_GP_SGMII_MODE BIT(4) // SGMII mode
+#define EQ5_GP_RGMII_DRV GENMASK(8, 5) // RGMII drive strength
+
+#define EQ5_SGMII_PWR_EN BIT(0)
+#define EQ5_SGMII_RST_DIS BIT(1)
+#define EQ5_SGMII_PLL_EN BIT(2)
+#define EQ5_SGMII_SIG_DET_SW BIT(3)
+#define EQ5_SGMII_PWR_STATE BIT(4)
+#define EQ5_SGMII_PLL_ACK BIT(18)
+#define EQ5_SGMII_PWR_STATE_ACK GENMASK(24, 20)
+
+struct eq5_phy_inst {
+ struct eq5_phy_private *priv;
+ struct phy *phy;
+ void __iomem *gp, *sgmii;
+ phy_interface_t phy_interface;
+};
+
+struct eq5_phy_private {
+ struct device *dev;
+ struct eq5_phy_inst phys[EQ5_PHY_COUNT];
+};
+
+static int eq5_phy_init(struct phy *phy)
+{
+ struct eq5_phy_inst *inst = phy_get_drvdata(phy);
+ struct eq5_phy_private *priv = inst->priv;
+ struct device *dev = priv->dev;
+ u32 reg;
+
+ dev_dbg(dev, "phy_init(inst=%td)\n", inst - priv->phys);
+
+ writel(0, inst->gp);
+ writel(0, inst->sgmii);
+
+ udelay(5);
+
+ reg = readl(inst->gp) | EQ5_GP_TX_SWRST_DIS | EQ5_GP_TX_M_CLKE |
+ EQ5_GP_SYS_SWRST_DIS | EQ5_GP_SYS_M_CLKE |
+ FIELD_PREP(EQ5_GP_RGMII_DRV, 0x9);
+ writel(reg, inst->gp);
+
+ return 0;
+}
+
+static int eq5_phy_exit(struct phy *phy)
+{
+ struct eq5_phy_inst *inst = phy_get_drvdata(phy);
+ struct eq5_phy_private *priv = inst->priv;
+ struct device *dev = priv->dev;
+
+ dev_dbg(dev, "phy_exit(inst=%td)\n", inst - priv->phys);
+
+ writel(0, inst->gp);
+ writel(0, inst->sgmii);
+ udelay(5);
+
+ return 0;
+}
+
+static int eq5_phy_set_mode(struct phy *phy, enum phy_mode mode, int submode)
+{
+ struct eq5_phy_inst *inst = phy_get_drvdata(phy);
+ struct eq5_phy_private *priv = inst->priv;
+ struct device *dev = priv->dev;
+
+ dev_dbg(dev, "phy_set_mode(inst=%td, mode=%d, submode=%d)\n",
+ inst - priv->phys, mode, submode);
+
+ if (mode != PHY_MODE_ETHERNET)
+ return -EOPNOTSUPP;
+
+ if (!phy_interface_mode_is_rgmii(submode) &&
+ submode != PHY_INTERFACE_MODE_SGMII)
+ return -EOPNOTSUPP;
+
+ inst->phy_interface = submode;
+ return 0;
+}
+
+static int eq5_phy_power_on(struct phy *phy)
+{
+ struct eq5_phy_inst *inst = phy_get_drvdata(phy);
+ struct eq5_phy_private *priv = inst->priv;
+ struct device *dev = priv->dev;
+ u32 reg;
+
+ dev_dbg(dev, "phy_power_on(inst=%td)\n", inst - priv->phys);
+
+ if (inst->phy_interface == PHY_INTERFACE_MODE_SGMII) {
+ writel(readl(inst->gp) | EQ5_GP_SGMII_MODE, inst->gp);
+
+ reg = EQ5_SGMII_PWR_EN | EQ5_SGMII_RST_DIS | EQ5_SGMII_PLL_EN;
+ writel(reg, inst->sgmii);
+
+ if (readl_poll_timeout(inst->sgmii, reg,
+ reg & EQ5_SGMII_PLL_ACK, 1, 100)) {
+ dev_err(dev, "PLL timeout\n");
+ return -ETIMEDOUT;
+ }
+
+ reg = readl(inst->sgmii);
+ reg |= EQ5_SGMII_PWR_STATE | EQ5_SGMII_SIG_DET_SW;
+ writel(reg, inst->sgmii);
+ } else {
+ writel(readl(inst->gp) & ~EQ5_GP_SGMII_MODE, inst->gp);
+ writel(0, inst->sgmii);
+ }
+
+ return 0;
+}
+
+static int eq5_phy_power_off(struct phy *phy)
+{
+ struct eq5_phy_inst *inst = phy_get_drvdata(phy);
+ struct eq5_phy_private *priv = inst->priv;
+ struct device *dev = priv->dev;
+
+ dev_dbg(dev, "phy_power_off(inst=%td)\n", inst - priv->phys);
+
+ writel(readl(inst->gp) & ~EQ5_GP_SGMII_MODE, inst->gp);
+ writel(0, inst->sgmii);
+
+ return 0;
+}
+
+static const struct phy_ops eq5_phy_ops = {
+ .init = eq5_phy_init,
+ .exit = eq5_phy_exit,
+ .set_mode = eq5_phy_set_mode,
+ .power_on = eq5_phy_power_on,
+ .power_off = eq5_phy_power_off,
+};
+
+static struct phy *eq5_phy_xlate(struct device *dev,
+ const struct of_phandle_args *args)
+{
+ struct eq5_phy_private *priv = dev_get_drvdata(dev);
+
+ if (args->args_count != 1 || args->args[0] >= EQ5_PHY_COUNT)
+ return ERR_PTR(-EINVAL);
+
+ return priv->phys[args->args[0]].phy;
+}
+
+static int eq5_phy_probe_phy(struct eq5_phy_private *priv, unsigned int index,
+ void __iomem *base, unsigned int gp,
+ unsigned int sgmii)
+{
+ struct eq5_phy_inst *inst = &priv->phys[index];
+ struct device *dev = priv->dev;
+ struct phy *phy;
+
+ phy = devm_phy_create(dev, dev->of_node, &eq5_phy_ops);
+ if (IS_ERR(phy))
+ return dev_err_probe(dev, PTR_ERR(phy),
+ "failed to create PHY %u\n", index);
+
+ inst->priv = priv;
+ inst->phy = phy;
+ inst->gp = base + gp;
+ inst->sgmii = base + sgmii;
+ inst->phy_interface = PHY_INTERFACE_MODE_NA;
+ phy_set_drvdata(phy, inst);
+
+ return 0;
+}
+
+static int eq5_phy_probe(struct auxiliary_device *adev,
+ const struct auxiliary_device_id *id)
+{
+ struct device *dev = &adev->dev;
+ struct phy_provider *provider;
+ struct eq5_phy_private *priv;
+ void __iomem *base;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->dev = dev;
+ dev_set_drvdata(dev, priv);
+
+ base = (void __iomem *)dev_get_platdata(dev);
+
+ ret = eq5_phy_probe_phy(priv, 0, base, EQ5_PHY0_GP, EQ5_PHY0_SGMII);
+ if (ret)
+ return ret;
+
+ ret = eq5_phy_probe_phy(priv, 1, base, EQ5_PHY1_GP, EQ5_PHY1_SGMII);
+ if (ret)
+ return ret;
+
+ provider = devm_of_phy_provider_register(dev, eq5_phy_xlate);
+ if (IS_ERR(provider))
+ return dev_err_probe(dev, PTR_ERR(provider),
+ "registering provider failed\n");
+
+ return 0;
+}
+
+static const struct auxiliary_device_id eq5_phy_id_table[] = {
+ { .name = "clk_eyeq.phy" },
+ {}
+};
+MODULE_DEVICE_TABLE(auxiliary, eq5_phy_id_table);
+
+static struct auxiliary_driver eq5_phy_driver = {
+ .probe = eq5_phy_probe,
+ .id_table = eq5_phy_id_table,
+};
+module_auxiliary_driver(eq5_phy_driver);
+
+MODULE_DESCRIPTION("EyeQ5 Ethernet PHY driver");
+MODULE_AUTHOR("Théo Lebrun <theo.lebrun@bootlin.com>");
+MODULE_LICENSE("GPL");
--
2.52.0
On 15-12-25, 17:26, Théo Lebrun wrote:
> EyeQ5 embeds a system-controller called OLB. It features many unrelated
> registers, and some of those are registers used to configure the
> integration of the RGMII/SGMII Cadence PHY used by MACB/GEM instances.
>
> Wrap in a neat generic PHY provider, exposing two PHYs with standard
> phy_init() / phy_set_mode() / phy_power_on() operations.
>
> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
> MAINTAINERS | 1 +
> drivers/phy/Kconfig | 13 +++
> drivers/phy/Makefile | 1 +
> drivers/phy/phy-eyeq5-eth.c | 249 ++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 264 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5b11839cba9d..2f67ec9fad57 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17605,6 +17605,7 @@ F: arch/mips/boot/dts/mobileye/
> F: arch/mips/configs/eyeq5_defconfig
> F: arch/mips/mobileye/board-epm5.its.S
> F: drivers/clk/clk-eyeq.c
> +F: drivers/phy/phy-eyeq5-eth.c
> F: drivers/pinctrl/pinctrl-eyeq5.c
> F: drivers/reset/reset-eyeq.c
> F: include/dt-bindings/clock/mobileye,eyeq5-clk.h
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 678dd0452f0a..1aa6eff12dbc 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -101,6 +101,19 @@ config PHY_NXP_PTN3222
> schemes. It supports all three USB 2.0 data rates: Low Speed, Full
> Speed and High Speed.
>
> +config PHY_EYEQ5_ETH
sorted please
> + tristate "Ethernet PHY Driver on EyeQ5"
> + depends on OF
> + depends on MACH_EYEQ5 || COMPILE_TEST
> + select AUXILIARY_BUS
> + select GENERIC_PHY
> + default MACH_EYEQ5
hmmm why should it be default? Maybe add this is respective defconfig for
platform instead..?
> + help
> + Enable this to support the Ethernet PHY integrated on EyeQ5.
> + It supports both RGMII and SGMII. Registers are located in a
> + shared register region called OLB. If M is selected, the
> + module will be called phy-eyeq5-eth.
> +
> source "drivers/phy/allwinner/Kconfig"
> source "drivers/phy/amlogic/Kconfig"
> source "drivers/phy/broadcom/Kconfig"
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index bfb27fb5a494..8289497ece55 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_PHY_SNPS_EUSB2) += phy-snps-eusb2.o
> obj-$(CONFIG_USB_LGM_PHY) += phy-lgm-usb.o
> obj-$(CONFIG_PHY_AIROHA_PCIE) += phy-airoha-pcie.o
> obj-$(CONFIG_PHY_NXP_PTN3222) += phy-nxp-ptn3222.o
> +obj-$(CONFIG_PHY_EYEQ5_ETH) += phy-eyeq5-eth.o
sorted please
> obj-y += allwinner/ \
> amlogic/ \
> broadcom/ \
> diff --git a/drivers/phy/phy-eyeq5-eth.c b/drivers/phy/phy-eyeq5-eth.c
> new file mode 100644
> index 000000000000..6e28f7e24835
> --- /dev/null
> +++ b/drivers/phy/phy-eyeq5-eth.c
> @@ -0,0 +1,249 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/auxiliary_bus.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/gfp_types.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/phy.h>
> +#include <linux/phy/phy.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +
> +#define EQ5_PHY_COUNT 2
> +
> +#define EQ5_PHY0_GP 0x128
> +#define EQ5_PHY1_GP 0x12c
> +#define EQ5_PHY0_SGMII 0x134
> +#define EQ5_PHY1_SGMII 0x138
> +
> +#define EQ5_GP_TX_SWRST_DIS BIT(0) // Tx SW reset
> +#define EQ5_GP_TX_M_CLKE BIT(1) // Tx M clock enable
> +#define EQ5_GP_SYS_SWRST_DIS BIT(2) // Sys SW reset
> +#define EQ5_GP_SYS_M_CLKE BIT(3) // Sys clock enable
> +#define EQ5_GP_SGMII_MODE BIT(4) // SGMII mode
> +#define EQ5_GP_RGMII_DRV GENMASK(8, 5) // RGMII drive strength
> +
> +#define EQ5_SGMII_PWR_EN BIT(0)
> +#define EQ5_SGMII_RST_DIS BIT(1)
> +#define EQ5_SGMII_PLL_EN BIT(2)
> +#define EQ5_SGMII_SIG_DET_SW BIT(3)
> +#define EQ5_SGMII_PWR_STATE BIT(4)
> +#define EQ5_SGMII_PLL_ACK BIT(18)
> +#define EQ5_SGMII_PWR_STATE_ACK GENMASK(24, 20)
> +
> +struct eq5_phy_inst {
> + struct eq5_phy_private *priv;
> + struct phy *phy;
> + void __iomem *gp, *sgmii;
> + phy_interface_t phy_interface;
> +};
> +
> +struct eq5_phy_private {
> + struct device *dev;
> + struct eq5_phy_inst phys[EQ5_PHY_COUNT];
> +};
> +
> +static int eq5_phy_init(struct phy *phy)
> +{
> + struct eq5_phy_inst *inst = phy_get_drvdata(phy);
> + struct eq5_phy_private *priv = inst->priv;
> + struct device *dev = priv->dev;
> + u32 reg;
> +
> + dev_dbg(dev, "phy_init(inst=%td)\n", inst - priv->phys);
> +
> + writel(0, inst->gp);
> + writel(0, inst->sgmii);
> +
> + udelay(5);
> +
> + reg = readl(inst->gp) | EQ5_GP_TX_SWRST_DIS | EQ5_GP_TX_M_CLKE |
> + EQ5_GP_SYS_SWRST_DIS | EQ5_GP_SYS_M_CLKE |
> + FIELD_PREP(EQ5_GP_RGMII_DRV, 0x9);
> + writel(reg, inst->gp);
> +
> + return 0;
> +}
> +
> +static int eq5_phy_exit(struct phy *phy)
> +{
> + struct eq5_phy_inst *inst = phy_get_drvdata(phy);
> + struct eq5_phy_private *priv = inst->priv;
> + struct device *dev = priv->dev;
> +
> + dev_dbg(dev, "phy_exit(inst=%td)\n", inst - priv->phys);
> +
> + writel(0, inst->gp);
> + writel(0, inst->sgmii);
> + udelay(5);
this is same patter in init as well...?
> +
> + return 0;
> +}
> +
> +static int eq5_phy_set_mode(struct phy *phy, enum phy_mode mode, int submode)
> +{
> + struct eq5_phy_inst *inst = phy_get_drvdata(phy);
> + struct eq5_phy_private *priv = inst->priv;
> + struct device *dev = priv->dev;
> +
> + dev_dbg(dev, "phy_set_mode(inst=%td, mode=%d, submode=%d)\n",
> + inst - priv->phys, mode, submode);
these are good for debug but not for upstream, please drop
> +
> + if (mode != PHY_MODE_ETHERNET)
> + return -EOPNOTSUPP;
> +
> + if (!phy_interface_mode_is_rgmii(submode) &&
> + submode != PHY_INTERFACE_MODE_SGMII)
> + return -EOPNOTSUPP;
> +
> + inst->phy_interface = submode;
> + return 0;
> +}
> +
> +static int eq5_phy_power_on(struct phy *phy)
> +{
> + struct eq5_phy_inst *inst = phy_get_drvdata(phy);
> + struct eq5_phy_private *priv = inst->priv;
> + struct device *dev = priv->dev;
> + u32 reg;
> +
> + dev_dbg(dev, "phy_power_on(inst=%td)\n", inst - priv->phys);
> +
> + if (inst->phy_interface == PHY_INTERFACE_MODE_SGMII) {
> + writel(readl(inst->gp) | EQ5_GP_SGMII_MODE, inst->gp);
> +
> + reg = EQ5_SGMII_PWR_EN | EQ5_SGMII_RST_DIS | EQ5_SGMII_PLL_EN;
> + writel(reg, inst->sgmii);
> +
> + if (readl_poll_timeout(inst->sgmii, reg,
> + reg & EQ5_SGMII_PLL_ACK, 1, 100)) {
> + dev_err(dev, "PLL timeout\n");
> + return -ETIMEDOUT;
> + }
> +
> + reg = readl(inst->sgmii);
> + reg |= EQ5_SGMII_PWR_STATE | EQ5_SGMII_SIG_DET_SW;
> + writel(reg, inst->sgmii);
> + } else {
> + writel(readl(inst->gp) & ~EQ5_GP_SGMII_MODE, inst->gp);
> + writel(0, inst->sgmii);
> + }
> +
> + return 0;
> +}
> +
> +static int eq5_phy_power_off(struct phy *phy)
> +{
> + struct eq5_phy_inst *inst = phy_get_drvdata(phy);
> + struct eq5_phy_private *priv = inst->priv;
> + struct device *dev = priv->dev;
> +
> + dev_dbg(dev, "phy_power_off(inst=%td)\n", inst - priv->phys);
> +
> + writel(readl(inst->gp) & ~EQ5_GP_SGMII_MODE, inst->gp);
> + writel(0, inst->sgmii);
> +
> + return 0;
> +}
> +
> +static const struct phy_ops eq5_phy_ops = {
> + .init = eq5_phy_init,
> + .exit = eq5_phy_exit,
> + .set_mode = eq5_phy_set_mode,
> + .power_on = eq5_phy_power_on,
> + .power_off = eq5_phy_power_off,
> +};
> +
> +static struct phy *eq5_phy_xlate(struct device *dev,
> + const struct of_phandle_args *args)
> +{
> + struct eq5_phy_private *priv = dev_get_drvdata(dev);
> +
> + if (args->args_count != 1 || args->args[0] >= EQ5_PHY_COUNT)
> + return ERR_PTR(-EINVAL);
> +
> + return priv->phys[args->args[0]].phy;
> +}
> +
> +static int eq5_phy_probe_phy(struct eq5_phy_private *priv, unsigned int index,
> + void __iomem *base, unsigned int gp,
> + unsigned int sgmii)
> +{
> + struct eq5_phy_inst *inst = &priv->phys[index];
> + struct device *dev = priv->dev;
> + struct phy *phy;
> +
> + phy = devm_phy_create(dev, dev->of_node, &eq5_phy_ops);
> + if (IS_ERR(phy))
> + return dev_err_probe(dev, PTR_ERR(phy),
> + "failed to create PHY %u\n", index);
> +
> + inst->priv = priv;
> + inst->phy = phy;
> + inst->gp = base + gp;
> + inst->sgmii = base + sgmii;
> + inst->phy_interface = PHY_INTERFACE_MODE_NA;
> + phy_set_drvdata(phy, inst);
> +
> + return 0;
> +}
> +
> +static int eq5_phy_probe(struct auxiliary_device *adev,
> + const struct auxiliary_device_id *id)
> +{
> + struct device *dev = &adev->dev;
> + struct phy_provider *provider;
> + struct eq5_phy_private *priv;
> + void __iomem *base;
> + int ret;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->dev = dev;
> + dev_set_drvdata(dev, priv);
> +
> + base = (void __iomem *)dev_get_platdata(dev);
no need to cast for void *
--
~Vinod
Hello Vinod,
On Tue Dec 23, 2025 at 4:53 PM CET, Vinod Koul wrote:
> On 15-12-25, 17:26, Théo Lebrun wrote:
>> EyeQ5 embeds a system-controller called OLB. It features many unrelated
>> registers, and some of those are registers used to configure the
>> integration of the RGMII/SGMII Cadence PHY used by MACB/GEM instances.
>>
>> Wrap in a neat generic PHY provider, exposing two PHYs with standard
>> phy_init() / phy_set_mode() / phy_power_on() operations.
>>
>> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
>> ---
>> MAINTAINERS | 1 +
>> drivers/phy/Kconfig | 13 +++
>> drivers/phy/Makefile | 1 +
>> drivers/phy/phy-eyeq5-eth.c | 249 ++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 264 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 5b11839cba9d..2f67ec9fad57 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -17605,6 +17605,7 @@ F: arch/mips/boot/dts/mobileye/
>> F: arch/mips/configs/eyeq5_defconfig
>> F: arch/mips/mobileye/board-epm5.its.S
>> F: drivers/clk/clk-eyeq.c
>> +F: drivers/phy/phy-eyeq5-eth.c
>> F: drivers/pinctrl/pinctrl-eyeq5.c
>> F: drivers/reset/reset-eyeq.c
>> F: include/dt-bindings/clock/mobileye,eyeq5-clk.h
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index 678dd0452f0a..1aa6eff12dbc 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -101,6 +101,19 @@ config PHY_NXP_PTN3222
>> schemes. It supports all three USB 2.0 data rates: Low Speed, Full
>> Speed and High Speed.
>>
>> +config PHY_EYEQ5_ETH
>
> sorted please
I wouldn't mind, but entries are currently (v6.19-rc4) not sorted:
$ diff -U100 <(grep ^config drivers/phy/Kconfig) \
<(grep ^config drivers/phy/Kconfig | sort)
--- /dev/fd/63 2026-01-05 15:55:53.891922890 +0100
+++ /dev/fd/62 2026-01-05 15:55:53.891922890 +0100
@@ -1,11 +1,11 @@
config GENERIC_PHY
config GENERIC_PHY_MIPI_DPHY
+config PHY_AIROHA_PCIE
+config PHY_CAN_TRANSCEIVER
+config PHY_EYEQ5_ETH
config PHY_LPC18XX_USB_OTG
+config PHY_NXP_PTN3222
config PHY_PISTACHIO_USB
config PHY_SNPS_EUSB2
config PHY_XGENE
config USB_LGM_PHY
-config PHY_CAN_TRANSCEIVER
-config PHY_AIROHA_PCIE
-config PHY_NXP_PTN3222
-config PHY_EYEQ5_ETH
This is why I appended. In V2, I'll send a first patch to reorder
entries to then be able to add PHY_EYEQ5_ETH in the correct location.
>> + tristate "Ethernet PHY Driver on EyeQ5"
>> + depends on OF
>> + depends on MACH_EYEQ5 || COMPILE_TEST
>> + select AUXILIARY_BUS
>> + select GENERIC_PHY
>> + default MACH_EYEQ5
>
> hmmm why should it be default? Maybe add this is respective defconfig for
> platform instead..?
We have been doing this for other parts of OLB. If you are doing a
config for EyeQ5, most probably you want this enabled.
One example usecase this default field makes config easier: when you
migrate an eyeq* config to eyeq5 without resetting the full config by
applying eyeq5_defconfig. With default field you get this driver
enabled, otherwise you don't and Ethernet doesn't work.
I'd prefer keeping this default but we can drop it if you lean strongly
against it.
>> + help
>> + Enable this to support the Ethernet PHY integrated on EyeQ5.
>> + It supports both RGMII and SGMII. Registers are located in a
>> + shared register region called OLB. If M is selected, the
>> + module will be called phy-eyeq5-eth.
>> +
>> source "drivers/phy/allwinner/Kconfig"
>> source "drivers/phy/amlogic/Kconfig"
>> source "drivers/phy/broadcom/Kconfig"
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index bfb27fb5a494..8289497ece55 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -13,6 +13,7 @@ obj-$(CONFIG_PHY_SNPS_EUSB2) += phy-snps-eusb2.o
>> obj-$(CONFIG_USB_LGM_PHY) += phy-lgm-usb.o
>> obj-$(CONFIG_PHY_AIROHA_PCIE) += phy-airoha-pcie.o
>> obj-$(CONFIG_PHY_NXP_PTN3222) += phy-nxp-ptn3222.o
>> +obj-$(CONFIG_PHY_EYEQ5_ETH) += phy-eyeq5-eth.o
>
> sorted please
Same as for Kconfig. Will do it in two steps: first sort, then add the
CONFIG_PHY_EYEQ5_ETH line.
$ diff -U100 <(grep ^obj-\\$ drivers/phy/Makefile) \
<(grep ^obj-\\$ drivers/phy/Makefile | sort)
--- /dev/fd/63 2026-01-05 16:11:10.977537425 +0100
+++ /dev/fd/62 2026-01-05 16:11:10.978537396 +0100
@@ -1,11 +1,11 @@
-obj-$(CONFIG_GENERIC_PHY) += phy-core.o
obj-$(CONFIG_GENERIC_PHY_MIPI_DPHY) += phy-core-mipi-dphy.o
+obj-$(CONFIG_GENERIC_PHY) += phy-core.o
+obj-$(CONFIG_PHY_AIROHA_PCIE) += phy-airoha-pcie.o
obj-$(CONFIG_PHY_CAN_TRANSCEIVER) += phy-can-transceiver.o
+obj-$(CONFIG_PHY_EYEQ5_ETH) += phy-eyeq5-eth.o
obj-$(CONFIG_PHY_LPC18XX_USB_OTG) += phy-lpc18xx-usb-otg.o
-obj-$(CONFIG_PHY_XGENE) += phy-xgene.o
+obj-$(CONFIG_PHY_NXP_PTN3222) += phy-nxp-ptn3222.o
obj-$(CONFIG_PHY_PISTACHIO_USB) += phy-pistachio-usb.o
obj-$(CONFIG_PHY_SNPS_EUSB2) += phy-snps-eusb2.o
+obj-$(CONFIG_PHY_XGENE) += phy-xgene.o
obj-$(CONFIG_USB_LGM_PHY) += phy-lgm-usb.o
-obj-$(CONFIG_PHY_AIROHA_PCIE) += phy-airoha-pcie.o
-obj-$(CONFIG_PHY_NXP_PTN3222) += phy-nxp-ptn3222.o
-obj-$(CONFIG_PHY_EYEQ5_ETH) += phy-eyeq5-eth.o
[...]
>> +static int eq5_phy_exit(struct phy *phy)
>> +{
>> + struct eq5_phy_inst *inst = phy_get_drvdata(phy);
>> + struct eq5_phy_private *priv = inst->priv;
>> + struct device *dev = priv->dev;
>> +
>> + dev_dbg(dev, "phy_exit(inst=%td)\n", inst - priv->phys);
>> +
>> + writel(0, inst->gp);
>> + writel(0, inst->sgmii);
>> + udelay(5);
>
> this is same patter in init as well...?
Yes! phy_ops::init() must reinit the HW to ensure its config fields
are well taken into account. We might inherit an already initialised
PHY from the bootloader.
We could in theory move those three ops (writel+writel+udelay) into a
helper function but I feel like it would decrease readability without
increasing code quality.
>> +
>> + return 0;
>> +}
>> +
>> +static int eq5_phy_set_mode(struct phy *phy, enum phy_mode mode, int submode)
>> +{
>> + struct eq5_phy_inst *inst = phy_get_drvdata(phy);
>> + struct eq5_phy_private *priv = inst->priv;
>> + struct device *dev = priv->dev;
>> +
>> + dev_dbg(dev, "phy_set_mode(inst=%td, mode=%d, submode=%d)\n",
>> + inst - priv->phys, mode, submode);
>
> these are good for debug but not for upstream, please drop
Ah this is surprising! They helped me debug this driver and fix one bug
or two so I thought I'd leave them in. They get compiled out by
default. And there is no ftrace events equivalent which would make
those dev_dbg() moot.
⟩ git grep -F dev_dbg\( drivers/ | wc -l
25174
⟩ git grep -F dev_dbg\( drivers/phy/ | wc -l
260
[...]
>> +static int eq5_phy_probe(struct auxiliary_device *adev,
>> + const struct auxiliary_device_id *id)
>> +{
>> + struct device *dev = &adev->dev;
>> + struct phy_provider *provider;
>> + struct eq5_phy_private *priv;
>> + void __iomem *base;
>> + int ret;
>> +
>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + priv->dev = dev;
>> + dev_set_drvdata(dev, priv);
>> +
>> + base = (void __iomem *)dev_get_platdata(dev);
>
> no need to cast for void *
Yes! My initial goal was to prevent a sparse warning about the implicit
cast from `void *` as returned by dev_get_platdata() and
`void __iomem *base`.
But it does not matter in our case (and the correct solution for
explicit cast would have implied __force).
--
I'll wait for feedback on `default MACH_EYEQ5` and `dev_dbg()` before
sending the next revision.
Thanks for the review!
--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
© 2016 - 2026 Red Hat, Inc.