This is attempt to support Ethernet PHY on AC200 MFD chip. I'm sending
this as RFC because I stumbled on a problem how to properly describe it
in DT. Proper DT documentation will be added later, once DT issue is
solved.
Before Ethernet PHY can be actually used, few things must happen:
1. 24 MHz clock must be enabled and connected to input pin of this
chip. In this case, PWM is set to generate 24 MHz signal with 50%
duty cycle.
2. Chip must be put out of reset through I2C
3. Ethernet PHY must be enabled and configured through I2C
All above suggest that AC200 chip must be child node of I2C and Ethernet
PHY child node of AC200 node. This is done in patch 3.
However, mdio bus binding also requires that ethernet PHY is child node
of mdio bus node which can't be, because it's already child node of
AC200 MFD node.
Currently I'm using workaround to have another PHY defined in mdio bus
node as can be seen in patch 4. Then, with careful module loading order,
I make sure that ethernet controller driver is loaded last, after AC200
ethernet PHY driver is loaded. But that's fragile and not acceptable.
Suggestions how to solve that are highly appreciated.
One possible solution is that mdio bus node would contain phandle to
PHY node instead of having actual PHY child node.
Documentation of this chip can be found at
http://linux-sunxi.org/File:AC200_Datasheet_V1.1.pdf
Note that in this case, AC200 IC is copackaged with Allwinner H6 SoC and
all connections between them are internal. So, for example, PWM is the
only way to provide 24 MHz clock to this chip.
Best regards,
Jernej
Jernej Skrabec (4):
mfd: Add support for AC200
net: phy: Add support for AC200 EPHY
arm64: dts: allwinner: h6: Add AC200 EPHY related nodes
arm64: dts: allwinner: h6: tanix-tx6: Enable ethernet
.../dts/allwinner/sun50i-h6-tanix-tx6.dts | 32 +++
arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 63 ++++++
drivers/mfd/Kconfig | 9 +
drivers/mfd/Makefile | 1 +
drivers/mfd/ac200.c | 188 ++++++++++++++++
drivers/net/phy/Kconfig | 7 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/ac200.c | 206 +++++++++++++++++
include/linux/mfd/ac200.h | 210 ++++++++++++++++++
9 files changed, 717 insertions(+)
create mode 100644 drivers/mfd/ac200.c
create mode 100644 drivers/net/phy/ac200.c
create mode 100644 include/linux/mfd/ac200.h
--
2.26.0
AC200 MFD IC supports Fast Ethernet PHY. Add a driver for it.
Signed-off-by: Jernej Skrabec <[email protected]>
---
drivers/net/phy/Kconfig | 7 ++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/ac200.c | 206 +++++++++++++++++++++++++++++++++++++++
3 files changed, 214 insertions(+)
create mode 100644 drivers/net/phy/ac200.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 3fa33d27eeba..16af69f69eaf 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -288,6 +288,13 @@ config ADIN_PHY
- ADIN1300 - Robust,Industrial, Low Latency 10/100/1000 Gigabit
Ethernet PHY
+config AC200_PHY
+ tristate "AC200 EPHY"
+ depends on NVMEM
+ depends on OF
+ help
+ Fast ethernet PHY as found in X-Powers AC200 multi-function device.
+
config AMD_PHY
tristate "AMD PHYs"
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 2f5c7093a65b..b0c5b91900fa 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_SFP) += sfp.o
sfp-obj-$(CONFIG_SFP) += sfp-bus.o
obj-y += $(sfp-obj-y) $(sfp-obj-m)
+obj-$(CONFIG_AC200_PHY) += ac200.o
obj-$(CONFIG_ADIN_PHY) += adin.o
obj-$(CONFIG_AMD_PHY) += amd.o
aquantia-objs += aquantia_main.o
diff --git a/drivers/net/phy/ac200.c b/drivers/net/phy/ac200.c
new file mode 100644
index 000000000000..3d7856ff8f91
--- /dev/null
+++ b/drivers/net/phy/ac200.c
@@ -0,0 +1,206 @@
+// SPDX-License-Identifier: GPL-2.0+
+/**
+ * Driver for AC200 Ethernet PHY
+ *
+ * Copyright (c) 2020 Jernej Skrabec <[email protected]>
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mfd/ac200.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+
+#define AC200_EPHY_ID 0x00441400
+#define AC200_EPHY_ID_MASK 0x0ffffff0
+
+/* macros for system ephy control 0 register */
+#define AC200_EPHY_RESET_INVALID BIT(0)
+#define AC200_EPHY_SYSCLK_GATING BIT(1)
+
+/* macros for system ephy control 1 register */
+#define AC200_EPHY_E_EPHY_MII_IO_EN BIT(0)
+#define AC200_EPHY_E_LNK_LED_IO_EN BIT(1)
+#define AC200_EPHY_E_SPD_LED_IO_EN BIT(2)
+#define AC200_EPHY_E_DPX_LED_IO_EN BIT(3)
+
+/* macros for ephy control register */
+#define AC200_EPHY_SHUTDOWN BIT(0)
+#define AC200_EPHY_LED_POL BIT(1)
+#define AC200_EPHY_CLK_SEL BIT(2)
+#define AC200_EPHY_ADDR(x) (((x) & 0x1F) << 4)
+#define AC200_EPHY_XMII_SEL BIT(11)
+#define AC200_EPHY_CALIB(x) (((x) & 0xF) << 12)
+
+struct ac200_ephy_dev {
+ struct phy_driver *ephy;
+ struct regmap *regmap;
+};
+
+static char *ac200_phy_name = "AC200 EPHY";
+
+static int ac200_ephy_config_init(struct phy_device *phydev)
+{
+ const struct ac200_ephy_dev *priv = phydev->drv->driver_data;
+ unsigned int value;
+ int ret;
+
+ phy_write(phydev, 0x1f, 0x0100); /* Switch to Page 1 */
+ phy_write(phydev, 0x12, 0x4824); /* Disable APS */
+
+ phy_write(phydev, 0x1f, 0x0200); /* Switch to Page 2 */
+ phy_write(phydev, 0x18, 0x0000); /* PHYAFE TRX optimization */
+
+ phy_write(phydev, 0x1f, 0x0600); /* Switch to Page 6 */
+ phy_write(phydev, 0x14, 0x708f); /* PHYAFE TX optimization */
+ phy_write(phydev, 0x13, 0xF000); /* PHYAFE RX optimization */
+ phy_write(phydev, 0x15, 0x1530);
+
+ phy_write(phydev, 0x1f, 0x0800); /* Switch to Page 6 */
+ phy_write(phydev, 0x18, 0x00bc); /* PHYAFE TRX optimization */
+
+ phy_write(phydev, 0x1f, 0x0100); /* switch to page 1 */
+ phy_clear_bits(phydev, 0x17, BIT(3)); /* disable intelligent IEEE */
+
+ /* next two blocks disable 802.3az IEEE */
+ phy_write(phydev, 0x1f, 0x0200); /* switch to page 2 */
+ phy_write(phydev, 0x18, 0x0000);
+
+ phy_write(phydev, 0x1f, 0x0000); /* switch to page 0 */
+ phy_clear_bits_mmd(phydev, 0x7, 0x3c, BIT(1));
+
+ if (phydev->interface == PHY_INTERFACE_MODE_RMII)
+ value = AC200_EPHY_XMII_SEL;
+ else
+ value = 0;
+
+ ret = regmap_update_bits(priv->regmap, AC200_EPHY_CTL,
+ AC200_EPHY_XMII_SEL, value);
+ if (ret)
+ return ret;
+
+ /* FIXME: This is H6 specific */
+ phy_set_bits(phydev, 0x13, BIT(12));
+
+ return 0;
+}
+
+static int ac200_ephy_probe(struct platform_device *pdev)
+{
+ struct ac200_dev *ac200 = dev_get_drvdata(pdev->dev.parent);
+ struct device *dev = &pdev->dev;
+ struct ac200_ephy_dev *priv;
+ struct nvmem_cell *calcell;
+ struct phy_driver *ephy;
+ u16 *caldata, calib;
+ size_t callen;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ ephy = devm_kzalloc(dev, sizeof(*ephy), GFP_KERNEL);
+ if (!ephy)
+ return -ENOMEM;
+
+ calcell = devm_nvmem_cell_get(dev, "calibration");
+ if (IS_ERR(calcell)) {
+ dev_err(dev, "Unable to find calibration data!\n");
+ return PTR_ERR(calcell);
+ }
+
+ caldata = nvmem_cell_read(calcell, &callen);
+ if (IS_ERR(caldata)) {
+ dev_err(dev, "Unable to read calibration data!\n");
+ return PTR_ERR(caldata);
+ }
+
+ if (callen != 2) {
+ dev_err(dev, "Calibration data has wrong length: 2 != %zu\n",
+ callen);
+ kfree(caldata);
+ return -EINVAL;
+ }
+
+ calib = *caldata + 3;
+ kfree(caldata);
+
+ ephy->phy_id = AC200_EPHY_ID;
+ ephy->phy_id_mask = AC200_EPHY_ID_MASK;
+ ephy->name = ac200_phy_name;
+ ephy->driver_data = priv;
+ ephy->soft_reset = genphy_soft_reset;
+ ephy->config_init = ac200_ephy_config_init;
+ ephy->suspend = genphy_suspend;
+ ephy->resume = genphy_resume;
+
+ priv->ephy = ephy;
+ priv->regmap = ac200->regmap;
+ platform_set_drvdata(pdev, priv);
+
+ ret = regmap_write(ac200->regmap, AC200_SYS_EPHY_CTL0,
+ AC200_EPHY_RESET_INVALID |
+ AC200_EPHY_SYSCLK_GATING);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(ac200->regmap, AC200_SYS_EPHY_CTL1,
+ AC200_EPHY_E_EPHY_MII_IO_EN |
+ AC200_EPHY_E_LNK_LED_IO_EN |
+ AC200_EPHY_E_SPD_LED_IO_EN |
+ AC200_EPHY_E_DPX_LED_IO_EN);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(ac200->regmap, AC200_EPHY_CTL,
+ AC200_EPHY_LED_POL |
+ AC200_EPHY_CLK_SEL |
+ AC200_EPHY_ADDR(1) |
+ AC200_EPHY_CALIB(calib));
+ if (ret)
+ return ret;
+
+ ret = phy_driver_register(priv->ephy, THIS_MODULE);
+ if (ret) {
+ dev_err(dev, "Unable to register phy\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ac200_ephy_remove(struct platform_device *pdev)
+{
+ struct ac200_ephy_dev *priv = platform_get_drvdata(pdev);
+
+ phy_driver_unregister(priv->ephy);
+
+ regmap_write(priv->regmap, AC200_EPHY_CTL, AC200_EPHY_SHUTDOWN);
+ regmap_write(priv->regmap, AC200_SYS_EPHY_CTL1, 0);
+ regmap_write(priv->regmap, AC200_SYS_EPHY_CTL0, 0);
+
+ return 0;
+}
+
+static const struct of_device_id ac200_ephy_match[] = {
+ { .compatible = "x-powers,ac200-ephy" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ac200_ephy_match);
+
+static struct platform_driver ac200_ephy_driver = {
+ .probe = ac200_ephy_probe,
+ .remove = ac200_ephy_remove,
+ .driver = {
+ .name = "ac200-ephy",
+ .of_match_table = ac200_ephy_match,
+ },
+};
+module_platform_driver(ac200_ephy_driver);
+
+MODULE_AUTHOR("Jernej Skrabec <[email protected]>");
+MODULE_DESCRIPTION("AC200 Ethernet PHY driver");
+MODULE_LICENSE("GPL");
--
2.26.0
On 16.04.2020 20:57, Jernej Skrabec wrote:
> AC200 MFD IC supports Fast Ethernet PHY. Add a driver for it.
>
> Signed-off-by: Jernej Skrabec <[email protected]>
> ---
> drivers/net/phy/Kconfig | 7 ++
> drivers/net/phy/Makefile | 1 +
> drivers/net/phy/ac200.c | 206 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 214 insertions(+)
> create mode 100644 drivers/net/phy/ac200.c
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 3fa33d27eeba..16af69f69eaf 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -288,6 +288,13 @@ config ADIN_PHY
> - ADIN1300 - Robust,Industrial, Low Latency 10/100/1000 Gigabit
> Ethernet PHY
>
> +config AC200_PHY
> + tristate "AC200 EPHY"
> + depends on NVMEM
> + depends on OF
> + help
> + Fast ethernet PHY as found in X-Powers AC200 multi-function device.
> +
> config AMD_PHY
> tristate "AMD PHYs"
> ---help---
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 2f5c7093a65b..b0c5b91900fa 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -53,6 +53,7 @@ obj-$(CONFIG_SFP) += sfp.o
> sfp-obj-$(CONFIG_SFP) += sfp-bus.o
> obj-y += $(sfp-obj-y) $(sfp-obj-m)
>
> +obj-$(CONFIG_AC200_PHY) += ac200.o
> obj-$(CONFIG_ADIN_PHY) += adin.o
> obj-$(CONFIG_AMD_PHY) += amd.o
> aquantia-objs += aquantia_main.o
> diff --git a/drivers/net/phy/ac200.c b/drivers/net/phy/ac200.c
> new file mode 100644
> index 000000000000..3d7856ff8f91
> --- /dev/null
> +++ b/drivers/net/phy/ac200.c
> @@ -0,0 +1,206 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/**
> + * Driver for AC200 Ethernet PHY
> + *
> + * Copyright (c) 2020 Jernej Skrabec <[email protected]>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mfd/ac200.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/of.h>
> +#include <linux/phy.h>
> +#include <linux/platform_device.h>
> +
> +#define AC200_EPHY_ID 0x00441400
> +#define AC200_EPHY_ID_MASK 0x0ffffff0
> +
You could use PHY_ID_MATCH_MODEL() here.
> +/* macros for system ephy control 0 register */
> +#define AC200_EPHY_RESET_INVALID BIT(0)
> +#define AC200_EPHY_SYSCLK_GATING BIT(1)
> +
> +/* macros for system ephy control 1 register */
> +#define AC200_EPHY_E_EPHY_MII_IO_EN BIT(0)
> +#define AC200_EPHY_E_LNK_LED_IO_EN BIT(1)
> +#define AC200_EPHY_E_SPD_LED_IO_EN BIT(2)
> +#define AC200_EPHY_E_DPX_LED_IO_EN BIT(3)
> +
> +/* macros for ephy control register */
> +#define AC200_EPHY_SHUTDOWN BIT(0)
> +#define AC200_EPHY_LED_POL BIT(1)
> +#define AC200_EPHY_CLK_SEL BIT(2)
> +#define AC200_EPHY_ADDR(x) (((x) & 0x1F) << 4)
> +#define AC200_EPHY_XMII_SEL BIT(11)
> +#define AC200_EPHY_CALIB(x) (((x) & 0xF) << 12)
> +
> +struct ac200_ephy_dev {
> + struct phy_driver *ephy;
Why embedding a pointer and not a struct phy_driver directly?
Then you could omit the separate allocation.
ephy is not the best naming. It may be read as a phy_device.
Better use phydrv.
> + struct regmap *regmap;
> +};
> +
> +static char *ac200_phy_name = "AC200 EPHY";
> +
Why not using the name directly in the assignment?
And better naming: "AC200 Fast Ethernet"
> +static int ac200_ephy_config_init(struct phy_device *phydev)
> +{
> + const struct ac200_ephy_dev *priv = phydev->drv->driver_data;
> + unsigned int value;
> + int ret;
> +
> + phy_write(phydev, 0x1f, 0x0100); /* Switch to Page 1 */
> + phy_write(phydev, 0x12, 0x4824); /* Disable APS */
> +
> + phy_write(phydev, 0x1f, 0x0200); /* Switch to Page 2 */
> + phy_write(phydev, 0x18, 0x0000); /* PHYAFE TRX optimization */
> +
> + phy_write(phydev, 0x1f, 0x0600); /* Switch to Page 6 */
> + phy_write(phydev, 0x14, 0x708f); /* PHYAFE TX optimization */
> + phy_write(phydev, 0x13, 0xF000); /* PHYAFE RX optimization */
> + phy_write(phydev, 0x15, 0x1530);
> +
> + phy_write(phydev, 0x1f, 0x0800); /* Switch to Page 6 */
> + phy_write(phydev, 0x18, 0x00bc); /* PHYAFE TRX optimization */
> +
> + phy_write(phydev, 0x1f, 0x0100); /* switch to page 1 */
> + phy_clear_bits(phydev, 0x17, BIT(3)); /* disable intelligent IEEE */
> +
> + /* next two blocks disable 802.3az IEEE */
> + phy_write(phydev, 0x1f, 0x0200); /* switch to page 2 */
> + phy_write(phydev, 0x18, 0x0000);
> +
> + phy_write(phydev, 0x1f, 0x0000); /* switch to page 0 */
> + phy_clear_bits_mmd(phydev, 0x7, 0x3c, BIT(1));
Better use the following:
phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0x0000);
It makes clear that you disable advertising EEE completely.
> +
> + if (phydev->interface == PHY_INTERFACE_MODE_RMII)
> + value = AC200_EPHY_XMII_SEL;
> + else
> + value = 0;
> +
> + ret = regmap_update_bits(priv->regmap, AC200_EPHY_CTL,
> + AC200_EPHY_XMII_SEL, value);
> + if (ret)
> + return ret;
> +
I had a brief look at the spec, and it's not fully clear
to me what this register setting does. Does it affect the
MAC side and/or the PHY side?
If it affects the PHY side, then I'd expect that the chip
has to talk to the PHY via the MDIO bus. Means there should
be a PHY register for setting MII vs. RMII.
In this case the setup could be very much simplified.
Then the PHY driver wouldn't have to be embedded in the
platform driver.
> + /* FIXME: This is H6 specific */
> + phy_set_bits(phydev, 0x13, BIT(12));
> +
This seems to indicate that the same PHY is used in a slightly
different version with other Hx models. Do they use different
PHY ID's?
> + return 0;
> +}
> +
> +static int ac200_ephy_probe(struct platform_device *pdev)
> +{
> + struct ac200_dev *ac200 = dev_get_drvdata(pdev->dev.parent);
> + struct device *dev = &pdev->dev;
> + struct ac200_ephy_dev *priv;
> + struct nvmem_cell *calcell;
> + struct phy_driver *ephy;
> + u16 *caldata, calib;
> + size_t callen;
> + int ret;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + ephy = devm_kzalloc(dev, sizeof(*ephy), GFP_KERNEL);
> + if (!ephy)
> + return -ENOMEM;
> +
> + calcell = devm_nvmem_cell_get(dev, "calibration");
> + if (IS_ERR(calcell)) {
> + dev_err(dev, "Unable to find calibration data!\n");
> + return PTR_ERR(calcell);
> + }
> +
> + caldata = nvmem_cell_read(calcell, &callen);
> + if (IS_ERR(caldata)) {
> + dev_err(dev, "Unable to read calibration data!\n");
> + return PTR_ERR(caldata);
> + }
> +
> + if (callen != 2) {
> + dev_err(dev, "Calibration data has wrong length: 2 != %zu\n",
> + callen);
> + kfree(caldata);
> + return -EINVAL;
> + }
> +
> + calib = *caldata + 3;
> + kfree(caldata);
> +
> + ephy->phy_id = AC200_EPHY_ID;
> + ephy->phy_id_mask = AC200_EPHY_ID_MASK;
> + ephy->name = ac200_phy_name;
> + ephy->driver_data = priv;
> + ephy->soft_reset = genphy_soft_reset;
> + ephy->config_init = ac200_ephy_config_init;
> + ephy->suspend = genphy_suspend;
> + ephy->resume = genphy_resume;
> +
> + priv->ephy = ephy;
> + priv->regmap = ac200->regmap;
> + platform_set_drvdata(pdev, priv);
> +
> + ret = regmap_write(ac200->regmap, AC200_SYS_EPHY_CTL0,
> + AC200_EPHY_RESET_INVALID |
> + AC200_EPHY_SYSCLK_GATING);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(ac200->regmap, AC200_SYS_EPHY_CTL1,
> + AC200_EPHY_E_EPHY_MII_IO_EN |
> + AC200_EPHY_E_LNK_LED_IO_EN |
> + AC200_EPHY_E_SPD_LED_IO_EN |
> + AC200_EPHY_E_DPX_LED_IO_EN);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(ac200->regmap, AC200_EPHY_CTL,
> + AC200_EPHY_LED_POL |
> + AC200_EPHY_CLK_SEL |
> + AC200_EPHY_ADDR(1) |
> + AC200_EPHY_CALIB(calib));
> + if (ret)
> + return ret;
> +
> + ret = phy_driver_register(priv->ephy, THIS_MODULE);
> + if (ret) {
> + dev_err(dev, "Unable to register phy\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int ac200_ephy_remove(struct platform_device *pdev)
> +{
> + struct ac200_ephy_dev *priv = platform_get_drvdata(pdev);
> +
> + phy_driver_unregister(priv->ephy);
> +
> + regmap_write(priv->regmap, AC200_EPHY_CTL, AC200_EPHY_SHUTDOWN);
> + regmap_write(priv->regmap, AC200_SYS_EPHY_CTL1, 0);
> + regmap_write(priv->regmap, AC200_SYS_EPHY_CTL0, 0);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ac200_ephy_match[] = {
> + { .compatible = "x-powers,ac200-ephy" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, ac200_ephy_match);
> +
> +static struct platform_driver ac200_ephy_driver = {
> + .probe = ac200_ephy_probe,
> + .remove = ac200_ephy_remove,
> + .driver = {
> + .name = "ac200-ephy",
> + .of_match_table = ac200_ephy_match,
> + },
> +};
> +module_platform_driver(ac200_ephy_driver);
> +
> +MODULE_AUTHOR("Jernej Skrabec <[email protected]>");
> +MODULE_DESCRIPTION("AC200 Ethernet PHY driver");
> +MODULE_LICENSE("GPL");
>
Tanix TX6 uses ethernet PHY from copackaged AC200 IC.
Enable it.
Signed-off-by: Jernej Skrabec <[email protected]>
---
.../dts/allwinner/sun50i-h6-tanix-tx6.dts | 32 +++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-tanix-tx6.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-tanix-tx6.dts
index 83e6cb0e59ce..41a2e3454be5 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-tanix-tx6.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-tanix-tx6.dts
@@ -12,6 +12,7 @@ / {
compatible = "oranth,tanix-tx6", "allwinner,sun50i-h6";
aliases {
+ ethernet0 = &emac;
serial0 = &uart0;
};
@@ -39,6 +40,14 @@ reg_vcc3v3: vcc3v3 {
};
};
+&ac200_ephy {
+ status = "okay";
+};
+
+&ac200_pwm_clk {
+ status = "okay";
+};
+
&de {
status = "okay";
};
@@ -47,6 +56,14 @@ &dwc3 {
status = "okay";
};
+&emac {
+ pinctrl-names = "default";
+ pinctrl-0 = <&ext_rmii_pins>;
+ phy-mode = "rmii";
+ phy-handle = <&ext_rmii_phy>;
+ status = "okay";
+};
+
&ehci0 {
status = "okay";
};
@@ -69,6 +86,17 @@ hdmi_out_con: endpoint {
};
};
+&i2c3 {
+ status = "okay";
+};
+
+&mdio {
+ ext_rmii_phy: ethernet-phy@1 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <1>;
+ };
+};
+
&mmc0 {
pinctrl-names = "default";
pinctrl-0 = <&mmc0_pins>;
@@ -86,6 +114,10 @@ &ohci3 {
status = "okay";
};
+&pwm {
+ status = "okay";
+};
+
&r_ir {
linux,rc-map-name = "rc-tanix-tx5max";
status = "okay";
--
2.26.0
On Thu, Apr 16, 2020 at 08:57:54PM +0200, Jernej Skrabec wrote:
> This is attempt to support Ethernet PHY on AC200 MFD chip. I'm sending
> this as RFC because I stumbled on a problem how to properly describe it
> in DT. Proper DT documentation will be added later, once DT issue is
> solved.
>
> Before Ethernet PHY can be actually used, few things must happen:
> 1. 24 MHz clock must be enabled and connected to input pin of this
> chip. In this case, PWM is set to generate 24 MHz signal with 50%
> duty cycle.
> 2. Chip must be put out of reset through I2C
> 3. Ethernet PHY must be enabled and configured through I2C
Hi Jernej
This is going to be interesting to describe in DT.
At what point does the PHY start responding to MDIO request? In
particular, you can read the ID registers 2 and 3? You list above what
is needed to make it usable. But we are also interested in what is
required to make is probe'able on the MDIO bus. We have more
flexibility if we can first probe it, and then later make it usable.
Thanks
Andrew
Dne četrtek, 16. april 2020 ob 22:18:52 CEST je Heiner Kallweit napisal(a):
> On 16.04.2020 20:57, Jernej Skrabec wrote:
> > AC200 MFD IC supports Fast Ethernet PHY. Add a driver for it.
> >
> > Signed-off-by: Jernej Skrabec <[email protected]>
> > ---
> >
> > drivers/net/phy/Kconfig | 7 ++
> > drivers/net/phy/Makefile | 1 +
> > drivers/net/phy/ac200.c | 206 +++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 214 insertions(+)
> > create mode 100644 drivers/net/phy/ac200.c
> >
> > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> > index 3fa33d27eeba..16af69f69eaf 100644
> > --- a/drivers/net/phy/Kconfig
> > +++ b/drivers/net/phy/Kconfig
> > @@ -288,6 +288,13 @@ config ADIN_PHY
> >
> > - ADIN1300 - Robust,Industrial, Low Latency 10/100/1000 Gigabit
> >
> > Ethernet PHY
> >
> > +config AC200_PHY
> > + tristate "AC200 EPHY"
> > + depends on NVMEM
> > + depends on OF
> > + help
> > + Fast ethernet PHY as found in X-Powers AC200 multi-function
device.
> > +
> >
> > config AMD_PHY
> >
> > tristate "AMD PHYs"
> > ---help---
> >
> > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> > index 2f5c7093a65b..b0c5b91900fa 100644
> > --- a/drivers/net/phy/Makefile
> > +++ b/drivers/net/phy/Makefile
> > @@ -53,6 +53,7 @@ obj-$(CONFIG_SFP) += sfp.o
> >
> > sfp-obj-$(CONFIG_SFP) += sfp-bus.o
> > obj-y += $(sfp-obj-y) $(sfp-obj-m)
> >
> > +obj-$(CONFIG_AC200_PHY) += ac200.o
> >
> > obj-$(CONFIG_ADIN_PHY) += adin.o
> > obj-$(CONFIG_AMD_PHY) += amd.o
> > aquantia-objs += aquantia_main.o
> >
> > diff --git a/drivers/net/phy/ac200.c b/drivers/net/phy/ac200.c
> > new file mode 100644
> > index 000000000000..3d7856ff8f91
> > --- /dev/null
> > +++ b/drivers/net/phy/ac200.c
> > @@ -0,0 +1,206 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/**
> > + * Driver for AC200 Ethernet PHY
> > + *
> > + * Copyright (c) 2020 Jernej Skrabec <[email protected]>
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/mfd/ac200.h>
> > +#include <linux/nvmem-consumer.h>
> > +#include <linux/of.h>
> > +#include <linux/phy.h>
> > +#include <linux/platform_device.h>
> > +
> > +#define AC200_EPHY_ID 0x00441400
> > +#define AC200_EPHY_ID_MASK 0x0ffffff0
> > +
>
> You could use PHY_ID_MATCH_MODEL() here.
Ok.
>
> > +/* macros for system ephy control 0 register */
> > +#define AC200_EPHY_RESET_INVALID BIT(0)
> > +#define AC200_EPHY_SYSCLK_GATING BIT(1)
> > +
> > +/* macros for system ephy control 1 register */
> > +#define AC200_EPHY_E_EPHY_MII_IO_EN BIT(0)
> > +#define AC200_EPHY_E_LNK_LED_IO_EN BIT(1)
> > +#define AC200_EPHY_E_SPD_LED_IO_EN BIT(2)
> > +#define AC200_EPHY_E_DPX_LED_IO_EN BIT(3)
> > +
> > +/* macros for ephy control register */
> > +#define AC200_EPHY_SHUTDOWN BIT(0)
> > +#define AC200_EPHY_LED_POL BIT(1)
> > +#define AC200_EPHY_CLK_SEL BIT(2)
> > +#define AC200_EPHY_ADDR(x) (((x) & 0x1F) << 4)
> > +#define AC200_EPHY_XMII_SEL BIT(11)
> > +#define AC200_EPHY_CALIB(x) (((x) & 0xF) << 12)
> > +
> > +struct ac200_ephy_dev {
> > + struct phy_driver *ephy;
>
> Why embedding a pointer and not a struct phy_driver directly?
> Then you could omit the separate allocation.
Right.
>
> ephy is not the best naming. It may be read as a phy_device.
> Better use phydrv.
Ok.
>
> > + struct regmap *regmap;
> > +};
> > +
> > +static char *ac200_phy_name = "AC200 EPHY";
> > +
>
> Why not using the name directly in the assignment?
phy_driver->name is pointer. Wouldn't that mean that string is allocated on
stack and next time pointer is used, it will return garbage?
> And better naming: "AC200 Fast Ethernet"
Ok.
>
> > +static int ac200_ephy_config_init(struct phy_device *phydev)
> > +{
> > + const struct ac200_ephy_dev *priv = phydev->drv->driver_data;
> > + unsigned int value;
> > + int ret;
> > +
> > + phy_write(phydev, 0x1f, 0x0100); /* Switch to Page 1 */
> > + phy_write(phydev, 0x12, 0x4824); /* Disable APS */
> > +
> > + phy_write(phydev, 0x1f, 0x0200); /* Switch to Page 2 */
> > + phy_write(phydev, 0x18, 0x0000); /* PHYAFE TRX optimization */
> > +
> > + phy_write(phydev, 0x1f, 0x0600); /* Switch to Page 6 */
> > + phy_write(phydev, 0x14, 0x708f); /* PHYAFE TX optimization */
> > + phy_write(phydev, 0x13, 0xF000); /* PHYAFE RX optimization */
> > + phy_write(phydev, 0x15, 0x1530);
> > +
> > + phy_write(phydev, 0x1f, 0x0800); /* Switch to Page 6 */
> > + phy_write(phydev, 0x18, 0x00bc); /* PHYAFE TRX optimization */
> > +
> > + phy_write(phydev, 0x1f, 0x0100); /* switch to page 1 */
> > + phy_clear_bits(phydev, 0x17, BIT(3)); /* disable intelligent
IEEE */
> > +
> > + /* next two blocks disable 802.3az IEEE */
> > + phy_write(phydev, 0x1f, 0x0200); /* switch to page 2 */
> > + phy_write(phydev, 0x18, 0x0000);
> > +
> > + phy_write(phydev, 0x1f, 0x0000); /* switch to page 0 */
> > + phy_clear_bits_mmd(phydev, 0x7, 0x3c, BIT(1));
>
> Better use the following:
> phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0x0000);
> It makes clear that you disable advertising EEE completely.
Ok.
>
> > +
> > + if (phydev->interface == PHY_INTERFACE_MODE_RMII)
> > + value = AC200_EPHY_XMII_SEL;
> > + else
> > + value = 0;
> > +
> > + ret = regmap_update_bits(priv->regmap, AC200_EPHY_CTL,
> > + AC200_EPHY_XMII_SEL, value);
> > + if (ret)
> > + return ret;
> > +
>
> I had a brief look at the spec, and it's not fully clear
> to me what this register setting does. Does it affect the
> MAC side and/or the PHY side?
It's my understanding that it selects interface mode on PHY. Besides datasheet
mentioned in cover letter, BSP drivers (one for MFD and one for PHY) are the
only other source of information. BSP PHY driver is located here:
https://github.com/Allwinner-Homlet/H6-BSP4.9-linux/blob/master/drivers/net/
phy/sunxi-ephy.c
> If it affects the PHY side, then I'd expect that the chip
> has to talk to the PHY via the MDIO bus. Means there should
> be a PHY register for setting MII vs. RMII.
> In this case the setup could be very much simplified.
> Then the PHY driver wouldn't have to be embedded in the
> platform driver.
Actually, PHY has to be configured first through I2C and then through MDIO. I2C
is used to enable it (power it up), configure LED polarity, set PHY address,
write calibration value stored elsewhere.
Based on all available documentation I have (code and datasheet), this I2C
register is the only way to select MII or RMII mode.
>
> > + /* FIXME: This is H6 specific */
> > + phy_set_bits(phydev, 0x13, BIT(12));
> > +
>
> This seems to indicate that the same PHY is used in a slightly
> different version with other Hx models. Do they use different
> PHY ID's?
Situation is a bit complicated. Same PHY, at least with same PHY ID, is used
in different ways.
1. as part of standalone AC200 MFD IC
2. as part of AC200 wafer copackaged with H6 SoC wafer in same package. This
in theory shouldn't be any different than standalone IC, but it apparently is,
based on the BSP driver code.
3. integrated directly in SoCs like H3, H5 and V3s. There is no I2C access to
configuration register. Instead, it's memory mapped and slightly different.
In all cases PHY ID is same, just glue logic is different.
I asked Allwinner if above setting is really necessary for H6 and what it
does, but I didn't get any useful answer back.
So maybe another compatible is needed for H6.
Best regards,
Jernej
>
> > + return 0;
> > +}
> > +
> > +static int ac200_ephy_probe(struct platform_device *pdev)
> > +{
> > + struct ac200_dev *ac200 = dev_get_drvdata(pdev->dev.parent);
> > + struct device *dev = &pdev->dev;
> > + struct ac200_ephy_dev *priv;
> > + struct nvmem_cell *calcell;
> > + struct phy_driver *ephy;
> > + u16 *caldata, calib;
> > + size_t callen;
> > + int ret;
> > +
> > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > + if (!priv)
> > + return -ENOMEM;
> > +
> > + ephy = devm_kzalloc(dev, sizeof(*ephy), GFP_KERNEL);
> > + if (!ephy)
> > + return -ENOMEM;
> > +
> > + calcell = devm_nvmem_cell_get(dev, "calibration");
> > + if (IS_ERR(calcell)) {
> > + dev_err(dev, "Unable to find calibration data!\n");
> > + return PTR_ERR(calcell);
> > + }
> > +
> > + caldata = nvmem_cell_read(calcell, &callen);
> > + if (IS_ERR(caldata)) {
> > + dev_err(dev, "Unable to read calibration data!\n");
> > + return PTR_ERR(caldata);
> > + }
> > +
> > + if (callen != 2) {
> > + dev_err(dev, "Calibration data has wrong length: 2 !=
%zu\n",
> > + callen);
> > + kfree(caldata);
> > + return -EINVAL;
> > + }
> > +
> > + calib = *caldata + 3;
> > + kfree(caldata);
> > +
> > + ephy->phy_id = AC200_EPHY_ID;
> > + ephy->phy_id_mask = AC200_EPHY_ID_MASK;
> > + ephy->name = ac200_phy_name;
> > + ephy->driver_data = priv;
> > + ephy->soft_reset = genphy_soft_reset;
> > + ephy->config_init = ac200_ephy_config_init;
> > + ephy->suspend = genphy_suspend;
> > + ephy->resume = genphy_resume;
> > +
> > + priv->ephy = ephy;
> > + priv->regmap = ac200->regmap;
> > + platform_set_drvdata(pdev, priv);
> > +
> > + ret = regmap_write(ac200->regmap, AC200_SYS_EPHY_CTL0,
> > + AC200_EPHY_RESET_INVALID |
> > + AC200_EPHY_SYSCLK_GATING);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_write(ac200->regmap, AC200_SYS_EPHY_CTL1,
> > + AC200_EPHY_E_EPHY_MII_IO_EN |
> > + AC200_EPHY_E_LNK_LED_IO_EN |
> > + AC200_EPHY_E_SPD_LED_IO_EN |
> > + AC200_EPHY_E_DPX_LED_IO_EN);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_write(ac200->regmap, AC200_EPHY_CTL,
> > + AC200_EPHY_LED_POL |
> > + AC200_EPHY_CLK_SEL |
> > + AC200_EPHY_ADDR(1) |
> > + AC200_EPHY_CALIB(calib));
> > + if (ret)
> > + return ret;
> > +
> > + ret = phy_driver_register(priv->ephy, THIS_MODULE);
> > + if (ret) {
> > + dev_err(dev, "Unable to register phy\n");
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int ac200_ephy_remove(struct platform_device *pdev)
> > +{
> > + struct ac200_ephy_dev *priv = platform_get_drvdata(pdev);
> > +
> > + phy_driver_unregister(priv->ephy);
> > +
> > + regmap_write(priv->regmap, AC200_EPHY_CTL, AC200_EPHY_SHUTDOWN);
> > + regmap_write(priv->regmap, AC200_SYS_EPHY_CTL1, 0);
> > + regmap_write(priv->regmap, AC200_SYS_EPHY_CTL0, 0);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id ac200_ephy_match[] = {
> > + { .compatible = "x-powers,ac200-ephy" },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, ac200_ephy_match);
> > +
> > +static struct platform_driver ac200_ephy_driver = {
> > + .probe = ac200_ephy_probe,
> > + .remove = ac200_ephy_remove,
> > + .driver = {
> > + .name = "ac200-ephy",
> > + .of_match_table = ac200_ephy_match,
> > + },
> > +};
> > +module_platform_driver(ac200_ephy_driver);
> > +
> > +MODULE_AUTHOR("Jernej Skrabec <[email protected]>");
> > +MODULE_DESCRIPTION("AC200 Ethernet PHY driver");
> > +MODULE_LICENSE("GPL");
Dne četrtek, 16. april 2020 ob 22:18:52 CEST je Heiner Kallweit napisal(a):
> On 16.04.2020 20:57, Jernej Skrabec wrote:
> > AC200 MFD IC supports Fast Ethernet PHY. Add a driver for it.
> >
> > Signed-off-by: Jernej Skrabec <[email protected]>
> > ---
> >
> > drivers/net/phy/Kconfig | 7 ++
> > drivers/net/phy/Makefile | 1 +
> > drivers/net/phy/ac200.c | 206 +++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 214 insertions(+)
> > create mode 100644 drivers/net/phy/ac200.c
> >
> > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> > index 3fa33d27eeba..16af69f69eaf 100644
> > --- a/drivers/net/phy/Kconfig
> > +++ b/drivers/net/phy/Kconfig
> > @@ -288,6 +288,13 @@ config ADIN_PHY
> >
> > - ADIN1300 - Robust,Industrial, Low Latency 10/100/1000 Gigabit
> >
> > Ethernet PHY
> >
> > +config AC200_PHY
> > + tristate "AC200 EPHY"
> > + depends on NVMEM
> > + depends on OF
> > + help
> > + Fast ethernet PHY as found in X-Powers AC200 multi-function
device.
> > +
> >
> > config AMD_PHY
> >
> > tristate "AMD PHYs"
> > ---help---
> >
> > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> > index 2f5c7093a65b..b0c5b91900fa 100644
> > --- a/drivers/net/phy/Makefile
> > +++ b/drivers/net/phy/Makefile
> > @@ -53,6 +53,7 @@ obj-$(CONFIG_SFP) += sfp.o
> >
> > sfp-obj-$(CONFIG_SFP) += sfp-bus.o
> > obj-y += $(sfp-obj-y) $(sfp-obj-m)
> >
> > +obj-$(CONFIG_AC200_PHY) += ac200.o
> >
> > obj-$(CONFIG_ADIN_PHY) += adin.o
> > obj-$(CONFIG_AMD_PHY) += amd.o
> > aquantia-objs += aquantia_main.o
> >
> > diff --git a/drivers/net/phy/ac200.c b/drivers/net/phy/ac200.c
> > new file mode 100644
> > index 000000000000..3d7856ff8f91
> > --- /dev/null
> > +++ b/drivers/net/phy/ac200.c
> > @@ -0,0 +1,206 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/**
> > + * Driver for AC200 Ethernet PHY
> > + *
> > + * Copyright (c) 2020 Jernej Skrabec <[email protected]>
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/mfd/ac200.h>
> > +#include <linux/nvmem-consumer.h>
> > +#include <linux/of.h>
> > +#include <linux/phy.h>
> > +#include <linux/platform_device.h>
> > +
> > +#define AC200_EPHY_ID 0x00441400
> > +#define AC200_EPHY_ID_MASK 0x0ffffff0
> > +
>
> You could use PHY_ID_MATCH_MODEL() here.
Hm... This doesn't work with dynamically allocated memory, right?
Best regards,
Jernej
On 17.04.2020 18:15, Jernej Škrabec wrote:
> Dne četrtek, 16. april 2020 ob 22:18:52 CEST je Heiner Kallweit napisal(a):
>> On 16.04.2020 20:57, Jernej Skrabec wrote:
>>> AC200 MFD IC supports Fast Ethernet PHY. Add a driver for it.
>>>
>>> Signed-off-by: Jernej Skrabec <[email protected]>
>>> ---
>>>
>>> drivers/net/phy/Kconfig | 7 ++
>>> drivers/net/phy/Makefile | 1 +
>>> drivers/net/phy/ac200.c | 206 +++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 214 insertions(+)
>>> create mode 100644 drivers/net/phy/ac200.c
>>>
>>> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
>>> index 3fa33d27eeba..16af69f69eaf 100644
>>> --- a/drivers/net/phy/Kconfig
>>> +++ b/drivers/net/phy/Kconfig
>>> @@ -288,6 +288,13 @@ config ADIN_PHY
>>>
>>> - ADIN1300 - Robust,Industrial, Low Latency 10/100/1000 Gigabit
>>>
>>> Ethernet PHY
>>>
>>> +config AC200_PHY
>>> + tristate "AC200 EPHY"
>>> + depends on NVMEM
>>> + depends on OF
>>> + help
>>> + Fast ethernet PHY as found in X-Powers AC200 multi-function
> device.
>>> +
>>>
>>> config AMD_PHY
>>>
>>> tristate "AMD PHYs"
>>> ---help---
>>>
>>> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
>>> index 2f5c7093a65b..b0c5b91900fa 100644
>>> --- a/drivers/net/phy/Makefile
>>> +++ b/drivers/net/phy/Makefile
>>> @@ -53,6 +53,7 @@ obj-$(CONFIG_SFP) += sfp.o
>>>
>>> sfp-obj-$(CONFIG_SFP) += sfp-bus.o
>>> obj-y += $(sfp-obj-y) $(sfp-obj-m)
>>>
>>> +obj-$(CONFIG_AC200_PHY) += ac200.o
>>>
>>> obj-$(CONFIG_ADIN_PHY) += adin.o
>>> obj-$(CONFIG_AMD_PHY) += amd.o
>>> aquantia-objs += aquantia_main.o
>>>
>>> diff --git a/drivers/net/phy/ac200.c b/drivers/net/phy/ac200.c
>>> new file mode 100644
>>> index 000000000000..3d7856ff8f91
>>> --- /dev/null
>>> +++ b/drivers/net/phy/ac200.c
>>> @@ -0,0 +1,206 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/**
>>> + * Driver for AC200 Ethernet PHY
>>> + *
>>> + * Copyright (c) 2020 Jernej Skrabec <[email protected]>
>>> + */
>>> +
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/mfd/ac200.h>
>>> +#include <linux/nvmem-consumer.h>
>>> +#include <linux/of.h>
>>> +#include <linux/phy.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +#define AC200_EPHY_ID 0x00441400
>>> +#define AC200_EPHY_ID_MASK 0x0ffffff0
>>> +
>>
>> You could use PHY_ID_MATCH_MODEL() here.
>
> Hm... This doesn't work with dynamically allocated memory, right?
>
Right ..
> Best regards,
> Jernej
>
>
On 17.04.2020 18:03, Jernej Škrabec wrote:
> Dne četrtek, 16. april 2020 ob 22:18:52 CEST je Heiner Kallweit napisal(a):
>> On 16.04.2020 20:57, Jernej Skrabec wrote:
>>> AC200 MFD IC supports Fast Ethernet PHY. Add a driver for it.
>>>
>>> Signed-off-by: Jernej Skrabec <[email protected]>
>>> ---
>>>
>>> drivers/net/phy/Kconfig | 7 ++
>>> drivers/net/phy/Makefile | 1 +
>>> drivers/net/phy/ac200.c | 206 +++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 214 insertions(+)
>>> create mode 100644 drivers/net/phy/ac200.c
>>>
>>> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
>>> index 3fa33d27eeba..16af69f69eaf 100644
>>> --- a/drivers/net/phy/Kconfig
>>> +++ b/drivers/net/phy/Kconfig
>>> @@ -288,6 +288,13 @@ config ADIN_PHY
>>>
>>> - ADIN1300 - Robust,Industrial, Low Latency 10/100/1000 Gigabit
>>>
>>> Ethernet PHY
>>>
>>> +config AC200_PHY
>>> + tristate "AC200 EPHY"
>>> + depends on NVMEM
>>> + depends on OF
>>> + help
>>> + Fast ethernet PHY as found in X-Powers AC200 multi-function
> device.
>>> +
>>>
>>> config AMD_PHY
>>>
>>> tristate "AMD PHYs"
>>> ---help---
>>>
>>> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
>>> index 2f5c7093a65b..b0c5b91900fa 100644
>>> --- a/drivers/net/phy/Makefile
>>> +++ b/drivers/net/phy/Makefile
>>> @@ -53,6 +53,7 @@ obj-$(CONFIG_SFP) += sfp.o
>>>
>>> sfp-obj-$(CONFIG_SFP) += sfp-bus.o
>>> obj-y += $(sfp-obj-y) $(sfp-obj-m)
>>>
>>> +obj-$(CONFIG_AC200_PHY) += ac200.o
>>>
>>> obj-$(CONFIG_ADIN_PHY) += adin.o
>>> obj-$(CONFIG_AMD_PHY) += amd.o
>>> aquantia-objs += aquantia_main.o
>>>
>>> diff --git a/drivers/net/phy/ac200.c b/drivers/net/phy/ac200.c
>>> new file mode 100644
>>> index 000000000000..3d7856ff8f91
>>> --- /dev/null
>>> +++ b/drivers/net/phy/ac200.c
>>> @@ -0,0 +1,206 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/**
>>> + * Driver for AC200 Ethernet PHY
>>> + *
>>> + * Copyright (c) 2020 Jernej Skrabec <[email protected]>
>>> + */
>>> +
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/mfd/ac200.h>
>>> +#include <linux/nvmem-consumer.h>
>>> +#include <linux/of.h>
>>> +#include <linux/phy.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +#define AC200_EPHY_ID 0x00441400
>>> +#define AC200_EPHY_ID_MASK 0x0ffffff0
>>> +
>>
>> You could use PHY_ID_MATCH_MODEL() here.
>
> Ok.
>
>>
>>> +/* macros for system ephy control 0 register */
>>> +#define AC200_EPHY_RESET_INVALID BIT(0)
>>> +#define AC200_EPHY_SYSCLK_GATING BIT(1)
>>> +
>>> +/* macros for system ephy control 1 register */
>>> +#define AC200_EPHY_E_EPHY_MII_IO_EN BIT(0)
>>> +#define AC200_EPHY_E_LNK_LED_IO_EN BIT(1)
>>> +#define AC200_EPHY_E_SPD_LED_IO_EN BIT(2)
>>> +#define AC200_EPHY_E_DPX_LED_IO_EN BIT(3)
>>> +
>>> +/* macros for ephy control register */
>>> +#define AC200_EPHY_SHUTDOWN BIT(0)
>>> +#define AC200_EPHY_LED_POL BIT(1)
>>> +#define AC200_EPHY_CLK_SEL BIT(2)
>>> +#define AC200_EPHY_ADDR(x) (((x) & 0x1F) << 4)
>>> +#define AC200_EPHY_XMII_SEL BIT(11)
>>> +#define AC200_EPHY_CALIB(x) (((x) & 0xF) << 12)
>>> +
>>> +struct ac200_ephy_dev {
>>> + struct phy_driver *ephy;
>>
>> Why embedding a pointer and not a struct phy_driver directly?
>> Then you could omit the separate allocation.
>
> Right.
>
>>
>> ephy is not the best naming. It may be read as a phy_device.
>> Better use phydrv.
>
> Ok.
>
>>
>>> + struct regmap *regmap;
>>> +};
>>> +
>>> +static char *ac200_phy_name = "AC200 EPHY";
>>> +
>>
>> Why not using the name directly in the assignment?
>
> phy_driver->name is pointer. Wouldn't that mean that string is allocated on
> stack and next time pointer is used, it will return garbage?
>
No, it's not on the stack. No problem here.
>> And better naming: "AC200 Fast Ethernet"
>
> Ok.
>
>>
>>> +static int ac200_ephy_config_init(struct phy_device *phydev)
>>> +{
>>> + const struct ac200_ephy_dev *priv = phydev->drv->driver_data;
>>> + unsigned int value;
>>> + int ret;
>>> +
>>> + phy_write(phydev, 0x1f, 0x0100); /* Switch to Page 1 */
>>> + phy_write(phydev, 0x12, 0x4824); /* Disable APS */
>>> +
>>> + phy_write(phydev, 0x1f, 0x0200); /* Switch to Page 2 */
>>> + phy_write(phydev, 0x18, 0x0000); /* PHYAFE TRX optimization */
>>> +
>>> + phy_write(phydev, 0x1f, 0x0600); /* Switch to Page 6 */
>>> + phy_write(phydev, 0x14, 0x708f); /* PHYAFE TX optimization */
>>> + phy_write(phydev, 0x13, 0xF000); /* PHYAFE RX optimization */
>>> + phy_write(phydev, 0x15, 0x1530);
>>> +
>>> + phy_write(phydev, 0x1f, 0x0800); /* Switch to Page 6 */
>>> + phy_write(phydev, 0x18, 0x00bc); /* PHYAFE TRX optimization */
>>> +
>>> + phy_write(phydev, 0x1f, 0x0100); /* switch to page 1 */
>>> + phy_clear_bits(phydev, 0x17, BIT(3)); /* disable intelligent
> IEEE */
>>> +
>>> + /* next two blocks disable 802.3az IEEE */
>>> + phy_write(phydev, 0x1f, 0x0200); /* switch to page 2 */
>>> + phy_write(phydev, 0x18, 0x0000);
>>> +
>>> + phy_write(phydev, 0x1f, 0x0000); /* switch to page 0 */
>>> + phy_clear_bits_mmd(phydev, 0x7, 0x3c, BIT(1));
>>
>> Better use the following:
>> phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0x0000);
>> It makes clear that you disable advertising EEE completely.
>
> Ok.
>
>>
>>> +
>>> + if (phydev->interface == PHY_INTERFACE_MODE_RMII)
>>> + value = AC200_EPHY_XMII_SEL;
>>> + else
>>> + value = 0;
>>> +
>>> + ret = regmap_update_bits(priv->regmap, AC200_EPHY_CTL,
>>> + AC200_EPHY_XMII_SEL, value);
>>> + if (ret)
>>> + return ret;
>>> +
>>
>> I had a brief look at the spec, and it's not fully clear
>> to me what this register setting does. Does it affect the
>> MAC side and/or the PHY side?
>
> It's my understanding that it selects interface mode on PHY. Besides datasheet
> mentioned in cover letter, BSP drivers (one for MFD and one for PHY) are the
> only other source of information. BSP PHY driver is located here:
> https://github.com/Allwinner-Homlet/H6-BSP4.9-linux/blob/master/drivers/net/
> phy/sunxi-ephy.c
>
>> If it affects the PHY side, then I'd expect that the chip
>> has to talk to the PHY via the MDIO bus. Means there should
>> be a PHY register for setting MII vs. RMII.
>> In this case the setup could be very much simplified.
>> Then the PHY driver wouldn't have to be embedded in the
>> platform driver.
>
> Actually, PHY has to be configured first through I2C and then through MDIO. I2C
> is used to enable it (power it up), configure LED polarity, set PHY address,
> write calibration value stored elsewhere.
>
> Based on all available documentation I have (code and datasheet), this I2C
> register is the only way to select MII or RMII mode.
>
Then how and where is the PHY interface mode configured on the MAC side?
If there is no such setting, then I'd assume that this register bit
configures both sides. This leads to the question whether the interface
mode really needs to be set in the PHY driver's config_init().
If we could avoid this, then you could make the PHY driver static.
You could set the PHY interface mode as soon as the PHY interface mode
is read from DT. So why not set the interface mode at the place where
you configure the other values like PHY address?
>>
>>> + /* FIXME: This is H6 specific */
>>> + phy_set_bits(phydev, 0x13, BIT(12));
>>> +
>>
>> This seems to indicate that the same PHY is used in a slightly
>> different version with other Hx models. Do they use different
>> PHY ID's?
>
> Situation is a bit complicated. Same PHY, at least with same PHY ID, is used
> in different ways.
> 1. as part of standalone AC200 MFD IC
> 2. as part of AC200 wafer copackaged with H6 SoC wafer in same package. This
> in theory shouldn't be any different than standalone IC, but it apparently is,
> based on the BSP driver code.
> 3. integrated directly in SoCs like H3, H5 and V3s. There is no I2C access to
> configuration register. Instead, it's memory mapped and slightly different.
>
> In all cases PHY ID is same, just glue logic is different.
>
> I asked Allwinner if above setting is really necessary for H6 and what it
> does, but I didn't get any useful answer back.
>
> So maybe another compatible is needed for H6.
>
> Best regards,
> Jernej
>
>>
>>> + return 0;
>>> +}
>>> +
>>> +static int ac200_ephy_probe(struct platform_device *pdev)
>>> +{
>>> + struct ac200_dev *ac200 = dev_get_drvdata(pdev->dev.parent);
>>> + struct device *dev = &pdev->dev;
>>> + struct ac200_ephy_dev *priv;
>>> + struct nvmem_cell *calcell;
>>> + struct phy_driver *ephy;
>>> + u16 *caldata, calib;
>>> + size_t callen;
>>> + int ret;
>>> +
>>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>> + if (!priv)
>>> + return -ENOMEM;
>>> +
>>> + ephy = devm_kzalloc(dev, sizeof(*ephy), GFP_KERNEL);
>>> + if (!ephy)
>>> + return -ENOMEM;
>>> +
>>> + calcell = devm_nvmem_cell_get(dev, "calibration");
>>> + if (IS_ERR(calcell)) {
>>> + dev_err(dev, "Unable to find calibration data!\n");
>>> + return PTR_ERR(calcell);
>>> + }
>>> +
>>> + caldata = nvmem_cell_read(calcell, &callen);
>>> + if (IS_ERR(caldata)) {
>>> + dev_err(dev, "Unable to read calibration data!\n");
>>> + return PTR_ERR(caldata);
>>> + }
>>> +
>>> + if (callen != 2) {
>>> + dev_err(dev, "Calibration data has wrong length: 2 !=
> %zu\n",
>>> + callen);
>>> + kfree(caldata);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + calib = *caldata + 3;
>>> + kfree(caldata);
>>> +
>>> + ephy->phy_id = AC200_EPHY_ID;
>>> + ephy->phy_id_mask = AC200_EPHY_ID_MASK;
>>> + ephy->name = ac200_phy_name;
>>> + ephy->driver_data = priv;
>>> + ephy->soft_reset = genphy_soft_reset;
>>> + ephy->config_init = ac200_ephy_config_init;
>>> + ephy->suspend = genphy_suspend;
>>> + ephy->resume = genphy_resume;
>>> +
>>> + priv->ephy = ephy;
>>> + priv->regmap = ac200->regmap;
>>> + platform_set_drvdata(pdev, priv);
>>> +
>>> + ret = regmap_write(ac200->regmap, AC200_SYS_EPHY_CTL0,
>>> + AC200_EPHY_RESET_INVALID |
>>> + AC200_EPHY_SYSCLK_GATING);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = regmap_write(ac200->regmap, AC200_SYS_EPHY_CTL1,
>>> + AC200_EPHY_E_EPHY_MII_IO_EN |
>>> + AC200_EPHY_E_LNK_LED_IO_EN |
>>> + AC200_EPHY_E_SPD_LED_IO_EN |
>>> + AC200_EPHY_E_DPX_LED_IO_EN);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = regmap_write(ac200->regmap, AC200_EPHY_CTL,
>>> + AC200_EPHY_LED_POL |
>>> + AC200_EPHY_CLK_SEL |
>>> + AC200_EPHY_ADDR(1) |
>>> + AC200_EPHY_CALIB(calib));
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = phy_driver_register(priv->ephy, THIS_MODULE);
>>> + if (ret) {
>>> + dev_err(dev, "Unable to register phy\n");
>>> + return ret;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int ac200_ephy_remove(struct platform_device *pdev)
>>> +{
>>> + struct ac200_ephy_dev *priv = platform_get_drvdata(pdev);
>>> +
>>> + phy_driver_unregister(priv->ephy);
>>> +
>>> + regmap_write(priv->regmap, AC200_EPHY_CTL, AC200_EPHY_SHUTDOWN);
>>> + regmap_write(priv->regmap, AC200_SYS_EPHY_CTL1, 0);
>>> + regmap_write(priv->regmap, AC200_SYS_EPHY_CTL0, 0);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct of_device_id ac200_ephy_match[] = {
>>> + { .compatible = "x-powers,ac200-ephy" },
>>> + { /* sentinel */ }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, ac200_ephy_match);
>>> +
>>> +static struct platform_driver ac200_ephy_driver = {
>>> + .probe = ac200_ephy_probe,
>>> + .remove = ac200_ephy_remove,
>>> + .driver = {
>>> + .name = "ac200-ephy",
>>> + .of_match_table = ac200_ephy_match,
>>> + },
>>> +};
>>> +module_platform_driver(ac200_ephy_driver);
>>> +
>>> +MODULE_AUTHOR("Jernej Skrabec <[email protected]>");
>>> +MODULE_DESCRIPTION("AC200 Ethernet PHY driver");
>>> +MODULE_LICENSE("GPL");
>
>
>
>
> > You could use PHY_ID_MATCH_MODEL() here.
>
> Hm... This doesn't work with dynamically allocated memory, right?
I would suggest we get the right structure first, then figure out
details like this.
Depending on when the device will respond to MDIO, we might be able to
make this a normal PHY driver. It then probes in the normal way, and
all the horrible dependencies you talked about, module loading order,
etc all go away.
There were 3 things you talked about to make the PHY usable:
1) Clock
2) Reset
3) Must be enabled and configured through I2C
We already have the concept of a PHY device having a reset controller
as a property. e.g. Documentation/devicetree/bindings/net/ethernet-phy.yaml
resets = <&rst 8>;
So if the MFD exports a reset controller, we can control that from the
PHY core. If the MFD has not probed yet, the reset core code will
return EPROBE_DEFFER, and the PHY probe will get differed until late.
That solves a lot of probe order issues.
The clock can be handled in two different ways, depending on if the
clock needs to be ticking to read the PHY ID registers. If it does
need to be ticking, we add support for a clks property in just the
same way we have support for the reset property. The PHY core will
clk_enable_prepare() the clock before probing the PHY. If the clock is
not needed for probing, the PHY driver can enable the clock as needed.
The last part, Must be enabled and configured through I2C, we need to
look at the details. It could be the reset controller also enabled the
PHY. If that is enough that the PHY then probes, the PHY driver can
then configure the PHY as needed via i2c.
Andrew
Dne petek, 17. april 2020 ob 18:29:04 CEST je Heiner Kallweit napisal(a):
> On 17.04.2020 18:03, Jernej Škrabec wrote:
> > Dne četrtek, 16. april 2020 ob 22:18:52 CEST je Heiner Kallweit
napisal(a):
> >> On 16.04.2020 20:57, Jernej Skrabec wrote:
> >>> AC200 MFD IC supports Fast Ethernet PHY. Add a driver for it.
> >>>
> >>> Signed-off-by: Jernej Skrabec <[email protected]>
> >>> ---
> >>>
> >>> drivers/net/phy/Kconfig | 7 ++
> >>> drivers/net/phy/Makefile | 1 +
> >>> drivers/net/phy/ac200.c | 206 +++++++++++++++++++++++++++++++++++++++
> >>> 3 files changed, 214 insertions(+)
> >>> create mode 100644 drivers/net/phy/ac200.c
> >>>
> >>> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> >>> index 3fa33d27eeba..16af69f69eaf 100644
> >>> --- a/drivers/net/phy/Kconfig
> >>> +++ b/drivers/net/phy/Kconfig
> >>> @@ -288,6 +288,13 @@ config ADIN_PHY
> >>>
> >>> - ADIN1300 - Robust,Industrial, Low Latency 10/100/1000 Gigabit
> >>>
> >>> Ethernet PHY
> >>>
> >>> +config AC200_PHY
> >>> + tristate "AC200 EPHY"
> >>> + depends on NVMEM
> >>> + depends on OF
> >>> + help
> >>> + Fast ethernet PHY as found in X-Powers AC200 multi-function
> >
> > device.
> >
> >>> +
> >>>
> >>> config AMD_PHY
> >>>
> >>> tristate "AMD PHYs"
> >>> ---help---
> >>>
> >>> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> >>> index 2f5c7093a65b..b0c5b91900fa 100644
> >>> --- a/drivers/net/phy/Makefile
> >>> +++ b/drivers/net/phy/Makefile
> >>> @@ -53,6 +53,7 @@ obj-$(CONFIG_SFP) += sfp.o
> >>>
> >>> sfp-obj-$(CONFIG_SFP) += sfp-bus.o
> >>> obj-y += $(sfp-obj-y) $(sfp-obj-m)
> >>>
> >>> +obj-$(CONFIG_AC200_PHY) += ac200.o
> >>>
> >>> obj-$(CONFIG_ADIN_PHY) += adin.o
> >>> obj-$(CONFIG_AMD_PHY) += amd.o
> >>> aquantia-objs += aquantia_main.o
> >>>
> >>> diff --git a/drivers/net/phy/ac200.c b/drivers/net/phy/ac200.c
> >>> new file mode 100644
> >>> index 000000000000..3d7856ff8f91
> >>> --- /dev/null
> >>> +++ b/drivers/net/phy/ac200.c
> >>> @@ -0,0 +1,206 @@
> >>> +// SPDX-License-Identifier: GPL-2.0+
> >>> +/**
> >>> + * Driver for AC200 Ethernet PHY
> >>> + *
> >>> + * Copyright (c) 2020 Jernej Skrabec <[email protected]>
> >>> + */
> >>> +
> >>> +#include <linux/kernel.h>
> >>> +#include <linux/module.h>
> >>> +#include <linux/mfd/ac200.h>
> >>> +#include <linux/nvmem-consumer.h>
> >>> +#include <linux/of.h>
> >>> +#include <linux/phy.h>
> >>> +#include <linux/platform_device.h>
> >>> +
> >>> +#define AC200_EPHY_ID 0x00441400
> >>> +#define AC200_EPHY_ID_MASK 0x0ffffff0
> >>> +
> >>
> >> You could use PHY_ID_MATCH_MODEL() here.
> >
> > Ok.
> >
> >>> +/* macros for system ephy control 0 register */
> >>> +#define AC200_EPHY_RESET_INVALID BIT(0)
> >>> +#define AC200_EPHY_SYSCLK_GATING BIT(1)
> >>> +
> >>> +/* macros for system ephy control 1 register */
> >>> +#define AC200_EPHY_E_EPHY_MII_IO_EN BIT(0)
> >>> +#define AC200_EPHY_E_LNK_LED_IO_EN BIT(1)
> >>> +#define AC200_EPHY_E_SPD_LED_IO_EN BIT(2)
> >>> +#define AC200_EPHY_E_DPX_LED_IO_EN BIT(3)
> >>> +
> >>> +/* macros for ephy control register */
> >>> +#define AC200_EPHY_SHUTDOWN BIT(0)
> >>> +#define AC200_EPHY_LED_POL BIT(1)
> >>> +#define AC200_EPHY_CLK_SEL BIT(2)
> >>> +#define AC200_EPHY_ADDR(x) (((x) & 0x1F) << 4)
> >>> +#define AC200_EPHY_XMII_SEL BIT(11)
> >>> +#define AC200_EPHY_CALIB(x) (((x) & 0xF) << 12)
> >>> +
> >>> +struct ac200_ephy_dev {
> >>> + struct phy_driver *ephy;
> >>
> >> Why embedding a pointer and not a struct phy_driver directly?
> >> Then you could omit the separate allocation.
> >
> > Right.
> >
> >> ephy is not the best naming. It may be read as a phy_device.
> >> Better use phydrv.
> >
> > Ok.
> >
> >>> + struct regmap *regmap;
> >>> +};
> >>> +
> >>> +static char *ac200_phy_name = "AC200 EPHY";
> >>> +
> >>
> >> Why not using the name directly in the assignment?
> >
> > phy_driver->name is pointer. Wouldn't that mean that string is allocated
> > on
> > stack and next time pointer is used, it will return garbage?
>
> No, it's not on the stack. No problem here.
>
> >> And better naming: "AC200 Fast Ethernet"
> >
> > Ok.
> >
> >>> +static int ac200_ephy_config_init(struct phy_device *phydev)
> >>> +{
> >>> + const struct ac200_ephy_dev *priv = phydev->drv->driver_data;
> >>> + unsigned int value;
> >>> + int ret;
> >>> +
> >>> + phy_write(phydev, 0x1f, 0x0100); /* Switch to Page 1 */
> >>> + phy_write(phydev, 0x12, 0x4824); /* Disable APS */
> >>> +
> >>> + phy_write(phydev, 0x1f, 0x0200); /* Switch to Page 2 */
> >>> + phy_write(phydev, 0x18, 0x0000); /* PHYAFE TRX optimization */
> >>> +
> >>> + phy_write(phydev, 0x1f, 0x0600); /* Switch to Page 6 */
> >>> + phy_write(phydev, 0x14, 0x708f); /* PHYAFE TX optimization */
> >>> + phy_write(phydev, 0x13, 0xF000); /* PHYAFE RX optimization */
> >>> + phy_write(phydev, 0x15, 0x1530);
> >>> +
> >>> + phy_write(phydev, 0x1f, 0x0800); /* Switch to Page 6 */
> >>> + phy_write(phydev, 0x18, 0x00bc); /* PHYAFE TRX optimization */
> >>> +
> >>> + phy_write(phydev, 0x1f, 0x0100); /* switch to page 1 */
> >>> + phy_clear_bits(phydev, 0x17, BIT(3)); /* disable intelligent
> >
> > IEEE */
> >
> >>> +
> >>> + /* next two blocks disable 802.3az IEEE */
> >>> + phy_write(phydev, 0x1f, 0x0200); /* switch to page 2 */
> >>> + phy_write(phydev, 0x18, 0x0000);
> >>> +
> >>> + phy_write(phydev, 0x1f, 0x0000); /* switch to page 0 */
> >>> + phy_clear_bits_mmd(phydev, 0x7, 0x3c, BIT(1));
> >>
> >> Better use the following:
> >> phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0x0000);
> >> It makes clear that you disable advertising EEE completely.
> >
> > Ok.
> >
> >>> +
> >>> + if (phydev->interface == PHY_INTERFACE_MODE_RMII)
> >>> + value = AC200_EPHY_XMII_SEL;
> >>> + else
> >>> + value = 0;
> >>> +
> >>> + ret = regmap_update_bits(priv->regmap, AC200_EPHY_CTL,
> >>> + AC200_EPHY_XMII_SEL, value);
> >>> + if (ret)
> >>> + return ret;
> >>> +
> >>
> >> I had a brief look at the spec, and it's not fully clear
> >> to me what this register setting does. Does it affect the
> >> MAC side and/or the PHY side?
> >
> > It's my understanding that it selects interface mode on PHY. Besides
> > datasheet mentioned in cover letter, BSP drivers (one for MFD and one for
> > PHY) are the only other source of information. BSP PHY driver is located
> > here:
> > https://github.com/Allwinner-Homlet/H6-BSP4.9-linux/blob/master/drivers/ne
> > t/ phy/sunxi-ephy.c
> >
> >> If it affects the PHY side, then I'd expect that the chip
> >> has to talk to the PHY via the MDIO bus. Means there should
> >> be a PHY register for setting MII vs. RMII.
> >> In this case the setup could be very much simplified.
> >> Then the PHY driver wouldn't have to be embedded in the
> >> platform driver.
> >
> > Actually, PHY has to be configured first through I2C and then through
> > MDIO. I2C is used to enable it (power it up), configure LED polarity, set
> > PHY address, write calibration value stored elsewhere.
> >
> > Based on all available documentation I have (code and datasheet), this I2C
> > register is the only way to select MII or RMII mode.
>
> Then how and where is the PHY interface mode configured on the MAC side?
> If there is no such setting, then I'd assume that this register bit
> configures both sides. This leads to the question whether the interface
> mode really needs to be set in the PHY driver's config_init().
> If we could avoid this, then you could make the PHY driver static.
Check patch 4. There is emac node added with property phy-mode = "rmii";
AC200 and H6 are internally connected through RMII interface.
Note that MAC has multiplexed interface and can either uses this copackaged
PHY or external PHY. External PHY usually uses RGMII interface.
In 99% cases, this PHY driver will be used for copackaged AC200 with H6 SoC,
which means it will be used in RMII mode. Even if someone uses standalone
AC200 IC with mainline Linux, will probably still use RMII. But there is still
very small chance that someone will use it in MII mode.
Anyway, if that special setting for H6 proves important, we will still need a
way to convey that information from platform driver to PHY. Easiest way to do
that is through driver_data.
>
> You could set the PHY interface mode as soon as the PHY interface mode
> is read from DT. So why not set the interface mode at the place where
> you configure the other values like PHY address?
PHY interface mode is actually set on MAC side in DT. This PHY has acutally
programmable PHY address through I2C. Currently I hardcoded it to 1.
As I explained in cover letter, I don't really know how to properly present
this device in DT. Based on current DT documentation, PHY node would have to
be child node of mdio bus node and MFD node at the same time which is not
possible.
>
> >>> + /* FIXME: This is H6 specific */
> >>> + phy_set_bits(phydev, 0x13, BIT(12));
> >>> +
> >>
> >> This seems to indicate that the same PHY is used in a slightly
> >> different version with other Hx models. Do they use different
> >> PHY ID's?
> >
> > Situation is a bit complicated. Same PHY, at least with same PHY ID, is
> > used in different ways.
> > 1. as part of standalone AC200 MFD IC
> > 2. as part of AC200 wafer copackaged with H6 SoC wafer in same package.
> > This in theory shouldn't be any different than standalone IC, but it
> > apparently is, based on the BSP driver code.
> > 3. integrated directly in SoCs like H3, H5 and V3s. There is no I2C access
> > to configuration register. Instead, it's memory mapped and slightly
> > different.
> >
> > In all cases PHY ID is same, just glue logic is different.
> >
> > I asked Allwinner if above setting is really necessary for H6 and what it
> > does, but I didn't get any useful answer back.
> >
> > So maybe another compatible is needed for H6.
> >
> > Best regards,
> > Jernej
> >
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int ac200_ephy_probe(struct platform_device *pdev)
> >>> +{
> >>> + struct ac200_dev *ac200 = dev_get_drvdata(pdev->dev.parent);
> >>> + struct device *dev = &pdev->dev;
> >>> + struct ac200_ephy_dev *priv;
> >>> + struct nvmem_cell *calcell;
> >>> + struct phy_driver *ephy;
> >>> + u16 *caldata, calib;
> >>> + size_t callen;
> >>> + int ret;
> >>> +
> >>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> >>> + if (!priv)
> >>> + return -ENOMEM;
> >>> +
> >>> + ephy = devm_kzalloc(dev, sizeof(*ephy), GFP_KERNEL);
> >>> + if (!ephy)
> >>> + return -ENOMEM;
> >>> +
> >>> + calcell = devm_nvmem_cell_get(dev, "calibration");
> >>> + if (IS_ERR(calcell)) {
> >>> + dev_err(dev, "Unable to find calibration data!\n");
> >>> + return PTR_ERR(calcell);
> >>> + }
> >>> +
> >>> + caldata = nvmem_cell_read(calcell, &callen);
> >>> + if (IS_ERR(caldata)) {
> >>> + dev_err(dev, "Unable to read calibration data!\n");
> >>> + return PTR_ERR(caldata);
> >>> + }
> >>> +
> >>> + if (callen != 2) {
> >>> + dev_err(dev, "Calibration data has wrong length: 2 !=
> >
> > %zu\n",
> >
> >>> + callen);
> >>> + kfree(caldata);
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + calib = *caldata + 3;
> >>> + kfree(caldata);
> >>> +
> >>> + ephy->phy_id = AC200_EPHY_ID;
> >>> + ephy->phy_id_mask = AC200_EPHY_ID_MASK;
> >>> + ephy->name = ac200_phy_name;
> >>> + ephy->driver_data = priv;
> >>> + ephy->soft_reset = genphy_soft_reset;
> >>> + ephy->config_init = ac200_ephy_config_init;
> >>> + ephy->suspend = genphy_suspend;
> >>> + ephy->resume = genphy_resume;
> >>> +
> >>> + priv->ephy = ephy;
> >>> + priv->regmap = ac200->regmap;
> >>> + platform_set_drvdata(pdev, priv);
> >>> +
> >>> + ret = regmap_write(ac200->regmap, AC200_SYS_EPHY_CTL0,
> >>> + AC200_EPHY_RESET_INVALID |
> >>> + AC200_EPHY_SYSCLK_GATING);
> >>> + if (ret)
> >>> + return ret;
> >>> +
> >>> + ret = regmap_write(ac200->regmap, AC200_SYS_EPHY_CTL1,
> >>> + AC200_EPHY_E_EPHY_MII_IO_EN |
> >>> + AC200_EPHY_E_LNK_LED_IO_EN |
> >>> + AC200_EPHY_E_SPD_LED_IO_EN |
> >>> + AC200_EPHY_E_DPX_LED_IO_EN);
> >>> + if (ret)
> >>> + return ret;
> >>> +
> >>> + ret = regmap_write(ac200->regmap, AC200_EPHY_CTL,
> >>> + AC200_EPHY_LED_POL |
> >>> + AC200_EPHY_CLK_SEL |
> >>> + AC200_EPHY_ADDR(1) |
> >>> + AC200_EPHY_CALIB(calib));
> >>> + if (ret)
> >>> + return ret;
> >>> +
> >>> + ret = phy_driver_register(priv->ephy, THIS_MODULE);
> >>> + if (ret) {
> >>> + dev_err(dev, "Unable to register phy\n");
> >>> + return ret;
> >>> + }
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int ac200_ephy_remove(struct platform_device *pdev)
> >>> +{
> >>> + struct ac200_ephy_dev *priv = platform_get_drvdata(pdev);
> >>> +
> >>> + phy_driver_unregister(priv->ephy);
> >>> +
> >>> + regmap_write(priv->regmap, AC200_EPHY_CTL, AC200_EPHY_SHUTDOWN);
> >>> + regmap_write(priv->regmap, AC200_SYS_EPHY_CTL1, 0);
> >>> + regmap_write(priv->regmap, AC200_SYS_EPHY_CTL0, 0);
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static const struct of_device_id ac200_ephy_match[] = {
> >>> + { .compatible = "x-powers,ac200-ephy" },
> >>> + { /* sentinel */ }
> >>> +};
> >>> +MODULE_DEVICE_TABLE(of, ac200_ephy_match);
> >>> +
> >>> +static struct platform_driver ac200_ephy_driver = {
> >>> + .probe = ac200_ephy_probe,
> >>> + .remove = ac200_ephy_remove,
> >>> + .driver = {
> >>> + .name = "ac200-ephy",
> >>> + .of_match_table = ac200_ephy_match,
> >>> + },
> >>> +};
> >>> +module_platform_driver(ac200_ephy_driver);
> >>> +
> >>> +MODULE_AUTHOR("Jernej Skrabec <[email protected]>");
> >>> +MODULE_DESCRIPTION("AC200 Ethernet PHY driver");
> >>> +MODULE_LICENSE("GPL");