[PATCH] phy: add of_set_phy_supported() helper, call from phy_config()
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.villemoes@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); }
On Tue, Aug 9, 2022 at 2:53 PM Rasmus Villemoes rasmus.villemoes@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.villemoes@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);}
2.31.1
Reviewed-by: Ramon Fried rfried.dev@gmail.com
On 18/09/2022 08.13, Ramon Fried wrote:
On Tue, Aug 9, 2022 at 2:53 PM Rasmus Villemoes rasmus.villemoes@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.villemoes@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);}
2.31.1
Reviewed-by: Ramon Fried rfried.dev@gmail.com
This seems to have fallen through the cracks. Can it be picked up please?
Rasmus
On Thu, Jan 26, 2023 at 09:29:29AM +0100, Rasmus Villemoes wrote:
On 18/09/2022 08.13, Ramon Fried wrote:
On Tue, Aug 9, 2022 at 2:53 PM Rasmus Villemoes rasmus.villemoes@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.villemoes@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);}
2.31.1
Reviewed-by: Ramon Fried rfried.dev@gmail.com
This seems to have fallen through the cracks. Can it be picked up please?
Thanks for spotting, yes, I'll pick this up for real soon.
Hi Rasmus,
On Tue, 9 Aug 2022 at 05:53, Rasmus Villemoes rasmus.villemoes@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.villemoes@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);}
2.31.1
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?
Regards, Simon
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.villemoes@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.villemoes@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);}
2.31.1
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?
As written, the patch also needs a #ifdef or similar around OF_CONTROL or this fails to build on a platform or two. I was about to apply with that change made when I saw Simon's question, to which I'm waiting for an answer / follow-up on.
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.villemoes@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.villemoes@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
On Mon, Jan 30, 2023 at 10:36:40PM +0100, Rasmus Villemoes wrote:
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.villemoes@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.villemoes@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.
Is this a good enough rationale, Simon?
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".
Doing the guard in phy_config itself was fine (and I'm coming down harder on "do $this to avoid #ifdef" when it's not making code read easier).
participants (4)
-
Ramon Fried -
Rasmus Villemoes -
Simon Glass -
Tom Rini