Messages in this thread | | | From | <> | Subject | Re: [PATCH] spi: Limit the spi device max speed to controller's max speed | Date | Wed, 16 Dec 2020 08:25:04 +0000 |
| |
On 12/15/20 4:24 PM, Geert Uytterhoeven wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Hi Mark, Tudor,
Hi, Geert,
> > On Fri, Dec 11, 2020 at 8:02 PM Mark Brown <broonie@kernel.org> wrote: >> On Wed, 9 Dec 2020 19:35:14 +0200, Tudor Ambarus wrote: >>> Make sure the max_speed_hz of spi_device does not override >>> the max_speed_hz of controller. >> >> Applied to >> >> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next >> >> Thanks! >> >> [1/1] spi: Limit the spi device max speed to controller's max speed >> commit: 9326e4f1e5dd1a4410c429638d3c412b6fc17040 > >> - if (!spi->max_speed_hz) >> + if (!spi->max_speed_hz || >> + spi->max_speed_hz > spi->controller->max_speed_hz) >> spi->max_speed_hz = spi->controller->max_speed_hz; > > If spi->controller->max_speed_hz is zero, a non-zero spi->max_speed_hz > will be overwritten by zero. >
oh, yes, you're right.
> Hence this broke spi-sh-msiof, which has the following check in > sh_msiof_spi_set_clk_regs(): > > if (!spi_hz || !parent_rate) { > WARN(1, "Invalid clock rate parameters %lu and %u\n", > parent_rate, spi_hz); > return; > } > > Without this, the driver would trigger a division-by-zero later... > > Arguably all SPI controller drivers should fill in > spi_controller.{min,max}_speed_hz, but as long as that is not the case, > I think this patch should be reverted, or the check should be enhanced > to make sure spi->controller->max_speed_hz is non-zero. >
I'm fine both ways, since this should be improved anyway. Will send a patch with the non-zero check and add your Reported-by, Suggested-by tags, in case Mark decides to keep it.
Cheers, ta
> Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds >
| |