2009-01-23 19:50:22

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH resend 0/6] OpenFirmware support for the spi_mpc83xx driver

On Tue, Jan 06, 2009 at 10:28:10PM -0600, Kumar Gala wrote:
> On Dec 5, 2008, at 2:09 PM, Anton Vorontsov wrote:
>> Hi all,
>>
>> The patch series are used to support SPI via the OF SPI subsystem
>> (driver/of/of_spi.c). Now the driver is able to manage its own
>> chip selects, and doesn't need any auxiliary support from the
>> board files or fsl_soc constructors.
>>
>> The series also contains PowerPC portions of the MMC SPI support.
>>
>> Since the series touches spi and powerpc trees, I think it would
>> be most convenient to pass it via one of these trees. The patches
>> doesn't touch any SPI functionality, only some probe routines, so
>> I believe powerpc.git is the best candidate...
>>
>> The other reason for powerpc tree is that the patches depends on
>> other patches as found in paulus/powerpc.git + of_gpio_count()
>> as found here:
>> http://ozlabs.org/pipermail/linuxppc-dev/2008-December/065917.html
>>
>> David, if you're OK with the patches, may I ask you to sign off on
>> the ones that touch drivers/spi so that we could pass it via Kumar's
>> powerpc.git?
>>
>> The queue:
>>
>> [1/7] powerpc: Implement get_brgfreq() and get_baudrate() stubs
>> [2/7] spi_mpc83xx: Fix sparse warnings
>> [3/7] spi_mpc83xx: Rework chip selects handling
>> [4/7] spi_mpc83xx: Add OF platform driver bindings
>> [5/7] powerpc/fsl_soc: Isolate legacy fsl_spi support to mpc832x_rdb
>> boards
>> [6/7] powerpc: Add mmc-spi-slot bindings
>> [7/7] powerpc/83xx: Add mmc-spi support via the device tree for
>> MPC8323E-RDB
>>
>
> David,
>
> Any status on these patches?

Ping? No single comment on this patch set for ~2 months...

Resending...

Thanks,

--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2


2009-01-23 19:50:46

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 1/6] spi_mpc83xx: Fix sparse warnings

The patch fixes following sparse warnings:

CHECK spi_mpc83xx.c
spi_mpc83xx.c:145:1: warning: symbol 'mpc83xx_spi_rx_buf_u8' was not declared. Should it be static?
spi_mpc83xx.c:146:1: warning: symbol 'mpc83xx_spi_rx_buf_u16' was not declared. Should it be static?
spi_mpc83xx.c:147:1: warning: symbol 'mpc83xx_spi_rx_buf_u32' was not declared. Should it be static?
spi_mpc83xx.c:148:1: warning: symbol 'mpc83xx_spi_tx_buf_u8' was not declared. Should it be static?
spi_mpc83xx.c:149:1: warning: symbol 'mpc83xx_spi_tx_buf_u16' was not declared. Should it be static?
spi_mpc83xx.c:150:1: warning: symbol 'mpc83xx_spi_tx_buf_u32' was not declared. Should it be static?
spi_mpc83xx.c:175:32: warning: incorrect type in initializer (different address spaces)
spi_mpc83xx.c:175:32: expected void *tmp_ptr
spi_mpc83xx.c:175:32: got unsigned int [noderef] <asn:2>*<noident>
spi_mpc83xx.c:183:26: warning: incorrect type in argument 1 (different address spaces)
spi_mpc83xx.c:183:26: expected unsigned int [noderef] [usertype] <asn:2>*reg
spi_mpc83xx.c:183:26: got void *tmp_ptr
spi_mpc83xx.c:184:26: warning: incorrect type in argument 1 (different address spaces)
spi_mpc83xx.c:184:26: expected unsigned int [noderef] [usertype] <asn:2>*reg
spi_mpc83xx.c:184:26: got void *tmp_ptr
spi_mpc83xx.c:287:31: warning: incorrect type in initializer (different address spaces)
spi_mpc83xx.c:287:31: expected void *tmp_ptr
spi_mpc83xx.c:287:31: got unsigned int [noderef] <asn:2>*<noident>
spi_mpc83xx.c:295:25: warning: incorrect type in argument 1 (different address spaces)
spi_mpc83xx.c:295:25: expected unsigned int [noderef] [usertype] <asn:2>*reg
spi_mpc83xx.c:295:25: got void *tmp_ptr
spi_mpc83xx.c:296:25: warning: incorrect type in argument 1 (different address spaces)
spi_mpc83xx.c:296:25: expected unsigned int [noderef] [usertype] <asn:2>*reg
spi_mpc83xx.c:296:25: got void *tmp_ptr
spi_mpc83xx.c:486:13: warning: symbol 'mpc83xx_spi_irq' was not declared. Should it be static?

Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/spi/spi_mpc83xx.c | 16 +++++++++-------
1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi_mpc83xx.c b/drivers/spi/spi_mpc83xx.c
index ac0e3e4..6ef8937 100644
--- a/drivers/spi/spi_mpc83xx.c
+++ b/drivers/spi/spi_mpc83xx.c
@@ -123,6 +123,7 @@ static inline u32 mpc83xx_spi_read_reg(__be32 __iomem * reg)
}

#define MPC83XX_SPI_RX_BUF(type) \
+static \
void mpc83xx_spi_rx_buf_##type(u32 data, struct mpc83xx_spi *mpc83xx_spi) \
{ \
type * rx = mpc83xx_spi->rx; \
@@ -131,6 +132,7 @@ void mpc83xx_spi_rx_buf_##type(u32 data, struct mpc83xx_spi *mpc83xx_spi) \
}

#define MPC83XX_SPI_TX_BUF(type) \
+static \
u32 mpc83xx_spi_tx_buf_##type(struct mpc83xx_spi *mpc83xx_spi) \
{ \
u32 data; \
@@ -172,7 +174,7 @@ static void mpc83xx_spi_chipselect(struct spi_device *spi, int value)

if (cs->hw_mode != regval) {
unsigned long flags;
- void *tmp_ptr = &mpc83xx_spi->base->mode;
+ __be32 __iomem *mode = &mpc83xx_spi->base->mode;

regval = cs->hw_mode;
/* Turn off IRQs locally to minimize time that
@@ -180,8 +182,8 @@ static void mpc83xx_spi_chipselect(struct spi_device *spi, int value)
*/
local_irq_save(flags);
/* Turn off SPI unit prior changing mode */
- mpc83xx_spi_write_reg(tmp_ptr, regval & ~SPMODE_ENABLE);
- mpc83xx_spi_write_reg(tmp_ptr, regval);
+ mpc83xx_spi_write_reg(mode, regval & ~SPMODE_ENABLE);
+ mpc83xx_spi_write_reg(mode, regval);
local_irq_restore(flags);
}
if (mpc83xx_spi->activate_cs)
@@ -284,7 +286,7 @@ int mpc83xx_spi_setup_transfer(struct spi_device *spi, struct spi_transfer *t)
regval = mpc83xx_spi_read_reg(&mpc83xx_spi->base->mode);
if (cs->hw_mode != regval) {
unsigned long flags;
- void *tmp_ptr = &mpc83xx_spi->base->mode;
+ __be32 __iomem *mode = &mpc83xx_spi->base->mode;

regval = cs->hw_mode;
/* Turn off IRQs locally to minimize time
@@ -292,8 +294,8 @@ int mpc83xx_spi_setup_transfer(struct spi_device *spi, struct spi_transfer *t)
*/
local_irq_save(flags);
/* Turn off SPI unit prior changing mode */
- mpc83xx_spi_write_reg(tmp_ptr, regval & ~SPMODE_ENABLE);
- mpc83xx_spi_write_reg(tmp_ptr, regval);
+ mpc83xx_spi_write_reg(mode, regval & ~SPMODE_ENABLE);
+ mpc83xx_spi_write_reg(mode, regval);
local_irq_restore(flags);
}
return 0;
@@ -483,7 +485,7 @@ static int mpc83xx_spi_setup(struct spi_device *spi)
return 0;
}

-irqreturn_t mpc83xx_spi_irq(s32 irq, void *context_data)
+static irqreturn_t mpc83xx_spi_irq(s32 irq, void *context_data)
{
struct mpc83xx_spi *mpc83xx_spi = context_data;
u32 event;
--
1.5.6.5

2009-01-23 19:51:31

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 2/6] spi_mpc83xx: Rework chip selects handling

The main purpose of this patch is to pass 'struct spi_device' to the
chip select handling routines. This is needed so that we could implement
full-fledged OpenFirmware support for this driver.

While at it, also:
- Replace two {de,activate}_cs routines by single cs_contol().
- Don't duplicate platform data callbacks in mpc83xx_spi struct.

Signed-off-by: Anton Vorontsov <[email protected]>
---
arch/powerpc/platforms/83xx/mpc832x_rdb.c | 16 ++++------------
arch/powerpc/sysdev/fsl_soc.c | 14 ++++++--------
arch/powerpc/sysdev/fsl_soc.h | 5 +++--
drivers/spi/spi_mpc83xx.c | 20 +++++++-------------
include/linux/fsl_devices.h | 5 +++--
5 files changed, 23 insertions(+), 37 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/mpc832x_rdb.c b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
index 2a1295f..28e23cd 100644
--- a/arch/powerpc/platforms/83xx/mpc832x_rdb.c
+++ b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
@@ -39,16 +39,10 @@
#endif

#ifdef CONFIG_QUICC_ENGINE
-static void mpc83xx_spi_activate_cs(u8 cs, u8 polarity)
+static void mpc83xx_spi_cs_control(struct spi_device *spi, bool on)
{
- pr_debug("%s %d %d\n", __func__, cs, polarity);
- par_io_data_set(3, 13, polarity);
-}
-
-static void mpc83xx_spi_deactivate_cs(u8 cs, u8 polarity)
-{
- pr_debug("%s %d %d\n", __func__, cs, polarity);
- par_io_data_set(3, 13, !polarity);
+ pr_debug("%s %d %d\n", __func__, spi->chip_select, on);
+ par_io_data_set(3, 13, on);
}

static struct mmc_spi_platform_data mpc832x_mmc_pdata = {
@@ -74,9 +68,7 @@ static int __init mpc832x_spi_init(void)
par_io_config_pin(3, 14, 2, 0, 0, 0); /* SD_INSERT, I */
par_io_config_pin(3, 15, 2, 0, 0, 0); /* SD_PROTECT,I */

- return fsl_spi_init(&mpc832x_spi_boardinfo, 1,
- mpc83xx_spi_activate_cs,
- mpc83xx_spi_deactivate_cs);
+ return fsl_spi_init(&mpc832x_spi_boardinfo, 1, mpc83xx_spi_cs_control);
}
machine_device_initcall(mpc832x_rdb, mpc832x_spi_init);
#endif /* CONFIG_QUICC_ENGINE */
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 115cb16..0a545a9 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -416,8 +416,8 @@ arch_initcall(fsl_usb_of_init);
static int __init of_fsl_spi_probe(char *type, char *compatible, u32 sysclk,
struct spi_board_info *board_infos,
unsigned int num_board_infos,
- void (*activate_cs)(u8 cs, u8 polarity),
- void (*deactivate_cs)(u8 cs, u8 polarity))
+ void (*cs_control)(struct spi_device *dev,
+ bool on))
{
struct device_node *np;
unsigned int i = 0;
@@ -429,8 +429,7 @@ static int __init of_fsl_spi_probe(char *type, char *compatible, u32 sysclk,
struct resource res[2];
struct platform_device *pdev;
struct fsl_spi_platform_data pdata = {
- .activate_cs = activate_cs,
- .deactivate_cs = deactivate_cs,
+ .cs_control = cs_control,
};

memset(res, 0, sizeof(res));
@@ -497,8 +496,7 @@ next:

int __init fsl_spi_init(struct spi_board_info *board_infos,
unsigned int num_board_infos,
- void (*activate_cs)(u8 cs, u8 polarity),
- void (*deactivate_cs)(u8 cs, u8 polarity))
+ void (*cs_control)(struct spi_device *spi, bool on))
{
u32 sysclk = -1;
int ret;
@@ -514,10 +512,10 @@ int __init fsl_spi_init(struct spi_board_info *board_infos,
}

ret = of_fsl_spi_probe(NULL, "fsl,spi", sysclk, board_infos,
- num_board_infos, activate_cs, deactivate_cs);
+ num_board_infos, cs_control);
if (!ret)
of_fsl_spi_probe("spi", "fsl_spi", sysclk, board_infos,
- num_board_infos, activate_cs, deactivate_cs);
+ num_board_infos, cs_control);

return spi_register_board_info(board_infos, num_board_infos);
}
diff --git a/arch/powerpc/sysdev/fsl_soc.h b/arch/powerpc/sysdev/fsl_soc.h
index 9c744e4..b5f3456 100644
--- a/arch/powerpc/sysdev/fsl_soc.h
+++ b/arch/powerpc/sysdev/fsl_soc.h
@@ -4,6 +4,8 @@

#include <asm/mmu.h>

+struct spi_device;
+
extern phys_addr_t get_immrbase(void);
#if defined(CONFIG_CPM2) || defined(CONFIG_QUICC_ENGINE) || defined(CONFIG_8xx)
extern u32 get_brgfreq(void);
@@ -19,8 +21,7 @@ struct device_node;

extern int fsl_spi_init(struct spi_board_info *board_infos,
unsigned int num_board_infos,
- void (*activate_cs)(u8 cs, u8 polarity),
- void (*deactivate_cs)(u8 cs, u8 polarity));
+ void (*cs_control)(struct spi_device *spi, bool on));

extern void fsl_rstcr_restart(char *cmd);

diff --git a/drivers/spi/spi_mpc83xx.c b/drivers/spi/spi_mpc83xx.c
index 6ef8937..056d200 100644
--- a/drivers/spi/spi_mpc83xx.c
+++ b/drivers/spi/spi_mpc83xx.c
@@ -89,9 +89,6 @@ struct mpc83xx_spi {

bool qe_mode;

- void (*activate_cs) (u8 cs, u8 polarity);
- void (*deactivate_cs) (u8 cs, u8 polarity);
-
u8 busy;

struct workqueue_struct *workqueue;
@@ -153,15 +150,14 @@ MPC83XX_SPI_TX_BUF(u32)

static void mpc83xx_spi_chipselect(struct spi_device *spi, int value)
{
- struct mpc83xx_spi *mpc83xx_spi;
- u8 pol = spi->mode & SPI_CS_HIGH ? 1 : 0;
+ struct mpc83xx_spi *mpc83xx_spi = spi_master_get_devdata(spi->master);
+ struct fsl_spi_platform_data *pdata = spi->dev.parent->platform_data;
+ bool pol = spi->mode & SPI_CS_HIGH;
struct spi_mpc83xx_cs *cs = spi->controller_state;

- mpc83xx_spi = spi_master_get_devdata(spi->master);
-
if (value == BITBANG_CS_INACTIVE) {
- if (mpc83xx_spi->deactivate_cs)
- mpc83xx_spi->deactivate_cs(spi->chip_select, pol);
+ if (pdata->cs_control)
+ pdata->cs_control(spi, !pol);
}

if (value == BITBANG_CS_ACTIVE) {
@@ -186,8 +182,8 @@ static void mpc83xx_spi_chipselect(struct spi_device *spi, int value)
mpc83xx_spi_write_reg(mode, regval);
local_irq_restore(flags);
}
- if (mpc83xx_spi->activate_cs)
- mpc83xx_spi->activate_cs(spi->chip_select, pol);
+ if (pdata->cs_control)
+ pdata->cs_control(spi, pol);
}
}

@@ -582,8 +578,6 @@ static int __init mpc83xx_spi_probe(struct platform_device *dev)
master->cleanup = mpc83xx_spi_cleanup;

mpc83xx_spi = spi_master_get_devdata(master);
- mpc83xx_spi->activate_cs = pdata->activate_cs;
- mpc83xx_spi->deactivate_cs = pdata->deactivate_cs;
mpc83xx_spi->qe_mode = pdata->qe_mode;
mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u8;
mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u8;
diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h
index d9051d7..7bc1b64 100644
--- a/include/linux/fsl_devices.h
+++ b/include/linux/fsl_devices.h
@@ -95,14 +95,15 @@ struct fsl_usb2_platform_data {
#define FSL_USB2_PORT0_ENABLED 0x00000001
#define FSL_USB2_PORT1_ENABLED 0x00000002

+struct spi_device;
+
struct fsl_spi_platform_data {
u32 initial_spmode; /* initial SPMODE value */
u16 bus_num;
bool qe_mode;
/* board specific information */
u16 max_chipselect;
- void (*activate_cs)(u8 cs, u8 polarity);
- void (*deactivate_cs)(u8 cs, u8 polarity);
+ void (*cs_control)(struct spi_device *spi, bool on);
u32 sysclk;
};

--
1.5.6.5

2009-01-23 19:51:50

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 3/6] spi_mpc83xx: Add OF platform driver bindings

This patch implements full support for OF SPI bindings. Now the driver
can manage its own chip selects without any help from the board files
and/or fsl_soc constructors.

The "legacy" code is well isolated and could be removed as time goes by.

Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/spi/spi_mpc83xx.c | 330 ++++++++++++++++++++++++++++++++++++++-----
include/linux/fsl_devices.h | 2 +-
2 files changed, 295 insertions(+), 37 deletions(-)

diff --git a/drivers/spi/spi_mpc83xx.c b/drivers/spi/spi_mpc83xx.c
index 056d200..09081bb 100644
--- a/drivers/spi/spi_mpc83xx.c
+++ b/drivers/spi/spi_mpc83xx.c
@@ -14,6 +14,8 @@
#include <linux/init.h>
#include <linux/types.h>
#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/err.h>
#include <linux/completion.h>
#include <linux/interrupt.h>
#include <linux/delay.h>
@@ -23,7 +25,13 @@
#include <linux/spi/spi_bitbang.h>
#include <linux/platform_device.h>
#include <linux/fsl_devices.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/of_spi.h>

+#include <sysdev/fsl_soc.h>
#include <asm/irq.h>
#include <asm/io.h>

@@ -79,7 +87,7 @@ struct mpc83xx_spi {
u32(*get_tx) (struct mpc83xx_spi *);

unsigned int count;
- int irq;
+ unsigned int irq;

unsigned nsecs; /* (clock cycle time)/2 */

@@ -543,36 +551,23 @@ static void mpc83xx_spi_cleanup(struct spi_device *spi)
kfree(spi->controller_state);
}

-static int __init mpc83xx_spi_probe(struct platform_device *dev)
+static struct spi_master * __devinit
+mpc83xx_spi_probe(struct device *dev, struct resource *mem, unsigned int irq)
{
+ struct fsl_spi_platform_data *pdata = dev->platform_data;
struct spi_master *master;
struct mpc83xx_spi *mpc83xx_spi;
- struct fsl_spi_platform_data *pdata;
- struct resource *r;
u32 regval;
int ret = 0;

- /* Get resources(memory, IRQ) associated with the device */
- master = spi_alloc_master(&dev->dev, sizeof(struct mpc83xx_spi));
-
+ master = spi_alloc_master(dev, sizeof(struct mpc83xx_spi));
if (master == NULL) {
ret = -ENOMEM;
goto err;
}

- platform_set_drvdata(dev, master);
- pdata = dev->dev.platform_data;
-
- if (pdata == NULL) {
- ret = -ENODEV;
- goto free_master;
- }
+ dev_set_drvdata(dev, master);

- r = platform_get_resource(dev, IORESOURCE_MEM, 0);
- if (r == NULL) {
- ret = -ENODEV;
- goto free_master;
- }
master->setup = mpc83xx_spi_setup;
master->transfer = mpc83xx_spi_transfer;
master->cleanup = mpc83xx_spi_cleanup;
@@ -592,18 +587,13 @@ static int __init mpc83xx_spi_probe(struct platform_device *dev)

init_completion(&mpc83xx_spi->done);

- mpc83xx_spi->base = ioremap(r->start, r->end - r->start + 1);
+ mpc83xx_spi->base = ioremap(mem->start, mem->end - mem->start + 1);
if (mpc83xx_spi->base == NULL) {
ret = -ENOMEM;
goto put_master;
}

- mpc83xx_spi->irq = platform_get_irq(dev, 0);
-
- if (mpc83xx_spi->irq < 0) {
- ret = -ENXIO;
- goto unmap_io;
- }
+ mpc83xx_spi->irq = irq;

/* Register for SPI Interrupt */
ret = request_irq(mpc83xx_spi->irq, mpc83xx_spi_irq,
@@ -645,9 +635,9 @@ static int __init mpc83xx_spi_probe(struct platform_device *dev)

printk(KERN_INFO
"%s: MPC83xx SPI Controller driver at 0x%p (irq = %d)\n",
- dev->dev.bus_id, mpc83xx_spi->base, mpc83xx_spi->irq);
+ dev_name(dev), mpc83xx_spi->base, mpc83xx_spi->irq);

- return ret;
+ return master;

unreg_master:
destroy_workqueue(mpc83xx_spi->workqueue);
@@ -657,18 +647,16 @@ unmap_io:
iounmap(mpc83xx_spi->base);
put_master:
spi_master_put(master);
-free_master:
- kfree(master);
err:
- return ret;
+ return ERR_PTR(ret);
}

-static int __exit mpc83xx_spi_remove(struct platform_device *dev)
+static int __devexit mpc83xx_spi_remove(struct device *dev)
{
struct mpc83xx_spi *mpc83xx_spi;
struct spi_master *master;

- master = platform_get_drvdata(dev);
+ master = dev_get_drvdata(dev);
mpc83xx_spi = spi_master_get_devdata(master);

flush_workqueue(mpc83xx_spi->workqueue);
@@ -681,23 +669,293 @@ static int __exit mpc83xx_spi_remove(struct platform_device *dev)
return 0;
}

+struct mpc83xx_spi_probe_info {
+ struct fsl_spi_platform_data pdata;
+ int *gpios;
+ bool *alow_flags;
+};
+
+static struct mpc83xx_spi_probe_info *
+to_of_pinfo(struct fsl_spi_platform_data *pdata)
+{
+ return container_of(pdata, struct mpc83xx_spi_probe_info, pdata);
+}
+
+static void mpc83xx_spi_cs_control(struct spi_device *spi, bool on)
+{
+ struct device *dev = spi->dev.parent;
+ struct mpc83xx_spi_probe_info *pinfo = to_of_pinfo(dev->platform_data);
+ u16 cs = spi->chip_select;
+ int gpio = pinfo->gpios[cs];
+ bool alow = pinfo->alow_flags[cs];
+
+ gpio_set_value(gpio, on ^ alow);
+}
+
+static int of_mpc83xx_spi_get_chipselects(struct device *dev)
+{
+ struct device_node *np = dev_archdata_get_node(&dev->archdata);
+ struct fsl_spi_platform_data *pdata = dev->platform_data;
+ struct mpc83xx_spi_probe_info *pinfo = to_of_pinfo(pdata);
+ unsigned int ngpios;
+ int i = 0;
+ int ret;
+
+ ngpios = of_gpio_count(np);
+ if (!ngpios) {
+ /*
+ * SPI w/o chip-select line. One SPI device is still permitted
+ * though.
+ */
+ pdata->max_chipselect = 1;
+ return 0;
+ }
+
+ pinfo->gpios = kmalloc(ngpios * sizeof(pinfo->gpios), GFP_KERNEL);
+ if (!pinfo->gpios)
+ return -ENOMEM;
+ memset(pinfo->gpios, -1, ngpios * sizeof(pinfo->gpios));
+
+ pinfo->alow_flags = kzalloc(ngpios * sizeof(pinfo->alow_flags),
+ GFP_KERNEL);
+ if (!pinfo->alow_flags) {
+ ret = -ENOMEM;
+ goto err_alloc_flags;
+ }
+
+ for (; i < ngpios; i++) {
+ int gpio;
+ enum of_gpio_flags flags;
+
+ gpio = of_get_gpio_flags(np, i, &flags);
+ if (!gpio_is_valid(gpio)) {
+ dev_err(dev, "invalid gpio #%d: %d\n", i, gpio);
+ goto err_loop;
+ }
+
+ ret = gpio_request(gpio, dev_name(dev));
+ if (ret) {
+ dev_err(dev, "can't request gpio #%d: %d\n", i, ret);
+ goto err_loop;
+ }
+
+ pinfo->gpios[i] = gpio;
+ pinfo->alow_flags[i] = flags & OF_GPIO_ACTIVE_LOW;
+
+ ret = gpio_direction_output(pinfo->gpios[i],
+ pinfo->alow_flags[i]);
+ if (ret) {
+ dev_err(dev, "can't set output direction for gpio "
+ "#%d: %d\n", i, ret);
+ goto err_loop;
+ }
+ }
+
+ pdata->max_chipselect = ngpios;
+ pdata->cs_control = mpc83xx_spi_cs_control;
+
+ return 0;
+
+err_loop:
+ while (i >= 0) {
+ if (gpio_is_valid(pinfo->gpios[i]))
+ gpio_free(pinfo->gpios[i]);
+ i--;
+ }
+
+ kfree(pinfo->alow_flags);
+ pinfo->alow_flags = NULL;
+err_alloc_flags:
+ kfree(pinfo->gpios);
+ pinfo->gpios = NULL;
+ return ret;
+}
+
+static int of_mpc83xx_spi_free_chipselects(struct device *dev)
+{
+ struct fsl_spi_platform_data *pdata = dev->platform_data;
+ struct mpc83xx_spi_probe_info *pinfo = to_of_pinfo(pdata);
+ int i;
+
+ if (!pinfo->gpios)
+ return 0;
+
+ for (i = 0; i < pdata->max_chipselect; i++) {
+ if (gpio_is_valid(pinfo->gpios[i]))
+ gpio_free(pinfo->gpios[i]);
+ }
+
+ kfree(pinfo->gpios);
+ kfree(pinfo->alow_flags);
+ return 0;
+}
+
+static int __devinit of_mpc83xx_spi_probe(struct of_device *ofdev,
+ const struct of_device_id *ofid)
+{
+ struct device *dev = &ofdev->dev;
+ struct device_node *np = ofdev->node;
+ struct mpc83xx_spi_probe_info *pinfo;
+ struct fsl_spi_platform_data *pdata;
+ struct spi_master *master;
+ struct resource mem;
+ struct resource irq;
+ const void *prop;
+ int ret = -ENOMEM;
+
+ pinfo = kzalloc(sizeof(*pinfo), GFP_KERNEL);
+ if (!pinfo)
+ return -ENOMEM;
+
+ pdata = &pinfo->pdata;
+ dev->platform_data = pdata;
+
+ /* Allocate bus num dynamically. */
+ pdata->bus_num = -1;
+
+ /* SPI controller is either clocked from QE or SoC clock. */
+ pdata->sysclk = get_brgfreq();
+ if (pdata->sysclk == -1) {
+ pdata->sysclk = fsl_get_sys_freq();
+ if (pdata->sysclk == -1) {
+ ret = -ENODEV;
+ goto err_clk;
+ }
+ }
+
+ prop = of_get_property(np, "mode", NULL);
+ if (prop && !strcmp(prop, "cpu-qe"))
+ pdata->qe_mode = 1;
+
+ ret = of_mpc83xx_spi_get_chipselects(dev);
+ if (ret)
+ goto err;
+
+ ret = of_address_to_resource(np, 0, &mem);
+ if (ret)
+ goto err;
+
+ ret = of_irq_to_resource(np, 0, &irq);
+ if (!ret) {
+ ret = -EINVAL;
+ goto err;
+ }
+
+ master = mpc83xx_spi_probe(dev, &mem, irq.start);
+ if (IS_ERR(master)) {
+ ret = PTR_ERR(master);
+ goto err;
+ }
+
+ of_register_spi_devices(master, np);
+
+ return 0;
+
+err:
+ of_mpc83xx_spi_free_chipselects(dev);
+err_clk:
+ kfree(pinfo);
+ return ret;
+}
+
+static int __devexit of_mpc83xx_spi_remove(struct of_device *ofdev)
+{
+ int ret;
+
+ ret = mpc83xx_spi_remove(&ofdev->dev);
+ if (ret)
+ return ret;
+ of_mpc83xx_spi_free_chipselects(&ofdev->dev);
+ return 0;
+}
+
+static const struct of_device_id of_mpc83xx_spi_match[] = {
+ { .compatible = "fsl,spi" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, of_mpc83xx_spi_match);
+
+static struct of_platform_driver of_mpc83xx_spi_driver = {
+ .name = "mpc83xx_spi",
+ .match_table = of_mpc83xx_spi_match,
+ .probe = of_mpc83xx_spi_probe,
+ .remove = __devexit_p(of_mpc83xx_spi_remove),
+};
+
+#ifdef CONFIG_MPC832x_RDB
+/*
+ * XXX XXX XXX
+ * This is "legacy" platform driver, was used by the MPC8323E-RDB boards
+ * only. The driver should go away soon, since newer MPC8323E-RDB's device
+ * tree can work with OpenFirmware driver. But for now we support old trees
+ * as well.
+ */
+static int __devinit plat_mpc83xx_spi_probe(struct platform_device *pdev)
+{
+ struct resource *mem;
+ unsigned int irq;
+ struct spi_master *master;
+
+ if (!pdev->dev.platform_data)
+ return -EINVAL;
+
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!mem)
+ return -EINVAL;
+
+ irq = platform_get_irq(pdev, 0);
+ if (!irq)
+ return -EINVAL;
+
+ master = mpc83xx_spi_probe(&pdev->dev, mem, irq);
+ if (IS_ERR(master))
+ return PTR_ERR(master);
+ return 0;
+}
+
+static int __devexit plat_mpc83xx_spi_remove(struct platform_device *pdev)
+{
+ return mpc83xx_spi_remove(&pdev->dev);
+}
+
MODULE_ALIAS("platform:mpc83xx_spi");
static struct platform_driver mpc83xx_spi_driver = {
- .remove = __exit_p(mpc83xx_spi_remove),
+ .probe = plat_mpc83xx_spi_probe,
+ .remove = __exit_p(plat_mpc83xx_spi_remove),
.driver = {
.name = "mpc83xx_spi",
.owner = THIS_MODULE,
},
};

+static bool legacy_driver_failed;
+
+static void __init legacy_driver_register(void)
+{
+ legacy_driver_failed = platform_driver_register(&mpc83xx_spi_driver);
+}
+
+static void __exit legacy_driver_unregister(void)
+{
+ if (legacy_driver_failed)
+ return;
+ platform_driver_unregister(&mpc83xx_spi_driver);
+}
+#else
+static void __init legacy_driver_register(void) {}
+static void __exit legacy_driver_unregister(void) {}
+#endif /* CONFIG_MPC832x_RDB */
+
static int __init mpc83xx_spi_init(void)
{
- return platform_driver_probe(&mpc83xx_spi_driver, mpc83xx_spi_probe);
+ legacy_driver_register();
+ return of_register_platform_driver(&of_mpc83xx_spi_driver);
}

static void __exit mpc83xx_spi_exit(void)
{
- platform_driver_unregister(&mpc83xx_spi_driver);
+ of_unregister_platform_driver(&of_mpc83xx_spi_driver);
+ legacy_driver_unregister();
}

module_init(mpc83xx_spi_init);
diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h
index 7bc1b64..7ef1caf 100644
--- a/include/linux/fsl_devices.h
+++ b/include/linux/fsl_devices.h
@@ -99,7 +99,7 @@ struct spi_device;

struct fsl_spi_platform_data {
u32 initial_spmode; /* initial SPMODE value */
- u16 bus_num;
+ s16 bus_num;
bool qe_mode;
/* board specific information */
u16 max_chipselect;
--
1.5.6.5

2009-01-23 19:52:16

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 4/6] powerpc: Add mmc-spi-slot bindings

The bindings describes a case where MMC/SD/SDIO slot directly connected
to a SPI bus. Such setups are widely used on embedded PowerPC boards.

The patch also adds the mmc-spi-slot entry to the OpenFirmware modalias
table.

Signed-off-by: Anton Vorontsov <[email protected]>
---
.../powerpc/dts-bindings/mmc-spi-slot.txt | 23 ++++++++++++++++++++
drivers/of/base.c | 1 +
2 files changed, 24 insertions(+), 0 deletions(-)
create mode 100644 Documentation/powerpc/dts-bindings/mmc-spi-slot.txt

diff --git a/Documentation/powerpc/dts-bindings/mmc-spi-slot.txt b/Documentation/powerpc/dts-bindings/mmc-spi-slot.txt
new file mode 100644
index 0000000..c39ac28
--- /dev/null
+++ b/Documentation/powerpc/dts-bindings/mmc-spi-slot.txt
@@ -0,0 +1,23 @@
+MMC/SD/SDIO slot directly connected to a SPI bus
+
+Required properties:
+- compatible : should be "mmc-spi-slot".
+- reg : should specify SPI address (chip-select number).
+- spi-max-frequency : maximum frequency for this device (Hz).
+- voltage-ranges : two cells are required, first cell specifies minimum
+ slot voltage (mV), second cell specifies maximum slot voltage (mV).
+ Several ranges could be specified.
+- gpios : (optional) may specify GPIOs in this order: Card-Detect GPIO,
+ Write-Protect GPIO.
+
+Example:
+
+ mmc-slot@0 {
+ compatible = "fsl,mpc8323rdb-mmc-slot",
+ "mmc-spi-slot";
+ reg = <0>;
+ gpios = <&qe_pio_d 14 1
+ &qe_pio_d 15 0>;
+ voltage-ranges = <3300 3300>;
+ spi-max-frequency = <50000000>;
+ };
diff --git a/drivers/of/base.c b/drivers/of/base.c
index cd17092..41c5dfd 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -446,6 +446,7 @@ struct of_modalias_table {
};
static struct of_modalias_table of_modalias_table[] = {
{ "fsl,mcu-mpc8349emitx", "mcu-mpc8349emitx" },
+ { "mmc-spi-slot", "mmc_spi" },
};

/**
--
1.5.6.5

2009-01-23 19:52:34

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 5/6] powerpc/83xx: Add mmc-spi support via the device tree for MPC8323E-RDB

- Add gpio-controller node to manage QE GPIO Bank D;
- Add mmc-spi node;
- Modify board file so that it won't use legacy SPI support with the new
device trees.

Signed-off-by: Anton Vorontsov <[email protected]>
---
arch/powerpc/boot/dts/mpc832x_rdb.dts | 24 ++++++++++++++++++++++++
arch/powerpc/platforms/83xx/mpc832x_rdb.c | 6 ++++++
2 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc832x_rdb.dts b/arch/powerpc/boot/dts/mpc832x_rdb.dts
index dea3091..4319bd7 100644
--- a/arch/powerpc/boot/dts/mpc832x_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc832x_rdb.dts
@@ -152,10 +152,21 @@
};

par_io@1400 {
+ #address-cells = <1>;
+ #size-cells = <1>;
reg = <0x1400 0x100>;
+ ranges = <3 0x1448 0x18>;
+ compatible = "fsl,mpc8323-qe-pario";
device_type = "par_io";
num-ports = <7>;

+ qe_pio_d: gpio-controller@1448 {
+ #gpio-cells = <2>;
+ compatible = "fsl,mpc8323-qe-pario-bank";
+ reg = <3 0x18>;
+ gpio-controller;
+ };
+
ucc2pio:ucc_pin@02 {
pio-map = <
/* port pin dir open_drain assignment has_irq */
@@ -225,12 +236,25 @@
};

spi@4c0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
cell-index = <0>;
compatible = "fsl,spi";
reg = <0x4c0 0x40>;
interrupts = <2>;
interrupt-parent = <&qeic>;
+ gpios = <&qe_pio_d 13 0>;
mode = "cpu-qe";
+
+ mmc-slot@0 {
+ compatible = "fsl,mpc8323rdb-mmc-slot",
+ "mmc-spi-slot";
+ reg = <0>;
+ gpios = <&qe_pio_d 14 1
+ &qe_pio_d 15 0>;
+ voltage-ranges = <3300 3300>;
+ spi-max-frequency = <50000000>;
+ };
};

spi@500 {
diff --git a/arch/powerpc/platforms/83xx/mpc832x_rdb.c b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
index 28e23cd..e1e7eeb 100644
--- a/arch/powerpc/platforms/83xx/mpc832x_rdb.c
+++ b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
@@ -68,6 +68,12 @@ static int __init mpc832x_spi_init(void)
par_io_config_pin(3, 14, 2, 0, 0, 0); /* SD_INSERT, I */
par_io_config_pin(3, 15, 2, 0, 0, 0); /* SD_PROTECT,I */

+ /*
+ * Don't bother with legacy stuff when device tree contains
+ * mmc-spi-slot node.
+ */
+ if (of_find_compatible_node(NULL, NULL, "mmc-spi-slot"))
+ return 0;
return fsl_spi_init(&mpc832x_spi_boardinfo, 1, mpc83xx_spi_cs_control);
}
machine_device_initcall(mpc832x_rdb, mpc832x_spi_init);
--
1.5.6.5

2009-01-23 19:52:52

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 6/6] powerpc/fsl_soc: Isolate legacy fsl_spi support to mpc832x_rdb boards

The advantages of this:
- Don't encourage legacy support;
- Less external symbols, less code to compile-in for !MPC832x_RDB
platforms.

Signed-off-by: Anton Vorontsov <[email protected]>
---
arch/powerpc/platforms/83xx/mpc832x_rdb.c | 107 +++++++++++++++++++++++++++++
arch/powerpc/sysdev/fsl_soc.c | 107 -----------------------------
arch/powerpc/sysdev/fsl_soc.h | 4 -
3 files changed, 107 insertions(+), 111 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/mpc832x_rdb.c b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
index e1e7eeb..567ded7 100644
--- a/arch/powerpc/platforms/83xx/mpc832x_rdb.c
+++ b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
@@ -20,6 +20,7 @@
#include <linux/spi/mmc_spi.h>
#include <linux/mmc/host.h>
#include <linux/of_platform.h>
+#include <linux/fsl_devices.h>

#include <asm/time.h>
#include <asm/ipic.h>
@@ -39,6 +40,112 @@
#endif

#ifdef CONFIG_QUICC_ENGINE
+static int __init of_fsl_spi_probe(char *type, char *compatible, u32 sysclk,
+ struct spi_board_info *board_infos,
+ unsigned int num_board_infos,
+ void (*cs_control)(struct spi_device *dev,
+ bool on))
+{
+ struct device_node *np;
+ unsigned int i = 0;
+
+ for_each_compatible_node(np, type, compatible) {
+ int ret;
+ unsigned int j;
+ const void *prop;
+ struct resource res[2];
+ struct platform_device *pdev;
+ struct fsl_spi_platform_data pdata = {
+ .cs_control = cs_control,
+ };
+
+ memset(res, 0, sizeof(res));
+
+ pdata.sysclk = sysclk;
+
+ prop = of_get_property(np, "reg", NULL);
+ if (!prop)
+ goto err;
+ pdata.bus_num = *(u32 *)prop;
+
+ prop = of_get_property(np, "cell-index", NULL);
+ if (prop)
+ i = *(u32 *)prop;
+
+ prop = of_get_property(np, "mode", NULL);
+ if (prop && !strcmp(prop, "cpu-qe"))
+ pdata.qe_mode = 1;
+
+ for (j = 0; j < num_board_infos; j++) {
+ if (board_infos[j].bus_num == pdata.bus_num)
+ pdata.max_chipselect++;
+ }
+
+ if (!pdata.max_chipselect)
+ continue;
+
+ ret = of_address_to_resource(np, 0, &res[0]);
+ if (ret)
+ goto err;
+
+ ret = of_irq_to_resource(np, 0, &res[1]);
+ if (ret == NO_IRQ)
+ goto err;
+
+ pdev = platform_device_alloc("mpc83xx_spi", i);
+ if (!pdev)
+ goto err;
+
+ ret = platform_device_add_data(pdev, &pdata, sizeof(pdata));
+ if (ret)
+ goto unreg;
+
+ ret = platform_device_add_resources(pdev, res,
+ ARRAY_SIZE(res));
+ if (ret)
+ goto unreg;
+
+ ret = platform_device_add(pdev);
+ if (ret)
+ goto unreg;
+
+ goto next;
+unreg:
+ platform_device_del(pdev);
+err:
+ pr_err("%s: registration failed\n", np->full_name);
+next:
+ i++;
+ }
+
+ return i;
+}
+
+static int __init fsl_spi_init(struct spi_board_info *board_infos,
+ unsigned int num_board_infos,
+ void (*cs_control)(struct spi_device *spi,
+ bool on))
+{
+ u32 sysclk = -1;
+ int ret;
+
+ /* SPI controller is either clocked from QE or SoC clock */
+ sysclk = get_brgfreq();
+ if (sysclk == -1) {
+ sysclk = fsl_get_sys_freq();
+ if (sysclk == -1)
+ return -ENODEV;
+ }
+
+ ret = of_fsl_spi_probe(NULL, "fsl,spi", sysclk, board_infos,
+ num_board_infos, cs_control);
+ if (!ret)
+ of_fsl_spi_probe("spi", "fsl_spi", sysclk, board_infos,
+ num_board_infos, cs_control);
+
+ return spi_register_board_info(board_infos, num_board_infos);
+}
+
static void mpc83xx_spi_cs_control(struct spi_device *spi, bool on)
{
pr_debug("%s %d %d\n", __func__, spi->chip_select, on);
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 0a545a9..18e49ef 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -413,113 +413,6 @@ err:

arch_initcall(fsl_usb_of_init);

-static int __init of_fsl_spi_probe(char *type, char *compatible, u32 sysclk,
- struct spi_board_info *board_infos,
- unsigned int num_board_infos,
- void (*cs_control)(struct spi_device *dev,
- bool on))
-{
- struct device_node *np;
- unsigned int i = 0;
-
- for_each_compatible_node(np, type, compatible) {
- int ret;
- unsigned int j;
- const void *prop;
- struct resource res[2];
- struct platform_device *pdev;
- struct fsl_spi_platform_data pdata = {
- .cs_control = cs_control,
- };
-
- memset(res, 0, sizeof(res));
-
- pdata.sysclk = sysclk;
-
- prop = of_get_property(np, "reg", NULL);
- if (!prop)
- goto err;
- pdata.bus_num = *(u32 *)prop;
-
- prop = of_get_property(np, "cell-index", NULL);
- if (prop)
- i = *(u32 *)prop;
-
- prop = of_get_property(np, "mode", NULL);
- if (prop && !strcmp(prop, "cpu-qe"))
- pdata.qe_mode = 1;
-
- for (j = 0; j < num_board_infos; j++) {
- if (board_infos[j].bus_num == pdata.bus_num)
- pdata.max_chipselect++;
- }
-
- if (!pdata.max_chipselect)
- continue;
-
- ret = of_address_to_resource(np, 0, &res[0]);
- if (ret)
- goto err;
-
- ret = of_irq_to_resource(np, 0, &res[1]);
- if (ret == NO_IRQ)
- goto err;
-
- pdev = platform_device_alloc("mpc83xx_spi", i);
- if (!pdev)
- goto err;
-
- ret = platform_device_add_data(pdev, &pdata, sizeof(pdata));
- if (ret)
- goto unreg;
-
- ret = platform_device_add_resources(pdev, res,
- ARRAY_SIZE(res));
- if (ret)
- goto unreg;
-
- ret = platform_device_add(pdev);
- if (ret)
- goto unreg;
-
- goto next;
-unreg:
- platform_device_del(pdev);
-err:
- pr_err("%s: registration failed\n", np->full_name);
-next:
- i++;
- }
-
- return i;
-}
-
-int __init fsl_spi_init(struct spi_board_info *board_infos,
- unsigned int num_board_infos,
- void (*cs_control)(struct spi_device *spi, bool on))
-{
- u32 sysclk = -1;
- int ret;
-
-#ifdef CONFIG_QUICC_ENGINE
- /* SPI controller is either clocked from QE or SoC clock */
- sysclk = get_brgfreq();
-#endif
- if (sysclk == -1) {
- sysclk = fsl_get_sys_freq();
- if (sysclk == -1)
- return -ENODEV;
- }
-
- ret = of_fsl_spi_probe(NULL, "fsl,spi", sysclk, board_infos,
- num_board_infos, cs_control);
- if (!ret)
- of_fsl_spi_probe("spi", "fsl_spi", sysclk, board_infos,
- num_board_infos, cs_control);
-
- return spi_register_board_info(board_infos, num_board_infos);
-}
-
#if defined(CONFIG_PPC_85xx) || defined(CONFIG_PPC_86xx)
static __be32 __iomem *rstcr;

diff --git a/arch/powerpc/sysdev/fsl_soc.h b/arch/powerpc/sysdev/fsl_soc.h
index b5f3456..42381bb 100644
--- a/arch/powerpc/sysdev/fsl_soc.h
+++ b/arch/powerpc/sysdev/fsl_soc.h
@@ -19,10 +19,6 @@ extern u32 fsl_get_sys_freq(void);
struct spi_board_info;
struct device_node;

-extern int fsl_spi_init(struct spi_board_info *board_infos,
- unsigned int num_board_infos,
- void (*cs_control)(struct spi_device *spi, bool on));
-
extern void fsl_rstcr_restart(char *cmd);

#if defined(CONFIG_FB_FSL_DIU) || defined(CONFIG_FB_FSL_DIU_MODULE)
--
1.5.6.5

2009-03-09 16:54:54

by Kumar Gala

[permalink] [raw]
Subject: Re: [PATCH resend 0/6] OpenFirmware support for the spi_mpc83xx driver


On Jan 23, 2009, at 1:49 PM, Anton Vorontsov wrote:

> On Tue, Jan 06, 2009 at 10:28:10PM -0600, Kumar Gala wrote:
>> On Dec 5, 2008, at 2:09 PM, Anton Vorontsov wrote:
>>> Hi all,
>>>
>>> The patch series are used to support SPI via the OF SPI subsystem
>>> (driver/of/of_spi.c). Now the driver is able to manage its own
>>> chip selects, and doesn't need any auxiliary support from the
>>> board files or fsl_soc constructors.
>>>
>>> The series also contains PowerPC portions of the MMC SPI support.
>>>
>>> Since the series touches spi and powerpc trees, I think it would
>>> be most convenient to pass it via one of these trees. The patches
>>> doesn't touch any SPI functionality, only some probe routines, so
>>> I believe powerpc.git is the best candidate...
>>>
>>> The other reason for powerpc tree is that the patches depends on
>>> other patches as found in paulus/powerpc.git + of_gpio_count()
>>> as found here:
>>> http://ozlabs.org/pipermail/linuxppc-dev/2008-December/065917.html
>>>
>>> David, if you're OK with the patches, may I ask you to sign off on
>>> the ones that touch drivers/spi so that we could pass it via Kumar's
>>> powerpc.git?
>>>
>>> The queue:
>>>
>>> [1/7] powerpc: Implement get_brgfreq() and get_baudrate() stubs
>>> [2/7] spi_mpc83xx: Fix sparse warnings
>>> [3/7] spi_mpc83xx: Rework chip selects handling
>>> [4/7] spi_mpc83xx: Add OF platform driver bindings
>>> [5/7] powerpc/fsl_soc: Isolate legacy fsl_spi support to mpc832x_rdb
>>> boards
>>> [6/7] powerpc: Add mmc-spi-slot bindings
>>> [7/7] powerpc/83xx: Add mmc-spi support via the device tree for
>>> MPC8323E-RDB
>>>
>>
>> David,
>>
>> Any status on these patches?
>
> Ping? No single comment on this patch set for ~2 months...
>
> Resending...
>
> Thanks,

David,

Anything going on here? Should we send this via some other channel?

- k

2009-04-08 09:19:04

by Peter Korsgaard

[permalink] [raw]
Subject: Re: [PATCH 6/6] powerpc/fsl_soc: Isolate legacy fsl_spi support to mpc832x_rdb boards

>>>>> "Anton" == Anton Vorontsov <[email protected]> writes:

Hi,

Anton> The advantages of this:
Anton> - Don't encourage legacy support;
Anton> - Less external symbols, less code to compile-in for !MPC832x_RDB
Anton> platforms.

It's nice with your cleanups, but I wonder how to handle more
complicated chip select handling than simply toggling a single gpio.

I have a board (or 2 actually, but they are similar in this regard)
with a mpc8347 using SPI to a number of addon boards. For signal
integrity reasons the SPI signals are routed to a MUX, so the chip
select logic has to set the MUX in addition to controlling the CS line
of the device.

I've been using code like this since late 2007, but this patch
ofcourse breaks it:

static void thinx_spi_activate_cs(u8 cs, u8 polarity)
{
static u8 old_cs = 255;

if (cs != old_cs) {
/* mux setup (cs 2:1)*/
gpio_set_value(gpio1 + GPIO_SPI_MUX_NOE, 1);
gpio_set_value(gpio1 + GPIO_SPI_MUX_SEL0, cs&2);
gpio_set_value(gpio1 + GPIO_SPI_MUX_SEL1, cs&4);
gpio_set_value(gpio1 + GPIO_SPI_MUX_NOE, 0);
old_cs = cs;
}

switch (cs) {
case 0: gpio_set_value(gpio1 + GPIO_SPI_CS_BKL1, polarity); break;
case 1: gpio_set_value(gpio1 + GPIO_SPI_CS_BKL2, polarity); break;
case 2: gpio_set_value(gpio1 + GPIO_SPI_CS_OPT1, polarity); break;
case 3: gpio_set_value(gpio1 + GPIO_SPI_CS_OPT2, polarity); break;
}
}

static void thinx_spi_deactivate_cs(u8 cs, u8 polarity)
{
switch (cs) {
case 0: gpio_set_value(gpio1 + GPIO_SPI_CS_BKL1, !polarity); break;
case 1: gpio_set_value(gpio1 + GPIO_SPI_CS_BKL2, !polarity); break;
case 2: gpio_set_value(gpio1 + GPIO_SPI_CS_OPT1, !polarity); break;
case 3: gpio_set_value(gpio1 + GPIO_SPI_CS_OPT2, !polarity); break;
}
}

static __init int thinx_spi_init(void)
{
struct device_node *np;
struct of_gpio_chip *gc;
static const int gpios[] = {
GPIO_SPI_CS_BKL1,
GPIO_SPI_CS_BKL2,
GPIO_SPI_CS_OPT1,
GPIO_SPI_CS_OPT2,
GPIO_SPI_MUX_NOE,
GPIO_SPI_MUX_SEL0,
GPIO_SPI_MUX_SEL1
};
int i;

np = of_find_node_by_name(NULL, "gpio-controller");
if (!np || !np->data) {
printk(KERN_ERR
"gpio1 node not found or controller not registerred\n");
return -ENODEV;
}
gc = np->data;
gpio1 = gc->gc.base;

for (i=0; i<ARRAY_SIZE(gpios); i++) {
gpio_request(gpio1 + gpios[i], "spi");
gpio_direction_output(gpio1 + gpios[i], 1);
}

fsl_spi_init(thinx_spi_boardinfo, ARRAY_SIZE(thinx_spi_boardinfo),
thinx_spi_activate_cs, thinx_spi_deactivate_cs);

return 0;
}

Now, I don't quite see how to handle this with the new OF bindings -
Any ideas?

--
Bye, Peter Korsgaard

2009-04-17 05:14:16

by Peter Korsgaard

[permalink] [raw]
Subject: Re: [PATCH 6/6] powerpc/fsl_soc: Isolate legacy fsl_spi support to mpc832x_rdb boards

>>>>> "Peter" == Peter Korsgaard <[email protected]> writes:

Anyone? I've locally reverted the commit, but most likely I'm not the
only one using the spi_mpc83xx driver without direct gpio controlled
chip select handling.

Anton> The advantages of this:
Anton> - Don't encourage legacy support;
Anton> - Less external symbols, less code to compile-in for !MPC832x_RDB
Anton> platforms.

Peter> It's nice with your cleanups, but I wonder how to handle more
Peter> complicated chip select handling than simply toggling a single gpio.

Peter> I have a board (or 2 actually, but they are similar in this regard)
Peter> with a mpc8347 using SPI to a number of addon boards. For signal
Peter> integrity reasons the SPI signals are routed to a MUX, so the chip
Peter> select logic has to set the MUX in addition to controlling the CS line
Peter> of the device.

Peter> I've been using code like this since late 2007, but this patch
Peter> ofcourse breaks it:

Peter> static void thinx_spi_activate_cs(u8 cs, u8 polarity)
Peter> {
Peter> static u8 old_cs = 255;

Peter> if (cs != old_cs) {
Peter> /* mux setup (cs 2:1)*/
Peter> gpio_set_value(gpio1 + GPIO_SPI_MUX_NOE, 1);
Peter> gpio_set_value(gpio1 + GPIO_SPI_MUX_SEL0, cs&2);
Peter> gpio_set_value(gpio1 + GPIO_SPI_MUX_SEL1, cs&4);
Peter> gpio_set_value(gpio1 + GPIO_SPI_MUX_NOE, 0);
Peter> old_cs = cs;
Peter> }

Peter> switch (cs) {
Peter> case 0: gpio_set_value(gpio1 + GPIO_SPI_CS_BKL1, polarity); break;
Peter> case 1: gpio_set_value(gpio1 + GPIO_SPI_CS_BKL2, polarity); break;
Peter> case 2: gpio_set_value(gpio1 + GPIO_SPI_CS_OPT1, polarity); break;
Peter> case 3: gpio_set_value(gpio1 + GPIO_SPI_CS_OPT2, polarity); break;
Peter> }
Peter> }

Peter> static void thinx_spi_deactivate_cs(u8 cs, u8 polarity)
Peter> {
Peter> switch (cs) {
Peter> case 0: gpio_set_value(gpio1 + GPIO_SPI_CS_BKL1, !polarity); break;
Peter> case 1: gpio_set_value(gpio1 + GPIO_SPI_CS_BKL2, !polarity); break;
Peter> case 2: gpio_set_value(gpio1 + GPIO_SPI_CS_OPT1, !polarity); break;
Peter> case 3: gpio_set_value(gpio1 + GPIO_SPI_CS_OPT2, !polarity); break;
Peter> }
Peter> }

Peter> static __init int thinx_spi_init(void)
Peter> {
Peter> struct device_node *np;
Peter> struct of_gpio_chip *gc;
Peter> static const int gpios[] = {
Peter> GPIO_SPI_CS_BKL1,
Peter> GPIO_SPI_CS_BKL2,
Peter> GPIO_SPI_CS_OPT1,
Peter> GPIO_SPI_CS_OPT2,
Peter> GPIO_SPI_MUX_NOE,
Peter> GPIO_SPI_MUX_SEL0,
Peter> GPIO_SPI_MUX_SEL1
Peter> };
Peter> int i;

Peter> np = of_find_node_by_name(NULL, "gpio-controller");
Peter> if (!np || !np->data) {
Peter> printk(KERN_ERR
Peter> "gpio1 node not found or controller not registerred\n");
Peter> return -ENODEV;
Peter> }
Peter> gc = np->data;
Peter> gpio1 = gc->gc.base;

Peter> for (i=0; i<ARRAY_SIZE(gpios); i++) {
Peter> gpio_request(gpio1 + gpios[i], "spi");
Peter> gpio_direction_output(gpio1 + gpios[i], 1);
Peter> }

Peter> fsl_spi_init(thinx_spi_boardinfo, ARRAY_SIZE(thinx_spi_boardinfo),
Peter> thinx_spi_activate_cs, thinx_spi_deactivate_cs);

Peter> return 0;
Peter> }

Peter> Now, I don't quite see how to handle this with the new OF bindings -
Peter> Any ideas?

--
Bye, Peter Korsgaard

2009-04-17 12:24:18

by Anton Vorontsov

[permalink] [raw]
Subject: Re: [PATCH 6/6] powerpc/fsl_soc: Isolate legacy fsl_spi support to mpc832x_rdb boards

Hi Peter,

Sorry for the late response (and don't hesitate to ping me if I
don't answer, some things get lost in my inbox traffic, sorry).

On Wed, Apr 08, 2009 at 11:18:43AM +0200, Peter Korsgaard wrote:
> >>>>> "Anton" == Anton Vorontsov <[email protected]> writes:
>
> Hi,
>
> Anton> The advantages of this:
> Anton> - Don't encourage legacy support;
> Anton> - Less external symbols, less code to compile-in for !MPC832x_RDB
> Anton> platforms.
>
> It's nice with your cleanups, but I wonder how to handle more
> complicated chip select handling than simply toggling a single gpio.
>
> I have a board (or 2 actually, but they are similar in this regard)
> with a mpc8347 using SPI to a number of addon boards. For signal
> integrity reasons the SPI signals are routed to a MUX, so the chip
> select logic has to set the MUX in addition to controlling the CS line
> of the device.

So that's just a bit complicated GPIO controller. I believe you
should describe it in the device tree and use it's muxed lines as
usual GPIOs.

Something ling

muxed_pio: gpio-controller@.. {
#gpio-cells = <2>;
compatible = "..,<board>-muxed-gpios";
reg = <...>;
gpios = <...>; <- specify pure BLK1, BLK2, OPT1, OPT2 GPIOs
gpio-controller;
};

And then,

> I've been using code like this since late 2007, but this patch
> ofcourse breaks it:
>
> static void thinx_spi_activate_cs(u8 cs, u8 polarity)
> {
> static u8 old_cs = 255;
>
> if (cs != old_cs) {
> /* mux setup (cs 2:1)*/
> gpio_set_value(gpio1 + GPIO_SPI_MUX_NOE, 1);
> gpio_set_value(gpio1 + GPIO_SPI_MUX_SEL0, cs&2);
> gpio_set_value(gpio1 + GPIO_SPI_MUX_SEL1, cs&4);
> gpio_set_value(gpio1 + GPIO_SPI_MUX_NOE, 0);
> old_cs = cs;
> }
>
> switch (cs) {
> case 0: gpio_set_value(gpio1 + GPIO_SPI_CS_BKL1, polarity); break;
> case 1: gpio_set_value(gpio1 + GPIO_SPI_CS_BKL2, polarity); break;
> case 2: gpio_set_value(gpio1 + GPIO_SPI_CS_OPT1, polarity); break;
> case 3: gpio_set_value(gpio1 + GPIO_SPI_CS_OPT2, polarity); break;
> }
> }

^^^ Put this logic into the "<board>-muxed-gpios" GPIO controller
driver.

Then spi node would look like this:

spi-controller@.. {
...
gpios = <&muxed_pio cs1 0 /* muxed CS_BLK1 */
&muxed_pio cs2 0 /* muxed CS_BLK2 */
&muxed_pio cs3 0 /* muxed CS_OPT1 */
&muxed_pio cs4 0>; /* muxed CS_OPT2 */
};

Hope this helps,

--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2