My message had a second part, did you miss it?
On Wed. 4 Oct. 2023 at 21:55, Vincent Mailhol <vincent.mailhol@xxxxxxxxx> wrote:
> On Wed. 4 Oct. 2023 at 18:24, Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> 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
>
> > + *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.
>
> 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.
>
> > 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.
