* [PATCH 2/2] SPI: make the moved loopback test work @ 2010-07-11 0:43 Linus Walleij 2010-07-19 5:51 ` Grant Likely 0 siblings, 1 reply; 9+ messages in thread From: Linus Walleij @ 2010-07-11 0:43 UTC (permalink / raw) To: spi-devel-general, Grant Likely; +Cc: Linus Walleij, linux-arm-kernel This completes the move of the SPI test chip out of the U300 architecture by modifying the loopback test to register itself on every PL022 master if it is compiled in. Also updates some minor things like naming to make the code stricter. Signed-off-by: Linus Walleij <linus.walleij@stericsson.com> --- arch/arm/mach-u300/Kconfig | 12 ---- arch/arm/mach-u300/Makefile | 1 - arch/arm/mach-u300/spi.c | 61 ------------------- drivers/spi/Kconfig | 13 ++++ drivers/spi/Makefile | 1 + drivers/spi/amba-pl022-looptest.c | 120 +++++++++++++++++++++++++++++------- drivers/spi/amba-pl022-looptest.h | 11 ++++ drivers/spi/amba-pl022.c | 6 ++ 8 files changed, 127 insertions(+), 98 deletions(-) create mode 100644 drivers/spi/amba-pl022-looptest.h diff --git a/arch/arm/mach-u300/Kconfig b/arch/arm/mach-u300/Kconfig index 801b21e..337b9aa 100644 --- a/arch/arm/mach-u300/Kconfig +++ b/arch/arm/mach-u300/Kconfig @@ -81,18 +81,6 @@ config MACH_U300_SEMI_IS_SHARED Memory Interface) from both from access and application side. -config MACH_U300_SPIDUMMY - bool "SSP/SPI dummy chip" - select SPI - select SPI_MASTER - select SPI_PL022 - help - This creates a small kernel module that creates a dummy - SPI device to be used for loopback tests. Regularly used - to test reference designs. If you're not testing SPI, - you don't need it. Selecting this will activate the - SPI framework and ARM PL022 support. - comment "All the settings below must match the bootloader's settings" config MACH_U300_ACCESS_MEM_SIZE diff --git a/arch/arm/mach-u300/Makefile b/arch/arm/mach-u300/Makefile index fab46fe..1f0f4b8 100644 --- a/arch/arm/mach-u300/Makefile +++ b/arch/arm/mach-u300/Makefile @@ -10,6 +10,5 @@ obj- := obj-$(CONFIG_ARCH_U300) += u300.o obj-$(CONFIG_MMC) += mmc.o obj-$(CONFIG_SPI_PL022) += spi.o -obj-$(CONFIG_MACH_U300_SPIDUMMY) += dummyspichip.o obj-$(CONFIG_I2C_STU300) += i2c.o obj-$(CONFIG_REGULATOR_AB3100) += regulator.o diff --git a/arch/arm/mach-u300/spi.c b/arch/arm/mach-u300/spi.c index f0e887b..0790cce 100644 --- a/arch/arm/mach-u300/spi.c +++ b/arch/arm/mach-u300/spi.c @@ -16,68 +16,8 @@ /* * The following is for the actual devices on the SSP/SPI bus */ -#ifdef CONFIG_MACH_U300_SPIDUMMY -static void select_dummy_chip(u32 chipselect) -{ - pr_debug("CORE: %s called with CS=0x%x (%s)\n", - __func__, - chipselect, - chipselect ? "unselect chip" : "select chip"); - /* - * Here you would write the chip select value to the GPIO pins if - * this was a real chip (but this is a loopback dummy). - */ -} - -struct pl022_config_chip dummy_chip_info = { - /* Nominally this is LOOPBACK_DISABLED, but this is our dummy chip! */ - .lbm = LOOPBACK_ENABLED, - /* - * available POLLING_TRANSFER and INTERRUPT_TRANSFER, - * DMA_TRANSFER does not work - */ - .com_mode = INTERRUPT_TRANSFER, - .iface = SSP_INTERFACE_MOTOROLA_SPI, - /* We can only act as master but SSP_SLAVE is possible in theory */ - .hierarchy = SSP_MASTER, - /* 0 = drive TX even as slave, 1 = do not drive TX as slave */ - .slave_tx_disable = 0, - /* LSB first */ - .endian_tx = SSP_TX_LSB, - .endian_rx = SSP_RX_LSB, - .data_size = SSP_DATA_BITS_8, /* used to be 12 in some default */ - .rx_lev_trig = SSP_RX_1_OR_MORE_ELEM, - .tx_lev_trig = SSP_TX_1_OR_MORE_EMPTY_LOC, - .clk_phase = SSP_CLK_SECOND_EDGE, - .clk_pol = SSP_CLK_POL_IDLE_LOW, - .ctrl_len = SSP_BITS_12, - .wait_state = SSP_MWIRE_WAIT_ZERO, - .duplex = SSP_MICROWIRE_CHANNEL_FULL_DUPLEX, - /* - * This is where you insert a call to a function to enable CS - * (usually GPIO) for a certain chip. - */ - .cs_control = select_dummy_chip, -}; -#endif static struct spi_board_info u300_spi_devices[] = { -#ifdef CONFIG_MACH_U300_SPIDUMMY - { - /* A dummy chip used for loopback tests */ - .modalias = "spi-dummy", - /* Really dummy, pass in additional chip config here */ - .platform_data = NULL, - /* This defines how the controller shall handle the device */ - .controller_data = &dummy_chip_info, - /* .irq - no external IRQ routed from this device */ - .max_speed_hz = 1000000, - .bus_num = 0, /* Only one bus on this chip */ - .chip_select = 0, - /* Means SPI_CS_HIGH, change if e.g low CS */ - .mode = 0, - }, -#endif }; static struct pl022_ssp_controller ssp_platform_data = { @@ -94,7 +34,6 @@ static struct pl022_ssp_controller ssp_platform_data = { .num_chipselect = 3, }; - void __init u300_spi_init(struct amba_device *adev) { struct pmx *pmx; diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 91c2f4f..9e584dc 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -227,6 +227,19 @@ config SPI_PL022 controller. If you have an embedded system with an AMBA(R) bus and a PL022 controller, say Y or M here. +config SPI_PL022_LOOPTEST + bool "PL022 loopback chip" + depends on SPI_PL022 + help + The PL022 supports a loopback mode where the output is + looped back to the input for testing. + + This creates a small kernel module that creates a dummy + SPI device on each PL022 SPI bus to be used for loopback + tests. You find the device in the /sys filesystem, e.g. + /sys/bus/spi/devices/spi0.0/looptest and the test is run by + issuing cat <looptest>. + config SPI_PPC4xx tristate "PPC4xx SPI Controller" depends on PPC32 && 4xx && SPI_MASTER diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index e9cbd18..9ac8e95 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -31,6 +31,7 @@ obj-$(CONFIG_SPI_OMAP24XX) += omap2_mcspi.o obj-$(CONFIG_SPI_OMAP_100K) += omap_spi_100k.o obj-$(CONFIG_SPI_ORION) += orion_spi.o obj-$(CONFIG_SPI_PL022) += amba-pl022.o +obj-$(CONFIG_SPI_PL022_LOOPTEST) += amba-pl022-looptest.o obj-$(CONFIG_SPI_MPC512x_PSC) += mpc512x_psc_spi.o obj-$(CONFIG_SPI_MPC52xx_PSC) += mpc52xx_psc_spi.o obj-$(CONFIG_SPI_MPC52xx) += mpc52xx_spi.o diff --git a/drivers/spi/amba-pl022-looptest.c b/drivers/spi/amba-pl022-looptest.c index 5f55012..561886a 100644 --- a/drivers/spi/amba-pl022-looptest.c +++ b/drivers/spi/amba-pl022-looptest.c @@ -1,7 +1,5 @@ /* - * arch/arm/mach-u300/dummyspichip.c - * - * Copyright (C) 2007-2009 ST-Ericsson AB + * Copyright (C) 2007-2010 ST-Ericsson SA * License terms: GNU General Public License (GPL) version 2 * This is a dummy loopback SPI "chip" used for testing SPI. * Author: Linus Walleij <linus.walleij@stericsson.com> @@ -25,19 +23,93 @@ */ #include <linux/amba/pl022.h> -struct dummy { +struct pl022_loopback { struct device *dev; struct mutex lock; }; #define DMA_TEST_SIZE 2048 -/* When we cat /sys/bus/spi/devices/spi0.0/looptest this will be triggered */ -static ssize_t dummy_looptest(struct device *dev, +/* + * The following code self-registers the PL022 loopback + * chip on each PL022 SPI bus. + */ + +static void pl022_select_loopback_chip(u32 chipselect) +{ + pr_debug("PL022 loopback chip: %s called with CS=0x%x (%s)\n", + __func__, + chipselect, + chipselect ? "unselect chip" : "select chip"); + /* + * Here you would write the chip select value to the GPIO pins if + * this was a real chip (but this is a loopback dummy). + */ +} + +struct pl022_config_chip pl022_loopback_chip_info = { + /* Nominally this is LOOPBACK_DISABLED, but this is our dummy chip! */ + .lbm = LOOPBACK_ENABLED, + .com_mode = INTERRUPT_TRANSFER, + .iface = SSP_INTERFACE_MOTOROLA_SPI, + .hierarchy = SSP_MASTER, + .slave_tx_disable = 0, + /* LSB first */ + .endian_tx = SSP_TX_LSB, + .endian_rx = SSP_RX_LSB, + .data_size = SSP_DATA_BITS_8, + .rx_lev_trig = SSP_RX_1_OR_MORE_ELEM, + .tx_lev_trig = SSP_TX_1_OR_MORE_EMPTY_LOC, + .clk_phase = SSP_CLK_SECOND_EDGE, + .clk_pol = SSP_CLK_POL_IDLE_LOW, + .ctrl_len = SSP_BITS_12, + .wait_state = SSP_MWIRE_WAIT_ZERO, + .duplex = SSP_MICROWIRE_CHANNEL_FULL_DUPLEX, + .cs_control = pl022_select_loopback_chip, +}; + +static struct spi_board_info pl022_loopback_device = { + /* A dummy chip used for loopback tests */ + .modalias = "pl022-loop", + /* Really dummy, pass in additional chip config here */ + .platform_data = NULL, + /* This defines how the controller shall handle the device */ + .controller_data = &pl022_loopback_chip_info, + /* .irq - no external IRQ routed from this device */ + .max_speed_hz = 1000000, + .bus_num = 0, /* Only one bus on this chip */ + .chip_select = 0, + /* Means SPI_CS_HIGH, change if e.g low CS */ + .mode = 0, +}; + +/* + * This is called from the PL022 driver to register a loopback + * on itself if this module is compiled in. + */ +void pl022_loopback_register(struct spi_master *master) +{ + struct spi_device *spidev; + + spidev = spi_new_device(master, &pl022_loopback_device); + if (!spidev) + dev_err(&master->dev, + "could not register loopback test chip\n"); + else + dev_err(&master->dev, + "registered loopback test chip\n"); +} + +/* + * This is where the tests begin. When we issue: + * cat /sys/bus/spi/devices/spi0.0/looptest + * this will be triggered. + */ +static ssize_t pl022_looptest(struct device *dev, struct device_attribute *attr, char *buf) { struct spi_device *spi = to_spi_device(dev); - struct dummy *p_dummy = dev_get_drvdata(&spi->dev); + struct pl022_loopback *p_loop = dev_get_drvdata(&spi->dev); /* * WARNING! Do not dereference the chip-specific data in any normal @@ -55,7 +127,7 @@ static ssize_t dummy_looptest(struct device *dev, u8 *bigtxbuf_virtual; u8 *bigrxbuf_virtual; - if (mutex_lock_interruptible(&p_dummy->lock)) + if (mutex_lock_interruptible(&p_loop->lock)) return -ERESTARTSYS; bigtxbuf_virtual = kmalloc(DMA_TEST_SIZE, GFP_KERNEL); @@ -217,25 +289,25 @@ static ssize_t dummy_looptest(struct device *dev, kfree(bigrxbuf_virtual); kfree(bigtxbuf_virtual); out: - mutex_unlock(&p_dummy->lock); + mutex_unlock(&p_loop->lock); return status; } -static DEVICE_ATTR(looptest, S_IRUGO, dummy_looptest, NULL); +static DEVICE_ATTR(looptest, S_IRUGO, pl022_looptest, NULL); -static int __devinit pl022_dummy_probe(struct spi_device *spi) +static int __devinit pl022_loopback_probe(struct spi_device *spi) { - struct dummy *p_dummy; + struct pl022_loopback *p_loop; int status; dev_info(&spi->dev, "probing dummy SPI device\n"); - p_dummy = kzalloc(sizeof *p_dummy, GFP_KERNEL); - if (!p_dummy) + p_loop = kzalloc(sizeof *p_loop, GFP_KERNEL); + if (!p_loop) return -ENOMEM; - dev_set_drvdata(&spi->dev, p_dummy); - mutex_init(&p_dummy->lock); + dev_set_drvdata(&spi->dev, p_loop); + mutex_init(&p_loop->lock); /* sysfs hook */ status = device_create_file(&spi->dev, &dev_attr_looptest); @@ -248,29 +320,29 @@ static int __devinit pl022_dummy_probe(struct spi_device *spi) out_dev_create_looptest_failed: dev_set_drvdata(&spi->dev, NULL); - kfree(p_dummy); + kfree(p_loop); return status; } -static int __devexit pl022_dummy_remove(struct spi_device *spi) +static int __devexit pl022_loopback_remove(struct spi_device *spi) { - struct dummy *p_dummy = dev_get_drvdata(&spi->dev); + struct pl022_loopback *p_loop = dev_get_drvdata(&spi->dev); dev_info(&spi->dev, "removing dummy SPI device\n"); device_remove_file(&spi->dev, &dev_attr_looptest); dev_set_drvdata(&spi->dev, NULL); - kfree(p_dummy); + kfree(p_loop); return 0; } static struct spi_driver pl022_dummy_driver = { .driver = { - .name = "spi-dummy", + .name = "pl022-loop", .owner = THIS_MODULE, }, - .probe = pl022_dummy_probe, - .remove = __devexit_p(pl022_dummy_remove), + .probe = pl022_loopback_probe, + .remove = __devexit_p(pl022_loopback_remove), }; static int __init pl022_init_dummy(void) @@ -287,5 +359,5 @@ module_init(pl022_init_dummy); module_exit(pl022_exit_dummy); MODULE_AUTHOR("Linus Walleij <linus.walleij@stericsson.com>"); -MODULE_DESCRIPTION("PL022 SSP/SPI DUMMY Linux driver"); +MODULE_DESCRIPTION("PL022 loopback test driver"); MODULE_LICENSE("GPL"); diff --git a/drivers/spi/amba-pl022-looptest.h b/drivers/spi/amba-pl022-looptest.h new file mode 100644 index 0000000..1bedcc6 --- /dev/null +++ b/drivers/spi/amba-pl022-looptest.h @@ -0,0 +1,11 @@ +/* + * Defines a single function for the master to create a loopback + * on itself. + */ +#ifdef CONFIG_SPI_PL022_LOOPTEST +void pl022_loopback_register(struct spi_master *master); +#else +static void inline pl022_loopback_register(struct spi_master *master) +{ +} +#endif diff --git a/drivers/spi/amba-pl022.c b/drivers/spi/amba-pl022.c index f0a1418..6e4a657 100644 --- a/drivers/spi/amba-pl022.c +++ b/drivers/spi/amba-pl022.c @@ -46,6 +46,8 @@ #include <linux/io.h> #include <linux/slab.h> +#include "amba-pl022-looptest.h" + /* * This macro is used to define some register default values. * reg is masked with mask, the OR:ed with an (again masked) @@ -1818,6 +1820,10 @@ pl022_probe(struct amba_device *adev, struct amba_id *id) goto err_spi_register; } dev_dbg(dev, "probe succeded\n"); + + /* Register a loopback test if desired */ + pl022_loopback_register(master); + return 0; err_spi_register: -- 1.7.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] SPI: make the moved loopback test work 2010-07-11 0:43 [PATCH 2/2] SPI: make the moved loopback test work Linus Walleij @ 2010-07-19 5:51 ` Grant Likely 2010-07-19 10:16 ` [spi-devel-general] " David Brownell 0 siblings, 1 reply; 9+ messages in thread From: Grant Likely @ 2010-07-19 5:51 UTC (permalink / raw) To: Linus Walleij; +Cc: spi-devel-general, linux-arm-kernel On Sat, Jul 10, 2010 at 6:43 PM, Linus Walleij <linus.walleij@stericsson.com> wrote: > This completes the move of the SPI test chip out of the U300 > architecture by modifying the loopback test to register itself on > every PL022 master if it is compiled in. Also updates some minor > things like naming to make the code stricter. > > Signed-off-by: Linus Walleij <linus.walleij@stericsson.com> Having a test driver in common code that purposefully does bad things is a sure fire way to get people to copy those bad practices when they write their own drivers (despite all the comments in the code that say don't do this). I'm talking about things like the dereferencing of controller data, and calling the setup() hook in something that looks like a normal spi_device driver. As-is, I'm not excited about merging this patch (and it isn't bisectable either). If you want this loopback functionality, It should be integrated into the pl022 driver itself. It should not pretend to be an spi_device. > --- > arch/arm/mach-u300/Kconfig | 12 ---- > arch/arm/mach-u300/Makefile | 1 - > arch/arm/mach-u300/spi.c | 61 ------------------- > drivers/spi/Kconfig | 13 ++++ > drivers/spi/Makefile | 1 + > drivers/spi/amba-pl022-looptest.c | 120 +++++++++++++++++++++++++++++------- > drivers/spi/amba-pl022-looptest.h | 11 ++++ > drivers/spi/amba-pl022.c | 6 ++ > 8 files changed, 127 insertions(+), 98 deletions(-) > create mode 100644 drivers/spi/amba-pl022-looptest.h > > diff --git a/arch/arm/mach-u300/Kconfig b/arch/arm/mach-u300/Kconfig > index 801b21e..337b9aa 100644 > --- a/arch/arm/mach-u300/Kconfig > +++ b/arch/arm/mach-u300/Kconfig > @@ -81,18 +81,6 @@ config MACH_U300_SEMI_IS_SHARED > Memory Interface) from both from access and application > side. > > -config MACH_U300_SPIDUMMY > - bool "SSP/SPI dummy chip" > - select SPI > - select SPI_MASTER > - select SPI_PL022 > - help > - This creates a small kernel module that creates a dummy > - SPI device to be used for loopback tests. Regularly used > - to test reference designs. If you're not testing SPI, > - you don't need it. Selecting this will activate the > - SPI framework and ARM PL022 support. > - > comment "All the settings below must match the bootloader's settings" > > config MACH_U300_ACCESS_MEM_SIZE > diff --git a/arch/arm/mach-u300/Makefile b/arch/arm/mach-u300/Makefile > index fab46fe..1f0f4b8 100644 > --- a/arch/arm/mach-u300/Makefile > +++ b/arch/arm/mach-u300/Makefile > @@ -10,6 +10,5 @@ obj- := > obj-$(CONFIG_ARCH_U300) += u300.o > obj-$(CONFIG_MMC) += mmc.o > obj-$(CONFIG_SPI_PL022) += spi.o > -obj-$(CONFIG_MACH_U300_SPIDUMMY) += dummyspichip.o > obj-$(CONFIG_I2C_STU300) += i2c.o > obj-$(CONFIG_REGULATOR_AB3100) += regulator.o > diff --git a/arch/arm/mach-u300/spi.c b/arch/arm/mach-u300/spi.c > index f0e887b..0790cce 100644 > --- a/arch/arm/mach-u300/spi.c > +++ b/arch/arm/mach-u300/spi.c > @@ -16,68 +16,8 @@ > /* > * The following is for the actual devices on the SSP/SPI bus > */ > -#ifdef CONFIG_MACH_U300_SPIDUMMY > -static void select_dummy_chip(u32 chipselect) > -{ > - pr_debug("CORE: %s called with CS=0x%x (%s)\n", > - __func__, > - chipselect, > - chipselect ? "unselect chip" : "select chip"); > - /* > - * Here you would write the chip select value to the GPIO pins if > - * this was a real chip (but this is a loopback dummy). > - */ > -} > - > -struct pl022_config_chip dummy_chip_info = { > - /* Nominally this is LOOPBACK_DISABLED, but this is our dummy chip! */ > - .lbm = LOOPBACK_ENABLED, > - /* > - * available POLLING_TRANSFER and INTERRUPT_TRANSFER, > - * DMA_TRANSFER does not work > - */ > - .com_mode = INTERRUPT_TRANSFER, > - .iface = SSP_INTERFACE_MOTOROLA_SPI, > - /* We can only act as master but SSP_SLAVE is possible in theory */ > - .hierarchy = SSP_MASTER, > - /* 0 = drive TX even as slave, 1 = do not drive TX as slave */ > - .slave_tx_disable = 0, > - /* LSB first */ > - .endian_tx = SSP_TX_LSB, > - .endian_rx = SSP_RX_LSB, > - .data_size = SSP_DATA_BITS_8, /* used to be 12 in some default */ > - .rx_lev_trig = SSP_RX_1_OR_MORE_ELEM, > - .tx_lev_trig = SSP_TX_1_OR_MORE_EMPTY_LOC, > - .clk_phase = SSP_CLK_SECOND_EDGE, > - .clk_pol = SSP_CLK_POL_IDLE_LOW, > - .ctrl_len = SSP_BITS_12, > - .wait_state = SSP_MWIRE_WAIT_ZERO, > - .duplex = SSP_MICROWIRE_CHANNEL_FULL_DUPLEX, > - /* > - * This is where you insert a call to a function to enable CS > - * (usually GPIO) for a certain chip. > - */ > - .cs_control = select_dummy_chip, > -}; > -#endif > > static struct spi_board_info u300_spi_devices[] = { > -#ifdef CONFIG_MACH_U300_SPIDUMMY > - { > - /* A dummy chip used for loopback tests */ > - .modalias = "spi-dummy", > - /* Really dummy, pass in additional chip config here */ > - .platform_data = NULL, > - /* This defines how the controller shall handle the device */ > - .controller_data = &dummy_chip_info, > - /* .irq - no external IRQ routed from this device */ > - .max_speed_hz = 1000000, > - .bus_num = 0, /* Only one bus on this chip */ > - .chip_select = 0, > - /* Means SPI_CS_HIGH, change if e.g low CS */ > - .mode = 0, > - }, > -#endif > }; > > static struct pl022_ssp_controller ssp_platform_data = { > @@ -94,7 +34,6 @@ static struct pl022_ssp_controller ssp_platform_data = { > .num_chipselect = 3, > }; > > - > void __init u300_spi_init(struct amba_device *adev) > { > struct pmx *pmx; > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig > index 91c2f4f..9e584dc 100644 > --- a/drivers/spi/Kconfig > +++ b/drivers/spi/Kconfig > @@ -227,6 +227,19 @@ config SPI_PL022 > controller. If you have an embedded system with an AMBA(R) > bus and a PL022 controller, say Y or M here. > > +config SPI_PL022_LOOPTEST > + bool "PL022 loopback chip" > + depends on SPI_PL022 > + help > + The PL022 supports a loopback mode where the output is > + looped back to the input for testing. > + > + This creates a small kernel module that creates a dummy > + SPI device on each PL022 SPI bus to be used for loopback > + tests. You find the device in the /sys filesystem, e.g. > + /sys/bus/spi/devices/spi0.0/looptest and the test is run by > + issuing cat <looptest>. Questionable usage of sysfs. sysfs files should contain single values only, or a list of things-of-the-same-type, and by convention sysfs files are supposed to be available immediately when the struct device is registered. It looks like debugfs should really be used instead. > + > config SPI_PPC4xx > tristate "PPC4xx SPI Controller" > depends on PPC32 && 4xx && SPI_MASTER > diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile > index e9cbd18..9ac8e95 100644 > --- a/drivers/spi/Makefile > +++ b/drivers/spi/Makefile > @@ -31,6 +31,7 @@ obj-$(CONFIG_SPI_OMAP24XX) += omap2_mcspi.o > obj-$(CONFIG_SPI_OMAP_100K) += omap_spi_100k.o > obj-$(CONFIG_SPI_ORION) += orion_spi.o > obj-$(CONFIG_SPI_PL022) += amba-pl022.o > +obj-$(CONFIG_SPI_PL022_LOOPTEST) += amba-pl022-looptest.o > obj-$(CONFIG_SPI_MPC512x_PSC) += mpc512x_psc_spi.o > obj-$(CONFIG_SPI_MPC52xx_PSC) += mpc52xx_psc_spi.o > obj-$(CONFIG_SPI_MPC52xx) += mpc52xx_spi.o > diff --git a/drivers/spi/amba-pl022-looptest.c b/drivers/spi/amba-pl022-looptest.c > index 5f55012..561886a 100644 > --- a/drivers/spi/amba-pl022-looptest.c > +++ b/drivers/spi/amba-pl022-looptest.c > @@ -1,7 +1,5 @@ > /* > - * arch/arm/mach-u300/dummyspichip.c > - * > - * Copyright (C) 2007-2009 ST-Ericsson AB > + * Copyright (C) 2007-2010 ST-Ericsson SA > * License terms: GNU General Public License (GPL) version 2 > * This is a dummy loopback SPI "chip" used for testing SPI. > * Author: Linus Walleij <linus.walleij@stericsson.com> > @@ -25,19 +23,93 @@ > */ > #include <linux/amba/pl022.h> > > -struct dummy { > +struct pl022_loopback { > struct device *dev; > struct mutex lock; > }; > > #define DMA_TEST_SIZE 2048 > > -/* When we cat /sys/bus/spi/devices/spi0.0/looptest this will be triggered */ > -static ssize_t dummy_looptest(struct device *dev, > +/* > + * The following code self-registers the PL022 loopback > + * chip on each PL022 SPI bus. > + */ > + > +static void pl022_select_loopback_chip(u32 chipselect) > +{ > + pr_debug("PL022 loopback chip: %s called with CS=0x%x (%s)\n", > + __func__, > + chipselect, > + chipselect ? "unselect chip" : "select chip"); > + /* > + * Here you would write the chip select value to the GPIO pins if > + * this was a real chip (but this is a loopback dummy). > + */ > +} > + > +struct pl022_config_chip pl022_loopback_chip_info = { > + /* Nominally this is LOOPBACK_DISABLED, but this is our dummy chip! */ > + .lbm = LOOPBACK_ENABLED, > + .com_mode = INTERRUPT_TRANSFER, > + .iface = SSP_INTERFACE_MOTOROLA_SPI, > + .hierarchy = SSP_MASTER, > + .slave_tx_disable = 0, > + /* LSB first */ > + .endian_tx = SSP_TX_LSB, > + .endian_rx = SSP_RX_LSB, > + .data_size = SSP_DATA_BITS_8, > + .rx_lev_trig = SSP_RX_1_OR_MORE_ELEM, > + .tx_lev_trig = SSP_TX_1_OR_MORE_EMPTY_LOC, > + .clk_phase = SSP_CLK_SECOND_EDGE, > + .clk_pol = SSP_CLK_POL_IDLE_LOW, > + .ctrl_len = SSP_BITS_12, > + .wait_state = SSP_MWIRE_WAIT_ZERO, > + .duplex = SSP_MICROWIRE_CHANNEL_FULL_DUPLEX, > + .cs_control = pl022_select_loopback_chip, > +}; > + > +static struct spi_board_info pl022_loopback_device = { > + /* A dummy chip used for loopback tests */ > + .modalias = "pl022-loop", > + /* Really dummy, pass in additional chip config here */ > + .platform_data = NULL, > + /* This defines how the controller shall handle the device */ > + .controller_data = &pl022_loopback_chip_info, > + /* .irq - no external IRQ routed from this device */ > + .max_speed_hz = 1000000, > + .bus_num = 0, /* Only one bus on this chip */ > + .chip_select = 0, > + /* Means SPI_CS_HIGH, change if e.g low CS */ > + .mode = 0, > +}; > + > +/* > + * This is called from the PL022 driver to register a loopback > + * on itself if this module is compiled in. > + */ > +void pl022_loopback_register(struct spi_master *master) > +{ > + struct spi_device *spidev; > + > + spidev = spi_new_device(master, &pl022_loopback_device); > + if (!spidev) > + dev_err(&master->dev, > + "could not register loopback test chip\n"); > + else > + dev_err(&master->dev, > + "registered loopback test chip\n"); > +} > + > +/* > + * This is where the tests begin. When we issue: > + * cat /sys/bus/spi/devices/spi0.0/looptest > + * this will be triggered. > + */ > +static ssize_t pl022_looptest(struct device *dev, > struct device_attribute *attr, char *buf) > { > struct spi_device *spi = to_spi_device(dev); > - struct dummy *p_dummy = dev_get_drvdata(&spi->dev); > + struct pl022_loopback *p_loop = dev_get_drvdata(&spi->dev); > > /* > * WARNING! Do not dereference the chip-specific data in any normal > @@ -55,7 +127,7 @@ static ssize_t dummy_looptest(struct device *dev, > u8 *bigtxbuf_virtual; > u8 *bigrxbuf_virtual; > > - if (mutex_lock_interruptible(&p_dummy->lock)) > + if (mutex_lock_interruptible(&p_loop->lock)) > return -ERESTARTSYS; > > bigtxbuf_virtual = kmalloc(DMA_TEST_SIZE, GFP_KERNEL); > @@ -217,25 +289,25 @@ static ssize_t dummy_looptest(struct device *dev, > kfree(bigrxbuf_virtual); > kfree(bigtxbuf_virtual); > out: > - mutex_unlock(&p_dummy->lock); > + mutex_unlock(&p_loop->lock); > return status; > } > > -static DEVICE_ATTR(looptest, S_IRUGO, dummy_looptest, NULL); > +static DEVICE_ATTR(looptest, S_IRUGO, pl022_looptest, NULL); > > -static int __devinit pl022_dummy_probe(struct spi_device *spi) > +static int __devinit pl022_loopback_probe(struct spi_device *spi) > { > - struct dummy *p_dummy; > + struct pl022_loopback *p_loop; > int status; > > dev_info(&spi->dev, "probing dummy SPI device\n"); > > - p_dummy = kzalloc(sizeof *p_dummy, GFP_KERNEL); > - if (!p_dummy) > + p_loop = kzalloc(sizeof *p_loop, GFP_KERNEL); > + if (!p_loop) > return -ENOMEM; > > - dev_set_drvdata(&spi->dev, p_dummy); > - mutex_init(&p_dummy->lock); > + dev_set_drvdata(&spi->dev, p_loop); > + mutex_init(&p_loop->lock); > > /* sysfs hook */ > status = device_create_file(&spi->dev, &dev_attr_looptest); > @@ -248,29 +320,29 @@ static int __devinit pl022_dummy_probe(struct spi_device *spi) > > out_dev_create_looptest_failed: > dev_set_drvdata(&spi->dev, NULL); > - kfree(p_dummy); > + kfree(p_loop); > return status; > } > > -static int __devexit pl022_dummy_remove(struct spi_device *spi) > +static int __devexit pl022_loopback_remove(struct spi_device *spi) > { > - struct dummy *p_dummy = dev_get_drvdata(&spi->dev); > + struct pl022_loopback *p_loop = dev_get_drvdata(&spi->dev); > > dev_info(&spi->dev, "removing dummy SPI device\n"); > device_remove_file(&spi->dev, &dev_attr_looptest); > dev_set_drvdata(&spi->dev, NULL); > - kfree(p_dummy); > + kfree(p_loop); > > return 0; > } > > static struct spi_driver pl022_dummy_driver = { > .driver = { > - .name = "spi-dummy", > + .name = "pl022-loop", > .owner = THIS_MODULE, > }, > - .probe = pl022_dummy_probe, > - .remove = __devexit_p(pl022_dummy_remove), > + .probe = pl022_loopback_probe, > + .remove = __devexit_p(pl022_loopback_remove), > }; > > static int __init pl022_init_dummy(void) > @@ -287,5 +359,5 @@ module_init(pl022_init_dummy); > module_exit(pl022_exit_dummy); > > MODULE_AUTHOR("Linus Walleij <linus.walleij@stericsson.com>"); > -MODULE_DESCRIPTION("PL022 SSP/SPI DUMMY Linux driver"); > +MODULE_DESCRIPTION("PL022 loopback test driver"); > MODULE_LICENSE("GPL"); > diff --git a/drivers/spi/amba-pl022-looptest.h b/drivers/spi/amba-pl022-looptest.h > new file mode 100644 > index 0000000..1bedcc6 > --- /dev/null > +++ b/drivers/spi/amba-pl022-looptest.h > @@ -0,0 +1,11 @@ > +/* > + * Defines a single function for the master to create a loopback > + * on itself. > + */ > +#ifdef CONFIG_SPI_PL022_LOOPTEST > +void pl022_loopback_register(struct spi_master *master); > +#else > +static void inline pl022_loopback_register(struct spi_master *master) > +{ > +} > +#endif > diff --git a/drivers/spi/amba-pl022.c b/drivers/spi/amba-pl022.c > index f0a1418..6e4a657 100644 > --- a/drivers/spi/amba-pl022.c > +++ b/drivers/spi/amba-pl022.c > @@ -46,6 +46,8 @@ > #include <linux/io.h> > #include <linux/slab.h> > > +#include "amba-pl022-looptest.h" > + > /* > * This macro is used to define some register default values. > * reg is masked with mask, the OR:ed with an (again masked) > @@ -1818,6 +1820,10 @@ pl022_probe(struct amba_device *adev, struct amba_id *id) > goto err_spi_register; > } > dev_dbg(dev, "probe succeded\n"); > + > + /* Register a loopback test if desired */ > + pl022_loopback_register(master); > + > return 0; > > err_spi_register: > -- > 1.7.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [spi-devel-general] [PATCH 2/2] SPI: make the moved loopback test work 2010-07-19 5:51 ` Grant Likely @ 2010-07-19 10:16 ` David Brownell [not found] ` <377310.49674.qm-4JhmkcZgSkmORdMXk8NaZPu2YVrzzGjVVpNB7YpNyf8@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: David Brownell @ 2010-07-19 10:16 UTC (permalink / raw) To: Linus Walleij, Grant Likely; +Cc: spi-devel-general, linux-arm-kernel A dedicated loopback test is the wrong model, given that SPI_LOOP exists. And is even used in the sample userspace code ... - Dave ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <377310.49674.qm-4JhmkcZgSkmORdMXk8NaZPu2YVrzzGjVVpNB7YpNyf8@public.gmane.org>]
* Re: [PATCH 2/2] SPI: make the moved loopback test work [not found] ` <377310.49674.qm-4JhmkcZgSkmORdMXk8NaZPu2YVrzzGjVVpNB7YpNyf8@public.gmane.org> @ 2010-07-19 16:03 ` Linus Walleij 2010-07-19 23:53 ` Linus Walleij 1 sibling, 0 replies; 9+ messages in thread From: Linus Walleij @ 2010-07-19 16:03 UTC (permalink / raw) To: David Brownell Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r 2010/7/19 David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>: > A dedicated loopback test is the wrong model, > given that SPI_LOOP exists. And is even used > in the sample userspace code ... Yeah I see, I never quite understood how SPI_LOOP was intended really. >From there to writing a *generic* loopback test is a bit of road to cover though, let's see if I can figure it out. The way I read the code I still have to provide some kind of dummychip on the bus in order to perform a loopback test on a bus with no devices connected, am I right? Yours, Linus Walleij ------------------------------------------------------------------------------ This SF.net email is sponsored by Sprint What will you do first with EVO, the first 4G phone? Visit sprint.com/first -- http://p.sf.net/sfu/sprint-com-first ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] SPI: make the moved loopback test work [not found] ` <377310.49674.qm-4JhmkcZgSkmORdMXk8NaZPu2YVrzzGjVVpNB7YpNyf8@public.gmane.org> 2010-07-19 16:03 ` Linus Walleij @ 2010-07-19 23:53 ` Linus Walleij 2010-07-20 1:32 ` [spi-devel-general] " David Brownell 1 sibling, 1 reply; 9+ messages in thread From: Linus Walleij @ 2010-07-19 23:53 UTC (permalink / raw) To: David Brownell, Anton Vorontsov Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r 2010/7/19 David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>: > A dedicated loopback test is the wrong model, > given that SPI_LOOP exists. And is even used > in the sample userspace code ... Hm looking over and grepping everywhere about this gives me no clues as to how an in-kernel SPI loopback test could use spi->mode |= SPI_LOOP. It can be set, and then it expects the hardware to set the corresponding bit. No matter how much I look at it, it seems like all use of this require you to first register *some* device, whether real or not, then you may play with it from kernelspace or userspace alike. So for a generic loopback in-kernel test I would need something like: loop over all masters for each master if it supports SPI_LOOP look for devices on master - make sure first device is a loopback dummy locate loopback dummy perform loopback test on the dummy This means I still would have to register some dummychip on the devices that should be able to run the test. Anton, any hints on this, execept that you added it for spidev? I'll try to dream some use model up otherwise. Yours, Linus Walleij ------------------------------------------------------------------------------ This SF.net email is sponsored by Sprint What will you do first with EVO, the first 4G phone? Visit sprint.com/first -- http://p.sf.net/sfu/sprint-com-first ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [spi-devel-general] [PATCH 2/2] SPI: make the moved loopback test work 2010-07-19 23:53 ` Linus Walleij @ 2010-07-20 1:32 ` David Brownell [not found] ` <812288.2599.qm-4JhmkcZgSkkA0QRgWO9Mevu2YVrzzGjVVpNB7YpNyf8@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: David Brownell @ 2010-07-20 1:32 UTC (permalink / raw) To: Anton Vorontsov, Linus Walleij; +Cc: spi-devel-general, linux-arm-kernel > No matter how much I look at it, it seems like > all use of this require you to first register > *some* device, OBVIOUSLY. You can't use SPI without a device; it's like any other driver framework in Linux. What else should you expect??? Look at the sample code though, ISTR you'll see that when the SPI master works correctly, putting it into loopback mode just means that the writes and reads (should) get the same data -- full duplex. No matter what the device is (given SPI_LOOP). - Dave ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <812288.2599.qm-4JhmkcZgSkkA0QRgWO9Mevu2YVrzzGjVVpNB7YpNyf8@public.gmane.org>]
* Re: [PATCH 2/2] SPI: make the moved loopback test work [not found] ` <812288.2599.qm-4JhmkcZgSkkA0QRgWO9Mevu2YVrzzGjVVpNB7YpNyf8@public.gmane.org> @ 2010-07-20 21:10 ` Linus Walleij 2010-07-25 7:48 ` [spi-devel-general] " Grant Likely 0 siblings, 1 reply; 9+ messages in thread From: Linus Walleij @ 2010-07-20 21:10 UTC (permalink / raw) To: David Brownell Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Anton Vorontsov, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r 2010/7/20 David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>: >> No matter how much I look at it, it seems like >> all use of this require you to first register >> *some* device, > > OBVIOUSLY. You can't use SPI without a device; > it's like any other driver framework in Linux. > What else should you expect??? I didn't expect anything else, it just makes it clear that the part of my patch registering a dummy device is not what is being disagreed upon, so this should be OK. Yours, Linus Walleij ------------------------------------------------------------------------------ This SF.net email is sponsored by Sprint What will you do first with EVO, the first 4G phone? Visit sprint.com/first -- http://p.sf.net/sfu/sprint-com-first ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [spi-devel-general] [PATCH 2/2] SPI: make the moved loopback test work 2010-07-20 21:10 ` Linus Walleij @ 2010-07-25 7:48 ` Grant Likely 2010-07-25 8:14 ` Linus Walleij 0 siblings, 1 reply; 9+ messages in thread From: Grant Likely @ 2010-07-25 7:48 UTC (permalink / raw) To: Linus Walleij Cc: David Brownell, spi-devel-general, Anton Vorontsov, linux-arm-kernel On Tue, Jul 20, 2010 at 3:10 PM, Linus Walleij <linus.ml.walleij@gmail.com> wrote: > 2010/7/20 David Brownell <david-b@pacbell.net>: > >>> No matter how much I look at it, it seems like >>> all use of this require you to first register >>> *some* device, >> >> OBVIOUSLY. You can't use SPI without a device; >> it's like any other driver framework in Linux. >> What else should you expect??? > > I didn't expect anything else, it just makes it clear that > the part of my patch registering a dummy device is > not what is being disagreed upon, so this should be OK. What's disagreed with is the dummy device knowing the intimate working details of the SPI bus driver. g. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] SPI: make the moved loopback test work 2010-07-25 7:48 ` [spi-devel-general] " Grant Likely @ 2010-07-25 8:14 ` Linus Walleij 0 siblings, 0 replies; 9+ messages in thread From: Linus Walleij @ 2010-07-25 8:14 UTC (permalink / raw) To: Grant Likely Cc: David Brownell, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Anton Vorontsov, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r 2010/7/25 Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>: > What's disagreed with is the dummy device knowing the intimate working > details of the SPI bus driver. OK I'll send the other patch I've been holding back, it merges that part into the driver itself so it sits in the apropriate containment, and solves that specific issue. But I understood Davids criticism like this: "you shouldn't make tests using loopback for a specific driver, it should be generic, i.e. being able to perform a loopback test on any driver supporting SPI_LOOP". A generic loopback test is a lot harder for me to produce, that's why I was troubled... Yours, Linus Walleij ------------------------------------------------------------------------------ This SF.net email is sponsored by Sprint What will you do first with EVO, the first 4G phone? Visit sprint.com/first -- http://p.sf.net/sfu/sprint-com-first ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-07-25 8:14 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-11 0:43 [PATCH 2/2] SPI: make the moved loopback test work Linus Walleij
2010-07-19 5:51 ` Grant Likely
2010-07-19 10:16 ` [spi-devel-general] " David Brownell
[not found] ` <377310.49674.qm-4JhmkcZgSkmORdMXk8NaZPu2YVrzzGjVVpNB7YpNyf8@public.gmane.org>
2010-07-19 16:03 ` Linus Walleij
2010-07-19 23:53 ` Linus Walleij
2010-07-20 1:32 ` [spi-devel-general] " David Brownell
[not found] ` <812288.2599.qm-4JhmkcZgSkkA0QRgWO9Mevu2YVrzzGjVVpNB7YpNyf8@public.gmane.org>
2010-07-20 21:10 ` Linus Walleij
2010-07-25 7:48 ` [spi-devel-general] " Grant Likely
2010-07-25 8:14 ` Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).