| Message ID | 20220330083826.829473-1-sr@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Ramon Fried |
| Headers | show |
| Series | [1/2] net: phy: marvell: Support reg config via "marvell, reg-init" DT property | expand |
On Wed, 30 Mar 2022 10:38:25 +0200 Stefan Roese <sr@denx.de> wrote: > This patch adds support for the "marvell,reg-init" DT property, which > is used to describe board specific Marvell PHY register configurations > in the board dts file. This DT property is supported in the Linux Kernel > since a longer time. Adding it to U-Boot now, enables the boards which > describe the register settings in their DT files here as well. > > I've included calling this marvell_of_reg_init() to all foo_config() > functions in this patch as well. If CONFIG_DM_ETH is not set, there is > no ofnode, or no "marvell,reg-init" property, the PHY initialization is > unchanged. > > The function marvell_of_reg_init() is a port of the Linux version. > Please note that I explicitly did not add error checking and handling > to the U-Boot version, as this is basically not done for phy_read/write > in this Marvell PHY code. > > This will be used by the upcoming ethernet support on the MIPS > Octeon EBB 7304 board. > > Signed-off-by: Stefan Roese <sr@denx.de> > Cc: Ramon Fried <rfried.dev@gmail.com> > Cc: Joe Hershberger <joe.hershberger@ni.com> > Cc: Aaron Williams <awilliams@marvell.com> > Cc: Chandrakala Chavva <cchavva@marvell.com> Hi Stefan, I know this property is used also in kernel, but what do you want to use it for? Because in kernel the idea is to deprecate it. It is used for example to confiure INT pin, but that should have it's own property once someone implements it. It is also used to configure LEDs, and that is hopefully gonna be obsoleted by supporting the LEDs via the LED subsystem, so afterwards we will write ethernet-phy@1 { leds { led@0 { color = ...; }; }; }; Marek
On 3/30/22 14:16, Marek Behún wrote: > On Wed, 30 Mar 2022 10:38:25 +0200 > Stefan Roese <sr@denx.de> wrote: > >> This patch adds support for the "marvell,reg-init" DT property, which >> is used to describe board specific Marvell PHY register configurations >> in the board dts file. This DT property is supported in the Linux Kernel >> since a longer time. Adding it to U-Boot now, enables the boards which >> describe the register settings in their DT files here as well. >> >> I've included calling this marvell_of_reg_init() to all foo_config() >> functions in this patch as well. If CONFIG_DM_ETH is not set, there is >> no ofnode, or no "marvell,reg-init" property, the PHY initialization is >> unchanged. >> >> The function marvell_of_reg_init() is a port of the Linux version. >> Please note that I explicitly did not add error checking and handling >> to the U-Boot version, as this is basically not done for phy_read/write >> in this Marvell PHY code. >> >> This will be used by the upcoming ethernet support on the MIPS >> Octeon EBB 7304 board. >> >> Signed-off-by: Stefan Roese <sr@denx.de> >> Cc: Ramon Fried <rfried.dev@gmail.com> >> Cc: Joe Hershberger <joe.hershberger@ni.com> >> Cc: Aaron Williams <awilliams@marvell.com> >> Cc: Chandrakala Chavva <cchavva@marvell.com> > > Hi Stefan, I know this property is used also in kernel, but what do you > want to use it for? Frankly, I do not know exactly. It's part of the MIPS Octeon ethernet support in mainline U-Boot, which also uses this DT property on the EBB7304 board. I've not analyzed the register settings in detail to check which register is being configured to which value to see, why this might be the case. Dropping these register settings might have resulted in problems not directly spotted on the board. So my plan was to integrate these as well. For reference, I've added the current DT reg-init setting below [1]. This is for the 88e1240 PHY, and taking a quick look in the manual, this seems to be LED related (see below). > Because in kernel the idea is to deprecate it. This is new to me. > It is used for example to confiure INT pin, but that should have it's > own property once someone implements it. It is also used to configure > LEDs, and that is hopefully gonna be obsoleted by supporting the LEDs > via the LED subsystem, so afterwards we will write > ethernet-phy@1 { > leds { > led@0 { > color = ...; > }; > }; > }; That would be very cool indeed. Even though it's not possible "right now". So how to continue? Add this "reg-init" "feature" for now? And perhaps drop it once this LED configuration is possible via the way you describe above? Or drop it once it's also dropped in the Kernel? My personal feeling is, that supporting "reg-init" does not "hurt" and might even be handy for some testing / debugging of network PHY setups. Perhaps Ramon and/or Joe also have some comments on this. Thanks, Stefan [1] marvell,reg-init = <3 0x10 0 0x8665>, <3 0x11 0 0x00aa>, <3 0x12 0 0x4105>, <3 0x13 0 0x8a08>;
On Wed, 30 Mar 2022 15:33:34 +0200 Stefan Roese <sr@denx.de> wrote: > So how to continue? Add this "reg-init" "feature" for now? And perhaps > drop it once this LED configuration is possible via the way you describe > above? Or drop it once it's also dropped in the Kernel? My personal > feeling is, that supporting "reg-init" does not "hurt" and might even be > handy for some testing / debugging of network PHY setups. Add it now, maybe we will drop it sometime in future if no dts uses it anymore. Kernel will need to support it forever because of backwards compatibility with older DTBs. Marek
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index a62c695c5c84..41520c6d392a 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -104,6 +104,88 @@ #define MIIM_88E151x_MODE_SGMII 1 #define MIIM_88E151x_RESET_OFFS 15 +static int marvell_read_page(struct phy_device *phydev) +{ + return phy_read(phydev, MDIO_DEVAD_NONE, MII_MARVELL_PHY_PAGE); +} + +static int marvell_write_page(struct phy_device *phydev, int page) +{ + return phy_write(phydev, MDIO_DEVAD_NONE, MII_MARVELL_PHY_PAGE, page); +} + +#if IS_ENABLED(CONFIG_DM_ETH) +/* Set and/or override some configuration registers based on the + * marvell,reg-init property stored in the of_node for the phydev. + * + * marvell,reg-init = <reg-page reg mask value>,...; + * + * There may be one or more sets of <reg-page reg mask value>: + * + * reg-page: which register bank to use. + * reg: the register. + * mask: if non-zero, ANDed with existing register value. + * value: ORed with the masked value and written to the regiser. + * + */ +static int marvell_of_reg_init(struct phy_device *phydev) +{ + const __be32 *prop; + int len, i, saved_page, current_page, ret = 0; + + if (!ofnode_valid(phydev->node)) + return 0; + + prop = ofnode_get_property(phydev->node, "marvell,reg-init", &len); + if (!prop) + return 0; + + saved_page = marvell_read_page(phydev); + if (saved_page < 0) + goto err; + current_page = saved_page; + + len /= sizeof(*prop); + for (i = 0; i < len - 3; i += 4) { + u16 page = be32_to_cpup(prop + i); + u16 reg = be32_to_cpup(prop + i + 1); + u16 mask = be32_to_cpup(prop + i + 2); + u16 val_bits = be32_to_cpup(prop + i + 3); + int val; + + if (page != current_page) { + current_page = page; + ret = marvell_write_page(phydev, page); + if (ret < 0) + goto err; + } + + val = 0; + if (mask) { + val = phy_read(phydev, MDIO_DEVAD_NONE, reg); + if (val < 0) { + ret = val; + goto err; + } + val &= mask; + } + val |= val_bits; + + ret = phy_write(phydev, MDIO_DEVAD_NONE, reg, val); + if (ret < 0) + goto err; + } + +err: + return marvell_write_page(phydev, saved_page); +} +#else +static int marvell_of_reg_init(struct phy_device *phydev) +{ + return 0; +} +#endif /* CONFIG_DM_ETH */ + static int m88e1xxx_phy_extread(struct phy_device *phydev, int addr, int devaddr, int regnum) { @@ -143,6 +225,8 @@ static int m88e1011s_config(struct phy_device *phydev) phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, BMCR_RESET); + marvell_of_reg_init(phydev); + genphy_config_aneg(phydev); return 0; @@ -298,6 +382,8 @@ static int m88e1111s_config(struct phy_device *phydev) /* soft reset */ phy_reset(phydev); + marvell_of_reg_init(phydev); + genphy_config_aneg(phydev); genphy_restart_aneg(phydev); @@ -397,6 +483,8 @@ static int m88e151x_config(struct phy_device *phydev) /* soft reset */ phy_reset(phydev); + marvell_of_reg_init(phydev); + genphy_config_aneg(phydev); genphy_restart_aneg(phydev); @@ -417,6 +505,8 @@ static int m88e1118_config(struct phy_device *phydev) /* Change Page Number */ phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE, 0x0000); + marvell_of_reg_init(phydev); + return genphy_config_aneg(phydev); } @@ -439,6 +529,8 @@ static int m88e1121_config(struct phy_device *phydev) { int pg; + marvell_of_reg_init(phydev); + /* Configure the PHY */ genphy_config_aneg(phydev); @@ -479,6 +571,8 @@ static int m88e1145_config(struct phy_device *phydev) MIIM_M88E1145_RGMII_TX_DELAY; phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1145_PHY_EXT_CR, reg); + marvell_of_reg_init(phydev); + genphy_config_aneg(phydev); /* soft reset */ @@ -511,6 +605,8 @@ static int m88e1149_config(struct phy_device *phydev) phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x0); phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x100); + marvell_of_reg_init(phydev); + genphy_config_aneg(phydev); phy_reset(phydev); @@ -544,6 +640,8 @@ static int m88e1310_config(struct phy_device *phydev) /* Ensure to return to page 0 */ phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1310_PHY_PAGE, 0x0000); + marvell_of_reg_init(phydev); + return genphy_config_aneg(phydev); } @@ -578,6 +676,8 @@ static int m88e1680_config(struct phy_device *phydev) phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE, 0x0000); phy_write(phydev, MDIO_DEVAD_NONE, 0, 0x9140); + marvell_of_reg_init(phydev); + res = genphy_config_aneg(phydev); if (res < 0) return res;
This patch adds support for the "marvell,reg-init" DT property, which is used to describe board specific Marvell PHY register configurations in the board dts file. This DT property is supported in the Linux Kernel since a longer time. Adding it to U-Boot now, enables the boards which describe the register settings in their DT files here as well. I've included calling this marvell_of_reg_init() to all foo_config() functions in this patch as well. If CONFIG_DM_ETH is not set, there is no ofnode, or no "marvell,reg-init" property, the PHY initialization is unchanged. The function marvell_of_reg_init() is a port of the Linux version. Please note that I explicitly did not add error checking and handling to the U-Boot version, as this is basically not done for phy_read/write in this Marvell PHY code. This will be used by the upcoming ethernet support on the MIPS Octeon EBB 7304 board. Signed-off-by: Stefan Roese <sr@denx.de> Cc: Ramon Fried <rfried.dev@gmail.com> Cc: Joe Hershberger <joe.hershberger@ni.com> Cc: Aaron Williams <awilliams@marvell.com> Cc: Chandrakala Chavva <cchavva@marvell.com> --- drivers/net/phy/marvell.c | 100 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+)