mbox series

[v2,00/14] Add support for TI PRU ICSS

Message ID 1549290167-876-1-git-send-email-rogerq@ti.com
Headers show
Series Add support for TI PRU ICSS | expand

Message

Roger Quadros Feb. 4, 2019, 2:22 p.m. UTC
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

Comments

Andrew Davis Feb. 4, 2019, 2:52 p.m. UTC | #1
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 */
>
Andrew Davis Feb. 4, 2019, 3:11 p.m. UTC | #2
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.

> +};
> +
> +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.

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 */
> +
>
Andrew Davis Feb. 4, 2019, 3:19 p.m. UTC | #3
On 2/4/19 8:22 AM, Roger Quadros wrote:
> From: David Lechner <david@lechnology.com>
> 
> This adds a special handler to the default remoteproc ELF firmware
> loader that looks up the memory map on TI PRU firmware files.
> 
> These processors have multiple memory maps that share the same address
> space, so we need to know the page in addition to the physical address
> in order to translate the address to a local CPU address.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  drivers/remoteproc/remoteproc_elf_loader.c | 117 +++++++++++++++++++++++++++--
>  include/uapi/linux/elf-em.h                |   1 +
>  2 files changed, 112 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
> index 8888d39..79c9d39 100644
> --- a/drivers/remoteproc/remoteproc_elf_loader.c
> +++ b/drivers/remoteproc/remoteproc_elf_loader.c
> @@ -32,6 +32,103 @@
>  
>  #include "remoteproc_internal.h"
>  
> +#define SHT_TI_PHATTRS 0x7F000004
> +#define SHT_TI_SH_PAGE 0x7F000007
> +
> +struct elf32_ti_phattrs {
> +	Elf32_Half pha_seg_id; /* Segment id */
> +	Elf32_Half pha_tag_id; /* Attribute kind id */
> +	union {
> +		Elf32_Off pha_offset; /* byte offset within the section */
> +		Elf32_Word pha_value; /* Constant tag value */
> +	} pha_un;
> +};
> +
> +/* this struct is reverse engineered, so not sure what most of the values are */

Do we really not know? Someone should go check this, if they are not
used then for now label them "unused", not "unknown".

Andrew

> +struct ti_section_page {
> +	u32 unk0;
> +	u32 unk1;
> +	u32 unk2;
> +	u32 unk3;
> +	u32 unk4;
> +	u16 size;
> +	u16 unk5;
> +	u16 unk6;
> +	u8 data[0]; /* array of size */
> +};
> +
> +/**
> + * rproc_elf_segment_to_map() - Gets memory map for segment
> + * @id: segment id
> + * @elf_data: pointer to ELF file data
> + *
> + * Returns the memory map for the segment.
> + */
> +static int rproc_elf_segment_to_map(u32 id, const u8 *elf_data)
> +{
> +	struct elf32_hdr *ehdr;
> +	struct elf32_shdr *shdr;
> +	struct elf32_ti_phattrs *ti_attrs = NULL;
> +	int i;
> +
> +	ehdr = (struct elf32_hdr *)elf_data;
> +	shdr = (struct elf32_shdr *)(elf_data + ehdr->e_shoff);
> +
> +	if (ehdr->e_machine != EM_TI_PRU)
> +		return 0;
> +
> +	for (i = 0; i < ehdr->e_shnum; i++, shdr++) {
> +		if (shdr->sh_type == SHT_TI_PHATTRS) {
> +			ti_attrs = (struct elf32_ti_phattrs *)(elf_data + shdr->sh_offset);
> +			break;
> +		}
> +	}
> +
> +	if (!ti_attrs)
> +		return 0;
> +
> +	/* list is terminated by tag id == 0 (PHA_NULL) */
> +	for (; ti_attrs->pha_tag_id; ti_attrs++) {
> +		if (ti_attrs->pha_tag_id == 3 && ti_attrs->pha_seg_id == id)
> +			return ti_attrs->pha_un.pha_value;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * rproc_elf_section_to_map() - Gets memory map for section
> + * @id: segment id
> + * @elf_data: pointer to ELF file data
> + *
> + * Returns the memory map for the section.
> + */
> +static int rproc_elf_section_to_map(u32 id, const u8 *elf_data)
> +{
> +	struct elf32_hdr *ehdr;
> +	struct elf32_shdr *shdr;
> +	struct ti_section_page *map = NULL;
> +	int i;
> +
> +	ehdr = (struct elf32_hdr *)elf_data;
> +	shdr = (struct elf32_shdr *)(elf_data + ehdr->e_shoff);
> +
> +	if (ehdr->e_machine != EM_TI_PRU)
> +		return 0;
> +
> +	for (i = 0; i < ehdr->e_shnum; i++, shdr++) {
> +		if (shdr->sh_type == SHT_TI_SH_PAGE) {
> +			map = (struct ti_section_page *)(elf_data + shdr->sh_offset);
> +			break;
> +		}
> +	}
> +
> +	if (!map || id >= map->size)
> +		return 0;
> +
> +	return map->data[id];
> +}
> +
>  /**
>   * rproc_elf_sanity_check() - Sanity Check ELF firmware image
>   * @rproc: the remote processor handle
> @@ -147,7 +244,7 @@ int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
>  	struct device *dev = &rproc->dev;
>  	struct elf32_hdr *ehdr;
>  	struct elf32_phdr *phdr;
> -	int i, ret = 0;
> +	int i, map, ret = 0;
>  	const u8 *elf_data = fw->data;
>  
>  	ehdr = (struct elf32_hdr *)elf_data;
> @@ -181,8 +278,10 @@ int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
>  			break;
>  		}
>  
> +		map = rproc_elf_segment_to_map(i, elf_data);
> +
>  		/* grab the kernel address for this device address */
> -		ptr = rproc_da_to_va(rproc, da, memsz, 0);
> +		ptr = rproc_da_to_va(rproc, da, memsz, map);
>  		if (!ptr) {
>  			dev_err(dev, "bad phdr da 0x%x mem 0x%x\n", da, memsz);
>  			ret = -EINVAL;
> @@ -209,7 +308,7 @@ int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
>  EXPORT_SYMBOL(rproc_elf_load_segments);
>  
>  static struct elf32_shdr *
> -find_table(struct device *dev, struct elf32_hdr *ehdr, size_t fw_size)
> +find_table(struct device *dev, struct elf32_hdr *ehdr, size_t fw_size, int *id)
>  {
>  	struct elf32_shdr *shdr;
>  	int i;
> @@ -261,6 +360,9 @@ find_table(struct device *dev, struct elf32_hdr *ehdr, size_t fw_size)
>  			return NULL;
>  		}
>  
> +		if (id)
> +			*id = i;
> +
>  		return shdr;
>  	}
>  
> @@ -288,7 +390,7 @@ int rproc_elf_load_rsc_table(struct rproc *rproc, const struct firmware *fw)
>  
>  	ehdr = (struct elf32_hdr *)elf_data;
>  
> -	shdr = find_table(dev, ehdr, fw->size);
> +	shdr = find_table(dev, ehdr, fw->size, NULL);
>  	if (!shdr)
>  		return -EINVAL;
>  
> @@ -328,11 +430,14 @@ struct resource_table *rproc_elf_find_loaded_rsc_table(struct rproc *rproc,
>  {
>  	struct elf32_hdr *ehdr = (struct elf32_hdr *)fw->data;
>  	struct elf32_shdr *shdr;
> +	int id, map;
>  
> -	shdr = find_table(&rproc->dev, ehdr, fw->size);
> +	shdr = find_table(&rproc->dev, ehdr, fw->size, &id);
>  	if (!shdr)
>  		return NULL;
>  
> -	return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size, 0);
> +	map = rproc_elf_section_to_map(id, fw->data);
> +
> +	return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size, map);
>  }
>  EXPORT_SYMBOL(rproc_elf_find_loaded_rsc_table);
> diff --git a/include/uapi/linux/elf-em.h b/include/uapi/linux/elf-em.h
> index 0c3000fa..70b487a 100644
> --- a/include/uapi/linux/elf-em.h
> +++ b/include/uapi/linux/elf-em.h
> @@ -38,6 +38,7 @@
>  #define EM_BLACKFIN     106     /* ADI Blackfin Processor */
>  #define EM_ALTERA_NIOS2	113	/* Altera Nios II soft-core processor */
>  #define EM_TI_C6000	140	/* TI C6X DSPs */
> +#define EM_TI_PRU	144	/* TI Programmable Realtime Unit */
>  #define EM_AARCH64	183	/* ARM 64 bit */
>  #define EM_TILEPRO	188	/* Tilera TILEPro */
>  #define EM_MICROBLAZE	189	/* Xilinx MicroBlaze */
>
Andrew Davis Feb. 4, 2019, 3:26 p.m. UTC | #4
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");
>
Roger Quadros Feb. 4, 2019, 3:32 p.m. UTC | #5
On 04/02/19 16:52, Andrew F. Davis wrote:
> 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.

Good catch.
> 
>> +
>> +	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.

Yes.

> 
>> +
>> +	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 ?

Will add.


cheers,
-roger

> 
> 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 */
>>
Roger Quadros Feb. 4, 2019, 3:33 p.m. UTC | #6
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 */
>> +
>>
Tony Lindgren Feb. 4, 2019, 4:35 p.m. UTC | #7
* Andrew F. Davis <afd@ti.com> [190204 14:52]:
> On 2/4/19 8:22 AM, Roger Quadros wrote:
> > From: Suman Anna <s-anna@ti.com>
> > +++ 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

Just a nitpick comment, we have n as the default already,
so default n can be dropped.

Regards,

Tony
Tony Lindgren Feb. 4, 2019, 6:15 p.m. UTC | #8
* 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
Roger Quadros Feb. 5, 2019, 8:51 a.m. UTC | #9
+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?

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 */
>>> +
>>>
>
Roger Quadros Feb. 5, 2019, 10:35 a.m. UTC | #10
On 04/02/19 20:15, Tony Lindgren wrote:
> * 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.

Good idea. I will send irqchip related patches separately.

cheers,
-roger
> 
> Regards,
> 
> Tony
>
Marc Zyngier Feb. 5, 2019, 11:04 a.m. UTC | #11
On Tue, 05 Feb 2019 10:35:44 +0000,
Roger Quadros <rogerq@ti.com> wrote:
> 
> On 04/02/19 20:15, Tony Lindgren wrote:
> > * 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.
> 
> Good idea. I will send irqchip related patches separately.

Yes please. But also please document why you have so many non
irq-related entry points in this irqchip driver. It seems to replicate
the same "events vs irq" stuff we're trying to get rid of in the K3
patches...

Thanks,

	M.
Suman Anna Feb. 14, 2019, 2:15 a.m. UTC | #12
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 */
>>>> +
>>>>
>>
>
Suman Anna Feb. 14, 2019, 2:16 a.m. UTC | #13
On 2/5/19 5:04 AM, Marc Zyngier wrote:
> On Tue, 05 Feb 2019 10:35:44 +0000,
> Roger Quadros <rogerq@ti.com> wrote:
>>
>> On 04/02/19 20:15, Tony Lindgren wrote:
>>> * 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.
>>
>> Good idea. I will send irqchip related patches separately.
> 
> Yes please. But also please document why you have so many non
> irq-related entry points in this irqchip driver. It seems to replicate
> the same "events vs irq" stuff we're trying to get rid of in the K3
> patches...

This is not the same, the whole INTC is a sub-module within the
sub-system serving interrupts to both the PRUs and the main host
processor. In anycase, we can add more details when we send out the
series separately.

regards
Suman
Suman Anna Feb. 14, 2019, 2:22 a.m. UTC | #14
On 2/4/19 9:19 AM, Andrew F. Davis wrote:
> On 2/4/19 8:22 AM, Roger Quadros wrote:
>> From: David Lechner <david@lechnology.com>
>>
>> This adds a special handler to the default remoteproc ELF firmware
>> loader that looks up the memory map on TI PRU firmware files.
>>
>> These processors have multiple memory maps that share the same address
>> space, so we need to know the page in addition to the physical address
>> in order to translate the address to a local CPU address.
>>
>> Signed-off-by: David Lechner <david@lechnology.com>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  drivers/remoteproc/remoteproc_elf_loader.c | 117 +++++++++++++++++++++++++++--
>>  include/uapi/linux/elf-em.h                |   1 +
>>  2 files changed, 112 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
>> index 8888d39..79c9d39 100644
>> --- a/drivers/remoteproc/remoteproc_elf_loader.c
>> +++ b/drivers/remoteproc/remoteproc_elf_loader.c
>> @@ -32,6 +32,103 @@
>>  
>>  #include "remoteproc_internal.h"
>>  
>> +#define SHT_TI_PHATTRS 0x7F000004
>> +#define SHT_TI_SH_PAGE 0x7F000007
>> +
>> +struct elf32_ti_phattrs {
>> +	Elf32_Half pha_seg_id; /* Segment id */
>> +	Elf32_Half pha_tag_id; /* Attribute kind id */
>> +	union {
>> +		Elf32_Off pha_offset; /* byte offset within the section */
>> +		Elf32_Word pha_value; /* Constant tag value */
>> +	} pha_un;
>> +};
>> +
>> +/* this struct is reverse engineered, so not sure what most of the values are */
> 
> Do we really not know? Someone should go check this, if they are not
> used then for now label them "unused", not "unknown".

I am following up internally within TI to get more concrete definitions
around this.

In anycase, on AM65x SoC, I actually had to implement a custom ELF
loading function due to the differences in hardware behavior [1], so I
think we should consider moving this whole logic into the PRU remoteproc
driver itself, and see if it is possible to keep the generic ELF loader
code clean and without having to introduce the flag parameter to
rproc_da_to_va() function.

regards
Suman

[1]
http://git.ti.com/gitweb/?p=ti-linux-kernel/ti-linux-kernel.git;a=commitdiff;h=529c7767b4f3cff0568c8867cb18b39526d07785

> 
> Andrew
> 
>> +struct ti_section_page {
>> +	u32 unk0;
>> +	u32 unk1;
>> +	u32 unk2;
>> +	u32 unk3;
>> +	u32 unk4;
>> +	u16 size;
>> +	u16 unk5;
>> +	u16 unk6;
>> +	u8 data[0]; /* array of size */
>> +};
>> +
>> +/**
>> + * rproc_elf_segment_to_map() - Gets memory map for segment
>> + * @id: segment id
>> + * @elf_data: pointer to ELF file data
>> + *
>> + * Returns the memory map for the segment.
>> + */
>> +static int rproc_elf_segment_to_map(u32 id, const u8 *elf_data)
>> +{
>> +	struct elf32_hdr *ehdr;
>> +	struct elf32_shdr *shdr;
>> +	struct elf32_ti_phattrs *ti_attrs = NULL;
>> +	int i;
>> +
>> +	ehdr = (struct elf32_hdr *)elf_data;
>> +	shdr = (struct elf32_shdr *)(elf_data + ehdr->e_shoff);
>> +
>> +	if (ehdr->e_machine != EM_TI_PRU)
>> +		return 0;
>> +
>> +	for (i = 0; i < ehdr->e_shnum; i++, shdr++) {
>> +		if (shdr->sh_type == SHT_TI_PHATTRS) {
>> +			ti_attrs = (struct elf32_ti_phattrs *)(elf_data + shdr->sh_offset);
>> +			break;
>> +		}
>> +	}
>> +
>> +	if (!ti_attrs)
>> +		return 0;
>> +
>> +	/* list is terminated by tag id == 0 (PHA_NULL) */
>> +	for (; ti_attrs->pha_tag_id; ti_attrs++) {
>> +		if (ti_attrs->pha_tag_id == 3 && ti_attrs->pha_seg_id == id)
>> +			return ti_attrs->pha_un.pha_value;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * rproc_elf_section_to_map() - Gets memory map for section
>> + * @id: segment id
>> + * @elf_data: pointer to ELF file data
>> + *
>> + * Returns the memory map for the section.
>> + */
>> +static int rproc_elf_section_to_map(u32 id, const u8 *elf_data)
>> +{
>> +	struct elf32_hdr *ehdr;
>> +	struct elf32_shdr *shdr;
>> +	struct ti_section_page *map = NULL;
>> +	int i;
>> +
>> +	ehdr = (struct elf32_hdr *)elf_data;
>> +	shdr = (struct elf32_shdr *)(elf_data + ehdr->e_shoff);
>> +
>> +	if (ehdr->e_machine != EM_TI_PRU)
>> +		return 0;
>> +
>> +	for (i = 0; i < ehdr->e_shnum; i++, shdr++) {
>> +		if (shdr->sh_type == SHT_TI_SH_PAGE) {
>> +			map = (struct ti_section_page *)(elf_data + shdr->sh_offset);
>> +			break;
>> +		}
>> +	}
>> +
>> +	if (!map || id >= map->size)
>> +		return 0;
>> +
>> +	return map->data[id];
>> +}
>> +
>>  /**
>>   * rproc_elf_sanity_check() - Sanity Check ELF firmware image
>>   * @rproc: the remote processor handle
>> @@ -147,7 +244,7 @@ int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
>>  	struct device *dev = &rproc->dev;
>>  	struct elf32_hdr *ehdr;
>>  	struct elf32_phdr *phdr;
>> -	int i, ret = 0;
>> +	int i, map, ret = 0;
>>  	const u8 *elf_data = fw->data;
>>  
>>  	ehdr = (struct elf32_hdr *)elf_data;
>> @@ -181,8 +278,10 @@ int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
>>  			break;
>>  		}
>>  
>> +		map = rproc_elf_segment_to_map(i, elf_data);
>> +
>>  		/* grab the kernel address for this device address */
>> -		ptr = rproc_da_to_va(rproc, da, memsz, 0);
>> +		ptr = rproc_da_to_va(rproc, da, memsz, map);
>>  		if (!ptr) {
>>  			dev_err(dev, "bad phdr da 0x%x mem 0x%x\n", da, memsz);
>>  			ret = -EINVAL;
>> @@ -209,7 +308,7 @@ int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
>>  EXPORT_SYMBOL(rproc_elf_load_segments);
>>  
>>  static struct elf32_shdr *
>> -find_table(struct device *dev, struct elf32_hdr *ehdr, size_t fw_size)
>> +find_table(struct device *dev, struct elf32_hdr *ehdr, size_t fw_size, int *id)
>>  {
>>  	struct elf32_shdr *shdr;
>>  	int i;
>> @@ -261,6 +360,9 @@ find_table(struct device *dev, struct elf32_hdr *ehdr, size_t fw_size)
>>  			return NULL;
>>  		}
>>  
>> +		if (id)
>> +			*id = i;
>> +
>>  		return shdr;
>>  	}
>>  
>> @@ -288,7 +390,7 @@ int rproc_elf_load_rsc_table(struct rproc *rproc, const struct firmware *fw)
>>  
>>  	ehdr = (struct elf32_hdr *)elf_data;
>>  
>> -	shdr = find_table(dev, ehdr, fw->size);
>> +	shdr = find_table(dev, ehdr, fw->size, NULL);
>>  	if (!shdr)
>>  		return -EINVAL;
>>  
>> @@ -328,11 +430,14 @@ struct resource_table *rproc_elf_find_loaded_rsc_table(struct rproc *rproc,
>>  {
>>  	struct elf32_hdr *ehdr = (struct elf32_hdr *)fw->data;
>>  	struct elf32_shdr *shdr;
>> +	int id, map;
>>  
>> -	shdr = find_table(&rproc->dev, ehdr, fw->size);
>> +	shdr = find_table(&rproc->dev, ehdr, fw->size, &id);
>>  	if (!shdr)
>>  		return NULL;
>>  
>> -	return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size, 0);
>> +	map = rproc_elf_section_to_map(id, fw->data);
>> +
>> +	return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size, map);
>>  }
>>  EXPORT_SYMBOL(rproc_elf_find_loaded_rsc_table);
>> diff --git a/include/uapi/linux/elf-em.h b/include/uapi/linux/elf-em.h
>> index 0c3000fa..70b487a 100644
>> --- a/include/uapi/linux/elf-em.h
>> +++ b/include/uapi/linux/elf-em.h
>> @@ -38,6 +38,7 @@
>>  #define EM_BLACKFIN     106     /* ADI Blackfin Processor */
>>  #define EM_ALTERA_NIOS2	113	/* Altera Nios II soft-core processor */
>>  #define EM_TI_C6000	140	/* TI C6X DSPs */
>> +#define EM_TI_PRU	144	/* TI Programmable Realtime Unit */
>>  #define EM_AARCH64	183	/* ARM 64 bit */
>>  #define EM_TILEPRO	188	/* Tilera TILEPro */
>>  #define EM_MICROBLAZE	189	/* Xilinx MicroBlaze */
>>
Suman Anna Feb. 14, 2019, 2:35 a.m. UTC | #15
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).

> 
> [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>
> ---
>  drivers/remoteproc/Kconfig           |  14 +
>  drivers/remoteproc/Makefile          |   1 +
>  drivers/remoteproc/pru_rproc.c       | 930 +++++++++++++++++++++++++++++++++++
>  drivers/remoteproc/pru_rproc.h       |  54 ++
>  include/linux/remoteproc/pru_rproc.h |  27 +
>  5 files changed, 1026 insertions(+)
>  create mode 100644 drivers/remoteproc/pru_rproc.c
>  create mode 100644 drivers/remoteproc/pru_rproc.h
>  create mode 100644 include/linux/remoteproc/pru_rproc.h
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index f0abd26..333666e 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -197,6 +197,20 @@ config ST_REMOTEPROC
>  config ST_SLIM_REMOTEPROC
>  	tristate
>  
> +config PRUSS_REMOTEPROC
> +	tristate "TI PRUSS remoteproc support"
> +	depends on TI_PRUSS
> +	default n
> +	help
> +	  Support for TI PRU-ICSS remote processors via the remote processor
> +	  framework.
> +
> +	  Currently supported on AM33xx SoCs.
> +
> +	  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 # REMOTEPROC
>  
>  endmenu
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index ce5d061..88a86cc 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -26,3 +26,4 @@ qcom_wcnss_pil-y			+= qcom_wcnss.o
>  qcom_wcnss_pil-y			+= qcom_wcnss_iris.o
>  obj-$(CONFIG_ST_REMOTEPROC)		+= st_remoteproc.o
>  obj-$(CONFIG_ST_SLIM_REMOTEPROC)	+= st_slim_rproc.o
> +obj-$(CONFIG_PRUSS_REMOTEPROC)		+= pru_rproc.o
> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
> new file mode 100644
> index 0000000..ddd4b64
> --- /dev/null
> +++ b/drivers/remoteproc/pru_rproc.c
> @@ -0,0 +1,930 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PRU-ICSS remoteproc 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/bitops.h>
> +#include <linux/debugfs.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqchip/irq-pruss-intc.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
> +#include <linux/remoteproc.h>
> +#include <linux/remoteproc/pru_rproc.h>
> +#include <linux/pruss.h>
> +
> +#include "remoteproc_internal.h"
> +#include "pru_rproc.h"
> +
> +/* ELF PAGE ID */
> +#define PRU_PAGE_IRAM	0
> +#define PRU_PAGE_DRAM	1
> +
> +/* IRAM offsets */
> +#define PRU0_IRAM_START		0x34000
> +#define PRU1_IRAM_START		0x38000
> +#define PRU_IRAM_MASK		0x3ffff
> +
> +/* PRU_ICSS_PRU_CTRL registers */
> +#define PRU_CTRL_CTRL		0x0000
> +#define PRU_CTRL_STS		0x0004
> +#define PRU_CTRL_WAKEUP_EN	0x0008
> +#define PRU_CTRL_CYCLE		0x000C
> +#define PRU_CTRL_STALL		0x0010
> +#define PRU_CTRL_CTBIR0		0x0020
> +#define PRU_CTRL_CTBIR1		0x0024
> +#define PRU_CTRL_CTPPR0		0x0028
> +#define PRU_CTRL_CTPPR1		0x002C
> +
> +/* CTRL register bit-fields */
> +#define CTRL_CTRL_SOFT_RST_N	BIT(0)
> +#define CTRL_CTRL_EN		BIT(1)
> +#define CTRL_CTRL_SLEEPING	BIT(2)
> +#define CTRL_CTRL_CTR_EN	BIT(3)
> +#define CTRL_CTRL_SINGLE_STEP	BIT(8)
> +#define CTRL_CTRL_RUNSTATE	BIT(15)
> +
> +/* PRU_ICSS_PRU_DEBUG registers */
> +#define PRU_DEBUG_GPREG(x)	(0x0000 + (x) * 4)
> +#define PRU_DEBUG_CT_REG(x)	(0x0080 + (x) * 4)
> +
> +/**
> + * struct pru_rproc - PRU remoteproc structure
> + * @id: id of the PRU core within the PRUSS
> + * @pruss: back-reference to parent PRUSS structure
> + * @rproc: remoteproc pointer for this PRU core
> + * @iram_region: PRU IRAM IOMEM
> + * @ctrl_regmap: regmap to PRU CTRL IOMEM
> + * @debug_regmap: regmap to PRU DEBUG IOMEM
> + * @client_np: client device node
> + * @intc_config: PRU INTC configuration data
> + * @dram0: PRUSS DRAM0 region
> + * @dram1: PRUSS DRAM1 region
> + * @shrdram: PRUSS SHARED RAM region
> + * @iram_da: device address of Instruction RAM for this PRU
> + * @pdram_da: device address of primary Data RAM for this PRU
> + * @sdram_da: device address of secondary Data RAM for this PRU
> + * @shrdram_da: device address of shared Data RAM
> + * @cfg: regmap to CFG module
> + * @gpcfg_reg: offset to gpcfg register of this PRU
> + * @fw_name: name of firmware image used during loading
> + * @gpmux_save: saved value for gpmux config
> + * @dt_irqs: number of irqs configured from DT
> + * @fw_irqs: number of irqs configured from FW
> + * @lock: mutex to protect client usage
> + * @dbg_single_step: debug state variable to set PRU into single step mode
> + * @ctrl_saved_state: saved CTRL state to return to normal mode
> + */
> +struct pru_rproc {
> +	int id;
> +	struct pruss *pruss;
> +	struct rproc *rproc;
> +	struct pruss_mem_region iram_region;
> +	struct regmap *ctrl_regmap;
> +	struct regmap *debug_regmap;
> +	struct device_node *client_np;
> +	struct pruss_intc_config intc_config;
> +	struct pruss_mem_region dram0;
> +	struct pruss_mem_region dram1;
> +	struct pruss_mem_region shrdram;
> +	u32 iram_da;
> +	u32 pdram_da;
> +	u32 sdram_da;
> +	u32 shrdram_da;
> +	struct regmap *cfg;
> +	int gpcfg_reg;
> +	const char *fw_name;
> +	u8 gpmux_save;
> +	int dt_irqs;
> +	int fw_irqs;
> +	struct mutex lock; /* client access lock */
> +	bool dbg_single_step;
> +	u32 ctrl_saved_state;
> +};
> +
> +/**
> + * pru_rproc_set_firmware() - set firmware for a pru core
> + * @rproc: the rproc instance of the PRU
> + * @fw_name: the new firmware name, or NULL if default is desired
> + */
> +static int pru_rproc_set_firmware(struct rproc *rproc, const char *fw_name)
> +{
> +	struct pru_rproc *pru = rproc->priv;
> +
> +	if (!fw_name)
> +		fw_name = pru->fw_name;
> +
> +	return rproc_set_firmware(rproc, fw_name);
> +}
> +
> +/*
> + * pru_get_gpmux() - get the current GPMUX value for a PRU device
> + * @pru: pru_rproc instance
> + * @mux: pointer to store the current mux value into
> + *
> + * returns 0 on success, negative error value otherwise.
> + */
> +static int pru_get_gpmux(struct pru_rproc *pru, u8 *mux)
> +{
> +	int ret;
> +	unsigned int val;
> +
> +	ret = regmap_read(pru->cfg, pru->gpcfg_reg, &val);
> +	if (ret)
> +		return ret;
> +
> +	*mux = (u8)((val & PRUSS_GPCFG_PRU_MUX_SEL_MASK) >>
> +		    PRUSS_GPCFG_PRU_MUX_SEL_SHIFT);
> +
> +	return 0;
> +}
> +
> +/**
> + * pru_set_gpmux() - set the GPMUX value for a PRU device
> + * @pru: pru_rproc instance
> + * @mux: new mux value for PRU
> + *
> + * returns 0 on success, negative error value otherwise.
> + */
> +static int pru_set_gpmux(struct pru_rproc *pru, u8 mux)
> +{
> +	if (mux >= PRUSS_GP_MUX_SEL_MAX)
> +		return -EINVAL;
> +
> +	return regmap_update_bits(pru->cfg, pru->gpcfg_reg,
> +				  PRUSS_GPCFG_PRU_MUX_SEL_MASK,
> +				  (u32)mux << PRUSS_GPCFG_PRU_MUX_SEL_SHIFT);
> +}
> +
> +static int pru_get_intc_dt_config(struct pru_rproc *pru,
> +				  const char *propname, int index,
> +				  struct pruss_intc_config *intc_config)
> +{
> +	struct device *dev = &pru->rproc->dev;
> +	struct device_node *np = pru->client_np;
> +	struct property *prop;
> +	int ret = 0, entries, i;
> +	int dt_irqs = 0;
> +	u32 *arr;
> +	int max_system_events, max_pru_channels, max_pru_host_ints;
> +
> +	max_system_events = MAX_PRU_SYS_EVENTS;
> +	max_pru_channels = MAX_PRU_CHANNELS;
> +	max_pru_host_ints = MAX_PRU_CHANNELS;
> +
> +	prop = of_find_property(np, propname, NULL);
> +	if (!prop)
> +		return 0;
> +
> +	entries = of_property_count_u32_elems(np, propname);
> +	if (entries <= 0 || entries % 4)
> +		return -EINVAL;
> +
> +	arr = kmalloc_array(entries, sizeof(u32), GFP_KERNEL);
> +	if (!arr)
> +		return -ENOMEM;
> +
> +	ret = of_property_read_u32_array(np, propname, arr, entries);
> +	if (ret)
> +		return -EINVAL;
> +
> +	for (i = 0; i < ARRAY_SIZE(intc_config->sysev_to_ch); i++)
> +		intc_config->sysev_to_ch[i] = -1;
> +
> +	for (i = 0; i < ARRAY_SIZE(intc_config->ch_to_host); i++)
> +		intc_config->ch_to_host[i] = -1;
> +
> +	for (i = 0; i < entries; i += 4) {
> +		if (arr[i] != index)
> +			continue;
> +
> +		if (arr[i + 1] < 0 ||
> +		    arr[i + 1] >= max_system_events) {
> +			dev_err(dev, "bad sys event %d\n", arr[i + 1]);
> +			ret = -EINVAL;
> +			goto err;
> +		}
> +
> +		if (arr[i + 2] < 0 ||
> +		    arr[i + 2] >= max_pru_channels) {
> +			dev_err(dev, "bad channel %d\n", arr[i + 2]);
> +			ret = -EINVAL;
> +			goto err;
> +		}
> +
> +		if (arr[i + 3] < 0 ||
> +		    arr[i + 3] >= max_pru_host_ints) {
> +			dev_err(dev, "bad irq %d\n", arr[i + 3]);
> +			ret = -EINVAL;
> +			goto err;
> +		}
> +
> +		intc_config->sysev_to_ch[arr[i + 1]] = arr[i + 2];
> +		dev_dbg(dev, "sysevt-to-ch[%d] -> %d\n", arr[i + 1],
> +			arr[i + 2]);
> +
> +		intc_config->ch_to_host[arr[i + 2]] = arr[i + 3];
> +		dev_dbg(dev, "chnl-to-host[%d] -> %d\n", arr[i + 2],
> +			arr[i + 3]);
> +
> +		dt_irqs++;
> +	}
> +
> +	kfree(arr);
> +	return dt_irqs;
> +
> +err:
> +	kfree(arr);
> +	return ret;
> +}
> +
> +static struct rproc *__pru_rproc_get(struct device_node *np, int index)
> +{
> +	struct device_node *rproc_np = NULL;
> +	struct platform_device *pdev;
> +	struct rproc *rproc;
> +
> +	rproc_np = of_parse_phandle(np, "prus", index);
> +	if (!rproc_np || !of_device_is_available(rproc_np))
> +		return ERR_PTR(-ENODEV);
> +
> +	pdev = of_find_device_by_node(rproc_np);
> +	of_node_put(rproc_np);
> +
> +	if (!pdev)
> +		/* probably PRU not yet probed */
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	/* TODO: replace the crude string based check to make sure it is PRU */
> +	if (!strstr(dev_name(&pdev->dev), "pru")) {
> +		put_device(&pdev->dev);
> +		return ERR_PTR(-ENODEV);
> +	}
> +
> +	rproc = platform_get_drvdata(pdev);
> +	put_device(&pdev->dev);
> +	if (!rproc)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	get_device(&rproc->dev);
> +
> +	return rproc;
> +}
> +
> +/**
> + * pru_rproc_get() - get the PRU rproc instance from a device node
> + * @np: the user/client device node
> + * @index: index to use for the prus property
> + *
> + * This function looks through a client device node's "prus" property at index
> + * @index and returns the rproc handle for a valid PRU remote processor if
> + * found. The function allows only one user to own the PRU rproc resource at
> + * a time. Caller must call pru_rproc_put() when done with using the rproc,
> + * not required if the function returns a failure.
> + *
> + * Returns the rproc handle on success, and an ERR_PTR on failure using one
> + * of the following error values
> + *    -ENODEV if device is not found
> + *    -EBUSY if PRU is already acquired by anyone
> + *    -EPROBE_DEFER is PRU device is not probed yet
> + */
> +struct rproc *pru_rproc_get(struct device_node *np, int index)
> +{
> +	struct rproc *rproc;
> +	struct pru_rproc *pru;
> +	struct device *dev;
> +	int ret;
> +	u32 mux;
> +	const char *fw_name;
> +
> +	rproc = __pru_rproc_get(np, index);
> +	if (IS_ERR(rproc))
> +		return rproc;
> +
> +	pru = rproc->priv;
> +	dev = &rproc->dev;
> +
> +	mutex_lock(&pru->lock);
> +
> +	if (pru->client_np) {
> +		mutex_unlock(&pru->lock);
> +		put_device(&rproc->dev);
> +		return ERR_PTR(-EBUSY);
> +	}
> +
> +	pru->client_np = np;
> +
> +	mutex_unlock(&pru->lock);
> +
> +	ret = pru_get_gpmux(pru, &pru->gpmux_save);
> +	if (ret) {
> +		dev_err(dev, "failed to get cfg gpmux: %d\n", ret);
> +		goto err;
> +	}
> +
> +	ret = of_property_read_u32_index(np, "ti,pruss-gp-mux-sel", index,
> +					 &mux);
> +	if (!ret) {
> +		ret = pru_set_gpmux(pru, mux);
> +		if (ret) {
> +			dev_err(dev, "failed to set cfg gpmux: %d\n", ret);
> +			goto err;
> +		}
> +	}
> +
> +	ret = of_property_read_string_index(np, "firmware-name", index,
> +					    &fw_name);
> +	if (!ret) {
> +		ret = pru_rproc_set_firmware(rproc, fw_name);
> +		if (ret) {
> +			dev_err(dev, "failed to set firmware: %d\n", ret);
> +			goto err;
> +		}
> +	}
> +
> +	ret = pru_get_intc_dt_config(pru, "ti,pru-interrupt-map",
> +				     index, &pru->intc_config);
> +	if (ret < 0) {
> +		dev_err(dev, "error getting DT interrupt map: %d\n", ret);
> +		goto err;
> +	}
> +
> +	pru->dt_irqs = ret;
> +
> +	return rproc;
> +
> +err:
> +	pru_rproc_put(rproc);
> +	return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(pru_rproc_get);
> +
> +/**
> + * pru_rproc_put() - release the PRU rproc resource
> + * @rproc: the rproc resource to release
> + *
> + * Releases the PRU rproc resource and makes it available to other
> + * users.
> + */
> +void pru_rproc_put(struct rproc *rproc)
> +{
> +	struct pru_rproc *pru;
> +
> +	if (IS_ERR_OR_NULL(rproc))
> +		return;
> +
> +	/* TODO: replace the crude string based check to make sure it is PRU */
> +	if (!strstr(dev_name(rproc->dev.parent), "pru"))
> +		return;
> +
> +	pru = rproc->priv;
> +	if (!pru->client_np)
> +		return;
> +
> +	pru_rproc_set_firmware(rproc, NULL);
> +	pru_set_gpmux(pru, pru->gpmux_save);
> +
> +	mutex_lock(&pru->lock);
> +	pru->client_np = NULL;
> +	mutex_unlock(&pru->lock);
> +
> +	put_device(&rproc->dev);
> +}
> +EXPORT_SYMBOL_GPL(pru_rproc_put);
> +
> +/*
> + * Control PRU single-step mode
> + *
> + * This is a debug helper function used for controlling the single-step
> + * mode of the PRU. The PRU Debug registers are not accessible when the
> + * PRU is in RUNNING state.
> + *
> + * Writing a non-zero value sets the PRU into single-step mode irrespective
> + * of its previous state. The PRU mode is saved only on the first set into
> + * a single-step mode. Writing a zero value will restore the PRU into its
> + * original mode.
> + */
> +static int pru_rproc_debug_ss_set(void *data, u64 val)
> +{
> +	struct rproc *rproc = data;
> +	struct pru_rproc *pru = rproc->priv;
> +	u32 reg_val;
> +	bool debug_ss;
> +
> +	debug_ss = !!val;
> +	if (!debug_ss && !pru->dbg_single_step)
> +		return 0;
> +
> +	regmap_read(pru->ctrl_regmap, PRU_CTRL_CTRL, &reg_val);
> +
> +	if (debug_ss && !pru->dbg_single_step)
> +		pru->ctrl_saved_state = reg_val;
> +
> +	if (debug_ss)
> +		reg_val |= CTRL_CTRL_SINGLE_STEP | CTRL_CTRL_EN;
> +	else
> +		reg_val = pru->ctrl_saved_state;
> +
> +	pru->dbg_single_step = debug_ss;
> +	regmap_write(pru->ctrl_regmap, PRU_CTRL_CTRL, reg_val);
> +
> +	return 0;
> +}
> +
> +static int pru_rproc_debug_ss_get(void *data, u64 *val)
> +{
> +	struct rproc *rproc = data;
> +	struct pru_rproc *pru = rproc->priv;
> +
> +	*val = pru->dbg_single_step;
> +
> +	return 0;
> +}
> +DEFINE_SIMPLE_ATTRIBUTE(pru_rproc_debug_ss_fops, pru_rproc_debug_ss_get,
> +			pru_rproc_debug_ss_set, "%llu\n");
> +
> +/*
> + * Create PRU-specific debugfs entries
> + *
> + * The entries are created only if the parent remoteproc debugfs directory
> + * exists, and will be cleaned up by the remoteproc core.
> + */
> +static void pru_rproc_create_debug_entries(struct rproc *rproc)
> +{
> +	if (!rproc->dbg_dir)
> +		return;
> +
> +	debugfs_create_file("single_step", 0600, rproc->dbg_dir,
> +			    rproc, &pru_rproc_debug_ss_fops);
> +}
> +
> +static void *pru_d_da_to_va(struct pru_rproc *pru, u32 da, int len);
> +
> +/*
> + * parse the custom interrupt map resource and save the intc_config
> + * for use when booting the processor.
> + */
> +static int pru_handle_vendor_intrmap(struct rproc *rproc,
> +				     struct fw_rsc_pruss_intrmap *rsc)
> +{
> +	int fw_irqs = 0, i, ret = 0;
> +	u8 *arr;
> +	struct device *dev = &rproc->dev;
> +	struct pru_rproc *pru = rproc->priv;
> +
> +	dev_dbg(dev, "vendor rsc intc: version %d\n", rsc->version);
> +
> +	/*
> +	 * 0 was prototyping version. Not supported.
> +	 * 1 is currently supported version.
> +	 */
> +	if (rsc->version == 0 || rsc->version > 1) {
> +		dev_err(dev, "Unsupported version %d\n", rsc->version);
> +		return -EINVAL;
> +	}
> +
> +	/* DT provided INTC config takes precedence */
> +	if (pru->dt_irqs) {
> +		dev_info(dev, "INTC config in DT and FW. Using DT config.\n");
> +		return 0;
> +	}
> +
> +	arr = rsc->data;
> +
> +	for (i = 0; i < ARRAY_SIZE(pru->intc_config.sysev_to_ch); i++)
> +		pru->intc_config.sysev_to_ch[i] = -1;
> +
> +	for (i = 0; i < ARRAY_SIZE(pru->intc_config.ch_to_host); i++)
> +		pru->intc_config.ch_to_host[i] = -1;
> +
> +	for (i = 0; i < rsc->num_maps * 3; i += 3) {
> +		if (arr[i] < 0 ||
> +		    arr[i] >= MAX_PRU_SYS_EVENTS) {
> +			dev_err(dev, "bad sys event %d\n", arr[i]);
> +			ret = -EINVAL;
> +			goto err;
> +		}
> +
> +		if (arr[i + 1] < 0 ||
> +		    arr[i + 1] >= MAX_PRU_CHANNELS) {
> +			dev_err(dev, "bad channel %d\n", arr[i + 1]);
> +			ret = -EINVAL;
> +			goto err;
> +		}
> +
> +		if (arr[i + 2] < 0 ||
> +		    arr[i + 2] >= MAX_PRU_CHANNELS) {
> +			dev_err(dev, "bad host irq %d\n", arr[i + 2]);
> +				ret = -EINVAL;
> +			goto err;
> +		}
> +
> +		pru->intc_config.sysev_to_ch[arr[i]] = arr[i + 1];
> +		dev_dbg(dev, "sysevt-to-ch[%d] -> %d\n", arr[i],
> +			arr[i + 1]);
> +
> +		pru->intc_config.ch_to_host[arr[i + 1]] = arr[i + 2];
> +		dev_dbg(dev, "chnl-to-host[%d] -> %d\n", arr[i + 1],
> +			arr[i + 2]);
> +
> +		fw_irqs++;
> +	}
> +
> +	pru->fw_irqs = fw_irqs;
> +	return 0;
> +
> +err:
> +	pru->fw_irqs = 0;
> +	return ret;
> +}
> +
> +/* PRU-specific vendor resource handler */
> +static int pru_rproc_handle_vendor_rsc(struct rproc *rproc,
> +				       struct fw_rsc_vendor *ven_rsc)
> +{
> +	struct device *dev = rproc->dev.parent;
> +	int ret = -EINVAL;
> +
> +	struct fw_rsc_pruss_intrmap *rsc;
> +
> +	rsc = (struct fw_rsc_pruss_intrmap *)ven_rsc->data;
> +
> +	switch (rsc->type) {
> +	case PRUSS_RSC_INTRS:
> +		ret = pru_handle_vendor_intrmap(rproc, rsc);
> +		break;
> +	default:
> +		dev_err(dev, "%s: cannot handle unknown type %d\n", __func__,
> +			rsc->type);
> +	}
> +
> +	return ret;
> +}
> +
> +/* start a PRU core */
> +static int pru_rproc_start(struct rproc *rproc)
> +{
> +	struct device *dev = &rproc->dev;
> +	struct pru_rproc *pru = rproc->priv;
> +	u32 val;
> +	int ret;
> +
> +	dev_dbg(dev, "starting PRU%d: entry-point = 0x%x\n",
> +		pru->id, (rproc->bootaddr >> 2));
> +
> +	if (pru->dt_irqs || pru->fw_irqs) {
> +		ret = pruss_intc_configure(rproc->dev.parent,
> +					   &pru->intc_config);
> +		if (ret) {
> +			dev_err(dev, "failed to configure intc %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	val = CTRL_CTRL_EN | ((rproc->bootaddr >> 2) << 16);
> +	regmap_write(pru->ctrl_regmap, PRU_CTRL_CTRL, val);
> +
> +	return 0;
> +}
> +
> +/* stop/disable a PRU core */
> +static int pru_rproc_stop(struct rproc *rproc)
> +{
> +	struct device *dev = &rproc->dev;
> +	struct pru_rproc *pru = rproc->priv;
> +
> +	dev_dbg(dev, "stopping PRU%d\n", pru->id);
> +	regmap_update_bits(pru->ctrl_regmap, PRU_CTRL_CTRL, CTRL_CTRL_EN, 0);
> +
> +	/* undo INTC config */
> +	if (pru->dt_irqs || pru->fw_irqs)
> +		pruss_intc_unconfigure(rproc->dev.parent, &pru->intc_config);
> +
> +	return 0;
> +}
> +
> +/*
> + * Convert PRU device address (data spaces only) to kernel virtual address
> + *
> + * Each PRU has access to all data memories within the PRUSS, accessible at
> + * different ranges. So, look through both its primary and secondary Data
> + * RAMs as well as any shared Data RAM to convert a PRU device address to
> + * kernel virtual address. Data RAM0 is primary Data RAM for PRU0 and Data
> + * RAM1 is primary Data RAM for PRU1.
> + */
> +static void *pru_d_da_to_va(struct pru_rproc *pru, u32 da, int len)
> +{
> +	struct pruss_mem_region dram0, dram1, shrd_ram;
> +	u32 offset;
> +	void *va = NULL;
> +
> +	if (len <= 0)
> +		return NULL;
> +
> +	dram0 = pru->dram0;
> +	dram1 = pru->dram1;
> +	/* PRU1 has its local RAM addresses reversed */
> +	if (pru->id == 1)
> +		swap(dram0, dram1);
> +	shrd_ram = pru->shrdram;
> +
> +	if (da >= pru->pdram_da && da + len <= pru->pdram_da + dram0.size) {
> +		offset = da - pru->pdram_da;
> +		va = (__force void *)(dram0.va + offset);
> +	} else if (da >= pru->sdram_da &&
> +		   da + len <= pru->sdram_da + dram1.size) {
> +		offset = da - pru->sdram_da;
> +		va = (__force void *)(dram1.va + offset);
> +	} else if (da >= pru->shrdram_da &&
> +		   da + len <= pru->shrdram_da + shrd_ram.size) {
> +		offset = da - pru->shrdram_da;
> +		va = (__force void *)(shrd_ram.va + offset);
> +	}
> +
> +	return va;
> +}
> +
> +/*
> + * Convert PRU device address (instruction space) to kernel virtual address
> + *
> + * A PRU does not have an unified address space. Each PRU has its very own
> + * private Instruction RAM, and its device address is identical to that of
> + * its primary Data RAM device address.
> + */
> +static void *pru_i_da_to_va(struct pru_rproc *pru, u32 da, int len)
> +{
> +	u32 offset;
> +	void *va = NULL;
> +
> +	if (len <= 0)
> +		return NULL;
> +
> +	if (da >= pru->iram_da &&
> +	    da + len <= pru->iram_da + pru->iram_region.size) {
> +		offset = da - pru->iram_da;
> +		va = (__force void *)(pru->iram_region.va + offset);
> +	}
> +
> +	return va;
> +}
> +
> +/* PRU-specific address translator */
> +static void *pru_da_to_va(struct rproc *rproc, u64 da, int len, int map)
> +{
> +	struct pru_rproc *pru = rproc->priv;
> +	void *va;
> +
> +	switch (map) {
> +	case PRU_PAGE_IRAM:
> +		va = pru_i_da_to_va(pru, da, len);
> +		break;
> +	case PRU_PAGE_DRAM:
> +		va = pru_d_da_to_va(pru, da, len);
> +		break;
> +	default:
> +		dev_info(&rproc->dev, "%s: invalid page %d\n", __func__, map);
> +		return 0;
> +	};
> +
> +	return va;
> +}
> +
> +static struct rproc_ops pru_rproc_ops = {
> +	.start			= pru_rproc_start,
> +	.stop			= pru_rproc_stop,
> +	.da_to_va		= pru_da_to_va,
> +	.handle_vendor_rsc	= pru_rproc_handle_vendor_rsc,
> +};
> +
> +static int pru_rproc_set_id(struct pru_rproc *pru)
> +{
> +	int ret = 0;
> +	u32 iram_start = pru->iram_region.pa & PRU_IRAM_MASK;
> +
> +	if (iram_start == PRU0_IRAM_START)
> +		pru->id = 0;
> +	else if (iram_start == PRU1_IRAM_START)
> +		pru->id = 1;
> +	else
> +		ret = -EINVAL;
> +
> +	return ret;
> +}
> +
> +static struct regmap_config pru_ctrl_regmap_config = {
> +	.name = "ctrl",
> +	.reg_bits = 32,
> +	.reg_stride = 4,
> +	.val_bits = 32,
> +	.max_register = PRU_CTRL_CTPPR1,
> +};
> +
> +static struct regmap_config pru_debug_regmap_config = {
> +	.name = "debug",
> +	.reg_bits = 32,
> +	.reg_stride = 4,
> +	.val_bits = 32,
> +	.max_register = PRU_DEBUG_CT_REG(31),
> +};
> +
> +static int pru_rproc_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	struct platform_device *ppdev = to_platform_device(dev->parent);
> +	struct pru_rproc *pru;
> +	const char *fw_name;
> +	struct rproc *rproc = NULL;
> +	struct resource *res;
> +	int ret;
> +	void __iomem *va;
> +
> +	if (!np) {
> +		dev_err(dev, "Non-DT platform device not supported\n");
> +		return -ENODEV;
> +	}
> +
> +	ret = of_property_read_string(np, "firmware-name", &fw_name);
> +	if (ret) {
> +		dev_err(dev, "unable to retrieve firmware-name %d\n", ret);
> +		return ret;
> +	}
> +
> +	rproc = rproc_alloc(dev, pdev->name, &pru_rproc_ops, fw_name,
> +			    sizeof(*pru));
> +	if (!rproc) {
> +		dev_err(dev, "rproc_alloc failed\n");
> +		return -ENOMEM;
> +	}
> +
> +	pru = rproc->priv;
> +	pru->cfg = syscon_regmap_lookup_by_phandle(np, "gpcfg");
> +	if (IS_ERR(pru->cfg)) {
> +		if (PTR_ERR(pru->cfg) != -EPROBE_DEFER)
> +			dev_err(dev, "can't get gpcfg syscon regmap\n");
> +
> +		return PTR_ERR(pru->cfg);
> +	}
> +
> +	if (of_property_read_u32_index(np, "gpcfg", 1, &pru->gpcfg_reg)) {
> +		dev_err(dev, "couldn't get gpcfg reg. offset\n");
> +		return -EINVAL;
> +	}
> +
> +	/* error recovery is not supported for PRUs */
> +	rproc->recovery_disabled = true;
> +
> +	/*
> +	 * rproc_add will auto-boot the processor normally, but this is
> +	 * not desired with PRU client driven boot-flow methodology. A PRU
> +	 * application/client driver will boot the corresponding PRU
> +	 * remote-processor as part of its state machine either through
> +	 * the remoteproc sysfs interface or through the equivalent kernel API
> +	 */
> +	rproc->auto_boot = false;
> +
> +	pru->pruss = platform_get_drvdata(ppdev);
> +	pru->rproc = rproc;
> +	pru->fw_name = fw_name;
> +	mutex_init(&pru->lock);
> +
> +	ret = pruss_request_mem_region(pru->pruss, PRUSS_MEM_DRAM0,
> +				       &pru->dram0);
> +	if (ret) {
> +		dev_err(dev, "couldn't get PRUSS DRAM0: %d\n", ret);
> +		return ret;
> +	}
> +	pruss_release_mem_region(pru->pruss, &pru->dram0);
> +
> +	ret = pruss_request_mem_region(pru->pruss, PRUSS_MEM_DRAM1,
> +				       &pru->dram1);
> +	if (ret) {
> +		dev_err(dev, "couldn't get PRUSS DRAM1: %d\n", ret);
> +		return ret;
> +	}
> +	pruss_release_mem_region(pru->pruss, &pru->dram1);
> +
> +	ret = pruss_request_mem_region(pru->pruss, PRUSS_MEM_SHRD_RAM2,
> +				       &pru->shrdram);
> +	if (ret) {
> +		dev_err(dev, "couldn't get PRUSS Shared RAM: %d\n", ret);
> +		return ret;
> +	}
> +	pruss_release_mem_region(pru->pruss, &pru->shrdram);

Are you doing this because now you don't have access to the pruss
structure?

> +
> +	/* XXX: get this from match data if different in the future */
> +	pru->iram_da = 0;
> +	pru->pdram_da = 0;
> +	pru->sdram_da = 0x2000;
> +	pru->shrdram_da = 0x10000;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "iram");
> +	pru->iram_region.va = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(pru->iram_region.va)) {
> +		ret = PTR_ERR(pru->iram_region.va);
> +		dev_err(dev, "failed to get iram resource: %d\n", ret);
> +		goto free_rproc;
> +	}
> +
> +	pru->iram_region.pa = res->start;
> +	pru->iram_region.size = resource_size(res);
> +
> +	dev_dbg(dev, "iram: pa %pa size 0x%zx va %p\n",
> +		&pru->iram_region.pa, pru->iram_region.size,
> +		pru->iram_region.va);
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "control");
> +	va = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(va)) {
> +		ret = PTR_ERR(va);
> +		dev_err(dev, "failed to get control resource: %d\n", ret);
> +		goto free_rproc;
> +	}
> +
> +	pru->ctrl_regmap = devm_regmap_init_mmio(dev, va,
> +						 &pru_ctrl_regmap_config);
> +	if (IS_ERR(pru->ctrl_regmap)) {
> +		ret = PTR_ERR(pru->ctrl_regmap);
> +		dev_err(dev, "CTRL regmap init failed: %d\n", ret);
> +		goto free_rproc;
> +	}
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "debug");
> +	va = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(va)) {
> +		ret = PTR_ERR(va);
> +		dev_err(dev, "failed to get debug resource: %d\n", ret);
> +		goto free_rproc;
> +	}
> +	pru->debug_regmap = devm_regmap_init_mmio(dev, va,
> +						  &pru_debug_regmap_config);
> +	if (IS_ERR(pru->debug_regmap)) {
> +		ret = PTR_ERR(pru->debug_regmap);
> +		dev_err(dev, "DEBUG regmap init failed: %d\n", ret);
> +		goto free_rproc;
> +	}

I am not a big fan of this regmap conversion. Using it just for the sake
of leveraging regmap debugfs seems overkill, where does the debugfs
files show up for that? Also, I had logic around when the registers can
be accessed, so not sure if you have tested the behavior when the PRUs
are executing.

> +
> +	ret = pru_rproc_set_id(pru);
> +	if (ret < 0)
> +		goto free_rproc;
> +
> +	platform_set_drvdata(pdev, rproc);
> +
> +	ret = rproc_add(pru->rproc);
> +	if (ret) {
> +		dev_err(dev, "rproc_add failed: %d\n", ret);
> +		goto free_rproc;
> +	}
> +
> +	pru_rproc_create_debug_entries(rproc);
> +
> +	dev_info(dev, "PRU rproc node %s probed successfully\n", np->full_name);
> +
> +	return 0;
> +
> +free_rproc:
> +	rproc_free(rproc);
> +	return ret;
> +}
> +
> +static int pru_rproc_remove(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct rproc *rproc = platform_get_drvdata(pdev);
> +
> +	dev_info(dev, "%s: removing rproc %s\n", __func__, rproc->name);
> +
> +	rproc_del(rproc);
> +	rproc_free(rproc);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id pru_rproc_match[] = {
> +	{ .compatible = "ti,am3356-pru", },
> +	{ .compatible = "ti,am4376-pru", },
> +	{ .compatible = "ti,am5728-pru", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, pru_rproc_match);
> +
> +static struct platform_driver pru_rproc_driver = {
> +	.driver = {
> +		.name   = "pru-rproc",
> +		.of_match_table = pru_rproc_match,
> +		.suppress_bind_attrs = true,
> +	},
> +	.probe  = pru_rproc_probe,
> +	.remove = pru_rproc_remove,
> +};
> +module_platform_driver(pru_rproc_driver);
> +
> +MODULE_AUTHOR("Suman Anna <s-anna@ti.com>");
> +MODULE_DESCRIPTION("PRU-ICSS Remote Processor Driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/remoteproc/pru_rproc.h b/drivers/remoteproc/pru_rproc.h
> new file mode 100644
> index 0000000..bbcf392
> --- /dev/null
> +++ b/drivers/remoteproc/pru_rproc.h
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
> +/*
> + * PRUSS Remote Processor specific types
> + *
> + * Copyright (C) 2014-2019 Texas Instruments Incorporated - http://www.ti.com/
> + * All rights reserved.
> + */
> +
> +#ifndef _PRU_REMOTEPROC_H_
> +#define _PRU_REMOTEPROC_H_
> +
> +/**
> + * enum pruss_rsc_types - PRU specific resource types
> + *
> + * @PRUSS_RSC_INTRS: Resource holding information on PRU PINTC configuration
> + * @PRUSS_RSC_MAX: Indicates end of known/defined PRU resource types.
> + *		   This should be the last definition.
> + *
> + * Introduce new custom resource types before PRUSS_RSC_MAX.
> + */
> +enum pruss_rsc_types {
> +	PRUSS_RSC_INTRS	= 1,
> +	PRUSS_RSC_MAX	= 2,
> +};
> +
> +/**
> + * struct fw_rsc_pruss_intrmap - vendor resource to define PRU interrupts
> + * @type: should be PRUSS_RSC_INTRS
> + * @version: should be 1 or greater. 0 was for prototyping and is not supported
> + * @num_maps: number of interrupt mappings that follow
> + * @data: Array of 'num_maps' mappings.
> + *		Each mapping is a triplet {s, c, h}
> + *		s - system event id
> + *		c - channel id
> + *		h - host interrupt id
> + *
> + * PRU system events are mapped to channels, and these channels are mapped
> + * to host interrupts. Events can be mapped to channels in a one-to-one or
> + * many-to-one ratio (multiple events per channel), and channels can be
> + * mapped to host interrupts in a one-to-one or many-to-one ratio (multiple
> + * channels per interrupt).
> + *
> + * This resource is variable length due to the nature of INTC map.
> + * The below data structure is scalable so it can support sufficiently
> + * large number of sysevents and hosts.
> + */
> +struct fw_rsc_pruss_intrmap {
> +	u16 type;
> +	u16 version;
> +	u8 num_maps;
> +	u8 data[];

I can comment more on this once I get a testable version, but I explored
this idea initially before abandoning this. We would want to prefer
using as small an amount of memory as possible, as the resource table
has to be loaded into memory. The channel to host mapping need to be
defined only once for a channel, and not needed for every event.

Also, I am not sure if your current logic is handling the resource size
availability check with this open-ended structure definition.

regards
Suman

> +} __packed;
> +
> +#endif	/* _PRU_REMOTEPROC_H_ */
> diff --git a/include/linux/remoteproc/pru_rproc.h b/include/linux/remoteproc/pru_rproc.h
> new file mode 100644
> index 0000000..841da09
> --- /dev/null
> +++ b/include/linux/remoteproc/pru_rproc.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/**
> + * PRU-ICSS Remote Subsystem user interfaces
> + *
> + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com
> + */
> +
> +#ifndef __LINUX_REMOTEPROC_PRU_RPROC_H
> +#define __LINUX_REMOTEPROC_PRU_RPROC_H
> +
> +#if IS_ENABLED(CONFIG_PRUSS_REMOTEPROC)
> +
> +struct rproc *pru_rproc_get(struct device_node *node, int index);
> +void pru_rproc_put(struct rproc *rproc);
> +
> +#else
> +
> +static inline struct rproc *pru_rproc_get(struct device_node *node, int index)
> +{
> +	return ERR_PTR(-ENOTSUPP);
> +}
> +
> +static inline void pru_rproc_put(struct rproc *rproc) { }
> +
> +#endif /* CONFIG_PRUSS_REMOTEPROC */
> +
> +#endif /* __LINUX_REMOTEPROC_PRU_RPROC_H */
>
Suman Anna Feb. 14, 2019, 3:44 a.m. UTC | #16
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>
>> ---