Patchwork spi_mpc8xxx.c: chip select polarity problem

login
register
mail settings
Submitter Anton Vorontsov
Date Nov. 16, 2009, 6 p.m.
Message ID <20091116180008.GA6748@oksana.dev.rtsoft.ru>
Download mbox | patch
Permalink /patch/38532/
State Superseded
Delegated to: Grant Likely
Headers show

Comments

Anton Vorontsov - Nov. 16, 2009, 6 p.m.
On Mon, Nov 16, 2009 at 08:10:37PM +0300, Anton Vorontsov wrote:
[...]
> > I also tried to set alow_flags[x] = 0 for active low. In this case the 
> > transfer works, but the initial value for the CS is wrong (Low instead of 
> > High).
> 
> So it might be better to fix up initial value in the platform code?

Oh, we actually cannot, because the driver calls
gpio_direction_output().

And since we don't know the mode prior to SPI device's driver
probe() finished, we'll have to set up an initial state in the
first SPI transfer. I.e. something like this:
Torsten Fleischer - Nov. 17, 2009, 8:09 p.m.
On Mon, Nov 16, 2009 at 19:00PM, Anton Vorontsov wrote:
[...]
> > So it might be better to fix up initial value in the platform code?
> 
> Oh, we actually cannot, because the driver calls
> gpio_direction_output().
> 
> And since we don't know the mode prior to SPI device's driver
> probe() finished, we'll have to set up an initial state in the
> first SPI transfer. I.e. something like this:

In most cases the device drivers perform SPI transfers already in their 
probe() function. How can it be ensured that the CS of all other devices are 
inactive even if they are not initialized at that time?

Regards
Torsten Fleischer
Anton Vorontsov - Nov. 17, 2009, 8:22 p.m.
On Tue, Nov 17, 2009 at 09:09:28PM +0100, Torsten Fleischer wrote:
> On Mon, Nov 16, 2009 at 19:00PM, Anton Vorontsov wrote:
> [...]
> > > So it might be better to fix up initial value in the platform code?
> > 
> > Oh, we actually cannot, because the driver calls
> > gpio_direction_output().
> > 
> > And since we don't know the mode prior to SPI device's driver
> > probe() finished, we'll have to set up an initial state in the
> > first SPI transfer. I.e. something like this:
> 
> In most cases the device drivers perform SPI transfers already in their 
> probe() function. How can it be ensured that the CS of all other devices are 
> inactive even if they are not initialized at that time?

Good question. Oh, well... then we have to use spi-cs-high,
no matter that it is a duplication of the 'compatible' property.
SPI bus drivers don't know all the devices and their CS level,
and so spi-cs-high is the only way to tell that information. :-(
Anton Vorontsov - Nov. 17, 2009, 11:28 p.m.
On Tue, Nov 17, 2009 at 11:22:11PM +0300, Anton Vorontsov wrote:
> On Tue, Nov 17, 2009 at 09:09:28PM +0100, Torsten Fleischer wrote:
> > On Mon, Nov 16, 2009 at 19:00PM, Anton Vorontsov wrote:
> > [...]
> > > > So it might be better to fix up initial value in the platform code?
> > > 
> > > Oh, we actually cannot, because the driver calls
> > > gpio_direction_output().
> > > 
> > > And since we don't know the mode prior to SPI device's driver
> > > probe() finished, we'll have to set up an initial state in the
> > > first SPI transfer. I.e. something like this:
> > 
> > In most cases the device drivers perform SPI transfers already in their 
> > probe() function. How can it be ensured that the CS of all other devices are 
> > inactive even if they are not initialized at that time?
> 
> Good question. Oh, well... then we have to use spi-cs-high,
> no matter that it is a duplication of the 'compatible' property.
> SPI bus drivers don't know all the devices and their CS level,
> and so spi-cs-high is the only way to tell that information. :-(

Oh. On the other hand, we can postpone the gpio_direction_output()
call, and still require that the platform code (or firmware)
should be responsible for setting a sane default values on the
chip selects.

Patch

diff --git a/drivers/spi/spi_mpc8xxx.c b/drivers/spi/spi_mpc8xxx.c
index 0fd0ec4..3b95382 100644
--- a/drivers/spi/spi_mpc8xxx.c
+++ b/drivers/spi/spi_mpc8xxx.c
@@ -114,6 +114,7 @@  struct spi_mpc8xxx_cs {
 	u32 rx_shift;		/* RX data reg shift when in qe mode */
 	u32 tx_shift;		/* TX data reg shift when in qe mode */
 	u32 hw_mode;		/* Holds HW mode register settings */
+	int initialized;
 };
 
 static inline void mpc8xxx_spi_write_reg(__be32 __iomem *reg, u32 val)
@@ -507,11 +508,17 @@  static int mpc8xxx_spi_transfer(struct spi_device *spi,
 				struct spi_message *m)
 {
 	struct mpc8xxx_spi *mpc8xxx_spi = spi_master_get_devdata(spi->master);
+	struct spi_mpc8xxx_cs *cs = spi->controller_state;
 	unsigned long flags;
 
 	m->actual_length = 0;
 	m->status = -EINPROGRESS;
 
+	if (cs && !cs->initialized) {
+		mpc8xxx_spi_chipselect(spi, BITBANG_CS_INACTIVE);
+		cs->initialized = 1;
+	}
+
 	spin_lock_irqsave(&mpc8xxx_spi->lock, flags);
 	list_add_tail(&m->queue, &mpc8xxx_spi->queue);
 	queue_work(mpc8xxx_spi->workqueue, &mpc8xxx_spi->work);