diff mbox

[net-next,2/4] net: systemport: use the new fixed PHY helpers

Message ID 1400528625-32744-3-git-send-email-f.fainelli@gmail.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Florian Fainelli May 19, 2014, 7:43 p.m. UTC
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/bcmsysport.c | 18 ++++++++++++++++--
 drivers/net/ethernet/broadcom/bcmsysport.h |  1 +
 2 files changed, 17 insertions(+), 2 deletions(-)

Comments

Thomas Petazzoni May 19, 2014, 8:19 p.m. UTC | #1
Dear Florian Fainelli,

On Mon, 19 May 2014 12:43:43 -0700, Florian Fainelli wrote:
> +	priv->phydev = of_phy_connect(dev, priv->phy_dn, bcm_sysport_adj_link,
> +					0, priv->phy_interface);
>  	if (!priv->phydev) {
>  		netdev_err(dev, "could not attach to PHY\n");
>  		return -ENODEV;
> @@ -1551,6 +1551,20 @@ static int bcm_sysport_probe(struct platform_device *pdev)
>  	if (priv->phy_interface < 0)
>  		priv->phy_interface = PHY_INTERFACE_MODE_GMII;
>  
> +	/* SYSTEMPORT uses the old 'fixed-link' 5-digit property which is
> +	 * deprecated, we need to register the fixed PHY using the Ethernet
> +	 * MAC node since we do not have a 'fixed-link' subnode.
> +	 */

Actually even if you use the new fixed PHY DT binding, the DT node
linked to the PHY is the Ethernet MAC node. This is because I didn't
want to have a difference between how things are handled between the
old and the new binding.

Thomas
Florian Fainelli May 19, 2014, 9:52 p.m. UTC | #2
2014-05-19 13:19 GMT-07:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Dear Florian Fainelli,
>
> On Mon, 19 May 2014 12:43:43 -0700, Florian Fainelli wrote:
>> +     priv->phydev = of_phy_connect(dev, priv->phy_dn, bcm_sysport_adj_link,
>> +                                     0, priv->phy_interface);
>>       if (!priv->phydev) {
>>               netdev_err(dev, "could not attach to PHY\n");
>>               return -ENODEV;
>> @@ -1551,6 +1551,20 @@ static int bcm_sysport_probe(struct platform_device *pdev)
>>       if (priv->phy_interface < 0)
>>               priv->phy_interface = PHY_INTERFACE_MODE_GMII;
>>
>> +     /* SYSTEMPORT uses the old 'fixed-link' 5-digit property which is
>> +      * deprecated, we need to register the fixed PHY using the Ethernet
>> +      * MAC node since we do not have a 'fixed-link' subnode.
>> +      */
>
> Actually even if you use the new fixed PHY DT binding, the DT node
> linked to the PHY is the Ethernet MAC node. This is because I didn't
> want to have a difference between how things are handled between the
> old and the new binding.

Sure, I agree, and the comment still stands, I am just expressing it
that we are not looking for the 'fixed-link' subnode, but the
'fixed-link' 5-digit property here. Would you want me to rephrase
that?
Thomas Petazzoni May 19, 2014, 10:04 p.m. UTC | #3
Dear Florian Fainelli,

On Mon, 19 May 2014 14:52:57 -0700, Florian Fainelli wrote:

> >> +     /* SYSTEMPORT uses the old 'fixed-link' 5-digit property which is
> >> +      * deprecated, we need to register the fixed PHY using the Ethernet
> >> +      * MAC node since we do not have a 'fixed-link' subnode.
> >> +      */
> >
> > Actually even if you use the new fixed PHY DT binding, the DT node
> > linked to the PHY is the Ethernet MAC node. This is because I didn't
> > want to have a difference between how things are handled between the
> > old and the new binding.
> 
> Sure, I agree, and the comment still stands, I am just expressing it
> that we are not looking for the 'fixed-link' subnode, but the
> 'fixed-link' 5-digit property here. Would you want me to rephrase
> that?

What I found misleading in the comment is that you simply to imply that
because you're not using the new DT binding for fixed-link, the DT node
that is associated to the PHY is the Ethernet MAC.

The truth is that the DT node associated to the fixed PHY is always the
Ethernet MAC, regardless of whether you're using the old or the new DT
binding.

Thomas
Florian Fainelli May 19, 2014, 11:31 p.m. UTC | #4
2014-05-19 15:04 GMT-07:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Dear Florian Fainelli,
>
> On Mon, 19 May 2014 14:52:57 -0700, Florian Fainelli wrote:
>
>> >> +     /* SYSTEMPORT uses the old 'fixed-link' 5-digit property which is
>> >> +      * deprecated, we need to register the fixed PHY using the Ethernet
>> >> +      * MAC node since we do not have a 'fixed-link' subnode.
>> >> +      */
>> >
>> > Actually even if you use the new fixed PHY DT binding, the DT node
>> > linked to the PHY is the Ethernet MAC node. This is because I didn't
>> > want to have a difference between how things are handled between the
>> > old and the new binding.
>>
>> Sure, I agree, and the comment still stands, I am just expressing it
>> that we are not looking for the 'fixed-link' subnode, but the
>> 'fixed-link' 5-digit property here. Would you want me to rephrase
>> that?
>
> What I found misleading in the comment is that you simply to imply that
> because you're not using the new DT binding for fixed-link, the DT node
> that is associated to the PHY is the Ethernet MAC.
>
> The truth is that the DT node associated to the fixed PHY is always the
> Ethernet MAC, regardless of whether you're using the old or the new DT
> binding.

Right, I will respin without this misleading comment and also to
address the formatting issue spotted by Sergei. Thanks!
diff mbox

Patch

diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index d40c5b969e9e..6368a085bdae 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -1327,8 +1327,8 @@  static int bcm_sysport_open(struct net_device *dev)
 	/* Read CRC forward */
 	priv->crc_fwd = !!(umac_readl(priv, UMAC_CMD) & CMD_CRC_FWD);
 
-	priv->phydev = of_phy_connect_fixed_link(dev, bcm_sysport_adj_link,
-							priv->phy_interface);
+	priv->phydev = of_phy_connect(dev, priv->phy_dn, bcm_sysport_adj_link,
+					0, priv->phy_interface);
 	if (!priv->phydev) {
 		netdev_err(dev, "could not attach to PHY\n");
 		return -ENODEV;
@@ -1551,6 +1551,20 @@  static int bcm_sysport_probe(struct platform_device *pdev)
 	if (priv->phy_interface < 0)
 		priv->phy_interface = PHY_INTERFACE_MODE_GMII;
 
+	/* SYSTEMPORT uses the old 'fixed-link' 5-digit property which is
+	 * deprecated, we need to register the fixed PHY using the Ethernet
+	 * MAC node since we do not have a 'fixed-link' subnode.
+	 */
+	if (of_phy_is_fixed_link(dn)) {
+		ret = of_phy_register_fixed_link(dn);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to register fixed PHY\n");
+			goto err;
+		}
+
+		priv->phy_dn = dn;
+	}
+
 	/* Initialize netdevice members */
 	macaddr = of_get_mac_address(dn);
 	if (!macaddr || !is_valid_ether_addr(macaddr)) {
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
index abdeb62616df..73fd04a94797 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.h
+++ b/drivers/net/ethernet/broadcom/bcmsysport.h
@@ -656,6 +656,7 @@  struct bcm_sysport_priv {
 	unsigned int		rx_c_index;
 
 	/* PHY device */
+	struct device_node	*phy_dn;
 	struct phy_device	*phydev;
 	phy_interface_t		phy_interface;
 	int			old_pause;