Changes since v2:
- Dropped AM33xx/OMAP support from series.
- Changed the DA850 L3 RAM gen_pool support to be based
on a previous Davinci SRAM series from Subhasish Ghosh
and Ben Gardiner.
Changes since v1:
- Replaced uio_pruss private SRAM API use with genalloc
- Added DA850 platform device and clock support
- Added DA850 L3 RAM gen_pool support
- Split out DT binding
This series enables uio_pruss on DA850 and removes use of the
private SRAM API by the driver. The driver previously was not
enabled by any platform and the private SRAM API was accessing
an invalid SRAM bank.
Ben Gardiner (2):
ARM: davinci: sram: ioremap the davinci_soc_info specified sram
regions
ARM: davinci: da850-dm646x: remove the SRAM_VIRT iotable entry
Matt Porter (3):
ARM: davinci: add platform hook to fetch the SRAM pool
ARM: davinci: Add support for PRUSS on DA850
uio: uio_pruss: replace private SRAM API with genalloc
Subhasish Ghosh (1):
ARM: davinci: da850: changed SRAM allocator to shared ram.
arch/arm/mach-davinci/board-da850-evm.c | 12 +++++
arch/arm/mach-davinci/da850.c | 17 +++----
arch/arm/mach-davinci/devices-da8xx.c | 66 +++++++++++++++++++++++++++
arch/arm/mach-davinci/dm355.c | 6 ---
arch/arm/mach-davinci/dm365.c | 6 ---
arch/arm/mach-davinci/dm644x.c | 6 ---
arch/arm/mach-davinci/dm646x.c | 6 ---
arch/arm/mach-davinci/include/mach/common.h | 2 -
arch/arm/mach-davinci/include/mach/da8xx.h | 3 ++
arch/arm/mach-davinci/include/mach/sram.h | 3 ++
arch/arm/mach-davinci/sram.c | 22 +++++++--
drivers/uio/Kconfig | 1 +
drivers/uio/uio_pruss.c | 24 +++++++---
include/linux/platform_data/uio_pruss.h | 3 +-
14 files changed, 132 insertions(+), 45 deletions(-)
--
1.7.9.5
From: Subhasish Ghosh <[email protected]>
This patch modifies the sram allocator to allocate memory
from the DA8XX shared RAM.
Signed-off-by: Subhasish Ghosh <[email protected]>
[rebased onto consolidated SRAM patches]
Signed-off-by: Ben Gardiner <[email protected]>
[rebased to mainline as consolidated SRAM patches were dropped]
Signed-off-by: Matt Porter <[email protected]>
---
arch/arm/mach-davinci/da850.c | 4 ++--
arch/arm/mach-davinci/include/mach/da8xx.h | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index b4b324f..d8d69de 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -1081,8 +1081,8 @@ static struct davinci_soc_info davinci_soc_info_da850 = {
.gpio_irq = IRQ_DA8XX_GPIO0,
.serial_dev = &da8xx_serial_device,
.emac_pdata = &da8xx_emac_pdata,
- .sram_dma = DA8XX_ARM_RAM_BASE,
- .sram_len = SZ_8K,
+ .sram_dma = DA8XX_SHARED_RAM_BASE,
+ .sram_len = SZ_128K,
};
void __init da850_init(void)
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
index c9ee723..20553cf 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -68,6 +68,7 @@ extern unsigned int da850_max_speed;
#define DA8XX_AEMIF_CS2_BASE 0x60000000
#define DA8XX_AEMIF_CS3_BASE 0x62000000
#define DA8XX_AEMIF_CTL_BASE 0x68000000
+#define DA8XX_SHARED_RAM_BASE 0x80000000
#define DA8XX_ARM_RAM_BASE 0xffff0000
void __init da830_init(void);
--
1.7.9.5
From: Ben Gardiner <[email protected]>
The sram regions defined for da850-dm646x in their iotable entries are also
defined in their davinci_soc_info's.
Remove this duplicate information which is now uneccessary since sram
init will ioremap the regions defined by their davinci_soc_info's.
Since this removal completely removes all uses of SRAM_VIRT, also remove
the SRAM_VIRT definition.
Signed-off-by: Ben Gardiner <[email protected]>
Tested-by: Matt Porter <[email protected]>
---
arch/arm/mach-davinci/da850.c | 6 ------
arch/arm/mach-davinci/dm355.c | 6 ------
arch/arm/mach-davinci/dm365.c | 6 ------
arch/arm/mach-davinci/dm644x.c | 6 ------
arch/arm/mach-davinci/dm646x.c | 6 ------
arch/arm/mach-davinci/include/mach/common.h | 2 --
6 files changed, 32 deletions(-)
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 6676dee..b4b324f 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -713,12 +713,6 @@ static struct map_desc da850_io_desc[] = {
.length = DA8XX_CP_INTC_SIZE,
.type = MT_DEVICE
},
- {
- .virtual = SRAM_VIRT,
- .pfn = __phys_to_pfn(DA8XX_ARM_RAM_BASE),
- .length = SZ_8K,
- .type = MT_DEVICE
- },
};
static u32 da850_psc_bases[] = { DA8XX_PSC0_BASE, DA8XX_PSC1_BASE };
diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c
index a255434..b49c3b7 100644
--- a/arch/arm/mach-davinci/dm355.c
+++ b/arch/arm/mach-davinci/dm355.c
@@ -758,12 +758,6 @@ static struct map_desc dm355_io_desc[] = {
.length = IO_SIZE,
.type = MT_DEVICE
},
- {
- .virtual = SRAM_VIRT,
- .pfn = __phys_to_pfn(0x00010000),
- .length = SZ_32K,
- .type = MT_MEMORY_NONCACHED,
- },
};
/* Contents of JTAG ID register used to identify exact cpu type */
diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c
index b680c83..6c39805 100644
--- a/arch/arm/mach-davinci/dm365.c
+++ b/arch/arm/mach-davinci/dm365.c
@@ -985,12 +985,6 @@ static struct map_desc dm365_io_desc[] = {
.length = IO_SIZE,
.type = MT_DEVICE
},
- {
- .virtual = SRAM_VIRT,
- .pfn = __phys_to_pfn(0x00010000),
- .length = SZ_32K,
- .type = MT_MEMORY_NONCACHED,
- },
};
static struct resource dm365_ks_resources[] = {
diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c
index 0755d46..f8aaa7d 100644
--- a/arch/arm/mach-davinci/dm644x.c
+++ b/arch/arm/mach-davinci/dm644x.c
@@ -795,12 +795,6 @@ static struct map_desc dm644x_io_desc[] = {
.length = IO_SIZE,
.type = MT_DEVICE
},
- {
- .virtual = SRAM_VIRT,
- .pfn = __phys_to_pfn(0x00008000),
- .length = SZ_16K,
- .type = MT_MEMORY_NONCACHED,
- },
};
/* Contents of JTAG ID register used to identify exact cpu type */
diff --git a/arch/arm/mach-davinci/dm646x.c b/arch/arm/mach-davinci/dm646x.c
index 97c0f8e..ac7b431 100644
--- a/arch/arm/mach-davinci/dm646x.c
+++ b/arch/arm/mach-davinci/dm646x.c
@@ -756,12 +756,6 @@ static struct map_desc dm646x_io_desc[] = {
.length = IO_SIZE,
.type = MT_DEVICE
},
- {
- .virtual = SRAM_VIRT,
- .pfn = __phys_to_pfn(0x00010000),
- .length = SZ_32K,
- .type = MT_MEMORY_NONCACHED,
- },
};
/* Contents of JTAG ID register used to identify exact cpu type */
diff --git a/arch/arm/mach-davinci/include/mach/common.h b/arch/arm/mach-davinci/include/mach/common.h
index bdc4aa8..046c723 100644
--- a/arch/arm/mach-davinci/include/mach/common.h
+++ b/arch/arm/mach-davinci/include/mach/common.h
@@ -104,8 +104,6 @@ int davinci_pm_init(void);
static inline int davinci_pm_init(void) { return 0; }
#endif
-/* standard place to map on-chip SRAMs; they *may* support DMA */
-#define SRAM_VIRT 0xfffe0000
#define SRAM_SIZE SZ_128K
#endif /* __ARCH_ARM_MACH_DAVINCI_COMMON_H */
--
1.7.9.5
Adds PRUSS clock, registers the L3RAM pool, and registers the
platform device for uio_pruss on DA850.
Signed-off-by: Matt Porter <[email protected]>
---
arch/arm/mach-davinci/board-da850-evm.c | 12 +++++
arch/arm/mach-davinci/da850.c | 7 +++
arch/arm/mach-davinci/devices-da8xx.c | 66 ++++++++++++++++++++++++++++
arch/arm/mach-davinci/include/mach/da8xx.h | 2 +
4 files changed, 87 insertions(+)
diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c
index 1295e61..acc6e84 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -29,6 +29,7 @@
#include <linux/regulator/machine.h>
#include <linux/regulator/tps6507x.h>
#include <linux/input/tps6507x-ts.h>
+#include <linux/platform_data/uio_pruss.h>
#include <linux/spi/spi.h>
#include <linux/spi/flash.h>
#include <linux/delay.h>
@@ -42,6 +43,7 @@
#include <mach/da8xx.h>
#include <linux/platform_data/mtd-davinci.h>
#include <mach/mux.h>
+#include <mach/sram.h>
#include <linux/platform_data/mtd-davinci-aemif.h>
#include <linux/platform_data/spi-davinci.h>
@@ -1253,6 +1255,10 @@ static __init int da850_wl12xx_init(void)
#endif /* CONFIG_DA850_WL12XX */
+struct uio_pruss_pdata da8xx_pruss_uio_pdata = {
+ .pintc_base = 0x4000,
+};
+
#define DA850EVM_SATA_REFCLKPN_RATE (100 * 1000 * 1000)
static __init void da850_evm_init(void)
@@ -1339,6 +1345,12 @@ static __init void da850_evm_init(void)
pr_warning("da850_evm_init: lcdcntl mux setup failed: %d\n",
ret);
+ da8xx_pruss_uio_pdata.l3ram_pool = sram_get_gen_pool();
+ ret = da8xx_register_pruss_uio(&da8xx_pruss_uio_pdata);
+ if (ret)
+ pr_warning("pruss_uio initialization failed: %d\n",
+ ret);
+
/* Handle board specific muxing for LCD here */
ret = davinci_cfg_reg_list(da850_evm_lcdc_pins);
if (ret)
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index d8d69de..7c01d31 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -212,6 +212,12 @@ static struct clk tptc2_clk = {
.flags = ALWAYS_ENABLED,
};
+static struct clk pruss_clk = {
+ .name = "pruss",
+ .parent = &pll0_sysclk2,
+ .lpsc = DA8XX_LPSC0_PRUSS,
+};
+
static struct clk uart0_clk = {
.name = "uart0",
.parent = &pll0_sysclk2,
@@ -378,6 +384,7 @@ static struct clk_lookup da850_clks[] = {
CLK(NULL, "tptc1", &tptc1_clk),
CLK(NULL, "tpcc1", &tpcc1_clk),
CLK(NULL, "tptc2", &tptc2_clk),
+ CLK(NULL, "pruss", &pruss_clk),
CLK(NULL, "uart0", &uart0_clk),
CLK(NULL, "uart1", &uart1_clk),
CLK(NULL, "uart2", &uart2_clk),
diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index bd2f72b..7c2e0d2 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -518,6 +518,72 @@ void __init da8xx_register_mcasp(int id, struct snd_platform_data *pdata)
}
}
+#define DA8XX_PRUSS_MEM_BASE 0x01C30000
+
+static struct resource da8xx_pruss_resources[] = {
+ {
+ .start = DA8XX_PRUSS_MEM_BASE,
+ .end = DA8XX_PRUSS_MEM_BASE + 0xFFFF,
+ .flags = IORESOURCE_MEM,
+ },
+ {
+ .start = IRQ_DA8XX_EVTOUT0,
+ .end = IRQ_DA8XX_EVTOUT0,
+ .flags = IORESOURCE_IRQ,
+ },
+ {
+ .start = IRQ_DA8XX_EVTOUT1,
+ .end = IRQ_DA8XX_EVTOUT1,
+ .flags = IORESOURCE_IRQ,
+ },
+ {
+ .start = IRQ_DA8XX_EVTOUT2,
+ .end = IRQ_DA8XX_EVTOUT2,
+ .flags = IORESOURCE_IRQ,
+ },
+ {
+ .start = IRQ_DA8XX_EVTOUT3,
+ .end = IRQ_DA8XX_EVTOUT3,
+ .flags = IORESOURCE_IRQ,
+ },
+ {
+ .start = IRQ_DA8XX_EVTOUT4,
+ .end = IRQ_DA8XX_EVTOUT4,
+ .flags = IORESOURCE_IRQ,
+ },
+ {
+ .start = IRQ_DA8XX_EVTOUT5,
+ .end = IRQ_DA8XX_EVTOUT5,
+ .flags = IORESOURCE_IRQ,
+ },
+ {
+ .start = IRQ_DA8XX_EVTOUT6,
+ .end = IRQ_DA8XX_EVTOUT6,
+ .flags = IORESOURCE_IRQ,
+ },
+ {
+ .start = IRQ_DA8XX_EVTOUT7,
+ .end = IRQ_DA8XX_EVTOUT7,
+ .flags = IORESOURCE_IRQ,
+ },
+};
+
+static struct platform_device da8xx_pruss_uio_dev = {
+ .name = "pruss_uio",
+ .id = -1,
+ .num_resources = ARRAY_SIZE(da8xx_pruss_resources),
+ .resource = da8xx_pruss_resources,
+ .dev = {
+ .coherent_dma_mask = 0xffffffff,
+ }
+};
+
+int __init da8xx_register_pruss_uio(struct uio_pruss_pdata *config)
+{
+ da8xx_pruss_uio_dev.dev.platform_data = config;
+ return platform_device_register(&da8xx_pruss_uio_dev);
+}
+
static const struct display_panel disp_panel = {
QVGA,
16,
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
index 20553cf..138e618 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -25,6 +25,7 @@
#include <linux/platform_data/mmc-davinci.h>
#include <linux/platform_data/usb-davinci.h>
#include <linux/platform_data/spi-davinci.h>
+#include <linux/platform_data/uio_pruss.h>
extern void __iomem *da8xx_syscfg0_base;
extern void __iomem *da8xx_syscfg1_base;
@@ -83,6 +84,7 @@ int da8xx_register_watchdog(void);
int da8xx_register_usb20(unsigned mA, unsigned potpgt);
int da8xx_register_usb11(struct da8xx_ohci_root_hub *pdata);
int da8xx_register_emac(void);
+int da8xx_register_pruss_uio(struct uio_pruss_pdata *config);
int da8xx_register_lcdc(struct da8xx_lcdc_platform_data *pdata);
int da8xx_register_mmcsd0(struct davinci_mmc_config *config);
int da850_register_mmcsd1(struct davinci_mmc_config *config);
--
1.7.9.5
Remove the use of the private DaVinci SRAM API in favor
of genalloc. The pool to be used is provided by platform
data.
Signed-off-by: Matt Porter <[email protected]>
---
drivers/uio/Kconfig | 1 +
drivers/uio/uio_pruss.c | 24 +++++++++++++++++-------
include/linux/platform_data/uio_pruss.h | 3 ++-
3 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
index 6f3ea9b..c48b938 100644
--- a/drivers/uio/Kconfig
+++ b/drivers/uio/Kconfig
@@ -97,6 +97,7 @@ config UIO_NETX
config UIO_PRUSS
tristate "Texas Instruments PRUSS driver"
depends on ARCH_DAVINCI_DA850
+ select GENERIC_ALLOCATOR
help
PRUSS driver for OMAPL138/DA850/AM18XX devices
PRUSS driver requires user space components, examples and user space
diff --git a/drivers/uio/uio_pruss.c b/drivers/uio/uio_pruss.c
index 33a7a27..1b55db3 100644
--- a/drivers/uio/uio_pruss.c
+++ b/drivers/uio/uio_pruss.c
@@ -25,7 +25,7 @@
#include <linux/clk.h>
#include <linux/dma-mapping.h>
#include <linux/slab.h>
-#include <mach/sram.h>
+#include <linux/genalloc.h>
#define DRV_NAME "pruss_uio"
#define DRV_VERSION "1.0"
@@ -65,10 +65,11 @@ struct uio_pruss_dev {
dma_addr_t sram_paddr;
dma_addr_t ddr_paddr;
void __iomem *prussio_vaddr;
- void *sram_vaddr;
+ unsigned long sram_vaddr;
void *ddr_vaddr;
unsigned int hostirq_start;
unsigned int pintc_base;
+ struct gen_pool *sram_pool;
};
static irqreturn_t pruss_handler(int irq, struct uio_info *info)
@@ -106,7 +107,9 @@ static void pruss_cleanup(struct platform_device *dev,
gdev->ddr_paddr);
}
if (gdev->sram_vaddr)
- sram_free(gdev->sram_vaddr, sram_pool_sz);
+ gen_pool_free(gdev->sram_pool,
+ gdev->sram_vaddr,
+ sram_pool_sz);
kfree(gdev->info);
clk_put(gdev->pruss_clk);
kfree(gdev);
@@ -152,10 +155,17 @@ static int __devinit pruss_probe(struct platform_device *dev)
goto out_free;
}
- gdev->sram_vaddr = sram_alloc(sram_pool_sz, &(gdev->sram_paddr));
- if (!gdev->sram_vaddr) {
- dev_err(&dev->dev, "Could not allocate SRAM pool\n");
- goto out_free;
+ if (pdata->l3ram_pool) {
+ gdev->sram_pool = pdata->l3ram_pool;
+ gdev->sram_vaddr =
+ gen_pool_alloc(gdev->sram_pool, sram_pool_sz);
+ if (!gdev->sram_vaddr) {
+ dev_err(&dev->dev, "Could not allocate SRAM pool\n");
+ goto out_free;
+ }
+ gdev->sram_paddr =
+ gen_pool_virt_to_phys(gdev->sram_pool,
+ gdev->sram_vaddr);
}
gdev->ddr_vaddr = dma_alloc_coherent(&dev->dev, extram_pool_sz,
diff --git a/include/linux/platform_data/uio_pruss.h b/include/linux/platform_data/uio_pruss.h
index f39140a..e500fe6 100644
--- a/include/linux/platform_data/uio_pruss.h
+++ b/include/linux/platform_data/uio_pruss.h
@@ -20,6 +20,7 @@
/* To configure the PRUSS INTC base offset for UIO driver */
struct uio_pruss_pdata {
- u32 pintc_base;
+ u32 pintc_base;
+ struct gen_pool *l3ram_pool;
};
#endif /* _UIO_PRUSS_H_ */
--
1.7.9.5
Adds sram_get_gen_pool() which allows platform code to get
the SRAM gen_pool for purposes of providing it in platform
data for driver genalloc use.
Signed-off-by: Matt Porter <[email protected]>
---
arch/arm/mach-davinci/include/mach/sram.h | 3 +++
arch/arm/mach-davinci/sram.c | 5 +++++
2 files changed, 8 insertions(+)
diff --git a/arch/arm/mach-davinci/include/mach/sram.h b/arch/arm/mach-davinci/include/mach/sram.h
index 111f7cc..4e5db56 100644
--- a/arch/arm/mach-davinci/include/mach/sram.h
+++ b/arch/arm/mach-davinci/include/mach/sram.h
@@ -24,4 +24,7 @@
extern void *sram_alloc(size_t len, dma_addr_t *dma);
extern void sram_free(void *addr, size_t len);
+/* Get the struct gen_pool * for use in platform data */
+extern struct gen_pool *sram_get_gen_pool(void);
+
#endif /* __MACH_SRAM_H */
diff --git a/arch/arm/mach-davinci/sram.c b/arch/arm/mach-davinci/sram.c
index 0e8ca4f..b47b64d 100644
--- a/arch/arm/mach-davinci/sram.c
+++ b/arch/arm/mach-davinci/sram.c
@@ -18,6 +18,11 @@
static struct gen_pool *sram_pool;
+struct gen_pool *sram_get_gen_pool(void)
+{
+ return sram_pool;
+}
+
void *sram_alloc(size_t len, dma_addr_t *dma)
{
unsigned long vaddr;
--
1.7.9.5
From: Ben Gardiner <[email protected]>
The current davinci init sets up SRAM in iotables. There has been an observed
failure to boot a da850 with 128K specified in the iotable.
Make the davinci sram allocator -- now based on RMK's consolidated SRAM
support -- do an ioremap of the region specified by the entries in
davinci_soc_info before registering with gen_pool_add_virt().
This commit breaks runtime of davinci boards since the regions that
the sram init is now trying to ioremap have been iomapped by their
iotable entries. The iotable entries will be removed in the patches
to come.
Signed-off-by: Ben Gardiner <[email protected]>
[rebased to mainline as the consolidated SRAM support was dropped]
Signed-off-by: Matt Porter <[email protected]>
---
arch/arm/mach-davinci/sram.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-davinci/sram.c b/arch/arm/mach-davinci/sram.c
index db0f778..0e8ca4f 100644
--- a/arch/arm/mach-davinci/sram.c
+++ b/arch/arm/mach-davinci/sram.c
@@ -10,6 +10,7 @@
*/
#include <linux/module.h>
#include <linux/init.h>
+#include <linux/io.h>
#include <linux/genalloc.h>
#include <mach/common.h>
@@ -32,7 +33,7 @@ void *sram_alloc(size_t len, dma_addr_t *dma)
return NULL;
if (dma)
- *dma = dma_base + (vaddr - SRAM_VIRT);
+ *dma = gen_pool_virt_to_phys(sram_pool, vaddr);
return (void *)vaddr;
}
@@ -53,8 +54,10 @@ EXPORT_SYMBOL(sram_free);
*/
static int __init sram_init(void)
{
+ phys_addr_t phys = davinci_soc_info.sram_dma;
unsigned len = davinci_soc_info.sram_len;
int status = 0;
+ void *addr;
if (len) {
len = min_t(unsigned, len, SRAM_SIZE);
@@ -62,8 +65,16 @@ static int __init sram_init(void)
if (!sram_pool)
status = -ENOMEM;
}
- if (sram_pool)
- status = gen_pool_add(sram_pool, SRAM_VIRT, len, -1);
+
+ if (sram_pool) {
+ addr = ioremap(phys, len);
+ if (!addr)
+ return -ENOMEM;
+ if((status = gen_pool_add_virt(sram_pool, (unsigned)addr,
+ phys, len, -1)))
+ iounmap(addr);
+ }
+
WARN_ON(status < 0);
return status;
}
--
1.7.9.5
Hi Matt,
On 10/3/12, Matt Porter <[email protected]> wrote:
> This series enables uio_pruss on DA850 and removes use of the
> private SRAM API by the driver. The driver previously was not
> enabled by any platform and the private SRAM API was accessing
> an invalid SRAM bank.
have you seen my SRAM patch series at https://lkml.org/lkml/2012/9/7/281
"Add device tree support for on-chip SRAM" ?
I think the generic SRAM/genalloc driver (https://lkml.org/lkml/2012/9/7/282)
could be useful to map the L3RAM on Davinci.
With the gen_pool lookup patch (https://lkml.org/lkml/2012/9/7/284) the
uio_pruss driver could then use the gen_pool_find_by_phys() (or
of_get_named_gen_pool() for initialization from device tree) to
retrieve the struct gen_pool*.
This way you could avoid handing it over via platform data and you could
get rid of arch/arm/mach-davinci/{sram.c,include/mach/sram.h} completely.
regards
Philipp
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 10/3/2012 8:25 PM, Matt Porter wrote:
> From: Ben Gardiner <[email protected]>
>
> The current davinci init sets up SRAM in iotables. There has been an observed
> failure to boot a da850 with 128K specified in the iotable.
>
> Make the davinci sram allocator -- now based on RMK's consolidated SRAM
> support -- do an ioremap of the region specified by the entries in
The part about being based on RMK's consolidated SRAM support should be
dropped.
> davinci_soc_info before registering with gen_pool_add_virt().
>
> This commit breaks runtime of davinci boards since the regions that
> the sram init is now trying to ioremap have been iomapped by their
> iotable entries. The iotable entries will be removed in the patches
> to come.
I would prefer merging 2/6 into this for this reason.
>
> Signed-off-by: Ben Gardiner <[email protected]>
> [rebased to mainline as the consolidated SRAM support was dropped]
> Signed-off-by: Matt Porter <[email protected]>
> ---
> arch/arm/mach-davinci/sram.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/sram.c b/arch/arm/mach-davinci/sram.c
> index db0f778..0e8ca4f 100644
> --- a/arch/arm/mach-davinci/sram.c
> +++ b/arch/arm/mach-davinci/sram.c
> @@ -10,6 +10,7 @@
> */
> #include <linux/module.h>
> #include <linux/init.h>
> +#include <linux/io.h>
> #include <linux/genalloc.h>
>
> #include <mach/common.h>
> @@ -32,7 +33,7 @@ void *sram_alloc(size_t len, dma_addr_t *dma)
> return NULL;
>
> if (dma)
> - *dma = dma_base + (vaddr - SRAM_VIRT);
> + *dma = gen_pool_virt_to_phys(sram_pool, vaddr);
> return (void *)vaddr;
>
> }
> @@ -53,8 +54,10 @@ EXPORT_SYMBOL(sram_free);
> */
> static int __init sram_init(void)
> {
> + phys_addr_t phys = davinci_soc_info.sram_dma;
> unsigned len = davinci_soc_info.sram_len;
> int status = 0;
> + void *addr;
>
> if (len) {
> len = min_t(unsigned, len, SRAM_SIZE);
> @@ -62,8 +65,16 @@ static int __init sram_init(void)
> if (!sram_pool)
> status = -ENOMEM;
> }
> - if (sram_pool)
> - status = gen_pool_add(sram_pool, SRAM_VIRT, len, -1);
> +
> + if (sram_pool) {
> + addr = ioremap(phys, len);
> + if (!addr)
> + return -ENOMEM;
> + if((status = gen_pool_add_virt(sram_pool, (unsigned)addr,
> + phys, len, -1)))
Nit: prefer to set status outside of if().
Looks good otherwise. Thanks for reviving this.
Thanks,
Sekhar
On 10/3/2012 8:25 PM, Matt Porter wrote:
> From: Ben Gardiner <[email protected]>
>
> The sram regions defined for da850-dm646x in their iotable entries are also
> defined in their davinci_soc_info's.
>
> Remove this duplicate information which is now uneccessary since sram
> init will ioremap the regions defined by their davinci_soc_info's.
>
> Since this removal completely removes all uses of SRAM_VIRT, also remove
> the SRAM_VIRT definition.
>
> Signed-off-by: Ben Gardiner <[email protected]>
> Tested-by: Matt Porter <[email protected]>
What testing was done with this patch? Can you please add that
information to the commit text as well.
Thanks,
Sekhar
Matt,
On 10/3/2012 8:25 PM, Matt Porter wrote:
> From: Subhasish Ghosh <[email protected]>
>
> This patch modifies the sram allocator to allocate memory
> from the DA8XX shared RAM.
>
> Signed-off-by: Subhasish Ghosh <[email protected]>
> [rebased onto consolidated SRAM patches]
> Signed-off-by: Ben Gardiner <[email protected]>
> [rebased to mainline as consolidated SRAM patches were dropped]
> Signed-off-by: Matt Porter <[email protected]>
Were you able to test PM with this change or you need my help?
Thanks,
Sekhar
On 10/3/2012 8:25 PM, Matt Porter wrote:
> Adds PRUSS clock, registers the L3RAM pool, and registers the
> platform device for uio_pruss on DA850.
>
> Signed-off-by: Matt Porter <[email protected]>
I am interested in knowing how this patch was tested.
> ---
> arch/arm/mach-davinci/board-da850-evm.c | 12 +++++
> arch/arm/mach-davinci/da850.c | 7 +++
> arch/arm/mach-davinci/devices-da8xx.c | 66 ++++++++++++++++++++++++++++
> arch/arm/mach-davinci/include/mach/da8xx.h | 2 +
> 4 files changed, 87 insertions(+)
>
> diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c
> index 1295e61..acc6e84 100644
> --- a/arch/arm/mach-davinci/board-da850-evm.c
> +++ b/arch/arm/mach-davinci/board-da850-evm.c
> @@ -29,6 +29,7 @@
> #include <linux/regulator/machine.h>
> #include <linux/regulator/tps6507x.h>
> #include <linux/input/tps6507x-ts.h>
> +#include <linux/platform_data/uio_pruss.h>
> #include <linux/spi/spi.h>
> #include <linux/spi/flash.h>
> #include <linux/delay.h>
> @@ -42,6 +43,7 @@
> #include <mach/da8xx.h>
> #include <linux/platform_data/mtd-davinci.h>
> #include <mach/mux.h>
> +#include <mach/sram.h>
> #include <linux/platform_data/mtd-davinci-aemif.h>
> #include <linux/platform_data/spi-davinci.h>
I know you did not introduce the mess, but can you include a patch to
fix the mixture of mach/ and linux/ includes here? mach/ includes should
follow the linux/ includes.
>
> @@ -1253,6 +1255,10 @@ static __init int da850_wl12xx_init(void)
>
> #endif /* CONFIG_DA850_WL12XX */
>
> +struct uio_pruss_pdata da8xx_pruss_uio_pdata = {
> + .pintc_base = 0x4000,
> +};
> +
> #define DA850EVM_SATA_REFCLKPN_RATE (100 * 1000 * 1000)
>
> static __init void da850_evm_init(void)
> @@ -1339,6 +1345,12 @@ static __init void da850_evm_init(void)
> pr_warning("da850_evm_init: lcdcntl mux setup failed: %d\n",
> ret);
>
> + da8xx_pruss_uio_pdata.l3ram_pool = sram_get_gen_pool();
I wonder if this platform data should still be called l3ram pool. L3RAM
is OMAP terminology. May be just call it sram_pool? Also this patch
should follow 6/6 to prevent build breakage.
> + ret = da8xx_register_pruss_uio(&da8xx_pruss_uio_pdata);
> + if (ret)
> + pr_warning("pruss_uio initialization failed: %d\n",
> + ret);
> +
> /* Handle board specific muxing for LCD here */
> ret = davinci_cfg_reg_list(da850_evm_lcdc_pins);
> if (ret)
> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
> index d8d69de..7c01d31 100644
> --- a/arch/arm/mach-davinci/da850.c
> +++ b/arch/arm/mach-davinci/da850.c
Can you separate out board and SoC changes into different patches?
> @@ -212,6 +212,12 @@ static struct clk tptc2_clk = {
> .flags = ALWAYS_ENABLED,
> };
>
> +static struct clk pruss_clk = {
> + .name = "pruss",
> + .parent = &pll0_sysclk2,
> + .lpsc = DA8XX_LPSC0_PRUSS,
> +};
> +
> static struct clk uart0_clk = {
> .name = "uart0",
> .parent = &pll0_sysclk2,
> @@ -378,6 +384,7 @@ static struct clk_lookup da850_clks[] = {
> CLK(NULL, "tptc1", &tptc1_clk),
> CLK(NULL, "tpcc1", &tpcc1_clk),
> CLK(NULL, "tptc2", &tptc2_clk),
> + CLK(NULL, "pruss", &pruss_clk),
This is actually incorrect since we should use device name rather than
con_id for matching the clock. If there is just one clock that the
driver needs, connection id should be NULL. Looking at the driver now,
the clk_get() call seems to pass a valid device pointer. So, I wonder
how you are able to look up the clock even with a NULL device name.
> CLK(NULL, "uart0", &uart0_clk),
> CLK(NULL, "uart1", &uart1_clk),
> CLK(NULL, "uart2", &uart2_clk),
> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> index bd2f72b..7c2e0d2 100644
> --- a/arch/arm/mach-davinci/devices-da8xx.c
> +++ b/arch/arm/mach-davinci/devices-da8xx.c
> @@ -518,6 +518,72 @@ void __init da8xx_register_mcasp(int id, struct snd_platform_data *pdata)
> }
> }
>
> +#define DA8XX_PRUSS_MEM_BASE 0x01C30000
In this file all base addresses are added at the top of the file. The
defines are sorted in ascending order of address. If that's broken, its
all my fault, but please don't add to the breakage when adding this entry :)
> +
> +static struct resource da8xx_pruss_resources[] = {
> + {
> + .start = DA8XX_PRUSS_MEM_BASE,
> + .end = DA8XX_PRUSS_MEM_BASE + 0xFFFF,
> + .flags = IORESOURCE_MEM,
> + },
> + {
> + .start = IRQ_DA8XX_EVTOUT0,
> + .end = IRQ_DA8XX_EVTOUT0,
> + .flags = IORESOURCE_IRQ,
> + },
> + {
> + .start = IRQ_DA8XX_EVTOUT1,
> + .end = IRQ_DA8XX_EVTOUT1,
> + .flags = IORESOURCE_IRQ,
> + },
> + {
> + .start = IRQ_DA8XX_EVTOUT2,
> + .end = IRQ_DA8XX_EVTOUT2,
> + .flags = IORESOURCE_IRQ,
> + },
> + {
> + .start = IRQ_DA8XX_EVTOUT3,
> + .end = IRQ_DA8XX_EVTOUT3,
> + .flags = IORESOURCE_IRQ,
> + },
> + {
> + .start = IRQ_DA8XX_EVTOUT4,
> + .end = IRQ_DA8XX_EVTOUT4,
> + .flags = IORESOURCE_IRQ,
> + },
> + {
> + .start = IRQ_DA8XX_EVTOUT5,
> + .end = IRQ_DA8XX_EVTOUT5,
> + .flags = IORESOURCE_IRQ,
> + },
> + {
> + .start = IRQ_DA8XX_EVTOUT6,
> + .end = IRQ_DA8XX_EVTOUT6,
> + .flags = IORESOURCE_IRQ,
> + },
> + {
> + .start = IRQ_DA8XX_EVTOUT7,
> + .end = IRQ_DA8XX_EVTOUT7,
> + .flags = IORESOURCE_IRQ,
> + },
> +};
> +
> +static struct platform_device da8xx_pruss_uio_dev = {
> + .name = "pruss_uio",
> + .id = -1,
> + .num_resources = ARRAY_SIZE(da8xx_pruss_resources),
> + .resource = da8xx_pruss_resources,
> + .dev = {
> + .coherent_dma_mask = 0xffffffff,
DMA_BIT_MASK(32)?
Thanks,
Sekhar
On Thu, Oct 04, 2012 at 11:11:45AM +0200, Philipp Zabel wrote:
> Hi Matt,
>
> On 10/3/12, Matt Porter <[email protected]> wrote:
> > This series enables uio_pruss on DA850 and removes use of the
> > private SRAM API by the driver. The driver previously was not
> > enabled by any platform and the private SRAM API was accessing
> > an invalid SRAM bank.
>
> have you seen my SRAM patch series at https://lkml.org/lkml/2012/9/7/281
> "Add device tree support for on-chip SRAM" ?
Yes.
> I think the generic SRAM/genalloc driver (https://lkml.org/lkml/2012/9/7/282)
> could be useful to map the L3RAM on Davinci.
> With the gen_pool lookup patch (https://lkml.org/lkml/2012/9/7/284) the
> uio_pruss driver could then use the gen_pool_find_by_phys() (or
> of_get_named_gen_pool() for initialization from device tree) to
> retrieve the struct gen_pool*.
>
> This way you could avoid handing it over via platform data and you could
> get rid of arch/arm/mach-davinci/{sram.c,include/mach/sram.h} completely.
I did miss the gen_pool_find_by_phys() call in that series. That does
look useful. I actually mentioned your series in an earlier posting
since I like it, but since the initialization of the driver was inherently
tied to DT it's not usable for DaVinci that's just starting to convert
to DT and needs !DT support as well.
I do see it moving to your driver exclusively, but I wanted to make this
series focused on only getting rid of the private SRAM API using the
existing pdata framework that's already there. I think once
gen_pool_find_by_phys() goes upstream we can switch to that and get the
address from a resource in the !DT case. I guess we should see if Sekhar
would like to see this happen in two steps or just have us depend on
the gen_pool_find_by_phys() patch now.
BTW, I was going to post a patch for your driver to allow
configurability of the allocation order, but have been busy with other
things. We'll eventually need that when switching to it as the
hardcoded page size order isn't going to work for all cases.
-Matt
On Thu, Oct 04, 2012 at 05:18:41PM +0530, Sekhar Nori wrote:
> On 10/3/2012 8:25 PM, Matt Porter wrote:
> > From: Ben Gardiner <[email protected]>
> >
> > The current davinci init sets up SRAM in iotables. There has been an observed
> > failure to boot a da850 with 128K specified in the iotable.
> >
> > Make the davinci sram allocator -- now based on RMK's consolidated SRAM
> > support -- do an ioremap of the region specified by the entries in
>
> The part about being based on RMK's consolidated SRAM support should be
> dropped.
Ok, cruft from trying to preserve Ben's original patch comments. Will
remove.
> > davinci_soc_info before registering with gen_pool_add_virt().
> >
> > This commit breaks runtime of davinci boards since the regions that
> > the sram init is now trying to ioremap have been iomapped by their
> > iotable entries. The iotable entries will be removed in the patches
> > to come.
>
> I would prefer merging 2/6 into this for this reason.
Ok, makes sense. This again comes from me trying to just use the
original patches. I'll squash these together for v4.
>
> >
> > Signed-off-by: Ben Gardiner <[email protected]>
> > [rebased to mainline as the consolidated SRAM support was dropped]
> > Signed-off-by: Matt Porter <[email protected]>
> > ---
> > arch/arm/mach-davinci/sram.c | 17 ++++++++++++++---
> > 1 file changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/mach-davinci/sram.c b/arch/arm/mach-davinci/sram.c
> > index db0f778..0e8ca4f 100644
> > --- a/arch/arm/mach-davinci/sram.c
> > +++ b/arch/arm/mach-davinci/sram.c
> > @@ -10,6 +10,7 @@
> > */
> > #include <linux/module.h>
> > #include <linux/init.h>
> > +#include <linux/io.h>
> > #include <linux/genalloc.h>
> >
> > #include <mach/common.h>
> > @@ -32,7 +33,7 @@ void *sram_alloc(size_t len, dma_addr_t *dma)
> > return NULL;
> >
> > if (dma)
> > - *dma = dma_base + (vaddr - SRAM_VIRT);
> > + *dma = gen_pool_virt_to_phys(sram_pool, vaddr);
> > return (void *)vaddr;
> >
> > }
> > @@ -53,8 +54,10 @@ EXPORT_SYMBOL(sram_free);
> > */
> > static int __init sram_init(void)
> > {
> > + phys_addr_t phys = davinci_soc_info.sram_dma;
> > unsigned len = davinci_soc_info.sram_len;
> > int status = 0;
> > + void *addr;
> >
> > if (len) {
> > len = min_t(unsigned, len, SRAM_SIZE);
> > @@ -62,8 +65,16 @@ static int __init sram_init(void)
> > if (!sram_pool)
> > status = -ENOMEM;
> > }
> > - if (sram_pool)
> > - status = gen_pool_add(sram_pool, SRAM_VIRT, len, -1);
> > +
> > + if (sram_pool) {
> > + addr = ioremap(phys, len);
> > + if (!addr)
> > + return -ENOMEM;
> > + if((status = gen_pool_add_virt(sram_pool, (unsigned)addr,
> > + phys, len, -1)))
>
> Nit: prefer to set status outside of if().
Ok. Will change.
>
> Looks good otherwise. Thanks for reviving this.
No problem, I'm just glad we seem to have a workable solution now.
Also, if you could check the conversation with Phillip about
gen_pool_find_by_phys(), I wonder what you think about depending on that
now, or waiting to convert to it when it goes upstream.
I'm torn as I see all the pdata going away anyway when Davinci is fully
converted to DT, and then also any use of find_by_phys() would go away
in favor of using the of helpers with his driver.
-Matt
On Thu, Oct 04, 2012 at 05:23:34PM +0530, Sekhar Nori wrote:
> On 10/3/2012 8:25 PM, Matt Porter wrote:
> > From: Ben Gardiner <[email protected]>
> >
> > The sram regions defined for da850-dm646x in their iotable entries are also
> > defined in their davinci_soc_info's.
> >
> > Remove this duplicate information which is now uneccessary since sram
> > init will ioremap the regions defined by their davinci_soc_info's.
> >
> > Since this removal completely removes all uses of SRAM_VIRT, also remove
> > the SRAM_VIRT definition.
> >
> > Signed-off-by: Ben Gardiner <[email protected]>
> > Tested-by: Matt Porter <[email protected]>
>
> What testing was done with this patch? Can you please add that
> information to the commit text as well.
I have only tested on AM180x with uio_pruss and davinci-pcm (series
for that posted a short while ago). In the uio_pruss case, I run the
sample applications from the userspace PRU package that test
communication with the shared SRAM, DDR, and internal PRU SRAM. For
davinci-pcm, my series enables shared SRAM ping-pong buffering which is
working comparably to pcm capture/playback without ping-pong buffering
enabled on AM180x.
I have not tested PM at all...yet.
I'll add this detail to the commit.
-Matt
On 10/4/2012 6:12 PM, Matt Porter wrote:
> On Thu, Oct 04, 2012 at 11:11:45AM +0200, Philipp Zabel wrote:
>> Hi Matt,
>>
>> On 10/3/12, Matt Porter <[email protected]> wrote:
>>> This series enables uio_pruss on DA850 and removes use of the
>>> private SRAM API by the driver. The driver previously was not
>>> enabled by any platform and the private SRAM API was accessing
>>> an invalid SRAM bank.
>>
>> have you seen my SRAM patch series at https://lkml.org/lkml/2012/9/7/281
>> "Add device tree support for on-chip SRAM" ?
>
> Yes.
>
>> I think the generic SRAM/genalloc driver (https://lkml.org/lkml/2012/9/7/282)
>> could be useful to map the L3RAM on Davinci.
>> With the gen_pool lookup patch (https://lkml.org/lkml/2012/9/7/284) the
>> uio_pruss driver could then use the gen_pool_find_by_phys() (or
>> of_get_named_gen_pool() for initialization from device tree) to
>> retrieve the struct gen_pool*.
>>
>> This way you could avoid handing it over via platform data and you could
>> get rid of arch/arm/mach-davinci/{sram.c,include/mach/sram.h} completely.
>
> I did miss the gen_pool_find_by_phys() call in that series. That does
> look useful. I actually mentioned your series in an earlier posting
> since I like it, but since the initialization of the driver was inherently
> tied to DT it's not usable for DaVinci that's just starting to convert
> to DT and needs !DT support as well.
>
> I do see it moving to your driver exclusively, but I wanted to make this
> series focused on only getting rid of the private SRAM API using the
> existing pdata framework that's already there. I think once
> gen_pool_find_by_phys() goes upstream we can switch to that and get the
> address from a resource in the !DT case. I guess we should see if Sekhar
> would like to see this happen in two steps or just have us depend on
> the gen_pool_find_by_phys() patch now.
I prefer going with this series now and switching to the SRAM driver
once it is available mainline.
Thanks,
Sekhar
On Thu, Oct 04, 2012 at 05:27:23PM +0530, Sekhar Nori wrote:
> Matt,
>
> On 10/3/2012 8:25 PM, Matt Porter wrote:
> > From: Subhasish Ghosh <[email protected]>
> >
> > This patch modifies the sram allocator to allocate memory
> > from the DA8XX shared RAM.
> >
> > Signed-off-by: Subhasish Ghosh <[email protected]>
> > [rebased onto consolidated SRAM patches]
> > Signed-off-by: Ben Gardiner <[email protected]>
> > [rebased to mainline as consolidated SRAM patches were dropped]
> > Signed-off-by: Matt Porter <[email protected]>
>
> Were you able to test PM with this change or you need my help?
I'll try it out now and if I have problems I may need to enlist your
help here. :)
-Matt
On Thu, Oct 04, 2012 at 05:52:45PM +0530, Sekhar Nori wrote:
> On 10/3/2012 8:25 PM, Matt Porter wrote:
> > Adds PRUSS clock, registers the L3RAM pool, and registers the
> > platform device for uio_pruss on DA850.
> >
> > Signed-off-by: Matt Porter <[email protected]>
>
> I am interested in knowing how this patch was tested.
I'll add similar test information in the commit that I mention in
my other reply. Basically, I use the examples in the PRU package
downloadable from ti.com. There's a PRU_memAccessL3andDDR example
which will fail miserably if the uio_pruss driver SRAM resource
is not configured properly. With the complete series in place, it's
able to complete execution successfully, communicating via the
shared sram pool.
> > ---
> > arch/arm/mach-davinci/board-da850-evm.c | 12 +++++
> > arch/arm/mach-davinci/da850.c | 7 +++
> > arch/arm/mach-davinci/devices-da8xx.c | 66 ++++++++++++++++++++++++++++
> > arch/arm/mach-davinci/include/mach/da8xx.h | 2 +
> > 4 files changed, 87 insertions(+)
> >
> > diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c
> > index 1295e61..acc6e84 100644
> > --- a/arch/arm/mach-davinci/board-da850-evm.c
> > +++ b/arch/arm/mach-davinci/board-da850-evm.c
> > @@ -29,6 +29,7 @@
> > #include <linux/regulator/machine.h>
> > #include <linux/regulator/tps6507x.h>
> > #include <linux/input/tps6507x-ts.h>
> > +#include <linux/platform_data/uio_pruss.h>
> > #include <linux/spi/spi.h>
> > #include <linux/spi/flash.h>
> > #include <linux/delay.h>
> > @@ -42,6 +43,7 @@
> > #include <mach/da8xx.h>
> > #include <linux/platform_data/mtd-davinci.h>
> > #include <mach/mux.h>
> > +#include <mach/sram.h>
> > #include <linux/platform_data/mtd-davinci-aemif.h>
> > #include <linux/platform_data/spi-davinci.h>
>
> I know you did not introduce the mess, but can you include a patch to
> fix the mixture of mach/ and linux/ includes here? mach/ includes should
> follow the linux/ includes.
Ok. Yeah, I agree it's a mess there.
> > @@ -1253,6 +1255,10 @@ static __init int da850_wl12xx_init(void)
> >
> > #endif /* CONFIG_DA850_WL12XX */
> >
> > +struct uio_pruss_pdata da8xx_pruss_uio_pdata = {
> > + .pintc_base = 0x4000,
> > +};
> > +
> > #define DA850EVM_SATA_REFCLKPN_RATE (100 * 1000 * 1000)
> >
> > static __init void da850_evm_init(void)
> > @@ -1339,6 +1345,12 @@ static __init void da850_evm_init(void)
> > pr_warning("da850_evm_init: lcdcntl mux setup failed: %d\n",
> > ret);
> >
> > + da8xx_pruss_uio_pdata.l3ram_pool = sram_get_gen_pool();
>
> I wonder if this platform data should still be called l3ram pool. L3RAM
> is OMAP terminology. May be just call it sram_pool? Also this patch
> should follow 6/6 to prevent build breakage.
I agree, I'll change to sram_pool. I went back and forth on this because
I found even the userspace PRU tools had it called L3...I think it's
wise just say sram and avoid confusion. I'll move the patch too.
> > + ret = da8xx_register_pruss_uio(&da8xx_pruss_uio_pdata);
> > + if (ret)
> > + pr_warning("pruss_uio initialization failed: %d\n",
> > + ret);
> > +
> > /* Handle board specific muxing for LCD here */
> > ret = davinci_cfg_reg_list(da850_evm_lcdc_pins);
> > if (ret)
> > diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
> > index d8d69de..7c01d31 100644
> > --- a/arch/arm/mach-davinci/da850.c
> > +++ b/arch/arm/mach-davinci/da850.c
>
> Can you separate out board and SoC changes into different patches?
Ok, yes.
> > @@ -212,6 +212,12 @@ static struct clk tptc2_clk = {
> > .flags = ALWAYS_ENABLED,
> > };
> >
> > +static struct clk pruss_clk = {
> > + .name = "pruss",
> > + .parent = &pll0_sysclk2,
> > + .lpsc = DA8XX_LPSC0_PRUSS,
> > +};
> > +
> > static struct clk uart0_clk = {
> > .name = "uart0",
> > .parent = &pll0_sysclk2,
> > @@ -378,6 +384,7 @@ static struct clk_lookup da850_clks[] = {
> > CLK(NULL, "tptc1", &tptc1_clk),
> > CLK(NULL, "tpcc1", &tpcc1_clk),
> > CLK(NULL, "tptc2", &tptc2_clk),
> > + CLK(NULL, "pruss", &pruss_clk),
>
> This is actually incorrect since we should use device name rather than
> con_id for matching the clock. If there is just one clock that the
> driver needs, connection id should be NULL. Looking at the driver now,
> the clk_get() call seems to pass a valid device pointer. So, I wonder
> how you are able to look up the clock even with a NULL device name.
Hrm, I'll look into this. This part was a not looked at closely, just a
quick copy/paste/modify and worked as tested with the PRU examples so
I didn't look further. I'll look again let you know why it actually works.
I'm guessing I got lucky here, and should be a simple fix.
> > CLK(NULL, "uart0", &uart0_clk),
> > CLK(NULL, "uart1", &uart1_clk),
> > CLK(NULL, "uart2", &uart2_clk),
> > diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> > index bd2f72b..7c2e0d2 100644
> > --- a/arch/arm/mach-davinci/devices-da8xx.c
> > +++ b/arch/arm/mach-davinci/devices-da8xx.c
> > @@ -518,6 +518,72 @@ void __init da8xx_register_mcasp(int id, struct snd_platform_data *pdata)
> > }
> > }
> >
> > +#define DA8XX_PRUSS_MEM_BASE 0x01C30000
>
> In this file all base addresses are added at the top of the file. The
> defines are sorted in ascending order of address. If that's broken, its
> all my fault, but please don't add to the breakage when adding this entry :)
Ok :) Will fix.
> > +
> > +static struct resource da8xx_pruss_resources[] = {
> > + {
> > + .start = DA8XX_PRUSS_MEM_BASE,
> > + .end = DA8XX_PRUSS_MEM_BASE + 0xFFFF,
> > + .flags = IORESOURCE_MEM,
> > + },
> > + {
> > + .start = IRQ_DA8XX_EVTOUT0,
> > + .end = IRQ_DA8XX_EVTOUT0,
> > + .flags = IORESOURCE_IRQ,
> > + },
> > + {
> > + .start = IRQ_DA8XX_EVTOUT1,
> > + .end = IRQ_DA8XX_EVTOUT1,
> > + .flags = IORESOURCE_IRQ,
> > + },
> > + {
> > + .start = IRQ_DA8XX_EVTOUT2,
> > + .end = IRQ_DA8XX_EVTOUT2,
> > + .flags = IORESOURCE_IRQ,
> > + },
> > + {
> > + .start = IRQ_DA8XX_EVTOUT3,
> > + .end = IRQ_DA8XX_EVTOUT3,
> > + .flags = IORESOURCE_IRQ,
> > + },
> > + {
> > + .start = IRQ_DA8XX_EVTOUT4,
> > + .end = IRQ_DA8XX_EVTOUT4,
> > + .flags = IORESOURCE_IRQ,
> > + },
> > + {
> > + .start = IRQ_DA8XX_EVTOUT5,
> > + .end = IRQ_DA8XX_EVTOUT5,
> > + .flags = IORESOURCE_IRQ,
> > + },
> > + {
> > + .start = IRQ_DA8XX_EVTOUT6,
> > + .end = IRQ_DA8XX_EVTOUT6,
> > + .flags = IORESOURCE_IRQ,
> > + },
> > + {
> > + .start = IRQ_DA8XX_EVTOUT7,
> > + .end = IRQ_DA8XX_EVTOUT7,
> > + .flags = IORESOURCE_IRQ,
> > + },
> > +};
> > +
> > +static struct platform_device da8xx_pruss_uio_dev = {
> > + .name = "pruss_uio",
> > + .id = -1,
> > + .num_resources = ARRAY_SIZE(da8xx_pruss_resources),
> > + .resource = da8xx_pruss_resources,
> > + .dev = {
> > + .coherent_dma_mask = 0xffffffff,
>
> DMA_BIT_MASK(32)?
Yeah, more copy/paste/modify stuff. I'll fix that.
-Matt
On Thu, Oct 04, 2012 at 06:24:56PM +0530, Sekhar Nori wrote:
> On 10/4/2012 6:12 PM, Matt Porter wrote:
> > On Thu, Oct 04, 2012 at 11:11:45AM +0200, Philipp Zabel wrote:
> >> Hi Matt,
> >>
> >> On 10/3/12, Matt Porter <[email protected]> wrote:
> >>> This series enables uio_pruss on DA850 and removes use of the
> >>> private SRAM API by the driver. The driver previously was not
> >>> enabled by any platform and the private SRAM API was accessing
> >>> an invalid SRAM bank.
> >>
> >> have you seen my SRAM patch series at https://lkml.org/lkml/2012/9/7/281
> >> "Add device tree support for on-chip SRAM" ?
> >
> > Yes.
> >
> >> I think the generic SRAM/genalloc driver (https://lkml.org/lkml/2012/9/7/282)
> >> could be useful to map the L3RAM on Davinci.
> >> With the gen_pool lookup patch (https://lkml.org/lkml/2012/9/7/284) the
> >> uio_pruss driver could then use the gen_pool_find_by_phys() (or
> >> of_get_named_gen_pool() for initialization from device tree) to
> >> retrieve the struct gen_pool*.
> >>
> >> This way you could avoid handing it over via platform data and you could
> >> get rid of arch/arm/mach-davinci/{sram.c,include/mach/sram.h} completely.
> >
> > I did miss the gen_pool_find_by_phys() call in that series. That does
> > look useful. I actually mentioned your series in an earlier posting
> > since I like it, but since the initialization of the driver was inherently
> > tied to DT it's not usable for DaVinci that's just starting to convert
> > to DT and needs !DT support as well.
> >
> > I do see it moving to your driver exclusively, but I wanted to make this
> > series focused on only getting rid of the private SRAM API using the
> > existing pdata framework that's already there. I think once
> > gen_pool_find_by_phys() goes upstream we can switch to that and get the
> > address from a resource in the !DT case. I guess we should see if Sekhar
> > would like to see this happen in two steps or just have us depend on
> > the gen_pool_find_by_phys() patch now.
>
> I prefer going with this series now and switching to the SRAM driver
> once it is available mainline.
Ok, thanks. In that case, I'll also plan to keep the davinci-pcm series
using the same approach for now too.
-Matt
On Thu, Oct 04, 2012 at 08:42:53AM -0400, Matt Porter wrote:
> > I think the generic SRAM/genalloc driver (https://lkml.org/lkml/2012/9/7/282)
> > could be useful to map the L3RAM on Davinci.
> > With the gen_pool lookup patch (https://lkml.org/lkml/2012/9/7/284) the
> > uio_pruss driver could then use the gen_pool_find_by_phys() (or
> > of_get_named_gen_pool() for initialization from device tree) to
> > retrieve the struct gen_pool*.
> >
> > This way you could avoid handing it over via platform data and you could
> > get rid of arch/arm/mach-davinci/{sram.c,include/mach/sram.h} completely.
>
> I did miss the gen_pool_find_by_phys() call in that series. That does
> look useful. I actually mentioned your series in an earlier posting
> since I like it,
That I did miss.
> but since the initialization of the driver was inherently
> tied to DT it's not usable for DaVinci that's just starting to convert
> to DT and needs !DT support as well.
There should be no dependency on DT in the sram driver. It just requests
and remaps the first given iomem resource and creates a gen_pool from that.
This should work just as well for the !DT case.
Maybe it's just my choice of patch series subject gave you that
impression? If there's a real issue for !DT, I should fix it.
> I do see it moving to your driver exclusively, but I wanted to make this
> series focused on only getting rid of the private SRAM API using the
> existing pdata framework that's already there. I think once
> gen_pool_find_by_phys() goes upstream we can switch to that and get the
> address from a resource in the !DT case. I guess we should see if Sekhar
> would like to see this happen in two steps or just have us depend on
> the gen_pool_find_by_phys() patch now.
Thanks, I'm glad you are aware of the sram driver and consider it useful.
> BTW, I was going to post a patch for your driver to allow
> configurability of the allocation order, but have been busy with other
> things. We'll eventually need that when switching to it as the
> hardcoded page size order isn't going to work for all cases.
Good point.
regards
Philipp
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Thu, Oct 04, 2012 at 03:35:53PM +0200, Philipp Zabel wrote:
> On Thu, Oct 04, 2012 at 08:42:53AM -0400, Matt Porter wrote:
> > > I think the generic SRAM/genalloc driver (https://lkml.org/lkml/2012/9/7/282)
> > > could be useful to map the L3RAM on Davinci.
> > > With the gen_pool lookup patch (https://lkml.org/lkml/2012/9/7/284) the
> > > uio_pruss driver could then use the gen_pool_find_by_phys() (or
> > > of_get_named_gen_pool() for initialization from device tree) to
> > > retrieve the struct gen_pool*.
> > >
> > > This way you could avoid handing it over via platform data and you could
> > > get rid of arch/arm/mach-davinci/{sram.c,include/mach/sram.h} completely.
> >
> > I did miss the gen_pool_find_by_phys() call in that series. That does
> > look useful. I actually mentioned your series in an earlier posting
> > since I like it,
>
> That I did miss.
>
> > but since the initialization of the driver was inherently
> > tied to DT it's not usable for DaVinci that's just starting to convert
> > to DT and needs !DT support as well.
>
> There should be no dependency on DT in the sram driver. It just requests
> and remaps the first given iomem resource and creates a gen_pool from that.
> This should work just as well for the !DT case.
> Maybe it's just my choice of patch series subject gave you that
> impression? If there's a real issue for !DT, I should fix it.
*sigh*, I see now. I looked at v2 and got wrapped up in the DT use case
and missed your platform device support. I think it will work just fine
for us to use in a "phase 2" of this work, replacing the backend of
davinci sram allocation with this as Sekhar seems to be open to.
> > I do see it moving to your driver exclusively, but I wanted to make this
> > series focused on only getting rid of the private SRAM API using the
> > existing pdata framework that's already there. I think once
> > gen_pool_find_by_phys() goes upstream we can switch to that and get the
> > address from a resource in the !DT case. I guess we should see if Sekhar
> > would like to see this happen in two steps or just have us depend on
> > the gen_pool_find_by_phys() patch now.
>
> Thanks, I'm glad you are aware of the sram driver and consider it useful.
>
> > BTW, I was going to post a patch for your driver to allow
> > configurability of the allocation order, but have been busy with other
> > things. We'll eventually need that when switching to it as the
> > hardcoded page size order isn't going to work for all cases.
>
> Good point.
I think this is the only blocker to DaVinci adopting it once it goes
upstream. I can add a patch in your driver thread if that helps.
-Matt
On Thu, Oct 04, 2012 at 09:54:33AM -0400, Matt Porter wrote:
> *sigh*, I see now. I looked at v2 and got wrapped up in the DT use case
> and missed your platform device support. I think it will work just fine
> for us to use in a "phase 2" of this work, replacing the backend of
> davinci sram allocation with this as Sekhar seems to be open to.
>
> > > I do see it moving to your driver exclusively, but I wanted to make this
> > > series focused on only getting rid of the private SRAM API using the
> > > existing pdata framework that's already there. I think once
> > > gen_pool_find_by_phys() goes upstream we can switch to that and get the
> > > address from a resource in the !DT case. I guess we should see if Sekhar
> > > would like to see this happen in two steps or just have us depend on
> > > the gen_pool_find_by_phys() patch now.
> >
> > Thanks, I'm glad you are aware of the sram driver and consider it useful.
> >
> > > BTW, I was going to post a patch for your driver to allow
> > > configurability of the allocation order, but have been busy with other
> > > things. We'll eventually need that when switching to it as the
> > > hardcoded page size order isn't going to work for all cases.
> >
> > Good point.
>
> I think this is the only blocker to DaVinci adopting it once it goes
> upstream. I can add a patch in your driver thread if that helps.
Yes, that would be welcome.
regards
Philipp
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Thu, Oct 04, 2012 at 05:52:45PM +0530, Sekhar Nori wrote:
> On 10/3/2012 8:25 PM, Matt Porter wrote:
> > +static struct clk pruss_clk = {
> > + .name = "pruss",
> > + .parent = &pll0_sysclk2,
> > + .lpsc = DA8XX_LPSC0_PRUSS,
> > +};
> > +
> > static struct clk uart0_clk = {
> > .name = "uart0",
> > .parent = &pll0_sysclk2,
> > @@ -378,6 +384,7 @@ static struct clk_lookup da850_clks[] = {
> > CLK(NULL, "tptc1", &tptc1_clk),
> > CLK(NULL, "tpcc1", &tpcc1_clk),
> > CLK(NULL, "tptc2", &tptc2_clk),
> > + CLK(NULL, "pruss", &pruss_clk),
>
> This is actually incorrect since we should use device name rather than
> con_id for matching the clock. If there is just one clock that the
> driver needs, connection id should be NULL. Looking at the driver now,
> the clk_get() call seems to pass a valid device pointer. So, I wonder
> how you are able to look up the clock even with a NULL device name.
I doublechecked the clk_get() find implentation to confirm that I indeed
did get lucky here. Since pruss has only one instance and the clk_find()
implementation looks for the best found match, it's able to find the
clock just by the con_id, though it's not the "best possible" match in
the find implementation...it's the "best found" match.. As you noted,
this is incorrect though for a clock expected to be used from a driver
context so I will address this in the update.
-Matt
On Thu, Oct 04, 2012 at 05:27:23PM +0530, Sekhar Nori wrote:
> Matt,
>
> On 10/3/2012 8:25 PM, Matt Porter wrote:
> > From: Subhasish Ghosh <[email protected]>
> >
> > This patch modifies the sram allocator to allocate memory
> > from the DA8XX shared RAM.
> >
> > Signed-off-by: Subhasish Ghosh <[email protected]>
> > [rebased onto consolidated SRAM patches]
> > Signed-off-by: Ben Gardiner <[email protected]>
> > [rebased to mainline as consolidated SRAM patches were dropped]
> > Signed-off-by: Matt Porter <[email protected]>
>
> Were you able to test PM with this change or you need my help?
I verified suspend/resume is working fine with this change on
AM180x EVM. Will note this all in v4.
-Matt
On 10/4/2012 10:05 PM, Matt Porter wrote:
> On Thu, Oct 04, 2012 at 05:52:45PM +0530, Sekhar Nori wrote:
>> On 10/3/2012 8:25 PM, Matt Porter wrote:
>>> +static struct clk pruss_clk = {
>>> + .name = "pruss",
>>> + .parent = &pll0_sysclk2,
>>> + .lpsc = DA8XX_LPSC0_PRUSS,
>>> +};
>>> +
>>> static struct clk uart0_clk = {
>>> .name = "uart0",
>>> .parent = &pll0_sysclk2,
>>> @@ -378,6 +384,7 @@ static struct clk_lookup da850_clks[] = {
>>> CLK(NULL, "tptc1", &tptc1_clk),
>>> CLK(NULL, "tpcc1", &tpcc1_clk),
>>> CLK(NULL, "tptc2", &tptc2_clk),
>>> + CLK(NULL, "pruss", &pruss_clk),
>>
>> This is actually incorrect since we should use device name rather than
>> con_id for matching the clock. If there is just one clock that the
>> driver needs, connection id should be NULL. Looking at the driver now,
>> the clk_get() call seems to pass a valid device pointer. So, I wonder
>> how you are able to look up the clock even with a NULL device name.
>
> I doublechecked the clk_get() find implentation to confirm that I indeed
> did get lucky here. Since pruss has only one instance and the clk_find()
> implementation looks for the best found match, it's able to find the
> clock just by the con_id, though it's not the "best possible" match in
> the find implementation...it's the "best found" match.. As you noted,
> this is incorrect though for a clock expected to be used from a driver
> context so I will address this in the update.
Thanks for debugging and clarifying.
Regards,
Sekhar