[05/14] phylib: add *_direct() variants of phy_connect and phy_attach functions
diff mbox

Message ID 20090331082709.1427.48689.stgit@localhost.localdomain
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Grant Likely March 31, 2009, 8:27 a.m. UTC
From: Grant Likely <grant.likely@secretlab.ca>

Add phy_connect_direct() and phy_attach_direct() functions so that
drivers can use a pointer to the phy_device instead of trying to determine
the phy's bus_id string.

This patch is useful for OF device tree descriptions of phy devices where
the driver doesn't need or know what the bus_id value in order to get a
phy_device pointer.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

 drivers/net/phy/phy_device.c |  118 ++++++++++++++++++++++++++++++------------
 include/linux/phy.h          |    5 ++
 2 files changed, 90 insertions(+), 33 deletions(-)



--
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

Comments

Andy Fleming April 15, 2009, 9:10 p.m. UTC | #1
On Mar 31, 2009, at 3:27 AM, Grant Likely wrote:

> From: Grant Likely <grant.likely@secretlab.ca>
>
> Add phy_connect_direct() and phy_attach_direct() functions so that
> drivers can use a pointer to the phy_device instead of trying to  
> determine
> the phy's bus_id string.
>
> This patch is useful for OF device tree descriptions of phy devices  
> where
> the driver doesn't need or know what the bus_id value in order to  
> get a
> phy_device pointer.
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> @@ -312,18 +339,21 @@ struct phy_device * phy_connect(struct  
> net_device *dev, const char *bus_id,
> 		phy_interface_t interface)
> {
> 	struct phy_device *phydev;
> +	struct device *d;
> +	int rc;
>
> -	phydev = phy_attach(dev, bus_id, flags, interface);
> -
> -	if (IS_ERR(phydev))
> -		return phydev;
> -
> -	phy_prepare_link(phydev, handler);
> -
> -	phy_start_machine(phydev, NULL);
> +	/* Search the list of PHY devices on the mdio bus for the
> +	 * PHY with the requested name */
> +	d = bus_find_device_by_name(&mdio_bus_type, NULL, bus_id);
> +	if (!d) {
> +		pr_err("PHY %s not found\n", bus_id);
> +		return ERR_PTR(-ENODEV);
> +	}
> +	phydev = to_phy_device(d);
>
> -	if (phydev->irq > 0)
> -		phy_start_interrupts(phydev);
> +	rc = phy_attach_direct(dev, phydev, flags, interface);
> +	if (rc)
> +		return ERR_PTR(rc);


Why not just invoke phy_attach(), here, and thereby avoid the  
duplicate search code?

Otherwise, looks good.

Andy
--
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
Grant Likely April 18, 2009, 6:08 a.m. UTC | #2
On Wed, Apr 15, 2009 at 3:10 PM, Andy Fleming <afleming@freescale.com> wrote:
>
> On Mar 31, 2009, at 3:27 AM, Grant Likely wrote:
>
>> From: Grant Likely <grant.likely@secretlab.ca>
>>
>> Add phy_connect_direct() and phy_attach_direct() functions so that
>> drivers can use a pointer to the phy_device instead of trying to determine
>> the phy's bus_id string.
>>
>> This patch is useful for OF device tree descriptions of phy devices where
>> the driver doesn't need or know what the bus_id value in order to get a
>> phy_device pointer.
>>
>> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>> ---
>> @@ -312,18 +339,21 @@ struct phy_device * phy_connect(struct net_device
>> *dev, const char *bus_id,
>>                phy_interface_t interface)
>> {
>>        struct phy_device *phydev;
>> +       struct device *d;
>> +       int rc;
>>
>> -       phydev = phy_attach(dev, bus_id, flags, interface);
>> -
>> -       if (IS_ERR(phydev))
>> -               return phydev;
>> -
>> -       phy_prepare_link(phydev, handler);
>> -
>> -       phy_start_machine(phydev, NULL);
>> +       /* Search the list of PHY devices on the mdio bus for the
>> +        * PHY with the requested name */
>> +       d = bus_find_device_by_name(&mdio_bus_type, NULL, bus_id);
>> +       if (!d) {
>> +               pr_err("PHY %s not found\n", bus_id);
>> +               return ERR_PTR(-ENODEV);
>> +       }
>> +       phydev = to_phy_device(d);
>>
>> -       if (phydev->irq > 0)
>> -               phy_start_interrupts(phydev);
>> +       rc = phy_attach_direct(dev, phydev, flags, interface);
>> +       if (rc)
>> +               return ERR_PTR(rc);
>
>
> Why not just invoke phy_attach(), here, and thereby avoid the duplicate
> search code?

Yeah, you're right.  I had done it this way for symmetry, but calling
phy_attach_direct is probably better.

g.
Grant Likely April 18, 2009, 6:18 a.m. UTC | #3
On Sat, Apr 18, 2009 at 12:08 AM, Grant Likely
<grant.likely@secretlab.ca> wrote:
> On Wed, Apr 15, 2009 at 3:10 PM, Andy Fleming <afleming@freescale.com> wrote:
>>
>> On Mar 31, 2009, at 3:27 AM, Grant Likely wrote:
>>
>>> From: Grant Likely <grant.likely@secretlab.ca>
>>>
>>> Add phy_connect_direct() and phy_attach_direct() functions so that
>>> drivers can use a pointer to the phy_device instead of trying to determine
>>> the phy's bus_id string.
>>>
>>> This patch is useful for OF device tree descriptions of phy devices where
>>> the driver doesn't need or know what the bus_id value in order to get a
>>> phy_device pointer.
>>>
>>> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>>> ---
>>> @@ -312,18 +339,21 @@ struct phy_device * phy_connect(struct net_device
>>> *dev, const char *bus_id,
>>>                phy_interface_t interface)
>>> {
>>>        struct phy_device *phydev;
>>> +       struct device *d;
>>> +       int rc;
>>>
>>> -       phydev = phy_attach(dev, bus_id, flags, interface);
>>> -
>>> -       if (IS_ERR(phydev))
>>> -               return phydev;
>>> -
>>> -       phy_prepare_link(phydev, handler);
>>> -
>>> -       phy_start_machine(phydev, NULL);
>>> +       /* Search the list of PHY devices on the mdio bus for the
>>> +        * PHY with the requested name */
>>> +       d = bus_find_device_by_name(&mdio_bus_type, NULL, bus_id);
>>> +       if (!d) {
>>> +               pr_err("PHY %s not found\n", bus_id);
>>> +               return ERR_PTR(-ENODEV);
>>> +       }
>>> +       phydev = to_phy_device(d);
>>>
>>> -       if (phydev->irq > 0)
>>> -               phy_start_interrupts(phydev);
>>> +       rc = phy_attach_direct(dev, phydev, flags, interface);
>>> +       if (rc)
>>> +               return ERR_PTR(rc);
>>
>>
>> Why not just invoke phy_attach(), here, and thereby avoid the duplicate
>> search code?
>
> Yeah, you're right.  I had done it this way for symmetry, but calling
> phy_attach_direct is probably better.

Actually, no, my change is buggy.  I should be calling
phy_connect_direct() here, not phy_attach_direct().  Plus, either way
I do it there will be a few lines of duplicate code.  It will be fixed
in the next version.

g.

Patch
diff mbox

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 9352ca8..3c8c1de 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -292,6 +292,33 @@  void phy_prepare_link(struct phy_device *phydev,
 }
 
 /**
+ * phy_connect_direct - connect an ethernet device to a specific phy_device
+ * @dev: the network device to connect
+ * @phydev: the pointer to the phy device
+ * @handler: callback function for state change notifications
+ * @flags: PHY device's dev_flags
+ * @interface: PHY device's interface
+ */
+int phy_connect_direct(struct net_device *dev, struct phy_device *phydev,
+		       void (*handler)(struct net_device *), u32 flags,
+		       phy_interface_t interface)
+{
+	int rc;
+
+	rc = phy_attach_direct(dev, phydev, flags, interface);
+	if (rc)
+		return rc;
+
+	phy_prepare_link(phydev, handler);
+	phy_start_machine(phydev, NULL);
+	if (phydev->irq > 0)
+		phy_start_interrupts(phydev);
+
+	return 0;
+}
+EXPORT_SYMBOL(phy_connect_direct);
+
+/**
  * phy_connect - connect an ethernet device to a PHY device
  * @dev: the network device to connect
  * @bus_id: the id string of the PHY device to connect
@@ -312,18 +339,21 @@  struct phy_device * phy_connect(struct net_device *dev, const char *bus_id,
 		phy_interface_t interface)
 {
 	struct phy_device *phydev;
+	struct device *d;
+	int rc;
 
-	phydev = phy_attach(dev, bus_id, flags, interface);
-
-	if (IS_ERR(phydev))
-		return phydev;
-
-	phy_prepare_link(phydev, handler);
-
-	phy_start_machine(phydev, NULL);
+	/* Search the list of PHY devices on the mdio bus for the
+	 * PHY with the requested name */
+	d = bus_find_device_by_name(&mdio_bus_type, NULL, bus_id);
+	if (!d) {
+		pr_err("PHY %s not found\n", bus_id);
+		return ERR_PTR(-ENODEV);
+	}
+	phydev = to_phy_device(d);
 
-	if (phydev->irq > 0)
-		phy_start_interrupts(phydev);
+	rc = phy_attach_direct(dev, phydev, flags, interface);
+	if (rc)
+		return ERR_PTR(rc);
 
 	return phydev;
 }
@@ -347,9 +377,9 @@  void phy_disconnect(struct phy_device *phydev)
 EXPORT_SYMBOL(phy_disconnect);
 
 /**
- * phy_attach - attach a network device to a particular PHY device
+ * phy_attach_direct - attach a network device to a given PHY device pointer
  * @dev: network device to attach
- * @bus_id: PHY device to attach
+ * @phydev: Pointer to phy_device to attach
  * @flags: PHY device's dev_flags
  * @interface: PHY device's interface
  *
@@ -360,22 +390,10 @@  EXPORT_SYMBOL(phy_disconnect);
  *     the attaching device, and given a callback for link status
  *     change.  The phy_device is returned to the attaching driver.
  */
-struct phy_device *phy_attach(struct net_device *dev,
-		const char *bus_id, u32 flags, phy_interface_t interface)
+int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
+		      u32 flags, phy_interface_t interface)
 {
-	struct bus_type *bus = &mdio_bus_type;
-	struct phy_device *phydev;
-	struct device *d;
-
-	/* Search the list of PHY devices on the mdio bus for the
-	 * PHY with the requested name */
-	d = bus_find_device_by_name(bus, NULL, bus_id);
-	if (d) {
-		phydev = to_phy_device(d);
-	} else {
-		printk(KERN_ERR "%s not found\n", bus_id);
-		return ERR_PTR(-ENODEV);
-	}
+	struct device *d = &phydev->dev;
 
 	/* Assume that if there is no driver, that it doesn't
 	 * exist, and we should use the genphy driver. */
@@ -388,13 +406,12 @@  struct phy_device *phy_attach(struct net_device *dev,
 			err = device_bind_driver(d);
 
 		if (err)
-			return ERR_PTR(err);
+			return err;
 	}
 
 	if (phydev->attached_dev) {
-		printk(KERN_ERR "%s: %s already attached\n",
-				dev->name, bus_id);
-		return ERR_PTR(-EBUSY);
+		dev_err(&dev->dev, "PHY already attached\n");
+		return -EBUSY;
 	}
 
 	phydev->attached_dev = dev;
@@ -412,13 +429,48 @@  struct phy_device *phy_attach(struct net_device *dev,
 		err = phy_scan_fixups(phydev);
 
 		if (err < 0)
-			return ERR_PTR(err);
+			return err;
 
 		err = phydev->drv->config_init(phydev);
 
 		if (err < 0)
-			return ERR_PTR(err);
+			return err;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(phy_attach_direct);
+
+/**
+ * phy_attach - attach a network device to a particular PHY device
+ * @dev: network device to attach
+ * @bus_id: Bus ID of PHY device to attach
+ * @flags: PHY device's dev_flags
+ * @interface: PHY device's interface
+ *
+ * Description: Same as phy_attach_direct() except that a PHY bus_id
+ *     string is passed instead of a pointer to a struct phy_device.
+ */
+struct phy_device *phy_attach(struct net_device *dev,
+		const char *bus_id, u32 flags, phy_interface_t interface)
+{
+	struct bus_type *bus = &mdio_bus_type;
+	struct phy_device *phydev;
+	struct device *d;
+	int rc;
+
+	/* Search the list of PHY devices on the mdio bus for the
+	 * PHY with the requested name */
+	d = bus_find_device_by_name(bus, NULL, bus_id);
+	if (!d) {
+		pr_err("PHY %s not found\n", bus_id);
+		return ERR_PTR(-ENODEV);
 	}
+	phydev = to_phy_device(d);
+
+	rc = phy_attach_direct(dev, phydev, flags, interface);
+	if (rc)
+		return ERR_PTR(rc);
 
 	return phydev;
 }
diff --git a/include/linux/phy.h b/include/linux/phy.h
index c0cf354..cd404f1 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -441,8 +441,13 @@  struct phy_device* get_phy_device(struct mii_bus *bus, int addr);
 int phy_device_register(struct phy_device *phy);
 int phy_clear_interrupt(struct phy_device *phydev);
 int phy_config_interrupt(struct phy_device *phydev, u32 interrupts);
+int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
+		u32 flags, phy_interface_t interface);
 struct phy_device * phy_attach(struct net_device *dev,
 		const char *bus_id, u32 flags, phy_interface_t interface);
+int phy_connect_direct(struct net_device *dev, struct phy_device *phydev,
+		void (*handler)(struct net_device *), u32 flags,
+		phy_interface_t interface);
 struct phy_device * phy_connect(struct net_device *dev, const char *bus_id,
 		void (*handler)(struct net_device *), u32 flags,
 		phy_interface_t interface);