netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/4] net: phy: neaten phy_print_status
@ 2014-01-23 20:17 Florian Fainelli
  2014-01-23 20:17 ` [PATCH net-next v2 1/4] net: phy: use network device in phy_print_status Florian Fainelli
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Florian Fainelli @ 2014-01-23 20:17 UTC (permalink / raw)
  To: netdev; +Cc: davem, joe, Florian Fainelli

David, Joe,

This patchset neatens phy_print_status based on earlier feedback from Joe.

Thanks!

Florian Fainelli (4):
  net: phy: use network device in phy_print_status
  net: phy: update phy_print_status to show pause settings
  net: phy: display human readable PHY speed settings
  net: phy: remove unneeded parenthesis

 drivers/net/phy/phy.c | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

-- 
1.8.3.2

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH net-next v2 1/4] net: phy: use network device in phy_print_status
  2014-01-23 20:17 [PATCH net-next v2 0/4] net: phy: neaten phy_print_status Florian Fainelli
@ 2014-01-23 20:17 ` Florian Fainelli
  2014-01-23 20:17 ` [PATCH net-next v2 2/4] net: phy: update phy_print_status to show pause settings Florian Fainelli
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Florian Fainelli @ 2014-01-23 20:17 UTC (permalink / raw)
  To: netdev; +Cc: davem, joe, Florian Fainelli

phy_print_status() currently uses dev_name(&phydev->dev) which will
usually result in printing something along those lines for Device Tree
aware drivers:

libphy: f0b60000.etherne:0a - Link is Down
libphy: f0ba0000.etherne:00 - Link is Up - 1000/Full

This is not terribly useful for network administrators or users since we
expect a network interface name to be able to correlate link events with
interfaces. Update phy_print_status() to use netdev_info() with
phydev->attached_dev which is the backing network device for our PHY
device. The leading dash is removed since netdev_info() prefixes the
messages with "<interface>: " already.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/phy.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 19c9eca..c35b2e7 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -45,12 +45,11 @@
 void phy_print_status(struct phy_device *phydev)
 {
 	if (phydev->link) {
-		pr_info("%s - Link is Up - %d/%s\n",
-			dev_name(&phydev->dev),
+		netdev_info(phydev->attached_dev, "Link is Up - %d/%s\n",
 			phydev->speed,
 			DUPLEX_FULL == phydev->duplex ? "Full" : "Half");
 	} else	{
-		pr_info("%s - Link is Down\n", dev_name(&phydev->dev));
+		netdev_info(phydev->attached_dev, "Link is Down\n");
 	}
 }
 EXPORT_SYMBOL(phy_print_status);
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net-next v2 2/4] net: phy: update phy_print_status to show pause settings
  2014-01-23 20:17 [PATCH net-next v2 0/4] net: phy: neaten phy_print_status Florian Fainelli
  2014-01-23 20:17 ` [PATCH net-next v2 1/4] net: phy: use network device in phy_print_status Florian Fainelli
@ 2014-01-23 20:17 ` Florian Fainelli
  2014-01-23 20:17 ` [PATCH net-next v2 3/4] net: phy: display human readable PHY speed settings Florian Fainelli
  2014-01-23 20:17 ` [PATCH net-next v2 4/4] net: phy: remove unneeded parenthesis Florian Fainelli
  3 siblings, 0 replies; 9+ messages in thread
From: Florian Fainelli @ 2014-01-23 20:17 UTC (permalink / raw)
  To: netdev; +Cc: davem, joe, Florian Fainelli

Update phy_print_status() to also display the PHY device pause settings
(rx/tx or off).

Suggested-by: Joe Perches <joe@perches.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/phy.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c35b2e7..8ae2260 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -45,9 +45,11 @@
 void phy_print_status(struct phy_device *phydev)
 {
 	if (phydev->link) {
-		netdev_info(phydev->attached_dev, "Link is Up - %d/%s\n",
+		netdev_info(phydev->attached_dev,
+			"Link is Up - %d/%s - flow control %s\n",
 			phydev->speed,
-			DUPLEX_FULL == phydev->duplex ? "Full" : "Half");
+			DUPLEX_FULL == phydev->duplex ? "Full" : "Half",
+			phydev->pause ? "rx/tx" : "off");
 	} else	{
 		netdev_info(phydev->attached_dev, "Link is Down\n");
 	}
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net-next v2 3/4] net: phy: display human readable PHY speed settings
  2014-01-23 20:17 [PATCH net-next v2 0/4] net: phy: neaten phy_print_status Florian Fainelli
  2014-01-23 20:17 ` [PATCH net-next v2 1/4] net: phy: use network device in phy_print_status Florian Fainelli
  2014-01-23 20:17 ` [PATCH net-next v2 2/4] net: phy: update phy_print_status to show pause settings Florian Fainelli
@ 2014-01-23 20:17 ` Florian Fainelli
  2014-01-23 20:17 ` [PATCH net-next v2 4/4] net: phy: remove unneeded parenthesis Florian Fainelli
  3 siblings, 0 replies; 9+ messages in thread
From: Florian Fainelli @ 2014-01-23 20:17 UTC (permalink / raw)
  To: netdev; +Cc: davem, joe, Florian Fainelli

Use a convenience function: phy_speed_to_str() which will display human
readable speeds.

Suggested-by: Joe Perches <joe@perches.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Joe,

This is highly inspired from the version you sent to me, although
I renamed to phy_speed_to_str() for clarity as well as added the
unit suffix to each speed because your version would show:

Unknownmbps for instance.

 drivers/net/phy/phy.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 8ae2260..36fc6e1 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -38,6 +38,26 @@
 
 #include <asm/irq.h>
 
+static const char *phy_speed_to_str(int speed)
+{
+	switch (speed) {
+	case SPEED_10:
+		return "10Mbps";
+	case SPEED_100:
+		return "100Mbps";
+	case SPEED_1000:
+		return "1Gbps";
+	case SPEED_2500:
+		return "2.5Gbps";
+	case SPEED_10000:
+		return "10Gbps";
+	case SPEED_UNKNOWN:
+		return "Unknown";
+	default:
+		return "Unsupported (update phy.c)";
+	}
+}
+
 /**
  * phy_print_status - Convenience function to print out the current phy status
  * @phydev: the phy_device struct
@@ -46,8 +66,8 @@ void phy_print_status(struct phy_device *phydev)
 {
 	if (phydev->link) {
 		netdev_info(phydev->attached_dev,
-			"Link is Up - %d/%s - flow control %s\n",
-			phydev->speed,
+			"Link is Up - %s/%s - flow control %s\n",
+			phy_speed_to_str(phydev->speed),
 			DUPLEX_FULL == phydev->duplex ? "Full" : "Half",
 			phydev->pause ? "rx/tx" : "off");
 	} else	{
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net-next v2 4/4] net: phy: remove unneeded parenthesis
  2014-01-23 20:17 [PATCH net-next v2 0/4] net: phy: neaten phy_print_status Florian Fainelli
                   ` (2 preceding siblings ...)
  2014-01-23 20:17 ` [PATCH net-next v2 3/4] net: phy: display human readable PHY speed settings Florian Fainelli
@ 2014-01-23 20:17 ` Florian Fainelli
  2014-01-23 22:37   ` Sergei Shtylyov
  2014-01-23 23:39   ` Fabio Estevam
  3 siblings, 2 replies; 9+ messages in thread
From: Florian Fainelli @ 2014-01-23 20:17 UTC (permalink / raw)
  To: netdev; +Cc: davem, joe, Florian Fainelli

Our if/else statement in phy_print_status() is only comprised of one
line for each, remove the parenthesis.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/phy.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 36fc6e1..59aa85e 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -64,15 +64,14 @@ static const char *phy_speed_to_str(int speed)
  */
 void phy_print_status(struct phy_device *phydev)
 {
-	if (phydev->link) {
+	if (phydev->link)
 		netdev_info(phydev->attached_dev,
 			"Link is Up - %s/%s - flow control %s\n",
 			phy_speed_to_str(phydev->speed),
 			DUPLEX_FULL == phydev->duplex ? "Full" : "Half",
 			phydev->pause ? "rx/tx" : "off");
-	} else	{
+	else
 		netdev_info(phydev->attached_dev, "Link is Down\n");
-	}
 }
 EXPORT_SYMBOL(phy_print_status);
 
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next v2 4/4] net: phy: remove unneeded parenthesis
  2014-01-23 20:17 ` [PATCH net-next v2 4/4] net: phy: remove unneeded parenthesis Florian Fainelli
@ 2014-01-23 22:37   ` Sergei Shtylyov
  2014-01-23 22:39     ` Florian Fainelli
  2014-01-23 23:39   ` Fabio Estevam
  1 sibling, 1 reply; 9+ messages in thread
From: Sergei Shtylyov @ 2014-01-23 22:37 UTC (permalink / raw)
  To: Florian Fainelli, netdev; +Cc: davem, joe

Hello.

On 24-01-2014 0:17, Florian Fainelli wrote:

> Our if/else statement in phy_print_status() is only comprised of one
> line for each, remove the parenthesis.

    I protest, the *if* arm is multi-line, though single statement. :-)
Could we avoid changing that code to and fro during 3.14-rc1?

> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>   drivers/net/phy/phy.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)

> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 36fc6e1..59aa85e 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -64,15 +64,14 @@ static const char *phy_speed_to_str(int speed)
>    */
>   void phy_print_status(struct phy_device *phydev)
>   {
> -	if (phydev->link) {
> +	if (phydev->link)
>   		netdev_info(phydev->attached_dev,
>   			"Link is Up - %s/%s - flow control %s\n",
>   			phy_speed_to_str(phydev->speed),
>   			DUPLEX_FULL == phydev->duplex ? "Full" : "Half",
>   			phydev->pause ? "rx/tx" : "off");
> -	} else	{
> +	else
>   		netdev_info(phydev->attached_dev, "Link is Down\n");
> -	}
>   }
>   EXPORT_SYMBOL(phy_print_status);

WBR, Sergei

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next v2 4/4] net: phy: remove unneeded parenthesis
  2014-01-23 22:37   ` Sergei Shtylyov
@ 2014-01-23 22:39     ` Florian Fainelli
  2014-01-23 22:59       ` Sergei Shtylyov
  0 siblings, 1 reply; 9+ messages in thread
From: Florian Fainelli @ 2014-01-23 22:39 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: netdev, David Miller, Joe Perches

2014/1/23 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>:
> Hello.
>
>
> On 24-01-2014 0:17, Florian Fainelli wrote:
>
>> Our if/else statement in phy_print_status() is only comprised of one
>> line for each, remove the parenthesis.
>
>
>    I protest, the *if* arm is multi-line, though single statement. :-)
> Could we avoid changing that code to and fro during 3.14-rc1?

Is it that big of a problem? Does that make checkpatch.pl unhappy,
does that make you unhappy? Since this is intentionally the last patch
in the series, it would be trivial for David to ignore it I suppose.

>
>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>   drivers/net/phy/phy.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>
>
>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>> index 36fc6e1..59aa85e 100644
>> --- a/drivers/net/phy/phy.c
>> +++ b/drivers/net/phy/phy.c
>> @@ -64,15 +64,14 @@ static const char *phy_speed_to_str(int speed)
>>    */
>>   void phy_print_status(struct phy_device *phydev)
>>   {
>> -       if (phydev->link) {
>> +       if (phydev->link)
>>                 netdev_info(phydev->attached_dev,
>>                         "Link is Up - %s/%s - flow control %s\n",
>>                         phy_speed_to_str(phydev->speed),
>>                         DUPLEX_FULL == phydev->duplex ? "Full" : "Half",
>>                         phydev->pause ? "rx/tx" : "off");
>> -       } else  {
>> +       else
>>                 netdev_info(phydev->attached_dev, "Link is Down\n");
>> -       }
>>   }
>>   EXPORT_SYMBOL(phy_print_status);
>
>
> WBR, Sergei
>



-- 
Florian

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next v2 4/4] net: phy: remove unneeded parenthesis
  2014-01-23 22:39     ` Florian Fainelli
@ 2014-01-23 22:59       ` Sergei Shtylyov
  0 siblings, 0 replies; 9+ messages in thread
From: Sergei Shtylyov @ 2014-01-23 22:59 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: netdev, David Miller, Joe Perches

Hello.

On 24-01-2014 2:39, Florian Fainelli wrote:

>>> Our if/else statement in phy_print_status() is only comprised of one
>>> line for each, remove the parenthesis.

>>     I protest, the *if* arm is multi-line, though single statement. :-)
>> Could we avoid changing that code to and fro during 3.14-rc1?

> Is it that big of a problem?

    You should be clearer in the changelog, at least. :-)

> Does that make checkpatch.pl unhappy,

    No, it doesn't.

> does that make you unhappy?

    Kind of, as it was me who added {} in net-next, IIRC.

> Since this is intentionally the last patch
> in the series, it would be trivial for David to ignore it I suppose.

    Would be fine...

>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>> ---
>>>    drivers/net/phy/phy.c | 5 ++---
>>>    1 file changed, 2 insertions(+), 3 deletions(-)

>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>>> index 36fc6e1..59aa85e 100644
>>> --- a/drivers/net/phy/phy.c
>>> +++ b/drivers/net/phy/phy.c
>>> @@ -64,15 +64,14 @@ static const char *phy_speed_to_str(int speed)
>>>     */
>>>    void phy_print_status(struct phy_device *phydev)
>>>    {
>>> -       if (phydev->link) {
>>> +       if (phydev->link)
>>>                  netdev_info(phydev->attached_dev,
>>>                          "Link is Up - %s/%s - flow control %s\n",
>>>                          phy_speed_to_str(phydev->speed),
>>>                          DUPLEX_FULL == phydev->duplex ? "Full" : "Half",
>>>                          phydev->pause ? "rx/tx" : "off");
>>> -       } else  {
>>> +       else
>>>                  netdev_info(phydev->attached_dev, "Link is Down\n");
>>> -       }
>>>    }
>>>    EXPORT_SYMBOL(phy_print_status);

WBR, Sergei

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next v2 4/4] net: phy: remove unneeded parenthesis
  2014-01-23 20:17 ` [PATCH net-next v2 4/4] net: phy: remove unneeded parenthesis Florian Fainelli
  2014-01-23 22:37   ` Sergei Shtylyov
@ 2014-01-23 23:39   ` Fabio Estevam
  1 sibling, 0 replies; 9+ messages in thread
From: Fabio Estevam @ 2014-01-23 23:39 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: netdev@vger.kernel.org, David S. Miller, Joe Perches

On Thu, Jan 23, 2014 at 6:17 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> Our if/else statement in phy_print_status() is only comprised of one
> line for each, remove the parenthesis.

Your patch removes the braces, not the parenthesis ;-)

Regards,

Fabio Estevam

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-01-23 23:39 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-23 20:17 [PATCH net-next v2 0/4] net: phy: neaten phy_print_status Florian Fainelli
2014-01-23 20:17 ` [PATCH net-next v2 1/4] net: phy: use network device in phy_print_status Florian Fainelli
2014-01-23 20:17 ` [PATCH net-next v2 2/4] net: phy: update phy_print_status to show pause settings Florian Fainelli
2014-01-23 20:17 ` [PATCH net-next v2 3/4] net: phy: display human readable PHY speed settings Florian Fainelli
2014-01-23 20:17 ` [PATCH net-next v2 4/4] net: phy: remove unneeded parenthesis Florian Fainelli
2014-01-23 22:37   ` Sergei Shtylyov
2014-01-23 22:39     ` Florian Fainelli
2014-01-23 22:59       ` Sergei Shtylyov
2014-01-23 23:39   ` Fabio Estevam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).