| Message ID | 20200211151947.26091-4-rasmus.villemoes@prevas.dk |
|---|---|
| State | Superseded |
| Headers | show |
| Series | spi: mpc8xxx_spi: bug fixes, real ->set_speed and a pseudo-gpio driver | expand |
On Tue, Feb 11, 2020 at 03:20:24PM +0000, Rasmus Villemoes wrote: > Currently, max_cs is write-only; it's just set in > mpc8xxx_spi_ofdata_to_platdata and not otherwise used. > > My mpc8309 was always resetting during an "sf probe 0". It turns out > dm_gpio_set_dir_flags() was being called with garbage, since nothing > had initialized priv->gpios[0] - our device tree used "cs-gpios" > rather than "gpios", so gpio_request_list_by_name() had returned 0. > > That would have been a lot easier to figure out if the chip select > index was sanity checked, so rename max_cs to cs_count, and reject a > xfer with a too large cs index. > > Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk> Applied to u-boot/master, thanks!
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index 1c7bf10f91..ac4d0a9bae 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -35,7 +35,7 @@ enum { struct mpc8xxx_priv { spi8xxx_t *spi; struct gpio_desc gpios[16]; - int max_cs; + int cs_count; }; static inline u32 to_prescale_mod(u32 val) @@ -74,7 +74,7 @@ static int mpc8xxx_spi_ofdata_to_platdata(struct udevice *dev) if (ret < 0) return -EINVAL; - priv->max_cs = ret; + priv->cs_count = ret; return 0; } @@ -131,6 +131,11 @@ static int mpc8xxx_spi_xfer(struct udevice *dev, uint bitlen, debug("%s: slave %s:%u dout %08X din %08X bitlen %u\n", __func__, bus->name, platdata->cs, *(uint *)dout, *(uint *)din, bitlen); + if (platdata->cs >= priv->cs_count) { + dev_err(dev, "chip select index %d too large (cs_count=%d)\n", + platdata->cs, priv->cs_count); + return -EINVAL; + } if (flags & SPI_XFER_BEGIN) mpc8xxx_spi_cs_activate(dev);
Currently, max_cs is write-only; it's just set in mpc8xxx_spi_ofdata_to_platdata and not otherwise used. My mpc8309 was always resetting during an "sf probe 0". It turns out dm_gpio_set_dir_flags() was being called with garbage, since nothing had initialized priv->gpios[0] - our device tree used "cs-gpios" rather than "gpios", so gpio_request_list_by_name() had returned 0. That would have been a lot easier to figure out if the chip select index was sanity checked, so rename max_cs to cs_count, and reject a xfer with a too large cs index. Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk> --- drivers/spi/mpc8xxx_spi.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)