* [PATCH net-next 0/2] Provide direct access to 1588 one step register
@ 2022-02-14 16:03 Radu Bulie
2022-02-14 16:03 ` [PATCH net-next 1/2] dpaa2-eth: Update dpni_get_single_step_cfg command Radu Bulie
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Radu Bulie @ 2022-02-14 16:03 UTC (permalink / raw)
To: davem, kuba; +Cc: netdev, ioana.ciornei, yangbo.lu, Radu Bulie
DPAA2 MAC supports 1588 one step timestamping.
If this option is enabled then for each transmitted PTP event packet,
the 1588 SINGLE_STEP register is accessed to modify the following fields:
-offset of the correction field inside the PTP packet
-UDP checksum update bit, in case the PTP event packet has
UDP encapsulation
These values can change any time, because there may be multiple
PTP clients connected, that receive various 1588 frame types:
- L2 only frame
- UDP / Ipv4
- UDP / Ipv6
- other
The current implementation uses dpni_set_single_step_cfg to update the
SINLGE_STEP register.
Using an MC command on the Tx datapath for each transmitted 1588 message
introduces high delays, leading to low throughput and consequently to a
small number of supported PTP clients. Besides these, the nanosecond
correction field from the PTP packet will contain the high delay from the
driver which together with the originTimestamp will render timestamp
values that are unacceptable in a GM clock implementation.
This patch series replaces the dpni_set_single_step_cfg function call from
the Tx datapath for 1588 messages (when one step timestamping is enabled)
with a callback that either implements direct access to the SINGLE_STEP
register, eliminating the overhead caused by the MC command that will need
to be dispatched by the MC firmware through the MC command portal
interface or falls back to the dpni_set_single_step_cfg in case the MC
version does not have support for returning the single step register
base address.
In other words all the delay introduced by dpni_set_single_step_cfg
function will be eliminated (if MC version has support for returning the
base address of the single step register), improving the egress driver
performance for PTP packets when single step timestamping is enabled.
The first patch adds a new attribute that contains the base address of
the SINGLE_STEP register. It will be used to directly update the register
on the Tx datapath.
The second patch updates the driver such that the SINGLE_STEP
register is either accessed directly if MC version >= 10.32 or is
accessed through dpni_set_single_step_cfg command when 1588 messages
are transmitted.
Radu Bulie (2):
dpaa2-eth: Update dpni_get_single_step_cfg command
dpaa2-eth: Provide direct access to 1588 one step register
.../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 96 +++++++++++++++++--
.../net/ethernet/freescale/dpaa2/dpaa2-eth.h | 12 ++-
.../net/ethernet/freescale/dpaa2/dpni-cmd.h | 6 +-
drivers/net/ethernet/freescale/dpaa2/dpni.c | 2 +
drivers/net/ethernet/freescale/dpaa2/dpni.h | 6 ++
5 files changed, 110 insertions(+), 12 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net-next 1/2] dpaa2-eth: Update dpni_get_single_step_cfg command
2022-02-14 16:03 [PATCH net-next 0/2] Provide direct access to 1588 one step register Radu Bulie
@ 2022-02-14 16:03 ` Radu Bulie
2022-02-14 16:03 ` [PATCH net-next 2/2] dpaa2-eth: Update SINGLE_STEP register access Radu Bulie
2022-02-14 19:18 ` [PATCH net-next 0/2] Provide direct access to 1588 one step register Ioana Ciornei
2 siblings, 0 replies; 5+ messages in thread
From: Radu Bulie @ 2022-02-14 16:03 UTC (permalink / raw)
To: davem, kuba; +Cc: netdev, ioana.ciornei, yangbo.lu, Radu Bulie
dpni_get_single_step_cfg is an MC firmware command used for
retrieving the contents of SINGLE_STEP 1588 register available
in a DPMAC.
This patch adds a new version of this command that returns as an extra
argument the physical base address of the aforementioned register.
The address will be used to directly modify the contents of the
SINGLE_STEP register instead of invoking the MC command
dpni_set_single_step_cgf. The former approach introduced huge delays on
the TX datapath when one step PTP events were transmitted. This led to low
throughput and high latencies observed in the PTP correction field.
Signed-off-by: Radu Bulie <radu-andrei.bulie@nxp.com>
---
drivers/net/ethernet/freescale/dpaa2/dpni-cmd.h | 6 +++++-
drivers/net/ethernet/freescale/dpaa2/dpni.c | 2 ++
drivers/net/ethernet/freescale/dpaa2/dpni.h | 6 ++++++
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpni-cmd.h b/drivers/net/ethernet/freescale/dpaa2/dpni-cmd.h
index 9f80bdfeedec..828f538097af 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpni-cmd.h
+++ b/drivers/net/ethernet/freescale/dpaa2/dpni-cmd.h
@@ -98,7 +98,7 @@
#define DPNI_CMDID_GET_LINK_CFG DPNI_CMD(0x278)
#define DPNI_CMDID_SET_SINGLE_STEP_CFG DPNI_CMD(0x279)
-#define DPNI_CMDID_GET_SINGLE_STEP_CFG DPNI_CMD(0x27a)
+#define DPNI_CMDID_GET_SINGLE_STEP_CFG DPNI_CMD_V2(0x27a)
/* Macros for accessing command fields smaller than 1byte */
#define DPNI_MASK(field) \
@@ -658,12 +658,16 @@ struct dpni_cmd_single_step_cfg {
__le16 flags;
__le16 offset;
__le32 peer_delay;
+ __le32 ptp_onestep_reg_base;
+ __le32 pad0;
};
struct dpni_rsp_single_step_cfg {
__le16 flags;
__le16 offset;
__le32 peer_delay;
+ __le32 ptp_onestep_reg_base;
+ __le32 pad0;
};
struct dpni_cmd_enable_vlan_filter {
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpni.c b/drivers/net/ethernet/freescale/dpaa2/dpni.c
index d6afada99fb6..6c3b36f20fb8 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpni.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpni.c
@@ -2136,6 +2136,8 @@ int dpni_get_single_step_cfg(struct fsl_mc_io *mc_io,
ptp_cfg->ch_update = dpni_get_field(le16_to_cpu(rsp_params->flags),
PTP_CH_UPDATE) ? 1 : 0;
ptp_cfg->peer_delay = le32_to_cpu(rsp_params->peer_delay);
+ ptp_cfg->ptp_onestep_reg_base =
+ le32_to_cpu(rsp_params->ptp_onestep_reg_base);
return err;
}
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpni.h b/drivers/net/ethernet/freescale/dpaa2/dpni.h
index 7de0562bbf59..6fffd519aa00 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpni.h
+++ b/drivers/net/ethernet/freescale/dpaa2/dpni.h
@@ -1074,12 +1074,18 @@ int dpni_set_tx_shaping(struct fsl_mc_io *mc_io,
* @peer_delay: For peer-to-peer transparent clocks add this value to the
* correction field in addition to the transient time update.
* The value expresses nanoseconds.
+ * @ptp_onestep_reg_base: 1588 SINGLE_STEP register base address. This address
+ * is used to update directly the register contents.
+ * User has to create an address mapping for it.
+ *
+ *
*/
struct dpni_single_step_cfg {
u8 en;
u8 ch_update;
u16 offset;
u32 peer_delay;
+ u32 ptp_onestep_reg_base;
};
int dpni_set_single_step_cfg(struct fsl_mc_io *mc_io,
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net-next 2/2] dpaa2-eth: Update SINGLE_STEP register access
2022-02-14 16:03 [PATCH net-next 0/2] Provide direct access to 1588 one step register Radu Bulie
2022-02-14 16:03 ` [PATCH net-next 1/2] dpaa2-eth: Update dpni_get_single_step_cfg command Radu Bulie
@ 2022-02-14 16:03 ` Radu Bulie
2022-02-16 4:56 ` Jakub Kicinski
2022-02-14 19:18 ` [PATCH net-next 0/2] Provide direct access to 1588 one step register Ioana Ciornei
2 siblings, 1 reply; 5+ messages in thread
From: Radu Bulie @ 2022-02-14 16:03 UTC (permalink / raw)
To: davem, kuba; +Cc: netdev, ioana.ciornei, yangbo.lu, Radu Bulie
DPAA2 MAC supports 1588 one step timestamping.
If this option is enabled then for each transmitted PTP event packet,
the 1588 SINGLE_STEP register is accessed to modify the following fields:
-offset of the correction field inside the PTP packet
-UDP checksum update bit, in case the PTP event packet has
UDP encapsulation
These values can change any time, because there may be multiple
PTP clients connected, that receive various 1588 frame types:
- L2 only frame
- UDP / Ipv4
- UDP / Ipv6
- other
The current implementation uses dpni_set_single_step_cfg to update the
SINLGE_STEP register.
Using an MC command on the Tx datapath for each transmitted 1588 message
introduces high delays, leading to low throughput and consequently to a
small number of supported PTP clients. Besides these, the nanosecond
correction field from the PTP packet will contain the high delay from the
driver which together with the originTimestamp will render timestamp
values that are unacceptable in a GM clock implementation.
This patch updates the Tx datapath for 1588 messages when single step
timestamp is enabled and provides direct access to SINGLE_STEP register,
eliminating the overhead caused by the dpni_set_single_step_cfg
MC command. MC version >= 10.32 implements this functionality.
If the MC version does not have support for returning the
single step register base address, the driver will use
dpni_set_single_step_cfg command for updates operations.
All the delay introduced by dpni_set_single_step_cfg
function will be eliminated (if MC version has support for returning the
base address of the single step register), improving the egress driver
performance for PTP packets when single step timestamping is enabled.
Before these changes the maximum throughput for 1588 messages with
single step hardware timestamp enabled was around 2000pps.
After the updates the throughput increased up to 32.82 Mbps / 46631.02 pps.
Signed-off-by: Radu Bulie <radu-andrei.bulie@nxp.com>
---
.../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 96 +++++++++++++++++--
.../net/ethernet/freescale/dpaa2/dpaa2-eth.h | 12 ++-
2 files changed, 97 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
index c4a48e6f1758..7221c7299cb4 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -35,6 +35,85 @@ MODULE_DESCRIPTION("Freescale DPAA2 Ethernet Driver");
struct ptp_qoriq *dpaa2_ptp;
EXPORT_SYMBOL(dpaa2_ptp);
+static void (*dpaa2_set_onestep_params_cb)(struct dpaa2_eth_priv *priv,
+ u32 offset, u8 udp);
+
+static void dpaa2_eth_detect_features(struct dpaa2_eth_priv *priv)
+{
+ priv->features = 0;
+
+ if (dpaa2_eth_cmp_dpni_ver(priv, DPNI_PTP_ONESTEP_VER_MAJOR,
+ DPNI_PTP_ONESTEP_VER_MINOR) >= 0)
+ priv->features |= DPAA2_ETH_FEATURE_ONESTEP_CFG_DIRECT;
+}
+
+static void dpaa2_update_ptp_onestep_indirect(struct dpaa2_eth_priv *priv,
+ u32 offset, u8 udp)
+{
+ struct dpni_single_step_cfg cfg;
+
+ if (priv->ptp_correction_off == offset)
+ return;
+
+ cfg.en = 1;
+ cfg.ch_update = udp;
+ cfg.offset = offset;
+ cfg.peer_delay = 0;
+
+ if (dpni_set_single_step_cfg(priv->mc_io, 0, priv->mc_token, &cfg))
+ WARN_ONCE(1, "Failed to set single step register");
+
+ priv->ptp_correction_off = offset;
+}
+
+static void dpaa2_update_ptp_onestep_direct(struct dpaa2_eth_priv *priv,
+ u32 offset, u8 udp)
+{
+ u32 val = 0;
+
+ if (priv->ptp_correction_off == offset)
+ return;
+
+ val = DPAA2_PTP_SINGLE_STEP_ENABLE |
+ DPAA2_PTP_SINGLE_CORRECTION_OFF(offset);
+
+ if (udp)
+ val |= DPAA2_PTP_SINGLE_STEP_CH;
+
+ if (priv->onestep_reg_base)
+ writel(val, priv->onestep_reg_base);
+
+ priv->ptp_correction_off = offset;
+}
+
+static void dpaa2_ptp_onestep_reg_update_method(struct dpaa2_eth_priv *priv)
+{
+ struct device *dev = priv->net_dev->dev.parent;
+ struct dpni_single_step_cfg ptp_cfg = {0};
+
+ dpaa2_set_onestep_params_cb = dpaa2_update_ptp_onestep_indirect;
+
+ if (!(priv->features & DPAA2_ETH_FEATURE_ONESTEP_CFG_DIRECT))
+ return;
+
+ if (dpni_get_single_step_cfg(priv->mc_io, 0, priv->mc_token, &ptp_cfg))
+ goto fallback;
+
+ if (!ptp_cfg.ptp_onestep_reg_base)
+ goto fallback;
+
+ priv->onestep_reg_base = ioremap(ptp_cfg.ptp_onestep_reg_base, sizeof(u32));
+ if (!priv->onestep_reg_base)
+ goto fallback;
+
+ dpaa2_set_onestep_params_cb = dpaa2_update_ptp_onestep_direct;
+
+ return;
+
+fallback:
+ dev_err(dev, "1588 onestep reg not available, falling back to indirect update\n");
+}
+
static void *dpaa2_iova_to_virt(struct iommu_domain *domain,
dma_addr_t iova_addr)
{
@@ -696,7 +775,6 @@ static void dpaa2_eth_enable_tx_tstamp(struct dpaa2_eth_priv *priv,
struct sk_buff *skb)
{
struct ptp_tstamp origin_timestamp;
- struct dpni_single_step_cfg cfg;
u8 msgtype, twostep, udp;
struct dpaa2_faead *faead;
struct dpaa2_fas *fas;
@@ -750,14 +828,7 @@ static void dpaa2_eth_enable_tx_tstamp(struct dpaa2_eth_priv *priv,
htonl(origin_timestamp.sec_lsb);
*(__be32 *)(data + offset2 + 6) = htonl(origin_timestamp.nsec);
- cfg.en = 1;
- cfg.ch_update = udp;
- cfg.offset = offset1;
- cfg.peer_delay = 0;
-
- if (dpni_set_single_step_cfg(priv->mc_io, 0, priv->mc_token,
- &cfg))
- WARN_ONCE(1, "Failed to set single step register");
+ dpaa2_set_onestep_params_cb(priv, offset1, udp);
}
}
@@ -2407,6 +2478,9 @@ static int dpaa2_eth_ts_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
config.rx_filter = HWTSTAMP_FILTER_ALL;
}
+ if (priv->tx_tstamp_type == HWTSTAMP_TX_ONESTEP_SYNC)
+ dpaa2_ptp_onestep_reg_update_method(priv);
+
return copy_to_user(rq->ifr_data, &config, sizeof(config)) ?
-EFAULT : 0;
}
@@ -4300,6 +4374,8 @@ static int dpaa2_eth_netdev_init(struct net_device *net_dev)
return err;
}
+ dpaa2_eth_detect_features(priv);
+
/* Capabilities listing */
supported |= IFF_LIVE_ADDR_CHANGE;
@@ -4758,6 +4834,8 @@ static int dpaa2_eth_remove(struct fsl_mc_device *ls_dev)
dpaa2_eth_free_dpbp(priv);
dpaa2_eth_free_dpio(priv);
dpaa2_eth_free_dpni(priv);
+ if (priv->onestep_reg_base)
+ iounmap(priv->onestep_reg_base);
fsl_mc_portal_free(priv->mc_io);
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
index b79831cd1a94..ed3d441e81bc 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
@@ -526,12 +526,13 @@ struct dpaa2_eth_priv {
u8 num_channels;
struct dpaa2_eth_channel *channel[DPAA2_ETH_MAX_DPCONS];
struct dpaa2_eth_sgt_cache __percpu *sgt_cache;
-
+ unsigned long features;
struct dpni_attr dpni_attrs;
u16 dpni_ver_major;
u16 dpni_ver_minor;
u16 tx_data_offset;
-
+ void __iomem *onestep_reg_base;
+ u8 ptp_correction_off;
struct fsl_mc_device *dpbp_dev;
u16 rx_buf_size;
u16 bpid;
@@ -673,6 +674,13 @@ enum dpaa2_eth_rx_dist {
#define DPAA2_ETH_DIST_L4DST BIT(8)
#define DPAA2_ETH_DIST_ALL (~0ULL)
+#define DPNI_PTP_ONESTEP_VER_MAJOR 8
+#define DPNI_PTP_ONESTEP_VER_MINOR 2
+#define DPAA2_ETH_FEATURE_ONESTEP_CFG_DIRECT BIT(0)
+#define DPAA2_PTP_SINGLE_STEP_ENABLE BIT(31)
+#define DPAA2_PTP_SINGLE_STEP_CH BIT(7)
+#define DPAA2_PTP_SINGLE_CORRECTION_OFF(v) ((v) << 8)
+
#define DPNI_PAUSE_VER_MAJOR 7
#define DPNI_PAUSE_VER_MINOR 13
#define dpaa2_eth_has_pause_support(priv) \
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next 0/2] Provide direct access to 1588 one step register
2022-02-14 16:03 [PATCH net-next 0/2] Provide direct access to 1588 one step register Radu Bulie
2022-02-14 16:03 ` [PATCH net-next 1/2] dpaa2-eth: Update dpni_get_single_step_cfg command Radu Bulie
2022-02-14 16:03 ` [PATCH net-next 2/2] dpaa2-eth: Update SINGLE_STEP register access Radu Bulie
@ 2022-02-14 19:18 ` Ioana Ciornei
2 siblings, 0 replies; 5+ messages in thread
From: Ioana Ciornei @ 2022-02-14 19:18 UTC (permalink / raw)
To: Radu-Andrei Bulie
Cc: davem@davemloft.net, kuba@kernel.org, netdev@vger.kernel.org,
Y.B. Lu
On Mon, Feb 14, 2022 at 06:03:46PM +0200, Radu Bulie wrote:
> DPAA2 MAC supports 1588 one step timestamping.
> If this option is enabled then for each transmitted PTP event packet,
> the 1588 SINGLE_STEP register is accessed to modify the following fields:
>
> -offset of the correction field inside the PTP packet
> -UDP checksum update bit, in case the PTP event packet has
> UDP encapsulation
>
> These values can change any time, because there may be multiple
> PTP clients connected, that receive various 1588 frame types:
> - L2 only frame
> - UDP / Ipv4
> - UDP / Ipv6
> - other
>
> The current implementation uses dpni_set_single_step_cfg to update the
> SINLGE_STEP register.
> Using an MC command on the Tx datapath for each transmitted 1588 message
> introduces high delays, leading to low throughput and consequently to a
> small number of supported PTP clients. Besides these, the nanosecond
> correction field from the PTP packet will contain the high delay from the
> driver which together with the originTimestamp will render timestamp
> values that are unacceptable in a GM clock implementation.
>
> This patch series replaces the dpni_set_single_step_cfg function call from
> the Tx datapath for 1588 messages (when one step timestamping is enabled)
> with a callback that either implements direct access to the SINGLE_STEP
> register, eliminating the overhead caused by the MC command that will need
> to be dispatched by the MC firmware through the MC command portal
> interface or falls back to the dpni_set_single_step_cfg in case the MC
> version does not have support for returning the single step register
> base address.
>
> In other words all the delay introduced by dpni_set_single_step_cfg
> function will be eliminated (if MC version has support for returning the
> base address of the single step register), improving the egress driver
> performance for PTP packets when single step timestamping is enabled.
>
> The first patch adds a new attribute that contains the base address of
> the SINGLE_STEP register. It will be used to directly update the register
> on the Tx datapath.
>
> The second patch updates the driver such that the SINGLE_STEP
> register is either accessed directly if MC version >= 10.32 or is
> accessed through dpni_set_single_step_cfg command when 1588 messages
> are transmitted.
>
> Radu Bulie (2):
> dpaa2-eth: Update dpni_get_single_step_cfg command
> dpaa2-eth: Provide direct access to 1588 one step register
Reviewed-by: Ioana Ciornei <ioana.ciornei@nxp.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next 2/2] dpaa2-eth: Update SINGLE_STEP register access
2022-02-14 16:03 ` [PATCH net-next 2/2] dpaa2-eth: Update SINGLE_STEP register access Radu Bulie
@ 2022-02-16 4:56 ` Jakub Kicinski
0 siblings, 0 replies; 5+ messages in thread
From: Jakub Kicinski @ 2022-02-16 4:56 UTC (permalink / raw)
To: Radu Bulie; +Cc: davem, netdev, ioana.ciornei, yangbo.lu
On Mon, 14 Feb 2022 18:03:48 +0200 Radu Bulie wrote:
> DPAA2 MAC supports 1588 one step timestamping.
> If this option is enabled then for each transmitted PTP event packet,
> the 1588 SINGLE_STEP register is accessed to modify the following fields:
>
> -offset of the correction field inside the PTP packet
> -UDP checksum update bit, in case the PTP event packet has
> UDP encapsulation
>
> These values can change any time, because there may be multiple
> PTP clients connected, that receive various 1588 frame types:
> - L2 only frame
> - UDP / Ipv4
> - UDP / Ipv6
> - other
>
> The current implementation uses dpni_set_single_step_cfg to update the
> SINLGE_STEP register.
> Using an MC command on the Tx datapath for each transmitted 1588 message
> introduces high delays, leading to low throughput and consequently to a
> small number of supported PTP clients. Besides these, the nanosecond
> correction field from the PTP packet will contain the high delay from the
> driver which together with the originTimestamp will render timestamp
> values that are unacceptable in a GM clock implementation.
>
> This patch updates the Tx datapath for 1588 messages when single step
> timestamp is enabled and provides direct access to SINGLE_STEP register,
> eliminating the overhead caused by the dpni_set_single_step_cfg
> MC command. MC version >= 10.32 implements this functionality.
> If the MC version does not have support for returning the
> single step register base address, the driver will use
> dpni_set_single_step_cfg command for updates operations.
>
> All the delay introduced by dpni_set_single_step_cfg
> function will be eliminated (if MC version has support for returning the
> base address of the single step register), improving the egress driver
> performance for PTP packets when single step timestamping is enabled.
>
> Before these changes the maximum throughput for 1588 messages with
> single step hardware timestamp enabled was around 2000pps.
> After the updates the throughput increased up to 32.82 Mbps / 46631.02 pps.
>
> Signed-off-by: Radu Bulie <radu-andrei.bulie@nxp.com>
You need to CC Richard on PTP patches. Please do so when posting v2.
> +static void (*dpaa2_set_onestep_params_cb)(struct dpaa2_eth_priv *priv,
> + u32 offset, u8 udp);
Global variables are generally unacceptable in drivers.
Put the callback in the right structure, or just record that
a capability is enabled and use an if. The indirect call seems
like an overkill.
> +static void dpaa2_eth_detect_features(struct dpaa2_eth_priv *priv)
> +{
> + priv->features = 0;
> +
> + if (dpaa2_eth_cmp_dpni_ver(priv, DPNI_PTP_ONESTEP_VER_MAJOR,
> + DPNI_PTP_ONESTEP_VER_MINOR) >= 0)
> + priv->features |= DPAA2_ETH_FEATURE_ONESTEP_CFG_DIRECT;
> +}
> +
> +static void dpaa2_update_ptp_onestep_indirect(struct dpaa2_eth_priv *priv,
> + u32 offset, u8 udp)
> +{
> + struct dpni_single_step_cfg cfg;
> +
> + if (priv->ptp_correction_off == offset)
> + return;
You can avoid repeating this condition in both handlers.
> + cfg.en = 1;
> + cfg.ch_update = udp;
> + cfg.offset = offset;
> + cfg.peer_delay = 0;
> +
> + if (dpni_set_single_step_cfg(priv->mc_io, 0, priv->mc_token, &cfg))
> + WARN_ONCE(1, "Failed to set single step register");
> +
> + priv->ptp_correction_off = offset;
And this.
> +}
> +
> +static void dpaa2_update_ptp_onestep_direct(struct dpaa2_eth_priv *priv,
> + u32 offset, u8 udp)
> +{
> + u32 val = 0;
> +
> + if (priv->ptp_correction_off == offset)
> + return;
> +
> + val = DPAA2_PTP_SINGLE_STEP_ENABLE |
double space after =
> + DPAA2_PTP_SINGLE_CORRECTION_OFF(offset);
> +
> + if (udp)
> + val |= DPAA2_PTP_SINGLE_STEP_CH;
> +
> + if (priv->onestep_reg_base)
> + writel(val, priv->onestep_reg_base);
> +
> + priv->ptp_correction_off = offset;
> +}
> +
> +static void dpaa2_ptp_onestep_reg_update_method(struct dpaa2_eth_priv *priv)
> +{
> + struct device *dev = priv->net_dev->dev.parent;
> + struct dpni_single_step_cfg ptp_cfg = {0};
no need for the 0
> +
> + dpaa2_set_onestep_params_cb = dpaa2_update_ptp_onestep_indirect;
> +
> + if (!(priv->features & DPAA2_ETH_FEATURE_ONESTEP_CFG_DIRECT))
> + return;
> +
> + if (dpni_get_single_step_cfg(priv->mc_io, 0, priv->mc_token, &ptp_cfg))
> + goto fallback;
> +
> + if (!ptp_cfg.ptp_onestep_reg_base)
> + goto fallback;
> +
> + priv->onestep_reg_base = ioremap(ptp_cfg.ptp_onestep_reg_base, sizeof(u32));
> + if (!priv->onestep_reg_base)
> + goto fallback;
goto is acceptable in the kernel for multi-step error handling,
which this is not. Please rewrite this without the goto.
> + dpaa2_set_onestep_params_cb = dpaa2_update_ptp_onestep_direct;
> +
> + return;
> +
> +fallback:
> + dev_err(dev, "1588 onestep reg not available, falling back to indirect update\n");
> +}
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-02-16 4:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-14 16:03 [PATCH net-next 0/2] Provide direct access to 1588 one step register Radu Bulie
2022-02-14 16:03 ` [PATCH net-next 1/2] dpaa2-eth: Update dpni_get_single_step_cfg command Radu Bulie
2022-02-14 16:03 ` [PATCH net-next 2/2] dpaa2-eth: Update SINGLE_STEP register access Radu Bulie
2022-02-16 4:56 ` Jakub Kicinski
2022-02-14 19:18 ` [PATCH net-next 0/2] Provide direct access to 1588 one step register Ioana Ciornei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox