On Thu, Apr 01, 2021 at 10:56:03AM -0700, Shannon Nelson wrote:
> @@ -0,0 +1,589 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright(c) 2017 - 2021 Pensando Systems, Inc */
> +
> +#include <linux/netdevice.h>
> +#include <linux/etherdevice.h>
> +
> +#include "ionic.h"
> +#include "ionic_bus.h"
> +#include "ionic_lif.h"
> +#include "ionic_ethtool.h"
> +
> +static int ionic_hwstamp_tx_mode(int config_tx_type)
> +{
> + switch (config_tx_type) {
> + case HWTSTAMP_TX_OFF:
> + return IONIC_TXSTAMP_OFF;
> + case HWTSTAMP_TX_ON:
> + return IONIC_TXSTAMP_ON;
> + case HWTSTAMP_TX_ONESTEP_SYNC:
> + return IONIC_TXSTAMP_ONESTEP_SYNC;
> +#ifdef HAVE_HWSTAMP_TX_ONESTEP_P2P
> + case HWTSTAMP_TX_ONESTEP_P2P:
> + return IONIC_TXSTAMP_ONESTEP_P2P;
> +#endif
This ifdef is not needed. (I guess you have to support older kernel
versions, but my understanding of the policy is that new code
shouldn't carry such stuff).
> + default:
> + return -ERANGE;
> + }
> +}
> +int ionic_lif_hwstamp_set(struct ionic_lif *lif, struct ifreq *ifr)
> +{
> + struct ionic *ionic = lif->ionic;
> + struct hwtstamp_config config;
> + int tx_mode = 0;
> + u64 rx_filt = 0;
> + int err, err2;
> + bool rx_all;
> + __le64 mask;
> +
> + if (!lif->phc || !lif->phc->ptp)
> + return -EOPNOTSUPP;
> +
> + if (ifr) {
> + if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
> + return -EFAULT;
> + } else {
> + /* if called with ifr == NULL, behave as if called with the
> + * current ts_config from the initial cleared state.
> + */
This check is unneeded, because the ioctl layer never passes NULL here.
> + memcpy(&config, &lif->phc->ts_config, sizeof(config));
> + memset(&lif->phc->ts_config, 0, sizeof(config));
> + }
Thanks,
Richard
