* [RFC 3/6] Documentation: dt: add bindings for ti bb2d
From: Nishanth Menon @ 2016-11-18 2:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118024436.13447-3-robertcnelson@gmail.com>
On 11/17/2016 08:44 PM, Robert Nelson wrote:
> From: Gowtham Tammana <g-tammana@ti.com>
>
> Add documentation for device tree bindings description for
> 2D GPU blitter module present in Texas Instruments family of SoCs.
>
> Signed-off-by: Gowtham Tammana <g-tammana@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
Might want to point at the source of the patch -> just for the record,
this came from TI vendor kernel..
> Documentation/devicetree/bindings/gpu/ti-bb2d.txt | 27 +++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpu/ti-bb2d.txt
>
> diff --git a/Documentation/devicetree/bindings/gpu/ti-bb2d.txt b/Documentation/devicetree/bindings/gpu/ti-bb2d.txt
> new file mode 100644
> index 0000000..af47488
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpu/ti-bb2d.txt
> @@ -0,0 +1,27 @@
> +* Texas Instruments BB2D blitter module
> +
> +This binding describes the 2D BitBlit (BB2D) graphics accelerator
> +subsystem based on the GC320 core from Vivante Corporation available
> +in Texas Instruments SoCs.
> +
> +Required properties:
> + - compatible: value should take the following format:
> + "ti,<soc>-bb2d", "vivante,<gpuversion>"
> + accepted values:
> + (a) "ti,dra7-bb2d", "vivante,gc320" for TI DRA7xx / AM57x
> +
> + - reg : base address and length of BB2D IP registers
> + - interrupts : BB2D interrupt line number
> + - ti,hwmods : name of the hwmod associated with BB2D module
> + - clocks : handle to BB2D functional clock
> + - clock-names : fclk
> +
> +Example for DRA7x SoC:
> + bb2d: bb2d at 59000000 {
> + compatible = "ti,dra7-bb2d", "vivante,gc320";
> + reg = <0x59000000 0x0700>;
> + interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
> + ti,hwmods = "bb2d";
> + clocks = <&dpll_core_h24x2_ck>;
> + clock-names = "fclk";
> + };
>
--
Regards,
Nishanth Menon
^ permalink raw reply
* [RFC 6/6] ARM: dts: am57xx-beagle-x15-common: enable etnaviv
From: Nishanth Menon @ 2016-11-18 2:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118024436.13447-6-robertcnelson@gmail.com>
On 11/17/2016 08:44 PM, Robert Nelson wrote:
again.. a short commit message at least please? :)
> Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
> CC: Julien <jboulnois@gmail.com>
> CC: Nishanth Menon <nm@ti.com>
> CC: Tomi Valkeinen <tomi.valkeinen@ti.com>
> CC: Tony Lindgren <tony@atomide.com>
> ---
> arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi b/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
> index 6df7829..3bc47be 100644
> --- a/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
> +++ b/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
> @@ -97,6 +97,12 @@
> #cooling-cells = <2>;
> };
>
> + gpu-subsystem {
A) do we want to make things clear that this is gpu subsystem for gc320?
B) How about other platforms that could equally reuse?
> + compatible = "ti,gc320-gpu-subsystem";
> + cores = <&bb2d>;
> + status = "okay";
> + };
> +
> hdmi0: connector {
> compatible = "hdmi-connector";
> label = "hdmi";
> @@ -190,6 +196,11 @@
> >;
> };
> };
> +
> +&bb2d {
> + status = "okay";
> +};
> +
> &i2c1 {
> status = "okay";
> clock-frequency = <400000>;
>
--
Regards,
Nishanth Menon
^ permalink raw reply
* [PATCH V8 0/3] Integration of function trace with System Trace IP blocks
From: Chunyan Zhang @ 2016-11-18 3:07 UTC (permalink / raw)
To: linux-arm-kernel
IP blocks allowing a variety of trace sources to log debugging
information to a pre-defined area have been introduced on a couple of
architecture [1][2]. These system trace blocks (also known as STM)
typically follow the MIPI STPv2 protocol [3] and provide a system wide
logging facility to any device, running a kernel or not, with access
to the block's log entry port(s). Since each trace message has a
timestamp, it is possible to correlate events happening in the entire
system rather than being confined to the logging facility of a single
entity.
This patchset is trying to use STM IP blocks to store function tracing
information produced by Ftrace and I'm taking the Function trace
(trace type is TRACE_FN) as the example in this patchset, but other
types of traces also can be supported.
Logging information generated by the Ftrace subsystem to STM and gathered
in the sink device can be used in conjunction with trace data from other
board components, also collected in the same trace sink.
This example is using ARM coresight STM but the same would apply to any
other architecture wishing to do the same.
Comments would be greatly appreciated.
Thanks,
Chunyan
[1]. https://lwn.net/Articles/674746/
[2]. http://lxr.free-electrons.com/source/drivers/hwtracing/intel_th/
[3]. http://mipi.org/specifications/debug#STP
Changes from v7:
* Addressed comments from Steven Rostedt:
- Removed unnessecery code;
- Shrinked the dependence of 'STM_SOURCE_FTRACE' from 'TRACING' to 'FUNCTION_TRACER';
- Changed the parameter type from char* to void*, that makes more sense.
Changes from v6:
* Rebased on v4.9-rc1;
* Removed unused the declaration and header file including from trace.h
which was added in patch 1 of this series;
* Revised a bit the comments in trace.h .
Changes from v5:
* Addressed comments from Steven Rostedt:
- Removed .commit() from trace_export;
- Changed to directly call trace_process_export() instead of trace_export::commit();
- Used 'ring_buffer_event_length(event)' instead to get the trace size;
- Removed trace_export pointer from trace_array structure.
* Revised commit message a little to make the description more accurate.
Changes from v4:
* Addressed comments from Steven Rostedt:
- Removed 'inline' annotations from the function which is called via function pointer;
- Removed useless components from structure trace_export;
- Added '__rcu' annotation to the RCU variables;
- Used 'rcu_assign_pointer' to do every RCU assignment;
- Added WARN_ON_ONCE() when the .write() is not assigned;
- In order to reduce the overhead caused by adding this feature, this revision used an
global array instead of a big "if statement" to get the size of trace entry according
to the trace type.
- In order to keep the current logic unchanged, made ftrace_exports() only being called if
there's something have been added, i.e. if trace_export to stm_ftrace has been added in
this patchset.
Changes from v3:
* Addressed comments from Steven Rostedt:
- Added comments for the element 'name' and 'next' of trace_export;
- Changed to use RCU functions instead of mutex in function callback;
- Moved the check for name to register trace_export function;
* Renamed 'trace_function_exports' to 'trace_exports' since its
implementation is not only for function_trace; The similar changes on
'add_trace_export' and 'rm_trace_export'.
Changes from v2:
* Rebased on v4.8-rc1.
* Added trace_export class, and make traces can be exported to not only
ring buffer but also other area such as STM.
* Made stm_ftrace as an trace_export.
* More detailed changes are described in change log of each patch.
Changes from v1:
* Addressed comments from Alexander Shishkin:
- Modified some ambiguous change logs.
- Decoupled stm_ftrace and trace_output interface to STM.
- Changed the file name from stm_ftrace.c to stm/ftrace.c.
- Implemented link/unlink hooks for stm_ftrace.
* Removed useless header file include from stm/ftrace.c
* Added Acked-by from Steven Rostedt on 4/4.
Chunyan Zhang (3):
tracing: add a possibility of exporting function trace to other places
instead of ring buffer only
stm class: ftrace: Add ftrace-export-over-stm driver
stm: Mark the functions of writing buffer with notrace
drivers/hwtracing/coresight/coresight-stm.c | 2 +-
drivers/hwtracing/intel_th/sth.c | 11 ++-
drivers/hwtracing/stm/Kconfig | 11 +++
drivers/hwtracing/stm/Makefile | 2 +
drivers/hwtracing/stm/core.c | 7 +-
drivers/hwtracing/stm/dummy_stm.c | 2 +-
drivers/hwtracing/stm/ftrace.c | 87 +++++++++++++++++++
include/linux/stm.h | 4 +-
include/linux/trace.h | 28 ++++++
kernel/trace/trace.c | 129 +++++++++++++++++++++++++++-
10 files changed, 271 insertions(+), 12 deletions(-)
create mode 100644 drivers/hwtracing/stm/ftrace.c
create mode 100644 include/linux/trace.h
--
2.7.4
^ permalink raw reply
* [PATCH V8 1/3] tracing: add a possibility of exporting function trace to other places instead of ring buffer only
From: Chunyan Zhang @ 2016-11-18 3:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479438454-28650-1-git-send-email-zhang.chunyan@linaro.org>
Currently Function traces can be only exported to ring buffer, this
patch added trace_export concept which can process traces and export
them to a registered destination as an addition to the current only
one output of Ftrace - i.e. ring buffer.
In this way, if we want Function traces to be sent to other destination
rather than ring buffer only, we just need to register a new trace_export
and implement its own .write() function for writing traces to storage.
With this patch, only Function trace (trace type is TRACE_FN)
is supported.
Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
---
include/linux/trace.h | 28 +++++++++++
kernel/trace/trace.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 156 insertions(+), 1 deletion(-)
create mode 100644 include/linux/trace.h
diff --git a/include/linux/trace.h b/include/linux/trace.h
new file mode 100644
index 0000000..9330a58
--- /dev/null
+++ b/include/linux/trace.h
@@ -0,0 +1,28 @@
+#ifndef _LINUX_TRACE_H
+#define _LINUX_TRACE_H
+
+#ifdef CONFIG_TRACING
+/*
+ * The trace export - an export of Ftrace output. The trace_export
+ * can process traces and export them to a registered destination as
+ * an addition to the current only output of Ftrace - i.e. ring buffer.
+ *
+ * If you want traces to be sent to some other place rather than ring
+ * buffer only, just need to register a new trace_export and implement
+ * its own .write() function for writing traces to the storage.
+ *
+ * next - pointer to the next trace_export
+ * write - copy traces which have been delt with ->commit() to
+ * the destination
+ */
+struct trace_export {
+ struct trace_export __rcu *next;
+ void (*write)(const void *, unsigned int);
+};
+
+int register_ftrace_export(struct trace_export *export);
+int unregister_ftrace_export(struct trace_export *export);
+
+#endif /* CONFIG_TRACING */
+
+#endif /* _LINUX_TRACE_H */
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 8696ce6..038291d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -40,6 +40,7 @@
#include <linux/poll.h>
#include <linux/nmi.h>
#include <linux/fs.h>
+#include <linux/trace.h>
#include <linux/sched/rt.h>
#include "trace.h"
@@ -2128,6 +2129,129 @@ void trace_buffer_unlock_commit_regs(struct trace_array *tr,
ftrace_trace_userstack(buffer, flags, pc);
}
+static void
+trace_process_export(struct trace_export *export,
+ struct ring_buffer_event *event)
+{
+ struct trace_entry *entry;
+ unsigned int size = 0;
+
+ entry = ring_buffer_event_data(event);
+ size = ring_buffer_event_length(event);
+ export->write(entry, size);
+}
+
+static DEFINE_MUTEX(ftrace_export_lock);
+
+static struct trace_export __rcu *ftrace_exports_list __read_mostly;
+
+static DEFINE_STATIC_KEY_FALSE(ftrace_exports_enabled);
+
+static inline void ftrace_exports_enable(void)
+{
+ static_branch_enable(&ftrace_exports_enabled);
+}
+
+static inline void ftrace_exports_disable(void)
+{
+ static_branch_disable(&ftrace_exports_enabled);
+}
+
+void ftrace_exports(struct ring_buffer_event *event)
+{
+ struct trace_export *export;
+
+ preempt_disable_notrace();
+
+ export = rcu_dereference_raw_notrace(ftrace_exports_list);
+ while (export) {
+ trace_process_export(export, event);
+ export = rcu_dereference_raw_notrace(export->next);
+ }
+
+ preempt_enable_notrace();
+}
+
+static inline void
+add_trace_export(struct trace_export **list, struct trace_export *export)
+{
+ rcu_assign_pointer(export->next, *list);
+ /*
+ * We are entering export into the list but another
+ * CPU might be walking that list. We need to make sure
+ * the export->next pointer is valid before another CPU sees
+ * the export pointer included into the list.
+ */
+ rcu_assign_pointer(*list, export);
+}
+
+static inline int
+rm_trace_export(struct trace_export **list, struct trace_export *export)
+{
+ struct trace_export **p;
+
+ for (p = list; *p != NULL; p = &(*p)->next)
+ if (*p == export)
+ break;
+
+ if (*p != export)
+ return -1;
+
+ rcu_assign_pointer(*p, (*p)->next);
+
+ return 0;
+}
+
+static inline void
+add_ftrace_export(struct trace_export **list, struct trace_export *export)
+{
+ if (*list == NULL)
+ ftrace_exports_enable();
+
+ add_trace_export(list, export);
+}
+
+static inline int
+rm_ftrace_export(struct trace_export **list, struct trace_export *export)
+{
+ int ret;
+
+ ret = rm_trace_export(list, export);
+ if (*list == NULL)
+ ftrace_exports_disable();
+
+ return ret;
+}
+
+int register_ftrace_export(struct trace_export *export)
+{
+ if (WARN_ON_ONCE(!export->write))
+ return -1;
+
+ mutex_lock(&ftrace_export_lock);
+
+ add_ftrace_export(&ftrace_exports_list, export);
+
+ mutex_unlock(&ftrace_export_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(register_ftrace_export);
+
+int unregister_ftrace_export(struct trace_export *export)
+{
+ int ret;
+
+ mutex_lock(&ftrace_export_lock);
+
+ ret = rm_ftrace_export(&ftrace_exports_list, export);
+
+ mutex_unlock(&ftrace_export_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(unregister_ftrace_export);
+
void
trace_function(struct trace_array *tr,
unsigned long ip, unsigned long parent_ip, unsigned long flags,
@@ -2146,8 +2270,11 @@ trace_function(struct trace_array *tr,
entry->ip = ip;
entry->parent_ip = parent_ip;
- if (!call_filter_check_discard(call, entry, buffer, event))
+ if (!call_filter_check_discard(call, entry, buffer, event)) {
+ if (static_branch_unlikely(&ftrace_exports_enabled))
+ ftrace_exports(event);
__buffer_unlock_commit(buffer, event);
+ }
}
#ifdef CONFIG_STACKTRACE
--
2.7.4
^ permalink raw reply related
* [PATCH V8 2/3] stm class: ftrace: Add ftrace-export-over-stm driver
From: Chunyan Zhang @ 2016-11-18 3:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479438454-28650-1-git-send-email-zhang.chunyan@linaro.org>
This patch adds a driver that models itself as an stm_source called
stm_ftrace. Once the stm device and stm_ftrace have been linked via
sysfs, the driver registers itself as a trace_export and everything
passed to the interface from Ftrace subsystem will end up in the STM
trace engine.
Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
---
drivers/hwtracing/stm/Kconfig | 11 ++++++
drivers/hwtracing/stm/Makefile | 2 +
drivers/hwtracing/stm/ftrace.c | 87 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 100 insertions(+)
create mode 100644 drivers/hwtracing/stm/ftrace.c
diff --git a/drivers/hwtracing/stm/Kconfig b/drivers/hwtracing/stm/Kconfig
index 847a39b..723e2d9 100644
--- a/drivers/hwtracing/stm/Kconfig
+++ b/drivers/hwtracing/stm/Kconfig
@@ -39,4 +39,15 @@ config STM_SOURCE_HEARTBEAT
If you want to send heartbeat messages over STM devices,
say Y.
+config STM_SOURCE_FTRACE
+ tristate "Copy the output from kernel Ftrace to STM engine"
+ depends on FUNCTION_TRACER
+ help
+ This option can be used to copy the output from kernel Ftrace
+ to STM engine. Enabling this option will introduce a slight
+ timing effect.
+
+ If you want to send kernel Ftrace messages over STM devices,
+ say Y.
+
endif
diff --git a/drivers/hwtracing/stm/Makefile b/drivers/hwtracing/stm/Makefile
index a9ce3d4..3abd84c 100644
--- a/drivers/hwtracing/stm/Makefile
+++ b/drivers/hwtracing/stm/Makefile
@@ -6,6 +6,8 @@ obj-$(CONFIG_STM_DUMMY) += dummy_stm.o
obj-$(CONFIG_STM_SOURCE_CONSOLE) += stm_console.o
obj-$(CONFIG_STM_SOURCE_HEARTBEAT) += stm_heartbeat.o
+obj-$(CONFIG_STM_SOURCE_FTRACE) += stm_ftrace.o
stm_console-y := console.o
stm_heartbeat-y := heartbeat.o
+stm_ftrace-y := ftrace.o
diff --git a/drivers/hwtracing/stm/ftrace.c b/drivers/hwtracing/stm/ftrace.c
new file mode 100644
index 0000000..bd126a7
--- /dev/null
+++ b/drivers/hwtracing/stm/ftrace.c
@@ -0,0 +1,87 @@
+/*
+ * Simple kernel driver to link kernel Ftrace and an STM device
+ * Copyright (c) 2016, Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ * STM Ftrace will be registered as a trace_export.
+ */
+
+#include <linux/module.h>
+#include <linux/stm.h>
+#include <linux/trace.h>
+
+#define STM_FTRACE_NR_CHANNELS 1
+#define STM_FTRACE_CHAN 0
+
+static int stm_ftrace_link(struct stm_source_data *data);
+static void stm_ftrace_unlink(struct stm_source_data *data);
+
+static struct stm_ftrace {
+ struct stm_source_data data;
+ struct trace_export ftrace;
+} stm_ftrace = {
+ .data = {
+ .name = "ftrace",
+ .nr_chans = STM_FTRACE_NR_CHANNELS,
+ .link = stm_ftrace_link,
+ .unlink = stm_ftrace_unlink,
+ },
+};
+
+/**
+ * stm_ftrace_write() - write data to STM via 'stm_ftrace' source
+ * @buf: buffer containing the data packet
+ * @len: length of the data packet
+ */
+static void notrace
+stm_ftrace_write(const void *buf, unsigned int len)
+{
+ stm_source_write(&stm_ftrace.data, STM_FTRACE_CHAN, buf, len);
+}
+
+static int stm_ftrace_link(struct stm_source_data *data)
+{
+ struct stm_ftrace *sf = container_of(data, struct stm_ftrace, data);
+
+ sf->ftrace.write = stm_ftrace_write;
+
+ return register_ftrace_export(&sf->ftrace);
+}
+
+static void stm_ftrace_unlink(struct stm_source_data *data)
+{
+ struct stm_ftrace *sf = container_of(data, struct stm_ftrace, data);
+
+ unregister_ftrace_export(&sf->ftrace);
+}
+
+static int __init stm_ftrace_init(void)
+{
+ int ret;
+
+ ret = stm_source_register_device(NULL, &stm_ftrace.data);
+ if (ret)
+ pr_err("Failed to register stm_source - ftrace.\n");
+
+ return ret;
+}
+
+static void __exit stm_ftrace_exit(void)
+{
+ stm_source_unregister_device(&stm_ftrace.data);
+}
+
+module_init(stm_ftrace_init);
+module_exit(stm_ftrace_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("stm_ftrace driver");
+MODULE_AUTHOR("Chunyan Zhang <zhang.chunyan@linaro.org>");
--
2.7.4
^ permalink raw reply related
* [PATCH V8 3/3] stm: Mark the functions of writing buffer with notrace
From: Chunyan Zhang @ 2016-11-18 3:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479438454-28650-1-git-send-email-zhang.chunyan@linaro.org>
If CONFIG_STM_SOURCE_FTRACE is selected, Function trace data can be writen
to sink via STM, all functions that related to writing data packets to
STM should be marked 'notrace' to avoid being traced by Ftrace, otherwise
the program would stall into an endless loop.
Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
---
drivers/hwtracing/coresight/coresight-stm.c | 2 +-
drivers/hwtracing/intel_th/sth.c | 11 +++++++----
drivers/hwtracing/stm/core.c | 7 ++++---
drivers/hwtracing/stm/dummy_stm.c | 2 +-
include/linux/stm.h | 4 ++--
5 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
index 49e0f1b..b7543bd 100644
--- a/drivers/hwtracing/coresight/coresight-stm.c
+++ b/drivers/hwtracing/coresight/coresight-stm.c
@@ -406,7 +406,7 @@ static long stm_generic_set_options(struct stm_data *stm_data,
return 0;
}
-static ssize_t stm_generic_packet(struct stm_data *stm_data,
+static ssize_t notrace stm_generic_packet(struct stm_data *stm_data,
unsigned int master,
unsigned int channel,
unsigned int packet,
diff --git a/drivers/hwtracing/intel_th/sth.c b/drivers/hwtracing/intel_th/sth.c
index e1aee61..b034446 100644
--- a/drivers/hwtracing/intel_th/sth.c
+++ b/drivers/hwtracing/intel_th/sth.c
@@ -67,10 +67,13 @@ static void sth_iowrite(void __iomem *dest, const unsigned char *payload,
}
}
-static ssize_t sth_stm_packet(struct stm_data *stm_data, unsigned int master,
- unsigned int channel, unsigned int packet,
- unsigned int flags, unsigned int size,
- const unsigned char *payload)
+static ssize_t notrace sth_stm_packet(struct stm_data *stm_data,
+ unsigned int master,
+ unsigned int channel,
+ unsigned int packet,
+ unsigned int flags,
+ unsigned int size,
+ const unsigned char *payload)
{
struct sth_device *sth = container_of(stm_data, struct sth_device, stm);
struct intel_th_channel __iomem *out =
diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
index 51f81d6..37d3bcb 100644
--- a/drivers/hwtracing/stm/core.c
+++ b/drivers/hwtracing/stm/core.c
@@ -425,7 +425,7 @@ static int stm_file_assign(struct stm_file *stmf, char *id, unsigned int width)
return ret;
}
-static ssize_t stm_write(struct stm_data *data, unsigned int master,
+static ssize_t notrace stm_write(struct stm_data *data, unsigned int master,
unsigned int channel, const char *buf, size_t count)
{
unsigned int flags = STP_PACKET_TIMESTAMPED;
@@ -1121,8 +1121,9 @@ void stm_source_unregister_device(struct stm_source_data *data)
}
EXPORT_SYMBOL_GPL(stm_source_unregister_device);
-int stm_source_write(struct stm_source_data *data, unsigned int chan,
- const char *buf, size_t count)
+int notrace stm_source_write(struct stm_source_data *data,
+ unsigned int chan,
+ const char *buf, size_t count)
{
struct stm_source_device *src = data->src;
struct stm_device *stm;
diff --git a/drivers/hwtracing/stm/dummy_stm.c b/drivers/hwtracing/stm/dummy_stm.c
index a86612d..c5f94ca 100644
--- a/drivers/hwtracing/stm/dummy_stm.c
+++ b/drivers/hwtracing/stm/dummy_stm.c
@@ -21,7 +21,7 @@
#include <linux/slab.h>
#include <linux/stm.h>
-static ssize_t
+static ssize_t notrace
dummy_stm_packet(struct stm_data *stm_data, unsigned int master,
unsigned int channel, unsigned int packet, unsigned int flags,
unsigned int size, const unsigned char *payload)
diff --git a/include/linux/stm.h b/include/linux/stm.h
index 8369d8a..210ff22 100644
--- a/include/linux/stm.h
+++ b/include/linux/stm.h
@@ -133,7 +133,7 @@ int stm_source_register_device(struct device *parent,
struct stm_source_data *data);
void stm_source_unregister_device(struct stm_source_data *data);
-int stm_source_write(struct stm_source_data *data, unsigned int chan,
- const char *buf, size_t count);
+int notrace stm_source_write(struct stm_source_data *data, unsigned int chan,
+ const char *buf, size_t count);
#endif /* _STM_H_ */
--
2.7.4
^ permalink raw reply related
* System/uncore PMUs and unit aggregation
From: Leeder, Neil @ 2016-11-18 3:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161117181708.GT22855@arm.com>
Thanks for opening up the discussion on this Will.
For the Qualcomm L2 driver, one objection I had to exposing each unit is
that there are so many of them - the minimum starting point is a dozen,
so trying to start 9 counters on each means a perf command line
specifying 100+ events. Future chips are only likely to increase that.
There is a single CPU node so from an end-user perspective it would
seems to make sense to also have a single L2 node. perf already has the
ability to create events on multiple units using cpumask, aggregate the
results, and split them out per unit with perf stat -a -A, so the user
can get that granularity. Exposing separate units would make userspace
duplicate a lot of that functionality. This may rely on each uncore unit
being associated with a CPU, which is the case with L2.
I agree with your comments regarding groups and I can see that a
standard way of representing topology could be useful - in this case,
which CPUs are within the same L2 cluster. Perhaps a list of cpumasks,
one per L2 unit.
On 11/17/2016 1:17 PM, Will Deacon wrote:
[...]
> 3. Summing the counters across units is only permitted if the units
> can all be started and stopped atomically. Otherwise, the counters
> should be exposed individually. It's up to the driver author to
> decide what makes sense to sum.
If I understand your your point 3 correctly, I'm not sure about the need
to start and stop them all atomically. That seems to be a tighter
requirement than we require for CPU PMUs. For them, perf stat -a creates
events/groups on each CPU, then starts and stops them sequentially and
sums the results. If that model is acceptable for the CPU to collect and
aggregate counts, that should be the same bar that uncore PMUs need to
reach. In the L2 case, the driver isn't summing the results, it's still
perf doing it, so I may be misinterpreting your comment about where the
summation is permitted.
Thanks,
Neil
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH 1/2] phy: rockchip-inno-usb2: fix uninitialized tmout variable
From: wlf @ 2016-11-18 3:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161116142259.2123506-1-arnd@arndb.de>
Hi Arnd,
? 2016?11?16? 22:22, Arnd Bergmann ??:
> The newly added OTG support has an obvious uninitialized variable
> access that gcc warns about:
>
> drivers/phy/phy-rockchip-inno-usb2.c: In function 'rockchip_chg_detect_work':
> drivers/phy/phy-rockchip-inno-usb2.c:717:7: error: 'tmout' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> This replaces the use of the uninitialized variable with what
> the value was in the previous USB_CHG_STATE_WAIT_FOR_DCD
> state.
>
> Fixes: 0c42fe48fd23 ("phy: rockchip-inno-usb2: support otg-port for rk3399")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/phy/phy-rockchip-inno-usb2.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/phy/phy-rockchip-inno-usb2.c b/drivers/phy/phy-rockchip-inno-usb2.c
> index eb89de59b68f..2f99ec95079c 100644
> --- a/drivers/phy/phy-rockchip-inno-usb2.c
> +++ b/drivers/phy/phy-rockchip-inno-usb2.c
> @@ -714,7 +714,7 @@ static void rockchip_chg_detect_work(struct work_struct *work)
> delay = CHG_SECONDARY_DET_TIME;
> rphy->chg_state = USB_CHG_STATE_PRIMARY_DONE;
> } else {
> - if (tmout) {
> + if (rphy->dcd_retries == CHG_DCD_MAX_RETRIES) {
> /* floating charger found */
> rphy->chg_type = POWER_SUPPLY_TYPE_USB_DCP;
> rphy->chg_state = USB_CHG_STATE_DETECTED;
Thanks very much for your help.
Reviewed-by: William Wu <wulf@rock-chips.com>
Best Regards,
wulf
^ permalink raw reply
* [PATCH v9 09/10] drm/mediatek: update DSI sub driver flow for sending commands to panel
From: Daniel Kurtz @ 2016-11-18 3:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478865346-19043-10-git-send-email-yt.shen@mediatek.com>
Hi YT,
Sorry for the very late review.
My biggest problem with this patch is it describes itself as adding
support for a new use case "DSI -> panel", but makes many changes to
the existing working flow "DSI -> bridge -> panel".
If these changes are really needed, or improve the existing flow, I'd
expect to see those changes added first in a preparatory patch,
followed by a second smaller, simpler
patch that adds any additional functionality required to enable the new flow.
See detailed comments inline.
On Fri, Nov 11, 2016 at 7:55 PM, YT Shen <yt.shen@mediatek.com> wrote:
>
> This patch update enable/disable flow of DSI module and MIPI TX module.
> Original flow works on there is a bridge chip: DSI -> bridge -> panel.
> In this case: DSI -> panel, the DSI sub driver flow should be updated.
> We need to initialize DSI first so that we can send commands to panel.
>
> Signed-off-by: shaoming chen <shaoming.chen@mediatek.com>
> Signed-off-by: YT Shen <yt.shen@mediatek.com>
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 110 ++++++++++++++++++++++++++-------
> drivers/gpu/drm/mediatek/mtk_mipi_tx.c | 32 +++++-----
> 2 files changed, 103 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 860b84f..12a1206 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -94,6 +94,8 @@
> #define DSI_RACK 0x84
> #define RACK BIT(0)
>
> +#define DSI_MEM_CONTI 0x90
> +
> #define DSI_PHY_LCCON 0x104
> #define LC_HS_TX_EN BIT(0)
> #define LC_ULPM_EN BIT(1)
> @@ -126,6 +128,10 @@
> #define CLK_HS_POST (0xff << 8)
> #define CLK_HS_EXIT (0xff << 16)
>
> +#define DSI_VM_CMD_CON 0x130
> +#define VM_CMD_EN BIT(0)
> +#define TS_VFP_EN BIT(5)
> +
> #define DSI_CMDQ0 0x180
> #define CONFIG (0xff << 0)
> #define SHORT_PACKET 0
> @@ -219,12 +225,12 @@ static void mtk_dsi_phy_timconfig(struct mtk_dsi *dsi)
> writel(timcon3, dsi->regs + DSI_PHY_TIMECON3);
> }
>
> -static void mtk_dsi_enable(struct mtk_dsi *dsi)
> +static void mtk_dsi_engine_enable(struct mtk_dsi *dsi)
I don't think we need to change these names.
> {
> mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_EN, DSI_EN);
> }
>
> -static void mtk_dsi_disable(struct mtk_dsi *dsi)
> +static void mtk_dsi_engine_disable(struct mtk_dsi *dsi)
> {
> mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_EN, 0);
> }
> @@ -249,7 +255,9 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
> * mipi_ratio is mipi clk coefficient for balance the pixel clk in mipi.
> * we set mipi_ratio is 1.05.
> */
> - dsi->data_rate = dsi->vm.pixelclock * 3 * 21 / (1 * 1000 * 10);
> + dsi->data_rate = dsi->vm.pixelclock * 12 * 21;
> + dsi->data_rate /= (dsi->lanes * 1000 * 10);
> + dev_info(dev, "set mipitx's data rate: %dMHz\n", dsi->data_rate);
I don't think we want to spam the log like this. Use dev_dbg.... or
use the DRM_() messaging like elsewhere in this driver?
>
> ret = clk_set_rate(dsi->hs_clk, dsi->data_rate * 1000000);
> if (ret < 0) {
> @@ -271,7 +279,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
> goto err_disable_engine_clk;
> }
>
> - mtk_dsi_enable(dsi);
> + mtk_dsi_engine_enable(dsi);
> mtk_dsi_reset_engine(dsi);
> mtk_dsi_phy_timconfig(dsi);
>
> @@ -289,7 +297,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
> static void mtk_dsi_clk_ulp_mode_enter(struct mtk_dsi *dsi)
> {
> mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_HS_TX_EN, 0);
> - mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, 0);
> + mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, LC_ULPM_EN);
What does this change do?
It looks like a pure bug fix (ie, previoulsy we were'nt actually
enabling ULP MODE before).
If so, can you please move it to a separate preliminary patch.
> }
>
> static void mtk_dsi_clk_ulp_mode_leave(struct mtk_dsi *dsi)
> @@ -302,7 +310,7 @@ static void mtk_dsi_clk_ulp_mode_leave(struct mtk_dsi *dsi)
> static void mtk_dsi_lane0_ulp_mode_enter(struct mtk_dsi *dsi)
> {
> mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_HS_TX_EN, 0);
> - mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, 0);
> + mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, LD0_ULPM_EN);
Same here.
> }
>
> static void mtk_dsi_lane0_ulp_mode_leave(struct mtk_dsi *dsi)
> @@ -338,11 +346,21 @@ static void mtk_dsi_set_mode(struct mtk_dsi *dsi)
> if ((dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) &&
> !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE))
> vid_mode = BURST_MODE;
> + else
> + vid_mode = SYNC_EVENT_MODE;
So, when do we use SYNC_PULSE_MODE (set just before the 'if')?
> }
>
> writel(vid_mode, dsi->regs + DSI_MODE_CTRL);
> }
>
> +static void mtk_dsi_set_vm_cmd(struct mtk_dsi *dsi)
> +{
> + writel(0x3c, dsi->regs + DSI_MEM_CONTI);
Please use #defined constants, especially if this register is a bit field.
Also, this looks like new behavior which doesn't seem related to
changing the enable order.
If this is a general fix, please use a separate patch.
> +
> + mtk_dsi_mask(dsi, DSI_VM_CMD_CON, VM_CMD_EN, VM_CMD_EN);
> + mtk_dsi_mask(dsi, DSI_VM_CMD_CON, TS_VFP_EN, TS_VFP_EN);
> +}
> +
> static void mtk_dsi_ps_control_vact(struct mtk_dsi *dsi)
> {
> struct videomode *vm = &dsi->vm;
> @@ -399,6 +417,9 @@ static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi)
> break;
> }
>
> + tmp_reg |= (dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) << 6;
> + tmp_reg |= (dsi->mode_flags & MIPI_DSI_MODE_EOT_PACKET) >> 3;
> +
ditto
> writel(tmp_reg, dsi->regs + DSI_TXRX_CTRL);
> }
>
> @@ -477,6 +498,16 @@ static void mtk_dsi_start(struct mtk_dsi *dsi)
> writel(1, dsi->regs + DSI_START);
> }
>
> +static void mtk_dsi_stop(struct mtk_dsi *dsi)
> +{
> + writel(0, dsi->regs + DSI_START);
> +}
> +
> +static void mtk_dsi_set_cmd_mode(struct mtk_dsi *dsi)
> +{
> + writel(CMD_MODE, dsi->regs + DSI_MODE_CTRL);
> +}
> +
> static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
> {
> u32 inten = LPRX_RD_RDY_INT_FLAG | CMD_DONE_INT_FLAG | VM_DONE_INT_FLAG;
> @@ -506,7 +537,7 @@ static s32 mtk_dsi_wait_for_irq_done(struct mtk_dsi *dsi, u32 irq_flag,
> if (ret == 0) {
> dev_info(dsi->dev, "Wait DSI IRQ(0x%08x) Timeout\n", irq_flag);
>
> - mtk_dsi_enable(dsi);
> + mtk_dsi_engine_enable(dsi);
> mtk_dsi_reset_engine(dsi);
> }
>
> @@ -535,6 +566,17 @@ static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> +static s32 mtk_dsi_switch_to_cmd_mode(struct mtk_dsi *dsi, u8 irq_flag, u32 t)
> +{
> + mtk_dsi_irq_data_clear(dsi, irq_flag);
> + mtk_dsi_set_cmd_mode(dsi);
> +
> + if (!mtk_dsi_wait_for_irq_done(dsi, irq_flag, t))
> + return -1;
No, use a real linux errno, and return an int, and print an error
message if this is unexpected.
> + else
> + return 0;
> +}
> +
> static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
> {
> if (WARN_ON(dsi->refcount == 0))
> @@ -543,11 +585,6 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
> if (--dsi->refcount != 0)
> return;
>
> - mtk_dsi_lane0_ulp_mode_enter(dsi);
> - mtk_dsi_clk_ulp_mode_enter(dsi);
> -
> - mtk_dsi_disable(dsi);
> -
> clk_disable_unprepare(dsi->engine_clk);
> clk_disable_unprepare(dsi->digital_clk);
>
> @@ -561,35 +598,45 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
> if (dsi->enabled)
> return;
>
> - if (dsi->panel) {
> - if (drm_panel_prepare(dsi->panel)) {
> - DRM_ERROR("failed to setup the panel\n");
> - return;
> - }
> - }
> -
> ret = mtk_dsi_poweron(dsi);
> if (ret < 0) {
> DRM_ERROR("failed to power on dsi\n");
> return;
> }
>
> + usleep_range(20000, 21000);
> +
Why are you adding a 20 ms delay where there was none before?
> mtk_dsi_rxtx_control(dsi);
> + mtk_dsi_phy_timconfig(dsi);
> + mtk_dsi_ps_control_vact(dsi);
> + mtk_dsi_set_vm_cmd(dsi);
> + mtk_dsi_config_vdo_timing(dsi);
> + mtk_dsi_set_interrupt_enable(dsi);
>
> + mtk_dsi_engine_enable(dsi);
> mtk_dsi_clk_ulp_mode_leave(dsi);
> mtk_dsi_lane0_ulp_mode_leave(dsi);
> mtk_dsi_clk_hs_mode(dsi, 0);
> - mtk_dsi_set_mode(dsi);
>
> - mtk_dsi_ps_control_vact(dsi);
> - mtk_dsi_config_vdo_timing(dsi);
> - mtk_dsi_set_interrupt_enable(dsi);
> + if (dsi->panel) {
> + if (drm_panel_prepare(dsi->panel)) {
> + DRM_ERROR("failed to prepare the panel\n");
> + return;
> + }
> + }
>
> mtk_dsi_set_mode(dsi);
> mtk_dsi_clk_hs_mode(dsi, 1);
>
> mtk_dsi_start(dsi);
>
> + if (dsi->panel) {
> + if (drm_panel_enable(dsi->panel)) {
> + DRM_ERROR("failed to enable the panel\n");
In case of error, you must undo everything done to this point. At least:
(1) unprepare the panel
(2) stop dsi
(3) poweroff dsi
> + return;
> + }
> + }
> +
> dsi->enabled = true;
> }
>
> @@ -605,6 +652,21 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi)
> }
> }
>
> + mtk_dsi_stop(dsi);
> + mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500);
This function can return an error, so please check it. Although,
there probably isn't much you can do here about it.
> +
> + if (dsi->panel) {
> + if (drm_panel_unprepare(dsi->panel)) {
> + DRM_ERROR("failed to unprepare the panel\n");
> + return;
I think you should probably just ignore this error and continue
disabling dsi, since it isn't really recoverable and you can't roll
back and re-enable dsi.
> + }
> + }
> +
> + mtk_dsi_reset_engine(dsi);
> + mtk_dsi_lane0_ulp_mode_enter(dsi);
> + mtk_dsi_clk_ulp_mode_enter(dsi);
> + mtk_dsi_engine_disable(dsi);
> +
> mtk_dsi_poweroff(dsi);
>
> dsi->enabled = false;
> @@ -845,7 +907,7 @@ static void mtk_dsi_wait_for_idle(struct mtk_dsi *dsi)
> if (timeout_ms == 0) {
> dev_info(dsi->dev, "polling dsi wait not busy timeout!\n");
>
> - mtk_dsi_enable(dsi);
> + mtk_dsi_engine_enable(dsi);
> mtk_dsi_reset_engine(dsi);
> }
> }
> diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> index 108d31a..34e95c6 100644
> --- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> +++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> @@ -177,7 +177,9 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
>
> dev_dbg(mipi_tx->dev, "prepare: %u Hz\n", mipi_tx->data_rate);
>
> - if (mipi_tx->data_rate >= 500000000) {
> + if (mipi_tx->data_rate > 1250000000) {
> + return -EINVAL;
> + } else if (mipi_tx->data_rate >= 500000000) {
Capping the max data rate looks like an unrelated fix.
> txdiv = 1;
> txdiv0 = 0;
> txdiv1 = 0;
> @@ -201,6 +203,10 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
> return -EINVAL;
> }
>
> + mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_TOP_CON,
> + RG_DSI_LNT_IMP_CAL_CODE | RG_DSI_LNT_HS_BIAS_EN,
> + (8 << 4) | RG_DSI_LNT_HS_BIAS_EN);
> +
> mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_BG_CON,
> RG_DSI_VOUT_MSK |
> RG_DSI_BG_CKEN | RG_DSI_BG_CORE_EN,
> @@ -210,24 +216,18 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
>
> usleep_range(30, 100);
>
> - mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_TOP_CON,
> - RG_DSI_LNT_IMP_CAL_CODE | RG_DSI_LNT_HS_BIAS_EN,
> - (8 << 4) | RG_DSI_LNT_HS_BIAS_EN);
> -
> - mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_CON,
> - RG_DSI_CKG_LDOOUT_EN | RG_DSI_LDOCORE_EN);
> + mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_CON,
> + RG_DSI_CKG_LDOOUT_EN | RG_DSI_LDOCORE_EN,
> + RG_DSI_CKG_LDOOUT_EN | RG_DSI_LDOCORE_EN);
Changing from set_bits to update_bits does not do anything. Please
leave this alone.
>
> mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_PWR,
> RG_DSI_MPPLL_SDM_PWR_ON |
> RG_DSI_MPPLL_SDM_ISO_EN,
> RG_DSI_MPPLL_SDM_PWR_ON);
>
> - mtk_mipi_tx_clear_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
> - RG_DSI_MPPLL_PLL_EN);
> -
Why don't you need to disable the PLL first now?
> mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
> - RG_DSI_MPPLL_TXDIV0 | RG_DSI_MPPLL_TXDIV1 |
> - RG_DSI_MPPLL_PREDIV,
> + RG_DSI_MPPLL_PREDIV | RG_DSI_MPPLL_TXDIV0 |
> + RG_DSI_MPPLL_TXDIV1 | RG_DSI_MPPLL_POSDIV,
> (txdiv0 << 3) | (txdiv1 << 5));
If I read this right, the only thing you are changing is clearing
"RG_DSI_MPPLL_POSDIV".
This would be more clear if you kept the field order: TXDIV0, TXDIV1, PREDIV.
And why are you making this change in this patch?
>
> /*
> @@ -242,10 +242,12 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
> 26000000);
> writel(pcw, mipi_tx->regs + MIPITX_DSI_PLL_CON2);
>
> - mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_PLL_CON1,
> - RG_DSI_MPPLL_SDM_FRA_EN);
> + mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_CON1,
> + RG_DSI_MPPLL_SDM_FRA_EN,
> + RG_DSI_MPPLL_SDM_FRA_EN);
AFAICT, this change does not do anything but make the code more confusing.
>
> - mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_PLL_CON0, RG_DSI_MPPLL_PLL_EN);
> + mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
> + RG_DSI_MPPLL_PLL_EN, RG_DSI_MPPLL_PLL_EN);
AFAICT, this change does not do anything but make the code more confusing.
>
> usleep_range(20, 100);
>
> --
> 1.9.1
>
^ permalink raw reply
* [PATCH v5] drm/mediatek: fixed the calc method of data rate per lane
From: Daniel Kurtz @ 2016-11-18 3:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479361006.13083.7.camel@mtksdaap41>
Hi CK,
On Thu, Nov 17, 2016 at 1:36 PM, CK Hu <ck.hu@mediatek.com> wrote:
> Hi, Jitao:
>
>
> On Wed, 2016-11-16 at 11:20 +0800, Jitao Shi wrote:
>> Tune dsi frame rate by pixel clock, dsi add some extra signal (i.e.
>> Tlpx, Ths-prepare, Ths-zero, Ths-trail,Ths-exit) when enter and exit LP
>> mode, those signals will cause h-time larger than normal and reduce FPS.
>> So need to multiply a coefficient to offset the extra signal's effect.
>> coefficient = ((htotal*bpp/lane_number)+Tlpx+Ths_prep+Ths_zero+
>> Ths_trail+Ths_exit)/(htotal*bpp/lane_number)
>>
>> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
>
> It looks good to me.
> But this patch conflict with [1] which is one patch of MT2701 series. I
> want to apply MT2701 patches first, so please help to refine this patch
> based on MT2701 patches.
I don't think the MT2701 DSI patches are quite ready yet (I just
reviewed the one below).
Can we instead land Jitao's small targeted change first, and then
rebase the MT2701 series on top.
Thanks,
-Dan
>
> [1] https://patchwork.kernel.org/patch/9422821/
>
> Regards,
> CK
>
>> ---
>> Change since v4:
>> - tune the calc comment more clear.
>> - define the phy timings as constants.
>>
>> Chnage since v3:
>> - wrapp the commit msg.
>> - fix alignment of some lines.
>>
>> Change since v2:
>> - move phy timing back to dsi_phy_timconfig.
>>
>> Change since v1:
>> - phy_timing2 and phy_timing3 refer clock cycle time.
>> - define values of LPX HS_PRPR HS_ZERO HS_TRAIL TA_GO TA_SURE TA_GET DA_HS_EXIT.
>> ---
>>
>
^ permalink raw reply
* [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU
From: pankaj.dubey @ 2016-11-18 3:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3360559.T0zQaY0FJ3@wuerfel>
On Thursday 17 November 2016 10:33 PM, Arnd Bergmann wrote:
> On Thursday, November 17, 2016 9:50:27 AM CET pankaj.dubey wrote:
>>
>>>>> of_scu_enable() which _only_ looks up the SCU address in DT and enables
>>>>> it if it finds it, otherwise returning failure.
>>>>>
>>>>> a9_scu_enable() which tries to use the A9 provided SCU address and
>>>>> enables it if it finds it, otherwise returning failure.
>>>>>
>>
>> OK, In that case I can see need for following four helpers as:
>>
>> 1: of_scu_enable() which will __only__ lookup the SCU address in DT and
>> enables it if it finds, otherwise return -ENOMEM failure.
>> This helper APIs is required and sufficient for most of platforms such
>> as exynos, berlin, realview, socfpga, STi, ux500, vexpress, rockchip and
>> mvebu
>>
>> 2: a9_scu_enable(), which will __only__ use A9 provided SCU address and
>> enables it, if address mapped successfully, otherwise returning failure.
>> This helper APIs is required and sufficient for two ARM platforms as of
>> now tegra and hisi.
>>
>> 3: of_scu_get_base() which will lookup the SCU address in DT and if node
>> found maps address and returns ioremapped address to caller.
>> This helper APIs is required for three ARM plaforms rockchip, mvebu and
>> ux500, along with scu_enable() API to enable and find number_of_cores.
>>
>> 4: s9_scu_iomap_base() which will internally use s9_scu_get_base() and
>> do ioremap of scu address and returns ioremapped address to the caller
>> along with ownership (caller has responsibility to unmap it).
>> This helper APIs is required to simplify SCU enable and related code in
>> two ARM plaforms BCM ans ZX.
>>
>> For remaining two ARM platforms (IMX and ZYNQ), none of these helpers
>> are useful for the time-being, as they need SCU mapping very early of
>> boot, where we can't use iomap APIs. So I will drop patches related to
>> these platforms in v2 version.
>>
>> Please let me know if any concern in this approach.
>
> I think ideally we wouldn't even need to know the virtual address
> outside of smp_scu.c. If we can move all users of the address
> into that file directly, it could become a local variable and
> we change scu_power_mode() and scu_get_core_count() instead to
> not require the address argument.
>
If we change scu_get_core_count() without address argument, what about
those platforms which are calling this API very early boot (mostly in
smp_init_cpus), during this stage we can't use iomap. These platforms
are doing static mapping of SCU base, and passing virtual address.
Currently following are platforms which needs SCU virtual address at
very early stage mostly for calling scu_get_core_count(scu_address):
IMX, ZYNQ, SPEAr, and OMAP2.
I can think of handling of these platform's early need of SCU in the
following way:
Probably we need something like early_a9_scu_get_core_count() which can
be handled in this way in smp_scu.c file itself:
+static struct map_desc scu_map __initdata = {
+ .length = SZ_256,
+ .type = MT_DEVICE,
+};
+
+static void __iomem *early_scu_map_io(void)
+{
+ unsigned long base;
+ void __iomem *scu_base;
+
+ base = scu_a9_get_base();
+ scu_map.pfn = __phys_to_pfn(base);
+ scu_map.virtual = base;
+ iotable_init(&scu_map, 1);
+ scu_base = (void __iomem *)base;
+ return scu_base;
+}
+
+/*
+ * early_a9_scu_get_core_count - Get number of CPU cores at very early boot
+ * Only platforms which needs number_of_cores during early boot should
call this
+ */
+unsigned int __init early_a9_scu_get_core_count(void)
+{
+ void __iomem *scu_base;
+
+ scu_base = early_scu_map_io();
+ return scu_get_core_count(scu_base);
+}
+
Please let me know how about using above approach to simplify platform
specific code of early users of SCU address.
Also for rest platforms, which are not using scu base at very early
stage, but are using virtual address which is mapped either from SCU
device node or using s9_scu_get_base() to map to SCU virtual address. To
separate these two we discussed to separate scu_enable in two helper
APIs as of_scu_enable and s9_scu_enable. So if we change
scu_get_core_count which do not requires address argument, this also
needs to be separated in two as of_scu_get_core_count and
s9_scu_get_core_count.
Thanks,
Pankaj Dubey
> The only user I could find outside of that file is
>
> static int shmobile_smp_scu_psr_core_disabled(int cpu)
> {
> unsigned long mask = SCU_PM_POWEROFF << (cpu * 8);
>
> if ((__raw_readl(shmobile_scu_base + 8) & mask) == mask)
> return 1;
>
> return 0;
> }
>
> which can be done in the same file as well.
>
>>>>> Then callers can decide which of these to call, and what error messages
>>>>> to print on their failures.
>>>>
>>>> Splitting the function in two is probably simpler overall, but
>>>> we may still have to look at all the callers: Any platform that
>>>> currently tries to map it on any CPU and doesn't warn about the
>>>> absence of the device node (or about scu_a9_has_base() == false)
>>>> should really continue not to warn about that.
>>>
>>> Did you miss the bit where none of of_scu_enable() or a9_scu_enable()
>>> should produce any warnings or errors to be printed. It's up to the
>>> caller to report the failure, otherwise doing this doesn't make sense:
>>>
>>> if (of_scu_enable() < 0 && a9_scu_enable() < 0)
>>> pr_err("Failed to map and enable the SCU\n");
>>>
>>> because if of_scu_enable() prints a warning/error, then it's patently
>>> misleading.
>>>
>
> That's why I said "otherwise we can leave the warning in the caller
> after checking the return code of the new APIs." for the case where
> we actually need it.
>
>> I will move out error message out of these helpers and let caller
>> (platform specific code) handle and print error if required.
>
> Ok.
>
> Arnd
>
>
>
^ permalink raw reply
* [PATCH next] arm64: remove "SMP: Total of %d processors activated." message
From: Kefeng Wang @ 2016-11-18 3:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161117142213.GI22855@arm.com>
On 2016/11/17 22:22, Will Deacon wrote:
> On Thu, Nov 17, 2016 at 03:32:26PM +0800, Kefeng Wang wrote:
>> There is a common SMP boot message in generic code on all arches,
>> kill "SMP: Total of %d processors activated." in smp_cpus_done()
>> on arm64.
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>> Boot message on qemu.
>> [ 0.375116] smp: Brought up 1 node, 8 CPUs
>> [ 0.383749] SMP: Total of 8 processors activated.
>>
>> arch/arm64/kernel/smp.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
>> index cb87234..9db4a95 100644
>> --- a/arch/arm64/kernel/smp.c
>> +++ b/arch/arm64/kernel/smp.c
>> @@ -428,7 +428,6 @@ static void __init hyp_mode_check(void)
>>
>> void __init smp_cpus_done(unsigned int max_cpus)
>> {
>> - pr_info("SMP: Total of %d processors activated.\n", num_online_cpus());
>> setup_cpu_features();
>> hyp_mode_check();
>> apply_alternatives_all();
>
> Why? Are you proposing the same change to other architectures? Are you paid
> per patch?
The message provides no further information than the generic code, so kill it.
Or show BogoMIPS like arm32?
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index cb87234..6bb33cd 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -428,7 +428,17 @@ static void __init hyp_mode_check(void)
void __init smp_cpus_done(unsigned int max_cpus)
{
- pr_info("SMP: Total of %d processors activated.\n", num_online_cpus());
+ int cpu;
+ unsigned long bogosum = 0;
+
+ for_each_online_cpu(cpu)
+ bogosum += loops_per_jiffy;
+
+ pr_info("SMP: Total of %d processors activated "
+ "(%lu.%02lu BogoMIPS).\n",
+ num_online_cpus(),
+ bogosum / (500000/HZ),
+ (bogosum / (5000/HZ)) % 100);
setup_cpu_features();
hyp_mode_check();
apply_alternatives_all()
>
> Will
>
> .
>
^ permalink raw reply related
* [RFC 6/6] ARM: dts: am57xx-beagle-x15-common: enable etnaviv
From: Robert Nelson @ 2016-11-18 3:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <e69a797c-bec3-2243-5c19-9d7633092369@ti.com>
On Thu, Nov 17, 2016 at 8:56 PM, Nishanth Menon <nm@ti.com> wrote:
> On 11/17/2016 08:44 PM, Robert Nelson wrote:
> again.. a short commit message at least please? :)
yeah, i'll fix all those. ;)
>
>> Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
>> CC: Julien <jboulnois@gmail.com>
>> CC: Nishanth Menon <nm@ti.com>
>> CC: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> CC: Tony Lindgren <tony@atomide.com>
>> ---
>> arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
>> b/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
>> index 6df7829..3bc47be 100644
>> --- a/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
>> +++ b/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
>> @@ -97,6 +97,12 @@
>> #cooling-cells = <2>;
>> };
>>
>> + gpu-subsystem {
>
> A) do we want to make things clear that this is gpu subsystem for gc320?
> B) How about other platforms that could equally reuse?
so the 'gpu-subsystem' comes from etnaviv:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt?id=refs/tags/v4.9-rc5
For a generic name, it's currently only tied to the etnaviv driver:
gpu-subsystem {
compatible = "fsl,imx-gpu-subsystem";
cores = <&gpu_2d>, <&gpu_3d>;
};
it would make sense to make that more generic, so you could tie a 2d
vivante and a imgtec/sgx 3d core.. <sad laugh> but that would require
adding a imgtec/sgx driver/bindings to the kernel mainline... </sad
laugh>
>
>> + compatible = "ti,gc320-gpu-subsystem";
>> + cores = <&bb2d>;
>> + status = "okay";
>> + };
>> +
>> hdmi0: connector {
>> compatible = "hdmi-connector";
>> label = "hdmi";
>> @@ -190,6 +196,11 @@
>> >;
>> };
>> };
>> +
>> +&bb2d {
>> + status = "okay";
>> +};
>> +
>> &i2c1 {
>> status = "okay";
>> clock-frequency = <400000>;
>>
Regards,
--
Robert Nelson
https://rcn-ee.com/
^ permalink raw reply
* [RFC 6/6] ARM: dts: am57xx-beagle-x15-common: enable etnaviv
From: Nishanth Menon @ 2016-11-18 4:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOCHtYiUz1cjYw9nNa4YZGtzNVgSgjEa=4Lqiktz07rszsSksw@mail.gmail.com>
On 11/17/2016 09:44 PM, Robert Nelson wrote:
> On Thu, Nov 17, 2016 at 8:56 PM, Nishanth Menon <nm@ti.com> wrote:
>> On 11/17/2016 08:44 PM, Robert Nelson wrote:
>> again.. a short commit message at least please? :)
>
> yeah, i'll fix all those. ;)
>
>>
>>> Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
>>> CC: Julien <jboulnois@gmail.com>
>>> CC: Nishanth Menon <nm@ti.com>
>>> CC: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>> CC: Tony Lindgren <tony@atomide.com>
>>> ---
>>> arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi | 11 +++++++++++
>>> 1 file changed, 11 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
>>> b/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
>>> index 6df7829..3bc47be 100644
>>> --- a/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
>>> +++ b/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
>>> @@ -97,6 +97,12 @@
>>> #cooling-cells = <2>;
>>> };
>>>
>>> + gpu-subsystem {
>>
>> A) do we want to make things clear that this is gpu subsystem for gc320?
>> B) How about other platforms that could equally reuse?
>
> so the 'gpu-subsystem' comes from etnaviv:
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt?id=refs/tags/v4.9-rc5
>
> For a generic name, it's currently only tied to the etnaviv driver:
>
I was only complaining about "gpu-subsystem {", not the compatible. it
is not the only gpu subsystem on the SoC. either "gpu-subsystem0 {" or
something like gpu-subsystem-gc320 might be helpful to clarify.
> gpu-subsystem {
> compatible = "fsl,imx-gpu-subsystem";
> cores = <&gpu_2d>, <&gpu_3d>;
> };
>
> it would make sense to make that more generic, so you could tie a 2d
> vivante and a imgtec/sgx 3d core.. <sad laugh> but that would require
> adding a imgtec/sgx driver/bindings to the kernel mainline... </sad
> laugh>
>
I should have clarified... I meant other dra7 devices to reuse the
same definitions. this definition is not by any means constrained to
EVM - it is a SoC definition, it should be moved to appropriate place
(convention for dra7 is to mark them as disabled by default in
SoC.dtsi to prevent proliferation of paper spin dtsi and just do
"status = okay" in board file to indicate presence in the variation
for the board).
Yes - I guess some day there might be a bunch of folks like etnaviv
who might make an community driver possible..
--
Regards,
Nishanth Menon
^ permalink raw reply
* [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family flash
From: Yao Yuan @ 2016-11-18 4:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AM4PR0701MB213027FC738E6E377A593B60FEB10@AM4PR0701MB2130.eurprd07.prod.outlook.com>
On Thu, Nov 17, 2016 at 10:14:55AM +0000, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote:
> > On Thu, Nov 17, 2016 at 06:50:55AM +0000, Krzeminski, Marcin (Nokia -
> > PL/Wroclaw) wrote:
> > > > > > On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui
> > > > > > <B56489@freescale.com>
> > > > > > wrote:
> > > > > > > From: Yunhui Cui <yunhui.cui@nxp.com>
> > > > > > >
> > > > > > > With the physical sectors combination, S25FS-S family flash
> > > > > > > requires some special operations for read/write functions.
> > > > > > >
> > > > > > > Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> > > > > > > ---
> > > > > > > drivers/mtd/spi-nor/spi-nor.c | 56
> > > > > > > +++++++++++++++++++++++++++++++++++++++++++
> > > > > > > 1 file changed, 56 insertions(+)
> > > > > > >
> > > > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > b/drivers/mtd/spi-nor/spi-nor.c index d0fc165..495d0bb
> > > > > > > 100644
> > > > > > > --- a/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > @@ -39,6 +39,10 @@
> > > > > > >
> > > > > > > #define SPI_NOR_MAX_ID_LEN 6
> > > > > > > #define SPI_NOR_MAX_ADDR_WIDTH 4
> > > > > > > +/* Added for S25FS-S family flash */
> > > > > > > +#define SPINOR_CONFIG_REG3_OFFSET 0x800004
> > > > > > > +#define CR3V_4KB_ERASE_UNABLE 0x8 #define
> > > > > > > +SPINOR_S25FS_FAMILY_EXT_JEDEC 0x81
> > > > > > >
> > > > > > > struct flash_info {
> > > > > > > char *name;
> > > > > > > @@ -78,6 +82,7 @@ struct flash_info { };
> > > > > > >
> > > > > > > #define JEDEC_MFR(info) ((info)->id[0])
> > > > > > > +#define EXT_JEDEC(info) ((info)->id[5])
> > > > > > >
> > > > > > > static const struct flash_info *spi_nor_match_id(const char
> > > > > > > *name);
> > > > > > >
> > > > > > > @@ -899,6 +904,7 @@ static const struct flash_info spi_nor_ids[] = {
> > > > > > > */
> > > > > > > { "s25sl032p", INFO(0x010215, 0x4d00, 64 * 1024,
> > > > > > > 64,
> > > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > { "s25sl064p", INFO(0x010216, 0x4d00, 64 * 1024,
> > > > > > > 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > + { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 * 1024,
> > > > > > > + 512, 0)},
> > > > > > > { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
> > > > > > > { "s25fl256s1", INFO(0x010219, 0x4d01, 64 * 1024,
> > > > > > > 512,
> > > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > { "s25fl512s", INFO(0x010220, 0x4d00, 256 * 1024,
> > > > > > > 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, @@ -1036,6
> > > > +1042,50
> > > > > > @@ static const struct flash_info *spi_nor_read_id(struct
> > > > > > spi_nor
> > > > > > *nor)
> > > > > > > return ERR_PTR(-ENODEV); }
> > > > > > >
> > > > > > > +/*
> > > > > > > + * The S25FS-S family physical sectors may be configured as
> > > > > > > +a
> > > > > > > + * hybrid combination of eight 4-kB parameter sectors
> > > > > > > + * at the top or bottom of the address space with all
> > > > > > > + * but one of the remaining sectors being uniform size.
> > > > > > > + * The Parameter Sector Erase commands (20h or 21h) must
> > > > > > > + * be used to erase the 4-kB parameter sectors individually.
> > > > > > > + * The Sector (uniform sector) Erase commands (D8h or DCh)
> > > > > > > + * must be used to erase any of the remaining
> > > > > > > + * sectors, including the portion of highest or lowest
> > > > > > > +address
> > > > > > > + * sector that is not overlaid by the parameter sectors.
> > > > > > > + * The uniform sector erase command has no effect on
> > > > > > > +parameter
> > > > > > sectors.
> > > > > > > + */
> > > > > > > +static int spansion_s25fs_disable_4kb_erase(struct spi_nor *nor) {
> > > > > > > + u32 cr3v_addr = SPINOR_CONFIG_REG3_OFFSET;
> > > > > > > + u8 cr3v = 0x0;
> > > > > > > + int ret = 0x0;
> > > > > > > +
> > > > > > > + nor->cmd_buf[2] = cr3v_addr >> 16;
> > > > > > > + nor->cmd_buf[1] = cr3v_addr >> 8;
> > > > > > > + nor->cmd_buf[0] = cr3v_addr >> 0;
> > > > > > > +
> > > > > > > + ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR,
> > &cr3v, 1);
> > > > > > > + if (ret)
> > > > > > > + return ret;
> > > > > > > + if (cr3v & CR3V_4KB_ERASE_UNABLE)
> > > > > > > + return 0;
> > > > > > > + ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
> > > > > > > + if (ret)
> > > > > > > + return ret;
> > > > > > > + cr3v = CR3V_4KB_ERASE_UNABLE;
> > > > > > > + nor->program_opcode = SPINOR_OP_SPANSION_WRAR;
> > > > > > > + nor->write(nor, cr3v_addr, 1, &cr3v);
> > > > > > > +
> > > > > > > + ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR,
> > &cr3v, 1);
> > > > > > > + if (ret)
> > > > > > > + return ret;
> > > > > > > + if (!(cr3v & CR3V_4KB_ERASE_UNABLE))
> > > > > > > + return -EPERM;
> > > > > > > +
> > > > > > > + return 0;
> > > > > > > +}
> > > > > > > +
> > > > > > > static int spi_nor_read(struct mtd_info *mtd, loff_t from,
> > > > > > > size_t
> > len,
> > > > > > > size_t *retlen, u_char *buf) { @@
> > > > > > > -1361,6
> > > > > > > +1411,12 @@ int spi_nor_scan(struct spi_nor *nor, const char
> > > > > > > +*name,
> > > > > > enum read_mode mode)
> > > > > > > spi_nor_wait_till_ready(nor);
> > > > > > > }
> > > > > > >
> > > > > > > + if (EXT_JEDEC(info) == SPINOR_S25FS_FAMILY_EXT_JEDEC) {
> > > > > > > + ret = spansion_s25fs_disable_4kb_erase(nor);
> > > > > > > + if (ret)
> > > > > > > + return ret;
> > > > > > > + }
> > > > > > > +
> > > > > > > if (!mtd->name)
> > > > > > > mtd->name = dev_name(dev);
> > > > > > > mtd->priv = nor;
> > > > > > > --
> > > > > > > 2.1.0.27.g96db324
> > > > > > >
> > > > > > >
> > > > > > Hi Brian, I will ack this change but still feel it's kind of hacking code.
> > > > > >
> > > > > > Acked-by: Han xu <han.xu@nxp.com>
> > > > >
> > > > > I am new on the list so I am not sure if this topic has been discussed.
> > > > > Generally our product functionality relay on those 4KiB sectors.
> > > > > I know that this hack is already in u-boot, but if you
> > > > > mainstream this you will force users of those 4KiB sectors to do hack the
> hack...
> > > > > I believe the proper solution here is to use erase regions
> > > > > functionality, I send and RFS about that some time ago.
> > > >
> > > > Do you mind to send me a link for reference?
> > > >
> > > Han,
> > >
> > > Sorry, It seem I have not posted erase region changes (only those
> > > regarding DUAL/QUAD I/O).
> > > Generally, in this flash you need to create 3 erase regions (because
> > > in FS-S family support only 4KiB erase on parameters sector - eg.
> > > 1.2.2.4 in
> > S25FS512S).
> > > In my case regions are:
> > > 1. 0-32KiB (8*4KiB) - 4K_ERASE (0x20/0x21) 2. 32 - 256 - SE_CMD
> > (0xd8/0xdc) 3.
> > > Rest of the flash SE_CMD (0xd8/0xdc)
> > >
> > > To erase whole flash you can also use CHIP_ERASE_CMD (0x60/0xC7)
> > > command, you just need to add one more mtd partition that will cover
> > whole flash.
> > >
> >
> > Hi Krzeminski,
> >
> > Do you think is there any great advantages for enable 4KB?
> > Because for NXP(Freescale) QSPI controller, there is only support max
> > to 16 groups command.
> >
> > So It's hard to give 3 groups command just for erase (0x21,0xdc and 0xc7).
> > So we have to disable the 4kb erase and only use 256kbytes in this patch.
> >
> Yes, if you disable parameters sector in spi-nor framework you will disable it for
> all spi-nor clients not only for NXP QSPI controller. There are users (at least me)
> that relay on parameters sector functionality. This patch will brake it.
>
> Thanks,
Hi Krzeminski,
Get it.
So do you think how about that I add a flag in dts to select it?
The user want's disable 4kb, he can add the flag.
In spi-nor.c:
if (of_property_read_bool(np, "spi-nor, disable-4kb")) {
spansion_s25fs_disable_4kb_erase();
}
else
...
In dts:
&qspi {
num-cs = <2>;
bus-num = <0>;
status = "okay";
qflash0: s25fs512s at 0 {
compatible = "spansion, s25fs512s";
spi-nor, disable-4kb
#address-cells = <1>;
#size-cells = <1>;
spi-max-frequency = <20000000>;
reg = <0>;
};
I think it should be a better way.
How about your think?
Thanks.
^ permalink raw reply
* [RFC 6/6] ARM: dts: am57xx-beagle-x15-common: enable etnaviv
From: Robert Nelson @ 2016-11-18 4:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3eb346ad-1675-e613-93ef-bd2d07bf2ab8@ti.com>
On Thu, Nov 17, 2016 at 10:15 PM, Nishanth Menon <nm@ti.com> wrote:
> On 11/17/2016 09:44 PM, Robert Nelson wrote:
>>
>> On Thu, Nov 17, 2016 at 8:56 PM, Nishanth Menon <nm@ti.com> wrote:
>>>
>>> On 11/17/2016 08:44 PM, Robert Nelson wrote:
>>> again.. a short commit message at least please? :)
>>
>>
>> yeah, i'll fix all those. ;)
>>
>>>
>>>> Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
>>>> CC: Julien <jboulnois@gmail.com>
>>>> CC: Nishanth Menon <nm@ti.com>
>>>> CC: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>>> CC: Tony Lindgren <tony@atomide.com>
>>>> ---
>>>> arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi | 11 +++++++++++
>>>> 1 file changed, 11 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
>>>> b/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
>>>> index 6df7829..3bc47be 100644
>>>> --- a/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
>>>> +++ b/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
>>>> @@ -97,6 +97,12 @@
>>>> #cooling-cells = <2>;
>>>> };
>>>>
>>>> + gpu-subsystem {
>>>
>>>
>>> A) do we want to make things clear that this is gpu subsystem for gc320?
>>> B) How about other platforms that could equally reuse?
>>
>>
>> so the 'gpu-subsystem' comes from etnaviv:
>>
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt?id=refs/tags/v4.9-rc5
>>
>> For a generic name, it's currently only tied to the etnaviv driver:
>>
>
> I was only complaining about "gpu-subsystem {", not the compatible. it is
> not the only gpu subsystem on the SoC. either "gpu-subsystem0 {" or
> something like gpu-subsystem-gc320 might be helpful to clarify.
>
>> gpu-subsystem {
>> compatible = "fsl,imx-gpu-subsystem";
>> cores = <&gpu_2d>, <&gpu_3d>;
>> };
>>
>> it would make sense to make that more generic, so you could tie a 2d
>> vivante and a imgtec/sgx 3d core.. <sad laugh> but that would require
>> adding a imgtec/sgx driver/bindings to the kernel mainline... </sad
>> laugh>
>>
>
> I should have clarified... I meant other dra7 devices to reuse the same
> definitions. this definition is not by any means constrained to EVM - it is
> a SoC definition, it should be moved to appropriate place (convention for
> dra7 is to mark them as disabled by default in SoC.dtsi to prevent
> proliferation of paper spin dtsi and just do "status = okay" in board file
> to indicate presence in the variation for the board).
Oh yeah, defintely, we can move gpu-subsystem to the base dra7.dtsi,
as the whole dra.dtsi family has a gc320 and then the board device
tree can enable it via:
&bb2d {
status = "okay";
};
Regards,
--
Robert Nelson
https://rcn-ee.com/
^ permalink raw reply
* [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family flash
From: Han Xu @ 2016-11-18 4:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <DB6PR0401MB24077A46D3FC5147DC4C69D989B10@DB6PR0401MB2407.eurprd04.prod.outlook.com>
On Thu, Nov 17, 2016 at 3:14 AM, Yao Yuan <yao.yuan@nxp.com> wrote:
> On Thu, Nov 17, 2016 at 06:50:55AM +0000, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote:
>> > > > On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui <B56489@freescale.com>
>> > > > wrote:
>> > > > > From: Yunhui Cui <yunhui.cui@nxp.com>
>> > > > >
>> > > > > With the physical sectors combination, S25FS-S family flash
>> > > > > requires some special operations for read/write functions.
>> > > > >
>> > > > > Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
>> > > > > ---
>> > > > > drivers/mtd/spi-nor/spi-nor.c | 56
>> > > > > +++++++++++++++++++++++++++++++++++++++++++
>> > > > > 1 file changed, 56 insertions(+)
>> > > > >
>> > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c
>> > > > > b/drivers/mtd/spi-nor/spi-nor.c index d0fc165..495d0bb 100644
>> > > > > --- a/drivers/mtd/spi-nor/spi-nor.c
>> > > > > +++ b/drivers/mtd/spi-nor/spi-nor.c
>> > > > > @@ -39,6 +39,10 @@
>> > > > >
>> > > > > #define SPI_NOR_MAX_ID_LEN 6
>> > > > > #define SPI_NOR_MAX_ADDR_WIDTH 4
>> > > > > +/* Added for S25FS-S family flash */
>> > > > > +#define SPINOR_CONFIG_REG3_OFFSET 0x800004
>> > > > > +#define CR3V_4KB_ERASE_UNABLE 0x8 #define
>> > > > > +SPINOR_S25FS_FAMILY_EXT_JEDEC 0x81
>> > > > >
>> > > > > struct flash_info {
>> > > > > char *name;
>> > > > > @@ -78,6 +82,7 @@ struct flash_info { };
>> > > > >
>> > > > > #define JEDEC_MFR(info) ((info)->id[0])
>> > > > > +#define EXT_JEDEC(info) ((info)->id[5])
>> > > > >
>> > > > > static const struct flash_info *spi_nor_match_id(const char
>> > > > > *name);
>> > > > >
>> > > > > @@ -899,6 +904,7 @@ static const struct flash_info spi_nor_ids[] = {
>> > > > > */
>> > > > > { "s25sl032p", INFO(0x010215, 0x4d00, 64 * 1024, 64,
>> > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>> > > > > { "s25sl064p", INFO(0x010216, 0x4d00, 64 * 1024, 128,
>> > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>> > > > > + { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 * 1024,
>> > > > > + 512, 0)},
>> > > > > { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
>> > > > > { "s25fl256s1", INFO(0x010219, 0x4d01, 64 * 1024, 512,
>> > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>> > > > > { "s25fl512s", INFO(0x010220, 0x4d00, 256 * 1024, 256,
>> > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, @@ -1036,6
>> > +1042,50
>> > > > @@ static const struct flash_info *spi_nor_read_id(struct spi_nor
>> > > > *nor)
>> > > > > return ERR_PTR(-ENODEV); }
>> > > > >
>> > > > > +/*
>> > > > > + * The S25FS-S family physical sectors may be configured as a
>> > > > > + * hybrid combination of eight 4-kB parameter sectors
>> > > > > + * at the top or bottom of the address space with all
>> > > > > + * but one of the remaining sectors being uniform size.
>> > > > > + * The Parameter Sector Erase commands (20h or 21h) must
>> > > > > + * be used to erase the 4-kB parameter sectors individually.
>> > > > > + * The Sector (uniform sector) Erase commands (D8h or DCh)
>> > > > > + * must be used to erase any of the remaining
>> > > > > + * sectors, including the portion of highest or lowest address
>> > > > > + * sector that is not overlaid by the parameter sectors.
>> > > > > + * The uniform sector erase command has no effect on parameter
>> > > > sectors.
>> > > > > + */
>> > > > > +static int spansion_s25fs_disable_4kb_erase(struct spi_nor *nor) {
>> > > > > + u32 cr3v_addr = SPINOR_CONFIG_REG3_OFFSET;
>> > > > > + u8 cr3v = 0x0;
>> > > > > + int ret = 0x0;
>> > > > > +
>> > > > > + nor->cmd_buf[2] = cr3v_addr >> 16;
>> > > > > + nor->cmd_buf[1] = cr3v_addr >> 8;
>> > > > > + nor->cmd_buf[0] = cr3v_addr >> 0;
>> > > > > +
>> > > > > + ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR, &cr3v, 1);
>> > > > > + if (ret)
>> > > > > + return ret;
>> > > > > + if (cr3v & CR3V_4KB_ERASE_UNABLE)
>> > > > > + return 0;
>> > > > > + ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
>> > > > > + if (ret)
>> > > > > + return ret;
>> > > > > + cr3v = CR3V_4KB_ERASE_UNABLE;
>> > > > > + nor->program_opcode = SPINOR_OP_SPANSION_WRAR;
>> > > > > + nor->write(nor, cr3v_addr, 1, &cr3v);
>> > > > > +
>> > > > > + ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR, &cr3v, 1);
>> > > > > + if (ret)
>> > > > > + return ret;
>> > > > > + if (!(cr3v & CR3V_4KB_ERASE_UNABLE))
>> > > > > + return -EPERM;
>> > > > > +
>> > > > > + return 0;
>> > > > > +}
>> > > > > +
>> > > > > static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
>> > > > > size_t *retlen, u_char *buf) { @@
>> > > > > -1361,6
>> > > > > +1411,12 @@ int spi_nor_scan(struct spi_nor *nor, const char
>> > > > > +*name,
>> > > > enum read_mode mode)
>> > > > > spi_nor_wait_till_ready(nor);
>> > > > > }
>> > > > >
>> > > > > + if (EXT_JEDEC(info) == SPINOR_S25FS_FAMILY_EXT_JEDEC) {
>> > > > > + ret = spansion_s25fs_disable_4kb_erase(nor);
>> > > > > + if (ret)
>> > > > > + return ret;
>> > > > > + }
>> > > > > +
>> > > > > if (!mtd->name)
>> > > > > mtd->name = dev_name(dev);
>> > > > > mtd->priv = nor;
>> > > > > --
>> > > > > 2.1.0.27.g96db324
>> > > > >
>> > > > >
>> > > > Hi Brian, I will ack this change but still feel it's kind of hacking code.
>> > > >
>> > > > Acked-by: Han xu <han.xu@nxp.com>
>> > >
>> > > I am new on the list so I am not sure if this topic has been discussed.
>> > > Generally our product functionality relay on those 4KiB sectors.
>> > > I know that this hack is already in u-boot, but if you mainstream
>> > > this you will force users of those 4KiB sectors to do hack the hack...
>> > > I believe the proper solution here is to use erase regions
>> > > functionality, I send and RFS about that some time ago.
>> >
>> > Do you mind to send me a link for reference?
>> >
>> Han,
>>
>> Sorry, It seem I have not posted erase region changes (only those regarding
>> DUAL/QUAD I/O).
>> Generally, in this flash you need to create 3 erase regions (because in FS-S
>> family support only 4KiB erase on parameters sector - eg. 1.2.2.4 in S25FS512S).
>> In my case regions are:
>> 1. 0-32KiB (8*4KiB) - 4K_ERASE (0x20/0x21) 2. 32 - 256 - SE_CMD (0xd8/0xdc) 3.
>> Rest of the flash SE_CMD (0xd8/0xdc)
>>
>> To erase whole flash you can also use CHIP_ERASE_CMD (0x60/0xC7) command,
>> you just need to add one more mtd partition that will cover whole flash.
>>
>
> Hi Krzeminski,
>
> Do you think is there any great advantages for enable 4KB?
> Because for NXP(Freescale) QSPI controller, there is only support max to 16 groups command.
If it's really necessary to support all command groups, you can try
apply my dynamic lut patch first.
https://patchwork.ozlabs.org/patch/676791/
>
> So It's hard to give 3 groups command just for erase (0x21,0xdc and 0xc7).
> So we have to disable the 4kb erase and only use 256kbytes in this patch.
>
>
--
Sincerely,
Han XU
^ permalink raw reply
* [RFC 6/6] ARM: dts: am57xx-beagle-x15-common: enable etnaviv
From: Nishanth Menon @ 2016-11-18 4:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOCHtYh8QebrYA2ioaXgURdc47QY4x+EwZLBXLLGP1-k7eAMmw@mail.gmail.com>
On 11/17/2016 10:26 PM, Robert Nelson wrote:
[...]
> Oh yeah, defintely, we can move gpu-subsystem to the base dra7.dtsi,
> as the whole dra.dtsi family has a gc320 and then the board device
> tree can enable it via:
>
> &bb2d {
> status = "okay";
> };
Yep, thanks.
--
Regards,
Nishanth Menon
^ permalink raw reply
* [PATCH v9 02/10] drm/mediatek: add *driver_data for different hardware settings
From: Daniel Kurtz @ 2016-11-18 4:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478865346-19043-3-git-send-email-yt.shen@mediatek.com>
Hi YT,
I don't see a reason to handle device_data in such a generic way at
the generic mtk_ddp_comp layer.
The device data is very component specific, so just define different
structs for different comp types, ie:
struct mtk_disp_ovl_driver_data {
unsigned int reg_ovl_addr;
unsigned int fmt_rgb565;
unsigned int fmt_rgb888;
};
struct mtk_disp_rdma_driver_data {
unsigned int fifo_pseudo_size;
};
struct mtk_disp_color_driver_data {
unsigned int color_offset;
};
Then add typed pointers to the local structs that use them, for example:
struct mtk_disp_ovl {
struct mtk_ddp_comp ddp_comp;
struct drm_crtc *crtc;
const struct mtk_disp_ovl_driver_data *data;
};
And fetch the device specific driver data directly in .probe, as you
are already doing:
static int mtk_disp_ovl_probe(struct platform_device *pdev) {
...
priv->data = of_device_get_match_data(dev);
...
}
More comments in-line...
On Fri, Nov 11, 2016 at 7:55 PM, YT Shen <yt.shen@mediatek.com> wrote:
> There are some hardware settings changed, between MT8173 & MT2701:
> DISP_OVL address offset changed, color format definition changed.
> DISP_RDMA fifo size changed.
> DISP_COLOR offset changed.
> MIPI_TX pll setting changed.
> And add prefix for mtk_ddp_main & mtk_ddp_ext & mutex_mod.
Nit: I think it would make sense to combine this patch with
drm/mediatek: rename macros, add chip prefix
>
> Signed-off-by: YT Shen <yt.shen@mediatek.com>
> ---
> drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 27 ++++++++++++++++-----------
> drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 11 +++++++++--
> drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 11 +++++++----
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 27 +++++++++++++++++++++------
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 13 +++++++++++++
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 25 ++++++++++++++++++-------
> drivers/gpu/drm/mediatek/mtk_drm_drv.h | 8 ++++++++
> drivers/gpu/drm/mediatek/mtk_mipi_tx.c | 24 +++++++++++++++++++++++-
> 8 files changed, 115 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 019b7ca..1139834 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -35,13 +35,10 @@
> #define DISP_REG_OVL_PITCH(n) (0x0044 + 0x20 * (n))
> #define DISP_REG_OVL_RDMA_CTRL(n) (0x00c0 + 0x20 * (n))
> #define DISP_REG_OVL_RDMA_GMC(n) (0x00c8 + 0x20 * (n))
> -#define DISP_REG_OVL_ADDR(n) (0x0f40 + 0x20 * (n))
Also, I would still use the "#define macros", for example
"DISP_REG_OVL_ADDR offsets, and use the named constant in the
driver_data:
#define DISP_REG_OVL_ADDR_MT8173 0x0f40
(and in a later patch:
#define DISP_REG_OVL_ADDR_MT2701 0x0040
)
Also, I would still use the macro rather than open coding the "0x20 *
(n)", and just pass 'ovl' to the overlay macros that depend on
hardware type.
Something like the following:
#define DISP_REG_OVL_ADDR(ovl, n) ((ovl)->data->ovl_addr + 0x20 * (n))
>
> #define OVL_RDMA_MEM_GMC 0x40402020
>
> #define OVL_CON_BYTE_SWAP BIT(24)
> -#define OVL_CON_CLRFMT_RGB565 (0 << 12)
> -#define OVL_CON_CLRFMT_RGB888 (1 << 12)
This seems like a really random and unnecessary hardware change.
Why chip designers, why!!?!?
For this one, it seems the polarity is either one way or the other, so
we can just use a bool to distinguish:
bool fmt_rgb565_is_0;
> +static const struct mtk_ddp_comp_driver_data mt8173_ovl_driver_data = {
> + .ovl = { DISP_REG_OVL_ADDR_MT8173, .fmt_rgb565_is_0 = true }
> +};
For use at runtime, the defines could become:
#define OVL_CON_CLRFMT_RGB565(ovl) ((ovl)->data->fmt_rgb565_is_0 ? 0
: OVL_CON_CLRFMT_RGB888)
#define OVL_CON_CLRFMT_RGB888(ovl) ((ovl)->data->fmt_rgb565_is_0 ?
OVL_CON_CLRFMT_RGB888 : 0)
> #define OVL_CON_CLRFMT_RGBA8888 (2 << 12)
> #define OVL_CON_CLRFMT_ARGB8888 (3 << 12)
> #define OVL_CON_AEN BIT(8)
> @@ -137,18 +134,18 @@ static void mtk_ovl_layer_off(struct mtk_ddp_comp *comp, unsigned int idx)
> writel(0x0, comp->regs + DISP_REG_OVL_RDMA_CTRL(idx));
> }
>
> -static unsigned int ovl_fmt_convert(unsigned int fmt)
> +static unsigned int ovl_fmt_convert(struct mtk_ddp_comp *comp, unsigned int fmt)
> {
> switch (fmt) {
> default:
> case DRM_FORMAT_RGB565:
> - return OVL_CON_CLRFMT_RGB565;
> + return comp->data->ovl.fmt_rgb565;
It will be nice to define a helper function for converting from the
generic 'mtk_ddp_comp' to the specific 'mtk_disp_ovl':
static inline struct mtk_disp_ovl *comp_to_ovl(struct mtk_ddp_comp *comp) {
return container_of(comp, struct mtk_disp_ovl, ddp_comp);
}
Then these could become:
return OVL_CON_CLRFMT_RGB565(comp_to_ovl(comp));
Or maybe cleaner, do the conversion once at the top of the function:
struct mtk_disp_ovl *ovl = comp_to_ovl(comp);
And then just:
return OVL_CON_CLRFMT_RGB565(ovl);
> case DRM_FORMAT_BGR565:
> - return OVL_CON_CLRFMT_RGB565 | OVL_CON_BYTE_SWAP;
> + return comp->data->ovl.fmt_rgb565 | OVL_CON_BYTE_SWAP;
> case DRM_FORMAT_RGB888:
> - return OVL_CON_CLRFMT_RGB888;
> + return comp->data->ovl.fmt_rgb888;
> case DRM_FORMAT_BGR888:
> - return OVL_CON_CLRFMT_RGB888 | OVL_CON_BYTE_SWAP;
> + return comp->data->ovl.fmt_rgb888 | OVL_CON_BYTE_SWAP;
> case DRM_FORMAT_RGBX8888:
> case DRM_FORMAT_RGBA8888:
> return OVL_CON_CLRFMT_ARGB8888;
[snip]
> diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> index 1c366f8..935a8ef 100644
> --- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> +++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> @@ -16,6 +16,7 @@
> #include <linux/delay.h>
> #include <linux/io.h>
> #include <linux/module.h>
> +#include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <linux/phy/phy.h>
>
> @@ -87,6 +88,9 @@
>
> #define MIPITX_DSI_PLL_CON2 0x58
>
> +#define MIPITX_DSI_PLL_TOP 0x64
> +#define RG_DSI_MPPLL_PRESERVE (0xff << 8)
> +
> #define MIPITX_DSI_PLL_PWR 0x68
> #define RG_DSI_MPPLL_SDM_PWR_ON BIT(0)
> #define RG_DSI_MPPLL_SDM_ISO_EN BIT(1)
> @@ -123,10 +127,15 @@
> #define SW_LNT2_HSTX_PRE_OE BIT(24)
> #define SW_LNT2_HSTX_OE BIT(25)
>
> +struct mtk_mipitx_data {
> + const u32 data;
Use a better name, like "mppll_preserve".
Ok, that's it for now.
Actually, the patch set in general looks pretty good.
-Dan
^ permalink raw reply
* [PATCH v5] drm/mediatek: fixed the calc method of data rate per lane
From: CK Hu @ 2016-11-18 5:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGS+omBpdRH5ZnvcApX_pevwSBHwUon77DhhjX0p9aQXuOy4DA@mail.gmail.com>
Hi, Daniel:
On Fri, 2016-11-18 at 11:22 +0800, Daniel Kurtz wrote:
> Hi CK,
>
> On Thu, Nov 17, 2016 at 1:36 PM, CK Hu <ck.hu@mediatek.com> wrote:
> > Hi, Jitao:
> >
> >
> > On Wed, 2016-11-16 at 11:20 +0800, Jitao Shi wrote:
> >> Tune dsi frame rate by pixel clock, dsi add some extra signal (i.e.
> >> Tlpx, Ths-prepare, Ths-zero, Ths-trail,Ths-exit) when enter and exit LP
> >> mode, those signals will cause h-time larger than normal and reduce FPS.
> >> So need to multiply a coefficient to offset the extra signal's effect.
> >> coefficient = ((htotal*bpp/lane_number)+Tlpx+Ths_prep+Ths_zero+
> >> Ths_trail+Ths_exit)/(htotal*bpp/lane_number)
> >>
> >> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> >
> > It looks good to me.
> > But this patch conflict with [1] which is one patch of MT2701 series. I
> > want to apply MT2701 patches first, so please help to refine this patch
> > based on MT2701 patches.
>
> I don't think the MT2701 DSI patches are quite ready yet (I just
> reviewed the one below).
> Can we instead land Jitao's small targeted change first, and then
> rebase the MT2701 series on top.
>
> Thanks,
> -Dan
MT2701 series looks still have some defect to be fixed.
Therefore, I would apply this patch first.
Thanks for your help.
Regards,
CK
> >
> > [1] https://patchwork.kernel.org/patch/9422821/
> >
> > Regards,
> > CK
> >
> >> ---
> >> Change since v4:
> >> - tune the calc comment more clear.
> >> - define the phy timings as constants.
> >>
> >> Chnage since v3:
> >> - wrapp the commit msg.
> >> - fix alignment of some lines.
> >>
> >> Change since v2:
> >> - move phy timing back to dsi_phy_timconfig.
> >>
> >> Change since v1:
> >> - phy_timing2 and phy_timing3 refer clock cycle time.
> >> - define values of LPX HS_PRPR HS_ZERO HS_TRAIL TA_GO TA_SURE TA_GET DA_HS_EXIT.
> >> ---
> >>
> >
^ permalink raw reply
* [RFC v3 00/10] KVM PCIe/MSI passthrough on ARM/ARM64 and IOVA reserved regions
From: Bharat Bhushan @ 2016-11-18 5:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479215363-2898-1-git-send-email-eric.auger@redhat.com>
Hi Eric,
Have you sent out QEMU side patches based on this new approach? In case I missed please point me the patches?
Thanks
-Bharat
> -----Original Message-----
> From: iommu-bounces at lists.linux-foundation.org [mailto:iommu-
> bounces at lists.linux-foundation.org] On Behalf Of Eric Auger
> Sent: Tuesday, November 15, 2016 6:39 PM
> To: eric.auger at redhat.com; eric.auger.pro at gmail.com;
> christoffer.dall at linaro.org; marc.zyngier at arm.com;
> robin.murphy at arm.com; alex.williamson at redhat.com;
> will.deacon at arm.com; joro at 8bytes.org; tglx at linutronix.de;
> jason at lakedaemon.net; linux-arm-kernel at lists.infradead.org
> Cc: drjones at redhat.com; kvm at vger.kernel.org; punit.agrawal at arm.com;
> linux-kernel at vger.kernel.org; iommu at lists.linux-foundation.org;
> pranav.sawargaonkar at gmail.com
> Subject: [RFC v3 00/10] KVM PCIe/MSI passthrough on ARM/ARM64 and
> IOVA reserved regions
>
> Following LPC discussions, we now report reserved regions through iommu-
> group sysfs reserved_regions attribute file.
>
> Reserved regions are populated through the IOMMU get_resv_region
> callback (former get_dm_regions), now implemented by amd-iommu, intel-
> iommu and arm-smmu.
>
> The intel-iommu reports the [FEE0_0000h - FEF0_000h] MSI window as an
> IOMMU_RESV_NOMAP reserved region.
>
> arm-smmu reports the MSI window (arbitrarily located at 0x8000000 and 1MB
> large) and the PCI host bridge windows.
>
> The series integrates a not officially posted patch from Robin:
> "iommu/dma: Allow MSI-only cookies".
>
> This series currently does not address IRQ safety assessment.
>
> Best Regards
>
> Eric
>
> Git: complete series available at
> https://github.com/eauger/linux/tree/v4.9-rc5-reserved-rfc-v3
>
> History:
> RFC v2 -> v3:
> - switch to an iommu-group sysfs API
> - use new dummy allocator provided by Robin
> - dummy allocator initialized by vfio-iommu-type1 after enumerating
> the reserved regions
> - at the moment ARM MSI base address/size is left unchanged compared
> to v2
> - we currently report reserved regions and not usable IOVA regions as
> requested by Alex
>
> RFC v1 -> v2:
> - fix intel_add_reserved_regions
> - add mutex lock/unlock in vfio_iommu_type1
>
>
> Eric Auger (10):
> iommu/dma: Allow MSI-only cookies
> iommu: Rename iommu_dm_regions into iommu_resv_regions
> iommu: Add new reserved IOMMU attributes
> iommu: iommu_alloc_resv_region
> iommu: Do not map reserved regions
> iommu: iommu_get_group_resv_regions
> iommu: Implement reserved_regions iommu-group sysfs file
> iommu/vt-d: Implement reserved region get/put callbacks
> iommu/arm-smmu: Implement reserved region get/put callbacks
> vfio/type1: Get MSI cookie
>
> drivers/iommu/amd_iommu.c | 20 +++---
> drivers/iommu/arm-smmu.c | 52 +++++++++++++++
> drivers/iommu/dma-iommu.c | 116 ++++++++++++++++++++++++++----
> ---
> drivers/iommu/intel-iommu.c | 50 ++++++++++----
> drivers/iommu/iommu.c | 141
> ++++++++++++++++++++++++++++++++++++----
> drivers/vfio/vfio_iommu_type1.c | 26 ++++++++
> include/linux/dma-iommu.h | 7 ++
> include/linux/iommu.h | 49 ++++++++++----
> 8 files changed, 391 insertions(+), 70 deletions(-)
>
> --
> 1.9.1
>
> _______________________________________________
> iommu mailing list
> iommu at lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply
* [PATCH] kirkwood: fix spelling mistake
From: p.wassi at gmx.at @ 2016-11-18 6:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161117210227.GL5574@lunn.ch>
Hi Andrew,
> You should also send the patch to the Marvell MVEBU
> maintainers.
how do I find Marvell mvebu maintainers?
The list in
https://www.kernel.org/doc/linux/MAINTAINERS
(line 1429) references mvebu/kirkwood and gave me the
mailing list's address. Should the other two maintainers
be addressed directly?
Best regards,
P. Wassi
^ permalink raw reply
* [PATCH 1/3] arm: hisi: add ARCH_MULTI_V5 support
From: Jiancheng Xue @ 2016-11-18 6:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <114569cb-79ab-a46d-8582-6169f182a32c@hisilicon.com>
Hi Marty,
On 2016/11/17 11:03, Jiancheng Xue wrote:
> Hi Wei?
>
> On 2016/11/16 17:31, Wei Xu wrote:
>> Hi Pan,
>>
>> On 2016/11/16 8:56, wenpan wrote:
>>> Hi Marty?
>>> Does this confict with your patch? If not?I hope this could be merged first. Besides could you tell me the link to your related patch?
>>
>> This is the link: https://patchwork.kernel.org/patch/9334743/
>>
Could you give your comments on this patch?
If you have any objections to it, please let us know.
>
> Thank you for offering this.If I want to give some comments on Marty's patch,
> what should I do?
>
> For Marty's patch, I think there's no need to add specific config item ARCH_HIxxxx
> for every chipset. Some existing chipsets depend on ARCH_HISI directly like Hi3519
> and Hi3798CV200. If some options like ARM_GIC is removed from ARCH_HISI, this kind
> of chipsets will must choose other place to select it. I suggest we should keep selecting
> ARM_GIC under ARCH_HISI as Pan's patch do.
>
> The code may be like this:
>
> config ARCH_HISI
> bool "Hisilicon SoC Support"
> - depends on ARCH_MULTI_V7
> + depends on ARCH_MULTI_V5 || ARCH_MULTI_V6 || ARCH_MULTI_V7
> select ARM_AMBA
> - select ARM_GIC
> + select ARM_GIC if ARCH_MULTI_V7
> + select ARM_VIC if ARCH_MULTI_V5 || depends on ARCH_MULTI_V6
> select ARM_TIMER_SP804
> select POWER_RESET
> select POWER_RESET_HISI
> select POWER_SUPPLY
>
What's your opinion about this?
Best Regards,
Jiancheng
>>> On 2016/10/17 21:48, Arnd Bergmann wrote:
>>>> On Monday, October 17, 2016 8:07:03 PM CEST Pan Wen wrote:
>>>>> Add support for some HiSilicon SoCs which depend on ARCH_MULTI_V5.
>>>>>
>>>>> Signed-off-by: Pan Wen <wenpan@hisilicon.com>
>>>>>
>>>>
>>>> Looks ok. I've added Marty Plummer to Cc, he was recently proposing
>>>> patches for Hi3520, which I think is closely related to this one.
>>>> Please try to work together so the patches don't conflict. It should
>>>> be fairly straightforward since you are basically doing the same
>>>> change here.
>>>>
>
>
>
> .
>
^ permalink raw reply
* [PATCH] ARM: Drop fixed 200 Hz timer requirement from Exynos platforms
From: Krzysztof Kozlowski @ 2016-11-18 6:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3145378.USf2WOPoV2@wuerfel>
On Thu, Nov 17, 2016 at 01:35:45PM +0100, Arnd Bergmann wrote:
> On Monday, November 14, 2016 8:27:05 PM CET Krzysztof Kozlowski wrote:
> > @@ -1497,7 +1497,7 @@ source kernel/Kconfig.preempt
> > config HZ_FIXED
> > int
> > default 200 if ARCH_EBSA110 || ARCH_S3C24XX || \
> > - ARCH_S5PV210 || ARCH_EXYNOS4
> > + ARCH_S5PV210
> > default 128 if SOC_AT91RM9200
> > default 0
>
> After further research, I've concluded that we should also drop the
> settings for ARCH_S5PV210 and ARCH_S3C24XX here.
>
> ARCH_S5PV210 behaves exactly like EXYNOS here, it has 32-bit timers
> so there won't be any overflow with 100Hz.
>
> For ARCH_S3C24XX, it the requirement was that HZ_100 could not
> be used with the old arch/arm/plat-samsung/time.c code that would
> overflow its 16-bit counter.
> However, the new drivers/clocksource/samsung_pwm_timer.c configures
> the clock divider to '50' instead of '6', so there is no longer
> a 16-bit overflow before the 100Hz tick, it now overflows every
> 3.7ms for the typical 12MHz clock.
I can send an updated version however testing would be nice... I know
Sylwester has a S3C6410 platform running, maybe S3C24xx as well.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH] clk: sunxi-ng: sun6i-a31: Enable PLL-MIPI LDOs when ungating it
From: Chen-Yu Tsai @ 2016-11-18 7:15 UTC (permalink / raw)
To: linux-arm-kernel
The PLL-MIPI clock is somewhat special as it has its own LDOs which
need to be turned on for this PLL to actually work and output a clock
signal.
Add the 2 LDO enable bits to the gate bits. This fixes issues with
the TCON not sending vblank interrupts when the tcon and dot clock are
indirectly clocked from the PLL-MIPI clock.
Fixes: c6e6c96d8fa6 ("clk: sunxi-ng: Add A31/A31s clocks")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
This can be queued for either 4.9 or 4.10.
The clock driver was introduced in 4.9,
but the users won't appear until 4.10.
---
drivers/clk/sunxi-ng/ccu-sun6i-a31.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/sunxi-ng/ccu-sun6i-a31.c b/drivers/clk/sunxi-ng/ccu-sun6i-a31.c
index 4a82a49cff5e..fc75a335a7ce 100644
--- a/drivers/clk/sunxi-ng/ccu-sun6i-a31.c
+++ b/drivers/clk/sunxi-ng/ccu-sun6i-a31.c
@@ -143,7 +143,7 @@ static SUNXI_CCU_NKM_WITH_MUX_GATE_LOCK(pll_mipi_clk, "pll-mipi",
4, 2, /* K */
0, 4, /* M */
21, 0, /* mux */
- BIT(31), /* gate */
+ BIT(31) | BIT(23) | BIT(22), /* gate */
BIT(28), /* lock */
CLK_SET_RATE_UNGATE);
--
2.10.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox