Patchwork spi/mpc52xx: check for invalid PSC usage

login
register
mail settings
Submitter Wolfram Sang
Date Nov. 2, 2009, 1:53 p.m.
Message ID <20091102135244.GC4696@pengutronix.de>
Download mbox | patch
Permalink /patch/37417/
State Awaiting Upstream
Delegated to: Grant Likely
Headers show

Comments

Wolfram Sang - Nov. 2, 2009, 1:53 p.m.
> I wouldn't even bother.  It's not actively dangerous to try and use
> PSC{4,5} in SPI mode.  It just not going to work.  Besides, the
> MPC5200 common code already checks for an invalid PSC number when
> setting the clock divisor.
> 
> Have you seen cases of users trying to do the wrong thing with the
> crippled PSCs?

Yes, that was the reason for this patch :) How about this patch to give users a
better idea than just -ENODEV via set_psc_clkdiv? ...[/me hacks]...
Uuuh, there is even a bug which makes the case go unnoticed.
Grant Likely - Nov. 2, 2009, 6:03 p.m.
On Mon, Nov 2, 2009 at 6:53 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>> I wouldn't even bother.  It's not actively dangerous to try and use
>> PSC{4,5} in SPI mode.  It just not going to work.  Besides, the
>> MPC5200 common code already checks for an invalid PSC number when
>> setting the clock divisor.
>>
>> Have you seen cases of users trying to do the wrong thing with the
>> crippled PSCs?
>
> Yes, that was the reason for this patch :) How about this patch to give users a
> better idea than just -ENODEV via set_psc_clkdiv? ...[/me hacks]...
> Uuuh, there is even a bug which makes the case go unnoticed.

Sure.  This looks okay to me.  I'll pick it up.

g.

Patch

===

Subject: [PATCH] spi/mpc52xx-psc-spi: check for valid PSC

This driver calls mpc52xx_set_psc_clkdiv() but doesn't check its return value
to see if the PSC is actually valid for SPI use. Add the check and a hint for
the user.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/spi/mpc52xx_psc_spi.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/mpc52xx_psc_spi.c b/drivers/spi/mpc52xx_psc_spi.c
index 1b74d5c..e268437 100644
--- a/drivers/spi/mpc52xx_psc_spi.c
+++ b/drivers/spi/mpc52xx_psc_spi.c
@@ -313,11 +313,13 @@  static int mpc52xx_psc_spi_port_config(int psc_id, struct mpc52xx_psc_spi *mps)
 	struct mpc52xx_psc __iomem *psc = mps->psc;
 	struct mpc52xx_psc_fifo __iomem *fifo = mps->fifo;
 	u32 mclken_div;
-	int ret = 0;
+	int ret;
 
 	/* default sysclk is 512MHz */
 	mclken_div = (mps->sysclk ? mps->sysclk : 512000000) / MCLK;
-	mpc52xx_set_psc_clkdiv(psc_id, mclken_div);
+	ret = mpc52xx_set_psc_clkdiv(psc_id, mclken_div);
+	if (ret)
+		return ret;
 
 	/* Reset the PSC into a known state */
 	out_8(&psc->command, MPC52xx_PSC_RST_RX);
@@ -341,7 +343,7 @@  static int mpc52xx_psc_spi_port_config(int psc_id, struct mpc52xx_psc_spi *mps)
 
 	mps->bits_per_word = 8;
 
-	return ret;
+	return 0;
 }
 
 static irqreturn_t mpc52xx_psc_spi_isr(int irq, void *dev_id)
@@ -410,8 +412,10 @@  static int __init mpc52xx_psc_spi_do_probe(struct device *dev, u32 regaddr,
 		goto free_master;
 
 	ret = mpc52xx_psc_spi_port_config(master->bus_num, mps);
-	if (ret < 0)
+	if (ret < 0) {
+		dev_err(dev, "can't configure PSC! Is it capable of SPI?\n");
 		goto free_irq;
+	}
 
 	spin_lock_init(&mps->lock);
 	init_completion(&mps->done);