Hi Vincent,
On 15.12.24 10:21, Vincent Mailhol wrote:
I am done reading (and understanding) ISO 11898-1:2024, so let's resume
the work!
Good! ;-)
On 04/12/2024 at 16:56, Oliver Hartkopp wrote:
Hi Vincent,
On 03.12.24 10:45, Vincent Mailhol wrote:
https://lore.kernel.org/linux-can/20241201112333.6950-1-
socketcan@xxxxxxxxxxxx/T/#u
https://lore.kernel.org/linux-can/20241201112230.6917-1-
socketcan@xxxxxxxxxxxx/T/#t
Thanks for all the testing and the fixes. Because of the lack of
testing of this RFC on my side, I was expecting such issues. But I
really appreciate that you took time to investigate and debug, really
helpful! I will make sure to incorporate these fixes in the next
version.
I'll send out an extended RFC V2 which is my current test base for you
in some minutes.
Either for the kernel and iproute2 there are two new patches that add
new controlmode flags.
The next series I send will add the pwm and drop the RFC patch.
Excellent!
I assume it will follow the TDC pattern with
CAN_CTRLMODE_XL_PWM_AUTO and CAN_CTRLMODE_XL_PWM_MANUAL
or something similar?
I am not sure why we would need a tristate here. The reason why I put
the tristate for the TDC is because some transceiver (the majority) will
automatically measure TDCV but some may give the option for the user to
manually set it.
From what I understand from the specification, PWMO is not something
which is dynamically measured. If I got it right, it is just the
remainder whenever the bit time is not a multiple of PWM. Something like
this:
pwmo = bit_time_in_minimum_time_quantum % pwm;
Same for the PWMS and PWML. I am not yet sure how to calculate those
(more on this below) but these seem to be calculated once for all (like
any bittimming value other than the TDCs).
Let me know if I missed something, the PWM is very new to me :)
So, for the implementation, I am thinking of:
The PWM stuff is some very CAN XL specific therefore I would suggest to
bring this into the naming too:
1. Add a CAN_CTRLMODE_PWM mode and a new nest for the PWM in netlink.h
CAN_CTRLMODE_XL_PWM or CAN_CTRLMODE_XLPWM
2. Add these to bittiming.c:
struct can_pwm {
can_xl_pwm or can_xlpwm
u32 pwms; /* PWM short phase length */
u32 pwml; /* PWM long phase length */
u32 pwmo; /* PWM offset */
that's fine
};
struct can_pwm_const {
dito
u32 pwms_max;
u32 pwml_max;
u32 pwmo_max;
};
static inline u32 can_get_pwm(const struct can_pwm *pwm)
{
return pwm->pwms + pwm->pwml;
}
??
The minimum value of all those configurable ranges is already specified
to be one minimum time quantum (tq min), so I do not think we need a
field for the minimums.
At the moment, I am stuck on the PWM calculation. I do not know how to
derive good PWMS and PWML values out of the const ranges.
The ISO 11898-1:2024 tells me that CiA 612-2 has recommendations on the
topic. But getting access to this document requires a subscription
(which I do not have):
https://www.can-cia.org/can-knowledge/cia-612-series-can-xl-guidelines-and-application-notes
Do any of you have access to this document? Or do any of you know a good
formula for the PWMS and PWML calculation?
I have to check for the referenced document.
Btw. this is some code that shows the idea of how to calculate the PWM
values based on the CAN clock and the XL data bitrate:
if (CanClock_mhz == 100 && XlDatarate_mbps == 5) {
// @5Mbit/s
pwmo = 0;
pwms = 6;
pwml = 14;
} else if (CanClock_mhz == 100) {
// @10Mbit/s
pwmo = 0;
pwms = 3;
pwml = 7;
} else if (CanClock_mhz == 160) {
// @10Mbit/s
pwmo = 0;
pwms = 2;
pwml = 6;
}
For that reason I was thinking about some default calculation provided
by the kernel (CAN_CTRLMODE_XL_PWM_AUTO) and some manual setting if
people want to set the values analog to tseg1 and friends instead of
setting a single bitrate.
Best regards,
Oliver
