mbox series

[v4,0/3] net: mvpp2: tai: add extts support

Message ID 20230430170656.137549-1-shmuel.h@siklu.com
Headers show
Series net: mvpp2: tai: add extts support | expand

Message

Shmuel Hazan April 30, 2023, 5:06 p.m. UTC
This patch series adds support for PTP event capture on the Aramda
80x0/70x0. This feature is mainly used by tools linux ts2phc(3) in order
to synchronize a timestamping unit (like the mvpp2's TAI) and a system
DPLL on the same PCB. 

The patch series includes 3 patches: the second one implements the
actual extts function.

Changes in v2:
	* Fixed a deadlock in the poll worker.
	* Removed tabs from comments.

Changes in v3:
	* Added more explanation about the change in behavior in mvpp22_tai_start.
	* Explain the reason for choosing 95ms as a polling rate.

Changes in v4:
	* Add additional lock for the polling worker reference count. 

Shmuel Hazan (3):
  net: mvpp2: tai: add refcount for ptp worker
  net: mvpp2: tai: add extts support
  dt-bindings: net: marvell,pp2: add extts docs

 .../devicetree/bindings/net/marvell,pp2.yaml  |  18 +
 .../net/ethernet/marvell/mvpp2/mvpp2_tai.c    | 332 ++++++++++++++++--
 2 files changed, 316 insertions(+), 34 deletions(-)


base-commit: 3e7bb4f2461710b70887704af7f175383251088e

Comments

Alexander Duyck May 1, 2023, 3:04 p.m. UTC | #1
On Sun, 2023-04-30 at 20:06 +0300, Shmuel Hazan wrote:
> In some configurations, a single TAI can be responsible for multiple
> mvpp2 interfaces. However, the mvpp2 driver will call mvpp22_tai_stop
> and mvpp22_tai_start per interface RX timestamp disable/enable.
> 
> As a result, disabling timestamping for one interface would stop the
> worker and corrupt the other interface's RX timestamps.
> 
> This commit solves the issue by introducing a simpler ref count for each
> TAI instance.
> 
> Due to the ref count, we need now to lock tai->refcount_lock before
> doing anything. As a result, we can't call mvpp22_tai_do_aux_work as it
> will cause a deadlock. Therefore, we will just schedule the worker to
> start immediately.
> 
> Fixes: ce3497e2072e ("net: mvpp2: ptp: add support for receive timestamping")
> Signed-off-by: Shmuel Hazan <shmuel.h@siklu.com>
> ---
> v1 -> v2: lock tai->lock before touching poll_worker_refcount.
> v2 -> v3: no change
> v3 -> v4: added additional lock for poll_worker_refcount due to
> 	  a possible deadlock.
> ---
>  .../net/ethernet/marvell/mvpp2/mvpp2_tai.c    | 28 +++++++++++++++++--
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 

So this patch looks fine to me. However you submitted this and the
other 2 for net. The other 2 patches in this series seem to be a
feature add rather than a fix. As such you may want to split up this
set and submit the other two patches for net-next instead of net.

Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
Russell King (Oracle) May 1, 2023, 7:45 p.m. UTC | #2
Hi,

I've been on a two week vacation, so I'm going to be catching up with a
lot of email - and I do want to review this before it's merged.

On Sun, Apr 30, 2023 at 08:06:53PM +0300, Shmuel Hazan wrote:
> This patch series adds support for PTP event capture on the Aramda
> 80x0/70x0. This feature is mainly used by tools linux ts2phc(3) in order
> to synchronize a timestamping unit (like the mvpp2's TAI) and a system
> DPLL on the same PCB. 
> 
> The patch series includes 3 patches: the second one implements the
> actual extts function.
> 
> Changes in v2:
> 	* Fixed a deadlock in the poll worker.
> 	* Removed tabs from comments.
> 
> Changes in v3:
> 	* Added more explanation about the change in behavior in mvpp22_tai_start.
> 	* Explain the reason for choosing 95ms as a polling rate.
> 
> Changes in v4:
> 	* Add additional lock for the polling worker reference count. 
> 
> Shmuel Hazan (3):
>   net: mvpp2: tai: add refcount for ptp worker
>   net: mvpp2: tai: add extts support
>   dt-bindings: net: marvell,pp2: add extts docs
> 
>  .../devicetree/bindings/net/marvell,pp2.yaml  |  18 +
>  .../net/ethernet/marvell/mvpp2/mvpp2_tai.c    | 332 ++++++++++++++++--
>  2 files changed, 316 insertions(+), 34 deletions(-)
> 
> 
> base-commit: 3e7bb4f2461710b70887704af7f175383251088e
> -- 
> 2.40.1
> 
>