Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v7 REPOST 8/9] arm: add sysfs cpu_capacity attribute
From: Juri Lelli @ 2016-11-03  5:29 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, peterz-wEGCiKHe2LqWVfeAwA7xHQ,
	vincent.guittot-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	sudeep.holla-5wv7dgnIgG8, lorenzo.pieralisi-5wv7dgnIgG8,
	catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
	morten.rasmussen-5wv7dgnIgG8, dietmar.eggemann-5wv7dgnIgG8,
	broonie-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <20161030204551.GM1041-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org>

Hi,

apologies for the delay in replying, but I'm attending Linux Plumbers
this week.

On 30/10/16 20:45, Russell King - ARM Linux wrote:
> On Mon, Oct 17, 2016 at 04:46:49PM +0100, Juri Lelli wrote:
> > +#ifdef CONFIG_PROC_SYSCTL
> > +#include <asm/cpu.h>
> > +#include <linux/string.h>
> 
> Include files at the top of the file please.  No need to ifdef them.
> They're sorted alphabetically, so new additions should be alphabetical.
> (That's a general rule - if something is already alphabetical, do not
> make it non-alphabetical.)
> 
> > +static ssize_t show_cpu_capacity(struct device *dev,
> > +				 struct device_attribute *attr,
> > +				 char *buf)
> > +{
> > +	struct cpu *cpu = container_of(dev, struct cpu, dev);
> > +	ssize_t rc;
> > +	int cpunum = cpu->dev.id;
> > +	unsigned long capacity = arch_scale_cpu_capacity(NULL, cpunum);
> > +
> > +	rc = sprintf(buf, "%lu\n", capacity);
> > +
> > +	return rc;
> 
> Way too many lines for such a simple function.  This can be simplified
> to just:
> 
> 	struct cpu *cpu = container_of(dev, struct cpu, dev);
> 
> 	return sprintf(buf, "%lu\n", arch_scale_cpu_capacity(NULL, cpu->dev.id);
> 
> If you don't like the last line ending on column 79, then feel free to
> break it across two lines after the format string.
> 
> > +}
> > +
> > +static ssize_t store_cpu_capacity(struct device *dev,
> > +				  struct device_attribute *attr,
> > +				  const char *buf,
> > +				  size_t count)
> > +{
> > +	struct cpu *cpu = container_of(dev, struct cpu, dev);
> > +	int this_cpu = cpu->dev.id, i;
> > +	unsigned long new_capacity;
> > +	ssize_t ret;
> > +
> > +	if (count) {
> > +		char *p = (char *) buf;
> > +
> > +		ret = kstrtoul(p, 0, &new_capacity);
> 
> Unnecessary cast - kstrtoul takes a const char pointer, and in any case
> it's really bad form to cast away the "const-ness" of any pointer.  So,
> just:
> 
> 	if (count) {
> 		ret = kstrtoul(buf, 0, &new_capacity);
> 
> should work just fine.
> 
> > +		if (ret)
> > +			return ret;
> > +		if (new_capacity > SCHED_CAPACITY_SCALE)
> > +			return -EINVAL;
> > +
> > +		mutex_lock(&cpu_scale_mutex);
> > +		for_each_cpu(i, &cpu_topology[this_cpu].core_sibling)
> > +			set_capacity_scale(i, new_capacity);
> > +		mutex_unlock(&cpu_scale_mutex);
> > +	}
> > +
> > +	return count;
> > +}
> > +
> > +static DEVICE_ATTR(cpu_capacity,
> > +		   0644,
> > +		   show_cpu_capacity,
> > +		   store_cpu_capacity);
> 
> There's a move to use the named DEVICE_ATTR_RW() for this kind of thing,
> it'll want the functions named xxx_show() and xxx_store().  I see
> there's some recent patches to do this conversion across the kernel, so
> this should probably be done before submission.
> 
> > +
> > +static int register_cpu_capacity_sysctl(void)
> > +{
> > +	int i;
> > +	struct device *cpu;
> > +
> > +	for_each_possible_cpu(i) {
> > +		cpu = get_cpu_device(i);
> > +		if (!cpu) {
> > +			pr_err("%s: too early to get CPU%d device!\n",
> > +			       __func__, i);
> > +			continue;
> > +		}
> > +		device_create_file(cpu, &dev_attr_cpu_capacity);
> > +	}
> > +
> > +	return 0;
> > +}
> > +late_initcall(register_cpu_capacity_sysctl);
> 
> Hmm, this is really weird.  topology_init() in arch/arm/kernel/setup.c
> is where these devices get created, and they're created at
> subsys_initcall() time.  By that point, the list of possible CPUs has
> to be static, it's not going to change.  I don't see why this has to be
> done at late_initcall() - and since topology.c will be linked after
> setup.c, I don't see why it shouldn't be at subsys_initcall() level to
> follow on after topology_init().
> 

I should have addressed your comments with the updated version below. If
it looks good to you I'll superseed the old version with this new one.

Best,

- Juri

--->8---
>From 14c0f21d403ad47843896eecc042334d4e0ed8dd Mon Sep 17 00:00:00 2001
From: Juri Lelli <juri.lelli-5wv7dgnIgG8@public.gmane.org>
Date: Thu, 15 Oct 2015 13:53:37 +0100
Subject: [PATCH v7.1 8/9] arm: add sysfs cpu_capacity attribute

Add a sysfs cpu_capacity attribute with which it is possible to read and
write (thus over-writing default values) CPUs capacity. This might be
useful in situations where values needs changing after boot.

The new attribute shows up as:

 /sys/devices/system/cpu/cpu*/cpu_capacity

Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Signed-off-by: Juri Lelli <juri.lelli-5wv7dgnIgG8@public.gmane.org>
---

Changes from v5:
  - add mutex to protect cpu_scale (as pointed out by Morten off-line)

Changes from v7:
  - include files moved at top of file
  - show_cpu_capacity simplified to less lines of code
  - unnecessary cast removed in store_cpu_capacity
  - use DEVICE_ATTR_RW() instead of DEVICE_ATTR()
  - use subsys_initcall instead of late_initcall
---
 arch/arm/kernel/topology.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index aa63287d9a10..ebf47d91b804 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -22,7 +22,9 @@
 #include <linux/of.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
+#include <linux/string.h>
 
+#include <asm/cpu.h>
 #include <asm/cputype.h>
 #include <asm/topology.h>
 
@@ -42,6 +44,7 @@
  * updated during this sequence.
  */
 static DEFINE_PER_CPU(unsigned long, cpu_scale) = SCHED_CAPACITY_SCALE;
+static DEFINE_MUTEX(cpu_scale_mutex);
 
 unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
 {
@@ -53,6 +56,65 @@ static void set_capacity_scale(unsigned int cpu, unsigned long capacity)
 	per_cpu(cpu_scale, cpu) = capacity;
 }
 
+#ifdef CONFIG_PROC_SYSCTL
+static ssize_t cpu_capacity_show(struct device *dev,
+				 struct device_attribute *attr,
+				 char *buf)
+{
+	struct cpu *cpu = container_of(dev, struct cpu, dev);
+
+	return sprintf(buf, "%lu\n",
+			arch_scale_cpu_capacity(NULL, cpu->dev.id));
+}
+
+static ssize_t cpu_capacity_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf,
+				  size_t count)
+{
+	struct cpu *cpu = container_of(dev, struct cpu, dev);
+	int this_cpu = cpu->dev.id, i;
+	unsigned long new_capacity;
+	ssize_t ret;
+
+	if (count) {
+		ret = kstrtoul(buf, 0, &new_capacity);
+		if (ret)
+			return ret;
+		if (new_capacity > SCHED_CAPACITY_SCALE)
+			return -EINVAL;
+
+		mutex_lock(&cpu_scale_mutex);
+		for_each_cpu(i, &cpu_topology[this_cpu].core_sibling)
+			set_capacity_scale(i, new_capacity);
+		mutex_unlock(&cpu_scale_mutex);
+	}
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(cpu_capacity);
+
+static int register_cpu_capacity_sysctl(void)
+{
+	int i;
+	struct device *cpu;
+
+	for_each_possible_cpu(i) {
+		cpu = get_cpu_device(i);
+		if (!cpu) {
+			pr_err("%s: too early to get CPU%d device!\n",
+			       __func__, i);
+			continue;
+		}
+		device_create_file(cpu, &dev_attr_cpu_capacity);
+	}
+
+	return 0;
+}
+subsys_initcall(register_cpu_capacity_sysctl);
+#endif
+
 #ifdef CONFIG_OF
 struct cpu_efficiency {
 	const char *compatible;
@@ -132,6 +194,7 @@ static void normalize_cpu_capacity(void)
 		return;
 
 	pr_debug("cpu_capacity: capacity_scale=%u\n", capacity_scale);
+	mutex_lock(&cpu_scale_mutex);
 	for_each_possible_cpu(cpu) {
 		capacity = (raw_capacity[cpu] << SCHED_CAPACITY_SHIFT)
 			/ capacity_scale;
@@ -139,6 +202,7 @@ static void normalize_cpu_capacity(void)
 		pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n",
 			cpu, arch_scale_cpu_capacity(NULL, cpu));
 	}
+	mutex_unlock(&cpu_scale_mutex);
 }
 
 #ifdef CONFIG_CPU_FREQ
-- 
2.10.0

--
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] of: base: Export symbol of __of_find_all_nodes()
From: Laxman Dewangan @ 2016-11-03  5:24 UTC (permalink / raw)
  To: Rob Herring
  Cc: Frank Rowand, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_JsqJnAzJDeTS1Sr2H66++78zF+8wJfN8iYAQ3sbxJw1Axdw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>


On Thursday 03 November 2016 09:09 AM, Rob Herring wrote:
> On Fri, Oct 28, 2016 at 6:28 AM, Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> The function __of_find_all_nodes() is used in the public header
>> linux/of.h and if any driver developed using this API then
>> it reports error as unknown symbol if compiled as module.
>>
>> Export this APIs using EXPORT_SYMBOL() so that it can be used
>> from driver compiled as module.
> What driver needs this? This isn't really a function I'd expect drivers to use.
>

I am using the for loop for each node using the macro 
for_each_of_allnodes_from() which is define as

#define for_each_of_allnodes_from(from, dn) \
         for (dn = __of_find_all_nodes(from); dn; dn = 
__of_find_all_nodes(dn))

and this is using the above APIs.


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
--
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 0/2] mmc: dw_mmc: fix the wrong operation for reset controller
From: Jaehoon Chung @ 2016-11-03  5:19 UTC (permalink / raw)
  To: John Stultz
  Cc: linux-mmc,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Ulf Hansson, Rob Herring, Mark Rutland, lkml, Guodong Xu, Leo Yan,
	Vincent Guittot
In-Reply-To: <CALAqxLXs704Y-TGAQsggSnDohyFDYvQw=zYuiHRu9UKgHEfbFw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi,

On 11/02/2016 06:56 AM, John Stultz wrote:
> On Sun, Oct 30, 2016 at 7:49 PM, Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>> This patch adds the "reset-names" as reset controller for dwmmc controller.
>> Linaro guys had reported the issue about booting stuck.
>>
>> Some SoCs are enabled the CONFIG_RESET_CONTROLLER.
>> then dwmmc controller are waiting for getting reset controller.
>> But if doesn't define "resets" property in device-tree, it should be stuck.
>>
>> If use the reset-names as reset controller for dwmmc controller,
>> it's more stable than now.
>> This commit is related with the below commit.
>>
>> Fixes: d6786fefe816 ("mmc: dw_mmc: add reset support to dwmmc host controller")
> 
> So I wasn't hit by the issue, as I don't tend to boot from SD on my
> hikey, but I did run these patches and I'm not seeing any issues with
> my Android environment.
> 
> So for what its worth:
> Acked-by: John Stultz <john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Applied on my repository (fixes branch).

Best Regards,
Jaehoon Chung

> 
> thanks
> -john
> 
> 
> 

--
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 v5 8/8] firmware: arm_scpi: add support for legacy SCPI compatible
From: Sudeep Holla @ 2016-11-03  4:52 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Sudeep Holla, Olof Johansson, linux-arm-kernel, devicetree,
	linux-kernel, linux-amlogic
In-Reply-To: <1478148731-11712-1-git-send-email-sudeep.holla@arm.com>

This patch adds new DT match table to setup the support for legacy SCPI
protocol. It aldo adds "arm,legacy-scpi" to the legacy match entry.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scpi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
index 902233642bd3..a38e1537fb8e 100644
--- a/drivers/firmware/arm_scpi.c
+++ b/drivers/firmware/arm_scpi.c
@@ -871,6 +871,11 @@ static int scpi_alloc_xfer_list(struct device *dev, struct scpi_chan *ch)
 	return 0;
 }
 
+static const struct of_device_id legacy_scpi_of_match[] = {
+	{.compatible = "arm,legacy-scpi"},
+	{},
+};
+
 static int scpi_probe(struct platform_device *pdev)
 {
 	int count, idx, ret;
@@ -883,6 +888,9 @@ static int scpi_probe(struct platform_device *pdev)
 	if (!scpi_info)
 		return -ENOMEM;
 
+	if (of_match_device(legacy_scpi_of_match, &pdev->dev))
+		scpi_info->is_legacy = true;
+
 	count = of_count_phandle_with_args(np, "mboxes", "#mbox-cells");
 	if (count < 0) {
 		dev_err(dev, "no mboxes property in '%s'\n", np->full_name);
@@ -984,6 +992,7 @@ static int scpi_probe(struct platform_device *pdev)
 
 static const struct of_device_id scpi_of_match[] = {
 	{.compatible = "arm,scpi"},
+	{.compatible = "arm,legacy-scpi"},
 	{},
 };
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH v5 7/8] Documentation: bindings: Add support for Amlogic GXBB SCPI protocol
From: Sudeep Holla @ 2016-11-03  4:52 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Sudeep Holla, Olof Johansson,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring
In-Reply-To: <1478148731-11712-1-git-send-email-sudeep.holla-5wv7dgnIgG8@public.gmane.org>

From: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
(decoupled from the generic scpi binding)
Signed-off-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
---
 .../devicetree/bindings/arm/amlogic,scpi.txt         | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/amlogic,scpi.txt

diff --git a/Documentation/devicetree/bindings/arm/amlogic,scpi.txt b/Documentation/devicetree/bindings/arm/amlogic,scpi.txt
new file mode 100644
index 000000000000..7b9a861e9306
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/amlogic,scpi.txt
@@ -0,0 +1,20 @@
+System Control and Power Interface (SCPI) Message Protocol
+(in addition to the standard binding in [0])
+----------------------------------------------------------
+Required properties
+
+- compatible : should be "amlogic,meson-gxbb-scpi"
+
+AMLOGIC SRAM and Shared Memory for SCPI
+------------------------------------
+
+Required properties:
+- compatible : should be "amlogic,meson-gxbb-sram"
+
+Each sub-node represents the reserved area for SCPI.
+
+Required sub-node properties:
+- compatible : should be "amlogic,meson-gxbb-scp-shmem" for SRAM based shared
+		memory on Amlogic GXBB SoC.
+
+[0] Documentation/devicetree/bindings/arm/arm,scpi.txt
-- 
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

* [PATCH v5 6/8] Documentation: bindings: add compatible specific to legacy SCPI protocol
From: Sudeep Holla @ 2016-11-03  4:52 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Sudeep Holla, Olof Johansson, linux-arm-kernel, devicetree,
	linux-kernel, linux-amlogic, Rob Herring
In-Reply-To: <1478148731-11712-1-git-send-email-sudeep.holla@arm.com>

This patch adds specific compatible to support legacy SCPI protocol.

Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 Documentation/devicetree/bindings/arm/arm,scpi.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt b/Documentation/devicetree/bindings/arm/arm,scpi.txt
index d1882c4540d0..ebd03fc93135 100644
--- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
+++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
@@ -7,7 +7,9 @@ by Linux to initiate various system control and power operations.
 
 Required properties:
 
-- compatible : should be "arm,scpi"
+- compatible : should be
+	* "arm,scpi" : For implementations complying to SCPI v1.0 or above
+	* "arm,legacy-scpi" : For implementations complying pre SCPI v1.0
 - mboxes: List of phandle and mailbox channel specifiers
 	  All the channels reserved by remote SCP firmware for use by
 	  SCPI message protocol should be specified in any order
-- 
1.9.1

^ permalink raw reply related

* [PATCH v5 5/8] Documentation: bindings: decouple juno specific details from generic binding
From: Sudeep Holla @ 2016-11-03  4:52 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Sudeep Holla, Olof Johansson, linux-arm-kernel, devicetree,
	linux-kernel, linux-amlogic, Rob Herring
In-Reply-To: <1478148731-11712-1-git-send-email-sudeep.holla@arm.com>

Since SCPI is a generic protocol and the bindings are intended to be
generic, we need to decouple all the platform specific binding details
out of the generic bindings.

This patch moves are the Juno platform specific details into a separate
binding document.

Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 Documentation/devicetree/bindings/arm/arm,scpi.txt | 20 ++++++-----------
 .../devicetree/bindings/arm/juno,scpi.txt          | 26 ++++++++++++++++++++++
 2 files changed, 33 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/juno,scpi.txt

diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt b/Documentation/devicetree/bindings/arm/arm,scpi.txt
index faa4b44572e3..d1882c4540d0 100644
--- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
+++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
@@ -59,18 +59,14 @@ SRAM and Shared Memory for SCPI
 A small area of SRAM is reserved for SCPI communication between application
 processors and SCP.
 
-Required properties:
-- compatible : should be "arm,juno-sram-ns" for Non-secure SRAM on Juno
-
-The rest of the properties should follow the generic mmio-sram description
-found in ../../sram/sram.txt
+The properties should follow the generic mmio-sram description found in [3]
 
 Each sub-node represents the reserved area for SCPI.
 
 Required sub-node properties:
 - reg : The base offset and size of the reserved area with the SRAM
-- compatible : should be "arm,juno-scp-shmem" for Non-secure SRAM based
-	       shared memory on Juno platforms
+- compatible : should be "arm,scp-shmem" for Non-secure SRAM based
+	       shared memory
 
 Sensor bindings for the sensors based on SCPI Message Protocol
 --------------------------------------------------------------
@@ -81,11 +77,9 @@ SCPI provides an API to access the various sensors on the SoC.
 - #thermal-sensor-cells: should be set to 1. This property follows the
 			 thermal device tree bindings[2].
 
-			 Valid cell values are raw identifiers (Sensor
-			 ID) as used by the firmware. Refer to
-			 platform documentation for your
-			 implementation for the IDs to use. For Juno
-			 R0 and Juno R1 refer to [3].
+			 Valid cell values are raw identifiers (Sensor ID)
+			 as used by the firmware. Refer to  platform details
+			 for your implementation for the IDs to use.
 
 Power domain bindings for the power domains based on SCPI Message Protocol
 ------------------------------------------------------------
@@ -112,7 +106,7 @@ PM domain consumers
 [0] http://infocenter.arm.com/help/topic/com.arm.doc.dui0922b/index.html
 [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
 [2] Documentation/devicetree/bindings/thermal/thermal.txt
-[3] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/apas03s22.html
+[3] Documentation/devicetree/bindings/sram/sram.txt
 [4] Documentation/devicetree/bindings/power/power_domain.txt
 
 Example:
diff --git a/Documentation/devicetree/bindings/arm/juno,scpi.txt b/Documentation/devicetree/bindings/arm/juno,scpi.txt
new file mode 100644
index 000000000000..2ace8696bbee
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/juno,scpi.txt
@@ -0,0 +1,26 @@
+System Control and Power Interface (SCPI) Message Protocol
+(in addition to the standard binding in [0])
+
+Juno SRAM and Shared Memory for SCPI
+------------------------------------
+
+Required properties:
+- compatible : should be "arm,juno-sram-ns" for Non-secure SRAM
+
+Each sub-node represents the reserved area for SCPI.
+
+Required sub-node properties:
+- reg : The base offset and size of the reserved area with the SRAM
+- compatible : should be "arm,juno-scp-shmem" for Non-secure SRAM based
+	       shared memory on Juno platforms
+
+Sensor bindings for the sensors based on SCPI Message Protocol
+--------------------------------------------------------------
+Required properties:
+- compatible : should be "arm,scpi-sensors".
+- #thermal-sensor-cells: should be set to 1.
+			 For Juno R0 and Juno R1 refer to [1] for the
+			 sensor identifiers
+
+[0] Documentation/devicetree/bindings/arm/arm,scpi.txt
+[1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/apas03s22.html
-- 
1.9.1

^ permalink raw reply related

* [PATCH v5 4/8] firmware: arm_scpi: allow firmware with get_capabilities not implemented
From: Sudeep Holla @ 2016-11-03  4:52 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: devicetree, linux-kernel, Sudeep Holla, Olof Johansson,
	linux-amlogic, linux-arm-kernel
In-Reply-To: <1478148731-11712-1-git-send-email-sudeep.holla@arm.com>

From: Neil Armstrong <narmstrong@baylibre.com>

On Amlogic SCPI legacy implementation, the GET_CAPABILITIES command is
not supported, failover by using 0.0.0 version.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
[sudeep.holla@arm.com: changed the subject]
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scpi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
index 2982bc7b8c33..902233642bd3 100644
--- a/drivers/firmware/arm_scpi.c
+++ b/drivers/firmware/arm_scpi.c
@@ -783,6 +783,10 @@ static int scpi_init_versions(struct scpi_drvinfo *info)
 		info->protocol_version = le32_to_cpu(caps.protocol_version);
 		info->firmware_version = le32_to_cpu(caps.platform_version);
 	}
+	/* Ignore error if not implemented */
+	if (scpi_info->is_legacy && ret == -EOPNOTSUPP)
+		return 0;
+
 	return ret;
 }
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH v5 3/8] firmware: arm_scpi: add alternative legacy structures, functions and macros
From: Sudeep Holla @ 2016-11-03  4:52 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: devicetree, linux-kernel, Sudeep Holla, Olof Johansson,
	linux-amlogic, linux-arm-kernel
In-Reply-To: <1478148731-11712-1-git-send-email-sudeep.holla@arm.com>

From: Neil Armstrong <narmstrong@baylibre.com>

This patch adds support for the Legacy SCPI protocol that is available
in very early JUNO versions and shipped Amlogic ARMv8 based SoCs. Some
Rockchip SoC are also known to use this version of protocol with
extended vendor commands.

In order to support the legacy SCPI protocol variant, we need to add the
structures and macros definitions that varies against the final SCPI v1.0
specification.

We add the indirection table for legacy commands set so that it can
co-exist with the standard v1.0 command set. It also adds bitmap field
for channel selection since the legacy protocol mandates to send only
selected subset of the commands on the high priority channel.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
[sudeep.holla@arm.com: Updated the changelog]
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scpi.c | 192 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 174 insertions(+), 18 deletions(-)

diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
index 21542a32ad9f..2982bc7b8c33 100644
--- a/drivers/firmware/arm_scpi.c
+++ b/drivers/firmware/arm_scpi.c
@@ -50,13 +50,20 @@
 #define CMD_TOKEN_ID_MASK	0xff
 #define CMD_DATA_SIZE_SHIFT	16
 #define CMD_DATA_SIZE_MASK	0x1ff
+#define CMD_LEGACY_DATA_SIZE_SHIFT	20
+#define CMD_LEGACY_DATA_SIZE_MASK	0x1ff
 #define PACK_SCPI_CMD(cmd_id, tx_sz)			\
 	((((cmd_id) & CMD_ID_MASK) << CMD_ID_SHIFT) |	\
 	(((tx_sz) & CMD_DATA_SIZE_MASK) << CMD_DATA_SIZE_SHIFT))
 #define ADD_SCPI_TOKEN(cmd, token)			\
 	((cmd) |= (((token) & CMD_TOKEN_ID_MASK) << CMD_TOKEN_ID_SHIFT))
+#define PACK_LEGACY_SCPI_CMD(cmd_id, tx_sz)				\
+	((((cmd_id) & CMD_ID_MASK) << CMD_ID_SHIFT) |			       \
+	(((tx_sz) & CMD_LEGACY_DATA_SIZE_MASK) << CMD_LEGACY_DATA_SIZE_SHIFT))
 
 #define CMD_SIZE(cmd)	(((cmd) >> CMD_DATA_SIZE_SHIFT) & CMD_DATA_SIZE_MASK)
+#define CMD_LEGACY_SIZE(cmd)	(((cmd) >> CMD_LEGACY_DATA_SIZE_SHIFT) & \
+					CMD_LEGACY_DATA_SIZE_MASK)
 #define CMD_UNIQ_MASK	(CMD_TOKEN_ID_MASK << CMD_TOKEN_ID_SHIFT | CMD_ID_MASK)
 #define CMD_XTRACT_UNIQ(cmd)	((cmd) & CMD_UNIQ_MASK)
 
@@ -133,7 +140,62 @@ enum scpi_std_cmd {
 	SCPI_CMD_COUNT
 };
 
-/* List of all commands used by this driver, used as indices */
+/* SCPI Legacy Commands */
+enum legacy_scpi_std_cmd {
+	LEGACY_SCPI_CMD_INVALID			= 0x00,
+	LEGACY_SCPI_CMD_SCPI_READY		= 0x01,
+	LEGACY_SCPI_CMD_SCPI_CAPABILITIES	= 0x02,
+	LEGACY_SCPI_CMD_EVENT			= 0x03,
+	LEGACY_SCPI_CMD_SET_CSS_PWR_STATE	= 0x04,
+	LEGACY_SCPI_CMD_GET_CSS_PWR_STATE	= 0x05,
+	LEGACY_SCPI_CMD_CFG_PWR_STATE_STAT	= 0x06,
+	LEGACY_SCPI_CMD_GET_PWR_STATE_STAT	= 0x07,
+	LEGACY_SCPI_CMD_SYS_PWR_STATE		= 0x08,
+	LEGACY_SCPI_CMD_L2_READY		= 0x09,
+	LEGACY_SCPI_CMD_SET_AP_TIMER		= 0x0a,
+	LEGACY_SCPI_CMD_CANCEL_AP_TIME		= 0x0b,
+	LEGACY_SCPI_CMD_DVFS_CAPABILITIES	= 0x0c,
+	LEGACY_SCPI_CMD_GET_DVFS_INFO		= 0x0d,
+	LEGACY_SCPI_CMD_SET_DVFS		= 0x0e,
+	LEGACY_SCPI_CMD_GET_DVFS		= 0x0f,
+	LEGACY_SCPI_CMD_GET_DVFS_STAT		= 0x10,
+	LEGACY_SCPI_CMD_SET_RTC			= 0x11,
+	LEGACY_SCPI_CMD_GET_RTC			= 0x12,
+	LEGACY_SCPI_CMD_CLOCK_CAPABILITIES	= 0x13,
+	LEGACY_SCPI_CMD_SET_CLOCK_INDEX		= 0x14,
+	LEGACY_SCPI_CMD_SET_CLOCK_VALUE		= 0x15,
+	LEGACY_SCPI_CMD_GET_CLOCK_VALUE		= 0x16,
+	LEGACY_SCPI_CMD_PSU_CAPABILITIES	= 0x17,
+	LEGACY_SCPI_CMD_SET_PSU			= 0x18,
+	LEGACY_SCPI_CMD_GET_PSU			= 0x19,
+	LEGACY_SCPI_CMD_SENSOR_CAPABILITIES	= 0x1a,
+	LEGACY_SCPI_CMD_SENSOR_INFO		= 0x1b,
+	LEGACY_SCPI_CMD_SENSOR_VALUE		= 0x1c,
+	LEGACY_SCPI_CMD_SENSOR_CFG_PERIODIC	= 0x1d,
+	LEGACY_SCPI_CMD_SENSOR_CFG_BOUNDS	= 0x1e,
+	LEGACY_SCPI_CMD_SENSOR_ASYNC_VALUE	= 0x1f,
+	LEGACY_SCPI_CMD_COUNT
+};
+
+/* List all commands that are required to go through the high priority link */
+static int legacy_hpriority_cmds[] = {
+	LEGACY_SCPI_CMD_GET_CSS_PWR_STATE,
+	LEGACY_SCPI_CMD_CFG_PWR_STATE_STAT,
+	LEGACY_SCPI_CMD_GET_PWR_STATE_STAT,
+	LEGACY_SCPI_CMD_SET_DVFS,
+	LEGACY_SCPI_CMD_GET_DVFS,
+	LEGACY_SCPI_CMD_SET_RTC,
+	LEGACY_SCPI_CMD_GET_RTC,
+	LEGACY_SCPI_CMD_SET_CLOCK_INDEX,
+	LEGACY_SCPI_CMD_SET_CLOCK_VALUE,
+	LEGACY_SCPI_CMD_GET_CLOCK_VALUE,
+	LEGACY_SCPI_CMD_SET_PSU,
+	LEGACY_SCPI_CMD_GET_PSU,
+	LEGACY_SCPI_CMD_SENSOR_CFG_PERIODIC,
+	LEGACY_SCPI_CMD_SENSOR_CFG_BOUNDS,
+};
+
+/* List all commands used by this driver, used as indexes */
 enum scpi_drv_cmds {
 	CMD_SCPI_CAPABILITIES = 0,
 	CMD_GET_CLOCK_INFO,
@@ -165,6 +227,21 @@ enum scpi_drv_cmds {
 	SCPI_CMD_GET_DEVICE_PWR_STATE,
 };
 
+static int scpi_legacy_commands[CMD_MAX_COUNT] = {
+	LEGACY_SCPI_CMD_SCPI_CAPABILITIES,
+	-1, /* GET_CLOCK_INFO */
+	LEGACY_SCPI_CMD_GET_CLOCK_VALUE,
+	LEGACY_SCPI_CMD_SET_CLOCK_VALUE,
+	LEGACY_SCPI_CMD_GET_DVFS,
+	LEGACY_SCPI_CMD_SET_DVFS,
+	LEGACY_SCPI_CMD_GET_DVFS_INFO,
+	LEGACY_SCPI_CMD_SENSOR_CAPABILITIES,
+	LEGACY_SCPI_CMD_SENSOR_INFO,
+	LEGACY_SCPI_CMD_SENSOR_VALUE,
+	-1, /* SET_DEVICE_PWR_STATE */
+	-1, /* GET_DEVICE_PWR_STATE */
+};
+
 struct scpi_xfer {
 	u32 slot; /* has to be first element */
 	u32 cmd;
@@ -193,8 +270,10 @@ struct scpi_chan {
 struct scpi_drvinfo {
 	u32 protocol_version;
 	u32 firmware_version;
+	bool is_legacy;
 	int num_chans;
 	int *commands;
+	DECLARE_BITMAP(cmd_priority, LEGACY_SCPI_CMD_COUNT);
 	atomic_t next_chan;
 	struct scpi_ops *scpi_ops;
 	struct scpi_chan *channels;
@@ -211,6 +290,11 @@ struct scpi_shared_mem {
 	u8 payload[0];
 } __packed;
 
+struct legacy_scpi_shared_mem {
+	__le32 status;
+	u8 payload[0];
+} __packed;
+
 struct scp_capabilities {
 	__le32 protocol_version;
 	__le32 event_version;
@@ -236,6 +320,12 @@ struct clk_set_value {
 	__le32 rate;
 } __packed;
 
+struct legacy_clk_set_value {
+	__le32 rate;
+	__le16 id;
+	__le16 reserved;
+} __packed;
+
 struct dvfs_info {
 	__le32 header;
 	struct {
@@ -307,19 +397,43 @@ static void scpi_process_cmd(struct scpi_chan *ch, u32 cmd)
 		return;
 	}
 
-	list_for_each_entry(t, &ch->rx_pending, node)
-		if (CMD_XTRACT_UNIQ(t->cmd) == CMD_XTRACT_UNIQ(cmd)) {
-			list_del(&t->node);
-			match = t;
-			break;
-		}
+	/* Command type is not replied by the SCP Firmware in legacy Mode
+	 * We should consider that command is the head of pending RX commands
+	 * if the list is not empty. In TX only mode, the list would be empty.
+	 */
+	if (scpi_info->is_legacy) {
+		match = list_first_entry(&ch->rx_pending, struct scpi_xfer,
+					 node);
+		list_del(&match->node);
+	} else {
+		list_for_each_entry(t, &ch->rx_pending, node)
+			if (CMD_XTRACT_UNIQ(t->cmd) == CMD_XTRACT_UNIQ(cmd)) {
+				list_del(&t->node);
+				match = t;
+				break;
+			}
+	}
 	/* check if wait_for_completion is in progress or timed-out */
 	if (match && !completion_done(&match->done)) {
-		struct scpi_shared_mem *mem = ch->rx_payload;
-		unsigned int len = min(match->rx_len, CMD_SIZE(cmd));
+		unsigned int len;
+
+		if (scpi_info->is_legacy) {
+			struct legacy_scpi_shared_mem *mem = ch->rx_payload;
+
+			/* RX Length is not replied by the legacy Firmware */
+			len = match->rx_len;
+
+			match->status = le32_to_cpu(mem->status);
+			memcpy_fromio(match->rx_buf, mem->payload, len);
+		} else {
+			struct scpi_shared_mem *mem = ch->rx_payload;
+
+			len = min(match->rx_len, CMD_SIZE(cmd));
+
+			match->status = le32_to_cpu(mem->status);
+			memcpy_fromio(match->rx_buf, mem->payload, len);
+		}
 
-		match->status = le32_to_cpu(mem->status);
-		memcpy_fromio(match->rx_buf, mem->payload, len);
 		if (match->rx_len > len)
 			memset(match->rx_buf + len, 0, match->rx_len - len);
 		complete(&match->done);
@@ -331,7 +445,10 @@ static void scpi_handle_remote_msg(struct mbox_client *c, void *msg)
 {
 	struct scpi_chan *ch = container_of(c, struct scpi_chan, cl);
 	struct scpi_shared_mem *mem = ch->rx_payload;
-	u32 cmd = le32_to_cpu(mem->command);
+	u32 cmd = 0;
+
+	if (!scpi_info->is_legacy)
+		cmd = le32_to_cpu(mem->command);
 
 	scpi_process_cmd(ch, cmd);
 }
@@ -343,8 +460,13 @@ static void scpi_tx_prepare(struct mbox_client *c, void *msg)
 	struct scpi_chan *ch = container_of(c, struct scpi_chan, cl);
 	struct scpi_shared_mem *mem = (struct scpi_shared_mem *)ch->tx_payload;
 
-	if (t->tx_buf)
-		memcpy_toio(mem->payload, t->tx_buf, t->tx_len);
+	if (t->tx_buf) {
+		if (scpi_info->is_legacy)
+			memcpy_toio(ch->tx_payload, t->tx_buf, t->tx_len);
+		else
+			memcpy_toio(mem->payload, t->tx_buf, t->tx_len);
+	}
+
 	if (t->rx_buf) {
 		if (!(++ch->token))
 			++ch->token;
@@ -353,7 +475,9 @@ static void scpi_tx_prepare(struct mbox_client *c, void *msg)
 		list_add_tail(&t->node, &ch->rx_pending);
 		spin_unlock_irqrestore(&ch->rx_lock, flags);
 	}
-	mem->command = cpu_to_le32(t->cmd);
+
+	if (!scpi_info->is_legacy)
+		mem->command = cpu_to_le32(t->cmd);
 }
 
 static struct scpi_xfer *get_scpi_xfer(struct scpi_chan *ch)
@@ -392,15 +516,24 @@ static int scpi_send_message(u8 idx, void *tx_buf, unsigned int tx_len,
 
 	cmd = scpi_info->commands[idx];
 
-	chan = atomic_inc_return(&scpi_info->next_chan) % scpi_info->num_chans;
+	if (scpi_info->is_legacy)
+		chan = test_bit(cmd, scpi_info->cmd_priority) ? 1 : 0;
+	else
+		chan = atomic_inc_return(&scpi_info->next_chan) %
+			scpi_info->num_chans;
 	scpi_chan = scpi_info->channels + chan;
 
 	msg = get_scpi_xfer(scpi_chan);
 	if (!msg)
 		return -ENOMEM;
 
-	msg->slot = BIT(SCPI_SLOT);
-	msg->cmd = PACK_SCPI_CMD(cmd, tx_len);
+	if (scpi_info->is_legacy) {
+		msg->cmd = PACK_LEGACY_SCPI_CMD(cmd, tx_len);
+		msg->slot = msg->cmd;
+	} else {
+		msg->slot = BIT(SCPI_SLOT);
+		msg->cmd = PACK_SCPI_CMD(cmd, tx_len);
+	}
 	msg->tx_buf = tx_buf;
 	msg->tx_len = tx_len;
 	msg->rx_buf = rx_buf;
@@ -470,6 +603,18 @@ static int scpi_clk_set_val(u16 clk_id, unsigned long rate)
 				 &stat, sizeof(stat));
 }
 
+static int legacy_scpi_clk_set_val(u16 clk_id, unsigned long rate)
+{
+	int stat;
+	struct legacy_clk_set_value clk = {
+		.id = cpu_to_le16(clk_id),
+		.rate = cpu_to_le32(rate)
+	};
+
+	return scpi_send_message(CMD_SET_CLOCK_VALUE, &clk, sizeof(clk),
+				 &stat, sizeof(stat));
+}
+
 static int scpi_dvfs_get_idx(u8 domain)
 {
 	int ret;
@@ -800,6 +945,17 @@ static int scpi_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, scpi_info);
 
+	if (scpi_info->is_legacy) {
+		/* Replace with legacy variants */
+		scpi_ops.clk_set_val = legacy_scpi_clk_set_val;
+		scpi_info->commands = scpi_legacy_commands;
+
+		/* Fill priority bitmap */
+		for (idx = 0; idx < ARRAY_SIZE(legacy_hpriority_cmds); idx++)
+			set_bit(legacy_hpriority_cmds[idx],
+				scpi_info->cmd_priority);
+	}
+
 	ret = scpi_init_versions(scpi_info);
 	if (ret) {
 		dev_err(dev, "incorrect or no SCP firmware found\n");
-- 
1.9.1

^ permalink raw reply related

* [PATCH v5 2/8] firmware: arm_scpi: increase MAX_DVFS_OPPS to 16 entries
From: Sudeep Holla @ 2016-11-03  4:52 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Sudeep Holla, Olof Johansson,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1478148731-11712-1-git-send-email-sudeep.holla-5wv7dgnIgG8@public.gmane.org>

From: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

Since Amlogic SoCs supports more than 8 OPPs per domains, we need increase
the OPP structure size.

This patch increases the MAX_DVFS_OPPS to 16.

Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Signed-off-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
---
 drivers/firmware/arm_scpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
index 9e9b022450eb..21542a32ad9f 100644
--- a/drivers/firmware/arm_scpi.c
+++ b/drivers/firmware/arm_scpi.c
@@ -63,7 +63,7 @@
 #define SCPI_SLOT		0
 
 #define MAX_DVFS_DOMAINS	8
-#define MAX_DVFS_OPPS		8
+#define MAX_DVFS_OPPS		16
 #define DVFS_LATENCY(hdr)	(le32_to_cpu(hdr) >> 16)
 #define DVFS_OPP_COUNT(hdr)	((le32_to_cpu(hdr) >> 8) & 0xff)
 
-- 
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

* [PATCH v5 1/8] firmware: arm_scpi: add command indirection to support legacy commands
From: Sudeep Holla @ 2016-11-03  4:52 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Sudeep Holla, Olof Johansson, linux-arm-kernel, devicetree,
	linux-kernel, linux-amlogic
In-Reply-To: <1478148731-11712-1-git-send-email-sudeep.holla@arm.com>

Since the legacy SCPI and the SCPI v1.0 differ in the command values,
it's better to create some sort of command indirection in the driver
to avoid repeated version check at multiple places.

This patch adds the indirection command table to allow different values
of the command across SCPI versions.

[narmstrong@baylibre.com: added cmd check in scpi_send_message]
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scpi.c | 71 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 57 insertions(+), 14 deletions(-)

diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
index ce2bc2a38101..9e9b022450eb 100644
--- a/drivers/firmware/arm_scpi.c
+++ b/drivers/firmware/arm_scpi.c
@@ -99,6 +99,7 @@ enum scpi_error_codes {
 	SCPI_ERR_MAX
 };
 
+/* SCPI Standard commands */
 enum scpi_std_cmd {
 	SCPI_CMD_INVALID		= 0x00,
 	SCPI_CMD_SCPI_READY		= 0x01,
@@ -132,6 +133,38 @@ enum scpi_std_cmd {
 	SCPI_CMD_COUNT
 };
 
+/* List of all commands used by this driver, used as indices */
+enum scpi_drv_cmds {
+	CMD_SCPI_CAPABILITIES = 0,
+	CMD_GET_CLOCK_INFO,
+	CMD_GET_CLOCK_VALUE,
+	CMD_SET_CLOCK_VALUE,
+	CMD_GET_DVFS,
+	CMD_SET_DVFS,
+	CMD_GET_DVFS_INFO,
+	CMD_SENSOR_CAPABILITIES,
+	CMD_SENSOR_INFO,
+	CMD_SENSOR_VALUE,
+	CMD_SET_DEVICE_PWR_STATE,
+	CMD_GET_DEVICE_PWR_STATE,
+	CMD_MAX_COUNT,
+};
+
+static int scpi_std_commands[CMD_MAX_COUNT] = {
+	SCPI_CMD_SCPI_CAPABILITIES,
+	SCPI_CMD_GET_CLOCK_INFO,
+	SCPI_CMD_GET_CLOCK_VALUE,
+	SCPI_CMD_SET_CLOCK_VALUE,
+	SCPI_CMD_GET_DVFS,
+	SCPI_CMD_SET_DVFS,
+	SCPI_CMD_GET_DVFS_INFO,
+	SCPI_CMD_SENSOR_CAPABILITIES,
+	SCPI_CMD_SENSOR_INFO,
+	SCPI_CMD_SENSOR_VALUE,
+	SCPI_CMD_SET_DEVICE_PWR_STATE,
+	SCPI_CMD_GET_DEVICE_PWR_STATE,
+};
+
 struct scpi_xfer {
 	u32 slot; /* has to be first element */
 	u32 cmd;
@@ -161,6 +194,7 @@ struct scpi_drvinfo {
 	u32 protocol_version;
 	u32 firmware_version;
 	int num_chans;
+	int *commands;
 	atomic_t next_chan;
 	struct scpi_ops *scpi_ops;
 	struct scpi_chan *channels;
@@ -344,14 +378,20 @@ static void put_scpi_xfer(struct scpi_xfer *t, struct scpi_chan *ch)
 	mutex_unlock(&ch->xfers_lock);
 }
 
-static int scpi_send_message(u8 cmd, void *tx_buf, unsigned int tx_len,
+static int scpi_send_message(u8 idx, void *tx_buf, unsigned int tx_len,
 			     void *rx_buf, unsigned int rx_len)
 {
 	int ret;
 	u8 chan;
+	u8 cmd;
 	struct scpi_xfer *msg;
 	struct scpi_chan *scpi_chan;
 
+	if (scpi_info->commands[idx] < 0)
+		return -EOPNOTSUPP;
+
+	cmd = scpi_info->commands[idx];
+
 	chan = atomic_inc_return(&scpi_info->next_chan) % scpi_info->num_chans;
 	scpi_chan = scpi_info->channels + chan;
 
@@ -397,7 +437,7 @@ static u32 scpi_get_version(void)
 	struct clk_get_info clk;
 	__le16 le_clk_id = cpu_to_le16(clk_id);
 
-	ret = scpi_send_message(SCPI_CMD_GET_CLOCK_INFO, &le_clk_id,
+	ret = scpi_send_message(CMD_GET_CLOCK_INFO, &le_clk_id,
 				sizeof(le_clk_id), &clk, sizeof(clk));
 	if (!ret) {
 		*min = le32_to_cpu(clk.min_rate);
@@ -412,8 +452,9 @@ static unsigned long scpi_clk_get_val(u16 clk_id)
 	struct clk_get_value clk;
 	__le16 le_clk_id = cpu_to_le16(clk_id);
 
-	ret = scpi_send_message(SCPI_CMD_GET_CLOCK_VALUE, &le_clk_id,
+	ret = scpi_send_message(CMD_GET_CLOCK_VALUE, &le_clk_id,
 				sizeof(le_clk_id), &clk, sizeof(clk));
+
 	return ret ? ret : le32_to_cpu(clk.rate);
 }
 
@@ -425,7 +466,7 @@ static int scpi_clk_set_val(u16 clk_id, unsigned long rate)
 		.rate = cpu_to_le32(rate)
 	};
 
-	return scpi_send_message(SCPI_CMD_SET_CLOCK_VALUE, &clk, sizeof(clk),
+	return scpi_send_message(CMD_SET_CLOCK_VALUE, &clk, sizeof(clk),
 				 &stat, sizeof(stat));
 }
 
@@ -434,8 +475,9 @@ static int scpi_dvfs_get_idx(u8 domain)
 	int ret;
 	u8 dvfs_idx;
 
-	ret = scpi_send_message(SCPI_CMD_GET_DVFS, &domain, sizeof(domain),
+	ret = scpi_send_message(CMD_GET_DVFS, &domain, sizeof(domain),
 				&dvfs_idx, sizeof(dvfs_idx));
+
 	return ret ? ret : dvfs_idx;
 }
 
@@ -444,7 +486,7 @@ static int scpi_dvfs_set_idx(u8 domain, u8 index)
 	int stat;
 	struct dvfs_set dvfs = {domain, index};
 
-	return scpi_send_message(SCPI_CMD_SET_DVFS, &dvfs, sizeof(dvfs),
+	return scpi_send_message(CMD_SET_DVFS, &dvfs, sizeof(dvfs),
 				 &stat, sizeof(stat));
 }
 
@@ -468,9 +510,8 @@ static struct scpi_dvfs_info *scpi_dvfs_get_info(u8 domain)
 	if (scpi_info->dvfs[domain])	/* data already populated */
 		return scpi_info->dvfs[domain];
 
-	ret = scpi_send_message(SCPI_CMD_GET_DVFS_INFO, &domain, sizeof(domain),
+	ret = scpi_send_message(CMD_GET_DVFS_INFO, &domain, sizeof(domain),
 				&buf, sizeof(buf));
-
 	if (ret)
 		return ERR_PTR(ret);
 
@@ -503,7 +544,7 @@ static int scpi_sensor_get_capability(u16 *sensors)
 	struct sensor_capabilities cap_buf;
 	int ret;
 
-	ret = scpi_send_message(SCPI_CMD_SENSOR_CAPABILITIES, NULL, 0, &cap_buf,
+	ret = scpi_send_message(CMD_SENSOR_CAPABILITIES, NULL, 0, &cap_buf,
 				sizeof(cap_buf));
 	if (!ret)
 		*sensors = le16_to_cpu(cap_buf.sensors);
@@ -517,7 +558,7 @@ static int scpi_sensor_get_info(u16 sensor_id, struct scpi_sensor_info *info)
 	struct _scpi_sensor_info _info;
 	int ret;
 
-	ret = scpi_send_message(SCPI_CMD_SENSOR_INFO, &id, sizeof(id),
+	ret = scpi_send_message(CMD_SENSOR_INFO, &id, sizeof(id),
 				&_info, sizeof(_info));
 	if (!ret) {
 		memcpy(info, &_info, sizeof(*info));
@@ -533,7 +574,7 @@ static int scpi_sensor_get_value(u16 sensor, u64 *val)
 	struct sensor_value buf;
 	int ret;
 
-	ret = scpi_send_message(SCPI_CMD_SENSOR_VALUE, &id, sizeof(id),
+	ret = scpi_send_message(CMD_SENSOR_VALUE, &id, sizeof(id),
 				&buf, sizeof(buf));
 	if (!ret)
 		*val = (u64)le32_to_cpu(buf.hi_val) << 32 |
@@ -548,7 +589,7 @@ static int scpi_device_get_power_state(u16 dev_id)
 	u8 pstate;
 	__le16 id = cpu_to_le16(dev_id);
 
-	ret = scpi_send_message(SCPI_CMD_GET_DEVICE_PWR_STATE, &id,
+	ret = scpi_send_message(CMD_GET_DEVICE_PWR_STATE, &id,
 				sizeof(id), &pstate, sizeof(pstate));
 	return ret ? ret : pstate;
 }
@@ -561,7 +602,7 @@ static int scpi_device_set_power_state(u16 dev_id, u8 pstate)
 		.pstate = pstate,
 	};
 
-	return scpi_send_message(SCPI_CMD_SET_DEVICE_PWR_STATE, &dev_set,
+	return scpi_send_message(CMD_SET_DEVICE_PWR_STATE, &dev_set,
 				 sizeof(dev_set), &stat, sizeof(stat));
 }
 
@@ -591,7 +632,7 @@ static int scpi_init_versions(struct scpi_drvinfo *info)
 	int ret;
 	struct scp_capabilities caps;
 
-	ret = scpi_send_message(SCPI_CMD_SCPI_CAPABILITIES, NULL, 0,
+	ret = scpi_send_message(CMD_SCPI_CAPABILITIES, NULL, 0,
 				&caps, sizeof(caps));
 	if (!ret) {
 		info->protocol_version = le32_to_cpu(caps.protocol_version);
@@ -755,6 +796,8 @@ static int scpi_probe(struct platform_device *pdev)
 
 	scpi_info->channels = scpi_chan;
 	scpi_info->num_chans = count;
+	scpi_info->commands = scpi_std_commands;
+
 	platform_set_drvdata(pdev, scpi_info);
 
 	ret = scpi_init_versions(scpi_info);
-- 
1.9.1

^ permalink raw reply related

* [PATCH 0/8] firmware: arm_scpi: add support for legacy SCPI protocol
From: Sudeep Holla @ 2016-11-03  4:52 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Sudeep Holla, Olof Johansson,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi,

This is minor rework of the series[1] from Neil Armstrong's to support
legacy SCPI protocol to make DT bindings more generic and move out all
the platform specific bindings out of the generic binding document.

--
Regards,
Sudeep

[1] http://www.spinics.net/lists/arm-kernel/msg534999.html

Neil Armstrong (4):
  firmware: arm_scpi: increase MAX_DVFS_OPPS to 16 entries
  firmware: arm_scpi: add alternative legacy structures, functions and
    macros
  firmware: arm_scpi: allow firmware with get_capabilities not
    implemented
  Documentation: bindings: Add support for Amlogic GXBB SCPI protocol

Sudeep Holla (4):
  firmware: arm_scpi: add command indirection to support legacy commands
  Documentation: bindings: decouple juno specific details from generic
    binding
  Documentation: bindings: add compatible specific to legacy SCPI
    protocol
  firmware: arm_scpi: add support for legacy SCPI compatible

 .../devicetree/bindings/arm/amlogic,scpi.txt       |  20 ++
 Documentation/devicetree/bindings/arm/arm,scpi.txt |  24 +-
 .../devicetree/bindings/arm/juno,scpi.txt          |  26 ++
 drivers/firmware/arm_scpi.c                        | 276 ++++++++++++++++++---
 4 files changed, 300 insertions(+), 46 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/amlogic,scpi.txt
 create mode 100644 Documentation/devicetree/bindings/arm/juno,scpi.txt

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

* Re: [PATCH v4 6/8] dt-bindings: Add support for Amlogic GXBB SCPI Interface
From: Sudeep Holla @ 2016-11-03  4:37 UTC (permalink / raw)
  To: Rob Herring
  Cc: Sudeep Holla, Olof Johansson, Neil Armstrong,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-amlogic, Kevin Hilman, Heiko Stübner, Caesar Wang,
	Frank Wang
In-Reply-To: <CAL_JsqJWABnX1-6SHZL8SOSt4OSPbTyXrCqTYCkXgxuO87gN-A@mail.gmail.com>



On 02/11/16 21:51, Rob Herring wrote:
> On Wed, Nov 2, 2016 at 5:20 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> On Sat, Oct 29, 2016 at 11:39:05AM -0700, Olof Johansson wrote:
>>> Hi,
>>>
>>>
>>> On Wed, Oct 5, 2016 at 12:33 AM, Neil Armstrong <narmstrong@baylibre.com> wrote:
>>>> Acked-by: Rob Herring <robh@kernel.org>
>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/arm/arm,scpi.txt | 8 +++++---
>>>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>>>> index faa4b44..04bc171 100644
>>>> --- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
>>>> +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>>>> @@ -7,7 +7,7 @@ by Linux to initiate various system control and power operations.
>>>>
>>>>  Required properties:
>>>>
>>>> -- compatible : should be "arm,scpi"
>>>> +- compatible : should be "arm,scpi" or "amlogic,meson-gxbb-scpi"
>>>
>>> This doesn't seem right to document here. If anything you might want
>>> to have a table of more-specific-compatibles for specific
>>> implementations, but "arm,scpi" should still be the compatible of the
>>> node (just not the most specific one).
>>>
>>
>> I completely agree with you and I was pushing for a generic "arm,legacy-scpi"
>> compatible until this binding was acked by Rob.
>
> Just because I ack something, that doesn't mean don't review or
> comment on it further.
>

Sorry I didn't mean to say that. I was initially pushing for the generic
binding and since it was reworked many times already, I didn't want to
postpone any further just for sake of that. I completely understand that
component maintainers have to review the bindings too. So clearly it was
my mistake.

>>
>> Anyways, I will add the generic compatible and post the changes.
>>
>>> Also, documenting it here indiciates that non-amlogic implementations
>>> can/should use that compatible, which is misleading.
>>>
>>
>> Agreed, it's better to keep them out of this generic binding document.
>>
>>>>  - mboxes: List of phandle and mailbox channel specifiers
>>>>           All the channels reserved by remote SCP firmware for use by
>>>>           SCPI message protocol should be specified in any order
>>>> @@ -60,7 +60,8 @@ A small area of SRAM is reserved for SCPI communication between application
>>>>  processors and SCP.
>>>>
>>>>  Required properties:
>>>> -- compatible : should be "arm,juno-sram-ns" for Non-secure SRAM on Juno
>>>> +- compatible : should be "arm,juno-sram-ns" for Non-secure SRAM on Juno,
>>>> +               or "amlogic,meson-gxbb-sram" for Amlogic GXBB SoC.
>>>
>>> Maybe you'd be better of with a meson-specific document that refers to
>>> these but with different compatible values.
>>>
>>> Come to think of it, the Juno-specific one maybe shouldn't be in
>>> arm,scpi at all, since that adds confusion here.
>>>
>>> It's somewhat confusing that ARM is both a platform, architecture and
>>> in some cases implementer of specific systems. :)
>>>
>>
>> Sorry for that, I will move all juno specific references in the binding
>> out of this document(except the examples, which I assume should be fine)
>>
>>>>  The rest of the properties should follow the generic mmio-sram description
>>>>  found in ../../sram/sram.txt
>>>> @@ -70,7 +71,8 @@ Each sub-node represents the reserved area for SCPI.
>>>>  Required sub-node properties:
>>>>  - reg : The base offset and size of the reserved area with the SRAM
>>>>  - compatible : should be "arm,juno-scp-shmem" for Non-secure SRAM based
>>>> -              shared memory on Juno platforms
>>>> +              shared memory on Juno platforms or
>>>> +              "amlogic,meson-gxbb-scp-shmem" for Amlogic GXBB SoC.
>>>
>>> Same here. It won't scale if all vendors are expected to add an entry here.
>>>
>>
>> I will rework the patches to address the concerns as I too did share same
>> concern.
>
> Guess I was optimistic that *every* platform wouldn't be different in
> some way. I should know better by now...

:)

-- 
Regards,
Sudeep

^ permalink raw reply

* Re: [PATCH v4 6/8] dt-bindings: Add support for Amlogic GXBB SCPI Interface
From: Rob Herring @ 2016-11-03  3:51 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Olof Johansson, Neil Armstrong,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-amlogic, Kevin Hilman, Heiko Stübner, Caesar Wang,
	Frank Wang
In-Reply-To: <20161102222050.GA32189@e106835-lin.cambridge.arm.com>

On Wed, Nov 2, 2016 at 5:20 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> On Sat, Oct 29, 2016 at 11:39:05AM -0700, Olof Johansson wrote:
>> Hi,
>>
>>
>> On Wed, Oct 5, 2016 at 12:33 AM, Neil Armstrong <narmstrong@baylibre.com> wrote:
>> > Acked-by: Rob Herring <robh@kernel.org>
>> > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> > ---
>> >  Documentation/devicetree/bindings/arm/arm,scpi.txt | 8 +++++---
>> >  1 file changed, 5 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>> > index faa4b44..04bc171 100644
>> > --- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
>> > +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>> > @@ -7,7 +7,7 @@ by Linux to initiate various system control and power operations.
>> >
>> >  Required properties:
>> >
>> > -- compatible : should be "arm,scpi"
>> > +- compatible : should be "arm,scpi" or "amlogic,meson-gxbb-scpi"
>>
>> This doesn't seem right to document here. If anything you might want
>> to have a table of more-specific-compatibles for specific
>> implementations, but "arm,scpi" should still be the compatible of the
>> node (just not the most specific one).
>>
>
> I completely agree with you and I was pushing for a generic "arm,legacy-scpi"
> compatible until this binding was acked by Rob.

Just because I ack something, that doesn't mean don't review or
comment on it further.

>
> Anyways, I will add the generic compatible and post the changes.
>
>> Also, documenting it here indiciates that non-amlogic implementations
>> can/should use that compatible, which is misleading.
>>
>
> Agreed, it's better to keep them out of this generic binding document.
>
>> >  - mboxes: List of phandle and mailbox channel specifiers
>> >           All the channels reserved by remote SCP firmware for use by
>> >           SCPI message protocol should be specified in any order
>> > @@ -60,7 +60,8 @@ A small area of SRAM is reserved for SCPI communication between application
>> >  processors and SCP.
>> >
>> >  Required properties:
>> > -- compatible : should be "arm,juno-sram-ns" for Non-secure SRAM on Juno
>> > +- compatible : should be "arm,juno-sram-ns" for Non-secure SRAM on Juno,
>> > +               or "amlogic,meson-gxbb-sram" for Amlogic GXBB SoC.
>>
>> Maybe you'd be better of with a meson-specific document that refers to
>> these but with different compatible values.
>>
>> Come to think of it, the Juno-specific one maybe shouldn't be in
>> arm,scpi at all, since that adds confusion here.
>>
>> It's somewhat confusing that ARM is both a platform, architecture and
>> in some cases implementer of specific systems. :)
>>
>
> Sorry for that, I will move all juno specific references in the binding
> out of this document(except the examples, which I assume should be fine)
>
>> >  The rest of the properties should follow the generic mmio-sram description
>> >  found in ../../sram/sram.txt
>> > @@ -70,7 +71,8 @@ Each sub-node represents the reserved area for SCPI.
>> >  Required sub-node properties:
>> >  - reg : The base offset and size of the reserved area with the SRAM
>> >  - compatible : should be "arm,juno-scp-shmem" for Non-secure SRAM based
>> > -              shared memory on Juno platforms
>> > +              shared memory on Juno platforms or
>> > +              "amlogic,meson-gxbb-scp-shmem" for Amlogic GXBB SoC.
>>
>> Same here. It won't scale if all vendors are expected to add an entry here.
>>
>
> I will rework the patches to address the concerns as I too did share same
> concern.

Guess I was optimistic that *every* platform wouldn't be different in
some way. I should know better by now...

Rob

^ permalink raw reply

* Re: [PATCH] of: base: Export symbol of __of_find_all_nodes()
From: Rob Herring @ 2016-11-03  3:39 UTC (permalink / raw)
  To: Laxman Dewangan
  Cc: Frank Rowand, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1477654092-12406-1-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

On Fri, Oct 28, 2016 at 6:28 AM, Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> The function __of_find_all_nodes() is used in the public header
> linux/of.h and if any driver developed using this API then
> it reports error as unknown symbol if compiled as module.
>
> Export this APIs using EXPORT_SYMBOL() so that it can be used
> from driver compiled as module.

What driver needs this? This isn't really a function I'd expect drivers to use.

Rob
--
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 v2] of, numa: Return NUMA_NO_NODE from disable of_node_to_nid() if nid not possible.
From: Rob Herring @ 2016-11-03  3:37 UTC (permalink / raw)
  To: David Daney
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Frank Rowand, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Will Deacon, Catalin Marinas, Robert Richter, Hanjun Guo,
	Ganapatrao Kulkarni, Gilbert Netzer, David Daney
In-Reply-To: <1477689302-32671-1-git-send-email-ddaney.cavm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Fri, Oct 28, 2016 at 4:15 PM, David Daney <ddaney.cavm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>
> On arm64 NUMA kernels we can pass "numa=off" on the command line to
> disable NUMA.  A side effect of this is that kmalloc_node() calls to
> non-zero nodes will crash the system with an OOPS:
>
> [    0.000000] ITS@0x0000901000020000: allocated 2097152 Devices @10002000000 (flat, esz 8, psz 64K, shr 1)
> [    0.000000] Unable to handle kernel NULL pointer dereference at virtual address 00001680
> [    0.000000] pgd = fffffc0009470000
> [    0.000000] [00001680] *pgd=0000010ffff90003, *pud=0000010ffff90003, *pmd=0000010ffff90003, *pte=0000000000000000
> [    0.000000] Internal error: Oops: 96000006 [#1] SMP
> .
> .
> .
> [    0.000000] [<fffffc00081c8950>] __alloc_pages_nodemask+0xa4/0xe68
> [    0.000000] [<fffffc000821fa70>] new_slab+0xd0/0x564
> [    0.000000] [<fffffc0008221e24>] ___slab_alloc+0x2e4/0x514
> [    0.000000] [<fffffc0008239498>] __slab_alloc+0x48/0x58
> [    0.000000] [<fffffc0008222c20>] __kmalloc_node+0xd0/0x2dc
> [    0.000000] [<fffffc0008115374>] __irq_domain_add+0x7c/0x164
> [    0.000000] [<fffffc0008b461dc>] its_probe+0x784/0x81c
> [    0.000000] [<fffffc0008b462bc>] its_init+0x48/0x1b0
> [    0.000000] [<fffffc0008b4543c>] gic_init_bases+0x228/0x360
> [    0.000000] [<fffffc0008b456bc>] gic_of_init+0x148/0x1cc
> [    0.000000] [<fffffc0008b5aec8>] of_irq_init+0x184/0x298
> [    0.000000] [<fffffc0008b43f9c>] irqchip_init+0x14/0x38
> [    0.000000] [<fffffc0008b12d60>] init_IRQ+0xc/0x30
> [    0.000000] [<fffffc0008b10a3c>] start_kernel+0x240/0x3b8
> [    0.000000] [<fffffc0008b101c4>] __primary_switched+0x30/0x6c
> [    0.000000] Code: 912ec2a0 b9403809 0a0902fb 37b007db (f9400300)
> .
> .
> .
>
> This is caused by code like this in kernel/irq/irqdomain.c
>
>     domain = kzalloc_node(sizeof(*domain) + (sizeof(unsigned int) * size),
>                   GFP_KERNEL, of_node_to_nid(of_node));
>
> When NUMA is disabled, the concept of a node is really undefined, so
> of_node_to_nid() should unconditionally return NUMA_NO_NODE.
>
> Fix by returning NUMA_NO_NODE when the nid is not in the set of
> possible nodes.
>
> Reported-by: Gilbert Netzer <noname-QVTQ4TwcpgDLoDKTGw+V6w@public.gmane.org>
> Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

Does this need to go in 4.9? stable? If so, since what kernel version?

Rob
--
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 0/4] PXA cpufreq conversion to clock API
From: Viresh Kumar @ 2016-11-03  2:16 UTC (permalink / raw)
  To: Robert Jarzmik
  Cc: Rafael J Wysocki, Daniel Mack, Haojian Zhuang, Rob Herring,
	Mark Rutland, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87d1idzh56.fsf-4ty26DBLk+jEm7gnYqmdkQ@public.gmane.org>

On 02-11-16, 22:49, Robert Jarzmik wrote:
> Would you or Rafael take patch 1/4 "cpufreq: pxa: use generic platdev driver for
> device-tree" through your tree please ?

Rafael is the one who applies the patches.

-- 
viresh
--
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 2/3] Documentation: dt: add bindings for ti-cpufreq
From: Viresh Kumar @ 2016-11-03  2:14 UTC (permalink / raw)
  To: Dave Gerlach
  Cc: Rob Herring, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rafael J . Wysocki,
	Tony Lindgren, Nishanth Menon
In-Reply-To: <9a185460-a241-4876-2915-6975c57527ca-l0cyMroinI0@public.gmane.org>

On 02-11-16, 11:03, Dave Gerlach wrote:
> >>+cpu0_opp_table: opp_table0 {
> >>+	compatible = "operating-points-v2-ti-am3352-cpu";
> >>+	ti,syscon-efuse = <&scm_conf 0x7fc 0x1fff 0>;
> >>+	ti,syscon-rev = <&scm_conf 0x600>;
> >>+
> >>+	/*
> >>+	 * The three following nodes are marked with opp-suspend
> >>+	 * because they can not be enabled simultaneously on a
> >>+	 * single SoC.
> >>+	 */

I missed reading this comment :(

>>+	opp50@300000000 {
> >>+		opp-hz = /bits/ 64 <300000000>;
> >>+		opp-microvolt = <950000 931000 969000>;
> >>+		opp-supported-hw = <0x06 0x0010>;
> >>+		opp-suspend;
> >>+	};
> >>+
> >>+	opp100@275000000 {
> >>+		opp-hz = /bits/ 64 <275000000>;
> >>+		opp-microvolt = <1100000 1078000 1122000>;
> >>+		opp-supported-hw = <0x01 0x00FF>;
> >>+		opp-suspend;
> >>+	};
> >>+
> >>+	opp100@300000000 {
> >>+		opp-hz = /bits/ 64 <300000000>;
> >>+		opp-microvolt = <1100000 1078000 1122000>;
> >>+		opp-supported-hw = <0x06 0x0020>;
> >>+		opp-suspend;
> >
> >Only one OPP in the table can be marked as suspend OPP.
> >
> 
> Does that still apply when opp-supported-hw is involved? Based on the
> comment at the start of the table, those OPPs are all mutually exclusive and
> will not ever be enabled on the same piece of silicon, they represent the
> lowest OPP for each of three different supported-hw configurations.

You are right, its fine.

-- 
viresh
--
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 2/2] arm64: dts: rockchip: add usb2-phy otg-port support for rk3399
From: William Wu @ 2016-11-03  2:06 UTC (permalink / raw)
  To: kishon, heiko
  Cc: linux-kernel, linux-arm-kernel, linux-rockchip, devicetree,
	robh+dt, frank.wang, huangtao, dianders, briannorris, groeck,
	wulf
In-Reply-To: <1478138774-3254-1-git-send-email-wulf@rock-chips.com>

Add otg-port nodes for both u2phy0 and u2phy1. The otg-port can
be used for USB2.0 part of USB3.0 OTG controller.

Signed-off-by: William Wu <wulf@rock-chips.com>
---
Changes in v2:
- None

 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index b65c193..ea2df51 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1095,6 +1095,17 @@
 			clock-output-names = "clk_usbphy0_480m";
 			status = "disabled";
 
+			u2phy0_otg: otg-port {
+				#phy-cells = <0>;
+				interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH 0>,
+					     <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH 0>,
+					     <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH 0>;
+				interrupt-names = "otg-bvalid", "otg-id",
+						  "linestate";
+				status = "disabled";
+			};
+
+
 			u2phy0_host: host-port {
 				#phy-cells = <0>;
 				interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH 0>;
@@ -1112,6 +1123,16 @@
 			clock-output-names = "clk_usbphy1_480m";
 			status = "disabled";
 
+			u2phy1_otg: otg-port {
+				#phy-cells = <0>;
+				interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH 0>,
+					     <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH 0>,
+					     <GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH 0>;
+				interrupt-names = "otg-bvalid", "otg-id",
+						  "linestate";
+				status = "disabled";
+			};
+
 			u2phy1_host: host-port {
 				#phy-cells = <0>;
 				interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH 0>;
-- 
2.0.0

^ permalink raw reply related

* [PATCH v2 1/2] phy: rockchip-inno-usb2: support otg-port for rk3399
From: William Wu @ 2016-11-03  2:06 UTC (permalink / raw)
  To: kishon-l0cyMroinI0, heiko-4mtYJXux2i+zQB+pC5nmwQ
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	frank.wang-TNX95d0MmH7DzftRWevZcw,
	huangtao-TNX95d0MmH7DzftRWevZcw, dianders-hpIqsD4AKlfQT0dZR+AlfA,
	briannorris-hpIqsD4AKlfQT0dZR+AlfA, groeck-hpIqsD4AKlfQT0dZR+AlfA,
	wulf-TNX95d0MmH7DzftRWevZcw
In-Reply-To: <1478138774-3254-1-git-send-email-wulf-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

The rk3399 SoC USB2 PHY is comprised of one Host port and
one OTG port. And OTG port is for USB2.0 part of USB3.0 OTG
controller, as a part to construct a fully feature Type-C
subsystem.

With this patch, we can support OTG port with the following
functions:
- Support BC1.2 charger detect, and use extcon notifier to
  send USB charger types to power driver.
- Support PHY suspend for power management.
- Support OTG Host only mode.

Also, correct 480MHz output clock stable time. We found that
the system crashed due to 480MHz output clock of USB2 PHY was
unstable after clock had been enabled by gpu module.

Theoretically, 1 millisecond is a critical value for 480 output
clock stable time, so we try changing the delay time to 1.2
millisecond to avoid this issue.

Signed-off-by: William Wu <wulf-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---
Changes in v2:
- remove wakelock

 drivers/phy/phy-rockchip-inno-usb2.c | 593 +++++++++++++++++++++++++++++++++--
 1 file changed, 562 insertions(+), 31 deletions(-)

diff --git a/drivers/phy/phy-rockchip-inno-usb2.c b/drivers/phy/phy-rockchip-inno-usb2.c
index ac20310..8f2d2b6 100644
--- a/drivers/phy/phy-rockchip-inno-usb2.c
+++ b/drivers/phy/phy-rockchip-inno-usb2.c
@@ -17,6 +17,7 @@
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/delay.h>
+#include <linux/extcon.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/gpio/consumer.h>
@@ -30,11 +31,15 @@
 #include <linux/of_platform.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
+#include <linux/power_supply.h>
 #include <linux/regmap.h>
 #include <linux/mfd/syscon.h>
+#include <linux/usb/of.h>
+#include <linux/usb/otg.h>
 
 #define BIT_WRITEABLE_SHIFT	16
-#define SCHEDULE_DELAY	(60 * HZ)
+#define SCHEDULE_DELAY		(60 * HZ)
+#define OTG_SCHEDULE_DELAY	(2 * HZ)
 
 enum rockchip_usb2phy_port_id {
 	USB2PHY_PORT_OTG,
@@ -49,6 +54,37 @@ enum rockchip_usb2phy_host_state {
 	PHY_STATE_FS_LS_ONLINE	= 4,
 };
 
+/**
+ * Different states involved in USB charger detection.
+ * USB_CHG_STATE_UNDEFINED	USB charger is not connected or detection
+ *				process is not yet started.
+ * USB_CHG_STATE_WAIT_FOR_DCD	Waiting for Data pins contact.
+ * USB_CHG_STATE_DCD_DONE	Data pin contact is detected.
+ * USB_CHG_STATE_PRIMARY_DONE	Primary detection is completed (Detects
+ *				between SDP and DCP/CDP).
+ * USB_CHG_STATE_SECONDARY_DONE	Secondary detection is completed (Detects
+ *				between DCP and CDP).
+ * USB_CHG_STATE_DETECTED	USB charger type is determined.
+ */
+enum usb_chg_state {
+	USB_CHG_STATE_UNDEFINED = 0,
+	USB_CHG_STATE_WAIT_FOR_DCD,
+	USB_CHG_STATE_DCD_DONE,
+	USB_CHG_STATE_PRIMARY_DONE,
+	USB_CHG_STATE_SECONDARY_DONE,
+	USB_CHG_STATE_DETECTED,
+};
+
+static const unsigned int rockchip_usb2phy_extcon_cable[] = {
+	EXTCON_USB,
+	EXTCON_USB_HOST,
+	EXTCON_CHG_USB_SDP,
+	EXTCON_CHG_USB_CDP,
+	EXTCON_CHG_USB_DCP,
+	EXTCON_CHG_USB_SLOW,
+	EXTCON_NONE,
+};
+
 struct usb2phy_reg {
 	unsigned int	offset;
 	unsigned int	bitend;
@@ -58,19 +94,55 @@ struct usb2phy_reg {
 };
 
 /**
+ * struct rockchip_chg_det_reg: usb charger detect registers
+ * @cp_det: charging port detected successfully.
+ * @dcp_det: dedicated charging port detected successfully.
+ * @dp_det: assert data pin connect successfully.
+ * @idm_sink_en: open dm sink curren.
+ * @idp_sink_en: open dp sink current.
+ * @idp_src_en: open dm source current.
+ * @rdm_pdwn_en: open dm pull down resistor.
+ * @vdm_src_en: open dm voltage source.
+ * @vdp_src_en: open dp voltage source.
+ * @opmode: utmi operational mode.
+ */
+struct rockchip_chg_det_reg {
+	struct usb2phy_reg	cp_det;
+	struct usb2phy_reg	dcp_det;
+	struct usb2phy_reg	dp_det;
+	struct usb2phy_reg	idm_sink_en;
+	struct usb2phy_reg	idp_sink_en;
+	struct usb2phy_reg	idp_src_en;
+	struct usb2phy_reg	rdm_pdwn_en;
+	struct usb2phy_reg	vdm_src_en;
+	struct usb2phy_reg	vdp_src_en;
+	struct usb2phy_reg	opmode;
+};
+
+/**
  * struct rockchip_usb2phy_port_cfg: usb-phy port configuration.
  * @phy_sus: phy suspend register.
+ * @bvalid_det_en: vbus valid rise detection enable register.
+ * @bvalid_det_st: vbus valid rise detection status register.
+ * @bvalid_det_clr: vbus valid rise detection clear register.
  * @ls_det_en: linestate detection enable register.
  * @ls_det_st: linestate detection state register.
  * @ls_det_clr: linestate detection clear register.
+ * @utmi_avalid: utmi vbus avalid status register.
+ * @utmi_bvalid: utmi vbus bvalid status register.
  * @utmi_ls: utmi linestate state register.
  * @utmi_hstdet: utmi host disconnect register.
  */
 struct rockchip_usb2phy_port_cfg {
 	struct usb2phy_reg	phy_sus;
+	struct usb2phy_reg	bvalid_det_en;
+	struct usb2phy_reg	bvalid_det_st;
+	struct usb2phy_reg	bvalid_det_clr;
 	struct usb2phy_reg	ls_det_en;
 	struct usb2phy_reg	ls_det_st;
 	struct usb2phy_reg	ls_det_clr;
+	struct usb2phy_reg	utmi_avalid;
+	struct usb2phy_reg	utmi_bvalid;
 	struct usb2phy_reg	utmi_ls;
 	struct usb2phy_reg	utmi_hstdet;
 };
@@ -80,31 +152,51 @@ struct rockchip_usb2phy_port_cfg {
  * @reg: the address offset of grf for usb-phy config.
  * @num_ports: specify how many ports that the phy has.
  * @clkout_ctl: keep on/turn off output clk of phy.
+ * @chg_det: charger detection registers.
  */
 struct rockchip_usb2phy_cfg {
 	unsigned int	reg;
 	unsigned int	num_ports;
 	struct usb2phy_reg	clkout_ctl;
 	const struct rockchip_usb2phy_port_cfg	port_cfgs[USB2PHY_NUM_PORTS];
+	const struct rockchip_chg_det_reg	chg_det;
 };
 
 /**
  * struct rockchip_usb2phy_port: usb-phy port data.
  * @port_id: flag for otg port or host port.
  * @suspended: phy suspended flag.
+ * @utmi_avalid: utmi avalid status usage flag.
+ *	true	- use avalid to get vbus status
+ *	flase	- use bvalid to get vbus status
+ * @vbus_attached: otg device vbus status.
+ * @bvalid_irq: IRQ number assigned for vbus valid rise detection.
  * @ls_irq: IRQ number assigned for linestate detection.
  * @mutex: for register updating in sm_work.
- * @sm_work: OTG state machine work.
+ * @chg_work: charge detect work.
+ * @otg_sm_work: OTG state machine work.
+ * @sm_work: HOST state machine work.
  * @phy_cfg: port register configuration, assigned by driver data.
+ * @event_nb: hold event notification callback.
+ * @state: define OTG enumeration states before device reset.
+ * @mode: the dr_mode of the controller.
  */
 struct rockchip_usb2phy_port {
 	struct phy	*phy;
 	unsigned int	port_id;
 	bool		suspended;
+	bool		utmi_avalid;
+	bool		vbus_attached;
+	int		bvalid_irq;
 	int		ls_irq;
 	struct mutex	mutex;
+	struct		delayed_work chg_work;
+	struct		delayed_work otg_sm_work;
 	struct		delayed_work sm_work;
 	const struct	rockchip_usb2phy_port_cfg *port_cfg;
+	struct notifier_block	event_nb;
+	enum usb_otg_state	state;
+	enum usb_dr_mode	mode;
 };
 
 /**
@@ -113,6 +205,11 @@ struct rockchip_usb2phy_port {
  * @clk: clock struct of phy input clk.
  * @clk480m: clock struct of phy output clk.
  * @clk_hw: clock struct of phy output clk management.
+ * @chg_state: states involved in USB charger detection.
+ * @chg_type: USB charger types.
+ * @dcd_retries: The retry count used to track Data contact
+ *		 detection process.
+ * @edev: extcon device for notification registration
  * @phy_cfg: phy register configuration, assigned by driver data.
  * @ports: phy port instance.
  */
@@ -122,6 +219,10 @@ struct rockchip_usb2phy {
 	struct clk	*clk;
 	struct clk	*clk480m;
 	struct clk_hw	clk480m_hw;
+	enum usb_chg_state	chg_state;
+	enum power_supply_type	chg_type;
+	u8			dcd_retries;
+	struct extcon_dev	*edev;
 	const struct rockchip_usb2phy_cfg	*phy_cfg;
 	struct rockchip_usb2phy_port	ports[USB2PHY_NUM_PORTS];
 };
@@ -166,7 +267,7 @@ static int rockchip_usb2phy_clk480m_enable(struct clk_hw *hw)
 			return ret;
 
 		/* waitting for the clk become stable */
-		mdelay(1);
+		udelay(1200);
 	}
 
 	return 0;
@@ -263,33 +364,84 @@ rockchip_usb2phy_clk480m_register(struct rockchip_usb2phy *rphy)
 	return ret;
 }
 
-static int rockchip_usb2phy_init(struct phy *phy)
+static int rockchip_usb2phy_extcon_register(struct rockchip_usb2phy *rphy)
 {
-	struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy);
-	struct rockchip_usb2phy *rphy = dev_get_drvdata(phy->dev.parent);
 	int ret;
+	struct device_node *node = rphy->dev->of_node;
+	struct extcon_dev *edev;
+
+	if (of_property_read_bool(node, "extcon")) {
+		edev = extcon_get_edev_by_phandle(rphy->dev, 0);
+		if (IS_ERR(edev)) {
+			if (PTR_ERR(edev) != -EPROBE_DEFER)
+				dev_err(rphy->dev, "Invalid or missing extcon\n");
+			return PTR_ERR(edev);
+		}
+	} else {
+		/* Initialize extcon device */
+		edev = devm_extcon_dev_allocate(rphy->dev,
+						rockchip_usb2phy_extcon_cable);
 
-	if (rport->port_id == USB2PHY_PORT_HOST) {
-		/* clear linestate and enable linestate detect irq */
-		mutex_lock(&rport->mutex);
+		if (IS_ERR(edev))
+			return -ENOMEM;
 
-		ret = property_enable(rphy, &rport->port_cfg->ls_det_clr, true);
+		ret = devm_extcon_dev_register(rphy->dev, edev);
 		if (ret) {
-			mutex_unlock(&rport->mutex);
+			dev_err(rphy->dev, "failed to register extcon device\n");
 			return ret;
 		}
+	}
 
-		ret = property_enable(rphy, &rport->port_cfg->ls_det_en, true);
-		if (ret) {
-			mutex_unlock(&rport->mutex);
-			return ret;
+	rphy->edev = edev;
+
+	return 0;
+}
+
+static int rockchip_usb2phy_init(struct phy *phy)
+{
+	struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy);
+	struct rockchip_usb2phy *rphy = dev_get_drvdata(phy->dev.parent);
+	int ret = 0;
+
+	mutex_lock(&rport->mutex);
+
+	if (rport->port_id == USB2PHY_PORT_OTG) {
+		if (rport->mode != USB_DR_MODE_HOST) {
+			/* clear bvalid status and enable bvalid detect irq */
+			ret = property_enable(rphy,
+					      &rport->port_cfg->bvalid_det_clr,
+					      true);
+			if (ret)
+				goto out;
+
+			ret = property_enable(rphy,
+					      &rport->port_cfg->bvalid_det_en,
+					      true);
+			if (ret)
+				goto out;
+
+			schedule_delayed_work(&rport->otg_sm_work,
+					      OTG_SCHEDULE_DELAY);
+		} else {
+			/* If OTG works in host only mode, do nothing. */
+			dev_dbg(&rport->phy->dev, "mode %d\n", rport->mode);
 		}
+	} else if (rport->port_id == USB2PHY_PORT_HOST) {
+		/* clear linestate and enable linestate detect irq */
+		ret = property_enable(rphy, &rport->port_cfg->ls_det_clr, true);
+		if (ret)
+			goto out;
+
+		ret = property_enable(rphy, &rport->port_cfg->ls_det_en, true);
+		if (ret)
+			goto out;
 
-		mutex_unlock(&rport->mutex);
 		schedule_delayed_work(&rport->sm_work, SCHEDULE_DELAY);
 	}
 
-	return 0;
+out:
+	mutex_unlock(&rport->mutex);
+	return ret;
 }
 
 static int rockchip_usb2phy_power_on(struct phy *phy)
@@ -340,7 +492,11 @@ static int rockchip_usb2phy_exit(struct phy *phy)
 {
 	struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy);
 
-	if (rport->port_id == USB2PHY_PORT_HOST)
+	if (rport->port_id == USB2PHY_PORT_OTG &&
+	    rport->mode != USB_DR_MODE_HOST) {
+		cancel_delayed_work_sync(&rport->otg_sm_work);
+		cancel_delayed_work_sync(&rport->chg_work);
+	} else if (rport->port_id == USB2PHY_PORT_HOST)
 		cancel_delayed_work_sync(&rport->sm_work);
 
 	return 0;
@@ -354,6 +510,249 @@ static const struct phy_ops rockchip_usb2phy_ops = {
 	.owner		= THIS_MODULE,
 };
 
+static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
+{
+	struct rockchip_usb2phy_port *rport =
+		container_of(work, struct rockchip_usb2phy_port,
+			     otg_sm_work.work);
+	struct rockchip_usb2phy *rphy = dev_get_drvdata(rport->phy->dev.parent);
+	static unsigned int cable;
+	unsigned long delay;
+	bool vbus_attach, sch_work, notify_charger;
+
+	if (rport->utmi_avalid)
+		vbus_attach =
+			property_enabled(rphy, &rport->port_cfg->utmi_avalid);
+	else
+		vbus_attach =
+			property_enabled(rphy, &rport->port_cfg->utmi_bvalid);
+
+	sch_work = false;
+	notify_charger = false;
+	delay = OTG_SCHEDULE_DELAY;
+	dev_dbg(&rport->phy->dev, "%s otg sm work\n",
+		usb_otg_state_string(rport->state));
+
+	switch (rport->state) {
+	case OTG_STATE_UNDEFINED:
+		rport->state = OTG_STATE_B_IDLE;
+		if (!vbus_attach)
+			rockchip_usb2phy_power_off(rport->phy);
+		/* fall through */
+	case OTG_STATE_B_IDLE:
+		if (extcon_get_cable_state_(rphy->edev, EXTCON_USB_HOST) > 0) {
+			dev_dbg(&rport->phy->dev, "usb otg host connect\n");
+			rport->state = OTG_STATE_A_HOST;
+			rockchip_usb2phy_power_on(rport->phy);
+			return;
+		} else if (vbus_attach) {
+			dev_dbg(&rport->phy->dev, "vbus_attach\n");
+			switch (rphy->chg_state) {
+			case USB_CHG_STATE_UNDEFINED:
+				schedule_delayed_work(&rport->chg_work, 0);
+				return;
+			case USB_CHG_STATE_DETECTED:
+				switch (rphy->chg_type) {
+				case POWER_SUPPLY_TYPE_USB:
+					dev_dbg(&rport->phy->dev,
+						"sdp cable is connecetd\n");
+					rockchip_usb2phy_power_on(rport->phy);
+					rport->state = OTG_STATE_B_PERIPHERAL;
+					notify_charger = true;
+					sch_work = true;
+					cable = EXTCON_CHG_USB_SDP;
+					break;
+				case POWER_SUPPLY_TYPE_USB_DCP:
+					dev_dbg(&rport->phy->dev,
+						"dcp cable is connecetd\n");
+					rockchip_usb2phy_power_off(rport->phy);
+					notify_charger = true;
+					sch_work = true;
+					cable = EXTCON_CHG_USB_DCP;
+					break;
+				case POWER_SUPPLY_TYPE_USB_CDP:
+					dev_dbg(&rport->phy->dev,
+						"cdp cable is connecetd\n");
+					rockchip_usb2phy_power_on(rport->phy);
+					rport->state = OTG_STATE_B_PERIPHERAL;
+					notify_charger = true;
+					sch_work = true;
+					cable = EXTCON_CHG_USB_CDP;
+					break;
+				default:
+					break;
+				}
+				break;
+			default:
+				break;
+			}
+		} else {
+			notify_charger = true;
+			rphy->chg_state = USB_CHG_STATE_UNDEFINED;
+			rphy->chg_type = POWER_SUPPLY_TYPE_UNKNOWN;
+		}
+
+		if (rport->vbus_attached != vbus_attach) {
+			rport->vbus_attached = vbus_attach;
+
+			if (notify_charger && rphy->edev)
+				extcon_set_cable_state_(rphy->edev,
+							cable, vbus_attach);
+		}
+		break;
+	case OTG_STATE_B_PERIPHERAL:
+		if (!vbus_attach) {
+			dev_dbg(&rport->phy->dev, "usb disconnect\n");
+			rphy->chg_state = USB_CHG_STATE_UNDEFINED;
+			rphy->chg_type = POWER_SUPPLY_TYPE_UNKNOWN;
+			rport->state = OTG_STATE_B_IDLE;
+			delay = 0;
+			rockchip_usb2phy_power_off(rport->phy);
+		}
+		sch_work = true;
+		break;
+	case OTG_STATE_A_HOST:
+		if (extcon_get_cable_state_(rphy->edev, EXTCON_USB_HOST) == 0) {
+			dev_dbg(&rport->phy->dev, "usb otg host disconnect\n");
+			rport->state = OTG_STATE_B_IDLE;
+			rockchip_usb2phy_power_off(rport->phy);
+		}
+		break;
+	default:
+		break;
+	}
+
+	if (sch_work)
+		schedule_delayed_work(&rport->otg_sm_work, delay);
+}
+
+static const char *chg_to_string(enum power_supply_type chg_type)
+{
+	switch (chg_type) {
+	case POWER_SUPPLY_TYPE_USB:
+		return "USB_SDP_CHARGER";
+	case POWER_SUPPLY_TYPE_USB_DCP:
+		return "USB_DCP_CHARGER";
+	case POWER_SUPPLY_TYPE_USB_CDP:
+		return "USB_CDP_CHARGER";
+	default:
+		return "INVALID_CHARGER";
+	}
+}
+
+static void rockchip_chg_enable_dcd(struct rockchip_usb2phy *rphy,
+				    bool en)
+{
+	property_enable(rphy, &rphy->phy_cfg->chg_det.rdm_pdwn_en, en);
+	property_enable(rphy, &rphy->phy_cfg->chg_det.idp_src_en, en);
+}
+
+static void rockchip_chg_enable_primary_det(struct rockchip_usb2phy *rphy,
+					    bool en)
+{
+	property_enable(rphy, &rphy->phy_cfg->chg_det.vdp_src_en, en);
+	property_enable(rphy, &rphy->phy_cfg->chg_det.idm_sink_en, en);
+}
+
+static void rockchip_chg_enable_secondary_det(struct rockchip_usb2phy *rphy,
+					      bool en)
+{
+	property_enable(rphy, &rphy->phy_cfg->chg_det.vdm_src_en, en);
+	property_enable(rphy, &rphy->phy_cfg->chg_det.idp_sink_en, en);
+}
+
+#define CHG_DCD_POLL_TIME	(100 * HZ / 1000)
+#define CHG_DCD_MAX_RETRIES	6
+#define CHG_PRIMARY_DET_TIME	(40 * HZ / 1000)
+#define CHG_SECONDARY_DET_TIME	(40 * HZ / 1000)
+static void rockchip_chg_detect_work(struct work_struct *work)
+{
+	struct rockchip_usb2phy_port *rport =
+		container_of(work, struct rockchip_usb2phy_port, chg_work.work);
+	struct rockchip_usb2phy *rphy = dev_get_drvdata(rport->phy->dev.parent);
+	bool is_dcd, tmout, vout;
+	unsigned long delay;
+
+	dev_dbg(&rport->phy->dev, "chg detection work state = %d\n",
+		rphy->chg_state);
+	switch (rphy->chg_state) {
+	case USB_CHG_STATE_UNDEFINED:
+		if (!rport->suspended)
+			rockchip_usb2phy_power_off(rport->phy);
+		/* put the controller in non-driving mode */
+		property_enable(rphy, &rphy->phy_cfg->chg_det.opmode, false);
+		/* Start DCD processing stage 1 */
+		rockchip_chg_enable_dcd(rphy, true);
+		rphy->chg_state = USB_CHG_STATE_WAIT_FOR_DCD;
+		rphy->dcd_retries = 0;
+		delay = CHG_DCD_POLL_TIME;
+		break;
+	case USB_CHG_STATE_WAIT_FOR_DCD:
+		/* get data contact detection status */
+		is_dcd = property_enabled(rphy, &rphy->phy_cfg->chg_det.dp_det);
+		tmout = ++rphy->dcd_retries == CHG_DCD_MAX_RETRIES;
+		/* stage 2 */
+		if (is_dcd || tmout) {
+			/* stage 4 */
+			/* Turn off DCD circuitry */
+			rockchip_chg_enable_dcd(rphy, false);
+			/* Voltage Source on DP, Probe on DM */
+			rockchip_chg_enable_primary_det(rphy, true);
+			delay = CHG_PRIMARY_DET_TIME;
+			rphy->chg_state = USB_CHG_STATE_DCD_DONE;
+		} else {
+			/* stage 3 */
+			delay = CHG_DCD_POLL_TIME;
+		}
+		break;
+	case USB_CHG_STATE_DCD_DONE:
+		vout = property_enabled(rphy, &rphy->phy_cfg->chg_det.cp_det);
+		rockchip_chg_enable_primary_det(rphy, false);
+		if (vout) {
+			/* Voltage Source on DM, Probe on DP  */
+			rockchip_chg_enable_secondary_det(rphy, true);
+			delay = CHG_SECONDARY_DET_TIME;
+			rphy->chg_state = USB_CHG_STATE_PRIMARY_DONE;
+		} else {
+			if (tmout) {
+				/* floating charger found */
+				rphy->chg_type = POWER_SUPPLY_TYPE_USB_DCP;
+				rphy->chg_state = USB_CHG_STATE_DETECTED;
+				delay = 0;
+			} else {
+				rphy->chg_type = POWER_SUPPLY_TYPE_USB;
+				rphy->chg_state = USB_CHG_STATE_DETECTED;
+				delay = 0;
+			}
+		}
+		break;
+	case USB_CHG_STATE_PRIMARY_DONE:
+		vout = property_enabled(rphy, &rphy->phy_cfg->chg_det.dcp_det);
+		/* Turn off voltage source */
+		rockchip_chg_enable_secondary_det(rphy, false);
+		if (vout)
+			rphy->chg_type = POWER_SUPPLY_TYPE_USB_DCP;
+		else
+			rphy->chg_type = POWER_SUPPLY_TYPE_USB_CDP;
+		/* fall through */
+	case USB_CHG_STATE_SECONDARY_DONE:
+		rphy->chg_state = USB_CHG_STATE_DETECTED;
+		delay = 0;
+		/* fall through */
+	case USB_CHG_STATE_DETECTED:
+		/* put the controller in normal mode */
+		property_enable(rphy, &rphy->phy_cfg->chg_det.opmode, true);
+		rockchip_usb2phy_otg_sm_work(&rport->otg_sm_work.work);
+		dev_info(&rport->phy->dev, "charger = %s\n",
+			 chg_to_string(rphy->chg_type));
+		return;
+	default:
+		return;
+	}
+
+	schedule_delayed_work(&rport->chg_work, delay);
+}
+
 /*
  * The function manage host-phy port state and suspend/resume phy port
  * to save power.
@@ -485,6 +884,26 @@ static irqreturn_t rockchip_usb2phy_linestate_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t rockchip_usb2phy_bvalid_irq(int irq, void *data)
+{
+	struct rockchip_usb2phy_port *rport = data;
+	struct rockchip_usb2phy *rphy = dev_get_drvdata(rport->phy->dev.parent);
+
+	if (!property_enabled(rphy, &rport->port_cfg->bvalid_det_st))
+		return IRQ_NONE;
+
+	mutex_lock(&rport->mutex);
+
+	/* clear bvalid detect irq pending status */
+	property_enable(rphy, &rport->port_cfg->bvalid_det_clr, true);
+
+	mutex_unlock(&rport->mutex);
+
+	rockchip_usb2phy_otg_sm_work(&rport->otg_sm_work.work);
+
+	return IRQ_HANDLED;
+}
+
 static int rockchip_usb2phy_host_port_init(struct rockchip_usb2phy *rphy,
 					   struct rockchip_usb2phy_port *rport,
 					   struct device_node *child_np)
@@ -509,13 +928,86 @@ static int rockchip_usb2phy_host_port_init(struct rockchip_usb2phy *rphy,
 					IRQF_ONESHOT,
 					"rockchip_usb2phy", rport);
 	if (ret) {
-		dev_err(rphy->dev, "failed to request irq handle\n");
+		dev_err(rphy->dev, "failed to request linestate irq handle\n");
 		return ret;
 	}
 
 	return 0;
 }
 
+static int rockchip_otg_event(struct notifier_block *nb,
+			      unsigned long event, void *ptr)
+{
+	struct rockchip_usb2phy_port *rport =
+		container_of(nb, struct rockchip_usb2phy_port, event_nb);
+
+	schedule_delayed_work(&rport->otg_sm_work, OTG_SCHEDULE_DELAY);
+
+	return NOTIFY_DONE;
+}
+
+static int rockchip_usb2phy_otg_port_init(struct rockchip_usb2phy *rphy,
+					  struct rockchip_usb2phy_port *rport,
+					  struct device_node *child_np)
+{
+	int ret;
+
+	rport->port_id = USB2PHY_PORT_OTG;
+	rport->port_cfg = &rphy->phy_cfg->port_cfgs[USB2PHY_PORT_OTG];
+	rport->state = OTG_STATE_UNDEFINED;
+
+	/*
+	 * set suspended flag to true, but actually don't
+	 * put phy in suspend mode, it aims to enable usb
+	 * phy and clock in power_on() called by usb controller
+	 * driver during probe.
+	 */
+	rport->suspended = true;
+	rport->vbus_attached = false;
+
+	mutex_init(&rport->mutex);
+
+	rport->mode = of_usb_get_dr_mode_by_phy(child_np, -1);
+	if (rport->mode == USB_DR_MODE_HOST) {
+		ret = 0;
+		goto out;
+	}
+
+	INIT_DELAYED_WORK(&rport->chg_work, rockchip_chg_detect_work);
+	INIT_DELAYED_WORK(&rport->otg_sm_work, rockchip_usb2phy_otg_sm_work);
+
+	rport->utmi_avalid =
+		of_property_read_bool(child_np, "rockchip,utmi-avalid");
+
+	rport->bvalid_irq = of_irq_get_byname(child_np, "otg-bvalid");
+	if (rport->bvalid_irq < 0) {
+		dev_err(rphy->dev, "no vbus valid irq provided\n");
+		ret = rport->bvalid_irq;
+		goto out;
+	}
+
+	ret = devm_request_threaded_irq(rphy->dev, rport->bvalid_irq, NULL,
+					rockchip_usb2phy_bvalid_irq,
+					IRQF_ONESHOT,
+					"rockchip_usb2phy_bvalid", rport);
+	if (ret) {
+		dev_err(rphy->dev, "failed to request otg-bvalid irq handle\n");
+		goto out;
+	}
+
+	if (!IS_ERR(rphy->edev)) {
+		rport->event_nb.notifier_call = rockchip_otg_event;
+
+		ret = extcon_register_notifier(rphy->edev, EXTCON_USB_HOST,
+					       &rport->event_nb);
+		if (ret)
+			dev_err(rphy->dev, "register USB HOST notifier failed\n");
+	}
+
+out:
+	return ret;
+}
+
 static int rockchip_usb2phy_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -553,8 +1045,14 @@ static int rockchip_usb2phy_probe(struct platform_device *pdev)
 
 	rphy->dev = dev;
 	phy_cfgs = match->data;
+	rphy->chg_state = USB_CHG_STATE_UNDEFINED;
+	rphy->chg_type = POWER_SUPPLY_TYPE_UNKNOWN;
 	platform_set_drvdata(pdev, rphy);
 
+	ret = rockchip_usb2phy_extcon_register(rphy);
+	if (ret)
+		return ret;
+
 	/* find out a proper config which can be matched with dt. */
 	index = 0;
 	while (phy_cfgs[index].reg) {
@@ -591,13 +1089,9 @@ static int rockchip_usb2phy_probe(struct platform_device *pdev)
 		struct rockchip_usb2phy_port *rport = &rphy->ports[index];
 		struct phy *phy;
 
-		/*
-		 * This driver aim to support both otg-port and host-port,
-		 * but unfortunately, the otg part is not ready in current,
-		 * so this comments and below codes are interim, which should
-		 * be changed after otg-port is supplied soon.
-		 */
-		if (of_node_cmp(child_np->name, "host-port"))
+		/* This driver aims to support both otg-port and host-port */
+		if (of_node_cmp(child_np->name, "host-port") &&
+		    of_node_cmp(child_np->name, "otg-port"))
 			goto next_child;
 
 		phy = devm_phy_create(dev, child_np, &rockchip_usb2phy_ops);
@@ -610,9 +1104,18 @@ static int rockchip_usb2phy_probe(struct platform_device *pdev)
 		rport->phy = phy;
 		phy_set_drvdata(rport->phy, rport);
 
-		ret = rockchip_usb2phy_host_port_init(rphy, rport, child_np);
-		if (ret)
-			goto put_child;
+		/* initialize otg/host port separately */
+		if (!of_node_cmp(child_np->name, "host-port")) {
+			ret = rockchip_usb2phy_host_port_init(rphy, rport,
+							      child_np);
+			if (ret)
+				goto put_child;
+		} else {
+			ret = rockchip_usb2phy_otg_port_init(rphy, rport,
+							     child_np);
+			if (ret)
+				goto put_child;
+		}
 
 next_child:
 		/* to prevent out of boundary */
@@ -654,10 +1157,18 @@ static const struct rockchip_usb2phy_cfg rk3366_phy_cfgs[] = {
 
 static const struct rockchip_usb2phy_cfg rk3399_phy_cfgs[] = {
 	{
-		.reg = 0xe450,
+		.reg		= 0xe450,
 		.num_ports	= 2,
 		.clkout_ctl	= { 0xe450, 4, 4, 1, 0 },
 		.port_cfgs	= {
+			[USB2PHY_PORT_OTG] = {
+				.phy_sus	= { 0xe454, 1, 0, 2, 1 },
+				.bvalid_det_en	= { 0xe3c0, 3, 3, 0, 1 },
+				.bvalid_det_st	= { 0xe3e0, 3, 3, 0, 1 },
+				.bvalid_det_clr	= { 0xe3d0, 3, 3, 0, 1 },
+				.utmi_avalid	= { 0xe2ac, 7, 7, 0, 1 },
+				.utmi_bvalid	= { 0xe2ac, 12, 12, 0, 1 },
+			},
 			[USB2PHY_PORT_HOST] = {
 				.phy_sus	= { 0xe458, 1, 0, 0x2, 0x1 },
 				.ls_det_en	= { 0xe3c0, 6, 6, 0, 1 },
@@ -667,12 +1178,32 @@ static const struct rockchip_usb2phy_cfg rk3399_phy_cfgs[] = {
 				.utmi_hstdet	= { 0xe2ac, 23, 23, 0, 1 }
 			}
 		},
+		.chg_det = {
+			.opmode		= { 0xe454, 3, 0, 5, 1 },
+			.cp_det		= { 0xe2ac, 2, 2, 0, 1 },
+			.dcp_det	= { 0xe2ac, 1, 1, 0, 1 },
+			.dp_det		= { 0xe2ac, 0, 0, 0, 1 },
+			.idm_sink_en	= { 0xe450, 8, 8, 0, 1 },
+			.idp_sink_en	= { 0xe450, 7, 7, 0, 1 },
+			.idp_src_en	= { 0xe450, 9, 9, 0, 1 },
+			.rdm_pdwn_en	= { 0xe450, 10, 10, 0, 1 },
+			.vdm_src_en	= { 0xe450, 12, 12, 0, 1 },
+			.vdp_src_en	= { 0xe450, 11, 11, 0, 1 },
+		},
 	},
 	{
-		.reg = 0xe460,
+		.reg		= 0xe460,
 		.num_ports	= 2,
 		.clkout_ctl	= { 0xe460, 4, 4, 1, 0 },
 		.port_cfgs	= {
+			[USB2PHY_PORT_OTG] = {
+				.phy_sus        = { 0xe464, 1, 0, 2, 1 },
+				.bvalid_det_en  = { 0xe3c0, 8, 8, 0, 1 },
+				.bvalid_det_st  = { 0xe3e0, 8, 8, 0, 1 },
+				.bvalid_det_clr = { 0xe3d0, 8, 8, 0, 1 },
+				.utmi_avalid	= { 0xe2ac, 10, 10, 0, 1 },
+				.utmi_bvalid    = { 0xe2ac, 16, 16, 0, 1 },
+			},
 			[USB2PHY_PORT_HOST] = {
 				.phy_sus	= { 0xe468, 1, 0, 0x2, 0x1 },
 				.ls_det_en	= { 0xe3c0, 11, 11, 0, 1 },
-- 
2.0.0


--
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 v2 0/2] support USB2 PHY OTG port for rk3399
From: William Wu @ 2016-11-03  2:06 UTC (permalink / raw)
  To: kishon-l0cyMroinI0, heiko-4mtYJXux2i+zQB+pC5nmwQ
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	frank.wang-TNX95d0MmH7DzftRWevZcw,
	huangtao-TNX95d0MmH7DzftRWevZcw, dianders-hpIqsD4AKlfQT0dZR+AlfA,
	briannorris-hpIqsD4AKlfQT0dZR+AlfA, groeck-hpIqsD4AKlfQT0dZR+AlfA,
	wulf-TNX95d0MmH7DzftRWevZcw

This series add support for rk3399 USB2 PHY0 and PHY1 OTG port.
rk3399 has two USB2 PHYs, and each USB2 PHY is comprised of one
Host port and one OTG port. We have supported Host port before,
and try to support OTG port now.

Test on rk3399-evb board.

William Wu (2):
  phy: rockchip-inno-usb2: support otg-port for rk3399
  arm64: dts: rockchip: add usb2-phy otg-port support for rk3399

 arch/arm64/boot/dts/rockchip/rk3399.dtsi |  21 ++
 drivers/phy/phy-rockchip-inno-usb2.c     | 593 +++++++++++++++++++++++++++++--
 2 files changed, 583 insertions(+), 31 deletions(-)

-- 
2.0.0


--
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 2/6] dt-bindings: qcom: clocks: Add msm8994 clock bindings
From: Jeremy McNicoll @ 2016-11-03  0:01 UTC (permalink / raw)
  To: Stephen Boyd, Jeremy McNicoll
  Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A,
	andy.gross-QSEj5FYQhm4dnm+yROfE0A, arnd-r2nGTMty4D4,
	bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	michael.scott-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <20161028000811.GI26139-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

On 2016-10-27 5:08 PM, Stephen Boyd wrote:
> On 10/25, Jeremy McNicoll wrote:
>> Signed-off-by: Jeremy McNicoll <jeremymc-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> ---
>>  .../devicetree/bindings/clock/qcom,gcc.txt         |   2 +
>>  include/dt-bindings/clock/qcom,gcc-msm8994.h       | 145 +++++++++++++++++++++
>>  2 files changed, 147 insertions(+)
>>  create mode 100644 include/dt-bindings/clock/qcom,gcc-msm8994.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc.txt b/Documentation/devicetree/bindings/clock/qcom,gcc.txt
>> index 3cf4421..58c7e76 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,gcc.txt
>> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc.txt
>> @@ -14,6 +14,8 @@ Required properties :
>>  			"qcom,gcc-msm8974"
>>  			"qcom,gcc-msm8974pro"
>>  			"qcom,gcc-msm8974pro-ac"
>> +			"qcom,gcc-msm8994"
>> +			"qcom,gcc-msm8994v2"
>
> Just drop the v2 and treat it like gcc-msm8994.


Right got it, so the 8994v2 is really the 8994.

-jeremy
>
>>  			"qcom,gcc-msm8996"
>>  			"qcom,gcc-mdm9615"
>>
>

--
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 1/6] arm64: dts: msm8992 SoC and LG Bullhead (Nexus 5X) support
From: Jeremy McNicoll @ 2016-11-02 23:59 UTC (permalink / raw)
  To: Stephen Boyd, Jeremy McNicoll
  Cc: linux-arm-msm, linux-soc, devicetree, robh, andy.gross, arnd,
	bjorn.andersson, mark.rutland, michael.scott
In-Reply-To: <20161028001153.GJ26139@codeaurora.org>

On 2016-10-27 5:11 PM, Stephen Boyd wrote:
> On 10/25, Jeremy McNicoll wrote:
>> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
>> index 5dd05de..439e40e 100644
>> --- a/arch/arm64/boot/dts/qcom/Makefile
>> +++ b/arch/arm64/boot/dts/qcom/Makefile
>> @@ -1,6 +1,8 @@
>> -dtb-$(CONFIG_ARCH_QCOM)	+= apq8016-sbc.dtb msm8916-mtp.dtb
>> -dtb-$(CONFIG_ARCH_QCOM)	+= msm8996-mtp.dtb
>> +dtb-$(CONFIG_ARCH_QCOM)	+= apq8016-sbc.dtb
>
> This is unrelated. Perhaps make another patch to "correct" it.
>

I rolled it into this change series as I was asked in previous feedback
to make them 1 per line.

>>  dtb-$(CONFIG_ARCH_QCOM)	+= apq8096-db820c.dtb
>> +dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-mtp.dtb
>> +dtb-$(CONFIG_ARCH_QCOM)	+= msm8992-bullhead-rev-101.dtb
>> +dtb-$(CONFIG_ARCH_QCOM)	+= msm8996-mtp.dtb
>>
>>  always		:= $(dtb-y)
>>  subdir-y	:= $(dts-dirs)
>> diff --git a/arch/arm64/boot/dts/qcom/msm8992.dtsi b/arch/arm64/boot/dts/qcom/msm8992.dtsi
>> new file mode 100644
>> index 0000000..8bbf4f3
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/msm8992.dtsi
>> @@ -0,0 +1,191 @@
>> +
>> +	soc: soc {
>
> Do we need the soc phandle? If it isn't used we should leave it
> out.
>

What about doing something like:

8992soc: soc {

This will allow it to be overriden.  Or would it be best to add it when
someone wants / needs to override it.

>> +
>> +		clock_gcc: qcom,gcc@fc400000 {
>
> s/qcom,gcc/clock-controller/
>

done.


> Also we typically just give it a "gcc" phandle to be terse.

is clock_gcc not a little more descriptive from a human perspective?

Sometimes being terse is hard for humans to parse.

>
>> +			compatible = "qcom,gcc-8994";
>
> qcom,gcc-msm8994


agreed.


>
>> +			#clock-cells = <1>;
>> +			#reset-cells = <1>;
>> +			#power-domain-cells = <1>;
>> +			reg = <0xfc400000 0x2000>;
>> +		};
>> +	};
>> +
>> +	memory {
>> +		#address-cells = <2>;
>> +		#size-cells = <2>;
>> +
>> +		device_type = "memory";
>> +		reg = <0 0 0 0>; // bootloader will update
>> +	};
>> +
>> +	clocks {
>> +		xo_board: xo_board {
>> +			compatible = "fixed-clock";
>> +			#clock-cells = <0>;
>> +			clock-frequency = <19200000>;
>> +			clock-output-names = "xo_board";
>> +		};
>> +
>> +		sleep_clk: sleep_clk {
>> +			compatible = "fixed-clock";
>> +			#clock-cells = <0>;
>> +			clock-frequency = <32768>;
>> +			clock-output-names = "sleep_clk";
>
> clock-output-names can be removed because we use the same string
> as the node name.
>

one less line works for me.

-jeremy

^ permalink raw reply

* Re: [PATCH V4] pinctrl: qcom: Add msm8994 pinctrl driver
From: Jeremy McNicoll @ 2016-11-02 23:22 UTC (permalink / raw)
  To: Stephen Boyd, Michael Scott
  Cc: linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Linus Walleij, Rob Herring,
	Mark Rutland, Andy Gross, David Brown, Bjorn Andersson,
	Joonwoo Park, Jeremy McNicoll
In-Reply-To: <20161102183607.GJ16026-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

On 2016-11-02 11:36 AM, Stephen Boyd wrote:
> On 11/01, Michael Scott wrote:
>>
>> On 11/01/2016 04:53 PM, Stephen Boyd wrote:
>>> On 10/31, Michael Scott wrote:
>>>> +
>>>> +static const struct msm_pingroup msm8994_groups[] = {
>>>> +	PINGROUP(0,   blsp_spi1, blsp_uart1, blsp_uim1, NA, NA, NA, NA, NA, NA,
>>>> +		 NA, NA),
>>> I see an hdmi_rcv group here after blsp_uim1. Please add it for
>>> this gpio.
>>
>> Hi Stephen,
>>
>> Thank you for reviewing the data-- this is really tedious stuff.
>>
>> I'd like to make sure that we're both using tech data for msm8994.
>> I don't see the hdmi_rcv function at all in my docs.
>>
>>>
>>>> +	PINGROUP(1,   blsp_spi1, blsp_uart1, blsp_uim1, NA, NA, NA, NA, NA, NA,
>>>> +		 NA, NA),
>>>> +	PINGROUP(2,   blsp_spi1, blsp_uart1, blsp_i2c1, NA, NA, NA, NA, NA, NA,
>>>> +		 NA, NA),
>>>> +	PINGROUP(3,   blsp_spi1, blsp_uart1, blsp_i2c1, NA, NA, NA, NA, NA, NA,
>>>> +		 NA, NA),
>>>> +	PINGROUP(4,   blsp_spi2, blsp_uart2, blsp_uim2, qdss_cti_trig_out_b,
>>>> +		 NA, NA, NA, NA, NA, NA, NA),
>>>> +	PINGROUP(5,   blsp_spi2, blsp_uart2, blsp_uim2, qdss_cti_trig_in_b, NA,
>>> The qdss_cti_* is in function 5 for both of these, not function
>>> 4.
>> These are indeed wrong, but my docs both show as function 7.
>
> Ok. I think your document is for 8994v1, which never got
> commercialized. I certainly see that this function assignment
> changed between v1 and v2 from 7 to 5.
>


This is a case where not having the docs and only downstream code as a
reference is probably a good thing (tm), although the downstream code
may end up hurting more than helping.

-jeremy

--
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: [RESEND/PATCH v6 3/3] clk: qcom: Add A53 clock driver
From: Stephen Boyd @ 2016-11-02 22:55 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Georgi Djakov, mturquette, linux-clk, linux-kernel, linux-arm-msm,
	devicetree, Rob Herring
In-Reply-To: <20161102205910.GQ25787@tuxbot>

On 11/02, Bjorn Andersson wrote:
> On Thu 27 Oct 18:54 PDT 2016, Stephen Boyd wrote:
> 
> > On 10/19, Georgi Djakov wrote:
> > > Add a driver for the A53 Clock Controller. It is a hardware block that
> > > implements a combined mux and half integer divider functionality. It can
> > > choose between a fixed-rate clock or the dedicated A53 PLL. The source
> > > and the divider can be set both at the same time.
> > > 
> > > This is required for enabling CPU frequency scaling on platforms like
> > > MSM8916.
> > > 
> > 
> > Please Cc DT reviewers.
> > 
> > > Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> > > ---
> > >  .../devicetree/bindings/clock/qcom,a53cc.txt       |  22 +++
> > >  drivers/clk/qcom/Kconfig                           |   8 ++
> > >  drivers/clk/qcom/Makefile                          |   1 +
> > >  drivers/clk/qcom/a53cc.c                           | 155 +++++++++++++++++++++
> > >  4 files changed, 186 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/clock/qcom,a53cc.txt
> > >  create mode 100644 drivers/clk/qcom/a53cc.c
> > > 
> > > diff --git a/Documentation/devicetree/bindings/clock/qcom,a53cc.txt b/Documentation/devicetree/bindings/clock/qcom,a53cc.txt
> > > new file mode 100644
> > > index 000000000000..a025f062f177
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/clock/qcom,a53cc.txt
> > > @@ -0,0 +1,22 @@
> > > +Qualcomm A53 CPU Clock Controller Binding
> > > +------------------------------------------------
> > > +The A53 CPU Clock Controller is hardware, which provides a combined
> > > +mux and divider functionality for the CPU clocks. It can choose between
> > > +a fixed rate clock and the dedicated A53 PLL.
> > > +
> > > +Required properties :
> > > +- compatible : shall contain:
> > > +
> > > +			"qcom,a53cc"
> > > +
> > > +- reg : shall contain base register location and length
> > > +	of the APCS region
> > > +- #clock-cells : shall contain 1
> > > +
> > > +Example:
> > > +
> > > +	apcs: syscon@b011000 {
> > > +		compatible = "qcom,a53cc", "syscon";
> > 
> > Why is it a syscon? Is that part used?
> > 
> 
> I use the register at offset 8 for interrupting the other subsystems, so
> this must be available as something I can poke.
> 
> Which makes me think that this should be described as a "simple-mfd" and
> "syscon" with the a53cc node as a child - grabbing the regmap of the
> syscon parent, rather then ioremapping the same region again.
> 

That's sort of a question for DT reviewers. The register space
certainly seems like a free for all with a tilt toward power
management of the CPU, similar to how this was done on Krait
based designs.

I wonder why we didn't make up some provider/consumer binding for
the "kicking" feature used by SMD/RPM code. Then this could be a
clock provider and a "kick" provider (haha #kick-cells) and the
usage of syscon/regmap wouldn't be mandatory.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ 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