* [PATCH v2 2/9] dt-bindings: interrupt-controller: add DT binding for meson GPIO interrupt controller
From: Jerome Brunet @ 2016-10-19 15:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476890480-8884-1-git-send-email-jbrunet@baylibre.com>
This commit adds the device tree bindings description for Amlogic's GPIO
interrupt controller available on the meson8, meson8b and gxbb SoC families
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
Rob, I did not include the Ack you gave for the RFC as bindings have slightly
changed. Only the interrupt property has be removed following a discussion I
had with Marc
.../amlogic,meson-gpio-intc.txt | 31 ++++++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/amlogic,meson-gpio-intc.txt
diff --git a/Documentation/devicetree/bindings/interrupt-controller/amlogic,meson-gpio-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/amlogic,meson-gpio-intc.txt
new file mode 100644
index 000000000000..2464d9a0865d
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/amlogic,meson-gpio-intc.txt
@@ -0,0 +1,31 @@
+Amlogic meson GPIO interrupt controller
+
+Meson SoCs contains an interrupt controller which is able watch the SoC pads
+and generate an interrupt on edges or level. The controller is essentially a
+256 pads to 8 GIC interrupt multiplexer, with a filter block to select edge
+or level and polarity. We don?t expose all 256 mux inputs because the
+documentation shows that upper part is not mapped to any pad. The actual number
+of interrupt exposed depends on the SoC.
+
+Required properties:
+
+- compatible : should be either
+ "amlogic,meson8-gpio-intc? for meson8 SoCs (AML7826MX) or
+ ?amlogic,meson8b-gpio-intc? for meson8b SoCs (S805) or
+ ?amlogic,meson-gxbb-gpio-intc? for GXBB SoCs (S905)
+- interrupt-parent : a phandle to the GIC the interrupts are routed to.
+ Usually this is provided at the root level of the device tree as it is
+ common to most of the SoC
+- reg : Specifies base physical address and size of the registers.
+- interrupt-controller : Identifies the node as an interrupt controller.
+- #interrupt-cells : Specifies the number of cells needed to encode an
+ interrupt source. The value must be 2.
+
+Example:
+
+gpio_interrupt: interrupt-controller at 9880 {
+ compatible = "amlogic,meson-gxbb-gpio-intc";
+ reg = <0x0 0x9880 0x0 0x10>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+};
--
2.7.4
^ permalink raw reply related
* [PATCH v2 1/9] irqchip: meson: add support for gpio interrupt controller
From: Jerome Brunet @ 2016-10-19 15:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476890480-8884-1-git-send-email-jbrunet@baylibre.com>
Add support for the interrupt gpio controller found on Amlogic's meson
SoC family.
Unlike what the IP name suggest, it is not directly linked to the gpio
subsystem. It is actually an independent IP that is able to spy on the
SoC pad. For that purpose, it can mux and filter (edge or level and
polarity) any single SoC pad to one of the 8 GIC's interrupts it owns.
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/irqchip/Kconfig | 9 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-meson-gpio.c | 423 +++++++++++++++++++++++++++++++++++++++
3 files changed, 433 insertions(+)
create mode 100644 drivers/irqchip/irq-meson-gpio.c
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 82b0b5daf3f5..168837263e80 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -279,3 +279,12 @@ config EZNPS_GIC
config STM32_EXTI
bool
select IRQ_DOMAIN
+
+config MESON_GPIO_IRQ
+ bool "Meson GPIO Interrupt Multiplexer"
+ depends on ARCH_MESON || COMPILE_TEST
+ select IRQ_DOMAIN
+ select IRQ_DOMAIN_HIERARCHY
+ help
+ Support Meson SoC Family GPIO Interrupt Multiplexer
+
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e4dbfc85abdb..33f913d037d0 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -74,3 +74,4 @@ obj-$(CONFIG_LS_SCFG_MSI) += irq-ls-scfg-msi.o
obj-$(CONFIG_EZNPS_GIC) += irq-eznps.o
obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o
obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o
+obj-$(CONFIG_MESON_GPIO_IRQ) += irq-meson-gpio.o
diff --git a/drivers/irqchip/irq-meson-gpio.c b/drivers/irqchip/irq-meson-gpio.c
new file mode 100644
index 000000000000..869b4df8c483
--- /dev/null
+++ b/drivers/irqchip/irq-meson-gpio.c
@@ -0,0 +1,423 @@
+/*
+ * Copyright (c) 2015 Endless Mobile, Inc.
+ * Author: Carlo Caione <carlo@endlessm.com>
+ * Copyright (c) 2016 BayLibre, SAS.
+ * Author: Jerome Brunet <jbrunet@baylibre.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * 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/>.
+ * The full GNU General Public License is included in this distribution
+ * in the file called COPYING.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/irqchip.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+#define IRQ_FREE (-1)
+
+#define REG_EDGE_POL 0x00
+#define REG_PIN_03_SEL 0x04
+#define REG_PIN_47_SEL 0x08
+#define REG_FILTER_SEL 0x0c
+
+#define REG_EDGE_POL_MASK(x) (BIT(x) | BIT(16 + (x)))
+#define REG_EDGE_POL_EDGE(x) BIT(x)
+#define REG_EDGE_POL_LOW(x) BIT(16 + (x))
+#define REG_PIN_SEL_SHIFT(x) (((x) % 4) * 8)
+#define REG_FILTER_SEL_SHIFT(x) ((x) * 4)
+
+struct meson_gpio_irq_params {
+ unsigned int nhwirq;
+ irq_hw_number_t *source;
+ int nsource;
+};
+
+struct meson_gpio_irq_domain {
+ void __iomem *base;
+ int *map;
+ const struct meson_gpio_irq_params *params;
+};
+
+struct meson_gpio_irq_chip_data {
+ void __iomem *base;
+ int index;
+};
+
+static irq_hw_number_t meson_parent_hwirqs[] = {
+ 64, 65, 66, 67, 68, 69, 70, 71,
+};
+
+static const struct meson_gpio_irq_params meson8_params = {
+ .nhwirq = 134,
+ .source = meson_parent_hwirqs,
+ .nsource = ARRAY_SIZE(meson_parent_hwirqs),
+};
+
+static const struct meson_gpio_irq_params meson8b_params = {
+ .nhwirq = 119,
+ .source = meson_parent_hwirqs,
+ .nsource = ARRAY_SIZE(meson_parent_hwirqs),
+};
+
+static const struct meson_gpio_irq_params meson_gxbb_params = {
+ .nhwirq = 133,
+ .source = meson_parent_hwirqs,
+ .nsource = ARRAY_SIZE(meson_parent_hwirqs),
+};
+
+static const struct of_device_id meson_irq_gpio_matches[] = {
+ {
+ .compatible = "amlogic,meson8-gpio-intc",
+ .data = &meson8_params
+ },
+ {
+ .compatible = "amlogic,meson8b-gpio-intc",
+ .data = &meson8b_params
+ },
+ {
+ .compatible = "amlogic,meson-gxbb-gpio-intc",
+ .data = &meson_gxbb_params
+ },
+ {}
+};
+
+static void meson_gpio_irq_update_bits(void __iomem *base, unsigned int reg,
+ u32 mask, u32 val)
+{
+ u32 tmp;
+
+ tmp = readl(base + reg);
+ tmp &= ~mask;
+ tmp |= val;
+
+ writel(tmp, base + reg);
+}
+
+static int meson_gpio_irq_get_index(struct meson_gpio_irq_domain *domain_data,
+ int hwirq)
+{
+ int i;
+
+ for (i = 0; i < domain_data->params->nsource; i++) {
+ if (domain_data->map[i] == hwirq)
+ return i;
+ }
+
+ return -1;
+}
+
+static int mesion_gpio_irq_map_source(struct meson_gpio_irq_domain *domain_data,
+ irq_hw_number_t hwirq,
+ irq_hw_number_t *source)
+{
+ int index;
+ unsigned int reg;
+
+ index = meson_gpio_irq_get_index(domain_data, IRQ_FREE);
+ if (index < 0) {
+ pr_err("No irq available\n");
+ return -ENOSPC;
+ }
+
+ domain_data->map[index] = hwirq;
+
+ reg = (index < 4) ? REG_PIN_03_SEL : REG_PIN_47_SEL;
+ meson_gpio_irq_update_bits(domain_data->base, reg,
+ 0xff << REG_PIN_SEL_SHIFT(index),
+ hwirq << REG_PIN_SEL_SHIFT(index));
+
+ *source = domain_data->params->source[index];
+
+ pr_debug("hwirq %lu assigned to channel %d - source %lu\n",
+ hwirq, index, *source);
+
+ return index;
+}
+
+static int meson_gpio_irq_type_setup(unsigned int type, void __iomem *base,
+ int index)
+{
+ u32 val = 0;
+
+ type &= IRQ_TYPE_SENSE_MASK;
+
+ if (type == IRQ_TYPE_EDGE_BOTH)
+ return -EINVAL;
+
+ if (type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING))
+ val |= REG_EDGE_POL_EDGE(index);
+
+ if (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_EDGE_FALLING))
+ val |= REG_EDGE_POL_LOW(index);
+
+ meson_gpio_irq_update_bits(base, REG_EDGE_POL,
+ REG_EDGE_POL_MASK(index), val);
+
+ return 0;
+}
+
+static unsigned int meson_gpio_irq_type_output(unsigned int type)
+{
+ unsigned int sense = type & IRQ_TYPE_SENSE_MASK;
+
+ type &= ~IRQ_TYPE_SENSE_MASK;
+
+ /*
+ * If the polarity of interrupt is low, the controller will
+ * invert the signal for gic
+ */
+ if (sense & (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW))
+ type |= IRQ_TYPE_LEVEL_HIGH;
+ else if (sense & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING))
+ type |= IRQ_TYPE_EDGE_RISING;
+
+ return type;
+}
+
+static int meson_gpio_irq_set_type(struct irq_data *data, unsigned int type)
+{
+ struct meson_gpio_irq_chip_data *cd = irq_data_get_irq_chip_data(data);
+ int ret;
+
+ pr_debug("set type of hwirq %lu to %u\n", data->hwirq, type);
+
+ ret = meson_gpio_irq_type_setup(type, cd->base, cd->index);
+ if (ret)
+ return ret;
+
+ return irq_chip_set_type_parent(data,
+ meson_gpio_irq_type_output(type));
+}
+
+static struct irq_chip meson_gpio_irq_chip = {
+ .name = "meson-gpio-irqchip",
+ .irq_mask = irq_chip_mask_parent,
+ .irq_unmask = irq_chip_unmask_parent,
+ .irq_eoi = irq_chip_eoi_parent,
+ .irq_set_type = meson_gpio_irq_set_type,
+ .irq_retrigger = irq_chip_retrigger_hierarchy,
+#ifdef CONFIG_SMP
+ .irq_set_affinity = irq_chip_set_affinity_parent,
+#endif
+};
+
+static int meson_gpio_irq_domain_translate(struct irq_domain *domain,
+ struct irq_fwspec *fwspec,
+ unsigned long *hwirq,
+ unsigned int *type)
+{
+ if (is_of_node(fwspec->fwnode)) {
+ if (fwspec->param_count != 2)
+ return -EINVAL;
+
+ *hwirq = fwspec->param[0];
+ *type = fwspec->param[1];
+
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int meson_gpio_irq_allocate_gic_irq(struct irq_domain *domain,
+ unsigned int virq,
+ irq_hw_number_t source,
+ unsigned int type)
+{
+ struct irq_fwspec fwspec;
+
+ if (!irq_domain_get_of_node(domain->parent))
+ return -EINVAL;
+
+ fwspec.fwnode = domain->parent->fwnode;
+ fwspec.param_count = 3;
+ fwspec.param[0] = 0; /* SPI */
+ fwspec.param[1] = source;
+ fwspec.param[2] = meson_gpio_irq_type_output(type);
+
+ return irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
+}
+
+static int meson_gpio_irq_domain_alloc(struct irq_domain *domain,
+ unsigned int virq,
+ unsigned int nr_irqs,
+ void *data)
+{
+ struct irq_fwspec *fwspec = data;
+ struct meson_gpio_irq_domain *domain_data = domain->host_data;
+ struct meson_gpio_irq_chip_data *cd;
+ unsigned long hwirq, source;
+ unsigned int type;
+ int i, index, ret;
+
+ ret = meson_gpio_irq_domain_translate(domain, fwspec, &hwirq, &type);
+ if (ret)
+ return ret;
+
+ pr_debug("irq %d, nr_irqs %d, hwirqs %lu\n", virq, nr_irqs, hwirq);
+
+ for (i = 0; i < nr_irqs; i++) {
+ index = mesion_gpio_irq_map_source(domain_data, hwirq + i,
+ &source);
+ if (index < 0)
+ return index;
+
+ ret = meson_gpio_irq_type_setup(type, domain_data->base,
+ index);
+ if (ret)
+ return ret;
+
+ cd = kzalloc(sizeof(*cd), GFP_KERNEL);
+ if (!cd)
+ return -ENOMEM;
+
+ cd->base = domain_data->base;
+ cd->index = index;
+
+ irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
+ &meson_gpio_irq_chip, cd);
+
+ ret = meson_gpio_irq_allocate_gic_irq(domain, virq + i,
+ source, type);
+ if (ret < 0)
+ return ret;
+ }
+
+ return 0;
+}
+
+static void meson_gpio_irq_domain_free(struct irq_domain *domain,
+ unsigned int virq,
+ unsigned int nr_irqs)
+{
+ struct meson_gpio_irq_domain *domain_data = domain->host_data;
+ struct meson_gpio_irq_chip_data *cd;
+ struct irq_data *irq_data;
+ int i;
+
+ for (i = 0; i < nr_irqs; i++) {
+ irq_data = irq_domain_get_irq_data(domain, virq + i);
+ cd = irq_data_get_irq_chip_data(irq_data);
+
+ domain_data->map[cd->index] = IRQ_FREE;
+ kfree(cd);
+ }
+
+ irq_domain_free_irqs_parent(domain, virq, nr_irqs);
+
+}
+
+static const struct irq_domain_ops meson_gpio_irq_domain_ops = {
+ .alloc = meson_gpio_irq_domain_alloc,
+ .free = meson_gpio_irq_domain_free,
+ .translate = meson_gpio_irq_domain_translate,
+};
+
+static int __init
+meson_gpio_irq_init_domain(struct device_node *node,
+ struct meson_gpio_irq_domain *domain_data,
+ const struct meson_gpio_irq_params *params)
+{
+ int i;
+ int nsource = params->nsource;
+ int *map;
+
+ map = kcalloc(nsource, sizeof(*map), GFP_KERNEL);
+ if (!map)
+ return -ENOMEM;
+
+ for (i = 0; i < nsource; i++)
+ map[i] = IRQ_FREE;
+
+ domain_data->map = map;
+ domain_data->params = params;
+
+ return 0;
+}
+
+static int __init meson_gpio_irq_of_init(struct device_node *node,
+ struct device_node *parent)
+{
+ struct irq_domain *domain, *parent_domain;
+ const struct of_device_id *match;
+ const struct meson_gpio_irq_params *params;
+ struct meson_gpio_irq_domain *domain_data;
+ int ret;
+
+ match = of_match_node(meson_irq_gpio_matches, node);
+ if (!match)
+ return -ENODEV;
+ params = match->data;
+
+ if (!parent) {
+ pr_err("missing parent interrupt node\n");
+ return -ENODEV;
+ }
+
+ parent_domain = irq_find_host(parent);
+ if (!parent_domain) {
+ pr_err("unable to obtain parent domain\n");
+ return -ENXIO;
+ }
+
+ domain_data = kzalloc(sizeof(*domain_data), GFP_KERNEL);
+ if (!domain_data)
+ return -ENOMEM;
+
+ domain_data->base = of_iomap(node, 0);
+ if (!domain_data->base) {
+ ret = -ENOMEM;
+ goto out_free_dev;
+ }
+
+ ret = meson_gpio_irq_init_domain(node, domain_data, params);
+ if (ret < 0)
+ goto out_free_dev_content;
+
+ domain = irq_domain_add_hierarchy(parent_domain, 0, params->nhwirq,
+ node, &meson_gpio_irq_domain_ops,
+ domain_data);
+
+ if (!domain) {
+ pr_err("failed to allocated domain\n");
+ ret = -ENOMEM;
+ goto out_free_dev_content;
+ }
+
+ pr_info("%d to %d gpio interrupt mux initialized\n",
+ params->nhwirq, params->nsource);
+
+ return 0;
+
+out_free_dev_content:
+ kfree(domain_data->map);
+ iounmap(domain_data->base);
+
+out_free_dev:
+ kfree(domain_data);
+
+ return ret;
+}
+
+IRQCHIP_DECLARE(meson8_gpio_intc, "amlogic,meson8-gpio-intc",
+ meson_gpio_irq_of_init);
+IRQCHIP_DECLARE(meson8b_gpio_intc, "amlogic,meson8b-gpio-intc",
+ meson_gpio_irq_of_init);
+IRQCHIP_DECLARE(gxbb_gpio_intc, "amlogic,meson-gxbb-gpio-intc",
+ meson_gpio_irq_of_init);
--
2.7.4
^ permalink raw reply related
* [PATCH v2 0/9] irqchip: meson: add support for the gpio interrupt controller
From: Jerome Brunet @ 2016-10-19 15:21 UTC (permalink / raw)
To: linux-arm-kernel
This patch series adds support for the GPIO interrupt controller found
on Amlogic's meson SoC families.
Unlike what the name suggests, this controller is not part of the SoC GPIO
subsystem. It's an indepedent controller which can watch almost all pad of
the SoC and generate and interrupt from it. Some pins, which are not part
of the public datasheet, don't seem to have this capability though.
Hardware wise, the controller is a 256 to 8 multiplexer. It can take up
to 256 input pads and route them to any of 8 GIC's interrupts. There is
also a filter block in the middle to select the appropriate edge or level.
The number of interrupt declared by the irqchip is lowered from 256 to the
actual number of signal routed to the controller on each SoC family. As we
have access to only 8 GIC?s interrupts, these are allocated when an
interrupt is requested from the controller, on a first come, first served
basis.
This series has been tested on Amlogic S905-P200 board with the front
panel power button. Directly passing an IRQ or using gpio_to_irq both work
with this driver.
This work is derived from the previous work of Carlo Caione [1].
Changes since RFC : [2]
* Remove interrupt property in device tree: the controller cannot generate
interrupts on its own and is merely routing the interrupt to the GIC,
therefore it should not use the interrupt property. This data is now
stored directly in the driver, same as the pinctrl data.
* Improve compatibility checking of meson pinctrl on its interrupt parent
to activate gpio_to_irq callback
* Drop IRQ_BOTH hack. Need more work to have an acceptable solution for
this
Changes since v1 : [3]
* Correct mistake in patch 4 when no compatible controller is found. Sorry
for the inconvenience.
[1] : http://lkml.kernel.org/r/1448987062-31225-1-git-send-email-carlo at caione.org
[2] : http://lkml.kernel.org/r/1475593708-10526-1-git-send-email-jbrunet at baylibre.com
[3] : http://lkml.kernel.org/r/1476871709-8359-1-git-send-email-jbrunet at baylibre.com
Jerome Brunet (9):
irqchip: meson: add support for gpio interrupt controller
dt-bindings: interrupt-controller: add DT binding for meson GPIO
interrupt controller
pinctrl: meson: update pinctrl data with gpio irq data
pinctrl: meson: allow gpio to request irq
dt-bindings: pinctrl: meson: update gpio dt-bindings
ARM64: meson: enable MESON_IRQ_GPIO in Kconfig
ARM: meson: enable MESON_IRQ_GPIO in Kconfig for meson8
ARM64: dts: amlogic: enable gpio interrupt controller on gxbb
ARM: dts: amlogic: enable gpio interrupt controller on meson8
.../amlogic,meson-gpio-intc.txt | 31 ++
.../devicetree/bindings/pinctrl/meson,pinctrl.txt | 4 +
arch/arm/boot/dts/meson8.dtsi | 11 +
arch/arm/boot/dts/meson8b.dtsi | 11 +
arch/arm/mach-meson/Kconfig | 2 +
arch/arm64/Kconfig.platforms | 1 +
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 9 +
drivers/irqchip/Kconfig | 9 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-meson-gpio.c | 423 +++++++++++++++++++++
drivers/pinctrl/Kconfig | 2 +
drivers/pinctrl/meson/pinctrl-meson-gxbb.c | 24 +-
drivers/pinctrl/meson/pinctrl-meson.c | 77 +++-
drivers/pinctrl/meson/pinctrl-meson.h | 17 +-
drivers/pinctrl/meson/pinctrl-meson8.c | 22 +-
drivers/pinctrl/meson/pinctrl-meson8b.c | 34 +-
16 files changed, 641 insertions(+), 37 deletions(-)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/amlogic,meson-gpio-intc.txt
create mode 100644 drivers/irqchip/irq-meson-gpio.c
--
2.7.4
^ permalink raw reply
* Build failure with v4.9-rc1 and GCC trunk -- compiler weirdness
From: Arnd Bergmann @ 2016-10-19 15:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu_wJWCtQGfQv2V_tNq+zygMzbassEa92c5sx90w0TW4vw@mail.gmail.com>
On Wednesday, October 19, 2016 4:01:58 PM CEST Ard Biesheuvel wrote:
> On 19 October 2016 at 15:59, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > On 19 October 2016 at 14:35, Will Deacon <will.deacon@arm.com> wrote:
> >> On Mon, Oct 17, 2016 at 08:43:19PM +0100, Ard Biesheuvel wrote:
> >>> On 17 October 2016 at 19:38, Will Deacon <will.deacon@arm.com> wrote:
> >
> > Yes, and that would be perfectly legal from a correctness point of
> > view, and would likely help performance as well. By using
> > __builtin_constant_p(), you are choosing to perform a build time
> > evaluation of an expression that would ordinarily be evaluated only at
> > runtime. This implies that you have to address undefined behavior at
> > build time rather than at runtime as well.
> >
> >>> If order_base_2() is not defined for input 0, it should BUG() in that
> >>> case, and the associated __builtin_unreachable() should prevent the
> >>> special version from being emitted. If order_base_2() is defined for input
> >>> 0, it should not invoke ilog2() with that argument, and the problem should
> >>> go away as well.
> >>
> >> I don't necessarily think it should BUG() if it's not defined for input
> >> 0; things like __ffs don't do that and we'd be introducing conditional
> >> checks for cases that should not happen. The comment above order_base_2
> >> does suggest that ob2(0) should return 0, but it can actually end up
> >> invoking ilog2(-1), which is obviously wrong.
> >>
> >> I could update the comment, but that doesn't fix the build issue.
> >>
> >
> > Fixing roundup_pow_of_two() [which is arguably incorrect]
>
> I just spotted the comment that says it is undefined. But that means
> it could legally return 1 for input 0, i suppose
I think having the link error in roundup_pow_of_two() is safer than
returning 1.
Why not turn it into a runtime warning in this driver?
diff --git a/drivers/clk/mvebu/armada-37xx-periph.c b/drivers/clk/mvebu/armada-37xx-periph.c
index cecb0fdfaef6..711d1d9842cc 100644
--- a/drivers/clk/mvebu/armada-37xx-periph.c
+++ b/drivers/clk/mvebu/armada-37xx-periph.c
@@ -349,8 +349,10 @@ static int armada_3700_add_composite_clk(const struct clk_periph_data *data,
rate->reg = reg + (u64)rate->reg;
for (clkt = rate->table; clkt->div; clkt++)
table_size++;
- rate->width = order_base_2(table_size);
- rate->lock = lock;
+ if (!WARN_ON(table_size == 0)) {
+ rate->width = order_base_2(table_size);
+ rate->lock = lock;
+ }
}
}
Arnd
^ permalink raw reply related
* Build failure with v4.9-rc1 and GCC trunk -- compiler weirdness
From: Ard Biesheuvel @ 2016-10-19 15:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu_ts83bKc4mYujf7NnzGVHoxP7TXgHbFxSgb9LkxH=wXQ@mail.gmail.com>
On 19 October 2016 at 15:59, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 19 October 2016 at 14:35, Will Deacon <will.deacon@arm.com> wrote:
>> Hi Ard,
>>
>> On Mon, Oct 17, 2016 at 08:43:19PM +0100, Ard Biesheuvel wrote:
>>> On 17 October 2016 at 19:38, Will Deacon <will.deacon@arm.com> wrote:
>>> > I'm seeing an arm64 build failure with -rc1 and GCC trunk, although I
>>> > believe that the new compiler behaviour at the heart of the problem
>>> > has the potential to affect other architectures and other pieces of
>>> > kernel code relying on dead-code elimination to remove deliberately
>>> > undefined functions.
>>> >
>>> > The failure looks like:
>>> >
>>> > | drivers/built-in.o: In function `armada_3700_add_composite_clk':
>>> > |
>>> > | linux/drivers/clk/mvebu/armada-37xx-periph.c:351:
>>> > | undefined reference to `____ilog2_NaN'
>>> > |
>>> > | linux/drivers/clk/mvebu/armada-37xx-periph.c:351:(.text+0xc72e0):
>>> > | relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol
>>> > | `____ilog2_NaN'
>>> > |
>>> > | make: *** [vmlinux] Error 1
>>> >
>>> > and if we look at the source for armada_3700_add_composite_clk, we see
>>> > that this is caused by:
>>> >
>>> > int table_size = 0;
>>> >
>>> > rate->reg = reg + (u64)rate->reg;
>>> > for (clkt = rate->table; clkt->div; clkt++)
>>> > table_size++;
>>> > rate->width = order_base_2(table_size);
>>> >
>>> > order_base_2 calls ilog2, which has the ____ilog2_NaN call:
>>> >
>>> > #define ilog2(n) \
>>> > ( \
>>> > __builtin_constant_p(n) ? ( \
>>> > (n) < 1 ? ____ilog2_NaN() : \
>>> >
>>> > This is because we're in a curious case where GCC has emitted a
>>> > special-cased version of armada_3700_add_composite_clk, with table_size
>>> > effectively constant-folded as 0. Whilst we shouldn't see this in a
>>> > non-buggy kernel (hence the deliberate call to the undefined function
>>> > ____ilog2_NaN), it means that the final link fails because we have a
>>> > ____ilog2_NaN in the code, with a runtime check on table_size.
>>> >
>>>
>>> This is indeed an unintended side effect, but I would not call it
>>> weird behaviour at all. The code in its current form does not handle
>>> the case where it could end up passing 0 into order_base_2(), and we
>>> simply need to handle that case.
>>
>> The reasons I think it's weird are:
>>
>> (1) The optimisation doesn't generate better code in this case --
>> optimising for the table_size == 0 case is uninformed, particularly
>> as that *cannot* happen at runtime (GCC probably can't tell, due
>> to things like container_of, but all the clock data is static).
>>
>
> AFAICT, the references to the static clock data are indirected via
> of_device_get_match_data(), which means there is no way the compiler
> can prove that table_size is always non-zero.
>
>> (2) __builtin_constant_p(n) could be interpreted by a developer as
>> "this code will execute with a constant n at runtime". With this
>> issue, GCC could (in theory) generate a specialisation for every
>> possible value of a variable, and return __builtin_constant_p as
>> true for all of them, which somewhat undermines the point of the
>> builtin.
>>
>
> Yes, and that would be perfectly legal from a correctness point of
> view, and would likely help performance as well. By using
> __builtin_constant_p(), you are choosing to perform a build time
> evaluation of an expression that would ordinarily be evaluated only at
> runtime. This implies that you have to address undefined behavior at
> build time rather than at runtime as well.
>
>>> If order_base_2() is not defined for input 0, it should BUG() in that
>>> case, and the associated __builtin_unreachable() should prevent the
>>> special version from being emitted. If order_base_2() is defined for input
>>> 0, it should not invoke ilog2() with that argument, and the problem should
>>> go away as well.
>>
>> I don't necessarily think it should BUG() if it's not defined for input
>> 0; things like __ffs don't do that and we'd be introducing conditional
>> checks for cases that should not happen. The comment above order_base_2
>> does suggest that ob2(0) should return 0, but it can actually end up
>> invoking ilog2(-1), which is obviously wrong.
>>
>> I could update the comment, but that doesn't fix the build issue.
>>
>
> Fixing roundup_pow_of_two() [which is arguably incorrect]
I just spotted the comment that says it is undefined. But that means
it could legally return 1 for input 0, i suppose
> would
> probably fix the build issue as well, no?
>
> diff --git a/include/linux/log2.h b/include/linux/log2.h
> index fd7ff3d91e6a..8a4be5e4223b 100644
> --- a/include/linux/log2.h
> +++ b/include/linux/log2.h
> @@ -168,7 +168,7 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
> #define roundup_pow_of_two(n) \
> ( \
> __builtin_constant_p(n) ? ( \
> - (n == 1) ? 1 : \
> + (n <= 1) ? 1 : \
> (1UL << (ilog2((n) - 1) + 1)) \
> ) : \
> __roundup_pow_of_two(n) \
^ permalink raw reply
* Build failure with v4.9-rc1 and GCC trunk -- compiler weirdness
From: Ard Biesheuvel @ 2016-10-19 14:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161019133500.GQ9193@arm.com>
On 19 October 2016 at 14:35, Will Deacon <will.deacon@arm.com> wrote:
> Hi Ard,
>
> On Mon, Oct 17, 2016 at 08:43:19PM +0100, Ard Biesheuvel wrote:
>> On 17 October 2016 at 19:38, Will Deacon <will.deacon@arm.com> wrote:
>> > I'm seeing an arm64 build failure with -rc1 and GCC trunk, although I
>> > believe that the new compiler behaviour at the heart of the problem
>> > has the potential to affect other architectures and other pieces of
>> > kernel code relying on dead-code elimination to remove deliberately
>> > undefined functions.
>> >
>> > The failure looks like:
>> >
>> > | drivers/built-in.o: In function `armada_3700_add_composite_clk':
>> > |
>> > | linux/drivers/clk/mvebu/armada-37xx-periph.c:351:
>> > | undefined reference to `____ilog2_NaN'
>> > |
>> > | linux/drivers/clk/mvebu/armada-37xx-periph.c:351:(.text+0xc72e0):
>> > | relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol
>> > | `____ilog2_NaN'
>> > |
>> > | make: *** [vmlinux] Error 1
>> >
>> > and if we look at the source for armada_3700_add_composite_clk, we see
>> > that this is caused by:
>> >
>> > int table_size = 0;
>> >
>> > rate->reg = reg + (u64)rate->reg;
>> > for (clkt = rate->table; clkt->div; clkt++)
>> > table_size++;
>> > rate->width = order_base_2(table_size);
>> >
>> > order_base_2 calls ilog2, which has the ____ilog2_NaN call:
>> >
>> > #define ilog2(n) \
>> > ( \
>> > __builtin_constant_p(n) ? ( \
>> > (n) < 1 ? ____ilog2_NaN() : \
>> >
>> > This is because we're in a curious case where GCC has emitted a
>> > special-cased version of armada_3700_add_composite_clk, with table_size
>> > effectively constant-folded as 0. Whilst we shouldn't see this in a
>> > non-buggy kernel (hence the deliberate call to the undefined function
>> > ____ilog2_NaN), it means that the final link fails because we have a
>> > ____ilog2_NaN in the code, with a runtime check on table_size.
>> >
>>
>> This is indeed an unintended side effect, but I would not call it
>> weird behaviour at all. The code in its current form does not handle
>> the case where it could end up passing 0 into order_base_2(), and we
>> simply need to handle that case.
>
> The reasons I think it's weird are:
>
> (1) The optimisation doesn't generate better code in this case --
> optimising for the table_size == 0 case is uninformed, particularly
> as that *cannot* happen at runtime (GCC probably can't tell, due
> to things like container_of, but all the clock data is static).
>
AFAICT, the references to the static clock data are indirected via
of_device_get_match_data(), which means there is no way the compiler
can prove that table_size is always non-zero.
> (2) __builtin_constant_p(n) could be interpreted by a developer as
> "this code will execute with a constant n at runtime". With this
> issue, GCC could (in theory) generate a specialisation for every
> possible value of a variable, and return __builtin_constant_p as
> true for all of them, which somewhat undermines the point of the
> builtin.
>
Yes, and that would be perfectly legal from a correctness point of
view, and would likely help performance as well. By using
__builtin_constant_p(), you are choosing to perform a build time
evaluation of an expression that would ordinarily be evaluated only at
runtime. This implies that you have to address undefined behavior at
build time rather than at runtime as well.
>> If order_base_2() is not defined for input 0, it should BUG() in that
>> case, and the associated __builtin_unreachable() should prevent the
>> special version from being emitted. If order_base_2() is defined for input
>> 0, it should not invoke ilog2() with that argument, and the problem should
>> go away as well.
>
> I don't necessarily think it should BUG() if it's not defined for input
> 0; things like __ffs don't do that and we'd be introducing conditional
> checks for cases that should not happen. The comment above order_base_2
> does suggest that ob2(0) should return 0, but it can actually end up
> invoking ilog2(-1), which is obviously wrong.
>
> I could update the comment, but that doesn't fix the build issue.
>
Fixing roundup_pow_of_two() [which is arguably incorrect] would
probably fix the build issue as well, no?
diff --git a/include/linux/log2.h b/include/linux/log2.h
index fd7ff3d91e6a..8a4be5e4223b 100644
--- a/include/linux/log2.h
+++ b/include/linux/log2.h
@@ -168,7 +168,7 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
#define roundup_pow_of_two(n) \
( \
__builtin_constant_p(n) ? ( \
- (n == 1) ? 1 : \
+ (n <= 1) ? 1 : \
(1UL << (ilog2((n) - 1) + 1)) \
) : \
__roundup_pow_of_two(n) \
^ permalink raw reply related
* [PATCH] mtd: nand: Add OX820 NAND Support
From: Neil Armstrong @ 2016-10-19 14:55 UTC (permalink / raw)
To: linux-arm-kernel
Add NAND driver to support the Oxford Semiconductor OX820 NAND Controller.
This is a simple memory mapped NAND controller with single chip select and
software ECC.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
.../devicetree/bindings/mtd/oxnas-nand.txt | 24 +++
drivers/mtd/nand/Kconfig | 5 +
drivers/mtd/nand/Makefile | 1 +
drivers/mtd/nand/oxnas_nand.c | 204 +++++++++++++++++++++
4 files changed, 234 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mtd/oxnas-nand.txt
create mode 100644 drivers/mtd/nand/oxnas_nand.c
Changes since RFC http://lkml.kernel.org/r/20161018090927.1990-1-narmstrong at baylibre.com :
- Avoid using chip->IO_ADDR*
- Use new DT structure
- Assign a chip for the subnode
- Use the nand_hw_control structure
- Cleanup probe
- Cleanup cmd_ctrl by using a context ctrl offset used in write_bytes
diff --git a/Documentation/devicetree/bindings/mtd/oxnas-nand.txt b/Documentation/devicetree/bindings/mtd/oxnas-nand.txt
new file mode 100644
index 0000000..83b684d
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/oxnas-nand.txt
@@ -0,0 +1,24 @@
+* Oxford Semiconductor OXNAS NAND Controller
+
+Please refer to nand.txt for generic information regarding MTD NAND bindings.
+
+Required properties:
+ - compatible: "oxsemi,ox820-nand"
+ - reg: Base address and length for NAND mapped memory.
+
+Optional Properties:
+ - clocks: phandle to the NAND gate clock if needed.
+ - resets: phandle to the NAND reset control if needed.
+
+Example:
+
+nand: nand at 41000000 {
+ compatible = "oxsemi,ox820-nand";
+ reg = <0x41000000 0x100000>;
+ nand-ecc-mode = "soft";
+ clocks = <&stdclk CLK_820_NAND>;
+ resets = <&reset RESET_NAND>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ status = "disabled";
+};
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 7b7a887..c023125 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -426,6 +426,11 @@ config MTD_NAND_ORION
No board specific support is done by this driver, each board
must advertise a platform_device for the driver to attach.
+config MTD_NAND_OXNAS
+ tristate "NAND Flash support for Oxford Semiconductor SoC"
+ help
+ This enables the NAND flash controller on Oxford Semiconductor SoCs.
+
config MTD_NAND_FSL_ELBC
tristate "NAND support for Freescale eLBC controllers"
depends on FSL_SOC
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index cafde6f..05fc054 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -35,6 +35,7 @@ obj-$(CONFIG_MTD_NAND_TMIO) += tmio_nand.o
obj-$(CONFIG_MTD_NAND_PLATFORM) += plat_nand.o
obj-$(CONFIG_MTD_NAND_PASEMI) += pasemi_nand.o
obj-$(CONFIG_MTD_NAND_ORION) += orion_nand.o
+obj-$(CONFIG_MTD_NAND_OXNAS) += oxnas_nand.o
obj-$(CONFIG_MTD_NAND_FSL_ELBC) += fsl_elbc_nand.o
obj-$(CONFIG_MTD_NAND_FSL_IFC) += fsl_ifc_nand.o
obj-$(CONFIG_MTD_NAND_FSL_UPM) += fsl_upm.o
diff --git a/drivers/mtd/nand/oxnas_nand.c b/drivers/mtd/nand/oxnas_nand.c
new file mode 100644
index 0000000..a9fe1ac
--- /dev/null
+++ b/drivers/mtd/nand/oxnas_nand.c
@@ -0,0 +1,204 @@
+/*
+ * Oxford Semiconductor OXNAS NAND driver
+
+ * Copyright (C) 2016 Neil Armstrong <narmstrong@baylibre.com>
+ * Heavily based on plat_nand.c :
+ * Author: Vitaly Wool <vitalywool@gmail.com>
+ * Copyright (C) 2013 Ma Haijun <mahaijuns@gmail.com>
+ * Copyright (C) 2012 John Crispin <blogic@openwrt.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/clk.h>
+#include <linux/reset.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/nand.h>
+#include <linux/mtd/partitions.h>
+#include <linux/of.h>
+
+/* Nand commands */
+#define OXNAS_NAND_CMD_ALE BIT(18)
+#define OXNAS_NAND_CMD_CLE BIT(19)
+
+#define OXNAS_NAND_MAX_CHIPS 1
+
+struct oxnas_nand {
+ struct nand_hw_control base;
+ void __iomem *io_base;
+ struct clk *clk;
+ struct nand_chip *chips[OXNAS_NAND_MAX_CHIPS];
+ unsigned long ctrl;
+};
+
+static uint8_t oxnas_nand_read_byte(struct mtd_info *mtd)
+{
+ struct nand_chip *chip = mtd_to_nand(mtd);
+ struct oxnas_nand *oxnas = nand_get_controller_data(chip);
+
+ return readb(oxnas->io_base);
+}
+
+static void oxnas_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+ struct nand_chip *chip = mtd_to_nand(mtd);
+ struct oxnas_nand *oxnas = nand_get_controller_data(chip);
+
+ ioread8_rep(oxnas->io_base, buf, len);
+}
+
+static void oxnas_nand_write_buf(struct mtd_info *mtd,
+ const uint8_t *buf, int len)
+{
+ struct nand_chip *chip = mtd_to_nand(mtd);
+ struct oxnas_nand *oxnas = nand_get_controller_data(chip);
+
+ iowrite8_rep(oxnas->io_base + oxnas->ctrl, buf, len);
+}
+
+/* Single CS command control */
+static void oxnas_nand_cmd_ctrl(struct mtd_info *mtd, int cmd,
+ unsigned int ctrl)
+{
+ struct nand_chip *chip = mtd_to_nand(mtd);
+ struct oxnas_nand *oxnas = nand_get_controller_data(chip);
+
+ if (ctrl & NAND_CTRL_CHANGE) {
+ if (ctrl & NAND_CLE)
+ oxnas->ctrl = OXNAS_NAND_CMD_CLE;
+ else if (ctrl & NAND_ALE)
+ oxnas->ctrl = OXNAS_NAND_CMD_ALE;
+ else
+ oxnas->ctrl = 0;
+ }
+
+ if (cmd != NAND_CMD_NONE)
+ writeb(cmd, oxnas->io_base + oxnas->ctrl);
+}
+
+/*
+ * Probe for the NAND device.
+ */
+static int oxnas_nand_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct device_node *nand_np;
+ struct oxnas_nand *oxnas;
+ struct nand_chip *chip;
+ struct mtd_info *mtd;
+ struct resource *res;
+ int nchips = 0;
+ int count = 0;
+ int err = 0;
+
+ /* Allocate memory for the device structure (and zero it) */
+ oxnas = devm_kzalloc(&pdev->dev, sizeof(struct nand_chip),
+ GFP_KERNEL);
+ if (!oxnas)
+ return -ENOMEM;
+
+ nand_hw_control_init(&oxnas->base);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ oxnas->io_base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(oxnas->io_base))
+ return PTR_ERR(oxnas->io_base);
+
+ oxnas->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(oxnas->clk))
+ oxnas->clk = NULL;
+
+ /* Only a single chip node is supported */
+ count = of_get_child_count(np);
+ if (count > 1)
+ return -EINVAL;
+
+ clk_prepare_enable(oxnas->clk);
+ device_reset_optional(&pdev->dev);
+
+ for_each_child_of_node(np, nand_np) {
+ chip = devm_kzalloc(&pdev->dev, sizeof(struct nand_chip),
+ GFP_KERNEL);
+ if (!chip)
+ return -ENOMEM;
+
+ chip->controller = &oxnas->base;
+
+ nand_set_flash_node(chip, nand_np);
+ nand_set_controller_data(chip, oxnas);
+
+ mtd = nand_to_mtd(chip);
+ mtd->dev.parent = &pdev->dev;
+ mtd->priv = chip;
+
+ chip->cmd_ctrl = oxnas_nand_cmd_ctrl;
+ chip->read_buf = oxnas_nand_read_buf;
+ chip->read_byte = oxnas_nand_read_byte;
+ chip->write_buf = oxnas_nand_write_buf;
+ chip->chip_delay = 30;
+
+ /* Scan to find existence of the device */
+ err = nand_scan(mtd, 1);
+ if (err)
+ return err;
+
+ err = mtd_device_register(mtd, NULL, 0);
+ if (err) {
+ nand_release(mtd);
+ return err;
+ }
+
+ oxnas->chips[nchips] = chip;
+ ++nchips;
+ }
+
+ /* Exit if no chips found */
+ if (!nchips)
+ return -ENODEV;
+
+ platform_set_drvdata(pdev, oxnas);
+
+ return 0;
+}
+
+static int oxnas_nand_remove(struct platform_device *pdev)
+{
+ struct oxnas_nand *oxnas = platform_get_drvdata(pdev);
+
+ if (oxnas->chips[0])
+ nand_release(nand_to_mtd(oxnas->chips[0]));
+
+ clk_disable_unprepare(oxnas->clk);
+
+ return 0;
+}
+
+static const struct of_device_id oxnas_nand_match[] = {
+ { .compatible = "oxsemi,ox820-nand" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, oxnas_nand_match);
+
+static struct platform_driver oxnas_nand_driver = {
+ .probe = oxnas_nand_probe,
+ .remove = oxnas_nand_remove,
+ .driver = {
+ .name = "oxnas_nand",
+ .of_match_table = oxnas_nand_match,
+ },
+};
+
+module_platform_driver(oxnas_nand_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
+MODULE_DESCRIPTION("Oxnas NAND driver");
+MODULE_ALIAS("platform:oxnas_nand");
--
2.7.0
^ permalink raw reply related
* [PATCH v5 0/5] Add support for legacy SCPI protocol
From: Sudeep Holla @ 2016-10-19 14:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476881472-13055-1-git-send-email-narmstrong@baylibre.com>
On 19/10/16 13:51, Neil Armstrong wrote:
> This patchset aims to support the legacy SCPI firmware implementation that was
> delivered as early technology preview for the JUNO platform.
>
> Finally a stable, maintained and public implementation for the SCPI protocol
> has been upstreamed part of the JUNO support and it is the recommended way
> of implementing SCP communication on ARMv8 platforms.
>
> The Amlogic GXBB platform is using this legacy protocol, as the RK3368 & RK3399
> platforms. This patchset will only add support for Amlogic GXBB SoC.
>
> This patchset add support for the legacy protocol in the arm_scpi.c file,
> avoiding code duplication.
>
> This patchset is rebased against scpi-updates/for-next from [2] and with
> already merged patches [3], [4] and [5] and ommited in this patchset.
>
> Last RFC discution thread can be found at : https://lkml.org/lkml/2016/8/9/210
>
> Changes since v4 at : http://lkml.kernel.org/r/1475652814-30619-1-git-send-email-narmstrong at baylibre.com
> - Removed legacy locking scheme
> - Removed cmd copy back after token insert
> - Various cleanups
>
> Changes since v3 at : http://lkml.kernel.org/r/1473262477-18045-1-git-send-email-narmstrong at baylibre.com
> - Changed back author to Sudeep Holla for first patch
> - Merged legacy functions to scpi_send_message, tx_prepare and handle_remote_message
> - Added legacy locking scheme
> - Merged back legacy_scpi_sensor_get_value into scpi_sensor_get_value
> - Rebased on linux-next-20161004 with patchset [1]
>
> Changes since v2 at : http://lkml.kernel.org/r/1471952816-30877-1-git-send-email-narmstrong at baylibre.com
> - Added command indirection table and use it in each commands
> - Added bitmap for high priority commands
> - Cleaned up legacy tx_prepare/handle_message to align to standard functions
> - Dropped legacy_scpi_ops
>
> Changes since v1 at : http://lkml.kernel.org/r/1471515066-3626-1-git-send-email-narmstrong at baylibre.com
> - Dropped vendor_send_message and rockchip vendor mechanism patches
> - Merged alternate functions into main functions using is_legacy boolean
> - Added DT match table to set is_legacy to true
> - Kept alternate scpi_ops structure for legacy
>
> [1] http://lkml.kernel.org/r/1475595430-30075-1-git-send-email-narmstrong at baylibre.com
> [2] git.kernel.org/sudeep.holla/linux
> [3] scpi: Add cmd indirection table to prepare for legacy commands
> [4] scpi: grow MAX_DVFS_OPPS to 16 entries
> [5] dt-bindings: Add support for Amlogic GXBB SCPI Interface
>
> Neil Armstrong (5):
> scpi: Add alternative legacy structures, functions and macros
> scpi: Do not fail if get_capabilities is not implemented
> scpi: Add support for Legacy match table for Amlogic GXBB SoC
> ARM64: dts: meson-gxbb: Add SRAM node
> ARM64: dts: meson-gxbb: Add SCPI with cpufreq & sensors Nodes
>
> arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 57 ++++++++
> drivers/firmware/arm_scpi.c | 206 +++++++++++++++++++++++++---
> 2 files changed, 245 insertions(+), 18 deletions(-)
>
Nice to see this diff stat from a whole new file legacy_scpi.c and 1000+
delta. Thanks for working on this. I have applied the first 3 patches in
this series with some subject/commit message changes to [1].
I assume the DT changes needs to go via the corresponding platform
maintainer.
--
Regards,
Sudeep
[1] git.kernel.org/sudeep.holla/linux/h/scpi-updates/for-next
^ permalink raw reply
* [PATCH v2 0/2] ARM: at91: properly handle LPDDR poweroff
From: Nicolas Ferre @ 2016-10-19 14:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMuHMdUnoHPOQeO=oSRkNWbfLGDzZbBZvPFr3dS7TOKPQFcUxQ@mail.gmail.com>
Le 19/10/2016 ? 16:07, Geert Uytterhoeven a ?crit :
> On Wed, Oct 19, 2016 at 1:44 PM, Alexandre Belloni
> <alexandre.belloni@free-electrons.com> wrote:
>> LPDDR memories can only handle up to 400 uncontrolled power offs in their
>> life. The proper power off sequence has to be applied before shutting down the
>> SoC.
>
> Interesting. How many boards have been killed during kernelci.org
> operation?
The reason given above is usually the one that prevent manufacturers
from using LPDDR on "evaluation" boards. So all Atmel boards use
SDRAM/DDR2/DDR3 not the LP variants.
Final products (and internal validation boards, obviously) though use
these type of RAM so we must be prepared to handle them.
Best regards,
--
Nicolas Ferre
^ permalink raw reply
* [PATCH 2/2] arm64: percpu: rewrite ll/sc loops in assembly
From: Mark Rutland @ 2016-10-19 14:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476874784-16214-2-git-send-email-will.deacon@arm.com>
On Wed, Oct 19, 2016 at 11:59:44AM +0100, Will Deacon wrote:
> Writing the outer loop of an LL/SC sequence using do {...} while
> constructs potentially allows the compiler to hoist memory accesses
> between the STXR and the branch back to the LDXR. On CPUs that do not
> guarantee forward progress of LL/SC loops when faced with memory
> accesses to the same ERG (up to 2k) between the failed STXR and the
> branch back, we may end up livelocking.
>
> This patch avoids this issue in our percpu atomics by rewriting the
> outer loop as part of the LL/SC inline assembly block.
>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
The new templates look correct to me, and appear to have been duplicated
correctly for each different size of access. My machines boot happily
with this applied, so FWIW:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
I take it this (and the previous patch) will be Cc'd to stable?
As an aside, I have a local patch which gets rid of the duplication
here; I'll rebase that and send it out once this is in.
Thanks,
Mark.
> ---
> arch/arm64/include/asm/percpu.h | 120 +++++++++++++++++++---------------------
> 1 file changed, 56 insertions(+), 64 deletions(-)
>
> diff --git a/arch/arm64/include/asm/percpu.h b/arch/arm64/include/asm/percpu.h
> index 2fee2f59288c..5394c8405e66 100644
> --- a/arch/arm64/include/asm/percpu.h
> +++ b/arch/arm64/include/asm/percpu.h
> @@ -44,48 +44,44 @@ static inline unsigned long __percpu_##op(void *ptr, \
> \
> switch (size) { \
> case 1: \
> - do { \
> - asm ("//__per_cpu_" #op "_1\n" \
> - "ldxrb %w[ret], %[ptr]\n" \
> + asm ("//__per_cpu_" #op "_1\n" \
> + "1: ldxrb %w[ret], %[ptr]\n" \
> #asm_op " %w[ret], %w[ret], %w[val]\n" \
> - "stxrb %w[loop], %w[ret], %[ptr]\n" \
> - : [loop] "=&r" (loop), [ret] "=&r" (ret), \
> - [ptr] "+Q"(*(u8 *)ptr) \
> - : [val] "Ir" (val)); \
> - } while (loop); \
> + " stxrb %w[loop], %w[ret], %[ptr]\n" \
> + " cbnz %w[loop], 1b" \
> + : [loop] "=&r" (loop), [ret] "=&r" (ret), \
> + [ptr] "+Q"(*(u8 *)ptr) \
> + : [val] "Ir" (val)); \
> break; \
> case 2: \
> - do { \
> - asm ("//__per_cpu_" #op "_2\n" \
> - "ldxrh %w[ret], %[ptr]\n" \
> + asm ("//__per_cpu_" #op "_2\n" \
> + "1: ldxrh %w[ret], %[ptr]\n" \
> #asm_op " %w[ret], %w[ret], %w[val]\n" \
> - "stxrh %w[loop], %w[ret], %[ptr]\n" \
> - : [loop] "=&r" (loop), [ret] "=&r" (ret), \
> - [ptr] "+Q"(*(u16 *)ptr) \
> - : [val] "Ir" (val)); \
> - } while (loop); \
> + " stxrh %w[loop], %w[ret], %[ptr]\n" \
> + " cbnz %w[loop], 1b" \
> + : [loop] "=&r" (loop), [ret] "=&r" (ret), \
> + [ptr] "+Q"(*(u16 *)ptr) \
> + : [val] "Ir" (val)); \
> break; \
> case 4: \
> - do { \
> - asm ("//__per_cpu_" #op "_4\n" \
> - "ldxr %w[ret], %[ptr]\n" \
> + asm ("//__per_cpu_" #op "_4\n" \
> + "1: ldxr %w[ret], %[ptr]\n" \
> #asm_op " %w[ret], %w[ret], %w[val]\n" \
> - "stxr %w[loop], %w[ret], %[ptr]\n" \
> - : [loop] "=&r" (loop), [ret] "=&r" (ret), \
> - [ptr] "+Q"(*(u32 *)ptr) \
> - : [val] "Ir" (val)); \
> - } while (loop); \
> + " stxr %w[loop], %w[ret], %[ptr]\n" \
> + " cbnz %w[loop], 1b" \
> + : [loop] "=&r" (loop), [ret] "=&r" (ret), \
> + [ptr] "+Q"(*(u32 *)ptr) \
> + : [val] "Ir" (val)); \
> break; \
> case 8: \
> - do { \
> - asm ("//__per_cpu_" #op "_8\n" \
> - "ldxr %[ret], %[ptr]\n" \
> + asm ("//__per_cpu_" #op "_8\n" \
> + "1: ldxr %[ret], %[ptr]\n" \
> #asm_op " %[ret], %[ret], %[val]\n" \
> - "stxr %w[loop], %[ret], %[ptr]\n" \
> - : [loop] "=&r" (loop), [ret] "=&r" (ret), \
> - [ptr] "+Q"(*(u64 *)ptr) \
> - : [val] "Ir" (val)); \
> - } while (loop); \
> + " stxr %w[loop], %[ret], %[ptr]\n" \
> + " cbnz %w[loop], 1b" \
> + : [loop] "=&r" (loop), [ret] "=&r" (ret), \
> + [ptr] "+Q"(*(u64 *)ptr) \
> + : [val] "Ir" (val)); \
> break; \
> default: \
> BUILD_BUG(); \
> @@ -150,44 +146,40 @@ static inline unsigned long __percpu_xchg(void *ptr, unsigned long val,
>
> switch (size) {
> case 1:
> - do {
> - asm ("//__percpu_xchg_1\n"
> - "ldxrb %w[ret], %[ptr]\n"
> - "stxrb %w[loop], %w[val], %[ptr]\n"
> - : [loop] "=&r"(loop), [ret] "=&r"(ret),
> - [ptr] "+Q"(*(u8 *)ptr)
> - : [val] "r" (val));
> - } while (loop);
> + asm ("//__percpu_xchg_1\n"
> + "1: ldxrb %w[ret], %[ptr]\n"
> + " stxrb %w[loop], %w[val], %[ptr]\n"
> + " cbnz %w[loop], 1b"
> + : [loop] "=&r"(loop), [ret] "=&r"(ret),
> + [ptr] "+Q"(*(u8 *)ptr)
> + : [val] "r" (val));
> break;
> case 2:
> - do {
> - asm ("//__percpu_xchg_2\n"
> - "ldxrh %w[ret], %[ptr]\n"
> - "stxrh %w[loop], %w[val], %[ptr]\n"
> - : [loop] "=&r"(loop), [ret] "=&r"(ret),
> - [ptr] "+Q"(*(u16 *)ptr)
> - : [val] "r" (val));
> - } while (loop);
> + asm ("//__percpu_xchg_2\n"
> + "1: ldxrh %w[ret], %[ptr]\n"
> + " stxrh %w[loop], %w[val], %[ptr]\n"
> + " cbnz %w[loop], 1b"
> + : [loop] "=&r"(loop), [ret] "=&r"(ret),
> + [ptr] "+Q"(*(u16 *)ptr)
> + : [val] "r" (val));
> break;
> case 4:
> - do {
> - asm ("//__percpu_xchg_4\n"
> - "ldxr %w[ret], %[ptr]\n"
> - "stxr %w[loop], %w[val], %[ptr]\n"
> - : [loop] "=&r"(loop), [ret] "=&r"(ret),
> - [ptr] "+Q"(*(u32 *)ptr)
> - : [val] "r" (val));
> - } while (loop);
> + asm ("//__percpu_xchg_4\n"
> + "1: ldxr %w[ret], %[ptr]\n"
> + " stxr %w[loop], %w[val], %[ptr]\n"
> + " cbnz %w[loop], 1b"
> + : [loop] "=&r"(loop), [ret] "=&r"(ret),
> + [ptr] "+Q"(*(u32 *)ptr)
> + : [val] "r" (val));
> break;
> case 8:
> - do {
> - asm ("//__percpu_xchg_8\n"
> - "ldxr %[ret], %[ptr]\n"
> - "stxr %w[loop], %[val], %[ptr]\n"
> - : [loop] "=&r"(loop), [ret] "=&r"(ret),
> - [ptr] "+Q"(*(u64 *)ptr)
> - : [val] "r" (val));
> - } while (loop);
> + asm ("//__percpu_xchg_8\n"
> + "1: ldxr %[ret], %[ptr]\n"
> + " stxr %w[loop], %[val], %[ptr]\n"
> + " cbnz %w[loop], 1b"
> + : [loop] "=&r"(loop), [ret] "=&r"(ret),
> + [ptr] "+Q"(*(u64 *)ptr)
> + : [val] "r" (val));
> break;
> default:
> BUILD_BUG();
> --
> 2.1.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH V2] gpu/drm/exynos/exynos_hdmi - Unmap region obtained by of_iomap
From: Inki Dae @ 2016-10-19 14:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476871456-1098-1-git-send-email-arvind.yadav.cs@gmail.com>
Will pick it up soon.
Thanks,
Inki Dae
2016-10-19 19:04 GMT+09:00 Arvind Yadav <arvind.yadav.cs@gmail.com>:
> Free memory mapping, if hdmi_probe is not successful.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
> drivers/gpu/drm/exynos/exynos_hdmi.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 2275efe..ba28dec 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -1901,6 +1901,8 @@ err_disable_pm_runtime:
> err_hdmiphy:
> if (hdata->hdmiphy_port)
> put_device(&hdata->hdmiphy_port->dev);
> + if (hdata->regs_hdmiphy)
> + iounmap(hdata->regs_hdmiphy);
> err_ddc:
> put_device(&hdata->ddc_adpt->dev);
>
> @@ -1923,6 +1925,9 @@ static int hdmi_remove(struct platform_device *pdev)
> if (hdata->hdmiphy_port)
> put_device(&hdata->hdmiphy_port->dev);
>
> + if (hdata->regs_hdmiphy)
> + iounmap(hdata->regs_hdmiphy);
> +
> put_device(&hdata->ddc_adpt->dev);
>
> return 0;
> --
> 1.7.9.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH] exynos-drm: Fix display manager failing to start without IOMMU problem
From: Inki Dae @ 2016-10-19 14:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ce7a035c-f881-82ac-97f6-869442d1ed47@osg.samsung.com>
Hi Shuah,
2016-10-13 8:11 GMT+09:00 Shuah Khan <shuahkh@osg.samsung.com>:
> Hi Inki,
>
> On 08/15/2016 10:40 PM, Inki Dae wrote:
>
>>>
>>> okay the very first commit that added IOMMU support
>>> introduced the code that rejects non-contig gem memory
>>> type without IOMMU.
>>>
>>> commit 0519f9a12d0113caab78980c48a7902d2bd40c2c
>>> Author: Inki Dae <inki.dae@samsung.com>
>>> Date: Sat Oct 20 07:53:42 2012 -0700
>>>
>>> drm/exynos: add iommu support for exynos drm framework
>>>
>
> I haven't given up on this yet. I am still seeing the following failure:
>
> Additional debug messages I added:
> [ 15.287403] exynos_drm_gem_create_ioctl() 1
> [ 15.287419] exynos_drm_gem_create() flags 1
>
> [ 15.311511] [drm:exynos_drm_framebuffer_init] *ERROR* Non-contiguous GEM memory is not supported.
>
> Additional debug message I added:
> [ 15.318981] [drm:exynos_user_fb_create] *ERROR* failed to initialize framebuffer
>
> This is what happens:
>
> 1. exynos_drm_gem_create_ioctl() gets called with EXYNOS_BO_NONCONTIG request
> 2. exynos_drm_gem_create(0 goes ahead and creates the GEM buffers
> 3. exynos_user_fb_create() tries to associate GEM to fb and fails during
> check_fb_gem_memory_type()
>
> At this point, there is no recovery and lightdm fails
>
> xf86-video-armsoc/src/drmmode_exynos/drmmode_exynos.c assumes contiguous
> allocations are not supported in some exynos drm versions: The following
> commit introduced this change:
>
> https://git.linaro.org/arm/xorg/driver/xf86-video-armsoc.git/commitdiff/3be1f6273441fe95dd442f44064387322e16b7e9
>
> excerpts from the diff:- if (create_gem->buf_type == ARMSOC_BO_SCANOUT)
> - create_exynos.flags = EXYNOS_BO_CONTIG;
> - else
> - create_exynos.flags = EXYNOS_BO_NONCONTIG;
> +
> + /* Contiguous allocations are not supported in some exynos drm versions.
> + * When they are supported all allocations are effectively contiguous
> + * anyway, so for simplicity we always request non contiguous buffers.
> + */
> + create_exynos.flags = EXYNOS_BO_NONCONTIG;
>
Above comment, "Contiguous allocations are not supported in some
exynos drm versions.", seems wrong assumption.
The root cause, contiguous allocation is not supported, would be that
they used Linux kernel which didn't have CMA region enough - as
default 16MB, or didn't declare CMA region enough for the DMA device.
So I think they should not force to flag EXYNOS_BO_NONCONTIG and they
should manage the error case if the allocation failed.
> There might have been logic on exynos_drm that forced Contig when it coudn't
> support NONCONTIG. At least, that is what this comment suggests. This assumption
> doesn't appear to be a good one and not sure if this change was made to fix a bug.
>
> After the IOMMU support, this assumption is no longer true. Hence, with IOMMU
> support, latest kernels have a mismatch with the installed xf86-video-armsoc
>
> This is what I am running into. This leads to the following question:
>
> 1. How do we ensure exynos_drm kernel changes don't break user-space
> specifically xf86-video-armsoc
> 2. This seems to have gone undetected for a while. I see a change in
> exynos_drm_gem_dumb_create() that is probably addressing this type
> of breakage. Commit 122beea84bb90236b1ae545f08267af58591c21b adds
> handling for IOMMU NONCONTIG case.
Seems this patch has a problem. This patch forces to flag NONCONTIG if
iommu is enabled. The flag should be depend on the argument from
user-space.
I.e., if user-space requested a gem allocation with CONTIG flag, then
Exynos drm driver should allocate contiguous memory even though iommu
is enabled.
>
> Anyway, I am interested in getting the exynos_drm kernel side code
> and xf86-video-armsoc in sync to resolve the issue.
>
> Could you recommend a going forward plan?
My opinion are,
1. Do not force to flag EXYNOS_BO_NONCONTIG at xf86-video-armsoc
2. Do not force to flag NONCONTIG at Exynos drm driver even though
iommu is enabled and flag allocation type with the argument from
user-space.
I think you could try to post above patches.
Thanks,
Inki Dae
>
> I can submit a patch to xf86-video-armsoc. I am also looking ahead to
> see if we can avoid such breaks in the future by keeping kernel and
> xf86-video-armsoc in sync.
>
> thanks,
> -- Shuah
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] ARM: imx: gpc: Initialize all power domains
From: Shawn Guo @ 2016-10-19 14:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476208413-11286-1-git-send-email-fabio.estevam@nxp.com>
On Tue, Oct 11, 2016 at 02:53:33PM -0300, Fabio Estevam wrote:
> When booting a kernel built with multi_v7_defconfig the following
> probe error is seen:
>
> imx-gpc: probe of 20dc000.gpc failed with error -22
>
> Later on the kernel crashes like this:
>
> [ 1.723358] Unable to handle kernel NULL pointer dereference at virtual address 00000040
> [ 1.731500] pgd = c0204000
> [ 1.731863] hctosys: unable to open rtc device (rtc0)
> [ 1.739301] [00000040] *pgd=00000000
> [ 1.739310] Internal error: Oops: 5 [#1] SMP ARM
> [ 1.739319] Modules linked in:
> [ 1.739328] CPU: 1 PID: 95 Comm: kworker/1:4 Not tainted 4.8.0-11897-g6b5e09a #1
> [ 1.739331] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [ 1.739352] Workqueue: pm genpd_power_off_work_fn
> [ 1.739356] task: ee63d400 task.stack: ee70a000
> [ 1.739365] PC is at mutex_lock+0xc/0x4c
> [ 1.739374] LR is at regulator_disable+0x2c/0x60
> [ 1.739379] pc : [<c0bc0da0>] lr : [<c06e4b10>] psr: 60000013
> [ 1.739379] sp : ee70beb0 ip : 10624dd3 fp : ee6e6280
> [ 1.739382] r10: eefb0900 r9 : 00000000 r8 : c1309918
> [ 1.739385] r7 : 00000000 r6 : 00000040 r5 : 00000000 r4 : 00000040
> [ 1.739390] r3 : 0000004c r2 : 7fffd540 r1 : 000001e4 r0 : 00000040
>
> The gpc probe fails because of_genpd_add_provider_onecell() checks
> if all the domains are initialized via pm_genpd_present() function
> and it returns an error on the multi_v7_defconfig case.
It's not clear to me why this is only with multi_v7_defconfig, not
imx_v6_v7_defconfig. Also, is it a regression or long-standing issue?
Shawn
>
> In order to fix this error, initialize all the imx_gpc_domains, not
> only the imx6q_pu_domain.base one.
>
> Reported-by: Olof's autobooter <build@lixom.net>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
> arch/arm/mach-imx/gpc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
> index 0df062d..d0463e9 100644
> --- a/arch/arm/mach-imx/gpc.c
> +++ b/arch/arm/mach-imx/gpc.c
> @@ -430,7 +430,8 @@ static int imx_gpc_genpd_init(struct device *dev, struct regulator *pu_reg)
> if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS))
> return 0;
>
> - pm_genpd_init(&imx6q_pu_domain.base, NULL, false);
> + for (i = 0; i < ARRAY_SIZE(imx_gpc_domains); i++)
> + pm_genpd_init(imx_gpc_domains[i], NULL, false);
> return of_genpd_add_provider_onecell(dev->of_node,
> &imx_gpc_onecell_data);
>
> --
> 2.7.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 0/2] ARM: at91: properly handle LPDDR poweroff
From: Geert Uytterhoeven @ 2016-10-19 14:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161019114420.15213-1-alexandre.belloni@free-electrons.com>
On Wed, Oct 19, 2016 at 1:44 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> LPDDR memories can only handle up to 400 uncontrolled power offs in their
> life. The proper power off sequence has to be applied before shutting down the
> SoC.
Interesting. How many boards have been killed during kernelci.org
operation?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH 1/2] arm64: swp emulation: bound LL/SC retries before rescheduling
From: Mark Rutland @ 2016-10-19 14:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476874784-16214-1-git-send-email-will.deacon@arm.com>
On Wed, Oct 19, 2016 at 11:59:43AM +0100, Will Deacon wrote:
> If a CPU does not implement a global monitor for certain memory types,
> then userspace can attempt a kernel DoS by issuing SWP instructions
> targetting the problematic memory (for example, a framebuffer mapped
> with non-cacheable attributes).
>
> The SWP emulation code protects against these sorts of attacks by
> checking for pending signals and potentially rescheduling when the STXR
> instruction fails during the emulation. Whilst this is good for avoiding
> livelock, it harms emulation of legitimate SWP instructions on CPUs
> where forward progress is not guaranteed if there are memory accesses to
> the same reservation granule (up to 2k) between the failing STXR and
> the retry of the LDXR.
>
> This patch solves the problem by retrying the STXR a bounded number of
> times (4) before breaking out of the LL/SC loop and looking for
> something else to do.
>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
Assuming I've followed all the operand numbering correctly, this looks
correct to me per my reading of the requirements in the ARM ARM.
FWIW:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
> ---
> arch/arm64/kernel/armv8_deprecated.c | 36 ++++++++++++++++++++++--------------
> 1 file changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
> index 42ffdb54e162..b0988bb1bf64 100644
> --- a/arch/arm64/kernel/armv8_deprecated.c
> +++ b/arch/arm64/kernel/armv8_deprecated.c
> @@ -280,35 +280,43 @@ static void __init register_insn_emulation_sysctl(struct ctl_table *table)
> /*
> * Error-checking SWP macros implemented using ldxr{b}/stxr{b}
> */
> -#define __user_swpX_asm(data, addr, res, temp, B) \
> +
> +/* Arbitrary constant to ensure forward-progress of the LL/SC loop */
> +#define __SWP_LL_SC_LOOPS 4
> +
> +#define __user_swpX_asm(data, addr, res, temp, temp2, B) \
> __asm__ __volatile__( \
> + " mov %w3, %w7\n" \
> ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN, \
> CONFIG_ARM64_PAN) \
> - "0: ldxr"B" %w2, [%3]\n" \
> - "1: stxr"B" %w0, %w1, [%3]\n" \
> + "0: ldxr"B" %w2, [%4]\n" \
> + "1: stxr"B" %w0, %w1, [%4]\n" \
> " cbz %w0, 2f\n" \
> - " mov %w0, %w4\n" \
> + " sub %w3, %w3, #1\n" \
> + " cbnz %w3, 0b\n" \
> + " mov %w0, %w5\n" \
> " b 3f\n" \
> "2:\n" \
> " mov %w1, %w2\n" \
> "3:\n" \
> " .pushsection .fixup,\"ax\"\n" \
> " .align 2\n" \
> - "4: mov %w0, %w5\n" \
> + "4: mov %w0, %w6\n" \
> " b 3b\n" \
> " .popsection" \
> _ASM_EXTABLE(0b, 4b) \
> _ASM_EXTABLE(1b, 4b) \
> ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_HAS_PAN, \
> CONFIG_ARM64_PAN) \
> - : "=&r" (res), "+r" (data), "=&r" (temp) \
> - : "r" (addr), "i" (-EAGAIN), "i" (-EFAULT) \
> + : "=&r" (res), "+r" (data), "=&r" (temp), "=&r" (temp2) \
> + : "r" (addr), "i" (-EAGAIN), "i" (-EFAULT), \
> + "i" (__SWP_LL_SC_LOOPS) \
> : "memory")
>
> -#define __user_swp_asm(data, addr, res, temp) \
> - __user_swpX_asm(data, addr, res, temp, "")
> -#define __user_swpb_asm(data, addr, res, temp) \
> - __user_swpX_asm(data, addr, res, temp, "b")
> +#define __user_swp_asm(data, addr, res, temp, temp2) \
> + __user_swpX_asm(data, addr, res, temp, temp2, "")
> +#define __user_swpb_asm(data, addr, res, temp, temp2) \
> + __user_swpX_asm(data, addr, res, temp, temp2, "b")
>
> /*
> * Bit 22 of the instruction encoding distinguishes between
> @@ -328,12 +336,12 @@ static int emulate_swpX(unsigned int address, unsigned int *data,
> }
>
> while (1) {
> - unsigned long temp;
> + unsigned long temp, temp2;
>
> if (type == TYPE_SWPB)
> - __user_swpb_asm(*data, address, res, temp);
> + __user_swpb_asm(*data, address, res, temp, temp2);
> else
> - __user_swp_asm(*data, address, res, temp);
> + __user_swp_asm(*data, address, res, temp, temp2);
>
> if (likely(res != -EAGAIN) || signal_pending(current))
> break;
> --
> 2.1.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH 1/2] iommu/dma: Implement dma_{map,unmap}_resource()
From: Will Deacon @ 2016-10-19 14:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <b6c78e6c18d98069b0fdfa219da51d2aed129aed.1476705500.git.robin.murphy@arm.com>
On Mon, Oct 17, 2016 at 01:05:29PM +0100, Robin Murphy wrote:
> With the new dma_{map,unmap}_resource() functions added to the DMA API
> for the benefit of cases like slave DMA, add suitable implementations to
> the arsenal of our generic layer. Since cache maintenance should not be
> a concern, these can both be standalone versions without the need for
> architecture-specific wrappers.
>
> CC: Joerg Roedel <joro@8bytes.org>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>
> Since patch 2 has a build dependency on this one, they should probably
> go together through either the arm64 tree or the iommu tree, but I can't
> make up my mind which one seems more appropriate...
I can take it via the smmu tree, if you like. However, comment below.
> drivers/iommu/dma-iommu.c | 13 +++++++++++++
> include/linux/dma-iommu.h | 4 ++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index c5ab8667e6f2..50acd71915db 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -624,6 +624,19 @@ void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nents,
> __iommu_dma_unmap(iommu_get_domain_for_dev(dev), sg_dma_address(sg));
> }
>
> +dma_addr_t iommu_dma_map_resource(struct device *dev, phys_addr_t phys,
> + size_t size, enum dma_data_direction dir, unsigned long attrs)
> +{
> + return iommu_dma_map_page(dev, phys_to_page(phys), offset_in_page(phys),
> + size, dma_direction_to_prot(dir, false) | IOMMU_MMIO);
> +}
>
> +void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
> + size_t size, enum dma_data_direction dir, unsigned long attrs)
> +{
> + __iommu_dma_unmap(iommu_get_domain_for_dev(dev), handle);
> +}
I think it's better to call iommu_dma_unmap_page instead. That said, are
you sure it's safe to ignore the "size" parameter here? Is it permitted
to unmap part of a region? If not, why does that parameter exist?
Will
^ permalink raw reply
* [PATCH 2/3] arm64: dts: uniphier: add CPU clock and OPP table for LD11 SoC
From: Viresh Kumar @ 2016-10-19 13:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAK7LNATc2XxAZf-rvMkRjGn3q_18yB=9aycAE1j9+n0==xzaHQ@mail.gmail.com>
On 19-10-16, 17:33, Masahiro Yamada wrote:
> Hi Viresh,
>
>
> 2016-10-18 20:25 GMT+09:00 Viresh Kumar <viresh.kumar@linaro.org>:
> > On 16-10-16, 23:59, Masahiro Yamada wrote:
> >> + cluster0_opp: opp_table {
> >> + compatible = "operating-points-v2";
> >> + opp-shared;
> >> +
> >> + opp at 245000000 {
> >> + opp-hz = /bits/ 64 <245000000>;
> >> + clock-latency-ns = <300>;
> >> + };
> >> + opp at 250000000 {
> >> + opp-hz = /bits/ 64 <250000000>;
> >> + clock-latency-ns = <300>;
> >> + };
> >> + opp at 490000000 {
> >> + opp-hz = /bits/ 64 <490000000>;
> >> + clock-latency-ns = <300>;
> >> + };
> >> + opp at 500000000 {
> >> + opp-hz = /bits/ 64 <500000000>;
> >> + clock-latency-ns = <300>;
> >> + };
> >> + opp at 653333333 {
> >
> > Why isn't ^^ matching with below values ? Same in next patch as well.
>
>
>
> When I try to update /sys/devices/system/cpu/cpufreq/policy*/scaling_max_freq,
> it did not work as I had expected.
>
>
> scaling_max_freq is specified by kHz unit,
> on the other hand, clock frequency in the clk driver is specified by Hz.
>
>
>
> If the operating point is 653333kHz, the cpufreq requests
> the clk driver to set 653333000, but it is lower than
> the exact clock, 653333333.
> So, the next lower frequency, 500000000 is selected.
> As a result, the operating point 653333kHz is never enabled.
>
>
> So, the operating point must be equal or a little bit bigger.
>
>
> Do you know a better way to solve this distortion?
I am not sure about how to fix that problem but there is no reason to
have the exact frequency in opp@*** name. Just use what you have used
in opp-hz line and you will have the exact same behavior.
Right now, its a bit confusing if we read the DT.
--
viresh
^ permalink raw reply
* [PATCH v2 1/4] cpufreq: pxa: use generic platdev driver for device-tree
From: Viresh Kumar @ 2016-10-19 13:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87r37d4qlw.fsf@belgarion.home>
On 18-10-16, 17:35, Robert Jarzmik wrote:
> Viresh Kumar <viresh.kumar@linaro.org> writes:
>
> > On 15-10-16, 21:57, Robert Jarzmik wrote:
> >> For device-tree based pxa25x and pxa27x platforms, cpufreq-dt driver is
> >> doing the job as well as pxa2xx-cpufreq, so add these platforms to the
> >> compatibility list.
> >>
> >> This won't work for legacy non device-tree platforms where
> >> pxa2xx-cpufreq is still required.
> >>
> >> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> >> ---
> >> drivers/cpufreq/cpufreq-dt-platdev.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
> >> index 0bb44d5b5df4..356825b5c9b8 100644
> >> --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> >> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> >> @@ -32,6 +32,8 @@ static const struct of_device_id machines[] __initconst = {
> >> { .compatible = "fsl,imx7d", },
> >>
> >> { .compatible = "marvell,berlin", },
> >> + { .compatible = "marvell,pxa250", },
> >> + { .compatible = "marvell,pxa270", },
> >>
> >> { .compatible = "samsung,exynos3250", },
> >> { .compatible = "samsung,exynos4210", },
> >
> > Isn't there a race between cpufreq-dt and the platform driver to
> > register first ?
> Ah, could you be more specific about the race you're talking of ?
>
> My understanding was that cpufreq-dt-platdev does create the device, and
> cpufreq-dt is a driver for it, so there is no race but a direct relationship
> AFAIU.
I mean that both the driver may try to register to the cpufreq core if
they are both compiled in a single image.
--
viresh
^ permalink raw reply
* [PATCH v2 2/4] ARM: dts: pxa: add pxa25x cpu operating points
From: Viresh Kumar @ 2016-10-19 13:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87vawp4qv0.fsf@belgarion.home>
On 18-10-16, 17:30, Robert Jarzmik wrote:
> Viresh Kumar <viresh.kumar@linaro.org> writes:
>
> > On 15-10-16, 21:57, Robert Jarzmik wrote:
> >> Add the relevant data taken from the PXA 25x Electrical, Mechanical, and
> >> Thermal Specfication. This will be input data for cpufreq-dt driver.
> >>
> >> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> >> ---
> >> arch/arm/boot/dts/pxa25x.dtsi | 25 +++++++++++++++++++++++++
> >> 1 file changed, 25 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/pxa25x.dtsi b/arch/arm/boot/dts/pxa25x.dtsi
> >> index 0d1e012178c4..16b4e8bad4a5 100644
> >> --- a/arch/arm/boot/dts/pxa25x.dtsi
> >> +++ b/arch/arm/boot/dts/pxa25x.dtsi
> >> @@ -89,4 +89,29 @@
> >> clocks = <&clktimer>;
> >> status = "okay";
> >> };
> >> +
> >> + pxa250_opp_table: opp_table0 {
> >> + compatible = "operating-points-v2";
> >> +
> >> + opp at 99500 {
> >
> > We have been keeping the values in ^^^ same as the values present
> > below. Any specific reason for making it different here ?
> No, that's a good comment, I'll change that.
>
> I wrote this incrementaly, first the node, then the opp-hz. Then I realized that
> the source crystal, at 3.8684 MHz didn't provide a round 99.5 MHz core clock,
> but a 99.5328 MHz clock.
>
> Anyway, I'll change that ... let's say into opp at 99533 in this case ?
Just write the whole value 99532800.
--
viresh
^ permalink raw reply
* [PATCH] ARM: dts: sun8i: Add SPI controller node in H3
From: Milo Kim @ 2016-10-19 13:46 UTC (permalink / raw)
To: linux-arm-kernel
H3 supports two SPI controllers. Four pins (MOSI, MISO, SCLK, SS) are
configured through the pinctrl subsystem. It is almost same as A31 SPI
except buffer size, so those DT properties are reusable.
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
---
arch/arm/boot/dts/sun8i-h3.dtsi | 46 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 75a8654..c38b028 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -381,6 +381,20 @@
allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
};
+ spi0_pins: spi0 {
+ allwinner,pins = "PC0", "PC1", "PC2", "PC3";
+ allwinner,function = "spi0";
+ allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+ allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+ };
+
+ spi1_pins: spi1 {
+ allwinner,pins = "PA15", "PA16", "PA14", "PA13";
+ allwinner,function = "spi1";
+ allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+ allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+ };
+
uart0_pins_a: uart0 at 0 {
allwinner,pins = "PA4", "PA5";
allwinner,function = "uart0";
@@ -425,6 +439,38 @@
clocks = <&osc24M>;
};
+ spi0: spi at 01c68000 {
+ compatible = "allwinner,sun8i-h3-spi";
+ reg = <0x01c68000 0x1000>;
+ interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_SPI0>, <&ccu CLK_SPI0>;
+ clock-names = "ahb", "mod";
+ dmas = <&dma 23>, <&dma 23>;
+ dma-names = "rx", "tx";
+ pinctrl-names = "default";
+ pinctrl-0 = <&spi0_pins>;
+ resets = <&ccu RST_BUS_SPI0>;
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ spi1: spi at 01c69000 {
+ compatible = "allwinner,sun8i-h3-spi";
+ reg = <0x01c69000 0x1000>;
+ interrupts = <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_SPI1>, <&ccu CLK_SPI1>;
+ clock-names = "ahb", "mod";
+ dmas = <&dma 24>, <&dma 24>;
+ dma-names = "rx", "tx";
+ pinctrl-names = "default";
+ pinctrl-0 = <&spi1_pins>;
+ resets = <&ccu RST_BUS_SPI1>;
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
wdt0: watchdog at 01c20ca0 {
compatible = "allwinner,sun6i-a31-wdt";
reg = <0x01c20ca0 0x20>;
--
2.9.3
^ permalink raw reply related
* [PATCH v2] arm64: Cortex-A53 errata workaround: check for kernel addresses
From: Andre Przywara @ 2016-10-19 13:40 UTC (permalink / raw)
To: linux-arm-kernel
Commit 7dd01aef0557 ("arm64: trap userspace "dc cvau" cache operation on
errata-affected core") adds code to execute cache maintenance instructions
in the kernel on behalf of userland on CPUs with certain ARM CPU errata.
It turns out that the address hasn't been checked to be a valid user
space address, allowing userland to clean cache lines in kernel space.
Fix this by introducing an access_ok() check before executing the
instructions on behalf of userland. We have to take care of tagged
pointers here, since the address is not coming in via a syscall (which
requires the tag to be 0).
Fixes: 7dd01aef0557 ("arm64: trap userspace "dc cvau" cache operation on errata-affected core")
Reported-by: Kristina Martsenko <kristina.martsenko@arm.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
Changelog v1 .. v2:
- use PAGE_SIZE instead of cache_line_size() for access check
- merge extra function into original macro
- replace access_ok_tagged() with untagged_addr() macro
arch/arm64/include/asm/uaccess.h | 8 ++++++++
arch/arm64/kernel/traps.c | 35 +++++++++++++++++++++++------------
2 files changed, 31 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index bcaf6fb..55d0adb 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -21,6 +21,7 @@
/*
* User space memory access functions
*/
+#include <linux/bitops.h>
#include <linux/kasan-checks.h>
#include <linux/string.h>
#include <linux/thread_info.h>
@@ -102,6 +103,13 @@ static inline void set_fs(mm_segment_t fs)
flag; \
})
+/*
+ * When dealing with data aborts or instruction traps we may end up with
+ * a tagged userland pointer. Clear the tag to get a sane pointer to pass
+ * on to access_ok(), for instance.
+ */
+#define untagged_addr(addr) sign_extend64(addr, 55)
+
#define access_ok(type, addr, size) __range_ok(addr, size)
#define user_addr_max get_fs
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 5ff020f..1e81328 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -433,19 +433,30 @@ void cpu_enable_cache_maint_trap(void *__unused)
config_sctlr_el1(SCTLR_EL1_UCI, 0);
}
+/*
+ * Technically the access_ok() check should be done on a cache line size
+ * granularity, but it's tedious the get the right one (dcache vs. icache,
+ * per-core vs. system wide). Since permission checks are based on pages
+ * anyway, just use PAGE_SIZE instead.
+ */
#define __user_cache_maint(insn, address, res) \
- asm volatile ( \
- "1: " insn ", %1\n" \
- " mov %w0, #0\n" \
- "2:\n" \
- " .pushsection .fixup,\"ax\"\n" \
- " .align 2\n" \
- "3: mov %w0, %w2\n" \
- " b 2b\n" \
- " .popsection\n" \
- _ASM_EXTABLE(1b, 3b) \
- : "=r" (res) \
- : "r" (address), "i" (-EFAULT) )
+ if (!access_ok(VERIFY_READ, \
+ untagged_addr(address & PAGE_MASK), \
+ PAGE_SIZE)) \
+ res = -EFAULT; \
+ else \
+ asm volatile ( \
+ "1: " insn ", %1\n" \
+ " mov %w0, #0\n" \
+ "2:\n" \
+ " .pushsection .fixup,\"ax\"\n" \
+ " .align 2\n" \
+ "3: mov %w0, %w2\n" \
+ " b 2b\n" \
+ " .popsection\n" \
+ _ASM_EXTABLE(1b, 3b) \
+ : "=r" (res) \
+ : "r" (address), "i" (-EFAULT) )
static void user_cache_maint_handler(unsigned int esr, struct pt_regs *regs)
{
--
2.9.0
^ permalink raw reply related
* Build failure with v4.9-rc1 and GCC trunk -- compiler weirdness
From: Will Deacon @ 2016-10-19 13:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu869PXA_cXaL6+uokRCS4EZ8-q6x2xnj4e5DWO28B5jCw@mail.gmail.com>
Hi Ard,
On Mon, Oct 17, 2016 at 08:43:19PM +0100, Ard Biesheuvel wrote:
> On 17 October 2016 at 19:38, Will Deacon <will.deacon@arm.com> wrote:
> > I'm seeing an arm64 build failure with -rc1 and GCC trunk, although I
> > believe that the new compiler behaviour at the heart of the problem
> > has the potential to affect other architectures and other pieces of
> > kernel code relying on dead-code elimination to remove deliberately
> > undefined functions.
> >
> > The failure looks like:
> >
> > | drivers/built-in.o: In function `armada_3700_add_composite_clk':
> > |
> > | linux/drivers/clk/mvebu/armada-37xx-periph.c:351:
> > | undefined reference to `____ilog2_NaN'
> > |
> > | linux/drivers/clk/mvebu/armada-37xx-periph.c:351:(.text+0xc72e0):
> > | relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol
> > | `____ilog2_NaN'
> > |
> > | make: *** [vmlinux] Error 1
> >
> > and if we look at the source for armada_3700_add_composite_clk, we see
> > that this is caused by:
> >
> > int table_size = 0;
> >
> > rate->reg = reg + (u64)rate->reg;
> > for (clkt = rate->table; clkt->div; clkt++)
> > table_size++;
> > rate->width = order_base_2(table_size);
> >
> > order_base_2 calls ilog2, which has the ____ilog2_NaN call:
> >
> > #define ilog2(n) \
> > ( \
> > __builtin_constant_p(n) ? ( \
> > (n) < 1 ? ____ilog2_NaN() : \
> >
> > This is because we're in a curious case where GCC has emitted a
> > special-cased version of armada_3700_add_composite_clk, with table_size
> > effectively constant-folded as 0. Whilst we shouldn't see this in a
> > non-buggy kernel (hence the deliberate call to the undefined function
> > ____ilog2_NaN), it means that the final link fails because we have a
> > ____ilog2_NaN in the code, with a runtime check on table_size.
> >
>
> This is indeed an unintended side effect, but I would not call it
> weird behaviour at all. The code in its current form does not handle
> the case where it could end up passing 0 into order_base_2(), and we
> simply need to handle that case.
The reasons I think it's weird are:
(1) The optimisation doesn't generate better code in this case --
optimising for the table_size == 0 case is uninformed, particularly
as that *cannot* happen at runtime (GCC probably can't tell, due
to things like container_of, but all the clock data is static).
(2) __builtin_constant_p(n) could be interpreted by a developer as
"this code will execute with a constant n at runtime". With this
issue, GCC could (in theory) generate a specialisation for every
possible value of a variable, and return __builtin_constant_p as
true for all of them, which somewhat undermines the point of the
builtin.
> If order_base_2() is not defined for input 0, it should BUG() in that
> case, and the associated __builtin_unreachable() should prevent the
> special version from being emitted. If order_base_2() is defined for input
> 0, it should not invoke ilog2() with that argument, and the problem should
> go away as well.
I don't necessarily think it should BUG() if it's not defined for input
0; things like __ffs don't do that and we'd be introducing conditional
checks for cases that should not happen. The comment above order_base_2
does suggest that ob2(0) should return 0, but it can actually end up
invoking ilog2(-1), which is obviously wrong.
I could update the comment, but that doesn't fix the build issue.
Will
^ permalink raw reply
* [PATCH V5 00/10] dmaengine: qcom_hidma: add MSI interrupt support
From: Vinod Koul @ 2016-10-19 13:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475817915-11976-1-git-send-email-okaya@codeaurora.org>
On Fri, Oct 07, 2016 at 01:25:05AM -0400, Sinan Kaya wrote:
> The new version of the HW supports MSI interrupts instead of wired
> interrupts. The MSI interrupts are especially useful for the guest machine
> execution. The wired interrupts usually trap to the hypervisor and then are
> relayed to the actual interrupt.
>
> The MSI interrupts can be directly fed into the interrupt controller.
>
> Adding a new OF compat string (qcom,hidma-1.1) and ACPI string (QCOM8062)
> to distinguish newer HW from the older ones.
I was only able to apply 6 patches in this series. Which tree were these
generated against?
Please rebase rest..
--
~Vinod
^ permalink raw reply
* [PATCH v2] mmc: sunxi: Prevent against null dereference for vmmc
From: Maxime Ripard @ 2016-10-19 13:33 UTC (permalink / raw)
To: linux-arm-kernel
VMMC is an optional regulator, which means that mmc_regulator_get_supply
will only return an error in case of a deferred probe, but not when the
regulator is not set in the DT.
However, the sunxi driver assumes that VMMC is always there, and doesn't
check the value of the regulator pointer before using it, which obviously
leads to a (close to) null pointer dereference.
Add proper checks to prevent that.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
Changes from v1:
- remove redundant error message
---
drivers/mmc/host/sunxi-mmc.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index c0a5c676d0e8..b1d1303389a7 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -822,10 +822,13 @@ static void sunxi_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
break;
case MMC_POWER_UP:
- host->ferror = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
- ios->vdd);
- if (host->ferror)
- return;
+ if (!IS_ERR(mmc->supply.vmmc)) {
+ host->ferror = mmc_regulator_set_ocr(mmc,
+ mmc->supply.vmmc,
+ ios->vdd);
+ if (host->ferror)
+ return;
+ }
if (!IS_ERR(mmc->supply.vqmmc)) {
host->ferror = regulator_enable(mmc->supply.vqmmc);
@@ -847,7 +850,9 @@ static void sunxi_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
case MMC_POWER_OFF:
dev_dbg(mmc_dev(mmc), "power off!\n");
sunxi_mmc_reset_host(host);
- mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
+ if (!IS_ERR(mmc->supply.vmmc))
+ mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
+
if (!IS_ERR(mmc->supply.vqmmc) && host->vqmmc_enabled)
regulator_disable(mmc->supply.vqmmc);
host->vqmmc_enabled = false;
--
2.9.3
^ permalink raw reply related
* [PATCH 2/3] arm64: hw_breakpoint: Handle inexact watchpoint addresses
From: Pavel Labath @ 2016-10-19 13:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161019120714.GM9193@arm.com>
I've send the test suite update to Pratyush via github yesterday. I
presume he will come with a v2 soon. :)
regards,
pavel
On 19 October 2016 at 13:07, Will Deacon <will.deacon@arm.com> wrote:
> On Fri, Oct 14, 2016 at 08:45:43AM +0530, Pratyush Anand wrote:
>>
>>
>> On Thursday 13 October 2016 10:33 PM, Pavel Labath wrote:
>> >>I think, its easier to go with your implementation. So, I have taken
>> >>> your patch and updated my perf/upstream_arm64_devel branch. May be you
>> >>> can give it a test for your test cases.
>> >I've checked out the new version of your branch, and it works great.
>> >I'll write a patch with additional test cases to go on top of your
>> >branch, as the tests there do not capture the bug I was fixing.
>>
>> That would be great. We can send them all together as V2.
>
> Did you send a v2? I've been holding off reviewing this, but I just want
> to make sure I didn't miss the update.
>
> Cheers,
>
> Will
^ 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