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:

quoted

Hello everyone,

As for this specific case, I am aware of it for longer time

quoted

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@cmp.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