diff mbox

[v2] net: phy: fixed: propagate fixed link values to struct

Message ID 1440601127-7663-1-git-send-email-madalin.bucur@freescale.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Madalin Bucur Aug. 26, 2015, 2:58 p.m. UTC
The fixed link values parsed from the device tree are stored in
the struct fixed_phy member status. The struct phy_device members
speed, duplex were not updated.

Signed-off-by: Madalin Bucur <madalin.bucur@freescale.com>
---
v2: always setting phy->link, thanks Stas

 drivers/net/phy/fixed_phy.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Stas Sergeev Aug. 26, 2015, 3:51 p.m. UTC | #1
26.08.2015 17:58, Madalin Bucur пишет:
> The fixed link values parsed from the device tree are stored in
> the struct fixed_phy member status. The struct phy_device members
> speed, duplex were not updated.

ACK, but IMHO it will make more sense if you include that
into your upcoming patch set rather than sending separately,
as otherwise there is simply no in-kernel users of that new
functionality (all the current users likely do not access
these fields as early as you want to, so they don't care).
In any case, the patch looks good to me and the policy is
up to others.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Madalin Bucur Aug. 26, 2015, 4:12 p.m. UTC | #2
> -----Original Message-----

> From: Stas Sergeev [mailto:stsp@list.ru]

> Sent: Wednesday, August 26, 2015 6:51 PM

> To: Bucur Madalin-Cristian-B32716 <madalin.bucur@freescale.com>;

> f.fainelli@gmail.com

> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Liberman Igal-

> B31950 <Igal.Liberman@freescale.com>

> Subject: Re: [v2] net: phy: fixed: propagate fixed link values to struct

> 

> 26.08.2015 17:58, Madalin Bucur пишет:

> > The fixed link values parsed from the device tree are stored in

> > the struct fixed_phy member status. The struct phy_device members

> > speed, duplex were not updated.

> 

> ACK, but IMHO it will make more sense if you include that

> into your upcoming patch set rather than sending separately,

> as otherwise there is simply no in-kernel users of that new

> functionality (all the current users likely do not access

> these fields as early as you want to, so they don't care).

> In any case, the patch looks good to me and the policy is

> up to others.


Given that it's more of a fix than a feature, I think it can be picked up separate
from a certain driver that accesses those fields early but I guess Florian, David
will decide this.

Thanks,
Madalin
Florian Fainelli Aug. 26, 2015, 6:48 p.m. UTC | #3
On 26/08/15 07:58, Madalin Bucur wrote:
> The fixed link values parsed from the device tree are stored in
> the struct fixed_phy member status. The struct phy_device members
> speed, duplex were not updated.

Arguably you need to start the PHY state machine for this to work
properly, but this looks fine to me.

> 
> Signed-off-by: Madalin Bucur <madalin.bucur@freescale.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

> ---
> v2: always setting phy->link, thanks Stas
> 
>  drivers/net/phy/fixed_phy.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
> index 479b93f..99d9bc1 100644
> --- a/drivers/net/phy/fixed_phy.c
> +++ b/drivers/net/phy/fixed_phy.c
> @@ -292,6 +292,15 @@ struct phy_device *fixed_phy_register(unsigned int irq,
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> +	/* propagate the fixed link values to struct phy_device */
> +	phy->link = status->link;
> +	if (status->link) {
> +		phy->speed = status->speed;
> +		phy->duplex = status->duplex;
> +		phy->pause = status->pause;
> +		phy->asym_pause = status->asym_pause;
> +	}
> +
>  	of_node_get(np);
>  	phy->dev.of_node = np;
>  
>
David Miller Aug. 27, 2015, 6:25 p.m. UTC | #4
From: Madalin Bucur <madalin.bucur@freescale.com>
Date: Wed, 26 Aug 2015 17:58:47 +0300

> The fixed link values parsed from the device tree are stored in
> the struct fixed_phy member status. The struct phy_device members
> speed, duplex were not updated.
> 
> Signed-off-by: Madalin Bucur <madalin.bucur@freescale.com>
> ---
> v2: always setting phy->link, thanks Stas

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index 479b93f..99d9bc1 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -292,6 +292,15 @@  struct phy_device *fixed_phy_register(unsigned int irq,
 		return ERR_PTR(-EINVAL);
 	}
 
+	/* propagate the fixed link values to struct phy_device */
+	phy->link = status->link;
+	if (status->link) {
+		phy->speed = status->speed;
+		phy->duplex = status->duplex;
+		phy->pause = status->pause;
+		phy->asym_pause = status->asym_pause;
+	}
+
 	of_node_get(np);
 	phy->dev.of_node = np;