Devicetree
 help / color / mirror / Atom feed
* [PATCH v8 10/12] mux: adg792a: add mux controller driver for ADG792A/G
From: Peter Rosin @ 2017-01-18 15:57 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, Andrew Morton, linux-i2c,
	devicetree, linux-iio, linux-doc
In-Reply-To: <1484755035-25927-1-git-send-email-peda@axentia.se>

Analog Devices ADG792A/G is a triple 4:1 mux.

Reviewed-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/mux/Kconfig       |  12 ++++
 drivers/mux/Makefile      |   1 +
 drivers/mux/mux-adg792a.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 180 insertions(+)
 create mode 100644 drivers/mux/mux-adg792a.c

diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
index 57e6f4e5fda9..d13fcf98958e 100644
--- a/drivers/mux/Kconfig
+++ b/drivers/mux/Kconfig
@@ -16,6 +16,18 @@ menuconfig MULTIPLEXER
 
 if MULTIPLEXER
 
+config MUX_ADG792A
+	tristate "Analog Devices ADG792A/ADG792G Multiplexers"
+	depends on I2C
+	help
+	  ADG792A and ADG792G Wide Bandwidth Triple 4:1 Multiplexers
+
+	  The driver supports both operating the three multiplexers in
+	  parallel and operating them independently.
+
+	  To compile the driver as a module, choose M here: the module will
+	  be called mux-adg792a.
+
 config MUX_GPIO
 	tristate "GPIO-controlled Multiplexer"
 	depends on OF && GPIOLIB
diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
index facc43da3648..fddbb073fc77 100644
--- a/drivers/mux/Makefile
+++ b/drivers/mux/Makefile
@@ -3,4 +3,5 @@
 #
 
 obj-$(CONFIG_MULTIPLEXER)      	+= mux-core.o
+obj-$(CONFIG_MUX_ADG792A)	+= mux-adg792a.o
 obj-$(CONFIG_MUX_GPIO)		+= mux-gpio.o
diff --git a/drivers/mux/mux-adg792a.c b/drivers/mux/mux-adg792a.c
new file mode 100644
index 000000000000..6a4c4c9f585b
--- /dev/null
+++ b/drivers/mux/mux-adg792a.c
@@ -0,0 +1,167 @@
+/*
+ * Multiplexer driver for Analog Devices ADG792A/G Triple 4:1 mux
+ *
+ * Copyright (C) 2017 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda@axentia.se>
+ *
+ * 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/err.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/mux.h>
+
+#define ADG792A_LDSW		BIT(0)
+#define ADG792A_RESET		BIT(1)
+#define ADG792A_DISABLE(mux)	(0x50 | (mux))
+#define ADG792A_DISABLE_ALL	(0x5f)
+#define ADG792A_MUX(mux, state)	(0xc0 | (((mux) + 1) << 2) | (state))
+#define ADG792A_MUX_ALL(state)	(0xc0 | (state))
+
+#define ADG792A_DISABLE_STATE	(4)
+
+static int adg792a_set(struct mux_control *mux, int state)
+{
+	struct i2c_client *i2c = to_i2c_client(mux->chip->dev.parent);
+	u8 cmd;
+
+	if (mux->chip->controllers == 1) {
+		/* parallel mux controller operation */
+		if (state == ADG792A_DISABLE_STATE)
+			cmd = ADG792A_DISABLE_ALL;
+		else
+			cmd = ADG792A_MUX_ALL(state);
+	} else {
+		unsigned int controller = mux_control_get_index(mux);
+
+		if (state == ADG792A_DISABLE_STATE)
+			cmd = ADG792A_DISABLE(controller);
+		else
+			cmd = ADG792A_MUX(controller, state);
+	}
+
+	return i2c_smbus_write_byte_data(i2c, cmd, ADG792A_LDSW);
+}
+
+static const struct mux_control_ops adg792a_ops = {
+	.set = adg792a_set,
+};
+
+static int adg792a_probe(struct i2c_client *i2c,
+			 const struct i2c_device_id *id)
+{
+	struct device *dev = &i2c->dev;
+	struct mux_chip *mux_chip;
+	bool parallel;
+	int count;
+	int ret;
+	int i;
+
+	parallel = of_property_read_bool(i2c->dev.of_node, "adi,parallel");
+
+	mux_chip = devm_mux_chip_alloc(dev, parallel ? 1 : 3, 0);
+	if (!mux_chip)
+		return -ENOMEM;
+
+	mux_chip->ops = &adg792a_ops;
+
+	ret = i2c_smbus_write_byte_data(i2c, ADG792A_DISABLE_ALL,
+					ADG792A_RESET | ADG792A_LDSW);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < mux_chip->controllers; ++i) {
+		struct mux_control *mux = &mux_chip->mux[i];
+
+		mux->states = 4;
+	}
+
+	count = of_property_count_u32_elems(dev->of_node, "adi,idle-state");
+	for (i = 0; i < count; i += 2) {
+		u32 index;
+		u32 idle_state;
+
+		ret = of_property_read_u32_index(dev->of_node,
+						 "adi,idle-state", i,
+						 &index);
+		if (ret < 0)
+			return ret;
+		if (index >= mux_chip->controllers) {
+			dev_err(dev, "invalid mux %u\n", index);
+			return -EINVAL;
+		}
+
+		ret = of_property_read_u32_index(dev->of_node,
+						 "adi,idle-state", i + 1,
+						 &idle_state);
+		if (ret < 0)
+			return ret;
+		if (idle_state >= ADG792A_DISABLE_STATE) {
+			dev_err(dev, "invalid idle-state %u for mux %u\n",
+				idle_state, index);
+			return -EINVAL;
+		}
+		mux_chip->mux[index].idle_state = idle_state;
+	}
+
+	count = of_property_count_u32_elems(dev->of_node,
+					    "adi,idle-high-impedance");
+	for (i = 0; i < count; ++i) {
+		u32 index;
+
+		ret = of_property_read_u32_index(dev->of_node,
+						 "adi,idle-high-impedance", i,
+						 &index);
+		if (ret < 0)
+			return ret;
+		if (index >= mux_chip->controllers) {
+			dev_err(dev, "invalid mux %u\n", index);
+			return -EINVAL;
+		}
+
+		mux_chip->mux[index].idle_state = ADG792A_DISABLE_STATE;
+	}
+
+	ret = devm_mux_chip_register(dev, mux_chip);
+	if (ret < 0)
+		return ret;
+
+	if (parallel)
+		dev_info(dev, "triple pole quadruple throw mux registered\n");
+	else
+		dev_info(dev, "3x single pole quadruple throw muxes registered\n");
+
+	return 0;
+}
+
+static const struct i2c_device_id adg792a_id[] = {
+	{ .name = "adg792a", },
+	{ .name = "adg792g", },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, adg792a_id);
+
+static const struct of_device_id adg792a_of_match[] = {
+	{ .compatible = "adi,adg792a", },
+	{ .compatible = "adi,adg792g", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, adg792a_of_match);
+
+static struct i2c_driver adg792a_driver = {
+	.driver		= {
+		.name		= "adg792a",
+		.of_match_table = of_match_ptr(adg792a_of_match),
+	},
+	.probe		= adg792a_probe,
+	.id_table	= adg792a_id,
+};
+module_i2c_driver(adg792a_driver);
+
+MODULE_DESCRIPTION("Analog Devices ADG792A/G Triple 4:1 mux driver");
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

^ permalink raw reply related

* [PATCH v8 11/12] dt-bindings: simplified bindings for single-user gpio mux
From: Peter Rosin @ 2017-01-18 15:57 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, Andrew Morton, linux-i2c,
	devicetree, linux-iio, linux-doc
In-Reply-To: <1484755035-25927-1-git-send-email-peda@axentia.se>

Allow bindings for a GPIO controlled mux to be specified in the
mux consumer node.

Acked-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../devicetree/bindings/mux/mux-controller.txt     | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/mux/mux-controller.txt b/Documentation/devicetree/bindings/mux/mux-controller.txt
index 42b2177e5ae1..4e89df8b2392 100644
--- a/Documentation/devicetree/bindings/mux/mux-controller.txt
+++ b/Documentation/devicetree/bindings/mux/mux-controller.txt
@@ -125,3 +125,29 @@ An example mux controller might look like this:
 		reg = <0x50>;
 		#mux-control-cells = <1>;
 	};
+
+
+Combinded controller and consumer of a GPIO mux
+-----------------------------------------------
+
+For the common case of a single consumer of a GPIO controlled mux, there is
+a simplified binding which will instantiate an implicit mux controller. Just
+specify a mux-gpios property with the same interpretation as in mux-gpio.txt.
+Note that other properties described in mux-gpio.txt are not available in
+this simplified form and that the mux controller is unnamed. If you need
+more than one mux controller, a shared mux controller or if you need a
+specific idle-state, use the more flexible binding with the mux controller
+in its own node.
+
+Example:
+
+	adc-mux {
+		compatible = "io-channel-mux";
+		io-channels = <&adc 0>;
+		io-channel-names = "parent";
+
+		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
+			    <&pioA 1 GPIO_ACTIVE_HIGH>;
+
+		channels = "sync-1", "in", "out", "sync-2";
+	};
-- 
2.1.4

^ permalink raw reply related

* [PATCH v8 12/12] mux: support simplified bindings for single-user gpio mux
From: Peter Rosin @ 2017-01-18 15:57 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, Andrew Morton, linux-i2c,
	devicetree, linux-iio, linux-doc
In-Reply-To: <1484755035-25927-1-git-send-email-peda@axentia.se>

Allow bindings for a GPIO controlled mux to be specified in the
mux consumer node.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/mux/Kconfig    |  5 +----
 drivers/mux/mux-core.c | 23 +++++++++++++++++++++--
 drivers/mux/mux-gpio.c | 28 +++++++++++++++++++++-------
 drivers/mux/mux-gpio.h | 13 +++++++++++++
 4 files changed, 56 insertions(+), 13 deletions(-)
 create mode 100644 drivers/mux/mux-gpio.h

diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
index d13fcf98958e..cd161c228537 100644
--- a/drivers/mux/Kconfig
+++ b/drivers/mux/Kconfig
@@ -29,7 +29,7 @@ config MUX_ADG792A
 	  be called mux-adg792a.
 
 config MUX_GPIO
-	tristate "GPIO-controlled Multiplexer"
+	bool "GPIO-controlled Multiplexer"
 	depends on OF && GPIOLIB
 	help
 	  GPIO-controlled Multiplexer controller.
@@ -39,7 +39,4 @@ config MUX_GPIO
 	  states. The GPIO pins can be connected (by the hardware) to several
 	  multiplexers, which in that case will be operated in parallel.
 
-	  To compile the driver as a module, choose M here: the module will
-	  be called mux-gpio.
-
 endif
diff --git a/drivers/mux/mux-core.c b/drivers/mux/mux-core.c
index 16a61253d164..0caafd6f5a77 100644
--- a/drivers/mux/mux-core.c
+++ b/drivers/mux/mux-core.c
@@ -21,6 +21,8 @@
 #include <linux/of_platform.h>
 #include <linux/slab.h>
 
+#include "mux-gpio.h"
+
 static struct class mux_class = {
 	.name = "mux",
 	.owner = THIS_MODULE,
@@ -314,9 +316,26 @@ struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
 	ret = of_parse_phandle_with_args(np,
 					 "mux-controls", "#mux-control-cells",
 					 index, &args);
+
+#ifdef CONFIG_MUX_GPIO
+	if (ret == -ENOENT && !mux_name) {
+		mux_chip = mux_gpio_alloc(dev);
+		if (!IS_ERR(mux_chip)) {
+			ret = devm_mux_chip_register(dev, mux_chip);
+			if (ret < 0)
+				return ERR_PTR(ret);
+			get_device(&mux_chip->dev);
+			return mux_chip->mux;
+		}
+
+		ret = PTR_ERR(mux_chip);
+	}
+#endif
+
 	if (ret) {
-		dev_err(dev, "%s: failed to get mux-control %s(%i)\n",
-			np->full_name, mux_name ?: "", index);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "%s: failed to get mux-control %s(%i)\n",
+				np->full_name, mux_name ?: "", index);
 		return ERR_PTR(ret);
 	}
 
diff --git a/drivers/mux/mux-gpio.c b/drivers/mux/mux-gpio.c
index 48ca4d6b4fc7..2ab8735e3415 100644
--- a/drivers/mux/mux-gpio.c
+++ b/drivers/mux/mux-gpio.c
@@ -18,6 +18,8 @@
 #include <linux/platform_device.h>
 #include <linux/property.h>
 
+#include "mux-gpio.h"
+
 struct mux_gpio {
 	struct gpio_descs *gpios;
 	int *val;
@@ -48,24 +50,21 @@ static const struct of_device_id mux_gpio_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, mux_gpio_dt_ids);
 
-static int mux_gpio_probe(struct platform_device *pdev)
+struct mux_chip *mux_gpio_alloc(struct device *dev)
 {
-	struct device *dev = &pdev->dev;
-	struct device_node *np = dev->of_node;
 	struct mux_chip *mux_chip;
 	struct mux_gpio *mux_gpio;
 	int pins;
-	u32 idle_state;
 	int ret;
 
 	pins = gpiod_count(dev, "mux");
 	if (pins < 0)
-		return pins;
+		return ERR_PTR(pins);
 
 	mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux_gpio) +
 				       pins * sizeof(*mux_gpio->val));
 	if (!mux_chip)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	mux_gpio = mux_chip_priv(mux_chip);
 	mux_gpio->val = (int *)(mux_gpio + 1);
@@ -76,11 +75,26 @@ static int mux_gpio_probe(struct platform_device *pdev)
 		ret = PTR_ERR(mux_gpio->gpios);
 		if (ret != -EPROBE_DEFER)
 			dev_err(dev, "failed to get gpios\n");
-		return ret;
+		return ERR_PTR(ret);
 	}
 	WARN_ON(pins != mux_gpio->gpios->ndescs);
 	mux_chip->mux->states = 1 << pins;
 
+	return mux_chip;
+}
+EXPORT_SYMBOL_GPL(mux_gpio_alloc);
+
+static int mux_gpio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mux_chip *mux_chip;
+	u32 idle_state;
+	int ret;
+
+	mux_chip = mux_gpio_alloc(dev);
+	if (IS_ERR(mux_chip))
+		return PTR_ERR(mux_chip);
+
 	ret = device_property_read_u32(dev, "idle-state", &idle_state);
 	if (ret >= 0) {
 		if (idle_state >= mux_chip->mux->states) {
diff --git a/drivers/mux/mux-gpio.h b/drivers/mux/mux-gpio.h
new file mode 100644
index 000000000000..fe3e8d0173aa
--- /dev/null
+++ b/drivers/mux/mux-gpio.h
@@ -0,0 +1,13 @@
+/*
+ * GPIO-controlled multiplexer driver interface
+ *
+ * Copyright (C) 2017 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda@axentia.se>
+ *
+ * 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.
+ */
+
+struct mux_chip *mux_gpio_alloc(struct device *dev);
-- 
2.1.4


^ permalink raw reply related

* Re: [PATCH v3] arm64: dts: exynos: Remove unneeded unit names in Exynos5433 nodes
From: Krzysztof Kozlowski @ 2017-01-18 16:27 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-kernel, Andi Shyti, Pankaj Dubey, Chanwoo Choi, devicetree,
	Kukjin Kim, Catalin Marinas, linux-samsung-soc, Rob Herring,
	Will Deacon, Mark Rutland, Krzysztof Kozlowski, linux-arm-kernel
In-Reply-To: <1484754613-2425-1-git-send-email-javier@osg.samsung.com>

On Wed, Jan 18, 2017 at 12:50:13PM -0300, Javier Martinez Canillas wrote:
> The "samsung,exynos5433-mipi-video-phy" and "samsung,exynos5250-dwusb3"
> DT bindings don't specify a reg property for these nodes, so having a
> unit name leads to the following DTC warnings:
> 
> Node /soc/video-phy@105c0710 has a unit name, but no reg property
> Node /soc/usb@15400000 has a unit name, but no reg property
> Node /soc/usb@15a00000 has a unit name, but no reg property
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> 
> ---
> 
> Changes in v3:
>   - Use proper device node names, suggested by Krzysztof.
> 
> Changes in v2:
>   - Fix subject line since I forgot the "exynos" prefix.
> 
>  arch/arm64/boot/dts/exynos/exynos5433.dtsi | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Thanks, applied.

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH v14 3/5] tee: add OP-TEE driver
From: Arnd Bergmann @ 2017-01-18 16:28 UTC (permalink / raw)
  To: Jens Wiklander
  Cc: valentin.manea, devicetree, javier, emmanuel.michel,
	scott.branden, Nishanth Menon, Greg Kroah-Hartman, broonie,
	Mark Rutland, Will Deacon, linux-kernel, Wei Xu,
	jean-michel.delorme, Jason Gunthorpe, Rob Herring, Al Viro,
	Olof Johansson, Andrew Morton, Andrew F . Davis, Michal Simek,
	linux-arm-kernel
In-Reply-To: <1484744296-30003-4-git-send-email-jens.wiklander@linaro.org>

On Wednesday, January 18, 2017 1:58:14 PM CET Jens Wiklander wrote:
> Adds a OP-TEE driver which also can be compiled as a loadable module.
> 
> * Targets ARM and ARM64
> * Supports using reserved memory from OP-TEE as shared memory
> * Probes OP-TEE version using SMCs
> * Accepts requests on privileged and unprivileged device
> * Uses OPTEE message protocol version 2 to communicate with secure world

I had not really followed the last versions, and I've looked through
it now for things that seemed odd to me, either because I don't understand
them or because they could be improved. I'll try to read it again after
I've seen clarifications on these points.

Generally speaking I haven't seen any show-stoppers so far.

> +struct optee_call_waiter {
> +	struct list_head list_node;
> +	struct completion c;
> +	bool completed;
> +};

It seems wrong to have both a 'struct completion' and 'bool completed' here,
as completion already contains such a flag and is designed to update that
atomically.

> +static void optee_cq_complete_one(struct optee_call_queue *cq)
> +{
> +	struct optee_call_waiter *w;
> +
> +	list_for_each_entry(w, &cq->waiters, list_node) {
> +		if (!w->completed) {
> +			complete(&w->c);
> +			w->completed = true;
> +			break;
> +		}
> +	}
> +}
> +
> +static void optee_cq_wait_final(struct optee_call_queue *cq,
> +				struct optee_call_waiter *w)
> +{
> +	mutex_lock(&cq->mutex);
> +
> +	/* Get out of the list */
> +	list_del(&w->list_node);
> +
> +	optee_cq_complete_one(cq);
> +	/*
> +	 * If we're completed we've got a completion that some other task
> +	 * could have used instead.
> +	 */
> +	if (w->completed)
> +		optee_cq_complete_one(cq);
> +
> +	mutex_unlock(&cq->mutex);
> +}

This deserves some more comments: the function name suggests that you are
waiting for a specific optee_call_waiter, but then it calls
optee_cq_complete_one(), which unconditionally completes the first
incomplete completion and it never waits.

> +static struct tee_shm_pool *
> +optee_config_shm_ioremap(struct device *dev, optee_invoke_fn *invoke_fn,
> +			 void __iomem **ioremaped_shm)
> +{
> +	union {
> +		struct arm_smccc_res smccc;
> +		struct optee_smc_get_shm_config_result result;
> +	} res;
> +	struct tee_shm_pool *pool;
> +	unsigned long vaddr;
> +	phys_addr_t paddr;
> +	size_t size;
> +	phys_addr_t begin;
> +	phys_addr_t end;
> +	void __iomem *va;
> +	struct tee_shm_pool_mem_info priv_info;
> +	struct tee_shm_pool_mem_info dmabuf_info;
> +
> +	invoke_fn(OPTEE_SMC_GET_SHM_CONFIG, 0, 0, 0, 0, 0, 0, 0, &res.smccc);
> +	if (res.result.status != OPTEE_SMC_RETURN_OK) {
> +		dev_info(dev, "shm service not available\n");
> +		return ERR_PTR(-ENOENT);
> +	}
> +
> +	if (res.result.settings != OPTEE_SMC_SHM_CACHED) {
> +		dev_err(dev, "only normal cached shared memory supported\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	begin = roundup(res.result.start, PAGE_SIZE);
> +	end = rounddown(res.result.start + res.result.size, PAGE_SIZE);
> +	paddr = begin;
> +	size = end - begin;
> +
> +	if (size < 2 * OPTEE_SHM_NUM_PRIV_PAGES * PAGE_SIZE) {
> +		dev_err(dev, "too small shared memory area\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	va = ioremap_cache(paddr, size);
> +	if (!va) {
> +		dev_err(dev, "shared memory ioremap failed\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +	vaddr = (unsigned long)va;

I think you should call memremap() instead of ioremap_cache() here and assume
that you are talking to actual RAM.

> +static int __init optee_driver_init(void)
> +{
> +	struct device_node *node;
> +
> +	/*
> +	 * Preferred path is /firmware/optee, but it's the matching that
> +	 * matters.
> +	 */
> +	for_each_matching_node(node, optee_match)
> +		of_platform_device_create(node, NULL, NULL);
> +
> +	return platform_driver_register(&optee_driver);
> +}
> +module_init(optee_driver_init);
> +
> +static void __exit optee_driver_exit(void)
> +{
> +	platform_driver_unregister(&optee_driver);
> +}
> +module_exit(optee_driver_exit);

What is the platform driver good for if the same module has to create the
platform devices itself?

I'd just skip it and do

	for_each_matching_node(node, optee_match)
		optee_probe(node);

I also suspect that module unloading is broken here if you don't clean
up the platform devices in the end, so you should already remove the
exit function to prevent unloading.

> +struct optee_msg_arg {
> +	u32 cmd;
> +	u32 func;
> +	u32 session;
> +	u32 cancel_id;
> +	u32 pad;
> +	u32 ret;
> +	u32 ret_origin;
> +	u32 num_params;
> +
> +	/*
> +	 * this struct is 8 byte aligned since the 'struct optee_msg_param'
> +	 * which follows requires 8 byte alignment.
> +	 *
> +	 * Commented out element used to visualize the layout dynamic part
> +	 * of the struct. This field is not available at all if
> +	 * num_params == 0.
> +	 *
> +	 * params is accessed through the macro OPTEE_MSG_GET_PARAMS
> +	 *
> +	 * struct optee_msg_param params[num_params];
> +	 */
> +} __aligned(8);
> +
> +/**
> + * OPTEE_MSG_GET_PARAMS - return pointer to struct optee_msg_param *
> + *
> + * @x: Pointer to a struct optee_msg_arg
> + *
> + * Returns a pointer to the params[] inside a struct optee_msg_arg.
> + */
> +#define OPTEE_MSG_GET_PARAMS(x) \
> +	(struct optee_msg_param *)(((struct optee_msg_arg *)(x)) + 1)

If you make the last member of optee_msg_arg

	struct optee_msg_param params[0];

then you can remove both the macro here and the alignment attribute.

> +/*****************************************************************************
> + * Part 2 - requests from normal world
> + *****************************************************************************/
> +
> +/*
> + * Return the following UID if using API specified in this file without
> + * further extensions:
> + * 384fb3e0-e7f8-11e3-af63-0002a5d5c51b.
> + * Represented in 4 32-bit words in OPTEE_MSG_UID_0, OPTEE_MSG_UID_1,
> + * OPTEE_MSG_UID_2, OPTEE_MSG_UID_3.
> + */
> +#define OPTEE_MSG_UID_0			0x384fb3e0
> +#define OPTEE_MSG_UID_1			0xe7f811e3
> +#define OPTEE_MSG_UID_2			0xaf630002
> +#define OPTEE_MSG_UID_3			0xa5d5c51b
> +#define OPTEE_MSG_FUNCID_CALLS_UID	0xFF01
> +
> +/*
> + * Returns 2.0 if using API specified in this file without further
> + * extensions. Represented in 2 32-bit words in OPTEE_MSG_REVISION_MAJOR
> + * and OPTEE_MSG_REVISION_MINOR
> + */
> +#define OPTEE_MSG_REVISION_MAJOR	2
> +#define OPTEE_MSG_REVISION_MINOR	0
> +#define OPTEE_MSG_FUNCID_CALLS_REVISION	0xFF03
> +
> +/*
> + * Get UUID of Trusted OS.
> + *
> + * Used by non-secure world to figure out which Trusted OS is installed.
> + * Note that returned UUID is the UUID of the Trusted OS, not of the API.
> + *
> + * Returns UUID in 4 32-bit words in the same way as
> + * OPTEE_MSG_FUNCID_CALLS_UID described above.
> + */
> +#define OPTEE_MSG_OS_OPTEE_UUID_0	0x486178e0
> +#define OPTEE_MSG_OS_OPTEE_UUID_1	0xe7f811e3
> +#define OPTEE_MSG_OS_OPTEE_UUID_2	0xbc5e0002
> +#define OPTEE_MSG_OS_OPTEE_UUID_3	0xa5d5c51b
> +#define OPTEE_MSG_FUNCID_GET_OS_UUID	0x0000
> +
> +/*
> + * Get revision of Trusted OS.
> + *
> + * Used by non-secure world to figure out which version of the Trusted OS
> + * is installed. Note that the returned revision is the revision of the
> + * Trusted OS, not of the API.
> + *
> + * Returns revision in 2 32-bit words in the same way as
> + * OPTEE_MSG_CALLS_REVISION described above.
> + */
> +#define OPTEE_MSG_OS_OPTEE_REVISION_MAJOR	1
> +#define OPTEE_MSG_OS_OPTEE_REVISION_MINOR	0
> +#define OPTEE_MSG_FUNCID_GET_OS_REVISION	0x0001

Just for my understanding, what is the significance of these numbers,
i.e. which code (user space, kernel driver, trusted OS) provides
the uuid and which one provides the version? The code comments almost
make sense to me, but I don't see why specific versions are listed
in this header.

What is the expected behavior when one side reports a version that
is unknown? Can one side claim to be backwards compatible with
a previous version, or does each new version need support on
all three sides?

> diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c
> new file mode 100644
> index 000000000000..0b9c1a2accd0
> --- /dev/null
> +++ b/drivers/tee/optee/rpc.c
> +static void handle_rpc_func_cmd_wq(struct optee *optee,
> +				   struct optee_msg_arg *arg)
> +{
> +	struct optee_msg_param *params;
> +
> +	if (arg->num_params != 1)
> +		goto bad;
> +
> +	params = OPTEE_MSG_GET_PARAMS(arg);
> +	if ((params->attr & OPTEE_MSG_ATTR_TYPE_MASK) !=
> +			OPTEE_MSG_ATTR_TYPE_VALUE_INPUT)
> +		goto bad;
> +
> +	switch (params->u.value.a) {
> +	case OPTEE_MSG_RPC_WAIT_QUEUE_SLEEP:
> +		wq_sleep(&optee->wait_queue, params->u.value.b);
> +		break;
> +	case OPTEE_MSG_RPC_WAIT_QUEUE_WAKEUP:
> +		wq_wakeup(&optee->wait_queue, params->u.value.b);
> +		break;
> +	default:
> +		goto bad;
> +	}
> +
> +	arg->ret = TEEC_SUCCESS;
> +	return;
> +bad:
> +	arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> +}
> +

I'm trying to understand what this is good for. What I can see is that
you have a user space process calling into the kernel asking the tee
to do some command, and then the tee can ask the kernel to wait for
something to happen, or notify it that something has happened.

If we wait here, the user process gets suspended until this has
actually happened.

Am I reading this correctly? If yes, what is the intended use case?
Is there some process that is meant to always wait here? What
if we ever need to wait for more than one thing at a time (think
select or poll?)

> +	params = OPTEE_MSG_GET_PARAMS(arg);
> +	if ((params->attr & OPTEE_MSG_ATTR_TYPE_MASK) !=
> +			OPTEE_MSG_ATTR_TYPE_VALUE_INPUT)
> +		goto bad;
> +
> +	msec_to_wait = params->u.value.a;
> +
> +	/* set task's state to interruptible sleep */
> +	set_current_state(TASK_INTERRUPTIBLE);
> +
> +	/* take a nap */
> +	schedule_timeout(msecs_to_jiffies(msec_to_wait));

This can be done simpler with msleep();

	Arnd

^ permalink raw reply

* [PATCH v5 0/2] iio: adc: Add Maxim MAX11100 driver
From: Jacopo Mondi @ 2017-01-18 16:30 UTC (permalink / raw)
  To: wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/,
	magnus.damm-Re5JQEeQqe8AvxtiuMwx3w, jic23-DgEjT+Ai2ygdnm+yROfE0A,
	knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
	pmeerw-jW+XmwGofnusTnJN9+BGXg, marek.vasut-Re5JQEeQqe8AvxtiuMwx3w,
	geert-Td1EMuHUCqxL1ZNQvxDV9g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA

Hello everyone,
  hopefully we have reached last iteration on this simple driver.

After having clarified the read_raw() measure unit question, I'm now sending
out v5 addressing Jonathan's comment on having spi destination aligned to
their own cache lines to support DMA-capable SPI controllers.

Also, I added indio_dev->name attribute, as suggested by Lars-Peter even if
the same value can be retrieved from of_node/name.


v1 -> v2:
    - incorporated pmeerw's review comments
    - retrieve vref from dts and use that to convert read_raw result
      to mV
    - add device tree bindings documentation

v2 -> v3:
    - add _SCALE bit of read_raw function and change _RAW bit accordingly
    - call regulator_get_voltage when accessing the _SCALE part of read_raw
      and not during probe
    - add back remove function as regulator has to be disabled when detaching
      the module. Do not use devm_ version of iio_register/unregister functions
      anymore but do unregister in the remove.
    - remove mutex as access to SPI bus is protected by SPI core. Thanks marex

v3 -> v4:
    - split device tree binding documentation and actual ADC driver
    - add "reg" to the list of required properties and use a better
      namimg for the adc device node in bindings documentation as suggested
      by Geert.

v4 -> v5:
    - make spi_read() destination buffer cacheline aligned
    - add indio_dev->name attribute.

Jacopo Mondi (2):
  iio: adc: Add Maxim MAX11100 driver
  dt-bindings: iio: document MAX11100 ADC

 .../devicetree/bindings/iio/adc/max11100.txt       |  19 ++
 drivers/iio/adc/Kconfig                            |   9 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/max11100.c                         | 192 +++++++++++++++++++++
 4 files changed, 221 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
 create mode 100644 drivers/iio/adc/max11100.c

-- 
2.7.4

^ permalink raw reply

* [PATCHv5 1/2] iio: adc: Add Maxim MAX11100 driver
From: Jacopo Mondi @ 2017-01-18 16:30 UTC (permalink / raw)
  To: wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/,
	magnus.damm-Re5JQEeQqe8AvxtiuMwx3w, jic23-DgEjT+Ai2ygdnm+yROfE0A,
	knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
	pmeerw-jW+XmwGofnusTnJN9+BGXg, marek.vasut-Re5JQEeQqe8AvxtiuMwx3w,
	geert-Td1EMuHUCqxL1ZNQvxDV9g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1484757053-7102-1-git-send-email-jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>

From: Jacopo Mondi <jacopo-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>

Add iio driver for Maxim MAX11100 single-channel ADC.

Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
Tested-by: Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/iio/adc/Kconfig    |   9 +++
 drivers/iio/adc/Makefile   |   1 +
 drivers/iio/adc/max11100.c | 192 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 202 insertions(+)
 create mode 100644 drivers/iio/adc/max11100.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 38bc319..c32bc7a 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -307,6 +307,15 @@ config MAX1027
 	  To compile this driver as a module, choose M here: the module will be
 	  called max1027.
 
+config MAX11100
+	tristate "Maxim max11100 ADC driver"
+	depends on SPI_MASTER
+	help
+	  Say yes here to build support for Maxim max11100 SPI ADC
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called max11100.
+
 config MAX1363
 	tristate "Maxim max1363 ADC driver"
 	depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index d36c4be..5684369 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
 obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
 obj-$(CONFIG_LTC2485) += ltc2485.o
 obj-$(CONFIG_MAX1027) += max1027.o
+obj-$(CONFIG_MAX11100) += max11100.o
 obj-$(CONFIG_MAX1363) += max1363.o
 obj-$(CONFIG_MCP320X) += mcp320x.o
 obj-$(CONFIG_MCP3422) += mcp3422.o
diff --git a/drivers/iio/adc/max11100.c b/drivers/iio/adc/max11100.c
new file mode 100644
index 0000000..b738ecf
--- /dev/null
+++ b/drivers/iio/adc/max11100.c
@@ -0,0 +1,192 @@
+/*
+ * iio/adc/max11100.c
+ * Maxim max11100 ADC Driver with IIO interface
+ *
+ * Copyright (C) 2016-17 Renesas Electronics Corporation
+ * Copyright (C) 2016-17 Jacopo Mondi
+ *
+ * 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/delay.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/driver.h>
+
+/*
+ * LSB is the ADC single digital step
+ * 1 LSB = (vref_mv / 2 ^ 16)
+ *
+ * LSB is used to calculate analog voltage value
+ * from the number of ADC steps count
+ *
+ * Ain = (count * LSB)
+ */
+#define MAX11100_LSB_DIV		(1 << 16)
+
+struct max11100_state {
+	const struct max11100_chip_desc *desc;
+	struct regulator *vref_reg;
+	struct spi_device *spi;
+
+	/*
+	 * DMA (thus cache coherency maintenance) requires the
+	 * transfer buffers to live in their own cache lines.
+	 */
+	u8 buffer[3] ____cacheline_aligned;
+};
+
+static struct iio_chan_spec max11100_channels[] = {
+	{ /* [0] */
+		.type = IIO_VOLTAGE,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+	},
+};
+
+static struct max11100_chip_desc {
+	unsigned int num_chan;
+	const struct iio_chan_spec *channels;
+} max11100_desc = {
+	.num_chan = ARRAY_SIZE(max11100_channels),
+	.channels = max11100_channels,
+};
+
+static int max11100_read_single(struct iio_dev *indio_dev, int *val)
+{
+	int ret;
+	struct max11100_state *state = iio_priv(indio_dev);
+
+	ret = spi_read(state->spi, state->buffer, sizeof(state->buffer));
+	if (ret) {
+		dev_err(&indio_dev->dev, "SPI transfer failed\n");
+		return ret;
+	}
+
+	/* the first 8 bits sent out from ADC must be 0s */
+	if (state->buffer[0]) {
+		dev_err(&indio_dev->dev, "Invalid value: buffer[0] != 0\n");
+		return -EINVAL;
+	}
+
+	*val = (state->buffer[1] << 8) | state->buffer[2];
+
+	return 0;
+}
+
+static int max11100_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long info)
+{
+	int ret, vref_uv;
+	struct max11100_state *state = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		ret = max11100_read_single(indio_dev, val);
+		if (ret)
+			return ret;
+
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		vref_uv = regulator_get_voltage(state->vref_reg);
+		if (vref_uv < 0)
+			/* dummy regulator "get_voltage" returns -EINVAL */
+			return -EINVAL;
+
+		*val =  vref_uv / 1000;
+		*val2 = MAX11100_LSB_DIV;
+		return IIO_VAL_FRACTIONAL;
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_info max11100_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = max11100_read_raw,
+};
+
+static int max11100_probe(struct spi_device *spi)
+{
+	int ret;
+	struct iio_dev *indio_dev;
+	struct max11100_state *state;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*state));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	spi_set_drvdata(spi, indio_dev);
+
+	state = iio_priv(indio_dev);
+	state->spi = spi;
+	state->desc = &max11100_desc;
+
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->dev.of_node = spi->dev.of_node;
+	indio_dev->name = "max11100";
+	indio_dev->info = &max11100_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = state->desc->channels;
+	indio_dev->num_channels = state->desc->num_chan;
+
+	state->vref_reg = devm_regulator_get(&spi->dev, "vref");
+	if (IS_ERR(state->vref_reg))
+		return PTR_ERR(state->vref_reg);
+
+	ret = regulator_enable(state->vref_reg);
+	if (ret)
+		return ret;
+
+	ret = iio_device_register(indio_dev);
+	if (ret)
+		goto disable_regulator;
+
+	return 0;
+
+disable_regulator:
+	regulator_disable(state->vref_reg);
+
+	return ret;
+}
+
+static int max11100_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct max11100_state *state = iio_priv(indio_dev);
+
+	regulator_disable(state->vref_reg);
+
+	iio_device_unregister(indio_dev);
+
+	return 0;
+}
+
+static const struct of_device_id max11100_ids[] = {
+	{.compatible = "maxim,max11100"},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, max11100_ids);
+
+static struct spi_driver max11100_driver = {
+	.driver = {
+		.name	= "max11100",
+		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(max11100_ids),
+	},
+	.probe		= max11100_probe,
+	.remove		= max11100_remove,
+};
+
+module_spi_driver(max11100_driver);
+
+MODULE_AUTHOR("Jacopo Mondi <jacopo-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>");
+MODULE_DESCRIPTION("Maxim max11100 ADC Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

^ permalink raw reply related

* [PATCHv5 2/2] dt-bindings: iio: document MAX11100 ADC
From: Jacopo Mondi @ 2017-01-18 16:30 UTC (permalink / raw)
  To: wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/,
	magnus.damm-Re5JQEeQqe8AvxtiuMwx3w, jic23-DgEjT+Ai2ygdnm+yROfE0A,
	knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
	pmeerw-jW+XmwGofnusTnJN9+BGXg, marek.vasut-Re5JQEeQqe8AvxtiuMwx3w,
	geert-Td1EMuHUCqxL1ZNQvxDV9g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1484757053-7102-1-git-send-email-jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>

Add device tree bindings documentation for Maxim MAX11100 single-channel
ADC

Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
Acked-by: Wolfram Sang <wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/@public.gmane.org>
---
 .../devicetree/bindings/iio/adc/max11100.txt          | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt

diff --git a/Documentation/devicetree/bindings/iio/adc/max11100.txt b/Documentation/devicetree/bindings/iio/adc/max11100.txt
new file mode 100644
index 0000000..ad0bc31
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/max11100.txt
@@ -0,0 +1,19 @@
+* Maxim max11100 Analog to Digital Converter (ADC)
+
+Required properties:
+  - compatible: Should be "maxim,max11100"
+  - reg: the adc unit address
+  - vref-supply: phandle to the regulator that provides reference voltage
+
+Optional properties:
+  - spi-max-frequency: SPI maximum frequency
+
+Example:
+
+max11100: adc@0 {
+        compatible = "maxim,max11100";
+        reg = <0>;
+        vref-supply = <&adc0_vref>;
+        spi-max-frequency = <240000>;
+};
+
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH 2/2] ARM: DTS: Fix register map for virt-capable GIC
From: Santosh Shilimkar @ 2017-01-18 16:39 UTC (permalink / raw)
  To: Marc Zyngier, linux-kernel, devicetree, linux-arm-kernel
  Cc: Mark Rutland, Heiko Stuebner, Tony Lindgren, arm, Magnus Damm,
	Russell King, Krzysztof Kozlowski, Javier Martinez Canillas,
	Chen-Yu Tsai, Kukjin Kim, Tsahee Zidenberg, Jason Cooper,
	Simon Horman, Santosh Shilimkar, Matthias Brugger,
	Thomas Gleixner, Sascha Hauer, Antoine Tenart, Rob Herring,
	Benoît Cousson, Fabio Estevam, Maxime Ripard, Shawn Guo
In-Reply-To: <1484736811-24002-3-git-send-email-marc.zyngier@arm.com>

On 1/18/2017 2:53 AM, Marc Zyngier wrote:
> Since everybody copied my own mistake from the DT binding example,
> let's address all the offenders in one swift go.
>
> Most of them got the CPU interface size wrong (4kB, while it should
> be 8kB), except for both keystone platforms which got the control
> interface wrong (4kB instead of 8kB).
>
> In the couple of cases were I knew for sure what implementation
> was used, I've added the "arm,gic-400" compatible string. I'm 99%
> sure that this is what everyong is using, but short of having the
> TRM for all the other SoCs, I've let them alone.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---

>  arch/arm/boot/dts/imx6ul.dtsi        | 4 ++--
>  arch/arm/boot/dts/keystone-k2g.dtsi  | 2 +-
>  arch/arm/boot/dts/keystone.dtsi      | 2 +-

For keystone bits,
Acked-by: Santosh Shilimkar <ssantosh@kernel.org>

^ permalink raw reply

* [PATCH v5] ARM64: dts: meson-gx: Add firmware reserved memory zones
From: Neil Armstrong @ 2017-01-18 16:50 UTC (permalink / raw)
  To: xypron.glpk-Mmb7MZpHnFY, khilman-rdvid1DuHRBWk0Htik3J/w,
	carlo-KA+7E9HrN00dnm+yROfE0A, afaerber-l3A5Bk7waGM
  Cc: Neil Armstrong, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

The Amlogic Meson GXBB/GXL/GXM secure monitor uses part of the memory space,
this patch adds these reserved zones.

Without such reserved memory zones, running the following stress command :
$ stress-ng --vm 16 --vm-bytes 128M --timeout 10s
multiple times:

Could lead to the following kernel crashes :
[   46.937975] Bad mode in Error handler detected on CPU1, code 0xbf000000 -- SError
...
[   47.058536] Internal error: Attempting to execute userspace memory: 8600000f [#3] PREEMPT SMP
...
Instead of the OOM killer.

Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Changes since v4 at [5]:
- Move start of ddr memory to reserved-memory node
- Drop memory node move
- Fix typo in sizes

Changes since resent v2 at [4]:
- Fix invalid comment of useable memory attributes

Changes since original v2 at [3]:
- Typo in commit 2GiB -> 1GiB, 4GiB -> 2GiB

Changes since v2 at [2]:
- Moved all memory node out of dtsi
- Added comment about useable memory
- Fixed comment about secmon reserved zone

Changes since v1 at [1] :
- Renamed reg into linux,usable-memory to ovveride u-boot memory
- only kept secmon memory zone

[1] http://lkml.kernel.org/r/20161212101801.28491-1-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org
[2] http://lkml.kernel.org/r/1483105232-6242-1-git-send-email-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org
[3] http://lkml.kernel.org/r/1484128128-22454-1-git-send-email-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org
[4] http://lkml.kernel.org/r/1484128540-22662-1-git-send-email-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org
[5] http://lkml.kernel.org/r/1484129414-23325-1-git-send-email-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org

diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index eada0b5..63d52b7 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -55,6 +55,24 @@
 	#address-cells = <2>;
 	#size-cells = <2>;
 
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		/* 16 MiB reserved for Hardware ROM Firmware */
+		hwrom: hwrom {
+			reg = <0x0 0x0 0x0 0x1000000>;
+			no-map;
+		};
+
+		/* 2 MiB reserved for ARM Trusted Firmware (BL31) */
+		secmon: secmon {
+			reg = <0x0 0x10000000 0x0 0x200000>;
+			no-map;
+		};
+	};
+
 	cpus {
 		#address-cells = <0x2>;
 		#size-cells = <0x0>;
-- 
1.9.1

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

* Re: [PATCH 1/5] ARM: OMAP2+: omap_hwmod: Add support for earlycon
From: Tony Lindgren @ 2017-01-18 17:00 UTC (permalink / raw)
  To: Lokesh Vutla
  Cc: Linux OMAP Mailing List, Device Tree Mailing List, Rob Herring,
	Tero Kristo, Sekhar Nori, Vignesh R, Nishanth Menon
In-Reply-To: <88830967-6e83-33fc-8a1c-d776fd694e2f-l0cyMroinI0@public.gmane.org>

* Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org> [170117 19:50]:
> 
> 
> On Wednesday 18 January 2017 04:53 AM, Tony Lindgren wrote:
> > * Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org> [170116 20:06]:
> >> Hwmod core tries to reset and idles each IP that is registered with hwmod.
> >> In case of earlycon, that specific uart IP cannot be reset or keep it in
> >> idle state else earlycon hangs once hwmod resets that uart IP. So add support
> >> to not reset uart that is being used as earlycon only if CONFIG_SERIAL_EARLYCON
> >> is enabled.
> > 
> > Nice :)
> > 
> > I guess this has no dependency to SERIAL_OMAP vs 8250_OMAP selection?
> 
> Unfortunately SERIAL_OMAP does not support earlycon yet. As I mentioned
> in my cover letter, I verified this series only with 8250_OMAP.

OK. So just to understand why it would not work with omap-serial, do we
need something implemented in drivers/tty/serial/earlycon.c for non 8250
drivers to make it work? Or are there other dependencies?

Regards,

Tony
--
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 v3 0/3] mmc: sh_mobile_sdhi: fix missing r7s72100 clocks
From: Chris Brandt @ 2017-01-18 17:24 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Mark Rutland, Simon Horman,
	Wolfram Sang, Geert Uytterhoeven
  Cc: devicetree, linux-mmc, linux-renesas-soc, Chris Brandt

At first this started out as a simple typo fix, until I realized
that the SDHI in the RZ/A1 has 2 clocks per channel and both need
to be turned on/off.

This patch series adds the ability to specify 2 clocks instead of
just 1, and does so for the RZ/A1 r7s72100.

This patch has been tested on an RZ/A1 RSK board. Card detect works
fine as well as bind/unbind.


Chris Brandt (3):
  mmc: sh_mobile_sdhi: add support for 2 clocks
  mmc: sh_mobile_sdhi: explain clock bindings
  ARM: dts: r7s72100: update sdhi clock bindings

 Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 21 +++++++++++++++++++++
 arch/arm/boot/dts/r7s72100.dtsi                    | 17 ++++++++++++-----
 drivers/mmc/host/sh_mobile_sdhi.c                  | 13 +++++++++++++
 include/dt-bindings/clock/r7s72100-clock.h         |  6 ++++--
 4 files changed, 50 insertions(+), 7 deletions(-)

-- 
2.10.1



^ permalink raw reply

* [PATCH v3 1/3] mmc: sh_mobile_sdhi: add support for 2 clocks
From: Chris Brandt @ 2017-01-18 17:25 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Mark Rutland, Simon Horman,
	Wolfram Sang, Geert Uytterhoeven
  Cc: devicetree, linux-mmc, linux-renesas-soc, Chris Brandt
In-Reply-To: <20170118172502.13876-1-chris.brandt@renesas.com>

Some controllers have 2 clock sources instead of 1, so they both need
to be turned on/off.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v2:
* changed clk2 to clk_cd
* disable clk if clk_cd enable fails
* changed clock name from "carddetect" to "cd"
---
 drivers/mmc/host/sh_mobile_sdhi.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 59db14b..a3f995e 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -143,6 +143,7 @@ MODULE_DEVICE_TABLE(of, sh_mobile_sdhi_of_match);
 
 struct sh_mobile_sdhi {
 	struct clk *clk;
+	struct clk *clk_cd;
 	struct tmio_mmc_data mmc_data;
 	struct tmio_mmc_dma dma_priv;
 	struct pinctrl *pinctrl;
@@ -190,6 +191,12 @@ static int sh_mobile_sdhi_clk_enable(struct tmio_mmc_host *host)
 	if (ret < 0)
 		return ret;
 
+	ret = clk_prepare_enable(priv->clk_cd);
+	if (ret < 0) {
+		clk_disable_unprepare(priv->clk);
+		return ret;
+	}
+
 	/*
 	 * The clock driver may not know what maximum frequency
 	 * actually works, so it should be set with the max-frequency
@@ -255,6 +262,8 @@ static void sh_mobile_sdhi_clk_disable(struct tmio_mmc_host *host)
 	struct sh_mobile_sdhi *priv = host_to_priv(host);
 
 	clk_disable_unprepare(priv->clk);
+	if (priv->clk_cd)
+		clk_disable_unprepare(priv->clk_cd);
 }
 
 static int sh_mobile_sdhi_card_busy(struct mmc_host *mmc)
@@ -572,6 +581,10 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 		goto eprobe;
 	}
 
+	priv->clk_cd = devm_clk_get(&pdev->dev, "cd");
+	if (IS_ERR(priv->clk_cd))
+		priv->clk_cd = NULL;
+
 	priv->pinctrl = devm_pinctrl_get(&pdev->dev);
 	if (!IS_ERR(priv->pinctrl)) {
 		priv->pins_default = pinctrl_lookup_state(priv->pinctrl,
-- 
2.10.1



^ permalink raw reply related

* [PATCH v3 2/3] mmc: sh_mobile_sdhi: explain clock bindings
From: Chris Brandt @ 2017-01-18 17:25 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Mark Rutland, Simon Horman,
	Wolfram Sang, Geert Uytterhoeven
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, Chris Brandt
In-Reply-To: <20170118172502.13876-1-chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

In the case of a single clock source, you don't need names. However,
if the controller has 2 clock sources, you need to name them correctly
so the driver can find the 2nd one.

Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
v2:
* fix spelling and change wording
* changed clock name from "carddetect" to "cd"
---
 Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
index a1650ed..90370cd 100644
--- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
@@ -25,8 +25,29 @@ Required properties:
 		"renesas,sdhi-r8a7795" - SDHI IP on R8A7795 SoC
 		"renesas,sdhi-r8a7796" - SDHI IP on R8A7796 SoC
 
+- clocks: Most controllers only have 1 clock source per channel. However, some
+	  have a second clock dedicated to card detection. If 2 clocks are
+	  specified, you must name them as "core" and "cd". If the controller
+	  only has 1 clock, naming is not required.
+
 Optional properties:
 - toshiba,mmc-wrprotect-disable: write-protect detection is unavailable
 - pinctrl-names: should be "default", "state_uhs"
 - pinctrl-0: should contain default/high speed pin ctrl
 - pinctrl-1: should contain uhs mode pin ctrl
+
+Example showing 2 clocks:
+	sdhi0: sd@e804e000 {
+		compatible = "renesas,sdhi-r7s72100";
+		reg = <0xe804e000 0x100>;
+		interrupts = <GIC_SPI 270 IRQ_TYPE_LEVEL_HIGH
+			      GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH
+			      GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
+
+		clocks = <&mstp12_clks R7S72100_CLK_SDHI00>,
+			 <&mstp12_clks R7S72100_CLK_SDHI01>;
+		clock-names = "core", "cd";
+		cap-sd-highspeed;
+		cap-sdio-irq;
+		status = "disabled";
+	};
-- 
2.10.1


--
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 v3 3/3] ARM: dts: r7s72100: update sdhi clock bindings
From: Chris Brandt @ 2017-01-18 17:25 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Mark Rutland, Simon Horman,
	Wolfram Sang, Geert Uytterhoeven
  Cc: devicetree, linux-mmc, linux-renesas-soc, Chris Brandt
In-Reply-To: <20170118172502.13876-1-chris.brandt@renesas.com>

The SDHI controller in the RZ/A1 has 2 clock sources per channel and both
need to be enabled/disabled for proper operation. This fixes the fact that
the define for R7S72100_CLK_SDHI1 was not correct to begin with (typo), and
that all 4 clock sources need to be defined an used.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
* add missing clock sources instead of just fixing typo
* changed clock name from "carddetect" to "cd"
---
 arch/arm/boot/dts/r7s72100.dtsi            | 17 ++++++++++++-----
 include/dt-bindings/clock/r7s72100-clock.h |  6 ++++--
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index 3dd427d..9d0b8d0 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -153,9 +153,12 @@
 			#clock-cells = <1>;
 			compatible = "renesas,r7s72100-mstp-clocks", "renesas,cpg-mstp-clocks";
 			reg = <0xfcfe0444 4>;
-			clocks = <&p1_clk>, <&p1_clk>;
-			clock-indices = <R7S72100_CLK_SDHI1 R7S72100_CLK_SDHI0>;
-			clock-output-names = "sdhi1", "sdhi0";
+			clocks = <&p1_clk>, <&p1_clk>, <&p1_clk>, <&p1_clk>;
+			clock-indices = <
+				R7S72100_CLK_SDHI00 R7S72100_CLK_SDHI01
+				R7S72100_CLK_SDHI10 R7S72100_CLK_SDHI11
+			>;
+			clock-output-names = "sdhi00", "sdhi01", "sdhi10", "sdhi11";
 		};
 	};
 
@@ -478,7 +481,9 @@
 			      GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH
 			      GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
 
-		clocks = <&mstp12_clks R7S72100_CLK_SDHI0>;
+		clocks = <&mstp12_clks R7S72100_CLK_SDHI00>,
+			 <&mstp12_clks R7S72100_CLK_SDHI01>;
+		clock-names = "core", "cd";
 		cap-sd-highspeed;
 		cap-sdio-irq;
 		status = "disabled";
@@ -491,7 +496,9 @@
 			      GIC_SPI 274 IRQ_TYPE_LEVEL_HIGH
 			      GIC_SPI 275 IRQ_TYPE_LEVEL_HIGH>;
 
-		clocks = <&mstp12_clks R7S72100_CLK_SDHI1>;
+		clocks = <&mstp12_clks R7S72100_CLK_SDHI10>,
+			 <&mstp12_clks R7S72100_CLK_SDHI11>;
+		clock-names = "core", "cd";
 		cap-sd-highspeed;
 		cap-sdio-irq;
 		status = "disabled";
diff --git a/include/dt-bindings/clock/r7s72100-clock.h b/include/dt-bindings/clock/r7s72100-clock.h
index 29e01ed..f2d8428 100644
--- a/include/dt-bindings/clock/r7s72100-clock.h
+++ b/include/dt-bindings/clock/r7s72100-clock.h
@@ -45,7 +45,9 @@
 #define R7S72100_CLK_SPI4	3
 
 /* MSTP12 */
-#define R7S72100_CLK_SDHI0	3
-#define R7S72100_CLK_SDHI1	2
+#define R7S72100_CLK_SDHI00	3
+#define R7S72100_CLK_SDHI01	2
+#define R7S72100_CLK_SDHI10	1
+#define R7S72100_CLK_SDHI11	0
 
 #endif /* __DT_BINDINGS_CLOCK_R7S72100_H__ */
-- 
2.10.1

^ permalink raw reply related

* Re: [PATCH v3 06/13] ARM: davinci: da850: model the SATA refclk
From: David Lechner @ 2017-01-18 17:26 UTC (permalink / raw)
  To: Bartosz Golaszewski, Kevin Hilman, Sekhar Nori, Patrick Titiano,
	Michael Turquette, Tejun Heo, Rob Herring, Mark Rutland,
	Russell King
  Cc: linux-ide, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <1484745601-4769-7-git-send-email-bgolaszewski@baylibre.com>

On 01/18/2017 07:19 AM, Bartosz Golaszewski wrote:
> Register a dummy clock modelling the external SATA oscillator for

modeling

> da850 (both DT and board file mode).
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  arch/arm/mach-davinci/da8xx-dt.c           |  8 ++++++++
>  arch/arm/mach-davinci/devices-da8xx.c      | 29 +++++++++++++++++++++++++++++
>  arch/arm/mach-davinci/include/mach/da8xx.h |  1 +
>  3 files changed, 38 insertions(+)
>
> diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
> index b83e5d1..0f981b5 100644
> --- a/arch/arm/mach-davinci/da8xx-dt.c
> +++ b/arch/arm/mach-davinci/da8xx-dt.c
> @@ -50,6 +50,9 @@ static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
>
>  static void __init da850_init_machine(void)
>  {
> +        /* All existing boards use 100MHz SATA refclkpn */
> +        static const unsigned long sata_refclkpn = 100 * 1000 * 1000;
> +
>  	int ret;
>
>  	ret = da8xx_register_usb20_phy_clk(false);
> @@ -61,6 +64,11 @@ static void __init da850_init_machine(void)
>  		pr_warn("%s: registering USB 1.1 PHY clock failed: %d",
>  			__func__, ret);
>
> +        ret = da850_register_sata_refclk(sata_refclkpn);
> +        if (ret)
> +		pr_warn("%s: registering SATA REFCLK failed: %d",
> +			__func__, ret);
> +
>  	of_platform_default_populate(NULL, da850_auxdata_lookup, NULL);
>  	davinci_pm_init();
>  }
> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> index c2457b3..cfceb32 100644
> --- a/arch/arm/mach-davinci/devices-da8xx.c
> +++ b/arch/arm/mach-davinci/devices-da8xx.c
> @@ -24,6 +24,7 @@
>  #include <mach/common.h>
>  #include <mach/time.h>
>  #include <mach/da8xx.h>
> +#include <mach/clock.h>
>  #include "cpuidle.h"
>  #include "sram.h"
>
> @@ -1023,6 +1024,28 @@ int __init da8xx_register_spi_bus(int instance, unsigned num_chipselect)
>  }
>
>  #ifdef CONFIG_ARCH_DAVINCI_DA850
> +static struct clk sata_refclk = {
> +	.name		= "sata_refclk",
> +	.set_rate	= davinci_simple_set_rate,
> +};
> +
> +static struct clk_lookup sata_refclk_lookup =
> +		CLK("ahci_da850", "refclk", &sata_refclk);
> +
> +int __init da850_register_sata_refclk(int rate)
> +{
> +	int ret;
> +
> +	sata_refclk.rate = rate;
> +	ret = clk_register(&sata_refclk);
> +	if (ret)
> +		return ret;
> +
> +	clkdev_add(&sata_refclk_lookup);
> +
> +	return 0;
> +}
> +
>  static struct resource da850_sata_resources[] = {
>  	{
>  		.start	= DA850_SATA_BASE,
> @@ -1055,9 +1078,15 @@ static struct platform_device da850_sata_device = {
>
>  int __init da850_register_sata(unsigned long refclkpn)
>  {
> +	int ret;
> +
>  	/* please see comment in drivers/ata/ahci_da850.c */
>  	BUG_ON(refclkpn != 100 * 1000 * 1000);

This BUG_ON() should be removed since the sata driver can now handle 
other clock frequencies.

>
> +	ret = da850_register_sata_refclk(refclkpn);
> +	if (ret)
> +		return ret;
> +
>  	return platform_device_register(&da850_sata_device);
>  }
>  #endif
> diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
> index 85ff218..7e46422 100644
> --- a/arch/arm/mach-davinci/include/mach/da8xx.h
> +++ b/arch/arm/mach-davinci/include/mach/da8xx.h
> @@ -95,6 +95,7 @@ int da8xx_register_usb11(struct da8xx_ohci_root_hub *pdata);
>  int da8xx_register_usb_refclkin(int rate);
>  int da8xx_register_usb20_phy_clk(bool use_usb_refclkin);
>  int da8xx_register_usb11_phy_clk(bool use_usb_refclkin);
> +int da850_register_sata_refclk(int rate);
>  int da8xx_register_emac(void);
>  int da8xx_register_uio_pruss(void);
>  int da8xx_register_lcdc(struct da8xx_lcdc_platform_data *pdata);
>


^ permalink raw reply

* Re: [PATCH 1/2] ARM: dts: dra7-evm: increase QSPI SPL partition size
From: Tony Lindgren @ 2017-01-18 17:39 UTC (permalink / raw)
  To: Sekhar Nori
  Cc: devicetree@vger.kernel.org, linux-omap@vger.kernel.org,
	bcousson@baylibre.com, linux-arm-kernel@lists.infradead.org,
	B, Ravi
In-Reply-To: <b586537d-49b6-50a6-ae60-11faf25b26f9@ti.com>

* Sekhar Nori <nsekhar@ti.com> [170118 03:59]:
> Hi Tony,
> 
> On Wednesday 18 January 2017 04:57 AM, Tony Lindgren wrote:
> > * B, Ravi <ravibabu@ti.com> [170117 00:15]:
> >> Hi Tony
> >>
> >>> * Ravi Babu <ravibabu@ti.com> [170113 04:41]:
> >>>> The SPL size for DRA74x platform has increased and is now more than 
> >>>> 64KB. Increase QSPI SPL partition size to 256KB for DRA74x EVM.
> >>>>
> >>>> QSPI partition numbering changes because of this.
> >>
> >>> And this will break the existing partitions potentially..
> >>> See what was discussed on the list few days ago in thread "[PATCH 1/6] ARM: dts: am335x-phycore-som: Update NAND partition table".
> >>
> >>> It's best to have these left empty or as they originally were and let u-boot configure the partitions.
> >>
> >> Agree with you. For dra7xx platform the SPL size has been increased to 256KB and hence the existing QSPI SPL partition in kernel (64K size) will break when latest mainline u-boot is used. 
> >> Here only SPL partition has been changed and other partition & size is NOT changed and kept intact. I feel it will not break the existing partitions for dra7xx platform.
> > 
> > What about the renumbering of partitions in your patch?
> 
> Thats true, partitions will get renumbered. But mtd numbering can change
> depending on probe order of devices anyway. So usespace which uses
> hardcoded mtd partition numbers is pretty fragile already, I guess.
> 
> > 
> > Probably just best to make the partition information empty in the
> > kernel as discussed.
> 
> Given that existing dtbs already have the partition information, wont
> this be treated as a regression for someone upgrading to new kernel?

Well these "flag day" type changes are not acceptable because they are
impossible to coordinate. You can't assume people update their bootloader
on regular basis, or update both bootloader and kernel to some specific
versions.

> Going forward, is the preference that new boards shall not have
> partition information in DT?

Yes or else it will have to stay as it was originally set because
of these issues.

Regards,

Tony

^ permalink raw reply

* Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips
From: Bjorn Andersson @ 2017-01-18 18:03 UTC (permalink / raw)
  To: Vivek Gautam
  Cc: Kishon Vijay Abraham I, robh+dt, linux-kernel, devicetree,
	mark.rutland, sboyd, srinivas.kandagatla, linux-arm-msm
In-Reply-To: <73a2f6ce-69d6-8d15-b28d-891bdf16672c@codeaurora.org>

On Wed 18 Jan 01:13 PST 2017, Vivek Gautam wrote:
> On 01/16/2017 02:15 PM, Kishon Vijay Abraham I wrote:
> > Hi,
> > 
> > On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
[..]
> > > +static const struct qusb2_phy_init_tbl msm8996_init_tbl[] = {
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE1, 0xf8),
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE2, 0xb3),
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE3, 0x83),
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE4, 0xc0),
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_TUNE, 0x30),
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL1, 0x79),
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL2, 0x21),
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TEST2, 0x14),
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_AUTOPGM_CTL1, 0x9f),
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_PWR_CTRL, 0x00),
> > > +};
> > I wish all this data comes from device tree and one API in phy-core can do all
> > these settings. Your other driver qcom-qmp also seems to have a bunch of
> > similar settings.
> > 
> > The problem is every vendor driver adds a bunch of code to perform the same
> > thing again and again when all of these settings can be done by a single phy API.
> 
> Yes, i understand this. You have commented similar thing in the patch from
> Jaehoon -
> [PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy
> 
> I would like to understand the requirements here.
> Would you like me to get all this information from the device tree -
> an array of register offset and value pair, which we can then program
> by calling a phy_ops (may be calibrate) ? Something of this sort:
> 
> phy-calibrate-data = <val1, register_offset1>,
>                                   <val2, register_offset2>,
>                                   <val3, register_offset3>,
>                                   ....
> 
> I am sure having such information in the driver (like i have in my patch)
> makes the driver look more clumsy.
> But, all this data can be pretty huge - a set of some 100+ register-value
> pairs
> for QMP phy, for example. So, will it be okay to get this from device tree ?
> We also note here that such information changes from one IP version to
> another.
> I remember Rob having some concerns about it.
> 

The devicetree is supposed to describe which hardware components a
certain device has, most of the time this carries a set of properties to
describe how this piece is connected and configured.

A dump of magic register values does not describe how the QMP is
connected to anything and is, as far as this patch shows, static for
this particular hardware block.

Further more moving this blob to devicetree will not allow us to treat
the various QMP configurations as one HW block, as there are other
differences as well.

Like many other drivers it's possible to create a generic version that
has every bit of logic driven by configuration from devicetree, but like
most of those cases this is not the way we split things.



And this has the side effect of keeping the dts files human readable,
human understandable and human maintainable.

Regards,
Bjorn

^ permalink raw reply

* Re: [PATCH] clk: meson-gxbb: Export HDMI clocks
From: Kevin Hilman @ 2017-01-18 18:12 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: devicetree, Neil Armstrong, mturquette, linux-kernel, carlo,
	linux-amlogic, linux-clk, linux-arm-kernel
In-Reply-To: <20170117183441.GU17126@codeaurora.org>

Stephen Boyd <sboyd@codeaurora.org> writes:

> On 01/17, Neil Armstrong wrote:
>> Export HDMI clock from internal to dt-bindings.
>> 
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>
> Acked-by: Stephen Boyd <sboyd@codeaurora.org>
>
> I think we decided this would go with the dts changes if more
> needed to be exposed.

Ah, that's right.  I'll queue it up in the amlogic dt branch with your
ack.

Thanks,

Kevin

^ permalink raw reply

* Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy
From: Bjorn Andersson @ 2017-01-18 18:22 UTC (permalink / raw)
  To: Vivek Gautam
  Cc: Kishon Vijay Abraham I, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
	sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
	srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <50612693-5345-55da-8207-8c5e721fb68a-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

On Tue 17 Jan 22:54 PST 2017, Vivek Gautam wrote:
> On 01/16/2017 02:19 PM, Kishon Vijay Abraham I wrote:
> > On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
[..]
> > > +		reset-names = "phy", "common", "cfg",
> > > +				"lane0", "lane1", "lane2";
> > Each lane has a separate clock, separate reset.. why not create sub-nodes for
> > each lane?
> 
> Yes, each lane has separate pipe clock and resets.
> I can have a binding such as written below.

+1

> Does it makes sense to pull in the tx, rx and pcs offsets as well
> to the child node, and iomap the entire address space of the phy ?
> 

Note that you don't have to follow the same structure in your device
driver as you describe your hardware in devicetree.

I would suggest that you replace the lane-offset and various lane
specific resources with subnodes, but keep the driver "as is".

Regards,
Bjorn
--
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 v3 09/13] sata: ahci: export ahci_do_hardreset() locally
From: Tejun Heo @ 2017-01-18 18:28 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Mark Rutland, devicetree, David Lechner, Kevin Hilman,
	Michael Turquette, Sekhar Nori, Russell King, linux-kernel,
	linux-ide, Rob Herring, Patrick Titiano, linux-arm-kernel
In-Reply-To: <1484745601-4769-10-git-send-email-bgolaszewski@baylibre.com>

Hello, Bartosz.

On Wed, Jan 18, 2017 at 02:19:57PM +0100, Bartosz Golaszewski wrote:
> We need a way to retrieve the information about the online state of
> the link in the ahci-da850 driver.
> 
> Create a new function: ahci_do_hardreset() which is called from
> ahci_hardreset() for backwards compatibility, but has an additional
> argument: 'online' - which can be used to check if the link is online
> after this function returns.

Please just add @online to ahci_hardreset() and update the callers.
Other than that, the sata changes look good to me.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH v9 2/5] i2c: Add STM32F4 I2C driver
From: Uwe Kleine-König @ 2017-01-18 18:42 UTC (permalink / raw)
  To: M'boumba Cedric Madianga
  Cc: devicetree, Alexandre Torgue, Wolfram Sang, linux-kernel,
	Linus Walleij, Patrice Chotard, Russell King, Rob Herring,
	linux-i2c, Maxime Coquelin, linux-arm-kernel
In-Reply-To: <CAOAejn0ea2j_oUijOdHgiL3JU4TM0HZoL+-ggzjpPL2eA7tDCw@mail.gmail.com>

Hello Cedric,

On Wed, Jan 18, 2017 at 04:21:17PM +0100, M'boumba Cedric Madianga wrote:
> >> +      * In standard mode, the maximum allowed SCL rise time is 1000 ns.
> >> +      * If, in the I2C_CR2 register, the value of FREQ[5:0] bits is equal to
> >> +      * 0x08 so period = 125 ns therefore the TRISE[5:0] bits must be
> >> +      * programmed with 09h.(1000 ns / 125 ns = 8 + 1)
> >
> >         * programmed with 0x9.
> > (1000 ns / 125 ns = 8)
> >
> >> +      * So, for I2C standard mode TRISE = FREQ[5:0] + 1
> >> +      *
> >> +      * In fast mode, the maximum allowed SCL rise time is 300 ns.
> >> +      * If, in the I2C_CR2 register, the value of FREQ[5:0] bits is equal to
> >> +      * 0x08 so period = 125 ns therefore the TRISE[5:0] bits must be
> >> +      * programmed with 03h.(300 ns / 125 ns = 2 + 1)
> >
> > as above s/03h/0x3/;
> 
> ok
> 
> > s/.(/. (/;
> ok
> 
> > s/+ 1//;
> This formula is use to understand how we find the result 0x3
> So, 0x3 => 300 ns / 125ns = 2 + 1

Yeah, I understood that, but writing 300 ns / 125ns = 2 + 1 is
irritating at best.

> >> +      * So, for I2C fast mode TRISE = FREQ[5:0] * 300 / 1000 + 1
> >> +      */
> >> +     if (i2c_dev->speed == STM32F4_I2C_SPEED_STANDARD)
> >> +             trise = freq + 1;
> >> +     else
> >> +             trise = freq * 300 / 1000 + 1;
> >
> > I'd use
> >
> >         * 3 / 10
> >
> > without downside and lesser chance to overflow.
> 
> There is no chance of overflow as the max freq value allowed is 46

ok

> >> +             /*
> >> +              * In fast mode, we compute CCR with duty = 0 as with low
> >> +              * frequencies we are not able to reach 400 kHz.
> >> +              * In that case:
> >> +              * t_scl_high = CCR * I2C parent clk period
> >> +              * t_scl_low = 2 * CCR * I2C parent clk period
> >> +              * So, CCR = I2C parent rate / (400 kHz * 3)
> >> +              *
> >> +              * For example with parent rate = 6 MHz:
> >> +              * CCR = 6000000 / (400000 * 3) = 5
> >> +              * t_scl_high = 5 * (1 / 6000000) = 833 ns > 600 ns
> >> +              * t_scl_low = 2 * 5 * (1 / 6000000) = 1667 ns > 1300 ns
> >> +              * t_scl_high + t_scl_low = 2500 ns so 400 kHz is reached
> >> +              */
> >
> > Huh, that's surprising. So you don't use DUTY any more. I found two
> > hints in the manual that contradict here:
> 
> Yes with the above formula we could use duty = 0 by default
> 
> >
> >         f_{PCLK1} must be at least 2 MHz to achieve Sm mode I2C frequencies
> 
> STM32F4_I2C_MIN_STANDARD_FREQ = 2
> 
> >         It must be at least 4 MHz to achieve Fm mode I2C frequencies.
> 
> STM32F4_I2C_MIN_FAST_FREQ = 6
> 
> > It must be a multiple of 10MHz to reach the 400 kHz maximum I2C Fm mode clock.
> 
> If we use this rule only 3 values are allowed 10 Mhz, 20 Mhz, 30 Mhz and 40 Mhz.
> It is very restrictive.
> So I don't take it into account in order to have more frequencies even
> if 400 Khz is not reached.
> Indeed, in many cases we are very close to 400 Khz.
> For example, the default I2C parent clock in my board is 45 Mhz
> I reach 395 kHz in theory and 390 kHz by testing.
> I am in Fast mode but not with the max freq but very close.

fine

> > and
> >
> >         [...]
> >         If DUTY = 1: (to reach 400 kHz)
> >
> > Strange.
> >
> >> +             val = DIV_ROUND_UP(i2c_dev->parent_rate, 400000 * 3);
> >
> > the manual reads:
> >
> >         The minimum allowed value is 0x04, except in FAST DUTY mode
> >         where the minimum allowed value is 0x01
> >
> > You don't check for that, right?
> 
> As the minimum freq value is 6 Mhz in fast mode the minimum CCR is 5
> as described in the comment.
> So I don't need to check that again as it is already done by checking
> parent frequency.

That would then go into a comment.

> > CCR is 11 bits wide. A comment confirming that this cannot overflow
> > would be nice.
> 
> Again there is no chance of overflow thanks to parent frequency check

Right, this time I saw this myself, so I requested a comment stating
this fact.
 
Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* Re: [RFC v2 1/5] UDC: Split the driver into amd (pci) and Synopsys core driver
From: Florian Fainelli @ 2017-01-18 18:45 UTC (permalink / raw)
  To: Raviteja Garimella, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
	Felipe Balbi
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1484640308-25976-2-git-send-email-raviteja.garimella-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

On 01/17/2017 12:05 AM, Raviteja Garimella wrote:
> This patch splits the amd5536udc driver into two -- one that does
> pci device registration and the other file that does the rest of
> the driver tasks like the gadget/ep ops etc for Synopsys UDC.
> 
> This way of splitting helps in exporting core driver symbols which
> can be used by any other platform/pci driver that is written for
> the same Synopsys USB device controller.
> 
> The current patch also includes a change in the Kconfig and Makefile.
> A new config option USB_SNP_CORE will be selected automatically when
> any one of the platform or pci driver for the same UDC is selected.
> 
> Signed-off-by: Raviteja Garimella <raviteja.garimella-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

Although the changes you have done make sense and it is most certainly a
good idea to split udc core from bus specific glue logic, it is really
hard to review the changes per-se because of the file rename, could that
happen at a later time?

Also, keep in mind that anytime a driver file is renamed, this poses a
backport/maintenance issue where backporting fixes from latest upstream
to a kernel version that has a different file/directory structure is a
major pain.
-- 
Florian
--
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: Initializing MAC address at run-time
From: Uwe Kleine-König @ 2017-01-18 18:54 UTC (permalink / raw)
  To: Mason
  Cc: Mark Rutland, DT, Arnd Bergmann, Kevin Hilman, netdev,
	Thibaud Cornic, Linux ARM
In-Reply-To: <e083ed68-0e8e-380e-23bd-5ad387c88575@free.fr>

Hello,

On Wed, Jan 18, 2017 at 03:03:57PM +0100, Mason wrote:
> When my system boots up, eth0 is given a seemingly random MAC address.
> 
> [    0.950734] nb8800 26000.ethernet eth0: MAC address ba:de:d6:38:b8:38
> [    0.957334] nb8800 26000.ethernet eth0: MAC address 6e:f1:48:de:d6:c4
> 
> 
> The DT node for eth0 is:
> 
> 	eth0: ethernet@26000 {
> 		compatible = "sigma,smp8734-ethernet";
> 		reg = <0x26000 0x800>;
> 		interrupts = <38 IRQ_TYPE_LEVEL_HIGH>;
> 		clocks = <&clkgen SYS_CLK>;
> 	};
> 
> Documentation/devicetree/bindings/net/ethernet.txt mentions
> - local-mac-address: array of 6 bytes, specifies the MAC address that was
>   assigned to the network device;
> 
> And indeed, if I define this property, eth0 ends up with the MAC address
> I specify in the device tree. But of course, I don't want all my boards
> to share the same MAC address. Every interface has a unique MAC address.
> 
> In fact, the boot loader (not Uboot, a custom non-DT boot loader) stores
> the MAC address somewhere in MMIO space, in some weird custom format.

Where does your machine get the dtb from? You write it to the boot
medium at a certain point of time I assume. So AFAICT you have the
following options (in no particular order):

 a) Describe in the dtb how to find out how the MAC address is stored
    (already pointed out Mark and Robin)
 b) Make your bootloader dt aware and let it provide the
    local-mac-address property.
 c) Adapt the dtb before it is written to the boot medium.
 d) Let the bootloader configure the device and teach the driver to pick
    up the mac from the device's address space.
 e) Accept that the mac address is random during development, and make
    Userspace configure the MAC address, which is early enough for
    production use.

Not sure d) is considered ok today, but some drivers have this feature.
I'd say b) is the best choice.

> I need to do something similar with the NAND partitions. The boot loader
> stores the partition offsets somewhere, and I need to pass this info
> to the NAND framework, so I assumed that inserting the corresponding
> properties at run-time was the correct way to do it.

The list of options here is similar to the list above. d) doesn't work,
but instead you can pass the partitioning on the kernel commandline.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* Re: [PATCH V2 3/4] arm64: dts: Enable SDHCI for Nexus 5X (msm8992)
From: Bjorn Andersson @ 2017-01-18 19:02 UTC (permalink / raw)
  To: Jeremy McNicoll
  Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	andy.gross-QSEj5FYQhm4dnm+yROfE0A, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
	robh-DgEjT+Ai2ygdnm+yROfE0A, arnd-r2nGTMty4D4,
	riteshh-sgV2jX0FEOL9JmXXK+q4OQ, git-LJ92rlH3Dns,
	ulf.hansson-QSEj5FYQhm4dnm+yROfE0A,
	jszhang-eYqpPyKDWXRBDgjK7y7TUQ
In-Reply-To: <1484614729-26751-4-git-send-email-jeremymc-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Mon 16 Jan 16:58 PST 2017, Jeremy McNicoll wrote:

> Add Nexus 5X (msm8992) SDHCI support, including initial regulator
> entries to support enabling the main SDHCI/MMC.
> 
> The RPM is common between 8992 & 8994 simply reflect reality with
> a shared DT entry.
> 
> The msm8994 RPM regulator talks over SMD to the APPS processor.
> 
> Signed-off-by: Jeremy McNicoll <jeremymc-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> 
> Dropped RobH's ACK explicitly after addressing all feedback.
> The reason is that msm8994-smd-rpm.dtsi was created to allow
> for sharing between 8992 & 8994 as the RPM is common between
> the two. 
> 
>  .../bindings/regulator/qcom,smd-rpm-regulator.txt  |  40 +++
>  .../boot/dts/qcom/msm8992-bullhead-rev-101.dts     |   2 +
>  arch/arm64/boot/dts/qcom/msm8992-pins.dtsi         |  82 ++++++
>  arch/arm64/boot/dts/qcom/msm8992.dtsi              | 153 ++++++++++++
>  arch/arm64/boot/dts/qcom/msm8994-smd-rpm.dtsi      | 276 +++++++++++++++++++++
>  drivers/regulator/qcom_smd-regulator.c             |  49 ++++
>  6 files changed, 602 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/msm8994-smd-rpm.dtsi
> 
> diff --git a/Documentation/devicetree/bindings/regulator/qcom,smd-rpm-regulator.txt b/Documentation/devicetree/bindings/regulator/qcom,smd-rpm-regulator.txt
> index 1f8d6f8..126989b 100644
> --- a/Documentation/devicetree/bindings/regulator/qcom,smd-rpm-regulator.txt
> +++ b/Documentation/devicetree/bindings/regulator/qcom,smd-rpm-regulator.txt
> @@ -23,6 +23,7 @@ Regulator nodes are identified by their compatible:
>  		    "qcom,rpm-pm8916-regulators"
>  		    "qcom,rpm-pm8941-regulators"
>  		    "qcom,rpm-pma8084-regulators"
> +		    "qcom,rpm-pm8994-regulators"
>  
>  - vdd_s1-supply:
>  - vdd_s2-supply:
> @@ -97,6 +98,40 @@ Regulator nodes are identified by their compatible:
>  	Definition: reference to regulator supplying the input pin, as
>  		    described in the data sheet
>  
> +- vdd_s1-supply:
> +- vdd_s2-supply:
> +- vdd_s3-supply:
> +- vdd_s4-supply:
> +- vdd_s5-supply:
> +- vdd_s6-supply:
> +- vdd_s7-supply:
> +- vdd_l1_l11-supply:
> +- vdd_l2_l3_l4_l27-supply:
> +- vdd_l5_l7-supply:
> +- vdd_l6_l12_l14_l15_l26-supply:
> +- vdd_l8-supply:
> +- vdd_l9_l10_l13_l20_l23_l24-supply:
> +- vdd_l1_l11-supply:
> +- vdd_l6_l12_l14_l15_l26-supply:
> +- vdd_l16_l25-supply:
> +- vdd_l17-supply:
> +- vdd_l18-supply:
> +- vdd_l19-supply:
> +- vdd_l21-supply:
> +- vdd_l22-supply:
> +- vdd_l16_l25-supply:
> +- vdd_l27-supply:
> +- vdd_l28-supply:
> +- vdd_l29-supply:
> +- vdd_l30-supply:
> +- vdd_l31-supply:
> +- vdd_l32-supply:
> +	Usage: optional (pm8994 only)
> +	Value type: <phandle>
> +	Definition: reference to regulator supplying the input pin, as
> +		    described in the data sheet.

This is not entirely correct and should be part of a "arm64: dts" patch.

It seems to be compatible with the pm8994 patch we've had sitting in the
Linaro tree for msm8996 for some time, so I did send this out; with you
Cc. Please give it a spin.

> +
> +
>  The regulator node houses sub-nodes for each regulator within the device. Each
>  sub-node is identified using the node's name, with valid values listed for each
>  of the pmics below.
> @@ -118,6 +153,11 @@ pma8084:
>  	l6, l7, l8, l9, l10, l11, l12, l13, l14, l15, l16, l17, l18, l19, l20,
>  	l21, l22, l23, l24, l25, l26, l27, lvs1, lvs2, lvs3, lvs4, 5vs1
>  
> +pm8994:
> +	s1, s2, s3, s4, s5, s6, s7, l1, l2, l3, l4, l5, l6, l7, l8, l9, l10, l11,
> +	l12, l13, l14, l15, l16, l17, l18, l19, l20, l21, l22, l23, l24, l25, l26,
> +	l27, l28, l29, l30, l31, l32, lvs1, lvs2
> +
>  The content of each sub-node is defined by the standard binding for regulators -
>  see regulator.txt.
>  
> diff --git a/arch/arm64/boot/dts/qcom/msm8992-bullhead-rev-101.dts b/arch/arm64/boot/dts/qcom/msm8992-bullhead-rev-101.dts
> index 4542133..3fc9a33 100644
> --- a/arch/arm64/boot/dts/qcom/msm8992-bullhead-rev-101.dts
> +++ b/arch/arm64/boot/dts/qcom/msm8992-bullhead-rev-101.dts
> @@ -39,3 +39,5 @@
>  		};
>  	};
>  };
> +
> +#include "msm8994-smd-rpm.dtsi"
> diff --git a/arch/arm64/boot/dts/qcom/msm8992-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8992-pins.dtsi
> index d2a26f0..d3ae5ab 100644
> --- a/arch/arm64/boot/dts/qcom/msm8992-pins.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8992-pins.dtsi
> @@ -35,4 +35,86 @@
>  			bias-pull-down;
>  		};
>  	};
> +
> +	/* 0-3 for sdc1 4-6 for sdc2 */
> +	/* Order of pins */
> +	/* SDC1: CLK -> 0, CMD -> 1, DATA -> 2, RCLK -> 3 */
> +	/* SDC2: CLK -> 4, CMD -> 5, DATA -> 6 */
> +	pmx-sdc1-clk {
> +		sdc1_clk_on: clk-on {
> +			pinmux {
> +				pins = "sdc1_clk";
> +			};

The name of these nodes are insignificant, so you don't have to have a
pinmux and a pinconf, you can describe all properties in one node. I
even think you can flatten this and drop the inner subnode.

> +			pinconf {
> +				pins = "sdc1_clk";
> +				bias-disable = <0>; /* No pull */
> +				drive-strength = <16>; /* 16mA */
> +			};
> +		};
[..]
> diff --git a/arch/arm64/boot/dts/qcom/msm8992.dtsi b/arch/arm64/boot/dts/qcom/msm8992.dtsi
> index 44b2d37..77edffc 100644
> --- a/arch/arm64/boot/dts/qcom/msm8992.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8992.dtsi
> @@ -82,6 +82,12 @@
>  				<0xf9002000 0x1000>;
>  		};
>  
> +		apcs: syscon@f900d000 {
> +			compatible = "syscon";
> +			reg = <0xf900d000 0x2000>;
> +		};
> +
> +

Please send the SMEM/SMD-ification in a separate patch from the sdhci
addition.

>  		timer@f9020000 {
>  			#address-cells = <1>;
>  			#size-cells = <1>;
> @@ -172,12 +178,159 @@
>  			#power-domain-cells = <1>;
>  			reg = <0xfc400000 0x2000>;
>  		};
> +
> +		sdhci1: mmc@f9824900 {
> +			compatible = "qcom,sdhci-msm-v4";
> +			reg = <0xf9824900 0x1a0>, <0xf9824000 0x800>;
> +			reg-names = "hc_mem", "core_mem";
> +
> +			interrupts = <GIC_SPI 123 IRQ_TYPE_NONE>,
> +					<GIC_SPI 138 IRQ_TYPE_NONE>;
> +			interrupt-names = "hc_irq", "pwr_irq";
> +
> +			clocks = <&clock_gcc GCC_SDCC1_APPS_CLK>,
> +				<&clock_gcc GCC_SDCC1_AHB_CLK>;
> +			clock-names = "core", "iface";
> +
> +			pinctrl-names = "default", "sleep";
> +			pinctrl-0 = <&sdc1_clk_on &sdc1_cmd_on &sdc1_data_on
> +					&sdc1_rclk_on>;
> +			pinctrl-1 = <&sdc1_clk_off &sdc1_cmd_off &sdc1_data_off
> +					&sdc1_rclk_off>;
> +
> +			vdd-supply = <&pm8994_l20>;
> +			qcom,vdd-voltage-level = <2950000 2950000>;
> +			qcom,vdd-current-level = <200 570000>;

These properties are not recognized upstream, please drop.

> +
> +			vdd-io-supply = <&pm8994_s4>;
> +			qcom,vdd-io-voltage-level = <1800000 1800000>;
> +			qcom,vdd-io-current-level = <200 325000>;
> +
> +			regulator-always-on;
> +			bus-width = <8>;
> +			mmc-hs400-1_8v;
> +			status = "okay";
> +		};
> +
> +		vreg_vph_pwr: vreg-vph-pwr {
> +			compatible = "regulator-fixed";
> +			status = "okay";
> +			regulator-name = "vph-pwr";
> +
> +			regulator-min-microvolt = <3600000>;
> +			regulator-max-microvolt = <3600000>;
> +
> +			regulator-always-on;
> +		};

This doesn't have a "reg", so please move it outside "soc"

> +
> +		rpm_msg_ram: memory@fc428000 {
> +			compatible = "qcom,rpm-msg-ram";
> +			reg = <0xfc428000 0x4000>;
> +		};
> +
> +		sfpb_mutex_regs: syscon@fd484000 {
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			compatible = "syscon";
> +			reg = <0xfd484000 0x400>;
> +		};
> +
> +		sfpb_mutex: hwmutex {
> +			compatible = "qcom,sfpb-mutex";
> +			syscon = <&sfpb_mutex_regs 0x0 0x100>;
> +			#hwlock-cells = <1>;
> +		};
> +
> +		smem {
> +			compatible = "qcom,smem";
> +			memory-region = <&smem_region>;
> +			qcom,rpm-msg-ram = <&rpm_msg_ram>;
> +			hwlocks = <&sfpb_mutex 3>;
> +		};

The smem enablement here looks reasonable, please split into a separate
patch.

>  	};
>  
>  	memory {
>  		device_type = "memory";
>  		reg = <0 0 0 0>; // bootloader will update
>  	};
> +
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		smem_region: smem@6a00000 {
> +			reg = <0x0 0x6a00000 0x0 0x200000>;
> +			no-map;
> +		};
> +	};
> +
> +	smd_rpm: smd {

You don't have to reference this by label, just saying "/smd" will work
just as well.

> +		compatible = "qcom,smd";
> +
> +		rpm {
> +			interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>;
> +			qcom,ipc = <&apcs 8 0>;
> +			qcom,smd-edge = <15>;
> +			qcom,local-pid = <0>;
> +			qcom,remote-pid = <6>;
> +
> +			rpm-requests {
> +				compatible = "qcom,rpm-msm8994";
> +				qcom,smd-channels = "rpm_requests";
> +
> +				rpmcc: qcom,rpmcc {
> +					/* TODO: update when rpmcc-msm8994 support added */
> +					compatible = "qcom,rpmcc-msm8916",
> +							"qcom,rpmcc";
> +					#clock-cells = <1>;
> +				};

You're not compatible with qcom,rpmcc-msm8916, so don't fool the kernel
to think you are. Just drop this node until you have a rpmcc and need
it.

> +
> +				smd_rpm_regulators: pm8994-regulators {

This label is unused.

> +					compatible = "qcom,rpm-pm8994-regulators";
> +
> +					pm8994_s1: s1 {};
> +					pm8994_s2: s2 {};
> +					pm8994_s3: s3 {};
> +					pm8994_s4: s4 {};
> +					pm8994_s5: s5 {};
> +					pm8994_s6: s6 {};
> +					pm8994_s7: s7 {};
> +
> +					pm8994_l1: l1 {};
> +					pm8994_l2: l2 {};
> +					pm8994_l3: l3 {};
> +					pm8994_l4: l4 {};
> +					pm8994_l6: l6 {};
> +					pm8994_l8: l8 {};
> +					pm8994_l9: l9 {};
> +					pm8994_l10: l10 {};
> +					pm8994_l11: l11 {};
> +					pm8994_l12: l12 {};
> +					pm8994_l13: l13 {};
> +					pm8994_l14: l14 {};
> +					pm8994_l15: l15 {};
> +					pm8994_l16: l16 {};
> +					pm8994_l17: l17 {};
> +					pm8994_l18: l18 {};
> +					pm8994_l19: l19 {};
> +					pm8994_l20: l20 {};
> +					pm8994_l21: l21 {};
> +					pm8994_l22: l22 {};
> +					pm8994_l23: l23 {};
> +					pm8994_l24: l24 {};
> +					pm8994_l25: l25 {};
> +					pm8994_l26: l26 {};
> +					pm8994_l27: l27 {};
> +					pm8994_l28: l28 {};
> +					pm8994_l29: l29 {};
> +					pm8994_l30: l30 {};
> +					pm8994_l31: l31 {};
> +					pm8994_l32: l32 {};

Add lvs1 & lvs2.

> +				};
> +			};
> +		};
> +	};
>  };
>  
>  
> diff --git a/arch/arm64/boot/dts/qcom/msm8994-smd-rpm.dtsi b/arch/arm64/boot/dts/qcom/msm8994-smd-rpm.dtsi

These rpm settings are not for msm8994, they are for your device. So
please drop this file and move below nodes into your device dts.

[..]
> +&smd_rpm {
> +	rpm {
> +		rpm_requests {
> +			pm8994-regulators {
> +
> +				vdd_l1-supply = <&pm8994_s1>;
> +				vdd_l2_26_28-supply = <&pm8994_s3>;
> +				vdd_l3_11-supply = <&pm8994_s3>;
> +				vdd_l4_27_31-supply = <&pm8994_s3>;
> +				vdd_l5_7-supply = <&pm8994_s3>;
> +				vdd_l6_12_32-supply = <&pm8994_s5>;
> +				vdd_l8_16_30-supply = <&vreg_vph_pwr>;
> +				vdd_l9_10_18_22-supply = <&vreg_vph_pwr>;
> +				vdd_l13_19_23_24-supply = <&vreg_vph_pwr>;
> +				vdd_l14_15-supply = <&pm8994_s5>;
> +				vdd_l17_29-supply = <&vreg_vph_pwr>;
> +				vdd_l20_21-supply = <&vreg_vph_pwr>;
> +				vdd_l25-supply = <&pm8994_s5>;
> +				/*vin_lvs1_2 = <&pm8994_s4>; */

I added this to the pm8994 regulator patch I just sent out, called it
"vdd_lvs1_2".

> +
[..]
> diff --git a/drivers/regulator/qcom_smd-regulator.c b/drivers/regulator/qcom_smd-regulator.c
[..]
>  
> +static const struct rpm_regulator_data rpm_pm8994_regulators[] = {
> +	{ "s1", QCOM_SMD_RPM_SMPA, 1, &pma8084_ftsmps, "vdd_s1" },

As with the binding, this isn't entirely correct. Please see my
submitted patch.

Regards,
Bjorn
--
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


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