diff mbox series

[net-next] net: phy: micrel: Add support for PTP_PF_PEROUT for lan8841

Message ID 20230217075213.2366042-1-horatiu.vultur@microchip.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: phy: micrel: Add support for PTP_PF_PEROUT for lan8841 | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 9 of 9 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: line length of 81 exceeds 80 columns WARNING: line length of 83 exceeds 80 columns WARNING: line length of 84 exceeds 80 columns WARNING: line length of 85 exceeds 80 columns WARNING: line length of 86 exceeds 80 columns WARNING: line length of 87 exceeds 80 columns WARNING: line length of 88 exceeds 80 columns WARNING: line length of 96 exceeds 80 columns WARNING: line length of 97 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Horatiu Vultur Feb. 17, 2023, 7:52 a.m. UTC
Lan8841 has 10 GPIOs and it has 2 events(EVENT_A and EVENT_B). It is
possible to assigned the 2 events to any of the GPIOs, but a GPIO can
have only 1 event at a time.
These events are used to generate periodic signals. It is possible to
configure the length, the start time and the period of the signal by
configuring the event.
Currently the SW uses only EVENT_A to generate the perout.

These events are generated by comparing the target time with the PHC
time. In case the PHC time is changed to a value bigger than the target
time + reload time, then it would generate only 1 event and then it
would stop because target time + reload time is small than PHC time.
Therefore it is required to change also the target time every time when
the PHC is changed. The same will apply also when the PHC time is
changed to a smaller value.

This was tested using:
testptp -L 6,2
testptp -p 1000000000 -w 200000000

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 drivers/net/phy/micrel.c | 363 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 363 insertions(+)

Comments

Russell King (Oracle) Feb. 17, 2023, 1:30 p.m. UTC | #1
On Fri, Feb 17, 2023 at 08:52:13AM +0100, Horatiu Vultur wrote:
> +static void lan8841_ptp_perout_off(struct kszphy_ptp_priv *ptp_priv, int pin)
> +{
> +	struct phy_device *phydev = ptp_priv->phydev;
> +	u16 tmp;
> +
> +	tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_EN) & LAN8841_PTP_GPIO_MASK;
> +	tmp &= ~BIT(pin);
> +	phy_write_mmd(phydev, 2, LAN8841_GPIO_EN, tmp);

Problem 1: doesn't check the return value of phy_read_mmd(), so a
spurious error results in an error code written back to the register.

Issue 2: please use phy_modify_mmd() and definitions for the MMD. It
probably also makes sense to cache the mask. Thus, this whole thing
becomes:

	u16 mask = ~(LAN8841_PTP_GPIO_MASK | BIT(pin));

	phy_modify_mmd(phydev, MDIO_MMD_WIS, LAN8841_GPIO_EN, mask, 0);
	phy_modify_mmd(phydev, MDIO_MMD_WIS, LAN8841_GPIO_DIR, mask, 0);
	phy_modify_mmd(phydev, MDIO_MMD_WIS, LAN8841_GPIO_BUF, mask, 0);

although I'm not sure why you need to mask off bits 15:11.

> +
> +	tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_DIR) & LAN8841_PTP_GPIO_MASK;
> +	tmp &= ~BIT(pin);
> +	phy_write_mmd(phydev, 2, LAN8841_GPIO_DIR, tmp);
> +
> +	tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_BUF) & LAN8841_PTP_GPIO_MASK;
> +	tmp &= ~BIT(pin);
> +	phy_write_mmd(phydev, 2, LAN8841_GPIO_BUF, tmp);
> +}
> +
> +static void lan8841_ptp_perout_on(struct kszphy_ptp_priv *ptp_priv, int pin)
> +{
> +	struct phy_device *phydev = ptp_priv->phydev;
> +	u16 tmp;
> +
> +	tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_EN) & LAN8841_PTP_GPIO_MASK;
> +	tmp |= BIT(pin);
> +	phy_write_mmd(phydev, 2, LAN8841_GPIO_EN, tmp);
> +
> +	tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_DIR) & LAN8841_PTP_GPIO_MASK;
> +	tmp |= BIT(pin);
> +	phy_write_mmd(phydev, 2, LAN8841_GPIO_DIR, tmp);
> +
> +	tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_BUF) & LAN8841_PTP_GPIO_MASK;
> +	tmp |= BIT(pin);
> +	phy_write_mmd(phydev, 2, LAN8841_GPIO_BUF, tmp);

Similar as above.

> +static void lan8841_ptp_remove_event(struct kszphy_ptp_priv *ptp_priv, int pin,
> +				     u8 event)
> +{
> +	struct phy_device *phydev = ptp_priv->phydev;
> +	u8 offset;
> +	u16 tmp;
> +
> +	/* Not remove pin from the event. GPIO_DATA_SEL1 contains the GPIO
> +	 * pins 0-4 while GPIO_DATA_SEL2 contains GPIO pins 5-9, therefore
> +	 * depending on the pin, it requires to read a different register
> +	 */
> +	if (pin < 5) {
> +		tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_DATA_SEL1);
> +		offset = pin;
> +	} else {
> +		tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_DATA_SEL2);
> +		offset = pin - 5;
> +	}
> +	tmp &= ~(LAN8841_GPIO_DATA_SEL_GPIO_DATA_SEL_EVENT_MASK << (3 * offset));
> +	if (pin < 5)
> +		phy_write_mmd(phydev, 2, LAN8841_GPIO_DATA_SEL1, tmp);
> +	else
> +		phy_write_mmd(phydev, 2, LAN8841_GPIO_DATA_SEL2, tmp);

This could be much simpler using phy_modify_mmd().

> +
> +	/* Disable the event */
> +	tmp = phy_read_mmd(phydev, 2, LAN8841_PTP_GENERAL_CONFIG);
> +	if (event == LAN8841_EVENT_A) {
> +		tmp &= ~LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_POL_A;
> +		tmp &= ~LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_A_MASK;
> +	} else {
> +		tmp &= ~LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_POL_A;
> +		tmp &= ~LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_A_MASK;
> +	}
> +	phy_write_mmd(phydev, 2, LAN8841_PTP_GENERAL_CONFIG, tmp);

Ditto... and the theme seems to continue throughout the rest of this
patch.

> +static int lan8841_ptp_perout(struct ptp_clock_info *ptp,
> +			      struct ptp_clock_request *rq, int on)
> +{
> +	struct kszphy_ptp_priv *ptp_priv = container_of(ptp, struct kszphy_ptp_priv,
> +							ptp_clock_info);
> +	struct phy_device *phydev = ptp_priv->phydev;
> +	struct timespec64 ts_on, ts_period;
> +	s64 on_nsec, period_nsec;
> +	int pulse_width;
> +	int pin;
> +
> +	if (rq->perout.flags & ~PTP_PEROUT_DUTY_CYCLE)
> +		return -EOPNOTSUPP;
> +
> +	pin = ptp_find_pin(ptp_priv->ptp_clock, PTP_PF_PEROUT, rq->perout.index);
> +	if (pin == -1 || pin >= LAN8841_PTP_GPIO_NUM)
> +		return -EINVAL;
> +
> +	if (!on) {
> +		lan8841_ptp_perout_off(ptp_priv, pin);
> +		lan8841_ptp_remove_event(ptp_priv, LAN8841_EVENT_A, pin);
> +		return 0;
> +	}
> +
> +	ts_on.tv_sec = rq->perout.on.sec;
> +	ts_on.tv_nsec = rq->perout.on.nsec;
> +	on_nsec = timespec64_to_ns(&ts_on);
> +
> +	ts_period.tv_sec = rq->perout.period.sec;
> +	ts_period.tv_nsec = rq->perout.period.nsec;
> +	period_nsec = timespec64_to_ns(&ts_period);
> +
> +	if (period_nsec < 200) {
> +		phydev_warn(phydev,
> +			    "perout period too small, minimum is 200 nsec\n");

I'm not sure using the kernel log to print such things is a good idea,
especially without rate limiting.

> @@ -3874,7 +4220,24 @@ static int lan8841_probe(struct phy_device *phydev)
>  	priv = phydev->priv;
>  	ptp_priv = &priv->ptp_priv;
>  
> +	ptp_priv->pin_config = devm_kmalloc_array(&phydev->mdio.dev,
> +						  LAN8841_PTP_GPIO_NUM,
> +						  sizeof(*ptp_priv->pin_config),
> +						  GFP_KERNEL);

devm_kcalloc() to avoid the memset() below?
Russell King (Oracle) Feb. 17, 2023, 1:33 p.m. UTC | #2
On Fri, Feb 17, 2023 at 01:30:06PM +0000, Russell King (Oracle) wrote:
> On Fri, Feb 17, 2023 at 08:52:13AM +0100, Horatiu Vultur wrote:
> > +static void lan8841_ptp_perout_off(struct kszphy_ptp_priv *ptp_priv, int pin)
> > +{
> > +	struct phy_device *phydev = ptp_priv->phydev;
> > +	u16 tmp;
> > +
> > +	tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_EN) & LAN8841_PTP_GPIO_MASK;
> > +	tmp &= ~BIT(pin);
> > +	phy_write_mmd(phydev, 2, LAN8841_GPIO_EN, tmp);
> 
> Problem 1: doesn't check the return value of phy_read_mmd(), so a
> spurious error results in an error code written back to the register.
> 
> Issue 2: please use phy_modify_mmd() and definitions for the MMD. It
> probably also makes sense to cache the mask. Thus, this whole thing
> becomes:
> 
> 	u16 mask = ~(LAN8841_PTP_GPIO_MASK | BIT(pin));
> 
> 	phy_modify_mmd(phydev, MDIO_MMD_WIS, LAN8841_GPIO_EN, mask, 0);
> 	phy_modify_mmd(phydev, MDIO_MMD_WIS, LAN8841_GPIO_DIR, mask, 0);
> 	phy_modify_mmd(phydev, MDIO_MMD_WIS, LAN8841_GPIO_BUF, mask, 0);
> 
> although I'm not sure why you need to mask off bits 15:11.

Or even use phy_clear_bits_mmd(). There's also phy_set_bits_mmd() which I
think would also be useful elsewhere in this driver.
Horatiu Vultur Feb. 17, 2023, 3:02 p.m. UTC | #3
The 02/17/2023 13:30, Russell King (Oracle) wrote:

Hi Russel,

> 
> On Fri, Feb 17, 2023 at 08:52:13AM +0100, Horatiu Vultur wrote:
> > +static void lan8841_ptp_perout_off(struct kszphy_ptp_priv *ptp_priv, int pin)
> > +{
> > +     struct phy_device *phydev = ptp_priv->phydev;
> > +     u16 tmp;
> > +
> > +     tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_EN) & LAN8841_PTP_GPIO_MASK;
> > +     tmp &= ~BIT(pin);
> > +     phy_write_mmd(phydev, 2, LAN8841_GPIO_EN, tmp);
> 
> Problem 1: doesn't check the return value of phy_read_mmd(), so a
> spurious error results in an error code written back to the register.
> 
> Issue 2: please use phy_modify_mmd() and definitions for the MMD. It
> probably also makes sense to cache the mask. Thus, this whole thing
> becomes:
> 
>         u16 mask = ~(LAN8841_PTP_GPIO_MASK | BIT(pin));
> 
>         phy_modify_mmd(phydev, MDIO_MMD_WIS, LAN8841_GPIO_EN, mask, 0);
>         phy_modify_mmd(phydev, MDIO_MMD_WIS, LAN8841_GPIO_DIR, mask, 0);
>         phy_modify_mmd(phydev, MDIO_MMD_WIS, LAN8841_GPIO_BUF, mask, 0);

Thanks for the review.
I will look at phy_modify_mmd and the other helper functions and try to
use them in the other perout functions.

> 
> although I'm not sure why you need to mask off bits 15:11.

It is not necessary, only that those bits are marked as reserved.

> 
> > +
> > +     tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_DIR) & LAN8841_PTP_GPIO_MASK;
> > +     tmp &= ~BIT(pin);
> > +     phy_write_mmd(phydev, 2, LAN8841_GPIO_DIR, tmp);
> > +
> > +     tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_BUF) & LAN8841_PTP_GPIO_MASK;
> > +     tmp &= ~BIT(pin);
> > +     phy_write_mmd(phydev, 2, LAN8841_GPIO_BUF, tmp);
> > +}
> > +
> > +static void lan8841_ptp_perout_on(struct kszphy_ptp_priv *ptp_priv, int pin)
> > +{
> > +     struct phy_device *phydev = ptp_priv->phydev;
> > +     u16 tmp;
> > +
> > +     tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_EN) & LAN8841_PTP_GPIO_MASK;
> > +     tmp |= BIT(pin);
> > +     phy_write_mmd(phydev, 2, LAN8841_GPIO_EN, tmp);
> > +
> > +     tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_DIR) & LAN8841_PTP_GPIO_MASK;
> > +     tmp |= BIT(pin);
> > +     phy_write_mmd(phydev, 2, LAN8841_GPIO_DIR, tmp);
> > +
> > +     tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_BUF) & LAN8841_PTP_GPIO_MASK;
> > +     tmp |= BIT(pin);
> > +     phy_write_mmd(phydev, 2, LAN8841_GPIO_BUF, tmp);
> 
> Similar as above.
> 
> > +static void lan8841_ptp_remove_event(struct kszphy_ptp_priv *ptp_priv, int pin,
> > +                                  u8 event)
> > +{
> > +     struct phy_device *phydev = ptp_priv->phydev;
> > +     u8 offset;
> > +     u16 tmp;
> > +
> > +     /* Not remove pin from the event. GPIO_DATA_SEL1 contains the GPIO
> > +      * pins 0-4 while GPIO_DATA_SEL2 contains GPIO pins 5-9, therefore
> > +      * depending on the pin, it requires to read a different register
> > +      */
> > +     if (pin < 5) {
> > +             tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_DATA_SEL1);
> > +             offset = pin;
> > +     } else {
> > +             tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_DATA_SEL2);
> > +             offset = pin - 5;
> > +     }
> > +     tmp &= ~(LAN8841_GPIO_DATA_SEL_GPIO_DATA_SEL_EVENT_MASK << (3 * offset));
> > +     if (pin < 5)
> > +             phy_write_mmd(phydev, 2, LAN8841_GPIO_DATA_SEL1, tmp);
> > +     else
> > +             phy_write_mmd(phydev, 2, LAN8841_GPIO_DATA_SEL2, tmp);
> 
> This could be much simpler using phy_modify_mmd().
> 
> > +
> > +     /* Disable the event */
> > +     tmp = phy_read_mmd(phydev, 2, LAN8841_PTP_GENERAL_CONFIG);
> > +     if (event == LAN8841_EVENT_A) {
> > +             tmp &= ~LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_POL_A;
> > +             tmp &= ~LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_A_MASK;
> > +     } else {
> > +             tmp &= ~LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_POL_A;
> > +             tmp &= ~LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_A_MASK;
> > +     }
> > +     phy_write_mmd(phydev, 2, LAN8841_PTP_GENERAL_CONFIG, tmp);
> 
> Ditto... and the theme seems to continue throughout the rest of this
> patch.
> 
> > +static int lan8841_ptp_perout(struct ptp_clock_info *ptp,
> > +                           struct ptp_clock_request *rq, int on)
> > +{
> > +     struct kszphy_ptp_priv *ptp_priv = container_of(ptp, struct kszphy_ptp_priv,
> > +                                                     ptp_clock_info);
> > +     struct phy_device *phydev = ptp_priv->phydev;
> > +     struct timespec64 ts_on, ts_period;
> > +     s64 on_nsec, period_nsec;
> > +     int pulse_width;
> > +     int pin;
> > +
> > +     if (rq->perout.flags & ~PTP_PEROUT_DUTY_CYCLE)
> > +             return -EOPNOTSUPP;
> > +
> > +     pin = ptp_find_pin(ptp_priv->ptp_clock, PTP_PF_PEROUT, rq->perout.index);
> > +     if (pin == -1 || pin >= LAN8841_PTP_GPIO_NUM)
> > +             return -EINVAL;
> > +
> > +     if (!on) {
> > +             lan8841_ptp_perout_off(ptp_priv, pin);
> > +             lan8841_ptp_remove_event(ptp_priv, LAN8841_EVENT_A, pin);
> > +             return 0;
> > +     }
> > +
> > +     ts_on.tv_sec = rq->perout.on.sec;
> > +     ts_on.tv_nsec = rq->perout.on.nsec;
> > +     on_nsec = timespec64_to_ns(&ts_on);
> > +
> > +     ts_period.tv_sec = rq->perout.period.sec;
> > +     ts_period.tv_nsec = rq->perout.period.nsec;
> > +     period_nsec = timespec64_to_ns(&ts_period);
> > +
> > +     if (period_nsec < 200) {
> > +             phydev_warn(phydev,
> > +                         "perout period too small, minimum is 200 nsec\n");
> 
> I'm not sure using the kernel log to print such things is a good idea,
> especially without rate limiting.

I think it would be nice to have these warnings as it would be nice to
know why it fails. I will use pr_warn_ratelimited in the next version.

> 
> > @@ -3874,7 +4220,24 @@ static int lan8841_probe(struct phy_device *phydev)
> >       priv = phydev->priv;
> >       ptp_priv = &priv->ptp_priv;
> >
> > +     ptp_priv->pin_config = devm_kmalloc_array(&phydev->mdio.dev,
> > +                                               LAN8841_PTP_GPIO_NUM,
> > +                                               sizeof(*ptp_priv->pin_config),
> > +                                               GFP_KERNEL);
> 
> devm_kcalloc() to avoid the memset() below?

Good point. I will do that.

> 
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Andrew Lunn Feb. 18, 2023, 12:55 a.m. UTC | #4
> +static void lan8841_ptp_update_target(struct kszphy_ptp_priv *ptp_priv,
> +				      const struct timespec64 *ts);
> +

Please avoid this. Move the code around so everything is in
order. Generally, i do such moves in an initial patch which only moves
code, making it easy to review.

      Andrew
Horatiu Vultur Feb. 18, 2023, 12:10 p.m. UTC | #5
The 02/18/2023 01:55, Andrew Lunn wrote:
> 
> > +static void lan8841_ptp_update_target(struct kszphy_ptp_priv *ptp_priv,
> > +                                   const struct timespec64 *ts);
> > +
> 
> Please avoid this. Move the code around so everything is in
> order. Generally, i do such moves in an initial patch which only moves
> code, making it easy to review.

Thanks for the suggestion. I will update this in the next version.

> 
>       Andrew
diff mbox series

Patch

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 2c84fccef4f64..ac818f1381942 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -318,6 +318,7 @@  struct kszphy_ptp_priv {
 	struct ptp_clock_info ptp_clock_info;
 	/* Lock for ptp_clock */
 	struct mutex ptp_lock;
+	struct ptp_pin_desc *pin_config;
 };
 
 struct kszphy_priv {
@@ -3665,6 +3666,9 @@  static int lan8841_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
 #define LAN8841_PTP_LTC_SET_NS_LO	266
 #define LAN8841_PTP_CMD_CTL_PTP_LTC_LOAD	BIT(4)
 
+static void lan8841_ptp_update_target(struct kszphy_ptp_priv *ptp_priv,
+				      const struct timespec64 *ts);
+
 static int lan8841_ptp_settime64(struct ptp_clock_info *ptp,
 				 const struct timespec64 *ts)
 {
@@ -3683,6 +3687,7 @@  static int lan8841_ptp_settime64(struct ptp_clock_info *ptp,
 	/* Set the command to load the LTC */
 	phy_write_mmd(phydev, 2, LAN8841_PTP_CMD_CTL,
 		      LAN8841_PTP_CMD_CTL_PTP_LTC_LOAD);
+	lan8841_ptp_update_target(ptp_priv, ts);
 	mutex_unlock(&ptp_priv->ptp_lock);
 
 	return 0;
@@ -3803,6 +3808,12 @@  static int lan8841_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 	}
 	mutex_unlock(&ptp_priv->ptp_lock);
 
+	/* Update the target clock */
+	ptp->gettime64(ptp, &ts);
+	mutex_lock(&ptp_priv->ptp_lock);
+	lan8841_ptp_update_target(ptp_priv, &ts);
+	mutex_unlock(&ptp_priv->ptp_lock);
+
 	return 0;
 }
 
@@ -3839,6 +3850,337 @@  static int lan8841_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
 	return 0;
 }
 
+static int lan8841_ptp_verify(struct ptp_clock_info *ptp, unsigned int pin,
+			      enum ptp_pin_function func, unsigned int chan)
+{
+	switch (func) {
+	case PTP_PF_NONE:
+	case PTP_PF_PEROUT:
+		break;
+	default:
+		return -1;
+	}
+
+	return 0;
+}
+
+#define LAN8841_PTP_GPIO_NUM	10
+#define LAN8841_PTP_GPIO_MASK   GENMASK(LAN8841_PTP_GPIO_NUM, 0)
+#define LAN8841_GPIO_EN		128
+#define LAN8841_GPIO_DIR	129
+#define LAN8841_GPIO_BUF	130
+
+static void lan8841_ptp_perout_off(struct kszphy_ptp_priv *ptp_priv, int pin)
+{
+	struct phy_device *phydev = ptp_priv->phydev;
+	u16 tmp;
+
+	tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_EN) & LAN8841_PTP_GPIO_MASK;
+	tmp &= ~BIT(pin);
+	phy_write_mmd(phydev, 2, LAN8841_GPIO_EN, tmp);
+
+	tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_DIR) & LAN8841_PTP_GPIO_MASK;
+	tmp &= ~BIT(pin);
+	phy_write_mmd(phydev, 2, LAN8841_GPIO_DIR, tmp);
+
+	tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_BUF) & LAN8841_PTP_GPIO_MASK;
+	tmp &= ~BIT(pin);
+	phy_write_mmd(phydev, 2, LAN8841_GPIO_BUF, tmp);
+}
+
+static void lan8841_ptp_perout_on(struct kszphy_ptp_priv *ptp_priv, int pin)
+{
+	struct phy_device *phydev = ptp_priv->phydev;
+	u16 tmp;
+
+	tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_EN) & LAN8841_PTP_GPIO_MASK;
+	tmp |= BIT(pin);
+	phy_write_mmd(phydev, 2, LAN8841_GPIO_EN, tmp);
+
+	tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_DIR) & LAN8841_PTP_GPIO_MASK;
+	tmp |= BIT(pin);
+	phy_write_mmd(phydev, 2, LAN8841_GPIO_DIR, tmp);
+
+	tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_BUF) & LAN8841_PTP_GPIO_MASK;
+	tmp |= BIT(pin);
+	phy_write_mmd(phydev, 2, LAN8841_GPIO_BUF, tmp);
+}
+
+#define LAN8841_GPIO_DATA_SEL1				131
+#define LAN8841_GPIO_DATA_SEL2				132
+#define LAN8841_GPIO_DATA_SEL_GPIO_DATA_SEL_EVENT_MASK	GENMASK(2, 0)
+#define LAN8841_GPIO_DATA_SEL_GPIO_DATA_SEL_EVENT_A	1
+#define LAN8841_GPIO_DATA_SEL_GPIO_DATA_SEL_EVENT_B	2
+#define LAN8841_PTP_GENERAL_CONFIG			257
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_POL_A	BIT(1)
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_POL_B	BIT(3)
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_A_MASK	GENMASK(7, 4)
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_B_MASK	GENMASK(11, 8)
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_A		4
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_B		7
+
+#define LAN8841_EVENT_A		0
+#define LAN8841_EVENT_B		1
+
+static void lan8841_ptp_remove_event(struct kszphy_ptp_priv *ptp_priv, int pin,
+				     u8 event)
+{
+	struct phy_device *phydev = ptp_priv->phydev;
+	u8 offset;
+	u16 tmp;
+
+	/* Not remove pin from the event. GPIO_DATA_SEL1 contains the GPIO
+	 * pins 0-4 while GPIO_DATA_SEL2 contains GPIO pins 5-9, therefore
+	 * depending on the pin, it requires to read a different register
+	 */
+	if (pin < 5) {
+		tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_DATA_SEL1);
+		offset = pin;
+	} else {
+		tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_DATA_SEL2);
+		offset = pin - 5;
+	}
+	tmp &= ~(LAN8841_GPIO_DATA_SEL_GPIO_DATA_SEL_EVENT_MASK << (3 * offset));
+	if (pin < 5)
+		phy_write_mmd(phydev, 2, LAN8841_GPIO_DATA_SEL1, tmp);
+	else
+		phy_write_mmd(phydev, 2, LAN8841_GPIO_DATA_SEL2, tmp);
+
+	/* Disable the event */
+	tmp = phy_read_mmd(phydev, 2, LAN8841_PTP_GENERAL_CONFIG);
+	if (event == LAN8841_EVENT_A) {
+		tmp &= ~LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_POL_A;
+		tmp &= ~LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_A_MASK;
+	} else {
+		tmp &= ~LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_POL_A;
+		tmp &= ~LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_A_MASK;
+	}
+	phy_write_mmd(phydev, 2, LAN8841_PTP_GENERAL_CONFIG, tmp);
+}
+
+static void lan8841_ptp_enable_event(struct kszphy_ptp_priv *ptp_priv, int pin,
+				     u8 event, int pulse_width)
+{
+	struct phy_device *phydev = ptp_priv->phydev;
+	u8 offset;
+	u16 tmp;
+
+	/* Enable the event */
+	tmp = phy_read_mmd(phydev, 2, LAN8841_PTP_GENERAL_CONFIG);
+	tmp |= event == LAN8841_EVENT_A ? LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_POL_A :
+					  LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_POL_B;
+	tmp &= event == LAN8841_EVENT_A ? ~LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_A_MASK :
+					  ~LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_B_MASK;
+	tmp |= event == LAN8841_EVENT_A ? pulse_width << LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_A :
+					  pulse_width << LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_B;
+	phy_write_mmd(phydev, 2, LAN8841_PTP_GENERAL_CONFIG, tmp);
+
+	/* Now connect the pin to the event. GPIO_DATA_SEL1 contains the GPIO
+	 * pins 0-4 while GPIO_DATA_SEL2 contains GPIO pins 5-9, therefore
+	 * depending on the pin, it requires to read a different register
+	 */
+	if (pin < 5) {
+		tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_DATA_SEL1);
+		offset = pin;
+	} else {
+		tmp = phy_read_mmd(phydev, 2, LAN8841_GPIO_DATA_SEL2);
+		offset = pin - 5;
+	}
+
+	if (event == LAN8841_EVENT_A)
+		tmp |= LAN8841_GPIO_DATA_SEL_GPIO_DATA_SEL_EVENT_A << (3 * offset);
+	else
+		tmp |= LAN8841_GPIO_DATA_SEL_GPIO_DATA_SEL_EVENT_B << (3 * offset);
+
+	if (pin < 5)
+		phy_write_mmd(phydev, 2, LAN8841_GPIO_DATA_SEL1, tmp);
+	else
+		phy_write_mmd(phydev, 2, LAN8841_GPIO_DATA_SEL2, tmp);
+}
+
+#define LAN8841_PTP_LTC_TARGET_SEC_HI(event)	((event) == LAN8841_EVENT_A ? 278 : 288)
+#define LAN8841_PTP_LTC_TARGET_SEC_LO(event)	((event) == LAN8841_EVENT_A ? 279 : 289)
+#define LAN8841_PTP_LTC_TARGET_NS_HI(event)	((event) == LAN8841_EVENT_A ? 280 : 290)
+#define LAN8841_PTP_LTC_TARGET_NS_LO(event)	((event) == LAN8841_EVENT_A ? 281 : 291)
+
+static void lan8841_ptp_set_target(struct kszphy_ptp_priv *ptp_priv, u8 event,
+				   s64 sec, u32 nsec)
+{
+	struct phy_device *phydev = ptp_priv->phydev;
+
+	phy_write_mmd(phydev, 2, LAN8841_PTP_LTC_TARGET_SEC_HI(event),
+		      upper_16_bits(sec));
+	phy_write_mmd(phydev, 2, LAN8841_PTP_LTC_TARGET_SEC_LO(event),
+		      lower_16_bits(sec));
+	phy_write_mmd(phydev, 2, LAN8841_PTP_LTC_TARGET_NS_HI(event) & 0x3fff,
+		      upper_16_bits(nsec));
+	phy_write_mmd(phydev, 2, LAN8841_PTP_LTC_TARGET_NS_LO(event),
+		      lower_16_bits(nsec));
+}
+
+#define LAN8841_BUFFER_TIME	2
+
+static void lan8841_ptp_update_target(struct kszphy_ptp_priv *ptp_priv,
+				      const struct timespec64 *ts)
+{
+	lan8841_ptp_set_target(ptp_priv, LAN8841_EVENT_A,
+			       ts->tv_sec + LAN8841_BUFFER_TIME, ts->tv_nsec);
+}
+
+#define LAN8841_PTP_LTC_TARGET_RELOAD_SEC_HI(event)	((event) == LAN8841_EVENT_A ? 282 : 292)
+#define LAN8841_PTP_LTC_TARGET_RELOAD_SEC_LO(event)	((event) == LAN8841_EVENT_A ? 283 : 293)
+#define LAN8841_PTP_LTC_TARGET_RELOAD_NS_HI(event)	((event) == LAN8841_EVENT_A ? 284 : 294)
+#define LAN8841_PTP_LTC_TARGET_RELOAD_NS_LO(event)	((event) == LAN8841_EVENT_A ? 285 : 295)
+
+static void lan8841_ptp_set_reload(struct kszphy_ptp_priv *ptp_priv, u8 event,
+				   s64 sec, u32 nsec)
+{
+	struct phy_device *phydev = ptp_priv->phydev;
+
+	phy_write_mmd(phydev, 2, LAN8841_PTP_LTC_TARGET_RELOAD_SEC_HI(event),
+		      upper_16_bits(sec));
+	phy_write_mmd(phydev, 2, LAN8841_PTP_LTC_TARGET_RELOAD_SEC_LO(event),
+		      lower_16_bits(sec));
+	phy_write_mmd(phydev, 2, LAN8841_PTP_LTC_TARGET_RELOAD_NS_HI(event) & 0x3fff,
+		      upper_16_bits(nsec));
+	phy_write_mmd(phydev, 2, LAN8841_PTP_LTC_TARGET_RELOAD_NS_LO(event),
+		      lower_16_bits(nsec));
+}
+
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_200MS	13
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_100MS	12
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_50MS	11
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_10MS	10
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_5MS	9
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_1MS	8
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_500US	7
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_100US	6
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_50US	5
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_10US	4
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_5US	3
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_1US	2
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_500NS	1
+#define LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_100NS	0
+
+static int lan8841_ptp_perout(struct ptp_clock_info *ptp,
+			      struct ptp_clock_request *rq, int on)
+{
+	struct kszphy_ptp_priv *ptp_priv = container_of(ptp, struct kszphy_ptp_priv,
+							ptp_clock_info);
+	struct phy_device *phydev = ptp_priv->phydev;
+	struct timespec64 ts_on, ts_period;
+	s64 on_nsec, period_nsec;
+	int pulse_width;
+	int pin;
+
+	if (rq->perout.flags & ~PTP_PEROUT_DUTY_CYCLE)
+		return -EOPNOTSUPP;
+
+	pin = ptp_find_pin(ptp_priv->ptp_clock, PTP_PF_PEROUT, rq->perout.index);
+	if (pin == -1 || pin >= LAN8841_PTP_GPIO_NUM)
+		return -EINVAL;
+
+	if (!on) {
+		lan8841_ptp_perout_off(ptp_priv, pin);
+		lan8841_ptp_remove_event(ptp_priv, LAN8841_EVENT_A, pin);
+		return 0;
+	}
+
+	ts_on.tv_sec = rq->perout.on.sec;
+	ts_on.tv_nsec = rq->perout.on.nsec;
+	on_nsec = timespec64_to_ns(&ts_on);
+
+	ts_period.tv_sec = rq->perout.period.sec;
+	ts_period.tv_nsec = rq->perout.period.nsec;
+	period_nsec = timespec64_to_ns(&ts_period);
+
+	if (period_nsec < 200) {
+		phydev_warn(phydev,
+			    "perout period too small, minimum is 200 nsec\n");
+		return -EOPNOTSUPP;
+	}
+
+	if (on_nsec >= period_nsec) {
+		phydev_warn(phydev,
+			    "pulse width must be smaller than period\n");
+		return -EINVAL;
+	}
+
+	switch (on_nsec) {
+	case 200000000:
+		pulse_width = LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_200MS;
+		break;
+	case 100000000:
+		pulse_width = LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_100MS;
+		break;
+	case 50000000:
+		pulse_width = LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_50MS;
+		break;
+	case 10000000:
+		pulse_width = LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_10MS;
+		break;
+	case 5000000:
+		pulse_width = LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_5MS;
+		break;
+	case 1000000:
+		pulse_width = LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_1MS;
+		break;
+	case 500000:
+		pulse_width = LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_500US;
+		break;
+	case 100000:
+		pulse_width = LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_100US;
+		break;
+	case 50000:
+		pulse_width = LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_50US;
+		break;
+	case 10000:
+		pulse_width = LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_10US;
+		break;
+	case 5000:
+		pulse_width = LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_5US;
+		break;
+	case 1000:
+		pulse_width = LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_1US;
+		break;
+	case 500:
+		pulse_width = LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_500NS;
+		break;
+	case 100:
+		pulse_width = LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_100NS;
+		break;
+	default:
+		phydev_warn(phydev, "Use default duty cycle of 100ns\n");
+		pulse_width = LAN8841_PTP_GENERAL_CONFIG_LTC_EVENT_100NS;
+		break;
+	}
+
+	mutex_lock(&ptp_priv->ptp_lock);
+	lan8841_ptp_set_target(ptp_priv, LAN8841_EVENT_A, rq->perout.start.sec,
+			       rq->perout.start.nsec);
+	mutex_unlock(&ptp_priv->ptp_lock);
+
+	lan8841_ptp_set_reload(ptp_priv, LAN8841_EVENT_A, rq->perout.period.sec,
+			       rq->perout.period.nsec);
+	lan8841_ptp_enable_event(ptp_priv, pin, LAN8841_EVENT_A, pulse_width);
+	lan8841_ptp_perout_on(ptp_priv, pin);
+
+	return 0;
+}
+
+static int lan8841_ptp_enable(struct ptp_clock_info *ptp,
+			      struct ptp_clock_request *rq, int on)
+{
+	switch (rq->type) {
+	case PTP_CLK_REQ_PEROUT:
+		return lan8841_ptp_perout(ptp, rq, on);
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
 static struct ptp_clock_info lan8841_ptp_clock_info = {
 	.owner		= THIS_MODULE,
 	.name		= "lan8841 ptp",
@@ -3847,6 +4189,10 @@  static struct ptp_clock_info lan8841_ptp_clock_info = {
 	.settime64	= lan8841_ptp_settime64,
 	.adjtime	= lan8841_ptp_adjtime,
 	.adjfine	= lan8841_ptp_adjfine,
+	.verify         = lan8841_ptp_verify,
+	.enable         = lan8841_ptp_enable,
+	.n_per_out      = LAN8841_PTP_GPIO_NUM,
+	.n_pins         = LAN8841_PTP_GPIO_NUM,
 };
 
 #define LAN8841_OPERATION_MODE_STRAP_LOW_REGISTER 3
@@ -3874,7 +4220,24 @@  static int lan8841_probe(struct phy_device *phydev)
 	priv = phydev->priv;
 	ptp_priv = &priv->ptp_priv;
 
+	ptp_priv->pin_config = devm_kmalloc_array(&phydev->mdio.dev,
+						  LAN8841_PTP_GPIO_NUM,
+						  sizeof(*ptp_priv->pin_config),
+						  GFP_KERNEL);
+	if (!ptp_priv->pin_config)
+		return -ENOMEM;
+
+	for (int i = 0; i < LAN8841_PTP_GPIO_NUM; ++i) {
+		struct ptp_pin_desc *p = &ptp_priv->pin_config[i];
+
+		memset(p, 0, sizeof(*p));
+		snprintf(p->name, sizeof(p->name), "pin%d", i);
+		p->index = i;
+		p->func = PTP_PF_NONE;
+	}
+
 	ptp_priv->ptp_clock_info = lan8841_ptp_clock_info;
+	ptp_priv->ptp_clock_info.pin_config = ptp_priv->pin_config;
 	ptp_priv->ptp_clock = ptp_clock_register(&ptp_priv->ptp_clock_info,
 						 &phydev->mdio.dev);
 	if (IS_ERR(ptp_priv->ptp_clock)) {