Hello Bernhard and Philippe,
On Thursday 23 of July 2026 12:26:41 Bernhard Beschow wrote:
> Am 23. Juli 2026 08:20:34 UTC schrieb "Philippe Mathieu-Daudé"
<[email protected]>:
> >On 23/7/26 09:00, Bernhard Beschow wrote:
> >> As indicated in the comments, the `mb[]` a.k.a. `mbs []` views should
> >> cover 0x400 (1024) bytes. However, the `mb[]` array covers four times
> >> the size since `sizeof(FlexcanRegsMessageBuffer)` returns the size in
> >> bytes instead of counting the number of uint32_t fields. This shifts the
> >> subsequent register offsets by 0xc00 which is not intended. Fix the size
> >> and thus the offsets by dropping the now unused `mb[]` view.
> >>
> >> Note that the different size changes the migration layout. Since the
> >> device model is new, no change in the version fields is needed.
> >
> >typedef struct FlexcanState {
> > ...
> > union {
> > FlexcanRegs regs;
> > uint32_t regs_raw[sizeof(FlexcanRegs) / 4];
> > };
> > ...
> >} FlexcanState;
> >
> >static const VMStateDescription vmstate_can = {
> > .name = TYPE_CAN_FLEXCAN,
> > .version_id = 1,
> > .minimum_version_id = 1,
> > .fields = (const VMStateField[]) {
> > ...
> > VMSTATE_UINT32_ARRAY(regs_raw, FlexcanState, sizeof(FlexcanRegs) /
> > 4),
> >
> > ^^^
> >
> >This "consider this structure as a casted array of u32" is very fragile
> >anyway, and will unlikely scale over time.
> >
> > ...
> > VMSTATE_END_OF_LIST(),
> > },
> >};
>
> Any suggestions what to do instead?
>
I would suggest to keep the state as uint32_t array.
It is accessed by this and only by this size as the
FlexCAN MemoryRegionOps are defined. In the fact
I would like/plan in longer term, even to switch
the mbs array to be plain area of uint32_t in the future,
that is remove field
FlexcanRegsMessageBuffer mbs[FLEXCAN_MAILBOX_COUNT];
in FlexcanRegs.
There should be only function which would return start
of given FlexcanRegsMessageBuffer based on index
argument and actual message buffers configuration.
It is required for CAN FD where message buffer
size is configurable. The raw/uint32_t area is kept
the same, but it is distributed to the indexed
buffers different way.
So my preference is to move from structured view
in direction to the offsets in the words array.
Even the registers could be defined as offsets
to this array in the theory but as the overlay
structure for register accesses is used even
in the Linux kernel driver, I do not think that
this is necessary.
But replacement of
FlexcanRegsMessageBuffer mbs[FLEXCAN_MAILBOX_COUNT];
with appropriate
uint32_t mbs[FLEXCAN_MAILBOX_AREA_WORD_COUNT];
and access functions would please me, because
it helps with future CAN FD support.
Best wishes,
Pavel Pisa
phone: +420 603531357
e-mail: [email protected]
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
