On 04.10.2023 21:55:41, Vincent Mailhol wrote:
> > @@ -638,7 +613,9 @@ static void at91_read_mb(struct net_device *dev, unsigned int mb,
> > else
> > cf->can_id = FIELD_GET(AT91_MID_MIDVA_MASK, reg_mid);
> >
> > - reg_msr = at91_read(priv, AT91_MSR(mb));
> > + /* extend timstamp to full 32 bit */
> ^^^^^^^^
>
> timestamp
fixed
> > + *timestamp = FIELD_GET(AT91_MSR_MTIMESTAMP_MASK, reg_msr) << 16;
>
> If I understand correctly, you only use the hardware timestamp for the
> napi but you do not report it to the userland.
ACK
> Not a criticism of this series, but it seems to me that it would be
> easy to add one follow-up patch that would populate
> skb_shared_hwtstamps->hwtstamp and update ethtool_ops->get_ts_info in
> order to report those hardware timestamps to the user.
Yes. I have proof-of-concept patches for it laying around, but I want to
get this mainline first. One limitation of the hardware is that the
timer is only 16 bits wide and runs on CAN clock, which means a maximum
of 1MHz. This causes the timer to overflow every 64ms, which in turn
requires a worker every 30ms or so. For this reason, I want hardware TS
to be configurable and this is not yet implemented. Also $CUSTOMER
doesn't need HW timestamps :)
> > cf->len = can_cc_dlc2len(FIELD_GET(AT91_MSR_MDLC_MASK, reg_msr));
> >
> > if (reg_msr & AT91_MSR_MRTR) {
> > @@ -652,151 +629,12 @@ static void at91_read_mb(struct net_device *dev, unsigned int mb,
> > at91_write(priv, AT91_MID(mb), AT91_MID_MIDE);
> >
> > if (unlikely(mb == get_mb_rx_last(priv) && reg_msr & AT91_MSR_MMI))
> > - at91_rx_overflow_err(dev);
> > -}
> > + at91_rx_overflow_err(offload->dev);
>
> (...)
>
> This concludes my "review" of this series. Because I was scrolling
> through it and not doing anything thorough, I will not be giving my
> review-by tag even if there is a follow-up v2. That said, aside from
> my comment on patch 01/27 and the random typo nitpick, nothing seemed
> off.
Thanks for looking into it.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
Attachment:
signature.asc
Description: PGP signature
