| Message ID | 20220808131556.163207-1-csokas.bence@prolan.hu (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | fec: Allow changing the PPS channel | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
On Mon, Aug 08, 2022 at 03:15:57PM +0200, Csókás Bence wrote: > +static ssize_t pps_ch_store(struct kobject *kobj, struct kobj_attribute *attr, const char *buf, size_t count) > +{ > + struct device *dev = container_of(kobj, struct device, kobj); > + struct net_device *ndev = to_net_dev(dev); > + struct fec_enet_private *fep = netdev_priv(ndev); > + int enable = fep->pps_enable; > + struct ptp_clock_request ptp_rq = { .type = PTP_CLK_REQ_PPS }; > + > + if (enable) > + fep->ptp_caps.enable(&fep->ptp_caps, &ptp_rq, 0); > + > + kstrtoint(buf, 0, &fep->pps_channel); > + > + if (enable) > + fep->ptp_caps.enable(&fep->ptp_caps, &ptp_rq, 1); NAK. Don't use a private, custom sysfs knob. The core PTP layer provides an API for that already. Thanks, Richard > + > + return count; > +}
Hi! >On Mon, Aug 08, 2022 at 03:15:57PM +0200, Csókás Bence wrote: >> +static ssize_t pps_ch_store(struct kobject *kobj, struct kobj_attribute *attr, const char *buf, size_t count) >> +{ >> + struct device *dev = container_of(kobj, struct device, kobj); >> + struct net_device *ndev = to_net_dev(dev); >> + struct fec_enet_private *fep = netdev_priv(ndev); >> + int enable = fep->pps_enable; >> + struct ptp_clock_request ptp_rq = { .type = PTP_CLK_REQ_PPS }; >> + >> + if (enable) >> + fep->ptp_caps.enable(&fep->ptp_caps, &ptp_rq, 0); >> + >> + kstrtoint(buf, 0, &fep->pps_channel); >> + >> + if (enable) >> + fep->ptp_caps.enable(&fep->ptp_caps, &ptp_rq, 1); > > NAK. > > Don't use a private, custom sysfs knob. The core PTP layer provides > an API for that already. Does it? I seem to have missed it. Can you point me at some docs? Also, does it have support for setting pulse mode (i.e. high, low, toggle)? > Thanks, > Richard Thanks, Bence > >> + >> + return count; >> +}
On Tue, Aug 09, 2022 at 07:36:55AM +0000, Csókás Bence wrote: > > Don't use a private, custom sysfs knob. The core PTP layer provides > > an API for that already. > > Does it? I seem to have missed it. Can you point me at some docs? See include/uapi/linux/ptp_clock.h: /* * Bits of the ptp_perout_request.flags field: */ #define PTP_PEROUT_ONE_SHOT (1<<0) #define PTP_PEROUT_DUTY_CYCLE (1<<1) #define PTP_PEROUT_PHASE (1<<2) /* * flag fields valid for the new PTP_PEROUT_REQUEST2 ioctl. */ #define PTP_PEROUT_VALID_FLAGS (PTP_PEROUT_ONE_SHOT | \ PTP_PEROUT_DUTY_CYCLE | \ PTP_PEROUT_PHASE) ... struct ptp_perout_request { union { /* * Absolute start time. * Valid only if (flags & PTP_PEROUT_PHASE) is unset. */ struct ptp_clock_time start; /* * Phase offset. The signal should start toggling at an * unspecified integer multiple of the period, plus this value. * The start time should be "as soon as possible". * Valid only if (flags & PTP_PEROUT_PHASE) is set. */ struct ptp_clock_time phase; }; struct ptp_clock_time period; /* Desired period, zero means disable. */ unsigned int index; /* Which channel to configure. */ unsigned int flags; union { /* * The "on" time of the signal. * Must be lower than the period. * Valid only if (flags & PTP_PEROUT_DUTY_CYCLE) is set. */ struct ptp_clock_time on; /* Reserved for future use. */ unsigned int rsv[4]; }; }; There is also an API to select pins, with ptp_pin_function, ptp_pin_desc, PTP_PIN_GETFUNC, PTP_PIN_SETFUNC. > Also, does it have support for setting pulse mode (i.e. high, low, toggle)? You can produce a signal with any duty cycle that you wish. No support for inverted output, but you could add it if it is important to you. Thanks, Richard
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 4112c1283c40..0c1c489c3826 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -47,6 +47,7 @@ #include <linux/bitops.h> #include <linux/io.h> #include <linux/irq.h> +#include <linux/kobject.h> #include <linux/clk.h> #include <linux/crc32.h> #include <linux/platform_device.h> @@ -3609,6 +3610,36 @@ static int fec_enet_init_stop_mode(struct fec_enet_private *fep, return ret; } +static ssize_t pps_ch_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) +{ + struct device *dev = container_of(kobj, struct device, kobj); + struct net_device *ndev = to_net_dev(dev); + struct fec_enet_private *fep = netdev_priv(ndev); + + return sprintf(buf, "%d", fep->pps_channel); +} + +static ssize_t pps_ch_store(struct kobject *kobj, struct kobj_attribute *attr, const char *buf, size_t count) +{ + struct device *dev = container_of(kobj, struct device, kobj); + struct net_device *ndev = to_net_dev(dev); + struct fec_enet_private *fep = netdev_priv(ndev); + int enable = fep->pps_enable; + struct ptp_clock_request ptp_rq = { .type = PTP_CLK_REQ_PPS }; + + if (enable) + fep->ptp_caps.enable(&fep->ptp_caps, &ptp_rq, 0); + + kstrtoint(buf, 0, &fep->pps_channel); + + if (enable) + fep->ptp_caps.enable(&fep->ptp_caps, &ptp_rq, 1); + + return count; +} + +struct kobj_attribute pps_ch_attr = __ATTR(pps_channel, 0660, pps_ch_show, pps_ch_store); + static int fec_probe(struct platform_device *pdev) { @@ -3705,6 +3736,9 @@ fec_probe(struct platform_device *pdev) fep->phy_interface = interface; } + if (of_property_read_u32(np, "fsl,pps-channel", &fep->pps_channel)) + fep->pps_channel = 0; + fep->clk_ipg = devm_clk_get(&pdev->dev, "ipg"); if (IS_ERR(fep->clk_ipg)) { ret = PTR_ERR(fep->clk_ipg); @@ -3817,6 +3851,9 @@ fec_probe(struct platform_device *pdev) if (ret) goto failed_register; + if (sysfs_create_file(&ndev->dev.kobj, &pps_ch_attr.attr)) + pr_err("Cannot create pps_channel sysfs file\n"); + device_init_wakeup(&ndev->dev, fep->wol_flag & FEC_WOL_HAS_MAGIC_PACKET); diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c index 69dfed4de4ef..a5077eff305b 100644 --- a/drivers/net/ethernet/freescale/fec_ptp.c +++ b/drivers/net/ethernet/freescale/fec_ptp.c @@ -86,8 +86,6 @@ #define FEC_CC_MULT (1 << 31) #define FEC_COUNTER_PERIOD (1 << 31) #define PPS_OUPUT_RELOAD_PERIOD NSEC_PER_SEC -#define FEC_CHANNLE_0 0 -#define DEFAULT_PPS_CHANNEL FEC_CHANNLE_0 /** * fec_ptp_enable_pps @@ -112,7 +110,6 @@ static int fec_ptp_enable_pps(struct fec_enet_private *fep, uint enable) if (fep->pps_enable == enable) return 0; - fep->pps_channel = DEFAULT_PPS_CHANNEL; fep->reload_period = PPS_OUPUT_RELOAD_PERIOD; spin_lock_irqsave(&fep->tmreg_lock, flags);
Makes the PPS channel configurable via the Device Tree (on startup) and sysfs (run-time) Signed-off-by: Csókás Bence <csokas.bence@prolan.hu> --- drivers/net/ethernet/freescale/fec_main.c | 37 +++++++++++++++++++++++ drivers/net/ethernet/freescale/fec_ptp.c | 3 -- 2 files changed, 37 insertions(+), 3 deletions(-)