[PATCH v3 0/3] dp83640: Get pin and master/slave configuration from DT

[PATCH v3 0/3] dp83640: Get pin and master/slave configuration from DT

[Stefan Sørensen]

This patch series add DT configuration to the DP83640 PHY driver and makes
the configuration of periodic output pins dynamic.

Changes since v2:
 - Add patch to properly configure perout triggers 0+1
 - Keep extts and perout numbers separate
 - Shorten pr_err lines

Changes since v1:
 - Add bindings documentation
 - Keep module parameters
 - Rename gpio->pin
 - Split patch into DT part and dynamic periodic output support

Stefan Sørensen (3):
  net:phy:dp83640: Program pulsewidth2 values of perout triggers 0 and 1
  net:phy:dp83640: Support a configurable number of periodic outputs
  net:phy:dp83640: Get pin and master/slave configuration from DT

 Documentation/devicetree/bindings/net/dp83640.txt |  29 ++++
 drivers/net/phy/dp83640.c                         | 199 +++++++++++++++-------
 2 files changed, 170 insertions(+), 58 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/dp83640.txt

-- 
1.8.5.3

[PATCH v3 1/3] net:phy:dp83640: Program pulsewidth2 values of perout triggers 0 and 1

[Stefan Sørensen]

Periodic output triggers 0 and 1 of the dp83640 has a programmable
duty-cycle which is controlled by the Pulsewidth2 field of the trigger
data register.  This field is not documented in the datasheet, but it
is described in the "PHYTER Software Development Guide" section
3.1.4.1. Failing to set the field can result in unstable/no trigger
output.

This patch add programming of the Pulsewidth2 field, setting it to the
same value as the Pulsewidth field for a 50% duty cycle.

Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com>
---
 drivers/net/phy/dp83640.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 5ff221d..a370814 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -312,6 +312,11 @@ static void periodic_output(struct dp83640_clock *clock,
 	ext_write(0, phydev, PAGE4, PTP_TDR, sec >> 16);       /* sec[31:16] */
 	ext_write(0, phydev, PAGE4, PTP_TDR, period & 0xffff); /* ns[15:0] */
 	ext_write(0, phydev, PAGE4, PTP_TDR, period >> 16);    /* ns[31:16] */
+	/* Triggers 0 and 1 has programmable pulsewidth2 */
+	if (trigger == 0 || trigger == 1) {
+		ext_write(0, phydev, PAGE4, PTP_TDR, period & 0xffff);
+		ext_write(0, phydev, PAGE4, PTP_TDR, period >> 16);
+	}
 
 	/*enable trigger*/
 	val &= ~TRIG_LOAD;
-- 
1.8.5.3

[PATCH v3 2/3] net:phy:dp83640: Support a configurable number of periodic outputs

[Stefan Sørensen]

The driver is currently limited to a single periodic output. This patch makes
the number of peridodic output dynamic by dropping the gpio_tab module
parameter and adding calibrate_pin, perout_pins, and extts_pins parameters.

Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com>
---
 drivers/net/phy/dp83640.c | 73 ++++++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 42 deletions(-)

diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index a370814..28a6e1d 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -38,15 +38,12 @@
 #define LAYER4		0x02
 #define LAYER2		0x01
 #define MAX_RXTS	64
-#define N_EXT_TS	6
+#define N_EXT		8
 #define PSF_PTPVER	2
 #define PSF_EVNT	0x4000
 #define PSF_RX		0x2000
 #define PSF_TX		0x1000
-#define EXT_EVENT	1
-#define CAL_EVENT	7
 #define CAL_TRIGGER	7
-#define PER_TRIGGER	6
 
 #define MII_DP83640_MICR 0x11
 #define MII_DP83640_MISR 0x12
@@ -146,32 +143,24 @@ struct dp83640_clock {
 	struct ptp_clock *ptp_clock;
 };
 
-/* globals */
-
-enum {
-	CALIBRATE_GPIO,
-	PEROUT_GPIO,
-	EXTTS0_GPIO,
-	EXTTS1_GPIO,
-	EXTTS2_GPIO,
-	EXTTS3_GPIO,
-	EXTTS4_GPIO,
-	EXTTS5_GPIO,
-	GPIO_TABLE_SIZE
-};
 
 static int chosen_phy = -1;
-static ushort gpio_tab[GPIO_TABLE_SIZE] = {
-	1, 2, 3, 4, 8, 9, 10, 11
-};
+static int calibrate_pin = 1;
+static int perout_pins[N_EXT] = {2};
+static int n_per_out = 1;
+static int extts_pins[N_EXT] = {3, 4, 8, 9, 10, 11};
+static int n_ext_ts = 6;
 
 module_param(chosen_phy, int, 0444);
-module_param_array(gpio_tab, ushort, NULL, 0444);
+module_param(calibrate_pin, int, 0444);
+module_param_array(perout_pins, int, &n_per_out, 0444);
+module_param_array(extts_pins, int, &n_ext_ts, 0444);
 
 MODULE_PARM_DESC(chosen_phy, \
 	"The address of the PHY to use for the ancillary clock features");
-MODULE_PARM_DESC(gpio_tab, \
-	"Which GPIO line to use for which purpose: cal,perout,extts1,...,extts6");
+MODULE_PARM_DESC(calibrate_pin, "Which pin to use for calibration");
+MODULE_PARM_DESC(perout_pins, "Which pins to use for periodic output");
+MODULE_PARM_DESC(extts_pins, "Which pins to use for external timestamping");
 
 /* a list of clocks and a mutex to protect it */
 static LIST_HEAD(phyter_clocks);
@@ -267,15 +256,15 @@ static u64 phy2txts(struct phy_txts *p)
 }
 
 static void periodic_output(struct dp83640_clock *clock,
-			    struct ptp_clock_request *clkreq, bool on)
+			    struct ptp_clock_request *clkreq, int trigger,
+			    bool on)
 {
 	struct dp83640_private *dp83640 = clock->chosen;
 	struct phy_device *phydev = dp83640->phydev;
 	u32 sec, nsec, period;
-	u16 gpio, ptp_trig, trigger, val;
+	u16 gpio, ptp_trig, val;
 
-	gpio = on ? gpio_tab[PEROUT_GPIO] : 0;
-	trigger = PER_TRIGGER;
+	gpio = on ? perout_pins[trigger] : 0;
 
 	ptp_trig = TRIG_WR |
 		(trigger & TRIG_CSEL_MASK) << TRIG_CSEL_SHIFT |
@@ -446,12 +435,12 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp,
 	switch (rq->type) {
 	case PTP_CLK_REQ_EXTTS:
 		index = rq->extts.index;
-		if (index < 0 || index >= N_EXT_TS)
+		if (index < 0 || index >= n_ext_ts)
 			return -EINVAL;
-		event_num = EXT_EVENT + index;
+		event_num = index;
 		evnt = EVNT_WR | (event_num & EVNT_SEL_MASK) << EVNT_SEL_SHIFT;
 		if (on) {
-			gpio_num = gpio_tab[EXTTS0_GPIO + index];
+			gpio_num = extts_pins[index];
 			evnt |= (gpio_num & EVNT_GPIO_MASK) << EVNT_GPIO_SHIFT;
 			if (rq->extts.flags & PTP_FALLING_EDGE)
 				evnt |= EVNT_FALL;
@@ -462,9 +451,10 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp,
 		return 0;
 
 	case PTP_CLK_REQ_PEROUT:
-		if (rq->perout.index != 0)
+		index = rq->perout.index;
+		if (index < 0 || index >= n_per_out)
 			return -EINVAL;
-		periodic_output(clock, rq, on);
+		periodic_output(clock, rq, index, on);
 		return 0;
 
 	default:
@@ -557,10 +547,9 @@ static void recalibrate(struct dp83640_clock *clock)
 	struct list_head *this;
 	struct dp83640_private *tmp;
 	struct phy_device *master = clock->chosen->phydev;
-	u16 cal_gpio, cfg0, evnt, ptp_trig, trigger, val;
+	u16 cfg0, evnt, ptp_trig, trigger, val;
 
 	trigger = CAL_TRIGGER;
-	cal_gpio = gpio_tab[CALIBRATE_GPIO];
 
 	mutex_lock(&clock->extreg_lock);
 
@@ -583,8 +572,8 @@ static void recalibrate(struct dp83640_clock *clock)
 	 * enable an event timestamp
 	 */
 	evnt = EVNT_WR | EVNT_RISE | EVNT_SINGLE;
-	evnt |= (CAL_EVENT & EVNT_SEL_MASK) << EVNT_SEL_SHIFT;
-	evnt |= (cal_gpio & EVNT_GPIO_MASK) << EVNT_GPIO_SHIFT;
+	evnt |= (trigger & EVNT_SEL_MASK) << EVNT_SEL_SHIFT;
+	evnt |= (calibrate_pin & EVNT_GPIO_MASK) << EVNT_GPIO_SHIFT;
 
 	list_for_each(this, &clock->phylist) {
 		tmp = list_entry(this, struct dp83640_private, list);
@@ -597,7 +586,7 @@ static void recalibrate(struct dp83640_clock *clock)
 	 */
 	ptp_trig = TRIG_WR | TRIG_IF_LATE | TRIG_PULSE;
 	ptp_trig |= (trigger  & TRIG_CSEL_MASK) << TRIG_CSEL_SHIFT;
-	ptp_trig |= (cal_gpio & TRIG_GPIO_MASK) << TRIG_GPIO_SHIFT;
+	ptp_trig |= (calibrate_pin & TRIG_GPIO_MASK) << TRIG_GPIO_SHIFT;
 	ext_write(0, master, PAGE5, PTP_TRIG, ptp_trig);
 
 	/* load trigger */
@@ -661,7 +650,7 @@ static void recalibrate(struct dp83640_clock *clock)
 
 static inline u16 exts_chan_to_edata(int ch)
 {
-	return 1 << ((ch + EXT_EVENT) * 2);
+	return 1 << ((ch) * 2);
 }
 
 static int decode_evnt(struct dp83640_private *dp83640,
@@ -695,14 +684,14 @@ static int decode_evnt(struct dp83640_private *dp83640,
 		parsed = words + 2;
 	} else {
 		parsed = words + 1;
-		i = ((ests >> EVNT_NUM_SHIFT) & EVNT_NUM_MASK) - EXT_EVENT;
+		i = ((ests >> EVNT_NUM_SHIFT) & EVNT_NUM_MASK);
 		ext_status = exts_chan_to_edata(i);
 	}
 
 	event.type = PTP_CLOCK_EXTTS;
 	event.timestamp = phy2txts(&dp83640->edata);
 
-	for (i = 0; i < N_EXT_TS; i++) {
+	for (i = 0; i < n_ext_ts; i++) {
 		if (ext_status & exts_chan_to_edata(i)) {
 			event.index = i;
 			ptp_clock_event(dp83640->clock->ptp_clock, &event);
@@ -908,8 +897,8 @@ static void dp83640_clock_init(struct dp83640_clock *clock, struct mii_bus *bus)
 	sprintf(clock->caps.name, "dp83640 timer");
 	clock->caps.max_adj	= 1953124;
 	clock->caps.n_alarm	= 0;
-	clock->caps.n_ext_ts	= N_EXT_TS;
-	clock->caps.n_per_out	= 1;
+	clock->caps.n_ext_ts	= n_ext_ts;
+	clock->caps.n_per_out	= n_per_out;
 	clock->caps.pps		= 0;
 	clock->caps.adjfreq	= ptp_dp83640_adjfreq;
 	clock->caps.adjtime	= ptp_dp83640_adjtime;
-- 
1.8.5.3

[PATCH v3 3/3] net:phy:dp83640: Get pin and master/slave configuration from DT

[Stefan Sørensen]

This patch adds configuration of the periodic output and external timestamp
pins available on the dp83640 family of PHYs. It also configures the
master/slave relationship in a group of PHYs on the same MDIO bus and the pins
used for clock calibration in the group.

The configuration is retrieved from DT through the properties
    dp83640,slave
    dp83640,calibrate-pin
    dp83640,perout-pins
    dp83640,extts-pins
The configuration module parameters are retained as fallback for the non-DT
case.

Since the pin configuration is now stored for each clock device, groups of
devices on different mdio busses can now have different pin configurations.

Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com>
---
 Documentation/devicetree/bindings/net/dp83640.txt |  29 +++++
 drivers/net/phy/dp83640.c                         | 139 ++++++++++++++++++----
 2 files changed, 143 insertions(+), 25 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/dp83640.txt

diff --git a/Documentation/devicetree/bindings/net/dp83640.txt b/Documentation/devicetree/bindings/net/dp83640.txt
new file mode 100644
index 0000000..b9a57c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dp83640.txt
@@ -0,0 +1,29 @@
+Required properties for the National DP83640 ethernet phy:
+
+- compatible : Must contain "national,dp83640"
+
+Optional properties:
+
+- dp83640,slave: If present, this phy will be slave to another dp83640
+  on the same mdio bus.
+- dp83640,perout-pins : List of the pin pins used for periodic output
+  triggers.
+- dp83640,extts-pins : List of the pin pins used for external event
+  timestamping.
+- dp83640,calibrate-pin : The pin used for master/slave calibration.
+
+Example:
+
+	ethernet-phy@1 {
+		compatible = "national,dp83640", "ethernet-phy-ieee802.3-c22";
+		reg = <1>;
+		dp83640,perout-pins = <2>;
+		dp83640,extts-pins = <3 4 8 9 10 11>;
+		dp83640,calibrate-pin = <1>;
+	};
+
+	ethernet-phy@2 {
+		compatible = "national,dp83640", "ethernet-phy-ieee802.3-c22";
+		reg = <2>;
+		dp83640,slave;
+	};
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 28a6e1d..077bdc2 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -30,6 +30,7 @@
 #include <linux/phy.h>
 #include <linux/ptp_classify.h>
 #include <linux/ptp_clock_kernel.h>
+#include <linux/of_device.h>
 
 #include "dp83640_reg.h"
 
@@ -120,6 +121,8 @@ struct dp83640_private {
 	/* queues of incoming and outgoing packets */
 	struct sk_buff_head rx_queue;
 	struct sk_buff_head tx_queue;
+	/* is this phyter a slave */
+	bool slave;
 };
 
 struct dp83640_clock {
@@ -141,6 +144,7 @@ struct dp83640_clock {
 	struct list_head phylist;
 	/* reference to our PTP hardware clock */
 	struct ptp_clock *ptp_clock;
+	u32 perout_pins[N_EXT], extts_pins[N_EXT], calibrate_pin;
 };
 
 
@@ -264,7 +268,7 @@ static void periodic_output(struct dp83640_clock *clock,
 	u32 sec, nsec, period;
 	u16 gpio, ptp_trig, val;
 
-	gpio = on ? perout_pins[trigger] : 0;
+	gpio = on ? clock->perout_pins[trigger] : 0;
 
 	ptp_trig = TRIG_WR |
 		(trigger & TRIG_CSEL_MASK) << TRIG_CSEL_SHIFT |
@@ -435,12 +439,12 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp,
 	switch (rq->type) {
 	case PTP_CLK_REQ_EXTTS:
 		index = rq->extts.index;
-		if (index < 0 || index >= n_ext_ts)
+		if (index < 0 || index >= clock->caps.n_ext_ts)
 			return -EINVAL;
 		event_num = index;
 		evnt = EVNT_WR | (event_num & EVNT_SEL_MASK) << EVNT_SEL_SHIFT;
 		if (on) {
-			gpio_num = extts_pins[index];
+			gpio_num = clock->extts_pins[index];
 			evnt |= (gpio_num & EVNT_GPIO_MASK) << EVNT_GPIO_SHIFT;
 			if (rq->extts.flags & PTP_FALLING_EDGE)
 				evnt |= EVNT_FALL;
@@ -452,7 +456,7 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp,
 
 	case PTP_CLK_REQ_PEROUT:
 		index = rq->perout.index;
-		if (index < 0 || index >= n_per_out)
+		if (index < 0 || index >= clock->caps.n_per_out)
 			return -EINVAL;
 		periodic_output(clock, rq, index, on);
 		return 0;
@@ -573,7 +577,7 @@ static void recalibrate(struct dp83640_clock *clock)
 	 */
 	evnt = EVNT_WR | EVNT_RISE | EVNT_SINGLE;
 	evnt |= (trigger & EVNT_SEL_MASK) << EVNT_SEL_SHIFT;
-	evnt |= (calibrate_pin & EVNT_GPIO_MASK) << EVNT_GPIO_SHIFT;
+	evnt |= (clock->calibrate_pin & EVNT_GPIO_MASK) << EVNT_GPIO_SHIFT;
 
 	list_for_each(this, &clock->phylist) {
 		tmp = list_entry(this, struct dp83640_private, list);
@@ -586,7 +590,7 @@ static void recalibrate(struct dp83640_clock *clock)
 	 */
 	ptp_trig = TRIG_WR | TRIG_IF_LATE | TRIG_PULSE;
 	ptp_trig |= (trigger  & TRIG_CSEL_MASK) << TRIG_CSEL_SHIFT;
-	ptp_trig |= (calibrate_pin & TRIG_GPIO_MASK) << TRIG_GPIO_SHIFT;
+	ptp_trig |= (clock->calibrate_pin & TRIG_GPIO_MASK) << TRIG_GPIO_SHIFT;
 	ext_write(0, master, PAGE5, PTP_TRIG, ptp_trig);
 
 	/* load trigger */
@@ -691,7 +695,7 @@ static int decode_evnt(struct dp83640_private *dp83640,
 	event.type = PTP_CLOCK_EXTTS;
 	event.timestamp = phy2txts(&dp83640->edata);
 
-	for (i = 0; i < n_ext_ts; i++) {
+	for (i = 0; i < dp83640->clock->caps.n_ext_ts; i++) {
 		if (ext_status & exts_chan_to_edata(i)) {
 			event.index = i;
 			ptp_clock_event(dp83640->clock->ptp_clock, &event);
@@ -893,12 +897,11 @@ static void dp83640_clock_init(struct dp83640_clock *clock, struct mii_bus *bus)
 	mutex_init(&clock->extreg_lock);
 	mutex_init(&clock->clock_lock);
 	INIT_LIST_HEAD(&clock->phylist);
+	clock->calibrate_pin = -1;
 	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;
-	clock->caps.n_per_out	= n_per_out;
 	clock->caps.pps		= 0;
 	clock->caps.adjfreq	= ptp_dp83640_adjfreq;
 	clock->caps.adjtime	= ptp_dp83640_adjtime;
@@ -911,18 +914,6 @@ static void dp83640_clock_init(struct dp83640_clock *clock, struct mii_bus *bus)
 	get_device(&bus->dev);
 }
 
-static int choose_this_phy(struct dp83640_clock *clock,
-			   struct phy_device *phydev)
-{
-	if (chosen_phy == -1 && !clock->chosen)
-		return 1;
-
-	if (chosen_phy == phydev->addr)
-		return 1;
-
-	return 0;
-}
-
 static struct dp83640_clock *dp83640_clock_get(struct dp83640_clock *clock)
 {
 	if (clock)
@@ -968,6 +959,86 @@ static void dp83640_clock_put(struct dp83640_clock *clock)
 	mutex_unlock(&clock->clock_lock);
 }
 
+#ifdef CONFIG_OF
+static int dp83640_probe_dt(struct device_node *node,
+			    struct dp83640_private *dp83640)
+{
+	struct dp83640_clock *clock = dp83640->clock;
+	struct property *prop;
+	int err, proplen, n_cal = 0;
+
+	dp83640->slave = of_property_read_bool(node, "dp83640,slave");
+	if (!dp83640->slave && clock->chosen) {
+		pr_err("More then one dp83640 master on the same bus");
+		return -EINVAL;
+	}
+
+	prop = of_find_property(node, "dp83640,calibrate-pin", &proplen);
+	if (prop) {
+		if (dp83640->slave) {
+			pr_err("dp83640 slave cannot have calibrate pin");
+			return -EINVAL;
+		}
+		of_property_read_u32(node, "dp83640,calibrate-pin",
+				     &clock->calibrate_pin);
+		n_cal = 1;
+	}
+
+	prop = of_find_property(node, "dp83640,perout-pins", &proplen);
+	if (prop) {
+		if (dp83640->slave) {
+			pr_err("dp83640 slave cannot have perout pins");
+			return -EINVAL;
+		}
+
+		clock->caps.n_per_out = proplen / sizeof(u32);
+		if (clock->caps.n_per_out + n_cal > N_EXT) {
+			pr_err("Too many dp83640,perout-pins");
+			return -EINVAL;
+		}
+		err = of_property_read_u32_array(node, "dp83640,perout-pins",
+						 clock->perout_pins,
+						 clock->caps.n_per_out);
+		if (err < 0)
+			return err;
+	}
+
+	prop = of_find_property(node, "dp83640,extts-pins", &proplen);
+	if (prop) {
+		if (dp83640->slave) {
+			pr_err("dp83640 slave cannot have extts pins");
+			return -EINVAL;
+		}
+
+		clock->caps.n_ext_ts = proplen / sizeof(u32);
+		if (clock->caps.n_ext_ts + n_cal > N_EXT) {
+			pr_err("Too many dp83640,extts-pins");
+			return -EINVAL;
+		}
+		err = of_property_read_u32_array(node, "dp83640,extts-pins",
+						 clock->extts_pins,
+						 clock->caps.n_ext_ts);
+		if (err < 0)
+			return err;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id dp83640_of_match_table[] = {
+	{ .compatible = "national,dp83640", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, dp83640_of_match_table);
+#else
+
+static inline int dp83640_probe_dt(struct device_node *node,
+				   struct dp83640_private *dp83640)
+{
+	return 0;
+}
+#endif
+
 static int dp83640_probe(struct phy_device *phydev)
 {
 	struct dp83640_clock *clock;
@@ -985,7 +1056,24 @@ static int dp83640_probe(struct phy_device *phydev)
 	if (!dp83640)
 		goto no_memory;
 
+	dp83640->clock = clock;
 	dp83640->phydev = phydev;
+
+	if (phydev->dev.of_node) {
+		err = dp83640_probe_dt(phydev->dev.of_node, dp83640);
+		if (err)
+			return err;
+	} else {
+		clock->calibrate_pin = calibrate_pin;
+		memcpy(clock->perout_pins, perout_pins,
+		       sizeof(clock->perout_pins));
+		memcpy(clock->extts_pins, extts_pins,
+		       sizeof(clock->extts_pins));
+		if (clock->chosen ||
+		    (chosen_phy != -1 && phydev->addr != chosen_phy))
+			dp83640->slave = true;
+	}
+
 	INIT_WORK(&dp83640->ts_work, rx_timestamp_work);
 
 	INIT_LIST_HEAD(&dp83640->rxts);
@@ -999,9 +1087,7 @@ static int dp83640_probe(struct phy_device *phydev)
 	skb_queue_head_init(&dp83640->rx_queue);
 	skb_queue_head_init(&dp83640->tx_queue);
 
-	dp83640->clock = clock;
-
-	if (choose_this_phy(clock, phydev)) {
+	if (!dp83640->slave) {
 		clock->chosen = dp83640;
 		clock->ptp_clock = ptp_clock_register(&clock->caps, &phydev->dev);
 		if (IS_ERR(clock->ptp_clock)) {
@@ -1353,7 +1439,10 @@ static struct phy_driver dp83640_driver = {
 	.hwtstamp	= dp83640_hwtstamp,
 	.rxtstamp	= dp83640_rxtstamp,
 	.txtstamp	= dp83640_txtstamp,
-	.driver		= {.owner = THIS_MODULE,}
+	.driver		= {
+		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(dp83640_of_match_table),
+	}
 };
 
 static int __init dp83640_init(void)
-- 
1.8.5.3

[PATCH v3 0/3] dp83640: Get pin and master/slave configuration from DT

[Stefan Sørensen]

This patch series add DT configuration to the DP83640 PHY driver and makes
the configuration of periodic output pins dynamic.

Changes since v2:
 - Add patch to properly configure perout triggers 0+1
 - Keep extts and perout numbers separate
 - Shorten pr_err lines

Changes since v1:
 - Add bindings documentation
 - Keep module parameters
 - Rename gpio->pin
 - Split patch into DT part and dynamic periodic output support

Stefan Sørensen (3):
  net:phy:dp83640: Program pulsewidth2 values of perout triggers 0 and 1
  net:phy:dp83640: Support a configurable number of periodic outputs
  net:phy:dp83640: Get pin and master/slave configuration from DT

 Documentation/devicetree/bindings/net/dp83640.txt |  29 ++++
 drivers/net/phy/dp83640.c                         | 199 +++++++++++++++-------
 2 files changed, 170 insertions(+), 58 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/dp83640.txt

-- 
1.8.5.3

[PATCH v3 1/3] net:phy:dp83640: Program pulsewidth2 values of perout triggers 0 and 1

[Stefan Sørensen]

Periodic output triggers 0 and 1 of the dp83640 has a programmable
duty-cycle which is controlled by the Pulsewidth2 field of the trigger
data register.  This field is not documented in the datasheet, but it
is described in the "PHYTER Software Development Guide" section
3.1.4.1. Failing to set the field can result in unstable/no trigger
output.

This patch add programming of the Pulsewidth2 field, setting it to the
same value as the Pulsewidth field for a 50% duty cycle.

Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com>
---
 drivers/net/phy/dp83640.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 5ff221d..a370814 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -312,6 +312,11 @@ static void periodic_output(struct dp83640_clock *clock,
 	ext_write(0, phydev, PAGE4, PTP_TDR, sec >> 16);       /* sec[31:16] */
 	ext_write(0, phydev, PAGE4, PTP_TDR, period & 0xffff); /* ns[15:0] */
 	ext_write(0, phydev, PAGE4, PTP_TDR, period >> 16);    /* ns[31:16] */
+	/* Triggers 0 and 1 has programmable pulsewidth2 */
+	if (trigger == 0 || trigger == 1) {
+		ext_write(0, phydev, PAGE4, PTP_TDR, period & 0xffff);
+		ext_write(0, phydev, PAGE4, PTP_TDR, period >> 16);
+	}
 
 	/*enable trigger*/
 	val &= ~TRIG_LOAD;
-- 
1.8.5.3

[PATCH v3 2/3] net:phy:dp83640: Support a configurable number of periodic outputs

[Stefan Sørensen]

The driver is currently limited to a single periodic output. This patch makes
the number of peridodic output dynamic by dropping the gpio_tab module
parameter and adding calibrate_pin, perout_pins, and extts_pins parameters.

Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com>
---
 drivers/net/phy/dp83640.c | 73 ++++++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 42 deletions(-)

diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index a370814..28a6e1d 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -38,15 +38,12 @@
 #define LAYER4		0x02
 #define LAYER2		0x01
 #define MAX_RXTS	64
-#define N_EXT_TS	6
+#define N_EXT		8
 #define PSF_PTPVER	2
 #define PSF_EVNT	0x4000
 #define PSF_RX		0x2000
 #define PSF_TX		0x1000
-#define EXT_EVENT	1
-#define CAL_EVENT	7
 #define CAL_TRIGGER	7
-#define PER_TRIGGER	6
 
 #define MII_DP83640_MICR 0x11
 #define MII_DP83640_MISR 0x12
@@ -146,32 +143,24 @@ struct dp83640_clock {
 	struct ptp_clock *ptp_clock;
 };
 
-/* globals */
-
-enum {
-	CALIBRATE_GPIO,
-	PEROUT_GPIO,
-	EXTTS0_GPIO,
-	EXTTS1_GPIO,
-	EXTTS2_GPIO,
-	EXTTS3_GPIO,
-	EXTTS4_GPIO,
-	EXTTS5_GPIO,
-	GPIO_TABLE_SIZE
-};
 
 static int chosen_phy = -1;
-static ushort gpio_tab[GPIO_TABLE_SIZE] = {
-	1, 2, 3, 4, 8, 9, 10, 11
-};
+static int calibrate_pin = 1;
+static int perout_pins[N_EXT] = {2};
+static int n_per_out = 1;
+static int extts_pins[N_EXT] = {3, 4, 8, 9, 10, 11};
+static int n_ext_ts = 6;
 
 module_param(chosen_phy, int, 0444);
-module_param_array(gpio_tab, ushort, NULL, 0444);
+module_param(calibrate_pin, int, 0444);
+module_param_array(perout_pins, int, &n_per_out, 0444);
+module_param_array(extts_pins, int, &n_ext_ts, 0444);
 
 MODULE_PARM_DESC(chosen_phy, \
 	"The address of the PHY to use for the ancillary clock features");
-MODULE_PARM_DESC(gpio_tab, \
-	"Which GPIO line to use for which purpose: cal,perout,extts1,...,extts6");
+MODULE_PARM_DESC(calibrate_pin, "Which pin to use for calibration");
+MODULE_PARM_DESC(perout_pins, "Which pins to use for periodic output");
+MODULE_PARM_DESC(extts_pins, "Which pins to use for external timestamping");
 
 /* a list of clocks and a mutex to protect it */
 static LIST_HEAD(phyter_clocks);
@@ -267,15 +256,15 @@ static u64 phy2txts(struct phy_txts *p)
 }
 
 static void periodic_output(struct dp83640_clock *clock,
-			    struct ptp_clock_request *clkreq, bool on)
+			    struct ptp_clock_request *clkreq, int trigger,
+			    bool on)
 {
 	struct dp83640_private *dp83640 = clock->chosen;
 	struct phy_device *phydev = dp83640->phydev;
 	u32 sec, nsec, period;
-	u16 gpio, ptp_trig, trigger, val;
+	u16 gpio, ptp_trig, val;
 
-	gpio = on ? gpio_tab[PEROUT_GPIO] : 0;
-	trigger = PER_TRIGGER;
+	gpio = on ? perout_pins[trigger] : 0;
 
 	ptp_trig = TRIG_WR |
 		(trigger & TRIG_CSEL_MASK) << TRIG_CSEL_SHIFT |
@@ -446,12 +435,12 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp,
 	switch (rq->type) {
 	case PTP_CLK_REQ_EXTTS:
 		index = rq->extts.index;
-		if (index < 0 || index >= N_EXT_TS)
+		if (index < 0 || index >= n_ext_ts)
 			return -EINVAL;
-		event_num = EXT_EVENT + index;
+		event_num = index;
 		evnt = EVNT_WR | (event_num & EVNT_SEL_MASK) << EVNT_SEL_SHIFT;
 		if (on) {
-			gpio_num = gpio_tab[EXTTS0_GPIO + index];
+			gpio_num = extts_pins[index];
 			evnt |= (gpio_num & EVNT_GPIO_MASK) << EVNT_GPIO_SHIFT;
 			if (rq->extts.flags & PTP_FALLING_EDGE)
 				evnt |= EVNT_FALL;
@@ -462,9 +451,10 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp,
 		return 0;
 
 	case PTP_CLK_REQ_PEROUT:
-		if (rq->perout.index != 0)
+		index = rq->perout.index;
+		if (index < 0 || index >= n_per_out)
 			return -EINVAL;
-		periodic_output(clock, rq, on);
+		periodic_output(clock, rq, index, on);
 		return 0;
 
 	default:
@@ -557,10 +547,9 @@ static void recalibrate(struct dp83640_clock *clock)
 	struct list_head *this;
 	struct dp83640_private *tmp;
 	struct phy_device *master = clock->chosen->phydev;
-	u16 cal_gpio, cfg0, evnt, ptp_trig, trigger, val;
+	u16 cfg0, evnt, ptp_trig, trigger, val;
 
 	trigger = CAL_TRIGGER;
-	cal_gpio = gpio_tab[CALIBRATE_GPIO];
 
 	mutex_lock(&clock->extreg_lock);
 
@@ -583,8 +572,8 @@ static void recalibrate(struct dp83640_clock *clock)
 	 * enable an event timestamp
 	 */
 	evnt = EVNT_WR | EVNT_RISE | EVNT_SINGLE;
-	evnt |= (CAL_EVENT & EVNT_SEL_MASK) << EVNT_SEL_SHIFT;
-	evnt |= (cal_gpio & EVNT_GPIO_MASK) << EVNT_GPIO_SHIFT;
+	evnt |= (trigger & EVNT_SEL_MASK) << EVNT_SEL_SHIFT;
+	evnt |= (calibrate_pin & EVNT_GPIO_MASK) << EVNT_GPIO_SHIFT;
 
 	list_for_each(this, &clock->phylist) {
 		tmp = list_entry(this, struct dp83640_private, list);
@@ -597,7 +586,7 @@ static void recalibrate(struct dp83640_clock *clock)
 	 */
 	ptp_trig = TRIG_WR | TRIG_IF_LATE | TRIG_PULSE;
 	ptp_trig |= (trigger  & TRIG_CSEL_MASK) << TRIG_CSEL_SHIFT;
-	ptp_trig |= (cal_gpio & TRIG_GPIO_MASK) << TRIG_GPIO_SHIFT;
+	ptp_trig |= (calibrate_pin & TRIG_GPIO_MASK) << TRIG_GPIO_SHIFT;
 	ext_write(0, master, PAGE5, PTP_TRIG, ptp_trig);
 
 	/* load trigger */
@@ -661,7 +650,7 @@ static void recalibrate(struct dp83640_clock *clock)
 
 static inline u16 exts_chan_to_edata(int ch)
 {
-	return 1 << ((ch + EXT_EVENT) * 2);
+	return 1 << ((ch) * 2);
 }
 
 static int decode_evnt(struct dp83640_private *dp83640,
@@ -695,14 +684,14 @@ static int decode_evnt(struct dp83640_private *dp83640,
 		parsed = words + 2;
 	} else {
 		parsed = words + 1;
-		i = ((ests >> EVNT_NUM_SHIFT) & EVNT_NUM_MASK) - EXT_EVENT;
+		i = ((ests >> EVNT_NUM_SHIFT) & EVNT_NUM_MASK);
 		ext_status = exts_chan_to_edata(i);
 	}
 
 	event.type = PTP_CLOCK_EXTTS;
 	event.timestamp = phy2txts(&dp83640->edata);
 
-	for (i = 0; i < N_EXT_TS; i++) {
+	for (i = 0; i < n_ext_ts; i++) {
 		if (ext_status & exts_chan_to_edata(i)) {
 			event.index = i;
 			ptp_clock_event(dp83640->clock->ptp_clock, &event);
@@ -908,8 +897,8 @@ static void dp83640_clock_init(struct dp83640_clock *clock, struct mii_bus *bus)
 	sprintf(clock->caps.name, "dp83640 timer");
 	clock->caps.max_adj	= 1953124;
 	clock->caps.n_alarm	= 0;
-	clock->caps.n_ext_ts	= N_EXT_TS;
-	clock->caps.n_per_out	= 1;
+	clock->caps.n_ext_ts	= n_ext_ts;
+	clock->caps.n_per_out	= n_per_out;
 	clock->caps.pps		= 0;
 	clock->caps.adjfreq	= ptp_dp83640_adjfreq;
 	clock->caps.adjtime	= ptp_dp83640_adjtime;
-- 
1.8.5.3

[PATCH v3 3/3] net:phy:dp83640: Get pin and master/slave configuration from DT

[Stefan Sørensen]

This patch adds configuration of the periodic output and external timestamp
pins available on the dp83640 family of PHYs. It also configures the
master/slave relationship in a group of PHYs on the same MDIO bus and the pins
used for clock calibration in the group.

The configuration is retrieved from DT through the properties
    dp83640,slave
    dp83640,calibrate-pin
    dp83640,perout-pins
    dp83640,extts-pins
The configuration module parameters are retained as fallback for the non-DT
case.

Since the pin configuration is now stored for each clock device, groups of
devices on different mdio busses can now have different pin configurations.

Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com>
---
 Documentation/devicetree/bindings/net/dp83640.txt |  29 +++++
 drivers/net/phy/dp83640.c                         | 139 ++++++++++++++++++----
 2 files changed, 143 insertions(+), 25 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/dp83640.txt

diff --git a/Documentation/devicetree/bindings/net/dp83640.txt b/Documentation/devicetree/bindings/net/dp83640.txt
new file mode 100644
index 0000000..b9a57c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dp83640.txt
@@ -0,0 +1,29 @@
+Required properties for the National DP83640 ethernet phy:
+
+- compatible : Must contain "national,dp83640"
+
+Optional properties:
+
+- dp83640,slave: If present, this phy will be slave to another dp83640
+  on the same mdio bus.
+- dp83640,perout-pins : List of the pin pins used for periodic output
+  triggers.
+- dp83640,extts-pins : List of the pin pins used for external event
+  timestamping.
+- dp83640,calibrate-pin : The pin used for master/slave calibration.
+
+Example:
+
+	ethernet-phy@1 {
+		compatible = "national,dp83640", "ethernet-phy-ieee802.3-c22";
+		reg = <1>;
+		dp83640,perout-pins = <2>;
+		dp83640,extts-pins = <3 4 8 9 10 11>;
+		dp83640,calibrate-pin = <1>;
+	};
+
+	ethernet-phy@2 {
+		compatible = "national,dp83640", "ethernet-phy-ieee802.3-c22";
+		reg = <2>;
+		dp83640,slave;
+	};
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 28a6e1d..077bdc2 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -30,6 +30,7 @@
 #include <linux/phy.h>
 #include <linux/ptp_classify.h>
 #include <linux/ptp_clock_kernel.h>
+#include <linux/of_device.h>
 
 #include "dp83640_reg.h"
 
@@ -120,6 +121,8 @@ struct dp83640_private {
 	/* queues of incoming and outgoing packets */
 	struct sk_buff_head rx_queue;
 	struct sk_buff_head tx_queue;
+	/* is this phyter a slave */
+	bool slave;
 };
 
 struct dp83640_clock {
@@ -141,6 +144,7 @@ struct dp83640_clock {
 	struct list_head phylist;
 	/* reference to our PTP hardware clock */
 	struct ptp_clock *ptp_clock;
+	u32 perout_pins[N_EXT], extts_pins[N_EXT], calibrate_pin;
 };
 
 
@@ -264,7 +268,7 @@ static void periodic_output(struct dp83640_clock *clock,
 	u32 sec, nsec, period;
 	u16 gpio, ptp_trig, val;
 
-	gpio = on ? perout_pins[trigger] : 0;
+	gpio = on ? clock->perout_pins[trigger] : 0;
 
 	ptp_trig = TRIG_WR |
 		(trigger & TRIG_CSEL_MASK) << TRIG_CSEL_SHIFT |
@@ -435,12 +439,12 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp,
 	switch (rq->type) {
 	case PTP_CLK_REQ_EXTTS:
 		index = rq->extts.index;
-		if (index < 0 || index >= n_ext_ts)
+		if (index < 0 || index >= clock->caps.n_ext_ts)
 			return -EINVAL;
 		event_num = index;
 		evnt = EVNT_WR | (event_num & EVNT_SEL_MASK) << EVNT_SEL_SHIFT;
 		if (on) {
-			gpio_num = extts_pins[index];
+			gpio_num = clock->extts_pins[index];
 			evnt |= (gpio_num & EVNT_GPIO_MASK) << EVNT_GPIO_SHIFT;
 			if (rq->extts.flags & PTP_FALLING_EDGE)
 				evnt |= EVNT_FALL;
@@ -452,7 +456,7 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp,
 
 	case PTP_CLK_REQ_PEROUT:
 		index = rq->perout.index;
-		if (index < 0 || index >= n_per_out)
+		if (index < 0 || index >= clock->caps.n_per_out)
 			return -EINVAL;
 		periodic_output(clock, rq, index, on);
 		return 0;
@@ -573,7 +577,7 @@ static void recalibrate(struct dp83640_clock *clock)
 	 */
 	evnt = EVNT_WR | EVNT_RISE | EVNT_SINGLE;
 	evnt |= (trigger & EVNT_SEL_MASK) << EVNT_SEL_SHIFT;
-	evnt |= (calibrate_pin & EVNT_GPIO_MASK) << EVNT_GPIO_SHIFT;
+	evnt |= (clock->calibrate_pin & EVNT_GPIO_MASK) << EVNT_GPIO_SHIFT;
 
 	list_for_each(this, &clock->phylist) {
 		tmp = list_entry(this, struct dp83640_private, list);
@@ -586,7 +590,7 @@ static void recalibrate(struct dp83640_clock *clock)
 	 */
 	ptp_trig = TRIG_WR | TRIG_IF_LATE | TRIG_PULSE;
 	ptp_trig |= (trigger  & TRIG_CSEL_MASK) << TRIG_CSEL_SHIFT;
-	ptp_trig |= (calibrate_pin & TRIG_GPIO_MASK) << TRIG_GPIO_SHIFT;
+	ptp_trig |= (clock->calibrate_pin & TRIG_GPIO_MASK) << TRIG_GPIO_SHIFT;
 	ext_write(0, master, PAGE5, PTP_TRIG, ptp_trig);
 
 	/* load trigger */
@@ -691,7 +695,7 @@ static int decode_evnt(struct dp83640_private *dp83640,
 	event.type = PTP_CLOCK_EXTTS;
 	event.timestamp = phy2txts(&dp83640->edata);
 
-	for (i = 0; i < n_ext_ts; i++) {
+	for (i = 0; i < dp83640->clock->caps.n_ext_ts; i++) {
 		if (ext_status & exts_chan_to_edata(i)) {
 			event.index = i;
 			ptp_clock_event(dp83640->clock->ptp_clock, &event);
@@ -893,12 +897,11 @@ static void dp83640_clock_init(struct dp83640_clock *clock, struct mii_bus *bus)
 	mutex_init(&clock->extreg_lock);
 	mutex_init(&clock->clock_lock);
 	INIT_LIST_HEAD(&clock->phylist);
+	clock->calibrate_pin = -1;
 	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;
-	clock->caps.n_per_out	= n_per_out;
 	clock->caps.pps		= 0;
 	clock->caps.adjfreq	= ptp_dp83640_adjfreq;
 	clock->caps.adjtime	= ptp_dp83640_adjtime;
@@ -911,18 +914,6 @@ static void dp83640_clock_init(struct dp83640_clock *clock, struct mii_bus *bus)
 	get_device(&bus->dev);
 }
 
-static int choose_this_phy(struct dp83640_clock *clock,
-			   struct phy_device *phydev)
-{
-	if (chosen_phy == -1 && !clock->chosen)
-		return 1;
-
-	if (chosen_phy == phydev->addr)
-		return 1;
-
-	return 0;
-}
-
 static struct dp83640_clock *dp83640_clock_get(struct dp83640_clock *clock)
 {
 	if (clock)
@@ -968,6 +959,86 @@ static void dp83640_clock_put(struct dp83640_clock *clock)
 	mutex_unlock(&clock->clock_lock);
 }
 
+#ifdef CONFIG_OF
+static int dp83640_probe_dt(struct device_node *node,
+			    struct dp83640_private *dp83640)
+{
+	struct dp83640_clock *clock = dp83640->clock;
+	struct property *prop;
+	int err, proplen, n_cal = 0;
+
+	dp83640->slave = of_property_read_bool(node, "dp83640,slave");
+	if (!dp83640->slave && clock->chosen) {
+		pr_err("More then one dp83640 master on the same bus");
+		return -EINVAL;
+	}
+
+	prop = of_find_property(node, "dp83640,calibrate-pin", &proplen);
+	if (prop) {
+		if (dp83640->slave) {
+			pr_err("dp83640 slave cannot have calibrate pin");
+			return -EINVAL;
+		}
+		of_property_read_u32(node, "dp83640,calibrate-pin",
+				     &clock->calibrate_pin);
+		n_cal = 1;
+	}
+
+	prop = of_find_property(node, "dp83640,perout-pins", &proplen);
+	if (prop) {
+		if (dp83640->slave) {
+			pr_err("dp83640 slave cannot have perout pins");
+			return -EINVAL;
+		}
+
+		clock->caps.n_per_out = proplen / sizeof(u32);
+		if (clock->caps.n_per_out + n_cal > N_EXT) {
+			pr_err("Too many dp83640,perout-pins");
+			return -EINVAL;
+		}
+		err = of_property_read_u32_array(node, "dp83640,perout-pins",
+						 clock->perout_pins,
+						 clock->caps.n_per_out);
+		if (err < 0)
+			return err;
+	}
+
+	prop = of_find_property(node, "dp83640,extts-pins", &proplen);
+	if (prop) {
+		if (dp83640->slave) {
+			pr_err("dp83640 slave cannot have extts pins");
+			return -EINVAL;
+		}
+
+		clock->caps.n_ext_ts = proplen / sizeof(u32);
+		if (clock->caps.n_ext_ts + n_cal > N_EXT) {
+			pr_err("Too many dp83640,extts-pins");
+			return -EINVAL;
+		}
+		err = of_property_read_u32_array(node, "dp83640,extts-pins",
+						 clock->extts_pins,
+						 clock->caps.n_ext_ts);
+		if (err < 0)
+			return err;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id dp83640_of_match_table[] = {
+	{ .compatible = "national,dp83640", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, dp83640_of_match_table);
+#else
+
+static inline int dp83640_probe_dt(struct device_node *node,
+				   struct dp83640_private *dp83640)
+{
+	return 0;
+}
+#endif
+
 static int dp83640_probe(struct phy_device *phydev)
 {
 	struct dp83640_clock *clock;
@@ -985,7 +1056,24 @@ static int dp83640_probe(struct phy_device *phydev)
 	if (!dp83640)
 		goto no_memory;
 
+	dp83640->clock = clock;
 	dp83640->phydev = phydev;
+
+	if (phydev->dev.of_node) {
+		err = dp83640_probe_dt(phydev->dev.of_node, dp83640);
+		if (err)
+			return err;
+	} else {
+		clock->calibrate_pin = calibrate_pin;
+		memcpy(clock->perout_pins, perout_pins,
+		       sizeof(clock->perout_pins));
+		memcpy(clock->extts_pins, extts_pins,
+		       sizeof(clock->extts_pins));
+		if (clock->chosen ||
+		    (chosen_phy != -1 && phydev->addr != chosen_phy))
+			dp83640->slave = true;
+	}
+
 	INIT_WORK(&dp83640->ts_work, rx_timestamp_work);
 
 	INIT_LIST_HEAD(&dp83640->rxts);
@@ -999,9 +1087,7 @@ static int dp83640_probe(struct phy_device *phydev)
 	skb_queue_head_init(&dp83640->rx_queue);
 	skb_queue_head_init(&dp83640->tx_queue);
 
-	dp83640->clock = clock;
-
-	if (choose_this_phy(clock, phydev)) {
+	if (!dp83640->slave) {
 		clock->chosen = dp83640;
 		clock->ptp_clock = ptp_clock_register(&clock->caps, &phydev->dev);
 		if (IS_ERR(clock->ptp_clock)) {
@@ -1353,7 +1439,10 @@ static struct phy_driver dp83640_driver = {
 	.hwtstamp	= dp83640_hwtstamp,
 	.rxtstamp	= dp83640_rxtstamp,
 	.txtstamp	= dp83640_txtstamp,
-	.driver		= {.owner = THIS_MODULE,}
+	.driver		= {
+		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(dp83640_of_match_table),
+	}
 };
 
 static int __init dp83640_init(void)
-- 
1.8.5.3

Re: [PATCH v3 0/3] dp83640: Get pin and master/slave configuration from DT

[David Miller]

From: Stefan Sørensen <stefan.sorensen@spectralink.com>
Date: Thu, 13 Feb 2014 15:35:22 +0100

> This patch series add DT configuration to the DP83640 PHY driver and makes
> the configuration of periodic output pins dynamic.
> 
> Changes since v2:
>  - Add patch to properly configure perout triggers 0+1
>  - Keep extts and perout numbers separate
>  - Shorten pr_err lines
> 
> Changes since v1:
>  - Add bindings documentation
>  - Keep module parameters
>  - Rename gpio->pin
>  - Split patch into DT part and dynamic periodic output support
> 
> Stefan Sørensen (3):
>   net:phy:dp83640: Program pulsewidth2 values of perout triggers 0 and 1
>   net:phy:dp83640: Support a configurable number of periodic outputs
>   net:phy:dp83640: Get pin and master/slave configuration from DT

I'm not applying this.

This is a great example of why I hate module parameters.

Whoever added these module parameters in the first place took the easy
road and didn't have to think about whether this was a good way or not
to specify these parameters.  Do you really think we can, like magic,
just remove this module parameter without having to think about the
consequences of that action at all?

The entities currently specifying this parameter, do they just lose?

Sorry, that's unacceptable.

You'll have to code this in a way such that people who currently specify
this parameter keep working.

If you don't like it, be angry at whoever added this module parameter
in the first place, not me.  Module parameters are 99 times out of 100
not the right way to configure something, really.

Thanks.

Re: [PATCH v3 0/3] dp83640: Get pin and master/slave configuration from DT

[Richard Cochran]

On Thu, Feb 13, 2014 at 05:39:57PM -0500, David Miller wrote:

> You'll have to code this in a way such that people who currently specify
> this parameter keep working.

So, do module parameters count as user land ABI?

[ In any case, for this module, there must be a way to retain the
  parameters and still offer DT, and letting DT have precedence. ]

People want to be able to configure the auxiliary functions on the
pins of their PTP devices. My preference for supporting this is:

1. additional ioctl on the PTP character device
2. ethtool ioctl
3. DT and/or ACPI

The first option seems best because that is how you activate the
auxiliary functions. I am working on something in this direction.

Can't we just drop the DT idea altogther?

> If you don't like it, be angry at whoever added this module parameter
> in the first place, not me.  Module parameters are 99 times out of 100
> not the right way to configure something, really.

Yeah, I am the one, be angry with me.

Sorry,
Richard

Re: [PATCH v3 0/3] dp83640: Get pin and master/slave configuration from DT

[David Miller]

From: Richard Cochran <richardcochran@gmail.com>
Date: Fri, 14 Feb 2014 10:06:01 +0100

> People want to be able to configure the auxiliary functions on the
> pins of their PTP devices. My preference for supporting this is:
> 
> 1. additional ioctl on the PTP character device
> 2. ethtool ioctl
> 3. DT and/or ACPI
> 
> The first option seems best because that is how you activate the
> auxiliary functions. I am working on something in this direction.

Probably since PTP already uses ioctls, and that is the activation
point, yes that seems the best approach.

Re: [PATCH v3 0/3] dp83640: Get pin and master/slave configuration from DT

[Richard Cochran]

On Fri, Feb 14, 2014 at 10:06:01AM +0100, Richard Cochran wrote:
> On Thu, Feb 13, 2014 at 05:39:57PM -0500, David Miller wrote:
> > If you don't like it, be angry at whoever added this module parameter
> > in the first place, not me.  Module parameters are 99 times out of 100
> > not the right way to configure something, really.
> 
> Yeah, I am the one, be angry with me.

Look back at this driver, there was in fact a good reason to have at
least the "calibration" pin as a module parameter. When using multiple
phyters, together they should act as one clock. The driver does this
automatically during the probe method, and so it needs to know which
pin to use well before user space is ready.

However, the other pin functions could have waited for user space
configuration, but I had just lumped them all together. Going forward,
I think the get/setpin ioctl will work fine both for this driver
(without changing the legacy module parameters) and for future ones.

Thanks,
Richard