Devicetree
 help / color / mirror / Atom feed
* [PATCH v7 2/7] doc: Add documentation for Coresight CPU debug
From: Leo Yan @ 2017-05-02  8:30 UTC (permalink / raw)
  To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
	Catalin Marinas, Will Deacon, Andy Gross, David Brown,
	Mathieu Poirier, Greg Kroah-Hartman, Suzuki K Poulose,
	Stephen Boyd, linux-doc, linux-kernel, devicetree,
	linux-arm-kernel, linux-arm-msm, linux-soc, Mike Leach,
	Sudeep Holla
  Cc: Leo Yan
In-Reply-To: <1493713805-26920-1-git-send-email-leo.yan@linaro.org>

Update kernel-parameters.txt to add new parameter:
coresight_cpu_debug.enable is a knob to enable debugging at boot time.

Add detailed documentation, which contains the implementation, Mike
Leach excellent summary for "clock and power domain". At the end some
examples on how to enable the debugging functionality are provided.

Suggested-by: Mike Leach <mike.leach@linaro.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 Documentation/admin-guide/kernel-parameters.txt |   7 +
 Documentation/trace/coresight-cpu-debug.txt     | 174 ++++++++++++++++++++++++
 2 files changed, 181 insertions(+)
 create mode 100644 Documentation/trace/coresight-cpu-debug.txt

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index facc20a..cf90146 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -650,6 +650,13 @@
 			/proc/<pid>/coredump_filter.
 			See also Documentation/filesystems/proc.txt.
 
+	coresight_cpu_debug.enable
+			[ARM,ARM64]
+			Format: <bool>
+			Enable/disable the CPU sampling based debugging.
+			0: default value, disable debugging
+			1: enable debugging at boot time
+
 	cpuidle.off=1	[CPU_IDLE]
 			disable the cpuidle sub-system
 
diff --git a/Documentation/trace/coresight-cpu-debug.txt b/Documentation/trace/coresight-cpu-debug.txt
new file mode 100644
index 0000000..fd3f07d
--- /dev/null
+++ b/Documentation/trace/coresight-cpu-debug.txt
@@ -0,0 +1,174 @@
+		Coresight CPU Debug Module
+		==========================
+
+   Author:   Leo Yan <leo.yan@linaro.org>
+   Date:     April 5th, 2017
+
+Introduction
+------------
+
+Coresight CPU debug module is defined in ARMv8-a architecture reference manual
+(ARM DDI 0487A.k) Chapter 'Part H: External debug', the CPU can integrate
+debug module and it is mainly used for two modes: self-hosted debug and
+external debug. Usually the external debug mode is well known as the external
+debugger connects with SoC from JTAG port; on the other hand the program can
+explore debugging method which rely on self-hosted debug mode, this document
+is to focus on this part.
+
+The debug module provides sample-based profiling extension, which can be used
+to sample CPU program counter, secure state and exception level, etc; usually
+every CPU has one dedicated debug module to be connected. Based on self-hosted
+debug mechanism, Linux kernel can access these related registers from mmio
+region when the kernel panic happens. The callback notifier for kernel panic
+will dump related registers for every CPU; finally this is good for assistant
+analysis for panic.
+
+
+Implementation
+--------------
+
+- During driver registration, use EDDEVID and EDDEVID1 two device ID
+  registers to decide if sample-based profiling is implemented or not. On some
+  platforms this hardware feature is fully or partialy implemented; and if
+  this feature is not supported then registration will fail.
+
+- When write this doc, the debug driver mainly relies on three sampling
+  registers. The kernel panic callback notifier gathers info from EDPCSR
+  EDVIDSR and EDCIDSR; from EDPCSR we can get program counter, EDVIDSR has
+  information for secure state, exception level, bit width, etc; EDCIDSR is
+  context ID value which contains the sampled value of CONTEXTIDR_EL1.
+
+- The driver supports CPU running mode with either AArch64 or AArch32. The
+  registers naming convention is a bit different between them, AArch64 uses
+  'ED' for register prefix (ARM DDI 0487A.k, chapter H9.1) and AArch32 uses
+  'DBG' as prefix (ARM DDI 0487A.k, chapter G5.1). The driver is unified to
+  use AArch64 naming convention.
+
+- ARMv8-a (ARM DDI 0487A.k) and ARMv7-a (ARM DDI 0406C.b) have different
+  register bits definition. So the driver consolidates two difference:
+
+  If PCSROffset=0b0000, on ARMv8-a the feature of EDPCSR is not implemented;
+  but ARMv7-a defines "PCSR samples are offset by a value that depends on the
+  instruction set state". For ARMv7-a, the driver checks furthermore if CPU
+  runs with ARM or thumb instruction set and calibrate PCSR value, the
+  detailed description for offset is in ARMv7-a ARM (ARM DDI 0406C.b) chapter
+  C11.11.34 "DBGPCSR, Program Counter Sampling Register".
+
+  If PCSROffset=0b0010, ARMv8-a defines "EDPCSR implemented, and samples have
+  no offset applied and do not sample the instruction set state in AArch32
+  state". So on ARMv8 if EDDEVID1.PCSROffset is 0b0010 and the CPU operates
+  in AArch32 state, EDPCSR is not sampled; when the CPU operates in AArch64
+  state EDPCSR is sampled and no offset are applied.
+
+
+Clock and power domain
+----------------------
+
+Before accessing debug registers, we should ensure the clock and power domain
+have been enabled properly. In ARMv8-a ARM (ARM DDI 0487A.k) chapter 'H9.1
+Debug registers', the debug registers are spread into two domains: the debug
+domain and the CPU domain.
+
+                                +---------------+
+                                |               |
+                                |               |
+                     +----------+--+            |
+          dbg_clk -->|          |**|            |<-- cpu_clk
+                     |    Debug |**|   CPU      |
+           dbg_pd -->|          |**|            |<-- cpu_pd
+                     +----------+--+            |
+                                |               |
+                                |               |
+                                +---------------+
+
+For debug domain, the user uses DT binding "clocks" and "power-domains" to
+specify the corresponding clock source and power supply for the debug logic.
+The driver calls the pm_runtime_{put|get} operations as needed to handle the
+debug power domain.
+
+For CPU domain, the different SoC designs have different power management
+schemes and finally this heavily impacts external debug module. So we can
+divide into below cases:
+
+- On systems with a sane power controller which can behave correctly with
+  respect to CPU power domain, the CPU power domain can be controlled by
+  register EDPRCR in driver. The driver firstly writes bit EDPRCR.COREPURQ
+  to power up the CPU, and then writes bit EDPRCR.CORENPDRQ for emulation
+  of CPU power down. As result, this can ensure the CPU power domain is
+  powered on properly during the period when access debug related registers;
+
+- Some designs will power down an entire cluster if all CPUs on the cluster
+  are powered down - including the parts of the debug registers that should
+  remain powered in the debug power domain. The bits in EDPRCR are not
+  respected in these cases, so these designs do not support debug over
+  power down in the way that the CoreSight / Debug designers anticipated.
+  This means that even checking EDPRSR has the potential to cause a bus hang
+  if the target register is unpowered.
+
+  In this case, accessing to the debug registers while they are not powered
+  is a recipe for disaster; so we need preventing CPU low power states at boot
+  time or when user enable module at the run time. Please see chapter
+  "How to use the module" for detailed usage info for this.
+
+
+Device Tree Bindings
+--------------------
+
+See Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt for details.
+
+
+How to use the module
+---------------------
+
+If you want to enable debugging functionality at boot time, you can add
+"coresight_cpu_debug.enable=1" to the kernel command line parameter.
+
+The driver also can work as module, so can enable the debugging when insmod
+module:
+# insmod coresight_cpu_debug.ko debug=1
+
+When boot time or insmod module you have not enabled the debugging, the driver
+uses the debugfs file system to provide a knob to dynamically enable or disable
+debugging:
+
+To enable it, write a '1' into /sys/kernel/debug/coresight_cpu_debug/enable:
+# echo 1 > /sys/kernel/debug/coresight_cpu_debug/enable
+
+To disable it, write a '0' into /sys/kernel/debug/coresight_cpu_debug/enable:
+# echo 0 > /sys/kernel/debug/coresight_cpu_debug/enable
+
+As explained in chapter "Clock and power domain", if you are working on one
+platform which has idle states to power off debug logic and the power
+controller cannot work well for the request from EDPRCR, then you should
+firstly constraint CPU idle states before enable CPU debugging feature; so can
+ensure the accessing to debug logic.
+
+If you want to limit idle states at boot time, you can use "nohlt" or
+"cpuidle.off=1" in the kernel command line.
+
+At the runtime you can disable idle states with below methods:
+
+Set latency request to /dev/cpu_dma_latency to disable all CPUs specific idle
+states (if latency = 0uS then disable all idle states):
+# echo "what_ever_latency_you_need_in_uS" > /dev/cpu_dma_latency
+
+Disable specific CPU's specific idle state:
+# echo 1 > /sys/devices/system/cpu/cpu$cpu/cpuidle/state$state/disable
+
+
+Output format
+-------------
+
+Here is an example of the debugging output format:
+
+ARM external debug module:
+CPU[0]:
+ EDPRSR:  0000000b (Power:On DLK:Unlock)
+ EDPCSR:  [<ffff00000808eb54>] handle_IPI+0xe4/0x150
+ EDCIDSR: 00000000
+ EDVIDSR: 90000000 (State:Non-secure Mode:EL1/0 Width:64bits VMID:0)
+CPU[1]:
+ EDPRSR:  0000000b (Power:On DLK:Unlock)
+ EDPCSR:  [<ffff0000087a64c0>] debug_notifier_call+0x108/0x288
+ EDCIDSR: 00000000
+ EDVIDSR: 90000000 (State:Non-secure Mode:EL1/0 Width:64bits VMID:0)
-- 
2.7.4


^ permalink raw reply related

* [PATCH v7 1/7] coresight: bindings for CPU debug module
From: Leo Yan @ 2017-05-02  8:29 UTC (permalink / raw)
  To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
	Catalin Marinas, Will Deacon, Andy Gross, David Brown,
	Mathieu Poirier, Greg Kroah-Hartman, Suzuki K Poulose,
	Stephen Boyd, linux-doc, linux-kernel, devicetree,
	linux-arm-kernel, linux-arm-msm, linux-soc, Mike Leach,
	Sudeep Holla
  Cc: Leo Yan
In-Reply-To: <1493713805-26920-1-git-send-email-leo.yan@linaro.org>

According to ARMv8 architecture reference manual (ARM DDI 0487A.k)
Chapter 'Part H: External debug', the CPU can integrate debug module
and it can support self-hosted debug and external debug. Especially
for supporting self-hosted debug, this means the program can access
the debug module from mmio region; and usually the mmio region is
integrated with coresight.

So add document for binding debug component, includes binding to APB
clock; and also need specify the CPU node which the debug module is
dedicated to specific CPU.

Suggested-by: Mike Leach <mike.leach@linaro.org>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 .../bindings/arm/coresight-cpu-debug.txt           | 49 ++++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt

diff --git a/Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt b/Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
new file mode 100644
index 0000000..2982912
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
@@ -0,0 +1,49 @@
+* CoreSight CPU Debug Component:
+
+CoreSight CPU debug component are compliant with the ARMv8 architecture
+reference manual (ARM DDI 0487A.k) Chapter 'Part H: External debug'. The
+external debug module is mainly used for two modes: self-hosted debug and
+external debug, and it can be accessed from mmio region from Coresight
+and eventually the debug module connects with CPU for debugging. And the
+debug module provides sample-based profiling extension, which can be used
+to sample CPU program counter, secure state and exception level, etc;
+usually every CPU has one dedicated debug module to be connected.
+
+Required properties:
+
+- compatible : should be "arm,coresight-cpu-debug"; supplemented with
+               "arm,primecell" since this driver is using the AMBA bus
+	       interface.
+
+- reg : physical base address and length of the register set.
+
+- clocks : the clock associated to this component.
+
+- clock-names : the name of the clock referenced by the code. Since we are
+                using the AMBA framework, the name of the clock providing
+		the interconnect should be "apb_pclk" and the clock is
+		mandatory. The interface between the debug logic and the
+		processor core is clocked by the internal CPU clock, so it
+		is enabled with CPU clock by default.
+
+- cpu : the CPU phandle the debug module is affined to. When omitted
+	the module is considered to belong to CPU0.
+
+Optional properties:
+
+- power-domains: a phandle to the debug power domain. We use "power-domains"
+                 binding to turn on the debug logic if it has own dedicated
+		 power domain and if necessary to use "cpuidle.off=1" or
+		 "nohlt" in the kernel command line or sysfs node to
+		 constrain idle states to ensure registers in the CPU power
+		 domain are accessible.
+
+Example:
+
+	debug@f6590000 {
+		compatible = "arm,coresight-cpu-debug","arm,primecell";
+		reg = <0 0xf6590000 0 0x1000>;
+		clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+		clock-names = "apb_pclk";
+		cpu = <&cpu0>;
+	};
-- 
2.7.4


^ permalink raw reply related

* [PATCH v7 0/7] coresight: enable debug module
From: Leo Yan @ 2017-05-02  8:29 UTC (permalink / raw)
  To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
	Catalin Marinas, Will Deacon, Andy Gross, David Brown,
	Mathieu Poirier, Greg Kroah-Hartman, Suzuki K Poulose,
	Stephen Boyd, linux-doc, linux-kernel, devicetree,
	linux-arm-kernel, linux-arm-msm, linux-soc, Mike Leach,
	Sudeep Holla
  Cc: Leo Yan

ARMv8 architecture reference manual (ARM DDI 0487A.k) Chapter H7 "The
Sample-based Profiling Extension" has description for sampling
registers, we can utilize these registers to check program counter
value with combined CPU exception level, secure state, etc. So this is
helpful for CPU lockup bugs, e.g. if one CPU has run into infinite loop
with IRQ disabled; the 'hang' CPU cannot switch context and handle any
interrupt, so it cannot handle SMP call for stack dump, etc.

This patch series is to enable coresight debug module with sample-based
registers and register call back notifier for PCSR register dumping
when panic happens, so we can see below dumping info for panic; and
this patch series has considered the conditions for access permission
for debug registers self, so this can avoid access debug registers when
CPU power domain is off; the driver also try to figure out the CPU is
in secure or non-secure state.

Patch 0001 is to document the dt binding; patch 0002 is to document
boot parameters used in kernel command line and add one detailed
document to describe the Coresight debug module implementation, the
clock and power domain impaction on the driver, some examples for
usage.

Patch 0003 is used to fix the func of_get_coresight_platform_data()
doesn't properly drop the reference to the CPU node pointer; and
patch 0004 is refactor to add new function of_coresight_get_cpu().

Patch 0005 is the driver for CPU debug module.

Patch 0006 in this series are to enable debug unit on 96boards Hikey,
Patch 0007 is to enable debug on 96boards DB410c. Have verified on both
two boards.

We can enable debugging with two methods, adding parameters into kernel
command line for build-in module:
  coresight_cpu_debug.enable=1

Or we can wait the system has booted up to use debugfs nodes to enable
debugging:
  # echo 1 > /sys/kernel/debug/coresight_cpu_debug/enable

As result we can get below log after input command:
echo c > /proc/sysrq-trigger:

ARM external debug module:
CPU[0]:
 EDPRSR:  0000000b (Power:On DLK:Unlock)
 EDPCSR:  [<ffff00000808eb54>] handle_IPI+0xe4/0x150
 EDCIDSR: 00000000
 EDVIDSR: 90000000 (State:Non-secure Mode:EL1/0 Width:64bits VMID:0)
CPU[1]:
 EDPRSR:  0000000b (Power:On DLK:Unlock)
 EDPCSR:  [<ffff0000087a64c0>] debug_notifier_call+0x108/0x288
 EDCIDSR: 00000000
 EDVIDSR: 90000000 (State:Non-secure Mode:EL1/0 Width:64bits VMID:0)

[...]

Changes from v6:
* According to Suzuki and Mathieu suggestions, refined debug module
  driver to install panic notifier when insmod module; refined function
  debug_force_cpu_powered_up() for CPU power state checking; some minor
  fixing for output log, adding comments for memory barrier, code
  alignment.

Changes from v5:
* According to Suzuki and Mathieu suggestions, refined debug module
  driver to drop unused structure members, refactored initialization
  code to distinguish hardware implementation features, refactored
  flow for forcing CPU powered up, supported pm_runtime operations.
* Added one new doc file: Documentation/trace/coresight-cpu-debug.txt,
  which is used to describe detailed info for implementation, clock
  and power domain impaction on debug module, and exmaples for common
  usage.
* Removed "idle constraints" from debug driver.

Changes from v4:
* This version is mainly credit to ARM colleagues many contribution
  ideas for better quality (Thanks a lot Suzuki, Mike and Sudeep!).
* According to Suzuki suggestion, refined debug module driver to avoid
  memory leak for drvdata struct, handle PCSAMPLE_MODE=1, use flag
  drvdata.pc_has_offset to indicate if PCSR has offset, minor fixes.
* According to Mathieu suggestion, refined dt binding description.
* Changed driver to support module mode;
* According to Mike suggestion and very appreciate the pseudo code,
  added support to force CPU powered up with register EDPRCR;
* According to discussions, added command line and debugfs nodes to
  support enabling debugging for boot time, or later can dynamically
  enable/disable debugging by debugfs.
* According to Rob Herring suggestion, one minor fixes in DT binding.
* According to Stephen Boyd suggestion, add const quality to structure
  device_node. And used use of_cpu_device_node_get() to replace
  of_get_cpu_node() in patch 0003.

Changes from v3:
* Added Suzuki K Poulose's patch to fix issue for the func
  of_get_coresight_platform_data() doesn't properly drop the reference
  to the CPU node pointer.
* According to Suzuki suggestion, added code to handl the corner case
  for ARMv8 CPU with aarch32 mode.
* According to Suzuki suggestion, changed compatible string to
  "arm,coresight-cpu-debug".
* According to Mathieu suggestion, added "power-domains" as optional
  properties.

Changes from v2:
* According to Mathieu Poirier suggestion, applied some minor fixes.
* Added two extra patches for enabling debug module on Hikey.

Changes from v1:
* According to Mike Leach suggestion, removed the binding for debug
  module clocks which have been directly provided by CPU clocks.
* According to Mathieu Poirier suggestion, added function
  of_coresight_get_cpu() and some minor refactors for debug module
  driver.

Changes from RFC:
* According to Mike Leach suggestion, added check for EDPRSR to avoid
  lockup; added supporting EDVIDSR and EDCIDSR registers.
* According to Mark Rutland and Mathieu Poirier suggestion, rewrote
  the documentation for DT binding.
* According to Mark and Mathieu suggestion, refined debug driver.


Leo Yan (6):
  coresight: bindings for CPU debug module
  doc: Add documentation for Coresight CPU debug
  coresight: refactor with function of_coresight_get_cpu
  coresight: add support for CPU debug module
  arm64: dts: hi6220: register debug module
  arm64: dts: qcom: msm8916: Add debug unit

Suzuki K Poulose (1):
  coresight: of_get_coresight_platform_data: Add missing of_node_put

 Documentation/admin-guide/kernel-parameters.txt    |   7 +
 .../bindings/arm/coresight-cpu-debug.txt           |  49 ++
 Documentation/trace/coresight-cpu-debug.txt        | 174 ++++++
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi          |  64 ++
 arch/arm64/boot/dts/qcom/msm8916.dtsi              |  32 +
 drivers/hwtracing/coresight/Kconfig                |  14 +
 drivers/hwtracing/coresight/Makefile               |   1 +
 drivers/hwtracing/coresight/coresight-cpu-debug.c  | 671 +++++++++++++++++++++
 drivers/hwtracing/coresight/of_coresight.c         |  40 +-
 include/linux/coresight.h                          |   2 +
 10 files changed, 1042 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
 create mode 100644 Documentation/trace/coresight-cpu-debug.txt
 create mode 100644 drivers/hwtracing/coresight/coresight-cpu-debug.c

-- 
2.7.4

^ permalink raw reply

* Re: [PATCH] ARM: dts: imx: add Gateworks Ventana GW5600 support
From: Shawn Guo @ 2017-05-02  8:22 UTC (permalink / raw)
  To: Tim Harvey
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1492197481-16638-1-git-send-email-tharvey-UMMOYl/HMS+akBO8gow8eQ@public.gmane.org>

On Fri, Apr 14, 2017 at 12:18:01PM -0700, Tim Harvey wrote:
> The Gateworks Ventana GW5600 is a media-centric single-board computer based on
> the NXP IMX6 SoC with the following features:
>  * PoE (emulated 802.3af)
>  * IMX6 DualLite Soc (supports IMX6S,IMX6DL,IMX6Q)
>  * 1GiB DDR3 DRAM (supports up to 4GiB)
>  * 8GB eMMC
>  * 1x microSD connector
>  * Gateworks System Controller:
>   - hardware watchdog
>   - hardware monitor
>   - pushbutton controller
>   - EEPROM storage
>   - power control
>  * 1x bi-color USER LED
>  * 1x front-panel pushbutton
>  * 1x front-panel GbE
>  * 2x front panel USB 2.0
>  * 1x front panel USB OTG
>  * 1x SIM socket
>  * 1x miniPCIe socket with SATA (mSATA)
>  * 1x miniPCIe socket with USB 2.0 (Modem)
>  * 1x miniPCIe socket with PCIe, USB 2.0, and SIM
>  * RS232/RS485 serial
>   - 2x RS232 UARTs (off-board connector)
>   - 1x RS485 (loading option)
>  * 4x digital I/O signals (PWM/I2C/GPIO/5V/3.3V options)
>  * 1x analog input (0 to 5V)
>  * 1x CAN (loading option)
>  * off-board LVDS:
>   - I2C
>   - 12V
>   - LED driver (4x 330mA strings)
>   - matrix keypad controller (8row x 10col)
>   - I2S
>   - dual-channel LVDS
>   - PWM
>  * off-board video input:
>   - 16bit parallel / MIPI (IPU1_CSI0)
>  * GPS (loading option)
>  * Analog Video Input (CVBS) 3 inputs (1 active at a time)
>  * Analog Audio Input/Output (2ch Line level, optional MIC/HP drivers)
>  * HDMI out
>  * JTAG programmable
>  * Inertial Module
> 
> Signed-off-by: Tim Harvey <tharvey-UMMOYl/HMS+akBO8gow8eQ@public.gmane.org>
> ---
>  arch/arm/boot/dts/Makefile            |   2 +
>  arch/arm/boot/dts/imx6dl-gw560x.dts   |  55 +++
>  arch/arm/boot/dts/imx6q-gw560x.dts    |  59 +++
>  arch/arm/boot/dts/imx6qdl-gw560x.dtsi | 761 ++++++++++++++++++++++++++++++++++
>  4 files changed, 877 insertions(+)
>  create mode 100644 arch/arm/boot/dts/imx6dl-gw560x.dts
>  create mode 100644 arch/arm/boot/dts/imx6q-gw560x.dts
>  create mode 100644 arch/arm/boot/dts/imx6qdl-gw560x.dtsi
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 0bff8e7..0c9cfbb 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -352,6 +352,7 @@ dtb-$(CONFIG_SOC_IMX6Q) += \
>  	imx6dl-gw551x.dtb \
>  	imx6dl-gw552x.dtb \
>  	imx6dl-gw553x.dtb \
> +	imx6dl-gw560x.dtb \
>  	imx6dl-gw5903.dtb \
>  	imx6dl-gw5904.dtb \
>  	imx6dl-hummingboard.dtb \
> @@ -397,6 +398,7 @@ dtb-$(CONFIG_SOC_IMX6Q) += \
>  	imx6q-gw551x.dtb \
>  	imx6q-gw552x.dtb \
>  	imx6q-gw553x.dtb \
> +	imx6q-gw560x.dtb \
>  	imx6q-gw5903.dtb \
>  	imx6q-gw5904.dtb \
>  	imx6q-h100.dtb \
> diff --git a/arch/arm/boot/dts/imx6dl-gw560x.dts b/arch/arm/boot/dts/imx6dl-gw560x.dts
> new file mode 100644
> index 0000000..21bdfaf
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6dl-gw560x.dts
> @@ -0,0 +1,55 @@
> +/*
> + * Copyright 2017 Gateworks Corporation
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + *     modify it under the terms of the GNU General Public License as
> + *     published by the Free Software Foundation; either version 2 of
> + *     the License, or (at your option) any later version.
> + *
> + *     This file is distributed in the hope that it will be useful,
> + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *     GNU General Public License for more details.
> + *
> + *     You should have received a copy of the GNU General Public
> + *     License along with this file; if not, write to the Free
> + *     Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
> + *     MA 02110-1301 USA
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + *     obtaining a copy of this software and associated documentation
> + *     files (the "Software"), to deal in the Software without
> + *     restriction, including without limitation the rights to use,
> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> + *     sell copies of the Software, and to permit persons to whom the
> + *     Software is furnished to do so, subject to the following
> + *     conditions:
> + *
> + *     The above copyright notice and this permission notice shall be
> + *     included in all copies or substantial portions of the Software.
> + *
> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + *     OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/dts-v1/;
> +#include "imx6dl.dtsi"
> +#include "imx6qdl-gw560x.dtsi"
> +
> +/ {
> +	model = "Gateworks Ventana i.MX6 DualLite/Solo GW560X";
> +	compatible = "gw,imx6dl-gw560x", "gw,ventana", "fsl,imx6dl";
> +};
> diff --git a/arch/arm/boot/dts/imx6q-gw560x.dts b/arch/arm/boot/dts/imx6q-gw560x.dts
> new file mode 100644
> index 0000000..735f2bb
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6q-gw560x.dts
> @@ -0,0 +1,59 @@
> +/*
> + * Copyright 2017 Gateworks Corporation
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + *     modify it under the terms of the GNU General Public License as
> + *     published by the Free Software Foundation; either version 2 of
> + *     the License, or (at your option) any later version.
> + *
> + *     This file is distributed in the hope that it will be useful,
> + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *     GNU General Public License for more details.
> + *
> + *     You should have received a copy of the GNU General Public
> + *     License along with this file; if not, write to the Free
> + *     Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
> + *     MA 02110-1301 USA
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + *     obtaining a copy of this software and associated documentation
> + *     files (the "Software"), to deal in the Software without
> + *     restriction, including without limitation the rights to use,
> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> + *     sell copies of the Software, and to permit persons to whom the
> + *     Software is furnished to do so, subject to the following
> + *     conditions:
> + *
> + *     The above copyright notice and this permission notice shall be
> + *     included in all copies or substantial portions of the Software.
> + *
> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + *     OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/dts-v1/;
> +#include "imx6q.dtsi"
> +#include "imx6qdl-gw560x.dtsi"
> +
> +/ {
> +	model = "Gateworks Ventana i.MX6 Dual/Quad GW560X";
> +	compatible = "gw,imx6q-gw560x", "gw,ventana", "fsl,imx6q";
> +};
> +
> +&sata {
> +	status = "okay";
> +};
> diff --git a/arch/arm/boot/dts/imx6qdl-gw560x.dtsi b/arch/arm/boot/dts/imx6qdl-gw560x.dtsi
> new file mode 100644
> index 0000000..dc52b87
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6qdl-gw560x.dtsi
> @@ -0,0 +1,761 @@
> +/*
> + * Copyright 2017 Gateworks Corporation
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + *     modify it under the terms of the GNU General Public License as
> + *     published by the Free Software Foundation; either version 2 of
> + *     the License, or (at your option) any later version.
> + *
> + *     This file is distributed in the hope that it will be useful,
> + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *     GNU General Public License for more details.
> + *
> + *     You should have received a copy of the GNU General Public
> + *     License along with this file; if not, write to the Free
> + *     Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
> + *     MA 02110-1301 USA
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + *     obtaining a copy of this software and associated documentation
> + *     files (the "Software"), to deal in the Software without
> + *     restriction, including without limitation the rights to use,
> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> + *     sell copies of the Software, and to permit persons to whom the
> + *     Software is furnished to do so, subject to the following
> + *     conditions:
> + *
> + *     The above copyright notice and this permission notice shall be
> + *     included in all copies or substantial portions of the Software.
> + *
> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + *     OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include <dt-bindings/gpio/gpio.h>
> +
> +/ {
> +	/* these are used by bootloader for disabling nodes */
> +	aliases {
> +		led0 = &led0;
> +		led1 = &led1;
> +		led2 = &led2;
> +		ssi0 = &ssi1;
> +		usb0 = &usbh1;
> +		usb1 = &usbotg;
> +	};
> +
> +	chosen {
> +		stdout-path = &uart2;
> +	};
> +
> +	backlight_display {

Can we consistently use hyphen instead of underscore in node name?

> +		compatible = "pwm-backlight";
> +		pwms = <&pwm4 0 5000000>;
> +		brightness-levels = <
> +			0  1  2  3  4  5  6  7  8  9
> +			10 11 12 13 14 15 16 17 18 19
> +			20 21 22 23 24 25 26 27 28 29
> +			30 31 32 33 34 35 36 37 38 39
> +			40 41 42 43 44 45 46 47 48 49
> +			50 51 52 53 54 55 56 57 58 59
> +			60 61 62 63 64 65 66 67 68 69
> +			70 71 72 73 74 75 76 77 78 79
> +			80 81 82 83 84 85 86 87 88 89
> +			90 91 92 93 94 95 96 97 98 99
> +			100
> +			>;
> +		default-brightness-level = <100>;
> +	};
> +
> +	backlight_keypad {

Ditto

> +		compatible = "gpio-backlight";
> +		gpios = <&gpio4 30 GPIO_ACTIVE_HIGH>;
> +		default-on;
> +	};
> +
> +	clocks {
> +		codec_osc: codec_osc {

I cannot find how this fixed clock is used/referenced.

> +			#clock-cells = <0>;
> +			compatible = "fixed-clock";
> +			clock-frequency = <12000000>;
> +		};
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_gpio_leds>;
> +
> +		led0: user1 {
> +			label = "user1";
> +			gpios = <&gpio4 6 GPIO_ACTIVE_HIGH>; /* MX6_PANLEDG */
> +			default-state = "on";
> +			linux,default-trigger = "heartbeat";
> +		};
> +
> +		led1: user2 {
> +			label = "user2";
> +			gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>; /* MX6_PANLEDR */
> +			default-state = "off";
> +		};
> +
> +		led2: user3 {
> +			label = "user3";
> +			gpios = <&gpio4 15 GPIO_ACTIVE_LOW>; /* MX6_LOCLED# */
> +			default-state = "off";
> +		};
> +	};
> +
> +	memory {

memory@10000000

> +		reg = <0x10000000 0x40000000>;
> +	};
> +
> +	pps {
> +		compatible = "pps-gpio";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_pps>;
> +		gpios = <&gpio1 26 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	reg_2p5v: regulator-2p5v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "2P5V";
> +		regulator-min-microvolt = <2500000>;
> +		regulator-max-microvolt = <2500000>;
> +		regulator-always-on;
> +	};
> +
> +	reg_3p3v: regulator-3p3v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "3P3V";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-always-on;
> +	};
> +
> +	reg_5p0v: regulator-5p0v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "5P0V";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-always-on;
> +	};
> +
> +	reg_12p0v: regulator-12p0v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "12P0V";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		gpio = <&gpio4 25 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	reg_1p4v: regulator-vddsoc {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vdd_soc";
> +		regulator-min-microvolt = <1400000>;
> +		regulator-max-microvolt = <1400000>;
> +		regulator-always-on;
> +	};
> +
> +	reg_usb_h1_vbus: regulator-usb-h1-vbus {
> +		compatible = "regulator-fixed";
> +		regulator-name = "usb_h1_vbus";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-always-on;
> +	};
> +
> +	reg_usb_otg_vbus: regulator-usb-otg-vbus {
> +		compatible = "regulator-fixed";
> +		regulator-name = "usb_otg_vbus";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		gpio = <&gpio3 22 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	sound {
> +		compatible = "fsl,imx6q-ventana-sgtl5000",
> +			     "fsl,imx-audio-sgtl5000";
> +		model = "sgtl5000-audio";
> +		ssi-controller = <&ssi1>;
> +		audio-codec = <&sgtl5000>;
> +		audio-routing =
> +			"MIC_IN", "Mic Jack",
> +			"Mic Jack", "Mic Bias",
> +			"Headphone Jack", "HP_OUT";
> +		mux-int-port = <1>;
> +		mux-ext-port = <4>;
> +	};
> +};
> +
> +&audmux {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_audmux>;
> +	status = "okay";
> +};
> +
> +&ecspi3 {
> +	cs-gpios = <&gpio4 24 GPIO_ACTIVE_HIGH>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_ecspi3>;
> +	status = "okay";
> +};
> +
> +&can1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_flexcan>;
> +	status = "okay";
> +};
> +
> +&clks {
> +	assigned-clocks = <&clks IMX6QDL_CLK_LDB_DI0_SEL>,
> +			  <&clks IMX6QDL_CLK_LDB_DI1_SEL>;
> +	assigned-clock-parents = <&clks IMX6QDL_CLK_PLL3_USB_OTG>,
> +				 <&clks IMX6QDL_CLK_PLL3_USB_OTG>;
> +};
> +
> +&fec {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_enet>;
> +	phy-mode = "rgmii-id";
> +	phy-reset-gpios = <&gpio1 30 GPIO_ACTIVE_LOW>;
> +	status = "okay";
> +};
> +
> +&hdmi {
> +	ddc-i2c-bus = <&i2c3>;
> +	status = "okay";
> +};
> +
> +&i2c1 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c1>;
> +	status = "okay";
> +
> +	eeprom1: eeprom@50 {
> +		compatible = "atmel,24c02";
> +		reg = <0x50>;
> +		pagesize = <16>;
> +	};
> +
> +	eeprom2: eeprom@51 {
> +		compatible = "atmel,24c02";
> +		reg = <0x51>;
> +		pagesize = <16>;
> +	};
> +
> +	eeprom3: eeprom@52 {
> +		compatible = "atmel,24c02";
> +		reg = <0x52>;
> +		pagesize = <16>;
> +	};
> +
> +	eeprom4: eeprom@53 {
> +		compatible = "atmel,24c02";
> +		reg = <0x53>;
> +		pagesize = <16>;
> +	};
> +
> +	pca9555: gpio@23 {
> +		compatible = "nxp,pca9555";
> +		reg = <0x23>;
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +	};
> +
> +	ds1672: rtc@68 {
> +		compatible = "dallas,ds1672";
> +		reg = <0x68>;
> +	};
> +};
> +
> +&i2c2 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c2>;
> +	status = "okay";
> +
> +	sgtl5000: codec@0a {

Drop the leading zero in unit-address.

> +		compatible = "fsl,sgtl5000";
> +		reg = <0x0a>;
> +		clocks = <&clks IMX6QDL_CLK_CKO>;
> +		VDDA-supply = <&reg_1p8v>;
> +		VDDIO-supply = <&reg_3p3v>;
> +	};
> +
> +	tca8418: keypad@34 {
> +		compatible = "ti,tca8418";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_keypad>;
> +		reg = <0x34>;
> +		interrupt-parent = <&gpio5>;
> +		interrupts = <11 IRQ_TYPE_EDGE_FALLING>;
> +		linux,keymap = <0x00010100	/* BTN_0 HOME */
> +			        0x00000101	/* BTN_1 MENU */
> +			        0x01010102	/* BTN_2 ESCAPE */
> +			        0x01000103	/* BTN_3 BACK */
> +			        0x02000104	/* BTN_4 SEARCH */
> +			        0x00030105	/* BTN_5 UP */
> +			        0x00020106	/* BTN_6 RIGHT */
> +			        0x01030107	/* BTN_7 LEFT */
> +			        0x01020108	/* BTN_8 DOWN */
> +			        0x02020109>;	/* BTN_9 ENTER */

Please use MATRIX_KEY() and defines in linux-event-codes.h, so that we
can save the comments.

> +		keypad,num-rows = <4>;
> +		keypad,num-columns = <4>;
> +	};
> +
> +	ltc3676: pmic@3c {
> +		compatible = "lltc,ltc3676";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_pmic>;
> +		reg = <0x3c>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <8 IRQ_TYPE_EDGE_FALLING>;
> +
> +		regulators {
> +			/* VDD_DDR (1+R1/R2 = 2.105) */
> +			reg_vdd_ddr: sw2 {
> +				regulator-name = "vddddr";
> +				regulator-min-microvolt = <868310>;
> +				regulator-max-microvolt = <1684000>;
> +				lltc,fb-voltage-divider = <221000 200000>;
> +				regulator-ramp-delay = <7000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			/* VDD_ARM (1+R1/R2 = 1.931) */
> +			reg_vdd_arm: sw3 {
> +				regulator-name = "vddarm";
> +				regulator-min-microvolt = <796551>;
> +				regulator-max-microvolt = <1544827>;
> +				lltc,fb-voltage-divider = <243000 261000>;
> +				regulator-ramp-delay = <7000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +				linux,phandle = <&reg_vdd_arm>;
> +			};
> +
> +			/* VDD_1P8 (1+R1/R2 = 2.505): GPS/VideoIn/ENET-PHY */
> +			reg_1p8v: sw4 {
> +				regulator-name = "vdd1p8";
> +				regulator-min-microvolt = <1033310>;
> +				regulator-max-microvolt = <2004000>;
> +				lltc,fb-voltage-divider = <301000 200000>;
> +				regulator-ramp-delay = <7000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			/* VDD_1P0 (1+R1/R2 = 1.39): PCIe/ENET-PHY */
> +			reg_1p0v: ldo2 {
> +				regulator-name = "vdd1p0";
> +				regulator-min-microvolt = <950000>;
> +				regulator-max-microvolt = <1050000>;
> +				lltc,fb-voltage-divider = <78700 200000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			/* VDD_AUD_1P8: Audio codec */
> +			reg_aud_1p8v: ldo3 {
> +				regulator-name = "vdd1p8a";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-boot-on;
> +			};
> +
> +			/* VDD_HIGH (1+R1/R2 = 4.17) */
> +			reg_3p0v: ldo4 {
> +				regulator-name = "vdd3p0";
> +				regulator-min-microvolt = <3023250>;
> +				regulator-max-microvolt = <3023250>;
> +				lltc,fb-voltage-divider = <634000 200000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +		};
> +	};
> +};
> +
> +&i2c3 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c3>;
> +	status = "okay";
> +
> +	egalax_ts: touchscreen@04 {

Drop the leading zero in unit-address.

> +		compatible = "eeti,egalax_ts";
> +		reg = <0x04>;
> +		interrupt-parent = <&gpio5>;
> +		interrupts = <12 IRQ_TYPE_EDGE_FALLING>;
> +		wakeup-gpios = <&gpio5 13 GPIO_ACTIVE_LOW>;
> +	};
> +};
> +
> +&ldb {
> +	fsl,dual-channel;
> +	status = "okay";
> +
> +	lvds-channel@0 {
> +		fsl,data-mapping = "spwg";
> +		fsl,data-width = <18>;
> +		status = "okay";
> +
> +		display-timings {
> +			native-mode = <&timing0>;
> +			timing0: hsd100pxn1 {
> +				clock-frequency = <65000000>;
> +				hactive = <1024>;
> +				vactive = <768>;
> +				hback-porch = <220>;
> +				hfront-porch = <40>;
> +				vback-porch = <21>;
> +				vfront-porch = <7>;
> +				hsync-len = <60>;
> +				vsync-len = <10>;
> +			};
> +		};
> +	};
> +};
> +
> +&pcie {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_pcie>;
> +	reset-gpio = <&gpio4 31 GPIO_ACTIVE_LOW>;
> +	status = "okay";
> +};
> +
> +&pwm2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_pwm2>; /* MX6_DIO1 */
> +	status = "disabled";
> +};
> +
> +&pwm3 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_pwm3>; /* MX6_DIO2 */
> +	status = "disabled";
> +};
> +
> +&pwm4 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_pwm4>;
> +	status = "okay";
> +};
> +
> +&ssi1 {
> +	status = "okay";
> +};
> +
> +&uart1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart1>;
> +	uart-has-rtscts;
> +	rts-gpios = <&gpio7 1 GPIO_ACTIVE_HIGH>;
> +	status = "okay";
> +};
> +
> +&uart2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart2>;
> +	status = "okay";
> +};
> +
> +&uart5 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart5>;
> +	status = "okay";
> +};
> +
> +&usbotg {
> +	vbus-supply = <&reg_usb_otg_vbus>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usbotg>;
> +	disable-over-current;
> +	status = "okay";
> +};
> +
> +&usbh1 {
> +	vbus-supply = <&reg_usb_h1_vbus>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usbh1>;
> +	status = "okay";
> +};
> +
> +&usdhc2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usdhc2>;
> +	bus-width = <8>;
> +	vmmc-supply = <&reg_3p3v>;
> +	non-removable;
> +	status = "okay";
> +};
> +
> +&usdhc3 {
> +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> +	pinctrl-0 = <&pinctrl_usdhc3>;
> +	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
> +	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
> +	cd-gpios = <&gpio7 0 GPIO_ACTIVE_HIGH>;
> +	vmmc-supply = <&reg_3p3v>;
> +	status = "okay";
> +};
> +
> +&wdog1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_wdog>;
> +	fsl,ext-reset-output;
> +};
> +
> +&iomuxc {
> +	imx6qdl-gw560x {

Drop this container node.

> +		pinctrl_audmux: audmuxgrp {
> +			fsl,pins = <
> +				/* AUD4 */
> +				MX6QDL_PAD_DISP0_DAT20__AUD4_TXC	0x130b0
> +				MX6QDL_PAD_DISP0_DAT21__AUD4_TXD	0x110b0
> +				MX6QDL_PAD_DISP0_DAT22__AUD4_TXFS	0x130b0
> +				MX6QDL_PAD_DISP0_DAT23__AUD4_RXD	0x130b0
> +				MX6QDL_PAD_GPIO_0__CCM_CLKO1		0x130b0 /* AUD4_MCK */
> +				/* AUD6 */
> +				MX6QDL_PAD_DI0_PIN2__AUD6_TXD		0x130b0
> +				MX6QDL_PAD_DI0_PIN3__AUD6_TXFS		0x130b0
> +				MX6QDL_PAD_DI0_PIN4__AUD6_RXD		0x130b0
> +				MX6QDL_PAD_DI0_PIN15__AUD6_TXC		0x130b0
> +			>;
> +		};
> +
> +		pinctrl_ecspi3: escpi3grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_DISP0_DAT0__ECSPI3_SCLK	0x100b1
> +				MX6QDL_PAD_DISP0_DAT1__ECSPI3_MOSI	0x100b1
> +				MX6QDL_PAD_DISP0_DAT2__ECSPI3_MISO	0x100b1
> +				MX6QDL_PAD_DISP0_DAT3__GPIO4_IO24	0x100b1
> +			>;
> +		};
> +
> +		pinctrl_enet: enetgrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_RGMII_RXC__RGMII_RXC		0x1b030
> +				MX6QDL_PAD_RGMII_RD0__RGMII_RD0		0x1b030
> +				MX6QDL_PAD_RGMII_RD1__RGMII_RD1		0x1b030
> +				MX6QDL_PAD_RGMII_RD2__RGMII_RD2		0x1b030
> +				MX6QDL_PAD_RGMII_RD3__RGMII_RD3		0x1b030
> +				MX6QDL_PAD_RGMII_RX_CTL__RGMII_RX_CTL	0x1b030
> +				MX6QDL_PAD_RGMII_TXC__RGMII_TXC		0x1b030
> +				MX6QDL_PAD_RGMII_TD0__RGMII_TD0		0x1b030
> +				MX6QDL_PAD_RGMII_TD1__RGMII_TD1		0x1b030
> +				MX6QDL_PAD_RGMII_TD2__RGMII_TD2		0x1b030
> +				MX6QDL_PAD_RGMII_TD3__RGMII_TD3		0x1b030
> +				MX6QDL_PAD_RGMII_TX_CTL__RGMII_TX_CTL	0x1b030
> +				MX6QDL_PAD_ENET_REF_CLK__ENET_TX_CLK	0x1b0b0
> +				MX6QDL_PAD_ENET_MDIO__ENET_MDIO		0x1b0b0
> +				MX6QDL_PAD_ENET_MDC__ENET_MDC		0x1b0b0
> +				MX6QDL_PAD_GPIO_16__ENET_REF_CLK	0x4001b0a8
> +				MX6QDL_PAD_ENET_TXD0__GPIO1_IO30	0x4001b0b0 /* PHY_RST# */
> +			>;
> +		};
> +
> +		pinctrl_flexcan: flexcangrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_KEY_ROW2__FLEXCAN1_RX	0x1b0b1
> +				MX6QDL_PAD_KEY_COL2__FLEXCAN1_TX	0x1b0b1
> +				MX6QDL_PAD_GPIO_2__GPIO1_IO02		0x4001b0b0 /* CAN_STBY */
> +			>;
> +		};
> +
> +		pinctrl_gpio_leds: gpioledsgrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_KEY_COL0__GPIO4_IO06		0x1b0b0
> +				MX6QDL_PAD_KEY_ROW0__GPIO4_IO07		0x1b0b0
> +				MX6QDL_PAD_KEY_ROW4__GPIO4_IO15		0x1b0b0
> +			>;
> +		};
> +
> +		pinctrl_i2c1: i2c1grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_EIM_D21__I2C1_SCL		0x4001b8b1
> +				MX6QDL_PAD_EIM_D28__I2C1_SDA		0x4001b8b1
> +			>;
> +		};
> +
> +		pinctrl_usbh1: usbh1grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_GPIO_9__GPIO1_IO09		0x4001b0b0 /* USBHUB_RST# */
> +			>;
> +		};

Please sort the pinctrl nodes alphabetically.

> +
> +		pinctrl_i2c2: i2c2grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_KEY_COL3__I2C2_SCL		0x4001b8b1
> +				MX6QDL_PAD_KEY_ROW3__I2C2_SDA		0x4001b8b1
> +			>;
> +		};
> +
> +		pinctrl_pmic: pmicgrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_GPIO_8__GPIO1_IO08		0x0001b0b0 /* PMIC_IRQ# */
> +			>;
> +		};
> +
> +		pinctrl_i2c3: i2c3grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_GPIO_3__I2C3_SCL		0x4001b8b1
> +				MX6QDL_PAD_GPIO_6__I2C3_SDA		0x4001b8b1
> +

Drop this newline.

> +				/* Misc */
> +				MX6QDL_PAD_GPIO_19__GPIO4_IO05		0x4001b0b0 /* DIOI2C_DIS# */
> +

Ditto

Shawn

> +				/* Backlight / Touch Connector */
> +				MX6QDL_PAD_DISP0_DAT18__GPIO5_IO12	0x0001b0b0 /* LVDS_TOUCH_IRQ# */
> +				MX6QDL_PAD_DISP0_DAT19__GPIO5_IO13	0x0001b0b0 /* LVDS_BACKEN */
> +			>;
> +		};
> +
> +		pinctrl_keypad: keypadgrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_DISP0_DAT17__GPIO5_IO11	0x0001b0b0 /* KEYPAD_IRQ# */
> +				MX6QDL_PAD_DISP0_DAT9__GPIO4_IO30	0x0001b0b0 /* KEYPAD_LED_EN */
> +			>;
> +		};
> +
> +		pinctrl_pcie: pciegrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_DISP0_DAT10__GPIO4_IO31	0x1b0b0    /* PCI_RST# */
> +				MX6QDL_PAD_GPIO_17__GPIO7_IO12		0x4001b0b0 /* PCIESKT_WDIS# */
> +			>;
> +		};
> +
> +		pinctrl_pps: ppsgrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_ENET_RXD1__GPIO1_IO26	0x1b0b1
> +			>;
> +		};
> +
> +		pinctrl_pwm2: pwm2grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD1_DAT2__PWM2_OUT		0x1b0b1
> +			>;
> +		};
> +
> +		pinctrl_pwm3: pwm3grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD1_DAT1__PWM3_OUT		0x1b0b1
> +			>;
> +		};
> +
> +		pinctrl_pwm4: pwm4grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD1_CMD__PWM4_OUT		0x1b0b1
> +			>;
> +		};
> +
> +		pinctrl_uart1: uart1grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD3_DAT7__UART1_TX_DATA	0x1b0b1
> +				MX6QDL_PAD_SD3_DAT6__UART1_RX_DATA	0x1b0b1
> +				MX6QDL_PAD_SD3_DAT4__GPIO7_IO01		0x4001b0b1 /* TEN */
> +			>;
> +		};
> +
> +		pinctrl_uart2: uart2grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD4_DAT7__UART2_TX_DATA	0x1b0b1
> +				MX6QDL_PAD_SD4_DAT4__UART2_RX_DATA	0x1b0b1
> +			>;
> +		};
> +
> +		pinctrl_uart5: uart5grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_KEY_COL1__UART5_TX_DATA	0x1b0b1
> +				MX6QDL_PAD_KEY_ROW1__UART5_RX_DATA	0x1b0b1
> +			>;
> +		};
> +
> +		pinctrl_usbotg: usbotggrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_GPIO_1__USB_OTG_ID		0x17059
> +				MX6QDL_PAD_EIM_D22__GPIO3_IO22		0x1b0b0 /* PWR_EN */
> +				MX6QDL_PAD_KEY_COL4__GPIO4_IO14		0x1b0b0 /* OC */
> +			>;
> +		};
> +
> +		pinctrl_usdhc2: usdhc2grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD2_CMD__SD2_CMD		0x170f9
> +				MX6QDL_PAD_SD2_CLK__SD2_CLK		0x100f9
> +				MX6QDL_PAD_SD2_DAT0__SD2_DATA0		0x170f9
> +				MX6QDL_PAD_SD2_DAT1__SD2_DATA1		0x170f9
> +				MX6QDL_PAD_SD2_DAT2__SD2_DATA2		0x170f9
> +				MX6QDL_PAD_SD2_DAT3__SD2_DATA3		0x170f9
> +				MX6QDL_PAD_NANDF_D4__SD2_DATA4		0x170f9
> +				MX6QDL_PAD_NANDF_D5__SD2_DATA5		0x170f9
> +				MX6QDL_PAD_NANDF_D6__SD2_DATA6		0x170f9
> +				MX6QDL_PAD_NANDF_D7__SD2_DATA7		0x170f9
> +			>;
> +		};
> +
> +		pinctrl_usdhc3: usdhc3grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD3_CMD__SD3_CMD		0x17059
> +				MX6QDL_PAD_SD3_CLK__SD3_CLK		0x10059
> +				MX6QDL_PAD_SD3_DAT0__SD3_DATA0		0x17059
> +				MX6QDL_PAD_SD3_DAT1__SD3_DATA1		0x17059
> +				MX6QDL_PAD_SD3_DAT2__SD3_DATA2		0x17059
> +				MX6QDL_PAD_SD3_DAT3__SD3_DATA3		0x17059
> +				MX6QDL_PAD_SD3_DAT5__GPIO7_IO00		0x17059 /* CD */
> +				MX6QDL_PAD_NANDF_CS1__SD3_VSELECT	0x17059
> +			>;
> +		};
> +
> +		pinctrl_usdhc3_100mhz: usdhc3grp100mhz {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD3_CMD__SD3_CMD		0x170b9
> +				MX6QDL_PAD_SD3_CLK__SD3_CLK		0x100b9
> +				MX6QDL_PAD_SD3_DAT0__SD3_DATA0		0x170b9
> +				MX6QDL_PAD_SD3_DAT1__SD3_DATA1		0x170b9
> +				MX6QDL_PAD_SD3_DAT2__SD3_DATA2		0x170b9
> +				MX6QDL_PAD_SD3_DAT3__SD3_DATA3		0x170b9
> +				MX6QDL_PAD_SD3_DAT5__GPIO7_IO00		0x170b9 /* CD */
> +				MX6QDL_PAD_NANDF_CS1__SD3_VSELECT	0x170b9
> +			>;
> +		};
> +
> +		pinctrl_usdhc3_200mhz: usdhc3grp200mhz {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD3_CMD__SD3_CMD		0x170f9
> +				MX6QDL_PAD_SD3_CLK__SD3_CLK		0x100f9
> +				MX6QDL_PAD_SD3_DAT0__SD3_DATA0		0x170f9
> +				MX6QDL_PAD_SD3_DAT1__SD3_DATA1		0x170f9
> +				MX6QDL_PAD_SD3_DAT2__SD3_DATA2		0x170f9
> +				MX6QDL_PAD_SD3_DAT3__SD3_DATA3		0x170f9
> +				MX6QDL_PAD_SD3_DAT5__GPIO7_IO00		0x170f9 /* CD */
> +				MX6QDL_PAD_NANDF_CS1__SD3_VSELECT	0x170f9
> +			>;
> +		};
> +
> +		pinctrl_wdog: wdoggrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_DISP0_DAT8__WDOG1_B		0x1b0b0
> +			>;
> +		};
> +	};
> +};
> -- 
> 2.7.4
> 
--
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/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.
From: Daniel Vetter @ 2017-05-02  8:16 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Mark Rutland, devicetree@vger.kernel.org,
	Linux Kernel Mailing List, dri-devel, Rob Herring
In-Reply-To: <CADnq5_OEX2kzLJ5ABfBs6fZnmhFmJAPbg1XjJWx9Pt_pu=bTEA@mail.gmail.com>

On Mon, Apr 24, 2017 at 10:26:45AM -0400, Alex Deucher wrote:
> On Fri, Apr 21, 2017 at 6:53 PM, Eric Anholt <eric@anholt.net> wrote:
> > Daniel Vetter <daniel@ffwll.ch> writes:
> >
> >> On Wed, Apr 19, 2017 at 7:55 PM, Eric Anholt <eric@anholt.net> wrote:
> >>> Daniel Vetter <daniel@ffwll.ch> writes:
> >>>> On Tue, Apr 18, 2017 at 9:11 PM, Eric Anholt <eric@anholt.net> wrote:
> >>>>> The FBDEV initialization would throw an error in dmesg, when we just
> >>>>> want to silently not initialize fbdev on a V3D-only VC4 instance.
> >>>>>
> >>>>> Signed-off-by: Eric Anholt <eric@anholt.net>
> >>>>
> >>>> Hm, this shouldn't be an error really, you might want to hotplug more
> >>>> connectors later on. What exactly complains?
> >>>
> >>> drm_fb_helper_init() throws an error if the passed in connector count is
> >>> 0, so drm_fb_cma_helper() printks an error.
> >>
> >> Oh, _that_ thing. The error in there is correct, but (almost) everyone
> >> gets this parameter wrong. This isn't the max number of connectors the
> >> fb helper will light up, but just the max number of connectors _per_
> >> crtc when driving in hw clone mode. There's two problems with that:
> >> - fb helpers don't support hw clone mode, we select 1:1 crtcs for each
> >> active connector
> >> - I mentioned that everyone gets this wrong?
> >>
> >> If you're moderately bored it'd be great to nuke the max_connector
> >> argument from drm_fb_helper_init, and hard-code it to 1 (with a big
> >> comment explaining that this needs to be changed, probably with
> >> dynamic reallocation, once someone gets around to implementing hw
> >> clone mode).
> >>
> >> If you're less bored, just hardcode this to 1 in vc4 and done. Plus a
> >> TODO.rst entry would be great in that case.
> >
> > If I'm driving a GPU with no display subsystem at all, it seems like I
> > shouldn't initialize fbdev for it, right?
> 
> That's what we do for radeon/amdgpu on hw without display blocks.

Yeah I got slightly distracted with the error :-)

Still might make sense to put this logic into the helpers, if there's no
crtc then don't bother initializing the fbcon. But imo the better cleanup
is getting rid of the connectors parameter, since most drivers get it
wrong, then respin the vc4 patch to explain what it's really doing (it's
not about the error really, it's about not registering and fbdev that's
not doing anything).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* [PATCH] ARM: dts: omap4-droid4: Add isl29030 ALS/proximity sensor
From: Sebastian Reichel @ 2017-05-02  8:03 UTC (permalink / raw)
  To: Sebastian Reichel, Tony Lindgren
  Cc: Rob Herring, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Sebastian Reichel

The Droid 4 has a isl29030 to measure ambient light (e.g. for
automatically adapting display brightness) and proximity.

Signed-off-by: Sebastian Reichel <sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
---
Hi,

The (trivial) driver changes have already been queued to iio-testing:

https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=testing&id=e530cc8408770e6451e4d5c5e8a2a45132689cc1

-- Sebastian
---
 arch/arm/boot/dts/omap4-droid4-xt894.dts | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/omap4-droid4-xt894.dts b/arch/arm/boot/dts/omap4-droid4-xt894.dts
index 89eb607f4a9e..1ad442ee6f30 100644
--- a/arch/arm/boot/dts/omap4-droid4-xt894.dts
+++ b/arch/arm/boot/dts/omap4-droid4-xt894.dts
@@ -348,6 +348,17 @@
 		interrupt-names = "irq", "wakeup";
 		wakeup-source;
 	};
+
+	isl29030@44 {
+		compatible = "isil,isl29030";
+		reg = <0x44>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&als_proximity_pins>;
+
+		interrupt-parent = <&gpio6>;
+		interrupts = <17 IRQ_TYPE_LEVEL_LOW>; /* gpio177 */
+	};
 };
 
 &omap4_pmx_core {
@@ -395,6 +406,12 @@
 		>;
 	};
 
+	als_proximity_pins: pinmux_als_proximity_pins {
+		pinctrl-single,pins = <
+		OMAP4_IOPAD(0x18c, PIN_INPUT_PULLUP | MUX_MODE3)
+		>;
+	};
+
 	usb_ulpi_pins: pinmux_usb_ulpi_pins {
 		pinctrl-single,pins = <
 		OMAP4_IOPAD(0x196, MUX_MODE7)
-- 
2.11.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 v4 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Benjamin Herrenschmidt @ 2017-05-02  7:59 UTC (permalink / raw)
  To: Joel Stanley, Greg Kroah-Hartman, Jiri Slaby
  Cc: Jeremy Kerr, linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Andy Shevchenko, Mark Rutland,
	Rob Herring, openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <20170502074543.1380-3-joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>

On Tue, 2017-05-02 at 17:15 +0930, Joel Stanley wrote:
> From: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> 
> This change adds a driver for the 16550-based Aspeed virtual UART
> device. We use a similar process to the of_serial driver for device
> probe, but expose some VUART-specific functions through sysfs too.
> 
> The VUART is two UART 'front ends' connected by their FIFO (no actual
> serial line in between). One is on the BMC side (management controller)
> and one is on the host CPU side.
> 
> This driver is for the BMC side. The sysfs files allow the BMC
> userspace, which owns the system configuration policy, to specify at
> what IO port and interrupt number the host side will appear to the host
> on the Host <-> BMC LPC bus. It could be different on a different system
> (though most of them use 3f8/4).
> 
> OpenPOWER host firmware doesn't like it when the host-side of the
> VUART's FIFO is not drained. This driver only disables host TX discard
> mode when the port is in use. We set the VUART enabled bit when we bind
> to the device, and clear it on unbind.
> 
> We don't want to do this on open/release, as the host may be using this
> bit to configure serial output modes, which is independent of whether
> the devices has been opened by BMC userspace.
> 
> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Reviewed-by: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>

> ---
> v4:
>  - Reorder if statements
>  - Remove uncessary comment
> 
> v3:
>  - remove whitespace in header
>  - reformat comment
>  - don't check for reg-io-width property; we don't need any special
>    accessors for reading/writing bytes
>  - move file to 8250_aspeed_vuart.c
> 
> v2:
>  - Use attribute groups and DEVICE_ATTR_RW
>  - Use platform_get_resource/devm_ioremap_resource
>  - of_find_property -> of_property_read_bool
>  - Drop unncessary 0xff mask
>  - Fix comment style
>  - Use BIT and GENMASK where pssible
>  - Move to 8250 directory
>  - Rename ast -> aspeed to match other Aspeed drivers
>  - Add documentation of sysfs file
>  - Add detail to the commit message
> 
>  Documentation/ABI/stable/sysfs-driver-aspeed-vuart |  15 +
>  Documentation/devicetree/bindings/serial/8250.txt  |   2 +
>  drivers/tty/serial/8250/8250_aspeed_vuart.c        | 323 +++++++++++++++++++++
>  drivers/tty/serial/8250/Kconfig                    |  10 +
>  drivers/tty/serial/8250/Makefile                   |   1 +
>  5 files changed, 351 insertions(+)
>  create mode 100644 Documentation/ABI/stable/sysfs-driver-aspeed-vuart
>  create mode 100644 drivers/tty/serial/8250/8250_aspeed_vuart.c
> 
> diff --git a/Documentation/ABI/stable/sysfs-driver-aspeed-vuart b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
> new file mode 100644
> index 000000000000..8062953ce77b
> --- /dev/null
> +++ b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
> @@ -0,0 +1,15 @@
> +What:		/sys/bus/platform/drivers/aspeed-vuart/*/lpc_address
> +Date:		April 2017
> +Contact:	Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> +Description:	Configures which IO port the host side of the UART
> +		will appear on the host <-> BMC LPC bus.
> +Users:		OpenBMC.  Proposed changes should be mailed to
> +		openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> +
> +What:		/sys/bus/platform/drivers/aspeed-vuart*/sirq
> +Date:		April 2017
> +Contact:	Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> +Description:	Configures which interrupt number the host side of
> +		the UART will appear on the host <-> BMC LPC bus.
> +Users:		OpenBMC.  Proposed changes should be mailed to
> +		openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
> index 10276a46ecef..656733949309 100644
> --- a/Documentation/devicetree/bindings/serial/8250.txt
> +++ b/Documentation/devicetree/bindings/serial/8250.txt
> @@ -20,6 +20,8 @@ Required properties:
>  	- "fsl,16550-FIFO64"
>  	- "fsl,ns16550"
>  	- "ti,da830-uart"
> +	- "aspeed,ast2400-vuart"
> +	- "aspeed,ast2500-vuart"
>  	- "serial" if the port type is unknown.
>  - reg : offset and length of the register set for the device.
>  - interrupts : should contain uart interrupt.
> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> new file mode 100644
> index 000000000000..822be4906763
> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> @@ -0,0 +1,323 @@
> +/*
> + *  Serial Port driver for Aspeed VUART device
> + *
> + *    Copyright (C) 2016 Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>, IBM Corp.
> + *    Copyright (C) 2006 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>, IBM Corp.
> + *
> + *  This program is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU General Public License
> + *  as published by the Free Software Foundation; either version
> + *  2 of the License, or (at your option) any later version.
> + */
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/clk.h>
> +
> +#include "8250.h"
> +
> +#define ASPEED_VUART_GCRA		0x20
> +#define ASPEED_VUART_GCRA_VUART_EN		BIT(0)
> +#define ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD BIT(5)
> +#define ASPEED_VUART_GCRB		0x24
> +#define ASPEED_VUART_GCRB_HOST_SIRQ_MASK	GENMASK(7, 4)
> +#define ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT	4
> +#define ASPEED_VUART_ADDRL		0x28
> +#define ASPEED_VUART_ADDRH		0x2c
> +
> +struct aspeed_vuart {
> +	struct device		*dev;
> +	void __iomem		*regs;
> +	struct clk		*clk;
> +	int			line;
> +};
> +
> +/*
> + * The VUART is basically two UART 'front ends' connected by their FIFO
> + * (no actual serial line in between). One is on the BMC side (management
> + * controller) and one is on the host CPU side.
> + *
> + * It allows the BMC to provide to the host a "UART" that pipes into
> + * the BMC itself and can then be turned by the BMC into a network console
> + * of some sort for example.
> + *
> + * This driver is for the BMC side. The sysfs files allow the BMC
> + * userspace which owns the system configuration policy, to specify
> + * at what IO port and interrupt number the host side will appear
> + * to the host on the Host <-> BMC LPC bus. It could be different on a
> + * different system (though most of them use 3f8/4).
> + */
> +
> +static ssize_t lpc_address_show(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> +	u16 addr;
> +
> +	addr = (readb(vuart->regs + ASPEED_VUART_ADDRH) << 8) |
> +		(readb(vuart->regs + ASPEED_VUART_ADDRL));
> +
> +	return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
> +}
> +
> +static ssize_t lpc_address_store(struct device *dev,
> +				 struct device_attribute *attr,
> +				 const char *buf, size_t count)
> +{
> +	struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> +	unsigned long val;
> +	int err;
> +
> +	err = kstrtoul(buf, 0, &val);
> +	if (err)
> +		return err;
> +
> +	writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
> +	writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR_RW(lpc_address);
> +
> +static ssize_t sirq_show(struct device *dev,
> +			 struct device_attribute *attr, char *buf)
> +{
> +	struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> +	u8 reg;
> +
> +	reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> +	reg &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> +	reg >>= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> +
> +	return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
> +}
> +
> +static ssize_t sirq_store(struct device *dev, struct device_attribute *attr,
> +			  const char *buf, size_t count)
> +{
> +	struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> +	unsigned long val;
> +	int err;
> +	u8 reg;
> +
> +	err = kstrtoul(buf, 0, &val);
> +	if (err)
> +		return err;
> +
> +	val <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> +	val &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> +
> +	reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> +	reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> +	reg |= val;
> +	writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR_RW(sirq);
> +
> +static struct attribute *aspeed_vuart_attrs[] = {
> +	&dev_attr_sirq.attr,
> +	&dev_attr_lpc_address.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group aspeed_vuart_attr_group = {
> +	.attrs = aspeed_vuart_attrs,
> +};
> +
> +static void aspeed_vuart_set_enabled(struct aspeed_vuart *vuart, bool enabled)
> +{
> +	u8 reg = readb(vuart->regs + ASPEED_VUART_GCRA);
> +
> +	if (enabled)
> +		reg |= ASPEED_VUART_GCRA_VUART_EN;
> +	else
> +		reg &= ~ASPEED_VUART_GCRA_VUART_EN;
> +
> +	writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
> +}
> +
> +static void aspeed_vuart_set_host_tx_discard(struct aspeed_vuart *vuart,
> +					     bool discard)
> +{
> +	u8 reg;
> +
> +	reg = readb(vuart->regs + ASPEED_VUART_GCRA);
> +
> +	/* If the DISABLE_HOST_TX_DISCARD bit is set, discard is disabled */
> +	if (!discard)
> +		reg |= ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
> +	else
> +		reg &= ~ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
> +
> +	writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
> +}
> +
> +static int aspeed_vuart_startup(struct uart_port *uart_port)
> +{
> +	struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
> +	struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
> +	int rc;
> +
> +	rc = serial8250_do_startup(uart_port);
> +	if (rc)
> +		return rc;
> +
> +	aspeed_vuart_set_host_tx_discard(vuart, false);
> +
> +	return 0;
> +}
> +
> +static void aspeed_vuart_shutdown(struct uart_port *uart_port)
> +{
> +	struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
> +	struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
> +
> +	aspeed_vuart_set_host_tx_discard(vuart, true);
> +
> +	serial8250_do_shutdown(uart_port);
> +}
> +
> +static int aspeed_vuart_probe(struct platform_device *pdev)
> +{
> +	struct uart_8250_port port;
> +	struct aspeed_vuart *vuart;
> +	struct device_node *np;
> +	struct resource *res;
> +	u32 clk, prop;
> +	int rc;
> +
> +	np = pdev->dev.of_node;
> +
> +	vuart = devm_kzalloc(&pdev->dev, sizeof(*vuart), GFP_KERNEL);
> +	if (!vuart)
> +		return -ENOMEM;
> +
> +	vuart->dev = &pdev->dev;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	vuart->regs = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(vuart->regs))
> +		return PTR_ERR(vuart->regs);
> +
> +	memset(&port, 0, sizeof(port));
> +	port.port.private_data = vuart;
> +	port.port.membase = vuart->regs;
> +	port.port.mapbase = res->start;
> +	port.port.mapsize = resource_size(res);
> +	port.port.startup = aspeed_vuart_startup;
> +	port.port.shutdown = aspeed_vuart_shutdown;
> +	port.port.dev = &pdev->dev;
> +
> +	rc = sysfs_create_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
> +	if (rc < 0)
> +		return rc;
> +
> +	if (of_property_read_u32(np, "clock-frequency", &clk)) {
> +		vuart->clk = devm_clk_get(&pdev->dev, NULL);
> +		if (IS_ERR(vuart->clk)) {
> +			dev_warn(&pdev->dev,
> +				"clk or clock-frequency not defined\n");
> +			return PTR_ERR(vuart->clk);
> +		}
> +
> +		rc = clk_prepare_enable(vuart->clk);
> +		if (rc < 0)
> +			return rc;
> +
> +		clk = clk_get_rate(vuart->clk);
> +	}
> +
> +	/* If current-speed was set, then try not to change it. */
> +	if (of_property_read_u32(np, "current-speed", &prop) == 0)
> +		port.port.custom_divisor = clk / (16 * prop);
> +
> +	/* Check for shifted address mapping */
> +	if (of_property_read_u32(np, "reg-offset", &prop) == 0)
> +		port.port.mapbase += prop;
> +
> +	/* Check for registers offset within the devices address range */
> +	if (of_property_read_u32(np, "reg-shift", &prop) == 0)
> +		port.port.regshift = prop;
> +
> +	/* Check for fifo size */
> +	if (of_property_read_u32(np, "fifo-size", &prop) == 0)
> +		port.port.fifosize = prop;
> +
> +	/* Check for a fixed line number */
> +	rc = of_alias_get_id(np, "serial");
> +	if (rc >= 0)
> +		port.port.line = rc;
> +
> +	port.port.irq = irq_of_parse_and_map(np, 0);
> +	port.port.irqflags = IRQF_SHARED;
> +	port.port.iotype = UPIO_MEM;
> +	port.port.type = PORT_16550A;
> +	port.port.uartclk = clk;
> +	port.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF
> +		| UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_NO_THRE_TEST;
> +
> +	if (of_property_read_bool(np, "no-loopback-test"))
> +		port.port.flags |= UPF_SKIP_TEST;
> +
> +	if (port.port.fifosize)
> +		port.capabilities = UART_CAP_FIFO;
> +
> +	if (of_property_read_bool(np, "auto-flow-control"))
> +		port.capabilities |= UART_CAP_AFE;
> +
> +	rc = serial8250_register_8250_port(&port);
> +	if (rc < 0)
> +		goto err_clk_disable;
> +
> +	vuart->line = rc;
> +
> +	aspeed_vuart_set_enabled(vuart, true);
> +	aspeed_vuart_set_host_tx_discard(vuart, true);
> +	platform_set_drvdata(pdev, vuart);
> +
> +	return 0;
> +
> +err_clk_disable:
> +	clk_disable_unprepare(vuart->clk);
> +	irq_dispose_mapping(port.port.irq);
> +	return rc;
> +}
> +
> +static int aspeed_vuart_remove(struct platform_device *pdev)
> +{
> +	struct aspeed_vuart *vuart = platform_get_drvdata(pdev);
> +
> +	aspeed_vuart_set_enabled(vuart, false);
> +	serial8250_unregister_port(vuart->line);
> +	sysfs_remove_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
> +	clk_disable_unprepare(vuart->clk);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id aspeed_vuart_table[] = {
> +	{ .compatible = "aspeed,ast2400-vuart" },
> +	{ .compatible = "aspeed,ast2500-vuart" },
> +	{ },
> +};
> +
> +static struct platform_driver aspeed_vuart_driver = {
> +	.driver = {
> +		.name = "aspeed-vuart",
> +		.of_match_table = aspeed_vuart_table,
> +	},
> +	.probe = aspeed_vuart_probe,
> +	.remove = aspeed_vuart_remove,
> +};
> +
> +module_platform_driver(aspeed_vuart_driver);
> +
> +MODULE_AUTHOR("Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Driver for Aspeed VUART device");
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index 0e3f529d50e9..a1161ec0256f 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -224,6 +224,16 @@ config SERIAL_8250_ACCENT
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called 8250_accent.
>  
> +config SERIAL_8250_ASPEED_VUART
> +	tristate "Aspeed Virtual UART"
> +	depends on SERIAL_8250
> +	depends on OF
> +	help
> +	  If you want to use the virtual UART (VUART) device on Aspeed
> +	  BMC platforms, enable this option. This enables the 16550A-
> +	  compatible device on the local LPC bus, giving a UART device
> +	  with no physical RS232 connections.
> +
>  config SERIAL_8250_BOCA
>  	tristate "Support Boca cards"
>  	depends on SERIAL_8250 != n && ISA && SERIAL_8250_MANY_PORTS
> diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
> index 2f30f9ecdb1b..a44a99a3e623 100644
> --- a/drivers/tty/serial/8250/Makefile
> +++ b/drivers/tty/serial/8250/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_SERIAL_8250_EXAR)		+= 8250_exar.o
>  obj-$(CONFIG_SERIAL_8250_HP300)		+= 8250_hp300.o
>  obj-$(CONFIG_SERIAL_8250_CS)		+= serial_cs.o
>  obj-$(CONFIG_SERIAL_8250_ACORN)		+= 8250_acorn.o
> +obj-$(CONFIG_SERIAL_8250_ASPEED_VUART)	+= 8250_aspeed_vuart.o
>  obj-$(CONFIG_SERIAL_8250_BCM2835AUX)	+= 8250_bcm2835aux.o
>  obj-$(CONFIG_SERIAL_8250_CONSOLE)	+= 8250_early.o
>  obj-$(CONFIG_SERIAL_8250_FOURPORT)	+= 8250_fourport.o
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v4 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Andy Shevchenko @ 2017-05-02  7:51 UTC (permalink / raw)
  To: Joel Stanley
  Cc: Greg Kroah-Hartman, Jiri Slaby, Jeremy Kerr,
	linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree,
	Benjamin Herrenschmidt, Mark Rutland, Rob Herring,
	openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <20170502074543.1380-3-joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>

On Tue, May 2, 2017 at 10:45 AM, Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org> wrote:
> From: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
>
> This change adds a driver for the 16550-based Aspeed virtual UART
> device. We use a similar process to the of_serial driver for device
> probe, but expose some VUART-specific functions through sysfs too.
>
> The VUART is two UART 'front ends' connected by their FIFO (no actual
> serial line in between). One is on the BMC side (management controller)
> and one is on the host CPU side.
>
> This driver is for the BMC side. The sysfs files allow the BMC
> userspace, which owns the system configuration policy, to specify at
> what IO port and interrupt number the host side will appear to the host
> on the Host <-> BMC LPC bus. It could be different on a different system
> (though most of them use 3f8/4).
>
> OpenPOWER host firmware doesn't like it when the host-side of the
> VUART's FIFO is not drained. This driver only disables host TX discard
> mode when the port is in use. We set the VUART enabled bit when we bind
> to the device, and clear it on unbind.
>
> We don't want to do this on open/release, as the host may be using this
> bit to configure serial output modes, which is independent of whether
> the devices has been opened by BMC userspace.
>
> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>

I have reviewed the driver and found it's quality good, though one
minor about IRQ getting is left untouched. Let's Greg or others to
decide, from my side FWIW:
Reviewed-by: Andy Shevchenko <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

> ---
> v4:
>  - Reorder if statements
>  - Remove uncessary comment
>
> v3:
>  - remove whitespace in header
>  - reformat comment
>  - don't check for reg-io-width property; we don't need any special
>    accessors for reading/writing bytes
>  - move file to 8250_aspeed_vuart.c
>
> v2:
>  - Use attribute groups and DEVICE_ATTR_RW
>  - Use platform_get_resource/devm_ioremap_resource
>  - of_find_property -> of_property_read_bool
>  - Drop unncessary 0xff mask
>  - Fix comment style
>  - Use BIT and GENMASK where pssible
>  - Move to 8250 directory
>  - Rename ast -> aspeed to match other Aspeed drivers
>  - Add documentation of sysfs file
>  - Add detail to the commit message
>
>  Documentation/ABI/stable/sysfs-driver-aspeed-vuart |  15 +
>  Documentation/devicetree/bindings/serial/8250.txt  |   2 +
>  drivers/tty/serial/8250/8250_aspeed_vuart.c        | 323 +++++++++++++++++++++
>  drivers/tty/serial/8250/Kconfig                    |  10 +
>  drivers/tty/serial/8250/Makefile                   |   1 +
>  5 files changed, 351 insertions(+)
>  create mode 100644 Documentation/ABI/stable/sysfs-driver-aspeed-vuart
>  create mode 100644 drivers/tty/serial/8250/8250_aspeed_vuart.c
>
> diff --git a/Documentation/ABI/stable/sysfs-driver-aspeed-vuart b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
> new file mode 100644
> index 000000000000..8062953ce77b
> --- /dev/null
> +++ b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
> @@ -0,0 +1,15 @@
> +What:          /sys/bus/platform/drivers/aspeed-vuart/*/lpc_address
> +Date:          April 2017
> +Contact:       Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> +Description:   Configures which IO port the host side of the UART
> +               will appear on the host <-> BMC LPC bus.
> +Users:         OpenBMC.  Proposed changes should be mailed to
> +               openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> +
> +What:          /sys/bus/platform/drivers/aspeed-vuart*/sirq
> +Date:          April 2017
> +Contact:       Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> +Description:   Configures which interrupt number the host side of
> +               the UART will appear on the host <-> BMC LPC bus.
> +Users:         OpenBMC.  Proposed changes should be mailed to
> +               openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
> index 10276a46ecef..656733949309 100644
> --- a/Documentation/devicetree/bindings/serial/8250.txt
> +++ b/Documentation/devicetree/bindings/serial/8250.txt
> @@ -20,6 +20,8 @@ Required properties:
>         - "fsl,16550-FIFO64"
>         - "fsl,ns16550"
>         - "ti,da830-uart"
> +       - "aspeed,ast2400-vuart"
> +       - "aspeed,ast2500-vuart"
>         - "serial" if the port type is unknown.
>  - reg : offset and length of the register set for the device.
>  - interrupts : should contain uart interrupt.
> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> new file mode 100644
> index 000000000000..822be4906763
> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> @@ -0,0 +1,323 @@
> +/*
> + *  Serial Port driver for Aspeed VUART device
> + *
> + *    Copyright (C) 2016 Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>, IBM Corp.
> + *    Copyright (C) 2006 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>, IBM Corp.
> + *
> + *  This program is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU General Public License
> + *  as published by the Free Software Foundation; either version
> + *  2 of the License, or (at your option) any later version.
> + */
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/clk.h>
> +
> +#include "8250.h"
> +
> +#define ASPEED_VUART_GCRA              0x20
> +#define ASPEED_VUART_GCRA_VUART_EN             BIT(0)
> +#define ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD BIT(5)
> +#define ASPEED_VUART_GCRB              0x24
> +#define ASPEED_VUART_GCRB_HOST_SIRQ_MASK       GENMASK(7, 4)
> +#define ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT      4
> +#define ASPEED_VUART_ADDRL             0x28
> +#define ASPEED_VUART_ADDRH             0x2c
> +
> +struct aspeed_vuart {
> +       struct device           *dev;
> +       void __iomem            *regs;
> +       struct clk              *clk;
> +       int                     line;
> +};
> +
> +/*
> + * The VUART is basically two UART 'front ends' connected by their FIFO
> + * (no actual serial line in between). One is on the BMC side (management
> + * controller) and one is on the host CPU side.
> + *
> + * It allows the BMC to provide to the host a "UART" that pipes into
> + * the BMC itself and can then be turned by the BMC into a network console
> + * of some sort for example.
> + *
> + * This driver is for the BMC side. The sysfs files allow the BMC
> + * userspace which owns the system configuration policy, to specify
> + * at what IO port and interrupt number the host side will appear
> + * to the host on the Host <-> BMC LPC bus. It could be different on a
> + * different system (though most of them use 3f8/4).
> + */
> +
> +static ssize_t lpc_address_show(struct device *dev,
> +                               struct device_attribute *attr, char *buf)
> +{
> +       struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> +       u16 addr;
> +
> +       addr = (readb(vuart->regs + ASPEED_VUART_ADDRH) << 8) |
> +               (readb(vuart->regs + ASPEED_VUART_ADDRL));
> +
> +       return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
> +}
> +
> +static ssize_t lpc_address_store(struct device *dev,
> +                                struct device_attribute *attr,
> +                                const char *buf, size_t count)
> +{
> +       struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> +       unsigned long val;
> +       int err;
> +
> +       err = kstrtoul(buf, 0, &val);
> +       if (err)
> +               return err;
> +
> +       writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
> +       writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
> +
> +       return count;
> +}
> +
> +static DEVICE_ATTR_RW(lpc_address);
> +
> +static ssize_t sirq_show(struct device *dev,
> +                        struct device_attribute *attr, char *buf)
> +{
> +       struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> +       u8 reg;
> +
> +       reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> +       reg &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> +       reg >>= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> +
> +       return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
> +}
> +
> +static ssize_t sirq_store(struct device *dev, struct device_attribute *attr,
> +                         const char *buf, size_t count)
> +{
> +       struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> +       unsigned long val;
> +       int err;
> +       u8 reg;
> +
> +       err = kstrtoul(buf, 0, &val);
> +       if (err)
> +               return err;
> +
> +       val <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> +       val &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> +
> +       reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> +       reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> +       reg |= val;
> +       writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
> +
> +       return count;
> +}
> +
> +static DEVICE_ATTR_RW(sirq);
> +
> +static struct attribute *aspeed_vuart_attrs[] = {
> +       &dev_attr_sirq.attr,
> +       &dev_attr_lpc_address.attr,
> +       NULL,
> +};
> +
> +static const struct attribute_group aspeed_vuart_attr_group = {
> +       .attrs = aspeed_vuart_attrs,
> +};
> +
> +static void aspeed_vuart_set_enabled(struct aspeed_vuart *vuart, bool enabled)
> +{
> +       u8 reg = readb(vuart->regs + ASPEED_VUART_GCRA);
> +
> +       if (enabled)
> +               reg |= ASPEED_VUART_GCRA_VUART_EN;
> +       else
> +               reg &= ~ASPEED_VUART_GCRA_VUART_EN;
> +
> +       writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
> +}
> +
> +static void aspeed_vuart_set_host_tx_discard(struct aspeed_vuart *vuart,
> +                                            bool discard)
> +{
> +       u8 reg;
> +
> +       reg = readb(vuart->regs + ASPEED_VUART_GCRA);
> +
> +       /* If the DISABLE_HOST_TX_DISCARD bit is set, discard is disabled */
> +       if (!discard)
> +               reg |= ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
> +       else
> +               reg &= ~ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
> +
> +       writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
> +}
> +
> +static int aspeed_vuart_startup(struct uart_port *uart_port)
> +{
> +       struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
> +       struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
> +       int rc;
> +
> +       rc = serial8250_do_startup(uart_port);
> +       if (rc)
> +               return rc;
> +
> +       aspeed_vuart_set_host_tx_discard(vuart, false);
> +
> +       return 0;
> +}
> +
> +static void aspeed_vuart_shutdown(struct uart_port *uart_port)
> +{
> +       struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
> +       struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
> +
> +       aspeed_vuart_set_host_tx_discard(vuart, true);
> +
> +       serial8250_do_shutdown(uart_port);
> +}
> +
> +static int aspeed_vuart_probe(struct platform_device *pdev)
> +{
> +       struct uart_8250_port port;
> +       struct aspeed_vuart *vuart;
> +       struct device_node *np;
> +       struct resource *res;
> +       u32 clk, prop;
> +       int rc;
> +
> +       np = pdev->dev.of_node;
> +
> +       vuart = devm_kzalloc(&pdev->dev, sizeof(*vuart), GFP_KERNEL);
> +       if (!vuart)
> +               return -ENOMEM;
> +
> +       vuart->dev = &pdev->dev;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       vuart->regs = devm_ioremap_resource(&pdev->dev, res);
> +       if (IS_ERR(vuart->regs))
> +               return PTR_ERR(vuart->regs);
> +
> +       memset(&port, 0, sizeof(port));
> +       port.port.private_data = vuart;
> +       port.port.membase = vuart->regs;
> +       port.port.mapbase = res->start;
> +       port.port.mapsize = resource_size(res);
> +       port.port.startup = aspeed_vuart_startup;
> +       port.port.shutdown = aspeed_vuart_shutdown;
> +       port.port.dev = &pdev->dev;
> +
> +       rc = sysfs_create_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
> +       if (rc < 0)
> +               return rc;
> +
> +       if (of_property_read_u32(np, "clock-frequency", &clk)) {
> +               vuart->clk = devm_clk_get(&pdev->dev, NULL);
> +               if (IS_ERR(vuart->clk)) {
> +                       dev_warn(&pdev->dev,
> +                               "clk or clock-frequency not defined\n");
> +                       return PTR_ERR(vuart->clk);
> +               }
> +
> +               rc = clk_prepare_enable(vuart->clk);
> +               if (rc < 0)
> +                       return rc;
> +
> +               clk = clk_get_rate(vuart->clk);
> +       }
> +
> +       /* If current-speed was set, then try not to change it. */
> +       if (of_property_read_u32(np, "current-speed", &prop) == 0)
> +               port.port.custom_divisor = clk / (16 * prop);
> +
> +       /* Check for shifted address mapping */
> +       if (of_property_read_u32(np, "reg-offset", &prop) == 0)
> +               port.port.mapbase += prop;
> +
> +       /* Check for registers offset within the devices address range */
> +       if (of_property_read_u32(np, "reg-shift", &prop) == 0)
> +               port.port.regshift = prop;
> +
> +       /* Check for fifo size */
> +       if (of_property_read_u32(np, "fifo-size", &prop) == 0)
> +               port.port.fifosize = prop;
> +
> +       /* Check for a fixed line number */
> +       rc = of_alias_get_id(np, "serial");
> +       if (rc >= 0)
> +               port.port.line = rc;
> +
> +       port.port.irq = irq_of_parse_and_map(np, 0);
> +       port.port.irqflags = IRQF_SHARED;
> +       port.port.iotype = UPIO_MEM;
> +       port.port.type = PORT_16550A;
> +       port.port.uartclk = clk;
> +       port.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF
> +               | UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_NO_THRE_TEST;
> +
> +       if (of_property_read_bool(np, "no-loopback-test"))
> +               port.port.flags |= UPF_SKIP_TEST;
> +
> +       if (port.port.fifosize)
> +               port.capabilities = UART_CAP_FIFO;
> +
> +       if (of_property_read_bool(np, "auto-flow-control"))
> +               port.capabilities |= UART_CAP_AFE;
> +
> +       rc = serial8250_register_8250_port(&port);
> +       if (rc < 0)
> +               goto err_clk_disable;
> +
> +       vuart->line = rc;
> +
> +       aspeed_vuart_set_enabled(vuart, true);
> +       aspeed_vuart_set_host_tx_discard(vuart, true);
> +       platform_set_drvdata(pdev, vuart);
> +
> +       return 0;
> +
> +err_clk_disable:
> +       clk_disable_unprepare(vuart->clk);
> +       irq_dispose_mapping(port.port.irq);
> +       return rc;
> +}
> +
> +static int aspeed_vuart_remove(struct platform_device *pdev)
> +{
> +       struct aspeed_vuart *vuart = platform_get_drvdata(pdev);
> +
> +       aspeed_vuart_set_enabled(vuart, false);
> +       serial8250_unregister_port(vuart->line);
> +       sysfs_remove_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
> +       clk_disable_unprepare(vuart->clk);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id aspeed_vuart_table[] = {
> +       { .compatible = "aspeed,ast2400-vuart" },
> +       { .compatible = "aspeed,ast2500-vuart" },
> +       { },
> +};
> +
> +static struct platform_driver aspeed_vuart_driver = {
> +       .driver = {
> +               .name = "aspeed-vuart",
> +               .of_match_table = aspeed_vuart_table,
> +       },
> +       .probe = aspeed_vuart_probe,
> +       .remove = aspeed_vuart_remove,
> +};
> +
> +module_platform_driver(aspeed_vuart_driver);
> +
> +MODULE_AUTHOR("Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Driver for Aspeed VUART device");
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index 0e3f529d50e9..a1161ec0256f 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -224,6 +224,16 @@ config SERIAL_8250_ACCENT
>           To compile this driver as a module, choose M here: the module
>           will be called 8250_accent.
>
> +config SERIAL_8250_ASPEED_VUART
> +       tristate "Aspeed Virtual UART"
> +       depends on SERIAL_8250
> +       depends on OF
> +       help
> +         If you want to use the virtual UART (VUART) device on Aspeed
> +         BMC platforms, enable this option. This enables the 16550A-
> +         compatible device on the local LPC bus, giving a UART device
> +         with no physical RS232 connections.
> +
>  config SERIAL_8250_BOCA
>         tristate "Support Boca cards"
>         depends on SERIAL_8250 != n && ISA && SERIAL_8250_MANY_PORTS
> diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
> index 2f30f9ecdb1b..a44a99a3e623 100644
> --- a/drivers/tty/serial/8250/Makefile
> +++ b/drivers/tty/serial/8250/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_SERIAL_8250_EXAR)                += 8250_exar.o
>  obj-$(CONFIG_SERIAL_8250_HP300)                += 8250_hp300.o
>  obj-$(CONFIG_SERIAL_8250_CS)           += serial_cs.o
>  obj-$(CONFIG_SERIAL_8250_ACORN)                += 8250_acorn.o
> +obj-$(CONFIG_SERIAL_8250_ASPEED_VUART) += 8250_aspeed_vuart.o
>  obj-$(CONFIG_SERIAL_8250_BCM2835AUX)   += 8250_bcm2835aux.o
>  obj-$(CONFIG_SERIAL_8250_CONSOLE)      += 8250_early.o
>  obj-$(CONFIG_SERIAL_8250_FOURPORT)     += 8250_fourport.o
> --
> 2.11.0
>



-- 
With Best Regards,
Andy Shevchenko
--
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 v4 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Joel Stanley @ 2017-05-02  7:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: Jeremy Kerr, linux-serial, linux-kernel, devicetree,
	Andy Shevchenko, Benjamin Herrenschmidt, Mark Rutland,
	Rob Herring, openbmc
In-Reply-To: <20170502074543.1380-1-joel@jms.id.au>

From: Jeremy Kerr <jk@ozlabs.org>

This change adds a driver for the 16550-based Aspeed virtual UART
device. We use a similar process to the of_serial driver for device
probe, but expose some VUART-specific functions through sysfs too.

The VUART is two UART 'front ends' connected by their FIFO (no actual
serial line in between). One is on the BMC side (management controller)
and one is on the host CPU side.

This driver is for the BMC side. The sysfs files allow the BMC
userspace, which owns the system configuration policy, to specify at
what IO port and interrupt number the host side will appear to the host
on the Host <-> BMC LPC bus. It could be different on a different system
(though most of them use 3f8/4).

OpenPOWER host firmware doesn't like it when the host-side of the
VUART's FIFO is not drained. This driver only disables host TX discard
mode when the port is in use. We set the VUART enabled bit when we bind
to the device, and clear it on unbind.

We don't want to do this on open/release, as the host may be using this
bit to configure serial output modes, which is independent of whether
the devices has been opened by BMC userspace.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>
Acked-by: Rob Herring <robh@kernel.org>

---
v4:
 - Reorder if statements
 - Remove uncessary comment

v3:
 - remove whitespace in header
 - reformat comment
 - don't check for reg-io-width property; we don't need any special
   accessors for reading/writing bytes
 - move file to 8250_aspeed_vuart.c

v2:
 - Use attribute groups and DEVICE_ATTR_RW
 - Use platform_get_resource/devm_ioremap_resource
 - of_find_property -> of_property_read_bool
 - Drop unncessary 0xff mask
 - Fix comment style
 - Use BIT and GENMASK where pssible
 - Move to 8250 directory
 - Rename ast -> aspeed to match other Aspeed drivers
 - Add documentation of sysfs file
 - Add detail to the commit message

 Documentation/ABI/stable/sysfs-driver-aspeed-vuart |  15 +
 Documentation/devicetree/bindings/serial/8250.txt  |   2 +
 drivers/tty/serial/8250/8250_aspeed_vuart.c        | 323 +++++++++++++++++++++
 drivers/tty/serial/8250/Kconfig                    |  10 +
 drivers/tty/serial/8250/Makefile                   |   1 +
 5 files changed, 351 insertions(+)
 create mode 100644 Documentation/ABI/stable/sysfs-driver-aspeed-vuart
 create mode 100644 drivers/tty/serial/8250/8250_aspeed_vuart.c

diff --git a/Documentation/ABI/stable/sysfs-driver-aspeed-vuart b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
new file mode 100644
index 000000000000..8062953ce77b
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
@@ -0,0 +1,15 @@
+What:		/sys/bus/platform/drivers/aspeed-vuart/*/lpc_address
+Date:		April 2017
+Contact:	Jeremy Kerr <jk@ozlabs.org>
+Description:	Configures which IO port the host side of the UART
+		will appear on the host <-> BMC LPC bus.
+Users:		OpenBMC.  Proposed changes should be mailed to
+		openbmc@lists.ozlabs.org
+
+What:		/sys/bus/platform/drivers/aspeed-vuart*/sirq
+Date:		April 2017
+Contact:	Jeremy Kerr <jk@ozlabs.org>
+Description:	Configures which interrupt number the host side of
+		the UART will appear on the host <-> BMC LPC bus.
+Users:		OpenBMC.  Proposed changes should be mailed to
+		openbmc@lists.ozlabs.org
diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
index 10276a46ecef..656733949309 100644
--- a/Documentation/devicetree/bindings/serial/8250.txt
+++ b/Documentation/devicetree/bindings/serial/8250.txt
@@ -20,6 +20,8 @@ Required properties:
 	- "fsl,16550-FIFO64"
 	- "fsl,ns16550"
 	- "ti,da830-uart"
+	- "aspeed,ast2400-vuart"
+	- "aspeed,ast2500-vuart"
 	- "serial" if the port type is unknown.
 - reg : offset and length of the register set for the device.
 - interrupts : should contain uart interrupt.
diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
new file mode 100644
index 000000000000..822be4906763
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -0,0 +1,323 @@
+/*
+ *  Serial Port driver for Aspeed VUART device
+ *
+ *    Copyright (C) 2016 Jeremy Kerr <jk@ozlabs.org>, IBM Corp.
+ *    Copyright (C) 2006 Arnd Bergmann <arnd@arndb.de>, IBM Corp.
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/clk.h>
+
+#include "8250.h"
+
+#define ASPEED_VUART_GCRA		0x20
+#define ASPEED_VUART_GCRA_VUART_EN		BIT(0)
+#define ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD BIT(5)
+#define ASPEED_VUART_GCRB		0x24
+#define ASPEED_VUART_GCRB_HOST_SIRQ_MASK	GENMASK(7, 4)
+#define ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT	4
+#define ASPEED_VUART_ADDRL		0x28
+#define ASPEED_VUART_ADDRH		0x2c
+
+struct aspeed_vuart {
+	struct device		*dev;
+	void __iomem		*regs;
+	struct clk		*clk;
+	int			line;
+};
+
+/*
+ * The VUART is basically two UART 'front ends' connected by their FIFO
+ * (no actual serial line in between). One is on the BMC side (management
+ * controller) and one is on the host CPU side.
+ *
+ * It allows the BMC to provide to the host a "UART" that pipes into
+ * the BMC itself and can then be turned by the BMC into a network console
+ * of some sort for example.
+ *
+ * This driver is for the BMC side. The sysfs files allow the BMC
+ * userspace which owns the system configuration policy, to specify
+ * at what IO port and interrupt number the host side will appear
+ * to the host on the Host <-> BMC LPC bus. It could be different on a
+ * different system (though most of them use 3f8/4).
+ */
+
+static ssize_t lpc_address_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+	u16 addr;
+
+	addr = (readb(vuart->regs + ASPEED_VUART_ADDRH) << 8) |
+		(readb(vuart->regs + ASPEED_VUART_ADDRL));
+
+	return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
+}
+
+static ssize_t lpc_address_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+	unsigned long val;
+	int err;
+
+	err = kstrtoul(buf, 0, &val);
+	if (err)
+		return err;
+
+	writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
+	writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(lpc_address);
+
+static ssize_t sirq_show(struct device *dev,
+			 struct device_attribute *attr, char *buf)
+{
+	struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+	u8 reg;
+
+	reg = readb(vuart->regs + ASPEED_VUART_GCRB);
+	reg &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+	reg >>= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
+
+	return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
+}
+
+static ssize_t sirq_store(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t count)
+{
+	struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+	unsigned long val;
+	int err;
+	u8 reg;
+
+	err = kstrtoul(buf, 0, &val);
+	if (err)
+		return err;
+
+	val <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
+	val &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+
+	reg = readb(vuart->regs + ASPEED_VUART_GCRB);
+	reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+	reg |= val;
+	writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(sirq);
+
+static struct attribute *aspeed_vuart_attrs[] = {
+	&dev_attr_sirq.attr,
+	&dev_attr_lpc_address.attr,
+	NULL,
+};
+
+static const struct attribute_group aspeed_vuart_attr_group = {
+	.attrs = aspeed_vuart_attrs,
+};
+
+static void aspeed_vuart_set_enabled(struct aspeed_vuart *vuart, bool enabled)
+{
+	u8 reg = readb(vuart->regs + ASPEED_VUART_GCRA);
+
+	if (enabled)
+		reg |= ASPEED_VUART_GCRA_VUART_EN;
+	else
+		reg &= ~ASPEED_VUART_GCRA_VUART_EN;
+
+	writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
+}
+
+static void aspeed_vuart_set_host_tx_discard(struct aspeed_vuart *vuart,
+					     bool discard)
+{
+	u8 reg;
+
+	reg = readb(vuart->regs + ASPEED_VUART_GCRA);
+
+	/* If the DISABLE_HOST_TX_DISCARD bit is set, discard is disabled */
+	if (!discard)
+		reg |= ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
+	else
+		reg &= ~ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
+
+	writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
+}
+
+static int aspeed_vuart_startup(struct uart_port *uart_port)
+{
+	struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
+	struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
+	int rc;
+
+	rc = serial8250_do_startup(uart_port);
+	if (rc)
+		return rc;
+
+	aspeed_vuart_set_host_tx_discard(vuart, false);
+
+	return 0;
+}
+
+static void aspeed_vuart_shutdown(struct uart_port *uart_port)
+{
+	struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
+	struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
+
+	aspeed_vuart_set_host_tx_discard(vuart, true);
+
+	serial8250_do_shutdown(uart_port);
+}
+
+static int aspeed_vuart_probe(struct platform_device *pdev)
+{
+	struct uart_8250_port port;
+	struct aspeed_vuart *vuart;
+	struct device_node *np;
+	struct resource *res;
+	u32 clk, prop;
+	int rc;
+
+	np = pdev->dev.of_node;
+
+	vuart = devm_kzalloc(&pdev->dev, sizeof(*vuart), GFP_KERNEL);
+	if (!vuart)
+		return -ENOMEM;
+
+	vuart->dev = &pdev->dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	vuart->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(vuart->regs))
+		return PTR_ERR(vuart->regs);
+
+	memset(&port, 0, sizeof(port));
+	port.port.private_data = vuart;
+	port.port.membase = vuart->regs;
+	port.port.mapbase = res->start;
+	port.port.mapsize = resource_size(res);
+	port.port.startup = aspeed_vuart_startup;
+	port.port.shutdown = aspeed_vuart_shutdown;
+	port.port.dev = &pdev->dev;
+
+	rc = sysfs_create_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
+	if (rc < 0)
+		return rc;
+
+	if (of_property_read_u32(np, "clock-frequency", &clk)) {
+		vuart->clk = devm_clk_get(&pdev->dev, NULL);
+		if (IS_ERR(vuart->clk)) {
+			dev_warn(&pdev->dev,
+				"clk or clock-frequency not defined\n");
+			return PTR_ERR(vuart->clk);
+		}
+
+		rc = clk_prepare_enable(vuart->clk);
+		if (rc < 0)
+			return rc;
+
+		clk = clk_get_rate(vuart->clk);
+	}
+
+	/* If current-speed was set, then try not to change it. */
+	if (of_property_read_u32(np, "current-speed", &prop) == 0)
+		port.port.custom_divisor = clk / (16 * prop);
+
+	/* Check for shifted address mapping */
+	if (of_property_read_u32(np, "reg-offset", &prop) == 0)
+		port.port.mapbase += prop;
+
+	/* Check for registers offset within the devices address range */
+	if (of_property_read_u32(np, "reg-shift", &prop) == 0)
+		port.port.regshift = prop;
+
+	/* Check for fifo size */
+	if (of_property_read_u32(np, "fifo-size", &prop) == 0)
+		port.port.fifosize = prop;
+
+	/* Check for a fixed line number */
+	rc = of_alias_get_id(np, "serial");
+	if (rc >= 0)
+		port.port.line = rc;
+
+	port.port.irq = irq_of_parse_and_map(np, 0);
+	port.port.irqflags = IRQF_SHARED;
+	port.port.iotype = UPIO_MEM;
+	port.port.type = PORT_16550A;
+	port.port.uartclk = clk;
+	port.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF
+		| UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_NO_THRE_TEST;
+
+	if (of_property_read_bool(np, "no-loopback-test"))
+		port.port.flags |= UPF_SKIP_TEST;
+
+	if (port.port.fifosize)
+		port.capabilities = UART_CAP_FIFO;
+
+	if (of_property_read_bool(np, "auto-flow-control"))
+		port.capabilities |= UART_CAP_AFE;
+
+	rc = serial8250_register_8250_port(&port);
+	if (rc < 0)
+		goto err_clk_disable;
+
+	vuart->line = rc;
+
+	aspeed_vuart_set_enabled(vuart, true);
+	aspeed_vuart_set_host_tx_discard(vuart, true);
+	platform_set_drvdata(pdev, vuart);
+
+	return 0;
+
+err_clk_disable:
+	clk_disable_unprepare(vuart->clk);
+	irq_dispose_mapping(port.port.irq);
+	return rc;
+}
+
+static int aspeed_vuart_remove(struct platform_device *pdev)
+{
+	struct aspeed_vuart *vuart = platform_get_drvdata(pdev);
+
+	aspeed_vuart_set_enabled(vuart, false);
+	serial8250_unregister_port(vuart->line);
+	sysfs_remove_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
+	clk_disable_unprepare(vuart->clk);
+
+	return 0;
+}
+
+static const struct of_device_id aspeed_vuart_table[] = {
+	{ .compatible = "aspeed,ast2400-vuart" },
+	{ .compatible = "aspeed,ast2500-vuart" },
+	{ },
+};
+
+static struct platform_driver aspeed_vuart_driver = {
+	.driver = {
+		.name = "aspeed-vuart",
+		.of_match_table = aspeed_vuart_table,
+	},
+	.probe = aspeed_vuart_probe,
+	.remove = aspeed_vuart_remove,
+};
+
+module_platform_driver(aspeed_vuart_driver);
+
+MODULE_AUTHOR("Jeremy Kerr <jk@ozlabs.org>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Driver for Aspeed VUART device");
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index 0e3f529d50e9..a1161ec0256f 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -224,6 +224,16 @@ config SERIAL_8250_ACCENT
 	  To compile this driver as a module, choose M here: the module
 	  will be called 8250_accent.
 
+config SERIAL_8250_ASPEED_VUART
+	tristate "Aspeed Virtual UART"
+	depends on SERIAL_8250
+	depends on OF
+	help
+	  If you want to use the virtual UART (VUART) device on Aspeed
+	  BMC platforms, enable this option. This enables the 16550A-
+	  compatible device on the local LPC bus, giving a UART device
+	  with no physical RS232 connections.
+
 config SERIAL_8250_BOCA
 	tristate "Support Boca cards"
 	depends on SERIAL_8250 != n && ISA && SERIAL_8250_MANY_PORTS
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index 2f30f9ecdb1b..a44a99a3e623 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_SERIAL_8250_EXAR)		+= 8250_exar.o
 obj-$(CONFIG_SERIAL_8250_HP300)		+= 8250_hp300.o
 obj-$(CONFIG_SERIAL_8250_CS)		+= serial_cs.o
 obj-$(CONFIG_SERIAL_8250_ACORN)		+= 8250_acorn.o
+obj-$(CONFIG_SERIAL_8250_ASPEED_VUART)	+= 8250_aspeed_vuart.o
 obj-$(CONFIG_SERIAL_8250_BCM2835AUX)	+= 8250_bcm2835aux.o
 obj-$(CONFIG_SERIAL_8250_CONSOLE)	+= 8250_early.o
 obj-$(CONFIG_SERIAL_8250_FOURPORT)	+= 8250_fourport.o
-- 
2.11.0

^ permalink raw reply related

* [PATCH v4 1/2] serial: 8250: Add flag so drivers can avoid THRE probe
From: Joel Stanley @ 2017-05-02  7:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-serial, linux-kernel, devicetree, Andy Shevchenko,
	Benjamin Herrenschmidt, Jeremy Kerr, Mark Rutland, Rob Herring,
	openbmc
In-Reply-To: <20170502074543.1380-1-joel@jms.id.au>

The probing of THRE irq behaviour assumes the other end will be reading
bytes out of the buffer in order to probe the port at driver init. In
some cases the other end cannot be relied upon to read these bytes, so
provide a flag for them to skip this step.

Bit 19 was chosen as the flags are a int and the top bits are taken.

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>

---
v3:
 - No change

v3:
 - Correct the bit number in the changelog

v2:
 - No change

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/tty/serial/8250/8250_port.c | 2 +-
 include/linux/serial_core.h         | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 6119516ef5fc..60a6c247340f 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2229,7 +2229,7 @@ int serial8250_do_startup(struct uart_port *port)
 		}
 	}
 
-	if (port->irq) {
+	if (port->irq && !(up->port.flags & UPF_NO_THRE_TEST)) {
 		unsigned char iir1;
 		/*
 		 * Test for UARTs that do not reassert THRE when the
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 58484fb35cc8..260245deec94 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -195,6 +195,7 @@ struct uart_port {
 #define UPF_NO_TXEN_TEST	((__force upf_t) (1 << 15))
 #define UPF_MAGIC_MULTIPLIER	((__force upf_t) ASYNC_MAGIC_MULTIPLIER /* 16 */ )
 
+#define UPF_NO_THRE_TEST	((__force upf_t) (1 << 19))
 /* Port has hardware-assisted h/w flow control */
 #define UPF_AUTO_CTS		((__force upf_t) (1 << 20))
 #define UPF_AUTO_RTS		((__force upf_t) (1 << 21))
-- 
2.11.0

^ permalink raw reply related

* [PATCH v4 0/2] drivers: serial: Aspeed VUART driver
From: Joel Stanley @ 2017-05-02  7:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Andy Shevchenko,
	Benjamin Herrenschmidt, Jeremy Kerr, Mark Rutland, Rob Herring,
	openbmc-uLR06cmDAlY/bJ5BZ2RsiQ

This is v4 of a driver for the Aspeed VUART. This version addresses feedback
from Andy and Greg, and includes Rob's ack for the bindings change.

The VUART is a serial device on the BMC side of the LPC bus that connects a BMC
to it's host processor.

We add a flag to the serial core to allow the driver to skip probing of the
THRE irq behaviour, which could hang due to the host not reading bytes out of
the buffer.

We've been using this on systems for over a year, so it has seen a good amount
of testing.

Cheers,

Joel

Jeremy Kerr (1):
  drivers/serial: Add driver for Aspeed virtual UART

Joel Stanley (1):
  serial: 8250: Add flag so drivers can avoid THRE probe

 Documentation/ABI/stable/sysfs-driver-aspeed-vuart |  15 +
 Documentation/devicetree/bindings/serial/8250.txt  |   2 +
 drivers/tty/serial/8250/8250_aspeed_vuart.c        | 323 +++++++++++++++++++++
 drivers/tty/serial/8250/8250_port.c                |   2 +-
 drivers/tty/serial/8250/Kconfig                    |  10 +
 drivers/tty/serial/8250/Makefile                   |   1 +
 include/linux/serial_core.h                        |   1 +
 7 files changed, 353 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/stable/sysfs-driver-aspeed-vuart
 create mode 100644 drivers/tty/serial/8250/8250_aspeed_vuart.c

-- 
2.11.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 v3 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Joel Stanley @ 2017-05-02  7:45 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Greg Kroah-Hartman, Jiri Slaby, Mark Rutland, Rob Herring,
	Jeremy Kerr, linux-serial@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree, Benjamin Herrenschmidt,
	OpenBMC Maillist
In-Reply-To: <CAHp75Vd=oONhzF34Pe8WwYKBWWxcsaQtgJy66aFD68iNKTxEfg@mail.gmail.com>

On Tue, Apr 11, 2017 at 9:45 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Apr 10, 2017 at 7:04 AM, Joel Stanley <joel@jms.id.au> wrote:
>> From: Jeremy Kerr <jk@ozlabs.org>
>>
>> This change adds a driver for the 16550-based Aspeed virtual UART
>> device. We use a similar process to the of_serial driver for device
>> probe, but expose some VUART-specific functions through sysfs too.
>>
>> The VUART is two UART 'front ends' connected by their FIFO (no actual
>> serial line in between). One is on the BMC side (management controller)
>> and one is on the host CPU side.
>>
>> This driver is for the BMC side. The sysfs files allow the BMC
>> userspace, which owns the system configuration policy, to specify at
>> what IO port and interrupt number the host side will appear to the host
>> on the Host <-> BMC LPC bus. It could be different on a different system
>> (though most of them use 3f8/4).
>>
>> OpenPOWER host firmware doesn't like it when the host-side of the
>> VUART's FIFO is not drained. This driver only disables host TX discard
>> mode when the port is in use. We set the VUART enabled bit when we bind
>> to the device, and clear it on unbind.
>>
>> We don't want to do this on open/release, as the host may be using this
>> bit to configure serial output modes, which is independent of whether
>> the devices has been opened by BMC userspace.
>
>> +static void aspeed_vuart_set_enabled(struct aspeed_vuart *vuart, bool enabled)
>> +{
>> +       u8 reg;
>> +
>> +       reg = readb(vuart->regs + ASPEED_VUART_GCRA);
>
>> +       reg &= ~ASPEED_VUART_GCRA_VUART_EN;
>> +       if (enabled)
>> +               reg |= ASPEED_VUART_GCRA_VUART_EN;
>
> Usually the pattern is
> if (something)
>  set x bit;
> else
>  clear x bit;
>
> It would make it one operation in any case and a bit more understandable.

I have made these ordering changes you requested.

>
>> +       port.port.irq = irq_of_parse_and_map(np, 0);
>
> The benefit of use platform_get_irq() is to get rid of some OF specific headers.

It's an of driver, and this function does exactly what we require. I
have left it in for v4.

Cheers,

Joel

^ permalink raw reply

* Re: [PATCH v3 1/2] ASoC: stm32: add bindings for SAI
From: Olivier MOYSAN @ 2017-05-02  7:45 UTC (permalink / raw)
  To: Rob Herring
  Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
	alsa-devel@alsa-project.org, Alexandre TORGUE,
	linux-kernel@vger.kernel.org, Arnaud POULIQUEN, tiwai@suse.com,
	lgirdwood@gmail.com, broonie@kernel.org,
	mcoquelin.stm32@gmail.com, Benjamin GAIGNARD,
	linux-arm-kernel@lists.infradead.org, kernel@stlinux.com
In-Reply-To: <20170428205358.fxjqaki44xqim4ta@rob-hp-laptop>

Hello Rob,

On 04/28/2017 10:53 PM, Rob Herring wrote:
> On Mon, Apr 10, 2017 at 05:19:55PM +0200, olivier moysan wrote:
>> This patch adds documentation of device tree bindings for the
>> STM32 SAI ASoC driver.
>>
>> Signed-off-by: olivier moysan <olivier.moysan@st.com>
>> ---
>>  .../devicetree/bindings/sound/st,stm32-sai.txt     | 89 ++++++++++++++++++++++
>>  1 file changed, 89 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/sound/st,stm32-sai.txt
>>
>> diff --git a/Documentation/devicetree/bindings/sound/st,stm32-sai.txt b/Documentation/devicetree/bindings/sound/st,stm32-sai.txt
>> new file mode 100644
>> index 0000000..c59a3d7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/st,stm32-sai.txt
>> @@ -0,0 +1,89 @@
>> +STMicroelectronics STM32 Serial Audio Interface (SAI).
>
> [...]
>
>> +	sai1b: audio-controller@40015824 {
>> +		#sound-dai-cells = <0>;
>> +		compatible = "st,stm32-sai-sub-b";
>> +		reg = <0x40015824 0x1C>;
>> +		clocks = <&rcc 1 CLK_SAI2>;
>> +		clock-names = "sai_ck";
>> +		dmas = <&dma2 5 0 0x400 0x0>;
>> +		dma-names = "tx";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_sai1b>;
>> +
>> +		ports {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			sai1b_port: port@0 {
>> +				reg = <0>;
>> +				cpu_endpoint: endpoint {
>> +					remote-endpoint = <&codec_endpoint>;
>> +					audio-graph-card,format = "i2s";
>> +					audio-graph-card,bitclock-master = <&codec_endpoint>;
>> +					audio-graph-card,frame-master = <&codec_endpoint>;
>
> These property names are wrong.
>

I have taken into account this comment (and previous ones).
They will be included in next update of this patch set.

>> +				};
>> +			};
>> +		};
>> +	};
>> +};
>> +
>> +audio-codec {
>> +	codec_port: port {
>> +		codec_endpoint: endpoint {
>> +			remote-endpoint = <&cpu_endpoint>;
>> +		};
>> +	};
>> +};
>> --
>> 1.9.1
>>

Best regards
Olivier

^ permalink raw reply

* BENEFIT
From: Mrs Julie Leach @ 2017-05-02  7:37 UTC (permalink / raw)
  To: Recipients

You are a recipient to Mrs Julie Leach Donation of $3 million USD. Contact(julieleach93-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) for claims.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v5 7/9] power: Adds support for Smart Battery System Manager
From: Sebastian Reichel @ 2017-05-02  7:12 UTC (permalink / raw)
  To: Phil Reid
  Cc: wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, peda-koto5C5qi+TLoDKTGw+V6w,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, Karl-Heinz Schneider
In-Reply-To: <bf4cf813-609c-1d92-2329-35e9176d0d28-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>

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

Hi Phil,

On Tue, May 02, 2017 at 10:20:22AM +0800, Phil Reid wrote:
> In regards the le to cpu conversion. grepping the supply folder
> finds a couple of other suspicious cases.
> 
> sbs-battery & the bq24735-cahrger do similar things.
> 
> thoughts on if they need to be corrected?

Yes, those drivers look incorrect. I guess nobody noticed, since
most people use little endian machines nowadays (so le16_to_cpu and
cpu_to_le16 do nothing).

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: FW: [PATCH 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe
From: Ryder Lee @ 2017-05-02  7:09 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Rob Herring, linux-pci-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Bjorn Helgaas,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4BAFE512E7223A4B91F29C40AC7A579616B5155E-FTLr7L0+/BWGRgb7Xm6L22/AB3i2Q03W@public.gmane.org>


Hi Arnd,

> 2017-04-28 19:41 GMT+08:00 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>:
> 
>         On Fri, Apr 28, 2017 at 4:46 AM, Ryder Lee
>         <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>         > On Thu, 2017-04-27 at 21:06 +0200, Arnd Bergmann wrote:
>         >> On Wed, Apr 26, 2017 at 10:10 AM, Ryder Lee
>         <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>         >> > On Tue, 2017-04-25 at 14:18 +0200, Arnd Bergmann wrote:
>         >> >> On Sun, Apr 23, 2017 at 10:19 AM, Ryder Lee
>         <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>         >> Are any of the registers the same at all, e.g. for MSI
>         handling?
>         >
>         > No, It doesn't support MSI. All I can do is using the
>         registers that designer provide to me. The others are inviable
>         for software. So I treat it as different hardware.
>         Furthermore, we hope that we can put all mediatek drivers
>         together regardless of in-house IP or lincense IP
>         >
>         > We have no particular IP name but just use chip name to call
>         it. So I will temporarily use "mediatek,gen2v1-pcie" in patch
>         v1.
>         
>         I think using the chip name as in the first version of your
>         patch name is better then, in particular since the 'gen2v1'
>         would not be an actual version number but just say which
>         variant got merged into mainline first.

Okay, i will correct it.

>         A related question would be on how general we want the binding
>         to be.
>         Your binding text starts out by describing that there are
>         three root ports and what their capabilities are.
>         
>         If you think there might be other (existing or future) chips
>         that use the same binding and driver, then being a little more
>         abstract could help in the long run.

Thanks for reminding me. If we decide to use the same driver in the
future, we will have a internal discussion about it.

Ryder.





--
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/2] dt-bindings: phy: Add documentation for Mediatek PCIe PHY
From: Ryder Lee @ 2017-05-02  7:08 UTC (permalink / raw)
  To: Rob Herring
  Cc: Kishon Vijay Abraham I, Matthias Brugger, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <20170428175200.laakck7ill2lxtxc@rob-hp-laptop>

On Fri, 2017-04-28 at 12:52 -0500, Rob Herring wrote:
> On Sun, Apr 23, 2017 at 04:17:33PM +0800, Ryder Lee wrote:
> > Add documentation for PCIe PHY available in MT7623 series SoCs.
> > 
> > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> > ---
> >  .../devicetree/bindings/phy/phy-mt7623-pcie.txt    | 67 ++++++++++++++++++++++
> >  1 file changed, 67 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/phy/phy-mt7623-pcie.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/phy-mt7623-pcie.txt b/Documentation/devicetree/bindings/phy/phy-mt7623-pcie.txt
> > new file mode 100644
> > index 0000000..27a9253
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/phy-mt7623-pcie.txt
> > @@ -0,0 +1,67 @@
> > +Mediatek MT7623 PCIe PHY
> > +-----------------------
> > +
> > +Required properties:
> > + - compatible: Should contain "mediatek,mt7623-pcie-phy"
> > + - #phy-cells: must be 0
> > + - clocks: Must contain an entry in clock-names.
> > +	See ../clocks/clock-bindings.txt for details.
> > + - clock-names: Must be "refclk"
> > + - resets: Must contain an entry in reset-names.
> > +	See ../reset/reset.txt for details.
> > + - reset-names: Must be "phy"
> > +
> > +Optional properties:
> > + - phy-switch: The PHY on PCIe port2 is shared with USB u3phy2. If you
> > +	want to enable port2, you should contain it.
> 
> Need to state what the value is (i.e. a phandle to ?). Also needs a 
> vendor prefix.

I will correct it.

> > +
> > +Example:
> > +
> > +	pcie0_phy: pciephy@1a149000 {
> 
> pcie-phy@...

Okay.
> > +		compatible = "mediatek,mt7623-pcie-phy";
> > +		reg = <0 0x1a149000 0 0x1000>;
> > +		clocks = <&clk26m>;
> > +		clock-names = "pciephya_ref";
> > +		#phy-cells = <0>;
> > +		status = "disabled";
> 
> Don't show status in examples.
I will drop it all.
> > +	};
> > +
> > +	pcie1_phy: pciephy@1a14a000 {
> > +		compatible = "mediatek,mt7623-pcie-phy";
> > +		reg = <0 0x1a14a000 0 0x1000>;
> > +		clocks = <&clk26m>;
> > +		clock-names = "pciephya_ref";
> > +		#phy-cells = <0>;
> > +		status = "disabled";
> > +	};
> > +
> > +	pcie2_phy: pciephy@1a244000 {
> > +		compatible = "mediatek,mt7623-pcie-phy";
> > +		reg = <0 0x1a244000 0 0x1000>;
> > +		clocks = <&clk26m>;
> > +		clock-names = "pciephya_ref";
> > +		#phy-cells = <0>;
> > +
> > +		phy-switch = <&hifsys>;
> > +		status = "disabled";
> > +	};
> > +
> > +Specifying phy control of devices
> > +---------------------------------
> > +
> > +Device nodes should specify the configuration required in their "phys"
> > +property, containing a phandle to the phy node and phy-names.
> > +
> > +Example:
> > +
> > +#include <dt-bindings/phy/phy.h>
> > +
> > +pcie: pcie@1a140000 {
> > +	...
> > +	pcie@1,0 {
> > +		...
> > +		phys = <&pcie0_phy>;
> > +		phy-names = "pcie-phy0";
> > +	}
> > +	...
> > +};

Thanks for your reviews.
> > -- 
> > 1.9.1
> > 

^ permalink raw reply

* Re: [PATCH v1 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe
From: Ryder Lee @ 2017-05-02  7:07 UTC (permalink / raw)
  To: Rob Herring
  Cc: Bjorn Helgaas, Arnd Bergmann, linux-pci-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Red Hung
In-Reply-To: <20170428210924.y6xdv4trqbmyn5me@rob-hp-laptop>

On Fri, 2017-04-28 at 16:09 -0500, Rob Herring wrote:
> On Fri, Apr 28, 2017 at 05:10:34PM +0800, Ryder Lee wrote:
> > Add binding document for Mediatek PCIe Gen2 v1 host controller driver.
> > 
> > Signed-off-by: Ryder Lee <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> >  .../bindings/pci/mediatek,gen2v1-pcie.txt          | 174 +++++++++++++++++++++
> >  1 file changed, 174 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pci/mediatek,gen2v1-pcie.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/mediatek,gen2v1-pcie.txt b/Documentation/devicetree/bindings/pci/mediatek,gen2v1-pcie.txt
> > new file mode 100644
> > index 0000000..545d8cf
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/mediatek,gen2v1-pcie.txt
> > @@ -0,0 +1,174 @@
> > +Mediatek Gen2 V1 PCIe controller
> > +
> > +PCIe subsys supports single root complex (RC) with 3 Root Ports. Each root
> > +ports supports a Gen2 1-lane Link. It includes one Host/PCI bridge and 3
> > +PCIe MAC. Each port has PIPE interface to PHY. There are 3 bus master for
> > +data access and 1 bus slave for Configuration and Status Register access.
> > +
> > +This controller is available on MT7623 series SoCs.
> > +  
> > +Required properties:
> > +- compatible: Should contain "mediatek,gen2v1-pcie".
> > +- device_type: Must be "pci"
> > +- reg: Base addresses and lengths of the PCIe controller.
> > +- #address-cells: Address representation for root ports (must be 3)
> > +  - cell 0 specifies the bus and device numbers of the root port:
> > +    [23:16]: bus number
> > +    [15:11]: device number
> > +  - cell 1 denotes the upper 32 address bits and should be 0
> > +  - cell 2 contains the lower 32 address bits and is used to translate to the
> > +    CPU address space
> 
> This is all standard PCI bus binding. You don't need to define it here. 
> "must be 3" is sufficient.

Okay.

> > +- #size-cells: Size representation for root ports (must be 2)
> > +- #interrupt-cells: Size representation for interrupts (must be 1)
> > +- interrupts: Three interrupt outputs of the controller. Must contain an
> > +  entry for each entry in the interrupt-names property.
> 
> Where's interrupt-names?

I will add it.

> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
> > +  Please refer to the standard PCI bus binding document for a more detailed
> > +  explanation.
> > +- clocks: Must contain an entry for each entry in clock-names.
> > +  See ../clocks/clock-bindings.txt for details.
> > +- clock-names: Must include the following entries:
> > +  - free_ck	:for reference clock of PCIe subsys
> > +  - sys_ck0	:for clock of Port0 MAC
> > +  - sys_ck1	:for clock of Port1 MAC
> > +  - sys_ck2	:for clock of Port2 MAC
> > +- resets: Must contain an entry for each entry in reset-names.
> > +  See ../reset/reset.txt for details.
> > +- reset-names: Must include the following entries:
> > +  - pcie-rst0	:port0 reset
> > +  - pcie-rst1	:port1 reset
> > +  - pcie-rst2	:port2 reset
> > +- phys: list of PHY specifiers (used by generic PHY framework)
> > +- phy-names : must be "pcie-phy0", "pcie-phy1", "pcie-phyN".. based on the
> > +  number of PHYs as specified in *phys* property.
> > +- power-domains: A phandle and power domain specifier pair to the power domain
> > +  which is responsible for collapsing and restoring power to the peripheral
> > +- bus-range: Range of bus numbers associated with this controller
> > +- ranges: Describes the translation of addresses for root ports and standard
> > +  PCI regions. The entries must be 6 cells each, where the first three cells
> > +  correspond to the address as described for the #address-cells property
> > +  above, the fourth cell is the physical CPU address to translate to and the
> > +  fifth and six cells are as described for the #size-cells property above.
> 
> Don't need to define what ranges is here, just what the entries should 
> be:

Okay.
> > +  - The first three entries are expected to translate the addresses for the root
> > +    port registers, which are referenced by the assigned-addresses property of
> > +    the root port nodes (see below).
> > +  - The remaining entries setup the mapping for the standard I/O and memory
> > +	regions.
> > +  Please refer to the standard PCI bus binding document for a more detailed
> > +  explanation.
> > +
> > +In addition, the device tree node must have sub-nodes describing each
> > +PCIe port interface, having the following mandatory properties:
> > +
> > +Required properties:
> > +- device_type: Must be "pci"
> > +- assigned-addresses: Address and size of the port configuration registers
> > +- reg: Only the first four bytes are used to refer to the correct bus number
> > +  and device number.
> > +- #address-cells: Must be 3
> > +- #size-cells: Must be 2
> > +- #interrupt-cells: Size representation for interrupts (must be 1)
> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
> > +  Please refer to the standard PCI bus binding document for a more detailed
> > +  explanation.
> > +- ranges: Sub-ranges distributed from the PCIe controller node. An empty
> > +  property is sufficient.
> > +- num-lanes: Number of lanes to use for this port.
> > +
> > +Examples:
> > +
> > +SoC dtsi:
> 
> Don't show the board vs. SoC split in examples. And drop all the status 
> properties.

Okay i will drop it all.
> > +
> > +	hifsys: syscon@1a000000 {
> > +		compatible = "mediatek,mt7623-hifsys", "syscon";
> > +		reg = <0 0x1a000000 0 0x1000>;
> > +		#clock-cells = <1>;
> > +		#reset-cells = <1>;
> > +	};
> > +
> > +	pcie: pcie-controller@1a140000 {
> > +		compatible = "mediatek,gen2v1-pcie";
> > +		device_type = "pci";
> > +		reg = <0 0x1a140000 0 0x1000>; /* PCIe shared registers */
> > +		#address-cells = <3>;
> > +		#size-cells = <2>;
> > +		#interrupt-cells = <1>;
> > +		interrupts = <GIC_SPI 193 IRQ_TYPE_LEVEL_LOW>,
> > +			     <GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>,
> > +			     <GIC_SPI 195 IRQ_TYPE_LEVEL_LOW>;
> > +		interrupt-map-mask = <0xf800 0 0 0>;
> > +		interrupt-map = <0x0000 0 0 0 &gic GIC_SPI 193 IRQ_TYPE_NONE>,
> > +				 <0x0800 0 0 0 &gic GIC_SPI 194 IRQ_TYPE_NONE>,
> > +			     <0x1000 0 0 0 &gic GIC_SPI 195 IRQ_TYPE_NONE>;
> > +		clocks = <&topckgen CLK_TOP_ETHIF_SEL>,
> > +			     <&hifsys CLK_HIFSYS_PCIE0>,
> > +				 <&hifsys CLK_HIFSYS_PCIE1>,
> > +				 <&hifsys CLK_HIFSYS_PCIE2>;
> > +		clock-names = "free_ck", "sys_ck0", "sys_ck1", "sys_ck2";
> > +		resets = <&hifsys MT2701_HIFSYS_PCIE0_RST>,
> > +			     <&hifsys MT2701_HIFSYS_PCIE1_RST>,
> > +			     <&hifsys MT2701_HIFSYS_PCIE2_RST>;
> > +		reset-names = "pcie-rst0", "pcie-rst1", "pcie-rst2";
> > +		phys = <&pcie0_phy>, <&pcie1_phy>, <&pcie2_phy>;
> > +		phy-names = "pcie-phy0", "pcie-phy1", "pcie-phy2";
> > +		power-domains = <&scpsys MT2701_POWER_DOMAIN_HIF>;
> > +		bus-range = <0x00 0xff>;
> > +		ranges = <0x82000000 0 0x1a142000 0 0x1a142000 0 0x1000 /* Port0 registers */
> > +			  0x82000000 0 0x1a143000 0 0x1a143000 0 0x1000 /* Port1 registers */
> > +			  0x82000000 0 0x1a144000 0 0x1a144000 0 0x1000 /* Port2 registers */
> > +			  0x81000000 0 0x1a160000 0 0x1a160000 0 0x00010000 /* I/O space */
> > +			  0x83000000 0 0x60000000 0 0x60000000 0 0x10000000>;	/* memory space */
> > +		status = "disabled";
> > +
> > +		pcie@1,0 {
> > +			device_type = "pci";
> > +			assigned-addresses = <0x82000000 0 0x1a142000 0 0x1000>;
> > +			reg = <0x0000 0 0 0 0>;
> > +			#address-cells = <3>;
> > +			#size-cells = <2>;
> > +			#interrupt-cells = <1>;
> > +			interrupt-map-mask = <0 0 0 0>;
> > +			interrupt-map = <0 0 0 0 &gic GIC_SPI 193 IRQ_TYPE_NONE>;
> > +			ranges;
> > +			num-lanes = <1>;
> > +			status = "disabled";
> > +		};
> > +
> > +		pcie@2,0 {
> > +			device_type = "pci";
> > +			assigned-addresses = <0x82000800 0 0x1a143000 0 0x1000>;
> > +			reg = <0x0800 0 0 0 0>;
> > +			#address-cells = <3>;
> > +			#size-cells = <2>;
> > +			#interrupt-cells = <1>;
> > +			interrupt-map-mask = <0 0 0 0>;
> > +			interrupt-map = <0 0 0 0 &gic GIC_SPI 194 IRQ_TYPE_NONE>;
> > +			ranges;
> > +			num-lanes = <1>;
> > +			status = "disabled";
> > +		};
> > +
> > +		pcie@3,0 {
> > +			device_type = "pci";
> > +			assigned-addresses = <0x82001000 0 0x1a144000 0 0x1000>;
> > +			reg = <0x1000 0 0 0 0>;
> > +			#address-cells = <3>;
> > +			#size-cells = <2>;
> > +			#interrupt-cells = <1>;
> > +			interrupt-map-mask = <0 0 0 0>;
> > +			interrupt-map = <0 0 0 0 &gic GIC_SPI 195 IRQ_TYPE_NONE>;
> > +			ranges;
> > +			num-lanes = <1>;
> > +			status = "disabled";
> > +		};
> > +	};
> > +
> > +Board dts:
> > +
> > +	&pcie {
> > +		status = "okay";
> > +
> > +		pcie@1,0 {
> > +			status = "okay";
> > +		};
> > +	};
> > -- 
> > 1.9.1

Thanks for your review.


--
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/2] dt-bindings: rockchip-dw-mshc: add optional rockchip,default-num-phases
From: Shawn Lin @ 2017-05-02  7:03 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Ulf Hansson, Ziyuan Xu,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA, Doug Anderson, Jaehoon Chung,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20170428133403.mmdvtp3ohff6v7gh@rob-hp-laptop>

Hi Rob,

在 2017/4/28 21:34, Rob Herring 写道:
> On Wed, Apr 19, 2017 at 05:00:33PM +0800, Shawn Lin wrote:
>> By default, dw_mmc-rockchip will execute tuning for each degree.
>> So we won't miss every point of the good sample windows. However,
>> probably the phases are linear inside the good sample window.
>> Actually we don't need to do tuning for each degree so that we could
>> save some time, for instance, probe the driver or resume from S3.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>>
>>  Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>> index 520d61d..ea47ec0 100644
>> --- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>> +++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>> @@ -31,6 +31,10 @@ Optional Properties:
>>    probing, low speeds or in case where all phases work at tuning time.
>>    If not specified 0 deg will be used.
>>
>> +* rockchip,default-num-phases: The default number of times that the host
>> +  execute tuning when needed. If not specified, the host will do tuning
>> +  for 360 times, namely tuning for each degree.
>
> How is it default when you specify it? I would think default here is
> 360.
>

I don't get your point here. Do you mean the name, default-num-phases,
isn't correct, so I need another one?

> Should this be common?
>
> Rob
>
>
>


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* Re: [PATCH 2/2] mmc: dw_mmc-rockchip: parse rockchip,default-num-phases from DT
From: Shawn Lin @ 2017-05-02  6:58 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Jaehoon Chung, Ulf Hansson, Rob Herring,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ziyuan Xu,
	open list:ARM/Rockchip SoC...,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAD=FV=Uyqvzg5H+Mg5RtQAWiCxVARx7uB4jxPe=2fcSmJQoOjw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Doug,

在 2017/4/25 0:18, Doug Anderson 写道:
> Hi,
>
> On Wed, Apr 19, 2017 at 6:21 PM, Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
>> Hi Doug,
>>
>> 在 2017/4/20 4:19, Doug Anderson 写道:
>>>
>>> Hi,
>>>
>>> On Wed, Apr 19, 2017 at 2:00 AM, Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>>> wrote:
>>>>
>>>> Currently we unconditionally do tuning for each degree, which
>>>> costs 900ms for each boot and resume.
>>>>
>>>> May someone argue that this is a question of accuracy VS time. But I
>>>> would say it's a trick of how we need to do decision for our boards.
>>>> If we don't care the time we spend at all, we could definitely do tuning
>>>> for each degree. But when we need to improve the user experience, for
>>>> instance, speed up resuming from S3, we should also have the right to
>>>> do that. This patch add parsing "rockchip,default-num-phases", for folks
>>>> to specify the number of doing tuning. If not specified, 360 will be used
>>>> as before.
>>>>
>>>> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>>>>
>>>> ---
>>>>
>>>>  drivers/mmc/host/dw_mmc-rockchip.c | 48
>>>> ++++++++++++++++++++++++--------------
>>>>  1 file changed, 30 insertions(+), 18 deletions(-)
>>>
>>>
>>> No huge objection here, but I do remember we ended up at the 360
>>> phases due to some of the craziness with dw_mmc delay elements on
>>> rk3288.  IIRC one of the big problems was that the delay elements
>>> changed _a lot_ with the "LOGIC" voltage and we tweaked the voltage at
>>> runtime for DDR DVFS.  That imposed an extra need to be very accurate
>>> on that SoC, at least on any board that was designed to support DDR
>>> DVFS.
>>>
>>
>> Not just with the vdd_logic but also with the process of Soc.
>> To better guaratee the accuracy, firstly we use delay element to do
>> tuning and then convert it to be combination of degree + delay element.
>> But as the dalay elements aren't accuracy themself, so all the math we
>> do here is trick.
>
> Yup.  I brought up the vdd logic specifically because it's something
> that can make the phases change quite dramatically on the same machine
> between the time you tuned and the time you used it.
>
>
>>> I also remember there being some weirdness on the Rockchip
>>> implementation where there was a certain set of phases that the MMC
>>> controller was essentially "blind".  This blind spot was in the middle
>>> of an otherwise good range of points.  Unfortunately this blind spot
>>> was somewhat hard to detect properly because it was not very big.
>>> ...the variability of the delay elements meant that there could be big
>>> ranges where we weren't getting any good test coverage, but also the
>>> fact that they changed with the LOGIC voltage might mean that we
>>> weren't in the "blind" spot and then suddenly we were.
>>
>>
>> I undertand all of these as I was suffering from it when bringing up
>> RK3288.
>>
>>>
>>> One other note is that i remember that the vast majority of time spent
>>> tuning was dealing with "bad" phases, not dealing with "good" phases.
>>> If you're looking to speed things up, maybe finding a way to make
>>> "bad" phases fail faster would be wise?  I think one of the reasons
>>> bad phases failed so slowly is because the dw_mmc timeouts are all so
>>> long.
>>
>>
>> Good point. I haven't thought of speeding up the handle of bad phases,
>> but I will take a look at this.
>>
>>>
>>> Oh, and I guess one last note is that I have no idea if folks will
>>> like the device bindings here.  Part of me thinks it should be
>>> something more "symbolic" like "rockchip,need-accurate-tuning" or
>>> something like that.  I guess I'd let the DT experts chime in.
>>>
>>>
>>> So I guess to summarize:
>>> * On rk3288 boards w/ DDR DVFS (or any other similar boards), 360
>>> seems to provide real benefit.
>>> * On other boards, probably you can get by with fewer phases.
>>>
>>
>> I would try to say it's a question of "900ms for a single time" VS.
>> "some of discrete tuning cost for more chance to do retune".
>>
>> (1)We could try to do a more accurate tuning process and spends 900ms.
>> Then we have less chance to do retune later.
>>
>> (2)We do a rough tuning and have more chance to do retune later.
>
> Ah, interesting point.  I haven't used newer versions of Linux much,
> but I seem to remember that they will automatically retune sometimes
> if they see errors.  That makes your strategy a bit more valid.
>
>
>> I also would say that this is a game , and we can't say which
>> one is better. Obvious now the "900ms" alwyas happens in the spot
>> routine, for instance, booting and resuming from S3.
>
> Is it really 900 ms?  I don't quite remember it being that long, but I
> could be remembering incorrectly.

I saw the worst case was nearly 900ms. But mostly we need 600ms there.

>
> -Doug
>
>
>

--
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 2/2] regulator: Allow for asymmetric settling times
From: Laxman Dewangan @ 2017-05-02  6:53 UTC (permalink / raw)
  To: Matthias Kaehlcke, Liam Girdwood, Mark Brown, Rob Herring,
	Mark Rutland
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Douglas Anderson, Brian Norris
In-Reply-To: <20170501183715.35375-2-mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>


On Tuesday 02 May 2017 12:07 AM, Matthias Kaehlcke wrote:
> Some regulators have different settling times for voltage increases and
> decreases. To avoid a time penalty on the faster transition allow for
> different settings for up- and downward transitions.
>
> Signed-off-by: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---

Acked-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

--
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 1/2] regulator: DT: Add properties for asymmetric settling times
From: Laxman Dewangan @ 2017-05-02  6:51 UTC (permalink / raw)
  To: Matthias Kaehlcke, Liam Girdwood, Mark Brown, Rob Herring,
	Mark Rutland
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Douglas Anderson, Brian Norris
In-Reply-To: <20170501183715.35375-1-mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>


On Tuesday 02 May 2017 12:07 AM, Matthias Kaehlcke wrote:
> Some regulators have different settling times for voltage increases and
> decreases. Add DT properties to define separate settling times for up-
> and downward voltage changes.
>
> Signed-off-by: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---

Acked-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

--
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 RFC] Documentation/devicetree: Add specification for FSI busses
From: Jeremy Kerr @ 2017-05-02  6:25 UTC (permalink / raw)
  To: Joel Stanley; +Cc: devicetree, OpenBMC Maillist
In-Reply-To: <CACPK8Xf2regdT4Y-TW4nOc1qYwDcH=zYPRqdjTqL9yvZH49bRA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Joel,

> Looks good to me Jeremy. One small question on the description, but
> please add Acked-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org> to future
> revisions.

Thanks for the review!

>> +The FSI bus is probe-able, so Linux is able to enumerate FSI slaves, and
>> +engines within those slaves. However, we have a facility to match devicetree
>> +nodes to probed engines. This allows for fsi engines to expose non-probeable
>> +busses, which are then exposed by the device tree. For example, a FSI engine
> 
> A nit: Can we can expose any device as part of the engine, not just a bus?

Absolutely. I'll clarify in a future version. Something like:

  This allows for fsi engines to expose non-probeable busses, or for
  drivers to access extra device properties, by data described in the
  device tree.

Cheers,


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: [PATCH v6 07/23] drivers/fsi: Implement slave initialisation
From: Joel Stanley @ 2017-05-02  6:24 UTC (permalink / raw)
  To: Christopher Bostic
  Cc: Rob Herring, Mark Rutland, Russell King,
	rostedt-nx8X9YLhiw1AfugRpC6u6w, mingo-H+wXaHxf7aLQT0dZR+AlfA,
	Greg KH, devicetree,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jeremy Kerr,
	Linux Kernel Mailing List, Andrew Jeffery, Alistair Popple,
	Benjamin Herrenschmidt
In-Reply-To: <20170410194706.64280-8-cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

On Tue, Apr 11, 2017 at 5:16 AM, Christopher Bostic
<cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> wrote:
> From: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
>
> Implement fsi_slave_init: if we can read a chip ID, create fsi_slave
> devices and register with the driver core.
>
> Includes changes from Chris Bostic <cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>.
>
> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Chris Bostic <cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
> ---
>  drivers/fsi/fsi-core.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 64 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index 6e1cfdf..c705ca2 100644
> --- a/drivers/fsi/fsi-core.c
> +++ b/drivers/fsi/fsi-core.c
> @@ -17,9 +17,12 @@
>  #include <linux/fsi.h>
>  #include <linux/idr.h>
>  #include <linux/module.h>
> +#include <linux/slab.h>
>
>  #include "fsi-master.h"
>
> +#define FSI_SLAVE_SIZE_23b             0x800000
> +
>  static DEFINE_IDA(master_ida);
>
>  struct fsi_slave {
> @@ -114,11 +117,70 @@ static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,
>                         addr, val, size);
>  }
>
> +static void fsi_slave_release(struct device *dev)
> +{
> +       struct fsi_slave *slave = to_fsi_slave(dev);
> +
> +       kfree(slave);
> +}
> +
>  static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
>  {
> -       /* todo: initialise slave device, perform engine scan */
> +       struct fsi_slave *slave;
> +       uint32_t chip_id;
> +       uint8_t crc;
> +       int rc;
> +
> +       /* Currently, we only support single slaves on a link, and use the
> +        * full 23-bit address range
> +        */
> +       if (id != 0)
> +               return -EINVAL;
> +
> +       rc = fsi_master_read(master, link, id, 0, &chip_id, sizeof(chip_id));
> +       if (rc) {
> +               dev_warn(&master->dev, "can't read slave %02x:%02x %d\n",
> +                               link, id, rc);

When I boot a system with this driver loaded, I get his warning:

[    9.740000] usbhid: USB HID core driver
[    9.840000]  fsi0: can't read slave 00:00 -5
[    9.840000] NET: Registered protocol family 10

Two things:

There's a space in front of "fsi0".

This warning isn't useful at that point. The slave is not readable as
the FSI master is not present (the P9 hasn't been turned on). Can we
avoid printing the warning at boot?

Cheers,

Joel

> +               return -ENODEV;
> +       }
> +       chip_id = be32_to_cpu(chip_id);
> +
> +       crc = fsi_crc4(0, chip_id, 32);
> +       if (crc) {
> +               dev_warn(&master->dev, "slave %02x:%02x invalid chip id CRC!\n",
> +                               link, id);
> +               return -EIO;
> +       }
> +
> +       dev_info(&master->dev, "fsi: found chip %08x at %02x:%02x:%02x\n",
> +                       chip_id, master->idx, link, id);
> +
> +       /* We can communicate with a slave; create the slave device and
> +        * register.
> +        */
> +       slave = kzalloc(sizeof(*slave), GFP_KERNEL);
> +       if (!slave)
> +               return -ENOMEM;
> +
> +       slave->master = master;
> +       slave->dev.parent = &master->dev;
> +       slave->dev.release = fsi_slave_release;
> +       slave->link = link;
> +       slave->id = id;
> +       slave->size = FSI_SLAVE_SIZE_23b;
> +
> +       dev_set_name(&slave->dev, "slave@%02x:%02x", link, id);
> +       rc = device_register(&slave->dev);
> +       if (rc < 0) {
> +               dev_warn(&master->dev, "failed to create slave device: %d\n",
> +                               rc);
> +               put_device(&slave->dev);
> +               return rc;
> +       }
> +
> +       /* todo: perform engine scan */
>
> -       return -ENODEV;
> +       return rc;
>  }
>
>  /* FSI master support */
> --
> 1.8.2.2
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH RFC] Documentation/devicetree: Add specification for FSI busses
From: Joel Stanley @ 2017-05-02  6:19 UTC (permalink / raw)
  To: Jeremy Kerr; +Cc: devicetree, OpenBMC Maillist
In-Reply-To: <1493704508-27119-1-git-send-email-jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>

On Tue, May 2, 2017 at 3:25 PM, Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org> wrote:
> This change introduces a proposed layout for describing FSI busses in
> the device tree. While the bus is probe-able, we'd still like a method
> of describing subordinate (eg i2c) busses that are behind FSI devices.
>
> The FSI core will be responsible for matching probed slaves & engines to
> their device tree nodes, so the FSI device drivers' probe() functions
> will be passed a struct device with the appropriate of_node populated
> where a matching DT node is found.
>
> RFC at this stage, so I'd welcome any feedback.
>
> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>

Looks good to me Jeremy. One small question on the description, but
please add Acked-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org> to future
revisions.

> ---
>  Documentation/devicetree/bindings/fsi/fsi.txt | 144 ++++++++++++++++++++++++++
>  1 file changed, 144 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/fsi/fsi.txt
>
> diff --git a/Documentation/devicetree/bindings/fsi/fsi.txt b/Documentation/devicetree/bindings/fsi/fsi.txt
> new file mode 100644
> index 0000000..b5e85c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fsi/fsi.txt
> @@ -0,0 +1,144 @@
> +FSI bus & engine generic device tree bindings
> +=============================================
> +
> +The FSI bus is probe-able, so Linux is able to enumerate FSI slaves, and
> +engines within those slaves. However, we have a facility to match devicetree
> +nodes to probed engines. This allows for fsi engines to expose non-probeable
> +busses, which are then exposed by the device tree. For example, a FSI engine

A nit: Can we can expose any device as part of the engine, not just a bus?

> +that is an I2C master - the I2C bus can be described by the device tree under
> +the engine's device tree node.

Cheers,

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

^ permalink raw reply


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