Devicetree
 help / color / mirror / Atom feed
* [PATCH 0/6] BCM59056 PMU regulator support
From: Matt Porter @ 2014-02-04 12:19 UTC (permalink / raw)
  To: Wolfram Sang, Tim Kryger, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Mark Brown, Christian Daudt
  Cc: Devicetree List, Linux I2C List, Linux ARM Kernel List,
	Linux Kernel Mailing List

The BCM59056 is a multi-function power management unit used with the
BCM281xx family of SoCs. This series adds an MFD and voltage regulator
driver to support the BCM59056. The bcm28155-ap DT support is updated
to enable use of regulators on the otg and sdhci peripherals.

Matt Porter (6):
  i2c: bcm-kona: register with subsys_initcall
  regulator: add bcm59056 pmu DT binding
  mfd: add bcm59056 pmu driver
  regulator: add bcm59056 regulator driver
  ARM: configs: bcm_defconfig: enable bcm59056 regulator support
  ARM: dts: add bcm59056 pmu support and enable for bcm28155-ap

 .../devicetree/bindings/regulator/bcm59056.txt     |  37 ++
 arch/arm/boot/dts/bcm28155-ap.dts                  |  41 ++
 arch/arm/boot/dts/bcm59056.dtsi                    | 158 ++++++++
 arch/arm/configs/bcm_defconfig                     |   7 +
 drivers/i2c/busses/i2c-bcm-kona.c                  |  14 +-
 drivers/mfd/Kconfig                                |   8 +
 drivers/mfd/Makefile                               |   1 +
 drivers/mfd/bcm59056.c                             | 119 ++++++
 drivers/regulator/Kconfig                          |   8 +
 drivers/regulator/Makefile                         |   1 +
 drivers/regulator/bcm59056-regulator.c             | 445 +++++++++++++++++++++
 include/linux/mfd/bcm59056.h                       |  35 ++
 12 files changed, 873 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/bcm59056.txt
 create mode 100644 arch/arm/boot/dts/bcm59056.dtsi
 create mode 100644 drivers/mfd/bcm59056.c
 create mode 100644 drivers/regulator/bcm59056-regulator.c
 create mode 100644 include/linux/mfd/bcm59056.h

-- 
1.8.4

^ permalink raw reply

* Re: [RFC] dt-bindings: configuration of parent clocks and clock frequency
From: Marek Szyprowski @ 2014-02-04 12:16 UTC (permalink / raw)
  To: Mike Turquette, Sylwester Nawrocki, linux-arm-kernel, devicetree
  Cc: Russell King - ARM Linux, Tomasz Figa, Kyungmin Park, Rob Herring,
	Mark Rutland, Pawel Moll, Ian Campbell, Kumar Gala
In-Reply-To: <20140201030614.9977.80501@quantum>

Hello,

On 2014-02-01 04:06, Mike Turquette wrote:
> Quoting Sylwester Nawrocki (2014-01-21 04:48:43)
> > 5. Similarly to the regulator bindings the clock names could be appended
> >   to name of a DT property:
> >
> >  [clk_name]-assigned-clock-parent = <...>;
> >  [clk_name]-assigned-clock-rate = <...>;
>
> I have always been partial to the way that the reg framework does its
> [reg_name]-supply. We could shorten it to something like:
>
> [clk-name]-asn-parent = ....
> [clk-name]-asn-rate = ...
>
> This is actually the format that was discussed at the ARM kernel summit
> IIRC.

I think that "[clk-name]-clk-parent" and "[clk-name]-clk-rate" will be a bit
more descriptive names instead of quite ambiguous "asn".

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v2 5/5] of: document bindings for reserved-memory nodes
From: Marek Szyprowski @ 2014-02-04 12:09 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linaro-mm-sig, devicetree,
	linux-doc
  Cc: Marek Szyprowski, Kyungmin Park, Benjamin Herrenschmidt,
	Arnd Bergmann, Michal Nazarewicz, Grant Likely, Tomasz Figa,
	Sascha Hauer, Laura Abbott, Rob Herring, Olof Johansson,
	Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
	Tomasz Figa, Kumar Gala, Nishanth Peethambaran, Marc,
	Josh Cartwright
In-Reply-To: <1391515773-6112-1-git-send-email-m.szyprowski@samsung.com>

From: Grant Likely <grant.likely@linaro.org>

Reserved memory nodes allow for the reservation of static (fixed
address) regions, or dynamically allocated regions for a specific
purpose.

[joshc: Based on binding document proposed (in non-patch form) here:
 http://lkml.kernel.org/g/20131030134702.19B57C402A0@trevor.secretlab.ca
 adapted to support #memory-region-cells]

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Laura Abbott <lauraa@codeaurora.org>
Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 .../bindings/reserved-memory/reserved-memory.txt   |  138 ++++++++++++++++++++
 1 file changed, 138 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

diff --git a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
new file mode 100644
index 000000000000..a606ce90c9c4
--- /dev/null
+++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
@@ -0,0 +1,138 @@
+*** Reserved memory regions ***
+
+Reserved memory is specified as a node under the /reserved-memory node.
+The operating system shall exclude reserved memory from normal usage
+one can create child nodes describing particular reserved (excluded from
+normal use) memory regions. Such memory regions are usually designed for
+the special usage by various device drivers.
+
+Parameters for each memory region can be encoded into the device tree
+with the following nodes:
+
+/reserved-memory node
+---------------------
+#address-cells, #size-cells (required) - standard definition
+    - Should use the same values as the root node
+#memory-region-cells (required) - dictates number of cells used in the child
+                                  nodes memory-region specifier
+ranges (required) - standard definition
+    - Should be empty
+
+/reserved-memory/ child nodes
+-----------------------------
+Each child of the reserved-memory node specifies one or more regions of
+reserved memory. Each child node may either use a 'reg' property to
+specify a specific range of reserved memory, or a 'size' property with
+optional constraints to request a dynamically allocated block of memory.
+
+Following the generic-names recommended practice, node names should
+reflect the purpose of the node (ie. "framebuffer" or "dma-pool"). Unit
+address (@<address>) should be appended to the name if the node is a
+static allocation.
+
+Properties:
+Requires either a) or b) below.
+a) static allocation
+   reg (required) - standard definition
+b) dynamic allocation
+   size (required) - length based on parent's #size-cells
+                   - Size in bytes of memory to reserve.
+   alignment (optional) - length based on parent's #size-cells
+                        - Address boundary for alignment of allocation.
+   alloc-ranges (optional) - prop-encoded-array (address, length pairs).
+                           - Specifies regions of memory that are
+                             acceptable to allocate from.
+
+If both reg and size are present, then the reg property takes precedence
+and size is ignored.
+
+Additional properties:
+compatible (optional) - standard definition
+    - may contain the following strings:
+        - shared-dma-pool: This indicates a region of memory meant to be
+          used as a shared pool of DMA buffers for a set of devices. It can
+          be used by an operating system to instanciate the necessary pool
+          management subsystem if necessary.
+        - vendor specific string in the form <vendor>,[<device>-]<usage>
+no-map (optional) - empty property
+    - Indicates the operating system must not create a virtual mapping
+      of the region as part of its standard mapping of system memory,
+      nor permit speculative access to it under any circumstances other
+      than under the control of the device driver using the region.
+reusable (optional) - empty property
+    - The operating system can use the memory in this region with the
+      limitation that the device driver(s) owning the region need to be
+      able to reclaim it back. Typically that means that the operating
+      system can use that region to store volatile or cached data that
+      can be otherwise regenerated or migrated elsewhere.
+
+Linux implementation note:
+- If a "linux,cma-default" property is present, then Linux will use the
+  region for the default pool of the contiguous memory allocator.
+
+Device node references to reserved memory
+-----------------------------------------
+Regions in the /reserved-memory node may be referenced by other device
+nodes by adding a memory-region property to the device node.
+
+memory-region (optional) - phandle, specifier pairs to children of /reserved-memory
+
+Example
+-------
+This example defines 3 contiguous regions are defined for Linux kernel:
+one default of all device drivers (named linux,cma@72000000 and 64MiB in size),
+one dedicated to the framebuffer device (named framebuffer@78000000, 8MiB), and
+one for multimedia processing (named multimedia-memory@77000000, 64MiB).
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	memory {
+		reg = <0x40000000 0x40000000>;
+	};
+
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		/* global autoconfigured region for contiguous allocations */
+		linux,cma {
+			compatible = "shared-dma-pool";
+			reusable;
+			#memory-region-cells = <0>;
+			size = <0x4000000>;
+			alignment = <0x2000>;
+			linux,cma-default;
+		};
+
+		display_reserved: framebuffer@78000000 {
+			#memory-region-cells = <0>;
+			reg = <0x78000000 0x800000>;
+		};
+
+		multimedia_reserved: multimedia@77000000 {
+			compatible = "acme,multimedia-memory";
+			#memory-region-cells = <1>;
+			reg = <0x77000000 0x4000000>;
+		};
+	};
+
+	/* ... */
+
+	fb0: video@12300000 {
+		memory-region = <&display_reserved>;
+		/* ... */
+	};
+
+	scaler: scaler@12500000 {
+		memory-region = <&multimedia_reserved 0xdeadbeef>;
+		/* ... */
+	};
+
+	codec: codec@12600000 {
+		memory-region = <&multimedia_reserved 0xfeebdaed>;
+		/* ... */
+	};
+};
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 4/5] ARM: init: add support for reserved memory defined by device tree
From: Marek Szyprowski @ 2014-02-04 12:09 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linaro-mm-sig, devicetree,
	linux-doc
  Cc: Marek Szyprowski, Kyungmin Park, Benjamin Herrenschmidt,
	Arnd Bergmann, Michal Nazarewicz, Grant Likely, Tomasz Figa,
	Sascha Hauer, Laura Abbott, Rob Herring, Olof Johansson,
	Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
	Tomasz Figa, Kumar Gala, Nishanth Peethambaran, Marc,
	Josh Cartwright
In-Reply-To: <1391515773-6112-1-git-send-email-m.szyprowski@samsung.com>

Enable reserved memory initialization from device tree.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Laura Abbott <lauraa@codeaurora.org>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/mm/init.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 804d61566a53..ebafdb479410 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -17,6 +17,7 @@
 #include <linux/nodemask.h>
 #include <linux/initrd.h>
 #include <linux/of_fdt.h>
+#include <linux/of_reserved_mem.h>
 #include <linux/highmem.h>
 #include <linux/gfp.h>
 #include <linux/memblock.h>
@@ -323,6 +324,8 @@ void __init arm_memblock_init(struct meminfo *mi,
 	if (mdesc->reserve)
 		mdesc->reserve();
 
+	early_init_dt_scan_reserved_mem();
+
 	/*
 	 * reserve memory for DMA contigouos allocations,
 	 * must come from DMA area inside low memory
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH v2 3/5] drivers: of: implement reserved-memory handling for cma
From: Marek Szyprowski @ 2014-02-04 12:09 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linaro-mm-sig, devicetree,
	linux-doc
  Cc: Marek Szyprowski, Kyungmin Park, Benjamin Herrenschmidt,
	Arnd Bergmann, Michal Nazarewicz, Grant Likely, Tomasz Figa,
	Sascha Hauer, Laura Abbott, Rob Herring, Olof Johansson,
	Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
	Tomasz Figa, Kumar Gala, Nishanth Peethambaran, Marc,
	Josh Cartwright
In-Reply-To: <1391515773-6112-1-git-send-email-m.szyprowski@samsung.com>

From: Josh Cartwright <joshc@codeaurora.org>

Add support for handling 'shared-dma-pool' reserved-memory nodes using
Contiguous Memory Allocator driver.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Laura Abbott <lauraa@codeaurora.org>
Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/of/Kconfig               |    7 ++++
 drivers/of/Makefile              |    1 +
 drivers/of/of_reserved_mem_cma.c |   75 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 83 insertions(+)
 create mode 100644 drivers/of/of_reserved_mem_cma.c

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 7ac330473ec9..5dd3a80910d2 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -81,6 +81,13 @@ config OF_RESERVED_MEM
 	help
 	  Helpers to allow for reservation of memory regions
 
+config OF_RESERVED_MEM_CMA
+	depends on OF_RESERVED_MEM
+	depends on DMA_CMA
+	def_bool y
+	help
+	  Helpers for reserving memory regions for DMA use
+
 config OF_RESERVED_MEM_DMA
 	depends on OF_RESERVED_MEM
 	depends on HAVE_GENERIC_DMA_COHERENT
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index 6142227ca854..49b9078637b8 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -10,4 +10,5 @@ obj-$(CONFIG_OF_PCI)	+= of_pci.o
 obj-$(CONFIG_OF_PCI_IRQ)  += of_pci_irq.o
 obj-$(CONFIG_OF_MTD)	+= of_mtd.o
 obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
+obj-$(CONFIG_OF_RESERVED_MEM_CMA) += of_reserved_mem_cma.o
 obj-$(CONFIG_OF_RESERVED_MEM_DMA) += of_reserved_mem_dma.o
diff --git a/drivers/of/of_reserved_mem_cma.c b/drivers/of/of_reserved_mem_cma.c
new file mode 100644
index 000000000000..601d80655102
--- /dev/null
+++ b/drivers/of/of_reserved_mem_cma.c
@@ -0,0 +1,75 @@
+/*
+ * Device tree based initialization code for DMA reserved regions.
+ *
+ * Copyright (c) 2013, The Linux Foundation. All Rights Reserved.
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ * Author: Marek Szyprowski <m.szyprowski@samsung.com>
+ * Author: Josh Cartwright <joshc@codeaurora.org>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License or (at your optional) any later version of the license.
+ */
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <linux/of_platform.h>
+#include <linux/mm.h>
+#include <linux/sizes.h>
+#include <linux/mm_types.h>
+#include <linux/dma-contiguous.h>
+#include <linux/of_reserved_mem.h>
+
+static void rmem_cma_device_init(struct reserved_mem *rmem,
+				 struct device *dev,
+				 struct of_phandle_args *args)
+{
+	struct cma *cma = rmem->priv;
+	dev_set_cma_area(dev, cma);
+}
+
+static const struct reserved_mem_ops rmem_cma_ops = {
+	.device_init	= rmem_cma_device_init,
+};
+
+static int __init rmem_cma_setup(struct reserved_mem *rmem,
+				 unsigned long node,
+				 const char *uname)
+{
+	struct cma *cma;
+	int err;
+
+	if (!of_get_flat_dt_prop(node, "reusable", NULL))
+		return -EINVAL;
+
+	err = of_parse_flat_dt_reg(node, uname, &rmem->base, &rmem->size);
+	if (!err)
+		goto out_done;
+
+	rmem->base = 0;
+	err = of_parse_flat_dt_size(node, uname, &rmem->size);
+	if (err)
+		goto out_err;
+
+out_done:
+	err = dma_contiguous_reserve_area(rmem->size, rmem->base, 0,
+					  &cma);
+	if (err) {
+		pr_err("Reserved memory: unable to setup CMA region\n");
+		return err;
+	}
+
+	if (of_get_flat_dt_prop(node, "linux,cma-default", NULL))
+		dma_contiguous_set_default(cma);
+
+	rmem->ops = &rmem_cma_ops;
+	rmem->priv = cma;
+
+	return 0;
+out_err:
+	pr_err("Reseved memory: malformed node '%s'.\n", uname);
+	return err;
+}
+RESERVEDMEM_OF_DECLARE(cma, "shared-dma-pool", rmem_cma_setup);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 2/5] drivers: of: implement reserved-memory handling for dma
From: Marek Szyprowski @ 2014-02-04 12:09 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linaro-mm-sig, devicetree,
	linux-doc
  Cc: Mark Rutland, Laura Abbott, Pawel Moll, Arnd Bergmann,
	Stephen Warren, Josh Cartwright, Benjamin Herrenschmidt,
	Tomasz Figa, Olof Johansson, Tomasz Figa, Michal Nazarewicz,
	Rob Herring, Kyungmin Park, Kumar Gala, Grant Likely,
	Ian Campbell, Nishanth Peethambaran, Sascha Hauer, Marc,
	Marek Szyprowski
In-Reply-To: <1391515773-6112-1-git-send-email-m.szyprowski@samsung.com>

From: Josh Cartwright <joshc@codeaurora.org>

Add support for handling 'shared-dma-pool' reserved-memory nodes using
dma exclusive driver (dma_alloc_coherent()).

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Laura Abbott <lauraa@codeaurora.org>
Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/of/Kconfig               |    7 ++++
 drivers/of/Makefile              |    1 +
 drivers/of/of_reserved_mem_dma.c |   78 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+)
 create mode 100644 drivers/of/of_reserved_mem_dma.c

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index aba13df56f3a..7ac330473ec9 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -81,4 +81,11 @@ config OF_RESERVED_MEM
 	help
 	  Helpers to allow for reservation of memory regions
 
+config OF_RESERVED_MEM_DMA
+	depends on OF_RESERVED_MEM
+	depends on HAVE_GENERIC_DMA_COHERENT
+	def_bool y
+	help
+	  Helpers for reserving memory regions for DMA use
+
 endmenu # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index ed9660adad77..6142227ca854 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_OF_PCI)	+= of_pci.o
 obj-$(CONFIG_OF_PCI_IRQ)  += of_pci_irq.o
 obj-$(CONFIG_OF_MTD)	+= of_mtd.o
 obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
+obj-$(CONFIG_OF_RESERVED_MEM_DMA) += of_reserved_mem_dma.o
diff --git a/drivers/of/of_reserved_mem_dma.c b/drivers/of/of_reserved_mem_dma.c
new file mode 100644
index 000000000000..53a4cac6084a
--- /dev/null
+++ b/drivers/of/of_reserved_mem_dma.c
@@ -0,0 +1,78 @@
+/*
+ * Device tree based initialization code for DMA reserved regions.
+ *
+ * Copyright (c) 2013, The Linux Foundation. All Rights Reserved.
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ * Author: Marek Szyprowski <m.szyprowski@samsung.com>
+ * Author: Josh Cartwright <joshc@codeaurora.org>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License or (at your optional) any later version of the license.
+ */
+#include <linux/memblock.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <linux/of_platform.h>
+#include <linux/mm.h>
+#include <linux/sizes.h>
+#include <linux/mm_types.h>
+#include <linux/dma-mapping.h>
+#include <linux/of_reserved_mem.h>
+
+static void rmem_dma_device_init(struct reserved_mem *rmem,
+				 struct device *dev,
+				 struct of_phandle_args *args)
+{
+	dma_declare_coherent_memory(dev, rmem->base, rmem->base,
+		rmem->size, DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE);
+}
+
+static void rmem_dma_device_release(struct reserved_mem *rmem,
+				    struct device *dev)
+{
+	dma_release_declared_memory(dev);
+}
+
+static const struct reserved_mem_ops rmem_dma_ops = {
+	.device_init	= rmem_dma_device_init,
+	.device_release	= rmem_dma_device_release,
+};
+
+static int __init rmem_dma_setup(struct reserved_mem *rmem,
+				 unsigned long node,
+				 const char *uname)
+{
+	int err;
+
+	if (of_get_flat_dt_prop(node, "reusable", NULL))
+		return -EINVAL;
+
+	err = of_parse_flat_dt_reg(node, uname, &rmem->base, &rmem->size);
+	if (!err)
+		goto out_done;
+
+	err = of_parse_flat_dt_size(node, uname, &rmem->size);
+	if (err)
+		goto out_err;
+
+	rmem->base = memblock_alloc_base(rmem->size, PAGE_SIZE,
+					 MEMBLOCK_ALLOC_ANYWHERE);
+	memblock_free(rmem->base, rmem->size);
+
+out_done:
+	rmem->ops = &rmem_dma_ops;
+	pr_info("Reserved memory: created DMA memory pool at %pa, size %ld MiB\n",
+		&rmem->base, (unsigned long)rmem->size / SZ_1M);
+
+	return memblock_remove(rmem->base, rmem->size);
+
+out_err:
+	pr_err("Reserved memory: unable to setup '%s' memory region for DMA.\n",
+	       uname);
+	return err;
+}
+RESERVEDMEM_OF_DECLARE(dma, "shared-dma-pool", rmem_dma_setup);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 1/5] drivers: of: add initialization code for reserved memory
From: Marek Szyprowski @ 2014-02-04 12:09 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linaro-mm-sig, devicetree,
	linux-doc
  Cc: Marek Szyprowski, Kyungmin Park, Benjamin Herrenschmidt,
	Arnd Bergmann, Michal Nazarewicz, Grant Likely, Tomasz Figa,
	Sascha Hauer, Laura Abbott, Rob Herring, Olof Johansson,
	Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
	Tomasz Figa, Kumar Gala, Nishanth Peethambaran, Marc,
	Josh Cartwright
In-Reply-To: <1391515773-6112-1-git-send-email-m.szyprowski@samsung.com>

This patch adds device tree support for contiguous and reserved memory
regions defined in device tree.

Large memory blocks can be reliably reserved only during early boot.
This must happen before the whole memory management subsystem is
initialized, because we need to ensure that the given contiguous blocks
are not yet allocated by kernel. Also it must happen before kernel
mappings for the whole low memory are created, to ensure that there will
be no mappings (for reserved blocks) or mapping with special properties
can be created (for CMA blocks). This all happens before device tree
structures are unflattened, so we need to get reserved memory layout
directly from fdt.

Later, those reserved memory regions are assigned to devices on each
device structure initialization.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Laura Abbott <lauraa@codeaurora.org>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
[joshc: rework to implement new DT binding, provide mechanism for
 plugging in new reserved-memory node handlers via
 RESERVEDMEM_OF_DECLARE]
Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
[mszyprow: little code cleanup]
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/of/Kconfig                |    6 +
 drivers/of/Makefile               |    1 +
 drivers/of/of_reserved_mem.c      |  219 +++++++++++++++++++++++++++++++++++++
 drivers/of/platform.c             |    7 ++
 include/asm-generic/vmlinux.lds.h |   11 ++
 include/linux/of_reserved_mem.h   |   62 +++++++++++
 6 files changed, 306 insertions(+)
 create mode 100644 drivers/of/of_reserved_mem.c
 create mode 100644 include/linux/of_reserved_mem.h

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index c6973f101a3e..aba13df56f3a 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -75,4 +75,10 @@ config OF_MTD
 	depends on MTD
 	def_bool y
 
+config OF_RESERVED_MEM
+	depends on HAVE_MEMBLOCK
+	def_bool y
+	help
+	  Helpers to allow for reservation of memory regions
+
 endmenu # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index efd05102c405..ed9660adad77 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_OF_MDIO)	+= of_mdio.o
 obj-$(CONFIG_OF_PCI)	+= of_pci.o
 obj-$(CONFIG_OF_PCI_IRQ)  += of_pci_irq.o
 obj-$(CONFIG_OF_MTD)	+= of_mtd.o
+obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
new file mode 100644
index 000000000000..f17cd56e68d9
--- /dev/null
+++ b/drivers/of/of_reserved_mem.c
@@ -0,0 +1,219 @@
+/*
+ * Device tree based initialization code for reserved memory.
+ *
+ * Copyright (c) 2013, The Linux Foundation. All Rights Reserved.
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ * Author: Marek Szyprowski <m.szyprowski@samsung.com>
+ * Author: Josh Cartwright <joshc@codeaurora.org>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License or (at your optional) any later version of the license.
+ */
+#include <linux/memblock.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <linux/of_platform.h>
+#include <linux/mm.h>
+#include <linux/sizes.h>
+#include <linux/of_reserved_mem.h>
+
+#define MAX_RESERVED_REGIONS	16
+static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
+static int reserved_mem_count;
+
+int __init of_parse_flat_dt_reg(unsigned long node, const char *uname,
+				   phys_addr_t *base, phys_addr_t *size)
+{
+	unsigned long len;
+	__be32 *prop;
+
+	prop = of_get_flat_dt_prop(node, "reg", &len);
+	if (!prop)
+		return -EINVAL;
+
+	if (len < (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32)) {
+		pr_err("Reserved memory: invalid reg property in '%s' node.\n",
+				uname);
+		return -EINVAL;
+	}
+
+	*base = dt_mem_next_cell(dt_root_addr_cells, &prop);
+	*size = dt_mem_next_cell(dt_root_size_cells, &prop);
+	return 0;
+}
+
+int __init of_parse_flat_dt_size(unsigned long node, const char *uname,
+				    phys_addr_t *size)
+{
+	unsigned long len;
+	__be32 *prop;
+
+	prop = of_get_flat_dt_prop(node, "size", &len);
+	if (!prop)
+		return -EINVAL;
+
+	if (len < dt_root_size_cells * sizeof(__be32)) {
+		pr_err("Reserved memory: invalid size property in '%s' node.\n",
+				uname);
+		return -EINVAL;
+	}
+
+	*size = dt_mem_next_cell(dt_root_size_cells, &prop);
+	return 0;
+}
+
+static int __init rmem_default_early_setup(struct reserved_mem *rmem,
+					   unsigned long node,
+					   const char *uname)
+{
+	int err;
+
+	if (of_get_flat_dt_prop(node, "compatible", NULL))
+		return -EINVAL;
+
+	err = of_parse_flat_dt_reg(node, uname, &rmem->base, &rmem->size);
+	if (err)
+		return err;
+
+	if (of_get_flat_dt_prop(node, "no-map", NULL))
+		err = memblock_remove(rmem->base, rmem->size);
+	else
+		err = memblock_reserve(rmem->base, rmem->size);
+
+	if (err == 0)
+		pr_info("Reserved memory: found '%s', memory base %pa, size %ld MiB\n",
+			uname, &rmem->base, (unsigned long)rmem->size / SZ_1M);
+
+	return err;
+}
+
+static const struct of_device_id rmem_default_id
+	__used __section(__reservedmem_of_table_end) = {
+	.data		= rmem_default_early_setup,
+};
+
+static int __init fdt_scan_reserved_mem(unsigned long node, const char *uname,
+					int depth, void *data)
+{
+	struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
+	extern const struct of_device_id __reservedmem_of_table[];
+	const struct of_device_id *id;
+	const char *status;
+
+	if (reserved_mem_count == ARRAY_SIZE(reserved_mem)) {
+		pr_err("Reserved memory: not enough space all defined regions.\n");
+		return -ENOSPC;
+	}
+
+	status = of_get_flat_dt_prop(node, "status", NULL);
+	if (status && strcmp(status, "okay") != 0)
+		return 0;
+
+	/*
+	 * The default handler above ensures this section is terminated with a
+	 * id whose compatible string is empty
+	 */
+	for (id = __reservedmem_of_table; id <= &rmem_default_id; id++) {
+		reservedmem_of_init_fn initfn = id->data;
+		const char *compat = id->compatible;
+
+		if (compat[0] && !of_flat_dt_is_compatible(node, compat))
+			continue;
+
+		if (initfn(rmem, node, uname) == 0) {
+			pr_info("Reserved memory: created %s node, compatible id %s\n",
+				uname, compat);
+			rmem->name = uname;
+			reserved_mem_count++;
+			break;
+		}
+	}
+
+	return 0;
+}
+
+static struct reserved_mem *find_rmem(struct device_node *np)
+{
+	const char *name;
+	unsigned int i;
+
+	name = kbasename(np->full_name);
+
+	for (i = 0; i < reserved_mem_count; i++)
+		if (strcmp(name, reserved_mem[i].name) == 0)
+			return &reserved_mem[i];
+
+	return NULL;
+}
+
+/**
+ * of_reserved_mem_device_init() - assign reserved memory region to given device
+ *
+ * This function assign memory region pointed by "memory-region" device tree
+ * property to the given device.
+ */
+void of_reserved_mem_device_init(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct reserved_mem *rmem;
+	struct of_phandle_args s;
+	unsigned int i;
+
+	for (i = 0; of_parse_phandle_with_args(np, "memory-region",
+				"#memory-region-cells", i, &s) == 0; i++) {
+
+		rmem = find_rmem(s.np);
+		if (!rmem || !rmem->ops || !rmem->ops->device_init) {
+			of_node_put(s.np);
+			continue;
+		}
+
+		rmem->ops->device_init(rmem, dev, &s);
+		dev_info(dev, "assigned reserved memory node %s\n",
+			 rmem->name);
+		of_node_put(s.np);
+		break;
+	}
+}
+
+/**
+ * of_reserved_mem_device_release() - release reserved memory device structures
+ *
+ * This function releases structures allocated for memory region handling for
+ * the given device.
+ */
+void of_reserved_mem_device_release(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct reserved_mem *rmem;
+	struct of_phandle_args s;
+	unsigned int i;
+
+	for (i = 0; of_parse_phandle_with_args(np, "memory-region",
+				"#memory-region-cells", i, &s) == 0; i++) {
+
+		rmem = find_rmem(s.np);
+		if (rmem && rmem->ops && rmem->ops->device_release)
+			rmem->ops->device_release(rmem, dev);
+
+		of_node_put(s.np);
+	}
+}
+
+/**
+ * early_init_dt_scan_reserved_mem() - create reserved memory regions
+ *
+ * This function grabs memory from early allocator for device exclusive use
+ * defined in device tree structures. It should be called by arch specific code
+ * once the early allocator (memblock) has been activated and all other
+ * subsystems have already allocated/reserved memory.
+ */
+void __init early_init_dt_scan_reserved_mem(void)
+{
+	of_scan_flat_dt_by_path("/reserved-memory", fdt_scan_reserved_mem,
+				NULL);
+}
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 404d1daebefa..3df0b1826e8b 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -21,6 +21,7 @@
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
+#include <linux/of_reserved_mem.h>
 #include <linux/platform_device.h>
 
 const struct of_device_id of_default_bus_match_table[] = {
@@ -220,6 +221,8 @@ static struct platform_device *of_platform_device_create_pdata(
 	dev->dev.bus = &platform_bus_type;
 	dev->dev.platform_data = platform_data;
 
+	of_reserved_mem_device_init(&dev->dev);
+
 	/* We do not fill the DMA ops for platform devices by default.
 	 * This is currently the responsibility of the platform code
 	 * to do such, possibly using a device notifier
@@ -227,6 +230,7 @@ static struct platform_device *of_platform_device_create_pdata(
 
 	if (of_device_add(dev) != 0) {
 		platform_device_put(dev);
+		of_reserved_mem_device_release(&dev->dev);
 		return NULL;
 	}
 
@@ -282,6 +286,8 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
 	else
 		of_device_make_bus_id(&dev->dev);
 
+	of_reserved_mem_device_init(&dev->dev);
+
 	/* Allow the HW Peripheral ID to be overridden */
 	prop = of_get_property(node, "arm,primecell-periphid", NULL);
 	if (prop)
@@ -308,6 +314,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
 	return dev;
 
 err_free:
+	of_reserved_mem_device_release(&dev->dev);
 	amba_device_put(dev);
 	return NULL;
 }
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index bc2121fa9132..f10f64fcc815 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -167,6 +167,16 @@
 #define CLK_OF_TABLES()
 #endif
 
+#ifdef CONFIG_OF_RESERVED_MEM
+#define RESERVEDMEM_OF_TABLES()				\
+	. = ALIGN(8);					\
+	VMLINUX_SYMBOL(__reservedmem_of_table) = .;	\
+	*(__reservedmem_of_table)			\
+	*(__reservedmem_of_table_end)
+#else
+#define RESERVEDMEM_OF_TABLES()
+#endif
+
 #define KERNEL_DTB()							\
 	STRUCT_ALIGN();							\
 	VMLINUX_SYMBOL(__dtb_start) = .;				\
@@ -490,6 +500,7 @@
 	TRACE_SYSCALLS()						\
 	MEM_DISCARD(init.rodata)					\
 	CLK_OF_TABLES()							\
+	RESERVEDMEM_OF_TABLES()						\
 	CLKSRC_OF_TABLES()						\
 	KERNEL_DTB()							\
 	IRQCHIP_OF_MATCH_TABLE()
diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
new file mode 100644
index 000000000000..41f43828e1db
--- /dev/null
+++ b/include/linux/of_reserved_mem.h
@@ -0,0 +1,62 @@
+#ifndef __OF_RESERVED_MEM_H
+#define __OF_RESERVED_MEM_H
+
+struct cma;
+struct platform_device;
+struct of_phandle_args;
+struct reserved_mem_ops;
+
+struct reserved_mem {
+	const char			*name;
+	const struct reserved_mem_ops	*ops;
+	phys_addr_t			base;
+	phys_addr_t			size;
+	void				*priv;
+};
+
+struct reserved_mem_ops {
+	void	(*device_init)(struct reserved_mem *rmem,
+			       struct device *dev,
+			       struct of_phandle_args *args);
+	void	(*device_release)(struct reserved_mem *rmem,
+				  struct device *dev);
+};
+
+typedef int (*reservedmem_of_init_fn)(struct reserved_mem *rmem,
+				      unsigned long node, const char *uname);
+
+#ifdef CONFIG_OF_RESERVED_MEM
+void of_reserved_mem_device_init(struct device *dev);
+void of_reserved_mem_device_release(struct device *dev);
+void early_init_dt_scan_reserved_mem(void);
+
+int of_parse_flat_dt_reg(unsigned long node, const char *uname,
+			 phys_addr_t *base, phys_addr_t *size);
+int of_parse_flat_dt_size(unsigned long node, const char *uname,
+			  phys_addr_t *size);
+
+#define RESERVEDMEM_OF_DECLARE(name, compat, init)			\
+	static const struct of_device_id __reservedmem_of_table_##name	\
+		__used __section(__reservedmem_of_table)		\
+		 = { .compatible = compat,				\
+		     .data = (init == (reservedmem_of_init_fn)NULL) ?	\
+				init : init }
+
+#else
+static inline void of_reserved_mem_device_init(struct device *dev) { }
+
+static inline
+void of_reserved_mem_device_release(struct device *dev) { }
+
+static inline void early_init_dt_scan_reserved_mem(void) { }
+
+#define RESERVEDMEM_OF_DECLARE(name, compat, init)			\
+	static const struct of_device_id __reservedmem_of_table_##name	\
+		__attribute__((unused))					\
+		 = { .compatible = compat,				\
+		     .data = (init == (reservedmem_of_init_fn)NULL) ?	\
+				init : init }
+
+#endif
+
+#endif /* __OF_RESERVED_MEM_H */
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 0/5] reserved-memory regions/CMA in devicetree, again
From: Marek Szyprowski @ 2014-02-04 12:09 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linaro-mm-sig, devicetree,
	linux-doc
  Cc: Marek Szyprowski, Kyungmin Park, Benjamin Herrenschmidt,
	Arnd Bergmann, Michal Nazarewicz, Grant Likely, Tomasz Figa,
	Sascha Hauer, Laura Abbott, Rob Herring, Olof Johansson,
	Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
	Tomasz Figa, Kumar Gala, Nishanth Peethambaran, Marc,
	Josh Cartwright

Hi all!

This is an updated version of the second attempt to add basic support
for dynamic allocation of memory reserved regions defined in device
tree.

The initial code for this feature were posted here [1], merged as commit
9d8eab7af79cb4ce2de5de39f82c455b1f796963 ("drivers: of: add
initialization code for dma reserved memory") and later reverted by
commit 1931ee143b0ab72924944bc06e363d837ba05063. For more information,
see [2]. Finally a new bindings has been proposed [3] and Josh
Cartwright a few days ago prepared some code which implements those
bindings [4]. This finally pushed me again to find some time to finish
this task and review the code. Josh agreed to give me the ownership of
this series to continue preparing them for mainline inclusion.

For more information please refer to the changlelog below.

[1]: http://lkml.kernel.org/g/1377527959-5080-1-git-send-email-m.szyprowski@samsung.com
[2]: http://lkml.kernel.org/g/1381476448-14548-1-git-send-email-m.szyprowski@samsung.com
[3]: http://lkml.kernel.org/g/20131030134702.19B57C402A0@trevor.secretlab.ca
[4]: http://thread.gmane.org/gmane.linux.documentation/19579

Changelog:

v2:
- removed copying of the node name
- split shared-dma-pool handling into separate files (one for CMA and one
  for dma_declare_coherent based implementations) for making the code easier
  to understand
- added support for AMBA devices, changed prototypes to use struct decice
  instead of struct platform_device
- renamed some functions to better match other names used in drivers/of/
- restructured the rest of the code a bit for better readability
- added 'reusable' property to exmaple linux,cma node in documentation
- exclusive dma (dma_coherent) is used for only handling 'shared-dma-pool'
  regions without 'reusable' property and CMA is used only for handling
  'shared-dma-pool' regions with 'reusable' property.

v1: http://thread.gmane.org/gmane.linux.documentation/19579
- initial version prepared by Josh Cartwright

Summary:

Grant Likely (1):
  of: document bindings for reserved-memory nodes

Josh Cartwright (2):
  drivers: of: implement reserved-memory handling for dma
  drivers: of: implement reserved-memory handling for cma

Marek Szyprowski (2):
  drivers: of: add initialization code for reserved memory
  ARM: init: add support for reserved memory defined by device tree

 .../bindings/reserved-memory/reserved-memory.txt   |  138 ++++++++++++
 arch/arm/mm/init.c                                 |    3 +
 drivers/of/Kconfig                                 |   20 ++
 drivers/of/Makefile                                |    3 +
 drivers/of/of_reserved_mem.c                       |  219 ++++++++++++++++++++
 drivers/of/of_reserved_mem_cma.c                   |   75 +++++++
 drivers/of/of_reserved_mem_dma.c                   |   78 +++++++
 drivers/of/platform.c                              |    7 +
 include/asm-generic/vmlinux.lds.h                  |   11 +
 include/linux/of_reserved_mem.h                    |   62 ++++++
 10 files changed, 616 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
 create mode 100644 drivers/of/of_reserved_mem.c
 create mode 100644 drivers/of/of_reserved_mem_cma.c
 create mode 100644 drivers/of/of_reserved_mem_dma.c
 create mode 100644 include/linux/of_reserved_mem.h

-- 
1.7.9.5


^ permalink raw reply

* Re: [PATCH] pci: Add support for creating a generic host_bridge from device tree
From: Liviu Dudau @ 2014-02-04 12:08 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-pci, Bjorn Helgaas, Catalin Marinas, Will Deacon, LKML,
	devicetree@vger.kernel.org, LAKML, linaro-kernel
In-Reply-To: <4524393.8AzpATULMB@wuerfel>

On Tue, Feb 04, 2014 at 10:09:44AM +0000, Arnd Bergmann wrote:
> On Monday 03 February 2014 22:17:44 Liviu Dudau wrote:
> > On Mon, Feb 03, 2014 at 07:31:31PM +0000, Arnd Bergmann wrote:
> > > Let's try to come up with nomenclature so we can talk about this better
> > >
> > > The ioport_resource is in "logical I/O space", which is a Linux fiction,
> > > it goes from 0 to IO_SPACE_LIMIT (2MB on ARM) and is mapped into "virtual
> > > I/O space", which start at (void __iomem *)PCI_IO_VIRT_BASE.
> > >
> > > Each PCI domain can have its own "bus I/O aperture", which is typically
> > > between 0x1000 and 0xffff and reflects the address that is used in PCI
> > > transactions and in BARs.
> > 
> > Actually, the bus I/O aperture can start from 0x0000 if you are talking about
> > PCI bus addresses.
> 
> Right.
> 
> > > The aperture here reflects the subset of the
> > > 4GB bus I/O space that is actually mapped into a CPU visible "physical
> > > I/O aperture" using an inbound mapping of the host bridge. The physical
> > > I/O aperture in turn gets mapped to the virtual I/O space using
> > > pci_ioremap_io.
> > 
> > Agree.
> > 
> > > The difference between a bus I/O address and a logical
> > > I/O address is stored in the io_offset.
> > 
> > Not exactly. If that would be true that means that for an I/O range that
> > start at bus I/O address zero but physical I/O apperture starts at
> > 0x40000000 the io_offset is zero. For me, the io_offset should be 0x40000000.
> 
> That's not how we do it on any of the existing host controllers.
> Typically the io_offset is zero for the first one, and may be
> either zero for all the others (meaning BARs get > 64KB values
> for secondary buses) or between 64KB and 2MB (meaning each bus
> starts at I/O port number 0).

In that case it is probably worth to rename my variable into phys_io_offset.

I need to go back over my driver code. My assumptions were probably wrong
wrt to meaning of the io_offset.

> 
> > Let me see if I can summarise this correctly, using only CPU addresses:
> > 
> > 0x0000 - IO_SPACE_LIMIT           <-  logical I/O address
> > 0xPPPPPPPP - 0xPPPPPPPP+IO_SIZE   <-  physical address for PCI I/O space
> > 0xVVVVVVVV - 0xVVVVVVVV+IO_SPACE_LIMIT <- virtual address for I/O
> > 
> > The io_offset then is 0xPPPPPPPP - logical I/O address. At least that is
> > the intent of the io_offset variable that I introduced in pci_host_bridge.
> 
> That is highly confusing then, because we already have something called
> io_offset with a different meaning. I would call 0xPPPPPPPP the io_phys_base
> if I had to come up with a variable name for it.
> 
> > The bus I/O address is generated by the host bridge, I think we can ignore
> > it here as it tends to confuse the message.
> 
> No, it's important because the PCI core code has to transform between
> bus I/O address and logical I/O address when accessing the BARs.
> 
> > > So much for basic definitions. When a device driver calls pci_request_region,
> > > the port number it sees is the bus I/O port number adjusted using the
> > > io_offset to turn it into a logical I/O port number, which should
> > > always be within the host bridge window, which in turn is a subset
> > > of the ioport_resource.
> > 
> > My understanding is that device drivers all user port numbers that are logical
> > I/O numbers, so no io_offset needs to be applied here. It is only when one
> > wants to access the port, that the translation happens. First, inb or outb
> > will add the PCI_IO_VIRT_BASE to generate the virtual address, the MMU will
> > then convert that address to physical address and the host bridge will
> > then translate the physical address into bus address.
> 
> This is correct. The bus I/O number is not visible to the device driver,
> but it is what you put into the 'ranges' property in DT, and it gets
> used during PCI resource scanning.
> 
> 
> > > > And that is why the code in probe.c has been added to deal with that. It is
> > > > too early to do the adjustments here as all we have is the list of resources
> > > > and that might get culled by the architecture fixup code. Remembering the
> > > > io_offset will happen once the pci_host_bridge gets created, and the resources
> > > > are then adjusted.
> > >
> > > So you want to register an incorrect I/O resource first and then
> > > have it fixed up later, rather than registering the correct
> > > one from the start as everyone else?
> > 
> > The incorrect I/O resource is added to a temporary list of resources, it has not
> > been attached yet to the list of windows in the bridge. What gets added is the
> > I/O resource as described if it would be an ordinary resource.
> 
> I'm not completely sure I'm following here, but let's work out the
> other things first, this will probably get clearer then.
> 
> > > > > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > > > > > index 6e34498..16febae 100644
> > > > > > --- a/drivers/pci/probe.c
> > > > > > +++ b/drivers/pci/probe.c
> > > > > > @@ -1787,6 +1787,17 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
> > > > > >     list_for_each_entry_safe(window, n, resources, list) {
> > > > > >             list_move_tail(&window->list, &bridge->windows);
> > > > > >             res = window->res;
> > > > > > +           /*
> > > > > > +            * IO resources are stored in the kernel with a CPU start
> > > > > > +            * address of zero. Adjust the data accordingly and remember
> > > > > > +            * the offset
> > > > > > +            */
> > > > > > +           if (resource_type(res) == IORESOURCE_IO) {
> > > > > > +                   bridge->io_offset = res->start;
> > > > > > +                   res->end -= res->start;
> > > > > > +                   window->offset -= res->start;
> > > > > > +                   res->start = 0;
> > > > > > +           }
> > 
> > Here, we correct for the fact that IORESOURCE_IO is not a normal resource, because Linux wants
> > a logical I/O as start and end address, not the physical CPU address. We adjust to that and
> > remember the offset.
> 
> But the offset (phys_base) doesn't actually matter to the PCI core or
> the driver. Why save it?

Because I need it later for the host bridge ATR setup.

> 
> > > > > >             offset = window->offset;
> > > > > >             if (res->flags & IORESOURCE_BUS)
> > > > >
> > > > > Won't this break all existing host bridges?
> > > >
> > > > I am not sure. I believe not, due to what I've explained earlier, but you might be right.
> > > >
> > > > The adjustment happens before the resource is added to the host bridge windows and translates
> > > > it from MMIO range into IO range.
> > >
> > > AFAICT, the resource_type of the resource you register above should be
> > > IORESOURCE_MEM, so you are not actually matching it here.
> > 
> > No, all resources are added here. For IORESOURCE_IO we do an adjustment.
> 
> But there should never be an IORESOURCE_IO resource structure that is
> not in IO space, i.e. within ioport_resource. Doing an "adjustment"
> is not an operation defined on this structure. What I meant above is that
> the pci range parser gets this right and gives you a resource that looks
> like { .flags = IORESOURCE_MEM, .start = phys_base, .end = phys_base +
> size - 1}, while the resource we want to register is { .flags = IORESOURCE_IO,
> .start = log_base, .end = log_base + size -1}. In the of_pci_range struct for
> the I/O space, the "pci_space" is IORESOURCE_IO (for the pci_addr), while the
> "flags" are IORESOURCE_MEM, to go along with the cpu_addr.

The pci range parser gives me a range with .flags = IORESOURCE_IO for IO space. It
does not convert it to IORESOURCE_MEM. Hence the need for adjustment.

Best regards,
Liviu

> 
> 	Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

^ permalink raw reply

* Re: [Patch v4 1/2] dmaengine: add Qualcomm BAM dma driver
From: Lars-Peter Clausen @ 2014-02-04 11:58 UTC (permalink / raw)
  To: Andy Gross
  Cc: Vinod Koul, Dan Williams, dmaengine, devicetree, linux-kernel,
	linux-arm-kernel, linux-arm-msm
In-Reply-To: <1391468687-4347-2-git-send-email-agross@codeaurora.org>

On 02/04/2014 12:04 AM, Andy Gross wrote:
[...]
> +static int bam_dma_remove(struct platform_device *pdev)
> +{
> +	struct bam_device *bdev = platform_get_drvdata(pdev);
> +	u32 i;
> +
> +	dma_async_device_unregister(&bdev->common);
> +	of_dma_controller_free(pdev->dev.of_node);

The controller should first be removed from the of lookup table, then free 
the device.

> +
> +	/* mask all interrupts for this execution environment */
> +	writel_relaxed(0, bdev->regs + BAM_IRQ_SRCS_MSK_EE(bdev->ee));

You still need to free the interrupt to make this race free, especially on a 
multi-processor system. free_irq() acts as a synchronization point that 
makes sure that interrupt handler has finished running and that no new 
interrupt handlers are being run after this point. Just masking the 
interrupt in the control register does not provide these guarantees.

> +
> +	for (i = 0; i < bdev->num_channels; i++) {
> +		bam_dma_terminate_all(&bdev->channels[i]);
> +		tasklet_kill(&bdev->channels[i].vc.task);
> +	}
> +
> +	tasklet_kill(&bdev->task);
> +
> +	clk_disable_unprepare(bdev->bamclk);
> +
> +	return 0;
> +}
[...]

^ permalink raw reply

* Re: [PATCH] arm64: Add architecture support for PCI
From: Arnd Bergmann @ 2014-02-04 11:54 UTC (permalink / raw)
  To: Liviu Dudau
  Cc: devicetree@vger.kernel.org, linaro-kernel@lists.linaro.org,
	linux-pci, LKML, Catalin Marinas, Bjorn Helgaas, LAKML
In-Reply-To: <20140204110922.GA27975@e106497-lin.cambridge.arm.com>

On Tuesday 04 February 2014 11:09:22 Liviu Dudau wrote:
> On Tue, Feb 04, 2014 at 08:44:36AM +0000, Arnd Bergmann wrote:

> > Well, I/O space never starts at physical zero in reality, so it is
> > broken in practice. The CONFIG_GENERIC_IOMAP option tries to solve
> > the problem of I/O spaces that are not memory mapped, which is
> > actually quite rare (x86, ia64, some rare powerpc bridges, and possibly
> > Alpha). The norm is that if you have I/O space, it is memory mapped
> > and you don't need GENERIC_IOMAP. I think most of the architectures
> > selecting GENERIC_IOMAP have incorrectly copied that from x86.
> 
> If you are talking about CPU addresses for I/O space, then you are (mostly?) right.
> I've seen some code in powerpc that tries to handle the case where I/O starts at zero.
>
> For MMIO, yes, it would be crazy to start at CPU address zero. But,
> the ioport_map takes a port number, and those do start at zero, right?

What I meant is that asm-generic/io.h assumes that the I/O ports are
mapped at /virtual/ address zero, which is even more crazy, since that
is normally in user space. Sorry for confusing it with physical address
zero.

Now the GENERIC_IOMAP uses a similar fiction by defining that virtual
address token 0x10000-0x1ffff are used to access I/O space when calling
inb/outb, but that is something you only need to do when you have
no memory mapped I/O port.

Some older ARM platforms (PXA for instance) also defined the I/O space
to start at virtual address zero, and use a per-bus io_offset that was
equal to the ioremapped I/O window. This actually works, but it means
that the logical port numbers are all high, and you have to set
IO_SPACE_LIMIT to ULONG_MAX, and it breaks horribly for any driver that
tries to store a port number in a type that is shorter than 'unsigned
long'. We definitely don't want to do this for new code.

> > > My main concern with the existing API is the requirement to have a subsys_initcall
> > > in your host bridge or mach code, due to the way the initialisation is done (you
> > > need the DT code to probe your driver, but you cannot start the scanning of the
> > > PCI bus until the arch code is initialised, so it gets deferred via
> > > subsys_initcall when it calls pci_common_init). I bet that if one tries to
> > > instantiate a Tegra PCI host bridge controller on a Marvell platform things will
> > > break pretty quickly (random example here).
> >
> > I'm not following here. All the new host controller drivers should
> > be platform drivers that only bind to the host devices in DT
> > that are present. Both mvebu and tegra use a normal "module_platform_driver"
> > for initialization, not a "subsys_initcall".
> 
> I was actually looking at mach-dove, I thought that was Marvell as well.

mach-dove is going away soon, it will get merged into mach-mvebu and
then use drivers/pci/host/pci-mvebu.c
 
> But both mvebu and tegra call pci_common_init_dev. The busnr gets assigned based on
> the registration order. I wonder if any of the host bridge code copes with having
> assigned a bus number other than zero for its "root bus".

I think all "new" host bridges now use nr_controllers=1, which means
that you always start at but number zero and use PCI domain if
you have multiple independent root bridges.

> >
> > Right. I guess we can support both interfaces on ARM32 for the forseeable
> > future (renaming the new one) and just change the existing implementation
> > to update the bitmap. Any cross-platform host controller driver would
> > have to use the new interface however.
> 
> OK, I can try to add the function to my patch. Call it pci_ioremap_iores?

Sounds ok, I can't think of anything better at least.

	Arnd

^ permalink raw reply

* Re: [PATCH 1/2] devicetree: Add devicetree bindings documentation for the ADAU1977
From: Lars-Peter Clausen @ 2014-02-04 11:48 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Brown, Liam Girdwood, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAMuHMdVQGZE+1YuWCnJefzy08kb6vNmRN8k+1-K82BubOBAOxQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 02/03/2014 04:15 PM, Geert Uytterhoeven wrote:
> On Mon, Feb 3, 2014 at 3:57 PM, Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org> wrote:
>> + - adi,micbias: Configures the voltage setting for the MICBIAS pin. If not
>> +   specified the default value will be 8.5 Volts. This property is only valid
>> +   for the ADAU1977.
>> +   Valid values for this property are:
>> +       0: 5.0V
>> +       1: 5.5V
>> +       2: 6.0V
>> +       3: 6.5V
>> +       4: 7.0V
>> +       5: 7.5V
>> +       6: 8.0V
>> +       7: 8.5V
>> +       8: 9.0V
>
> This means the casual DTS reader need to know about the meaning of
> the values. You can avoid that by:
>    1. Using preprocessor defines. Drawback is that you have to add an
>       include file.
>    2. Using mV, and specify minimum, maximum, and incrementals in the
>       bindings, cfr.
> Documentation/devicetree/bindings/power_supply/ti,bq24735.txt
>       Drawback is that the user can easily specify invalid values.


Is there a preference for either of them?

Thanks,
- Lars

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v4 2/3] clocksource: keystone: add bindings for keystone timer
From: Ivan Khoronzhuk @ 2014-02-04 11:42 UTC (permalink / raw)
  To: santosh.shilimkar, rob, linux, galak
  Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, daniel.lezcano,
	tglx, devicetree, linux-doc, linux-arm-kernel, linux-kernel,
	grygorii.strashko, Ivan Khoronzhuk
In-Reply-To: <1391513453-21140-3-git-send-email-ivan.khoronzhuk@ti.com>

Sorry I forgot to add
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

On 02/04/2014 01:30 PM, Ivan Khoronzhuk wrote:
> This patch provides bindings for the 64-bit timer in the KeyStone
> architecture devices. The timer can be configured as a general-purpose 64-bit
> timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
> timers, each half can operate in conjunction (chain mode) or independently
> (unchained mode) of each other.
>
> It is global timer is a free running up-counter and can generate interrupt
> when the counter reaches preset counter values.
>
> Documentation:
> http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
>   .../bindings/timer/ti,keystone-timer.txt           | 29 ++++++++++++++++++++++
>   1 file changed, 29 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
>
> diff --git a/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt b/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
> new file mode 100644
> index 0000000..5fbe361
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
> @@ -0,0 +1,29 @@
> +* Device tree bindings for Texas instruments Keystone timer
> +
> +This document provides bindings for the 64-bit timer in the KeyStone
> +architecture devices. The timer can be configured as a general-purpose 64-bit
> +timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
> +timers, each half can operate in conjunction (chain mode) or independently
> +(unchained mode) of each other.
> +
> +It is global timer is a free running up-counter and can generate interrupt
> +when the counter reaches preset counter values.
> +
> +Documentation:
> +http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf
> +
> +Required properties:
> +
> +- compatible : should be "ti,keystone-timer".
> +- reg : specifies base physical address and count of the registers.
> +- interrupts : interrupt generated by the timer.
> +- clocks : the clock feeding the timer clock.
> +
> +Example:
> +
> +timer@22f0000 {
> +	compatible = "ti,keystone-timer";
> +	reg = <0x022f0000 0x80>;
> +	interrupts = <GIC_SPI 110 IRQ_TYPE_EDGE_RISING>;
> +	clocks = <&clktimer15>;
> +};

-- 
Regards,
Ivan Khoronzhuk


^ permalink raw reply

* Re: [PATCH v4 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone
From: Ivan Khoronzhuk @ 2014-02-04 11:41 UTC (permalink / raw)
  To: santosh.shilimkar, rob, linux, galak
  Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, daniel.lezcano,
	tglx, devicetree, linux-doc, linux-arm-kernel, linux-kernel,
	grygorii.strashko, Ivan Khoronzhuk
In-Reply-To: <1391513453-21140-2-git-send-email-ivan.khoronzhuk@ti.com>

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
Acked-by: Santosh shilimkar <santosh.shilimkar@ti.com>

On 02/04/2014 01:30 PM, Ivan Khoronzhuk wrote:
> Add broadcast clock-event device for the Keystone arch.
>
> The timer can be configured as a general-purpose 64-bit timer,
> dual general-purpose 32-bit timers. When configured as dual 32-bit
> timers, each half can operate in conjunction (chain mode) or
> independently (unchained mode) of each other.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
>   drivers/clocksource/Makefile         |   1 +
>   drivers/clocksource/timer-keystone.c | 233 +++++++++++++++++++++++++++++++++++
>   2 files changed, 234 insertions(+)
>   create mode 100644 drivers/clocksource/timer-keystone.c
>
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index c7ca50a..4abe5aa 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -37,3 +37,4 @@ obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
>   obj-$(CONFIG_ARM_GLOBAL_TIMER)		+= arm_global_timer.o
>   obj-$(CONFIG_CLKSRC_METAG_GENERIC)	+= metag_generic.o
>   obj-$(CONFIG_ARCH_HAS_TICK_BROADCAST)	+= dummy_timer.o
> +obj-$(CONFIG_ARCH_KEYSTONE)		+= timer-keystone.o
> diff --git a/drivers/clocksource/timer-keystone.c b/drivers/clocksource/timer-keystone.c
> new file mode 100644
> index 0000000..cbac8d0
> --- /dev/null
> +++ b/drivers/clocksource/timer-keystone.c
> @@ -0,0 +1,233 @@
> +/*
> + * Keystone broadcast clock-event
> + *
> + * Copyright 2013 Texas Instruments, Inc.
> + *
> + * Author: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clockchips.h>
> +#include <linux/clocksource.h>
> +#include <linux/interrupt.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +
> +#define TIMER_NAME			"timer-keystone"
> +
> +/* Timer register offsets */
> +#define TIM12				0x10
> +#define TIM34				0x14
> +#define PRD12				0x18
> +#define PRD34				0x1c
> +#define TCR				0x20
> +#define TGCR				0x24
> +#define INTCTLSTAT			0x44
> +
> +/* Timer register bitfields */
> +#define TCR_ENAMODE_MASK		0xC0
> +#define TCR_ENAMODE_ONESHOT_MASK	0x40
> +#define TCR_ENAMODE_PERIODIC_MASK	0x80
> +
> +#define TGCR_TIM_UNRESET_MASK		0x03
> +#define INTCTLSTAT_ENINT_MASK		0x01
> +
> +/**
> + * struct keystone_timer: holds timer's data
> + * @base: timer memory base address
> + * @hz_period: cycles per HZ period
> + * @event_dev: event device based on timer
> + */
> +static struct keystone_timer {
> +	void __iomem *base;
> +	unsigned long hz_period;
> +	struct clock_event_device event_dev;
> +} timer;
> +
> +static inline u32 keystone_timer_readl(unsigned long rg)
> +{
> +	return readl_relaxed(timer.base + rg);
> +}
> +
> +static inline void keystone_timer_writel(u32 val, unsigned long rg)
> +{
> +	writel_relaxed(val, timer.base + rg);
> +}
> +
> +/**
> + * keystone_timer_config: configures timer to work in oneshot/periodic modes.
> + * @ mode: mode to configure
> + * @ period: cycles number to configure for
> + */
> +static int keystone_timer_config(u64 period, enum clock_event_mode mode)
> +{
> +	u32 tcr;
> +	u32 off;
> +
> +	tcr = keystone_timer_readl(TCR);
> +	off = tcr & ~(TCR_ENAMODE_MASK);
> +
> +	/* set enable mode */
> +	switch (mode) {
> +	case CLOCK_EVT_MODE_ONESHOT:
> +		tcr |= TCR_ENAMODE_ONESHOT_MASK;
> +		break;
> +	case CLOCK_EVT_MODE_PERIODIC:
> +		tcr |= TCR_ENAMODE_PERIODIC_MASK;
> +		break;
> +	default:
> +		return -1;
> +	}
> +
> +	/* disable timer */
> +	keystone_timer_writel(off, TCR);
> +	/* here we have to be sure the timer has been disabled */
> +	wmb();
> +
> +	/* reset counter to zero, set new period */
> +	keystone_timer_writel(0, TIM12);
> +	keystone_timer_writel(0, TIM34);
> +	keystone_timer_writel(period & 0xffffffff, PRD12);
> +	keystone_timer_writel(period >> 32, PRD34);
> +
> +	/*
> +	 * enable timer
> +	 * here we have to be sure that CNTLO, CNTHI, PRDLO, PRDHI registers
> +	 * have been written.
> +	 */
> +	wmb();
> +	keystone_timer_writel(tcr, TCR);
> +	return 0;
> +}
> +
> +static void keystone_timer_disable(void)
> +{
> +	u32 tcr;
> +
> +	tcr = keystone_timer_readl(TCR);
> +
> +	/* disable timer */
> +	tcr &= ~(TCR_ENAMODE_MASK);
> +	keystone_timer_writel(tcr, TCR);
> +}
> +
> +static irqreturn_t keystone_timer_interrupt(int irq, void *dev_id)
> +{
> +	struct clock_event_device *evt = dev_id;
> +
> +	evt->event_handler(evt);
> +	return IRQ_HANDLED;
> +}
> +
> +static int keystone_set_next_event(unsigned long cycles,
> +				  struct clock_event_device *evt)
> +{
> +	return keystone_timer_config(cycles, evt->mode);
> +}
> +
> +static void keystone_set_mode(enum clock_event_mode mode,
> +			     struct clock_event_device *evt)
> +{
> +	switch (mode) {
> +	case CLOCK_EVT_MODE_PERIODIC:
> +		keystone_timer_config(timer.hz_period, CLOCK_EVT_MODE_PERIODIC);
> +		break;
> +	case CLOCK_EVT_MODE_UNUSED:
> +	case CLOCK_EVT_MODE_SHUTDOWN:
> +	case CLOCK_EVT_MODE_ONESHOT:
> +		keystone_timer_disable();
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
> +static void __init keystone_timer_init(struct device_node *np)
> +{
> +	struct clock_event_device *event_dev = &timer.event_dev;
> +	unsigned long rate;
> +	struct clk *clk;
> +	int irq, error;
> +	u32 tgcr;
> +
> +	irq  = irq_of_parse_and_map(np, 0);
> +	if (irq == NO_IRQ) {
> +		pr_err("%s: failed to map interrupts\n", __func__);
> +		return;
> +	}
> +
> +	timer.base = of_iomap(np, 0);
> +	if (!timer.base) {
> +		pr_err("%s: failed to map registers\n", __func__);
> +		return;
> +	}
> +
> +	clk = of_clk_get(np, 0);
> +	if (!clk) {
> +		pr_err("%s: failed to get clock\n", __func__);
> +		iounmap(timer.base);
> +		return;
> +	}
> +
> +	error = clk_prepare_enable(clk);
> +	if (error) {
> +		pr_err("%s: failed to enable clock\n", __func__);
> +		goto err;
> +	}
> +
> +	rate = clk_get_rate(clk);
> +
> +	/* disable, use internal clock source */
> +	keystone_timer_writel(0, TCR);
> +	/* here we have to be sure the timer has been disabled */
> +	wmb();
> +
> +	/* reset timer as 64-bit, no pre-scaler, plus features are disabled */
> +	tgcr = 0;
> +	keystone_timer_writel(0, TGCR);
> +
> +	/* unreset timer */
> +	tgcr |= TGCR_TIM_UNRESET_MASK;
> +	keystone_timer_writel(tgcr, TGCR);
> +
> +	/* init counter to zero */
> +	keystone_timer_writel(0, TIM12);
> +	keystone_timer_writel(0, TIM34);
> +
> +	timer.hz_period = DIV_ROUND_UP(rate, HZ);
> +
> +	/* enable timer interrupts */
> +	keystone_timer_writel(INTCTLSTAT_ENINT_MASK, INTCTLSTAT);
> +
> +	error = request_irq(irq, keystone_timer_interrupt, IRQF_TIMER,
> +			    TIMER_NAME, event_dev);
> +	if (error) {
> +		pr_err("%s: failed to setup irq\n", __func__);
> +		goto err;
> +	}
> +
> +	/* setup clockevent */
> +	event_dev->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
> +	event_dev->set_next_event = keystone_set_next_event;
> +	event_dev->set_mode = keystone_set_mode;
> +	event_dev->cpumask = cpu_all_mask;
> +	event_dev->owner = THIS_MODULE;
> +	event_dev->name = TIMER_NAME;
> +	event_dev->irq = irq;
> +
> +	clockevents_config_and_register(event_dev, rate, 1, ULONG_MAX);
> +
> +	pr_info("keystone timer clock @%lu Hz\n", rate);
> +	return;
> +err:
> +	clk_put(clk);
> +	iounmap(timer.base);
> +}
> +
> +CLOCKSOURCE_OF_DECLARE(keystone_timer, "ti,keystone-timer",
> +					keystone_timer_init);

-- 
Regards,
Ivan Khoronzhuk


^ permalink raw reply

* [PATCH v4 3/3] arm: dts: keystone: add keystone timer entry
From: Ivan Khoronzhuk @ 2014-02-04 11:30 UTC (permalink / raw)
  To: santosh.shilimkar, rob, linux, galak
  Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, daniel.lezcano,
	tglx, devicetree, linux-doc, linux-arm-kernel, linux-kernel,
	grygorii.strashko, Ivan Khoronzhuk
In-Reply-To: <1391513453-21140-1-git-send-email-ivan.khoronzhuk@ti.com>

Add keystone timer entry to keystone device tree.
This 64-bit timer is used as backup clock event device.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 arch/arm/boot/dts/keystone-clocks.dtsi | 10 ++++++++++
 arch/arm/boot/dts/keystone.dtsi        |  7 +++++++
 2 files changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/keystone-clocks.dtsi b/arch/arm/boot/dts/keystone-clocks.dtsi
index 2363593..16d2aba 100644
--- a/arch/arm/boot/dts/keystone-clocks.dtsi
+++ b/arch/arm/boot/dts/keystone-clocks.dtsi
@@ -737,6 +737,16 @@ clocks {
 		domain-id = <0>;
 	};
 
+	clktimer15: clktimer15 {
+		#clock-cells = <0>;
+		compatible = "ti,keystone,psc-clock";
+		clocks = <&clkmodrst0>;
+		clock-output-names = "timer15";
+		reg = <0x02350000 0xb00>, <0x02350000 0x400>;
+		reg-names = "control", "domain";
+		domain-id = <0>;
+	};
+
 	clkuart0: clkuart0 {
 		#clock-cells = <0>;
 		compatible = "ti,keystone,psc-clock";
diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
index b420290..cac9841 100644
--- a/arch/arm/boot/dts/keystone.dtsi
+++ b/arch/arm/boot/dts/keystone.dtsi
@@ -208,5 +208,12 @@
 				usb-phy = <&usb_phy>, <&usb_phy>;
 			};
 		};
+
+		clock_event: timer@22f0000 {
+			compatible = "ti,keystone-timer";
+			reg = <0x022f0000 0x80>;
+			interrupts = <GIC_SPI 110 IRQ_TYPE_EDGE_RISING>;
+			clocks = <&clktimer15>;
+		};
 	};
 };
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH v4 2/3] clocksource: keystone: add bindings for keystone timer
From: Ivan Khoronzhuk @ 2014-02-04 11:30 UTC (permalink / raw)
  To: santosh.shilimkar-l0cyMroinI0, rob-VoJi6FS/r0vR7s880joybQ,
	linux-lFZ/pmaqli7XmaaqVzeoHQ, galak-sgV2jX0FEOL9JmXXK+q4OQ
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A,
	tglx-hfZtesqFncYOwBW4kG4KsQ, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	grygorii.strashko-l0cyMroinI0, Ivan Khoronzhuk
In-Reply-To: <1391513453-21140-1-git-send-email-ivan.khoronzhuk-l0cyMroinI0@public.gmane.org>

This patch provides bindings for the 64-bit timer in the KeyStone
architecture devices. The timer can be configured as a general-purpose 64-bit
timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
timers, each half can operate in conjunction (chain mode) or independently
(unchained mode) of each other.

It is global timer is a free running up-counter and can generate interrupt
when the counter reaches preset counter values.

Documentation:
http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk-l0cyMroinI0@public.gmane.org>
---
 .../bindings/timer/ti,keystone-timer.txt           | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/ti,keystone-timer.txt

diff --git a/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt b/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
new file mode 100644
index 0000000..5fbe361
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
@@ -0,0 +1,29 @@
+* Device tree bindings for Texas instruments Keystone timer
+
+This document provides bindings for the 64-bit timer in the KeyStone
+architecture devices. The timer can be configured as a general-purpose 64-bit
+timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
+timers, each half can operate in conjunction (chain mode) or independently
+(unchained mode) of each other.
+
+It is global timer is a free running up-counter and can generate interrupt
+when the counter reaches preset counter values.
+
+Documentation:
+http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf
+
+Required properties:
+
+- compatible : should be "ti,keystone-timer".
+- reg : specifies base physical address and count of the registers.
+- interrupts : interrupt generated by the timer.
+- clocks : the clock feeding the timer clock.
+
+Example:
+
+timer@22f0000 {
+	compatible = "ti,keystone-timer";
+	reg = <0x022f0000 0x80>;
+	interrupts = <GIC_SPI 110 IRQ_TYPE_EDGE_RISING>;
+	clocks = <&clktimer15>;
+};
-- 
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v4 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone
From: Ivan Khoronzhuk @ 2014-02-04 11:30 UTC (permalink / raw)
  To: santosh.shilimkar, rob, linux, galak
  Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, daniel.lezcano,
	tglx, devicetree, linux-doc, linux-arm-kernel, linux-kernel,
	grygorii.strashko, Ivan Khoronzhuk
In-Reply-To: <1391513453-21140-1-git-send-email-ivan.khoronzhuk@ti.com>

Add broadcast clock-event device for the Keystone arch.

The timer can be configured as a general-purpose 64-bit timer,
dual general-purpose 32-bit timers. When configured as dual 32-bit
timers, each half can operate in conjunction (chain mode) or
independently (unchained mode) of each other.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 drivers/clocksource/Makefile         |   1 +
 drivers/clocksource/timer-keystone.c | 233 +++++++++++++++++++++++++++++++++++
 2 files changed, 234 insertions(+)
 create mode 100644 drivers/clocksource/timer-keystone.c

diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index c7ca50a..4abe5aa 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -37,3 +37,4 @@ obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
 obj-$(CONFIG_ARM_GLOBAL_TIMER)		+= arm_global_timer.o
 obj-$(CONFIG_CLKSRC_METAG_GENERIC)	+= metag_generic.o
 obj-$(CONFIG_ARCH_HAS_TICK_BROADCAST)	+= dummy_timer.o
+obj-$(CONFIG_ARCH_KEYSTONE)		+= timer-keystone.o
diff --git a/drivers/clocksource/timer-keystone.c b/drivers/clocksource/timer-keystone.c
new file mode 100644
index 0000000..cbac8d0
--- /dev/null
+++ b/drivers/clocksource/timer-keystone.c
@@ -0,0 +1,233 @@
+/*
+ * Keystone broadcast clock-event
+ *
+ * Copyright 2013 Texas Instruments, Inc.
+ *
+ * Author: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/clocksource.h>
+#include <linux/interrupt.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+#define TIMER_NAME			"timer-keystone"
+
+/* Timer register offsets */
+#define TIM12				0x10
+#define TIM34				0x14
+#define PRD12				0x18
+#define PRD34				0x1c
+#define TCR				0x20
+#define TGCR				0x24
+#define INTCTLSTAT			0x44
+
+/* Timer register bitfields */
+#define TCR_ENAMODE_MASK		0xC0
+#define TCR_ENAMODE_ONESHOT_MASK	0x40
+#define TCR_ENAMODE_PERIODIC_MASK	0x80
+
+#define TGCR_TIM_UNRESET_MASK		0x03
+#define INTCTLSTAT_ENINT_MASK		0x01
+
+/**
+ * struct keystone_timer: holds timer's data
+ * @base: timer memory base address
+ * @hz_period: cycles per HZ period
+ * @event_dev: event device based on timer
+ */
+static struct keystone_timer {
+	void __iomem *base;
+	unsigned long hz_period;
+	struct clock_event_device event_dev;
+} timer;
+
+static inline u32 keystone_timer_readl(unsigned long rg)
+{
+	return readl_relaxed(timer.base + rg);
+}
+
+static inline void keystone_timer_writel(u32 val, unsigned long rg)
+{
+	writel_relaxed(val, timer.base + rg);
+}
+
+/**
+ * keystone_timer_config: configures timer to work in oneshot/periodic modes.
+ * @ mode: mode to configure
+ * @ period: cycles number to configure for
+ */
+static int keystone_timer_config(u64 period, enum clock_event_mode mode)
+{
+	u32 tcr;
+	u32 off;
+
+	tcr = keystone_timer_readl(TCR);
+	off = tcr & ~(TCR_ENAMODE_MASK);
+
+	/* set enable mode */
+	switch (mode) {
+	case CLOCK_EVT_MODE_ONESHOT:
+		tcr |= TCR_ENAMODE_ONESHOT_MASK;
+		break;
+	case CLOCK_EVT_MODE_PERIODIC:
+		tcr |= TCR_ENAMODE_PERIODIC_MASK;
+		break;
+	default:
+		return -1;
+	}
+
+	/* disable timer */
+	keystone_timer_writel(off, TCR);
+	/* here we have to be sure the timer has been disabled */
+	wmb();
+
+	/* reset counter to zero, set new period */
+	keystone_timer_writel(0, TIM12);
+	keystone_timer_writel(0, TIM34);
+	keystone_timer_writel(period & 0xffffffff, PRD12);
+	keystone_timer_writel(period >> 32, PRD34);
+
+	/*
+	 * enable timer
+	 * here we have to be sure that CNTLO, CNTHI, PRDLO, PRDHI registers
+	 * have been written.
+	 */
+	wmb();
+	keystone_timer_writel(tcr, TCR);
+	return 0;
+}
+
+static void keystone_timer_disable(void)
+{
+	u32 tcr;
+
+	tcr = keystone_timer_readl(TCR);
+
+	/* disable timer */
+	tcr &= ~(TCR_ENAMODE_MASK);
+	keystone_timer_writel(tcr, TCR);
+}
+
+static irqreturn_t keystone_timer_interrupt(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = dev_id;
+
+	evt->event_handler(evt);
+	return IRQ_HANDLED;
+}
+
+static int keystone_set_next_event(unsigned long cycles,
+				  struct clock_event_device *evt)
+{
+	return keystone_timer_config(cycles, evt->mode);
+}
+
+static void keystone_set_mode(enum clock_event_mode mode,
+			     struct clock_event_device *evt)
+{
+	switch (mode) {
+	case CLOCK_EVT_MODE_PERIODIC:
+		keystone_timer_config(timer.hz_period, CLOCK_EVT_MODE_PERIODIC);
+		break;
+	case CLOCK_EVT_MODE_UNUSED:
+	case CLOCK_EVT_MODE_SHUTDOWN:
+	case CLOCK_EVT_MODE_ONESHOT:
+		keystone_timer_disable();
+		break;
+	default:
+		break;
+	}
+}
+
+static void __init keystone_timer_init(struct device_node *np)
+{
+	struct clock_event_device *event_dev = &timer.event_dev;
+	unsigned long rate;
+	struct clk *clk;
+	int irq, error;
+	u32 tgcr;
+
+	irq  = irq_of_parse_and_map(np, 0);
+	if (irq == NO_IRQ) {
+		pr_err("%s: failed to map interrupts\n", __func__);
+		return;
+	}
+
+	timer.base = of_iomap(np, 0);
+	if (!timer.base) {
+		pr_err("%s: failed to map registers\n", __func__);
+		return;
+	}
+
+	clk = of_clk_get(np, 0);
+	if (!clk) {
+		pr_err("%s: failed to get clock\n", __func__);
+		iounmap(timer.base);
+		return;
+	}
+
+	error = clk_prepare_enable(clk);
+	if (error) {
+		pr_err("%s: failed to enable clock\n", __func__);
+		goto err;
+	}
+
+	rate = clk_get_rate(clk);
+
+	/* disable, use internal clock source */
+	keystone_timer_writel(0, TCR);
+	/* here we have to be sure the timer has been disabled */
+	wmb();
+
+	/* reset timer as 64-bit, no pre-scaler, plus features are disabled */
+	tgcr = 0;
+	keystone_timer_writel(0, TGCR);
+
+	/* unreset timer */
+	tgcr |= TGCR_TIM_UNRESET_MASK;
+	keystone_timer_writel(tgcr, TGCR);
+
+	/* init counter to zero */
+	keystone_timer_writel(0, TIM12);
+	keystone_timer_writel(0, TIM34);
+
+	timer.hz_period = DIV_ROUND_UP(rate, HZ);
+
+	/* enable timer interrupts */
+	keystone_timer_writel(INTCTLSTAT_ENINT_MASK, INTCTLSTAT);
+
+	error = request_irq(irq, keystone_timer_interrupt, IRQF_TIMER,
+			    TIMER_NAME, event_dev);
+	if (error) {
+		pr_err("%s: failed to setup irq\n", __func__);
+		goto err;
+	}
+
+	/* setup clockevent */
+	event_dev->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
+	event_dev->set_next_event = keystone_set_next_event;
+	event_dev->set_mode = keystone_set_mode;
+	event_dev->cpumask = cpu_all_mask;
+	event_dev->owner = THIS_MODULE;
+	event_dev->name = TIMER_NAME;
+	event_dev->irq = irq;
+
+	clockevents_config_and_register(event_dev, rate, 1, ULONG_MAX);
+
+	pr_info("keystone timer clock @%lu Hz\n", rate);
+	return;
+err:
+	clk_put(clk);
+	iounmap(timer.base);
+}
+
+CLOCKSOURCE_OF_DECLARE(keystone_timer, "ti,keystone-timer",
+					keystone_timer_init);
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH v4 0/3] Introduce clocksource driver for Keystone platform
From: Ivan Khoronzhuk @ 2014-02-04 11:30 UTC (permalink / raw)
  To: santosh.shilimkar, rob, linux, galak
  Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, daniel.lezcano,
	tglx, devicetree, linux-doc, linux-arm-kernel, linux-kernel,
	grygorii.strashko, Ivan Khoronzhuk

Add a broadcast timer64 based clockevent driver for keystone arch.
This driver uses timer in 64-bit general purpose mode as clock event
device.

Documentation:
    http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf

Based on
git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux-keystone.git
keystone/master

v3..v4:
	rebased on latest of linux-keystone.git keystone/master

v2..v3:
- clocksource: timer-keystone: introduce clocksource driver for
	changed "u64" type to "unsigned long" for hz_period as more appropriate
	hz_period rounded up by DIV_ROUND_UP(rate, HZ)
	corrected comments

v1..v2:
- clocksource: timer-keystone: introduce clocksource driver for
	renamed timer on "timer-keystone"
	in keystone_timer_interrupt() evet pointer is passed via "dev_id"
	used __relaxed variants of writel/readl and added explicit barriers
	added "keystone_timer_disable()" for using in keystone_set_mode()
	keystone_timer_config() is not used for disabling the timer any more
	in case of an unsupported mode the keystone_timer_config() returns -1.
	used request_irq() instead of setup_irq()
	assigned irq for event_device in event_dev->irq
	calculated timer.hz_period for CLOCK_EVT_MODE_PERIODIC at init
	deleted spare call of keystone_timer_config() in keystone_timer_init()
*

Ivan Khoronzhuk (3):
  clocksource: timer-keystone: introduce clocksource driver for Keystone
  clocksource: keystone: add bindings for keystone timer
  arm: dts: keystone: add keystone timer entry

 .../bindings/timer/ti,keystone-timer.txt           |  29 +++
 arch/arm/boot/dts/keystone-clocks.dtsi             |  10 +
 arch/arm/boot/dts/keystone.dtsi                    |   7 +
 drivers/clocksource/Makefile                       |   1 +
 drivers/clocksource/timer-keystone.c               | 233 +++++++++++++++++++++
 5 files changed, 280 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/ti,keystone-timer.txt
 create mode 100644 drivers/clocksource/timer-keystone.c

-- 
1.8.3.2


^ permalink raw reply

* Re: [PATCH] arm64: Add architecture support for PCI
From: Liviu Dudau @ 2014-02-04 11:09 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linaro-kernel@lists.linaro.org, devicetree@vger.kernel.org,
	Catalin Marinas, LKML, linux-pci, Bjorn Helgaas, LAKML
In-Reply-To: <3277167.UhkSU8Sf56@wuerfel>

On Tue, Feb 04, 2014 at 08:44:36AM +0000, Arnd Bergmann wrote:
> On Monday 03 February 2014 21:36:58 Liviu Dudau wrote:
> > On Mon, Feb 03, 2014 at 08:05:56PM +0000, Arnd Bergmann wrote:
> > > On Monday 03 February 2014 19:18:38 Liviu Dudau wrote:
> > > > So ... defining it should mean no legacy ISA devices, right?
> > > 
> > > I would read that comment as referring to systems that don't have
> > > any I/O space. If you have PCI, you can by definition have ISA
> > > compatible devices behind a bridge. A typical example would be
> > > a VGA card that supports the 03c0-03df port range.
> > 
> > But if you have PCI and don't want to support ISA do you have /dev/port? I guess yes.
> 
> Right, that is my interpretation. You could still have a tool
> that tries to poke /dev/port from user space for any I/O BAR
> the same way you'd use /dev/mem for memory BARs of PCI devices.
> 
> It's discouraged, but it's often the easiest solution for
> a quick hack, and I'd expect tools to use this.
> 
> > > > > >  #define IO_SPACE_LIMIT		0xffff
> > > > > 
> > > > > You probably want to increase this a bit, to allow multiple host bridges
> > > > > to have their own I/O space.
> > > > 
> > > > OK, but to what size?
> > > 
> > > 2 MB was a compromise on arm32 to allow up to 32 PCI host bridges but not
> > > take up too much virtual space. On arm64 it should be at least as big.
> > > Could be more than that, although I don't see a reason why it should be,
> > > unless we expect to see systems with tons of host bridges, or buses
> > > that exceed 64KB of I/O space.
> > 
> > I will increase the size to 2MB for v2.
> 
> Thinking about this some more, I'd go a little higher. In case of
> pci_mv, we already register a 1MB region for one logical host
> (which has multiple I/O spaces behind an emulated bridge), so
> going to 16MB or more would let us handle multiple 1MB windows
> for some amount of future proofing.
> 
> Maybe Catalin can assign us some virtual address space to use,
> with the constraints that it should be 16MB or more in an
> area that doesn't require additional kernel page table pages.

I'll pick Catalin's brain for suggestions.

>  
> > > > > > +#define ioport_map(port, nr)	(PCI_IOBASE + ((port) & IO_SPACE_LIMIT))
> > > > > > +#define ioport_unmap(addr)
> > > > > 
> > > > > inline functions?
> > > > 
> > > > Will do, thanks!
> > > 
> > > I suppose you can actually use the generic implementation from
> > > asm-generic/io.h, and fix it by using the definition you have
> > > above, since it's currently broken.
> > 
> > Not exactly broken, but it makes the assumption that your IO space starts at
> > physical address zero and you have not remapped it. It does guard the
> > definition with #ifndef CONFIG_GENERIC_IOMAP after all, so it does not
> > expect to be generic :)
> 
> Well, I/O space never starts at physical zero in reality, so it is
> broken in practice. The CONFIG_GENERIC_IOMAP option tries to solve
> the problem of I/O spaces that are not memory mapped, which is
> actually quite rare (x86, ia64, some rare powerpc bridges, and possibly
> Alpha). The norm is that if you have I/O space, it is memory mapped
> and you don't need GENERIC_IOMAP. I think most of the architectures
> selecting GENERIC_IOMAP have incorrectly copied that from x86.

If you are talking about CPU addresses for I/O space, then you are (mostly?) right.
I've seen some code in powerpc that tries to handle the case where I/O starts at zero.

For MMIO, yes, it would be crazy to start at CPU address zero. But, the ioport_map
takes a port number, and those do start at zero, right?

> 
> > > > > > diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h
> > > > > > new file mode 100644
> > > > > > index 0000000..dd084a3
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm64/include/asm/pci.h
> > > > > > @@ -0,0 +1,35 @@
> > > > > > +#ifndef __ASM_PCI_H
> > > > > > +#define __ASM_PCI_H
> > > > > > +#ifdef __KERNEL__
> > > > > > +
> > > > > > +#include <linux/types.h>
> > > > > > +#include <linux/slab.h>
> > > > > > +#include <linux/dma-mapping.h>
> > > > > > +
> > > > > > +#include <asm/io.h>
> > > > > > +#include <asm-generic/pci-bridge.h>
> > > > > > +#include <asm-generic/pci-dma-compat.h>
> > > > > > +
> > > > > > +#define PCIBIOS_MIN_IO		0
> > > > > > +#define PCIBIOS_MIN_MEM		0
> > > > > 
> > > > > PCIBIOS_MIN_IO is normally set to 0x1000, to stay out of the ISA range.
> > > > 
> > > > :) No ISA support! (Die ISA, die!!) 
> > > 
> > > If only it were that easy.
> > 
> > Lets try! :)
> > 
> > I wonder how many active devices that have an ISA slot are still supported
> > by mainline kernel.
> 
> This is missing the point, but any architecture that has a PCI
> slot can have an ISA device behind a bridge like this:
> 
> http://www.altera.com/products/ip/iup/pci/m-eur-pci-to-isa.html
> 
> The real reason is that a lot of PCI cards for practical reasons
> expose some non-relocatable memory and I/O BARs in ISA-compatible
> locations. Looking at /proc/ioports on my PC, I can spot a couple
> of things that may well show up on any ARM machine:
> 
> 0000-03af : PCI Bus 0000:00
>   02f8-02ff : serial
> 03c0-03df : PCI Bus 0000:40
>   03c0-03df : PCI Bus 0000:00
>     03c0-03df : vga+
> 03e0-0cf7 : PCI Bus 0000:00
>   03e8-03ef : serial
>   03f8-03ff : serial
>   0b00-0b0f : pnp 00:08
>     0b00-0b07 : piix4_smbus
>   0b20-0b3f : pnp 00:08
>     0b20-0b27 : piix4_smbus
>   0ca2-0ca3 : pnp 00:08
>     0ca2-0ca2 : ipmi_si
>     0ca3-0ca3 : ipmi_si
>   0cf8-0cff : PCI conf1
> 
> Nothing wrong with the above. Now, it's also possible that
> someone decides to build an ARM server by using a PC south
> bridge with integrated legacy PC peripherals, such as these:
> 
> 0000-03af : PCI Bus 0000:00
>   0000-001f : dma1
>   0020-0021 : pic1
>   0040-0043 : timer0
>   0050-0053 : timer1
>   0060-0060 : keyboard
>   0064-0064 : keyboard
>   0070-0071 : rtc0
>   0080-008f : dma page reg
>   00a0-00a1 : pic2
>   00b0-00b0 : APEI ERST
>   00c0-00df : dma2
>   00f0-00ff : fpu
> 
> There is some hope that it won't happen, but these things
> exist and come with a regular PCIe front-end bus in some
> cases.
> 
> Finally, there is the LPC bus, which can give you additional
> ISAPnP compatible devices.
> 
> > I will update PCIBIOS_MIN_xxxx to match arch/arm for v2.
> 
> Ok.
> 
> > > > > > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> > > > > > new file mode 100644
> > > > > > index 0000000..7b652cf
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm64/kernel/pci.c
> > > > > > @@ -0,0 +1,112 @@
> > > > > 
> > > > > None of this looks really arm64 specific, nor should it be. I think
> > > > > we should try a little harder to move this as a default implementation
> > > > > into common code, even if we start out by having all architectures
> > > > > override it.
> > > > 
> > > > Agree. This is the RFC version. I didn't dare to post a patch with fixes
> > > > for all architectures. :)
> > > 
> > > No need to change the other architectures. You can make it opt-in for
> > > now and just put the code into a common location.
> > >  
> > > An interesting question however is what the transition plan is to
> > > have the code shared between arm32 and arm64: We will certainly need
> > > to share at least the dw-pcie and the generic SBSA compliant pci
> > > implementation.
> > 
> > My vote would be for updating the host controllers to the new API and
> > to offer the CONFIG option to choose between arch APIs. The alternative
> > is to use the existing API to wrap the generic implementation.
> 
> The problem with an either/or CONFIG option is that it breaks
> multiplatform support. A lot of the arm32 PCI implementations
> are only used on platforms that are not multiplatform enabled
> though, so we could get away by requiring all multiplatform
> configurations to use the new API.

I was thinking more in terms of catching uses of the old API in the new host bridge
driver. The added functions should be able to live beside the old API for the
generic PCI framework, not sure how arm arch code should handle it.

> 
> > My main concern with the existing API is the requirement to have a subsys_initcall
> > in your host bridge or mach code, due to the way the initialisation is done (you
> > need the DT code to probe your driver, but you cannot start the scanning of the 
> > PCI bus until the arch code is initialised, so it gets deferred via 
> > subsys_initcall when it calls pci_common_init). I bet that if one tries to
> > instantiate a Tegra PCI host bridge controller on a Marvell platform things will
> > break pretty quickly (random example here).
> 
> I'm not following here. All the new host controller drivers should
> be platform drivers that only bind to the host devices in DT
> that are present. Both mvebu and tegra use a normal "module_platform_driver"
> for initialization, not a "subsys_initcall".

I was actually looking at mach-dove, I thought that was Marvell as well.

But both mvebu and tegra call pci_common_init_dev. The busnr gets assigned based on
the registration order. I wonder if any of the host bridge code copes with having
assigned a bus number other than zero for its "root bus".

> 
> > > Something like this (coded in mail client, don't try to compile):
> > > 
> > > #define IO_SPACE_PAGES (IO_SPACE_LIMIT + 1) / PAGE_SIZE)
> > > static DECLARE_BITMAP(pci_iospace, IO_SPACE_PAGES);
> > > unsigned long pci_ioremap_io(const struct resource *bus, const struct resource phys)
> > > {
> > > 	unsigned long start, len, virt_start;
> > > 	int error;
> > > 
> > > 	/* use logical address == bus address if possible */
> > > 	start = bus->start / PAGE_SIZE;
> > > 	if (start > IO_SPACE_LIMIT / PAGE_SIZE)
> > > 		start = 0;
> > > 
> > > 	/*
> > > 	 * try finding free space for the whole size first,
> > > 	 * fall back to 64K if not available
> > > 	 */
> > > 	len = min(resource_size(bus), resource_size(phys);
> > > 	start = bitmap_find_next_zero_area(pci_iospace, IO_SPACE_PAGES,
> > > 				start, len / PAGE_SIZE, 0);
> > > 	if (start == IO_SPACE_PAGES && len > SZ_64K)
> > > 		len = SZ_64K;
> > > 		start = 0;
> > > 		start = bitmap_find_next_zero_area(pci_iospace, IO_SPACE_PAGES,
> > > 				start, len / PAGE_SIZE, 0);
> > > 	}
> > > 
> > > 	/* no 64K area found */
> > > 	if (start == IO_SPACE_PAGES)
> > > 		return -ENOMEM;
> > > 
> > > 	/* ioremap physical aperture to virtual aperture */
> > > 	virt_start = start * PAGE_SIZE + (unsigned long)PCI_IOBASE;
> > > 	error = ioremap_page_range(virt_start, virt_start + len,
> > > 				    phys->start, __pgprot(PROT_DEVICE_nGnRE));
> > > 	if (error)
> > > 		return error;
> > > 
> > > 	bitmap_set(start, len / PAGE_SIZE);
> > > 
> > > 	/* return io_offset */
> > > 	return start * PAGE_SIZE - bus->start;
> > > }
> > > EXPORT_SYMBOL_GPL(pci_ioremap_io);
> > > 
> > > 	Arnd
> > > 
> > 
> > I see. I need to think how this will change the existing code. Current users
> > of pci_ioremap_io  ask for multiples of SZ_64K offsets regardless of the
> > actual need. 
> 
> Right. I guess we can support both interfaces on ARM32 for the forseeable
> future (renaming the new one) and just change the existing implementation
> to update the bitmap. Any cross-platform host controller driver would
> have to use the new interface however.
> 
> 	Arnd

OK, I can try to add the function to my patch. Call it pci_ioremap_iores?

Best regards,
Liviu

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

^ permalink raw reply

* Re: [PATCH v3 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver
From: Mark Brown @ 2014-02-04 11:02 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mike Turquette, Emilio Lopez, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	kevin.z.m.zh-Re5JQEeQqe8AvxtiuMwx3w,
	sunny-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf,
	shuge-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf,
	zhuzhenhua-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf, Kevin Hilman
In-Reply-To: <20140204090926.GI25625@lukather>

[-- Attachment #1: Type: text/plain, Size: 1283 bytes --]

On Tue, Feb 04, 2014 at 10:09:26AM +0100, Maxime Ripard wrote:
> On Tue, Feb 04, 2014 at 12:21:10AM +0000, Mark Brown wrote:

> > It isn't awesome, no.  Ideally the runtime PM code would do this but
> > then you couldn't ifdef the operations which as far as I can tell is the
> > main thing people want from disabling it and it gets complicated for
> > devices that genuinely do power up on startup so here we are.

> We discussed it with Kevin on IRC, and he suggested that we move that
> pm_runtime initialization to the SPI core, but I guess that would also
> mean that all drivers shouldn't ifdef the operations, so that the core
> can call the runtime_resume callback directly.

No, that's not going to be robust - it means drivers can't do any power
sequencing of their own.

> However, I don't really get why any driver should be doing so, since
> you still need these functions to at least to the device
> suspend/resume in the probe/remove, and you don't really want to
> duplicate the code.

I don't think it's particularly useful to support disabling runtime PM
in the first place but some drivers do different things when doing
runtime management to those they do on first init - for example there
may be additional steps that only need to be done during first power up.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v2] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support
From: Srikanth Thokala @ 2014-02-04 10:35 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Srikanth Thokala, Lars-Peter Clausen,
	dan.j.williams-ral2JQCrhuEAvxtiuMwx3w,
	michal.simek-gjFFaj9aHVfQT0dZR+AlfA, Grant Likely,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140204052810.GO10628-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

On Tue, Feb 4, 2014 at 10:58 AM, Vinod Koul <vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, Jan 31, 2014 at 12:22:52PM +0530, Srikanth Thokala wrote:
>> >>> >> [...]
>> >>> >>> +/**
>> >>> >>> + * xilinx_vdma_device_control - Configure DMA channel of the device
>> >>> >>> + * @dchan: DMA Channel pointer
>> >>> >>> + * @cmd: DMA control command
>> >>> >>> + * @arg: Channel configuration
>> >>> >>> + *
>> >>> >>> + * Return: '0' on success and failure value on error
>> >>> >>> + */
>> >>> >>> +static int xilinx_vdma_device_control(struct dma_chan *dchan,
>> >>> >>> +                                   enum dma_ctrl_cmd cmd, unsigned long arg)
>> >>> >>> +{
>> >>> >>> +     struct xilinx_vdma_chan *chan = to_xilinx_chan(dchan);
>> >>> >>> +
>> >>> >>> +     switch (cmd) {
>> >>> >>> +     case DMA_TERMINATE_ALL:
>> >>> >>> +             xilinx_vdma_terminate_all(chan);
>> >>> >>> +             return 0;
>> >>> >>> +     case DMA_SLAVE_CONFIG:
>> >>> >>> +             return xilinx_vdma_slave_config(chan,
>> >>> >>> +                                     (struct xilinx_vdma_config *)arg);
>> >>> >>
>> >>> >> You really shouldn't be overloading the generic API with your own semantics.
>> >>> >> DMA_SLAVE_CONFIG should take a dma_slave_config and nothing else.
>> >>> >
>> >>> > Ok.  The driver needs few additional configuration from the slave
>> >>> > device like Vertical
>> >>> > Size, Horizontal Size,  Stride etc., for the DMA transfers, in that case do you
>> >>> > suggest me to define a separate dma_ctrl_cmd like the one FSLDMA_EXTERNAL_START
>> >>> > defined for Freescale drivers?
>> >>>
>> >>> In my opinion it is not a good idea to have driver implement a generic API,
>> >>> but at the same time let the driver have custom semantics for those API
>> >>> calls. It's a bit like having a gpio driver that expects 23 and 42 as the
>> >>> values passed to gpio_set_value instead of 0 and 1. It completely defeats
>> >>> the purpose of a generic API, namely that you are able to write generic code
>> >>> that makes use of the API without having to know about which implementation
>> >>> API it is talking to. The dmaengine framework provides the
>> >>> dmaengine_prep_interleaved_dma() function to setup two dimensional
>> >>> transfers, e.g. take a look at sirf-dma.c or imx-dma.c.
>> >>
>> >> The question here i think would be waht this device supports? Is the hardware
>> >> capable of doing interleaved transfers, then would make sense.
>> >>
>> >> While we do try to get users use dma_slave_config, but there will always be
>> >> someone who have specfic params. If we can generalize then we might want to add
>> >> to the dma_slave_config as well
>> >
>> > There are many configuration parameters which are specific to IP and I
>> > would like to
>> > give an overview of some of parameteres here:
>> >
>> > 1) Park Mode ('cfg->park'): In Park mode, engine will park on frame
>> > referenced by
>> >     'cfg->park_frm', so user will have control on each frame in this mode.
>> >
>> > 2) Interrupt Coalesce ('cfg->coalesce'):  Used for setting interrupt
>> > threshold. This value
>> >    determines the number of frame buffers to process. To use this feature,
>> >    'cfg->frm_cnt_en' should be set.
>> >
>> > 3) Frame Synchronization Source ('cfg->ext_fsync'):  Can be an
>> > external/internal frame
>> >     synchronization source. Used to synchronize one channel (MM2S/S2MM) with
>> >     another (S2MM/MM2S) channel.
>> >
>> > 4) Genlock Synchronization ('cfg->genlock'): Used to avoid mismatch rate between
>> >     master and slave.  In master mode (cfg->master), frames are not dropped and
>> >     slave can drop frames to adjust to master frame rate.
>> >
>> > And in future, this Engine being a soft IP, we could expect some more additional
>> > parameters.  Isn't a good idea to have a private member in dma_slave_config for
>> > sharing additional configuration between slave device and dma engine? Or a new
>> > dma_ctrl_cmd like FSLDMA_EXTERNAL_START?
>
> The idea of a generic API is that we can use it for most of the controllers. Even
> if you are planning to support a family of controllers
>
> ATM, lets not discuss the possiblity of private member and try to exhanust all
> possible options. Worst case you can embed the dma_slave_config in
> xilinx_dma_slave_config and retrieve it in dmac driver

Ok.

Srikanth

>
> --
> ~Vinod
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v5 08/14] ahci-platform: "Library-ise" suspend / resume functionality
From: Hans de Goede @ 2014-02-04 10:20 UTC (permalink / raw)
  To: Arnd Bergmann, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Tejun Heo, devicetree, linux-ide-u79uwXL29TY76Z2rM5mHXA,
	Oliver Schinagl, Richard Zhu, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	Maxime Ripard, Roger Quadros
In-Reply-To: <201402031553.46083.arnd-r2nGTMty4D4@public.gmane.org>

Hi,

On 02/03/2014 03:53 PM, Arnd Bergmann wrote:
> On Wednesday 22 January 2014, Hans de Goede wrote:
>> --- a/include/linux/ahci_platform.h
>> +++ b/include/linux/ahci_platform.h
>> @@ -50,4 +50,11 @@ int ahci_platform_init_host(struct platform_device *pdev,
>>                             unsigned int force_port_map,
>>                             unsigned int mask_port_map);
>>  
>> +#ifdef CONFIG_PM_SLEEP
>> +int ahci_platform_suspend_host(struct device *dev);
>> +int ahci_platform_resume_host(struct device *dev);
>> +int ahci_platform_suspend(struct device *dev);
>> +int ahci_platform_resume(struct device *dev);
>> +#endif
>> +
> 
> Not sure if the #ifdef does any good here. Normally, we don't hide declarations
> so we can do stuff like

Thanks for the review, I'll remove the #ifdef in the next version of this patch-set.

Regards,

Hans

^ permalink raw reply

* Re: [PATCH v2 2/5] clk: sunxi: Add USB clock register defintions
From: Hans de Goede @ 2014-02-04 10:14 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Emilio Lopez, Mike Turquette, Philipp Zabel,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Roman Byshko
In-Reply-To: <20140204094051.GL25625@lukather>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

On 02/04/2014 10:40 AM, Maxime Ripard wrote:
> Hi Hans,
> 
> On Tue, Jan 28, 2014 at 11:00:45AM +0100, Hans de Goede wrote:
>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
>> 
>> Hi,
>> 
>> On 01/28/2014 10:44 AM, Maxime Ripard wrote:
>>> On Mon, Jan 27, 2014 at 03:54:14PM +0100, Hans de Goede wrote:
>>>>>> "allwinner,sun5i-a13-usb-gates-clk" - for usb gates + resets on A13
>>>>> 
>>>>> Maybe we can just remove the gates from there? Even though they are gates, they are also (a bit) more than that.
>>>> 
>>>> To be clear you mean s/usb-gates-clk/usb-clk/ right ?
>>> 
>>> Yep, exactly
>>> 
>>>>> I guess that means that we will have the OHCI0 gate declared with <&...-gates-clk 6>, while it's actually the first gate for this clock?
>>>> 
>>>> Correct.
>>>> 
>>>>> Maybe introducing an offset field in the gates_data would be a good idea, so that we always start from indexing the gates from 0 in the DT?
>>>> 
>>>> Well for the other "gates" type clks we also have holes in the range, and we always refer to the clk with the bit number in the reg as the clock-cell value.
>>> 
>>> Yes, we have holes, but I see two majors differences here: - the other gates are just gates, while the usb clocks are a bit more than that.
>> 
>> The usb-clk registers contain more then that, but the bits we are talking about now are gates.
>> 
>>> - the other gates' gating bits thus all start at bit 0, while - here, since it's kind of a "mixed" clock, the gating bits start - at bit 6 (on the A20 at least)
>> 
>> Right, still I believe that the consistent thing to do is keeping the bit-number for the bit in the register controlling the gate as the specifier.  When adding new dts entries / reviewing existing ones I'm used to matching the specifier to the bit-nr in the data-sheet, I think making things different just for this one register is counter productive.
> 
> And if you turn it the other way around, it would be inconsistent that all gates indices start at 0, and we would start at 6 here :)

I think the problem here is that you see the specifier part of the clk
phandle as an index, which it is not. All devicetree docs / code talks
about specifiers or arguments not indexes. Once you stop seeing this as
an index, you will hopefully also stop insisting this needs to
start at 0 :)

Also note that it already is not an index for existing sunxi clks which have
cells != 0, as there are holes in the bits used in the gates registers and
calling the specifier an index suggest we're dealing with an array, and
arrays never have holes.

The clk specifier as currently used in sunxi clks is a 1:1 mapping of the
gate register bit numbers, as is clearly documented in ie:
Documentation/devicetree/bindings/clock/sunxi/sun4i-a10-gates.txt
Where the datasheet is referenced as the source for (most) of the values
to put in the specifier.

My biggest objection is that this would loose 1:1 mapping we currently
have between the specifier and bit-nr in the register, which really is
convenient when writing new dts bindings.

When we add an offset users will need to first lookup which clk they need in
the datasheet and then look at both the dts bindings doc to find how this is
mapped to the specifier. In my experience such an extra level of indirection
in documentation is a PITA, and all that just so that some random number
(it is not an index!) can start at 0 ?

To me adding an offset here and making the clk gates different form all
our other clock gates just feels wrong.

Regards,

Hans
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlLwvZMACgkQF3VEtJrzE/sz0gCfQNwhM/RpimtbhumvKKQ4a4V+
Vo4AoLTNKRZXlPC84hi1JInPGYvZIxuR
=dA68
-----END PGP SIGNATURE-----

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/groups/opt_out.

^ permalink raw reply

* Re: [PATCH] pci: Add support for creating a generic host_bridge from device tree
From: Arnd Bergmann @ 2014-02-04 10:09 UTC (permalink / raw)
  To: Liviu Dudau
  Cc: linux-pci, Bjorn Helgaas, Catalin Marinas, Will Deacon, LKML,
	devicetree@vger.kernel.org, LAKML, linaro-kernel
In-Reply-To: <20140203221743.GB24036@e106497-lin.cambridge.arm.com>

On Monday 03 February 2014 22:17:44 Liviu Dudau wrote:
> On Mon, Feb 03, 2014 at 07:31:31PM +0000, Arnd Bergmann wrote:
> > Let's try to come up with nomenclature so we can talk about this better
> >
> > The ioport_resource is in "logical I/O space", which is a Linux fiction,
> > it goes from 0 to IO_SPACE_LIMIT (2MB on ARM) and is mapped into "virtual
> > I/O space", which start at (void __iomem *)PCI_IO_VIRT_BASE.
> >
> > Each PCI domain can have its own "bus I/O aperture", which is typically
> > between 0x1000 and 0xffff and reflects the address that is used in PCI
> > transactions and in BARs.
> 
> Actually, the bus I/O aperture can start from 0x0000 if you are talking about
> PCI bus addresses.

Right.

> > The aperture here reflects the subset of the
> > 4GB bus I/O space that is actually mapped into a CPU visible "physical
> > I/O aperture" using an inbound mapping of the host bridge. The physical
> > I/O aperture in turn gets mapped to the virtual I/O space using
> > pci_ioremap_io.
> 
> Agree.
> 
> > The difference between a bus I/O address and a logical
> > I/O address is stored in the io_offset.
> 
> Not exactly. If that would be true that means that for an I/O range that
> start at bus I/O address zero but physical I/O apperture starts at
> 0x40000000 the io_offset is zero. For me, the io_offset should be 0x40000000.

That's not how we do it on any of the existing host controllers.
Typically the io_offset is zero for the first one, and may be
either zero for all the others (meaning BARs get > 64KB values
for secondary buses) or between 64KB and 2MB (meaning each bus
starts at I/O port number 0).

> Let me see if I can summarise this correctly, using only CPU addresses:
> 
> 0x0000 - IO_SPACE_LIMIT           <-  logical I/O address
> 0xPPPPPPPP - 0xPPPPPPPP+IO_SIZE   <-  physical address for PCI I/O space
> 0xVVVVVVVV - 0xVVVVVVVV+IO_SPACE_LIMIT <- virtual address for I/O
> 
> The io_offset then is 0xPPPPPPPP - logical I/O address. At least that is
> the intent of the io_offset variable that I introduced in pci_host_bridge.

That is highly confusing then, because we already have something called
io_offset with a different meaning. I would call 0xPPPPPPPP the io_phys_base
if I had to come up with a variable name for it.

> The bus I/O address is generated by the host bridge, I think we can ignore
> it here as it tends to confuse the message.

No, it's important because the PCI core code has to transform between
bus I/O address and logical I/O address when accessing the BARs.

> > So much for basic definitions. When a device driver calls pci_request_region,
> > the port number it sees is the bus I/O port number adjusted using the
> > io_offset to turn it into a logical I/O port number, which should
> > always be within the host bridge window, which in turn is a subset
> > of the ioport_resource.
> 
> My understanding is that device drivers all user port numbers that are logical
> I/O numbers, so no io_offset needs to be applied here. It is only when one
> wants to access the port, that the translation happens. First, inb or outb
> will add the PCI_IO_VIRT_BASE to generate the virtual address, the MMU will
> then convert that address to physical address and the host bridge will
> then translate the physical address into bus address.

This is correct. The bus I/O number is not visible to the device driver,
but it is what you put into the 'ranges' property in DT, and it gets
used during PCI resource scanning.


> > > And that is why the code in probe.c has been added to deal with that. It is
> > > too early to do the adjustments here as all we have is the list of resources
> > > and that might get culled by the architecture fixup code. Remembering the
> > > io_offset will happen once the pci_host_bridge gets created, and the resources
> > > are then adjusted.
> >
> > So you want to register an incorrect I/O resource first and then
> > have it fixed up later, rather than registering the correct
> > one from the start as everyone else?
> 
> The incorrect I/O resource is added to a temporary list of resources, it has not
> been attached yet to the list of windows in the bridge. What gets added is the
> I/O resource as described if it would be an ordinary resource.

I'm not completely sure I'm following here, but let's work out the
other things first, this will probably get clearer then.

> > > > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > > > > index 6e34498..16febae 100644
> > > > > --- a/drivers/pci/probe.c
> > > > > +++ b/drivers/pci/probe.c
> > > > > @@ -1787,6 +1787,17 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
> > > > >     list_for_each_entry_safe(window, n, resources, list) {
> > > > >             list_move_tail(&window->list, &bridge->windows);
> > > > >             res = window->res;
> > > > > +           /*
> > > > > +            * IO resources are stored in the kernel with a CPU start
> > > > > +            * address of zero. Adjust the data accordingly and remember
> > > > > +            * the offset
> > > > > +            */
> > > > > +           if (resource_type(res) == IORESOURCE_IO) {
> > > > > +                   bridge->io_offset = res->start;
> > > > > +                   res->end -= res->start;
> > > > > +                   window->offset -= res->start;
> > > > > +                   res->start = 0;
> > > > > +           }
> 
> Here, we correct for the fact that IORESOURCE_IO is not a normal resource, because Linux wants
> a logical I/O as start and end address, not the physical CPU address. We adjust to that and
> remember the offset.

But the offset (phys_base) doesn't actually matter to the PCI core or
the driver. Why save it?

> > > > >             offset = window->offset;
> > > > >             if (res->flags & IORESOURCE_BUS)
> > > >
> > > > Won't this break all existing host bridges?
> > >
> > > I am not sure. I believe not, due to what I've explained earlier, but you might be right.
> > >
> > > The adjustment happens before the resource is added to the host bridge windows and translates
> > > it from MMIO range into IO range.
> >
> > AFAICT, the resource_type of the resource you register above should be
> > IORESOURCE_MEM, so you are not actually matching it here.
> 
> No, all resources are added here. For IORESOURCE_IO we do an adjustment.

But there should never be an IORESOURCE_IO resource structure that is
not in IO space, i.e. within ioport_resource. Doing an "adjustment"
is not an operation defined on this structure. What I meant above is that
the pci range parser gets this right and gives you a resource that looks
like { .flags = IORESOURCE_MEM, .start = phys_base, .end = phys_base +
size - 1}, while the resource we want to register is { .flags = IORESOURCE_IO,
.start = log_base, .end = log_base + size -1}. In the of_pci_range struct for
the I/O space, the "pci_space" is IORESOURCE_IO (for the pci_addr), while the
"flags" are IORESOURCE_MEM, to go along with the cpu_addr.

	Arnd

^ permalink raw reply

* Re: [PATCH net-next 1/2] dts: Add bindings for multicast hash bins and perfect filter entries
From: Giuseppe CAVALLARO @ 2014-02-04  9:58 UTC (permalink / raw)
  To: Vince Bridgers, devicetree, netdev
  Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	dinguyen, rayagond
In-Reply-To: <1391199356-27226-2-git-send-email-vbridgers2013@gmail.com>

Hello Vince

On 1/31/2014 9:15 PM, Vince Bridgers wrote:
> This change adds bindings for the number of multicast hash bins and perfect
> filter entries supported by the Synopsys EMAC. The Synopsys EMAC core is
> configurable at device creation time, and can be configured for a different
> number of multicast hash bins and a different number of perfect filter entries.
> The device does not provide a way to query these parameters, therefore
> parameters are required. The Altera Cyclone V SOC has support for 256
> multicast hash bins and 128 perfect filter entries, and is different than
> what's currently provided in the stmmac driver.
>

thanks a lot for these patches, I have just added some comment for the
patch #2

peppe

> Signed-off-by: Vince Bridgers <vbridgers2013@gmail.com>
> ---
>   Documentation/devicetree/bindings/net/stmmac.txt |    6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
> index 9d92d42..dbf7498 100644
> --- a/Documentation/devicetree/bindings/net/stmmac.txt
> +++ b/Documentation/devicetree/bindings/net/stmmac.txt
> @@ -34,6 +34,10 @@ Optional properties:
>   	reset phandle is given
>   - max-frame-size:	Maximum Transfer Unit (IEEE defined MTU), rather
>   			than the maximum frame size.
> +- snps,multicast-filter-bins:	Number of multicast filter hash bins
> +				supported by this device instance
> +- snps,perfect-filter-entries:	Number of perfect filter entries supported
> +				by this device instance
>
>   Examples:
>
> @@ -46,4 +50,6 @@ Examples:
>   		mac-address = [000000000000]; /* Filled in by U-Boot */
>   		max-frame-size = <3800>;
>   		phy-mode = "gmii";
> +		snps,multicast-filter-bins = <256>;
> +		snps,perfect-filter-entries = <128>;
>   	};
>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox