[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP5jrPE3wpVBHvyS-C4PN71QgKXrA5GVsa+D=RSaBOjEKnD2vw@mail.gmail.com>
Date: Wed, 26 Apr 2023 22:00:43 -0600
From: Max Georgiev <glipus@...il.com>
To: Köry Maincent <kory.maincent@...tlin.com>
Cc: kuba@...nel.org, netdev@...r.kernel.org,
maxime.chevallier@...tlin.com, vladimir.oltean@....com,
vadim.fedorenko@...ux.dev, richardcochran@...il.com,
gerhard@...leder-embedded.com, thomas.petazzoni@...tlin.com
Subject: Re: [RFC PATCH v4 0/5] New NDO methods ndo_hwtstamp_get/set
On Wed, Apr 26, 2023 at 10:58 AM Köry Maincent
<kory.maincent@...tlin.com> wrote:
>
> From: Kory Maincent <kory.maincent@...tlin.com>
>
> You patch series work on my side with the macb MAC controller and this
> patch.
> I don't know if you are waiting for more reviews but it seems good enough
> to drop the RFC tag.
>
> ---
>
> drivers/net/ethernet/cadence/macb.h | 10 ++++++--
> drivers/net/ethernet/cadence/macb_main.c | 15 ++++--------
> drivers/net/ethernet/cadence/macb_ptp.c | 30 ++++++++++++++----------
> 3 files changed, 29 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index cfbdd0022764..bc73b080093e 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -1350,8 +1350,14 @@ static inline void gem_ptp_do_rxstamp(struct macb *bp, struct sk_buff *skb, stru
>
> gem_ptp_rxstamp(bp, skb, desc);
> }
> -int gem_get_hwtst(struct net_device *dev, struct ifreq *rq);
> -int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd);
> +
> +int gem_get_hwtst(struct net_device *dev,
> + struct kernel_hwtstamp_config *kernel_config,
> + struct netlink_ext_ack *extack);
> +int gem_set_hwtst(struct net_device *dev,
> + struct kernel_hwtstamp_config *kernel_config,
> + struct netlink_ext_ack *extack);
> +
> #else
> static inline void gem_ptp_init(struct net_device *ndev) { }
> static inline void gem_ptp_remove(struct net_device *ndev) { }
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 45f63df5bdc4..c1d65be88835 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -3402,8 +3402,6 @@ static struct macb_ptp_info gem_ptp_info = {
> .get_ptp_max_adj = gem_get_ptp_max_adj,
> .get_tsu_rate = gem_get_tsu_rate,
> .get_ts_info = gem_get_ts_info,
> - .get_hwtst = gem_get_hwtst,
> - .set_hwtst = gem_set_hwtst,
> };
> #endif
>
> @@ -3764,15 +3762,6 @@ static int macb_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
> if (!netif_running(dev))
> return -EINVAL;
>
> - if (!phy_has_hwtstamp(dev->phydev) && bp->ptp_info) {
> - switch (cmd) {
> - case SIOCSHWTSTAMP:
> - return bp->ptp_info->set_hwtst(dev, rq, cmd);
> - case SIOCGHWTSTAMP:
> - return bp->ptp_info->get_hwtst(dev, rq);
> - }
> - }
> -
> return phylink_mii_ioctl(bp->phylink, rq, cmd);
> }
>
> @@ -3875,6 +3864,10 @@ static const struct net_device_ops macb_netdev_ops = {
> #endif
> .ndo_set_features = macb_set_features,
> .ndo_features_check = macb_features_check,
> +#ifdef CONFIG_MACB_USE_HWSTAMP
> + .ndo_hwtstamp_get = gem_get_hwtst,
> + .ndo_hwtstamp_set = gem_set_hwtst,
> +#endif
> };
>
> /* Configure peripheral capabilities according to device tree
> diff --git a/drivers/net/ethernet/cadence/macb_ptp.c b/drivers/net/ethernet/cadence/macb_ptp.c
> index 51d26fa190d7..eddacc5df435 100644
> --- a/drivers/net/ethernet/cadence/macb_ptp.c
> +++ b/drivers/net/ethernet/cadence/macb_ptp.c
> @@ -374,19 +374,22 @@ static int gem_ptp_set_ts_mode(struct macb *bp,
> return 0;
> }
>
> -int gem_get_hwtst(struct net_device *dev, struct ifreq *rq)
> +int gem_get_hwtst(struct net_device *dev,
> + struct kernel_hwtstamp_config *kernel_config,
> + struct netlink_ext_ack *extack)
> {
> struct hwtstamp_config *tstamp_config;
> struct macb *bp = netdev_priv(dev);
>
> + if (phy_has_hwtstamp(dev->phydev))
> + return phylink_mii_ioctl(bp->phylink, kernel_config->ifr, SIOCGHWTSTAMP);
> +
> tstamp_config = &bp->tstamp_config;
> if ((bp->hw_dma_cap & HW_DMA_CAP_PTP) == 0)
> return -EOPNOTSUPP;
>
> - if (copy_to_user(rq->ifr_data, tstamp_config, sizeof(*tstamp_config)))
> - return -EFAULT;
> - else
> - return 0;
> + hwtstamp_config_to_kernel(kernel_config, tstamp_config);
> + return 0;
> }
>
> static void gem_ptp_set_one_step_sync(struct macb *bp, u8 enable)
> @@ -401,7 +404,9 @@ static void gem_ptp_set_one_step_sync(struct macb *bp, u8 enable)
> macb_writel(bp, NCR, reg_val & ~MACB_BIT(OSSMODE));
> }
>
> -int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd)
> +int gem_set_hwtst(struct net_device *dev,
> + struct kernel_hwtstamp_config *kernel_config,
> + struct netlink_ext_ack *extack)
> {
> enum macb_bd_control tx_bd_control = TSTAMP_DISABLED;
> enum macb_bd_control rx_bd_control = TSTAMP_DISABLED;
> @@ -409,13 +414,14 @@ int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd)
> struct macb *bp = netdev_priv(dev);
> u32 regval;
>
> + if (phy_has_hwtstamp(dev->phydev))
> + return phylink_mii_ioctl(bp->phylink, kernel_config->ifr, SIOCSHWTSTAMP);
> +
> tstamp_config = &bp->tstamp_config;
> if ((bp->hw_dma_cap & HW_DMA_CAP_PTP) == 0)
> return -EOPNOTSUPP;
>
> - if (copy_from_user(tstamp_config, ifr->ifr_data,
> - sizeof(*tstamp_config)))
> - return -EFAULT;
> + hwtstamp_config_from_kernel(tstamp_config, kernel_config);
>
> switch (tstamp_config->tx_type) {
> case HWTSTAMP_TX_OFF:
> @@ -466,9 +472,7 @@ int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd)
> if (gem_ptp_set_ts_mode(bp, tx_bd_control, rx_bd_control) != 0)
> return -ERANGE;
>
> - if (copy_to_user(ifr->ifr_data, tstamp_config, sizeof(*tstamp_config)))
> - return -EFAULT;
> - else
> - return 0;
> + hwtstamp_config_to_kernel(kernel_config, tstamp_config);
> + return 0;
> }
>
> --
> 2.25.1
>
Thank you for giving it a try!
I'll drop the RFC tag starting from the next iteration.
Powered by blists - more mailing lists