netdev - Re: ctucanfd: possible coding error in ctucan_set_secondary_sample

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <202512270139.08818.pisa@fel.cvut.cz>
Date: Sat, 27 Dec 2025 01:39:08 +0100
From: Pavel Pisa <pisa@....cvut.cz>
To: Ondrej Ille <ondrej.ille@...il.com>
Cc: David Laight <david.laight.linux@...il.com>,
 "Marc Kleine-Budde" <mkl@...gutronix.de>,
 Andrea Daoud <andreadaoud6@...il.com>,
 linux-can@...r.kernel.org,
 Wolfgang Grandegger <wg@...ndegger.com>,
 "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>,
 Christophe JAILLET <christophe.jaillet@...adoo.fr>,
 netdev@...r.kernel.org
Subject: Re: ctucanfd: possible coding error in ctucan_set_secondary_sample_point causing SSP not enabled

Correction

On Saturday 27 of December 2025 01:12:19 Pavel Pisa wrote:
> Dear Ondrej Ille,
>
> On Friday 26 of December 2025 23:45:55 Ondrej Ille wrote:
> > Hello everyone,
> >
> > As for this specific case, I am aware of it for longer time
> >
> > > but the last time when we met with Ondrej Ille this part
> > > as been the last one on the table and the firm confirmation
> > > what is the best value have not been stated.
> >
> > Pavel, the sample point should definitely be set to the variant of
> > "measured + offset". That is
> > what why the offset is calculated above the way it is. The aim is to
> > place the sample point
> > on CAN_RX "as-if the same as normal sample point", just with the TX->RX
> > delay accounted for.
> > One only needs to be careful that the value is not in Time Quantas, but
> > in "number of cycles".
> > But AFAICT should be accounted for by the ssp_offset calculation.
> >
> > As for leaving there comment, or leaving it up to the compiler to
> > optimize away since the value is
> > anyway initialized to zero, I don't know what is the preferred way in
> > kernel.
> > Personally, I would not leave any "meaningless code", so, I think comment
> > is better.
>
> I am not sure, I would probably prefer code there because it can be
> easily modified to other value or make it configurable. So this
> would look like
>
>         if (dbt->bitrate > 1000000) {
>                 /* Calculate SSP in minimal time quanta */
>                 ssp_offset = (priv->can.clock.freq / 1000) *
> dbt->sample_point / dbt->bitrate;
>
>                 if (ssp_offset > 127) {
>                         netdev_warn(ndev, "SSP offset saturated to 127\n");
>                         ssp_offset = 127;
>                 }
>
>                 ssp_cfg = FIELD_PREP(REG_TRV_DELAY_SSP_OFFSET, ssp_offset);
>                 ssp_cfg |= FIELD_PREP(REG_TRV_DELAY_SSP_SRC, 0x0);
>         }
>
> It matches currents state of the driver in the IP CORE repository where
> you have propagated change lat week
>
> https://gitlab.fel.cvut.cz/canbus/ctucanfd_ip_core/-/commit/6188ca673f82387
>3f7b37dbb588a2d4c0a0cc98c
>
> But I would suggest to cover even else case for dbt->bitrate <= 1 Mbit/s by
> else statement which should be probably
>
>         } else {
>                 ssp_cfg = FIELD_PREP(REG_TRV_DELAY_SSP_OFFSET, 0);
>                 ssp_cfg |= FIELD_PREP(REG_TRV_DELAY_SSP_SRC, 0x0);
>         }

As I have looked only onto patch, I have overlooked that whole
register is reset to zero if condition (dbt->bitrate > 1000000)
is not true. So then the

ssp_cfg |= FIELD_PREP(REG_TRV_DELAY_SSP_SRC, 0x0);

can is equivalent to the default and can be left in common
path (if we consider value 0 as right value for all cases)
or removed alltogether. So I am not sure at this moment
if the whole line should be removed or set to zero
and moved out of if block.

Best wishes,

                Pavel

                Pavel Pisa
    phone:      +420 603531357
    e-mail:     pisa@....felk.cvut.cz
    Department of Control Engineering FEE CVUT
    Karlovo namesti 13, 121 35, Prague 2
    university: http://control.fel.cvut.cz/
    personal:   http://cmp.felk.cvut.cz/~pisa
    social:     https://social.kernel.org/ppisa
    projects:   https://www.openhub.net/accounts/ppisa
    CAN related:http://canbus.pages.fel.cvut.cz/
    RISC-V education: https://comparch.edu.cvut.cz/
    Open Technologies Research Education and Exchange Services
    https://gitlab.fel.cvut.cz/otrees/org/-/wikis/home