RE: [PATCH v7 net-next 07/10] net: fec: delete fec_ptp_disable_hwts()

[Wei Fang]

> -----Original Message-----
> From: Vladimir Oltean <vladimir.oltean@xxxxxxx>
> Sent: 2023年7月13日 20:19
> To: netdev@xxxxxxxxxxxxxxx
> Cc: David S. Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet
> <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni
> <pabeni@xxxxxxxxxx>; Andrew Lunn <andrew@xxxxxxx>; Florian Fainelli
> <f.fainelli@xxxxxxxxx>; Maxim Georgiev <glipus@xxxxxxxxx>; Horatiu Vultur
> <horatiu.vultur@xxxxxxxxxxxxx>; Köry Maincent
> <kory.maincent@xxxxxxxxxxx>; Maxime Chevallier
> <maxime.chevallier@xxxxxxxxxxx>; Richard Cochran
> <richardcochran@xxxxxxxxx>; Vadim Fedorenko
> <vadim.fedorenko@xxxxxxxxx>; Gerhard Engleder
> <gerhard@xxxxxxxxxxxxxxxxxxxxx>; Hangbin Liu <liuhangbin@xxxxxxxxx>;
> Russell King <linux@xxxxxxxxxxxxxxx>; Heiner Kallweit
> <hkallweit1@xxxxxxxxx>; Jacob Keller <jacob.e.keller@xxxxxxxxx>; Jay
> Vosburgh <j.vosburgh@xxxxxxxxx>; Andy Gospodarek <andy@xxxxxxxxxxxxx>;
> Wei Fang <wei.fang@xxxxxxx>; Shenwei Wang <shenwei.wang@xxxxxxx>;
> Clark Wang <xiaoning.wang@xxxxxxx>; dl-linux-imx <linux-imx@xxxxxxx>;
> UNGLinuxDriver@xxxxxxxxxxxxx; Lars Povlsen <lars.povlsen@xxxxxxxxxxxxx>;
> Steen Hegelund <Steen.Hegelund@xxxxxxxxxxxxx>; Daniel Machon
> <daniel.machon@xxxxxxxxxxxxx>; Simon Horman
> <simon.horman@xxxxxxxxxxxx>; Casper Andersson
> <casper.casan@xxxxxxxxx>; Sergey Organov <sorganov@xxxxxxxxx>;
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: [PATCH v7 net-next 07/10] net: fec: delete fec_ptp_disable_hwts()
> 
> Commit 340746398b67 ("net: fec: fix hardware time stamping by external
> devices") was overly cautious with calling fec_ptp_disable_hwts() when
> cmd == SIOCSHWTSTAMP and use_fec_hwts == false, because use_fec_hwts is
> based on a runtime invariant (phy_has_hwtstamp()). Thus, if use_fec_hwts
> is false, then fep->hwts_tx_en and fep->hwts_rx_en cannot be changed at
> runtime; their values depend on the initial memory allocation, which
> already sets them to zeroes.
> 
> If the core will ever gain support for switching timestamping layers,
> it will arrange for a more organized calling convention and disable
> timestamping in the previous layer as a first step. This means that the
> code in the FEC driver is not necessary in any case.
> 
> The purpose of this change is to arrange the phy_has_hwtstamp() code in
> a way in which it can be refactored away into generic logic.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@xxxxxxx>
> ---
> Changes in v7:
> - Patch is new
> 
>  drivers/net/ethernet/freescale/fec.h      |  1 -
>  drivers/net/ethernet/freescale/fec_main.c |  5 +----
>  drivers/net/ethernet/freescale/fec_ptp.c  | 12 ------------
>  3 files changed, 1 insertion(+), 17 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec.h
> b/drivers/net/ethernet/freescale/fec.h
> index d4ae0e7c0a44..e273129d44c9 100644
> --- a/drivers/net/ethernet/freescale/fec.h
> +++ b/drivers/net/ethernet/freescale/fec.h
> @@ -690,7 +690,6 @@ struct fec_enet_private {
>  void fec_ptp_init(struct platform_device *pdev, int irq_idx);
>  void fec_ptp_stop(struct platform_device *pdev);
>  void fec_ptp_start_cyclecounter(struct net_device *ndev);
> -void fec_ptp_disable_hwts(struct net_device *ndev);
>  int fec_ptp_set(struct net_device *ndev, struct kernel_hwtstamp_config
> *config,
>  		struct netlink_ext_ack *extack);
>  void fec_ptp_get(struct net_device *ndev, struct kernel_hwtstamp_config
> *config);
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index c0b68fc3ec8b..08744e8164e3 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3878,11 +3878,8 @@ static int fec_hwtstamp_set(struct net_device
> *ndev,
>  	struct fec_enet_private *fep = netdev_priv(ndev);
>  	struct phy_device *phydev = ndev->phydev;
> 
> -	if (phy_has_hwtstamp(phydev)) {
> -		fec_ptp_disable_hwts(ndev);
> -
> +	if (phy_has_hwtstamp(phydev))
>  		return phy_mii_ioctl(phydev, config->ifr, SIOCSHWTSTAMP);
> -	}
> 
>  	if (!fep->bufdesc_ex)
>  		return -EOPNOTSUPP;
> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c
> b/drivers/net/ethernet/freescale/fec_ptp.c
> index 3f53b8ae57dd..55cf98557e02 100644
> --- a/drivers/net/ethernet/freescale/fec_ptp.c
> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
> @@ -606,18 +606,6 @@ static int fec_ptp_enable(struct ptp_clock_info *ptp,
>  	}
>  }
> 
> -/**
> - * fec_ptp_disable_hwts - disable hardware time stamping
> - * @ndev: pointer to net_device
> - */
> -void fec_ptp_disable_hwts(struct net_device *ndev)
> -{
> -	struct fec_enet_private *fep = netdev_priv(ndev);
> -
> -	fep->hwts_tx_en = 0;
> -	fep->hwts_rx_en = 0;
> -}
> -
>  int fec_ptp_set(struct net_device *ndev, struct kernel_hwtstamp_config
> *config,
>  		struct netlink_ext_ack *extack)
>  {
> --
> 2.34.1

Thanks!
Reviewed-by: Wei Fang < wei.fang@xxxxxxx >