2018-11-08 18:00:22

by Kyle Roeschley

Subject: [PATCH] net: phy: leds: Don't make our own link speed names

The phy core provides a handy phy_speed_to_str() helper, so use that
instead of doing our own formatting of the different known link speeds.

Signed-off-by: Kyle Roeschley <[email protected]>
---
drivers/net/phy/phy_led_triggers.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/net/phy/phy_led_triggers.c b/drivers/net/phy/phy_led_triggers.c
index 491efc1bf5c4..2827eb413c9c 100644
--- a/drivers/net/phy/phy_led_triggers.c
+++ b/drivers/net/phy/phy_led_triggers.c
@@ -77,20 +77,9 @@ static int phy_led_trigger_register(struct phy_device *phy,
struct phy_led_trigger *plt,
unsigned int speed)
{
- char name_suffix[PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE];
-
plt->speed = speed;
-
- if (speed < SPEED_1000)
- snprintf(name_suffix, sizeof(name_suffix), "%dMbps", speed);
- else if (speed == SPEED_2500)
- snprintf(name_suffix, sizeof(name_suffix), "2.5Gbps");
- else
- snprintf(name_suffix, sizeof(name_suffix), "%dGbps",
- DIV_ROUND_CLOSEST(speed, 1000));
-
phy_led_trigger_format_name(phy, plt->name, sizeof(plt->name),
- name_suffix);
+ phy_speed_to_str(speed));
plt->trigger.name = plt->name;

return led_trigger_register(&plt->trigger);
--
2.19.1

2018-11-08 18:00:54

by Florian Fainelli

Subject: Re: [PATCH] net: phy: leds: Don't make our own link speed names

On 11/8/18 9:51 AM, Kyle Roeschley wrote:
> The phy core provides a handy phy_speed_to_str() helper, so use that
> instead of doing our own formatting of the different known link speeds.

In case the speed is not supported, phy_speed_to_str() would return
"Unsupported (update phy-core.c)" which is bigger (by 21 characters)
than name_suffix.

If you bumped name_suffix/PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE to 11
characters, that would be just large enough to accommodate for the
"Unsupported" part of the string and that might be an acceptable
solution in between.

>
> Signed-off-by: Kyle Roeschley <[email protected]>
> ---
> drivers/net/phy/phy_led_triggers.c | 13 +------------
> 1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/drivers/net/phy/phy_led_triggers.c b/drivers/net/phy/phy_led_triggers.c
> index 491efc1bf5c4..2827eb413c9c 100644
> --- a/drivers/net/phy/phy_led_triggers.c
> +++ b/drivers/net/phy/phy_led_triggers.c
> @@ -77,20 +77,9 @@ static int phy_led_trigger_register(struct phy_device *phy,
> struct phy_led_trigger *plt,
> unsigned int speed)
> {
> - char name_suffix[PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE];
> -
> plt->speed = speed;
> -
> - if (speed < SPEED_1000)
> - snprintf(name_suffix, sizeof(name_suffix), "%dMbps", speed);
> - else if (speed == SPEED_2500)
> - snprintf(name_suffix, sizeof(name_suffix), "2.5Gbps");
> - else
> - snprintf(name_suffix, sizeof(name_suffix), "%dGbps",
> - DIV_ROUND_CLOSEST(speed, 1000));
> -
> phy_led_trigger_format_name(phy, plt->name, sizeof(plt->name),
> - name_suffix);
> + phy_speed_to_str(speed));
> plt->trigger.name = plt->name;
>
> return led_trigger_register(&plt->trigger);
>


--
Florian

2018-11-08 19:34:35

by Kyle Roeschley

Subject: Re: [PATCH] net: phy: leds: Don't make our own link speed names

On Thu, Nov 08, 2018 at 09:59:53AM -0800, Florian Fainelli wrote:
> On 11/8/18 9:51 AM, Kyle Roeschley wrote:
> > The phy core provides a handy phy_speed_to_str() helper, so use that
> > instead of doing our own formatting of the different known link speeds.
>
> In case the speed is not supported, phy_speed_to_str() would return
> "Unsupported (update phy-core.c)" which is bigger (by 21 characters)
> than name_suffix.
>
> If you bumped name_suffix/PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE to 11
> characters, that would be just large enough to accommodate for the
> "Unsupported" part of the string and that might be an acceptable
> solution in between.
>

Thanks for catching that, I'll send a v2.

--
Kyle Roeschley
Software Engineer
National Instruments