[U-Boot] [PATCH 0/9] phy: atheros: cleanup and device tree bindings
This series cleans up the Atheros PHY AR803x PHY driver and adds a device tree binding for the most commonly used PHY settings like clock output.
If you're a board maintainer you're getting this mail because you probably use an AR803x PHY on your board. Please have a look at your board specific code and see if you can use the device tree bindings instead. Let me know, if something is missing.
Michael Walle (9): phy: atheros: introduce debug read and write functions phy: atheros: move delay config to common function phy: atheros: ar8035: remove extra delay config phy: atheros: ar8035: use phy_{read|write}_mmd() phy: atheros: don't overwrite debug register values phy: atheros: fix delay configuration phy: atheros: Add device tree bindings and config phy: atheros: ar8035: remove static clock config phy: atheros: consolidate {ar8031|ar8035}_config()
doc/device-tree-bindings/net/phy/atheros.txt | 22 ++ drivers/net/phy/atheros.c | 262 +++++++++++++++---- 2 files changed, 240 insertions(+), 44 deletions(-) create mode 100644 doc/device-tree-bindings/net/phy/atheros.txt
Cc: Joe Hershberger joe.hershberger@ni.com Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Adam Ford aford173@gmail.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Ian Ray ian.ray@ge.com Cc: Jason Liu jason.hui.liu@nxp.com Cc: Ye Li ye.li@nxp.com Cc: Uri Mashiach uri.mashiach@compulab.co.il Cc: Nikita Kiryanov nikita@compulab.co.il Cc: Lukasz Majewski lukma@denx.de Cc: Fabio Estevam festevam@gmail.com Cc: Vanessa Maegima vanessa.maegima@nxp.com Cc: Eric Bénard eric@eukrea.com
Provide functions to read and write the Atheros debug registers.
Signed-off-by: Michael Walle michael@walle.cc --- drivers/net/phy/atheros.c | 72 ++++++++++++++++++++++++++++----------- 1 file changed, 53 insertions(+), 19 deletions(-)
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index 3783d155e7..b25aa02108 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -17,11 +17,52 @@ #define AR803x_DEBUG_REG_0 0x0 #define AR803x_RGMII_RX_CLK_DLY 0x8000
+static int ar803x_debug_reg_read(struct phy_device *phydev, u16 reg) +{ + int ret; + + ret = phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG, + reg); + if (ret < 0) + return ret; + + return phy_read(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG); +} + +static int ar803x_debug_reg_write(struct phy_device *phydev, u16 reg, u16 val) +{ + int ret; + + ret = phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG, + reg); + if (ret < 0) + return ret; + + return phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, + val); +} + +static int ar803x_debug_reg_mask(struct phy_device *phydev, u16 reg, + u16 clear, u16 set) +{ + int val; + + val = ar803x_debug_reg_read(phydev, reg); + if (val < 0) + return val; + + val &= 0xffff; + val &= ~clear; + val |= set; + + return phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, + val); +} + static int ar8021_config(struct phy_device *phydev) { phy_write(phydev, MDIO_DEVAD_NONE, 0x00, 0x1200); - phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05); - phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x3D47); + ar803x_debug_reg_write(phydev, AR803x_DEBUG_REG_5, 0x3D47);
phydev->supported = phydev->drv->features; return 0; @@ -31,18 +72,14 @@ static int ar8031_config(struct phy_device *phydev) { if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID || phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) { - phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG, - AR803x_DEBUG_REG_5); - phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, - AR803x_RGMII_TX_CLK_DLY); + ar803x_debug_reg_write(phydev, AR803x_DEBUG_REG_5, + AR803x_RGMII_TX_CLK_DLY); }
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID || phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) { - phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG, - AR803x_DEBUG_REG_0); - phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, - AR803x_RGMII_RX_CLK_DLY); + ar803x_debug_reg_write(phydev, AR803x_DEBUG_REG_0, + AR803x_RGMII_RX_CLK_DLY); }
phydev->supported = phydev->drv->features; @@ -63,24 +100,21 @@ static int ar8035_config(struct phy_device *phydev) regval = phy_read(phydev, MDIO_DEVAD_NONE, 0xe); phy_write(phydev, MDIO_DEVAD_NONE, 0xe, (regval|0x0018));
- phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05); - regval = phy_read(phydev, MDIO_DEVAD_NONE, 0x1e); - phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, (regval|0x0100)); + ar803x_debug_reg_mask(phydev, AR803x_DEBUG_REG_5, + 0, AR803x_RGMII_TX_CLK_DLY);
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) || (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)) { - /* select debug reg 5 */ - phy_write(phydev, MDIO_DEVAD_NONE, 0x1D, 0x5); /* enable tx delay */ - phy_write(phydev, MDIO_DEVAD_NONE, 0x1E, 0x0100); + ar803x_debug_reg_write(phydev, AR803x_DEBUG_REG_5, + AR803x_RGMII_TX_CLK_DLY); }
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) || (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)) { - /* select debug reg 0 */ - phy_write(phydev, MDIO_DEVAD_NONE, 0x1D, 0x0); /* enable rx delay */ - phy_write(phydev, MDIO_DEVAD_NONE, 0x1E, 0x8000); + ar803x_debug_reg_write(phydev, AR803x_DEBUG_REG_0, + AR803x_RGMII_RX_CLK_DLY); }
phydev->supported = phydev->drv->features;
On Fri, Oct 25, 2019 at 7:28 PM Michael Walle michael@walle.cc wrote:
Provide functions to read and write the Atheros debug registers.
Signed-off-by: Michael Walle michael@walle.cc
Acked-by: Joe Hershberger joe.hershberger@ni.com
Am 2019-11-30 02:11, schrieb Joe Hershberger:
On Fri, Oct 25, 2019 at 7:28 PM Michael Walle michael@walle.cc wrote:
Provide functions to read and write the Atheros debug registers.
Signed-off-by: Michael Walle michael@walle.cc
Acked-by: Joe Hershberger joe.hershberger@ni.com
Sorry this was superseeded by https://patchwork.ozlabs.org/project/uboot/list/?series=146771
Unfortunately, I've forgot to add the mailinglist to the original series. So it never ended up in the patchwork system :(
-michael
Am 2019-11-30 02:11, schrieb Joe Hershberger:
On Fri, Oct 25, 2019 at 7:28 PM Michael Walle michael@walle.cc wrote:
Provide functions to read and write the Atheros debug registers.
Signed-off-by: Michael Walle michael@walle.cc
Acked-by: Joe Hershberger joe.hershberger@ni.com
Sorry this series superseeded by https://patchwork.ozlabs.org/project/uboot/list/?series=146771
Unfortunately, I've forgot to add the mailinglist to the original series. So it never ended up in the patchwork system :(
-michael
Hi Michael,
On Fri, Oct 25, 2019 at 7:28 PM Michael Walle michael@walle.cc wrote:
Provide functions to read and write the Atheros debug registers.
Signed-off-by: Michael Walle michael@walle.cc
This series is adding too much size to several of the boards' SPL it seems.
https://travis-ci.org/jhershbe/u-boot/builds/620804934
Please address this and resend.
Thanks, -Joe
Hi Joe, Hi Tom,
Am 2019-12-05 16:55, schrieb Joe Hershberger:
Hi Michael,
On Fri, Oct 25, 2019 at 7:28 PM Michael Walle michael@walle.cc wrote:
Provide functions to read and write the Atheros debug registers.
Signed-off-by: Michael Walle michael@walle.cc
This series is adding too much size to several of the boards' SPL it seems.
https://travis-ci.org/jhershbe/u-boot/builds/620804934
Please address this and resend.
So first of all, this was the old series. There was a v2 series, but unfortunately, I've forgot to add the mailing to the recipients, so it never ended up in the patchwork system. sorry :(
I've resend the v2 series here: https://patchwork.ozlabs.org/project/uboot/list/?series=146771
Now coming to the real problem here. The sizes, or like some boards handle the SPL stuff. Btw. I could not reproduce it on the am335x_boneblack_vboot board with a gcc-8. I've seen the travis ci job uses the gcc-7 so this also depends on the gcc. gcc-8 seems to produce smaller code, because on the am335x_evm the overflow was only by some 100 bytes.
So taking the am335x_evm board for example. It has the following options set: CONFIG_DM_ETH=y CONFIG_SPL_NET_SUPPORT=y CONFIG_PHY_ATHEROS=y (this one is set in the config.h!)
So adding a new binding for the phy obviously increases the code size. But the hard question is, how could that be fixed. IMHO the board has wrong settings. I really don't know how that could be "fixed" other than not applying this series. Well, we could make the additions conditional and introduce a new Kconfig setting, but that is a relly ugly hack and won't last long, would it? Doh!
-michael
On Fri, Dec 06, 2019 at 12:27:39AM +0100, Michael Walle wrote:
Hi Joe, Hi Tom,
Am 2019-12-05 16:55, schrieb Joe Hershberger:
Hi Michael,
On Fri, Oct 25, 2019 at 7:28 PM Michael Walle michael@walle.cc wrote:
Provide functions to read and write the Atheros debug registers.
Signed-off-by: Michael Walle michael@walle.cc
This series is adding too much size to several of the boards' SPL it seems.
https://travis-ci.org/jhershbe/u-boot/builds/620804934
Please address this and resend.
So first of all, this was the old series. There was a v2 series, but unfortunately, I've forgot to add the mailing to the recipients, so it never ended up in the patchwork system. sorry :(
I've resend the v2 series here: https://patchwork.ozlabs.org/project/uboot/list/?series=146771
Now coming to the real problem here. The sizes, or like some boards handle the SPL stuff. Btw. I could not reproduce it on the am335x_boneblack_vboot board with a gcc-8. I've seen the travis ci job uses the gcc-7 so this also depends on the gcc. gcc-8 seems to produce smaller code, because on the am335x_evm the overflow was only by some 100 bytes.
So taking the am335x_evm board for example. It has the following options set: CONFIG_DM_ETH=y CONFIG_SPL_NET_SUPPORT=y CONFIG_PHY_ATHEROS=y (this one is set in the config.h!)
So adding a new binding for the phy obviously increases the code size. But the hard question is, how could that be fixed. IMHO the board has wrong settings. I really don't know how that could be "fixed" other than not applying this series. Well, we could make the additions conditional and introduce a new Kconfig setting, but that is a relly ugly hack and won't last long, would it? Doh!
So, the gcc-7 from kernel.org is the min required and must work toolchain. Maybe once gcc-9 is mature enough for people to have made stand-alone toolchains for it we'll move up to that but gcc-8 for everyone ends up being too hard. For the boneblack_vboot config, we could just drop SPL networking, it's not super critical to that particular example. But am335x_evm is the kitchen-sink EVM and it is used and supported there.
That said, looking over the u-boot-spl.map, it looks like nfs stuff doesn't get discarded for some reason, I'm going to look in to that.
Hi Tom,
Am 2019-12-06 00:58, schrieb Tom Rini:
On Fri, Dec 06, 2019 at 12:27:39AM +0100, Michael Walle wrote:
Hi Joe, Hi Tom,
Am 2019-12-05 16:55, schrieb Joe Hershberger:
Hi Michael,
On Fri, Oct 25, 2019 at 7:28 PM Michael Walle michael@walle.cc wrote:
Provide functions to read and write the Atheros debug registers.
Signed-off-by: Michael Walle michael@walle.cc
This series is adding too much size to several of the boards' SPL it seems.
https://travis-ci.org/jhershbe/u-boot/builds/620804934
Please address this and resend.
So first of all, this was the old series. There was a v2 series, but unfortunately, I've forgot to add the mailing to the recipients, so it never ended up in the patchwork system. sorry :(
I've resend the v2 series here: https://patchwork.ozlabs.org/project/uboot/list/?series=146771
Now coming to the real problem here. The sizes, or like some boards handle the SPL stuff. Btw. I could not reproduce it on the am335x_boneblack_vboot board with a gcc-8. I've seen the travis ci job uses the gcc-7 so this also depends on the gcc. gcc-8 seems to produce smaller code, because on the am335x_evm the overflow was only by some 100 bytes.
So taking the am335x_evm board for example. It has the following options set: CONFIG_DM_ETH=y CONFIG_SPL_NET_SUPPORT=y CONFIG_PHY_ATHEROS=y (this one is set in the config.h!)
So adding a new binding for the phy obviously increases the code size. But the hard question is, how could that be fixed. IMHO the board has wrong settings. I really don't know how that could be "fixed" other than not applying this series. Well, we could make the additions conditional and introduce a new Kconfig setting, but that is a relly ugly hack and won't last long, would it? Doh!
So, the gcc-7 from kernel.org is the min required and must work toolchain.
Ok.
Maybe once gcc-9 is mature enough for people to have made stand-alone toolchains for it we'll move up to that but gcc-8 for everyone ends up being too hard. For the boneblack_vboot config, we could just drop SPL networking, it's not super critical to that particular example. But am335x_evm is the kitchen-sink EVM and it is used and supported there.
That said, looking over the u-boot-spl.map, it looks like nfs stuff doesn't get discarded for some reason, I'm going to look in to that.
Well but that is just some kind of band aid. These two boards were just an example. There were also other boards which failed. I doubt the DM stuff is getting smaller (I know there is some discussion how to improve the situation). But there seems to be no way to disable the ethernet device model for the SPL, correct? And unfortunately, this also pulls the PHY library (with potential DM bindings).
-michael
On Fri, Dec 06, 2019 at 01:39:29AM +0100, Michael Walle wrote:
Hi Tom,
Am 2019-12-06 00:58, schrieb Tom Rini:
On Fri, Dec 06, 2019 at 12:27:39AM +0100, Michael Walle wrote:
Hi Joe, Hi Tom,
Am 2019-12-05 16:55, schrieb Joe Hershberger:
Hi Michael,
On Fri, Oct 25, 2019 at 7:28 PM Michael Walle michael@walle.cc wrote:
Provide functions to read and write the Atheros debug registers.
Signed-off-by: Michael Walle michael@walle.cc
This series is adding too much size to several of the boards' SPL it seems.
https://travis-ci.org/jhershbe/u-boot/builds/620804934
Please address this and resend.
So first of all, this was the old series. There was a v2 series, but unfortunately, I've forgot to add the mailing to the recipients, so it never ended up in the patchwork system. sorry :(
I've resend the v2 series here: https://patchwork.ozlabs.org/project/uboot/list/?series=146771
Now coming to the real problem here. The sizes, or like some boards handle the SPL stuff. Btw. I could not reproduce it on the am335x_boneblack_vboot board with a gcc-8. I've seen the travis ci job uses the gcc-7 so this also depends on the gcc. gcc-8 seems to produce smaller code, because on the am335x_evm the overflow was only by some 100 bytes.
So taking the am335x_evm board for example. It has the following options set: CONFIG_DM_ETH=y CONFIG_SPL_NET_SUPPORT=y CONFIG_PHY_ATHEROS=y (this one is set in the config.h!)
So adding a new binding for the phy obviously increases the code size. But the hard question is, how could that be fixed. IMHO the board has wrong settings. I really don't know how that could be "fixed" other than not applying this series. Well, we could make the additions conditional and introduce a new Kconfig setting, but that is a relly ugly hack and won't last long, would it? Doh!
So, the gcc-7 from kernel.org is the min required and must work toolchain.
Ok.
Maybe once gcc-9 is mature enough for people to have made stand-alone toolchains for it we'll move up to that but gcc-8 for everyone ends up being too hard. For the boneblack_vboot config, we could just drop SPL networking, it's not super critical to that particular example. But am335x_evm is the kitchen-sink EVM and it is used and supported there.
That said, looking over the u-boot-spl.map, it looks like nfs stuff doesn't get discarded for some reason, I'm going to look in to that.
Well but that is just some kind of band aid. These two boards were just an example. There were also other boards which failed. I doubt the DM stuff is getting smaller (I know there is some discussion how to improve the situation). But there seems to be no way to disable the ethernet device model for the SPL, correct? And unfortunately, this also pulls the PHY library (with potential DM bindings).
So, yes, keeping in mind that SPL matters for networking and we need to keep that in mind when adding features. Thanks!
Am 2019-12-06 00:58, schrieb Tom Rini:
That said, looking over the u-boot-spl.map, it looks like nfs stuff doesn't get discarded for some reason, I'm going to look in to that.
CONFIG_CMD_NFS will pull that. There are also other network related cmds whose code is pulled into the SPL through net/net.c.
-michael
On Fri, Dec 06, 2019 at 01:53:57AM +0100, Michael Walle wrote:
Am 2019-12-06 00:58, schrieb Tom Rini:
That said, looking over the u-boot-spl.map, it looks like nfs stuff doesn't get discarded for some reason, I'm going to look in to that.
CONFIG_CMD_NFS will pull that. There are also other network related cmds whose code is pulled into the SPL through net/net.c.
Right. The way the networking stack behaves today I bet we have other functionality being pulled in to SPL that we would rather be able to discard as it's functionally unused. What I sent to keep NFS out is a little hacky looking but is the best we can do with the current structure. There might be a few other options we can drop out from the loop.
Hi Tom, Hi Joe,
Am 2019-12-06 00:58, schrieb Tom Rini:
On Fri, Dec 06, 2019 at 12:27:39AM +0100, Michael Walle wrote:
Hi Joe, Hi Tom,
Am 2019-12-05 16:55, schrieb Joe Hershberger:
Hi Michael,
On Fri, Oct 25, 2019 at 7:28 PM Michael Walle michael@walle.cc wrote:
Provide functions to read and write the Atheros debug registers.
Signed-off-by: Michael Walle michael@walle.cc
This series is adding too much size to several of the boards' SPL it seems.
https://travis-ci.org/jhershbe/u-boot/builds/620804934
Please address this and resend.
So first of all, this was the old series. There was a v2 series, but unfortunately, I've forgot to add the mailing to the recipients, so it never ended up in the patchwork system. sorry :(
I've resend the v2 series here: https://patchwork.ozlabs.org/project/uboot/list/?series=146771
Now coming to the real problem here. The sizes, or like some boards handle the SPL stuff. Btw. I could not reproduce it on the am335x_boneblack_vboot board with a gcc-8. I've seen the travis ci job uses the gcc-7 so this also depends on the gcc. gcc-8 seems to produce smaller code, because on the am335x_evm the overflow was only by some 100 bytes.
So taking the am335x_evm board for example. It has the following options set: CONFIG_DM_ETH=y CONFIG_SPL_NET_SUPPORT=y CONFIG_PHY_ATHEROS=y (this one is set in the config.h!)
So adding a new binding for the phy obviously increases the code size. But the hard question is, how could that be fixed. IMHO the board has wrong settings. I really don't know how that could be "fixed" other than not applying this series. Well, we could make the additions conditional and introduce a new Kconfig setting, but that is a relly ugly hack and won't last long, would it? Doh!
So, the gcc-7 from kernel.org is the min required and must work toolchain. Maybe once gcc-9 is mature enough for people to have made stand-alone toolchains for it we'll move up to that but gcc-8 for everyone ends up being too hard. For the boneblack_vboot config, we could just drop SPL networking, it's not super critical to that particular example. But am335x_evm is the kitchen-sink EVM and it is used and supported there.
That said, looking over the u-boot-spl.map, it looks like nfs stuff doesn't get discarded for some reason, I'm going to look in to that.
So do I need to do something now? I guess removing the NFS stuff will make enough room to fit this. But how do we make sure, it will be applied with this series?
-michael
On Mon, Dec 09, 2019 at 07:42:00PM +0100, Michael Walle wrote:
Hi Tom, Hi Joe,
Am 2019-12-06 00:58, schrieb Tom Rini:
On Fri, Dec 06, 2019 at 12:27:39AM +0100, Michael Walle wrote:
Hi Joe, Hi Tom,
Am 2019-12-05 16:55, schrieb Joe Hershberger:
Hi Michael,
On Fri, Oct 25, 2019 at 7:28 PM Michael Walle michael@walle.cc wrote:
Provide functions to read and write the Atheros debug registers.
Signed-off-by: Michael Walle michael@walle.cc
This series is adding too much size to several of the boards' SPL it seems.
https://travis-ci.org/jhershbe/u-boot/builds/620804934
Please address this and resend.
So first of all, this was the old series. There was a v2 series, but unfortunately, I've forgot to add the mailing to the recipients, so it never ended up in the patchwork system. sorry :(
I've resend the v2 series here: https://patchwork.ozlabs.org/project/uboot/list/?series=146771
Now coming to the real problem here. The sizes, or like some boards handle the SPL stuff. Btw. I could not reproduce it on the am335x_boneblack_vboot board with a gcc-8. I've seen the travis ci job uses the gcc-7 so this also depends on the gcc. gcc-8 seems to produce smaller code, because on the am335x_evm the overflow was only by some 100 bytes.
So taking the am335x_evm board for example. It has the following options set: CONFIG_DM_ETH=y CONFIG_SPL_NET_SUPPORT=y CONFIG_PHY_ATHEROS=y (this one is set in the config.h!)
So adding a new binding for the phy obviously increases the code size. But the hard question is, how could that be fixed. IMHO the board has wrong settings. I really don't know how that could be "fixed" other than not applying this series. Well, we could make the additions conditional and introduce a new Kconfig setting, but that is a relly ugly hack and won't last long, would it? Doh!
So, the gcc-7 from kernel.org is the min required and must work toolchain. Maybe once gcc-9 is mature enough for people to have made stand-alone toolchains for it we'll move up to that but gcc-8 for everyone ends up being too hard. For the boneblack_vboot config, we could just drop SPL networking, it's not super critical to that particular example. But am335x_evm is the kitchen-sink EVM and it is used and supported there.
That said, looking over the u-boot-spl.map, it looks like nfs stuff doesn't get discarded for some reason, I'm going to look in to that.
So do I need to do something now? I guess removing the NFS stuff will make enough room to fit this. But how do we make sure, it will be applied with this series?
In this case, Joe has it in the -net PR right now. Thanks!
Hi Tom,
On Mon, Dec 9, 2019 at 12:47 PM Tom Rini trini@konsulko.com wrote:
On Mon, Dec 09, 2019 at 07:42:00PM +0100, Michael Walle wrote:
Hi Tom, Hi Joe,
Am 2019-12-06 00:58, schrieb Tom Rini:
On Fri, Dec 06, 2019 at 12:27:39AM +0100, Michael Walle wrote:
Hi Joe, Hi Tom,
Am 2019-12-05 16:55, schrieb Joe Hershberger:
Hi Michael,
On Fri, Oct 25, 2019 at 7:28 PM Michael Walle michael@walle.cc wrote:
Provide functions to read and write the Atheros debug registers.
Signed-off-by: Michael Walle michael@walle.cc
This series is adding too much size to several of the boards' SPL it seems.
https://travis-ci.org/jhershbe/u-boot/builds/620804934
Please address this and resend.
So first of all, this was the old series. There was a v2 series, but unfortunately, I've forgot to add the mailing to the recipients, so it never ended up in the patchwork system. sorry :(
I've resend the v2 series here: https://patchwork.ozlabs.org/project/uboot/list/?series=146771
Now coming to the real problem here. The sizes, or like some boards handle the SPL stuff. Btw. I could not reproduce it on the am335x_boneblack_vboot board with a gcc-8. I've seen the travis ci job uses the gcc-7 so this also depends on the gcc. gcc-8 seems to produce smaller code, because on the am335x_evm the overflow was only by some 100 bytes.
So taking the am335x_evm board for example. It has the following options set: CONFIG_DM_ETH=y CONFIG_SPL_NET_SUPPORT=y CONFIG_PHY_ATHEROS=y (this one is set in the config.h!)
So adding a new binding for the phy obviously increases the code size. But the hard question is, how could that be fixed. IMHO the board has wrong settings. I really don't know how that could be "fixed" other than not applying this series. Well, we could make the additions conditional and introduce a new Kconfig setting, but that is a relly ugly hack and won't last long, would it? Doh!
So, the gcc-7 from kernel.org is the min required and must work toolchain. Maybe once gcc-9 is mature enough for people to have made stand-alone toolchains for it we'll move up to that but gcc-8 for everyone ends up being too hard. For the boneblack_vboot config, we could just drop SPL networking, it's not super critical to that particular example. But am335x_evm is the kitchen-sink EVM and it is used and supported there.
That said, looking over the u-boot-spl.map, it looks like nfs stuff doesn't get discarded for some reason, I'm going to look in to that.
So do I need to do something now? I guess removing the NFS stuff will make enough room to fit this. But how do we make sure, it will be applied with this series?
In this case, Joe has it in the -net PR right now. Thanks!
It's actually excluded from the PR. I need to go back and review the rest of the v2 series.
Cheers, -Joe
On Mon, Dec 09, 2019 at 01:46:32PM -0600, Joe Hershberger wrote:
Hi Tom,
On Mon, Dec 9, 2019 at 12:47 PM Tom Rini trini@konsulko.com wrote:
On Mon, Dec 09, 2019 at 07:42:00PM +0100, Michael Walle wrote:
Hi Tom, Hi Joe,
Am 2019-12-06 00:58, schrieb Tom Rini:
On Fri, Dec 06, 2019 at 12:27:39AM +0100, Michael Walle wrote:
Hi Joe, Hi Tom,
Am 2019-12-05 16:55, schrieb Joe Hershberger:
Hi Michael,
On Fri, Oct 25, 2019 at 7:28 PM Michael Walle michael@walle.cc wrote: > > Provide functions to read and write the Atheros debug registers. > > Signed-off-by: Michael Walle michael@walle.cc
This series is adding too much size to several of the boards' SPL it seems.
https://travis-ci.org/jhershbe/u-boot/builds/620804934
Please address this and resend.
So first of all, this was the old series. There was a v2 series, but unfortunately, I've forgot to add the mailing to the recipients, so it never ended up in the patchwork system. sorry :(
I've resend the v2 series here: https://patchwork.ozlabs.org/project/uboot/list/?series=146771
Now coming to the real problem here. The sizes, or like some boards handle the SPL stuff. Btw. I could not reproduce it on the am335x_boneblack_vboot board with a gcc-8. I've seen the travis ci job uses the gcc-7 so this also depends on the gcc. gcc-8 seems to produce smaller code, because on the am335x_evm the overflow was only by some 100 bytes.
So taking the am335x_evm board for example. It has the following options set: CONFIG_DM_ETH=y CONFIG_SPL_NET_SUPPORT=y CONFIG_PHY_ATHEROS=y (this one is set in the config.h!)
So adding a new binding for the phy obviously increases the code size. But the hard question is, how could that be fixed. IMHO the board has wrong settings. I really don't know how that could be "fixed" other than not applying this series. Well, we could make the additions conditional and introduce a new Kconfig setting, but that is a relly ugly hack and won't last long, would it? Doh!
So, the gcc-7 from kernel.org is the min required and must work toolchain. Maybe once gcc-9 is mature enough for people to have made stand-alone toolchains for it we'll move up to that but gcc-8 for everyone ends up being too hard. For the boneblack_vboot config, we could just drop SPL networking, it's not super critical to that particular example. But am335x_evm is the kitchen-sink EVM and it is used and supported there.
That said, looking over the u-boot-spl.map, it looks like nfs stuff doesn't get discarded for some reason, I'm going to look in to that.
So do I need to do something now? I guess removing the NFS stuff will make enough room to fit this. But how do we make sure, it will be applied with this series?
In this case, Joe has it in the -net PR right now. Thanks!
It's actually excluded from the PR. I need to go back and review the rest of the v2 series.
Sorry I meant the NFS patch to reduce size.
Signed-off-by: Michael Walle michael@walle.cc --- drivers/net/phy/atheros.c | 44 +++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 18 deletions(-)
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index b25aa02108..402998c8d5 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -68,20 +68,37 @@ static int ar8021_config(struct phy_device *phydev) return 0; }
-static int ar8031_config(struct phy_device *phydev) +static int ar803x_delay_config(struct phy_device *phydev) { + int ret; + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID || phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) { - ar803x_debug_reg_write(phydev, AR803x_DEBUG_REG_5, - AR803x_RGMII_TX_CLK_DLY); + ret = ar803x_debug_reg_write(phydev, AR803x_DEBUG_REG_5, + AR803x_RGMII_TX_CLK_DLY); + if (ret < 0) + return ret; }
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID || phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) { - ar803x_debug_reg_write(phydev, AR803x_DEBUG_REG_0, - AR803x_RGMII_RX_CLK_DLY); + ret = ar803x_debug_reg_write(phydev, AR803x_DEBUG_REG_0, + AR803x_RGMII_RX_CLK_DLY); + if (ret < 0) + return ret; }
+ return 0; +} + +static int ar8031_config(struct phy_device *phydev) +{ + int ret; + + ret = ar803x_delay_config(phydev); + if (ret < 0) + return ret; + phydev->supported = phydev->drv->features;
genphy_config_aneg(phydev); @@ -92,6 +109,7 @@ static int ar8031_config(struct phy_device *phydev)
static int ar8035_config(struct phy_device *phydev) { + int ret; int regval;
phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x0007); @@ -103,19 +121,9 @@ static int ar8035_config(struct phy_device *phydev) ar803x_debug_reg_mask(phydev, AR803x_DEBUG_REG_5, 0, AR803x_RGMII_TX_CLK_DLY);
- if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) || - (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)) { - /* enable tx delay */ - ar803x_debug_reg_write(phydev, AR803x_DEBUG_REG_5, - AR803x_RGMII_TX_CLK_DLY); - } - - if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) || - (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)) { - /* enable rx delay */ - ar803x_debug_reg_write(phydev, AR803x_DEBUG_REG_0, - AR803x_RGMII_RX_CLK_DLY); - } + ret = ar803x_delay_config(phydev); + if (ret < 0) + return ret;
phydev->supported = phydev->drv->features;
On Fri, Oct 25, 2019 at 7:28 PM Michael Walle michael@walle.cc wrote:
Signed-off-by: Michael Walle michael@walle.cc
Acked-by: Joe Hershberger joe.hershberger@ni.com
Remove the hard-coded delay configuration. The AR8035 config() always enabled the TX delay mode, although it will be set according to the PHY interface mode, too.
If bisecting shows that this commit breaks your board you probably have a wrong PHY interface mode. You probably want the PHY_INTERFACE_MODE_RGMII_TXID or PHY_INTERFACE_MODE_RGMII_ID mode.
Signed-off-by: Michael Walle michael@walle.cc --- drivers/net/phy/atheros.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index 402998c8d5..629c6b192a 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -118,9 +118,6 @@ static int ar8035_config(struct phy_device *phydev) regval = phy_read(phydev, MDIO_DEVAD_NONE, 0xe); phy_write(phydev, MDIO_DEVAD_NONE, 0xe, (regval|0x0018));
- ar803x_debug_reg_mask(phydev, AR803x_DEBUG_REG_5, - 0, AR803x_RGMII_TX_CLK_DLY); - ret = ar803x_delay_config(phydev); if (ret < 0) return ret;
On Fri, Oct 25, 2019 at 7:29 PM Michael Walle michael@walle.cc wrote:
Remove the hard-coded delay configuration. The AR8035 config() always enabled the TX delay mode, although it will be set according to the PHY interface mode, too.
If bisecting shows that this commit breaks your board you probably have a wrong PHY interface mode. You probably want the PHY_INTERFACE_MODE_RGMII_TXID or PHY_INTERFACE_MODE_RGMII_ID mode.
Signed-off-by: Michael Walle michael@walle.cc
Acked-by: Joe Hershberger joe.hershberger@ni.com
Signed-off-by: Michael Walle michael@walle.cc --- drivers/net/phy/atheros.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index 629c6b192a..113374f03f 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -110,13 +110,12 @@ static int ar8031_config(struct phy_device *phydev) static int ar8035_config(struct phy_device *phydev) { int ret; - int regval;
- phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x0007); - phy_write(phydev, MDIO_DEVAD_NONE, 0xe, 0x8016); - phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x4007); - regval = phy_read(phydev, MDIO_DEVAD_NONE, 0xe); - phy_write(phydev, MDIO_DEVAD_NONE, 0xe, (regval|0x0018)); + ret = phy_read_mmd(phydev, 7, 0x8016); + if (ret < 0) + return ret; + ret |= 0x0018; + phy_write_mmd(phydev, 7, 0x8016, ret);
ret = ar803x_delay_config(phydev); if (ret < 0)
On Fri, Oct 25, 2019 at 7:31 PM Michael Walle michael@walle.cc wrote:
Signed-off-by: Michael Walle michael@walle.cc
Acked-by: Joe Hershberger joe.hershberger@ni.com
Instead of doing a hard write, do a read-modify-write.
Signed-off-by: Michael Walle michael@walle.cc --- drivers/net/phy/atheros.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index 113374f03f..4b7a1fb9c4 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -12,10 +12,10 @@ #define AR803x_PHY_DEBUG_DATA_REG 0x1e
#define AR803x_DEBUG_REG_5 0x5 -#define AR803x_RGMII_TX_CLK_DLY 0x100 +#define AR803x_RGMII_TX_CLK_DLY BIT(8)
#define AR803x_DEBUG_REG_0 0x0 -#define AR803x_RGMII_RX_CLK_DLY 0x8000 +#define AR803x_RGMII_RX_CLK_DLY BIT(15)
static int ar803x_debug_reg_read(struct phy_device *phydev, u16 reg) { @@ -74,16 +74,16 @@ static int ar803x_delay_config(struct phy_device *phydev)
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID || phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) { - ret = ar803x_debug_reg_write(phydev, AR803x_DEBUG_REG_5, - AR803x_RGMII_TX_CLK_DLY); + ret = ar803x_debug_reg_mask(phydev, AR803x_DEBUG_REG_5, + 0, AR803x_RGMII_TX_CLK_DLY); if (ret < 0) return ret; }
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID || phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) { - ret = ar803x_debug_reg_write(phydev, AR803x_DEBUG_REG_0, - AR803x_RGMII_RX_CLK_DLY); + ret = ar803x_debug_reg_mask(phydev, AR803x_DEBUG_REG_0, + 0, AR803x_RGMII_RX_CLK_DLY); if (ret < 0) return ret; }
On Fri, Oct 25, 2019 at 7:30 PM Michael Walle michael@walle.cc wrote:
Instead of doing a hard write, do a read-modify-write.
Signed-off-by: Michael Walle michael@walle.cc
Acked-by: Joe Hershberger joe.hershberger@ni.com
The delay_config() code could only set the delay bit. Thus, it could only enable the delay mode, but not disable it. To make things worse, the RX delay mode is enabled by default after a hardware reset, so it could never be disabled. Fix this, by always setting or clearing the bits. This is also how the linux kernel configures the PHY.
If bisecting shows that this commit breaks your board you probably have a wrong PHY interface mode. You probably want the PHY_INTERFACE_MODE_RGMII_RXID or PHY_INTERFACE_MODE_RGMII_ID mode.
Signed-off-by: Michael Walle michael@walle.cc --- drivers/net/phy/atheros.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index 4b7a1fb9c4..8bf26626ff 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -78,6 +78,11 @@ static int ar803x_delay_config(struct phy_device *phydev) 0, AR803x_RGMII_TX_CLK_DLY); if (ret < 0) return ret; + } else { + ret = ar803x_debug_reg_mask(phydev, AR803x_DEBUG_REG_5, + AR803x_RGMII_TX_CLK_DLY, 0); + if (ret < 0) + return ret; }
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID || @@ -86,6 +91,11 @@ static int ar803x_delay_config(struct phy_device *phydev) 0, AR803x_RGMII_RX_CLK_DLY); if (ret < 0) return ret; + } else { + ret = ar803x_debug_reg_mask(phydev, AR803x_DEBUG_REG_0, + AR803x_RGMII_RX_CLK_DLY, 0); + if (ret < 0) + return ret; }
return 0;
On Sat, 26 Oct 2019 at 03:31, Michael Walle michael@walle.cc wrote:
The delay_config() code could only set the delay bit. Thus, it could only enable the delay mode, but not disable it. To make things worse, the RX delay mode is enabled by default after a hardware reset, so it could never be disabled. Fix this, by always setting or clearing the bits. This is also how the linux kernel configures the PHY.
If bisecting shows that this commit breaks your board you probably have a wrong PHY interface mode. You probably want the PHY_INTERFACE_MODE_RGMII_RXID or PHY_INTERFACE_MODE_RGMII_ID mode.
Signed-off-by: Michael Walle michael@walle.cc
drivers/net/phy/atheros.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index 4b7a1fb9c4..8bf26626ff 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -78,6 +78,11 @@ static int ar803x_delay_config(struct phy_device *phydev) 0, AR803x_RGMII_TX_CLK_DLY); if (ret < 0) return ret;
} else {ret = ar803x_debug_reg_mask(phydev, AR803x_DEBUG_REG_5,AR803x_RGMII_TX_CLK_DLY, 0);if (ret < 0)return ret; } if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||@@ -86,6 +91,11 @@ static int ar803x_delay_config(struct phy_device *phydev) 0, AR803x_RGMII_RX_CLK_DLY); if (ret < 0) return ret;
} else {ret = ar803x_debug_reg_mask(phydev, AR803x_DEBUG_REG_0,AR803x_RGMII_RX_CLK_DLY, 0);if (ret < 0)return ret; } return 0;-- 2.20.1
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Hi Michael,
Are you aware of this discussion thread? https://www.mail-archive.com/u-boot@lists.denx.de/msg326821.html
Thanks, -Vladimir
On Fri, Oct 25, 2019 at 7:30 PM Michael Walle michael@walle.cc wrote:
The delay_config() code could only set the delay bit. Thus, it could only enable the delay mode, but not disable it. To make things worse, the RX delay mode is enabled by default after a hardware reset, so it could never be disabled. Fix this, by always setting or clearing the bits. This is also how the linux kernel configures the PHY.
If bisecting shows that this commit breaks your board you probably have a wrong PHY interface mode. You probably want the PHY_INTERFACE_MODE_RGMII_RXID or PHY_INTERFACE_MODE_RGMII_ID mode.
Signed-off-by: Michael Walle michael@walle.cc
Either the offending dts's are already updated or they will be when this breaks them. It's been 6 months.
Acked-by: Joe Hershberger joe.hershberger@ni.com
Add support for configuring the CLK_25M pin as well as the RGMII I/O voltage by the device tree.
By default the AT803x PHYs outputs the 25MHz clock of the XTAL input. But this output can also be changed by software to other frequencies. This commit introduces a generic way to configure this output.
Also the PHY supports different RGMII I/O voltages: 1.5V, 1.8V and 2.5V. An internal LDO is able to provide 1.5V (default) and 1.8V. The 2.5V option needs an external supply voltage. This commit adds support to switch the internal LDO to 1.8V.
Signed-off-by: Michael Walle michael@walle.cc --- doc/device-tree-bindings/net/phy/atheros.txt | 22 +++ drivers/net/phy/atheros.c | 160 ++++++++++++++++++- 2 files changed, 180 insertions(+), 2 deletions(-) create mode 100644 doc/device-tree-bindings/net/phy/atheros.txt
diff --git a/doc/device-tree-bindings/net/phy/atheros.txt b/doc/device-tree-bindings/net/phy/atheros.txt new file mode 100644 index 0000000000..112250114f --- /dev/null +++ b/doc/device-tree-bindings/net/phy/atheros.txt @@ -0,0 +1,22 @@ +* Atheros PHY Device Tree binding + +Required properties: +- reg: PHY address + +Optional properties: +- atheros,clk-out-frequency: Clock frequency of the CLK_25M pin in Hz. + Either 25000000, 50000000, 62500000 or 125000000. +- atheros,clk-out-strength: Clock output buffer driver strength. + Either "full", "half" or "quarter". +- atheros,keep-pll-enabled: Keep the PLL running if no link is present. + Don't go into hibernation mode. +- atheros,rgmii-io-1v8: Use 1.8V as RGMII I/O voltage, the default is 1.5V. + +Example: + + ethernet-phy@0 { + reg = <0>; + atheros-clk-out-frequency = <125000000>; + atheros,keep-pll-enabled; + atheros,rgmii-io-1v8; + }; diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index 8bf26626ff..1c8c9b4e75 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -4,6 +4,7 @@ * * Copyright 2011, 2013 Freescale Semiconductor, Inc. * author Andy Fleming + * Copyright (c) 2019 Michael Walle michael@walle.cc */ #include <common.h> #include <phy.h> @@ -11,11 +12,41 @@ #define AR803x_PHY_DEBUG_ADDR_REG 0x1d #define AR803x_PHY_DEBUG_DATA_REG 0x1e
+/* Debug registers */ +#define AR803x_DEBUG_REG_0 0x0 +#define AR803x_RGMII_RX_CLK_DLY BIT(15) + #define AR803x_DEBUG_REG_5 0x5 #define AR803x_RGMII_TX_CLK_DLY BIT(8)
-#define AR803x_DEBUG_REG_0 0x0 -#define AR803x_RGMII_RX_CLK_DLY BIT(15) +#define AR803x_DEBUG_REG_1F 0x1f +#define AR803x_PLL_ON BIT(2) +#define AR803x_RGMII_1V8 BIT(3) + +/* MMD registers */ +#define AR803x_MMD7_CLK25M 0x8016 +#define AR803x_CLK_OUT_25MHZ_XTAL (0 << 2) +#define AR803x_CLK_OUT_25MHZ_DSP (1 << 2) +#define AR803x_CLK_OUT_50MHZ_PLL (2 << 2) +#define AR803x_CLK_OUT_50MHZ_DSP (3 << 2) +#define AR803x_CLK_OUT_62_5MHZ_PLL (4 << 2) +#define AR803x_CLK_OUT_62_5MHZ_DSP (5 << 2) +#define AR803x_CLK_OUT_125MHZ_PLL (6 << 2) +#define AR803x_CLK_OUT_125MHZ_DSP (7 << 2) +#define AR803x_CLK_OUT_MASK (7 << 2) + +#define AR803x_CLK_OUT_STRENGTH_FULL (0 << 6) +#define AR803x_CLK_OUT_STRENGTH_HALF (1 << 6) +#define AR803x_CLK_OUT_STRENGTH_QUARTER (2 << 6) +#define AR803x_CLK_OUT_STRENGTH_MASK (3 << 6) + +struct ar803x_priv { + int flags; +#define AR803x_FLAG_KEEP_PLL_ENABLED BIT(0) /* don't turn off internal PLL */ +#define AR803x_FLAG_RGMII_1V8 BIT(1) /* use 1.8V RGMII I/O voltage */ + u16 clk_25m_reg; + u16 clk_25m_mask; +};
static int ar803x_debug_reg_read(struct phy_device *phydev, u16 reg) { @@ -101,14 +132,131 @@ static int ar803x_delay_config(struct phy_device *phydev) return 0; }
+static int ar803x_regs_config(struct phy_device *phydev) +{ + struct ar803x_priv *priv = phydev->priv; + u16 set = 0, clear = 0; + int val; + int ret; + + /* no configuration available */ + if (!priv) + return 0; + + if (priv->flags & AR803x_FLAG_KEEP_PLL_ENABLED) + set |= AR803x_PLL_ON; + else + clear |= AR803x_PLL_ON; + + if (priv->flags & AR803x_FLAG_RGMII_1V8) + set |= AR803x_RGMII_1V8; + else + clear |= AR803x_RGMII_1V8; + + ret = ar803x_debug_reg_mask(phydev, AR803x_DEBUG_REG_1F, clear, set); + if (ret < 0) + return ret; + + /* save the write access if the mask is empty */ + if (priv->clk_25m_mask) { + val = phy_read_mmd(phydev, 7, AR803x_MMD7_CLK25M); + if (val < 0) + return val; + val &= ~priv->clk_25m_mask; + val |= priv->clk_25m_reg; + ret = phy_write_mmd(phydev, 7, AR803x_MMD7_CLK25M, val); + if (ret < 0) + return ret; + } + + return 0; +} + +static int ar803x_of_init(struct phy_device *phydev) +{ +#if defined(CONFIG_DM_ETH) + struct ar803x_priv *priv; + ofnode node; + const char *strength; + u32 freq; + + priv = malloc(sizeof(*priv)); + if (!priv) + return -ENOMEM; + memset(priv, 0, sizeof(*priv)); + + phydev->priv = priv; + + node = phy_get_ofnode(phydev); + if (!ofnode_valid(node)) + return -EINVAL; + + if (ofnode_read_bool(node, "atheros,keep-pll-enabled")) + priv->flags |= AR803x_FLAG_KEEP_PLL_ENABLED; + if (ofnode_read_bool(node, "atheros,rgmii-io-1v8")) + priv->flags |= AR803x_FLAG_RGMII_1V8; + + /* + * Get the CLK_OUT frequency from the device tree. Only XTAL and PLL + * sources are supported right now. There is also the possibilty to use + * the DSP as frequency reference, this is used for synchronous + * ethernet. + */ + freq = ofnode_read_u32_default(node, "atheros,clk-out-frequency", 0); + if (freq) { + priv->clk_25m_mask |= AR803x_CLK_OUT_MASK; + if (freq == 25000000) { + priv->clk_25m_reg |= AR803x_CLK_OUT_25MHZ_XTAL; + } else if (freq == 50000000) { + priv->clk_25m_reg |= AR803x_CLK_OUT_50MHZ_PLL; + } else if (freq == 62500000) { + priv->clk_25m_reg |= AR803x_CLK_OUT_62_5MHZ_PLL; + } else if (freq == 125000000) { + priv->clk_25m_reg |= AR803x_CLK_OUT_125MHZ_PLL; + } else { + dev_err(phydev->dev, + "invalid atheros,clk-out-frequency\n"); + free(priv); + return -EINVAL; + } + } + + strength = ofnode_read_string(node, "atheros,clk-out-strength"); + if (strength) { + priv->clk_25m_mask |= AR803x_CLK_OUT_STRENGTH_MASK; + if (!strcmp(strength, "full")) { + priv->clk_25m_reg |= AR803x_CLK_OUT_STRENGTH_FULL; + } else if (!strcmp(strength, "half")) { + priv->clk_25m_reg |= AR803x_CLK_OUT_STRENGTH_HALF; + } else if (!strcmp(strength, "quarter")) { + priv->clk_25m_reg |= AR803x_CLK_OUT_STRENGTH_QUARTER; + } else { + dev_err(phydev->dev, "invalid atheros,strength\n"); + free(priv); + return -EINVAL; + } + } +#endif + + return 0; +} + static int ar8031_config(struct phy_device *phydev) { int ret;
+ ret = ar803x_of_init(phydev); + if (ret < 0) + return ret; + ret = ar803x_delay_config(phydev); if (ret < 0) return ret;
+ ret = ar803x_regs_config(phydev); + if (ret < 0) + return ret; + phydev->supported = phydev->drv->features;
genphy_config_aneg(phydev); @@ -121,6 +269,10 @@ static int ar8035_config(struct phy_device *phydev) { int ret;
+ ret = ar803x_of_init(phydev); + if (ret < 0) + return ret; + ret = phy_read_mmd(phydev, 7, 0x8016); if (ret < 0) return ret; @@ -131,6 +283,10 @@ static int ar8035_config(struct phy_device *phydev) if (ret < 0) return ret;
+ ret = ar803x_regs_config(phydev); + if (ret < 0) + return ret; + phydev->supported = phydev->drv->features;
genphy_config_aneg(phydev);
On Fri, Oct 25, 2019 at 7:29 PM Michael Walle michael@walle.cc wrote:
Add support for configuring the CLK_25M pin as well as the RGMII I/O voltage by the device tree.
By default the AT803x PHYs outputs the 25MHz clock of the XTAL input. But this output can also be changed by software to other frequencies. This commit introduces a generic way to configure this output.
Also the PHY supports different RGMII I/O voltages: 1.5V, 1.8V and 2.5V. An internal LDO is able to provide 1.5V (default) and 1.8V. The 2.5V option needs an external supply voltage. This commit adds support to switch the internal LDO to 1.8V.
Signed-off-by: Michael Walle michael@walle.cc
Acked-by: Joe Hershberger joe.hershberger@ni.com
We can configure the clock output in the device tree. Disable the hardcoded one in here. This is highly board-specific and should have never been enabled in the PHY driver.
If bisecting shows that this commit breaks your board it probably depends on the clock output of your Atheros AR8035 PHY. Please have a look at doc/device-tree-bindings/net/phy/atheros.txt. You need to set "clk-out-frequency = <125000000>" because that value was the hardcoded value until this commit.
Signed-off-by: Michael Walle michael@walle.cc --- drivers/net/phy/atheros.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index 1c8c9b4e75..91fcbf912a 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -273,12 +273,6 @@ static int ar8035_config(struct phy_device *phydev) if (ret < 0) return ret;
- ret = phy_read_mmd(phydev, 7, 0x8016); - if (ret < 0) - return ret; - ret |= 0x0018; - phy_write_mmd(phydev, 7, 0x8016, ret); - ret = ar803x_delay_config(phydev); if (ret < 0) return ret;
On Fri, Oct 25, 2019 at 7:29 PM Michael Walle michael@walle.cc wrote:
We can configure the clock output in the device tree. Disable the hardcoded one in here. This is highly board-specific and should have never been enabled in the PHY driver.
If bisecting shows that this commit breaks your board it probably depends on the clock output of your Atheros AR8035 PHY. Please have a look at doc/device-tree-bindings/net/phy/atheros.txt. You need to set "clk-out-frequency = <125000000>" because that value was the hardcoded value until this commit.
Signed-off-by: Michael Walle michael@walle.cc
Acked-by: Joe Hershberger joe.hershberger@ni.com
The two functions are now exactly the same, remove one of them.
Signed-off-by: Michael Walle michael@walle.cc --- drivers/net/phy/atheros.c | 30 +++--------------------------- 1 file changed, 3 insertions(+), 27 deletions(-)
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index 91fcbf912a..922dc91835 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -241,31 +241,7 @@ static int ar803x_of_init(struct phy_device *phydev) return 0; }
-static int ar8031_config(struct phy_device *phydev) -{ - int ret; - - ret = ar803x_of_init(phydev); - if (ret < 0) - return ret; - - ret = ar803x_delay_config(phydev); - if (ret < 0) - return ret; - - ret = ar803x_regs_config(phydev); - if (ret < 0) - return ret; - - phydev->supported = phydev->drv->features; - - genphy_config_aneg(phydev); - genphy_restart_aneg(phydev); - - return 0; -} - -static int ar8035_config(struct phy_device *phydev) +static int ar803x_config(struct phy_device *phydev) { int ret;
@@ -304,7 +280,7 @@ static struct phy_driver AR8031_driver = { .uid = 0x4dd074, .mask = 0xffffffef, .features = PHY_GBIT_FEATURES, - .config = ar8031_config, + .config = ar803x_config, .startup = genphy_startup, .shutdown = genphy_shutdown, }; @@ -314,7 +290,7 @@ static struct phy_driver AR8035_driver = { .uid = 0x4dd072, .mask = 0xffffffef, .features = PHY_GBIT_FEATURES, - .config = ar8035_config, + .config = ar803x_config, .startup = genphy_startup, .shutdown = genphy_shutdown, };
On Fri, Oct 25, 2019 at 7:29 PM Michael Walle michael@walle.cc wrote:
The two functions are now exactly the same, remove one of them.
Signed-off-by: Michael Walle michael@walle.cc
Acked-by: Joe Hershberger joe.hershberger@ni.com
On Fri, Oct 25, 2019 at 7:27 PM Michael Walle michael@walle.cc wrote:
This series cleans up the Atheros PHY AR803x PHY driver and adds a device tree binding for the most commonly used PHY settings like clock output.
If you're a board maintainer you're getting this mail because you probably use an AR803x PHY on your board. Please have a look at your board specific code and see if you can use the device tree bindings instead. Let me know, if something is missing.
Thank you for this!
I was able to remove board_phy_config and the supporting ar8031_phy_fixup functions to a total of nearly 30 lines of code.
For the whole series... Tested-by: Adam Ford aford173@gmail.com # imx6q_logic
Michael Walle (9): phy: atheros: introduce debug read and write functions phy: atheros: move delay config to common function phy: atheros: ar8035: remove extra delay config phy: atheros: ar8035: use phy_{read|write}_mmd() phy: atheros: don't overwrite debug register values phy: atheros: fix delay configuration phy: atheros: Add device tree bindings and config phy: atheros: ar8035: remove static clock config phy: atheros: consolidate {ar8031|ar8035}_config()
doc/device-tree-bindings/net/phy/atheros.txt | 22 ++ drivers/net/phy/atheros.c | 262 +++++++++++++++---- 2 files changed, 240 insertions(+), 44 deletions(-) create mode 100644 doc/device-tree-bindings/net/phy/atheros.txt
Cc: Joe Hershberger joe.hershberger@ni.com Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Adam Ford aford173@gmail.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Ian Ray ian.ray@ge.com Cc: Jason Liu jason.hui.liu@nxp.com Cc: Ye Li ye.li@nxp.com Cc: Uri Mashiach uri.mashiach@compulab.co.il Cc: Nikita Kiryanov nikita@compulab.co.il Cc: Lukasz Majewski lukma@denx.de Cc: Fabio Estevam festevam@gmail.com Cc: Vanessa Maegima vanessa.maegima@nxp.com Cc: Eric Bénard eric@eukrea.com -- 2.20.1
On Fri, Oct 25, 2019 at 10:20 PM Adam Ford aford173@gmail.com wrote:
On Fri, Oct 25, 2019 at 7:27 PM Michael Walle michael@walle.cc wrote:
This series cleans up the Atheros PHY AR803x PHY driver and adds a device tree binding for the most commonly used PHY settings like clock output.
If you're a board maintainer you're getting this mail because you probably use an AR803x PHY on your board. Please have a look at your board specific code and see if you can use the device tree bindings instead. Let me know, if something is missing.
Thank you for this!
I was able to remove board_phy_config and the supporting ar8031_phy_fixup functions to a total of nearly 30 lines of code.
Please disregard my comment. From a cold boot, I cannot remove these lines.
adam
For the whole series... Tested-by: Adam Ford aford173@gmail.com # imx6q_logic
Michael Walle (9): phy: atheros: introduce debug read and write functions phy: atheros: move delay config to common function phy: atheros: ar8035: remove extra delay config phy: atheros: ar8035: use phy_{read|write}_mmd() phy: atheros: don't overwrite debug register values phy: atheros: fix delay configuration phy: atheros: Add device tree bindings and config phy: atheros: ar8035: remove static clock config phy: atheros: consolidate {ar8031|ar8035}_config()
doc/device-tree-bindings/net/phy/atheros.txt | 22 ++ drivers/net/phy/atheros.c | 262 +++++++++++++++---- 2 files changed, 240 insertions(+), 44 deletions(-) create mode 100644 doc/device-tree-bindings/net/phy/atheros.txt
Cc: Joe Hershberger joe.hershberger@ni.com Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Adam Ford aford173@gmail.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Ian Ray ian.ray@ge.com Cc: Jason Liu jason.hui.liu@nxp.com Cc: Ye Li ye.li@nxp.com Cc: Uri Mashiach uri.mashiach@compulab.co.il Cc: Nikita Kiryanov nikita@compulab.co.il Cc: Lukasz Majewski lukma@denx.de Cc: Fabio Estevam festevam@gmail.com Cc: Vanessa Maegima vanessa.maegima@nxp.com Cc: Eric Bénard eric@eukrea.com -- 2.20.1
Hi Adam,
Am 2019-10-26 05:28, schrieb Adam Ford:
On Fri, Oct 25, 2019 at 10:20 PM Adam Ford aford173@gmail.com wrote:
On Fri, Oct 25, 2019 at 7:27 PM Michael Walle michael@walle.cc wrote:
This series cleans up the Atheros PHY AR803x PHY driver and adds a device tree binding for the most commonly used PHY settings like clock output.
If you're a board maintainer you're getting this mail because you probably use an AR803x PHY on your board. Please have a look at your board specific code and see if you can use the device tree bindings instead. Let me know, if something is missing.
Thank you for this!
I was able to remove board_phy_config and the supporting ar8031_phy_fixup functions to a total of nearly 30 lines of code.
Please disregard my comment. From a cold boot, I cannot remove these lines.
Thank you for testing though. I guess your network drivers needs something like that: https://patchwork.ozlabs.org/patch/1184523/
So here is a cheap shot (very hacky, doesn't work with CONFIG_FEC_MXC_PHYADDR, completely untested, not even compiled ;). Could you try that? I need to add some debug messages to the Atheros PHY driver, so one could see if the device tree binding is working correctly.
--- a/drivers/net/fec_mxc.c +++ b/drivers/net/fec_mxc.c @@ -1264,7 +1264,7 @@ static const struct eth_ops fecmxc_ops = { .read_rom_hwaddr = fecmxc_read_rom_hwaddr, };
-static int device_get_phy_addr(struct udevice *dev) +static int device_get_phy_addr(struct udevice *dev, struct ofnode *phy_node) { struct ofnode_phandle_args phandle_args; int reg; @@ -1276,6 +1276,7 @@ static int device_get_phy_addr(struct udevice *dev) }
reg = ofnode_read_u32_default(phandle_args.node, "reg", 0); + phy_node = &phandle_args.node;
return reg; } @@ -1284,8 +1285,9 @@ static int fec_phy_init(struct fec_priv *priv, struct udevice *dev) { struct phy_device *phydev; int addr; + ofnode *phy_node;
- addr = device_get_phy_addr(dev); + addr = device_get_phy_addr(dev, &phy_node); #ifdef CONFIG_FEC_MXC_PHYADDR addr = CONFIG_FEC_MXC_PHYADDR; #endif @@ -1294,6 +1296,7 @@ static int fec_phy_init(struct fec_priv *priv, struct udevice *dev) if (!phydev) return -ENODEV;
+ phydev->node = phy_node; priv->phydev = phydev; phy_config(phydev);
-michael
On Sat, Oct 26, 2019 at 3:57 AM Michael Walle michael@walle.cc wrote:
Hi Adam,
Am 2019-10-26 05:28, schrieb Adam Ford:
On Fri, Oct 25, 2019 at 10:20 PM Adam Ford aford173@gmail.com wrote:
On Fri, Oct 25, 2019 at 7:27 PM Michael Walle michael@walle.cc wrote:
This series cleans up the Atheros PHY AR803x PHY driver and adds a device tree binding for the most commonly used PHY settings like clock output.
If you're a board maintainer you're getting this mail because you probably use an AR803x PHY on your board. Please have a look at your board specific code and see if you can use the device tree bindings instead. Let me know, if something is missing.
Thank you for this!
I was able to remove board_phy_config and the supporting ar8031_phy_fixup functions to a total of nearly 30 lines of code.
Please disregard my comment. From a cold boot, I cannot remove these lines.
Thank you for testing though. I guess your network drivers needs something like that: https://patchwork.ozlabs.org/patch/1184523/
I tried with the 2nd series also applied with no luck either on my i.MX6Q.
So here is a cheap shot (very hacky, doesn't work with CONFIG_FEC_MXC_PHYADDR, completely untested, not even compiled ;). Could you try that? I need to add some debug messages to the Atheros PHY driver, so one could see if the device tree binding is working correctly.
I will look at the following stuff when I have more time.
--- a/drivers/net/fec_mxc.c +++ b/drivers/net/fec_mxc.c @@ -1264,7 +1264,7 @@ static const struct eth_ops fecmxc_ops = { .read_rom_hwaddr = fecmxc_read_rom_hwaddr, };
-static int device_get_phy_addr(struct udevice *dev) +static int device_get_phy_addr(struct udevice *dev, struct ofnode *phy_node) { struct ofnode_phandle_args phandle_args; int reg; @@ -1276,6 +1276,7 @@ static int device_get_phy_addr(struct udevice *dev) }
reg = ofnode_read_u32_default(phandle_args.node, "reg", 0);
phy_node = &phandle_args.node; return reg;}
@@ -1284,8 +1285,9 @@ static int fec_phy_init(struct fec_priv *priv, struct udevice *dev) { struct phy_device *phydev; int addr;
ofnode *phy_node;
addr = device_get_phy_addr(dev);
addr = device_get_phy_addr(dev, &phy_node);@@ -1294,6 +1296,7 @@ static int fec_phy_init(struct fec_priv *priv, struct udevice *dev) if (!phydev) return -ENODEV;
phydev->node = phy_node; priv->phydev = phydev; phy_config(phydev);-michael
The network driver has to set the PHY node correctly. If that is not the case, ar803x_of_init() will fail. Add some debugging output.
If the device tree binding is not working for you have a look at the ar803x_of_init: found PHY node: phy@0 output. In the case above "phy@0" is the phy node in the device tree. If instead the node of your network device is displayed, you have to set the phydev->node property in your network device driver. For example, the following patch adds it to the fsl_enetc driver: https://patchwork.ozlabs.org/patch/1184523/
Signed-off-by: Michael Walle michael@walle.cc --- drivers/net/phy/atheros.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index 922dc91835..f68e5f2cab 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -191,6 +191,8 @@ static int ar803x_of_init(struct phy_device *phydev) if (!ofnode_valid(node)) return -EINVAL;
+ debug("%s: found PHY node: %s\n", __func__, ofnode_get_name(node)); + if (ofnode_read_bool(node, "atheros,keep-pll-enabled")) priv->flags |= AR803x_FLAG_KEEP_PLL_ENABLED; if (ofnode_read_bool(node, "atheros,rgmii-io-1v8")) @@ -236,6 +238,9 @@ static int ar803x_of_init(struct phy_device *phydev) return -EINVAL; } } + + debug("%s: flags=%x clk_25m_reg=%04x\n", __func__, priv->flags, + priv->clk_25m_reg); #endif
return 0;
On Sun, Oct 27, 2019 at 3:38 PM Michael Walle michael@walle.cc wrote:
The network driver has to set the PHY node correctly. If that is not the case, ar803x_of_init() will fail. Add some debugging output.
If the device tree binding is not working for you have a look at the ar803x_of_init: found PHY node: phy@0 output. In the case above "phy@0" is the phy node in the device tree. If instead the node of your network device is displayed, you have to set the phydev->node property in your network device driver. For example, the following patch adds it to the fsl_enetc driver: https://patchwork.ozlabs.org/patch/1184523/
Signed-off-by: Michael Walle michael@walle.cc
Acked-by: Joe Hershberger joe.hershberger@ni.com
Hi Michael,
On Sun, Oct 27, 2019 at 3:38 PM Michael Walle michael@walle.cc wrote:
The network driver has to set the PHY node correctly. If that is not the case, ar803x_of_init() will fail. Add some debugging output.
If the device tree binding is not working for you have a look at the ar803x_of_init: found PHY node: phy@0 output. In the case above "phy@0" is the phy node in the device tree. If instead the node of your network device is displayed, you have to set the phydev->node property in your network device driver. For example, the following patch adds it to the fsl_enetc driver: https://patchwork.ozlabs.org/patch/1184523/
Signed-off-by: Michael Walle michael@walle.cc
This doesn't apply so I assume it's either superseded or not needed any more. If those are not the case, please rebase and resend on top of u-boot-net/master
Thanks, -Joe
participants (6)
-
Adam Ford -
Joe Hershberger -
Joe Hershberger -
Michael Walle -
Tom Rini -
Vladimir Oltean