* [PATCH 1/5] irq: add declaration of irq_domain_simple_ops to irqdomain.h
From: Rob Herring @ 2011-09-14 16:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316017900-19918-1-git-send-email-robherring2@gmail.com>
From: Rob Herring <rob.herring@calxeda.com>
irq_domain_simple_ops is exported, but is not declared in irqdomain.h, so
add it.
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
include/linux/irqdomain.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index e807ad6..3ad553e 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -80,6 +80,7 @@ extern void irq_domain_del(struct irq_domain *domain);
#endif /* CONFIG_IRQ_DOMAIN */
#if defined(CONFIG_IRQ_DOMAIN) && defined(CONFIG_OF_IRQ)
+extern struct irq_domain_ops irq_domain_simple_ops;
extern void irq_domain_add_simple(struct device_node *controller, int irq_base);
extern void irq_domain_generate_simple(const struct of_device_id *match,
u64 phys_base, unsigned int irq_start);
--
1.7.5.4
^ permalink raw reply related
* [PATCH 2/5] irq: fix existing domain check in irq_domain_add
From: Rob Herring @ 2011-09-14 16:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316017900-19918-1-git-send-email-robherring2@gmail.com>
From: Rob Herring <rob.herring@calxeda.com>
irq_data will normally exist, so the domain was prevented from being set.
The simple domain code did not hit this as nr_irq is always 0.
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
kernel/irq/irqdomain.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index d5828da..84f4110 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -29,7 +29,7 @@ void irq_domain_add(struct irq_domain *domain)
*/
for (hwirq = 0; hwirq < domain->nr_irq; hwirq++) {
d = irq_get_irq_data(irq_domain_to_irq(domain, hwirq));
- if (d || d->domain) {
+ if (d && d->domain) {
/* things are broken; just report, don't clean up */
WARN(1, "error: irq_desc already assigned to a domain");
return;
--
1.7.5.4
^ permalink raw reply related
* [PATCH 3/5] of/irq: introduce of_irq_init
From: Rob Herring @ 2011-09-14 16:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316017900-19918-1-git-send-email-robherring2@gmail.com>
From: Rob Herring <rob.herring@calxeda.com>
of_irq_init will scan the devicetree for matching interrupt controller
nodes. Then it calls an initialization function for each found controller
in the proper order with parent nodes initialized before child nodes.
Based on initial pseudo code from Grant Likely.
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
---
drivers/of/irq.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/of_irq.h | 1 +
2 files changed, 97 insertions(+), 0 deletions(-)
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 9f689f1..a0cd7e8 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -19,10 +19,13 @@
*/
#include <linux/errno.h>
+#include <linux/list.h>
+#include <linux/list_sort.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_irq.h>
#include <linux/string.h>
+#include <linux/slab.h>
/* For archs that don't support NO_IRQ (such as x86), provide a dummy value */
#ifndef NO_IRQ
@@ -386,3 +389,96 @@ int of_irq_to_resource_table(struct device_node *dev, struct resource *res,
return i;
}
+
+struct intc_desc {
+ struct list_head list;
+ struct device_node *dev;
+ struct device_node *parent;
+};
+
+typedef void (*irq_init_cb_t)(struct device_node *, struct device_node *);
+
+static int __init irq_cmp_intc_desc(void *unused, struct list_head *a,
+ struct list_head *b)
+{
+ const struct intc_desc *da = list_entry(a, typeof(*da), list);
+ const struct intc_desc *db = list_entry(b, typeof(*db), list);
+
+ /* same parent, so order doesn't matter */
+ if (da->parent == db->parent)
+ return 0;
+
+ /* NULL parent comes first */
+ if (!da->parent && db->parent)
+ return -1;
+ if (!db->parent && da->parent)
+ return 1;
+
+ /* parent node must be before child node */
+ if (da->dev == db->parent)
+ return -1;
+ if (db->dev == da->parent)
+ return 1;
+
+ return 0;
+}
+
+/**
+ * of_irq_init - Scan the device tree for matching interrupt controllers and
+ * call their initialization functions in order with parents first.
+ * @matches: 0 terminated array of nodes to match and initialization function
+ * to call on match
+ */
+void __init of_irq_init(const struct of_device_id *matches)
+{
+ struct device_node *np;
+ const struct of_device_id *match;
+ struct intc_desc *desc;
+ struct intc_desc *temp_desc;
+ struct list_head intc_desc_list;
+
+ INIT_LIST_HEAD(&intc_desc_list);
+
+ for_each_matching_node(np, matches) {
+ if (!of_find_property(np, "interrupt-controller", NULL))
+ continue;
+ /* Here, we allocate and populate an intc_desc with the node
+ * pointer, interrupt-parent device_node etc. */
+ desc = kzalloc(sizeof(*desc), GFP_KERNEL);
+ if (!desc) {
+ WARN_ON(1);
+ goto err;
+ }
+ desc->dev = np;
+ desc->parent = of_irq_find_parent(np);
+ list_add(&desc->list, &intc_desc_list);
+ }
+ if (list_empty(&intc_desc_list))
+ return;
+
+ /*
+ * The root irq controller is the one without an interrupt-parent.
+ * That one goes first, followed by the controllers that reference it,
+ * followed by the ones that reference the 2nd level controllers, etc
+ */
+ list_sort(NULL, &intc_desc_list, irq_cmp_intc_desc);
+
+ list_for_each_entry_safe(desc, temp_desc, &intc_desc_list, list) {
+ match = of_match_node(matches, desc->dev);
+ if (match && match->data) {
+ irq_init_cb_t irq_init_cb = match->data;
+ pr_debug("of_irq_init: init %s @ %p, parent %p\n",
+ match->compatible, desc->dev, desc->parent);
+ irq_init_cb(desc->dev, desc->parent);
+ }
+ list_del(&desc->list);
+ kfree(desc);
+ }
+ return;
+
+err:
+ list_for_each_entry_safe(desc, temp_desc, &intc_desc_list, list) {
+ list_del(&desc->list);
+ kfree(desc);
+ }
+}
diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
index cd2e61c..032d76c 100644
--- a/include/linux/of_irq.h
+++ b/include/linux/of_irq.h
@@ -73,6 +73,7 @@ extern int of_irq_to_resource_table(struct device_node *dev,
struct resource *res, int nr_irqs);
extern struct device_node *of_irq_find_parent(struct device_node *child);
+extern void of_irq_init(const struct of_device_id *matches);
#endif /* CONFIG_OF_IRQ */
#endif /* CONFIG_OF */
--
1.7.5.4
^ permalink raw reply related
* [PATCH 4/5] ARM: gic: allow irq_start to be 0
From: Rob Herring @ 2011-09-14 16:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316017900-19918-1-git-send-email-robherring2@gmail.com>
From: Rob Herring <rob.herring@calxeda.com>
There's really no need to set irq_start per platform for the primary gic.
The SGIs and PPIs are not handled as normal irqs, so how irqs 0-31 are
setup doesn't really matter. So allow irq_start to be set to 0 to match
the linux irq numbering.
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
arch/arm/common/gic.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 666b278..d1ccc72 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -359,7 +359,7 @@ void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
gic = &gic_data[gic_nr];
gic->dist_base = dist_base;
gic->cpu_base = cpu_base;
- gic->irq_offset = (irq_start - 1) & ~31;
+ gic->irq_offset = irq_start ? (irq_start - 1) & ~31 : 0;
if (gic_nr == 0)
gic_cpu_base_addr = cpu_base;
--
1.7.5.4
^ permalink raw reply related
* [PATCH 5/5] ARM: gic: add OF based initialization
From: Rob Herring @ 2011-09-14 16:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316017900-19918-1-git-send-email-robherring2@gmail.com>
From: Rob Herring <rob.herring@calxeda.com>
This adds gic initialization using device tree data. The initialization
functions are intended to be called by a generic OF interrupt
controller parsing function once the right pieces are in place.
PPIs are handled using 3rd cell of interrupts properties to specify the cpu
mask the PPI is assigned to.
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
Documentation/devicetree/bindings/arm/gic.txt | 53 ++++++++++++++++++++++++
arch/arm/common/gic.c | 55 +++++++++++++++++++++++--
arch/arm/include/asm/hardware/gic.h | 10 +++++
3 files changed, 114 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
new file mode 100644
index 0000000..6c513de
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/gic.txt
@@ -0,0 +1,53 @@
+* ARM Generic Interrupt Controller
+
+ARM SMP cores are often associated with a GIC, providing per processor
+interrupts (PPI), shared processor interrupts (SPI) and software
+generated interrupts (SGI).
+
+Primary GIC is attached directly to the CPU and typically has PPIs and SGIs.
+Secondary GICs are cascaded into the upward interrupt controller and do not
+have PPIs or SGIs.
+
+Main node required properties:
+
+- compatible : should be one of:
+ "arm,cortex-a9-gic"
+ "arm,arm11mp-gic"
+- interrupt-controller : Identifies the node as an interrupt controller
+- #interrupt-cells : Specifies the number of cells needed to encode an
+ interrupt source. The type shall be a <u32> and the value shall be 3.
+
+ The 1st cell is the interrupt number. 0-15 are reserved for SGIs. 16-31 are
+ for PPIs.
+
+ The 2nd cell is the level-sense information, encoded as follows:
+ 1 = low-to-high edge triggered
+ 2 = high-to-low edge triggered
+ 4 = active high level-sensitive
+ 8 = active low level-sensitive
+
+ Only values of 1 and 4 are valid for GIC 1.0 spec.
+
+ The 3rd cell contains the mask of the cpu number for the interrupt source.
+ The cpu mask is only valid for PPIs and shall be 0 for SPIs. This value shall
+ be 0 for PPIs.
+
+- reg : Specifies base physical address(s) and size of the GIC registers. The
+ first 2 values are the GIC distributor register base and size. The 2nd 2
+ values are the GIC cpu interface register base and size.
+
+Optional
+- interrupts : Interrupt source of the parent interrupt controller. Only
+ present on secondary GICs.
+
+Example:
+
+ intc: interrupt-controller at fff11000 {
+ compatible = "arm,cortex-a9-gic";
+ #interrupt-cells = <3>;
+ #address-cells = <1>;
+ interrupt-controller;
+ reg = <0xfff11000 0x1000>,
+ <0xfff10100 0x100>;
+ };
+
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index d1ccc72..14de380 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -28,6 +28,10 @@
#include <linux/smp.h>
#include <linux/cpumask.h>
#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/irqdomain.h>
#include <asm/irq.h>
#include <asm/mach/irq.h>
@@ -255,6 +259,15 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
irq_set_chained_handler(irq, gic_handle_cascade_irq);
}
+static int gic_irq_count(void __iomem *dist_base)
+{
+ int gic_irqs = readl_relaxed(dist_base + GIC_DIST_CTR) & 0x1f;
+ gic_irqs = (gic_irqs + 1) * 32;
+ if (gic_irqs > 1020)
+ gic_irqs = 1020;
+ return gic_irqs;
+}
+
static void __init gic_dist_init(struct gic_chip_data *gic,
unsigned int irq_start)
{
@@ -277,10 +290,7 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
* Find out how many interrupts are supported.
* The GIC only supports up to 1020 interrupt sources.
*/
- gic_irqs = readl_relaxed(base + GIC_DIST_CTR) & 0x1f;
- gic_irqs = (gic_irqs + 1) * 32;
- if (gic_irqs > 1020)
- gic_irqs = 1020;
+ gic_irqs = gic_irq_count(base);
/*
* Set all global interrupts to be level triggered, active low.
@@ -405,3 +415,40 @@ void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
writel_relaxed(map << 16 | irq, gic_data[0].dist_base + GIC_DIST_SOFTINT);
}
#endif
+
+#ifdef CONFIG_OF
+static int gic_cnt __initdata = 0;
+
+extern struct irq_domain_ops irq_domain_simple_ops;
+
+void __init gic_of_init(struct device_node *node, struct device_node *parent)
+{
+ void __iomem *cpu_base;
+ void __iomem *dist_base;
+ int irq;
+ struct irq_domain *domain = &gic_data[gic_cnt].domain;
+
+ if (WARN_ON(!node))
+ return;
+
+ dist_base = of_iomap(node, 0);
+ WARN(!dist_base, "unable to map gic dist registers\n");
+
+ cpu_base = of_iomap(node, 1);
+ WARN(!cpu_base, "unable to map gic cpu registers\n");
+
+ domain->nr_irq = gic_irq_count(dist_base);
+ domain->irq_base = irq_alloc_descs(-1, 0, domain->nr_irq, numa_node_id());
+ domain->of_node = of_node_get(node);
+ domain->ops = &irq_domain_simple_ops;
+ irq_domain_add(domain);
+
+ gic_init(gic_cnt, domain->irq_base, dist_base, cpu_base);
+
+ if (parent) {
+ irq = irq_of_parse_and_map(node, 0);
+ gic_cascade_irq(gic_cnt, irq);
+ }
+ gic_cnt++;
+}
+#endif
diff --git a/arch/arm/include/asm/hardware/gic.h b/arch/arm/include/asm/hardware/gic.h
index 435d3f8..ea1ca08 100644
--- a/arch/arm/include/asm/hardware/gic.h
+++ b/arch/arm/include/asm/hardware/gic.h
@@ -33,10 +33,19 @@
#define GIC_DIST_SOFTINT 0xf00
#ifndef __ASSEMBLY__
+#include <linux/irqdomain.h>
+
extern void __iomem *gic_cpu_base_addr;
extern struct irq_chip gic_arch_extn;
void gic_init(unsigned int, unsigned int, void __iomem *, void __iomem *);
+#ifdef CONFIG_OF
+void gic_of_init(struct device_node *node, struct device_node *parent);
+#else
+static inline void gic_of_init(struct device_node *node,
+ struct device_node *parent)
+{}
+#endif
void gic_secondary_init(unsigned int);
void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
void gic_raise_softirq(const struct cpumask *mask, unsigned int irq);
@@ -46,6 +55,7 @@ struct gic_chip_data {
unsigned int irq_offset;
void __iomem *dist_base;
void __iomem *cpu_base;
+ struct irq_domain domain;
};
#endif
--
1.7.5.4
^ permalink raw reply related
* [PATCH 2/5] irq: fix existing domain check in irq_domain_add
From: Thomas Gleixner @ 2011-09-14 16:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316017900-19918-3-git-send-email-robherring2@gmail.com>
On Wed, 14 Sep 2011, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> irq_data will normally exist, so the domain was prevented from being set.
> The simple domain code did not hit this as nr_irq is always 0.
>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Thomas Gleixner <tglx@linutronix.de>
I take this and 1/5 through irq/urgent
Thanks,
tglx
> ---
> kernel/irq/irqdomain.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index d5828da..84f4110 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -29,7 +29,7 @@ void irq_domain_add(struct irq_domain *domain)
> */
> for (hwirq = 0; hwirq < domain->nr_irq; hwirq++) {
> d = irq_get_irq_data(irq_domain_to_irq(domain, hwirq));
> - if (d || d->domain) {
> + if (d && d->domain) {
> /* things are broken; just report, don't clean up */
> WARN(1, "error: irq_desc already assigned to a domain");
> return;
> --
> 1.7.5.4
>
>
^ permalink raw reply
* [RFC 1/2] virtio: Add AMBA bus driver for virtio device
From: Pawel Moll @ 2011-09-14 16:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87ehzkht7x.fsf@rustcorp.com.au>
> > 2. "Select word -> read flags" interface:
> >
> > * 0x010 32 HostFeaturesFlags Features supported by the host
> > * 0x014 32 HostFeaturesWord Set of features to access
> >
> > * 0x020 32 GuestFeaturesFlags Features activated by the guest
> > * 0x024 32 GuestFeaturesWord Set of features to set
> >
> > In this case the algorithm would be:
> >
> > writel(0, HostFeaturesWord);
> > bits_0_31 = readl(HostFeaturesFlags);
> > writel(1, HostFeaturesWord);
> > bits_32_63 = readl(HostFeaturesFlags);
> > <etc>
> >
> > The latter seems more "future-proof" to me, if slightly more complex...
> > Any other suggestions?
>
> The former is almost certainly sufficient (remember, we can abuse the
> final feature bit to indicate there are extended features somewhere
> else, if we really have to). But agree the latter is preferable: this
> isn't time-critical.
Ok, I'll do this in v3 then.
> > > We can sort that out later... the Guest may try some ambitious large
> > > allocation and fall back if it fails.
> >
> > So, to my mind, the negotiations could look like this (from the Guest's
> > point of view):
> >
> > 1. (optional) The Guest is asking what size is "suggested" by the Host:
> >
> > size = readl(QueueNum);
> >
> > 2. The Guest requests size it would like to use:
> >
> > writel(optimal_size(size), QueueNum);
> >
> > 3. The Host does the best it can to accommodate the Guest and the Guest
> > checks the effective size:
> >
> > size = real(QueueNum);
> >
> > Does it make some sense?
>
> Does Host care? It is sufficient for it to publish min/max values?
Hm. You're right, it should no care. Most likely the queue size will
impact some Host's memory allocation size, but that's the Host's
problem :-)
At the same time, how the Host would know what the max value is?
Generally it looks like the matter was getting back to value suggested
by the Host (as it is now) that may be changed by the Guest. Single R/W
register should be fine, I believe?
> You might want to throw the alignment value in there; there has been
> talk about adding that to PCI, for really small rings (less than a
> page).
Sure thing, will do.
v3 will be ready tomorrow.
Cheers!
Pawe?
^ permalink raw reply
* [PATCH 2/2] input: samsung-keypad: Add device tree support
From: Thomas Abraham @ 2011-09-14 16:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110914161144.GF3134@ponder.secretlab.ca>
Hi Grant,
On 14 September 2011 21:41, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Tue, Sep 13, 2011 at 05:56:19PM +0530, Thomas Abraham wrote:
>> Add device tree based discovery support for Samsung's keypad controller.
>>
>> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
>> Cc: Donghwa Lee <dh09.lee@samsung.com>
>> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
>> ---
>> ?.../devicetree/bindings/input/samsung-keypad.txt ? | ? 88 ++++++++++
>> ?drivers/input/keyboard/samsung-keypad.c ? ? ? ? ? ?| ?177 ++++++++++++++++++--
>> ?2 files changed, 253 insertions(+), 12 deletions(-)
>> ?create mode 100644 Documentation/devicetree/bindings/input/samsung-keypad.txt
>>
>> diff --git a/Documentation/devicetree/bindings/input/samsung-keypad.txt b/Documentation/devicetree/bindings/input/samsung-keypad.txt
>> new file mode 100644
>> index 0000000..e1c7237
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/samsung-keypad.txt
>> @@ -0,0 +1,88 @@
>> +* Samsung's Keypad Controller device tree bindings
>> +
>> +Samsung's Keypad controller is used to interface a SoC with a matrix-type
>> +keypad device. The keypad controller supports multiple row and column lines.
>> +A key can be placed at each intersection of a unique row and a unique column.
>> +The keypad controller can sense a key-press and key-release and report the
>> +event using a interrupt to the cpu.
>> +
>> +Required SoC Specific Properties:
>> +- compatible: should be one of the following
>> + ?- "samsung,s3c6410-keypad": For controllers compatible with s3c6410 keypad
>> + ? ?controller.
>> + ?- "samsung,s5pv210-keypad": For controllers compatible with s5pv210 keypad
>> + ? ?controller.
>> +
>> +- reg: physical base address of the controller and length of memory mapped
>> + ?region.
>> +
>> +- interrupts: The interrupt number to the cpu.
>> +
>> +Required Board Specific Properties:
>> +- samsung,keypad-num-rows: Number of row lines connected to the keypad
>> + ?controller.
>> +
>> +- samsung,keypad-num-columns: Number of column lines connected to the
>> + ?keypad controller.
>> +
>> +- row-gpios: List of gpios used as row lines. The gpio specifier for
>> + ?this property depends on the gpio controller to which these row lines
>> + ?are connected.
>> +
>> +- col-gpios: List of gpios used as column lines. The gpio specifier for
>> + ?this property depends on the gpio controller to which these column
>> + ?lines are connected.
>
> I know we've got overlapping terminology here. ?When you say GPIOs
> here, does it refer to the pin multiplexing feature of the samsung
> parts, and thus how the keypad controller IO is routed to the pins?
> Or does the driver manipulate GPIO lines manually?
It is intended to mean gpio and not the pinmux. But the way the gpio
specifier is specified in the dts file, it includes information about
both gpio number and the pin-control details. For instance, if 'gpa0'
is a gpio controller (which includes pin-control functionality as well
in the hardware), then the gpio specifier for the keypad would be as
below.
row-gpios = <&gpa0 0 3 3 0>,
<&gpa0 1 3 3 0>;
The syntax for the gpio specifier is
<[phandle of gpio controller] [pin number within the gpio controller]
[mux function] [pull up/down] [drive strength]>
The keypad driver does not know or use the mux function, pull up/down
and drive strength details. The driver would call of_get_gpio to get
the linux gpio number and then do a gpio_request on that gpio number.
The gpio dt support manges the pin-mux and other settings
transparently for the keypad driver. So even though the gpio specifier
format changes, the dt support code for the driver does not change.
The driver does not manipulate the gpios directly. It just requests
for the gpio number and makes a note of the number to free it when the
driver unbinds.
>
> If it is pin multiplexing, then using the GPIO binding seems rather
> odd. It may be entirely appropriate, but it will need to play well
> with the pinmux stuff that linusw is working on.
When pinmux/pin-ctrl functionality which linusw is developing is used
for this driver, it would be a extension to this patch. The driver
would request for the gpio and then use the pin-ctrl subsystem api to
setup the pin-control. In that case, the gpio-specifier would change
but that change would be break anything which this patch does.
>
>> +
>> +- Keys represented as child nodes: Each key connected to the keypad
>> + ?controller is represented as a child node to the keypad controller
>> + ?device node and should include the following properties.
>> + ?- keypad,row: the row number to which the key is connected.
>> + ?- keypad,column: the column number to which the key is connected.
>> + ?- keypad,key-code: the key-code to be reported when the key is pressed
>> + ? ?and released.
>
> What defines the meanings of the key codes?
The key-code could be any value which the system would want the keypad
driver to report when that key is pressed.
>
>> +
>> +Optional Properties specific to linux:
>> +- linux,keypad-no-autorepeat: do no enable autorepeat feature.
>> +- linux,keypad-wakeup: use any event on keypad as wakeup event.
>
> Is this really a linux-specific feature?
Yes, it is a property defined by the linux subsystems. A non-linux
system might not have such features.
>
>> +
>> +
>> +Example:
>> + ? ? keypad at 100A0000 {
>> + ? ? ? ? ? ? compatible = "samsung,s5pv210-keypad";
>> + ? ? ? ? ? ? reg = <0x100A0000 0x100>;
>> + ? ? ? ? ? ? interrupts = <173>;
>> + ? ? ? ? ? ? samsung,keypad-num-rows = <2>;
>> + ? ? ? ? ? ? samsung,keypad-num-columns = <8>;
>> + ? ? ? ? ? ? linux,input-no-autorepeat;
>> + ? ? ? ? ? ? linux,input-wakeup;
>> +
>> + ? ? ? ? ? ? row-gpios = <&gpx2 0 3 3 0
>> + ? ? ? ? ? ? ? ? ? ? ? ? ?&gpx2 1 3 3 0>;
>> +
>> + ? ? ? ? ? ? col-gpios = <&gpx1 0 3 0 0
>> + ? ? ? ? ? ? ? ? ? ? ? ? ?&gpx1 1 3 0 0
>> + ? ? ? ? ? ? ? ? ? ? ? ? ?&gpx1 2 3 0 0
>> + ? ? ? ? ? ? ? ? ? ? ? ? ?&gpx1 3 3 0 0
>> + ? ? ? ? ? ? ? ? ? ? ? ? ?&gpx1 4 3 0 0
>> + ? ? ? ? ? ? ? ? ? ? ? ? ?&gpx1 5 3 0 0
>> + ? ? ? ? ? ? ? ? ? ? ? ? ?&gpx1 6 3 0 0
>> + ? ? ? ? ? ? ? ? ? ? ? ? ?&gpx1 7 3 0 0>;
>> +
>> + ? ? ? ? ? ? key_1 {
>> + ? ? ? ? ? ? ? ? ? ? keypad,row = <0>;
>> + ? ? ? ? ? ? ? ? ? ? keypad,column = <3>;
>> + ? ? ? ? ? ? ? ? ? ? keypad,key-code = <2>;
>> + ? ? ? ? ? ? };
>> +
>> + ? ? ? ? ? ? key_2 {
>> + ? ? ? ? ? ? ? ? ? ? keypad,row = <0>;
>> + ? ? ? ? ? ? ? ? ? ? keypad,column = <4>;
>> + ? ? ? ? ? ? ? ? ? ? keypad,key-code = <3>;
>> + ? ? ? ? ? ? };
>> +
>> + ? ? ? ? ? ? key_3 {
>> + ? ? ? ? ? ? ? ? ? ? keypad,row = <0>;
>> + ? ? ? ? ? ? ? ? ? ? keypad,column = <5>;
>> + ? ? ? ? ? ? ? ? ? ? keypad,key-code = <4>;
>> + ? ? ? ? ? ? };
>> + ? ? };
>> diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
>> index f689f49..cf01a56 100644
>> --- a/drivers/input/keyboard/samsung-keypad.c
>> +++ b/drivers/input/keyboard/samsung-keypad.c
>> @@ -21,6 +21,8 @@
>> ?#include <linux/module.h>
>> ?#include <linux/platform_device.h>
>> ?#include <linux/slab.h>
>> +#include <linux/of.h>
>> +#include <linux/of_gpio.h>
>> ?#include <linux/sched.h>
>> ?#include <plat/keypad.h>
>>
>> @@ -68,31 +70,26 @@ struct samsung_keypad {
>> ? ? ? wait_queue_head_t wait;
>> ? ? ? bool stopped;
>> ? ? ? int irq;
>> + ? ? enum samsung_keypad_type type;
>> ? ? ? unsigned int row_shift;
>> ? ? ? unsigned int rows;
>> ? ? ? unsigned int cols;
>> ? ? ? unsigned int row_state[SAMSUNG_MAX_COLS];
>> +#ifdef CONFIG_OF
>> + ? ? int row_gpios[SAMSUNG_MAX_ROWS];
>> + ? ? int col_gpios[SAMSUNG_MAX_COLS];
>> +#endif
>> ? ? ? unsigned short keycodes[];
>> ?};
>>
>> -static int samsung_keypad_is_s5pv210(struct device *dev)
>> -{
>> - ? ? struct platform_device *pdev = to_platform_device(dev);
>> - ? ? enum samsung_keypad_type type =
>> - ? ? ? ? ? ? platform_get_device_id(pdev)->driver_data;
>> -
>> - ? ? return type == KEYPAD_TYPE_S5PV210;
>> -}
>> -
>> ?static void samsung_keypad_scan(struct samsung_keypad *keypad,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned int *row_state)
>> ?{
>> - ? ? struct device *dev = keypad->input_dev->dev.parent;
>> ? ? ? unsigned int col;
>> ? ? ? unsigned int val;
>>
>> ? ? ? for (col = 0; col < keypad->cols; col++) {
>> - ? ? ? ? ? ? if (samsung_keypad_is_s5pv210(dev)) {
>> + ? ? ? ? ? ? if (keypad->type == KEYPAD_TYPE_S5PV210) {
>> ? ? ? ? ? ? ? ? ? ? ? val = S5PV210_KEYIFCOLEN_MASK;
>> ? ? ? ? ? ? ? ? ? ? ? val &= ~(1 << col) << 8;
>> ? ? ? ? ? ? ? } else {
>> @@ -235,6 +232,129 @@ static void samsung_keypad_close(struct input_dev *input_dev)
>> ? ? ? samsung_keypad_stop(keypad);
>> ?}
>>
>> +#ifdef CONFIG_OF
>> +static
>> +struct samsung_keypad_platdata *samsung_keypad_parse_dt(struct device *dev)
>
> Nit: keep everything up to the function name on the same line. ?It is
> more grep-friendly to make line breaks in the parameters list.
Ok. I will change this.
>
>> +{
>> + ? ? struct samsung_keypad_platdata *pdata;
>> + ? ? struct matrix_keymap_data *keymap_data;
>> + ? ? uint32_t *keymap;
>> + ? ? struct device_node *np = dev->of_node, *key_np;
>> + ? ? unsigned int key_count = 0;
>> +
>> + ? ? pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>> + ? ? if (!pdata) {
>> + ? ? ? ? ? ? dev_err(dev, "could not allocate memory for platform data\n");
>> + ? ? ? ? ? ? return NULL;
>> + ? ? }
>> +
>> + ? ? of_property_read_u32(np, "samsung,keypad-num-rows", &pdata->rows);
>> + ? ? of_property_read_u32(np, "samsung,keypad-num-columns", &pdata->cols);
>
> pdata->rows and ->cols needs to be changed to a u32 for this to be
> safe.
Ok.
>
>> + ? ? if (!pdata->rows || !pdata->cols) {
>> + ? ? ? ? ? ? dev_err(dev, "number of keypad rows/columns not specified\n");
>> + ? ? ? ? ? ? return NULL;
>> + ? ? }
>> +
>> + ? ? keymap_data = devm_kzalloc(dev, sizeof(*keymap_data), GFP_KERNEL);
>> + ? ? if (!keymap_data) {
>> + ? ? ? ? ? ? dev_err(dev, "could not allocate memory for keymap data\n");
>> + ? ? ? ? ? ? return NULL;
>> + ? ? }
>> + ? ? pdata->keymap_data = keymap_data;
>> +
>> + ? ? for_each_child_of_node(np, key_np)
>> + ? ? ? ? ? ? key_count++;
>> +
>> + ? ? keymap_data->keymap_size = key_count;
>> + ? ? keymap = devm_kzalloc(dev, sizeof(uint32_t) * key_count, GFP_KERNEL);
>> + ? ? if (!keymap) {
>> + ? ? ? ? ? ? dev_err(dev, "could not allocate memory for keymap\n");
>> + ? ? ? ? ? ? return NULL;
>> + ? ? }
>> + ? ? keymap_data->keymap = keymap;
>> +
>> + ? ? for_each_child_of_node(np, key_np) {
>> + ? ? ? ? ? ? unsigned int row, col, key_code;
>
> These need to be u32
Ok.
>
>> + ? ? ? ? ? ? of_property_read_u32(key_np, "keypad,row", &row);
>> + ? ? ? ? ? ? of_property_read_u32(key_np, "keypad,column", &col);
>> + ? ? ? ? ? ? of_property_read_u32(key_np, "keypad,key-code", &key_code);
>> + ? ? ? ? ? ? *keymap++ = KEY(row, col, key_code);
>> + ? ? }
>> +
>> + ? ? if (of_get_property(np, "linux,input-no-autorepeat", NULL))
>> + ? ? ? ? ? ? pdata->no_autorepeat = true;
>> + ? ? if (of_get_property(np, "linux,input-wakeup", NULL))
>> + ? ? ? ? ? ? pdata->wakeup = true;
>> +
>> + ? ? return pdata;
>> +}
>> +
>> +static void samsung_keypad_parse_dt_gpio(struct device *dev,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct samsung_keypad *keypad)
>> +{
>> + ? ? struct device_node *np = dev->of_node;
>> + ? ? int gpio, ret, row, col;
>> +
>> + ? ? for (row = 0; row < keypad->rows; row++) {
>> + ? ? ? ? ? ? gpio = of_get_named_gpio(np, "row-gpios", row);
>> + ? ? ? ? ? ? keypad->row_gpios[row] = gpio;
>> + ? ? ? ? ? ? if (!gpio_is_valid(gpio)) {
>> + ? ? ? ? ? ? ? ? ? ? dev_err(dev, "keypad row[%d]: invalid gpio %d\n",
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? row, gpio);
>> + ? ? ? ? ? ? ? ? ? ? continue;
>> + ? ? ? ? ? ? }
>> +
>> + ? ? ? ? ? ? ret = gpio_request(gpio, "keypad-row");
>> + ? ? ? ? ? ? if (ret)
>> + ? ? ? ? ? ? ? ? ? ? dev_err(dev, "keypad row[%d] gpio request failed\n",
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? row);
>> + ? ? }
>> +
>> + ? ? for (col = 0; col < keypad->cols; col++) {
>> + ? ? ? ? ? ? gpio = of_get_named_gpio(np, "col-gpios", col);
>> + ? ? ? ? ? ? keypad->col_gpios[col] = gpio;
>> + ? ? ? ? ? ? if (!gpio_is_valid(gpio)) {
>> + ? ? ? ? ? ? ? ? ? ? dev_err(dev, "keypad column[%d]: invalid gpio %d\n",
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? col, gpio);
>> + ? ? ? ? ? ? ? ? ? ? continue;
>> + ? ? ? ? ? ? }
>> +
>> + ? ? ? ? ? ? ret = gpio_request(col, "keypad-col");
>> + ? ? ? ? ? ? if (ret)
>> + ? ? ? ? ? ? ? ? ? ? dev_err(dev, "keypad column[%d] gpio request failed\n",
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? col);
>> + ? ? }
>> +}
>> +
>> +static void samsung_keypad_dt_gpio_free(struct samsung_keypad *keypad)
>> +{
>> + ? ? int cnt;
>> +
>> + ? ? for (cnt = 0; cnt < keypad->rows; cnt++)
>> + ? ? ? ? ? ? if (gpio_is_valid(keypad->row_gpios[cnt]))
>> + ? ? ? ? ? ? ? ? ? ? gpio_free(keypad->row_gpios[cnt]);
>> +
>> + ? ? for (cnt = 0; cnt < keypad->cols; cnt++)
>> + ? ? ? ? ? ? if (gpio_is_valid(keypad->col_gpios[cnt]))
>> + ? ? ? ? ? ? ? ? ? ? gpio_free(keypad->col_gpios[cnt]);
>> +}
>> +#else
>> +static
>> +struct samsung_keypad_platdata *samsung_keypad_parse_dt(struct device *dev)
>> +{
>> + ? ? return NULL;
>> +}
>> +
>> +static void samsung_keypad_parse_dt_gpio(struct device *dev,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct samsung_keypad *keypad)
>> +{
>> +}
>> +
>> +static void samsung_keypad_dt_gpio_free(struct samsung_keypad *keypad)
>> +{
>> +}
>> +#endif
>> +
>> ?static int __devinit samsung_keypad_probe(struct platform_device *pdev)
>> ?{
>> ? ? ? const struct samsung_keypad_platdata *pdata;
>> @@ -246,7 +366,10 @@ static int __devinit samsung_keypad_probe(struct platform_device *pdev)
>> ? ? ? unsigned int keymap_size;
>> ? ? ? int error;
>>
>> - ? ? pdata = pdev->dev.platform_data;
>> + ? ? if (pdev->dev.of_node)
>> + ? ? ? ? ? ? pdata = samsung_keypad_parse_dt(&pdev->dev);
>> + ? ? else
>> + ? ? ? ? ? ? pdata = pdev->dev.platform_data;
>> ? ? ? if (!pdata) {
>> ? ? ? ? ? ? ? dev_err(&pdev->dev, "no platform data defined\n");
>> ? ? ? ? ? ? ? return -EINVAL;
>> @@ -303,6 +426,16 @@ static int __devinit samsung_keypad_probe(struct platform_device *pdev)
>> ? ? ? keypad->cols = pdata->cols;
>> ? ? ? init_waitqueue_head(&keypad->wait);
>>
>> + ? ? if (pdev->dev.of_node) {
>> + ? ? ? ? ? ? samsung_keypad_parse_dt_gpio(&pdev->dev, keypad);
>> +#ifdef CONFIG_OF
>> + ? ? ? ? ? ? keypad->type = of_device_is_compatible(pdev->dev.of_node,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "samsung,s5pv210-keypad");
>> +#endif
>
> It is odd having the #ifdef around only part of the if() block.
I did not want to do it this way. But any other way would have
increased the number of lines added. If is against the coding rules or
style, I will change it.
>
>> + ? ? } else {
>> + ? ? ? ? ? ? keypad->type = platform_get_device_id(pdev)->driver_data;
>> + ? ? }
>> +
>> ? ? ? input_dev->name = pdev->name;
>> ? ? ? input_dev->id.bustype = BUS_HOST;
>> ? ? ? input_dev->dev.parent = &pdev->dev;
>> @@ -343,12 +476,19 @@ static int __devinit samsung_keypad_probe(struct platform_device *pdev)
>>
>> ? ? ? device_init_wakeup(&pdev->dev, pdata->wakeup);
>> ? ? ? platform_set_drvdata(pdev, keypad);
>> +
>> + ? ? if (pdev->dev.of_node) {
>> + ? ? ? ? ? ? devm_kfree(&pdev->dev, (void *)pdata->keymap_data->keymap);
>> + ? ? ? ? ? ? devm_kfree(&pdev->dev, (void *)pdata->keymap_data);
>> + ? ? ? ? ? ? devm_kfree(&pdev->dev, (void *)pdata);
>> + ? ? }
>
> Don't need to do this. ?The driver core will take care of freeing the
> devm for you when removed.
Dmitry Torokhov suggested that since the allocated pdata is not used
after the probe, it can be deallocated to save some memory. So this
was added.
>
> A few things that should be fixed up, but otherwise looks pretty good.
>
Thanks for your review and suggestions.
Regards,
Thomas.
[...]
^ permalink raw reply
* [PATCH 13/25] OMAP4: PM: Add WakeupGen module as OMAP gic_arch_extn
From: Santosh @ 2011-09-14 16:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110914152116.GM24252@atomide.com>
On Wednesday 14 September 2011 08:51 PM, Tony Lindgren wrote:
> * Shilimkar, Santosh<santosh.shilimkar@ti.com> [110913 22:01]:
>> Tony,
>> On Wed, Sep 14, 2011 at 2:06 AM, Tony Lindgren<tony@atomide.com> wrote:
>>> * Santosh Shilimkar<santosh.shilimkar@ti.com> [110904 06:23]:
>>>> OMAP WakeupGen is the interrupt controller extension used along
>>>> with ARM GIC to wake the CPU out from low power states on
>>>> external interrupts.
>>>>
>>>> The WakeupGen unit is responsible for generating wakeup event
>>>> from the incoming interrupts and enable bits. It is implemented
>>>> in MPU always ON power domain. During normal operation,
>>>> WakeupGen delivers external interrupts directly to the GIC.
>>> ...
>>>
>>>> + /*
>>>> + * Override GIC architecture specific functions to add
>>>> + * OMAP WakeupGen interrupt controller along with GIC
>>>> + */
>>>> + gic_arch_extn.irq_mask = wakeupgen_mask;
>>>> + gic_arch_extn.irq_unmask = wakeupgen_unmask;
>>>> + gic_arch_extn.irq_set_wake = wakeupgen_set_wake;
>>>> + gic_arch_extn.flags = IRQCHIP_MASK_ON_SUSPEND;
>>>
>>> As I've commented before, there should not be any need to tweak
>>> the wakeupgen registers for each interrupt during the runtime.
>>>
>> And I gave you all the reasons why it needs to be done this way.
>
> Hmm, I don't think you ever answered the main question:
>
> Why would you need to write the wakeupgen registers for every
> interrupt during the runtime instead of arming them only when
> entering idle?
>
I thought I did that in last thread.
Let me try again,
First and foremost, I have to go with the approach here because MPUSS
hardware team put a requirement that GIC and wakeupgen should always be
kept in sync. If needed we can discuss this off-the list with Richard.
Below is the extract from the veyron func specs.
-------------------------------------
Version 1.6 of veyron spec has this.
From page 95, paragraph 2 on version 1.6:
"It is SW responsibility to program interrupt enabling/disabling
coherently in the GIC and in the Wugen enable registers. That is, a
given interrupt for a given CPU is either enable at both GIC and Wugen,
or disable at both, but no mix."
-------------------------------------
The way understand this IP is, even in normal scenario's every IRQ
will come to wakeupgen and then it will pass that to GIC. CPU clock
domains are kept under HW supervised always and they can enter inactive
any time without WFI. Only wakeup gen can bring the CPU out of inactive
state.
That's requirement really lead to this design choice. Just to add
all ARM SOC's using GIC has a gic extension interrupt controller and
follow the same approach for the secondary IRQCHIPO.
Below points as such don't matter after the strict hardware
requirement. Still .....
Let's say, we ignore the hardware recommendation and try
to do what you are suggesting.
How will you know while entering in idle which IRQ's to be
masked and which are to be unmasked ?
The only way is to run though entire 1024 possible IRQ's from GIC
and then check the state of each IRQ and set things accordingly.
At GIC level, mask and unmask registers are different so you will
end up reading those many registers. That also means you need to
export some non-standard APIs from GIC driver.
In system wide suspend, the core irq code, communicates
the wakeup and non-wakeup functionality using standard mask/
unmask APIs when used with IRQCHIP_MASK_ON_SUSPEND.
With what you are suggesting it won't work
as desired. Because that information is only passed
to the IRQ chips. So you will still need IRQCHIP and
mask/unmask APIs. That can be done as part of set_wake()
handler as well though.
The wakeupgen is within CPU cluster and the accesses
to it are not as expensive as like accessing 32 K timer or
GP timer.
By making the wakeupgen as an IRQCHIP, we meet the hardware
requirement and also make use of this IP properly for the
desired functionality using standard IRQCHIP interfaces
No need of non-standard hacking.
It also avoid platform code monkeing with common GIC code
and irq subsystem to hack the stuff.
Btw, not exactly related here, but because of common code consolidation,
we need to actually use GIC common
save/restore hooks, even though OMAP has very
optimal software save and hardware restore mechanism
for GIC.
Hope this email summarise all previous multiple discussions
in one place.
Regards
Santosh
^ permalink raw reply
* [PATCH] ARM: mx31pdk: Fix build by passing IMX_HAVE_PLATFORM_MXC_MMC
From: Fabio Estevam @ 2011-09-14 17:04 UTC (permalink / raw)
To: linux-arm-kernel
mx31pdk target fails to build:
CC init/version.o
LD init/built-in.o
LD .tmp_vmlinux1
arch/arm/mach-imx/built-in.o: In function `mx31_3ds_init':
mach-mx31_3ds.c:(.init.text+0x548): undefined reference to `imx_add_mxc_mmc'
mach-mx31_3ds.c:(.init.text+0x69c): undefined reference to `imx31_mxc_mmc_data'
make: *** [.tmp_vmlinux1] Error
Fix this by passsing IMX_HAVE_PLATFORM_MXC_MMC option.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
arch/arm/mach-imx/Kconfig | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 0519dd7..e9c2968 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -449,6 +449,7 @@ config MACH_MX31_3DS
select IMX_HAVE_PLATFORM_IMX_UART
select IMX_HAVE_PLATFORM_IPU_CORE
select IMX_HAVE_PLATFORM_MXC_EHCI
+ select IMX_HAVE_PLATFORM_MXC_MMC
select IMX_HAVE_PLATFORM_MXC_NAND
select IMX_HAVE_PLATFORM_SPI_IMX
select MXC_ULPI if USB_ULPI
--
1.7.1
^ permalink raw reply related
* [PATCH 13/25] OMAP4: PM: Add WakeupGen module as OMAP gic_arch_extn
From: Tony Lindgren @ 2011-09-14 17:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4E70DB33.5070501@ti.com>
* Santosh <santosh.shilimkar@ti.com> [110914 09:16]:
>
> First and foremost, I have to go with the approach here because MPUSS
> hardware team put a requirement that GIC and wakeupgen should always be
> kept in sync. If needed we can discuss this off-the list with Richard.
>
> Below is the extract from the veyron func specs.
> -------------------------------------
> Version 1.6 of veyron spec has this.
>
> From page 95, paragraph 2 on version 1.6:
>
> "It is SW responsibility to program interrupt enabling/disabling
> coherently in the GIC and in the Wugen enable registers. That is, a
> given interrupt for a given CPU is either enable at both GIC and Wugen,
> or disable at both, but no mix."
> -------------------------------------
>
> The way understand this IP is, even in normal scenario's every IRQ
> will come to wakeupgen and then it will pass that to GIC. CPU clock
> domains are kept under HW supervised always and they can enter inactive
> any time without WFI. Only wakeup gen can bring the CPU out of inactive
> state.
>
> That's requirement really lead to this design choice. Just to add
> all ARM SOC's using GIC has a gic extension interrupt controller and
> follow the same approach for the secondary IRQCHIPO.
Thanks for the clarification. It seems to me the spec is most likely
wrong as we've had the GIC working for over two years now without
doing anything with the wakeup gen registers :)
Of course the wakeup events probably don't work currently, but the
GIC interrupts certainly do. So most likely there's no need to
continuously syncronize the wakeup gen registers with the GIC
registers.
> Below points as such don't matter after the strict hardware
> requirement. Still .....
I think the issue is that you're assuming the spec is correct,
which does not seem to be the case here.
> Let's say, we ignore the hardware recommendation and try
> to do what you are suggesting.
>
> How will you know while entering in idle which IRQ's to be
> masked and which are to be unmasked ?
> The only way is to run though entire 1024 possible IRQ's from GIC
> and then check the state of each IRQ and set things accordingly.
> At GIC level, mask and unmask registers are different so you will
> end up reading those many registers. That also means you need to
> export some non-standard APIs from GIC driver.
When entering idle, we have plenty of time to do that. Sounds like
that could be easily implemented in a generic way.
> In system wide suspend, the core irq code, communicates
> the wakeup and non-wakeup functionality using standard mask/
> unmask APIs when used with IRQCHIP_MASK_ON_SUSPEND.
> With what you are suggesting it won't work
> as desired. Because that information is only passed
> to the IRQ chips. So you will still need IRQCHIP and
> mask/unmask APIs. That can be done as part of set_wake()
> handler as well though.
>
> The wakeupgen is within CPU cluster and the accesses
> to it are not as expensive as like accessing 32 K timer or
> GP timer.
Sure, but it still causes unnecesary writes for every interrupt.
There's no technical reason to do that.
> By making the wakeupgen as an IRQCHIP, we meet the hardware
> requirement and also make use of this IP properly for the
> desired functionality using standard IRQCHIP interfaces
> No need of non-standard hacking.
Well it seems the "hardware requirement" is based on a buggy
spec considering things are currently working.
> It also avoid platform code monkeing with common GIC code
> and irq subsystem to hack the stuff.
What I'm suggesting can be implemented in a generic way.
> Btw, not exactly related here, but because of common code
> consolidation, we need to actually use GIC common
> save/restore hooks, even though OMAP has very
> optimal software save and hardware restore mechanism
> for GIC.
>
> Hope this email summarise all previous multiple discussions
> in one place.
Thanks, but unfortunately it does not. To me it still seems
this is the wrong approach for the wakeup triggers.
Regards,
Tony
^ permalink raw reply
* [PATCH 2/2] input: samsung-keypad: Add device tree support
From: Grant Likely @ 2011-09-14 17:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAJuYYwSNaVCnHFTMtW0RZLGLaS+iKBibWYwn5mqNT-0EW31yAQ@mail.gmail.com>
On Wed, Sep 14, 2011 at 10:19:22PM +0530, Thomas Abraham wrote:
> Hi Grant,
>
> On 14 September 2011 21:41, Grant Likely <grant.likely@secretlab.ca> wrote:
> > On Tue, Sep 13, 2011 at 05:56:19PM +0530, Thomas Abraham wrote:
> >> Add device tree based discovery support for Samsung's keypad controller.
> >>
> >> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> >> Cc: Donghwa Lee <dh09.lee@samsung.com>
> >> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> >> ---
> >> ?.../devicetree/bindings/input/samsung-keypad.txt ? | ? 88 ++++++++++
> >> ?drivers/input/keyboard/samsung-keypad.c ? ? ? ? ? ?| ?177 ++++++++++++++++++--
> >> ?2 files changed, 253 insertions(+), 12 deletions(-)
> >> ?create mode 100644 Documentation/devicetree/bindings/input/samsung-keypad.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/input/samsung-keypad.txt b/Documentation/devicetree/bindings/input/samsung-keypad.txt
> >> new file mode 100644
> >> index 0000000..e1c7237
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/input/samsung-keypad.txt
> >> @@ -0,0 +1,88 @@
> >> +* Samsung's Keypad Controller device tree bindings
> >> +
> >> +Samsung's Keypad controller is used to interface a SoC with a matrix-type
> >> +keypad device. The keypad controller supports multiple row and column lines.
> >> +A key can be placed at each intersection of a unique row and a unique column.
> >> +The keypad controller can sense a key-press and key-release and report the
> >> +event using a interrupt to the cpu.
> >> +
> >> +Required SoC Specific Properties:
> >> +- compatible: should be one of the following
> >> + ?- "samsung,s3c6410-keypad": For controllers compatible with s3c6410 keypad
> >> + ? ?controller.
> >> + ?- "samsung,s5pv210-keypad": For controllers compatible with s5pv210 keypad
> >> + ? ?controller.
> >> +
> >> +- reg: physical base address of the controller and length of memory mapped
> >> + ?region.
> >> +
> >> +- interrupts: The interrupt number to the cpu.
> >> +
> >> +Required Board Specific Properties:
> >> +- samsung,keypad-num-rows: Number of row lines connected to the keypad
> >> + ?controller.
> >> +
> >> +- samsung,keypad-num-columns: Number of column lines connected to the
> >> + ?keypad controller.
> >> +
> >> +- row-gpios: List of gpios used as row lines. The gpio specifier for
> >> + ?this property depends on the gpio controller to which these row lines
> >> + ?are connected.
> >> +
> >> +- col-gpios: List of gpios used as column lines. The gpio specifier for
> >> + ?this property depends on the gpio controller to which these column
> >> + ?lines are connected.
> >
> > I know we've got overlapping terminology here. ?When you say GPIOs
> > here, does it refer to the pin multiplexing feature of the samsung
> > parts, and thus how the keypad controller IO is routed to the pins?
> > Or does the driver manipulate GPIO lines manually?
>
> It is intended to mean gpio and not the pinmux. But the way the gpio
> specifier is specified in the dts file, it includes information about
> both gpio number and the pin-control details. For instance, if 'gpa0'
> is a gpio controller (which includes pin-control functionality as well
> in the hardware), then the gpio specifier for the keypad would be as
> below.
>
> row-gpios = <&gpa0 0 3 3 0>,
> <&gpa0 1 3 3 0>;
>
> The syntax for the gpio specifier is
> <[phandle of gpio controller] [pin number within the gpio controller]
> [mux function] [pull up/down] [drive strength]>
>
> The keypad driver does not know or use the mux function, pull up/down
> and drive strength details. The driver would call of_get_gpio to get
> the linux gpio number and then do a gpio_request on that gpio number.
> The gpio dt support manges the pin-mux and other settings
> transparently for the keypad driver. So even though the gpio specifier
> format changes, the dt support code for the driver does not change.
>
> The driver does not manipulate the gpios directly. It just requests
> for the gpio number and makes a note of the number to free it when the
> driver unbinds.
>
> >
> > If it is pin multiplexing, then using the GPIO binding seems rather
> > odd. It may be entirely appropriate, but it will need to play well
> > with the pinmux stuff that linusw is working on.
>
> When pinmux/pin-ctrl functionality which linusw is developing is used
> for this driver, it would be a extension to this patch. The driver
> would request for the gpio and then use the pin-ctrl subsystem api to
> setup the pin-control. In that case, the gpio-specifier would change
> but that change would be break anything which this patch does.
>
> >
> >> +
> >> +- Keys represented as child nodes: Each key connected to the keypad
> >> + ?controller is represented as a child node to the keypad controller
> >> + ?device node and should include the following properties.
> >> + ?- keypad,row: the row number to which the key is connected.
> >> + ?- keypad,column: the column number to which the key is connected.
> >> + ?- keypad,key-code: the key-code to be reported when the key is pressed
> >> + ? ?and released.
> >
> > What defines the meanings of the key codes?
>
> The key-code could be any value which the system would want the keypad
> driver to report when that key is pressed.
Are they linux keycodes? If so, then this property name can
probably be linux,code. There is already precedence for that
usage in Documentation/devicetree/bindings/gpio/gpio-keys.txt. (I
would personally prefer "linux,key-code", but sometimes it is better
to go with existing precidence) You could also use linux,input-type as
specified in that binding.
>
> >
> >> +
> >> +Optional Properties specific to linux:
> >> +- linux,keypad-no-autorepeat: do no enable autorepeat feature.
> >> +- linux,keypad-wakeup: use any event on keypad as wakeup event.
> >
> > Is this really a linux-specific feature?
>
> Yes, it is a property defined by the linux subsystems. A non-linux
> system might not have such features.
I was specifically referring to the wakeup property, but okay, I can
see the argument that it is linux-specific because it is usage-policy
related.
g.
^ permalink raw reply
* [PATCH 13/25] OMAP4: PM: Add WakeupGen module as OMAP gic_arch_extn
From: Santosh @ 2011-09-14 17:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110914170803.GN24252@atomide.com>
On Wednesday 14 September 2011 10:38 PM, Tony Lindgren wrote:
> * Santosh<santosh.shilimkar@ti.com> [110914 09:16]:
>>
>> First and foremost, I have to go with the approach here because MPUSS
>> hardware team put a requirement that GIC and wakeupgen should always be
>> kept in sync. If needed we can discuss this off-the list with Richard.
>>
>> Below is the extract from the veyron func specs.
>> -------------------------------------
>> Version 1.6 of veyron spec has this.
>>
>> From page 95, paragraph 2 on version 1.6:
>>
>> "It is SW responsibility to program interrupt enabling/disabling
>> coherently in the GIC and in the Wugen enable registers. That is, a
>> given interrupt for a given CPU is either enable at both GIC and Wugen,
>> or disable at both, but no mix."
>> -------------------------------------
>>
>> The way understand this IP is, even in normal scenario's every IRQ
>> will come to wakeupgen and then it will pass that to GIC. CPU clock
>> domains are kept under HW supervised always and they can enter inactive
>> any time without WFI. Only wakeup gen can bring the CPU out of inactive
>> state.
>>
>> That's requirement really lead to this design choice. Just to add
>> all ARM SOC's using GIC has a gic extension interrupt controller and
>> follow the same approach for the secondary IRQCHIPO.
>
> Thanks for the clarification. It seems to me the spec is most likely
> wrong as we've had the GIC working for over two years now without
> doing anything with the wakeup gen registers :)
>
It's working because CPU clockdomain are never put under HW
supervised mode and they are kept in force wakeup. Clock-domain
never idles on mainline code. PM series will put the clock-domains
under HW supervison as needed to achieve any low power states and
then all sorts of corner cases will come out.
Regards
Santosh
^ permalink raw reply
* [PATCH 13/25] OMAP4: PM: Add WakeupGen module as OMAP gic_arch_extn
From: Tony Lindgren @ 2011-09-14 17:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4E70E0CC.80300@ti.com>
* Santosh <santosh.shilimkar@ti.com> [110914 09:40]:
> On Wednesday 14 September 2011 10:38 PM, Tony Lindgren wrote:
> >* Santosh<santosh.shilimkar@ti.com> [110914 09:16]:
> >
> >Thanks for the clarification. It seems to me the spec is most likely
> >wrong as we've had the GIC working for over two years now without
> >doing anything with the wakeup gen registers :)
> >
> It's working because CPU clockdomain are never put under HW
> supervised mode and they are kept in force wakeup. Clock-domain
> never idles on mainline code. PM series will put the clock-domains
> under HW supervison as needed to achieve any low power states and
> then all sorts of corner cases will come out.
But again the wakeup gen triggers only do something when hitting
idle. There should be no use for them during runtime, right?
Tony
^ permalink raw reply
* [PATCH 13/25] OMAP4: PM: Add WakeupGen module as OMAP gic_arch_extn
From: Santosh @ 2011-09-14 17:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110914171800.GO24252@atomide.com>
On Wednesday 14 September 2011 10:48 PM, Tony Lindgren wrote:
> * Santosh<santosh.shilimkar@ti.com> [110914 09:40]:
>> On Wednesday 14 September 2011 10:38 PM, Tony Lindgren wrote:
>>> * Santosh<santosh.shilimkar@ti.com> [110914 09:16]:
>>>
>>> Thanks for the clarification. It seems to me the spec is most likely
>>> wrong as we've had the GIC working for over two years now without
>>> doing anything with the wakeup gen registers :)
>>>
>> It's working because CPU clockdomain are never put under HW
>> supervised mode and they are kept in force wakeup. Clock-domain
>> never idles on mainline code. PM series will put the clock-domains
>> under HW supervison as needed to achieve any low power states and
>> then all sorts of corner cases will come out.
>
> But again the wakeup gen triggers only do something when hitting
> idle. There should be no use for them during runtime, right?
>
You missed my point in the description. Clockdomain inactive
doesn't depend on idle or WFI execution. Under HW supervison
CPU clock domain can get into inactive when CPU is stalled and
waiting for a read response from slow interconnect.
Regards
Santosh
^ permalink raw reply
* [PATCH 13/25] OMAP4: PM: Add WakeupGen module as OMAP gic_arch_extn
From: Santosh @ 2011-09-14 17:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110914171800.GO24252@atomide.com>
On Wednesday 14 September 2011 10:48 PM, Tony Lindgren wrote:
> * Santosh<santosh.shilimkar@ti.com> [110914 09:40]:
>> On Wednesday 14 September 2011 10:38 PM, Tony Lindgren wrote:
>>> * Santosh<santosh.shilimkar@ti.com> [110914 09:16]:
>>>
>>> Thanks for the clarification. It seems to me the spec is most likely
>>> wrong as we've had the GIC working for over two years now without
>>> doing anything with the wakeup gen registers :)
>>>
>> It's working because CPU clockdomain are never put under HW
>> supervised mode and they are kept in force wakeup. Clock-domain
>> never idles on mainline code. PM series will put the clock-domains
>> under HW supervison as needed to achieve any low power states and
>> then all sorts of corner cases will come out.
>
> But again the wakeup gen triggers only do something when hitting
> idle. There should be no use for them during runtime, right?
>
You missed my point in the description. Clockdomain inactive
doesn't depend on idle or WFI execution. Under HW supervison
CPU clock domain can get into inactive when CPU is stalled and
waiting for a read response from slow interconnect.
One thing for sure. Designers has chosen a wrong name to this
IP. Wakeugen apears like needed only for low power wakeup which
not seems to be entirely correct as per specs
Regards
Santosh
^ permalink raw reply
* [PATCH v3 4/6] DMA: PL330: Add device tree support
From: Thomas Abraham @ 2011-09-14 17:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110914162401.GK3134@ponder.secretlab.ca>
Hi Grant,
On 14 September 2011 21:54, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Mon, Sep 12, 2011 at 11:59:23PM +0530, Thomas Abraham wrote:
>> For PL330 dma controllers instantiated from device tree, the channel
>> lookup is based on phandle of the dma controller and dma request id
>> specified by the client node. During probe, the private data of each
>> channel of the controller is set to point to the device node of the
>> dma controller. The 'chan_id' of the each channel is used as the
>> dma request id.
>>
>> Client driver requesting dma channels specify the phandle of the
>> dma controller and the request id. The pl330 filter function
>> converts the phandle to the device node pointer and matches that
>> with channel's private data. If a match is found, the request id
>> from the client node and the 'chan_id' of the channel is matched.
>> A channel is found if both the values match.
>>
>> Cc: Jassi Brar <jassisinghbrar@gmail.com>
>> Cc: Boojin Kim <boojin.kim@samsung.com>
>> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
>> Reviewed-by: Rob Herring <rob.herring@calxeda.com>
>> ---
>> ?.../devicetree/bindings/dma/arm-pl330.txt ? ? ? ? ?| ? 29 +++++++++++++++++
>> ?drivers/dma/pl330.c ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| ? 33 +++++++++++++++++--
>> ?2 files changed, 58 insertions(+), 4 deletions(-)
>> ?create mode 100644 Documentation/devicetree/bindings/dma/arm-pl330.txt
>>
>> diff --git a/Documentation/devicetree/bindings/dma/arm-pl330.txt b/Documentation/devicetree/bindings/dma/arm-pl330.txt
>> new file mode 100644
>> index 0000000..05b007d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/dma/arm-pl330.txt
>> @@ -0,0 +1,29 @@
>> +* ARM PrimeCell PL330 DMA Controller
>> +
>> +The ARM PrimeCell PL330 DMA controller can move blocks of memory contents
>> +between memory and peripherals or memory to memory.
>> +
>> +Required properties:
>> + ?- compatible: should include both "arm,pl330" and "arm,primecell".
>> + ?- reg: physical base address of the controller and length of memory mapped
>> + ? ?region.
>> + ?- interrupts: interrupt number to the cpu.
>> +
>> +Example: (from Samsung's Exynos4 processor dtsi file)
>> +
>> + ? ? pdma0: pdma at 12680000 {
>> + ? ? ? ? ? ? compatible = "arm,pl330", "arm,primecell";
>> + ? ? ? ? ? ? reg = <0x12680000 0x1000>;
>> + ? ? ? ? ? ? interrupts = <99>;
>> + ? ? };
>> +
>> +Client drivers (device nodes requiring dma transfers from dev-to-mem or
>> +mem-to-dev) should specify the DMA channel numbers using a two-value pair
>> +as shown below.
>> +
>> + ?[property name] ?= <[phandle of the dma controller] [dma request id]>;
>> +
>> + ? ? ?where 'dma request id' is the dma request number which is connected
>> + ? ? ?to the client controller.
>> +
>> + ?Example: ?tx-dma-channel = <&pdma0 12>;
>
> Looks good to me. ?You may want to specify that the property name
> should be in the form: <name>-dma-channel just to enforce a bit of
> convention on the users.
Ok. This will be added as a suggestion for property name specifying
the dma channel.
>
>> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
>> index 992bf82..7a4ebf1 100644
>> --- a/drivers/dma/pl330.c
>> +++ b/drivers/dma/pl330.c
>> @@ -19,6 +19,7 @@
>> ?#include <linux/amba/pl330.h>
>> ?#include <linux/pm_runtime.h>
>> ?#include <linux/scatterlist.h>
>> +#include <linux/of.h>
>>
>> ?#define NR_DEFAULT_DESC ? ? ?16
>>
>> @@ -277,6 +278,20 @@ bool pl330_filter(struct dma_chan *chan, void *param)
>> ? ? ? if (chan->device->dev->driver != &pl330_driver.drv)
>> ? ? ? ? ? ? ? return false;
>>
>> +#ifdef CONFIG_OF
>> + ? ? if (chan->device->dev->of_node) {
>> + ? ? ? ? ? ? const __be32 *prop_value;
>> + ? ? ? ? ? ? phandle phandle;
>> + ? ? ? ? ? ? struct device_node *node;
>> +
>> + ? ? ? ? ? ? prop_value = ((struct property *)param)->value;
>> + ? ? ? ? ? ? phandle = be32_to_cpup(prop_value++);
>> + ? ? ? ? ? ? node = of_find_node_by_phandle(phandle);
>> + ? ? ? ? ? ? return ((chan->private == node) &&
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? (chan->chan_id == be32_to_cpup(prop_value)));
>
> Don't open code this. ?There is already a function to decode a phandle
> property. ?of_parse_phandle() should do the trick.
The parameter to this function 'void *param' is already pointing to a
'struct property'. That 'struct property' has a value of type
<[phandle of dma controller] [channel id]>.
Since the property is already known, the of_get_property() call within
the of_parse_phandle() would complicate the above code. And,
of_parse_phandle requires a 'property name' inside the dma client
device node, which the above code fragment does not know about
(property names are defined by client drivers).
So, I would prefer to continue with the above implementation.
>
> Otherwise looks good to me.
>
> g.
Thanks for your review.
Regards,
Thomas.
[...]
^ permalink raw reply
* [PATCH 5/5] ARM: gic: add OF based initialization
From: Marc Zyngier @ 2011-09-14 17:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316017900-19918-6-git-send-email-robherring2@gmail.com>
On 14/09/11 17:31, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> This adds gic initialization using device tree data. The initialization
> functions are intended to be called by a generic OF interrupt
> controller parsing function once the right pieces are in place.
>
> PPIs are handled using 3rd cell of interrupts properties to specify the cpu
> mask the PPI is assigned to.
>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> ---
> Documentation/devicetree/bindings/arm/gic.txt | 53 ++++++++++++++++++++++++
> arch/arm/common/gic.c | 55 +++++++++++++++++++++++--
> arch/arm/include/asm/hardware/gic.h | 10 +++++
> 3 files changed, 114 insertions(+), 4 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
>
> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
> new file mode 100644
> index 0000000..6c513de
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/gic.txt
> @@ -0,0 +1,53 @@
> +* ARM Generic Interrupt Controller
> +
> +ARM SMP cores are often associated with a GIC, providing per processor
> +interrupts (PPI), shared processor interrupts (SPI) and software
> +generated interrupts (SGI).
> +
> +Primary GIC is attached directly to the CPU and typically has PPIs and SGIs.
> +Secondary GICs are cascaded into the upward interrupt controller and do not
> +have PPIs or SGIs.
> +
> +Main node required properties:
> +
> +- compatible : should be one of:
> + "arm,cortex-a9-gic"
> + "arm,arm11mp-gic"
> +- interrupt-controller : Identifies the node as an interrupt controller
> +- #interrupt-cells : Specifies the number of cells needed to encode an
> + interrupt source. The type shall be a <u32> and the value shall be 3.
> +
> + The 1st cell is the interrupt number. 0-15 are reserved for SGIs. 16-31 are
> + for PPIs.
> +
> + The 2nd cell is the level-sense information, encoded as follows:
> + 1 = low-to-high edge triggered
> + 2 = high-to-low edge triggered
> + 4 = active high level-sensitive
> + 8 = active low level-sensitive
> +
> + Only values of 1 and 4 are valid for GIC 1.0 spec.
> +
> + The 3rd cell contains the mask of the cpu number for the interrupt source.
> + The cpu mask is only valid for PPIs and shall be 0 for SPIs. This value shall
> + be 0 for PPIs.
^^^^^^^^^^^^^
Typo here ? The way I understand it, it should read "For PPIs, this
value shall be the mask of the possible CPU numbers for the interrupt
source" (or something to similar effect...).
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* Git pull request: support the appending of a DTB to zImage
From: Nicolas Pitre @ 2011-09-14 17:56 UTC (permalink / raw)
To: linux-arm-kernel
Russell, please pull the following:
git://git.linaro.org/people/nico/linux zImage_DTB_append
This series enables the concatenation of a zImage and a DTB file
together, and a compatibility wrapper converting some ATAGs into DT
properties, for systems with a bootloader that cannot accommodate the
normal passing of a device tree.
This code has been around for quite a while, with a couple revisions
posted and commented on the mailing list. This is in use by a couple
platforms moving to DT already.
I was hoping to clean the Makefile part a bit with the use of some new
Kbuild feature, but that is not in mainline yet, and it is not clear if
that would work correctly in this case, so I prefer revisiting that part
later if a good way to improve the build really shows up.
Based on v3.1-rc4 for the next merge window.
John Bonesio (1):
ARM: zImage: Allow the appending of a device tree binary
Nicolas Pitre (5):
ARM: zImage: ensure it is always a multiple of 64 bits in size
ARM: zImage: make sure appended DTB doesn't get overwritten by kernel .bss
ARM: zImage: gather some string functions into string.c
ARM: zImage: allow supplementing appended DTB with traditional ATAG data
ARM: zImage: prevent constant copy+rebuild of lib1funcs.S
arch/arm/Kconfig | 32 ++++++
arch/arm/boot/compressed/.gitignore | 9 ++
arch/arm/boot/compressed/Makefile | 32 ++++++-
arch/arm/boot/compressed/atags_to_fdt.c | 97 +++++++++++++++++++
arch/arm/boot/compressed/head.S | 121 ++++++++++++++++++++++--
arch/arm/boot/compressed/libfdt_env.h | 15 +++
arch/arm/boot/compressed/misc.c | 42 +--------
arch/arm/boot/compressed/string.c | 127 +++++++++++++++++++++++++
arch/arm/boot/compressed/vmlinux.lds.in | 4 +
9 files changed, 427 insertions(+), 52 deletions(-)
^ permalink raw reply
* [PATCH v3 6/6] ARM: EXYNOS4: Limit usage of pl330 device instance to non-dt build
From: Thomas Abraham @ 2011-09-14 17:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110914162633.GL3134@ponder.secretlab.ca>
Hi Grant,
On 14 September 2011 21:56, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Mon, Sep 12, 2011 at 11:59:25PM +0530, Thomas Abraham wrote:
>> The pl330 device instances and associated platform data is required only
>> for non-device-tree builds. With device tree enabled, the data about the
>> platform is obtained from the device tree. For images that include both
>> dt and non-dt platforms, an addditional check is added to ensure that
>> static amba device registrations is applicable to only non-dt platforms.
>>
>> Cc: Kukjin Kim <kgene.kim@samsung.com>
>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
>> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
>> ---
>> diff --git a/arch/arm/mach-exynos4/dma.c b/arch/arm/mach-exynos4/dma.c
>> index c3c0d17..3203a31 100644
>> --- a/arch/arm/mach-exynos4/dma.c
>> +++ b/arch/arm/mach-exynos4/dma.c
>> @@ -24,6 +24,7 @@
>> ?#include <linux/dma-mapping.h>
>> ?#include <linux/amba/bus.h>
>> ?#include <linux/amba/pl330.h>
>> +#include <linux/of.h>
>>
>> ?#include <asm/irq.h>
>> ?#include <plat/devs.h>
>> @@ -138,6 +139,11 @@ struct amba_device exynos4_device_pdma1 = {
>>
>> ?static int __init exynos4_dma_init(void)
>> ?{
>> +#ifdef CONFIG_OF
>> + ? ? if (of_have_populated_dt())
>> + ? ? ? ? ? ? return 0;
>> +#endif
>> +
>
> Drop the #ifdef. ?of_have_populated_dt() has an empty stub for
> !CONFIG_OF. ?Otherwise looks good to me. ?Well done not breaking
> non-DT support when CONFIG_OF is enabled. ?:-)
Ok. I will drop the #ifdef. Well, I actually learnt from you on not
breaking non-dt support because of your comments on my earlier patches
which did break non-dt support. Thanks.
>
> The other patches in this series look good to me too.
>
> g.
Thanks for your review. I will do the changes you have recommended for
the pl330 dt support patches and resubmit. I would like to add your
Ack to the patches when resubmitting.
Regards,
Thomas.
^ permalink raw reply
* [PATCH 5/5] ARM: gic: add OF based initialization
From: Rob Herring @ 2011-09-14 17:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4E70E88E.4090503@arm.com>
Marc,
On 09/14/2011 12:46 PM, Marc Zyngier wrote:
> On 14/09/11 17:31, Rob Herring wrote:
>> From: Rob Herring <rob.herring@calxeda.com>
>>
>> This adds gic initialization using device tree data. The initialization
>> functions are intended to be called by a generic OF interrupt
>> controller parsing function once the right pieces are in place.
>>
>> PPIs are handled using 3rd cell of interrupts properties to specify the cpu
>> mask the PPI is assigned to.
>>
>> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
>> ---
>> Documentation/devicetree/bindings/arm/gic.txt | 53 ++++++++++++++++++++++++
>> arch/arm/common/gic.c | 55 +++++++++++++++++++++++--
>> arch/arm/include/asm/hardware/gic.h | 10 +++++
>> 3 files changed, 114 insertions(+), 4 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
>>
>> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
>> new file mode 100644
>> index 0000000..6c513de
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/gic.txt
>> @@ -0,0 +1,53 @@
>> +* ARM Generic Interrupt Controller
>> +
>> +ARM SMP cores are often associated with a GIC, providing per processor
>> +interrupts (PPI), shared processor interrupts (SPI) and software
>> +generated interrupts (SGI).
>> +
>> +Primary GIC is attached directly to the CPU and typically has PPIs and SGIs.
>> +Secondary GICs are cascaded into the upward interrupt controller and do not
>> +have PPIs or SGIs.
>> +
>> +Main node required properties:
>> +
>> +- compatible : should be one of:
>> + "arm,cortex-a9-gic"
>> + "arm,arm11mp-gic"
>> +- interrupt-controller : Identifies the node as an interrupt controller
>> +- #interrupt-cells : Specifies the number of cells needed to encode an
>> + interrupt source. The type shall be a <u32> and the value shall be 3.
>> +
>> + The 1st cell is the interrupt number. 0-15 are reserved for SGIs. 16-31 are
>> + for PPIs.
>> +
>> + The 2nd cell is the level-sense information, encoded as follows:
>> + 1 = low-to-high edge triggered
>> + 2 = high-to-low edge triggered
>> + 4 = active high level-sensitive
>> + 8 = active low level-sensitive
>> +
>> + Only values of 1 and 4 are valid for GIC 1.0 spec.
>> +
>> + The 3rd cell contains the mask of the cpu number for the interrupt source.
>> + The cpu mask is only valid for PPIs and shall be 0 for SPIs. This value shall
>> + be 0 for PPIs.
> ^^^^^^^^^^^^^
>
> Typo here ? The way I understand it, it should read "For PPIs, this
> value shall be the mask of the possible CPU numbers for the interrupt
> source" (or something to similar effect...).
>
Cut and paste error. This sentence goes in the previous paragraph. What
I meant is the 2nd cell should contain 0 for PPIs as you cannot set the
edge/level on PPIs (that is always true, right?). I probably should also
add 0 in the list of values.
I take it you are otherwise fine with this binding?
Rob
^ permalink raw reply
* [PATCH v4] power: bq20z75: devicetree init support
From: Rhyland Klein @ 2011-09-14 18:04 UTC (permalink / raw)
To: linux-arm-kernel
Adding support to generate platform data when kernel is configured
through device tree.
Also adding the binding for the TI bq20z75 fuel gadge and the
bq20z75 driver.
Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
v2: Fixed typo in binding description
v3: Changed to use "ti," for properties
Changed to use single gpio entry for battery detect info.
Removed unnecessary call to of_match_device.
v4: squashed bindings and drivers changes together.
fixed case where CONFIG_OF is not selected, to return existing
pdata pointer if it exists.
.../bindings/power_supply/ti_bq20z75.txt | 23 ++++++
drivers/power/bq20z75.c | 75 ++++++++++++++++++++
2 files changed, 98 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power_supply/ti_bq20z75.txt
diff --git a/Documentation/devicetree/bindings/power_supply/ti_bq20z75.txt b/Documentation/devicetree/bindings/power_supply/ti_bq20z75.txt
new file mode 100644
index 0000000..7571294
--- /dev/null
+++ b/Documentation/devicetree/bindings/power_supply/ti_bq20z75.txt
@@ -0,0 +1,23 @@
+TI bq20z75
+~~~~~~~~~~
+
+Required properties :
+ - compatible : "ti,bq20z75"
+
+Optional properties :
+ - ti,i2c-retry-count : The number of times to retry i2c transactions on i2c
+ IO failure.
+ - ti,poll-retry-count : The number of times to try looking for new status
+ after an external change notification.
+ - ti,battery-detect-gpios : The gpio which signals battery detection and
+ a flag specifying its polarity.
+
+Example:
+
+ bq20z75 at b {
+ compatible = "ti,bq20z75";
+ reg = < 0xb >;
+ ti,i2c-retry-count = <2>;
+ ti,poll-retry-count = <10>;
+ ti,battery-detect-gpios = <&gpio-controller 122 1>;
+ }
diff --git a/drivers/power/bq20z75.c b/drivers/power/bq20z75.c
index 9c5e5be..4c4669e 100644
--- a/drivers/power/bq20z75.c
+++ b/drivers/power/bq20z75.c
@@ -613,6 +613,78 @@ static void bq20z75_delayed_work(struct work_struct *work)
}
}
+#if defined(CONFIG_OF)
+#include <linux/of_device.h>
+static const struct of_device_id bq20z75_dt_ids[] = {
+ { .compatible = "ti,bq20z75" },
+ { }
+};
+MODULE_DEVICE_TABLE(platform, bq20z75_dt_ids);
+
+static struct bq20z75_platform_data *bq20z75_of_populate_pdata(
+ struct i2c_client *client)
+{
+ const struct of_device_id *dtid;
+ struct device_node *of_node = client->dev.of_node;
+ struct bq20z75_platform_data *pdata = client->dev.platform_data;
+ enum of_gpio_flags gpio_flags;
+ int rc;
+ u32 prop;
+
+ /* verify this driver matches this device */
+ if (!of_node)
+ return NULL;
+
+ /* if platform data is set, honor it */
+ if (pdata)
+ return pdata;
+
+ /* first make sure at least one property is set, otherwise
+ * it won't change behavior from running without pdata.
+ */
+ if (!of_get_property(of_node, "ti,i2c-retry-count", NULL) &&
+ !of_get_property(of_node, "ti,poll-retry-count", NULL) &&
+ !of_get_property(of_node, "ti,battery-detect-gpios", NULL))
+ goto of_out;
+
+ pdata = devm_kzalloc(&client->dev, sizeof(struct bq20z75_platform_data),
+ GFP_KERNEL);
+ if (!pdata)
+ goto of_out;
+
+ rc = of_property_read_u32(of_node, "ti,i2c-retry-count", &prop);
+ if (!rc)
+ pdata->i2c_retry_count = prop;
+
+ rc = of_property_read_u32(of_node, "ti,poll-retry-count", &prop);
+ if (!rc)
+ pdata->poll_retry_count = prop;
+
+ if (!of_get_property(of_node, "ti,battery-detect-gpios", NULL)) {
+ pdata->battery_detect = -1;
+ goto of_out;
+ }
+
+ pdata->battery_detect = of_get_named_gpio_flags(of_node,
+ "ti,battery-detect-gpios", 0, &gpio_flags);
+
+ if (gpio_flags & OF_GPIO_ACTIVE_LOW)
+ pdata->battery_detect_present = 0;
+ else
+ pdata->battery_detect_present = 1;
+
+of_out:
+ return pdata;
+}
+#else
+#define bq20z75_dt_ids NULL
+static struct bq20z75_platform_data *bq20z75_of_populate_pdata(
+ struct i2c_client *client)
+{
+ return client->dev.platform_data;
+}
+#endif
+
static int __devinit bq20z75_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -642,6 +714,8 @@ static int __devinit bq20z75_probe(struct i2c_client *client,
bq20z75_device->power_supply.external_power_changed =
bq20z75_external_power_changed;
+ pdata = bq20z75_of_populate_pdata(client);
+
if (pdata) {
bq20z75_device->gpio_detect =
gpio_is_valid(pdata->battery_detect);
@@ -775,6 +849,7 @@ static struct i2c_driver bq20z75_battery_driver = {
.id_table = bq20z75_id,
.driver = {
.name = "bq20z75-battery",
+ .of_match_table = bq20z75_dt_ids,
},
};
--
1.7.6
^ permalink raw reply related
* [PATCH 2/2] input: samsung-keypad: Add device tree support
From: Thomas Abraham @ 2011-09-14 18:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110914171318.GA24425@ponder.secretlab.ca>
Hi Grant,
On 14 September 2011 22:43, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Wed, Sep 14, 2011 at 10:19:22PM +0530, Thomas Abraham wrote:
>> Hi Grant,
>>
>> On 14 September 2011 21:41, Grant Likely <grant.likely@secretlab.ca> wrote:
>> > On Tue, Sep 13, 2011 at 05:56:19PM +0530, Thomas Abraham wrote:
>> >> Add device tree based discovery support for Samsung's keypad controller.
>> >>
>> >> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
>> >> Cc: Donghwa Lee <dh09.lee@samsung.com>
>> >> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
>> >> ---
>> >> ?.../devicetree/bindings/input/samsung-keypad.txt ? | ? 88 ++++++++++
>> >> ?drivers/input/keyboard/samsung-keypad.c ? ? ? ? ? ?| ?177 ++++++++++++++++++--
>> >> ?2 files changed, 253 insertions(+), 12 deletions(-)
>> >> ?create mode 100644 Documentation/devicetree/bindings/input/samsung-keypad.txt
>> >>
[...]
>> >> +- Keys represented as child nodes: Each key connected to the keypad
>> >> + ?controller is represented as a child node to the keypad controller
>> >> + ?device node and should include the following properties.
>> >> + ?- keypad,row: the row number to which the key is connected.
>> >> + ?- keypad,column: the column number to which the key is connected.
>> >> + ?- keypad,key-code: the key-code to be reported when the key is pressed
>> >> + ? ?and released.
>> >
>> > What defines the meanings of the key codes?
>>
>> The key-code could be any value which the system would want the keypad
>> driver to report when that key is pressed.
>
> Are they linux keycodes? ?If so, then this property name can
> probably be linux,code. ?There is already precedence for that
> usage in Documentation/devicetree/bindings/gpio/gpio-keys.txt. ?(I
> would personally prefer "linux,key-code", but sometimes it is better
> to go with existing precidence) You could also use linux,input-type as
> specified in that binding.
Ok. For linux, "keypad,key-code" would mean linux keycodes. The
property name 'keypad,key-code' was chosen since it can be reused on
non-linux platforms as well. I did have a look at
Documentation/devicetree/bindings/gpio/gpio-keys.txt while doing this,
but preferred using 'keypad,key-code' since it would be generic. Given
a choice, I would like to retain this.
>
>>
>> >
>> >> +
>> >> +Optional Properties specific to linux:
>> >> +- linux,keypad-no-autorepeat: do no enable autorepeat feature.
>> >> +- linux,keypad-wakeup: use any event on keypad as wakeup event.
>> >
>> > Is this really a linux-specific feature?
>>
>> Yes, it is a property defined by the linux subsystems. A non-linux
>> system might not have such features.
>
> I was specifically referring to the wakeup property, but okay, I can
> see the argument that it is linux-specific because it is usage-policy
> related.
>
> g.
Thanks for your review.
Regards,
Thomas.
^ permalink raw reply
* [PATCH 5/5] ARM: gic: add OF based initialization
From: Marc Zyngier @ 2011-09-14 18:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4E70EB1F.4060000@gmail.com>
Hi Rob,
On 14/09/11 18:57, Rob Herring wrote:
> Marc,
>
> On 09/14/2011 12:46 PM, Marc Zyngier wrote:
>> On 14/09/11 17:31, Rob Herring wrote:
>>> From: Rob Herring <rob.herring@calxeda.com>
>>>
>>> This adds gic initialization using device tree data. The initialization
>>> functions are intended to be called by a generic OF interrupt
>>> controller parsing function once the right pieces are in place.
>>>
>>> PPIs are handled using 3rd cell of interrupts properties to specify the cpu
>>> mask the PPI is assigned to.
>>>
>>> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
>>> ---
>>> Documentation/devicetree/bindings/arm/gic.txt | 53 ++++++++++++++++++++++++
>>> arch/arm/common/gic.c | 55 +++++++++++++++++++++++--
>>> arch/arm/include/asm/hardware/gic.h | 10 +++++
>>> 3 files changed, 114 insertions(+), 4 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
>>> new file mode 100644
>>> index 0000000..6c513de
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/arm/gic.txt
>>> @@ -0,0 +1,53 @@
>>> +* ARM Generic Interrupt Controller
>>> +
>>> +ARM SMP cores are often associated with a GIC, providing per processor
>>> +interrupts (PPI), shared processor interrupts (SPI) and software
>>> +generated interrupts (SGI).
>>> +
>>> +Primary GIC is attached directly to the CPU and typically has PPIs and SGIs.
>>> +Secondary GICs are cascaded into the upward interrupt controller and do not
>>> +have PPIs or SGIs.
>>> +
>>> +Main node required properties:
>>> +
>>> +- compatible : should be one of:
>>> + "arm,cortex-a9-gic"
>>> + "arm,arm11mp-gic"
>>> +- interrupt-controller : Identifies the node as an interrupt controller
>>> +- #interrupt-cells : Specifies the number of cells needed to encode an
>>> + interrupt source. The type shall be a <u32> and the value shall be 3.
>>> +
>>> + The 1st cell is the interrupt number. 0-15 are reserved for SGIs. 16-31 are
>>> + for PPIs.
>>> +
>>> + The 2nd cell is the level-sense information, encoded as follows:
>>> + 1 = low-to-high edge triggered
>>> + 2 = high-to-low edge triggered
>>> + 4 = active high level-sensitive
>>> + 8 = active low level-sensitive
>>> +
>>> + Only values of 1 and 4 are valid for GIC 1.0 spec.
>>> +
>>> + The 3rd cell contains the mask of the cpu number for the interrupt source.
>>> + The cpu mask is only valid for PPIs and shall be 0 for SPIs. This value shall
>>> + be 0 for PPIs.
>> ^^^^^^^^^^^^^
>>
>> Typo here ? The way I understand it, it should read "For PPIs, this
>> value shall be the mask of the possible CPU numbers for the interrupt
>> source" (or something to similar effect...).
>>
>
> Cut and paste error. This sentence goes in the previous paragraph. What
> I meant is the 2nd cell should contain 0 for PPIs as you cannot set the
> edge/level on PPIs (that is always true, right?). I probably should also
> add 0 in the list of values.
Ah, right. It makes sense indeed. You're correct about PPIs polarity,
this is defined by the hardware and cannot be configured. But it may be
interesting to have the DT to reflect the way the hardware is actually
configured (on the Cortex-A9, some PPIs are configured active-low, and
others are rising-edge).
> I take it you are otherwise fine with this binding?
I very much like the fact that it (or at least that's the way I
understand it...) allows for a very compact expression of peripherals
connected to PPIs.
What I'd like to write is the following:
twd at 1f000600 {
compatible = "arm,11mpcore-twd";
reg = <0x1f000600 0x100>;
interrupt-parent = <&intc>;
interrupt = <29 0 0xf>;
}
where 0xf would indicate that the TWD is connected to all four cores. Is
that correct?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH/RFC] mm: add vm_area_add_early()
From: Nicolas Pitre @ 2011-09-14 18:35 UTC (permalink / raw)
To: linux-arm-kernel
The existing vm_area_register_early() allows for early vmalloc space
allocation. However upcoming cleanups in the ARM architecture require
that some fixed locations in the vmalloc area be reserved also very early.
The name "vm_area_register_early" would have been a good name for the
reservation part without the allocation. Since it is already in use with
different semantics, let's create vm_area_add_early() instead.
Both vm_area_register_early() and vm_area_add_early() can be used together
meaning that the former is now implemented using the later where it is
ensured that no conflicting areas are added, but no attempt is made to
make the allocation scheme in vm_area_register_early() more sophisticated.
After all, you must know what you're doing when using those functions.
Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
---
Comments / ACKs appreciated.
diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 9332e52ea8..e7d2cba995 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -130,6 +130,7 @@ extern long vwrite(char *buf, char *addr, unsigned long count);
*/
extern rwlock_t vmlist_lock;
extern struct vm_struct *vmlist;
+extern __init void vm_area_add_early(struct vm_struct *vm);
extern __init void vm_area_register_early(struct vm_struct *vm, size_t align);
#ifdef CONFIG_SMP
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 7ef0903058..bf20a0ff95 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1118,6 +1118,31 @@ void *vm_map_ram(struct page **pages, unsigned int count, int node, pgprot_t pro
EXPORT_SYMBOL(vm_map_ram);
/**
+ * vm_area_add_early - add vmap area early during boot
+ * @vm: vm_struct to add
+ *
+ * This function is used to add fixed kernel vm area to vmlist before
+ * vmalloc_init() is called. @vm->addr, @vm->size, and @vm->flags
+ * should contain proper values and the other fields should be zero.
+ *
+ * DO NOT USE THIS FUNCTION UNLESS YOU KNOW WHAT YOU'RE DOING.
+ */
+void __init vm_area_add_early(struct vm_struct *vm)
+{
+ struct vm_struct *tmp, **p;
+
+ for (p = &vmlist; (tmp = *p) != NULL; p = &tmp->next) {
+ if (tmp->addr >= vm->addr) {
+ BUG_ON(tmp->addr < vm->addr + vm->size);
+ break;
+ } else
+ BUG_ON(tmp->addr + tmp->size > vm->addr);
+ }
+ vm->next = *p;
+ *p = vm;
+}
+
+/**
* vm_area_register_early - register vmap area early during boot
* @vm: vm_struct to register
* @align: requested alignment
@@ -1139,8 +1164,7 @@ void __init vm_area_register_early(struct vm_struct *vm, size_t align)
vm->addr = (void *)addr;
- vm->next = vmlist;
- vmlist = vm;
+ vm_area_add_early(vm);
}
void __init vmalloc_init(void)
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox