Patchwork [net-next,3/3] ptp: derive the device name from the parent device

login
register
mail settings
Submitter Richard Cochran
Date Sept. 21, 2012, 5 p.m.
Message ID <9f2ed4a62d52182eeab86ad3dca7f6feeb68368a.1348245523.git.richardcochran@gmail.com>
Download mbox | patch
Permalink /patch/185840/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Richard Cochran - Sept. 21, 2012, 5 p.m.
PTP Hardware Clock device have a name that appears under sysfs that should
identify the underlying device. Instead of leaving it up to the driver to
invent a name, this patch changes the registration to automatically use
the name from the parent device.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/freescale/gianfar_ptp.c |    3 +--
 drivers/net/ethernet/intel/igb/igb_ptp.c     |    7 ++-----
 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c |    5 ++---
 drivers/net/phy/dp83640.c                    |    3 +--
 drivers/ptp/ptp_clock.c                      |    7 +++++--
 drivers/ptp/ptp_ixp46x.c                     |    3 +--
 drivers/ptp/ptp_pch.c                        |    3 +--
 drivers/ptp/ptp_private.h                    |    1 +
 drivers/ptp/ptp_sysfs.c                      |    2 +-
 include/linux/ptp_clock_kernel.h             |    8 +++++---
 10 files changed, 20 insertions(+), 22 deletions(-)
Ben Hutchings - Sept. 21, 2012, 5:19 p.m.
On Fri, 2012-09-21 at 19:00 +0200, Richard Cochran wrote:
> PTP Hardware Clock device have a name that appears under sysfs that should
> identify the underlying device. Instead of leaving it up to the driver to
> invent a name, this patch changes the registration to automatically use
> the name from the parent device.

I thought you wanted the driver name and not the parent device name?

[...]
> diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
> index 6e47450..60fa259 100644
> --- a/drivers/ptp/ptp_clock.c
> +++ b/drivers/ptp/ptp_clock.c
[...]
> @@ -219,11 +220,13 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info)
>         init_waitqueue_head(&ptp->tsev_wq);
>  
>         /* Create a new device in our class. */
> -       ptp->dev = device_create(ptp_class, NULL, ptp->devid, ptp,
> +       ptp->dev = device_create(ptp_class, parent, ptp->devid, ptp,
>                                  "ptp%d", ptp->index);
>         if (IS_ERR(ptp->dev))
>                 goto no_device;
>  
> +       ptp->name = parent ? dev_name(parent) : dev_name(ptp->dev);
> +

The fallback of using dev_name(ptp->dev) is a bit sad, as that's the
same name userland already has when it reads this attribute.

[...]
> diff --git a/drivers/ptp/ptp_ixp46x.c b/drivers/ptp/ptp_ixp46x.c
> index e03c406..c0939b1 100644
> --- a/drivers/ptp/ptp_ixp46x.c
> +++ b/drivers/ptp/ptp_ixp46x.c
> @@ -241,7 +241,6 @@ static int ptp_ixp_enable(struct ptp_clock_info *ptp,
>  
>  static struct ptp_clock_info ptp_ixp_caps = {
>  	.owner		= THIS_MODULE,
> -	.name		= "IXP46X timer",
>  	.max_adj	= 66666655,
>  	.n_ext_ts	= N_EXT_TS,
>  	.pps		= 0,
> @@ -298,7 +297,7 @@ static int __init ptp_ixp_init(void)
>  
>  	ixp_clock.caps = ptp_ixp_caps;
>  
> -	ixp_clock.ptp_clock = ptp_clock_register(&ixp_clock.caps);
> +	ixp_clock.ptp_clock = ptp_clock_register(&ixp_clock.caps, NULL);
[...]

I think this should really register a platform driver and a platform
device to be the parent of the clock device.  And then you don't need
the fallback for parent == NULL.

Since David has pulled the addition of PTP/PHC support to sfc, that will
need to be adjusted as well.

Ben.
Richard Cochran - Sept. 21, 2012, 5:37 p.m.
On Fri, Sep 21, 2012 at 06:19:25PM +0100, Ben Hutchings wrote:
> On Fri, 2012-09-21 at 19:00 +0200, Richard Cochran wrote:
> > PTP Hardware Clock device have a name that appears under sysfs that should
> > identify the underlying device. Instead of leaving it up to the driver to
> > invent a name, this patch changes the registration to automatically use
> > the name from the parent device.
> 
> I thought you wanted the driver name and not the parent device name?

Yes, I originally wanted the driver name, because it gives the user
information about the expected performance. But here I am bowing to
Jacob's suggestion that we really need to identify the device. Intel
cards with two ports have *two* clocks. Sad but true.

> > +       ptp->name = parent ? dev_name(parent) : dev_name(ptp->dev);
> > +
> 
> The fallback of using dev_name(ptp->dev) is a bit sad, as that's the
> same name userland already has when it reads this attribute.

This fallback only exists for the ixp46x, since that driver has no
proper platform or device tree device.

> > -	ixp_clock.ptp_clock = ptp_clock_register(&ixp_clock.caps);
> > +	ixp_clock.ptp_clock = ptp_clock_register(&ixp_clock.caps, NULL);
> [...]
> 
> I think this should really register a platform driver and a platform
> device to be the parent of the clock device.  And then you don't need
> the fallback for parent == NULL.

That chip is long past EOL, and its PTP capabilities are pretty poor,
and so it is not worth the trouble of fixing it up.

All other (and future) drivers must provide the pointer.
 
> Since David has pulled the addition of PTP/PHC support to sfc, that will
> need to be adjusted as well.

Okay, I will add that to the series.

Thanks,
Richard
--
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
Keller, Jacob E - Sept. 21, 2012, 5:39 p.m.
> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@gmail.com]
> Sent: Friday, September 21, 2012 10:37 AM
> To: Ben Hutchings
> Cc: netdev@vger.kernel.org; David Miller; Keller, Jacob E; Kirsher,
> Jeffrey T; John Stultz; Vick, Matthew
> Subject: Re: [PATCH net-next 3/3] ptp: derive the device name from the
> parent device
> 
> On Fri, Sep 21, 2012 at 06:19:25PM +0100, Ben Hutchings wrote:
> > On Fri, 2012-09-21 at 19:00 +0200, Richard Cochran wrote:
> > > PTP Hardware Clock device have a name that appears under sysfs that
> > > should identify the underlying device. Instead of leaving it up to
> > > the driver to invent a name, this patch changes the registration to
> > > automatically use the name from the parent device.
> >
> > I thought you wanted the driver name and not the parent device name?
> 
> Yes, I originally wanted the driver name, because it gives the user
> information about the expected performance. But here I am bowing to
> Jacob's suggestion that we really need to identify the device. Intel cards
> with two ports have *two* clocks. Sad but true.
> 

With the device, it is possible to lookup the driver.

- Jake

> Thanks,
> Richard
--
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
Ben Hutchings - Sept. 21, 2012, 5:40 p.m.
On Fri, 2012-09-21 at 17:39 +0000, Keller, Jacob E wrote:
> > -----Original Message-----
> > From: Richard Cochran [mailto:richardcochran@gmail.com]
> > Sent: Friday, September 21, 2012 10:37 AM
> > To: Ben Hutchings
> > Cc: netdev@vger.kernel.org; David Miller; Keller, Jacob E; Kirsher,
> > Jeffrey T; John Stultz; Vick, Matthew
> > Subject: Re: [PATCH net-next 3/3] ptp: derive the device name from the
> > parent device
> > 
> > On Fri, Sep 21, 2012 at 06:19:25PM +0100, Ben Hutchings wrote:
> > > On Fri, 2012-09-21 at 19:00 +0200, Richard Cochran wrote:
> > > > PTP Hardware Clock device have a name that appears under sysfs that
> > > > should identify the underlying device. Instead of leaving it up to
> > > > the driver to invent a name, this patch changes the registration to
> > > > automatically use the name from the parent device.
> > >
> > > I thought you wanted the driver name and not the parent device name?
> > 
> > Yes, I originally wanted the driver name, because it gives the user
> > information about the expected performance. But here I am bowing to
> > Jacob's suggestion that we really need to identify the device. Intel cards
> > with two ports have *two* clocks. Sad but true.
> > 
> 
> With the device, it is possible to lookup the driver.

For PCI devices, yes.  But there is really no guarantee that device
names (as in dev_name()) are globally unique either.

Ben.
Richard Cochran - Sept. 21, 2012, 6:11 p.m.
On Fri, Sep 21, 2012 at 06:40:58PM +0100, Ben Hutchings wrote:
> On Fri, 2012-09-21 at 17:39 +0000, Keller, Jacob E wrote:
> > With the device, it is possible to lookup the driver.
> 
> For PCI devices, yes.  But there is really no guarantee that device
> names (as in dev_name()) are globally unique either.

Ben, I tried to implement your suggestion from September 7th.
Don't you like it, or did I misunderstand you?

Thanks,
Richard
--
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
Ben Hutchings - Sept. 21, 2012, 6:23 p.m.
On Fri, 2012-09-21 at 20:11 +0200, Richard Cochran wrote:
> On Fri, Sep 21, 2012 at 06:40:58PM +0100, Ben Hutchings wrote:
> > On Fri, 2012-09-21 at 17:39 +0000, Keller, Jacob E wrote:
> > > With the device, it is possible to lookup the driver.
> > 
> > For PCI devices, yes.  But there is really no guarantee that device
> > names (as in dev_name()) are globally unique either.
> 
> Ben, I tried to implement your suggestion from September 7th.
> Don't you like it, or did I misunderstand you?

For programmatic purposes, setting the parent is the really useful
thing.

I think what I'm still missing from you is some explanation of what the
'clock name' is meant to be used as - a type name, a unique identifier,
a 'friendly name' for listing clocks in a user interface?

Ben.
Richard Cochran - Sept. 21, 2012, 6:40 p.m.
On Fri, Sep 21, 2012 at 07:23:03PM +0100, Ben Hutchings wrote:
> 
> I think what I'm still missing from you is some explanation of what the
> 'clock name' is meant to be used as - a type name, a unique identifier,
> a 'friendly name' for listing clocks in a user interface?

The original idea was a type/friendly kind of thing.

Imagine you are the admin of some random box that should run PTP. You
do 'cat /sys/class/ptp/ptp0/clock_name' and see "gianfar clock". Then
you think to yourself, "Excellent, I know that one, it works great,
and I can even get a PPS output."

But if you saw "IXP46X timer" then you would think, "Forget it, this
will never work."

That was the idea.

But before the ethtool thing came along, people started putting MAC
addresses in that string, and it continued even after the ethtool
method appeared. I wanted to correct the MAC abuses, but then I
thought you were concurring with putting some kind of semi-unique
device ID there.

I really don't care too much about the clock_name anyhow, because I
always know my own hardware. I don't mind changing it from type-string
or friendly-name to device ID, if people think that is more useful.

Thanks,
Richard



--
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
Ben Hutchings - Sept. 21, 2012, 7:01 p.m.
On Fri, 2012-09-21 at 20:40 +0200, Richard Cochran wrote:
> On Fri, Sep 21, 2012 at 07:23:03PM +0100, Ben Hutchings wrote:
> > 
> > I think what I'm still missing from you is some explanation of what the
> > 'clock name' is meant to be used as - a type name, a unique identifier,
> > a 'friendly name' for listing clocks in a user interface?
> 
> The original idea was a type/friendly kind of thing.
> 
> Imagine you are the admin of some random box that should run PTP. You
> do 'cat /sys/class/ptp/ptp0/clock_name' and see "gianfar clock". Then
> you think to yourself, "Excellent, I know that one, it works great,
> and I can even get a PPS output."
> 
> But if you saw "IXP46X timer" then you would think, "Forget it, this
> will never work."
> 
> That was the idea.
> 
> But before the ethtool thing came along, people started putting MAC
> addresses in that string, and it continued even after the ethtool
> method appeared. I wanted to correct the MAC abuses, but then I
> thought you were concurring with putting some kind of semi-unique
> device ID there.

I was confused about whether it was actually meant to be unique.

The ethtool command is useful but setting the parent device may be even
more useful, e.g. you will be able to write udev rules for PHC devices
based on the parent device's identity.

> I really don't care too much about the clock_name anyhow, because I
> always know my own hardware. I don't mind changing it from type-string
> or friendly-name to device ID, if people think that is more useful.

I don't mind either, just so long as the rule is either (1) implemented
in the PTP core code or (2) made very clear to driver writers.

The lack of a parent for the IXP device makes (1) hard to do well.

Ben.
Richard Cochran - Sept. 22, 2012, 5:43 a.m.
On Fri, Sep 21, 2012 at 08:01:46PM +0100, Ben Hutchings wrote:
> 
> The ethtool command is useful but setting the parent device may be even
> more useful, e.g. you will be able to write udev rules for PHC devices
> based on the parent device's identity.

Thinking about this a bit more, it makes no sense to put the parent
device name into clock_name, because that information is redundant.

  # ls -l /sys/class/ptp/ptp0/
  -r--r--r--    1 root root 8192 Jan  1 00:00 clock_name
  -r--r--r--    1 root root 8192 Jan  1 00:00 dev
  lrwxrwxrwx    1 root root    0 Jan  1 00:00 device -> ../../../fec-1:01

I agree that the parent device is useful, and I will add it. However,
I will leave the clock_name as it is, adding a bit more prose to the
ABI description.

The one case where clock_name becomes really important is when you
have a PHY clock, like in the above example, since it provides a way
to see that the clock is *not* related to the MAC.

Thanks,
Richard
--
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
Richard Cochran - Sept. 22, 2012, 5:46 a.m.
On Sat, Sep 22, 2012 at 07:43:46AM +0200, Richard Cochran wrote:
> 
> The one case where clock_name becomes really important is when you
> have a PHY clock, like in the above example, since it provides a way
> to see that the clock is *not* related to the MAC.

I mean that a PHY clock is not a part of the MAC. Of course it is
related to the MAC by virtue of being connected to it via a MDIO bus.

Thanks,
Richard

--
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
Keller, Jacob E - Sept. 24, 2012, 10:57 p.m.
> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@gmail.com]
> Sent: Friday, September 21, 2012 10:44 PM
> To: Ben Hutchings
> Cc: Keller, Jacob E; netdev@vger.kernel.org; David Miller; Kirsher,
> Jeffrey T; John Stultz; Vick, Matthew
> Subject: Re: [PATCH net-next 3/3] ptp: derive the device name from the
> parent device
> 
> On Fri, Sep 21, 2012 at 08:01:46PM +0100, Ben Hutchings wrote:
> >
> > The ethtool command is useful but setting the parent device may be
> > even more useful, e.g. you will be able to write udev rules for PHC
> > devices based on the parent device's identity.
> 
> Thinking about this a bit more, it makes no sense to put the parent device
> name into clock_name, because that information is redundant.
> 

Agreed.

>   # ls -l /sys/class/ptp/ptp0/
>   -r--r--r--    1 root root 8192 Jan  1 00:00 clock_name
>   -r--r--r--    1 root root 8192 Jan  1 00:00 dev
>   lrwxrwxrwx    1 root root    0 Jan  1 00:00 device -> ../../../fec-1:01
> 
> I agree that the parent device is useful, and I will add it. However, I
> will leave the clock_name as it is, adding a bit more prose to the ABI
> description.
> 
> The one case where clock_name becomes really important is when you have a
> PHY clock, like in the above example, since it provides a way to see that
> the clock is *not* related to the MAC.
> 

Agreed. Now that I understand your intent, and with the new parent and ethtool options, I will go ahead and change the ixgbe clock name. This makes more sense to me once I understood the intent.

- Jake

> Thanks,
> Richard
--
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

Patch

diff --git a/drivers/net/ethernet/freescale/gianfar_ptp.c b/drivers/net/ethernet/freescale/gianfar_ptp.c
index c08e5d4..a0eaafd 100644
--- a/drivers/net/ethernet/freescale/gianfar_ptp.c
+++ b/drivers/net/ethernet/freescale/gianfar_ptp.c
@@ -408,7 +408,6 @@  static int ptp_gianfar_enable(struct ptp_clock_info *ptp,
 
 static struct ptp_clock_info ptp_gianfar_caps = {
 	.owner		= THIS_MODULE,
-	.name		= "gianfar clock",
 	.max_adj	= 512000,
 	.n_alarm	= N_ALARM,
 	.n_ext_ts	= N_EXT_TS,
@@ -510,7 +509,7 @@  static int gianfar_ptp_probe(struct platform_device *dev)
 
 	spin_unlock_irqrestore(&etsects->lock, flags);
 
-	etsects->clock = ptp_clock_register(&etsects->caps);
+	etsects->clock = ptp_clock_register(&etsects->caps, &dev->dev);
 	if (IS_ERR(etsects->clock)) {
 		err = PTR_ERR(etsects->clock);
 		goto no_clock;
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index e13ba1d..546e9e2 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -665,11 +665,9 @@  int igb_ptp_hwtstamp_ioctl(struct net_device *netdev,
 void igb_ptp_init(struct igb_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
-	struct net_device *netdev = adapter->netdev;
 
 	switch (hw->mac.type) {
 	case e1000_82576:
-		snprintf(adapter->ptp_caps.name, 16, "%pm", netdev->dev_addr);
 		adapter->ptp_caps.owner = THIS_MODULE;
 		adapter->ptp_caps.max_adj = 1000000000;
 		adapter->ptp_caps.n_ext_ts = 0;
@@ -688,7 +686,6 @@  void igb_ptp_init(struct igb_adapter *adapter)
 		break;
 	case e1000_82580:
 	case e1000_i350:
-		snprintf(adapter->ptp_caps.name, 16, "%pm", netdev->dev_addr);
 		adapter->ptp_caps.owner = THIS_MODULE;
 		adapter->ptp_caps.max_adj = 62499999;
 		adapter->ptp_caps.n_ext_ts = 0;
@@ -707,7 +704,6 @@  void igb_ptp_init(struct igb_adapter *adapter)
 		break;
 	case e1000_i210:
 	case e1000_i211:
-		snprintf(adapter->ptp_caps.name, 16, "%pm", netdev->dev_addr);
 		adapter->ptp_caps.owner = THIS_MODULE;
 		adapter->ptp_caps.max_adj = 62499999;
 		adapter->ptp_caps.n_ext_ts = 0;
@@ -752,7 +748,8 @@  void igb_ptp_init(struct igb_adapter *adapter)
 		wr32(E1000_IMS, E1000_IMS_TS);
 	}
 
-	adapter->ptp_clock = ptp_clock_register(&adapter->ptp_caps);
+	adapter->ptp_clock = ptp_clock_register(&adapter->ptp_caps,
+						&adapter->pdev->dev);
 	if (IS_ERR(adapter->ptp_clock)) {
 		adapter->ptp_clock = NULL;
 		dev_err(&adapter->pdev->dev, "ptp_clock_register failed\n");
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
index 3456d56..6219c91 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
@@ -917,7 +917,6 @@  void ixgbe_ptp_init(struct ixgbe_adapter *adapter)
 
 	switch (adapter->hw.mac.type) {
 	case ixgbe_mac_X540:
-		snprintf(adapter->ptp_caps.name, 16, "%pm", netdev->dev_addr);
 		adapter->ptp_caps.owner = THIS_MODULE;
 		adapter->ptp_caps.max_adj = 250000000;
 		adapter->ptp_caps.n_alarm = 0;
@@ -931,7 +930,6 @@  void ixgbe_ptp_init(struct ixgbe_adapter *adapter)
 		adapter->ptp_caps.enable = ixgbe_ptp_enable;
 		break;
 	case ixgbe_mac_82599EB:
-		snprintf(adapter->ptp_caps.name, 16, "%pm", netdev->dev_addr);
 		adapter->ptp_caps.owner = THIS_MODULE;
 		adapter->ptp_caps.max_adj = 250000000;
 		adapter->ptp_caps.n_alarm = 0;
@@ -960,7 +958,8 @@  void ixgbe_ptp_init(struct ixgbe_adapter *adapter)
 	/* (Re)start the overflow check */
 	adapter->flags2 |= IXGBE_FLAG2_OVERFLOW_CHECK_ENABLED;
 
-	adapter->ptp_clock = ptp_clock_register(&adapter->ptp_caps);
+	adapter->ptp_clock = ptp_clock_register(&adapter->ptp_caps,
+						&adapter->pdev->dev);
 	if (IS_ERR(adapter->ptp_clock)) {
 		adapter->ptp_clock = NULL;
 		e_dev_err("ptp_clock_register failed\n");
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index b0da022..12a4c4c 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -871,7 +871,6 @@  static void dp83640_clock_init(struct dp83640_clock *clock, struct mii_bus *bus)
 	mutex_init(&clock->clock_lock);
 	INIT_LIST_HEAD(&clock->phylist);
 	clock->caps.owner = THIS_MODULE;
-	sprintf(clock->caps.name, "dp83640 timer");
 	clock->caps.max_adj	= 1953124;
 	clock->caps.n_alarm	= 0;
 	clock->caps.n_ext_ts	= N_EXT_TS;
@@ -980,7 +979,7 @@  static int dp83640_probe(struct phy_device *phydev)
 
 	if (choose_this_phy(clock, phydev)) {
 		clock->chosen = dp83640;
-		clock->ptp_clock = ptp_clock_register(&clock->caps);
+		clock->ptp_clock = ptp_clock_register(&clock->caps, &phydev->dev);
 		if (IS_ERR(clock->ptp_clock)) {
 			err = PTR_ERR(clock->ptp_clock);
 			goto no_register;
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 6e47450..60fa259 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -186,7 +186,8 @@  static void delete_ptp_clock(struct posix_clock *pc)
 
 /* public interface */
 
-struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info)
+struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
+				     struct device *parent)
 {
 	struct ptp_clock *ptp;
 	int err = 0, index, major = MAJOR(ptp_devt);
@@ -219,11 +220,13 @@  struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info)
 	init_waitqueue_head(&ptp->tsev_wq);
 
 	/* Create a new device in our class. */
-	ptp->dev = device_create(ptp_class, NULL, ptp->devid, ptp,
+	ptp->dev = device_create(ptp_class, parent, ptp->devid, ptp,
 				 "ptp%d", ptp->index);
 	if (IS_ERR(ptp->dev))
 		goto no_device;
 
+	ptp->name = parent ? dev_name(parent) : dev_name(ptp->dev);
+
 	dev_set_drvdata(ptp->dev, ptp);
 
 	err = ptp_populate_sysfs(ptp);
diff --git a/drivers/ptp/ptp_ixp46x.c b/drivers/ptp/ptp_ixp46x.c
index e03c406..c0939b1 100644
--- a/drivers/ptp/ptp_ixp46x.c
+++ b/drivers/ptp/ptp_ixp46x.c
@@ -241,7 +241,6 @@  static int ptp_ixp_enable(struct ptp_clock_info *ptp,
 
 static struct ptp_clock_info ptp_ixp_caps = {
 	.owner		= THIS_MODULE,
-	.name		= "IXP46X timer",
 	.max_adj	= 66666655,
 	.n_ext_ts	= N_EXT_TS,
 	.pps		= 0,
@@ -298,7 +297,7 @@  static int __init ptp_ixp_init(void)
 
 	ixp_clock.caps = ptp_ixp_caps;
 
-	ixp_clock.ptp_clock = ptp_clock_register(&ixp_clock.caps);
+	ixp_clock.ptp_clock = ptp_clock_register(&ixp_clock.caps, NULL);
 
 	if (IS_ERR(ixp_clock.ptp_clock))
 		return PTR_ERR(ixp_clock.ptp_clock);
diff --git a/drivers/ptp/ptp_pch.c b/drivers/ptp/ptp_pch.c
index 3a9c17e..0ae0c27 100644
--- a/drivers/ptp/ptp_pch.c
+++ b/drivers/ptp/ptp_pch.c
@@ -511,7 +511,6 @@  static int ptp_pch_enable(struct ptp_clock_info *ptp,
 
 static struct ptp_clock_info ptp_pch_caps = {
 	.owner		= THIS_MODULE,
-	.name		= "PCH timer",
 	.max_adj	= 50000000,
 	.n_ext_ts	= N_EXT_TS,
 	.pps		= 0,
@@ -627,7 +626,7 @@  pch_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	}
 
 	chip->caps = ptp_pch_caps;
-	chip->ptp_clock = ptp_clock_register(&chip->caps);
+	chip->ptp_clock = ptp_clock_register(&chip->caps, &pdev->dev);
 
 	if (IS_ERR(chip->ptp_clock))
 		return PTR_ERR(chip->ptp_clock);
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index 69d3207..a3bf959 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -42,6 +42,7 @@  struct ptp_clock {
 	struct posix_clock clock;
 	struct device *dev;
 	struct ptp_clock_info *info;
+	const char *name;
 	dev_t devid;
 	int index; /* index into clocks.map */
 	struct pps_device *pps_source;
diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index 2f93926..f3c4519 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -25,7 +25,7 @@  static ssize_t clock_name_show(struct device *dev,
 			       struct device_attribute *attr, char *page)
 {
 	struct ptp_clock *ptp = dev_get_drvdata(dev);
-	return snprintf(page, PAGE_SIZE-1, "%s\n", ptp->info->name);
+	return snprintf(page, PAGE_SIZE-1, "%s\n", ptp->name);
 }
 
 #define PTP_SHOW_INT(name)						\
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 945704c..e673de0 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -21,6 +21,7 @@ 
 #ifndef _PTP_CLOCK_KERNEL_H_
 #define _PTP_CLOCK_KERNEL_H_
 
+#include <linux/device.h>
 #include <linux/ptp_clock.h>
 
 
@@ -73,7 +74,6 @@  struct ptp_clock_request {
 
 struct ptp_clock_info {
 	struct module *owner;
-	char name[16];
 	s32 max_adj;
 	int n_alarm;
 	int n_ext_ts;
@@ -92,10 +92,12 @@  struct ptp_clock;
 /**
  * ptp_clock_register() - register a PTP hardware clock driver
  *
- * @info:  Structure describing the new clock.
+ * @info:   Structure describing the new clock.
+ * @parent: Pointer to the parent device of the new clock.
  */
 
-extern struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info);
+extern struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
+					    struct device *parent);
 
 /**
  * ptp_clock_unregister() - unregister a PTP hardware clock driver