[PATCH V2 net-next 0/4] Two new PTP Hardware Clock features

[PATCH V2 net-next 0/4] Two new PTP Hardware Clock features

[Richard Cochran]

This patch series adds two new features to the PHC code.

The first two patches let a user program find out the previously
dialed frequency adjustment. This is primarily useful when restarting
a PTP service, since without this information, the presumably correct
adjustment will bias the new frequency estimation.

The third patch links the phc class device to its parent device within
the driver model and sysfs.

The fourth patch adds a bit more documentation of the sysfs clock_name
attribute. This should help clarify the naming scheme.

Thanks,
Richard

Richard Cochran (4):
  ptp: remember the adjusted frequency
  ptp: provide the clock's adjusted frequency
  ptp: link the phc device to its parent device
  ptp: clarify the clock_name sysfs attribute

 Documentation/ABI/testing/sysfs-ptp          |    5 ++++-
 drivers/net/ethernet/freescale/gianfar_ptp.c |    2 +-
 drivers/net/ethernet/intel/igb/igb_ptp.c     |    3 ++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c |    3 ++-
 drivers/net/ethernet/sfc/ptp.c               |    3 ++-
 drivers/net/phy/dp83640.c                    |    2 +-
 drivers/ptp/ptp_clock.c                      |   11 +++++++++--
 drivers/ptp/ptp_ixp46x.c                     |    2 +-
 drivers/ptp/ptp_pch.c                        |    2 +-
 drivers/ptp/ptp_private.h                    |    1 +
 include/linux/ptp_clock_kernel.h             |    7 +++++--
 11 files changed, 29 insertions(+), 12 deletions(-)

-- 
1.7.2.5

[PATCH V2 net-next 1/4] ptp: remember the adjusted frequency

[Richard Cochran]

This patch adds a field to the representation of a PTP hardware clock in
order to remember the frequency adjustment value dialed by the user.

Adding this field will let us answer queries in the manner of adjtimex
in a follow on patch.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/ptp/ptp_clock.c   |    1 +
 drivers/ptp/ptp_private.h |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 966875d..67e628e 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -147,6 +147,7 @@ static int ptp_clock_adjtime(struct posix_clock *pc, struct timex *tx)
 	} else if (tx->modes & ADJ_FREQUENCY) {
 
 		err = ops->adjfreq(ops, scaled_ppm_to_ppb(tx->freq));
+		ptp->dialed_frequency = tx->freq;
 	}
 
 	return err;
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index 4d5b508..69d3207 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -45,6 +45,7 @@ struct ptp_clock {
 	dev_t devid;
 	int index; /* index into clocks.map */
 	struct pps_device *pps_source;
+	long dialed_frequency; /* remembers the frequency adjustment */
 	struct timestamp_event_queue tsevq; /* simple fifo for time stamps */
 	struct mutex tsevq_mux; /* one process at a time reading the fifo */
 	wait_queue_head_t tsev_wq;
-- 
1.7.2.5

[PATCH V2 net-next 2/4] ptp: provide the clock's adjusted frequency

[Richard Cochran]

If the timex.mode field indicates a query, then we provide the value of
the current frequency adjustment.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/ptp/ptp_clock.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 67e628e..6f7009a 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -148,6 +148,11 @@ static int ptp_clock_adjtime(struct posix_clock *pc, struct timex *tx)
 
 		err = ops->adjfreq(ops, scaled_ppm_to_ppb(tx->freq));
 		ptp->dialed_frequency = tx->freq;
+
+	} else if (tx->modes == 0) {
+
+		tx->freq = ptp->dialed_frequency;
+		err = 0;
 	}
 
 	return err;
-- 
1.7.2.5

[PATCH V2 net-next 3/4] ptp: link the phc device to its parent device

[Richard Cochran]

PTP Hardware Clock devices appear as class devices in sysfs. This patch
changes the registration API to use the parent device, clarifying the
clock's relationship to the underlying device.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/freescale/gianfar_ptp.c |    2 +-
 drivers/net/ethernet/intel/igb/igb_ptp.c     |    3 ++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c |    3 ++-
 drivers/net/ethernet/sfc/ptp.c               |    3 ++-
 drivers/net/phy/dp83640.c                    |    2 +-
 drivers/ptp/ptp_clock.c                      |    5 +++--
 drivers/ptp/ptp_ixp46x.c                     |    2 +-
 drivers/ptp/ptp_pch.c                        |    2 +-
 include/linux/ptp_clock_kernel.h             |    7 +++++--
 9 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar_ptp.c b/drivers/net/ethernet/freescale/gianfar_ptp.c
index c08e5d4..18762a3 100644
--- a/drivers/net/ethernet/freescale/gianfar_ptp.c
+++ b/drivers/net/ethernet/freescale/gianfar_ptp.c
@@ -510,7 +510,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..ee21445 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -752,7 +752,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..39881cb 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
@@ -960,7 +960,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/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 2b07a4e..3ed5d13 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -931,7 +931,8 @@ static int efx_ptp_probe_channel(struct efx_channel *channel)
 	ptp->phc_clock_info.settime = efx_phc_settime;
 	ptp->phc_clock_info.enable = efx_phc_enable;
 
-	ptp->phc_clock = ptp_clock_register(&ptp->phc_clock_info);
+	ptp->phc_clock = ptp_clock_register(&ptp->phc_clock_info,
+					    &channel->napi_dev->dev);
 	if (!ptp->phc_clock)
 		goto fail3;
 
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index b0da022..24e05c4 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -980,7 +980,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 6f7009a..b15a376 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,7 +220,7 @@ 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;
diff --git a/drivers/ptp/ptp_ixp46x.c b/drivers/ptp/ptp_ixp46x.c
index e03c406..d49b851 100644
--- a/drivers/ptp/ptp_ixp46x.c
+++ b/drivers/ptp/ptp_ixp46x.c
@@ -298,7 +298,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..e624e4d 100644
--- a/drivers/ptp/ptp_pch.c
+++ b/drivers/ptp/ptp_pch.c
@@ -627,7 +627,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/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index a644b29..56c71b2 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/pps_kernel.h>
 #include <linux/ptp_clock.h>
 
@@ -93,10 +94,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
-- 
1.7.2.5

[PATCH V2 net-next 4/4] ptp: clarify the clock_name sysfs attribute

[Richard Cochran]

There has been some confusion among PHC driver authors about the
intended purpose of the clock_name attribute. This patch expands the
documation in order to clarify how the clock_name field should be
understood.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 Documentation/ABI/testing/sysfs-ptp |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-ptp b/Documentation/ABI/testing/sysfs-ptp
index d40d2b5..c906488 100644
--- a/Documentation/ABI/testing/sysfs-ptp
+++ b/Documentation/ABI/testing/sysfs-ptp
@@ -19,7 +19,10 @@ Date:		September 2010
 Contact:	Richard Cochran <richardcochran@gmail.com>
 Description:
 		This file contains the name of the PTP hardware clock
-		as a human readable string.
+		as a human readable string. The purpose of this
+		attribute is to provide the user with a "friendly
+		name" and to help distinguish PHY based devices from
+		MAC based ones.
 
 What:		/sys/class/ptp/ptpN/max_adjustment
 Date:		September 2010
-- 
1.7.2.5

Re: [PATCH V2 net-next 0/4] Two new PTP Hardware Clock features

[Richard Cochran]

On Sat, Sep 22, 2012 at 09:42:52AM +0200, Richard Cochran wrote:
> This patch series adds two new features to the PHC code.

Forgot to say:

V2 preserves the clock_name attribute as it was meant to be, instead
of making any changes to it.

Thanks,
Richard

Re: [PATCH V2 net-next 0/4] Two new PTP Hardware Clock features

[Richard Cochran]

On Sat, Sep 22, 2012 at 09:45:53AM +0200, Richard Cochran wrote:
> On Sat, Sep 22, 2012 at 09:42:52AM +0200, Richard Cochran wrote:
> > This patch series adds two new features to the PHC code.
> 
> Forgot to say:
> 
> V2 preserves the clock_name attribute as it was meant to be, instead
> of making any changes to it.

... and covers the registration API change in the brand new solarflare
phc device, which was overlooked in V1.

Thanks,
Richard

Re: [PATCH V2 net-next 3/4] ptp: link the phc device to its parent device

[Ben Hutchings]

On Sat, 2012-09-22 at 09:42 +0200, Richard Cochran wrote:
> PTP Hardware Clock devices appear as class devices in sysfs. This patch
> changes the registration API to use the parent device, clarifying the
> clock's relationship to the underlying device.
[...]
> --- a/drivers/net/ethernet/sfc/ptp.c
> +++ b/drivers/net/ethernet/sfc/ptp.c
> @@ -931,7 +931,8 @@ static int efx_ptp_probe_channel(struct efx_channel *channel)
>  	ptp->phc_clock_info.settime = efx_phc_settime;
>  	ptp->phc_clock_info.enable = efx_phc_enable;
>  
> -	ptp->phc_clock = ptp_clock_register(&ptp->phc_clock_info);
> +	ptp->phc_clock = ptp_clock_register(&ptp->phc_clock_info,
> +					    &channel->napi_dev->dev);
[...]

channel->napi_dev is the net device, not the PCI device.  The parent
device should be &efx->pci_dev->dev.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH V2 net-next 4/4] ptp: clarify the clock_name sysfs attribute

[Ben Hutchings]

On Sat, 2012-09-22 at 09:42 +0200, Richard Cochran wrote:
> There has been some confusion among PHC driver authors about the
> intended purpose of the clock_name attribute. This patch expands the
> documation in order to clarify how the clock_name field should be
> understood.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
> ---
>  Documentation/ABI/testing/sysfs-ptp |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-ptp b/Documentation/ABI/testing/sysfs-ptp
> index d40d2b5..c906488 100644
> --- a/Documentation/ABI/testing/sysfs-ptp
> +++ b/Documentation/ABI/testing/sysfs-ptp
> @@ -19,7 +19,10 @@ Date:		September 2010
>  Contact:	Richard Cochran <richardcochran@gmail.com>
>  Description:
>  		This file contains the name of the PTP hardware clock
> -		as a human readable string.
> +		as a human readable string. The purpose of this
> +		attribute is to provide the user with a "friendly
> +		name" and to help distinguish PHY based devices from
> +		MAC based ones.

Might also be worth saying that it is not required to be unique.  And
this explanation should also go in the kernel-doc in ptp_clock_kernel.h,
which is what driver writers are most likely to read.

Ben.

>  What:		/sys/class/ptp/ptpN/max_adjustment
>  Date:		September 2010

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH V2 net-next 4/4] ptp: clarify the clock_name sysfs attribute

[Jeff Kirsher]

On 09/22/2012 08:44 AM, Ben Hutchings wrote:
> On Sat, 2012-09-22 at 09:42 +0200, Richard Cochran wrote:
>> There has been some confusion among PHC driver authors about the
>> intended purpose of the clock_name attribute. This patch expands the
>> documation in order to clarify how the clock_name field should be
>> understood.
>>
>> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
>> ---
>>   Documentation/ABI/testing/sysfs-ptp |    5 ++++-
>>   1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-ptp b/Documentation/ABI/testing/sysfs-ptp
>> index d40d2b5..c906488 100644
>> --- a/Documentation/ABI/testing/sysfs-ptp
>> +++ b/Documentation/ABI/testing/sysfs-ptp
>> @@ -19,7 +19,10 @@ Date:		September 2010
>>   Contact:	Richard Cochran <richardcochran@gmail.com>
>>   Description:
>>   		This file contains the name of the PTP hardware clock
>> -		as a human readable string.
>> +		as a human readable string. The purpose of this
>> +		attribute is to provide the user with a "friendly
>> +		name" and to help distinguish PHY based devices from
>> +		MAC based ones.
> Might also be worth saying that it is not required to be unique.  And
> this explanation should also go in the kernel-doc in ptp_clock_kernel.h,
> which is what driver writers are most likely to read.
>
> Ben
FWIW, I agree with Ben.