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
