On 30/01/2023 18.16, Tom Rini wrote: > On Thu, Jan 26, 2023 at 05:54:51PM -0700, Simon Glass wrote: >> Hi Rasmus, >> >> On Tue, 9 Aug 2022 at 05:53, Rasmus Villemoes >> <rasmus.villem...@prevas.dk> wrote: >>> >>> Currently, U-Boot doesn't parse a "max-speed" DT property in a phy's >>> DT node. That property is a standard binding which should be honoured, >>> and in linux that is done by the core phy code via a call to an >>> of_set_phy_supported() helper. (Some ethernet drivers support a >>> max-speed property in their DT node, but that's orthogonal to what the >>> phy supports.) >>> >>> Add a similar helper in U-Boot, and call it from phy_config(). >>> >>> Signed-off-by: Rasmus Villemoes <rasmus.villem...@prevas.dk> >>> --- >>> Resending, this time including the u-boot list in recipients. Sorry >>> for the duplicate. >>> >>> drivers/net/phy/phy.c | 20 ++++++++++++++++++++ >>> 1 file changed, 20 insertions(+) >>> >>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c >>> index e6e1755518..ec690361e6 100644 >>> --- a/drivers/net/phy/phy.c >>> +++ b/drivers/net/phy/phy.c >>> @@ -599,6 +599,20 @@ int phy_register(struct phy_driver *drv) >>> return 0; >>> } >>> >>> +static int of_set_phy_supported(struct phy_device *phydev) >>> +{ >>> + ofnode node = phy_get_ofnode(phydev); >>> + u32 max_speed; >>> + >>> + if (!ofnode_valid(node)) >>> + return 0; >>> + >>> + if (!ofnode_read_u32(node, "max-speed", &max_speed)) >>> + return phy_set_supported(phydev, max_speed); >>> + >>> + return 0; >>> +} >>> + >>> int phy_set_supported(struct phy_device *phydev, u32 max_speed) >>> { >>> /* The default values for phydev->supported are provided by the PHY >>> @@ -1070,6 +1084,12 @@ __weak int board_phy_config(struct phy_device >>> *phydev) >>> >>> int phy_config(struct phy_device *phydev) >>> { >>> + int ret; >>> + >>> + ret = of_set_phy_supported(phydev); >>> + if (ret) >>> + return ret; >>> + >>> /* Invoke an optional board-specific helper */ >>> return board_phy_config(phydev); >>> } >>> >> >> The only problem with this is that it is reading DT outside the >> of_to_plat() method. Is it possible to call it from there?
I didn't know that was verboten, and I certainly don't want to add the same code over and over to all phy drivers' methods, that was the point of doing it in a central place. If there's some other central place that's better I'm all ears. > As written, the patch also needs a #ifdef or similar around OF_CONTROL Sigh. But I suppose it's just adding 'if (!IS_ENABLED(CONFIG_OF_CONTROL)) return 0;' to the top of of_set_phy_supported(). But perhaps it will end up somewhere where that's "automatic". Rasmus