* [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Alexandre Courbot @ 2012-09-12 9:57 UTC (permalink / raw)
To: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann
Cc: Leela Krishna Amudala, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-pm-u79uwXL29TY76Z2rM5mHXA, linux-doc-u79uwXL29TY76Z2rM5mHXA,
Alexandre Courbot
In-Reply-To: <1347443867-18868-1-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Some device drivers (panel backlights especially) need to follow precise
sequences for powering on and off, involving gpios, regulators, PWMs
with a precise powering order and delays to respect between each steps.
These sequences are board-specific, and do not belong to a particular
driver - therefore they have been performed by board-specific hook
functions to far.
With the advent of the device tree and of ARM kernels that are not
board-tied, we cannot rely on these board-specific hooks anymore but
need a way to implement these sequences in a portable manner. This patch
introduces a simple interpreter that can execute such power sequences
encoded either as platform data or within the device tree.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
.../devicetree/bindings/power_seq/power_seq.txt | 122 ++++++
Documentation/power/power_seq.txt | 215 ++++++++++
drivers/power/Kconfig | 1 +
drivers/power/Makefile | 1 +
drivers/power/power_seq/Kconfig | 2 +
drivers/power/power_seq/Makefile | 1 +
drivers/power/power_seq/power_seq.c | 446 +++++++++++++++++++++
drivers/power/power_seq/power_seq_delay.c | 51 +++
drivers/power/power_seq/power_seq_gpio.c | 91 +++++
drivers/power/power_seq/power_seq_pwm.c | 87 ++++
drivers/power/power_seq/power_seq_regulator.c | 87 ++++
include/linux/power_seq.h | 172 ++++++++
12 files changed, 1276 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power_seq/power_seq.txt
create mode 100644 Documentation/power/power_seq.txt
create mode 100644 drivers/power/power_seq/Kconfig
create mode 100644 drivers/power/power_seq/Makefile
create mode 100644 drivers/power/power_seq/power_seq.c
create mode 100644 drivers/power/power_seq/power_seq_delay.c
create mode 100644 drivers/power/power_seq/power_seq_gpio.c
create mode 100644 drivers/power/power_seq/power_seq_pwm.c
create mode 100644 drivers/power/power_seq/power_seq_regulator.c
create mode 100644 include/linux/power_seq.h
diff --git a/Documentation/devicetree/bindings/power_seq/power_seq.txt b/Documentation/devicetree/bindings/power_seq/power_seq.txt
new file mode 100644
index 0000000..2b394eb
--- /dev/null
+++ b/Documentation/devicetree/bindings/power_seq/power_seq.txt
@@ -0,0 +1,122 @@
+Runtime Interpreted Power Sequences
+=================+
+Power sequences are sequential descriptions of actions to be performed on
+power-related resources. Having these descriptions in a well-defined data format
+allows us to take much of the board-specific power control code out of the
+kernel and place it into the device tree instead, making kernels less
+board-dependant.
+
+A device typically makes use of multiple power sequences, for different purposes
+such as powering on and off. All the power sequences of a given device are
+grouped into a set. In the device tree, this set is a sub-node of the device
+node named "power-sequences".
+
+Power Sequences Structure
+-------------------------
+Every device that makes use of power sequences must have a "power-sequences"
+node into which individual power sequences are declared as sub-nodes. The name
+of the node becomes the name of the sequence within the power sequences
+framework.
+
+Similarly, each power sequence declares its steps as sub-nodes of itself. Steps
+must be named sequentially, with the first step named step0, the second step1,
+etc. Failure to follow this rule will result in a parsing error.
+
+Power Sequences Steps
+---------------------
+Steps of a sequence describe an action to be performed on a resource. They
+always include a "type" property which indicates what kind of resource this
+step works on. Depending on the resource type, additional properties are defined
+to control the action to be performed.
+
+"delay" type required properties:
+ - delay: delay to wait (in microseconds)
+
+"regulator" type required properties:
+ - id: name of the regulator to use. Regulator is obtained by
+ regulator_get(dev, id)
+ - enable / disable: one of these two empty properties must be present to
+ enable or disable the resource
+
+"pwm" type required properties:
+ - id: name of the PWM to use. PWM is obtained by pwm_get(dev, id)
+ - enable / disable: one of these two empty properties must be present to
+ enable or disable the resource
+
+"gpio" type required properties:
+ - gpio: phandle of the GPIO to use.
+ - value: value this GPIO should take. Must be 0 or 1.
+
+Example
+-------
+Here are example sequences declared within a backlight device that use all the
+supported resources types:
+
+ backlight {
+ compatible = "pwm-backlight";
+ ...
+
+ /* resources used by the power sequences */
+ pwms = <&pwm 2 5000000>;
+ pwm-names = "backlight";
+ power-supply = <&backlight_reg>;
+
+ power-sequences {
+ power-on {
+ step0 {
+ type = "regulator";
+ id = "power";
+ enable;
+ };
+ step1 {
+ type = "delay";
+ delay = <10000>;
+ };
+ step2 {
+ type = "pwm";
+ id = "backlight";
+ enable;
+ };
+ step3 {
+ type = "gpio";
+ gpio = <&gpio 28 0>;
+ value = <1>;
+ };
+ };
+
+ power-off {
+ step0 {
+ type = "gpio";
+ gpio = <&gpio 28 0>;
+ value = <0>;
+ };
+ step1 {
+ type = "pwm";
+ id = "backlight";
+ disable;
+ };
+ step2 {
+ type = "delay";
+ delay = <10000>;
+ };
+ step3 {
+ type = "regulator";
+ id = "power";
+ disable;
+ };
+ };
+ };
+ };
+
+The first part lists the PWM and regulator resources used by the sequences.
+These resources will be requested on behalf of the backlight device when the
+sequences are built and are declared according to their own framework (for
+instance, regulators and pwms are resolved by name using regulator_get() or
+pwm_get()).
+
+After the resources declaration, two sequences follow for powering the backlight
+on and off. Their names are specified by the pwm-backlight device bindings. Once
+the sequences are built by calling devm_power_seq_set_build() with a NULL pseq
+argument, their names can be passed to power_seq_lookup() to retrieve an actual
+sequence.
diff --git a/Documentation/power/power_seq.txt b/Documentation/power/power_seq.txt
new file mode 100644
index 0000000..7f3f2d2
--- /dev/null
+++ b/Documentation/power/power_seq.txt
@@ -0,0 +1,215 @@
+Runtime Interpreted Power Sequences
+=================+
+Problem
+-------
+Very commonly, boards need the help of out-of-driver code to turn some of their
+devices on and off. For instance, SoC boards very commonly use a GPIO
+(abstracted to a regulator or not) to control the power supply of a backlight,
+disabling it when the backlight is not used in order to save power. The GPIO
+that should be used, however, as well as the exact power sequence that may
+also involve other resources, is board-dependent and thus unknown to the driver.
+
+This was previously addressed by having hooks in the device's platform data that
+are called whenever the state of the device might reflect a power change. This
+approach, however, introduces board-dependant code into the kernel and is not
+compatible with the device tree.
+
+The Runtime Interpreted Power Sequences (or power sequences for short) aim at
+turning this code into platform data or device tree nodes. Power sequences are
+described using a simple format and run by a lightweight interpreter whenever
+needed. This allows device drivers to work without power callbacks and makes the
+kernel less board-dependant.
+
+What are Power Sequences?
+-------------------------
+A power sequence is a suite of sequential steps each describing an action to be
+performed on a resource. The supported resources and actions operations are:
+- delay (just wait for a given number of microseconds)
+- GPIO (set to 0 or 1)
+- regulator (enable or disable)
+- PWM (enable or disable)
+
+When a power sequence is run, each of its steps is executed one after the other
+until one step fails or the end of the sequence is reached.
+
+Power sequences are named, and grouped into "sets" which correspond to all the
+sequences of a device as well as the resources they use.
+
+Power sequences can be declared as platform data or in the device tree.
+
+Platform Data Format
+--------------------
+All relevant data structures for declaring power sequences are located in
+include/linux/power_seq.h.
+
+The platform data for a device may include an instance of platform_power_seq_set
+which references instances of platform_power_seq. Each platform_power_seq is a
+single power sequence, and is itself made of a variable length array of steps.
+
+A step is a union of all the platform step structures, which are defined and
+documented in include/linux/power_seq.h. Which one is to be used depends on the
+type of the step. This might sound complicated, but the following example should
+make it clear how the platform data for power sequences is defined. It declares
+two power sequences named "power-on" and "power-off". The
+"power-on" sequence enables a regulator named "power" (retrieved using
+regulator_get()), waits for 10ms, and then set GPIO 110 to 1. "power-off" does
+the opposite.
+
+static struct platform_power_seq power_on_seq = {
+ .id = "power-on",
+ .num_steps = 3,
+ .steps = {
+ {
+ .type = POWER_SEQ_REGULATOR,
+ .regulator = {
+ .id = "power",
+ .enable = true,
+ },
+ },
+ {
+ .type = POWER_SEQ_DELAY,
+ .delay = {
+ .delay = 10000,
+ },
+ },
+ {
+ .type = POWER_SEQ_GPIO,
+ .gpio = {
+ .gpio = 110,
+ .value = 1,
+ },
+ },
+ },
+};
+
+static struct platform_power_seq power_off_seq = {
+ .id = "power-off",
+ .num_steps = 3,
+ .steps = {
+ {
+ .type = POWER_SEQ_GPIO,
+ .gpio = {
+ .gpio = 110,
+ .value = 0,
+ },
+ },
+ {
+ .type = POWER_SEQ_DELAY,
+ .delay = {
+ .delay = 10000,
+ },
+ },
+ {
+ .type = POWER_SEQ_REGULATOR,
+ .regulator = {
+ .id = "power",
+ .enable = false,
+ },
+ },
+ },
+};
+
+static struct platform_power_seq_set platform_power_sequences = {
+ .num_seqs = 2,
+ .seqs = {
+ &power_on_seq,
+ &power_off_seq,
+ },
+};
+
+"platform_power_sequences" can then be passed to devm_power_seq_set_build() in
+order to build the corresponding sequences set and allocate all the necessary
+resources. More on this later in this document.
+
+Device Tree
+-----------
+Power sequences can also be encoded as device tree nodes. The following
+properties and nodes are equivalent to the platform data defined previously:
+
+power-supply = <&power_reg>;
+
+power-sequences {
+ power-on {
+ step0 {
+ type = "regulator";
+ id = "power";
+ enable;
+ };
+ step1 {
+ type = "delay";
+ delay = <10000>;
+ };
+ step2 {
+ type = "gpio";
+ gpio = <&gpio 110 0>;
+ value = <1>;
+ };
+ }
+ power-off {
+ step0 {
+ type = "gpio";
+ gpio = <&gpio 110 0>;
+ value = <0>;
+ };
+ step1 {
+ type = "delay";
+ delay = <10000>;
+ };
+ step2 {
+ type = "regulator";
+ id = "power";
+ disable;
+ };
+ }
+};
+
+See Documentation/devicetree/bindings/power_seq/power_seq.txt for the complete
+syntax of the DT bindings.
+
+Usage by Drivers and Resources Management
+-----------------------------------------
+Power sequences make use of resources that must be properly allocated and
+managed. The devm_power_seq_set_build() function builds a power sequence set
+from platform data. It also takes care of resolving and allocating the resources
+referenced by the sequence:
+
+ struct power_seq_set *devm_power_seq_set_build(struct device *dev,
+ struct platform_power_seq_set *pseq);
+
+As its name states, all memory and resources are devm-allocated. The 'dev'
+argument is the device in the name of which the resources are to be allocated.
+
+If the pseq argument is NULL, then the power sequences are built from the device
+tree data, if any.
+
+On success, the function returns a devm allocated resolved sequences set for
+which all the resources are allocated. In case of failure, an error code is
+returned. If no set can be built because no platform data is passed and the
+device tree node for the device contains no power sequence, the function returns
+NULL.
+
+Once the set is built, power sequences can be retrieved by their name using
+power_seq_lookup:
+
+ struct power_seq *power_seq_lookup(struct power_seq_set *seqs,
+ const char *id);
+
+A retrieved power sequence can then be executed by power_seq_run:
+
+ int power_seq_run(struct power_seq *seq);
+
+It returns 0 if the sequence has successfully been run, or an error code if a
+problem occurred.
+
+Sometimes, you may want to browse the list of resources allocated by a sequence,
+for instance to ensure that a resource of a given type is present. The
+power_seq_set_resources() function returns a list head that can be used with
+the power_seq_for_each_resource() macro to browse all the resources of a set:
+
+ struct list_head *power_seq_set_resources(struct power_seq_set *seqs);
+ power_seq_for_each_resource(pos, seqs)
+
+Here "pos" will be a pointer to a struct power_seq_resource. This structure
+contains the type of the resource, the information used for identifying it, and
+the resolved resource itself.
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index fcc1bb0..5fdfd84 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -312,3 +312,4 @@ config AB8500_BATTERY_THERM_ON_BATCTRL
endif # POWER_SUPPLY
source "drivers/power/avs/Kconfig"
+source "drivers/power/power_seq/Kconfig"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index ee58afb..d3c893b 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -45,3 +45,4 @@ obj-$(CONFIG_CHARGER_MAX8997) += max8997_charger.o
obj-$(CONFIG_CHARGER_MAX8998) += max8998_charger.o
obj-$(CONFIG_POWER_AVS) += avs/
obj-$(CONFIG_CHARGER_SMB347) += smb347-charger.o
+obj-$(CONFIG_POWER_SEQ) += power_seq/
diff --git a/drivers/power/power_seq/Kconfig b/drivers/power/power_seq/Kconfig
new file mode 100644
index 0000000..3bff26e
--- /dev/null
+++ b/drivers/power/power_seq/Kconfig
@@ -0,0 +1,2 @@
+config POWER_SEQ
+ bool
diff --git a/drivers/power/power_seq/Makefile b/drivers/power/power_seq/Makefile
new file mode 100644
index 0000000..f77a359
--- /dev/null
+++ b/drivers/power/power_seq/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_POWER_SEQ) += power_seq.o
diff --git a/drivers/power/power_seq/power_seq.c b/drivers/power/power_seq/power_seq.c
new file mode 100644
index 0000000..0924a03
--- /dev/null
+++ b/drivers/power/power_seq/power_seq.c
@@ -0,0 +1,446 @@
+/*
+ * power_seq.c - A simple power sequence interpreter for platform devices
+ * and device tree.
+ *
+ * Author: Alexandre Courbot <acourbot@nvidia.com>
+ *
+ * Copyright (c) 2012 NVIDIA Corporation.
+ *
+ * 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; version 2 of the License.
+ *
+ * This program 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.
+ *
+ */
+
+#include <linux/power_seq.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/device.h>
+
+#include <linux/of.h>
+
+struct power_seq_set {
+ struct device *dev;
+ struct list_head resources;
+ struct list_head sequences;
+};
+
+struct power_seq_step {
+ /* Copy of the platform data */
+ struct platform_power_seq_step pdata;
+ /* Resolved resource */
+ struct power_seq_resource *resource;
+};
+
+struct power_seq {
+ /* Set this sequence belongs to */
+ struct power_seq_set *parent_set;
+ const char *id;
+ /* To thread into power_seqs structure */
+ struct list_head list;
+ unsigned int num_steps;
+ struct power_seq_step steps[];
+};
+
+#define power_seq_err(dev, seq, step_nbr, format, ...) \
+ dev_err(dev, "%s[%d]: " format, seq->id, step_nbr, ##__VA_ARGS__);
+
+/**
+ * struct power_seq_res_type - operators for power sequences resources
+ * @name: Name of the resource type. Set to null when a resource
+ * type support is not compiled in
+ * @need_resource: Whether a resource needs to be allocated when steps of
+ * this kind are met. If set to false, res_compare and
+ * res_alloc need not be set
+ * @of_parse: Parse a step for this kind of resource from a device
+ * tree node. The result of parsing must be written into
+ * step step_nbr of seq
+ * @step_run: Run a step for this kind of resource
+ * @res_compare: Return true if the resource used by the resource is the
+ * same as the one referenced by the step, false otherwise.
+ * @res_alloc: Resolve and allocate the resource used by pstep into
+ * seq. The memory for seq is already allocated and its
+ * type member is already set when this function is called.
+ * Return error code if the resource cannot be allocated, 0
+ * if allocation went successfully.
+ */
+struct power_seq_res_ops {
+ const char *name;
+ bool need_resource;
+ int (*of_parse)(struct device *dev, struct device_node *node,
+ struct platform_power_seq *seq, unsigned int step_nbr);
+ int (*step_run)(struct power_seq_step *step);
+ bool (*res_compare)(struct power_seq_resource *res,
+ struct platform_power_seq_step *step);
+ int (*res_alloc)(struct device *dev,
+ struct platform_power_seq_step *pstep,
+ struct power_seq_resource *seq);
+};
+
+static const struct power_seq_res_ops power_seq_types[POWER_SEQ_NUM_TYPES];
+
+#ifdef CONFIG_OF
+static int of_power_seq_parse_enable_properties(struct device *dev,
+ struct device_node *node,
+ struct platform_power_seq *seq,
+ unsigned int step_nbr,
+ bool *enable)
+{
+ if (of_find_property(node, "enable", NULL)) {
+ *enable = true;
+ } else if (of_find_property(node, "disable", NULL)) {
+ *enable = false;
+ } else {
+ power_seq_err(dev, seq, step_nbr,
+ "missing enable or disable property\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int of_power_seq_parse_step(struct device *dev, struct device_node *node,
+ struct platform_power_seq *seq,
+ unsigned int step_nbr)
+{
+ struct platform_power_seq_step *step = &seq->steps[step_nbr];
+ const char *type;
+ int i, err;
+
+ err = of_property_read_string(node, "type", &type);
+ if (err < 0) {
+ power_seq_err(dev, seq, step_nbr,
+ "cannot read type property\n");
+ return err;
+ }
+ for (i = 0; i < POWER_SEQ_NUM_TYPES; i++) {
+ if (power_seq_types[i].name = NULL)
+ continue;
+ if (!strcmp(type, power_seq_types[i].name))
+ break;
+ }
+ if (i >= POWER_SEQ_NUM_TYPES) {
+ power_seq_err(dev, seq, step_nbr, "unknown type %s\n", type);
+ return -EINVAL;
+ }
+ step->type = i;
+ err = power_seq_types[step->type].of_parse(dev, node, seq, step_nbr);
+
+ return err;
+}
+
+static struct platform_power_seq *of_parse_power_seq(struct device *dev,
+ struct device_node *node)
+{
+ struct device_node *child = NULL;
+ struct platform_power_seq *pseq;
+ int num_steps, sz;
+ int err;
+
+ if (!node)
+ return ERR_PTR(-EINVAL);
+
+ num_steps = of_get_child_count(node);
+ sz = sizeof(*pseq) + sizeof(pseq->steps[0]) * num_steps;
+ pseq = devm_kzalloc(dev, sz, GFP_KERNEL);
+ if (!pseq)
+ return ERR_PTR(-ENOMEM);
+ pseq->num_steps = num_steps;
+ pseq->id = node->name;
+
+ for_each_child_of_node(node, child) {
+ unsigned int pos;
+
+ /* Check that the name's format is correct and within bounds */
+ if (strncmp("step", child->name, 4)) {
+ err = -EINVAL;
+ goto parse_error;
+ }
+
+ err = kstrtouint(child->name + 4, 10, &pos);
+ if (err < 0)
+ goto parse_error;
+
+ if (pos >= num_steps || pseq->steps[pos].type != 0) {
+ err = -EINVAL;
+ goto parse_error;
+ }
+
+ err = of_power_seq_parse_step(dev, child, pseq, pos);
+ if (err)
+ return ERR_PTR(err);
+ }
+
+ return pseq;
+
+parse_error:
+ dev_err(dev, "%s: invalid power step name %s!\n", pseq->id,
+ child->name);
+ return ERR_PTR(err);
+}
+
+/*
+ * build power sequences platform data from a device tree node.
+ *
+ * Sequences must be contained into a subnode named "power-sequences" of the
+ * device root node.
+ *
+ * Memory for the platform sequence is allocated using devm_kzalloc on dev and
+ * can be freed by devm_kfree after power_seq_set_build returned. Beware that on
+ * top of the set itself, platform data for individual sequences should also be
+ * freed.
+ *
+ * Returns the built power sequence set on success, or an error code in case of
+ * failure.
+ */
+static
+struct platform_power_seq_set *devm_of_parse_power_seq_set(struct device *dev)
+{
+ struct platform_power_seq_set *seqs;
+ struct device_node *root = dev->of_node;
+ struct device_node *seq;
+ int num_seqs, sz, i = 0;
+
+ if (!root)
+ return NULL;
+
+ root = of_find_node_by_name(root, "power-sequences");
+ if (!root)
+ return NULL;
+
+ num_seqs = of_get_child_count(root);
+ sz = sizeof(*seqs) + sizeof(seqs->seqs[0]) * num_seqs;
+ seqs = devm_kzalloc(dev, sz, GFP_KERNEL);
+ if (!seqs)
+ return ERR_PTR(-ENOMEM);
+ seqs->num_seqs = num_seqs;
+
+ for_each_child_of_node(root, seq) {
+ struct platform_power_seq *pseq;
+
+ pseq = of_parse_power_seq(dev, seq);
+ if (IS_ERR(pseq))
+ return (void *)pseq;
+
+ seqs->seqs[i++] = pseq;
+ }
+
+ return seqs;
+}
+#endif /* CONFIG_OF */
+
+static struct power_seq_resource *
+power_seq_find_resource(struct list_head *ress,
+ struct platform_power_seq_step *step)
+{
+ struct power_seq_resource *res;
+
+ list_for_each_entry(res, ress, list) {
+ if (res->type != step->type)
+ continue;
+
+ if (power_seq_types[res->type].res_compare(res, step))
+ return res;
+ }
+
+ return NULL;
+}
+
+static struct power_seq *power_seq_build_one(struct device *dev,
+ struct power_seq_set *seqs,
+ struct platform_power_seq *pseq)
+{
+ struct power_seq *seq;
+ struct power_seq_resource *res;
+ int i, err;
+
+ seq = devm_kzalloc(dev, sizeof(*seq) + sizeof(seq->steps[0]) *
+ pseq->num_steps, GFP_KERNEL);
+ if (!seq)
+ return ERR_PTR(-ENOMEM);
+
+ INIT_LIST_HEAD(&seq->list);
+ seq->parent_set = seqs;
+ seq->num_steps = pseq->num_steps;
+ seq->id = pseq->id;
+
+ for (i = 0; i < seq->num_steps; i++) {
+ struct platform_power_seq_step *pstep = &pseq->steps[i];
+ struct power_seq_step *step = &seq->steps[i];
+
+ if (pstep->type >= POWER_SEQ_NUM_TYPES ||
+ power_seq_types[pstep->type].name = NULL) {
+ power_seq_err(dev, seq, i,
+ "invalid power sequence type %d!",
+ pstep->type);
+ return ERR_PTR(-EINVAL);
+ }
+
+ memcpy(&step->pdata, pstep, sizeof(step->pdata));
+
+ /* Steps without resource need not to continue */
+ if (!power_seq_types[pstep->type].need_resource)
+ continue;
+
+ /* create resource node if not referenced already */
+ res = power_seq_find_resource(&seqs->resources, pstep);
+ if (!res) {
+ res = devm_kzalloc(dev, sizeof(*res), GFP_KERNEL);
+ if (!res)
+ return ERR_PTR(-ENOMEM);
+
+ res->type = step->pdata.type;
+
+ err = power_seq_types[res->type].res_alloc(dev,
+ &step->pdata, res);
+ if (err < 0)
+ return ERR_PTR(err);
+
+ list_add_tail(&res->list, &seqs->resources);
+ }
+ step->resource = res;
+ }
+
+ return seq;
+}
+
+/**
+ * power_seq_set_build() - build a set of runnable sequences from platform data or device tree
+ * @dev: Device that will use the power sequences. All resources will be
+ * devm-allocated against it
+ * @pseq: Platform data for the power sequences. It can be freed after
+ * this function returns. If this parameter is null, power
+ * sequences are looked up in the device tree.
+ *
+ * All memory and resources (regulators, GPIOs, etc.) are allocated using devm
+ * functions.
+ *
+ * Returns the built sequence on success, an error code in case or failure,
+ * and NULL if neither platform data or device tree nodes are defined.
+ */
+struct power_seq_set *devm_power_seq_set_build(struct device *dev,
+ struct platform_power_seq_set *pseq)
+{
+ struct power_seq_set *seqs;
+ int i;
+ bool use_dt = false;
+
+ if (!pseq) {
+ use_dt = true;
+ pseq = devm_of_parse_power_seq_set(dev);
+ }
+
+ if (!pseq)
+ return NULL;
+
+ seqs = devm_kzalloc(dev, sizeof(*seqs), GFP_KERNEL);
+
+ if (!seqs)
+ return ERR_PTR(-ENOMEM);
+
+ INIT_LIST_HEAD(&seqs->resources);
+ INIT_LIST_HEAD(&seqs->sequences);
+ for (i = 0; i < pseq->num_seqs; i++) {
+ struct power_seq *seq;
+
+ seq = power_seq_build_one(dev, seqs, pseq->seqs[i]);
+ if (IS_ERR(seq))
+ return (void *)seq;
+
+ list_add_tail(&seq->list, &seqs->sequences);
+ }
+
+ /* if we used the DT, free the temporarily built platform data */
+ if (use_dt) {
+ for (i = 0; i < pseq->num_seqs; i++)
+ devm_kfree(dev, pseq->seqs[i]);
+
+ devm_kfree(dev, pseq);
+ }
+
+ return seqs;
+}
+EXPORT_SYMBOL_GPL(devm_power_seq_set_build);
+
+/**
+ * power_seq_lookup - Lookup a power sequence by name from a set
+ * @seqs: The set to look in
+ * @id: Name to look after
+ *
+ * Returns a matching power sequence if it exists, NULL if it does not.
+ */
+struct power_seq *power_seq_lookup(struct power_seq_set *seqs, const char *id)
+{
+ struct power_seq *seq;
+
+ list_for_each_entry(seq, &seqs->sequences, list) {
+ if (!strcmp(seq->id, id))
+ return seq;
+ }
+
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(power_seq_lookup);
+
+/**
+ * power_seq_set_resources - return a list of all the resources used by a set
+ * @seqs: Power sequences set we are interested in getting the resources
+ *
+ * The returned list can be parsed using the power_seq_for_each_resource macro.
+ */
+struct list_head *power_seq_set_resources(struct power_seq_set *seqs)
+{
+ return &seqs->resources;
+}
+EXPORT_SYMBOL_GPL(power_seq_set_resources);
+
+/**
+ * power_seq_run() - run a power sequence
+ * @seq: The power sequence to run
+ *
+ * Returns 0 on success, error code in case of failure.
+ */
+int power_seq_run(struct power_seq *seq)
+{
+ unsigned int i;
+ int err;
+
+ if (!seq)
+ return 0;
+
+ for (i = 0; i < seq->num_steps; i++) {
+ unsigned int type = seq->steps[i].pdata.type;
+
+ err = power_seq_types[type].step_run(&seq->steps[i]);
+ if (err) {
+ power_seq_err(seq->parent_set->dev, seq, i,
+ "error %d while running power sequence step\n",
+ err);
+ return err;
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(power_seq_run);
+
+#include "power_seq_delay.c"
+#include "power_seq_regulator.c"
+#include "power_seq_pwm.c"
+#include "power_seq_gpio.c"
+
+static const struct power_seq_res_ops power_seq_types[POWER_SEQ_NUM_TYPES] = {
+ [POWER_SEQ_DELAY] = POWER_SEQ_DELAY_TYPE,
+ [POWER_SEQ_REGULATOR] = POWER_SEQ_REGULATOR_TYPE,
+ [POWER_SEQ_PWM] = POWER_SEQ_PWM_TYPE,
+ [POWER_SEQ_GPIO] = POWER_SEQ_GPIO_TYPE,
+};
+
+MODULE_AUTHOR("Alexandre Courbot <acourbot@nvidia.com>");
+MODULE_DESCRIPTION("Runtime Interpreted Power Sequences");
+MODULE_LICENSE("GPL");
diff --git a/drivers/power/power_seq/power_seq_delay.c b/drivers/power/power_seq/power_seq_delay.c
new file mode 100644
index 0000000..ae94131
--- /dev/null
+++ b/drivers/power/power_seq/power_seq_delay.c
@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2012 NVIDIA Corporation.
+ *
+ * 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; version 2 of the License.
+ *
+ * This program 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.
+ *
+ */
+
+#include <linux/delay.h>
+
+#ifdef CONFIG_OF
+static int of_power_seq_parse_delay(struct device *dev,
+ struct device_node *node,
+ struct platform_power_seq *seq,
+ unsigned int step_nbr)
+{
+ struct platform_power_seq_step *step = &seq->steps[step_nbr];
+ int err;
+
+ err = of_property_read_u32(node, "delay",
+ &step->delay.delay);
+ if (err < 0)
+ power_seq_err(dev, seq, step_nbr,
+ "error reading delay property\n");
+
+ return err;
+}
+#else
+#define of_power_seq_parse_delay NULL
+#endif
+
+static int power_seq_step_run_delay(struct power_seq_step *step)
+{
+ usleep_range(step->pdata.delay.delay,
+ step->pdata.delay.delay + 1000);
+
+ return 0;
+}
+
+#define POWER_SEQ_DELAY_TYPE { \
+ .name = "delay", \
+ .need_resource = false, \
+ .of_parse = of_power_seq_parse_delay, \
+ .step_run = power_seq_step_run_delay, \
+}
diff --git a/drivers/power/power_seq/power_seq_gpio.c b/drivers/power/power_seq/power_seq_gpio.c
new file mode 100644
index 0000000..4f83d4c
--- /dev/null
+++ b/drivers/power/power_seq/power_seq_gpio.c
@@ -0,0 +1,91 @@
+/*
+ * Copyright (c) 2012 NVIDIA Corporation.
+ *
+ * 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; version 2 of the License.
+ *
+ * This program 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.
+ *
+ */
+
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+
+#ifdef CONFIG_OF
+static int power_seq_of_parse_gpio(struct device *dev,
+ struct device_node *node,
+ struct platform_power_seq *seq,
+ unsigned int step_nbr)
+{
+ struct platform_power_seq_step *step = &seq->steps[step_nbr];
+ int gpio;
+ int err;
+
+ gpio = of_get_named_gpio(node, "gpio", 0);
+ if (gpio < 0) {
+ power_seq_err(dev, seq, step_nbr,
+ "error reading gpio property\n");
+ return gpio;
+ }
+ step->gpio.gpio = gpio;
+
+ err = of_property_read_u32(node, "value", &step->gpio.value);
+ if (err < 0) {
+ power_seq_err(dev, seq, step_nbr,
+ "error reading value property\n");
+ } else if (step->gpio.value < 0 || step->gpio.value > 1) {
+ power_seq_err(dev, seq, step_nbr,
+ "value out of range (must be 0 or 1)\n");
+ err = -EINVAL;
+ }
+
+ return err;
+}
+#else
+#define of_power_seq_parse_gpio NULL
+#endif
+
+static bool power_seq_res_compare_gpio(struct power_seq_resource *res,
+ struct platform_power_seq_step *step)
+{
+ return res->gpio.gpio = step->gpio.gpio;
+}
+
+static int power_seq_res_alloc_gpio(struct device *dev,
+ struct platform_power_seq_step *pstep,
+ struct power_seq_resource *res)
+{
+ int err;
+
+ err = devm_gpio_request_one(dev, pstep->gpio.gpio,
+ GPIOF_OUT_INIT_LOW, dev_name(dev));
+ if (err) {
+ dev_err(dev, "cannot get gpio %d\n", pstep->gpio.gpio);
+ return err;
+ }
+
+ res->gpio.gpio = pstep->gpio.gpio;
+
+ return 0;
+}
+
+static int power_seq_step_run_gpio(struct power_seq_step *step)
+{
+ gpio_set_value_cansleep(step->resource->gpio.gpio,
+ step->pdata.gpio.value);
+
+ return 0;
+}
+
+#define POWER_SEQ_GPIO_TYPE { \
+ .name = "gpio", \
+ .need_resource = true, \
+ .of_parse = power_seq_of_parse_gpio, \
+ .step_run = power_seq_step_run_gpio, \
+ .res_compare = power_seq_res_compare_gpio, \
+ .res_alloc = power_seq_res_alloc_gpio, \
+}
diff --git a/drivers/power/power_seq/power_seq_pwm.c b/drivers/power/power_seq/power_seq_pwm.c
new file mode 100644
index 0000000..14f21e1
--- /dev/null
+++ b/drivers/power/power_seq/power_seq_pwm.c
@@ -0,0 +1,87 @@
+/*
+ * Copyright (c) 2012 NVIDIA Corporation.
+ *
+ * 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; version 2 of the License.
+ *
+ * This program 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.
+ *
+ */
+
+#ifdef CONFIG_PWM
+
+#include <linux/pwm.h>
+
+#ifdef CONFIG_OF
+static int power_seq_of_parse_pwm(struct device *dev,
+ struct device_node *node,
+ struct platform_power_seq *seq,
+ unsigned int step_nbr)
+{
+ struct platform_power_seq_step *step = &seq->steps[step_nbr];
+ int err;
+
+ err = of_property_read_string(node, "id",
+ &step->pwm.id);
+ if (err) {
+ power_seq_err(dev, seq, step_nbr,
+ "error reading id property\n");
+ return err;
+ }
+
+ err = of_power_seq_parse_enable_properties(dev, node, seq, step_nbr,
+ &step->pwm.enable);
+ return err;
+}
+#else
+#define of_power_seq_parse_pwm NULL
+#endif
+
+static bool power_seq_res_compare_pwm(struct power_seq_resource *res,
+ struct platform_power_seq_step *step)
+{
+ return !strcmp(res->pwm.id, step->pwm.id);
+}
+
+static int power_seq_res_alloc_pwm(struct device *dev,
+ struct platform_power_seq_step *pstep,
+ struct power_seq_resource *res)
+{
+ res->pwm.pwm = devm_pwm_get(dev, pstep->pwm.id);
+ if (IS_ERR(res->pwm.pwm)) {
+ dev_err(dev, "cannot get pwm \"%s\"\n", pstep->pwm.id);
+ return PTR_ERR(res->pwm.pwm);
+ }
+ res->pwm.id = pstep->pwm.id;
+
+ return 0;
+}
+
+static int power_seq_step_run_pwm(struct power_seq_step *step)
+{
+ if (step->pdata.pwm.enable) {
+ return pwm_enable(step->resource->pwm.pwm);
+ } else {
+ pwm_disable(step->resource->pwm.pwm);
+ return 0;
+ }
+}
+
+#define POWER_SEQ_PWM_TYPE { \
+ .name = "pwm", \
+ .need_resource = true, \
+ .of_parse = power_seq_of_parse_pwm, \
+ .step_run = power_seq_step_run_pwm, \
+ .res_compare = power_seq_res_compare_pwm, \
+ .res_alloc = power_seq_res_alloc_pwm, \
+}
+
+#else
+
+#define POWER_SEQ_PWM_TYPE {}
+
+#endif
diff --git a/drivers/power/power_seq/power_seq_regulator.c b/drivers/power/power_seq/power_seq_regulator.c
new file mode 100644
index 0000000..2356578
--- /dev/null
+++ b/drivers/power/power_seq/power_seq_regulator.c
@@ -0,0 +1,87 @@
+/*
+ * Copyright (c) 2012 NVIDIA Corporation.
+ *
+ * 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; version 2 of the License.
+ *
+ * This program 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.
+ *
+ */
+
+#ifdef CONFIG_REGULATOR
+
+#include <linux/regulator/consumer.h>
+
+#ifdef CONFIG_OF
+static int power_seq_of_parse_regulator(struct device *dev,
+ struct device_node *node,
+ struct platform_power_seq *seq,
+ unsigned int step_nbr)
+{
+ struct platform_power_seq_step *step = &seq->steps[step_nbr];
+ int err;
+
+ err = of_property_read_string(node, "id",
+ &step->regulator.id);
+ if (err) {
+ power_seq_err(dev, seq, step_nbr,
+ "error reading id property\n");
+ return err;
+ }
+
+ err = of_power_seq_parse_enable_properties(dev, node, seq, step_nbr,
+ &step->regulator.enable);
+ return err;
+}
+#else
+#define of_power_seq_parse_regulator NULL
+#endif
+
+static bool
+power_seq_res_compare_regulator(struct power_seq_resource *res,
+ struct platform_power_seq_step *step)
+{
+ return !strcmp(res->regulator.id, step->regulator.id);
+}
+
+static int power_seq_res_alloc_regulator(struct device *dev,
+ struct platform_power_seq_step *pstep,
+ struct power_seq_resource *res)
+{
+ res->regulator.regulator = devm_regulator_get(dev, pstep->regulator.id);
+ if (IS_ERR(res->regulator.regulator)) {
+ dev_err(dev, "cannot get regulator \"%s\"\n",
+ pstep->regulator.id);
+ return PTR_ERR(res->regulator.regulator);
+ }
+ res->regulator.id = pstep->regulator.id;
+
+ return 0;
+}
+
+static int power_seq_step_run_regulator(struct power_seq_step *step)
+{
+ if (step->pdata.regulator.enable)
+ return regulator_enable(step->resource->regulator.regulator);
+ else
+ return regulator_disable(step->resource->regulator.regulator);
+}
+
+#define POWER_SEQ_REGULATOR_TYPE { \
+ .name = "regulator", \
+ .need_resource = true, \
+ .of_parse = power_seq_of_parse_regulator, \
+ .step_run = power_seq_step_run_regulator, \
+ .res_compare = power_seq_res_compare_regulator, \
+ .res_alloc = power_seq_res_alloc_regulator, \
+}
+
+#else
+
+#define POWER_SEQ_REGULATOR_TYPE {}
+
+#endif
diff --git a/include/linux/power_seq.h b/include/linux/power_seq.h
new file mode 100644
index 0000000..437a61a
--- /dev/null
+++ b/include/linux/power_seq.h
@@ -0,0 +1,172 @@
+/*
+ * power_seq.h
+ *
+ * Simple interpreter for defining power sequences as platform data or device
+ * tree properties.
+ *
+ * Power sequences are designed to replace the callbacks typically used in
+ * board-specific files that implement board-specific power sequences of devices
+ * such as backlights. A power sequence is an array of resources (which can a
+ * regulator, a GPIO, a PWM, ...) with an action to perform on it (enable or
+ * disable) and optional pre and post step delays. By having them interpreted
+ * instead of arbitrarily executed, it is possible to describe these in the
+ * device tree and thus remove board-specific code from the kernel.
+ *
+ * Author: Alexandre Courbot <acourbot@nvidia.com>
+ *
+ * Copyright (c) 2012 NVIDIA Corporation.
+ *
+ * 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; version 2 of the License.
+ *
+ * This program 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.
+ *
+ */
+
+#ifndef __LINUX_POWER_SEQ_H
+#define __LINUX_POWER_SEQ_H
+
+#include <linux/types.h>
+#include <net/irda/parameters.h>
+
+struct device;
+struct regulator;
+struct pwm_device;
+struct device_node;
+
+/**
+ * The different kinds of resources that can be controlled during the sequences.
+ */
+enum power_seq_res_type {
+ POWER_SEQ_DELAY,
+ POWER_SEQ_REGULATOR,
+ POWER_SEQ_PWM,
+ POWER_SEQ_GPIO,
+ POWER_SEQ_NUM_TYPES,
+};
+
+/**
+ * struct platform_power_seq_delay_step - platform data for delay steps
+ * @delay: Amount of time to wait, in microseconds.
+ */
+struct platform_power_seq_delay_step {
+ unsigned int delay;
+};
+
+/**
+ * struct platform_power_seq_regulator_step - platform data for regulator steps
+ * @id: Name of the regulator to use. The regulator will be obtained
+ * using devm_regulator_get(dev, name)
+ * @enable: Whether to enable or disable the regulator during this step
+ */
+struct platform_power_seq_regulator_step {
+ const char *id;
+ bool enable;
+};
+
+/**
+ * struct platform_power_seq_pwm_step - platform data for PWM steps
+ * @id: Name of the pwm to use. The PWM will be obtained using
+ * devm_pwm_get(dev, name)
+ * @enable: Whether to enable or disable the PWM during this step
+ */
+struct platform_power_seq_pwm_step {
+ const char *id;
+ bool enable;
+};
+
+/**
+ * struct platform_power_seq_gpio_step - platform data for GPIO steps
+ * @gpio: Number of the GPIO to use. The GPIO will be obtained using
+ * devm_gpio_request_one(dev, number)
+ * @enable: Whether to enable or disable the GPIO during this step
+ */
+struct platform_power_seq_gpio_step {
+ int gpio;
+ int value;
+};
+
+/**
+ * struct platform_power_seq_step - platform data for power sequences steps
+ * @type: The type of this step. This decides which member of the union is
+ * valid for this step.
+ * @delay: Used if @type = POWER_SEQ_DELAY
+ * @regulator: Used if @type = POWER_SEQ_REGULATOR
+ * @pwm: Used if @type = POWER_SEQ_PWN
+ * @gpio: Used if @type = POWER_SEQ_GPIO
+ */
+struct platform_power_seq_step {
+ enum power_seq_res_type type;
+ union {
+ struct platform_power_seq_delay_step delay;
+ struct platform_power_seq_regulator_step regulator;
+ struct platform_power_seq_pwm_step pwm;
+ struct platform_power_seq_gpio_step gpio;
+ };
+};
+
+/**
+ * struct platform_power_seq - platform data for power sequences
+ * @id: Name through which this sequence is refered
+ * @num_steps: Number of steps in that sequence
+ * @steps: Array of num_steps steps describing the sequence
+ */
+struct platform_power_seq {
+ const char *id;
+ unsigned int num_steps;
+ struct platform_power_seq_step steps[];
+};
+
+/**
+ * struct platform_power_seq_set - platform data for sets of sequences
+ * @num_seqs: Number of sequences in this set
+ * @seqs: Array of pointers to individual sequences
+ */
+struct platform_power_seq_set {
+ unsigned int num_seqs;
+ struct platform_power_seq *seqs[];
+};
+
+/**
+ * struct power_seq_resource - resource used by a power sequence set
+ * @pdata: Pointer to the platform data used to resolve this resource
+ * @regulator: Resolved regulator if of type POWER_SEQ_REGULATOR
+ * @pwm: Resolved PWM if of type POWER_SEQ_PWM
+ * @list: Used to link resources together
+ */
+struct power_seq_resource {
+ enum power_seq_res_type type;
+ /* resolved resource and identifier */
+ union {
+ struct {
+ struct regulator *regulator;
+ const char *id;
+ } regulator;
+ struct {
+ struct pwm_device *pwm;
+ const char *id;
+ } pwm;
+ struct {
+ int gpio;
+ } gpio;
+ };
+ struct list_head list;
+};
+#define power_seq_for_each_resource(pos, seqs) \
+ list_for_each_entry(pos, power_seq_set_resources(seqs), list)
+
+struct power_seq_resource;
+struct power_seq;
+struct power_seq_set;
+
+struct power_seq_set *devm_power_seq_set_build(struct device *dev,
+ struct platform_power_seq_set *pseq);
+struct list_head *power_seq_set_resources(struct power_seq_set *seqs);
+struct power_seq *power_seq_lookup(struct power_seq_set *seqs, const char *id);
+int power_seq_run(struct power_seq *seq);
+
+#endif
--
1.7.12
^ permalink raw reply related
* [PATCH v6 0/4] Runtime Interpreted Power Sequences
From: Alexandre Courbot @ 2012-09-12 9:57 UTC (permalink / raw)
To: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Leela Krishna Amudala,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA
New revision of the power sequences, taking as usual the feedback that was
kindly provided about the last version.
I think now is a good time to discuss integrating this and to start looking for
a maintainer who would be willing to merge this into his/her tree (I am
especially thinking about the power framework maintainers, since this is where
the code is right now. The second patch in this series enables the pwm_backlight
driver to be used with the device tree, without relying on board-dependent
callbacks to support complex power sequences. We also plan to use power
sequences in other Tegra drivers, and other people have expressed interest in
this work during earlier reviews. See for instance
https://lists.ozlabs.org/pipermail/devicetree-discuss/2012-August/018532.html
and
https://lkml.org/lkml/2012/9/6/270
There is probably some more details to fix and improve, but the current shape
should be enough to know if we want this and where - therefore any sign from
a maintainer would be greatly appreciated!
Changes since v5:
* Removed pointers to platform data from resource structure for better code
clarity
* devm_power_seq_set_build() now automatically checks the DT if no platform
data is present, making it possible to remove the explicit DT parsing function
* Lots of fixes in the documentation which should be clearer now (thanks
Stephen!)
Alexandre Courbot (4):
Runtime Interpreted Power Sequences
pwm_backlight: use power sequences
tegra: dt: add label to tegra20's PWM
tegra: ventana: add pwm backlight DT nodes
.../devicetree/bindings/power_seq/power_seq.txt | 122 ++++++
.../bindings/video/backlight/pwm-backlight.txt | 65 ++-
Documentation/power/power_seq.txt | 215 ++++++++++
arch/arm/boot/dts/tegra20-ventana.dts | 59 ++-
arch/arm/boot/dts/tegra20.dtsi | 2 +-
drivers/power/Kconfig | 1 +
drivers/power/Makefile | 1 +
drivers/power/power_seq/Kconfig | 2 +
drivers/power/power_seq/Makefile | 1 +
drivers/power/power_seq/power_seq.c | 446 +++++++++++++++++++++
drivers/power/power_seq/power_seq_delay.c | 51 +++
drivers/power/power_seq/power_seq_gpio.c | 91 +++++
drivers/power/power_seq/power_seq_pwm.c | 87 ++++
drivers/power/power_seq/power_seq_regulator.c | 87 ++++
drivers/video/backlight/Kconfig | 1 +
drivers/video/backlight/pwm_bl.c | 180 ++++++---
include/linux/power_seq.h | 172 ++++++++
include/linux/pwm_backlight.h | 15 +-
18 files changed, 1543 insertions(+), 55 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power_seq/power_seq.txt
create mode 100644 Documentation/power/power_seq.txt
create mode 100644 drivers/power/power_seq/Kconfig
create mode 100644 drivers/power/power_seq/Makefile
create mode 100644 drivers/power/power_seq/power_seq.c
create mode 100644 drivers/power/power_seq/power_seq_delay.c
create mode 100644 drivers/power/power_seq/power_seq_gpio.c
create mode 100644 drivers/power/power_seq/power_seq_pwm.c
create mode 100644 drivers/power/power_seq/power_seq_regulator.c
create mode 100644 include/linux/power_seq.h
--
1.7.12
^ permalink raw reply
* [PATCH] video: exynos_dp: increase AUX channel voltage level
From: Jingoo Han @ 2012-09-12 7:52 UTC (permalink / raw)
To: linux-fbdev
The value of AUX channel differential amplitude current is changed
from 8 mA to 16 mA, in order to increase AUX channel voltage level.
In this case, AUX channel voltage level can be changed from 400 mV
to 800 mV, when resistance between AUX TX and RX is 100 ohm.
According to DP spec, although the normative voltage level is 390 mV,
the informative voltage level is 430 mV. So, 800 mV can be helpful
to improve voltage margin of AUX channel.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_reg.c | 2 +-
drivers/video/exynos/exynos_dp_reg.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 20e441f..5fd1214 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -77,7 +77,7 @@ void exynos_dp_init_analog_param(struct exynos_dp_device *dp)
writel(reg, dp->reg_base + EXYNOS_DP_ANALOG_CTL_3);
reg = PD_RING_OSC | AUX_TERMINAL_CTRL_50_OHM |
- TX_CUR1_2X | TX_CUR_8_MA;
+ TX_CUR1_2X | TX_CUR_16_MA;
writel(reg, dp->reg_base + EXYNOS_DP_PLL_FILTER_CTL_1);
reg = CH3_AMP_400_MV | CH2_AMP_400_MV |
diff --git a/drivers/video/exynos/exynos_dp_reg.h b/drivers/video/exynos/exynos_dp_reg.h
index 125b27c..0e79031 100644
--- a/drivers/video/exynos/exynos_dp_reg.h
+++ b/drivers/video/exynos/exynos_dp_reg.h
@@ -187,7 +187,7 @@
#define PD_RING_OSC (0x1 << 6)
#define AUX_TERMINAL_CTRL_50_OHM (0x2 << 4)
#define TX_CUR1_2X (0x1 << 2)
-#define TX_CUR_8_MA (0x2 << 0)
+#define TX_CUR_16_MA (0x3 << 0)
/* EXYNOS_DP_TX_AMP_TUNING_CTL */
#define CH3_AMP_400_MV (0x0 << 24)
--
1.7.1
^ permalink raw reply related
* [PATCH] video: exynos_dp: add bit-masking for LINK_TRAINING_CTL register
From: Jingoo Han @ 2012-09-12 4:34 UTC (permalink / raw)
To: linux-fbdev
This patch adds bit-masking for LINK_TRAINING_CTL register, when
pre-emphasis level is set. The bit 3 and bit 2 of LINK_TRAINING_CTL
register are used for pre-emphasis level setting, so other bits
should be masked.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_reg.c | 16 ++++++++++++----
drivers/video/exynos/exynos_dp_reg.h | 1 +
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 20e441f..365be69 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -895,7 +895,9 @@ void exynos_dp_set_lane0_pre_emphasis(struct exynos_dp_device *dp, u32 level)
{
u32 reg;
- reg = level << PRE_EMPHASIS_SET_SHIFT;
+ reg = readl(dp->reg_base + EXYNOS_DP_LN0_LINK_TRAINING_CTL);
+ reg &= ~PRE_EMPHASIS_SET_MASK;
+ reg |= level << PRE_EMPHASIS_SET_SHIFT;
writel(reg, dp->reg_base + EXYNOS_DP_LN0_LINK_TRAINING_CTL);
}
@@ -903,7 +905,9 @@ void exynos_dp_set_lane1_pre_emphasis(struct exynos_dp_device *dp, u32 level)
{
u32 reg;
- reg = level << PRE_EMPHASIS_SET_SHIFT;
+ reg = readl(dp->reg_base + EXYNOS_DP_LN1_LINK_TRAINING_CTL);
+ reg &= ~PRE_EMPHASIS_SET_MASK;
+ reg |= level << PRE_EMPHASIS_SET_SHIFT;
writel(reg, dp->reg_base + EXYNOS_DP_LN1_LINK_TRAINING_CTL);
}
@@ -911,7 +915,9 @@ void exynos_dp_set_lane2_pre_emphasis(struct exynos_dp_device *dp, u32 level)
{
u32 reg;
- reg = level << PRE_EMPHASIS_SET_SHIFT;
+ reg = readl(dp->reg_base + EXYNOS_DP_LN2_LINK_TRAINING_CTL);
+ reg &= ~PRE_EMPHASIS_SET_MASK;
+ reg |= level << PRE_EMPHASIS_SET_SHIFT;
writel(reg, dp->reg_base + EXYNOS_DP_LN2_LINK_TRAINING_CTL);
}
@@ -919,7 +925,9 @@ void exynos_dp_set_lane3_pre_emphasis(struct exynos_dp_device *dp, u32 level)
{
u32 reg;
- reg = level << PRE_EMPHASIS_SET_SHIFT;
+ reg = readl(dp->reg_base + EXYNOS_DP_LN3_LINK_TRAINING_CTL);
+ reg &= ~PRE_EMPHASIS_SET_MASK;
+ reg |= level << PRE_EMPHASIS_SET_SHIFT;
writel(reg, dp->reg_base + EXYNOS_DP_LN3_LINK_TRAINING_CTL);
}
diff --git a/drivers/video/exynos/exynos_dp_reg.h b/drivers/video/exynos/exynos_dp_reg.h
index 125b27c..9e9af50 100644
--- a/drivers/video/exynos/exynos_dp_reg.h
+++ b/drivers/video/exynos/exynos_dp_reg.h
@@ -285,6 +285,7 @@
#define SW_TRAINING_PATTERN_SET_NORMAL (0x0 << 0)
/* EXYNOS_DP_LN0_LINK_TRAINING_CTL */
+#define PRE_EMPHASIS_SET_MASK (0x3 << 3)
#define PRE_EMPHASIS_SET_SHIFT (3)
/* EXYNOS_DP_DEBUG_CTL */
--
1.7.1
^ permalink raw reply related
* RE: [PATCH] pwm-backlight: Use devm_pwm_get in pwm_bl.c
From: Jingoo Han @ 2012-09-12 0:55 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1347352949-17515-1-git-send-email-sachin.kamat@linaro.org>
On Tuesday, September 11, 2012 5:42 PM Sachin Kamat wrote
>
> This file already makes use of device managed functions.
> Convert pwm_get() too to use it.
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
CC'ed Thierry Reding.
Acked-by: Jingoo Han <jg1.han@samsung.com>
> ---
> drivers/video/backlight/pwm_bl.c | 7 ++-----
> 1 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index 4965408..0c91023 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -218,7 +218,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> pb->exit = data->exit;
> pb->dev = &pdev->dev;
>
> - pb->pwm = pwm_get(&pdev->dev, NULL);
> + pb->pwm = devm_pwm_get(&pdev->dev, NULL);
> if (IS_ERR(pb->pwm)) {
> dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
>
> @@ -251,7 +251,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> if (IS_ERR(bl)) {
> dev_err(&pdev->dev, "failed to register backlight\n");
> ret = PTR_ERR(bl);
> - goto err_bl;
> + goto err_alloc;
> }
>
> bl->props.brightness = data->dft_brightness;
> @@ -260,8 +260,6 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, bl);
> return 0;
>
> -err_bl:
> - pwm_put(pb->pwm);
> err_alloc:
> if (data->exit)
> data->exit(&pdev->dev);
> @@ -276,7 +274,6 @@ static int pwm_backlight_remove(struct platform_device *pdev)
> backlight_device_unregister(bl);
> pwm_config(pb->pwm, 0, pb->period);
> pwm_disable(pb->pwm);
> - pwm_put(pb->pwm);
> if (pb->exit)
> pb->exit(&pdev->dev);
> return 0;
> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] pwm-backlight: Use devm_pwm_get in pwm_bl.c
From: Sachin Kamat @ 2012-09-11 8:54 UTC (permalink / raw)
To: linux-fbdev
This file already makes use of device managed functions.
Convert pwm_get() too to use it.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
drivers/video/backlight/pwm_bl.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 4965408..0c91023 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -218,7 +218,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
pb->exit = data->exit;
pb->dev = &pdev->dev;
- pb->pwm = pwm_get(&pdev->dev, NULL);
+ pb->pwm = devm_pwm_get(&pdev->dev, NULL);
if (IS_ERR(pb->pwm)) {
dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
@@ -251,7 +251,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
if (IS_ERR(bl)) {
dev_err(&pdev->dev, "failed to register backlight\n");
ret = PTR_ERR(bl);
- goto err_bl;
+ goto err_alloc;
}
bl->props.brightness = data->dft_brightness;
@@ -260,8 +260,6 @@ static int pwm_backlight_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, bl);
return 0;
-err_bl:
- pwm_put(pb->pwm);
err_alloc:
if (data->exit)
data->exit(&pdev->dev);
@@ -276,7 +274,6 @@ static int pwm_backlight_remove(struct platform_device *pdev)
backlight_device_unregister(bl);
pwm_config(pb->pwm, 0, pb->period);
pwm_disable(pb->pwm);
- pwm_put(pb->pwm);
if (pb->exit)
pb->exit(&pdev->dev);
return 0;
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH v2] fbdev: Add Renesas vdc4 framebuffer driver
From: Jingoo Han @ 2012-09-11 2:31 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1347267372-22949-1-git-send-email-phil.edworthy@renesas.com>
On Monday, September 10, 2012 5:56 PM Phil Edworthy wrote
>
> The vdc4 display hardware is found on the sh7269 device.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Reviewed-by: Jingoo Han <jg1.han@samsung.com>
Best regards,
Jingoo Han
> ---
> v2:
> * Use devm_ variants of clk_get, ioremap_nocache, request_irq.
> * Replace spaces with tabs.
> * Check ren_vdc4_start return value.
> * Fix headers used.
>
> drivers/video/Kconfig | 10 +
> drivers/video/Makefile | 1 +
> drivers/video/ren_vdc4fb.c | 641 ++++++++++++++++++++++++++++++++++++++++++++
> include/video/ren_vdc4fb.h | 19 ++
> 4 files changed, 671 insertions(+), 0 deletions(-)
> create mode 100644 drivers/video/ren_vdc4fb.c
> create mode 100644 include/video/ren_vdc4fb.h
^ permalink raw reply
* [PATCH v3] OMAPDSS: Fix IRQ unregister race
From: Dimitar Dimitrov @ 2012-09-10 19:34 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Dimitar Dimitrov, stable
In-Reply-To: <201209102219.29105.dinuxbg@gmail.com>
Very rare kernel crashes are reported on a custom OMAP4 board. Kernel
panics due to corrupted completion structure while executing
dispc_irq_wait_handler(). Excerpt from kernel log:
Internal error: Oops - undefined instruction: 0 [#1] PREEMPT SMP
Unable to handle kernel paging request at virtual address 00400130
...
PC is at 0xebf205bc
LR is at __wake_up_common+0x54/0x94
...
(__wake_up_common+0x0/0x94)
(complete+0x0/0x60)
(dispc_irq_wait_handler.36902+0x0/0x14)
(omap_dispc_irq_handler+0x0/0x354)
(handle_irq_event_percpu+0x0/0x188)
(handle_irq_event+0x0/0x64)
(handle_fasteoi_irq+0x0/0x10c)
(generic_handle_irq+0x0/0x48)
(asm_do_IRQ+0x0/0xc0)
DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
unregister_isr() and DISPC IRQ might be running in parallel on different
CPUs. So there is a chance that a callback is executed even though it
has been unregistered. As omap_dispc_wait_for_irq_timeout() declares a
completion on stack, the dispc_irq_wait_handler() callback might try to
access a completion structure that is invalid. This leads to crashes and
hangs.
Solution is to divide unregister calls into two sets:
1. Non-strict unregistering of callbacks. Callbacks could safely be
executed after unregistering them. This is the case with unregister
calls from the IRQ handler itself.
2. Strict (synchronized) unregistering. Callbacks are not allowed
after unregistering. This is the case with completion waiting.
The above solution should satisfy one of the original intentions of the
driver: callbacks should be able to unregister themselves.
Also fix DSI IRQ unregister which has similar logic to DISPC IRQ handling.
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Dimitar Dimitrov <dinuxbg@gmail.com>
---
Changes since v2:
- Fix formatting per Tomi Valkeinen's comments.
- Restored accidental removal of EXPORT_SYMBOL for omap_dispc_register_isr.
drivers/staging/omapdrm/omap_plane.c | 2 +-
drivers/video/omap2/dss/apply.c | 2 +-
drivers/video/omap2/dss/dispc.c | 34 +++++++++++++++++++++++++++++++++-
drivers/video/omap2/dss/dsi.c | 15 +++++++++++++++
include/video/omapdss.h | 1 +
5 files changed, 51 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/omapdrm/omap_plane.c b/drivers/staging/omapdrm/omap_plane.c
index 7997be7..8d8aa5b 100644
--- a/drivers/staging/omapdrm/omap_plane.c
+++ b/drivers/staging/omapdrm/omap_plane.c
@@ -82,7 +82,7 @@ static void dispc_isr(void *arg, uint32_t mask)
struct omap_plane *omap_plane = to_omap_plane(plane);
struct omap_drm_private *priv = plane->dev->dev_private;
- omap_dispc_unregister_isr(dispc_isr, plane,
+ omap_dispc_unregister_isr_nosync(dispc_isr, plane,
id2irq[omap_plane->ovl->id]);
queue_work(priv->wq, &omap_plane->work);
diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index 0fefc68..9386834 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -847,7 +847,7 @@ static void dss_unregister_vsync_isr(void)
for (i = 0; i < num_mgrs; ++i)
mask |= dispc_mgr_get_framedone_irq(i);
- r = omap_dispc_unregister_isr(dss_apply_irq_handler, NULL, mask);
+ r = omap_dispc_unregister_isr_nosync(dss_apply_irq_handler, NULL, mask);
WARN_ON(r);
dss_data.irq_enabled = false;
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index ee9e296..6032252 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -3320,7 +3320,8 @@ err:
}
EXPORT_SYMBOL(omap_dispc_register_isr);
-int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
+/* WARNING: callback might be executed even after this function returns! */
+int omap_dispc_unregister_isr_nosync(omap_dispc_isr_t isr, void *arg, u32 mask)
{
int i;
unsigned long flags;
@@ -3352,6 +3353,37 @@ int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
return ret;
}
+EXPORT_SYMBOL(omap_dispc_unregister_isr_nosync);
+
+/*
+ * Ensure that callback <isr> will NOT be executed after this function
+ * returns. Must be called from sleepable context, though!
+ */
+int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
+{
+ int ret;
+
+ ret = omap_dispc_unregister_isr_nosync(isr, arg, mask);
+
+ /*
+ * Task context is not really needed. But if we're called from atomic
+ * context, it is probably from DISPC IRQ, where we will deadlock.
+ * So use might_sleep() to catch potential deadlocks.
+ */
+ might_sleep();
+
+ /*
+ * DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
+ * unregister_isr() and DISPC IRQ might be running in parallel on
+ * different CPUs. So there is a chance that a callback is executed
+ * even though it has been unregistered. Add a barrier, in order to
+ * ensure that after returning from this function, the new DISPC IRQ
+ * will use an updated callback array, and NOT its cached copy.
+ */
+ synchronize_irq(dispc.irq);
+
+ return ret;
+}
EXPORT_SYMBOL(omap_dispc_unregister_isr);
#ifdef DEBUG
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index b07e886..c0fcb68 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -960,6 +960,11 @@ static int dsi_unregister_isr(struct platform_device *dsidev,
spin_unlock_irqrestore(&dsi->irq_lock, flags);
+ might_sleep();
+
+ /* See notes for dispc.c:omap_dispc_unregister_isr() . */
+ synchronize_irq(dsi->irq);
+
return r;
}
@@ -1002,6 +1007,11 @@ static int dsi_unregister_isr_vc(struct platform_device *dsidev, int channel,
spin_unlock_irqrestore(&dsi->irq_lock, flags);
+ might_sleep();
+
+ /* See notes for dispc.c:omap_dispc_unregister_isr() . */
+ synchronize_irq(dsi->irq);
+
return r;
}
@@ -1042,6 +1052,11 @@ static int dsi_unregister_isr_cio(struct platform_device *dsidev,
spin_unlock_irqrestore(&dsi->irq_lock, flags);
+ might_sleep();
+
+ /* See notes for dispc.c:omap_dispc_unregister_isr() . */
+ synchronize_irq(dsi->irq);
+
return r;
}
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index a6267a2..769f981 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -708,6 +708,7 @@ void omapdss_default_get_timings(struct omap_dss_device *dssdev,
typedef void (*omap_dispc_isr_t) (void *arg, u32 mask);
int omap_dispc_register_isr(omap_dispc_isr_t isr, void *arg, u32 mask);
int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask);
+int omap_dispc_unregister_isr_nosync(omap_dispc_isr_t isr, void *arg, u32 mask);
int omap_dispc_wait_for_irq_timeout(u32 irqmask, unsigned long timeout);
int omap_dispc_wait_for_irq_interruptible_timeout(u32 irqmask,
--
1.7.10.4
^ permalink raw reply related
* Re: [RFC PATCH v2] OMAPDSS: Fix IRQ unregister race
From: Dimitar Dimitrov @ 2012-09-10 19:19 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1347263345.8127.22.camel@deskari>
On Monday 10 September 2012 10:49:05 Tomi Valkeinen wrote:
> On Sat, 2012-09-08 at 18:05 +0300, Dimitar Dimitrov wrote:
> > Very rare kernel crashes are reported on a custom OMAP4 board. Kernel
> > panics due to corrupted completion structure while executing
> >
> > dispc_irq_wait_handler(). Excerpt from kernel log:
> > Internal error: Oops - undefined instruction: 0 [#1] PREEMPT SMP
> > Unable to handle kernel paging request at virtual address 00400130
> > ...
> > PC is at 0xebf205bc
> > LR is at __wake_up_common+0x54/0x94
> > ...
> > (__wake_up_common+0x0/0x94)
> > (complete+0x0/0x60)
> > (dispc_irq_wait_handler.36902+0x0/0x14)
> > (omap_dispc_irq_handler+0x0/0x354)
> > (handle_irq_event_percpu+0x0/0x188)
> > (handle_irq_event+0x0/0x64)
> > (handle_fasteoi_irq+0x0/0x10c)
> > (generic_handle_irq+0x0/0x48)
> > (asm_do_IRQ+0x0/0xc0)
> >
> > DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
> > unregister_isr() and DISPC IRQ might be running in parallel on different
> > CPUs. So there is a chance that a callback is executed even though it
> > has been unregistered. As omap_dispc_wait_for_irq_timeout() declares a
> > completion on stack, the dispc_irq_wait_handler() callback might try to
> > access a completion structure that is invalid. This leads to crashes and
> > hangs.
> >
> > Solution is to divide unregister calls into two sets:
> > 1. Non-strict unregistering of callbacks. Callbacks could safely be
> >
> > executed after unregistering them. This is the case with unregister
> > calls from the IRQ handler itself.
> >
> > 2. Strict (synchronized) unregistering. Callbacks are not allowed
> >
> > after unregistering. This is the case with completion waiting.
> >
> > The above solution should satisfy one of the original intentions of the
> > driver: callbacks should be able to unregister themselves.
> >
> > Also fix DSI IRQ unregister which has similar logic to DISPC IRQ
> > handling.
> >
> > Signed-off-by: Dimitar Dimitrov <dinuxbg@gmail.com>
> > ---
> >
> > WARNING: This bug is quite old. The patch has been tested on v3.0. No
> > testing has been done after rebasing to v3.6. Hence the RFC tag.
> > Hopefully someone will beat me in testing with latest linux-omap/master.
> >
> > Changes since v1 per Tomi Valkeinen's comments:
> > - Don't rename omap_dispc_unregister_isr, just introduce nosync
> > variant. - Apply the same fix for DSI IRQ which suffers from the same
> > race condition.
>
> I made a quick test and works for me, although I haven't encountered the
> problem itself.
This issue is very rarely seen during normal operation. You may not hit it
even after days of stress testing. To speed up reproducing an artificial delay
in the IRQ routine could be used while stress-testing DSS, e.g.:
@@ -3462,6 +3462,8 @@ static irqreturn_t omap_dispc_irq_handler(int irq, void
*a
spin_unlock(&dispc.irq_lock);
+ { static int iii; if ((iii++ % 10) = 0) mdelay(100); }
+
for (i = 0; i < DISPC_MAX_NR_ISRS; i++) {
isr_data = ®istered_isr[i];
>
> Some mostly cosmetic comments below.
>
> This seems to apply cleanly to v3.4+ kernels, but not earlier ones. Do
> you want to make the needed modifications and mail this and the modified
> patches for stable kernels also? I can do that also if you don't want
> to.
I can do the rebase and cleanup. I'll send the modified patch per your
comments in the next hour.
Currently I cannot test, though. I hope to have a working setup by end of week
in order to send sanity-tested patches for stable kernels.
>
> > drivers/staging/omapdrm/omap_plane.c | 2 +-
> > drivers/video/omap2/dss/apply.c | 2 +-
> > drivers/video/omap2/dss/dispc.c | 45
> > +++++++++++++++++++++++++++++----- drivers/video/omap2/dss/dsi.c
> > | 19 ++++++++++++++
> > include/video/omapdss.h | 1 +
> > 5 files changed, 61 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/staging/omapdrm/omap_plane.c
> > b/drivers/staging/omapdrm/omap_plane.c index 7997be7..8d8aa5b 100644
> > --- a/drivers/staging/omapdrm/omap_plane.c
> > +++ b/drivers/staging/omapdrm/omap_plane.c
> > @@ -82,7 +82,7 @@ static void dispc_isr(void *arg, uint32_t mask)
> >
> > struct omap_plane *omap_plane = to_omap_plane(plane);
> > struct omap_drm_private *priv = plane->dev->dev_private;
> >
> > - omap_dispc_unregister_isr(dispc_isr, plane,
> > + omap_dispc_unregister_isr_nosync(dispc_isr, plane,
> >
> > id2irq[omap_plane->ovl->id]);
> >
> > queue_work(priv->wq, &omap_plane->work);
> >
> > diff --git a/drivers/video/omap2/dss/apply.c
> > b/drivers/video/omap2/dss/apply.c index 0fefc68..9386834 100644
> > --- a/drivers/video/omap2/dss/apply.c
> > +++ b/drivers/video/omap2/dss/apply.c
> > @@ -847,7 +847,7 @@ static void dss_unregister_vsync_isr(void)
> >
> > for (i = 0; i < num_mgrs; ++i)
> >
> > mask |= dispc_mgr_get_framedone_irq(i);
> >
> > - r = omap_dispc_unregister_isr(dss_apply_irq_handler, NULL, mask);
> > + r = omap_dispc_unregister_isr_nosync(dss_apply_irq_handler, NULL,
> > mask);
> >
> > WARN_ON(r);
> >
> > dss_data.irq_enabled = false;
> >
> > diff --git a/drivers/video/omap2/dss/dispc.c
> > b/drivers/video/omap2/dss/dispc.c index ee9e296..a67d92c 100644
> > --- a/drivers/video/omap2/dss/dispc.c
> > +++ b/drivers/video/omap2/dss/dispc.c
> > @@ -2421,8 +2421,8 @@ static void dispc_mgr_enable_digit_out(bool enable)
> >
> > enable ? "start" : "stop");
> >
> > }
> >
> > - r = omap_dispc_unregister_isr(dispc_disable_isr,
> > &frame_done_completion, - irq_mask);
> > + r = omap_dispc_unregister_isr(dispc_disable_isr,
> > + &frame_done_completion, irq_mask);
>
> This change is not needed.
Sorry, I missed those. Will remove and resend.
>
> > if (r)
> >
> > DSSERR("failed to unregister %x isr\n", irq_mask);
> >
> > @@ -3320,7 +3320,8 @@ err:
> > }
> > EXPORT_SYMBOL(omap_dispc_register_isr);
> >
> > -int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
> > +/* WARNING: callback might be executed even after this function returns!
> > */ +int omap_dispc_unregister_isr_nosync(omap_dispc_isr_t isr, void
> > *arg, u32 mask)
> >
> > {
> >
> > int i;
> > unsigned long flags;
> >
> > @@ -3352,7 +3353,37 @@ int omap_dispc_unregister_isr(omap_dispc_isr_t
> > isr, void *arg, u32 mask)
> >
> > return ret;
> >
> > }
> >
> > -EXPORT_SYMBOL(omap_dispc_unregister_isr);
> > +EXPORT_SYMBOL(omap_dispc_unregister_isr_nosync);
> > +
> > +/*
> > + * Ensure that callback <isr> will NOT be executed after this function
> > + * returns. Must be called from sleepable context, though!
> > + */
> > +int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
> > +{
> > + int ret;
> > +
> > + ret = omap_dispc_unregister_isr_nosync(isr, arg, mask);
> > +
> > + /* Task context is not really needed. But if we're called from atomic
> > + * context, it is probably from DISPC IRQ, where we will deadlock.
> > + * So use might_sleep() to catch potential deadlocks.
> > + */
>
> Use the kernel multi-line comment style, i.e.:
>
> /*
> * foobar
> */
Will fix.
>
> > + might_sleep();
> > +
> > +#if defined(CONFIG_SMP)
> > + /* DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
> > + * unregister_isr() and DISPC IRQ might be running in parallel on
> > + * different CPUs. So there is a chance that a callback is executed
> > + * even though it has been unregistered. Add a barrier, in order to
> > + * ensure that after returning from this function, the new DISPC IRQ
> > + * will use an updated callback array, and NOT its cached copy.
> > + */
>
> Comment style here also.
Will fix.
>
> > + synchronize_irq(dispc.irq);
> > +#endif
>
> Why do you use #if defined(CONFIG_SMP)? synchronize_irq is defined
> with !CONFIG_SMP also. In that case it becomes just barrier().
>
You're correct. I'll remove "defined(CONFIG_SMP)".
> > +
> > + return ret;
> > +}
I forgot to keep EXPORT_SYMBOL for omap_dispc_unregister_isr. Will add it.
> >
> > #ifdef DEBUG
> > static void print_irq_status(u32 status)
> >
> > @@ -3567,7 +3598,8 @@ int omap_dispc_wait_for_irq_timeout(u32 irqmask,
> > unsigned long timeout)
> >
> > timeout = wait_for_completion_timeout(&completion, timeout);
> >
> > - omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion,
> > irqmask); + omap_dispc_unregister_isr(dispc_irq_wait_handler,
> > &completion, + irqmask);
>
> Change not needed.
Will remove.
>
> > if (timeout = 0)
> >
> > return -ETIMEDOUT;
> >
> > @@ -3598,7 +3630,8 @@ int
> > omap_dispc_wait_for_irq_interruptible_timeout(u32 irqmask,
> >
> > timeout = wait_for_completion_interruptible_timeout(&completion,
> >
> > timeout);
> >
> > - omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion,
> > irqmask); + omap_dispc_unregister_isr(dispc_irq_wait_handler,
> > &completion, + irqmask);
>
> Change not needed.
Will remove.
>
> > if (timeout = 0)
> >
> > return -ETIMEDOUT;
> >
> > diff --git a/drivers/video/omap2/dss/dsi.c
> > b/drivers/video/omap2/dss/dsi.c index b07e886..24b4a3e 100644
> > --- a/drivers/video/omap2/dss/dsi.c
> > +++ b/drivers/video/omap2/dss/dsi.c
> > @@ -960,6 +960,13 @@ static int dsi_unregister_isr(struct platform_device
> > *dsidev,
> >
> > spin_unlock_irqrestore(&dsi->irq_lock, flags);
> >
> > + might_sleep();
> > +
> > +#if defined(CONFIG_SMP)
> > + /* See notes for dispc.c:omap_dispc_unregister_isr() . */
> > + synchronize_irq(dsi->irq);
> > +#endif
> > +
>
> Same SMP comment as with dispc.
>
> Tomi
Thanks,
Dimitar
^ permalink raw reply
* [PATCH 2/2] video: s3c2410: fix checkpatch warnings
From: Jingoo Han @ 2012-09-10 10:55 UTC (permalink / raw)
To: linux-fbdev
This patch fixes the checkpatch warnings listed below:
WARNING: usleep_range should not use min = max args; see Documentation/timers/timers-howto.txt
WARNING: quoted string split across lines
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/s3c2410fb.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/video/s3c2410fb.c b/drivers/video/s3c2410fb.c
index 1aa37ea..1083bb9 100644
--- a/drivers/video/s3c2410fb.c
+++ b/drivers/video/s3c2410fb.c
@@ -935,7 +935,7 @@ static int __devinit s3c24xxfb_probe(struct platform_device *pdev,
clk_enable(info->clk);
dprintk("got and enabled clock\n");
- usleep_range(1000, 1000);
+ usleep_range(1000, 1100);
info->clk_rate = clk_get_rate(info->clk);
@@ -1034,7 +1034,7 @@ static int __devexit s3c2410fb_remove(struct platform_device *pdev)
s3c2410fb_cpufreq_deregister(info);
s3c2410fb_lcd_enable(info, 0);
- usleep_range(1000, 1000);
+ usleep_range(1000, 1100);
s3c2410fb_unmap_video_memory(fbinfo);
@@ -1071,7 +1071,7 @@ static int s3c2410fb_suspend(struct platform_device *dev, pm_message_t state)
* the LCD DMA engine is not going to get back on the bus
* before the clock goes off again (bjd) */
- usleep_range(1000, 1000);
+ usleep_range(1000, 1100);
clk_disable(info->clk);
return 0;
@@ -1083,7 +1083,7 @@ static int s3c2410fb_resume(struct platform_device *dev)
struct s3c2410fb_info *info = fbinfo->par;
clk_enable(info->clk);
- usleep_range(1000, 1000);
+ usleep_range(1000, 1100);
s3c2410fb_init_registers(fbinfo);
@@ -1140,8 +1140,8 @@ static void __exit s3c2410fb_cleanup(void)
module_init(s3c2410fb_init);
module_exit(s3c2410fb_cleanup);
-MODULE_AUTHOR("Arnaud Patard <arnaud.patard@rtp-net.org>, "
- "Ben Dooks <ben-linux@fluff.org>");
+MODULE_AUTHOR("Arnaud Patard <arnaud.patard@rtp-net.org>");
+MODULE_AUTHOR("Ben Dooks <ben-linux@fluff.org>");
MODULE_DESCRIPTION("Framebuffer driver for the s3c2410");
MODULE_LICENSE("GPL");
MODULE_ALIAS("platform:s3c2410-lcd");
--
1.7.1
^ permalink raw reply related
* [PATCH 1/2] video: s3c2410: Use pr_* and dev_* instead of printk
From: Jingoo Han @ 2012-09-10 10:54 UTC (permalink / raw)
To: linux-fbdev
From: Sachin Kamat <sachin.kamat@linaro.org>
printk calls are replaced by pr_* and dev_* calls to silence
checkpatch warnings.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/s3c2410fb.c | 22 ++++++++++++++--------
1 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/video/s3c2410fb.c b/drivers/video/s3c2410fb.c
index 77f34c6..1aa37ea 100644
--- a/drivers/video/s3c2410fb.c
+++ b/drivers/video/s3c2410fb.c
@@ -11,6 +11,8 @@
* Driver based on skeletonfb.c, sa1100fb.c and others.
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/err.h>
@@ -48,7 +50,11 @@ static int debug = 1;
static int debug;
#endif
-#define dprintk(msg...) if (debug) printk(KERN_DEBUG "s3c2410fb: " msg);
+#define dprintk(msg...) \
+do { \
+ if (debug) \
+ pr_debug(msg); \
+} while (0)
/* useful functions */
@@ -598,11 +604,11 @@ static int s3c2410fb_debug_store(struct device *dev,
if (strnicmp(buf, "on", 2) = 0 ||
strnicmp(buf, "1", 1) = 0) {
debug = 1;
- printk(KERN_DEBUG "s3c2410fb: Debug On");
+ dev_dbg(dev, "s3c2410fb: Debug On");
} else if (strnicmp(buf, "off", 3) = 0 ||
strnicmp(buf, "0", 1) = 0) {
debug = 0;
- printk(KERN_DEBUG "s3c2410fb: Debug Off");
+ dev_dbg(dev, "s3c2410fb: Debug Off");
} else {
return -EINVAL;
}
@@ -921,7 +927,7 @@ static int __devinit s3c24xxfb_probe(struct platform_device *pdev,
info->clk = clk_get(NULL, "lcd");
if (IS_ERR(info->clk)) {
- printk(KERN_ERR "failed to get lcd clock source\n");
+ dev_err(&pdev->dev, "failed to get lcd clock source\n");
ret = PTR_ERR(info->clk);
goto release_irq;
}
@@ -947,7 +953,7 @@ static int __devinit s3c24xxfb_probe(struct platform_device *pdev,
/* Initialize video memory */
ret = s3c2410fb_map_video_memory(fbinfo);
if (ret) {
- printk(KERN_ERR "Failed to allocate video RAM: %d\n", ret);
+ dev_err(&pdev->dev, "Failed to allocate video RAM: %d\n", ret);
ret = -ENOMEM;
goto release_clock;
}
@@ -970,7 +976,7 @@ static int __devinit s3c24xxfb_probe(struct platform_device *pdev,
ret = register_framebuffer(fbinfo);
if (ret < 0) {
- printk(KERN_ERR "Failed to register framebuffer device: %d\n",
+ dev_err(&pdev->dev, "Failed to register framebuffer device: %d\n",
ret);
goto free_cpufreq;
}
@@ -978,9 +984,9 @@ static int __devinit s3c24xxfb_probe(struct platform_device *pdev,
/* create device files */
ret = device_create_file(&pdev->dev, &dev_attr_debug);
if (ret)
- printk(KERN_ERR "failed to add debug attribute\n");
+ dev_err(&pdev->dev, "failed to add debug attribute\n");
- printk(KERN_INFO "fb%d: %s frame buffer device\n",
+ dev_info(&pdev->dev, "fb%d: %s frame buffer device\n",
fbinfo->node, fbinfo->fix.id);
return 0;
--
1.7.1
^ permalink raw reply related
* [PATCH v2] fbdev: Add Renesas vdc4 framebuffer driver
From: Phil Edworthy @ 2012-09-10 8:56 UTC (permalink / raw)
To: linux-fbdev
The vdc4 display hardware is found on the sh7269 device.
Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
v2:
* Use devm_ variants of clk_get, ioremap_nocache, request_irq.
* Replace spaces with tabs.
* Check ren_vdc4_start return value.
* Fix headers used.
drivers/video/Kconfig | 10 +
drivers/video/Makefile | 1 +
drivers/video/ren_vdc4fb.c | 641 ++++++++++++++++++++++++++++++++++++++++++++
include/video/ren_vdc4fb.h | 19 ++
4 files changed, 671 insertions(+), 0 deletions(-)
create mode 100644 drivers/video/ren_vdc4fb.c
create mode 100644 include/video/ren_vdc4fb.h
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 0217f74..89c9250 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -1990,6 +1990,16 @@ config FB_W100
If unsure, say N.
+config FB_REN_VDC4FB
+ tristate "Renesas VDC4 framebuffer support"
+ depends on FB && CPU_SUBTYPE_SH7269
+ select FB_SYS_FILLRECT
+ select FB_SYS_COPYAREA
+ select FB_SYS_IMAGEBLIT
+ select FB_SYS_FOPS
+ ---help---
+ Frame buffer driver for the Renesas VDC4.
+
config FB_SH_MOBILE_LCDC
tristate "SuperH Mobile LCDC framebuffer support"
depends on FB && (SUPERH || ARCH_SHMOBILE) && HAVE_CLK
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index ee8dafb..ba69fcb 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -140,6 +140,7 @@ obj-$(CONFIG_SH_MIPI_DSI) += sh_mipi_dsi.o
obj-$(CONFIG_FB_SH_MOBILE_HDMI) += sh_mobile_hdmi.o
obj-$(CONFIG_FB_SH_MOBILE_MERAM) += sh_mobile_meram.o
obj-$(CONFIG_FB_SH_MOBILE_LCDC) += sh_mobile_lcdcfb.o
+obj-$(CONFIG_FB_REN_VDC4FB) += ren_vdc4fb.o
obj-$(CONFIG_FB_OMAP) += omap/
obj-y += omap2/
obj-$(CONFIG_XEN_FBDEV_FRONTEND) += xen-fbfront.o
diff --git a/drivers/video/ren_vdc4fb.c b/drivers/video/ren_vdc4fb.c
new file mode 100644
index 0000000..73daee3
--- /dev/null
+++ b/drivers/video/ren_vdc4fb.c
@@ -0,0 +1,641 @@
+/*
+ * Renesas VDC4 Framebuffer
+ *
+ * Based on sh_mobile_lcdcfb.c
+ * Copyright (c) 2012 Renesas Electronics Europe Ltd
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#include <linux/interrupt.h>
+#include <linux/vmalloc.h>
+#include <linux/module.h>
+#include <video/ren_vdc4fb.h>
+
+#define PALETTE_NR 16
+
+struct ren_vdc4_priv {
+ void __iomem *base;
+ int irq;
+ struct clk *dot_clk;
+ struct clk *clk;
+ struct fb_info *info;
+ dma_addr_t dma_handle;
+ struct ren_vdc4_info *cfg;
+ u32 pseudo_palette[PALETTE_NR];
+};
+
+/* Register offsets/reading and writing functions */
+enum {
+ SCL0_UPDATE, SCL0_FRC1, SCL0_FRC2, SCL0_FRC3,
+ SCL0_FRC4, SCL0_FRC5, SCL0_FRC6, SCL0_FRC7,
+ SCL0_DS1, SCL0_US1,
+
+ GR1_UPDATE, GR1_AB1,
+
+ GR2_UPDATE, GR2_AB1,
+
+ GR3_UPDATE, GR3_FLM_RD, GR3_FLM1, GR3_FLM2,
+ GR3_FLM3, GR3_FLM4, GR3_FLM5, GR3_FLM6, GR3_AB1,
+ GR3_AB2, GR3_AB3, GR3_AB4, GR3_AB5, GR3_AB6,
+ GR3_AB7, GR3_AB8, GR3_AB9, GR3_AB10, GR3_AB11,
+ GR3_BASE, GR3_CLUT_INT, GR3_MON,
+
+ TCON_UPDATE, TCON_TIM, TCON_TIM_STVA1, TCON_TIM_STVA2,
+ TCON_TIM_STVB1, TCON_TIM_STVB2, TCON_TIM_STH1,
+ TCON_TIM_STH2, TCON_TIM_STB1, TCON_TIM_STB2,
+ TCON_TIM_CPV1, TCON_TIM_CPV2, TCON_TIM_POLA1,
+ TCON_TIM_POLA2, TCON_TIM_POLB1, TCON_TIM_POLB2,
+ TCON_TIM_DE,
+
+ OUT_UPDATE, OUT_SET, OUT_BRIGHT1,
+ OUT_BRIGHT2, OUT_CONTRAST, OUT_PDTHA, OUT_CLK_PHASE,
+
+ SYSCNT_INT1, SYSCNT_INT2, SYSCNT_INT3, SYSCNT_INT4,
+ SYSCNT_PANEL_CLK, SYSCNT_CLUT
+};
+
+static unsigned long vdc4_offsets[] = {
+ [SCL0_UPDATE] = 0x0100,
+ [SCL0_FRC1] = 0x0104,
+ [SCL0_FRC2] = 0x0108,
+ [SCL0_FRC3] = 0x010C,
+ [SCL0_FRC4] = 0x0110,
+ [SCL0_FRC5] = 0x0114,
+ [SCL0_FRC6] = 0x0118,
+ [SCL0_FRC7] = 0x011C,
+ [SCL0_DS1] = 0x012C,
+ [SCL0_US1] = 0x0148,
+ [GR1_UPDATE] = 0x0200,
+ [GR1_AB1] = 0x0220,
+ [GR2_UPDATE] = 0x0300,
+ [GR2_AB1] = 0x0320,
+ [GR3_UPDATE] = 0x0380,
+ [GR3_FLM_RD] = 0x0384,
+ [GR3_FLM1] = 0x0388,
+ [GR3_FLM2] = 0x038C,
+ [GR3_FLM3] = 0x0390,
+ [GR3_FLM4] = 0x0394,
+ [GR3_FLM5] = 0x0398,
+ [GR3_FLM6] = 0x039C,
+ [GR3_AB1] = 0x03A0,
+ [GR3_AB2] = 0x03A4,
+ [GR3_AB3] = 0x03A8,
+ [GR3_AB4] = 0x03AC,
+ [GR3_AB5] = 0x03B0,
+ [GR3_AB6] = 0x03B4,
+ [GR3_AB7] = 0x03B8,
+ [GR3_AB8] = 0x03BC,
+ [GR3_AB9] = 0x03C0,
+ [GR3_AB10] = 0x03C4,
+ [GR3_AB11] = 0x03C8,
+ [GR3_BASE] = 0x03CC,
+ [GR3_CLUT_INT] = 0x03D0,
+ [GR3_MON] = 0x03D4,
+ [TCON_UPDATE] = 0x0580,
+ [TCON_TIM] = 0x0584,
+ [TCON_TIM_STVA1] = 0x0588,
+ [TCON_TIM_STVA2] = 0x058C,
+ [TCON_TIM_STVB1] = 0x0590,
+ [TCON_TIM_STVB2] = 0x0594,
+ [TCON_TIM_STH1] = 0x0598,
+ [TCON_TIM_STH2] = 0x059C,
+ [TCON_TIM_STB1] = 0x05A0,
+ [TCON_TIM_STB2] = 0x05A4,
+ [TCON_TIM_CPV1] = 0x05A8,
+ [TCON_TIM_CPV2] = 0x05AC,
+ [TCON_TIM_POLA1] = 0x05B0,
+ [TCON_TIM_POLA2] = 0x05B4,
+ [TCON_TIM_POLB1] = 0x05B8,
+ [TCON_TIM_POLB2] = 0x05BC,
+ [TCON_TIM_DE] = 0x05C0,
+ [OUT_UPDATE] = 0x0600,
+ [OUT_SET] = 0x0604,
+ [OUT_BRIGHT1] = 0x0608,
+ [OUT_BRIGHT2] = 0x060C,
+ [OUT_CONTRAST] = 0x0610,
+ [OUT_PDTHA] = 0x0614,
+ [OUT_CLK_PHASE] = 0x0624,
+ [SYSCNT_INT1] = 0x0680,
+ [SYSCNT_INT2] = 0x0684,
+ [SYSCNT_INT3] = 0x0688,
+ [SYSCNT_INT4] = 0x068C,
+ [SYSCNT_PANEL_CLK] = 0x0690, /* 16-bit */
+ [SYSCNT_CLUT] = 0x0692, /* 16-bit */
+};
+
+/* SYSCNT */
+#define ICKEN (1 << 8)
+
+/* SCL Syncs */
+#define FREE_RUN_VSYNC 0x0001
+
+/* OUTPUT */
+#define OUT_FMT_RGB666 (1 << 12)
+
+/* TCON Timings */
+#define STVB_SEL_BITS 0x0007
+#define STVB_HS_SEL 2
+
+#define STH2_SEL_BITS 0x0007
+#define STH2_DE_SEL 7
+
+/* OUTCLK */
+#define LCD_DATA_EDGE 0x0100
+#define STVB_EDGE 0x0020
+#define STH_EDGE 0x0010
+
+/* SCL_UPDATE */
+#define SCL0_UPDATE_BIT 0x0100
+#define SCL0_VEN_BIT 0x0010
+
+/* TCON_UPDATE */
+#define TCON_VEN_BIT 0x0001
+
+/* OUT_UPDATE */
+#define OUTCNT_VEN_BIT 0x0001
+
+/* GR_UPDATE */
+#define P_VEN_UPDATE 0x0010
+#define IBUS_VEN_UPDATE 0x0001
+
+/* GR_AB1 */
+#define DISPSEL_BCKGND 0x0000
+#define DISPSEL_LOWER 0x0001
+#define DISPSEL_CUR 0x0002
+
+/* GR_FLM_RD */
+#define FB_R_ENB 0x01
+
+static void vdc4_write(struct ren_vdc4_priv *priv,
+ unsigned long reg_offs, unsigned long data)
+{
+ if ((SYSCNT_PANEL_CLK = reg_offs) || (SYSCNT_CLUT = reg_offs))
+ iowrite16(data, priv->base + vdc4_offsets[reg_offs]);
+ else
+ iowrite32(data, priv->base + vdc4_offsets[reg_offs]);
+}
+
+static unsigned long vdc4_read(struct ren_vdc4_priv *priv,
+ unsigned long reg_offs)
+{
+ if ((SYSCNT_PANEL_CLK = reg_offs) || (SYSCNT_CLUT = reg_offs))
+ return ioread16(priv->base + vdc4_offsets[reg_offs]);
+ else
+ return ioread32(priv->base + vdc4_offsets[reg_offs]);
+}
+
+static irqreturn_t ren_vdc4_irq(int irq, void *data)
+{
+ /* Not currently implemented/used */
+ return IRQ_HANDLED;
+}
+
+static void lcd_clear_display(struct ren_vdc4_priv *priv)
+{
+ unsigned char *pdest;
+ unsigned long size;
+
+ pdest = (unsigned char *)priv->dma_handle;
+ size = priv->cfg->lcd_cfg.xres * priv->cfg->lcd_cfg.yres * 2;
+
+ memset(pdest, 0, size);
+}
+
+static void restart_tft_display(struct ren_vdc4_priv *priv,
+ int clock_source)
+{
+ struct fb_videomode *lcd;
+ unsigned long h;
+ unsigned long v;
+ unsigned long tmp;
+
+ /* FB setup */
+ lcd = &priv->cfg->lcd_cfg;
+ lcd_clear_display(priv);
+
+ /* VDC clock Setup */
+ tmp = priv->cfg->clock_divider;
+ tmp |= clock_source << 12;
+ tmp |= ICKEN;
+ vdc4_write(priv, SYSCNT_PANEL_CLK, tmp);
+
+ /* Clear and Disable all interrupts */
+ vdc4_write(priv, SYSCNT_INT1, 0);
+ vdc4_write(priv, SYSCNT_INT2, 0);
+ vdc4_write(priv, SYSCNT_INT3, 0);
+ vdc4_write(priv, SYSCNT_INT4, 0);
+
+ /* Setup free-running syncs */
+ vdc4_write(priv, SCL0_FRC3, FREE_RUN_VSYNC);
+
+ /* Disable scale up/down */
+ vdc4_write(priv, SCL0_DS1, 0);
+ vdc4_write(priv, SCL0_US1, 0);
+
+ /* Timing registers */
+ h = lcd->hsync_len + lcd->left_margin + lcd->xres + lcd->right_margin;
+ v = lcd->vsync_len + lcd->upper_margin + lcd->yres + lcd->lower_margin;
+ tmp = (v - 1) << 16;
+ tmp |= h - 1;
+ vdc4_write(priv, SCL0_FRC4, tmp);
+
+ vdc4_write(priv, TCON_TIM, (((h - 1) / 2) << 16));
+
+ tmp = (lcd->vsync_len + lcd->upper_margin) << 16;
+ tmp |= lcd->yres;
+ vdc4_write(priv, SCL0_FRC6, tmp);
+ vdc4_write(priv, TCON_TIM_STVB1, tmp);
+ vdc4_write(priv, GR3_AB2, tmp);
+
+ tmp = lcd->left_margin << 16;
+ tmp |= lcd->xres;
+ vdc4_write(priv, SCL0_FRC7, tmp);
+ vdc4_write(priv, TCON_TIM_STB1, tmp);
+ vdc4_write(priv, GR3_AB3, tmp);
+
+ vdc4_write(priv, SCL0_FRC1, 0);
+ vdc4_write(priv, SCL0_FRC2, 0);
+ vdc4_write(priv, SCL0_FRC5, 0);
+
+ /* Set output format */
+ vdc4_write(priv, OUT_SET, OUT_FMT_RGB666);
+
+ /* STH TCON Timing */
+ tmp = priv->cfg->hs_pulse_width;
+ tmp |= priv->cfg->hs_start_pos << 16;
+ vdc4_write(priv, TCON_TIM_STH1, tmp);
+
+ /* Setup STVB as HSYNC */
+ tmp = vdc4_read(priv, TCON_TIM_STVB2);
+ tmp &= ~STVB_SEL_BITS;
+ tmp |= STVB_HS_SEL;
+ vdc4_write(priv, TCON_TIM_STVB2, tmp);
+
+ tmp = vdc4_read(priv, OUT_CLK_PHASE);
+ tmp &= ~STVB_EDGE;
+ vdc4_write(priv, OUT_CLK_PHASE, tmp);
+
+ /* Setup STH as DE */
+ tmp = vdc4_read(priv, TCON_TIM_STH2);
+ tmp &= ~STH2_SEL_BITS;
+ tmp |= STH2_DE_SEL;
+ vdc4_write(priv, TCON_TIM_STH2, tmp);
+
+ tmp = vdc4_read(priv, OUT_CLK_PHASE);
+ tmp &= ~STH_EDGE;
+ vdc4_write(priv, OUT_CLK_PHASE, tmp);
+
+ /* Output clock rising edge */
+ tmp = vdc4_read(priv, OUT_CLK_PHASE);
+ tmp &= ~LCD_DATA_EDGE;
+ vdc4_write(priv, OUT_CLK_PHASE, tmp);
+
+ /* Setup graphics buffers and update all registers */
+ vdc4_write(priv, GR1_AB1, DISPSEL_BCKGND);
+ vdc4_write(priv, GR2_AB1, DISPSEL_LOWER);
+ vdc4_write(priv, GR3_AB1, DISPSEL_CUR);
+
+ /* Setup framebuffer base/output */
+ vdc4_write(priv, GR3_FLM_RD, FB_R_ENB);
+
+ vdc4_write(priv, GR3_FLM2, (unsigned long)priv->info->screen_base);
+
+ vdc4_write(priv, GR3_FLM3, (lcd->xres * 2) << 16);
+
+ tmp = vdc4_read(priv, GR3_FLM5);
+ tmp |= lcd->yres << 16;
+ vdc4_write(priv, GR3_FLM5, tmp);
+
+ tmp = lcd->xres << 16;
+ vdc4_write(priv, GR3_FLM6, tmp);
+
+ /* Apply all register settings */
+ vdc4_write(priv, SCL0_UPDATE, SCL0_VEN_BIT | SCL0_UPDATE_BIT);
+ vdc4_write(priv, GR1_UPDATE, P_VEN_UPDATE);
+ vdc4_write(priv, GR2_UPDATE, P_VEN_UPDATE);
+ vdc4_write(priv, GR3_UPDATE, P_VEN_UPDATE | IBUS_VEN_UPDATE);
+ vdc4_write(priv, OUT_UPDATE, OUTCNT_VEN_BIT);
+ vdc4_write(priv, TCON_UPDATE, TCON_VEN_BIT);
+}
+
+static int ren_vdc4_setup_clocks(struct platform_device *pdev,
+ int clock_source,
+ struct ren_vdc4_priv *priv)
+{
+ priv->clk = devm_clk_get(&pdev->dev, "vdc4");
+ if (IS_ERR(priv->clk)) {
+ dev_err(&pdev->dev, "cannot get clock \"vdc4\"\n");
+ return PTR_ERR(priv->clk);
+ }
+
+ if (clock_source = VDC4_PERI_CLK) {
+ priv->dot_clk = devm_clk_get(&pdev->dev, "peripheral_clk");
+ if (IS_ERR(priv->dot_clk)) {
+ dev_err(&pdev->dev, "cannot get peripheral clock\n");
+ return PTR_ERR(priv->dot_clk);
+ }
+ }
+
+ return 0;
+}
+
+static int ren_vdc4_setcolreg(u_int regno,
+ u_int red, u_int green, u_int blue,
+ u_int transp, struct fb_info *info)
+{
+ u32 *palette = info->pseudo_palette;
+
+ if (regno >= PALETTE_NR)
+ return -EINVAL;
+
+ /* only FB_VISUAL_TRUECOLOR supported */
+
+ red >>= 16 - info->var.red.length;
+ green >>= 16 - info->var.green.length;
+ blue >>= 16 - info->var.blue.length;
+ transp >>= 16 - info->var.transp.length;
+
+ palette[regno] = (red << info->var.red.offset) |
+ (green << info->var.green.offset) |
+ (blue << info->var.blue.offset) |
+ (transp << info->var.transp.offset);
+
+ return 0;
+}
+
+static struct fb_fix_screeninfo ren_vdc4_fix = {
+ .id = "Renesas VDC4FB",
+ .type = FB_TYPE_PACKED_PIXELS,
+ .visual = FB_VISUAL_TRUECOLOR,
+ .accel = FB_ACCEL_NONE,
+};
+
+static struct fb_ops ren_vdc4_ops = {
+ .owner = THIS_MODULE,
+ .fb_setcolreg = ren_vdc4_setcolreg,
+ .fb_read = fb_sys_read,
+ .fb_write = fb_sys_write,
+ .fb_fillrect = sys_fillrect,
+ .fb_copyarea = sys_copyarea,
+ .fb_imageblit = sys_imageblit,
+};
+
+static int ren_vdc4_set_bpp(struct fb_var_screeninfo *var, int bpp)
+{
+ switch (bpp) {
+ case 16: /* RGB 565 */
+ var->red.offset = 11;
+ var->red.length = 5;
+ var->green.offset = 5;
+ var->green.length = 6;
+ var->blue.offset = 0;
+ var->blue.length = 5;
+ var->transp.offset = 0;
+ var->transp.length = 0;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ var->bits_per_pixel = bpp;
+ var->red.msb_right = 0;
+ var->green.msb_right = 0;
+ var->blue.msb_right = 0;
+ var->transp.msb_right = 0;
+ return 0;
+}
+
+/* PM Functions */
+static int ren_vdc4_start(struct ren_vdc4_priv *priv,
+ int clock_source)
+{
+ int ret;
+
+ ret = clk_enable(priv->clk);
+ if (ret < 0)
+ return ret;
+
+ if (priv->dot_clk) {
+ ret = clk_enable(priv->dot_clk);
+ if (ret < 0)
+ return ret;
+ }
+
+ restart_tft_display(priv, clock_source);
+
+ return ret;
+}
+
+static void ren_vdc4_stop(struct ren_vdc4_priv *priv)
+{
+ if (priv->dot_clk)
+ clk_disable(priv->dot_clk);
+ clk_disable(priv->clk);
+}
+
+static int ren_vdc4_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+
+ ren_vdc4_stop(platform_get_drvdata(pdev));
+ return 0;
+}
+
+static int ren_vdc4_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct ren_vdc4_info *pdata = pdev->dev.platform_data;
+
+ return ren_vdc4_start(platform_get_drvdata(pdev), pdata->clock_source);
+}
+
+static const struct dev_pm_ops ren_vdc4_dev_pm_ops = {
+ .suspend = ren_vdc4_suspend,
+ .resume = ren_vdc4_resume,
+};
+
+static int ren_vdc4_remove(struct platform_device *pdev);
+
+static int __devinit ren_vdc4_probe(struct platform_device *pdev)
+{
+ struct fb_info *info;
+ struct ren_vdc4_priv *priv;
+ struct ren_vdc4_info *pdata = pdev->dev.platform_data;
+ struct resource *res;
+ void *buf;
+ int irq, error;
+
+ if (!pdata) {
+ dev_err(&pdev->dev, "no platform data defined\n");
+ return -EINVAL;
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ irq = platform_get_irq(pdev, 0);
+ if (!res || irq < 0) {
+ dev_err(&pdev->dev, "cannot get platform resources\n");
+ return -ENOENT;
+ }
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv) {
+ dev_err(&pdev->dev, "cannot allocate device data\n");
+ return -ENOMEM;
+ }
+
+ platform_set_drvdata(pdev, priv);
+
+ error = devm_request_irq(&pdev->dev, irq, ren_vdc4_irq, 0,
+ dev_name(&pdev->dev), priv);
+ if (error) {
+ dev_err(&pdev->dev, "unable to request irq\n");
+ goto err1;
+ }
+
+ priv->irq = irq;
+ pdata = pdev->dev.platform_data;
+
+ priv->cfg = pdata;
+
+ error = ren_vdc4_setup_clocks(pdev, pdata->clock_source, priv);
+ if (error) {
+ dev_err(&pdev->dev, "unable to setup clocks\n");
+ goto err1;
+ }
+
+ priv->base = devm_ioremap_nocache(&pdev->dev, res->start,
+ resource_size(res));
+ if (!priv->base) {
+ dev_err(&pdev->dev, "unable to ioremap\n");
+ goto err1;
+ }
+
+ priv->info = framebuffer_alloc(0, &pdev->dev);
+ if (!priv->info) {
+ dev_err(&pdev->dev, "unable to allocate fb_info\n");
+ goto err1;
+ }
+
+ info = priv->info;
+ info->fbops = &ren_vdc4_ops;
+ info->var.xres = info->var.xres_virtual = pdata->lcd_cfg.xres;
+ info->var.yres = info->var.yres_virtual = pdata->lcd_cfg.yres;
+ info->var.width = pdata->panel_width;
+ info->var.height = pdata->panel_height;
+ info->var.activate = FB_ACTIVATE_NOW;
+ info->pseudo_palette = priv->pseudo_palette;
+ error = ren_vdc4_set_bpp(&info->var, pdata->bpp);
+ if (error)
+ goto err1;
+
+ info->fix = ren_vdc4_fix;
+ info->fix.line_length = pdata->lcd_cfg.xres * (pdata->bpp / 8);
+ info->fix.smem_len = info->fix.line_length * pdata->lcd_cfg.yres;
+
+ buf = dma_alloc_coherent(&pdev->dev, info->fix.smem_len,
+ &priv->dma_handle, GFP_KERNEL);
+ if (!buf) {
+ dev_err(&pdev->dev, "unable to allocate buffer\n");
+ goto err1;
+ }
+
+ info->flags = FBINFO_FLAG_DEFAULT;
+
+ error = fb_alloc_cmap(&info->cmap, PALETTE_NR, 0);
+ if (error < 0) {
+ dev_err(&pdev->dev, "unable to allocate cmap\n");
+ goto err1;
+ }
+
+ memset(buf, 0, info->fix.smem_len);
+ info->fix.smem_start = priv->dma_handle;
+ info->screen_base = buf;
+ info->device = &pdev->dev;
+ info->par = priv;
+
+ if (error)
+ goto err1;
+
+ error = ren_vdc4_start(priv, pdata->clock_source);
+ if (error) {
+ dev_err(&pdev->dev, "unable to start hardware\n");
+ goto err1;
+ }
+
+ info = priv->info;
+
+ error = register_framebuffer(info);
+ if (error < 0)
+ goto err1;
+
+ dev_info(info->dev,
+ "registered %s as %udx%ud %dbpp.\n",
+ pdev->name,
+ (int) pdata->lcd_cfg.xres,
+ (int) pdata->lcd_cfg.yres,
+ pdata->bpp);
+
+ return 0;
+
+err1:
+ ren_vdc4_remove(pdev);
+ return error;
+}
+
+static int ren_vdc4_remove(struct platform_device *pdev)
+{
+ struct ren_vdc4_priv *priv = platform_get_drvdata(pdev);
+ struct fb_info *info;
+
+ if (priv->info->dev)
+ unregister_framebuffer(priv->info);
+
+ ren_vdc4_stop(priv);
+
+ info = priv->info;
+
+ if (!info || !info->device) {
+ dev_err(&pdev->dev, "Failed to dealloc/release fb_info\n");
+ } else {
+ fb_dealloc_cmap(&info->cmap);
+ framebuffer_release(info);
+ }
+
+ return 0;
+}
+
+static struct platform_driver ren_vdc4_driver = {
+ .driver = {
+ .name = "ren_vdc4fb",
+ .owner = THIS_MODULE,
+ .pm = &ren_vdc4_dev_pm_ops,
+ },
+ .probe = ren_vdc4_probe,
+ .remove = ren_vdc4_remove,
+};
+
+static int __init ren_vdc4_init(void)
+{
+ return platform_driver_register(&ren_vdc4_driver);
+}
+
+static void __exit ren_vdc4_exit(void)
+{
+ platform_driver_unregister(&ren_vdc4_driver);
+}
+
+module_init(ren_vdc4_init);
+module_exit(ren_vdc4_exit);
+
+MODULE_DESCRIPTION("Renesas VDC4 Framebuffer driver");
+MODULE_AUTHOR("Phil Edworthy <phil.edworthy@renesas.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/video/ren_vdc4fb.h b/include/video/ren_vdc4fb.h
new file mode 100644
index 0000000..e91a515
--- /dev/null
+++ b/include/video/ren_vdc4fb.h
@@ -0,0 +1,19 @@
+#ifndef __REN_VDC4_H__
+#define __REN_VDC4_H__
+
+#include <linux/fb.h>
+
+enum { VDC4_EXTCLK = 1, VDC4_PERI_CLK };
+
+struct ren_vdc4_info {
+ int bpp;
+ int clock_source;
+ int clock_divider;
+ int hs_pulse_width;
+ int hs_start_pos;
+ struct fb_videomode lcd_cfg;
+ unsigned long panel_width;
+ unsigned long panel_height;
+};
+
+#endif
--
1.7.5.4
^ permalink raw reply related
* [PATCHv3 4/4] video: mmp: add tpo hvga panel supported
From: Zhou Zhu @ 2012-09-10 8:27 UTC (permalink / raw)
To: linux-fbdev
From: Lisa Du <cldu@marvell.com>
Add tpo hvga panel support in marvell display framework.
This panel driver implements modes query and power on/off.
This panel driver gets panel config/ plat power on/off/ connected
path name from machine-info and registered as a spi device.
This panel driver uses mmp_disp supplied register_panel function to
register panel to path as machine-info defined.
Signed-off-by: Lisa Du <cldu@marvell.com>
Signed-off-by: Zhou Zhu <zzhu3@marvell.com>
---
drivers/video/mmp/Kconfig | 1 +
drivers/video/mmp/Makefile | 2 +-
drivers/video/mmp/panel/Kconfig | 6 +
drivers/video/mmp/panel/Makefile | 1 +
drivers/video/mmp/panel/tpo_tj032md01bw.c | 188 +++++++++++++++++++++++++++++
5 files changed, 197 insertions(+), 1 deletions(-)
create mode 100644 drivers/video/mmp/panel/Kconfig
create mode 100644 drivers/video/mmp/panel/Makefile
create mode 100644 drivers/video/mmp/panel/tpo_tj032md01bw.c
diff --git a/drivers/video/mmp/Kconfig b/drivers/video/mmp/Kconfig
index ed51d15..e9ea39e 100644
--- a/drivers/video/mmp/Kconfig
+++ b/drivers/video/mmp/Kconfig
@@ -6,5 +6,6 @@ menuconfig MMP_DISP
if MMP_DISP
source "drivers/video/mmp/hw/Kconfig"
+source "drivers/video/mmp/panel/Kconfig"
source "drivers/video/mmp/fb/Kconfig"
endif
diff --git a/drivers/video/mmp/Makefile b/drivers/video/mmp/Makefile
index 6999a09..a014cb3 100644
--- a/drivers/video/mmp/Makefile
+++ b/drivers/video/mmp/Makefile
@@ -1 +1 @@
-obj-y += core.o hw/ fb/
+obj-y += core.o hw/ panel/ fb/
diff --git a/drivers/video/mmp/panel/Kconfig b/drivers/video/mmp/panel/Kconfig
new file mode 100644
index 0000000..4b2c4f4
--- /dev/null
+++ b/drivers/video/mmp/panel/Kconfig
@@ -0,0 +1,6 @@
+config MMP_PANEL_TPOHVGA
+ bool "tpohvga panel TJ032MD01BW support"
+ depends on SPI_MASTER
+ default n
+ help
+ tpohvga panel support
diff --git a/drivers/video/mmp/panel/Makefile b/drivers/video/mmp/panel/Makefile
new file mode 100644
index 0000000..2f91611
--- /dev/null
+++ b/drivers/video/mmp/panel/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_MMP_PANEL_TPOHVGA) += tpo_tj032md01bw.o
diff --git a/drivers/video/mmp/panel/tpo_tj032md01bw.c b/drivers/video/mmp/panel/tpo_tj032md01bw.c
new file mode 100644
index 0000000..f54b8a7
--- /dev/null
+++ b/drivers/video/mmp/panel/tpo_tj032md01bw.c
@@ -0,0 +1,188 @@
+/*
+ * linux/drivers/video/mmp/panel/tpo_tj032md01bw.c
+ * active panel using spi interface to do init
+ *
+ * Copyright (C) 2012 Marvell Technology Group Ltd.
+ * Authors: Guoqing Li <ligq@marvell.com>
+ * Lisa Du <cldu@marvell.com>
+ * Zhou Zhu <zzhu3@marvell.com>
+ *
+ * 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.
+ *
+ * This program 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 program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/spi/spi.h>
+#include <video/mmp_disp.h>
+
+static u16 init[] = {
+ 0x0801,
+ 0x0800,
+ 0x0200,
+ 0x0304,
+ 0x040e,
+ 0x0903,
+ 0x0b18,
+ 0x0c53,
+ 0x0d01,
+ 0x0ee0,
+ 0x0f01,
+ 0x1058,
+ 0x201e,
+ 0x210a,
+ 0x220a,
+ 0x231e,
+ 0x2400,
+ 0x2532,
+ 0x2600,
+ 0x27ac,
+ 0x2904,
+ 0x2aa2,
+ 0x2b45,
+ 0x2c45,
+ 0x2d15,
+ 0x2e5a,
+ 0x2fff,
+ 0x306b,
+ 0x310d,
+ 0x3248,
+ 0x3382,
+ 0x34bd,
+ 0x35e7,
+ 0x3618,
+ 0x3794,
+ 0x3801,
+ 0x395d,
+ 0x3aae,
+ 0x3bff,
+ 0x07c9,
+};
+
+static u16 poweroff[] = {
+ 0x07d9,
+};
+
+struct tpohvga_plat_data {
+ void (*plat_onoff)(int status);
+ struct spi_device *spi;
+};
+
+static void tpohvga_onoff(struct mmp_panel *panel, int status)
+{
+ struct tpohvga_plat_data *plat = panel->plat_data;
+ int ret;
+
+ if (status) {
+ plat->plat_onoff(1);
+
+ ret = spi_write(plat->spi, init, sizeof(init));
+ if (ret < 0)
+ dev_warn(panel->dev, "init cmd failed(%d)\n", ret);
+ } else {
+ ret = spi_write(plat->spi, poweroff, sizeof(poweroff));
+ if (ret < 0)
+ dev_warn(panel->dev, "poweroff cmd failed(%d)\n", ret);
+
+ plat->plat_onoff(0);
+ }
+}
+
+static struct mmp_mode mmp_modes_tpohvga[] = {
+ [0] = {
+ .pixclock_freq = 10394400,
+ .refresh = 60,
+ .xres = 320,
+ .yres = 480,
+ .hsync_len = 10,
+ .left_margin = 15,
+ .right_margin = 10,
+ .vsync_len = 2,
+ .upper_margin = 4,
+ .lower_margin = 2,
+ .invert_pixclock = 1,
+ .pix_fmt_out = PIXFMT_RGB565,
+ },
+};
+
+static int tpohvga_get_modelist(struct mmp_panel *panel,
+ struct mmp_mode **modelist)
+{
+ *modelist = mmp_modes_tpohvga;
+ return 1;
+}
+
+static struct mmp_panel panel_tpohvga = {
+ .name = "tpohvga",
+ .panel_type = PANELTYPE_Active,
+ .get_modelist = tpohvga_get_modelist,
+ .set_onoff = tpohvga_onoff,
+};
+
+static int __devinit tpohvga_probe(struct spi_device *spi)
+{
+ struct mmp_mach_panel_info *mi;
+ int ret;
+ struct tpohvga_plat_data *plat_data;
+
+ /* get configs from platform data */
+ mi = spi->dev.platform_data;
+ if (mi = NULL) {
+ dev_err(&spi->dev, "%s: no platform data defined\n", __func__);
+ return -EINVAL;
+ }
+
+ /* setup spi related info */
+ spi->bits_per_word = 16;
+ ret = spi_setup(spi);
+ if (ret < 0) {
+ dev_err(&spi->dev, "spi setup failed %d", ret);
+ return ret;
+ }
+
+ plat_data = kzalloc(sizeof(*plat_data), GFP_KERNEL);
+ if (plat_data = NULL)
+ return -ENOMEM;
+
+ plat_data->spi = spi;
+ plat_data->plat_onoff = mi->plat_set_onoff;
+ panel_tpohvga.plat_data = plat_data;
+ panel_tpohvga.plat_path_name = mi->plat_path_name;
+ panel_tpohvga.dev = &spi->dev;
+
+ if (!mmp_register_panel(&panel_tpohvga)) {
+ dev_err(&spi->dev, "%s: register failed\n", __func__);
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static struct spi_driver panel_tpohvga_driver = {
+ .driver = {
+ .name = "tpo-hvga",
+ .owner = THIS_MODULE,
+ },
+ .probe = tpohvga_probe,
+};
+module_spi_driver(panel_tpohvga_driver);
+
+MODULE_AUTHOR("Lisa Du<cldu@marvell.com>");
+MODULE_DESCRIPTION("Panel driver for tpohvga");
+MODULE_LICENSE("GPL");
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] fbdev: Add Renesas vdc4 framebuffer driver
From: phil.edworthy @ 2012-09-10 8:14 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1344428095-12203-1-git-send-email-phil.edworthy@renesas.com>
Hi Jingoo Han,
> > I reviewed your patch.
> > Please refer to my comments.
> > Good luck.
Thanks for the review, I'll have a look at the items you have highlighted.
Phil
^ permalink raw reply
* Re: [RFC PATCH v2] OMAPDSS: Fix IRQ unregister race
From: Tomi Valkeinen @ 2012-09-10 7:49 UTC (permalink / raw)
To: Dimitar Dimitrov; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1347116753-17064-1-git-send-email-dinuxbg@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 8260 bytes --]
On Sat, 2012-09-08 at 18:05 +0300, Dimitar Dimitrov wrote:
> Very rare kernel crashes are reported on a custom OMAP4 board. Kernel
> panics due to corrupted completion structure while executing
> dispc_irq_wait_handler(). Excerpt from kernel log:
>
> Internal error: Oops - undefined instruction: 0 [#1] PREEMPT SMP
> Unable to handle kernel paging request at virtual address 00400130
> ...
> PC is at 0xebf205bc
> LR is at __wake_up_common+0x54/0x94
> ...
> (__wake_up_common+0x0/0x94)
> (complete+0x0/0x60)
> (dispc_irq_wait_handler.36902+0x0/0x14)
> (omap_dispc_irq_handler+0x0/0x354)
> (handle_irq_event_percpu+0x0/0x188)
> (handle_irq_event+0x0/0x64)
> (handle_fasteoi_irq+0x0/0x10c)
> (generic_handle_irq+0x0/0x48)
> (asm_do_IRQ+0x0/0xc0)
>
> DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
> unregister_isr() and DISPC IRQ might be running in parallel on different
> CPUs. So there is a chance that a callback is executed even though it
> has been unregistered. As omap_dispc_wait_for_irq_timeout() declares a
> completion on stack, the dispc_irq_wait_handler() callback might try to
> access a completion structure that is invalid. This leads to crashes and
> hangs.
>
> Solution is to divide unregister calls into two sets:
> 1. Non-strict unregistering of callbacks. Callbacks could safely be
> executed after unregistering them. This is the case with unregister
> calls from the IRQ handler itself.
> 2. Strict (synchronized) unregistering. Callbacks are not allowed
> after unregistering. This is the case with completion waiting.
>
> The above solution should satisfy one of the original intentions of the
> driver: callbacks should be able to unregister themselves.
>
> Also fix DSI IRQ unregister which has similar logic to DISPC IRQ handling.
>
> Signed-off-by: Dimitar Dimitrov <dinuxbg@gmail.com>
> ---
>
> WARNING: This bug is quite old. The patch has been tested on v3.0. No testing
> has been done after rebasing to v3.6. Hence the RFC tag. Hopefully someone
> will beat me in testing with latest linux-omap/master.
>
> Changes since v1 per Tomi Valkeinen's comments:
> - Don't rename omap_dispc_unregister_isr, just introduce nosync variant.
> - Apply the same fix for DSI IRQ which suffers from the same race condition.
I made a quick test and works for me, although I haven't encountered the
problem itself.
Some mostly cosmetic comments below.
This seems to apply cleanly to v3.4+ kernels, but not earlier ones. Do
you want to make the needed modifications and mail this and the modified
patches for stable kernels also? I can do that also if you don't want
to.
> drivers/staging/omapdrm/omap_plane.c | 2 +-
> drivers/video/omap2/dss/apply.c | 2 +-
> drivers/video/omap2/dss/dispc.c | 45 +++++++++++++++++++++++++++++-----
> drivers/video/omap2/dss/dsi.c | 19 ++++++++++++++
> include/video/omapdss.h | 1 +
> 5 files changed, 61 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/omapdrm/omap_plane.c b/drivers/staging/omapdrm/omap_plane.c
> index 7997be7..8d8aa5b 100644
> --- a/drivers/staging/omapdrm/omap_plane.c
> +++ b/drivers/staging/omapdrm/omap_plane.c
> @@ -82,7 +82,7 @@ static void dispc_isr(void *arg, uint32_t mask)
> struct omap_plane *omap_plane = to_omap_plane(plane);
> struct omap_drm_private *priv = plane->dev->dev_private;
>
> - omap_dispc_unregister_isr(dispc_isr, plane,
> + omap_dispc_unregister_isr_nosync(dispc_isr, plane,
> id2irq[omap_plane->ovl->id]);
>
> queue_work(priv->wq, &omap_plane->work);
> diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
> index 0fefc68..9386834 100644
> --- a/drivers/video/omap2/dss/apply.c
> +++ b/drivers/video/omap2/dss/apply.c
> @@ -847,7 +847,7 @@ static void dss_unregister_vsync_isr(void)
> for (i = 0; i < num_mgrs; ++i)
> mask |= dispc_mgr_get_framedone_irq(i);
>
> - r = omap_dispc_unregister_isr(dss_apply_irq_handler, NULL, mask);
> + r = omap_dispc_unregister_isr_nosync(dss_apply_irq_handler, NULL, mask);
> WARN_ON(r);
>
> dss_data.irq_enabled = false;
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index ee9e296..a67d92c 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -2421,8 +2421,8 @@ static void dispc_mgr_enable_digit_out(bool enable)
> enable ? "start" : "stop");
> }
>
> - r = omap_dispc_unregister_isr(dispc_disable_isr, &frame_done_completion,
> - irq_mask);
> + r = omap_dispc_unregister_isr(dispc_disable_isr,
> + &frame_done_completion, irq_mask);
This change is not needed.
> if (r)
> DSSERR("failed to unregister %x isr\n", irq_mask);
>
> @@ -3320,7 +3320,8 @@ err:
> }
> EXPORT_SYMBOL(omap_dispc_register_isr);
>
> -int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
> +/* WARNING: callback might be executed even after this function returns! */
> +int omap_dispc_unregister_isr_nosync(omap_dispc_isr_t isr, void *arg, u32 mask)
> {
> int i;
> unsigned long flags;
> @@ -3352,7 +3353,37 @@ int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
>
> return ret;
> }
> -EXPORT_SYMBOL(omap_dispc_unregister_isr);
> +EXPORT_SYMBOL(omap_dispc_unregister_isr_nosync);
> +
> +/*
> + * Ensure that callback <isr> will NOT be executed after this function
> + * returns. Must be called from sleepable context, though!
> + */
> +int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
> +{
> + int ret;
> +
> + ret = omap_dispc_unregister_isr_nosync(isr, arg, mask);
> +
> + /* Task context is not really needed. But if we're called from atomic
> + * context, it is probably from DISPC IRQ, where we will deadlock.
> + * So use might_sleep() to catch potential deadlocks.
> + */
Use the kernel multi-line comment style, i.e.:
/*
* foobar
*/
> + might_sleep();
> +
> +#if defined(CONFIG_SMP)
> + /* DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
> + * unregister_isr() and DISPC IRQ might be running in parallel on
> + * different CPUs. So there is a chance that a callback is executed
> + * even though it has been unregistered. Add a barrier, in order to
> + * ensure that after returning from this function, the new DISPC IRQ
> + * will use an updated callback array, and NOT its cached copy.
> + */
Comment style here also.
> + synchronize_irq(dispc.irq);
> +#endif
Why do you use #if defined(CONFIG_SMP)? synchronize_irq is defined
with !CONFIG_SMP also. In that case it becomes just barrier().
> +
> + return ret;
> +}
>
> #ifdef DEBUG
> static void print_irq_status(u32 status)
> @@ -3567,7 +3598,8 @@ int omap_dispc_wait_for_irq_timeout(u32 irqmask, unsigned long timeout)
>
> timeout = wait_for_completion_timeout(&completion, timeout);
>
> - omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion, irqmask);
> + omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion,
> + irqmask);
Change not needed.
>
> if (timeout == 0)
> return -ETIMEDOUT;
> @@ -3598,7 +3630,8 @@ int omap_dispc_wait_for_irq_interruptible_timeout(u32 irqmask,
> timeout = wait_for_completion_interruptible_timeout(&completion,
> timeout);
>
> - omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion, irqmask);
> + omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion,
> + irqmask);
Change not needed.
>
> if (timeout == 0)
> return -ETIMEDOUT;
> diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
> index b07e886..24b4a3e 100644
> --- a/drivers/video/omap2/dss/dsi.c
> +++ b/drivers/video/omap2/dss/dsi.c
> @@ -960,6 +960,13 @@ static int dsi_unregister_isr(struct platform_device *dsidev,
>
> spin_unlock_irqrestore(&dsi->irq_lock, flags);
>
> + might_sleep();
> +
> +#if defined(CONFIG_SMP)
> + /* See notes for dispc.c:omap_dispc_unregister_isr() . */
> + synchronize_irq(dsi->irq);
> +#endif
> +
Same SMP comment as with dispc.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] fbdev: Add Renesas vdc4 framebuffer driver
From: Jingoo Han @ 2012-09-10 4:37 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1344428095-12203-1-git-send-email-phil.edworthy@renesas.com>
On Monday, September 10, 2012 1:31 PM Jingoo Han wrote
>
> On Wednesday, August 08, 2012 9:15 PM Phil Edworthy wrote
>
> Hi Phil Edworthy,
>
> I reviewed your patch.
> Please refer to my comments.
> Good luck.
>
> Best regards,
> Jingoo Han
>
> >
> > The vdc4 display hardware is found on the sh7269 device.
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
>
> Please insert one line between the commit message and Signed-off-by.
>
> > ---
> > drivers/video/Kconfig | 10 +
> > drivers/video/Makefile | 1 +
> > drivers/video/ren_vdc4fb.c | 653 ++++++++++++++++++++++++++++++++++++++++++++
> > include/video/ren_vdc4fb.h | 19 ++
> > 4 files changed, 683 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/video/ren_vdc4fb.c
> > create mode 100644 include/video/ren_vdc4fb.h
> >
.....
> > +
> > +static int ren_vdc4_setup_clocks(struct platform_device *pdev,
> > + int clock_source,
> > + struct ren_vdc4_priv *priv)
> > +{
> > + priv->clk = clk_get(&pdev->dev, "vdc4");
>
> How about using devm_clk_get() instead of kzalloc()?
> It makes the code simpler.
Sorry, it's a typo.
It is not kzalloc(), but clk_get().
>
> > + if (IS_ERR(priv->clk)) {
> > + dev_err(&pdev->dev, "cannot get clock \"vdc4\"\n");
> > + return PTR_ERR(priv->clk);
> > + }
> > +
> > + if (clock_source = VDC4_PERI_CLK) {
> > + priv->dot_clk = clk_get(&pdev->dev, "peripheral_clk");
>
> How about using devm_clk_get() instead of kzalloc()?
> It makes the code simpler.
Same as above.
>
>
^ permalink raw reply
* Re: [PATCH] fbdev: Add Renesas vdc4 framebuffer driver
From: Jingoo Han @ 2012-09-10 4:31 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1344428095-12203-1-git-send-email-phil.edworthy@renesas.com>
On Wednesday, August 08, 2012 9:15 PM Phil Edworthy wrote
Hi Phil Edworthy,
I reviewed your patch.
Please refer to my comments.
Good luck.
Best regards,
Jingoo Han
>
> The vdc4 display hardware is found on the sh7269 device.
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Please insert one line between the commit message and Signed-off-by.
> ---
> drivers/video/Kconfig | 10 +
> drivers/video/Makefile | 1 +
> drivers/video/ren_vdc4fb.c | 653 ++++++++++++++++++++++++++++++++++++++++++++
> include/video/ren_vdc4fb.h | 19 ++
> 4 files changed, 683 insertions(+), 0 deletions(-)
> create mode 100644 drivers/video/ren_vdc4fb.c
> create mode 100644 include/video/ren_vdc4fb.h
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 0217f74..89c9250 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -1990,6 +1990,16 @@ config FB_W100
>
> If unsure, say N.
>
> +config FB_REN_VDC4FB
> + tristate "Renesas VDC4 framebuffer support"
> + depends on FB && CPU_SUBTYPE_SH7269
> + select FB_SYS_FILLRECT
> + select FB_SYS_COPYAREA
> + select FB_SYS_IMAGEBLIT
> + select FB_SYS_FOPS
> + ---help---
> + Frame buffer driver for the Renesas VDC4.
> +
> config FB_SH_MOBILE_LCDC
> tristate "SuperH Mobile LCDC framebuffer support"
> depends on FB && (SUPERH || ARCH_SHMOBILE) && HAVE_CLK
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index ee8dafb..ba69fcb 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -140,6 +140,7 @@ obj-$(CONFIG_SH_MIPI_DSI) += sh_mipi_dsi.o
> obj-$(CONFIG_FB_SH_MOBILE_HDMI) += sh_mobile_hdmi.o
> obj-$(CONFIG_FB_SH_MOBILE_MERAM) += sh_mobile_meram.o
> obj-$(CONFIG_FB_SH_MOBILE_LCDC) += sh_mobile_lcdcfb.o
> +obj-$(CONFIG_FB_REN_VDC4FB) += ren_vdc4fb.o
> obj-$(CONFIG_FB_OMAP) += omap/
> obj-y += omap2/
> obj-$(CONFIG_XEN_FBDEV_FRONTEND) += xen-fbfront.o
> diff --git a/drivers/video/ren_vdc4fb.c b/drivers/video/ren_vdc4fb.c
> new file mode 100644
> index 0000000..1a31e85
> --- /dev/null
> +++ b/drivers/video/ren_vdc4fb.c
> @@ -0,0 +1,653 @@
> +/*
> + * Renesas VDC4 Framebuffer
> + *
> + * Based on sh_mobile_lcdcfb.c
> + * Copyright (c) 2012 Renesas Electronics Europe Ltd
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License. See the file "COPYING" in the main directory of this archive
> + * for more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/mm.h>
> +#include <linux/clk.h>
> +#include <linux/sh_clk.h>
> +#include <linux/platform_device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/interrupt.h>
> +#include <linux/vmalloc.h>
> +#include <linux/module.h>
> +#include <video/ren_vdc4fb.h>
> +
> +#define PALETTE_NR 16
> +
> +struct ren_vdc4_priv {
> + void __iomem *base;
> + int irq;
> + struct clk *dot_clk;
> + struct clk *clk;
> + struct fb_info *info;
> + dma_addr_t dma_handle;
> + struct ren_vdc4_info *cfg;
> + u32 pseudo_palette[PALETTE_NR];
> +};
> +
> +/* Register offsets/reading and writing functions */
> +enum {
> + SCL0_UPDATE, SCL0_FRC1, SCL0_FRC2, SCL0_FRC3,
> + SCL0_FRC4, SCL0_FRC5, SCL0_FRC6, SCL0_FRC7,
> + SCL0_DS1, SCL0_US1,
> +
> + GR1_UPDATE, GR1_AB1,
> +
> + GR2_UPDATE, GR2_AB1,
> +
> + GR3_UPDATE, GR3_FLM_RD, GR3_FLM1, GR3_FLM2,
> + GR3_FLM3, GR3_FLM4, GR3_FLM5, GR3_FLM6, GR3_AB1,
> + GR3_AB2, GR3_AB3, GR3_AB4, GR3_AB5, GR3_AB6,
> + GR3_AB7, GR3_AB8, GR3_AB9, GR3_AB10, GR3_AB11,
> + GR3_BASE, GR3_CLUT_INT, GR3_MON,
> +
> + TCON_UPDATE, TCON_TIM, TCON_TIM_STVA1, TCON_TIM_STVA2,
> + TCON_TIM_STVB1, TCON_TIM_STVB2, TCON_TIM_STH1,
> + TCON_TIM_STH2, TCON_TIM_STB1, TCON_TIM_STB2,
> + TCON_TIM_CPV1, TCON_TIM_CPV2, TCON_TIM_POLA1,
> + TCON_TIM_POLA2, TCON_TIM_POLB1, TCON_TIM_POLB2,
> + TCON_TIM_DE,
> +
> + OUT_UPDATE, OUT_SET, OUT_BRIGHT1,
> + OUT_BRIGHT2, OUT_CONTRAST, OUT_PDTHA, OUT_CLK_PHASE,
> +
> + SYSCNT_INT1, SYSCNT_INT2, SYSCNT_INT3, SYSCNT_INT4,
> + SYSCNT_PANEL_CLK, SYSCNT_CLUT
> +};
> +
> +static unsigned long vdc4_offsets[] = {
> + [SCL0_UPDATE] = 0x0100,
> + [SCL0_FRC1] = 0x0104,
> + [SCL0_FRC2] = 0x0108,
> + [SCL0_FRC3] = 0x010C,
> + [SCL0_FRC4] = 0x0110,
> + [SCL0_FRC5] = 0x0114,
> + [SCL0_FRC6] = 0x0118,
> + [SCL0_FRC7] = 0x011C,
> + [SCL0_DS1] = 0x012C,
> + [SCL0_US1] = 0x0148,
> + [GR1_UPDATE] = 0x0200,
> + [GR1_AB1] = 0x0220,
> + [GR2_UPDATE] = 0x0300,
> + [GR2_AB1] = 0x0320,
> + [GR3_UPDATE] = 0x0380,
> + [GR3_FLM_RD] = 0x0384,
> + [GR3_FLM1] = 0x0388,
> + [GR3_FLM2] = 0x038C,
> + [GR3_FLM3] = 0x0390,
> + [GR3_FLM4] = 0x0394,
> + [GR3_FLM5] = 0x0398,
> + [GR3_FLM6] = 0x039C,
> + [GR3_AB1] = 0x03A0,
> + [GR3_AB2] = 0x03A4,
> + [GR3_AB3] = 0x03A8,
> + [GR3_AB4] = 0x03AC,
> + [GR3_AB5] = 0x03B0,
> + [GR3_AB6] = 0x03B4,
> + [GR3_AB7] = 0x03B8,
> + [GR3_AB8] = 0x03BC,
> + [GR3_AB9] = 0x03C0,
> + [GR3_AB10] = 0x03C4,
> + [GR3_AB11] = 0x03C8,
> + [GR3_BASE] = 0x03CC,
> + [GR3_CLUT_INT] = 0x03D0,
> + [GR3_MON] = 0x03D4,
> + [TCON_UPDATE] = 0x0580,
> + [TCON_TIM] = 0x0584,
> + [TCON_TIM_STVA1] = 0x0588,
> + [TCON_TIM_STVA2] = 0x058C,
> + [TCON_TIM_STVB1] = 0x0590,
> + [TCON_TIM_STVB2] = 0x0594,
> + [TCON_TIM_STH1] = 0x0598,
> + [TCON_TIM_STH2] = 0x059C,
> + [TCON_TIM_STB1] = 0x05A0,
> + [TCON_TIM_STB2] = 0x05A4,
> + [TCON_TIM_CPV1] = 0x05A8,
> + [TCON_TIM_CPV2] = 0x05AC,
> + [TCON_TIM_POLA1] = 0x05B0,
> + [TCON_TIM_POLA2] = 0x05B4,
> + [TCON_TIM_POLB1] = 0x05B8,
> + [TCON_TIM_POLB2] = 0x05BC,
> + [TCON_TIM_DE] = 0x05C0,
> + [OUT_UPDATE] = 0x0600,
> + [OUT_SET] = 0x0604,
> + [OUT_BRIGHT1] = 0x0608,
> + [OUT_BRIGHT2] = 0x060C,
> + [OUT_CONTRAST] = 0x0610,
> + [OUT_PDTHA] = 0x0614,
> + [OUT_CLK_PHASE] = 0x0624,
> + [SYSCNT_INT1] = 0x0680,
> + [SYSCNT_INT2] = 0x0684,
> + [SYSCNT_INT3] = 0x0688,
> + [SYSCNT_INT4] = 0x068C,
> + [SYSCNT_PANEL_CLK] = 0x0690, /* 16-bit */
> + [SYSCNT_CLUT] = 0x0692, /* 16-bit */
> +};
> +
> +/* SYSCNT */
> +#define ICKEN (1 << 8)
> +
> +/* SCL Syncs */
> +#define FREE_RUN_VSYNC 0x0001
> +
> +/* OUTPUT */
> +#define OUT_FMT_RGB666 (1 << 12)
> +
> +/* TCON Timings */
> +#define STVB_SEL_BITS 0x0007
> +#define STVB_HS_SEL 2
> +
> +#define STH2_SEL_BITS 0x0007
> +#define STH2_DE_SEL 7
> +
> +/* OUTCLK */
> +#define LCD_DATA_EDGE 0x0100
> +#define STVB_EDGE 0x0020
> +#define STH_EDGE 0x0010
> +
> +/* SCL_UPDATE */
> +#define SCL0_UPDATE_BIT 0x0100
> +#define SCL0_VEN_BIT 0x0010
> +
> +/* TCON_UPDATE */
> +#define TCON_VEN_BIT 0x0001
> +
> +/* OUT_UPDATE */
> +#define OUTCNT_VEN_BIT 0x0001
> +
> +/* GR_UPDATE */
> +#define P_VEN_UPDATE 0x0010
> +#define IBUS_VEN_UPDATE 0x0001
> +
> +/* GR_AB1 */
> +#define DISPSEL_BCKGND 0x0000
> +#define DISPSEL_LOWER 0x0001
> +#define DISPSEL_CUR 0x0002
> +
> +/* GR_FLM_RD */
> +#define FB_R_ENB 0x01
> +
> +
Please remove unnecessary line.
> +static void vdc4_write(struct ren_vdc4_priv *priv,
> + unsigned long reg_offs, unsigned long data)
> +{
> + if ((SYSCNT_PANEL_CLK = reg_offs) || (SYSCNT_CLUT = reg_offs))
> + iowrite16(data, priv->base + vdc4_offsets[reg_offs]);
> + else
> + iowrite32(data, priv->base + vdc4_offsets[reg_offs]);
> +}
> +
> +static unsigned long vdc4_read(struct ren_vdc4_priv *priv,
> + unsigned long reg_offs)
> +{
> + if ((SYSCNT_PANEL_CLK = reg_offs) || (SYSCNT_CLUT = reg_offs))
> + return ioread16(priv->base + vdc4_offsets[reg_offs]);
> + else
> + return ioread32(priv->base + vdc4_offsets[reg_offs]);
> +}
> +
> +static irqreturn_t ren_vdc4_irq(int irq, void *data)
> +{
> + /* Not currently implemented/used */
> + return IRQ_HANDLED;
> +}
> +
> +static void lcd_clear_display(struct ren_vdc4_priv *priv)
> +{
> + unsigned char *pdest;
> + unsigned long size;
> +
> + pdest = (unsigned char *)priv->dma_handle;
> + size = priv->cfg->lcd_cfg.xres * priv->cfg->lcd_cfg.yres * 2;
> +
> + memset(pdest, 0, size);
> +}
> +
> +static void restart_tft_display(struct ren_vdc4_priv *priv,
> + int clock_source)
> +{
> + struct fb_videomode *lcd;
> + unsigned long h;
> + unsigned long v;
> + unsigned long tmp;
> +
> + /* FB setup */
> + lcd = &priv->cfg->lcd_cfg;
> + lcd_clear_display(priv);
> +
> + /* VDC clock Setup */
> + tmp = priv->cfg->clock_divider;
> + tmp |= clock_source << 12;
> + tmp |= ICKEN;
> + vdc4_write(priv, SYSCNT_PANEL_CLK, tmp);
> +
> + /* Clear and Disable all interrupts */
> + vdc4_write(priv, SYSCNT_INT1, 0);
> + vdc4_write(priv, SYSCNT_INT2, 0);
> + vdc4_write(priv, SYSCNT_INT3, 0);
> + vdc4_write(priv, SYSCNT_INT4, 0);
> +
> + /* Setup free-running syncs */
> + vdc4_write(priv, SCL0_FRC3, FREE_RUN_VSYNC);
> +
> + /* Disable scale up/down */
> + vdc4_write(priv, SCL0_DS1, 0);
> + vdc4_write(priv, SCL0_US1, 0);
> +
> + /* Timing registers */
> + h = lcd->hsync_len + lcd->left_margin + lcd->xres + lcd->right_margin;
> + v = lcd->vsync_len + lcd->upper_margin + lcd->yres + lcd->lower_margin;
> + tmp = (v - 1) << 16;
> + tmp |= h - 1;
> + vdc4_write(priv, SCL0_FRC4, tmp);
> +
> + vdc4_write(priv, TCON_TIM, (((h - 1) / 2) << 16));
> +
> + tmp = (lcd->vsync_len + lcd->upper_margin) << 16;
> + tmp |= lcd->yres;
> + vdc4_write(priv, SCL0_FRC6, tmp);
> + vdc4_write(priv, TCON_TIM_STVB1, tmp);
> + vdc4_write(priv, GR3_AB2, tmp);
> +
> + tmp = lcd->left_margin << 16;
> + tmp |= lcd->xres;
> + vdc4_write(priv, SCL0_FRC7, tmp);
> + vdc4_write(priv, TCON_TIM_STB1, tmp);
> + vdc4_write(priv, GR3_AB3, tmp);
> +
> + vdc4_write(priv, SCL0_FRC1, 0);
> + vdc4_write(priv, SCL0_FRC2, 0);
> + vdc4_write(priv, SCL0_FRC5, 0);
> +
> + /* Set output format */
> + vdc4_write(priv, OUT_SET, OUT_FMT_RGB666);
> +
> + /* STH TCON Timing */
> + tmp = priv->cfg->hs_pulse_width;
> + tmp |= priv->cfg->hs_start_pos << 16;
> + vdc4_write(priv, TCON_TIM_STH1, tmp);
> +
> + /* Setup STVB as HSYNC */
> + tmp = vdc4_read(priv, TCON_TIM_STVB2);
> + tmp &= ~STVB_SEL_BITS;
> + tmp |= STVB_HS_SEL;
> + vdc4_write(priv, TCON_TIM_STVB2, tmp);
> +
> + tmp = vdc4_read(priv, OUT_CLK_PHASE);
> + tmp &= ~STVB_EDGE;
> + vdc4_write(priv, OUT_CLK_PHASE, tmp);
> +
> + /* Setup STH as DE */
> + tmp = vdc4_read(priv, TCON_TIM_STH2);
> + tmp &= ~STH2_SEL_BITS;
> + tmp |= STH2_DE_SEL;
> + vdc4_write(priv, TCON_TIM_STH2, tmp);
> +
> + tmp = vdc4_read(priv, OUT_CLK_PHASE);
> + tmp &= ~STH_EDGE;
> + vdc4_write(priv, OUT_CLK_PHASE, tmp);
> +
> + /* Output clock rising edge */
> + tmp = vdc4_read(priv, OUT_CLK_PHASE);
> + tmp &= ~LCD_DATA_EDGE;
> + vdc4_write(priv, OUT_CLK_PHASE, tmp);
> +
> + /* Setup graphics buffers and update all registers */
> + vdc4_write(priv, GR1_AB1, DISPSEL_BCKGND);
> + vdc4_write(priv, GR2_AB1, DISPSEL_LOWER);
> + vdc4_write(priv, GR3_AB1, DISPSEL_CUR);
> +
> + /* Setup framebuffer base/output */
> + vdc4_write(priv, GR3_FLM_RD, FB_R_ENB);
> +
> + vdc4_write(priv, GR3_FLM2, (unsigned long)priv->info->screen_base);
> +
> + vdc4_write(priv, GR3_FLM3, (lcd->xres * 2) << 16);
> +
> + tmp = vdc4_read(priv, GR3_FLM5);
> + tmp |= lcd->yres << 16;
> + vdc4_write(priv, GR3_FLM5, tmp);
> +
> + tmp = lcd->xres << 16;
> + vdc4_write(priv, GR3_FLM6, tmp);
> +
> + /* Apply all register settings */
> + vdc4_write(priv, SCL0_UPDATE, SCL0_VEN_BIT | SCL0_UPDATE_BIT);
> + vdc4_write(priv, GR1_UPDATE, P_VEN_UPDATE);
> + vdc4_write(priv, GR2_UPDATE, P_VEN_UPDATE);
> + vdc4_write(priv, GR3_UPDATE, P_VEN_UPDATE | IBUS_VEN_UPDATE);
> + vdc4_write(priv, OUT_UPDATE, OUTCNT_VEN_BIT);
> + vdc4_write(priv, TCON_UPDATE, TCON_VEN_BIT);
> +}
> +
> +static int ren_vdc4_setup_clocks(struct platform_device *pdev,
> + int clock_source,
> + struct ren_vdc4_priv *priv)
> +{
> + priv->clk = clk_get(&pdev->dev, "vdc4");
How about using devm_clk_get() instead of kzalloc()?
It makes the code simpler.
> + if (IS_ERR(priv->clk)) {
> + dev_err(&pdev->dev, "cannot get clock \"vdc4\"\n");
> + return PTR_ERR(priv->clk);
> + }
> +
> + if (clock_source = VDC4_PERI_CLK) {
> + priv->dot_clk = clk_get(&pdev->dev, "peripheral_clk");
How about using devm_clk_get() instead of kzalloc()?
It makes the code simpler.
> + if (IS_ERR(priv->dot_clk)) {
> + dev_err(&pdev->dev, "cannot get peripheral clock\n");
> + clk_put(priv->clk);
> + return PTR_ERR(priv->dot_clk);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int ren_vdc4_setcolreg(u_int regno,
> + u_int red, u_int green, u_int blue,
> + u_int transp, struct fb_info *info)
> +{
> + u32 *palette = info->pseudo_palette;
> +
> + if (regno >= PALETTE_NR)
> + return -EINVAL;
> +
> + /* only FB_VISUAL_TRUECOLOR supported */
> +
> + red >>= 16 - info->var.red.length;
> + green >>= 16 - info->var.green.length;
> + blue >>= 16 - info->var.blue.length;
> + transp >>= 16 - info->var.transp.length;
> +
> + palette[regno] = (red << info->var.red.offset) |
> + (green << info->var.green.offset) |
> + (blue << info->var.blue.offset) |
> + (transp << info->var.transp.offset);
> +
> + return 0;
> +}
> +
> +static struct fb_fix_screeninfo ren_vdc4_fix = {
> + .id = "Renesas VDC4FB",
> + .type = FB_TYPE_PACKED_PIXELS,
> + .visual = FB_VISUAL_TRUECOLOR,
> + .accel = FB_ACCEL_NONE,
> +};
> +
> +static struct fb_ops ren_vdc4_ops = {
> + .owner = THIS_MODULE,
^^^^^^^^
Please use tab instead of spaces.
> + .fb_setcolreg = ren_vdc4_setcolreg,
> + .fb_read = fb_sys_read,
> + .fb_write = fb_sys_write,
Same as above.
> + .fb_fillrect = sys_fillrect,
> + .fb_copyarea = sys_copyarea,
> + .fb_imageblit = sys_imageblit,
> +};
> +
> +static int ren_vdc4_set_bpp(struct fb_var_screeninfo *var, int bpp)
> +{
> + switch (bpp) {
> + case 16: /* RGB 565 */
> + var->red.offset = 11;
> + var->red.length = 5;
> + var->green.offset = 5;
> + var->green.length = 6;
> + var->blue.offset = 0;
> + var->blue.length = 5;
> + var->transp.offset = 0;
> + var->transp.length = 0;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + var->bits_per_pixel = bpp;
> + var->red.msb_right = 0;
> + var->green.msb_right = 0;
> + var->blue.msb_right = 0;
> + var->transp.msb_right = 0;
> + return 0;
> +}
> +
> +/* PM Functions */
> +static int ren_vdc4_start(struct ren_vdc4_priv *priv,
> + int clock_source)
> +{
> + int ret;
> +
> + ret = clk_enable(priv->clk);
> + if (ret < 0)
> + return ret;
> +
> + if (priv->dot_clk) {
> + ret = clk_enable(priv->dot_clk);
> + if (ret < 0)
> + return ret;
> + }
> +
> + restart_tft_display(priv, clock_source);
> +
> + return ret;
> +}
> +
> +static void ren_vdc4_stop(struct ren_vdc4_priv *priv)
> +{
> + if (priv->dot_clk)
> + clk_disable(priv->dot_clk);
> + clk_disable(priv->clk);
> +}
> +
> +static int ren_vdc4_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> +
> + ren_vdc4_stop(platform_get_drvdata(pdev));
> + return 0;
> +}
> +
> +static int ren_vdc4_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct ren_vdc4_info *pdata = pdev->dev.platform_data;
> +
> + return ren_vdc4_start(platform_get_drvdata(pdev), pdata->clock_source);
> +}
> +
> +static const struct dev_pm_ops ren_vdc4_dev_pm_ops = {
> + .suspend = ren_vdc4_suspend,
> + .resume = ren_vdc4_resume,
> +};
> +
> +static int ren_vdc4_remove(struct platform_device *pdev);
> +
> +static int __devinit ren_vdc4_probe(struct platform_device *pdev)
> +{
> + struct fb_info *info;
> + struct ren_vdc4_priv *priv;
> + struct ren_vdc4_info *pdata = pdev->dev.platform_data;
> + struct resource *res;
> + void *buf;
> + int irq, error;
> +
> + if (!pdata) {
> + dev_err(&pdev->dev, "no platform data defined\n");
> + return -EINVAL;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + irq = platform_get_irq(pdev, 0);
> + if (!res || irq < 0) {
> + dev_err(&pdev->dev, "cannot get platform resources\n");
> + return -ENOENT;
> + }
> +
> + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
How about using devm_kzalloc() instead of kzalloc()?
It makes the code simpler.
> + if (!priv) {
> + dev_err(&pdev->dev, "cannot allocate device data\n");
> + return -ENOMEM;
> + }
> +
> + platform_set_drvdata(pdev, priv);
> +
> + error = request_irq(irq, ren_vdc4_irq, 0, dev_name(&pdev->dev), priv);
How about using devm_request_irq()instead of request_irq()?
It makes the code simpler.
> + if (error) {
> + dev_err(&pdev->dev, "unable to request irq\n");
> + goto err1;
> + }
> +
> + priv->irq = irq;
> + pdata = pdev->dev.platform_data;
> +
> + priv->cfg = pdata;
> +
> + error = ren_vdc4_setup_clocks(pdev, pdata->clock_source, priv);
> + if (error) {
> + dev_err(&pdev->dev, "unable to setup clocks\n");
> + goto err1;
> + }
> +
> + priv->base = ioremap_nocache(res->start, resource_size(res));
How about using devm_ioremap_nocache() instead of ioremap_nocache()?
It makes the code simpler.
> + if (!priv->base) {
> + dev_err(&pdev->dev, "unable to ioremap\n");
> + goto err1;
> + }
> +
> + priv->info = framebuffer_alloc(0, &pdev->dev);
> + if (!priv->info) {
> + dev_err(&pdev->dev, "unable to allocate fb_info\n");
> + goto err1;
> + }
> +
> + info = priv->info;
> + info->fbops = &ren_vdc4_ops;
> + info->var.xres = info->var.xres_virtual = pdata->lcd_cfg.xres;
> + info->var.yres = info->var.yres_virtual = pdata->lcd_cfg.yres;
> + info->var.width = pdata->panel_width;
> + info->var.height = pdata->panel_height;
> + info->var.activate = FB_ACTIVATE_NOW;
> + info->pseudo_palette = priv->pseudo_palette;
> + error = ren_vdc4_set_bpp(&info->var, pdata->bpp);
> + if (error)
> + goto err1;
> +
> + info->fix = ren_vdc4_fix;
> + info->fix.line_length = pdata->lcd_cfg.xres * (pdata->bpp / 8);
> + info->fix.smem_len = info->fix.line_length * pdata->lcd_cfg.yres;
> +
> + buf = dma_alloc_coherent(&pdev->dev, info->fix.smem_len,
> + &priv->dma_handle, GFP_KERNEL);
> + if (!buf) {
> + dev_err(&pdev->dev, "unable to allocate buffer\n");
> + goto err1;
> + }
> +
> + info->flags = FBINFO_FLAG_DEFAULT;
> +
> + error = fb_alloc_cmap(&info->cmap, PALETTE_NR, 0);
> + if (error < 0) {
> + dev_err(&pdev->dev, "unable to allocate cmap\n");
> + goto err1;
> + }
> +
> + memset(buf, 0, info->fix.smem_len);
> + info->fix.smem_start = priv->dma_handle;
> + info->screen_base = buf;
> + info->device = &pdev->dev;
> + info->par = priv;
> +
> + if (error)
> + goto err1;
> +
> + ren_vdc4_start(priv, pdata->clock_source);
Return value should be checked as follow:
+ error = ren_vdc4_start(priv, pdata->clock_source);
> + if (error) {
> + dev_err(&pdev->dev, "unable to start hardware\n");
> + goto err1;
> + }
> +
> + info = priv->info;
> +
> + error = register_framebuffer(info);
> + if (error < 0)
> + goto err1;
> +
> + dev_info(info->dev,
> + "registered %s as %udx%ud %dbpp.\n",
> + pdev->name,
> + (int) pdata->lcd_cfg.xres,
> + (int) pdata->lcd_cfg.yres,
> + pdata->bpp);
> +
> + return 0;
> +
> +err1:
> + ren_vdc4_remove(pdev);
> + return error;
> +}
> +
> +static int ren_vdc4_remove(struct platform_device *pdev)
> +{
> + struct ren_vdc4_priv *priv = platform_get_drvdata(pdev);
> + struct fb_info *info;
> +
> + if (priv->info->dev)
> + unregister_framebuffer(priv->info);
> +
> + ren_vdc4_stop(priv);
> +
> + info = priv->info;
> +
> + if (!info || !info->device) {
> + dev_err(&pdev->dev, "Failed to dealloc/release fb_info\n");
> + } else {
> + fb_dealloc_cmap(&info->cmap);
> + framebuffer_release(info);
> + }
> +
> + if (priv->dot_clk)
> + clk_put(priv->dot_clk);
> + clk_put(priv->clk);
If devm_clk_get() is used in probe(), this clk_put() is not needed.
> +
> + if (priv->base)
> + iounmap(priv->base);
If devm_ioremap_nocache()is used in probe(), this iounmap() is not
needed.
> +
> + if (priv->irq)
> + free_irq(priv->irq, priv);
If devm_request_irq()is used in probe(), this free_irq() is not needed.
> +
> + kfree(priv);
If devm_kzalloc() is used in probe(), this kfree() is not needed.
> + return 0;
> +}
> +
> +static struct platform_driver ren_vdc4_driver = {
> + .driver = {
> + .name = "ren_vdc4fb",
> + .owner = THIS_MODULE,
> + .pm = &ren_vdc4_dev_pm_ops,
> + },
> + .probe = ren_vdc4_probe,
> + .remove = ren_vdc4_remove,
> +};
> +
> +static int __init ren_vdc4_init(void)
> +{
> + return platform_driver_register(&ren_vdc4_driver);
> +}
> +
> +static void __exit ren_vdc4_exit(void)
> +{
> + platform_driver_unregister(&ren_vdc4_driver);
> +}
> +
> +module_init(ren_vdc4_init);
> +module_exit(ren_vdc4_exit);
> +
> +MODULE_DESCRIPTION("Renesas VDC4 Framebuffer driver");
> +MODULE_AUTHOR("Phil Edworthy <phil.edworthy@renesas.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/video/ren_vdc4fb.h b/include/video/ren_vdc4fb.h
> new file mode 100644
> index 0000000..e91a515
> --- /dev/null
> +++ b/include/video/ren_vdc4fb.h
> @@ -0,0 +1,19 @@
> +#ifndef __REN_VDC4_H__
> +#define __REN_VDC4_H__
> +
> +#include <linux/fb.h>
> +
> +enum { VDC4_EXTCLK = 1, VDC4_PERI_CLK };
> +
> +struct ren_vdc4_info {
> + int bpp;
> + int clock_source;
> + int clock_divider;
> + int hs_pulse_width;
> + int hs_start_pos;
> + struct fb_videomode lcd_cfg;
> + unsigned long panel_width;
> + unsigned long panel_height;
> +};
> +
> +#endif
> --
> 1.7.5.4
>
> --
^ permalink raw reply
* Re: video: exynos_dp: Add device tree based discovery support
From: Jingoo Han @ 2012-09-10 1:31 UTC (permalink / raw)
To: 'Ajay Kumar'
Cc: linux-samsung-soc, linux-fbdev, FlorianSchandinat,
'Jingoo Han'
In-Reply-To: <1346835251-1276-1-git-send-email-~@samsung.com>
On Wednesday, September 05, 2012 5:54 PM Ajay Kumar wrote
>
> From: Ajay Kumar <ajaykumar.rs@samsung.com>
>
> Add device tree match table for Exynos DP
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
Please insert one line between the commit message and Signed-off-by.
Also, add '[PATCH]' to subject as follow:
[PATCH] video: exynos_dp: Add device tree based discovery support
Last, pay attention of your mail address and name.
Um, now, you are using "~@samsung.com" as mail address and name.
Please configure your mail program in order to set "Ajay Kumar"
and "ajaykumar.rs@samsung.com".
If you want to get reviewed, please keep the basic rules
as I mentioned above.
Best regards,
Jingoo Han
> ---
> drivers/video/exynos/exynos_dp_core.c | 10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index c6c016a..3bccd6b 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -18,6 +18,7 @@
> #include <linux/io.h>
> #include <linux/interrupt.h>
> #include <linux/delay.h>
> +#include <linux/of.h>
>
> #include <video/exynos_dp.h>
>
> @@ -1019,6 +1020,14 @@ static const struct dev_pm_ops exynos_dp_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(exynos_dp_suspend, exynos_dp_resume)
> };
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id exynos_dp_match[] = {
> + { .compatible = "samsung,exynos5-dp" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, exynos_dp_match);
> +#endif
> +
> static struct platform_driver exynos_dp_driver = {
> .probe = exynos_dp_probe,
> .remove = __devexit_p(exynos_dp_remove),
> @@ -1026,6 +1035,7 @@ static struct platform_driver exynos_dp_driver = {
> .name = "exynos-dp",
> .owner = THIS_MODULE,
> .pm = &exynos_dp_pm_ops,
> + .of_match_table = of_match_ptr(exynos_dp_match),
> },
> };
>
> --
> 1.7.0.4
^ permalink raw reply
* [PATCH] fbdev: jz4740: Use devm_request_and_ioremap
From: Lars-Peter Clausen @ 2012-09-09 15:38 UTC (permalink / raw)
To: Florian Tobias Schandinat
Cc: Damien Cassou, linux-fbdev, linux-kernel, Lars-Peter Clausen
Use devm_request_and_ioremap instead of request_mem_region + devm_ioremap.
This also fixes the following compile error introduced in commit b2ca7f4d
("drivers/video/jz4740_fb.c: use devm_ functions"):
drivers/video/jz4740_fb.c: In function 'jzfb_probe':
drivers/video/jz4740_fb.c:676:2: error: implicit declaration of function 'devm_ioremap'
drivers/video/jz4740_fb.c:676:13: warning: assignment makes pointer from integer without a cast
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
drivers/video/jz4740_fb.c | 24 +++---------------------
1 files changed, 3 insertions(+), 21 deletions(-)
diff --git a/drivers/video/jz4740_fb.c b/drivers/video/jz4740_fb.c
index 7669770..b0df279 100644
--- a/drivers/video/jz4740_fb.c
+++ b/drivers/video/jz4740_fb.c
@@ -632,23 +632,10 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
return -ENXIO;
}
- mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!mem) {
- dev_err(&pdev->dev, "Failed to get register memory resource\n");
- return -ENXIO;
- }
-
- mem = request_mem_region(mem->start, resource_size(mem), pdev->name);
- if (!mem) {
- dev_err(&pdev->dev, "Failed to request register memory region\n");
- return -EBUSY;
- }
-
fb = framebuffer_alloc(sizeof(struct jzfb), &pdev->dev);
if (!fb) {
dev_err(&pdev->dev, "Failed to allocate framebuffer device\n");
- ret = -ENOMEM;
- goto err_release_mem_region;
+ return -ENOMEM;
}
fb->fbops = &jzfb_ops;
@@ -657,7 +644,6 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
jzfb = fb->par;
jzfb->pdev = pdev;
jzfb->pdata = pdata;
- jzfb->mem = mem;
jzfb->ldclk = devm_clk_get(&pdev->dev, "lcd");
if (IS_ERR(jzfb->ldclk)) {
@@ -673,9 +659,9 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
goto err_framebuffer_release;
}
- jzfb->base = devm_ioremap(&pdev->dev, mem->start, resource_size(mem));
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ jzfb->base = devm_request_and_ioremap(&pdev->dev, mem);
if (!jzfb->base) {
- dev_err(&pdev->dev, "Failed to ioremap register memory region\n");
ret = -EBUSY;
goto err_framebuffer_release;
}
@@ -736,8 +722,6 @@ err_free_devmem:
jzfb_free_devmem(jzfb);
err_framebuffer_release:
framebuffer_release(fb);
-err_release_mem_region:
- release_mem_region(mem->start, resource_size(mem));
return ret;
}
@@ -750,8 +734,6 @@ static int __devexit jzfb_remove(struct platform_device *pdev)
jz_gpio_bulk_free(jz_lcd_ctrl_pins, jzfb_num_ctrl_pins(jzfb));
jz_gpio_bulk_free(jz_lcd_data_pins, jzfb_num_data_pins(jzfb));
- release_mem_region(jzfb->mem->start, resource_size(jzfb->mem));
-
fb_dealloc_cmap(&jzfb->fb->cmap);
jzfb_free_devmem(jzfb);
--
1.7.2.5
^ permalink raw reply related
* [RFC PATCH v2] OMAPDSS: Fix IRQ unregister race
From: Dimitar Dimitrov @ 2012-09-08 15:05 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Dimitar Dimitrov
In-Reply-To: <201209071742.11735.dinuxbg@gmail.com>
Very rare kernel crashes are reported on a custom OMAP4 board. Kernel
panics due to corrupted completion structure while executing
dispc_irq_wait_handler(). Excerpt from kernel log:
Internal error: Oops - undefined instruction: 0 [#1] PREEMPT SMP
Unable to handle kernel paging request at virtual address 00400130
...
PC is at 0xebf205bc
LR is at __wake_up_common+0x54/0x94
...
(__wake_up_common+0x0/0x94)
(complete+0x0/0x60)
(dispc_irq_wait_handler.36902+0x0/0x14)
(omap_dispc_irq_handler+0x0/0x354)
(handle_irq_event_percpu+0x0/0x188)
(handle_irq_event+0x0/0x64)
(handle_fasteoi_irq+0x0/0x10c)
(generic_handle_irq+0x0/0x48)
(asm_do_IRQ+0x0/0xc0)
DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
unregister_isr() and DISPC IRQ might be running in parallel on different
CPUs. So there is a chance that a callback is executed even though it
has been unregistered. As omap_dispc_wait_for_irq_timeout() declares a
completion on stack, the dispc_irq_wait_handler() callback might try to
access a completion structure that is invalid. This leads to crashes and
hangs.
Solution is to divide unregister calls into two sets:
1. Non-strict unregistering of callbacks. Callbacks could safely be
executed after unregistering them. This is the case with unregister
calls from the IRQ handler itself.
2. Strict (synchronized) unregistering. Callbacks are not allowed
after unregistering. This is the case with completion waiting.
The above solution should satisfy one of the original intentions of the
driver: callbacks should be able to unregister themselves.
Also fix DSI IRQ unregister which has similar logic to DISPC IRQ handling.
Signed-off-by: Dimitar Dimitrov <dinuxbg@gmail.com>
---
WARNING: This bug is quite old. The patch has been tested on v3.0. No testing
has been done after rebasing to v3.6. Hence the RFC tag. Hopefully someone
will beat me in testing with latest linux-omap/master.
Changes since v1 per Tomi Valkeinen's comments:
- Don't rename omap_dispc_unregister_isr, just introduce nosync variant.
- Apply the same fix for DSI IRQ which suffers from the same race condition.
drivers/staging/omapdrm/omap_plane.c | 2 +-
drivers/video/omap2/dss/apply.c | 2 +-
drivers/video/omap2/dss/dispc.c | 45 +++++++++++++++++++++++++++++-----
drivers/video/omap2/dss/dsi.c | 19 ++++++++++++++
include/video/omapdss.h | 1 +
5 files changed, 61 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/omapdrm/omap_plane.c b/drivers/staging/omapdrm/omap_plane.c
index 7997be7..8d8aa5b 100644
--- a/drivers/staging/omapdrm/omap_plane.c
+++ b/drivers/staging/omapdrm/omap_plane.c
@@ -82,7 +82,7 @@ static void dispc_isr(void *arg, uint32_t mask)
struct omap_plane *omap_plane = to_omap_plane(plane);
struct omap_drm_private *priv = plane->dev->dev_private;
- omap_dispc_unregister_isr(dispc_isr, plane,
+ omap_dispc_unregister_isr_nosync(dispc_isr, plane,
id2irq[omap_plane->ovl->id]);
queue_work(priv->wq, &omap_plane->work);
diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index 0fefc68..9386834 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -847,7 +847,7 @@ static void dss_unregister_vsync_isr(void)
for (i = 0; i < num_mgrs; ++i)
mask |= dispc_mgr_get_framedone_irq(i);
- r = omap_dispc_unregister_isr(dss_apply_irq_handler, NULL, mask);
+ r = omap_dispc_unregister_isr_nosync(dss_apply_irq_handler, NULL, mask);
WARN_ON(r);
dss_data.irq_enabled = false;
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index ee9e296..a67d92c 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -2421,8 +2421,8 @@ static void dispc_mgr_enable_digit_out(bool enable)
enable ? "start" : "stop");
}
- r = omap_dispc_unregister_isr(dispc_disable_isr, &frame_done_completion,
- irq_mask);
+ r = omap_dispc_unregister_isr(dispc_disable_isr,
+ &frame_done_completion, irq_mask);
if (r)
DSSERR("failed to unregister %x isr\n", irq_mask);
@@ -3320,7 +3320,8 @@ err:
}
EXPORT_SYMBOL(omap_dispc_register_isr);
-int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
+/* WARNING: callback might be executed even after this function returns! */
+int omap_dispc_unregister_isr_nosync(omap_dispc_isr_t isr, void *arg, u32 mask)
{
int i;
unsigned long flags;
@@ -3352,7 +3353,37 @@ int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
return ret;
}
-EXPORT_SYMBOL(omap_dispc_unregister_isr);
+EXPORT_SYMBOL(omap_dispc_unregister_isr_nosync);
+
+/*
+ * Ensure that callback <isr> will NOT be executed after this function
+ * returns. Must be called from sleepable context, though!
+ */
+int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
+{
+ int ret;
+
+ ret = omap_dispc_unregister_isr_nosync(isr, arg, mask);
+
+ /* Task context is not really needed. But if we're called from atomic
+ * context, it is probably from DISPC IRQ, where we will deadlock.
+ * So use might_sleep() to catch potential deadlocks.
+ */
+ might_sleep();
+
+#if defined(CONFIG_SMP)
+ /* DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
+ * unregister_isr() and DISPC IRQ might be running in parallel on
+ * different CPUs. So there is a chance that a callback is executed
+ * even though it has been unregistered. Add a barrier, in order to
+ * ensure that after returning from this function, the new DISPC IRQ
+ * will use an updated callback array, and NOT its cached copy.
+ */
+ synchronize_irq(dispc.irq);
+#endif
+
+ return ret;
+}
#ifdef DEBUG
static void print_irq_status(u32 status)
@@ -3567,7 +3598,8 @@ int omap_dispc_wait_for_irq_timeout(u32 irqmask, unsigned long timeout)
timeout = wait_for_completion_timeout(&completion, timeout);
- omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion, irqmask);
+ omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion,
+ irqmask);
if (timeout = 0)
return -ETIMEDOUT;
@@ -3598,7 +3630,8 @@ int omap_dispc_wait_for_irq_interruptible_timeout(u32 irqmask,
timeout = wait_for_completion_interruptible_timeout(&completion,
timeout);
- omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion, irqmask);
+ omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion,
+ irqmask);
if (timeout = 0)
return -ETIMEDOUT;
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index b07e886..24b4a3e 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -960,6 +960,13 @@ static int dsi_unregister_isr(struct platform_device *dsidev,
spin_unlock_irqrestore(&dsi->irq_lock, flags);
+ might_sleep();
+
+#if defined(CONFIG_SMP)
+ /* See notes for dispc.c:omap_dispc_unregister_isr() . */
+ synchronize_irq(dsi->irq);
+#endif
+
return r;
}
@@ -1002,6 +1009,12 @@ static int dsi_unregister_isr_vc(struct platform_device *dsidev, int channel,
spin_unlock_irqrestore(&dsi->irq_lock, flags);
+ might_sleep();
+
+#if defined(CONFIG_SMP)
+ /* See notes for dispc.c:omap_dispc_unregister_isr() . */
+ synchronize_irq(dsi->irq);
+#endif
return r;
}
@@ -1042,6 +1055,12 @@ static int dsi_unregister_isr_cio(struct platform_device *dsidev,
spin_unlock_irqrestore(&dsi->irq_lock, flags);
+ might_sleep();
+
+#if defined(CONFIG_SMP)
+ /* See notes for dispc.c:omap_dispc_unregister_isr() . */
+ synchronize_irq(dsi->irq);
+#endif
return r;
}
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index a6267a2..769f981 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -708,6 +708,7 @@ void omapdss_default_get_timings(struct omap_dss_device *dssdev,
typedef void (*omap_dispc_isr_t) (void *arg, u32 mask);
int omap_dispc_register_isr(omap_dispc_isr_t isr, void *arg, u32 mask);
int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask);
+int omap_dispc_unregister_isr_nosync(omap_dispc_isr_t isr, void *arg, u32 mask);
int omap_dispc_wait_for_irq_timeout(u32 irqmask, unsigned long timeout);
int omap_dispc_wait_for_irq_interruptible_timeout(u32 irqmask,
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH 0/8] OMAPDSS: Misc improvements
From: Tomi Valkeinen @ 2012-09-07 16:59 UTC (permalink / raw)
To: Tony Lindgren; +Cc: linux-omap, linux-fbdev, archit
In-Reply-To: <20120907162258.GT1303@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 1315 bytes --]
On Fri, 2012-09-07 at 09:22 -0700, Tony Lindgren wrote:
> * Tomi Valkeinen <tomi.valkeinen@ti.com> [120907 03:16]:
> > On Thu, 2012-09-06 at 13:13 -0700, Tony Lindgren wrote:
> > > * Tomi Valkeinen <tomi.valkeinen@ti.com> [120904 00:23]:
> > > > Hi Tony,
> > > >
> > > > Can you check the arch/arm patches below, and suggest how you'd like to
> > > > go forward with them?
> > >
> > > Acked them, then as soon as we have the initial immutable header
> > > move branch available, you should merge with that to avoid
> > > merge conflicts in upstream.
> >
> > Thanks, but you missed the first patch "[PATCH 1/8] OMAPDSS: HDMI: Move
> > GPIO handling to HDMI driver".
>
> Oops sorry, that's a nice one, acked that too. BTW, do you have any
> platform code callbacks remaining for DSS?
Yes, for quite many boards:
$ git grep platform_enable arch/arm/mach-omap2/|wc -l
17
Some of those are trivial, I just need to move the panel's reset gpio
handling to the panel driver. But some fiddle around with board specific
gpios that do not belong to the panel driver. Also some board files
contain backlight handling.
Those are for panel drivers, then there's also callbacks for the omapdss
driver itself. get_context_loss_count, set_min_bus_tput, and dsi pin
enable/disable.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v5 1/4] Runtime Interpreted Power Sequences
From: Stephen Warren @ 2012-09-07 16:36 UTC (permalink / raw)
To: Heiko Stübner
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Stephen Warren, linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mark Brown,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
Anton Vorontsov,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
David Woodhouse,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <201209071108.42083.heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
On 09/07/2012 03:08 AM, Heiko Stübner wrote:
> Am Freitag, 7. September 2012, 10:04:24 schrieb Alex Courbot:
>>> For your power_seq_run function you write that it simply returns an error
>>> code on failure and looking through it I also just found the error return
>>> statement. This would leave a device half turned on.
>>>
>>> So I'm wondering, if it shouldn't turn off all the things it turned on
>>> until the step that produced the error. All your possible step types
>>> (execpt the delay) are booleans, so it should be possible to simply
>>> negate them when backtracking through the previous steps.
>>
>> Indeed, I think you raised an important point. Right now all step types are
>> invertible, but we cannot rely on that statement to be true forever. For
>> instance, one short-term improvement will be to allow finer regulator
>> control, like voltage setting. In this case, how can we go back to the
>> initial state without recording it?
>>
>> If e.g. the power on sequence fails at step N (of M steps for that
>> sequence), one could try playing the corresponding power off sequence
>> (either completely of from step M - N), but then again we cannot rely on
>> sequences to be perfectly symetrical. Maybe this is more something for the
>> calling driver to check for and control?
>
> Am Freitag, 7. September 2012, 10:15:03 schrieb Mark Brown:
>> On Fri, Sep 07, 2012 at 05:04:24PM +0900, Alex Courbot wrote:
>>> If e.g. the power on sequence fails at step N (of M steps for that
>>> sequence), one could try playing the corresponding power off sequence
>>> (either completely of from step M - N), but then again we cannot rely on
>>> sequences to be perfectly symetrical. Maybe this is more something for
>>> the calling driver to check for and control?
>>
>> That had been my thought too - depending on what the sequence is for it
>> may be that the corrective action is something very different to
>> reversing the sequence, for example a device reset may be required.
>
> If I understood the description correctly, the power sequence should be
> transparent to the driver, as it implements board specific actions and
> shouldn't bother the driver with it to much.
Well, the contents/implementation of the sequence should be transparent
to the driver. The fact that a sequence exists and needs to be executed
obviously can't be transparent to the driver, since the driver needs to
call an API to execute the sequence.
I'd assert that requiring the driver to get back to a sane state by
executing sequence (b) if sequence (a) fails is fairly reasonable, and
doesn't give the driver any more knowledge of what the sequences are
than what it already has.
But then I start to wonder: What if the "help something went wrong"
sequence gets an error...
> Therefore my thoughts went along
> the lines how gpio_request_array handles this, always producing a sane state
> at the end.
>
> Recording the previous state, could be done by making a copy of the current
> sequence, and just noting the previous values (including voltages etc) in the
> respective entries. And in the error case running this new sequence from the
> error point instead to power down again.
>
>
> As both Alex and Mark wrote, reversing the sequence might be the action of
> choice only for some devices, but others might need to run a completely
> different powerdown sequence and still others would need special handling.
>
> Would it be possible to encode this in the sequence definition, something like
> on-error = "reverse"
>
> on-error = "sequence"
> error-seq = <&other_sequence>
>
> on-error = "driver"
> with better names and types of course.
>
> This would keep the power sequence transparent to most drivers and only the
> real esoteric ones would need to do their special handling on their own.
Yes, something like that sounds reasonable on the surface. I'm not sure
about the on-error="driver" case though; if the driver knows nothing
about the content of the sequences, I'm not sure how the driver could
possibly do anything other than execute some sequence to recover.
^ permalink raw reply
* Re: [PATCH 0/8] OMAPDSS: Misc improvements
From: Tony Lindgren @ 2012-09-07 16:22 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, archit
In-Reply-To: <1347012960.2646.5.camel@deskari>
* Tomi Valkeinen <tomi.valkeinen@ti.com> [120907 03:16]:
> On Thu, 2012-09-06 at 13:13 -0700, Tony Lindgren wrote:
> > * Tomi Valkeinen <tomi.valkeinen@ti.com> [120904 00:23]:
> > > Hi Tony,
> > >
> > > Can you check the arch/arm patches below, and suggest how you'd like to
> > > go forward with them?
> >
> > Acked them, then as soon as we have the initial immutable header
> > move branch available, you should merge with that to avoid
> > merge conflicts in upstream.
>
> Thanks, but you missed the first patch "[PATCH 1/8] OMAPDSS: HDMI: Move
> GPIO handling to HDMI driver".
Oops sorry, that's a nice one, acked that too. BTW, do you have any
platform code callbacks remaining for DSS?
Regards,
Tony
^ permalink raw reply
* Re: [PATCH 1/8] OMAPDSS: HDMI: Move GPIO handling to HDMI driver
From: Tony Lindgren @ 2012-09-07 16:21 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: archit, linux-omap, linux-fbdev
In-Reply-To: <1345729514-2441-2-git-send-email-tomi.valkeinen@ti.com>
* Tomi Valkeinen <tomi.valkeinen@ti.com> [120823 06:46]:
> We currently manage HDMI GPIOs in the board files via
> platform_enable/disable calls. This won't work with device tree, and in
> any case the correct place to manage the GPIOs is in the HDMI driver.
>
> This patch moves the handling of the GPIOs to the HDMI driver. The GPIO
> handling is moved to the common hdmi.c file, and this probably needs to
> be revisited when adding OMAP5 HDMI support to see if the GPIO handling
> needs to be moved to IP specific files.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
This too looks safe to merge via fb tree:
Acked-by: Tony Lindgren <tony@atomide.com>
^ permalink raw reply
* Re: [RFC PATCH] OMAPDSS: DISPC: Fix IRQ unregister race
From: Dimitar Dimitrov @ 2012-09-07 14:42 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1346938614.2737.68.camel@deskari>
Hi,
On Thursday 06 September 2012 16:36:54 Tomi Valkeinen wrote:
> Hi,
>
> On Sun, 2012-09-02 at 22:12 +0300, Dimitar Dimitrov wrote:
> > Very rare kernel crashes are reported on a custom OMAP4 board. Kernel
> > panics due to corrupted completion structure while executing
> >
> > dispc_irq_wait_handler(). Excerpt from kernel log:
> > Internal error: Oops - undefined instruction: 0 [#1] PREEMPT SMP
> > Unable to handle kernel paging request at virtual address 00400130
> > ...
> > PC is at 0xebf205bc
> > LR is at __wake_up_common+0x54/0x94
> > ...
> > (__wake_up_common+0x0/0x94)
> > (complete+0x0/0x60)
> > (dispc_irq_wait_handler.36902+0x0/0x14)
> > (omap_dispc_irq_handler+0x0/0x354)
> > (handle_irq_event_percpu+0x0/0x188)
> > (handle_irq_event+0x0/0x64)
> > (handle_fasteoi_irq+0x0/0x10c)
> > (generic_handle_irq+0x0/0x48)
> > (asm_do_IRQ+0x0/0xc0)
> >
> > DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
> > unregister_isr() and DISPC IRQ might be running in parallel on different
> > CPUs. So there is a chance that a callback is executed even though it
> > has been unregistered. As omap_dispc_wait_for_irq_timeout() declares a
> > completion on stack, the dispc_irq_wait_handler() callback might try to
> > access a completion structure that is invalid. This leads to crashes and
> > hangs.
> >
> > Solution is to divide unregister calls into two sets:
> > 1. Non-strict unregistering of callbacks. Callbacks could safely be
> >
> > executed after unregistering them. This is the case with unregister
> > calls from the IRQ handler itself.
> >
> > 2. Strict (synchronized) unregistering. Callbacks are not allowed
> >
> > after unregistering. This is the case with completion waiting.
> >
> > The above solution should satisfy one of the original intentions of the
> > driver: callbacks should be able to unregister themselves.
>
> I think it'd be better to create a new function for the nosync version,
> and keep the old name for the sync version. The reason for this is to
> minimize the amount of changes, as I think this one needs to be applied
> to stable kernel trees also.
My intention was to force all callers to pick sides. In case of rebase issues
we get link errors instead of rare and subtle run-time races. Still, if you
think we should leave the old name untouched then I'll change my
patch.
>
> Also, I think we need similar one for dsi.c, as it has the same kind of
> irq handling. But with a quick glance only sync version is needed there.
Thanks, I missed that. I'll try to fix and send again.
>
> However, I'm not quite sure about this approach. The fix makes sense,
> but it makes me think if the irq handling is designed the wrong way.
>
> While debugging and fixing this, did you think some other irq handling
> approach would be saner?
I tried but could not come up with better approach. The main difficulty is
that there are two contradicting requirements:
1. Some callbacks unregister other callbacks, including themselves, from
DISPC IRQ context.
2. Some functions expect that once a callback is unregistered it is never
executed.
Hence it is natural to split callers into two sets. I'm open for suggestions.
>
> Tomi
Thanks,
Dimitar
^ permalink raw reply
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