Re: [PATCH] spi: Limit the spi device max speed to controller's max speed

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Mark, Tudor,

On Fri, Dec 11, 2020 at 8:02 PM Mark Brown <broonie@xxxxxxxxxx> 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.

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.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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