Skip to content

hostmot2: fix input bounds, divisors and PWM clamping #4661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sleepybishop wants to merge 14 commits into
base: master
Choose a base branch
from
Open

hostmot2: fix input bounds, divisors and PWM clamping #4661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
1f8381d
hostmot2: validate UART sends and pack bytes without signed shifts
sleepybishop Oct 10, 2026
54043f9
hostmot2: validate BSPI channels and clock frequency
sleepybishop Oct 10, 2026
77467a9
hostmot2: clamp input scan divisors safely
sleepybishop Aug 30, 2026
b031eb5
hostmot2: bound connector pin names and correct port diagnostics
sleepybishop Oct 10, 2026
e5dedcd
hostmot2: reject zero resolver index divisors
sleepybishop Oct 10, 2026
510012e
hostmot2: decode the slowest RPi3 SPI clock divisor
sleepybishop Oct 10, 2026
c4af04e
hostmot2: clamp negative B and C phase PWM values
sleepybishop Oct 10, 2026
de916be
hostmot2: preserve UART send validation error returns
sleepybishop Oct 10, 2026
d68621a
hostmot2: handle UART and BSPI LLIO failures consistently
sleepybishop Oct 10, 2026
74bc083
hostmot2: round minimum INM and INMUX scan rates up
sleepybishop Oct 10, 2026
b9b5a0f
hostmot2: invalidate UART rate before reconfiguration
sleepybishop Oct 10, 2026
1ca33cf
hostmot2: align LLIO checks and test flags
sleepybishop Oct 12, 2026
813d668
hostmot2: trim packing and drop test harness
sleepybishop Nov 1, 2026
439d27a
hostmot2: restore original whitespace
sleepybishop Nov 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/src/man/man3/hm2_bspi_write_chan.3.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ Not available in non-realtime components.

== RETURN VALUE

Returns 0 on success and -1 on failure.
Returns a positive value on success and -1 on failure.

== SEE ALSO

Expand Down
18 changes: 16 additions & 2 deletions src/hal/drivers/mesa-hostmot2/bspi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ int hm2_tram_add_bspi_frame(char *name, int chan, rtapi_u32 **wbuff, rtapi_u32 *
HM2_ERR_NO_LL("Can not find BSPI instance %s.\n", name);
return -1;
}
if (chan < 0 || chan > 15) {
HM2_ERR("BSPI %s: channel number %i is out of range\n", name, chan);
return -1;
}
if (hm2->bspi.instance[i].conf_flag[chan] != true){
HM2_ERR("The selected write channel (%i) on bspi instance %s.\n"
"Has not been configured.\n", chan, name);
Expand Down Expand Up @@ -187,8 +191,9 @@ int hm2_bspi_clear_fifo(char * name)
}
rtapi_u32 zero = 0;
r = hm2->llio->write(hm2->llio, hm2->bspi.instance[i].count_addr, &zero, sizeof(rtapi_u32));
if (r < 0) {
if (!r) {
HM2_ERR("BSPI: hm2->llio->write failure %s\n", name);
return -1;
}
Comment on lines 193 to 197

@BsAtHome BsAtHome Oct 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

llio->write() and llio->read() functions are supposed to return not zero on success and zero on failure.
From hostmot2-lowlevel.h:

    // on success these two return TRUE (not zero)
    // on failure they return FALSE (0) and set *self->io_error (below) to TRUE
    int (*read)(hm2_lowlevel_io_t *self, rtapi_u32 addr, void *buffer, int size);
    int (*write)(hm2_lowlevel_io_t *self, rtapi_u32 addr, const void *buffer, int size);

Any implementation that does not adhere to this should be fixed.

(same test other places as well)

@sleepybishop sleepybishop Nov 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed in 1ca33cf: zero means failure; nonzero means success.


return r;
Expand All @@ -205,14 +210,19 @@ int hm2_bspi_write_chan(char* name, int chan, rtapi_u32 val)
HM2_ERR_NO_LL("Can not find BSPI instance %s.\n", name);
return -1;
}
if (chan < 0 || chan > 15) {
HM2_ERR("BSPI %s: channel number %i is out of range\n", name, chan);
return -1;
}
if (hm2->bspi.instance[i].conf_flag[chan] != true){
HM2_ERR("The selected write channel (%i) on bspi instance %s.\n"
"Has not been configured.\n", chan, name);
return -1;
}
r = hm2->llio->write(hm2->llio, hm2->bspi.instance[i].addr[chan], &buff, sizeof(rtapi_u32));
if (r < 0) {
if (!r) {
HM2_ERR("BSPI: hm2->llio->write failure %s\n", name);
return -1;
}

return r;
Expand Down Expand Up @@ -247,6 +257,10 @@ int hm2_bspi_setup_chan(char *name, int chan, int cs, int bits, double mhz,
"BSPI only supports 1-64 bits\n", name, chan, bits);
return -1;
}
if (mhz <= 0) {
HM2_ERR("BSPI %s: clock frequency must be greater than zero\n", name);
return -1;
}
if (delay < 0 || delay > 1e6){
HM2_ERR("The requested frame delay on channel %i of %inS seems "
"rather implausible for an SPI device. Exiting.\n", delay, chan);
Expand Down
35 changes: 30 additions & 5 deletions src/hal/drivers/mesa-hostmot2/inm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,21 @@ void hm2_inm_force_write(hostmot2_t *hm2) {
scanrate = 10000000;
hal_set_ui32(inst->hal.param.scan_rate, scanrate/inst->scanwidth);
}
divisor = (hm2->inm.clock_frequency / (4 * scanrate)) - 1;
if (scanrate <= 0) {
divisor = 1023;
// Round up so the normalized rate stays within the divisor limit.
hal_set_ui32(inst->hal.param.scan_rate,
(hm2->inm.clock_frequency - 1)
/ (4 * (divisor + 1) * inst->scanwidth) + 1);
} else {
divisor = (hm2->inm.clock_frequency / (4 * scanrate)) - 1;
if (divisor > 1023) {
divisor = 1023;
hal_set_ui32(inst->hal.param.scan_rate,
(hm2->inm.clock_frequency - 1)
/ (4 * (divisor + 1) * inst->scanwidth) + 1);
}
}
rtapi_u32 fast_scans = hal_get_ui32(inst->hal.param.fast_scans);
if (fast_scans > 63) {
fast_scans = hal_set_ui32(inst->hal.param.fast_scans, 63);
Expand Down Expand Up @@ -472,13 +486,24 @@ void hm2_inm_write(hostmot2_t *hm2) {
hal_set_ui32(inst->hal.param.scan_rate, scanrate/inst->scanwidth);
HM2_ERR("inm %d scanrate too high, resetting to %d \n", i, hal_get_ui32(inst->hal.param.scan_rate));
}
divisor = (hm2->inm.clock_frequency / (4 * scanrate)) - 1;
// bound divisor so we dont splatter into other fields
if ((divisor > 1023 ) | (scanrate == 0 )) {
if (scanrate <= 0) {
divisor = 1023;
hal_set_ui32(inst->hal.param.scan_rate, (hm2->inm.clock_frequency/4)/(divisor +1)
/inst->scanwidth);
// Round up so the normalized rate stays within the divisor limit.
hal_set_ui32(inst->hal.param.scan_rate,
(hm2->inm.clock_frequency - 1)
/ (4 * (divisor + 1) * inst->scanwidth) + 1);
HM2_ERR("inm %d scanrate too low, resetting to %d \n", i, hal_get_ui32(inst->hal.param.scan_rate));
} else {
divisor = (hm2->inm.clock_frequency / (4 * scanrate)) - 1;
if (divisor > 1023) {
divisor = 1023;
hal_set_ui32(inst->hal.param.scan_rate,
(hm2->inm.clock_frequency - 1)
/ (4 * (divisor + 1) * inst->scanwidth) + 1);
HM2_ERR("inm %d scanrate too low, resetting to %d \n",
i, hal_get_ui32(inst->hal.param.scan_rate));
}
}
rtapi_u32 fast_scans = hal_get_ui32(inst->hal.param.fast_scans);
if (fast_scans > 63) {
Expand Down
33 changes: 29 additions & 4 deletions src/hal/drivers/mesa-hostmot2/inmux.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,21 @@ void hm2_inmux_force_write(hostmot2_t *hm2) {
muxrate = 5000000;
hal_set_ui32(inst->hal.param.scan_rate, muxrate/inst->scanwidth);
}
divisor = (hm2->inmux.clock_frequency / (4 * muxrate)) - 1;
if (muxrate <= 0) {
divisor = 1023;
// Round up so the normalized rate stays within the divisor limit.
hal_set_ui32(inst->hal.param.scan_rate,
(hm2->inmux.clock_frequency - 1)
/ (4 * (divisor + 1) * inst->scanwidth) + 1);
} else {
divisor = (hm2->inmux.clock_frequency / (4 * muxrate)) - 1;
if (divisor > 1023) {
divisor = 1023;
hal_set_ui32(inst->hal.param.scan_rate,
(hm2->inmux.clock_frequency - 1)
/ (4 * (divisor + 1) * inst->scanwidth) + 1);
}
}
rtapi_u32 fast_scans = hal_get_ui32(inst->hal.param.fast_scans);
if (fast_scans > 63) {
fast_scans = hal_set_ui32(inst->hal.param.fast_scans, 63);
Expand Down Expand Up @@ -394,13 +408,24 @@ void hm2_inmux_write(hostmot2_t *hm2) {
hal_set_ui32(inst->hal.param.scan_rate, muxrate/inst->scanwidth);
HM2_ERR("InMux %d scanrate too high, resetting to %d \n", i, hal_get_ui32(inst->hal.param.scan_rate));
}
divisor = (hm2->inmux.clock_frequency / (4 * muxrate)) - 1;
// bound divisor so we dont splatter into other fields
if ((divisor > 1023 ) | (muxrate == 0 )) {
if (muxrate <= 0) {
divisor = 1023;
// Round up so the normalized rate stays within the divisor limit.
hal_set_ui32(inst->hal.param.scan_rate,
(hm2->inmux.clock_frequency/4) / (divisor +1) / inst->scanwidth);
(hm2->inmux.clock_frequency - 1)
/ (4 * (divisor + 1) * inst->scanwidth) + 1);
HM2_ERR("InMux %d scanrate too low, resetting to %d \n", i, hal_get_ui32(inst->hal.param.scan_rate));
} else {
divisor = (hm2->inmux.clock_frequency / (4 * muxrate)) - 1;
if (divisor > 1023) {
divisor = 1023;
hal_set_ui32(inst->hal.param.scan_rate,
(hm2->inmux.clock_frequency - 1)
/ (4 * (divisor + 1) * inst->scanwidth) + 1);
HM2_ERR("InMux %d scanrate too low, resetting to %d \n",
i, hal_get_ui32(inst->hal.param.scan_rate));
}
}
rtapi_u32 fast_scans = hal_get_ui32(inst->hal.param.fast_scans);
if (fast_scans > 63) {
Expand Down
6 changes: 4 additions & 2 deletions src/hal/drivers/mesa-hostmot2/pins.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ int hm2_read_pin_descriptors(hostmot2_t *hm2) {

if (pin->port_num >= hm2->llio->num_ioport_connectors) {
HM2_ERR("hm2_read_pin_descriptors: Calculated port number (%d) is "
"invalid\n", pin->port_pin );
"invalid\n", pin->port_num );
return -EINVAL;
}

Expand Down Expand Up @@ -894,7 +894,9 @@ void hm2_print_pin_usage(hostmot2_t *hm2) {

char connector_pin_name[100];

if (hm2->llio->io_connector_pin_names == NULL) {
if (hm2->llio->io_connector_pin_names == NULL
|| (unsigned)i >= (hm2->llio->num_ioport_connectors
* hm2->llio->pins_per_connector)) {
snprintf(connector_pin_name, sizeof(connector_pin_name), "%s-%02d", hm2->llio->ioport_connector_name[pin->port_num], pin->port_pin);
} else {
if (hm2->llio->io_connector_pin_names[i] == NULL) {
Expand Down
9 changes: 7 additions & 2 deletions src/hal/drivers/mesa-hostmot2/resolver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,12 @@ void hm2_resolver_process_tram_read(hostmot2_t *hm2, long period) {
for (i = 0; i < hm2->resolver.num_resolvers; i ++) {

res = &hm2->resolver.instance[i];
rtapi_u32 index_div = hal_get_ui32(res->hal.param.index_div);

if (index_div == 0) {
HM2_ERR("resolver.%02d.index-div == 0, bogus, setting to 1\n", i);
index_div = hal_set_ui32(res->hal.param.index_div, 1);
}

scale = hal_get_real(res->hal.param.scale);

Expand Down Expand Up @@ -326,7 +332,6 @@ void hm2_resolver_process_tram_read(hostmot2_t *hm2, long period) {
if ((res->old_reg > hm2->resolver.position_reg[i]) && (res->old_reg - hm2->resolver.position_reg[i] > 0x80000000)){
res->index_cnts++;
if (hal_get_bool(res->hal.pin.index_enable)){
rtapi_u32 index_div = hal_get_ui32(res->hal.param.index_div);
int r = (res->index_cnts % index_div);
if ((index_div > 1 && r == 1)
|| (index_div == 1 && r == 0)){
Expand All @@ -337,7 +342,7 @@ void hm2_resolver_process_tram_read(hostmot2_t *hm2, long period) {
}
else if ((res->old_reg < hm2->resolver.position_reg[i]) && (hm2->resolver.position_reg[i] - res->old_reg > 0x80000000)){
res->index_cnts--;
if (hal_get_bool(res->hal.pin.index_enable) && (res->index_cnts % hal_get_ui32(res->hal.param.index_div) == 0)){
if (hal_get_bool(res->hal.pin.index_enable) && (res->index_cnts % index_div == 0)){
res->offset = (res->accum - hm2->resolver.position_reg[i] + 0x100000000LL);
hal_set_bool(res->hal.pin.index_enable, 0);
}
Expand Down
4 changes: 2 additions & 2 deletions src/hal/drivers/mesa-hostmot2/spix_rpi3.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -850,8 +850,8 @@ static const spix_port_t *rpi3_open(int port, const spix_args_t *args)
if(!rpp->spiport) {
rpp->clkdivw = spi0_clkdiv_calc(spiclk_base, args->clkw);
rpp->clkdivr = spi0_clkdiv_calc(spiclk_base, args->clkr);
ccw = spiclk_base / rpp->clkdivw;
ccr = spiclk_base / rpp->clkdivr;
ccw = spiclk_base / (rpp->clkdivw ? rpp->clkdivw : 65536);
ccr = spiclk_base / (rpp->clkdivr ? rpp->clkdivr : 65536);
} else {
rpp->clkdivw = spi1_clkdiv_calc(spiclk_base, args->clkw);
rpp->clkdivr = spi1_clkdiv_calc(spiclk_base, args->clkr);
Expand Down
4 changes: 2 additions & 2 deletions src/hal/drivers/mesa-hostmot2/tp_pwmgen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,9 +443,9 @@ void hm2_tp_pwmgen_prepare_tram_write(hostmot2_t *hm2) {
if (scaled_Avalue > 1.0) scaled_Avalue = 1.0;
else if (scaled_Avalue < -1.0) scaled_Avalue = -1.0;
if (scaled_Bvalue > 1.0) scaled_Bvalue = 1.0;
else if (scaled_Avalue < -1.0) scaled_Avalue = -1.0;
else if (scaled_Bvalue < -1.0) scaled_Bvalue = -1.0;
if (scaled_Cvalue > 1.0) scaled_Cvalue = 1.0;
else if (scaled_Avalue < -1.0) scaled_Avalue = -1.0;
else if (scaled_Cvalue < -1.0) scaled_Cvalue = -1.0;

// duty_cycle goes from 0.0 to 1.0, and needs to be puffed out to 10 bits

Expand Down
Loading
Loading