diff mbox series

[resend,3/5] mpc8xxx_spi: put max_cs to use

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

Commit Message

Rasmus Villemoes Feb. 11, 2020, 3:20 p.m. UTC
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(-)

Comments

Tom Rini March 31, 2020, 7:10 p.m. UTC | #1
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 mbox series

Patch

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);