* [PATCH v3 1/3] clk: stm32f4: Add LSI & LSE clocks
From: gabriel.fernandez at st.com @ 2016-10-21 9:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477041810-12313-1-git-send-email-gabriel.fernandez@st.com>
From: Gabriel Fernandez <gabriel.fernandez@st.com>
This patch introduces the support of the LSI & LSE clocks.
The clock drivers needs to disable the power domain write protection
using syscon/regmap to enable these clocks.
Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
---
drivers/clk/clk-stm32f4.c | 131 ++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 126 insertions(+), 5 deletions(-)
diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
index 02d6810..6427e0f 100644
--- a/drivers/clk/clk-stm32f4.c
+++ b/drivers/clk/clk-stm32f4.c
@@ -19,10 +19,14 @@
#include <linux/clk-provider.h>
#include <linux/err.h>
#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/ioport.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/of.h>
#include <linux/of_address.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
#define STM32F4_RCC_PLLCFGR 0x04
#define STM32F4_RCC_CFGR 0x08
@@ -31,6 +35,8 @@
#define STM32F4_RCC_AHB3ENR 0x38
#define STM32F4_RCC_APB1ENR 0x40
#define STM32F4_RCC_APB2ENR 0x44
+#define STM32F4_RCC_BDCR 0x70
+#define STM32F4_RCC_CSR 0x74
struct stm32f4_gate_data {
u8 offset;
@@ -120,13 +126,12 @@ struct stm32f4_gate_data {
{ STM32F4_RCC_APB2ENR, 26, "ltdc", "apb2_div" },
};
+enum { SYSTICK, FCLK, CLK_LSI, CLK_LSE, END_PRIMARY_CLK };
/*
* MAX_CLKS is the maximum value in the enumeration below plus the combined
* hweight of stm32f42xx_gate_map (plus one).
*/
-#define MAX_CLKS 74
-
-enum { SYSTICK, FCLK };
+#define MAX_CLKS (71 + END_PRIMARY_CLK + 1)
/*
* This bitmask tells us which bit offsets (0..192) on STM32F4[23]xxx
@@ -140,6 +145,8 @@ struct stm32f4_gate_data {
static DEFINE_SPINLOCK(stm32f4_clk_lock);
static void __iomem *base;
+static struct regmap *pdrm;
+
/*
* "Multiplier" device for APBx clocks.
*
@@ -259,7 +266,7 @@ static int stm32f4_rcc_lookup_clk_idx(u8 primary, u8 secondary)
u64 table[ARRAY_SIZE(stm32f42xx_gate_map)];
if (primary == 1) {
- if (WARN_ON(secondary > FCLK))
+ if (WARN_ON(secondary >= END_PRIMARY_CLK))
return -EINVAL;
return secondary;
}
@@ -276,7 +283,7 @@ static int stm32f4_rcc_lookup_clk_idx(u8 primary, u8 secondary)
table[BIT_ULL_WORD(secondary)] &=
GENMASK_ULL(secondary % BITS_PER_LONG_LONG, 0);
- return FCLK + hweight64(table[0]) +
+ return END_PRIMARY_CLK - 1 + hweight64(table[0]) +
(BIT_ULL_WORD(secondary) >= 1 ? hweight64(table[1]) : 0) +
(BIT_ULL_WORD(secondary) >= 2 ? hweight64(table[2]) : 0);
}
@@ -292,6 +299,98 @@ static int stm32f4_rcc_lookup_clk_idx(u8 primary, u8 secondary)
return clks[i];
}
+#define to_rgclk(_rgate) container_of(_rgate, struct stm32_rgate, gate)
+
+static inline void disable_power_domain_write_protection(void)
+{
+ if (pdrm)
+ regmap_update_bits(pdrm, 0x00, (1 << 8), (1 << 8));
+}
+
+static inline void enable_power_domain_write_protection(void)
+{
+ if (pdrm)
+ regmap_update_bits(pdrm, 0x00, (1 << 8), (0 << 8));
+}
+
+struct stm32_rgate {
+ struct clk_gate gate;
+ u8 bit_rdy_idx;
+};
+
+#define RTC_TIMEOUT 1000000
+
+static int rgclk_enable(struct clk_hw *hw)
+{
+ struct clk_gate *gate = to_clk_gate(hw);
+ struct stm32_rgate *rgate = to_rgclk(gate);
+ u32 reg;
+ int ret;
+
+ disable_power_domain_write_protection();
+
+ clk_gate_ops.enable(hw);
+
+ ret = readl_relaxed_poll_timeout_atomic(gate->reg, reg,
+ reg & rgate->bit_rdy_idx, 1000, RTC_TIMEOUT);
+
+ enable_power_domain_write_protection();
+ return ret;
+}
+
+static void rgclk_disable(struct clk_hw *hw)
+{
+ clk_gate_ops.disable(hw);
+}
+
+static int rgclk_is_enabled(struct clk_hw *hw)
+{
+ return clk_gate_ops.is_enabled(hw);
+}
+
+static const struct clk_ops rgclk_ops = {
+ .enable = rgclk_enable,
+ .disable = rgclk_disable,
+ .is_enabled = rgclk_is_enabled,
+};
+
+static struct clk_hw *clk_register_rgate(struct device *dev, const char *name,
+ const char *parent_name, unsigned long flags,
+ void __iomem *reg, u8 bit_idx, u8 bit_rdy_idx,
+ u8 clk_gate_flags, spinlock_t *lock)
+{
+ struct stm32_rgate *rgate;
+ struct clk_init_data init = { NULL };
+ struct clk_hw *hw;
+ int ret;
+
+ rgate = kzalloc(sizeof(*rgate), GFP_KERNEL);
+ if (!rgate)
+ return ERR_PTR(-ENOMEM);
+
+ init.name = name;
+ init.ops = &rgclk_ops;
+ init.flags = flags;
+ init.parent_names = &parent_name;
+ init.num_parents = 1;
+
+ rgate->bit_rdy_idx = bit_rdy_idx;
+
+ rgate->gate.lock = lock;
+ rgate->gate.reg = reg;
+ rgate->gate.bit_idx = bit_idx;
+ rgate->gate.hw.init = &init;
+
+ hw = &rgate->gate.hw;
+ ret = clk_hw_register(dev, hw);
+ if (ret) {
+ kfree(rgate);
+ hw = ERR_PTR(ret);
+ }
+
+ return hw;
+}
+
static const char *sys_parents[] __initdata = { "hsi", NULL, "pll" };
static const struct clk_div_table ahb_div_table[] = {
@@ -319,6 +418,12 @@ static void __init stm32f4_rcc_init(struct device_node *np)
return;
}
+ pdrm = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
+ if (IS_ERR(pdrm)) {
+ pdrm = NULL;
+ pr_warn("%s: Unable to get syscfg\n", __func__);
+ }
+
hse_clk = of_clk_get_parent_name(np, 0);
clk_register_fixed_rate_with_accuracy(NULL, "hsi", NULL, 0,
@@ -371,6 +476,22 @@ static void __init stm32f4_rcc_init(struct device_node *np)
}
}
+ clks[CLK_LSI] = clk_register_rgate(NULL, "lsi", "clk-lsi", 0,
+ base + STM32F4_RCC_CSR, 0, 2, 0, &stm32f4_clk_lock);
+
+ if (IS_ERR(clks[CLK_LSI])) {
+ pr_err("Unable to register lsi clock\n");
+ goto fail;
+ }
+
+ clks[CLK_LSE] = clk_register_rgate(NULL, "lse", "clk-lse", 0,
+ base + STM32F4_RCC_BDCR, 0, 2, 0, &stm32f4_clk_lock);
+
+ if (IS_ERR(clks[CLK_LSE])) {
+ pr_err("Unable to register lse clock\n");
+ goto fail;
+ }
+
of_clk_add_hw_provider(np, stm32f4_rcc_lookup_clk, NULL);
return;
fail:
--
1.9.1
^ permalink raw reply related
* [PATCH v3 0/3] STM32F4 Add RTC & QSPI clocks
From: gabriel.fernandez at st.com @ 2016-10-21 9:23 UTC (permalink / raw)
To: linux-arm-kernel
From: Gabriel Fernandez <gabriel.fernandez@st.com>
v3:
- remove arm & DT patches from this patch-set
- solve issue of dependency with DT
- remove clk_hw from struct stm32_rgate (use hw of clk_gate)
- suppress CLK_IS_BASIC flags of clk_register_rgate()
- cosmetic changes
v2:
- rename compatible property "st,stm32f46xx-rcc" into "st,stm32f469-rcc"
- cosmetic: remove bad copy/paste
This patch-set introduce RTC and QSPI clocks for STM32F4 socs
RTC clock has 3 parents clock oscillators (lsi/lse/hse_rtc)
example to use rtc clock:
rtc: rtc at 40002800 {
compatible = "st,stm32-rtc";
reg = <0x40002800 0x400>;
...
clocks = <&rcc 1 CLK_RTC>;
assigned-clocks = <&rcc 1 CLK_RTC>;
assigned-clock-parents = <&rcc 1 CLK_LSE>;
...
};
Gabriel Fernandez (3):
clk: stm32f4: Add LSI & LSE clocks
clk: stm32f4: Add RTC clock
clk: stm32f469: Add QSPI clock
.../devicetree/bindings/clock/st,stm32-rcc.txt | 4 +-
drivers/clk/clk-stm32f4.c | 435 ++++++++++++++++++++-
2 files changed, 417 insertions(+), 22 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH] dt/bindings: arm-boards: Remove skeleton.dtsi inclusion from example
From: Geert Uytterhoeven @ 2016-10-21 9:20 UTC (permalink / raw)
To: linux-arm-kernel
As of commit 9c0da3cc61f1233c ("ARM: dts: explicitly mark skeleton.dtsi
as deprecated"), including skeleton.dtsi is deprecated.
Hence remove it from the example.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Documentation/devicetree/bindings/arm/arm-boards | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/arm-boards b/Documentation/devicetree/bindings/arm/arm-boards
index ab318a56fca2194f..e667ecbcf226dfe3 100644
--- a/Documentation/devicetree/bindings/arm/arm-boards
+++ b/Documentation/devicetree/bindings/arm/arm-boards
@@ -148,15 +148,14 @@ Example:
/dts-v1/;
#include <dt-bindings/interrupt-controller/irq.h>
-#include "skeleton.dtsi"
/ {
model = "ARM RealView PB1176 with device tree";
compatible = "arm,realview-pb1176";
+ #address-cells = <0>;
+ #size-cells = <1>;
soc {
- #address-cells = <1>;
- #size-cells = <1>;
compatible = "arm,realview-pb1176-soc", "simple-bus";
ranges;
--
1.9.1
^ permalink raw reply related
* [PATCH 5/5] ARM: dts: sh73a0: Remove skeleton.dtsi inclusion
From: Geert Uytterhoeven @ 2016-10-21 9:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477041370-22778-1-git-send-email-geert+renesas@glider.be>
As of commit 9c0da3cc61f1233c ("ARM: dts: explicitly mark skeleton.dtsi
as deprecated"), including skeleton.dtsi is deprecated.
This fixes the following warning with W=1:
Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
arch/arm/boot/dts/sh73a0.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/sh73a0.dtsi b/arch/arm/boot/dts/sh73a0.dtsi
index 032fe2f14b16998f..e1267590b57570e5 100644
--- a/arch/arm/boot/dts/sh73a0.dtsi
+++ b/arch/arm/boot/dts/sh73a0.dtsi
@@ -8,8 +8,6 @@
* kind, whether express or implied.
*/
-/include/ "skeleton.dtsi"
-
#include <dt-bindings/clock/sh73a0-clock.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/interrupt-controller/irq.h>
@@ -17,6 +15,8 @@
/ {
compatible = "renesas,sh73a0";
interrupt-parent = <&gic>;
+ #address-cells = <1>;
+ #size-cells = <1>;
cpus {
#address-cells = <1>;
--
1.9.1
^ permalink raw reply related
* [PATCH 4/5] ARM: dts: r8a7740: Remove skeleton.dtsi inclusion
From: Geert Uytterhoeven @ 2016-10-21 9:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477041370-22778-1-git-send-email-geert+renesas@glider.be>
As of commit 9c0da3cc61f1233c ("ARM: dts: explicitly mark skeleton.dtsi
as deprecated"), including skeleton.dtsi is deprecated.
This fixes the following warning with W=1:
Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
arch/arm/boot/dts/r8a7740.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/r8a7740.dtsi b/arch/arm/boot/dts/r8a7740.dtsi
index 159e04eb1b9e55da..34159a8349def81e 100644
--- a/arch/arm/boot/dts/r8a7740.dtsi
+++ b/arch/arm/boot/dts/r8a7740.dtsi
@@ -8,8 +8,6 @@
* kind, whether express or implied.
*/
-/include/ "skeleton.dtsi"
-
#include <dt-bindings/clock/r8a7740-clock.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/interrupt-controller/irq.h>
@@ -17,6 +15,8 @@
/ {
compatible = "renesas,r8a7740";
interrupt-parent = <&gic>;
+ #address-cells = <1>;
+ #size-cells = <1>;
cpus {
#address-cells = <1>;
--
1.9.1
^ permalink raw reply related
* [PATCH 3/5] ARM: dts: r8a7779: Remove skeleton.dtsi inclusion
From: Geert Uytterhoeven @ 2016-10-21 9:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477041370-22778-1-git-send-email-geert+renesas@glider.be>
As of commit 9c0da3cc61f1233c ("ARM: dts: explicitly mark skeleton.dtsi
as deprecated"), including skeleton.dtsi is deprecated.
This fixes the following warning with W=1:
Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
arch/arm/boot/dts/r8a7779.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/r8a7779.dtsi b/arch/arm/boot/dts/r8a7779.dtsi
index b9bbcce69dfbd5b9..cd8c52d5cb850766 100644
--- a/arch/arm/boot/dts/r8a7779.dtsi
+++ b/arch/arm/boot/dts/r8a7779.dtsi
@@ -9,8 +9,6 @@
* kind, whether express or implied.
*/
-/include/ "skeleton.dtsi"
-
#include <dt-bindings/clock/r8a7779-clock.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/interrupt-controller/irq.h>
@@ -19,6 +17,8 @@
/ {
compatible = "renesas,r8a7779";
interrupt-parent = <&gic>;
+ #address-cells = <1>;
+ #size-cells = <1>;
cpus {
#address-cells = <1>;
--
1.9.1
^ permalink raw reply related
* [PATCH 2/5] ARM: dts: r8a7778: Remove skeleton.dtsi inclusion
From: Geert Uytterhoeven @ 2016-10-21 9:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477041370-22778-1-git-send-email-geert+renesas@glider.be>
As of commit 9c0da3cc61f1233c ("ARM: dts: explicitly mark skeleton.dtsi
as deprecated"), including skeleton.dtsi is deprecated.
This fixes the following warning with W=1:
Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
arch/arm/boot/dts/r8a7778.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/r8a7778.dtsi b/arch/arm/boot/dts/r8a7778.dtsi
index e571d66ea0fe1721..f3ffe1d315443e7e 100644
--- a/arch/arm/boot/dts/r8a7778.dtsi
+++ b/arch/arm/boot/dts/r8a7778.dtsi
@@ -14,8 +14,6 @@
* kind, whether express or implied.
*/
-/include/ "skeleton.dtsi"
-
#include <dt-bindings/clock/r8a7778-clock.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/interrupt-controller/irq.h>
@@ -23,6 +21,8 @@
/ {
compatible = "renesas,r8a7778";
interrupt-parent = <&gic>;
+ #address-cells = <1>;
+ #size-cells = <1>;
cpus {
#address-cells = <1>;
--
1.9.1
^ permalink raw reply related
* [PATCH 1/5] ARM: dts: emev2: Remove skeleton.dtsi inclusion
From: Geert Uytterhoeven @ 2016-10-21 9:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477041370-22778-1-git-send-email-geert+renesas@glider.be>
As of commit 9c0da3cc61f1233c ("ARM: dts: explicitly mark skeleton.dtsi
as deprecated"), including skeleton.dtsi is deprecated.
This fixes the following warning with W=1:
Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
arch/arm/boot/dts/emev2.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/emev2.dtsi b/arch/arm/boot/dts/emev2.dtsi
index cd119400f440bb5a..0124faf175c8612b 100644
--- a/arch/arm/boot/dts/emev2.dtsi
+++ b/arch/arm/boot/dts/emev2.dtsi
@@ -8,13 +8,14 @@
* kind, whether express or implied.
*/
-#include "skeleton.dtsi"
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/interrupt-controller/irq.h>
/ {
compatible = "renesas,emev2";
interrupt-parent = <&gic>;
+ #address-cells = <1>;
+ #size-cells = <1>;
aliases {
gpio0 = &gpio0;
--
1.9.1
^ permalink raw reply related
* [PATCH 0/5] ARM: dts: renesas: Remove skeleton.dtsi inclusion
From: Geert Uytterhoeven @ 2016-10-21 9:16 UTC (permalink / raw)
To: linux-arm-kernel
Hi Simon, Magnus,
As of commit 9c0da3cc61f1233c ("ARM: dts: explicitly mark skeleton.dtsi
as deprecated"), including skeleton.dtsi is deprecated.
Hence this series removes its use for all Renesas 32-bit ARM SoCs.
This fixes the following warning seen with W=1:
Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name
Thanks for applying!
Geert Uytterhoeven (5):
ARM: dts: emev2: Remove skeleton.dtsi inclusion
ARM: dts: r8a7778: Remove skeleton.dtsi inclusion
ARM: dts: r8a7779: Remove skeleton.dtsi inclusion
ARM: dts: r8a7740: Remove skeleton.dtsi inclusion
ARM: dts: sh73a0: Remove skeleton.dtsi inclusion
arch/arm/boot/dts/emev2.dtsi | 3 ++-
arch/arm/boot/dts/r8a7740.dtsi | 4 ++--
arch/arm/boot/dts/r8a7778.dtsi | 4 ++--
arch/arm/boot/dts/r8a7779.dtsi | 4 ++--
arch/arm/boot/dts/sh73a0.dtsi | 4 ++--
5 files changed, 10 insertions(+), 9 deletions(-)
--
1.9.1
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] ARM: sti: stih407-clocks: Identify critical clocks
From: Peter Griffin @ 2016-10-21 9:08 UTC (permalink / raw)
To: linux-arm-kernel
Lots of platforms contain clocks which if turned off would prove fatal.
The only way to recover is to restart the board(s). This driver takes
references to clocks which are required to be always-on. The Common
Clk Framework will then take references to them. This way they will
not be turned off during the clk_disabled_unused() procedure.
In this patch we are identifying clocks, which if gated would render
the STiH407 development board unserviceable.
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
arch/arm/boot/dts/stih407-clock.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/stih407-clock.dtsi b/arch/arm/boot/dts/stih407-clock.dtsi
index 13029c0..34c119a 100644
--- a/arch/arm/boot/dts/stih407-clock.dtsi
+++ b/arch/arm/boot/dts/stih407-clock.dtsi
@@ -101,6 +101,7 @@
clocks = <&clk_sysin>;
clock-output-names = "clk-s-a0-pll-ofd-0";
+ clock-critical = <0>; /* clk-s-a0-pll-ofd-0 */
};
clk_s_a0_flexgen: clk-s-a0-flexgen {
@@ -112,6 +113,7 @@
<&clk_sysin>;
clock-output-names = "clk-ic-lmi0";
+ clock-critical = <CLK_IC_LMI0>;
};
};
@@ -126,6 +128,7 @@
"clk-s-c0-fs0-ch1",
"clk-s-c0-fs0-ch2",
"clk-s-c0-fs0-ch3";
+ clock-critical = <0>; /* clk-s-c0-fs0-ch0 */
};
clk_s_c0: clockgen-c at 09103000 {
@@ -139,6 +142,7 @@
clocks = <&clk_sysin>;
clock-output-names = "clk-s-c0-pll0-odf-0";
+ clock-critical = <0>; /* clk-s-c0-pll0-odf-0 */
};
clk_s_c0_pll1: clk-s-c0-pll1 {
@@ -194,6 +198,12 @@
"clk-main-disp",
"clk-aux-disp",
"clk-compo-dvp";
+ clock-critical = <CLK_PROC_STFE>,
+ <CLK_ICN_CPU>,
+ <CLK_TX_ICN_DMU>,
+ <CLK_EXT2F_A9>,
+ <CLK_ICN_LMI>,
+ <CLK_ICN_SBC>;
};
};
--
1.9.1
^ permalink raw reply related
* [PATCH 4/9] pinctrl: meson: allow gpio to request irq
From: Jerome Brunet @ 2016-10-21 9:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdaCZPS45t3eTcjJExOu=YT_YkjNkdpg09jQqvLQOJH=Qw@mail.gmail.com>
On Thu, 2016-10-20 at 21:21 +0200, Linus Walleij wrote:
> On Wed, Oct 19, 2016 at 12:08 PM, Jerome Brunet <jbrunet@baylibre.com
> > wrote:
>
> >
> > Add the ability for gpio to request irq from the gpio interrupt
> > controller
> > if present. We have to specificaly that the parent interrupt
> > controller is
> > the gpio interrupt controller because gpio on meson SoCs can't
> > generate
> > interrupt directly on the GIC.
> >
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> (...)
> >
> > +???????select IRQ_DOMAIN
> > ????????select OF_GPIO
> > +???????select OF_IRQ
> (...)
> >
> > +static int meson_gpio_to_hwirq(struct meson_bank *bank, unsigned
> > int offset)
> > +{
> > +???????unsigned int hwirq;
> > +
> > +???????if (bank->irq_first < 0)
> > +???????????????/* this bank cannot generate irqs */
> > +???????????????return -1;
> > +
> > +???????hwirq = offset - bank->first + bank->irq_first;
> > +
> > +???????if (hwirq > bank->irq_last)
> > +???????????????/* this pin cannot generate irqs */
> > +???????????????return -1;
> > +
> > +???????return hwirq;
> > +}
>
> This is reimplementing irqdomain.
>
> >
> > +static int meson_gpio_to_irq(struct gpio_chip *chip, unsigned int
> > offset)
> > +{
> (...)
> >
> > +???????hwirq = meson_gpio_to_hwirq(bank, offset);
> > +???????if (hwirq < 0) {
> > +???????????????dev_dbg(pc->dev, "no interrupt for pin %u\n",
> > offset);
> > +???????????????return 0;
> > +???????}
>
> Isn't this usecase (also as described in the cover letter) a textbook
> example of when you should be using hierarchical irqdomain?
>
> Please check with Marc et al on hierarchical irqdomains.
Linus,
Do you mean I should create a new hierarchical?irqdomains in each of
the two pinctrl instances we have in these SoC, these domains being
stacked on the one I just added for controller in irqchip ?
I did not understand this is what you meant when I asked you the
question at ELCE.
>
> Yours,
> Linus Walleij
^ permalink raw reply
* [PATCH v3 00/17] pinctrl: exynos/samsung: Add header with values used for configuration
From: Linus Walleij @ 2016-10-21 9:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1472987060-28293-1-git-send-email-krzk@kernel.org>
On Sun, Sep 4, 2016 at 1:04 PM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> pinctrl: dt-bindings: samsung: Add header with values used for
> configuration
> pinctrl: dt-bindings: samsung: Update documentation with new macros
These two:
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Please merge all of it through the Exynos tree and ARM SoC once
you're done with the rewrite.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH v2 2/2] ARM: oxnas: Add OX820 config and makefile entry
From: Neil Armstrong @ 2016-10-21 8:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161021085848.1754-1-narmstrong@baylibre.com>
Refactor the oxnas Kconfig entries among the OX810SE and OX820 configs,
and add the files to support the OX820 SMP feature.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
arch/arm/Makefile | 1 +
arch/arm/mach-oxnas/Kconfig | 30 +++++++++++++++++++++---------
2 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 6be9ee1..68312a9 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -191,6 +191,7 @@ machine-$(CONFIG_ARCH_MXS) += mxs
machine-$(CONFIG_ARCH_NETX) += netx
machine-$(CONFIG_ARCH_NOMADIK) += nomadik
machine-$(CONFIG_ARCH_NSPIRE) += nspire
+machine-$(CONFIG_ARCH_OXNAS) += oxnas
machine-$(CONFIG_ARCH_OMAP1) += omap1
machine-$(CONFIG_ARCH_OMAP2PLUS) += omap2
machine-$(CONFIG_ARCH_ORION5X) += orion5x
diff --git a/arch/arm/mach-oxnas/Kconfig b/arch/arm/mach-oxnas/Kconfig
index 29100be..8fa4557 100644
--- a/arch/arm/mach-oxnas/Kconfig
+++ b/arch/arm/mach-oxnas/Kconfig
@@ -1,9 +1,16 @@
menuconfig ARCH_OXNAS
bool "Oxford Semiconductor OXNAS Family SoCs"
select ARCH_HAS_RESET_CONTROLLER
+ select COMMON_CLK_OXNAS
select GPIOLIB
+ select MFD_SYSCON
+ select OXNAS_RPS_TIMER
+ select PINCTRL_OXNAS
+ select RESET_CONTROLLER
+ select RESET_OXNAS
+ select VERSATILE_FPGA_IRQ
select PINCTRL
- depends on ARCH_MULTI_V5
+ depends on ARCH_MULTI_V5 || ARCH_MULTI_V6
help
Support for OxNas SoC family developed by Oxford Semiconductor.
@@ -11,16 +18,21 @@ if ARCH_OXNAS
config MACH_OX810SE
bool "Support OX810SE Based Products"
- select ARCH_HAS_RESET_CONTROLLER
- select COMMON_CLK_OXNAS
+ depends on ARCH_MULTI_V5
select CPU_ARM926T
- select MFD_SYSCON
- select OXNAS_RPS_TIMER
- select PINCTRL_OXNAS
- select RESET_CONTROLLER
- select RESET_OXNAS
- select VERSATILE_FPGA_IRQ
help
Include Support for the Oxford Semiconductor OX810SE SoC Based Products.
+config MACH_OX820
+ bool "Support OX820 Based Products"
+ depends on ARCH_MULTI_V6
+ select ARM_GIC
+ select DMA_CACHE_RWFO if SMP
+ select CPU_V6K
+ select HAVE_SMP
+ select HAVE_ARM_SCU if SMP
+ select HAVE_ARM_TWD if SMP
+ help
+ Include Support for the Oxford Semiconductor OX820 SoC Based Products.
+
endif
--
2.7.0
^ permalink raw reply related
* [PATCH v2 1/2] ARM: oxnas: Add OX820 SMP support
From: Neil Armstrong @ 2016-10-21 8:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161021085848.1754-1-narmstrong@baylibre.com>
The Oxford Semiconductor OX820 is a ARM11MPcore based SoC sharing some
features with the OX810 earlier SoC.
This patch adds the core to wake up the second core.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
arch/arm/mach-oxnas/Makefile | 2 +
arch/arm/mach-oxnas/headsmp.S | 28 +++++++++++
arch/arm/mach-oxnas/hotplug.c | 111 ++++++++++++++++++++++++++++++++++++++++++
arch/arm/mach-oxnas/platsmp.c | 104 +++++++++++++++++++++++++++++++++++++++
4 files changed, 245 insertions(+)
create mode 100644 arch/arm/mach-oxnas/Makefile
create mode 100644 arch/arm/mach-oxnas/headsmp.S
create mode 100644 arch/arm/mach-oxnas/hotplug.c
create mode 100644 arch/arm/mach-oxnas/platsmp.c
diff --git a/arch/arm/mach-oxnas/Makefile b/arch/arm/mach-oxnas/Makefile
new file mode 100644
index 0000000..b625906
--- /dev/null
+++ b/arch/arm/mach-oxnas/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_SMP) += platsmp.o headsmp.o
+obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
diff --git a/arch/arm/mach-oxnas/headsmp.S b/arch/arm/mach-oxnas/headsmp.S
new file mode 100644
index 0000000..2a94dcb
--- /dev/null
+++ b/arch/arm/mach-oxnas/headsmp.S
@@ -0,0 +1,28 @@
+/*
+ * linux/arch/arm/mach-oxnas/headsmp.S
+ *
+ * Copyright (C) 2013 Ma Haijun <mahaijuns@gmail.com>
+ * Copyright (c) 2003 ARM Limited
+ * All Rights Reserved
+ *
+ * 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/linkage.h>
+#include <linux/init.h>
+
+ __INIT
+
+/*
+ * OX820 specific entry point for secondary CPUs.
+ */
+ENTRY(ox820_secondary_startup)
+ mov r4, #0
+ /* invalidate both caches and branch target cache */
+ mcr p15, 0, r4, c7, c7, 0
+ /*
+ * we've been released from the holding pen: secondary_stack
+ * should now contain the SVC stack for this core
+ */
+ b secondary_startup
diff --git a/arch/arm/mach-oxnas/hotplug.c b/arch/arm/mach-oxnas/hotplug.c
new file mode 100644
index 0000000..18fa814
--- /dev/null
+++ b/arch/arm/mach-oxnas/hotplug.c
@@ -0,0 +1,111 @@
+/*
+ * linux/arch/arm/mach-oxnas/hotplug.c
+ *
+ * Copyright (C) 2002 ARM Ltd.
+ * All Rights Reserved
+ *
+ * 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/kernel.h>
+#include <linux/errno.h>
+#include <linux/smp.h>
+
+#include <asm/cp15.h>
+#include <asm/smp_plat.h>
+
+static inline void cpu_enter_lowpower(void)
+{
+ unsigned int v;
+
+ asm volatile(
+ " mcr p15, 0, %1, c7, c5, 0\n"
+ " mcr p15, 0, %1, c7, c10, 4\n"
+ /*
+ * Turn off coherency
+ */
+ " mrc p15, 0, %0, c1, c0, 1\n"
+ " bic %0, %0, #0x20\n"
+ " mcr p15, 0, %0, c1, c0, 1\n"
+ " mrc p15, 0, %0, c1, c0, 0\n"
+ " bic %0, %0, %2\n"
+ " mcr p15, 0, %0, c1, c0, 0\n"
+ : "=&r" (v)
+ : "r" (0), "Ir" (CR_C)
+ : "cc");
+}
+
+static inline void cpu_leave_lowpower(void)
+{
+ unsigned int v;
+
+ asm volatile( "mrc p15, 0, %0, c1, c0, 0\n"
+ " orr %0, %0, %1\n"
+ " mcr p15, 0, %0, c1, c0, 0\n"
+ " mrc p15, 0, %0, c1, c0, 1\n"
+ " orr %0, %0, #0x20\n"
+ " mcr p15, 0, %0, c1, c0, 1\n"
+ : "=&r" (v)
+ : "Ir" (CR_C)
+ : "cc");
+}
+
+static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
+{
+ /*
+ * there is no power-control hardware on this platform, so all
+ * we can do is put the core into WFI; this is safe as the calling
+ * code will have already disabled interrupts
+ */
+ for (;;) {
+ /*
+ * here's the WFI
+ */
+ asm(".word 0xe320f003\n"
+ :
+ :
+ : "memory", "cc");
+
+ if (pen_release == cpu_logical_map(cpu)) {
+ /*
+ * OK, proper wakeup, we're done
+ */
+ break;
+ }
+
+ /*
+ * Getting here, means that we have come out of WFI without
+ * having been woken up - this shouldn't happen
+ *
+ * Just note it happening - when we're woken, we can report
+ * its occurrence.
+ */
+ (*spurious)++;
+ }
+}
+
+/*
+ * platform-specific code to shutdown a CPU
+ *
+ * Called with IRQs disabled
+ */
+void ox820_cpu_die(unsigned int cpu)
+{
+ int spurious = 0;
+
+ /*
+ * we're ready for shutdown now, so do it
+ */
+ cpu_enter_lowpower();
+ platform_do_lowpower(cpu, &spurious);
+
+ /*
+ * bring this CPU back into the world of cache
+ * coherency, and then restore interrupts
+ */
+ cpu_leave_lowpower();
+
+ if (spurious)
+ pr_warn("CPU%u: %u spurious wakeup calls\n", cpu, spurious);
+}
diff --git a/arch/arm/mach-oxnas/platsmp.c b/arch/arm/mach-oxnas/platsmp.c
new file mode 100644
index 0000000..fd263b3
--- /dev/null
+++ b/arch/arm/mach-oxnas/platsmp.c
@@ -0,0 +1,104 @@
+/*
+ * arch/arm/mach-oxnas/platsmp.c
+ *
+ * Copyright (C) 2016 Neil Armstrong <narmstrong@baylibre.com>
+ * Copyright (C) 2013 Ma Haijun <mahaijuns@gmail.com>
+ * Copyright (C) 2002 ARM Ltd.
+ * All Rights Reserved
+ *
+ * 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/io.h>
+#include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+#include <asm/cacheflush.h>
+#include <asm/cp15.h>
+#include <asm/smp_plat.h>
+#include <asm/smp_scu.h>
+
+extern void ox820_secondary_startup(void);
+extern void ox820_cpu_die(unsigned int cpu);
+
+static void __iomem *cpu_ctrl;
+static void __iomem *gic_cpu_ctrl;
+
+#define HOLDINGPEN_CPU_OFFSET 0xc8
+#define HOLDINGPEN_LOCATION_OFFSET 0xc4
+
+#define GIC_NCPU_OFFSET(cpu) (0x100 + (cpu)*0x100)
+#define GIC_CPU_CTRL 0x00
+#define GIC_CPU_CTRL_ENABLE 1
+
+int __init ox820_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+ /*
+ * Write the address of secondary startup into the
+ * system-wide flags register. The BootMonitor waits
+ * until it receives a soft interrupt, and then the
+ * secondary CPU branches to this address.
+ */
+ writel(virt_to_phys(ox820_secondary_startup),
+ cpu_ctrl + HOLDINGPEN_LOCATION_OFFSET);
+
+ writel(cpu, cpu_ctrl + HOLDINGPEN_CPU_OFFSET);
+
+ /*
+ * Enable GIC cpu interface in CPU Interface Control Register
+ */
+ writel(GIC_CPU_CTRL_ENABLE,
+ gic_cpu_ctrl + GIC_NCPU_OFFSET(cpu) + GIC_CPU_CTRL);
+
+ /*
+ * Send the secondary CPU a soft interrupt, thereby causing
+ * the boot monitor to read the system wide flags register,
+ * and branch to the address found there.
+ */
+ arch_send_wakeup_ipi_mask(cpumask_of(cpu));
+
+ return 0;
+}
+
+static void __init ox820_smp_prepare_cpus(unsigned int max_cpus)
+{
+ struct device_node *np;
+ void __iomem *scu_base;
+
+ np = of_find_compatible_node(NULL, NULL, "arm,arm11mp-scu");
+ scu_base = of_iomap(np, 0);
+ of_node_put(np);
+ if (!scu_base)
+ return;
+
+ /* Remap CPU Interrupt Interface Registers */
+ np = of_find_compatible_node(NULL, NULL, "arm,arm11mp-gic");
+ gic_cpu_ctrl = of_iomap(np, 1);
+ of_node_put(np);
+ if (!gic_cpu_ctrl)
+ goto unmap_scu;
+
+ np = of_find_compatible_node(NULL, NULL, "oxsemi,ox820-sys-ctrl");
+ cpu_ctrl = of_iomap(np, 0);
+ of_node_put(np);
+ if (!cpu_ctrl)
+ goto unmap_scu;
+
+ scu_enable(scu_base);
+ flush_cache_all();
+
+unmap_scu:
+ iounmap(scu_base);
+}
+
+static const struct smp_operations ox820_smp_ops __initconst = {
+ .smp_prepare_cpus = ox820_smp_prepare_cpus,
+ .smp_boot_secondary = ox820_boot_secondary,
+#ifdef CONFIG_HOTPLUG_CPU
+ .cpu_die = ox820_cpu_die,
+#endif
+};
+
+CPU_METHOD_OF_DECLARE(ox820_smp, "oxsemi,ox820-smp", &ox820_smp_ops);
--
2.7.0
^ permalink raw reply related
* [PATCH] arm64: mm: fix __page_to_voff definition
From: Neeraj Upadhyay @ 2016-10-21 8:58 UTC (permalink / raw)
To: linux-arm-kernel
Fix parameter name for __page_to_voff, to match its definition.
At present, we don't see any issue, as page_to_virt's caller
declares 'page'.
Fixes: 9f2875912dac ("arm64: mm: restrict virt_to_page() to the linear mapping")
Signed-off-by: Neeraj Upadhyay <neeraju@codeaurora.org>
---
arch/arm64/include/asm/memory.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index ba62df8..b71086d 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -217,7 +217,7 @@ static inline void *phys_to_virt(phys_addr_t x)
#define _virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
#else
#define __virt_to_pgoff(kaddr) (((u64)(kaddr) & ~PAGE_OFFSET) / PAGE_SIZE * sizeof(struct page))
-#define __page_to_voff(kaddr) (((u64)(page) & ~VMEMMAP_START) * PAGE_SIZE / sizeof(struct page))
+#define __page_to_voff(page) (((u64)(page) & ~VMEMMAP_START) * PAGE_SIZE / sizeof(struct page))
#define page_to_virt(page) ((void *)((__page_to_voff(page)) | PAGE_OFFSET))
#define virt_to_page(vaddr) ((struct page *)((__virt_to_pgoff(vaddr)) | VMEMMAP_START))
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related
* [PATCH v2 0/2] ARM: oxnas: Add SMP support for OX820
From: Neil Armstrong @ 2016-10-21 8:58 UTC (permalink / raw)
To: linux-arm-kernel
In order to support the SMP feature of the Oxford Semiconductor OX820 SoC,
add the necessary code to handle the wake-up, hotplug and cpu entry.
The OX820 has an ARM11MPCORE cluster with 2 cores and has proper hardware
support for secondary core booting.
Changes since v1 at http://lkml.kernel.org/r/20161017084303.20078-1-narmstrong at baylibre.com
- Remove useless holding pen loops and spinlock in boot_secondary
Neil Armstrong (2):
ARM: oxnas: Add OX820 SMP support
ARM: oxnas: Add OX820 config and makefile entry
arch/arm/Makefile | 1 +
arch/arm/mach-oxnas/Kconfig | 30 ++++++++----
arch/arm/mach-oxnas/Makefile | 2 +
arch/arm/mach-oxnas/headsmp.S | 28 +++++++++++
arch/arm/mach-oxnas/hotplug.c | 111 ++++++++++++++++++++++++++++++++++++++++++
arch/arm/mach-oxnas/platsmp.c | 104 +++++++++++++++++++++++++++++++++++++++
6 files changed, 267 insertions(+), 9 deletions(-)
create mode 100644 arch/arm/mach-oxnas/Makefile
create mode 100644 arch/arm/mach-oxnas/headsmp.S
create mode 100644 arch/arm/mach-oxnas/hotplug.c
create mode 100644 arch/arm/mach-oxnas/platsmp.c
--
2.7.0
^ permalink raw reply
* [PATCH] [media] c8sectpfe: Remove clk_disable_unprepare hacks
From: Peter Griffin @ 2016-10-21 8:55 UTC (permalink / raw)
To: linux-arm-kernel
Now that CLK_PROC_STFE is defined as a critical clock in
DT, we can remove the commented clk_disable_unprepare from
the c8sectpfe driver. This means we now have balanced
clk*enable/disable calls in the driver, but on STiH407
family the clock in reality will never actually be disabled.
This is due to a HW bug where once the IP has been configured
and the SLIM core is running, disabling the clock causes a
unrecoverable bus lockup.
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
index 30c148b..79d793b 100644
--- a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
+++ b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
@@ -888,8 +888,7 @@ static int c8sectpfe_probe(struct platform_device *pdev)
return 0;
err_clk_disable:
- /* TODO uncomment when upstream has taken a reference on this clk */
- /*clk_disable_unprepare(fei->c8sectpfeclk);*/
+ clk_disable_unprepare(fei->c8sectpfeclk);
return ret;
}
@@ -924,11 +923,8 @@ static int c8sectpfe_remove(struct platform_device *pdev)
if (readl(fei->io + SYS_OTHER_CLKEN))
writel(0, fei->io + SYS_OTHER_CLKEN);
- /* TODO uncomment when upstream has taken a reference on this clk */
- /*
if (fei->c8sectpfeclk)
clk_disable_unprepare(fei->c8sectpfeclk);
- */
return 0;
}
--
1.9.1
^ permalink raw reply related
* [PATCH v2 6/6] powerpc: dts: acadia: DT fix s/#interrupts-parent/#interrupt-parent/
From: Geert Uytterhoeven @ 2016-10-21 8:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477039877-20227-1-git-send-email-geert+renesas@glider.be>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Rob Herring <robh@kernel.org>
---
v2:
- Add Acked-by.
---
arch/powerpc/boot/dts/acadia.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/boot/dts/acadia.dts b/arch/powerpc/boot/dts/acadia.dts
index 57291f61ffe7021a..86266159521edac2 100644
--- a/arch/powerpc/boot/dts/acadia.dts
+++ b/arch/powerpc/boot/dts/acadia.dts
@@ -183,7 +183,7 @@
usb at ef603000 {
compatible = "ohci-be";
reg = <0xef603000 0x80>;
- interrupts-parent = <&UIC0>;
+ interrupt-parent = <&UIC0>;
interrupts = <0xd 0x4 0xe 0x4>;
};
--
1.9.1
^ permalink raw reply related
* [PATCH v2 5/6] dt: booting-without-of: DT fix s/#interrupt-cell/#interrupt-cells/
From: Geert Uytterhoeven @ 2016-10-21 8:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477039877-20227-1-git-send-email-geert+renesas@glider.be>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Rob Herring <robh@kernel.org>
---
v2:
- Add Acked-by.
---
Documentation/devicetree/booting-without-of.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/booting-without-of.txt b/Documentation/devicetree/booting-without-of.txt
index 3f1437fbca6b49f3..5bcea91c0cc65af2 100644
--- a/Documentation/devicetree/booting-without-of.txt
+++ b/Documentation/devicetree/booting-without-of.txt
@@ -1302,7 +1302,7 @@ number and level/sense information. All interrupt children in an
OpenPIC interrupt domain use 2 cells per interrupt in their interrupts
property.
-The PCI bus binding specifies a #interrupt-cell value of 1 to encode
+The PCI bus binding specifies a #interrupt-cells value of 1 to encode
which interrupt pin (INTA,INTB,INTC,INTD) is used.
2) interrupt-parent property
--
1.9.1
^ permalink raw reply related
* [PATCH v2 4/6] ASoC: davinci-mcbsp: DT fix s/interrupts-names/interrupt-names/
From: Geert Uytterhoeven @ 2016-10-21 8:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477039877-20227-1-git-send-email-geert+renesas@glider.be>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Rob Herring <robh@kernel.org>
---
v2:
- Add Acked-by,
- Split off from a completely unrelated patch.
---
Documentation/devicetree/bindings/sound/davinci-mcbsp.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt b/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
index 55b53e1fd72c9d6e..e0b6165c9cfcec19 100644
--- a/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
+++ b/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
@@ -43,7 +43,7 @@ mcbsp0: mcbsp at 1d10000 {
<0x00310000 0x1000>;
reg-names = "mpu", "dat";
interrupts = <97 98>;
- interrupts-names = "rx", "tx";
+ interrupt-names = "rx", "tx";
dmas = <&edma0 3 1
&edma0 2 1>;
dma-names = "tx", "rx";
--
1.9.1
^ permalink raw reply related
* [PATCH v2 3/6] arm64: dts: lg1313: DT fix s/#interrupts-cells/#interrupt-cells/
From: Geert Uytterhoeven @ 2016-10-21 8:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477039877-20227-1-git-send-email-geert+renesas@glider.be>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
- New.
---
arch/arm64/boot/dts/lg/lg1313.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/lg/lg1313.dtsi b/arch/arm64/boot/dts/lg/lg1313.dtsi
index e703e1149c757082..abb2162228e82d39 100644
--- a/arch/arm64/boot/dts/lg/lg1313.dtsi
+++ b/arch/arm64/boot/dts/lg/lg1313.dtsi
@@ -123,7 +123,7 @@
amba {
#address-cells = <2>;
#size-cells = <1>;
- #interrupts-cells = <3>;
+ #interrupt-cells = <3>;
compatible = "simple-bus";
interrupt-parent = <&gic>;
--
1.9.1
^ permalink raw reply related
* [PATCH v2 2/6] arm64: dts: lg1312: DT fix s/#interrupts-cells/#interrupt-cells/
From: Geert Uytterhoeven @ 2016-10-21 8:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477039877-20227-1-git-send-email-geert+renesas@glider.be>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Rob Herring <robh@kernel.org>
---
v2:
- Add Acked-by,
- Rebased.
---
arch/arm64/boot/dts/lg/lg1312.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/lg/lg1312.dtsi b/arch/arm64/boot/dts/lg/lg1312.dtsi
index fbafa24cd5335b90..647606a83c517448 100644
--- a/arch/arm64/boot/dts/lg/lg1312.dtsi
+++ b/arch/arm64/boot/dts/lg/lg1312.dtsi
@@ -123,7 +123,7 @@
amba {
#address-cells = <2>;
#size-cells = <1>;
- #interrupts-cells = <3>;
+ #interrupt-cells = <3>;
compatible = "simple-bus";
interrupt-parent = <&gic>;
--
1.9.1
^ permalink raw reply related
* [PATCH v2 1/6] ARM: dts: STiH407: DT fix s/interrupts-names/interrupt-names/
From: Geert Uytterhoeven @ 2016-10-21 8:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477039877-20227-1-git-send-email-geert+renesas@glider.be>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Rob Herring <robh@kernel.org>
---
v2:
- Add Acked-by.
---
arch/arm/boot/dts/stih407-pinctrl.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/stih407-pinctrl.dtsi b/arch/arm/boot/dts/stih407-pinctrl.dtsi
index c325cc059ae4bc80..daab16b5ae645cee 100644
--- a/arch/arm/boot/dts/stih407-pinctrl.dtsi
+++ b/arch/arm/boot/dts/stih407-pinctrl.dtsi
@@ -1157,7 +1157,7 @@
reg = <0x0923f080 0x4>;
reg-names = "irqmux";
interrupts = <GIC_SPI 192 IRQ_TYPE_NONE>;
- interrupts-names = "irqmux";
+ interrupt-names = "irqmux";
ranges = <0 0x09230000 0x3000>;
pio40: gpio at 09230000 {
--
1.9.1
^ permalink raw reply related
* [PATCH v2 0/6] DT: Fix spelling of standard properties
From: Geert Uytterhoeven @ 2016-10-21 8:51 UTC (permalink / raw)
To: linux-arm-kernel
Hi all,
This patch series fixes misspellings of various standard DT properties
in DT binding documentation, DTS files, and error messages.
While most of these are harmless, some of them may cause hard-to-debug
failures.
Changes compared to v2:
- Dropped patches that have been applied already,
- Add Rob Herring's Acked-by,
- Split off "[PATCH v2 4/6] ASoC: davinci-mcbsp: DT fix
s/interrupts-names/interrupt-names/" from the unrelated "[PATCH 06/14]
dmaengine: bcm2835: DT spelling s/interrupts-names/interrupt-names/",
- Add new patch "[PATCH v2 3/6] arm64: dts: lg1313: DT fix
s/#interrupts-cells/#interrupt-cells/".
Please apply where appropriate.
Thanks!
P.S. I used the following to detect misspellings:
words="(address|clock|cooling|dma|gpio|index|interrupt|mbox|msi|nvmem|phy|phys|power-domain|pwm|reset|size|sleep|sound-dai|thermal-sensor)"
git grep -Ew "${words}s-names"
git grep -E "[^-]\<${words}-name\>[^-]"
git grep -Ew "#${words}s-cells" # false positive phys-cells
git grep -E "#${words}-cell\>[^-]"
git grep -w adress-cells
git grep -Ew "interrupts-(map|parent)"
Geert Uytterhoeven (6):
ARM: dts: STiH407: DT fix s/interrupts-names/interrupt-names/
arm64: dts: lg1312: DT fix s/#interrupts-cells/#interrupt-cells/
arm64: dts: lg1313: DT fix s/#interrupts-cells/#interrupt-cells/
ASoC: davinci-mcbsp: DT fix s/interrupts-names/interrupt-names/
dt: booting-without-of: DT fix s/#interrupt-cell/#interrupt-cells/
powerpc: dts: acadia: DT fix s/#interrupts-parent/#interrupt-parent/
Documentation/devicetree/bindings/sound/davinci-mcbsp.txt | 2 +-
Documentation/devicetree/booting-without-of.txt | 2 +-
arch/arm/boot/dts/stih407-pinctrl.dtsi | 2 +-
arch/arm64/boot/dts/lg/lg1312.dtsi | 2 +-
arch/arm64/boot/dts/lg/lg1313.dtsi | 2 +-
arch/powerpc/boot/dts/acadia.dts | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
--
1.9.1
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 v2 1/9] irqchip: meson: add support for gpio interrupt controller
From: Jerome Brunet @ 2016-10-21 8:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ec297647-1ab7-7e6b-5945-be8360f92421@arm.com>
On Thu, 2016-10-20 at 17:33 +0100, Marc Zyngier wrote:
> Jerome,
>
> On 19/10/16 16:21, Jerome Brunet wrote:
> >
> > 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/licens
> > es/>.
> > + * 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 {
>
> The name of that structure is utterly confusing, as it doesn't
> contain
> anything related to an IRQ domain. Can you please clarify its usage?
This structure is holding the data of the controller. The name is
indeed misleading, I will change this. What about
'meson_gpio_irq_pdata' or 'meson_gpio_irq_controller' ?
>
> >
> > + 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,
> > +};
>
> If that a guarantee that these numbers will always represent the
> parent interrupt?
At the moment, the 3 supported SoC use these parent interrupts, but we
have absolutely no idea (or guarantee) that is will remain the same, or
even contiguous, in the upcoming SoC (like the GXM or GXL)
I reckon, it is likely that manufacturer will keep on using these
parent irqs for a while but I would prefer not make an assumption about
it in the driver.
If a SoC get a different set of interrupts I would have added a new
table like this and passed it to the appropriate params :
static irq_hw_number_t meson_new_parent_hwirqs[] = {
143, 144, 150, 151, 152, 173, 178, 179,
};
> It feels a bit odd not to get that information directly from
> the device tree, in the form of a device specific property. Something
> like:
>
> upstream-interrupts = <64 65 66 ... >;
>
I wondered about putting this information in DT or in the driver for a
while. Maybe DT would be a more suitable place holder for these data
(parent irq and number of provided hwirq) but I was under the
understanding that we should now put these information in the driver
and use the compatible property to get the appropriate parameters.
I'd love to get the view of the DT guys on this.
Also, since we already need to put some information about the pin the
pinctrl driver (to get the appropriate mux value of each pad), I
thought It would make sense to have the same approach here.
If there is some kind of policy saying this should be in DT, I'd be
happy to make the change.
> or even as a base/range:
>
> upstream-interrupts = <64 8>;
I would prefer to avoid using ranges as we have no guarantee the
manufacturer will keep the parent irqs contiguous in the future.
>
> Also, how does it work if you have more than a single device like
> this?
> This driver seems a do a great deal of dynamic allocation, and yet
> its
> core resource is completely static... Please pick a side! ;-)
I don't think we could have more than one instance per device and I
certainly did not have this case in mind while writing the code. If it
was the case someday, I guess having two compatibles would do the trick
:)
>
> >
> > +
> > +static const struct meson_gpio_irq_params meson8_params = {
> > + .nhwirq??= 134,
> > + .source??= meson_parent_hwirqs,
> > + .nsource = ARRAY_SIZE(meson_parent_hwirqs),
>
> I find it utterly confusing that you are calling source something
> that is an output for this controller.
I had the call sequence (GIC->GPIO_IRQ->handler) in mind while writing
it. I see your point and it is indeed confusing, I'll find something
better
> It makes my brain melt, and I don't like the feeling.
Ohhhh !! it's Halloween season and you don't need a costume anymore ;)
>
> >
> > +};
> > +
> > +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),
> > +};
>
> Same thing. How big is the variability of these structures? Are we
> going to see more of those? or is that now set into stone?
The number of pad mapped to the controller seems to change with every
SoC version. The parent irqs have not changed so far, but as explained
above, there is no guarantee it will keep on being this way.
So i'd say probably more of those ...
>
> +Mark: what's the policy to describe this kind of things?
Very interested about this question as well.
>
> >
> > +
> > +static const struct of_device_id meson_irq_gpio_matches[] = {
> > + {
> > + .compatible = "amlogic,meson8-gpio-intc",
>
> If it's an independent IP (as described in the commit message),
> shouldn't in be rescribed in a more SoC-independent way?
Ok, I was probably not very clear (again). What I meant in the cover
letter is that the interrupt gpio controller is independent for the
pad/gpio controller. For example, it could work even you did not setup
anything in pinmux or you did not instantiate pinctrl at all.
But, at least from my point of view, it is SoC dependent since the
number of pad routed to it is changing with every SoC version.
>
> >
> > + .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);
>
> Can't you use the relaxed accessors?
Indeed, this will be fixed.
>
> >
> > +}
> > +
> > +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;
>
> I'm a bit worried by this function. If you need an allocator, then
> having a simple bitmap is much better than iterating over an array.
>
> If you're using this to go from a hwirq to the structure describing
> your
> interrupt, this is wrong. You should never have to look-up something
> based on a hwirq, because that's what irq domains are for.
OK
>
> >
> > +}
> > +
> > +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);
>
> So assuming you turn this into a proper bitmap driven allocator...
>
> >
> > + if (index < 0) {
> > + pr_err("No irq available\n");
> > + return -ENOSPC;
> > + }
> > +
> > + domain_data->map[index] = hwirq;
>
> this can go away, as there is hardly any point in tracking the hwirq
> on
> its own. Actually, the whole map[] array looks totally useless.
>
> >
> > +
> > + 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;
>
> You can write this as:
>
> if (is_of_node() && fwspec->... == 2) {
OK
>
> >
> > +
> > + *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;
>
> How can you end-up here if the translate operation has failed?
You can't, will be removed
>
> >
> > +
> > + 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++) {
>
> This is a pattern that gets repeated over and over, for no good
> reason.
> The only reason we have this nr_irqs thing is to handle things like
> multi-MSI, where we have to *guarantee* that the hwirqs are allocated
> in
> a contiguous manner.
>
> Here, you don't enforce that guarantee, so you'd rather have a big
> fat:
>
> if (WARN_ON(nr_irqs != 1))
> return -EINVAL;
>
> and get rid of that loop, because I cannot imagine a case where you'd
> allocate more than a single interrupt at a time.
Thanks for this clarification. I was actually very confused about
getting a single fwspec and this 'nr_irqs' parameters. I could not
figure a case where one would want multiple irqs in one call, and
looking at the other irqchip drivers did not really help. In the end, I
tried to implement the API the best I could, thinking that somebody
probably had a very good reason for this.
I'm perfectly happy using your solution, it makes more sense.
>
> >
> > + 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;
>
> Why do you have to to this here? This should be handled by the core
> code already.
OK
>
> >
> > +
> > + cd = kzalloc(sizeof(*cd), GFP_KERNEL);
> > + if (!cd)
> > + return -ENOMEM;
> > +
> > + cd->base = domain_data->base;
> > + cd->index = index;
>
> So what is the exact purpose of this structure? The base address is
> essentially a global, or could be directly derived from the domain.
> The
> index is only used in set_type, and is the index of the pin connected
> to
> the GIC. It looks to me like you could have:
>
> irq_hw_number_t *out_line =
> &meson_parent_hwirqs[index];
meson_parent_hwirq is only declared this way to avoid writing the
parent irqs 3 times (in each SoC params).?
Using it this way would make it global and imply it is the same
whatever the SoC. This something I can't guarantee and I would prefer
to avoid that.
>
> >
> > +
> > + irq_domain_set_hwirq_and_chip(domain, virq + i,
> > hwirq + i,
> > + ??????&meson_gpio_irq_chip
> > , cd);
>
> and this written as
>
> irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
> ??????out_line);
>
> In your set_type function, you just compute the index back:
>
> irq_hw_number_t *out_line = irq_data_get_irq_chip_data(data);
> irq_hw_number_t index = out_line - meson_parent_hwirqs;
>
> and you're done.
>
Since I can derive the base address, I only need the index in set_type.
I'll rework this to get rid of this structure.
> >
> > +
> > + ret = meson_gpio_irq_allocate_gic_irq(domain, virq
> > + i,
> > + ??????source,
> > type);
>
> Resource leak on error.
Ok, Thx.
>
> >
> > + 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++) {
>
> Same comment as above.
OK
>
> >
> > + 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;
>
> You should now be able to kill most or all of this.
>
> >
> > + 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);
>
> Please be consistent in using the fwnode API instead of the of_node
> one.
OK
>
> >
> > +
> > + 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);
> >
>
> Overall, this driver is a bit of a mess. Tons of structures that
> don't
> make much sense, and a false sense of being able to support multiple
> instances of the device.
>
> I'll let Mark comment on the DT side of things.
>
> Thanks,
>
> M.
^ 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