| Message ID | 1549290167-876-1-git-send-email-rogerq@ti.com |
|---|---|
| Headers | show |
| Series | Add support for TI PRU ICSS | expand |
On 2/4/19 8:22 AM, Roger Quadros wrote: > From: Suman Anna <s-anna@ti.com> > > The Programmable Real-Time Unit - Industrial Communication > Subsystem (PRU-ICSS) is present on various TI SoCs such as > AM335x or AM437x or the Keystone 66AK2G. Each SoC can have > one or more PRUSS instances that may or may not be identical. > For example, AM335x SoCs have a single PRUSS, while AM437x has > two PRUSS instances PRUSS1 and PRUSS0, with the PRUSS0 being > a cut-down version of the PRUSS1. > > The PRUSS consists of dual 32-bit RISC cores called the > Programmable Real-Time Units (PRUs), with data and > instruction memories. It also contains various sub-modules > like MDIO, MII_RT, UART, etc. Each sub-module will be driven > by it's own driver. > > This PRUSS platform driver deals with the overall PRUSS and is > used for managing the subsystem level resources like various > memories and common CFG module. It is responsible for the > creation and deletion of the platform devices for the child PRU > devices and the various sub-modules. > > This design provides flexibility in representing the different > modules of PRUSS accordingly, and at the same time allowing the > PRUSS driver to add some instance specific configuration within > an SoC. > > pruss_get() and pruss_put() APIs allow client drivers to request > the 'struct pruss) device handle from the 'struct rproc' handle ) -> ' > for the respective PRU. This handle will be used by client drivers > to request various operations of the PRUSS platform driver through > below APIs. > > pruss_request_mem_region() & pruss_release_mem_region() allow > client drivers to acquire and release the common memory resources > present within a PRU-ICSS subsystem. This allows the client drivers > to directly manipulate the respective memories, > as per their design contract with the associated firmware. > > pruss_cfg_read() and pruss_cfg_update() allow other drivers to read > and update the registers in the CFG submodule within the PRUSS. > This interface provides a simple way for client drivers > without having them to include and parse these syscon nodes within > their respective device nodes. > > pruss_cfg_miirt_enable() and pruss_cfg_xfr_enable() allow the > client drivers to set MII_RT event enable/disable and > XFR (XIN XOUT) enable/disable respectively. > > Signed-off-by: Suman Anna <s-anna@ti.com> > Signed-off-by: Keerthy <j-keerthy@ti.com> > Signed-off-by: Andrew F. Davis <afd@ti.com> > Signed-off-by: Tero Kristo <t-kristo@ti.com> > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > drivers/soc/ti/Kconfig | 12 ++ > drivers/soc/ti/Makefile | 1 + > drivers/soc/ti/pruss.c | 347 ++++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/pruss.h | 211 +++++++++++++++++++++++++++++ > 4 files changed, 571 insertions(+) > create mode 100644 drivers/soc/ti/pruss.c > create mode 100644 include/linux/pruss.h > > diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig > index be4570b..789f2a8 100644 > --- a/drivers/soc/ti/Kconfig > +++ b/drivers/soc/ti/Kconfig > @@ -73,4 +73,16 @@ config TI_SCI_PM_DOMAINS > called ti_sci_pm_domains. Note this is needed early in boot before > rootfs may be available. > > +config TI_PRUSS > + tristate "TI PRU-ICSS Subsystem Platform drivers" > + depends on SOC_AM33XX || SOC_AM43XX || SOC_DRA7XX > + select MFD_SYSCON > + default n > + help > + TI PRU-ICSS Subsystem platform specific support. > + > + Say Y or M here to support the Programmable Realtime Unit (PRU) > + processors on various TI SoCs. It's safe to say N here if you're > + not interested in the PRU or if you are unsure. > + > endif # SOC_TI > diff --git a/drivers/soc/ti/Makefile b/drivers/soc/ti/Makefile > index a22edc0..55b4b04 100644 > --- a/drivers/soc/ti/Makefile > +++ b/drivers/soc/ti/Makefile > @@ -8,3 +8,4 @@ obj-$(CONFIG_KEYSTONE_NAVIGATOR_DMA) += knav_dma.o > obj-$(CONFIG_AMX3_PM) += pm33xx.o > obj-$(CONFIG_WKUP_M3_IPC) += wkup_m3_ipc.o > obj-$(CONFIG_TI_SCI_PM_DOMAINS) += ti_sci_pm_domains.o > +obj-$(CONFIG_TI_PRUSS) += pruss.o > diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c > new file mode 100644 > index 0000000..c9493983 > --- /dev/null > +++ b/drivers/soc/ti/pruss.c > @@ -0,0 +1,347 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * PRU-ICSS platform driver for various TI SoCs > + * > + * Copyright (C) 2014-2019 Texas Instruments Incorporated - http://www.ti.com/ > + * Suman Anna <s-anna@ti.com> > + * Andrew F. Davis <afd@ti.com> > + */ > + > +#include <linux/dma-mapping.h> > +#include <linux/module.h> > +#include <linux/io.h> > +#include <linux/mfd/syscon.h> > +#include <linux/of_address.h> > +#include <linux/of_device.h> > +#include <linux/pruss.h> > +#include <linux/regmap.h> > +#include <linux/remoteproc.h> > + > +/** > + * struct pruss - PRUSS parent structure > + * @dev: pruss device pointer > + * @cfg: regmap for config region > + * @mem_regions: data for each of the PRUSS memory regions > + * @mem_in_use: to indicate if memory resource is in use > + * @no_shared_ram: indicate that shared RAM is absent > + * @lock: mutex to serialize access to resources > + */ > +struct pruss { > + struct device *dev; > + struct regmap *cfg; > + struct pruss_mem_region mem_regions[PRUSS_MEM_MAX]; > + struct pruss_mem_region *mem_in_use[PRUSS_MEM_MAX]; > + bool no_shared_ram; > + struct mutex lock; /* PRU resource lock */ > +}; > + > +/** > + * pruss_get() - get the pruss for a given PRU remoteproc > + * @rproc: remoteproc handle of a PRU instance > + * > + * Finds the parent pruss device for a PRU given the @rproc handle of the > + * PRU remote processor. This function increments the pruss device's refcount, > + * so always use pruss_put() to decrement it back once pruss isn't needed > + * anymore. > + * > + * Returns the pruss handle on success, and an ERR_PTR on failure using one > + * of the following error values > + * -EINVAL if invalid parameter > + * -ENODEV if PRU device or PRUSS device is not found > + */ > +struct pruss *pruss_get(struct rproc *rproc) > +{ > + struct pruss *pruss; > + struct device *dev; > + struct platform_device *ppdev; > + > + if (IS_ERR(rproc)) > + return ERR_PTR(-EINVAL); > + > + dev = &rproc->dev; > + if (!dev->parent) > + return ERR_PTR(-ENODEV); > + > + /* rudimentary check to make sure rproc handle is for a PRU */ > + if (!strstr(dev_name(dev->parent), "pru")) > + return ERR_PTR(-ENODEV); > + > + ppdev = to_platform_device(dev->parent->parent); > + pruss = platform_get_drvdata(ppdev); > + if (pruss) > + get_device(pruss->dev); > + > + return pruss ? pruss : ERR_PTR(-ENODEV); > +} > +EXPORT_SYMBOL_GPL(pruss_get); > + > +/** > + * pruss_put() - decrement pruss device's usecount > + * @pruss: pruss handle > + * > + * Complimentary function for pruss_get(). Needs to be called > + * after the PRUSS is used, and only if the pruss_get() succeeds. > + */ > +void pruss_put(struct pruss *pruss) > +{ > + if (IS_ERR(pruss)) > + return; > + > + put_device(pruss->dev); > +} > +EXPORT_SYMBOL_GPL(pruss_put); > + > +/** > + * pruss_request_mem_region() - request a memory resource > + * @pruss: the pruss instance > + * @mem_id: the memory resource id > + * @region: pointer to memory region structure to be filled in > + * > + * This function allows a client driver to request a memory resource, > + * and if successful, will let the client driver own the particular > + * memory region until released using the pruss_release_mem_region() > + * API. > + * > + * Returns the memory region if requested resource is available, an > + * error otherwise > + */ > +int pruss_request_mem_region(struct pruss *pruss, enum pruss_mem mem_id, > + struct pruss_mem_region *region) > +{ > + if (IS_ERR(pruss) || !region) > + return -EINVAL; > + > + if (mem_id >= PRUSS_MEM_MAX) > + return -EINVAL; > + > + mutex_lock(&pruss->lock); > + > + if (pruss->mem_in_use[mem_id]) { > + mutex_unlock(&pruss->lock); > + return -EBUSY; > + } > + > + *region = pruss->mem_regions[mem_id]; > + pruss->mem_in_use[mem_id] = region; > + > + mutex_unlock(&pruss->lock); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(pruss_request_mem_region); > + > +/** > + * pruss_release_mem_region() - release a memory resource > + * @pruss: the pruss instance > + * @region: the memory region to release > + * > + * This function is the complimentary function to > + * pruss_request_mem_region(), and allows the client drivers to > + * release back a memory resource. > + * > + * Returns 0 on success, an error code otherwise > + */ > +int pruss_release_mem_region(struct pruss *pruss, > + struct pruss_mem_region *region) > +{ > + int id; > + > + if (IS_ERR(pruss) || !region) > + return -EINVAL; > + > + mutex_lock(&pruss->lock); > + > + /* find out the memory region being released */ > + for (id = 0; id < PRUSS_MEM_MAX; id++) { > + if (pruss->mem_in_use[id] == region) > + break; > + } > + > + if (id == PRUSS_MEM_MAX) { > + mutex_unlock(&pruss->lock); > + return -EINVAL; > + } > + > + pruss->mem_in_use[id] = NULL; > + > + mutex_unlock(&pruss->lock); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(pruss_release_mem_region); > + > +/** > + * pruss_cfg_read() - read a PRUSS CFG register > + * @pruss: the pruss instance handle > + * @reg: register offset within the CFG sub-module > + * @val: pointer to return the value in > + * > + * Reads a given register within CFG module of PRUSS > + * and returns it through the passed-in @val pointer > + * > + * Returns 0 on success, or an error code otherwise > + */ > +int pruss_cfg_read(struct pruss *pruss, unsigned int reg, unsigned int *val) > +{ > + if (IS_ERR(pruss)) > + return -EINVAL; > + > + return regmap_read(pruss->cfg, reg, val); > +} > +EXPORT_SYMBOL_GPL(pruss_cfg_read); > + > +/** > + * pruss_cfg_update() - update a PRUSS CFG register > + * @pruss: the pruss instance handle > + * @reg: register offset within the CFG sub-module > + * @mask: bit mask to use for programming the @val > + * @val: value to write > + * > + * Updates a given register within CFG sub-module of PRUSS > + * > + * Returns 0 on success, or an error code otherwise > + */ > +int pruss_cfg_update(struct pruss *pruss, unsigned int reg, > + unsigned int mask, unsigned int val) > +{ > + if (IS_ERR(pruss)) > + return -EINVAL; > + > + return regmap_update_bits(pruss->cfg, reg, mask, val); > +} > +EXPORT_SYMBOL_GPL(pruss_cfg_update); > + > +/** > + * struct pruss_match_private_data - private data to handle multiple instances > + * @device_name: device name of the PRUSS instance > + * @priv_data: PRUSS driver private data for this PRUSS instance > + */ > +struct pruss_match_private_data { > + const char *device_name; > + const struct pruss_private_data *priv_data; > +}; > + > +static const > +struct pruss_private_data *pruss_get_private_data(struct platform_device *pdev) > +{ > + const struct pruss_match_private_data *data; > + > + if (!of_device_is_compatible(pdev->dev.of_node, "ti,am4376-pruss")) > + return NULL; Been a while since I worked with all this, so refresh my memory, this was only so we could pull in the "shared RAM only on one PRUSS instance on am4376" quirk, right? If so it looks like this is now done with a DT flag. All this private_data stuff can now be dropped. > + > + data = of_device_get_match_data(&pdev->dev); > + for (; data && data->device_name; data++) { > + if (!strcmp(dev_name(&pdev->dev), data->device_name)) > + return data->priv_data; > + } > + > + return ERR_PTR(-ENODEV); > +} > + > +static int pruss_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *node = dev->of_node; > + struct device_node *np; > + struct pruss *pruss; > + struct resource *res; > + int ret, i; > + const struct pruss_private_data *data; > + const char *mem_names[PRUSS_MEM_MAX] = { "dram0", "dram1", "shrdram2" }; > + > + if (!node) { > + dev_err(dev, "Non-DT platform device not supported\n"); > + return -ENODEV; > + } > + > + data = pruss_get_private_data(pdev); > + if (IS_ERR(data)) { > + dev_err(dev, "missing private data\n"); > + return -ENODEV; > + } Above gets dropped. > + > + ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32)); > + if (ret) { > + dev_err(dev, "dma_set_coherent_mask: %d\n", ret); > + return ret; > + } > + > + pruss = devm_kzalloc(dev, sizeof(*pruss), GFP_KERNEL); > + if (!pruss) > + return -ENOMEM; > + > + pruss->dev = dev; > + mutex_init(&pruss->lock); > + > + pruss->no_shared_ram = of_property_read_bool(node, "no-shared-ram"); > + > + np = of_get_child_by_name(node, "cfg"); > + if (!np) > + return -ENODEV; > + > + pruss->cfg = syscon_node_to_regmap(np); > + of_node_put(np); > + if (IS_ERR(pruss->cfg)) > + return -ENODEV; > + > + for (i = 0; i < ARRAY_SIZE(mem_names); i++) { > + if (pruss->no_shared_ram && !strcmp(mem_names[i], "shrdram2")) > + continue; > + > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, > + mem_names[i]); > + pruss->mem_regions[i].va = devm_ioremap_resource(dev, res); > + if (!pruss->mem_regions[i].va) { > + dev_err(dev, "failed to get resource: %s\n", > + mem_names[i]); > + return -ENODEV; > + } > + pruss->mem_regions[i].pa = res->start; > + pruss->mem_regions[i].size = resource_size(res); > + > + dev_dbg(dev, "memory %8s: pa %pa size 0x%zx va %p\n", > + mem_names[i], &pruss->mem_regions[i].pa, > + pruss->mem_regions[i].size, pruss->mem_regions[i].va); > + } > + > + platform_set_drvdata(pdev, pruss); > + > + dev_info(&pdev->dev, "creating PRU cores and other child platform devices\n"); > + ret = of_platform_populate(node, NULL, NULL, &pdev->dev); > + if (ret) > + dev_err(dev, "of_platform_populate failed\n"); > + > + return ret; > +} > + > +static int pruss_remove(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + > + dev_info(dev, "remove PRU cores and other child platform devices\n"); > + of_platform_depopulate(dev); > + > + return 0; > +} > + > +static const struct of_device_id pruss_of_match[] = { > + { .compatible = "ti,am3356-pruss", }, > + { .compatible = "ti,am4376-pruss", }, > + { .compatible = "ti,am5728-pruss", }, ti,k2g-pruss ? Andrew > + { /* sentinel */ }, > +}; > +MODULE_DEVICE_TABLE(of, pruss_of_match); > + > +static struct platform_driver pruss_driver = { > + .driver = { > + .name = "pruss", > + .of_match_table = pruss_of_match, > + }, > + .probe = pruss_probe, > + .remove = pruss_remove, > +}; > +module_platform_driver(pruss_driver); > + > +MODULE_AUTHOR("Suman Anna <s-anna@ti.com>"); > +MODULE_DESCRIPTION("PRU-ICSS Subsystem Driver"); > +MODULE_LICENSE("GPL v2"); > diff --git a/include/linux/pruss.h b/include/linux/pruss.h > new file mode 100644 > index 0000000..b236b30 > --- /dev/null > +++ b/include/linux/pruss.h > @@ -0,0 +1,211 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/** > + * PRU-ICSS Subsystem user interfaces > + * > + * Copyright (C) 2015-2019 Texas Instruments Incorporated - http://www.ti.com > + * Suman Anna <s-anna@ti.com> > + * Tero Kristo <t-kristo@ti.com> > + */ > + > +#ifndef __LINUX_PRUSS_H > +#define __LINUX_PRUSS_H > + > +/* > + * PRU_ICSS_CFG registers > + * SYSCFG, ISRP, ISP, IESP, IECP, SCRP applicable on AMxxxx devices only > + */ > +#define PRUSS_CFG_REVID 0x00 > +#define PRUSS_CFG_SYSCFG 0x04 > +#define PRUSS_CFG_GPCFG(x) (0x08 + (x) * 4) > +#define PRUSS_CFG_CGR 0x10 > +#define PRUSS_CFG_ISRP 0x14 > +#define PRUSS_CFG_ISP 0x18 > +#define PRUSS_CFG_IESP 0x1C > +#define PRUSS_CFG_IECP 0x20 > +#define PRUSS_CFG_SCRP 0x24 > +#define PRUSS_CFG_PMAO 0x28 > +#define PRUSS_CFG_MII_RT 0x2C > +#define PRUSS_CFG_IEPCLK 0x30 > +#define PRUSS_CFG_SPP 0x34 > +#define PRUSS_CFG_PIN_MX 0x40 > + > +/* PRUSS_GPCFG register bits */ > +#define PRUSS_GPCFG_PRU_GPO_SH_SEL BIT(25) > + > +#define PRUSS_GPCFG_PRU_DIV1_SHIFT 20 > +#define PRUSS_GPCFG_PRU_DIV1_MASK GENMASK(24, 20) > + > +#define PRUSS_GPCFG_PRU_DIV0_SHIFT 15 > +#define PRUSS_GPCFG_PRU_DIV0_MASK GENMASK(15, 19) > + > +#define PRUSS_GPCFG_PRU_GPO_MODE BIT(14) > +#define PRUSS_GPCFG_PRU_GPO_MODE_DIRECT 0 > +#define PRUSS_GPCFG_PRU_GPO_MODE_SERIAL BIT(14) > + > +#define PRUSS_GPCFG_PRU_GPI_SB BIT(13) > + > +#define PRUSS_GPCFG_PRU_GPI_DIV1_SHIFT 8 > +#define PRUSS_GPCFG_PRU_GPI_DIV1_MASK GENMASK(12, 8) > + > +#define PRUSS_GPCFG_PRU_GPI_DIV0_SHIFT 3 > +#define PRUSS_GPCFG_PRU_GPI_DIV0_MASK GENMASK(7, 3) > + > +#define PRUSS_GPCFG_PRU_GPI_CLK_MODE_POSITIVE 0 > +#define PRUSS_GPCFG_PRU_GPI_CLK_MODE_NEGATIVE BIT(2) > +#define PRUSS_GPCFG_PRU_GPI_CLK_MODE BIT(2) > + > +#define PRUSS_GPCFG_PRU_GPI_MODE_MASK GENMASK(1, 0) > +#define PRUSS_GPCFG_PRU_GPI_MODE_SHIFT 0 > + > +#define PRUSS_GPCFG_PRU_MUX_SEL_SHIFT 26 > +#define PRUSS_GPCFG_PRU_MUX_SEL_MASK GENMASK(29, 26) > + > +/* PRUSS_MII_RT register bits */ > +#define PRUSS_MII_RT_EVENT_EN BIT(0) > + > +/* PRUSS_SPP register bits */ > +#define PRUSS_SPP_XFER_SHIFT_EN BIT(1) > +#define PRUSS_SPP_PRU1_PAD_HP_EN BIT(0) > + > +/** > + * enum pruss_gp_mux_sel - PRUSS GPI/O Mux modes for the > + * PRUSS_GPCFG0/1 registers > + * > + * NOTE: The below defines are the most common values, but there > + * are some exceptions like on 66AK2G, where the RESERVED and MII2 > + * values are interchanged. Also, this bit-field does not exist on > + * AM335x SoCs > + */ > +enum pruss_gp_mux_sel { > + PRUSS_GP_MUX_SEL_GP = 0, > + PRUSS_GP_MUX_SEL_ENDAT, > + PRUSS_GP_MUX_SEL_RESERVED, > + PRUSS_GP_MUX_SEL_SD, > + PRUSS_GP_MUX_SEL_MII2, > + PRUSS_GP_MUX_SEL_MAX, > +}; > + > +/** > + * enum pruss_gpi_mode - PRUSS GPI configuration modes, used > + * to program the PRUSS_GPCFG0/1 registers > + */ > +enum pruss_gpi_mode { > + PRUSS_GPI_MODE_DIRECT = 0, > + PRUSS_GPI_MODE_PARALLEL, > + PRUSS_GPI_MODE_28BIT_SHIFT, > + PRUSS_GPI_MODE_MII, > +}; > + > +/** > + * enum pruss_mem - PRUSS memory range identifiers > + */ > +enum pruss_mem { > + PRUSS_MEM_DRAM0 = 0, > + PRUSS_MEM_DRAM1, > + PRUSS_MEM_SHRD_RAM2, > + PRUSS_MEM_MAX, > +}; > + > +/** > + * struct pruss_mem_region - PRUSS memory region structure > + * @va: kernel virtual address of the PRUSS memory region > + * @pa: physical (bus) address of the PRUSS memory region > + * @size: size of the PRUSS memory region > + */ > +struct pruss_mem_region { > + void __iomem *va; > + phys_addr_t pa; > + size_t size; > +}; > + > +struct pruss; > +struct rproc; > + > +#if IS_ENABLED(CONFIG_TI_PRUSS) > + > +struct pruss *pruss_get(struct rproc *rproc); > +void pruss_put(struct pruss *pruss); > + > +int pruss_request_mem_region(struct pruss *pruss, enum pruss_mem mem_id, > + struct pruss_mem_region *region); > +int pruss_release_mem_region(struct pruss *pruss, > + struct pruss_mem_region *region); > + > +int pruss_cfg_read(struct pruss *pruss, unsigned int reg, unsigned int *val); > +int pruss_cfg_update(struct pruss *pruss, unsigned int reg, > + unsigned int mask, unsigned int val); > + > +/** > + * pruss_cfg_miirt_enable() - Enable/disable MII RT Events > + * @pruss: the pruss instance > + * @enable: enable/disable > + * > + * Enable/disable the MII RT Events for the PRUSS. > + */ > +static inline int pruss_cfg_miirt_enable(struct pruss *pruss, bool enable) > +{ > + u32 set = enable ? PRUSS_MII_RT_EVENT_EN : 0; > + > + return pruss_cfg_update(pruss, PRUSS_CFG_MII_RT, > + PRUSS_MII_RT_EVENT_EN, set); > +} > + > +/** > + * pruss_cfg_xfr_enable() - Enable/disable XIN XOUT shift functionality > + * @pruss: the pruss instance > + * @enable: enable/disable > + */ > +static inline int pruss_cfg_xfr_enable(struct pruss *pruss, bool enable) > +{ > + u32 set = enable ? PRUSS_SPP_XFER_SHIFT_EN : 0; > + > + return pruss_cfg_update(pruss, PRUSS_CFG_SPP, > + PRUSS_SPP_XFER_SHIFT_EN, set); > +} > +#else > + > +static inline struct pruss *pruss_get(struct rproc *rproc) > +{ > + return ERR_PTR(-ENOTSUPP); > +} > + > +static inline void pruss_put(struct pruss *pruss) { } > + > +static inline int pruss_request_mem_region(struct pruss *pruss, > + enum pruss_mem mem_id, > + struct pruss_mem_region *region) > +{ > + return -ENOTSUPP; > +} > + > +static inline int pruss_release_mem_region(struct pruss *pruss, > + struct pruss_mem_region *region) > +{ > + return -ENOTSUPP; > +} > + > +static inline int pruss_cfg_read(struct pruss *pruss, unsigned int reg, > + unsigned int *val) > +{ > + return -ENOTSUPP; > +} > + > +static inline int pruss_cfg_update(struct pruss *pruss, unsigned int reg, > + unsigned int mask, unsigned int val) > +{ > + return -ENOTSUPP; > +} > + > +static inline int pruss_cfg_miirt_enable(struct pruss *pruss, bool enable) > +{ > + return -ENOTSUPP; > +} > + > +static inline int pruss_cfg_xfr_enable(struct pruss *pruss, bool enable) > +{ > + return -ENOTSUPP; > +} > + > +#endif /* CONFIG_TI_PRUSS */ > + > +#endif /* __LINUX_PRUSS_H */ >
On 2/4/19 8:22 AM, Roger Quadros wrote: > From: Jason Reeder <jreeder@ti.com> > [...] > +/* .name matches on RPMsg Channels and causes a probe */ > +static const struct rpmsg_device_id rpmsg_driver_pru_id_table[] = { > + { .name = "rpmsg-pru" }, > + { }, > +}; > +MODULE_DEVICE_TABLE(rpmsg, rpmsg_driver_pru_id_table); > + > +static struct rpmsg_driver rpmsg_pru_driver = { > + .drv.name = KBUILD_MODNAME, > + .id_table = rpmsg_driver_pru_id_table, > + .probe = rpmsg_pru_probe, > + .callback = rpmsg_pru_cb, > + .remove = rpmsg_pru_remove, > +}; > + > +static int __init rpmsg_pru_init(void) > +{ > + int ret; > + > + rpmsg_pru_class = class_create(THIS_MODULE, "rpmsg_pru"); > + if (IS_ERR(rpmsg_pru_class)) { > + pr_err("Unable to create class\n"); > + ret = PTR_ERR(rpmsg_pru_class); > + goto fail_create_class; > + } > + > + ret = alloc_chrdev_region(&rpmsg_pru_devt, 0, PRU_MAX_DEVICES, > + "rpmsg_pru"); > + if (ret) { > + pr_err("Unable to allocate chrdev region\n"); > + goto fail_alloc_region; > + } > + > + ret = register_rpmsg_driver(&rpmsg_pru_driver); > + if (ret) { > + pr_err("Unable to register rpmsg driver"); > + goto fail_register_rpmsg_driver; > + } > + > + return 0; > + > +fail_register_rpmsg_driver: > + unregister_chrdev_region(rpmsg_pru_devt, PRU_MAX_DEVICES); > +fail_alloc_region: > + class_destroy(rpmsg_pru_class); > +fail_create_class: > + return ret; > +} > + > +static void __exit rpmsg_pru_exit(void) > +{ > + unregister_rpmsg_driver(&rpmsg_pru_driver); > + idr_destroy(&rpmsg_pru_minors); > + mutex_destroy(&rpmsg_pru_lock); > + class_destroy(rpmsg_pru_class); > + unregister_chrdev_region(rpmsg_pru_devt, PRU_MAX_DEVICES); > +} > + > +module_init(rpmsg_pru_init); > +module_exit(rpmsg_pru_exit); > + > +MODULE_AUTHOR("Jason Reeder <jreeder@ti.com>"); > +MODULE_ALIAS("rpmsg:rpmsg-pru"); MODULE_ALIAS not needed, MODULE_DEVICE_TABLE will generate the alias. Andrew > +MODULE_DESCRIPTION("PRU Remote Processor Messaging Driver"); > +MODULE_LICENSE("GPL v2"); >
On 04/02/19 17:11, Andrew F. Davis wrote: > On 2/4/19 8:22 AM, Roger Quadros wrote: >> From: "Andrew F. Davis" <afd@ti.com> >> > > [...] > >> +static const struct pruss_intc_match_data am437x_pruss_intc_data = { >> + .no_host7_intr = true, > > Like done for the PRUSS driver with 'has_no_sharedram' becoming a DT > flag the same could be done here, then all this match data stuff could > be dropped. Agreed. > >> +}; >> + >> +static const struct of_device_id pruss_intc_of_match[] = { >> + { >> + .compatible = "ti,am3356-pruss-intc", >> + .data = NULL, >> + }, >> + { >> + .compatible = "ti,am4376-pruss-intc", >> + .data = &am437x_pruss_intc_data, >> + }, >> + { >> + .compatible = "ti,am5728-pruss-intc", >> + .data = NULL, >> + }, >> + { /* sentinel */ }, >> +}; >> +MODULE_DEVICE_TABLE(of, pruss_intc_of_match); >> + >> +static struct platform_driver pruss_intc_driver = { >> + .driver = { >> + .name = "pruss-intc", >> + .of_match_table = pruss_intc_of_match, >> + }, >> + .probe = pruss_intc_probe, >> + .remove = pruss_intc_remove, >> +}; >> +module_platform_driver(pruss_intc_driver); >> + >> +MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>"); >> +MODULE_AUTHOR("Suman Anna <s-anna@ti.com>"); >> +MODULE_DESCRIPTION("PRU-ICSS INTC Driver"); >> +MODULE_LICENSE("GPL v2"); >> diff --git a/include/linux/irqchip/irq-pruss-intc.h b/include/linux/irqchip/irq-pruss-intc.h >> new file mode 100644 >> index 0000000..4538a0b >> --- /dev/null >> +++ b/include/linux/irqchip/irq-pruss-intc.h >> @@ -0,0 +1,94 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/** >> + * irq-pruss-intc.h - PRU-ICSS INTC management > > Filename not needed. OK. cheers, -roger > > Andrew > >> + * >> + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com >> + */ >> + >> +#ifndef __INCLUDE_LINUX_IRQCHIP_IRQ_PRUSS_INTC_H >> +#define __INCLUDE_LINUX_IRQCHIP_IRQ_PRUSS_INTC_H >> + >> +/* maximum number of system events */ >> +#define MAX_PRU_SYS_EVENTS 64 >> + >> +/* maximum number of interrupt channels */ >> +#define MAX_PRU_CHANNELS 10 >> + >> +/** >> + * struct pruss_intc_config - INTC configuration info >> + * @sysev_to_ch: system events to channel mapping information >> + * @ch_to_host: interrupt channel to host interrupt information >> + */ >> +struct pruss_intc_config { >> + s8 sysev_to_ch[MAX_PRU_SYS_EVENTS]; >> + s8 ch_to_host[MAX_PRU_CHANNELS]; >> +}; >> + >> +#if IS_ENABLED(CONFIG_TI_PRUSS) >> + >> +/** >> + * pruss_intc_configure() - configure the PRUSS INTC >> + * @dev: device >> + * @intc_config: PRU core-specific INTC configuration >> + * >> + * Configures the PRUSS INTC with the provided configuration from >> + * a PRU core. Any existing event to channel mappings or channel to >> + * host interrupt mappings are checked to make sure there are no >> + * conflicting configuration between both the PRU cores. The function >> + * is intended to be used only by the PRU remoteproc driver. >> + * >> + * Returns 0 on success, or a suitable error code otherwise >> + */ >> +int pruss_intc_configure(struct device *dev, >> + struct pruss_intc_config *intc_config); >> + >> +/** >> + * pruss_intc_unconfigure() - unconfigure the PRUSS INTC >> + * @dev: device >> + * @intc_config: PRU core specific INTC configuration >> + * >> + * Undo whatever was done in pruss_intc_configure() for a PRU core. >> + * It should be sufficient to just mark the resources free in the >> + * global map and disable the host interrupts and sysevents. >> + */ >> +int pruss_intc_unconfigure(struct device *dev, >> + struct pruss_intc_config *intc_config); >> +/** >> + * pruss_intc_trigger() - trigger a PRU system event >> + * @irq: linux IRQ number associated with a PRU system event >> + * >> + * Trigger an interrupt by signalling a specific PRU system event. >> + * This can be used by PRUSS client users to raise/send an event to >> + * a PRU or any other core that is listening on the host interrupt >> + * mapped to that specific PRU system event. The @irq variable is the >> + * Linux IRQ number associated with a specific PRU system event that >> + * a client user/application uses. The interrupt mappings for this is >> + * provided by the PRUSS INTC irqchip instance. >> + * >> + * Returns 0 on success, or an error value upon failure. >> + */ >> +int pruss_intc_trigger(unsigned int irq); >> + >> +#else >> + >> +static inline int pruss_intc_configure(struct device *dev, >> + struct pruss_intc_config *intc_config) >> +{ >> + return -ENOTSUPP; >> +} >> + >> +static inline int pruss_intc_unconfigure(struct device *dev, >> + struct pruss_intc_config *intc_config) >> +{ >> + return -ENOTSUPP; >> +} >> + >> +static inline int pruss_intc_trigger(unsigned int irq) >> +{ >> + return -ENOTSUPP; >> +} >> + >> +#endif /* CONFIG_TI_PRUSS */ >> + >> +#endif /* __INCLUDE_LINUX_IRQCHIP_IRQ_OMAP_INTC_H */ >> + >> -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Hi, * Roger Quadros <rogerq@ti.com> [190204 14:23]: > From: Suman Anna <s-anna@ti.com> ... > +Example: > +======== > +1. /* AM33xx PRU-ICSS */ > + > + pruss: pruss@0 { > + compatible = "ti,am3356-pruss"; > + reg = <0x0 0x2000>, > + <0x2000 0x2000>, > + <0x10000 0x3000>; > + reg-names = "dram0", "dram1", > + "shrdram2"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; Thanks for fixing up the reg ranges for the top level node. Ideally there would not even be a top level node here as AFAIK the whole PRUSS is a collection of devices on a PRU internal interconnect. So following that path a bit further.. How about just get rid of the top level node and just do: pruss: pruss@0 { dram0: memory@0 { device_type = "memory"; reg = <0x0 0x2000>; }; dram1: memory@2000 { device_type = "memory"; reg = <0x2000 0x2000>; }; shrdram2: memory@10000 { device_type = "memory"; reg = <0x10000 0x3000>; }; pruss_cfg: cfg@26000 { ... }; ... }; If the device_type = "memory" cannot be used here for being specific to the top level properties, then there's probably some other generic property usable here :) > + pruss_mii_rt: mii_rt@32000 { > + reg = <0x32000 0x58>; > + }; The node name should not have underscores so pruss_mii_rt: mii-rt@32000. Please check the others too, like app_node. > + app_node: app_node { > + prus = <&pru0>, <&pru1>; > + firmware-name = "pruss-app-fw", "pruss-app-fw-2"; > + ti,pruss-gp-mux-sel = <2>, <1>; > + /* setup interrupts for prus: > + prus[0] => pru1_0: ev=16, chnl=2, host-irq=7, > + prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */ > + ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>; > + } If the ti,pruss-gp-mux-sel and ti,pru-interrupt-map are firmware configuration options, maybe leave them out of the dts completely and make the app-node optional. And have a proper compatible for this node such as "ti,pruss-app-xyz". And this should be only set if the the hardware is wired up in such way that things need to be configured in the dts rather than by the firmware. And then you can just hide mux-sel and interrupt-map behind the compatible property for that hardware. And leave them out from the dts and have the handling driver would set mux-sel and interrupt-map based on the match->data during probe. Regards, Tony
* Roger Quadros <rogerq@ti.com> [190204 14:23]: > From: "Andrew F. Davis" <afd@ti.com> > > The Programmable Real-Time Unit Subsystem (PRUSS) contains an > interrupt controller (INTC) that can handle various system input > events and post interrupts back to the device-level initiators. > The INTC can support upto 64 input events with individual control > configuration and hardware prioritization. These events are mapped > onto 10 interrupt signals through two levels of many-to-one mapping > support. Different interrupt signals are routed to the individual > PRU cores or to the host CPU. > > The PRUSS INTC platform driver manages this PRUSS interrupt > controller and implements an irqchip driver to provide a Linux > standard way for the PRU client users to enable/disable/ack/ > re-trigger a PRUSS system event. The system events to interrupt > channels and host interrupts relies on the mapping configuration > provided through a firmware resource table for now. This will be > revisited and enhanced in the future for a better interface. The > mappings will currently be programmed during the boot/shutdown > of the PRU. > > The PRUSS INTC module is reference counted during the interrupt > setup phase through the irqchip's irq_request_resources() and > irq_release_resources() ops. This restricts the module from being > removed as long as there are active interrupt users. > > The PRUSS INTC can generate an interrupt to various processor > subsystems on the SoC through a set of 64 possible PRU system > events. These system events can be used by PRU client drivers > or applications for event notifications/signalling between PRUs > and MPU or other processors. An API, pruss_intc_trigger() is > provided to MPU-side PRU client drivers/applications to be able > to trigger an event/interrupt using IRQ numbers provided by the > PRUSS-INTC irqdomain chip. I suggest you send the binding patch and the interrupt controller driver separately to the irqchip guys. Maybe put the trigger function in to a separate patch that can be reviewed and applied separately. Regards, Tony
Hi Tony & Suman, On 04/02/19 18:33, Tony Lindgren wrote: > Hi, > > * Roger Quadros <rogerq@ti.com> [190204 14:23]: >> From: Suman Anna <s-anna@ti.com> > ... >> +Example: >> +======== >> +1. /* AM33xx PRU-ICSS */ >> + >> + pruss: pruss@0 { >> + compatible = "ti,am3356-pruss"; >> + reg = <0x0 0x2000>, >> + <0x2000 0x2000>, >> + <0x10000 0x3000>; >> + reg-names = "dram0", "dram1", >> + "shrdram2"; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + ranges; > > Thanks for fixing up the reg ranges for the top level node. > > Ideally there would not even be a top level node here as > AFAIK the whole PRUSS is a collection of devices on a PRU > internal interconnect. So following that path a bit further.. > How about just get rid of the top level node and just do: > > pruss: pruss@0 { > dram0: memory@0 { > device_type = "memory"; > reg = <0x0 0x2000>; > }; > > dram1: memory@2000 { > device_type = "memory"; > reg = <0x2000 0x2000>; > }; Actually dram0 and dram1 are data memories for PRU0 and PRU1 respectively. Isn't it better if they are moved to the pru node? e.g. pru0: pru@34000 { compatible = "ti,am3356-pru"; reg = <0x34000 0x2000>, <0x22000 0x400>, <0x22400 0x100>, <0x0 0x2000>; reg-names = "iram", "control", "debug", "dram"; ... }; pru1: pru@38000 { compatible = "ti,am3356-pru"; reg = <0x38000 0x2000>, <0x24000 0x400>, <0x24400 0x100>, <0x2000 0x2000>; reg-names = "iram", "control", "debug", "dram"; ... }; I think it is better to place a restriction that firmware on PRU0 cannot use data memory of PRU1 and vice versa. Application drivers do sometimes need to read/write to data memory. The pru_rproc driver could provide a API for the application drivers to get virtual address of the respective PRU's data memory. > > shrdram2: memory@10000 { > device_type = "memory"; > reg = <0x10000 0x3000>; > }; Shared RAM is not so straight forward. Both PRU firmwares and both application drivers might need to read/write here. The area split is decided by firmware design and there is no hardware protection to prevent from stomping on each others toes. We need a carveout based memory allocator at least I think that can do a allocate(base_offset, size); into shared RAM. This could be used by pru_rproc driver at firmware load time and by application drivers at initialization time. Thoughts? > > pruss_cfg: cfg@26000 { > ... > }; > ... > }; > > If the device_type = "memory" cannot be used here for > being specific to the top level properties, then > there's probably some other generic property usable > here :) > >> + pruss_mii_rt: mii_rt@32000 { >> + reg = <0x32000 0x58>; >> + }; > > The node name should not have underscores so > pruss_mii_rt: mii-rt@32000. Please check the others > too, like app_node. > OK. >> + app_node: app_node { >> + prus = <&pru0>, <&pru1>; >> + firmware-name = "pruss-app-fw", "pruss-app-fw-2"; >> + ti,pruss-gp-mux-sel = <2>, <1>; >> + /* setup interrupts for prus: >> + prus[0] => pru1_0: ev=16, chnl=2, host-irq=7, >> + prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */ >> + ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>; >> + } > > If the ti,pruss-gp-mux-sel and ti,pru-interrupt-map are > firmware configuration options, maybe leave them out of > the dts completely and make the app-node optional. Yes the app-node is optional. I will mention it. No, ti,pruss-gp-mux-sel and ti,pru-interrupt-map are not firmware options. But these settings are application/firmware specific. ti,pru-interrupt-map specifies the configuration to be used for the INTC interrupt controller. ti,pruss-gp-mux-sel is used to configure this register. "Table 30-20. PRUSS_GPCFG0" in http://www.tij.co.jp/jp/lit/ug/spruhz7h/spruhz7h.pdf "29:26 PR1_PRU0_GP_MUX_SEL" It configures how the pins from the PRUSS module are routed internally to the various modules. see "30.2.1 PRU-ICSS I/O Interface" and "Table 30-1. PRU-ICSS1 I/O Signals" > > And have a proper compatible for this node such as > "ti,pruss-app-xyz". And this should be only set if the the > hardware is wired up in such way that things need to be > configured in the dts rather than by the firmware. Yes, compatible is a required property as we need to load the appropriate application (kernel space) driver for it. I will fix the example. > > And then you can just hide mux-sel and interrupt-map > behind the compatible property for that hardware. And > leave them out from the dts and have the handling driver > would set mux-sel and interrupt-map based on the > match->data during probe. To summarize: I'll mark the app node as optional. Only required if a kernel driver is required for the application. Compatible is mandatory for app node. ti,pruss-gp-mux-sel and ti,pru-interrupt-map are optional for app node. cheers, -roger -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Hi Roger, On 02/05/2019 04:39 AM, Roger Quadros wrote: > Hi Tony & Suman, > > On 04/02/19 18:33, Tony Lindgren wrote: >> Hi, >> >> * Roger Quadros <rogerq@ti.com> [190204 14:23]: >>> From: Suman Anna <s-anna@ti.com> >> ... >>> +Example: >>> +======== >>> +1. /* AM33xx PRU-ICSS */ >>> + >>> + pruss: pruss@0 { >>> + compatible = "ti,am3356-pruss"; >>> + reg = <0x0 0x2000>, >>> + <0x2000 0x2000>, >>> + <0x10000 0x3000>; >>> + reg-names = "dram0", "dram1", >>> + "shrdram2"; >>> + #address-cells = <1>; >>> + #size-cells = <1>; >>> + ranges; >> >> Thanks for fixing up the reg ranges for the top level node. >> >> Ideally there would not even be a top level node here as >> AFAIK the whole PRUSS is a collection of devices on a PRU >> internal interconnect. So following that path a bit further.. >> How about just get rid of the top level node and just do: >> >> pruss: pruss@0 { >> dram0: memory@0 { >> device_type = "memory"; >> reg = <0x0 0x2000>; >> }; >> >> dram1: memory@2000 { >> device_type = "memory"; >> reg = <0x2000 0x2000>; >> }; > > Actually dram0 and dram1 are data memories for PRU0 and PRU1 respectively. > Isn't it better if they are moved to the pru node? > e.g. > > pru0: pru@34000 { > compatible = "ti,am3356-pru"; > reg = <0x34000 0x2000>, > <0x22000 0x400>, > <0x22400 0x100>, > <0x0 0x2000>; > reg-names = "iram", "control", "debug", "dram"; > ... > }; > > pru1: pru@38000 { > compatible = "ti,am3356-pru"; > reg = <0x38000 0x2000>, > <0x24000 0x400>, > <0x24400 0x100>, > <0x2000 0x2000>; > reg-names = "iram", "control", "debug", "dram"; > ... > }; > > I think it is better to place a restriction that firmware on PRU0 cannot use data > memory of PRU1 and vice versa. > That will not work as there are switch firmware cases where PRU access DRAM of other PRU and is a valid case to support in the future. So let us not do that. Murali > Application drivers do sometimes need to read/write to data memory. The pru_rproc > driver could provide a API for the application drivers to get virtual address of > the respective PRU's data memory. > >> >> shrdram2: memory@10000 { >> device_type = "memory"; >> reg = <0x10000 0x3000>; >> }; > > Shared RAM is not so straight forward. Both PRU firmwares and both application drivers > might need to read/write here. The area split is decided by firmware design and there > is no hardware protection to prevent from stomping on each others toes. > > We need a carveout based memory allocator at least I think that can do a > allocate(base_offset, size); into shared RAM. > > This could be used by pru_rproc driver at firmware load time and by application drivers > at initialization time. > > Thoughts? > >> >> pruss_cfg: cfg@26000 { >> ... >> }; >> ... >> }; >> >> If the device_type = "memory" cannot be used here for >> being specific to the top level properties, then >> there's probably some other generic property usable >> here :) >> >>> + pruss_mii_rt: mii_rt@32000 { >>> + reg = <0x32000 0x58>; >>> + }; >> >> The node name should not have underscores so >> pruss_mii_rt: mii-rt@32000. Please check the others >> too, like app_node. >> > > OK. > >>> + app_node: app_node { >>> + prus = <&pru0>, <&pru1>; >>> + firmware-name = "pruss-app-fw", "pruss-app-fw-2"; >>> + ti,pruss-gp-mux-sel = <2>, <1>; >>> + /* setup interrupts for prus: >>> + prus[0] => pru1_0: ev=16, chnl=2, host-irq=7, >>> + prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */ >>> + ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>; >>> + } >> >> If the ti,pruss-gp-mux-sel and ti,pru-interrupt-map are >> firmware configuration options, maybe leave them out of >> the dts completely and make the app-node optional. > > Yes the app-node is optional. I will mention it. > > No, ti,pruss-gp-mux-sel and ti,pru-interrupt-map are not firmware options. > But these settings are application/firmware specific. > > ti,pru-interrupt-map specifies the configuration to be used for the INTC interrupt > controller. > > ti,pruss-gp-mux-sel is used to configure this register. > "Table 30-20. PRUSS_GPCFG0" in http://www.tij.co.jp/jp/lit/ug/spruhz7h/spruhz7h.pdf > "29:26 PR1_PRU0_GP_MUX_SEL" > > It configures how the pins from the PRUSS module are routed internally > to the various modules. > > see "30.2.1 PRU-ICSS I/O Interface" > and "Table 30-1. PRU-ICSS1 I/O Signals" > >> >> And have a proper compatible for this node such as >> "ti,pruss-app-xyz". And this should be only set if the the >> hardware is wired up in such way that things need to be >> configured in the dts rather than by the firmware. > > Yes, compatible is a required property as we need to load > the appropriate application (kernel space) driver for it. > I will fix the example. > >> >> And then you can just hide mux-sel and interrupt-map >> behind the compatible property for that hardware. And >> leave them out from the dts and have the handling driver >> would set mux-sel and interrupt-map based on the >> match->data during probe. > > To summarize: > > I'll mark the app node as optional. Only required if a kernel > driver is required for the application. > Compatible is mandatory for app node. > ti,pruss-gp-mux-sel and ti,pru-interrupt-map are optional > for app node. > > cheers, > -roger >
* Murali Karicheri <m-karicheri2@ti.com> [190205 16:13]: > On 02/05/2019 10:41 AM, Roger Quadros wrote: > > What I'm suggesting here is that kernel remoteproc driver should have nothing to do > > with the other PRU's data RAM. > > > > The application driver if needs both PRUs then it can obviously access both DRAMs > > as it has a phandle to both PRUs. > > > That should be fine. That sounds good to me too. For dts, yeah please allocate the resources for the modules where the resources belong to on the PRUSS internal interconnect :) Devices can move around on the interconnect between SoCs and the modules can get swapped or added. Regards, Tony
On 2/5/19 2:51 AM, Roger Quadros wrote: > +Rob > > Andrew, > > On 04/02/19 17:33, Roger Quadros wrote: >> On 04/02/19 17:11, Andrew F. Davis wrote: >>> On 2/4/19 8:22 AM, Roger Quadros wrote: >>>> From: "Andrew F. Davis" <afd@ti.com> >>>> >>> >>> [...] >>> >>>> +static const struct pruss_intc_match_data am437x_pruss_intc_data = { >>>> + .no_host7_intr = true, >>> >>> Like done for the PRUSS driver with 'has_no_sharedram' becoming a DT >>> flag the same could be done here, then all this match data stuff could >>> be dropped. >> >> Agreed. >> > > Going back and looking at code here is a different perspective. > > The has_no_sharedram case was a an odd duck because the 2 ICSSG instances > within the same SoC (AM437x) had differences. So we couldn't use > the compatible to differentiate there. The DT flag makes sense there. > > In the no_host7_intr case, it SoC specific so we can use the compatible to > differentiate. And AM6 SoC has different number of system_events and host_interrupts > so that could come in macth_data as well. See below. > > static const struct pruss_intc_match_data am335x_am57xx_pruss_intc_data = { > .num_system_events = 64, > .num_host_intrs = 10, > .no_host7_intr = false, > }; > > static const struct pruss_intc_match_data am437x_k2g_pruss_intc_data = { > .num_system_events = 64, > .num_host_intrs = 10, > .no_host7_intr = true, > }; > > static const struct pruss_intc_match_data am6x_icssg_intc_data = { > .num_system_events = 160, > .num_host_intrs = 20, > .no_host7_intr = false, > }; > > Alternatively, we add a DT property each for all 3 of them and get rid > of match_data entirely. > > Which is a better approach? I prefer to retain the current reliance of using of_match_data, rather than having to add additional DT properties and parse them and define variables to store them. This has served well in terms of scaling up and get the variable storage for free. Rob, what is your recommendation here? regards Suman > > cheers, > -roger > > >>> >>>> +}; >>>> + >>>> +static const struct of_device_id pruss_intc_of_match[] = { >>>> + { >>>> + .compatible = "ti,am3356-pruss-intc", >>>> + .data = NULL, >>>> + }, >>>> + { >>>> + .compatible = "ti,am4376-pruss-intc", >>>> + .data = &am437x_pruss_intc_data, >>>> + }, >>>> + { >>>> + .compatible = "ti,am5728-pruss-intc", >>>> + .data = NULL, >>>> + }, >>>> + { /* sentinel */ }, >>>> +}; >>>> +MODULE_DEVICE_TABLE(of, pruss_intc_of_match); >>>> + >>>> +static struct platform_driver pruss_intc_driver = { >>>> + .driver = { >>>> + .name = "pruss-intc", >>>> + .of_match_table = pruss_intc_of_match, >>>> + }, >>>> + .probe = pruss_intc_probe, >>>> + .remove = pruss_intc_remove, >>>> +}; >>>> +module_platform_driver(pruss_intc_driver); >>>> + >>>> +MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>"); >>>> +MODULE_AUTHOR("Suman Anna <s-anna@ti.com>"); >>>> +MODULE_DESCRIPTION("PRU-ICSS INTC Driver"); >>>> +MODULE_LICENSE("GPL v2"); >>>> diff --git a/include/linux/irqchip/irq-pruss-intc.h b/include/linux/irqchip/irq-pruss-intc.h >>>> new file mode 100644 >>>> index 0000000..4538a0b >>>> --- /dev/null >>>> +++ b/include/linux/irqchip/irq-pruss-intc.h >>>> @@ -0,0 +1,94 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0 */ >>>> +/** >>>> + * irq-pruss-intc.h - PRU-ICSS INTC management >>> >>> Filename not needed. >> >> OK. >> >> cheers, >> -roger >> >>> >>> Andrew >>> >>>> + * >>>> + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com >>>> + */ >>>> + >>>> +#ifndef __INCLUDE_LINUX_IRQCHIP_IRQ_PRUSS_INTC_H >>>> +#define __INCLUDE_LINUX_IRQCHIP_IRQ_PRUSS_INTC_H >>>> + >>>> +/* maximum number of system events */ >>>> +#define MAX_PRU_SYS_EVENTS 64 >>>> + >>>> +/* maximum number of interrupt channels */ >>>> +#define MAX_PRU_CHANNELS 10 >>>> + >>>> +/** >>>> + * struct pruss_intc_config - INTC configuration info >>>> + * @sysev_to_ch: system events to channel mapping information >>>> + * @ch_to_host: interrupt channel to host interrupt information >>>> + */ >>>> +struct pruss_intc_config { >>>> + s8 sysev_to_ch[MAX_PRU_SYS_EVENTS]; >>>> + s8 ch_to_host[MAX_PRU_CHANNELS]; >>>> +}; >>>> + >>>> +#if IS_ENABLED(CONFIG_TI_PRUSS) >>>> + >>>> +/** >>>> + * pruss_intc_configure() - configure the PRUSS INTC >>>> + * @dev: device >>>> + * @intc_config: PRU core-specific INTC configuration >>>> + * >>>> + * Configures the PRUSS INTC with the provided configuration from >>>> + * a PRU core. Any existing event to channel mappings or channel to >>>> + * host interrupt mappings are checked to make sure there are no >>>> + * conflicting configuration between both the PRU cores. The function >>>> + * is intended to be used only by the PRU remoteproc driver. >>>> + * >>>> + * Returns 0 on success, or a suitable error code otherwise >>>> + */ >>>> +int pruss_intc_configure(struct device *dev, >>>> + struct pruss_intc_config *intc_config); >>>> + >>>> +/** >>>> + * pruss_intc_unconfigure() - unconfigure the PRUSS INTC >>>> + * @dev: device >>>> + * @intc_config: PRU core specific INTC configuration >>>> + * >>>> + * Undo whatever was done in pruss_intc_configure() for a PRU core. >>>> + * It should be sufficient to just mark the resources free in the >>>> + * global map and disable the host interrupts and sysevents. >>>> + */ >>>> +int pruss_intc_unconfigure(struct device *dev, >>>> + struct pruss_intc_config *intc_config); >>>> +/** >>>> + * pruss_intc_trigger() - trigger a PRU system event >>>> + * @irq: linux IRQ number associated with a PRU system event >>>> + * >>>> + * Trigger an interrupt by signalling a specific PRU system event. >>>> + * This can be used by PRUSS client users to raise/send an event to >>>> + * a PRU or any other core that is listening on the host interrupt >>>> + * mapped to that specific PRU system event. The @irq variable is the >>>> + * Linux IRQ number associated with a specific PRU system event that >>>> + * a client user/application uses. The interrupt mappings for this is >>>> + * provided by the PRUSS INTC irqchip instance. >>>> + * >>>> + * Returns 0 on success, or an error value upon failure. >>>> + */ >>>> +int pruss_intc_trigger(unsigned int irq); >>>> + >>>> +#else >>>> + >>>> +static inline int pruss_intc_configure(struct device *dev, >>>> + struct pruss_intc_config *intc_config) >>>> +{ >>>> + return -ENOTSUPP; >>>> +} >>>> + >>>> +static inline int pruss_intc_unconfigure(struct device *dev, >>>> + struct pruss_intc_config *intc_config) >>>> +{ >>>> + return -ENOTSUPP; >>>> +} >>>> + >>>> +static inline int pruss_intc_trigger(unsigned int irq) >>>> +{ >>>> + return -ENOTSUPP; >>>> +} >>>> + >>>> +#endif /* CONFIG_TI_PRUSS */ >>>> + >>>> +#endif /* __INCLUDE_LINUX_IRQCHIP_IRQ_OMAP_INTC_H */ >>>> + >>>> >> >
On 2/4/19 8:22 AM, Roger Quadros wrote: > From: "Andrew F. Davis" <afd@ti.com> > > The Programmable Real-Time Unit Subsystem (PRUSS) contains an > interrupt controller (INTC) that can handle various system input > events and post interrupts back to the device-level initiators. > The INTC can support upto 64 input events with individual control > configuration and hardware prioritization. These events are mapped > onto 10 interrupt signals through two levels of many-to-one mapping > support. Different interrupt signals are routed to the individual > PRU cores or to the host CPU. > > The PRUSS INTC platform driver manages this PRUSS interrupt > controller and implements an irqchip driver to provide a Linux > standard way for the PRU client users to enable/disable/ack/ > re-trigger a PRUSS system event. The system events to interrupt > channels and host interrupts relies on the mapping configuration > provided through a firmware resource table for now. This will be > revisited and enhanced in the future for a better interface. The > mappings will currently be programmed during the boot/shutdown > of the PRU. > > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Jason Cooper <jason@lakedaemon.net> > Cc: Marc Zyngier <marc.zyngier@arm.com> > Cc: Rob Herring <robh+dt@kernel.org> > Signed-off-by: Andrew F. Davis <afd@ti.com> > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > .../interrupt-controller/ti,pruss-intc-irq.txt | 51 ++++++++++++++++++++++ Better to name the file just ti,pruss-intc.txt. Also, one minor nit, %s/dt-binding/dt-bindings/ on the the patch title. regards Suman > 1 file changed, 51 insertions(+) > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc-irq.txt > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc-irq.txt b/Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc-irq.txt > new file mode 100644 > index 0000000..c70221c > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc-irq.txt > @@ -0,0 +1,51 @@ > +PRU ICSS INTC on TI SoCs > +======================== > + > +Each PRUSS has a single interrupt controller instance that is common to both > +the PRU cores. Each interrupt controller can detect 64 input events which are > +then mapped to 10 possible output interrupts through two levels of mapping. The > +input events can be triggered by either the PRUs and/or various other PRUSS > +internal and external peripherals. The first 2 output interrupts are fed > +exclusively to the internal PRU cores, with the remaining 8 connected to > +external interrupt controllers including the MPU. > + > +Required Properties: > +-------------------- > +- compatible : should be one of, > + "ti,am3356-pruss-intc" for AM335x family of SoCs > + "ti,am4376-pruss-intc" for AM437x family of SoCs > + "ti,am5728-pruss-intc" for AM57xx family of SoCs > + "ti,k2g-pruss-intc" for 66AK2G family of SoCs > +- reg : base address and size for the PRUSS INTC sub-module > +- reg-names : should contain the string "intc" > +- interrupts : all the interrupts generated towards the main host > + processor in the SoC. The format depends on the > + interrupt specifier for the particular SoC's MPU > + parent interrupt controller > +- interrupt-names: should use one of the following names for each interrupt, > + the name should match the corresponding host interrupt > + number, > + "host2", "host3", "host4", "host5", "host6", > + "host7", "host8" or "host9" > + NOTE: AM437x and 66AK2G SoCs do not have "host7" interrupt > + connected to MPU > +- interrupt-controller : mark this node as an interrupt controller > +- #interrupt-cells : should be 1. Client users shall use the PRU System > + event number (the interrupt source that the client > + is interested in) as the value of the interrupts > + property in their node > + > +Example: > +-------- > + pruss_intc: intc@20000 { > + compatible = "ti,am3356-pruss-intc"; > + reg = <0x20000 0x2000>; > + reg-names = "intc"; > + interrupt-controller; > + #interrupt-cells = <1>; > + interrupts = <20 21 22 23 24 25 26 27>; > + interrupt-names = "host2", "host3", "host4", > + "host5", "host6", "host7", > + "host8", "host9"; > + }; > + >
On 2/6/19 9:04 AM, Roger Quadros wrote: > > > On 05/02/19 18:19, Tony Lindgren wrote: >> * Murali Karicheri <m-karicheri2@ti.com> [190205 16:13]: >>> On 02/05/2019 10:41 AM, Roger Quadros wrote: >>>> What I'm suggesting here is that kernel remoteproc driver should have nothing to do >>>> with the other PRU's data RAM. >>>> >>>> The application driver if needs both PRUs then it can obviously access both DRAMs >>>> as it has a phandle to both PRUs. >>>> >>> That should be fine. >> >> That sounds good to me too. >> >> For dts, yeah please allocate the resources for the modules >> where the resources belong to on the PRUSS internal interconnect :) >> Devices can move around on the interconnect between SoCs and the >> modules can get swapped or added. > > If you take a look at "Figure 30-1. PRU-ICSS Overview" in > http://www.tij.co.jp/jp/lit/ug/spruhz7h/spruhz7h.pdf > > You can see that DRAM0 and DRAM1 are not part of PRU. That means > they shouldn't be in the PRU node then. Yes, they do not belong to a PRU, and should not be defined underneath one. Both are accessible from both PRU cores, so it is upto the application on how they can partition the usage. regards Suman
Hi Roger, On 2/4/19 8:22 AM, Roger Quadros wrote: > From: Suman Anna <s-anna@ti.com> > > This patch adds the bindings for the Programmable Real-Time Unit > and Industrial Communication Subsystem (PRU-ICSS) present on various > SoCs such as AM33xx, AM437x, AM57xx, Keystone 66AK2G SoC, etc. It is > present on the Davinci based OMAPL138 SoCs and K3 architecture > based AM65x SoCs as well (not covered for now). > > Signed-off-by: Suman Anna <s-anna@ti.com> > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > .../devicetree/bindings/soc/ti/ti,pruss.txt | 212 +++++++++++++++++++++ > 1 file changed, 212 insertions(+) > create mode 100644 Documentation/devicetree/bindings/soc/ti/ti,pruss.txt > > diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt > new file mode 100644 > index 0000000..5ac76fd > --- /dev/null > +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt > @@ -0,0 +1,212 @@ > +PRU-ICSS on TI SoCs > +=================== > + > +The Programmable Real-Time Unit and Industrial Communication Subsystem > +(PRU-ICSS) is present on various TI SoCs such as AM335x, AM437x, Keystone > +66AK2G, etc. A PRUSS consists of dual 32-bit RISC cores (Programmable > +Real-Time Units, or PRUs) with program memory and data memory. > + > +The programmable nature of the PRUs provide flexibility to implement > +custom peripheral interfaces, fast real-time responses, or specialized > +data handling. The common peripheral modules include the following, > + > + - Enhanced GPIO with async capture and serial support > + - an Ethernet MII_RT module with two MII ports > + - an MDIO port to control external Ethernet PHYs > + - an Industrial Ethernet Peripheral (IEP) to manage/generate Industrial > + Ethernet functions > + - an Enhanced Capture Module (eCAP) > + - a 16550-compatible UART to support PROFIBUS > + - Interrupt controller with 64 input events and 10 Host interrupts. > + > +A shared Data RAM, if present, can be accessed by both the PRU cores. The > +Interrupt Controller (INTC) and a CFG module are common to both the PRU > +cores. > + > +Various sub-modules within a PRU-ICSS subsystem are represented as individual > +nodes. > + > +PRUSS Node > +============= > + > +This node represents the entire ICSS instance and the various modules are > +contained as children. The PRUSS driver is responsible for managing the > +common resources i.e. DRAM0, DRAM1, SHARED_RAM and CFG space. > + > +Required Properties: > +-------------------- > +- compatible : should be one of, > + "ti,am3356-pruss" for AM335x family of SoCs > + "ti,am4376-pruss" for AM437x family of SoCs > + "ti,am5728-pruss" for AM57xx family of SoCs > + "ti,k2g-pruss" for 66AK2G family of SoCs > +- reg : base address and size for each of the Data RAMs as > + mentioned in reg-names, and in the same order as the > + reg-names Hmm, not sure what prompted you to deviate from the design we had on TI SDK kernels. Your revised bindings does not allow us to use this device for the scalability with either UIO/VFIO. Let's sort this out before you post the next series. regards Suman > +- reg-names : should contain a string(s) from among the following names, > + each representing a specific Data RAM region. Some PRU-ICSS > + instances on certain SoCs might not have Shared DRAM. > + "dram0" for Data RAM0, > + "dram1" for Data RAM1, > + "shrdram2" for Shared Data RAM, > +- #address-cells : should be 1 > +- #size-cells : should be 1 > +- ranges : no specific range translations required, child nodes have the > + same address view as the parent, so should be mentioned without > + any value for the property > + > +Optional Properties: > +-------------------- > +- no-shared-ram : Should be present if the instance doesn't have Shared RAM. > + e.g. AM4376 ICSS0 instance doesn't have Shared RAM. > + > +The PRUSS node will have one or more of the folowing child nodes. > + > +PRU CORES > +========= > +ICSS typically has 2 PRU cores. These should be represented as remoteproc devices. > + > +INTC node > +========= > +ICSS has one INTC interrupt controller module. This should be represented as > +a standard interrupt-controller node. > + > +CFG, IEP, MII_RT > +================ > +The individual sub-modules CFG, IEP and MII_RT are represented as a syscon > +node each with specific node names as below: > + "cfg" for CFG sub-module, > + "iep" for IEP sub-module, > + "mii_rt" for MII-RT sub-module, > + > +See Documentation/devicetree/bindings/mfd/syscon.txt for details. > + > +MDIO > +==== > +Each PRUSS has an MDIO module that can be used to control external PHYs. The > +MDIO module used within the PRU-ICSS is an instance of the MDIO Controller > +used in TI Davinci SoCs. Please refer to the corresponding binding document, > +Documentation/devicetree/bindings/net/davinci-mdio.txt for details. > + > +Application/User Nodes > +======================= > +A PRU application/user node typically uses one or more PRU device nodes to > +implement a PRU application/functionality. Each application/client node would > +need a reference to at least a PRU node, and optionally pass some configuration > +parameters. > + > +Required Properties: > +-------------------- > +- prus : phandles to the PRU nodes used > + > +Optional Properties: > +-------------------- > +- firmware-name : firmwares for the PRU cores, the default firmware > + for the core from the PRU node will be used if not > + provided. The firmware names should correspond to > + the PRU cores listed in the 'prus' property > +- ti,pruss-gp-mux-sel : array of values for the GP_MUX_SEL under PRUSS_GPCFG > + register for a PRU. This selects the internal muxing > + scheme for the PRU instance. If not provided, the > + default out-of-reset value (0) for the PRU core is > + used. Values should correspond to the PRU cores listed > + in the 'prus' property > +- ti,pru-interrupt-map : PRU interrupt mappings, containing an array of entries > + with each entry consisting of 4 cell-values. First one > + is an index towards the "prus" property to identify the > + PRU core for the interrupt map, second is the PRU > + System Event id, third is the PRU interrupt channel id > + and fourth is the PRU host interrupt id. If provided, > + this map will supercede any other configuration > + provided through firmware > + > +Example: > +======== > +1. /* AM33xx PRU-ICSS */ > + > + pruss: pruss@0 { > + compatible = "ti,am3356-pruss"; > + reg = <0x0 0x2000>, > + <0x2000 0x2000>, > + <0x10000 0x3000>; > + reg-names = "dram0", "dram1", > + "shrdram2"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + pruss_cfg: cfg@26000 { > + compatible = "syscon"; > + reg = <0x26000 0x2000>; > + }; > + > + pruss_iep: iep@2e000 { > + compatible = "syscon"; > + reg = <0x2e000 0x31c>; > + }; > + > + pruss_mii_rt: mii_rt@32000 { > + compatible = "syscon"; > + reg = <0x32000 0x58>; > + }; > + > + pruss_intc: intc@20000 { > + compatible = "ti,am3356-pruss-intc"; > + reg = <0x20000 0x2000>; > + reg-names = "intc"; > + interrupt-controller; > + #interrupt-cells = <1>; > + interrupts = <20 21 22 23 24 25 26 27>; > + interrupt-names = "host2", "host3", "host4", > + "host5", "host6", "host7", > + "host8", "host9"; > + }; > + > + pru0: pru@34000 { > + compatible = "ti,am3356-pru"; > + reg = <0x34000 0x2000>, > + <0x22000 0x400>, > + <0x22400 0x100>; > + reg-names = "iram", "control", "debug"; > + gpcfg = <&pruss_cfg 0x8>; > + firmware-name = "am335x-pru0-fw"; > + interrupt-parent = <&pruss_intc>; > + interrupts = <16>, <17>; > + interrupt-names = "vring", "kick"; > + }; > + > + pru1: pru@38000 { > + compatible = "ti,am3356-pru"; > + reg = <0x38000 0x2000>, > + <0x24000 0x400>, > + <0x24400 0x100>; > + reg-names = "iram", "control", "debug"; > + gpcfg = <&pruss_cfg 0xc>; > + firmware-name = "am335x-pru1-fw"; > + interrupt-parent = <&pruss_intc>; > + interrupts = <18>, <19>; > + interrupt-names = "vring", "kick"; > + }; > + > + pruss_mdio: mdio@32400 { > + compatible = "ti,davinci_mdio"; > + reg = <0x32400 0x90>; > + clocks = <&dpll_core_m4_ck>; > + clock-names = "fck"; > + bus_freq = <1000000>; > + #address-cells = <1>; > + #size-cells = <0>; > + status = "disabled"; > + }; > + }; > + > +2: /* PRU application node example */ > + app_node: app_node { > + prus = <&pru0>, <&pru1>; > + firmware-name = "pruss-app-fw", "pruss-app-fw-2"; > + ti,pruss-gp-mux-sel = <2>, <1>; > + /* setup interrupts for prus: > + prus[0] => pru1_0: ev=16, chnl=2, host-irq=7, > + prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */ > + ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>; > + } >
On 2/5/19 10:41 AM, Tony Lindgren wrote: > * Roger Quadros <rogerq@ti.com> [190205 09:40]: >> On 04/02/19 18:33, Tony Lindgren wrote: >>> >>> shrdram2: memory@10000 { >>> device_type = "memory"; >>> reg = <0x10000 0x3000>; >>> }; >> >> Shared RAM is not so straight forward. Both PRU firmwares and both application drivers >> might need to read/write here. The area split is decided by firmware design and there >> is no hardware protection to prevent from stomping on each others toes. >> >> We need a carveout based memory allocator at least I think that can do a >> allocate(base_offset, size); into shared RAM. >> >> This could be used by pru_rproc driver at firmware load time and by application drivers >> at initialization time. >> >> Thoughts? > > That sounds sane to me :) > >>> If the ti,pruss-gp-mux-sel and ti,pru-interrupt-map are >>> firmware configuration options, maybe leave them out of >>> the dts completely and make the app-node optional. >> >> Yes the app-node is optional. I will mention it. >> >> No, ti,pruss-gp-mux-sel and ti,pru-interrupt-map are not firmware options. >> But these settings are application/firmware specific. >> >> ti,pru-interrupt-map specifies the configuration to be used for the INTC interrupt >> controller. > > OK. So just to see if we have a standard solution available already.. > It sounds a bit similar to what we're doing with omap-wakeupgen.c > and stacked interrupts? I wonder if something similar might help > here? > >> ti,pruss-gp-mux-sel is used to configure this register. >> "Table 30-20. PRUSS_GPCFG0" in http://www.tij.co.jp/jp/lit/ug/spruhz7h/spruhz7h.pdf >> "29:26 PR1_PRU0_GP_MUX_SEL" >> >> It configures how the pins from the PRUSS module are routed internally >> to the various modules. Actually, that's not entirely accurate. This is an internal pinmux (not controllable per pin, but rather dictates a different sets of groups of pins at the PRUSS boundary, which are then again multiplexed at the SoC level using the standard padconf/pinmux. It is a single register per core into which you can set some values between 0 through 4 IIRC (unfortunately the values are also not uniform across the various SoCs). regards Suman >> >> see "30.2.1 PRU-ICSS I/O Interface" >> and "Table 30-1. PRU-ICSS1 I/O Signals" > > Well these are external signals for PRUSS processor (although not > necessarily external signals for the SoC). So why not handle them > with a standard pinctlr binding with #pinctrl-cells? > > Sure it may not even be the Linux pinctrl framework running on the > main SoC handling these pins, but after all you're describing > hardware for a processor. Maybe Linus W has some comments on this? > > Regards, > > Tony >
On 2/8/19 7:51 AM, Linus Walleij wrote: > On Mon, Feb 4, 2019 at 3:24 PM Roger Quadros <rogerq@ti.com> wrote: > >> From: Suman Anna <s-anna@ti.com> >> >> This patch adds the bindings for the Programmable Real-Time Unit >> and Industrial Communication Subsystem (PRU-ICSS) present on various >> SoCs such as AM33xx, AM437x, AM57xx, Keystone 66AK2G SoC, etc. It is >> present on the Davinci based OMAPL138 SoCs and K3 architecture >> based AM65x SoCs as well (not covered for now). >> >> Signed-off-by: Suman Anna <s-anna@ti.com> >> Signed-off-by: Roger Quadros <rogerq@ti.com> > > (...) >> + pruss_intc: intc@20000 { >> + compatible = "ti,am3356-pruss-intc"; >> + reg = <0x20000 0x2000>; >> + reg-names = "intc"; >> + interrupt-controller; >> + #interrupt-cells = <1>; >> + interrupts = <20 21 22 23 24 25 26 27>; >> + interrupt-names = "host2", "host3", "host4", >> + "host5", "host6", "host7", >> + "host8", "host9"; > > If thsese interrupts are mapped 1-to-1 to a parent interrupt controller > then this is a hierarchical interrupt domain and then these should > be handled locally in the driver as offset from child to parent > statically encoded in the driver. > > Several old drivers and old device tree bindings make this kind > of maps, but it is not how we do it anymore, if we can avoid it. > > To be able to use hierarchical interrupt domain in the kernel, the top > interrupt controller must use the hierarchical (v2) irqdomain, so > if this is anything else than the ARM GIC it will be an interesting > undertaking to handle this. These are interrupt lines coming towards the host processor running Linux and are directly connected to the ARM GIC. This INTC module is actually an PRUSS internal interrupt controller that can take in 64 (on most SoCs) external events/interrupt sources and multiplexing them through two layers of many-to-one events-to-intr channels & intr-channels-to-host interrupts. Couple of the host interrupts go to the PRU cores themselves while the remaining ones come out of the IP to connect to other GICs in the SoC. We have implemented this as an irqchip using chained interrupt handlers with the consumers using the event numbers on the Linux-side. The PRUs also access some of the associated registers for clearing an event source. regards Suman > > The more I understand of hierarchical irqdomains, the more of > workarounds where we should be using it I see, we really need > to spread this knowledge. Using it requires a lot of upfront work > sometimes, sorry about that but the end result is so much better. > > Yours, > Linus Walleij >
On 2/13/19 8:35 PM, Suman Anna wrote: > Hi Roger, > > On 2/4/19 8:22 AM, Roger Quadros wrote: >> From: Suman Anna <s-anna@ti.com> >> >> The Programmable Real-Time Unit Subsystem (PRUSS) consists of >> dual 32-bit RISC cores (Programmable Real-Time Units, or PRUs) >> for program execution. This patch adds a remoteproc platform >> driver for managing the individual PRU RISC cores life cycle. >> >> This remoteproc driver does not have support for error recovery >> and system suspend/resume features. Different compatibles are >> used to allow providing scalability for instance-specific device >> data if needed. The driver uses a default firmware-name retrieved >> from device-tree, and the firmwares are expected to be present >> in the standard Linux firmware search paths. They can also be >> adjusted by userspace if required through the sysfs interface >> provided by the remoteproc core. >> >> The PRU remoteproc driver uses a client-driven boot methodology >> - it does _not_ support auto-boot so that the PRU load and boot >> is dictated by the corresponding client drivers for achieving >> various usecases. This allows flexibility for the client drivers >> or applications to set a firmware name (if needed) based on their >> desired functionality and boot the PRU. The sysfs bind and unbind >> attributes have also been suppressed so that the PRU devices cannot >> be unbound and thereby shutdown a PRU from underneath a PRU client >> driver. >> >> A new entry 'single_step' is added to the remoteproc debugfs dir. >> The 'single_step' utilizes the single-step execution of the PRU >> cores. Writing a non-zero value performs a single step, and a >> zero value restores the PRU to execute in the same mode as the >> mode before the first single step. (note: if the PRU is halted >> because of a halt instruction, then no change occurs). >> >> pru_rproc_get() and pru_rproc_put() functions allow client drivers >> to acquire and release the remoteproc device associated with a PRU core. >> The PRU cores are treated as resources with only one client owning >> it at a time. >> >> PRU interrupt mapping can be provided via devicetree using >> ti,pru-interrupt-map property or via the resource table in the >> firmware blob. If both are provided, the config in DT takes precedence. >> For interrupt map via resource table, pru-software-support-package [1] >> has been historically using version 0 for this. However, the current >> data structure is not scaleable and is not self sufficient. >> 1) it hard codes number of channel to host mappings so is not >> scaleable to newer SoCs than have more of these. >> 2) it does not contain the event to channel mappings within >> itself but relies on a pointer to point to another section >> in data memory. This causes a weird complication that the >> respective data section must be loaded before we can really >> use the INTC map. >> >> With this patch we drop support for version 0 and support >> version 1 which is a more robust and scalable data structure. >> It should be able to support a sufficiently large number (255) of >> sysevents, channels and host interrupts and is self contained >> so it can be used without dependency on order of loading sections. > > Hmm, looks like you squashed a whole bunch of patches into this. I would > prefer some of the logical ones to be separated out just like before, > makes it easier to review the interfaces. > > Looks like you have also changed usage behavior on couple of things and > dropped some of my original changes. I was not able to test this series > either on AM335x or on AM57xx to comment more on the behavior (not even > seeing the pruss devices populated, not sure what pieces are missing still). Able to get the PRUSS devices show up on AM335x atleast after enabling the Reset Controller related configs, but no such luck on AM57xx. regards Suman > >> >> [1] git://git.ti.com/pru-software-support-package/pru-software-support-package.git >> >> Signed-off-by: Suman Anna <s-anna@ti.com> >> Signed-off-by: Andrew F. Davis <afd@ti.com> >> Signed-off-by: Tero Kristo <t-kristo@ti.com> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> ---
fixed DTML id. On 14/02/19 17:44, Roger Quadros wrote: > On 14/02/19 14:52, Marc Zyngier wrote: >> On Thu, 14 Feb 2019 10:55:10 +0000, >> Roger Quadros <rogerq@ti.com> wrote: >>> >>> >>> On 14/02/19 10:37, Linus Walleij wrote: >>>> On Thu, Feb 14, 2019 at 4:13 AM Suman Anna <s-anna@ti.com> wrote: >>>>> [Me] >>>> >>>>>> To be able to use hierarchical interrupt domain in the kernel, the top >>>>>> interrupt controller must use the hierarchical (v2) irqdomain, so >>>>>> if this is anything else than the ARM GIC it will be an interesting >>>>>> undertaking to handle this. >>>>> >>>>> These are interrupt lines coming towards the host processor running >>>>> Linux and are directly connected to the ARM GIC. This INTC module is >>>>> actually an PRUSS internal interrupt controller that can take in 64 (on >>>>> most SoCs) external events/interrupt sources and multiplexing them >>>>> through two layers of many-to-one events-to-intr channels & >>>>> intr-channels-to-host interrupts. Couple of the host interrupts go to >>>>> the PRU cores themselves while the remaining ones come out of the IP to >>>>> connect to other GICs in the SoC. >>>> >>>> If the muxing is static (like set up once at probe) so that while >>>> the system is running, there is one and one only event mapped to >>>> the GIC from the component below it, then it is hierarchical. >>> >>> This is how it looks. >>> >>> [GIC]<---8---[INTC]<---64---[events from peripherals] >>> >>> The 8 interrupt lines from INTC to the GIC are 1:1 mapped and fixed >>> per SoC. The muxing between 64 inputs to INTC and its 8 outputs are >>> programmable and might not necessarily be static per boot/probe as >>> it depends on what firmware is loaded on the PRU. >> >> But the point is that at any given time, there are at most 8 out of 64 >> inputs that are used, right? You *never* end-up with two (or more) of >> these "events" being multiplexed on a single output line. >> > > Since the INTC's internal logic allows assigning more than one event each outputs, > at most all 64 events can be assigned to one output or distributed among the 8 outputs. > >> If these assertions do hold, then your design is typical of a >> hierarchy, for which we have countless examples in the tree (including >> for some TI HW). > > OK. > Suman, Andrew, Lokesh, thoughts? > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
* Roger Quadros <rogerq@ti.com> [190214 11:09]: > Suman is mainly concerned about the following changes in v2 > 1) pruss node does not contain reg property representing entire ICSS. > 2) pruss node does not contain interrupts. > > Both of these are required if drivers/uio/uio_pruss.c or in future if > VFIO is to be used. > > The beagleboard community is a primary user of this driver and we need to > find a solution so that PRUSS is usable either via remoteproc or via UIO. > > Ideal case should allow user to use either of the drivers by just doing > a unbind and bind. > > I don't have a better idea than having a encapsulating node that has > the appropriate reg and interrupt properties. If there are existing use cases that need to be supported you should list them as non-standard usage in the binding and not recommended for future use. Rob may have some comments on how to deal with this. Then you can have device driver that needs to pass them parse them from the PRUSS parent node. That does not mean there needs to be a top level device driver for PRUSS, the child control module can just parse the non-standard bindings for compability from the parent node. And naturally in addition to handling the non-standard binding we need to have a proper standardized binding too :) Regards, Tony
On 2/14/19 9:48 AM, Roger Quadros wrote: > fixed DTML id. > > On 14/02/19 17:44, Roger Quadros wrote: >> On 14/02/19 14:52, Marc Zyngier wrote: >>> On Thu, 14 Feb 2019 10:55:10 +0000, >>> Roger Quadros <rogerq@ti.com> wrote: >>>> >>>> >>>> On 14/02/19 10:37, Linus Walleij wrote: >>>>> On Thu, Feb 14, 2019 at 4:13 AM Suman Anna <s-anna@ti.com> wrote: >>>>>> [Me] >>>>> >>>>>>> To be able to use hierarchical interrupt domain in the kernel, the top >>>>>>> interrupt controller must use the hierarchical (v2) irqdomain, so >>>>>>> if this is anything else than the ARM GIC it will be an interesting >>>>>>> undertaking to handle this. >>>>>> >>>>>> These are interrupt lines coming towards the host processor running >>>>>> Linux and are directly connected to the ARM GIC. This INTC module is >>>>>> actually an PRUSS internal interrupt controller that can take in 64 (on >>>>>> most SoCs) external events/interrupt sources and multiplexing them >>>>>> through two layers of many-to-one events-to-intr channels & >>>>>> intr-channels-to-host interrupts. Couple of the host interrupts go to >>>>>> the PRU cores themselves while the remaining ones come out of the IP to >>>>>> connect to other GICs in the SoC. >>>>> >>>>> If the muxing is static (like set up once at probe) so that while >>>>> the system is running, there is one and one only event mapped to >>>>> the GIC from the component below it, then it is hierarchical. >>>> >>>> This is how it looks. >>>> >>>> [GIC]<---8---[INTC]<---64---[events from peripherals] >>>> >>>> The 8 interrupt lines from INTC to the GIC are 1:1 mapped and fixed >>>> per SoC. The muxing between 64 inputs to INTC and its 8 outputs are >>>> programmable and might not necessarily be static per boot/probe as >>>> it depends on what firmware is loaded on the PRU. >>> >>> But the point is that at any given time, there are at most 8 out of 64 >>> inputs that are used, right? You *never* end-up with two (or more) of >>> these "events" being multiplexed on a single output line. >>> >> >> Since the INTC's internal logic allows assigning more than one event each outputs, >> at most all 64 events can be assigned to one output or distributed among the 8 outputs. >> >>> If these assertions do hold, then your design is typical of a >>> hierarchy, for which we have countless examples in the tree (including >>> for some TI HW). >> >> OK. >> Suman, Andrew, Lokesh, thoughts? >> Mark, Linus, So, I hope it is clear from Roger's responses that above assertions do not hold true to this INTC, and so want to confirm that we are good with the current non-hierarchical design. regards Suman
Hi Tony, On 2/14/19 9:56 AM, Tony Lindgren wrote: > * Roger Quadros <rogerq@ti.com> [190214 11:09]: >> Suman is mainly concerned about the following changes in v2 >> 1) pruss node does not contain reg property representing entire ICSS. >> 2) pruss node does not contain interrupts. >> >> Both of these are required if drivers/uio/uio_pruss.c or in future if >> VFIO is to be used. >> >> The beagleboard community is a primary user of this driver and we need to >> find a solution so that PRUSS is usable either via remoteproc or via UIO. >> >> Ideal case should allow user to use either of the drivers by just doing >> a unbind and bind. >> >> I don't have a better idea than having a encapsulating node that has >> the appropriate reg and interrupt properties. > > If there are existing use cases that need to be supported > you should list them as non-standard usage in the binding > and not recommended for future use. Rob may have some > comments on how to deal with this. > > Then you can have device driver that needs to pass them > parse them from the PRUSS parent node. That does not mean > there needs to be a top level device driver for PRUSS, > the child control module can just parse the non-standard > bindings for compability from the parent node. The PRUSS SoC bus driver was handling all possible architectures (OMAP, K2 and K3) which have different clocking and reset integration, and also catering to the UIO vs remoteproc usecases, by taking care of clocks and resets. I am ok to replace this layer with the ti-sysc layer on OMAP SoCs since most of the functionality added to the driver is associated with OCP, but we would still need a PRUSS driver. Not all sub-modules are peripherals and managed by respective peripheral drivers, and we still need a central entity managing the sub-system wide resources. Layering wise - it is similar if we would have done a device for the PRUSS local interconnect, but that driver wouldn't have much to do with interconnect functionality. K2 and K3 families uses TI-SCI and so you don't have a similar target-module concept that allows you to query the PRUSS parent node for PRUSS specific ranges or properties. In anycase, I don't think these drivers should depend on a parent interconnect driver. regards Suman > > And naturally in addition to handling the non-standard > binding we need to have a proper standardized binding > too :) > > Regards, > > Tony >
On Mon, 4 Feb 2019 16:22:42 +0200, Roger Quadros wrote: > From: Suman Anna <s-anna@ti.com> > > The Programmable Real-Time Unit Subsystem (PRUSS) consists of > dual 32-bit RISC cores (Programmable Real-Time Units, or PRUs) > for program execution. This patch adds a remoteproc platform > driver for managing the individual PRU RISC cores life cycle. > > Add DT binding documentation for that. > > Cc: Rob Herring <robh+dt@kernel.org> > Signed-off-by: Suman Anna <s-anna@ti.com> > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > .../bindings/remoteproc/ti,pru-rproc.txt | 56 ++++++++++++++++++++++ > 1 file changed, 56 insertions(+) > create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.txt > Reviewed-by: Rob Herring <robh@kernel.org>
Hi, The Programmable Real-Time Unit and Industrial Communication Subsystem (PRU-ICSS) is present on various TI SoCs such as AM335x, AM437x, AM57x, Keystone 66AK2G, etc. A PRUSS consists of dual 32-bit RISC cores (Programmable Real-Time Units, or PRUs), with instruction and data memories. The programmable nature of the PRUs provide flexibility to implement custom peripheral interfaces, fast real-time responses, or specialized data handling. The common peripheral modules include the following, - Enhanced GPIO with async capture and serial support - an Ethernet MII_RT module with two MII ports - an MDIO port to control external Ethernet PHYs - an Industrial Ethernet Peripheral (IEP) to manage/generate Industrial Ethernet functions - an Enhanced Capture Module (eCAP) - a 16550-compatible UART to support PROFIBUS - Interrupt controller with 64 input events and 10 Host interrupts. A typical usage scenario would be to load the application firmware into one or more of the PRU cores, initialize one or more of the peripherals and perform I/O through shared RAM (or MSMC RAM) from either a Kernel driver or directly from userspace. With this series we should be able to use the kernel RPMSG driver along with firmware and user-space examples in the pru-software-support-package [1]. We will also be able to get Dual Ethernet functionality using a kernel driver which will be posted later. This series contains 1) soc: pruss: This is the parent driver for the entire ICSS. Its main purpose is to populate the different modules and manage memories. (i.e. DRAM0, DRAM1 and SharedRAM) It will also kernel APIs to manage the common CFG module. 2) irqchip: pruss-intc: This driver supports the INTC module on the ICSS. 3) remoteproc: pru: This provides a remoteproc interface for the PRU cores. With this we can load firmware, start/stop PRU from kernel or userspace. It adds support for virtio RPMSG. It also provides some kernel APIs (e.g. pru_rproc_set_ctable()) that are PRU specific and required for in-kernel applications (e.g. ethernet) 4) rpmsg: pru: An RPMsg driver that exposes interfaces to user space, to allow applications to communicate with the PRU processors. Platform data and device tree files for AM33xx and other SoCs will be sent as a separate series. Testing: All kernel patches along with AM335x (beaglebone) and AM57xx (IDK) platform support are at [3] To test the code with example firmware you can try the LAB5 tutorial http://processors.wiki.ti.com/index.php/PRU_Training:_Hands-on_Labs#LAB_5:_RPMsg_Communication_between_ARM_and_PRU with the pru-software-support package at [2] and the sample rpmsg-client driver. NOTE: you no longer need to build and run PRU_Halt example as shown in the tutorial. [1] https://git.ti.com/pru-software-support-package NOTE: The repo needs update to the INTC resource data structures The updates that are required are listed in the below repo with one example fixed. [2] https://github.com/rogerq/pru-software-support-package/commits/upstream/pruss [3] https://github.com/rogerq/linux/commits/for-v5.1/pruss-2.0 Changelog: v2: - use IS_ERR() instead of IS_ERR_OR_NULL(). - sqashed related patches to reduce patch count. - fixed build error at patch 11 "soc: ti: pruss: add pruss_get()/put() API". - incorporated rproc_da_to_va patches from David Lechner. - got rid of enum pruss_pru_id. - got rid of pre & post loaders in device specific resource handler. - get/set gpmux operation is now private to pru_rproc.c - changed pruss_cfg_gpimode() to pru_rproc_set_gpimode(struct rproc *rproc, enum pruss_gpi_mode mode). - moved pruss_intc.c to driver/irqchip/irq-pruss-intc.c. INTC APIs moved to include/irqchip/irq-pruss-intc.h. Decoupled struct pruss from INTC code. Any device who's interrupt-parent is pruss-intc can call the INTC APIs. - removed device_name from pruss_private_data. use dt property for .has_no_sharedram. - moved DRAM0, DRAM1 and SHARED_RAM memories into PRUSS node. cheers, -roger Andrew F. Davis (2): dt-binding: irqchip: Add pruss-intc-irq driver for PRUSS interrupts irqchip: pruss: Add a PRUSS irqchip driver for PRUSS interrupts David Lechner (2): remoteproc: add map parameter to da_to_va remoteproc: add page lookup for TI PRU to ELF loader Jason Reeder (1): rpmsg: pru: add a PRU RPMsg driver Roger Quadros (1): remoteproc/pru: Add pru_rproc_set_ctable() and pru_rproc_set_gpimode() Suman Anna (8): dt-bindings: remoteproc: Add TI PRUSS bindings soc: ti: pruss: Add a platform driver for PRUSS in TI SoCs remoteproc: Add a rproc_set_firmware() API remoteproc: Add support to handle device specific resource types dt-binding: remoteproc: Add binding doc for PRU Cores in the PRU-ICSS remoteproc/pru: Add PRU remoteproc driver remoteproc/pru: Add support for virtio rpmsg stack rpmsg: virtio_rpmsg_bus: move back rpmsg_hdr into a public header .../interrupt-controller/ti,pruss-intc-irq.txt | 51 + .../bindings/remoteproc/ti,pru-rproc.txt | 56 + .../devicetree/bindings/soc/ti/ti,pruss.txt | 212 ++++ drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-pruss-intc.c | 630 +++++++++++ drivers/remoteproc/Kconfig | 16 + drivers/remoteproc/Makefile | 1 + drivers/remoteproc/imx_rproc.c | 2 +- drivers/remoteproc/keystone_remoteproc.c | 3 +- drivers/remoteproc/pru_rproc.c | 1141 ++++++++++++++++++++ drivers/remoteproc/pru_rproc.h | 54 + drivers/remoteproc/qcom_q6v5_mss.c | 2 +- drivers/remoteproc/qcom_wcnss.c | 2 +- drivers/remoteproc/remoteproc_core.c | 108 +- drivers/remoteproc/remoteproc_debugfs.c | 3 + drivers/remoteproc/remoteproc_elf_loader.c | 117 +- drivers/remoteproc/remoteproc_internal.h | 2 +- drivers/remoteproc/remoteproc_sysfs.c | 33 +- drivers/remoteproc/st_slim_rproc.c | 2 +- drivers/remoteproc/wkup_m3_rproc.c | 3 +- drivers/rpmsg/Kconfig | 14 + drivers/rpmsg/Makefile | 1 + drivers/rpmsg/rpmsg_pru.c | 360 ++++++ drivers/rpmsg/virtio_rpmsg_bus.c | 21 +- drivers/soc/ti/Kconfig | 12 + drivers/soc/ti/Makefile | 1 + drivers/soc/ti/pruss.c | 347 ++++++ include/linux/irqchip/irq-pruss-intc.h | 94 ++ include/linux/pruss.h | 211 ++++ include/linux/remoteproc.h | 20 +- include/linux/remoteproc/pru_rproc.h | 57 + include/linux/rpmsg/virtio_rpmsg.h | 26 + include/uapi/linux/elf-em.h | 1 + 33 files changed, 3533 insertions(+), 71 deletions(-) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc-irq.txt create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.txt create mode 100644 Documentation/devicetree/bindings/soc/ti/ti,pruss.txt create mode 100644 drivers/irqchip/irq-pruss-intc.c create mode 100644 drivers/remoteproc/pru_rproc.c create mode 100644 drivers/remoteproc/pru_rproc.h create mode 100644 drivers/rpmsg/rpmsg_pru.c create mode 100644 drivers/soc/ti/pruss.c create mode 100644 include/linux/irqchip/irq-pruss-intc.h create mode 100644 include/linux/pruss.h create mode 100644 include/linux/remoteproc/pru_rproc.h create mode 100644 include/linux/rpmsg/virtio_rpmsg.h -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki