| Message ID | 20221012062558.732930-3-matej.vasilevski@seznam.cz (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | can: ctucanfd: hardware rx timestamps reporting | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Series ignored based on subject, async |
Thanks for the work On Wednesday 12 of October 2022 08:25:56 Matej Vasilevski wrote: > This patch adds support for retrieving hardware timestamps to RX and > error CAN frames. It uses timecounter and cyclecounter structures, > because the timestamping counter width depends on the IP core integration > (it might not always be 64-bit). > For platform devices, you should specify "ts" clock in device tree. > For PCI devices, the timestamping frequency is assumed to be the same > as bus frequency. > > Signed-off-by: Matej Vasilevski <matej.vasilevski@seznam.cz> Acked-by: Pave Pisa <pisa@cmp.felk.cvut.cz> It would be great if the code gets in as a basic level for CTU CAN FD timestamping which we need for CAN latency test project. In the longer term, it could be usesfull to discuss if rx_filter == HWTSTAMP_FILTER_ALL and cfg.tx_type == HWTSTAMP_TX_ON should be divided to allow separate timestamping enable and disable for transmit and receive. Our actual focus is to receive and Tx is implemented by reading the timestamping counter in the message transmit done interrupt. There is option (for newer core version) to loop Tx frames into Rx loop which could allow to enhance precision of Tx timestamps to 10 ns. But that requires newer IP core and I wait even for some minor changes to allow identification of looped Tx frames into Rx queue. Switch to such processing mode will have some overhead etc... So it should stay configurable and used only when precise Tx timestamp are really required... When the current timestamping patch is accepted I plan to discuss use of clk_prepare_enable for the main IP core clocks. These clocks are AXI bus ones on our FPGA integration so they has to be up anyway and clk_prepare_enable etc.. does not change behavior, but I want to make that correct in long term. I hope/expect that it is not problem to call clk_prepare_enable twice on same reference when the clocks are the same. As I read the code the state is counted. If it is a problem then some if has to be put there when the core and timestamp clock are the same. Thanks for work and reviews, Pavel
On 16.10.2022 23:54:48, Pavel Pisa wrote: [...] > I hope/expect that it is not problem to call clk_prepare_enable twice > on same reference when the clocks are the same. As I read the code the > state is counted. If it is a problem then some if has to be put there > when the core and timestamp clock are the same. The clock prepare and enable are counting. If you call then twice, you have to call disable and unprepare twice, too, to shut it down. This is widely used in the kernel, e.g. if the same clock is passed to several IP cores. regards, Marc
On 12.10.2022 08:25:56, Matej Vasilevski wrote: > This patch adds support for retrieving hardware timestamps to RX and Later in the code you set struct ethtool_ts_info::tx_types but the driver doesn't set TX timestamps, does it? > error CAN frames. It uses timecounter and cyclecounter structures, > because the timestamping counter width depends on the IP core integration > (it might not always be 64-bit). > For platform devices, you should specify "ts" clock in device tree. > For PCI devices, the timestamping frequency is assumed to be the same > as bus frequency. > > Signed-off-by: Matej Vasilevski <matej.vasilevski@seznam.cz> [...] > diff --git a/drivers/net/can/ctucanfd/ctucanfd_base.c b/drivers/net/can/ctucanfd/ctucanfd_base.c > index b8da15ea6ad9..079819d53e23 100644 > --- a/drivers/net/can/ctucanfd/ctucanfd_base.c > +++ b/drivers/net/can/ctucanfd/ctucanfd_base.c [...] > @@ -950,6 +986,11 @@ static int ctucan_rx_poll(struct napi_struct *napi, int quota) > cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW; > stats->rx_packets++; > stats->rx_bytes += cf->can_dlc; > + if (priv->timestamp_enabled) { > + u64 tstamp = ctucan_read_timestamp_counter(priv); > + > + ctucan_skb_set_timestamp(priv, skb, tstamp); > + } > netif_rx(skb); > } > > @@ -1230,6 +1271,9 @@ static int ctucan_open(struct net_device *ndev) > goto err_chip_start; > } > > + if (priv->timestamp_possible) > + ctucan_timestamp_init(priv); > + This is racy. You have to init the timestamping before the start of the chip, i.e. enabling the IRQs. I had the same problem with the gs_usb driver: | https://lore.kernel.org/all/20220921081329.385509-1-mkl@pengutronix.de > netdev_info(ndev, "ctu_can_fd device registered\n"); > napi_enable(&priv->napi); > netif_start_queue(ndev); > @@ -1262,6 +1306,8 @@ static int ctucan_close(struct net_device *ndev) > ctucan_chip_stop(ndev); > free_irq(ndev->irq, ndev); > close_candev(ndev); > + if (priv->timestamp_possible) > + ctucan_timestamp_stop(priv); Can you make this symmetric with respect to the ctucan_open() function. > > pm_runtime_put(priv->dev); > > @@ -1294,15 +1340,88 @@ static int ctucan_get_berr_counter(const struct net_device *ndev, struct can_ber > return 0; > } [...] > @@ -1385,15 +1534,29 @@ int ctucan_probe_common(struct device *dev, void __iomem *addr, int irq, unsigne > > /* Getting the can_clk info */ > if (!can_clk_rate) { > - priv->can_clk = devm_clk_get(dev, NULL); > + priv->can_clk = devm_clk_get_optional(dev, "core"); > + if (!priv->can_clk) > + /* For compatibility with (older) device trees without clock-names */ > + priv->can_clk = devm_clk_get(dev, NULL); > if (IS_ERR(priv->can_clk)) { > - dev_err(dev, "Device clock not found.\n"); > + dev_err(dev, "Device clock not found: %pe.\n", priv->can_clk); > ret = PTR_ERR(priv->can_clk); > goto err_free; > } > can_clk_rate = clk_get_rate(priv->can_clk); > } > > + if (!timestamp_clk_rate) { > + priv->timestamp_clk = devm_clk_get(dev, "ts"); > + if (IS_ERR(priv->timestamp_clk)) { > + /* Take the core clock instead */ > + dev_info(dev, "Failed to get ts clk\n"); > + priv->timestamp_clk = priv->can_clk; > + } > + clk_prepare_enable(priv->timestamp_clk); > + timestamp_clk_rate = clk_get_rate(priv->timestamp_clk); > + } > + > priv->write_reg = ctucan_write32_le; > priv->read_reg = ctucan_read32_le; > > @@ -1424,6 +1587,50 @@ int ctucan_probe_common(struct device *dev, void __iomem *addr, int irq, unsigne > > priv->can.clock.freq = can_clk_rate; > > + /* Obtain timestamping counter bit size */ > + timestamp_bit_size = FIELD_GET(REG_ERR_CAPT_TS_BITS, > + ctucan_read32(priv, CTUCANFD_ERR_CAPT)); > + > + /* The register value is actually bit_size - 1 */ > + if (timestamp_bit_size) { > + timestamp_bit_size += 1; > + } else { > + /* For 2.x versions of the IP core, we will assume 64-bit counter > + * if there was a 0 in the register. > + */ > + u32 version_reg = ctucan_read32(priv, CTUCANFD_DEVICE_ID); > + u32 major = FIELD_GET(REG_DEVICE_ID_VER_MAJOR, version_reg); > + > + if (major == 2) > + timestamp_bit_size = 64; > + else > + priv->timestamp_possible = false; > + } > + > + /* Setup conversion constants and work delay */ > + if (priv->timestamp_possible) { > + u64 max_cycles; > + u64 work_delay_ns; > + u32 maxsec; > + > + priv->cc.read = ctucan_read_timestamp_cc_wrapper; > + priv->cc.mask = CYCLECOUNTER_MASK(timestamp_bit_size); > + maxsec = min_t(u32, CTUCANFD_MAX_WORK_DELAY_SEC, > + div_u64(priv->cc.mask, timestamp_clk_rate)); > + clocks_calc_mult_shift(&priv->cc.mult, &priv->cc.shift, > + timestamp_clk_rate, NSEC_PER_SEC, maxsec); > + > + /* shortened copy of clocks_calc_max_nsecs() */ > + max_cycles = div_u64(ULLONG_MAX, priv->cc.mult); > + max_cycles = min(max_cycles, priv->cc.mask); > + work_delay_ns = clocksource_cyc2ns(max_cycles, priv->cc.mult, > + priv->cc.shift) >> 2; I think we can use cyclecounter_cyc2ns() for this, see: | https://elixir.bootlin.com/linux/v6.0.3/source/drivers/net/ethernet/ti/cpts.c#L642 BTW: This is the only networking driver using clocks_calc_mult_shift() (so far) :D > + priv->work_delay_jiffies = nsecs_to_jiffies(work_delay_ns); > + > + if (priv->work_delay_jiffies == 0) > + priv->timestamp_possible = false; > + } > + regards, Marc
On 12.10.2022 08:25:56, Matej Vasilevski wrote: > This patch adds support for retrieving hardware timestamps to RX and > error CAN frames. It uses timecounter and cyclecounter structures, > because the timestamping counter width depends on the IP core integration > (it might not always be 64-bit). > For platform devices, you should specify "ts" clock in device tree. > For PCI devices, the timestamping frequency is assumed to be the same > as bus frequency. > > Signed-off-by: Matej Vasilevski <matej.vasilevski@seznam.cz> [...] > @@ -640,12 +663,16 @@ static netdev_tx_t ctucan_start_xmit(struct sk_buff *skb, struct net_device *nde > * @priv: Pointer to CTU CAN FD's private data > * @cf: Pointer to CAN frame struct > * @ffw: Previously read frame format word > + * @skb: Pointer to buffer to store timestamp > * > * Note: Frame format word must be read separately and provided in 'ffw'. > */ > -static void ctucan_read_rx_frame(struct ctucan_priv *priv, struct canfd_frame *cf, u32 ffw) > +static void ctucan_read_rx_frame(struct ctucan_priv *priv, struct canfd_frame *cf, > + u32 ffw, u64 *timestamp) | drivers/net/can/ctucanfd/ctucanfd_base.c:672: warning: Function parameter or member 'timestamp' not described in 'ctucan_read_rx_frame' | drivers/net/can/ctucanfd/ctucanfd_base.c:672: warning: Excess function parameter 'skb' description in 'ctucan_read_rx_frame' Marc
On 12.10.2022 08:25:56, Matej Vasilevski wrote: > This patch adds support for retrieving hardware timestamps to RX and > error CAN frames. It uses timecounter and cyclecounter structures, > because the timestamping counter width depends on the IP core integration > (it might not always be 64-bit). > For platform devices, you should specify "ts" clock in device tree. > For PCI devices, the timestamping frequency is assumed to be the same > as bus frequency. > > Signed-off-by: Matej Vasilevski <matej.vasilevski@seznam.cz> [...] > int ctucan_suspend(struct device *dev) > @@ -1337,12 +1456,41 @@ int ctucan_resume(struct device *dev) > } > EXPORT_SYMBOL(ctucan_resume); > > +int ctucan_runtime_suspend(struct device *dev) > +{ > + struct net_device *ndev = dev_get_drvdata(dev); > + struct ctucan_priv *priv = netdev_priv(ndev); > + > + clk_disable_unprepare(priv->timestamp_clk); > + > + return 0; > +} > +EXPORT_SYMBOL(ctucan_runtime_suspend); > + > +int ctucan_runtime_resume(struct device *dev) > +{ > + struct net_device *ndev = dev_get_drvdata(dev); > + struct ctucan_priv *priv = netdev_priv(ndev); > + int ret; > + > + ret = clk_prepare_enable(priv->timestamp_clk); > + if (ret) { > + dev_err(dev, "Cannot enable timestamping clock: %d\n", ret); > + return ret; > + } > + > + return 0; > +} > +EXPORT_SYMBOL(ctucan_runtime_resume); Regarding the timestamp_clk handling: If you prepare_enable the timestamp_clk during probe_common() and don't disable_unprepare it, it stays on the whole lifetime of the driver. So there's no need/reason for the runtime suspend/resume functions. So either keep the clock powered and remove the suspend/resume functions or shut down the clock after probe. If you want to make things 1000% clean, you can get the timestamp's clock rate during open() and re-calculate the mult and shift. The background is that the clock rate might change if the clock is not enabled (at least that's not guaranteed by the common clock framework). Actual HW implementations might differ. Marc
Hi Marc, thanks for another review from you. I'll merge the responses for all three mails from you, so I don't spam the mailing list too much. On Mon, Oct 24, 2022 at 10:02:38PM +0200, Marc Kleine-Budde wrote: > On 12.10.2022 08:25:56, Matej Vasilevski wrote: > > This patch adds support for retrieving hardware timestamps to RX and > > Later in the code you set struct ethtool_ts_info::tx_types but the > driver doesn't set TX timestamps, does it? > No, it doesn't explicitly. Unless something changed and I don't know about it, all the drivers using can_put_echo_skb() (includes ctucanfd) now report software (hardware if available) tx timestamps thanks to Vincent's patch. https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/commit/?id=12a18d79dc14c80b358dbd26461614b97f2ea4a6 > > error CAN frames. It uses timecounter and cyclecounter structures, > > because the timestamping counter width depends on the IP core integration > > (it might not always be 64-bit). > > For platform devices, you should specify "ts" clock in device tree. > > For PCI devices, the timestamping frequency is assumed to be the same > > as bus frequency. > > > > Signed-off-by: Matej Vasilevski <matej.vasilevski@seznam.cz> > > [...] > > > diff --git a/drivers/net/can/ctucanfd/ctucanfd_base.c b/drivers/net/can/ctucanfd/ctucanfd_base.c > > index b8da15ea6ad9..079819d53e23 100644 > > --- a/drivers/net/can/ctucanfd/ctucanfd_base.c > > +++ b/drivers/net/can/ctucanfd/ctucanfd_base.c > > [...] > > > @@ -950,6 +986,11 @@ static int ctucan_rx_poll(struct napi_struct *napi, int quota) > > cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW; > > stats->rx_packets++; > > stats->rx_bytes += cf->can_dlc; > > + if (priv->timestamp_enabled) { > > + u64 tstamp = ctucan_read_timestamp_counter(priv); > > + > > + ctucan_skb_set_timestamp(priv, skb, tstamp); > > + } > > netif_rx(skb); > > } > > > > @@ -1230,6 +1271,9 @@ static int ctucan_open(struct net_device *ndev) > > goto err_chip_start; > > } > > > > + if (priv->timestamp_possible) > > + ctucan_timestamp_init(priv); > > + > > This is racy. You have to init the timestamping before the start of the > chip, i.e. enabling the IRQs. I had the same problem with the gs_usb > driver: > > | https://lore.kernel.org/all/20220921081329.385509-1-mkl@pengutronix.de Thanks for pointing this out, I'll fix this. > > > netdev_info(ndev, "ctu_can_fd device registered\n"); > > napi_enable(&priv->napi); > > netif_start_queue(ndev); > > @@ -1262,6 +1306,8 @@ static int ctucan_close(struct net_device *ndev) > > ctucan_chip_stop(ndev); > > free_irq(ndev->irq, ndev); > > close_candev(ndev); > > + if (priv->timestamp_possible) > > + ctucan_timestamp_stop(priv); > > Can you make this symmetric with respect to the ctucan_open() function. Yes, will do. > > > > pm_runtime_put(priv->dev); > > > > @@ -1294,15 +1340,88 @@ static int ctucan_get_berr_counter(const struct net_device *ndev, struct can_ber > > return 0; > > } > > [...] > > > @@ -1385,15 +1534,29 @@ int ctucan_probe_common(struct device *dev, void __iomem *addr, int irq, unsigne > > > > /* Getting the can_clk info */ > > if (!can_clk_rate) { > > - priv->can_clk = devm_clk_get(dev, NULL); > > + priv->can_clk = devm_clk_get_optional(dev, "core"); > > + if (!priv->can_clk) > > + /* For compatibility with (older) device trees without clock-names */ > > + priv->can_clk = devm_clk_get(dev, NULL); > > if (IS_ERR(priv->can_clk)) { > > - dev_err(dev, "Device clock not found.\n"); > > + dev_err(dev, "Device clock not found: %pe.\n", priv->can_clk); > > ret = PTR_ERR(priv->can_clk); > > goto err_free; > > } > > can_clk_rate = clk_get_rate(priv->can_clk); > > } > > > > + if (!timestamp_clk_rate) { > > + priv->timestamp_clk = devm_clk_get(dev, "ts"); > > + if (IS_ERR(priv->timestamp_clk)) { > > + /* Take the core clock instead */ > > + dev_info(dev, "Failed to get ts clk\n"); > > + priv->timestamp_clk = priv->can_clk; > > + } > > + clk_prepare_enable(priv->timestamp_clk); > > + timestamp_clk_rate = clk_get_rate(priv->timestamp_clk); > > + } > > + > > priv->write_reg = ctucan_write32_le; > > priv->read_reg = ctucan_read32_le; > > > > @@ -1424,6 +1587,50 @@ int ctucan_probe_common(struct device *dev, void __iomem *addr, int irq, unsigne > > > > priv->can.clock.freq = can_clk_rate; > > > > + /* Obtain timestamping counter bit size */ > > + timestamp_bit_size = FIELD_GET(REG_ERR_CAPT_TS_BITS, > > + ctucan_read32(priv, CTUCANFD_ERR_CAPT)); > > + > > + /* The register value is actually bit_size - 1 */ > > + if (timestamp_bit_size) { > > + timestamp_bit_size += 1; > > + } else { > > + /* For 2.x versions of the IP core, we will assume 64-bit counter > > + * if there was a 0 in the register. > > + */ > > + u32 version_reg = ctucan_read32(priv, CTUCANFD_DEVICE_ID); > > + u32 major = FIELD_GET(REG_DEVICE_ID_VER_MAJOR, version_reg); > > + > > + if (major == 2) > > + timestamp_bit_size = 64; > > + else > > + priv->timestamp_possible = false; > > + } > > + > > + /* Setup conversion constants and work delay */ > > + if (priv->timestamp_possible) { > > + u64 max_cycles; > > + u64 work_delay_ns; > > + u32 maxsec; > > + > > + priv->cc.read = ctucan_read_timestamp_cc_wrapper; > > + priv->cc.mask = CYCLECOUNTER_MASK(timestamp_bit_size); > > + maxsec = min_t(u32, CTUCANFD_MAX_WORK_DELAY_SEC, > > + div_u64(priv->cc.mask, timestamp_clk_rate)); > > + clocks_calc_mult_shift(&priv->cc.mult, &priv->cc.shift, > > + timestamp_clk_rate, NSEC_PER_SEC, maxsec); > > + > > + /* shortened copy of clocks_calc_max_nsecs() */ > > + max_cycles = div_u64(ULLONG_MAX, priv->cc.mult); > > + max_cycles = min(max_cycles, priv->cc.mask); > > + work_delay_ns = clocksource_cyc2ns(max_cycles, priv->cc.mult, > > + priv->cc.shift) >> 2; > > I think we can use cyclecounter_cyc2ns() for this, see: > > | https://elixir.bootlin.com/linux/v6.0.3/source/drivers/net/ethernet/ti/cpts.c#L642 > > BTW: This is the only networking driver using clocks_calc_mult_shift() > (so far) :D > I don't really see the benefit at the moment (I have to include clocksource.h anyway due to the clocks_calc_mult_shift()), but sure, I'll use cyclecounter_cyc2ns(). Fun fact :-D I might look into the cpts.c > > + priv->work_delay_jiffies = nsecs_to_jiffies(work_delay_ns); > > + > > + if (priv->work_delay_jiffies == 0) > > + priv->timestamp_possible = false; > > + } > > + > > regards, > Marc > > -- > Pengutronix e.K. | Marc Kleine-Budde | > Embedded Linux | https://www.pengutronix.de | > Vertretung West/Dortmund | Phone: +49-231-2826-924 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | Mail 2: >Regarding the timestamp_clk handling: > >If you prepare_enable the timestamp_clk during probe_common() and don't >disable_unprepare it, it stays on the whole lifetime of the driver. So >there's no need/reason for the runtime suspend/resume functions. > >So either keep the clock powered and remove the suspend/resume functions >or shut down the clock after probe. > >If you want to make things 1000% clean, you can get the timestamp's >clock rate during open() and re-calculate the mult and shift. The >background is that the clock rate might change if the clock is not >enabled (at least that's not guaranteed by the common clock framework). >Actual HW implementations might differ. Hmm, I thought that pm_runtime_put() will eventually run runtime suspend callback, but now I see that it will run only the idle callback (which I haven't defined). I'll remove the runtime suspend/resume callbacks. Best regards, Matej
On 26.10.2022 00:22:37, Matej Vasilevski wrote: > On Mon, Oct 24, 2022 at 10:02:38PM +0200, Marc Kleine-Budde wrote: > > On 12.10.2022 08:25:56, Matej Vasilevski wrote: > > > This patch adds support for retrieving hardware timestamps to RX and > > > > Later in the code you set struct ethtool_ts_info::tx_types but the > > driver doesn't set TX timestamps, does it? > > No, it doesn't explicitly. Unless something changed and I don't know about it, > all the drivers using can_put_echo_skb() (includes ctucanfd) now report > software (hardware if available) tx timestamps thanks to Vincent's patch. > > https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/commit/?id=12a18d79dc14c80b358dbd26461614b97f2ea4a6 Yes, since that patch, drivers using can_put_echo_skb() support software TX timestamps. But you have to set the HW timestamp on the TX'ed CAN frame prior to the can_rx_offload_get_echo_skb() call for HW TX timestamps, e.g.: | https://elixir.bootlin.com/linux/v6.0.3/source/drivers/net/can/spi/mcp251xfd/mcp251xfd-tef.c#L112 [...] > > > + /* Setup conversion constants and work delay */ > > > + if (priv->timestamp_possible) { > > > + u64 max_cycles; > > > + u64 work_delay_ns; > > > + u32 maxsec; > > > + > > > + priv->cc.read = ctucan_read_timestamp_cc_wrapper; > > > + priv->cc.mask = CYCLECOUNTER_MASK(timestamp_bit_size); > > > + maxsec = min_t(u32, CTUCANFD_MAX_WORK_DELAY_SEC, > > > + div_u64(priv->cc.mask, timestamp_clk_rate)); > > > + clocks_calc_mult_shift(&priv->cc.mult, &priv->cc.shift, > > > + timestamp_clk_rate, NSEC_PER_SEC, maxsec); > > > + > > > + /* shortened copy of clocks_calc_max_nsecs() */ > > > + max_cycles = div_u64(ULLONG_MAX, priv->cc.mult); > > > + max_cycles = min(max_cycles, priv->cc.mask); > > > + work_delay_ns = clocksource_cyc2ns(max_cycles, priv->cc.mult, > > > + priv->cc.shift) >> 2; > > > > I think we can use cyclecounter_cyc2ns() for this, see: > > > > | https://elixir.bootlin.com/linux/v6.0.3/source/drivers/net/ethernet/ti/cpts.c#L642 > > > > BTW: This is the only networking driver using clocks_calc_mult_shift() > > (so far) :D > > > > I don't really see the benefit at the moment (I have to include > clocksource.h anyway due to the clocks_calc_mult_shift()), but sure, > I'll use cyclecounter_cyc2ns(). > > Fun fact :-D I might look into the cpts.c The benefit is less variance in the kernel tree, use the same pattern to calculate the delay if both register width and frequency are unknown at compile time. [...] > >Regarding the timestamp_clk handling: > > > >If you prepare_enable the timestamp_clk during probe_common() and don't > >disable_unprepare it, it stays on the whole lifetime of the driver. So > >there's no need/reason for the runtime suspend/resume functions. > > > >So either keep the clock powered and remove the suspend/resume functions > >or shut down the clock after probe. > > > >If you want to make things 1000% clean, you can get the timestamp's > >clock rate during open() and re-calculate the mult and shift. The > >background is that the clock rate might change if the clock is not > >enabled (at least that's not guaranteed by the common clock framework). > >Actual HW implementations might differ. > > Hmm, I thought that pm_runtime_put() will eventually run runtime suspend > callback, but now I see that it will run only the idle callback (which > I haven't defined). > I'll remove the runtime suspend/resume callbacks. If your clock stays enabled the whole driver lifetime you can use devm_clk_get_enabled(), devm will take care of the disable & unprepare. regards, Marc
diff --git a/drivers/net/can/ctucanfd/Kconfig b/drivers/net/can/ctucanfd/Kconfig index 6e2073351a8f..acd38e07146a 100644 --- a/drivers/net/can/ctucanfd/Kconfig +++ b/drivers/net/can/ctucanfd/Kconfig @@ -23,7 +23,7 @@ config CAN_CTUCANFD_PCI config CAN_CTUCANFD_PLATFORM tristate "CTU CAN-FD IP core platform (FPGA, SoC) driver" - depends on HAS_IOMEM && (OF || COMPILE_TEST) + depends on HAS_IOMEM && (OF || COMPILE_TEST) && PM select CAN_CTUCANFD help The core has been tested together with OpenCores SJA1000 diff --git a/drivers/net/can/ctucanfd/Makefile b/drivers/net/can/ctucanfd/Makefile index 8078f1f2c30f..a36e66f2cea7 100644 --- a/drivers/net/can/ctucanfd/Makefile +++ b/drivers/net/can/ctucanfd/Makefile @@ -4,7 +4,7 @@ # obj-$(CONFIG_CAN_CTUCANFD) := ctucanfd.o -ctucanfd-y := ctucanfd_base.o +ctucanfd-y := ctucanfd_base.o ctucanfd_timestamp.o obj-$(CONFIG_CAN_CTUCANFD_PCI) += ctucanfd_pci.o obj-$(CONFIG_CAN_CTUCANFD_PLATFORM) += ctucanfd_platform.o diff --git a/drivers/net/can/ctucanfd/ctucanfd.h b/drivers/net/can/ctucanfd/ctucanfd.h index 0e9904f6a05d..cf4d8cc5349e 100644 --- a/drivers/net/can/ctucanfd/ctucanfd.h +++ b/drivers/net/can/ctucanfd/ctucanfd.h @@ -23,6 +23,10 @@ #include <linux/netdevice.h> #include <linux/can/dev.h> #include <linux/list.h> +#include <linux/timecounter.h> +#include <linux/workqueue.h> + +#define CTUCANFD_MAX_WORK_DELAY_SEC 3600U enum ctu_can_fd_can_registers; @@ -51,6 +55,15 @@ struct ctucan_priv { u32 rxfrm_first_word; struct list_head peers_on_pdev; + + struct cyclecounter cc; + struct timecounter tc; + spinlock_t tc_lock; /* spinlock to guard access to tc->cycle_last */ + struct delayed_work timestamp; + struct clk *timestamp_clk; + unsigned long work_delay_jiffies; + bool timestamp_enabled; + bool timestamp_possible; }; /** @@ -75,8 +88,16 @@ int ctucan_probe_common(struct device *dev, void __iomem *addr, int pm_enable_call, void (*set_drvdata_fnc)(struct device *dev, struct net_device *ndev)); +void ctucan_remove_common(struct ctucan_priv *priv); int ctucan_suspend(struct device *dev) __maybe_unused; int ctucan_resume(struct device *dev) __maybe_unused; +int ctucan_runtime_resume(struct device *dev); +int ctucan_runtime_suspend(struct device *dev); +u64 ctucan_read_timestamp_cc_wrapper(const struct cyclecounter *cc); +u64 ctucan_read_timestamp_counter(struct ctucan_priv *priv); +void ctucan_skb_set_timestamp(struct ctucan_priv *priv, struct sk_buff *skb, u64 timestamp); +void ctucan_timestamp_init(struct ctucan_priv *priv); +void ctucan_timestamp_stop(struct ctucan_priv *priv); #endif /*__CTUCANFD__*/ diff --git a/drivers/net/can/ctucanfd/ctucanfd_base.c b/drivers/net/can/ctucanfd/ctucanfd_base.c index b8da15ea6ad9..079819d53e23 100644 --- a/drivers/net/can/ctucanfd/ctucanfd_base.c +++ b/drivers/net/can/ctucanfd/ctucanfd_base.c @@ -18,6 +18,7 @@ ******************************************************************************/ #include <linux/clk.h> +#include <linux/clocksource.h> #include <linux/errno.h> #include <linux/ethtool.h> #include <linux/init.h> @@ -25,6 +26,7 @@ #include <linux/interrupt.h> #include <linux/io.h> #include <linux/kernel.h> +#include <linux/math64.h> #include <linux/module.h> #include <linux/skbuff.h> #include <linux/string.h> @@ -148,6 +150,27 @@ static void ctucan_write_txt_buf(struct ctucan_priv *priv, enum ctu_can_fd_can_r priv->write_reg(priv, buf_base + offset, val); } +static inline u64 ctucan_concat_tstamp(u32 high, u32 low) +{ + return ((u64)high << 32) | ((u64)low); +} + +u64 ctucan_read_timestamp_counter(struct ctucan_priv *priv) +{ + u32 ts_low; + u32 ts_high; + u32 ts_high2; + + ts_high = ctucan_read32(priv, CTUCANFD_TIMESTAMP_HIGH); + ts_low = ctucan_read32(priv, CTUCANFD_TIMESTAMP_LOW); + ts_high2 = ctucan_read32(priv, CTUCANFD_TIMESTAMP_HIGH); + + if (ts_high2 != ts_high) + ts_low = priv->read_reg(priv, CTUCANFD_TIMESTAMP_LOW); + + return ctucan_concat_tstamp(ts_high2, ts_low); +} + #define CTU_CAN_FD_TXTNF(priv) (!!FIELD_GET(REG_STATUS_TXNF, ctucan_read32(priv, CTUCANFD_STATUS))) #define CTU_CAN_FD_ENABLED(priv) (!!FIELD_GET(REG_MODE_ENA, ctucan_read32(priv, CTUCANFD_MODE))) @@ -640,12 +663,16 @@ static netdev_tx_t ctucan_start_xmit(struct sk_buff *skb, struct net_device *nde * @priv: Pointer to CTU CAN FD's private data * @cf: Pointer to CAN frame struct * @ffw: Previously read frame format word + * @skb: Pointer to buffer to store timestamp * * Note: Frame format word must be read separately and provided in 'ffw'. */ -static void ctucan_read_rx_frame(struct ctucan_priv *priv, struct canfd_frame *cf, u32 ffw) +static void ctucan_read_rx_frame(struct ctucan_priv *priv, struct canfd_frame *cf, + u32 ffw, u64 *timestamp) { u32 idw; + u32 tstamp_high; + u32 tstamp_low; unsigned int i; unsigned int wc; unsigned int len; @@ -681,9 +708,10 @@ static void ctucan_read_rx_frame(struct ctucan_priv *priv, struct canfd_frame *c if (unlikely(len > wc * 4)) len = wc * 4; - /* Timestamp - Read and throw away */ - ctucan_read32(priv, CTUCANFD_RX_DATA); - ctucan_read32(priv, CTUCANFD_RX_DATA); + /* Timestamp */ + tstamp_low = ctucan_read32(priv, CTUCANFD_RX_DATA); + tstamp_high = ctucan_read32(priv, CTUCANFD_RX_DATA); + *timestamp = ctucan_concat_tstamp(tstamp_high, tstamp_low); /* Data */ for (i = 0; i < len; i += 4) { @@ -712,6 +740,7 @@ static int ctucan_rx(struct net_device *ndev) struct net_device_stats *stats = &ndev->stats; struct canfd_frame *cf; struct sk_buff *skb; + u64 timestamp; u32 ffw; if (test_bit(CTUCANFD_FLAG_RX_FFW_BUFFERED, &priv->drv_flags)) { @@ -735,7 +764,9 @@ static int ctucan_rx(struct net_device *ndev) return 0; } - ctucan_read_rx_frame(priv, cf, ffw); + ctucan_read_rx_frame(priv, cf, ffw, ×tamp); + if (priv->timestamp_enabled) + ctucan_skb_set_timestamp(priv, skb, timestamp); stats->rx_bytes += cf->len; stats->rx_packets++; @@ -905,6 +936,11 @@ static void ctucan_err_interrupt(struct net_device *ndev, u32 isr) if (skb) { stats->rx_packets++; stats->rx_bytes += cf->can_dlc; + if (priv->timestamp_enabled) { + u64 tstamp = ctucan_read_timestamp_counter(priv); + + ctucan_skb_set_timestamp(priv, skb, tstamp); + } netif_rx(skb); } } @@ -950,6 +986,11 @@ static int ctucan_rx_poll(struct napi_struct *napi, int quota) cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW; stats->rx_packets++; stats->rx_bytes += cf->can_dlc; + if (priv->timestamp_enabled) { + u64 tstamp = ctucan_read_timestamp_counter(priv); + + ctucan_skb_set_timestamp(priv, skb, tstamp); + } netif_rx(skb); } @@ -1230,6 +1271,9 @@ static int ctucan_open(struct net_device *ndev) goto err_chip_start; } + if (priv->timestamp_possible) + ctucan_timestamp_init(priv); + netdev_info(ndev, "ctu_can_fd device registered\n"); napi_enable(&priv->napi); netif_start_queue(ndev); @@ -1262,6 +1306,8 @@ static int ctucan_close(struct net_device *ndev) ctucan_chip_stop(ndev); free_irq(ndev->irq, ndev); close_candev(ndev); + if (priv->timestamp_possible) + ctucan_timestamp_stop(priv); pm_runtime_put(priv->dev); @@ -1294,15 +1340,88 @@ static int ctucan_get_berr_counter(const struct net_device *ndev, struct can_ber return 0; } +static int ctucan_hwtstamp_set(struct net_device *dev, struct ifreq *ifr) +{ + struct ctucan_priv *priv = netdev_priv(dev); + struct hwtstamp_config cfg; + + if (!priv->timestamp_possible) + return -EOPNOTSUPP; + + if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg))) + return -EFAULT; + + if (cfg.rx_filter == HWTSTAMP_FILTER_NONE && cfg.tx_type == HWTSTAMP_TX_OFF) { + priv->timestamp_enabled = false; + return 0; + } + if (cfg.rx_filter == HWTSTAMP_FILTER_ALL && cfg.tx_type == HWTSTAMP_TX_ON) { + priv->timestamp_enabled = true; + return 0; + } + return -ERANGE; +} + +static int ctucan_hwtstamp_get(struct net_device *dev, struct ifreq *ifr) +{ + struct ctucan_priv *priv = netdev_priv(dev); + struct hwtstamp_config cfg; + + if (!priv->timestamp_possible) + return -EOPNOTSUPP; + + cfg.flags = 0; + + if (priv->timestamp_enabled) { + cfg.tx_type = HWTSTAMP_TX_ON; + cfg.rx_filter = HWTSTAMP_FILTER_ALL; + } else { + cfg.tx_type = HWTSTAMP_TX_OFF; + cfg.rx_filter = HWTSTAMP_FILTER_NONE; + } + + if (copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg))) + return -EFAULT; + return 0; +} + +static int ctucan_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd) +{ + switch (cmd) { + case SIOCSHWTSTAMP: + return ctucan_hwtstamp_set(dev, ifr); + case SIOCGHWTSTAMP: + return ctucan_hwtstamp_get(dev, ifr); + default: + return -EOPNOTSUPP; + } +} + +static int ctucan_ethtool_get_ts_info(struct net_device *ndev, + struct ethtool_ts_info *info) +{ + struct ctucan_priv *priv = netdev_priv(ndev); + + if (!priv->timestamp_possible) + return ethtool_op_get_ts_info(ndev, info); + + can_ethtool_op_get_ts_info_hwts(ndev, info); + info->rx_filters |= BIT(HWTSTAMP_FILTER_NONE); + info->tx_types |= BIT(HWTSTAMP_TX_OFF); + + return 0; +} + static const struct net_device_ops ctucan_netdev_ops = { .ndo_open = ctucan_open, .ndo_stop = ctucan_close, .ndo_start_xmit = ctucan_start_xmit, .ndo_change_mtu = can_change_mtu, + .ndo_eth_ioctl = ctucan_ioctl, }; static const struct ethtool_ops ctucan_ethtool_ops = { - .get_ts_info = ethtool_op_get_ts_info, + .get_ts_info = ctucan_ethtool_get_ts_info, }; int ctucan_suspend(struct device *dev) @@ -1337,12 +1456,41 @@ int ctucan_resume(struct device *dev) } EXPORT_SYMBOL(ctucan_resume); +int ctucan_runtime_suspend(struct device *dev) +{ + struct net_device *ndev = dev_get_drvdata(dev); + struct ctucan_priv *priv = netdev_priv(ndev); + + clk_disable_unprepare(priv->timestamp_clk); + + return 0; +} +EXPORT_SYMBOL(ctucan_runtime_suspend); + +int ctucan_runtime_resume(struct device *dev) +{ + struct net_device *ndev = dev_get_drvdata(dev); + struct ctucan_priv *priv = netdev_priv(ndev); + int ret; + + ret = clk_prepare_enable(priv->timestamp_clk); + if (ret) { + dev_err(dev, "Cannot enable timestamping clock: %d\n", ret); + return ret; + } + + return 0; +} +EXPORT_SYMBOL(ctucan_runtime_resume); + int ctucan_probe_common(struct device *dev, void __iomem *addr, int irq, unsigned int ntxbufs, unsigned long can_clk_rate, int pm_enable_call, void (*set_drvdata_fnc)(struct device *dev, struct net_device *ndev)) { struct ctucan_priv *priv; struct net_device *ndev; + u32 timestamp_clk_rate = can_clk_rate; + u32 timestamp_bit_size = 0; int ret; /* Create a CAN device instance */ @@ -1372,6 +1520,7 @@ int ctucan_probe_common(struct device *dev, void __iomem *addr, int irq, unsigne | CAN_CTRLMODE_FD_NON_ISO | CAN_CTRLMODE_ONE_SHOT; priv->mem_base = addr; + priv->timestamp_possible = true; /* Get IRQ for the device */ ndev->irq = irq; @@ -1385,15 +1534,29 @@ int ctucan_probe_common(struct device *dev, void __iomem *addr, int irq, unsigne /* Getting the can_clk info */ if (!can_clk_rate) { - priv->can_clk = devm_clk_get(dev, NULL); + priv->can_clk = devm_clk_get_optional(dev, "core"); + if (!priv->can_clk) + /* For compatibility with (older) device trees without clock-names */ + priv->can_clk = devm_clk_get(dev, NULL); if (IS_ERR(priv->can_clk)) { - dev_err(dev, "Device clock not found.\n"); + dev_err(dev, "Device clock not found: %pe.\n", priv->can_clk); ret = PTR_ERR(priv->can_clk); goto err_free; } can_clk_rate = clk_get_rate(priv->can_clk); } + if (!timestamp_clk_rate) { + priv->timestamp_clk = devm_clk_get(dev, "ts"); + if (IS_ERR(priv->timestamp_clk)) { + /* Take the core clock instead */ + dev_info(dev, "Failed to get ts clk\n"); + priv->timestamp_clk = priv->can_clk; + } + clk_prepare_enable(priv->timestamp_clk); + timestamp_clk_rate = clk_get_rate(priv->timestamp_clk); + } + priv->write_reg = ctucan_write32_le; priv->read_reg = ctucan_read32_le; @@ -1424,6 +1587,50 @@ int ctucan_probe_common(struct device *dev, void __iomem *addr, int irq, unsigne priv->can.clock.freq = can_clk_rate; + /* Obtain timestamping counter bit size */ + timestamp_bit_size = FIELD_GET(REG_ERR_CAPT_TS_BITS, + ctucan_read32(priv, CTUCANFD_ERR_CAPT)); + + /* The register value is actually bit_size - 1 */ + if (timestamp_bit_size) { + timestamp_bit_size += 1; + } else { + /* For 2.x versions of the IP core, we will assume 64-bit counter + * if there was a 0 in the register. + */ + u32 version_reg = ctucan_read32(priv, CTUCANFD_DEVICE_ID); + u32 major = FIELD_GET(REG_DEVICE_ID_VER_MAJOR, version_reg); + + if (major == 2) + timestamp_bit_size = 64; + else + priv->timestamp_possible = false; + } + + /* Setup conversion constants and work delay */ + if (priv->timestamp_possible) { + u64 max_cycles; + u64 work_delay_ns; + u32 maxsec; + + priv->cc.read = ctucan_read_timestamp_cc_wrapper; + priv->cc.mask = CYCLECOUNTER_MASK(timestamp_bit_size); + maxsec = min_t(u32, CTUCANFD_MAX_WORK_DELAY_SEC, + div_u64(priv->cc.mask, timestamp_clk_rate)); + clocks_calc_mult_shift(&priv->cc.mult, &priv->cc.shift, + timestamp_clk_rate, NSEC_PER_SEC, maxsec); + + /* shortened copy of clocks_calc_max_nsecs() */ + max_cycles = div_u64(ULLONG_MAX, priv->cc.mult); + max_cycles = min(max_cycles, priv->cc.mask); + work_delay_ns = clocksource_cyc2ns(max_cycles, priv->cc.mult, + priv->cc.shift) >> 2; + priv->work_delay_jiffies = nsecs_to_jiffies(work_delay_ns); + + if (priv->work_delay_jiffies == 0) + priv->timestamp_possible = false; + } + netif_napi_add(ndev, &priv->napi, ctucan_rx_poll); ret = register_candev(ndev); @@ -1451,6 +1658,12 @@ int ctucan_probe_common(struct device *dev, void __iomem *addr, int irq, unsigne } EXPORT_SYMBOL(ctucan_probe_common); +void ctucan_remove_common(struct ctucan_priv *priv) +{ + clk_disable_unprepare(priv->timestamp_clk); +} +EXPORT_SYMBOL(ctucan_remove_common); + MODULE_LICENSE("GPL"); MODULE_AUTHOR("Martin Jerabek <martin.jerabek01@gmail.com>"); MODULE_AUTHOR("Pavel Pisa <pisa@cmp.felk.cvut.cz>"); diff --git a/drivers/net/can/ctucanfd/ctucanfd_pci.c b/drivers/net/can/ctucanfd/ctucanfd_pci.c index 8f2956a8ae43..d0c7ec0525d8 100644 --- a/drivers/net/can/ctucanfd/ctucanfd_pci.c +++ b/drivers/net/can/ctucanfd/ctucanfd_pci.c @@ -237,6 +237,8 @@ static void ctucan_pci_remove(struct pci_dev *pdev) return; } + ctucan_remove_common(priv); + /* disable interrupt in * Avalon-MM to PCI Express Interrupt Enable Register */ @@ -271,7 +273,10 @@ static void ctucan_pci_remove(struct pci_dev *pdev) kfree(bdata); } -static SIMPLE_DEV_PM_OPS(ctucan_pci_pm_ops, ctucan_suspend, ctucan_resume); +static const struct dev_pm_ops ctucan_pci_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(ctucan_suspend, ctucan_resume) + SET_RUNTIME_PM_OPS(ctucan_runtime_suspend, ctucan_runtime_resume, NULL) +}; static const struct pci_device_id ctucan_pci_tbl[] = { {PCI_DEVICE_DATA(TEDIA, CTUCAN_VER21, diff --git a/drivers/net/can/ctucanfd/ctucanfd_platform.c b/drivers/net/can/ctucanfd/ctucanfd_platform.c index f83684f006ea..0f1d58ec9850 100644 --- a/drivers/net/can/ctucanfd/ctucanfd_platform.c +++ b/drivers/net/can/ctucanfd/ctucanfd_platform.c @@ -95,6 +95,8 @@ static int ctucan_platform_remove(struct platform_device *pdev) netdev_dbg(ndev, "ctucan_remove"); + ctucan_remove_common(priv); + unregister_candev(ndev); pm_runtime_disable(&pdev->dev); netif_napi_del(&priv->napi); @@ -103,7 +105,10 @@ static int ctucan_platform_remove(struct platform_device *pdev) return 0; } -static SIMPLE_DEV_PM_OPS(ctucan_platform_pm_ops, ctucan_suspend, ctucan_resume); +static const struct dev_pm_ops ctucan_platform_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(ctucan_suspend, ctucan_resume) + SET_RUNTIME_PM_OPS(ctucan_runtime_suspend, ctucan_runtime_resume, NULL) +}; /* Match table for OF platform binding */ static const struct of_device_id ctucan_of_match[] = { diff --git a/drivers/net/can/ctucanfd/ctucanfd_timestamp.c b/drivers/net/can/ctucanfd/ctucanfd_timestamp.c new file mode 100644 index 000000000000..1fdfd8536f71 --- /dev/null +++ b/drivers/net/can/ctucanfd/ctucanfd_timestamp.c @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/******************************************************************************* + * + * CTU CAN FD IP Core + * + * Copyright (C) 2022 Matej Vasilevski <matej.vasilevski@seznam.cz> FEE CTU + * + * Project advisors: + * Jiri Novak <jnovak@fel.cvut.cz> + * Pavel Pisa <pisa@cmp.felk.cvut.cz> + * + * Department of Measurement (http://meas.fel.cvut.cz/) + * Faculty of Electrical Engineering (http://www.fel.cvut.cz) + * Czech Technical University (http://www.cvut.cz/) + ******************************************************************************/ + +#include "linux/spinlock.h" +#include <linux/bitops.h> +#include <linux/clocksource.h> +#include <linux/math64.h> +#include <linux/timecounter.h> +#include <linux/workqueue.h> + +#include "ctucanfd.h" +#include "ctucanfd_kregs.h" + +u64 ctucan_read_timestamp_cc_wrapper(const struct cyclecounter *cc) +{ + struct ctucan_priv *priv = container_of(cc, struct ctucan_priv, cc); + + lockdep_assert_held(&priv->tc_lock); + + return ctucan_read_timestamp_counter(priv); +} + +static void ctucan_timestamp_work(struct work_struct *work) +{ + struct delayed_work *delayed_work = to_delayed_work(work); + struct ctucan_priv *priv = container_of(delayed_work, struct ctucan_priv, timestamp); + unsigned long flags; + + spin_lock_irqsave(&priv->tc_lock, flags); + timecounter_read(&priv->tc); + spin_unlock_irqrestore(&priv->tc_lock, flags); + schedule_delayed_work(&priv->timestamp, priv->work_delay_jiffies); +} + +void ctucan_skb_set_timestamp(struct ctucan_priv *priv, struct sk_buff *skb, u64 timestamp) +{ + struct skb_shared_hwtstamps *hwtstamps = skb_hwtstamps(skb); + u64 ns; + unsigned long flags; + + spin_lock_irqsave(&priv->tc_lock, flags); + ns = timecounter_cyc2time(&priv->tc, timestamp); + spin_unlock_irqrestore(&priv->tc_lock, flags); + hwtstamps->hwtstamp = ns_to_ktime(ns); +} + +void ctucan_timestamp_init(struct ctucan_priv *priv) +{ + unsigned long flags; + + spin_lock_init(&priv->tc_lock); + + spin_lock_irqsave(&priv->tc_lock, flags); + timecounter_init(&priv->tc, &priv->cc, ktime_get_real_ns()); + spin_unlock_irqrestore(&priv->tc_lock, flags); + + INIT_DELAYED_WORK(&priv->timestamp, ctucan_timestamp_work); + schedule_delayed_work(&priv->timestamp, priv->work_delay_jiffies); +} + +void ctucan_timestamp_stop(struct ctucan_priv *priv) +{ + cancel_delayed_work_sync(&priv->timestamp); +}
This patch adds support for retrieving hardware timestamps to RX and error CAN frames. It uses timecounter and cyclecounter structures, because the timestamping counter width depends on the IP core integration (it might not always be 64-bit). For platform devices, you should specify "ts" clock in device tree. For PCI devices, the timestamping frequency is assumed to be the same as bus frequency. Signed-off-by: Matej Vasilevski <matej.vasilevski@seznam.cz> --- drivers/net/can/ctucanfd/Kconfig | 2 +- drivers/net/can/ctucanfd/Makefile | 2 +- drivers/net/can/ctucanfd/ctucanfd.h | 21 ++ drivers/net/can/ctucanfd/ctucanfd_base.c | 229 +++++++++++++++++- drivers/net/can/ctucanfd/ctucanfd_pci.c | 7 +- drivers/net/can/ctucanfd/ctucanfd_platform.c | 7 +- drivers/net/can/ctucanfd/ctucanfd_timestamp.c | 77 ++++++ 7 files changed, 333 insertions(+), 12 deletions(-) create mode 100644 drivers/net/can/ctucanfd/ctucanfd_timestamp.c