Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1521298 > unrolled thread
| Started by | Pankaj Dubey <pankaj.dubey@samsung.com> |
|---|---|
| First post | 2016-11-14 06:00 +0100 |
| Last post | 2016-11-14 06:10 +0100 |
| Articles | 20 on this page of 39 — 8 participants |
Back to article view | Back to linux.kernel
[PATCH 00/16] Provide support of generic function for SCU enable Pankaj Dubey <pankaj.dubey@samsung.com> - 2016-11-14 06:00 +0100
[PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU Pankaj Dubey <pankaj.dubey@samsung.com> - 2016-11-14 06:00 +0100
Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU Jisheng Zhang <jszhang@marvell.com> - 2016-11-14 07:20 +0100
Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU Jisheng Zhang <jszhang@marvell.com> - 2016-11-14 08:10 +0100
Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU "pankaj.dubey" <pankaj.dubey@samsung.com> - 2016-11-14 10:00 +0100
Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU Jisheng Zhang <jszhang@marvell.com> - 2016-11-14 10:00 +0100
Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU "pankaj.dubey" <pankaj.dubey@samsung.com> - 2016-11-14 09:40 +0100
Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU Arnd Bergmann <arnd@arndb.de> - 2016-11-14 13:10 +0100
Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-11-14 15:00 +0100
Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU Arnd Bergmann <arnd@arndb.de> - 2016-11-14 15:40 +0100
Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-11-14 16:00 +0100
Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU "pankaj.dubey" <pankaj.dubey@samsung.com> - 2016-11-17 05:30 +0100
Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU Arnd Bergmann <arnd@arndb.de> - 2016-11-17 18:10 +0100
Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU "pankaj.dubey" <pankaj.dubey@samsung.com> - 2016-11-18 04:30 +0100
Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU Arnd Bergmann <arnd@arndb.de> - 2016-11-18 13:20 +0100
Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-11-18 13:50 +0100
Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU Arnd Bergmann <arnd@arndb.de> - 2016-11-18 14:40 +0100
Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-11-14 14:50 +0100
Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU "pankaj.dubey" <pankaj.dubey@samsung.com> - 2016-11-17 03:20 +0100
[PATCH 09/16] ARM: BCM: use generic API for enabling SCU Pankaj Dubey <pankaj.dubey@samsung.com> - 2016-11-14 06:10 +0100
Re: [PATCH 09/16] ARM: BCM: use generic API for enabling SCU Florian Fainelli <f.fainelli@gmail.com> - 2016-11-14 07:20 +0100
[PATCH 06/16] ARM: STi: use generic API for enabling SCU Pankaj Dubey <pankaj.dubey@samsung.com> - 2016-11-14 06:10 +0100
[PATCH 15/16] ARM: mvebu: use generic API for enabling SCU Pankaj Dubey <pankaj.dubey@samsung.com> - 2016-11-14 06:10 +0100
[PATCH 12/16] ARM: imx: use generic API for enabling SCU Pankaj Dubey <pankaj.dubey@samsung.com> - 2016-11-14 06:10 +0100
Re: [PATCH 12/16] ARM: imx: use generic API for enabling SCU Shawn Guo <shawnguo@kernel.org> - 2016-11-14 15:30 +0100
Re: [PATCH 12/16] ARM: imx: use generic API for enabling SCU "pankaj.dubey" <pankaj.dubey@samsung.com> - 2016-11-17 05:30 +0100
[PATCH 13/16] ARM: zynq: use generic API for enabling SCU Pankaj Dubey <pankaj.dubey@samsung.com> - 2016-11-14 06:10 +0100
[PATCH 08/16] ARM: vexpress: use generic API for enabling SCU Pankaj Dubey <pankaj.dubey@samsung.com> - 2016-11-14 06:10 +0100
Re: [PATCH 08/16] ARM: vexpress: use generic API for enabling SCU Sudeep Holla <sudeep.holla@arm.com> - 2016-11-16 15:40 +0100
Re: [PATCH 08/16] ARM: vexpress: use generic API for enabling SCU "pankaj.dubey" <pankaj.dubey@samsung.com> - 2016-11-17 03:20 +0100
[PATCH 05/16] ARM: socfpga: use generic API for enabling SCU Pankaj Dubey <pankaj.dubey@samsung.com> - 2016-11-14 06:10 +0100
[PATCH 03/16] ARM: berlin: use generic API for enabling SCU Pankaj Dubey <pankaj.dubey@samsung.com> - 2016-11-14 06:10 +0100
Re: [PATCH 03/16] ARM: berlin: use generic API for enabling SCU Jisheng Zhang <jszhang@marvell.com> - 2016-11-14 10:00 +0100
Re: [PATCH 03/16] ARM: berlin: use generic API for enabling SCU Pankaj Dubey <pankaj.dubey@samsung.com> - 2016-11-14 17:30 +0100
[PATCH 10/16] ARM: tegra: use generic API for enabling SCU Pankaj Dubey <pankaj.dubey@samsung.com> - 2016-11-14 06:10 +0100
[PATCH 16/16] ARM: zx: use generic API for enabling SCU Pankaj Dubey <pankaj.dubey@samsung.com> - 2016-11-14 06:10 +0100
[PATCH 11/16] ARM: rockchip: use generic API for enabling SCU Pankaj Dubey <pankaj.dubey@samsung.com> - 2016-11-14 06:10 +0100
[PATCH 14/16] ARM: hisi: use generic API for enabling SCU Pankaj Dubey <pankaj.dubey@samsung.com> - 2016-11-14 06:10 +0100
[PATCH 07/16] ARM: ux500: use generic API for enabling SCU Pankaj Dubey <pankaj.dubey@samsung.com> - 2016-11-14 06:10 +0100
Page 1 of 2 [1] 2 Next page →
| From | Pankaj Dubey <pankaj.dubey@samsung.com> |
|---|---|
| Date | 2016-11-14 06:00 +0100 |
| Subject | [PATCH 00/16] Provide support of generic function for SCU enable |
| Message-ID | <sDhx7-26l-15@gated-at.bofh.it> |
During consolidation work on Exynos platform for SCU, it has been found that across many ARM platforms Cortex-A9 boards, SCU enabling code is duplicated. Subsequently it felt that [1] by introducing generic API support in arm/smp_scu.c file will help in removing all these duplicate work from all such platforms. This patch series introduces support for parsting SCU device node to enable SCU and get SCU base address. If a platform does not support SCU device node in DT, the API will fallback to scu_a9_get_base to obtain SCU base address. [1]: https://www.spinics.net/lists/arm-kernel/msg541830.html Arnd helped in consolidating various ARM platforms duplicating SCU code as below: a) of_iomap: berlin, exynos, mvebu, realview, rockchip, socfpga, sti, ux500, vexpress b) ioremap(scu_a9_get_base()): bcm63xx, bcm, hisi, zx c) iotable_init, scu_a9_get_base(): imx, omap4, tegra, zynq d) ioremap(constant): shmobile, spear13xx Based on his suggestion on how to move all these platform to use generic API I have modified EXYNOS, berlin, realview, socfpga, STi, ux500, vexpress, BCM, tegra, rockchip, imx, zynq, hisi, mvebu and ZX platform files to adopt generic API for SCU enabling to obtaining SCU base address. Only fourth case shmobile and spear13xx will be left to adopt to this new API after this, which I felt some details information about these platforms and I leave it to the maintainers of these platforms. NOTE: This patch series has been tested on EXYNOS platform using Exynos4210 based Origen board for SMP boot. Rest platforms are *ONLY* Compiled Tested. I request maintainers/developers having h/w of these platforms to test this series on their platforms. Pankaj Dubey (16): ARM: scu: Provide support for parsing SCU device node to enable SCU ARM: EXYNOS: use generic API to enable SCU ARM: berlin: use generic API for enabling SCU ARM: realview: use generic API for enabling SCU ARM: socfpga: use generic API for enabling SCU ARM: STi: use generic API for enabling SCU ARM: ux500: use generic API for enabling SCU ARM: vexpress: use generic API for enabling SCU ARM: BCM: use generic API for enabling SCU ARM: tegra: use generic API for enabling SCU ARM: rockchip: use generic API for enabling SCU ARM: imx: use generic API for enabling SCU ARM: zynq: use generic API for enabling SCU ARM: hisi: use generic API for enabling SCU ARM: mvebu: use generic API for enabling SCU ARM: zx: use generic API for enabling SCU arch/arm/include/asm/smp_scu.h | 4 +++ arch/arm/kernel/smp_scu.c | 56 +++++++++++++++++++++++++++++++++++++ arch/arm/mach-bcm/bcm63xx_smp.c | 18 ++---------- arch/arm/mach-bcm/platsmp.c | 46 +----------------------------- arch/arm/mach-berlin/platsmp.c | 17 ++++------- arch/arm/mach-exynos/common.h | 1 - arch/arm/mach-exynos/platsmp.c | 30 +++----------------- arch/arm/mach-exynos/pm.c | 4 +-- arch/arm/mach-exynos/suspend.c | 14 +++------- arch/arm/mach-hisi/platsmp.c | 24 ++++------------ arch/arm/mach-imx/common.h | 5 ---- arch/arm/mach-imx/mach-imx6q.c | 8 +----- arch/arm/mach-imx/platsmp.c | 32 ++++----------------- arch/arm/mach-imx/pm-imx6.c | 3 +- arch/arm/mach-mvebu/board-v7.c | 8 ++---- arch/arm/mach-realview/platsmp-dt.c | 29 ++++++------------- arch/arm/mach-rockchip/platsmp.c | 12 ++------ arch/arm/mach-socfpga/platsmp.c | 11 +------- arch/arm/mach-sti/platsmp.c | 9 +----- arch/arm/mach-tegra/platsmp.c | 2 +- arch/arm/mach-ux500/platsmp.c | 20 +------------ arch/arm/mach-vexpress/platsmp.c | 13 +-------- arch/arm/mach-zx/platsmp.c | 6 ++-- arch/arm/mach-zynq/common.c | 32 +-------------------- arch/arm/mach-zynq/common.h | 2 -- arch/arm/mach-zynq/platsmp.c | 2 ++ 26 files changed, 115 insertions(+), 293 deletions(-) -- 2.7.4
[toc] | [next] | [standalone]
| From | Pankaj Dubey <pankaj.dubey@samsung.com> |
|---|---|
| Date | 2016-11-14 06:00 +0100 |
| Subject | [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU |
| Message-ID | <sDhx7-26l-21@gated-at.bofh.it> |
| In reply to | #1521298 |
Many platforms are duplicating code for enabling SCU, lets add
common code to enable SCU by parsing SCU device node so the duplication
in each platform can be avoided.
CC: Krzysztof Kozlowski <krzk@kernel.org>
CC: Jisheng Zhang <jszhang@marvell.com>
CC: Russell King <linux@armlinux.org.uk>
CC: Dinh Nguyen <dinguyen@opensource.altera.com>
CC: Patrice Chotard <patrice.chotard@st.com>
CC: Linus Walleij <linus.walleij@linaro.org>
CC: Liviu Dudau <liviu.dudau@arm.com>
CC: Ray Jui <rjui@broadcom.com>
CC: Stephen Warren <swarren@wwwdotorg.org>
CC: Heiko Stuebner <heiko@sntech.de>
CC: Shawn Guo <shawnguo@kernel.org>
CC: Michal Simek <michal.simek@xilinx.com>
CC: Wei Xu <xuwei5@hisilicon.com>
CC: Andrew Lunn <andrew@lunn.ch>
CC: Jun Nie <jun.nie@linaro.org>
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
arch/arm/include/asm/smp_scu.h | 4 +++
arch/arm/kernel/smp_scu.c | 56 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+)
diff --git a/arch/arm/include/asm/smp_scu.h b/arch/arm/include/asm/smp_scu.h
index bfe163c..fdeec07 100644
--- a/arch/arm/include/asm/smp_scu.h
+++ b/arch/arm/include/asm/smp_scu.h
@@ -39,8 +39,12 @@ static inline int scu_power_mode(void __iomem *scu_base, unsigned int mode)
#if defined(CONFIG_SMP) && defined(CONFIG_HAVE_ARM_SCU)
void scu_enable(void __iomem *scu_base);
+void __iomem *of_scu_get_base(void);
+int of_scu_enable(void);
#else
static inline void scu_enable(void __iomem *scu_base) {}
+static inline void __iomem *of_scu_get_base(void) {return NULL; }
+static inline int of_scu_enable(void) {return 0; }
#endif
#endif
diff --git a/arch/arm/kernel/smp_scu.c b/arch/arm/kernel/smp_scu.c
index 72f9241..d0ac3ed 100644
--- a/arch/arm/kernel/smp_scu.c
+++ b/arch/arm/kernel/smp_scu.c
@@ -10,6 +10,7 @@
*/
#include <linux/init.h>
#include <linux/io.h>
+#include <linux/of_address.h>
#include <asm/smp_plat.h>
#include <asm/smp_scu.h>
@@ -70,6 +71,61 @@ void scu_enable(void __iomem *scu_base)
*/
flush_cache_all();
}
+
+static const struct of_device_id scu_match[] = {
+ { .compatible = "arm,cortex-a9-scu", },
+ { .compatible = "arm,cortex-a5-scu", },
+ { }
+};
+
+/*
+ * Helper API to get SCU base address
+ * In case platform DT do not have SCU node, or iomap fails
+ * this call will fallback and will try to map via call to
+ * scu_a9_get_base.
+ * This will return ownership of scu_base to the caller
+ */
+void __iomem *of_scu_get_base(void)
+{
+ unsigned long base = 0;
+ struct device_node *np;
+ void __iomem *scu_base;
+
+ np = of_find_matching_node(NULL, scu_match);
+ scu_base = of_iomap(np, 0);
+ of_node_put(np);
+ if (!scu_base) {
+ pr_err("%s failed to map scu_base via DT\n", __func__);
+ if (scu_a9_has_base()) {
+ base = scu_a9_get_base();
+ scu_base = ioremap(base, SZ_4K);
+ }
+ if (!scu_base) {
+ pr_err("%s failed to map scu_base\n", __func__);
+ return IOMEM_ERR_PTR(-ENOMEM);
+ }
+ }
+ return scu_base;
+}
+
+/*
+ * Enable SCU via mapping scu_base DT
+ * If scu_base mapped successfully scu will be enabled and in case of
+ * failure if will return non-zero error code
+ */
+int of_scu_enable(void)
+{
+ void __iomem *scu_base;
+
+ scu_base = of_scu_get_base();
+ if (!IS_ERR(scu_base)) {
+ scu_enable(scu_base);
+ iounmap(scu_base);
+ return 0;
+ }
+ return PTR_ERR(scu_base);
+}
+
#endif
/*
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Jisheng Zhang <jszhang@marvell.com> |
|---|---|
| Date | 2016-11-14 07:20 +0100 |
| Subject | Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU |
| Message-ID | <sDiMx-3cC-5@gated-at.bofh.it> |
| In reply to | #1521299 |
Hi Pankaj,
On Mon, 14 Nov 2016 10:31:56 +0530 Pankaj Dubey wrote:
> Many platforms are duplicating code for enabling SCU, lets add
> common code to enable SCU by parsing SCU device node so the duplication
> in each platform can be avoided.
>
> CC: Krzysztof Kozlowski <krzk@kernel.org>
> CC: Jisheng Zhang <jszhang@marvell.com>
> CC: Russell King <linux@armlinux.org.uk>
> CC: Dinh Nguyen <dinguyen@opensource.altera.com>
> CC: Patrice Chotard <patrice.chotard@st.com>
> CC: Linus Walleij <linus.walleij@linaro.org>
> CC: Liviu Dudau <liviu.dudau@arm.com>
> CC: Ray Jui <rjui@broadcom.com>
> CC: Stephen Warren <swarren@wwwdotorg.org>
> CC: Heiko Stuebner <heiko@sntech.de>
> CC: Shawn Guo <shawnguo@kernel.org>
> CC: Michal Simek <michal.simek@xilinx.com>
> CC: Wei Xu <xuwei5@hisilicon.com>
> CC: Andrew Lunn <andrew@lunn.ch>
> CC: Jun Nie <jun.nie@linaro.org>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> arch/arm/include/asm/smp_scu.h | 4 +++
> arch/arm/kernel/smp_scu.c | 56 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 60 insertions(+)
>
> diff --git a/arch/arm/include/asm/smp_scu.h b/arch/arm/include/asm/smp_scu.h
> index bfe163c..fdeec07 100644
> --- a/arch/arm/include/asm/smp_scu.h
> +++ b/arch/arm/include/asm/smp_scu.h
> @@ -39,8 +39,12 @@ static inline int scu_power_mode(void __iomem *scu_base, unsigned int mode)
>
> #if defined(CONFIG_SMP) && defined(CONFIG_HAVE_ARM_SCU)
> void scu_enable(void __iomem *scu_base);
> +void __iomem *of_scu_get_base(void);
> +int of_scu_enable(void);
> #else
> static inline void scu_enable(void __iomem *scu_base) {}
> +static inline void __iomem *of_scu_get_base(void) {return NULL; }
> +static inline int of_scu_enable(void) {return 0; }
> #endif
>
> #endif
> diff --git a/arch/arm/kernel/smp_scu.c b/arch/arm/kernel/smp_scu.c
> index 72f9241..d0ac3ed 100644
> --- a/arch/arm/kernel/smp_scu.c
> +++ b/arch/arm/kernel/smp_scu.c
> @@ -10,6 +10,7 @@
> */
> #include <linux/init.h>
> #include <linux/io.h>
> +#include <linux/of_address.h>
>
> #include <asm/smp_plat.h>
> #include <asm/smp_scu.h>
> @@ -70,6 +71,61 @@ void scu_enable(void __iomem *scu_base)
> */
> flush_cache_all();
> }
> +
> +static const struct of_device_id scu_match[] = {
> + { .compatible = "arm,cortex-a9-scu", },
> + { .compatible = "arm,cortex-a5-scu", },
> + { }
> +};
> +
> +/*
> + * Helper API to get SCU base address
> + * In case platform DT do not have SCU node, or iomap fails
> + * this call will fallback and will try to map via call to
> + * scu_a9_get_base.
> + * This will return ownership of scu_base to the caller
> + */
> +void __iomem *of_scu_get_base(void)
> +{
> + unsigned long base = 0;
> + struct device_node *np;
> + void __iomem *scu_base;
> +
> + np = of_find_matching_node(NULL, scu_match);
could we check np before calling of_iomap()?
> + scu_base = of_iomap(np, 0);
> + of_node_put(np);
> + if (!scu_base) {
> + pr_err("%s failed to map scu_base via DT\n", __func__);
For non-ca5, non-ca9 based SoCs, we'll see this error msg. We understand
what does it mean, but it may confuse normal users. In current version,
berlin doesn't complain like this for non-ca9 SoCs
> + if (scu_a9_has_base()) {
> + base = scu_a9_get_base();
> + scu_base = ioremap(base, SZ_4K);
> + }
> + if (!scu_base) {
> + pr_err("%s failed to map scu_base\n", __func__);
ditto
> + return IOMEM_ERR_PTR(-ENOMEM);
> + }
> + }
> + return scu_base;
> +}
> +
> +/*
> + * Enable SCU via mapping scu_base DT
> + * If scu_base mapped successfully scu will be enabled and in case of
> + * failure if will return non-zero error code
> + */
> +int of_scu_enable(void)
> +{
> + void __iomem *scu_base;
> +
> + scu_base = of_scu_get_base();
> + if (!IS_ERR(scu_base)) {
> + scu_enable(scu_base);
> + iounmap(scu_base);
> + return 0;
> + }
> + return PTR_ERR(scu_base);
> +}
> +
> #endif
>
> /*
[toc] | [prev] | [next] | [standalone]
| From | Jisheng Zhang <jszhang@marvell.com> |
|---|---|
| Date | 2016-11-14 08:10 +0100 |
| Subject | Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU |
| Message-ID | <sDjyW-3JZ-23@gated-at.bofh.it> |
| In reply to | #1521345 |
On Mon, 14 Nov 2016 14:12:51 +0800 Jisheng Zhang wrote:
> Hi Pankaj,
>
> On Mon, 14 Nov 2016 10:31:56 +0530 Pankaj Dubey wrote:
>
> > Many platforms are duplicating code for enabling SCU, lets add
> > common code to enable SCU by parsing SCU device node so the duplication
> > in each platform can be avoided.
> >
> > CC: Krzysztof Kozlowski <krzk@kernel.org>
> > CC: Jisheng Zhang <jszhang@marvell.com>
> > CC: Russell King <linux@armlinux.org.uk>
> > CC: Dinh Nguyen <dinguyen@opensource.altera.com>
> > CC: Patrice Chotard <patrice.chotard@st.com>
> > CC: Linus Walleij <linus.walleij@linaro.org>
> > CC: Liviu Dudau <liviu.dudau@arm.com>
> > CC: Ray Jui <rjui@broadcom.com>
> > CC: Stephen Warren <swarren@wwwdotorg.org>
> > CC: Heiko Stuebner <heiko@sntech.de>
> > CC: Shawn Guo <shawnguo@kernel.org>
> > CC: Michal Simek <michal.simek@xilinx.com>
> > CC: Wei Xu <xuwei5@hisilicon.com>
> > CC: Andrew Lunn <andrew@lunn.ch>
> > CC: Jun Nie <jun.nie@linaro.org>
> > Suggested-by: Arnd Bergmann <arnd@arndb.de>
> > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> > ---
> > arch/arm/include/asm/smp_scu.h | 4 +++
> > arch/arm/kernel/smp_scu.c | 56 ++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 60 insertions(+)
> >
> > diff --git a/arch/arm/include/asm/smp_scu.h b/arch/arm/include/asm/smp_scu.h
> > index bfe163c..fdeec07 100644
> > --- a/arch/arm/include/asm/smp_scu.h
> > +++ b/arch/arm/include/asm/smp_scu.h
> > @@ -39,8 +39,12 @@ static inline int scu_power_mode(void __iomem *scu_base, unsigned int mode)
> >
> > #if defined(CONFIG_SMP) && defined(CONFIG_HAVE_ARM_SCU)
> > void scu_enable(void __iomem *scu_base);
> > +void __iomem *of_scu_get_base(void);
> > +int of_scu_enable(void);
> > #else
> > static inline void scu_enable(void __iomem *scu_base) {}
> > +static inline void __iomem *of_scu_get_base(void) {return NULL; }
> > +static inline int of_scu_enable(void) {return 0; }
> > #endif
> >
> > #endif
> > diff --git a/arch/arm/kernel/smp_scu.c b/arch/arm/kernel/smp_scu.c
> > index 72f9241..d0ac3ed 100644
> > --- a/arch/arm/kernel/smp_scu.c
> > +++ b/arch/arm/kernel/smp_scu.c
> > @@ -10,6 +10,7 @@
> > */
> > #include <linux/init.h>
> > #include <linux/io.h>
> > +#include <linux/of_address.h>
> >
> > #include <asm/smp_plat.h>
> > #include <asm/smp_scu.h>
> > @@ -70,6 +71,61 @@ void scu_enable(void __iomem *scu_base)
> > */
> > flush_cache_all();
> > }
> > +
> > +static const struct of_device_id scu_match[] = {
> > + { .compatible = "arm,cortex-a9-scu", },
> > + { .compatible = "arm,cortex-a5-scu", },
> > + { }
> > +};
> > +
> > +/*
> > + * Helper API to get SCU base address
> > + * In case platform DT do not have SCU node, or iomap fails
> > + * this call will fallback and will try to map via call to
> > + * scu_a9_get_base.
> > + * This will return ownership of scu_base to the caller
> > + */
> > +void __iomem *of_scu_get_base(void)
> > +{
> > + unsigned long base = 0;
> > + struct device_node *np;
> > + void __iomem *scu_base;
> > +
> > + np = of_find_matching_node(NULL, scu_match);
>
> could we check np before calling of_iomap()?
>
> > + scu_base = of_iomap(np, 0);
> > + of_node_put(np);
> > + if (!scu_base) {
> > + pr_err("%s failed to map scu_base via DT\n", __func__);
>
> For non-ca5, non-ca9 based SoCs, we'll see this error msg. We understand
> what does it mean, but it may confuse normal users. In current version,
> berlin doesn't complain like this for non-ca9 SoCs
oops, I just realized that the non-ca9 berlin arm SoC version isn't upstreamed.
Below is the draft version I planed. Basically speaking, the code tries to
find "arm,cortex-a9-scu" node from DT, if can't, we think we don't need to
worry about SCU. Is there any elegant solution for my situation?
Thanks,
Jisheng
------------8<-------------------
--- a/arch/arm/mach-berlin/platsmp.c
+++ b/arch/arm/mach-berlin/platsmp.c
@@ -56,22 +56,25 @@ static void __init berlin_smp_prepare_cpus(unsigned int max_cpus)
void __iomem *vectors_base;
np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
- scu_base = of_iomap(np, 0);
- of_node_put(np);
- if (!scu_base)
- return;
+ if (np) {
+ scu_base = of_iomap(np, 0);
+ of_node_put(np);
+ if (!scu_base)
+ return;
+ scu_enable(scu_base);
+ iounmap(scu_base);
+ }
np = of_find_compatible_node(NULL, NULL, "marvell,berlin-cpu-ctrl");
cpu_ctrl = of_iomap(np, 0);
of_node_put(np);
if (!cpu_ctrl)
- goto unmap_scu;
+ return;
vectors_base = ioremap(CONFIG_VECTORS_BASE, SZ_32K);
if (!vectors_base)
- goto unmap_scu;
+ return;
- scu_enable(scu_base);
flush_cache_all();
/*
@@ -87,8 +90,6 @@ static void __init berlin_smp_prepare_cpus(unsigned int max_cpus)
writel(virt_to_phys(secondary_startup), vectors_base + SW_RESET_ADDR);
iounmap(vectors_base);
-unmap_scu:
- iounmap(scu_base);
}
static struct smp_operations berlin_smp_ops __initdata = {
>
> > + if (scu_a9_has_base()) {
> > + base = scu_a9_get_base();
> > + scu_base = ioremap(base, SZ_4K);
> > + }
> > + if (!scu_base) {
> > + pr_err("%s failed to map scu_base\n", __func__);
>
> ditto
>
> > + return IOMEM_ERR_PTR(-ENOMEM);
> > + }
> > + }
> > + return scu_base;
> > +}
> > +
> > +/*
> > + * Enable SCU via mapping scu_base DT
> > + * If scu_base mapped successfully scu will be enabled and in case of
> > + * failure if will return non-zero error code
> > + */
> > +int of_scu_enable(void)
> > +{
> > + void __iomem *scu_base;
> > +
> > + scu_base = of_scu_get_base();
> > + if (!IS_ERR(scu_base)) {
> > + scu_enable(scu_base);
> > + iounmap(scu_base);
> > + return 0;
> > + }
> > + return PTR_ERR(scu_base);
> > +}
> > +
> > #endif
> >
> > /*
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
[toc] | [prev] | [next] | [standalone]
| From | "pankaj.dubey" <pankaj.dubey@samsung.com> |
|---|---|
| Date | 2016-11-14 10:00 +0100 |
| Subject | Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU |
| Message-ID | <sDlhn-4FD-9@gated-at.bofh.it> |
| In reply to | #1521365 |
Hi Jisheng,
On Monday 14 November 2016 12:24 PM, Jisheng Zhang wrote:
>
> On Mon, 14 Nov 2016 14:12:51 +0800 Jisheng Zhang wrote:
>
>> Hi Pankaj,
>>
<snip>
>>> + * Helper API to get SCU base address
>>> + * In case platform DT do not have SCU node, or iomap fails
>>> + * this call will fallback and will try to map via call to
>>> + * scu_a9_get_base.
>>> + * This will return ownership of scu_base to the caller
>>> + */
>>> +void __iomem *of_scu_get_base(void)
>>> +{
>>> + unsigned long base = 0;
>>> + struct device_node *np;
>>> + void __iomem *scu_base;
>>> +
>>> + np = of_find_matching_node(NULL, scu_match);
>>
>> could we check np before calling of_iomap()?
>>
>>> + scu_base = of_iomap(np, 0);
>>> + of_node_put(np);
>>> + if (!scu_base) {
>>> + pr_err("%s failed to map scu_base via DT\n", __func__);
>>
>> For non-ca5, non-ca9 based SoCs, we'll see this error msg. We understand
>> what does it mean, but it may confuse normal users. In current version,
>> berlin doesn't complain like this for non-ca9 SoCs
>
> oops, I just realized that the non-ca9 berlin arm SoC version isn't upstreamed.
> Below is the draft version I planed. Basically speaking, the code tries to
> find "arm,cortex-a9-scu" node from DT, if can't, we think we don't need to
> worry about SCU. Is there any elegant solution for my situation?
>
To adopt new generic API I have submitted a patch for Berlin (along with
other various platforms) here:
https://patchwork.kernel.org/patch/9426457/
Please review and if possible test and let me know feedback.
Thanks,
Pankaj Dubey
> Thanks,
> Jisheng
>
>
> ------------8<-------------------
> --- a/arch/arm/mach-berlin/platsmp.c
> +++ b/arch/arm/mach-berlin/platsmp.c
> @@ -56,22 +56,25 @@ static void __init berlin_smp_prepare_cpus(unsigned int max_cpus)
> void __iomem *vectors_base;
>
> np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
> - scu_base = of_iomap(np, 0);
> - of_node_put(np);
> - if (!scu_base)
> - return;
> + if (np) {
> + scu_base = of_iomap(np, 0);
> + of_node_put(np);
> + if (!scu_base)
> + return;
> + scu_enable(scu_base);
> + iounmap(scu_base);
> + }
>
> np = of_find_compatible_node(NULL, NULL, "marvell,berlin-cpu-ctrl");
> cpu_ctrl = of_iomap(np, 0);
> of_node_put(np);
> if (!cpu_ctrl)
> - goto unmap_scu;
> + return;
>
> vectors_base = ioremap(CONFIG_VECTORS_BASE, SZ_32K);
> if (!vectors_base)
> - goto unmap_scu;
> + return;
>
> - scu_enable(scu_base);
> flush_cache_all();
>
> /*
> @@ -87,8 +90,6 @@ static void __init berlin_smp_prepare_cpus(unsigned int max_cpus)
> writel(virt_to_phys(secondary_startup), vectors_base + SW_RESET_ADDR);
>
> iounmap(vectors_base);
> -unmap_scu:
> - iounmap(scu_base);
> }
>
> static struct smp_operations berlin_smp_ops __initdata = {
>
>
>>
>>> + if (scu_a9_has_base()) {
>>> + base = scu_a9_get_base();
>>> + scu_base = ioremap(base, SZ_4K);
>>> + }
>>> + if (!scu_base) {
>>> + pr_err("%s failed to map scu_base\n", __func__);
>>
>> ditto
>>
>>> + return IOMEM_ERR_PTR(-ENOMEM);
>>> + }
>>> + }
>>> + return scu_base;
>>> +}
>>> +
>>> +/*
>>> + * Enable SCU via mapping scu_base DT
>>> + * If scu_base mapped successfully scu will be enabled and in case of
>>> + * failure if will return non-zero error code
>>> + */
>>> +int of_scu_enable(void)
>>> +{
>>> + void __iomem *scu_base;
>>> +
>>> + scu_base = of_scu_get_base();
>>> + if (!IS_ERR(scu_base)) {
>>> + scu_enable(scu_base);
>>> + iounmap(scu_base);
>>> + return 0;
>>> + }
>>> + return PTR_ERR(scu_base);
>>> +}
>>> +
>>> #endif
>>>
>>> /*
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>
>
>
[toc] | [prev] | [next] | [standalone]
| From | Jisheng Zhang <jszhang@marvell.com> |
|---|---|
| Date | 2016-11-14 10:00 +0100 |
| Subject | Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU |
| Message-ID | <sDlho-4FD-25@gated-at.bofh.it> |
| In reply to | #1521365 |
Hi Pankaj,
On Mon, 14 Nov 2016 14:54:59 +0800 Jisheng Zhang wrote:
> On Mon, 14 Nov 2016 14:12:51 +0800 Jisheng Zhang wrote:
>
> > Hi Pankaj,
> >
> > On Mon, 14 Nov 2016 10:31:56 +0530 Pankaj Dubey wrote:
> >
> > > Many platforms are duplicating code for enabling SCU, lets add
> > > common code to enable SCU by parsing SCU device node so the duplication
> > > in each platform can be avoided.
> > >
> > > CC: Krzysztof Kozlowski <krzk@kernel.org>
> > > CC: Jisheng Zhang <jszhang@marvell.com>
> > > CC: Russell King <linux@armlinux.org.uk>
> > > CC: Dinh Nguyen <dinguyen@opensource.altera.com>
> > > CC: Patrice Chotard <patrice.chotard@st.com>
> > > CC: Linus Walleij <linus.walleij@linaro.org>
> > > CC: Liviu Dudau <liviu.dudau@arm.com>
> > > CC: Ray Jui <rjui@broadcom.com>
> > > CC: Stephen Warren <swarren@wwwdotorg.org>
> > > CC: Heiko Stuebner <heiko@sntech.de>
> > > CC: Shawn Guo <shawnguo@kernel.org>
> > > CC: Michal Simek <michal.simek@xilinx.com>
> > > CC: Wei Xu <xuwei5@hisilicon.com>
> > > CC: Andrew Lunn <andrew@lunn.ch>
> > > CC: Jun Nie <jun.nie@linaro.org>
> > > Suggested-by: Arnd Bergmann <arnd@arndb.de>
> > > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> > > ---
> > > arch/arm/include/asm/smp_scu.h | 4 +++
> > > arch/arm/kernel/smp_scu.c | 56 ++++++++++++++++++++++++++++++++++++++++++
> > > 2 files changed, 60 insertions(+)
> > >
> > > diff --git a/arch/arm/include/asm/smp_scu.h b/arch/arm/include/asm/smp_scu.h
> > > index bfe163c..fdeec07 100644
> > > --- a/arch/arm/include/asm/smp_scu.h
> > > +++ b/arch/arm/include/asm/smp_scu.h
> > > @@ -39,8 +39,12 @@ static inline int scu_power_mode(void __iomem *scu_base, unsigned int mode)
> > >
> > > #if defined(CONFIG_SMP) && defined(CONFIG_HAVE_ARM_SCU)
> > > void scu_enable(void __iomem *scu_base);
> > > +void __iomem *of_scu_get_base(void);
> > > +int of_scu_enable(void);
> > > #else
> > > static inline void scu_enable(void __iomem *scu_base) {}
> > > +static inline void __iomem *of_scu_get_base(void) {return NULL; }
> > > +static inline int of_scu_enable(void) {return 0; }
> > > #endif
> > >
> > > #endif
> > > diff --git a/arch/arm/kernel/smp_scu.c b/arch/arm/kernel/smp_scu.c
> > > index 72f9241..d0ac3ed 100644
> > > --- a/arch/arm/kernel/smp_scu.c
> > > +++ b/arch/arm/kernel/smp_scu.c
> > > @@ -10,6 +10,7 @@
> > > */
> > > #include <linux/init.h>
> > > #include <linux/io.h>
> > > +#include <linux/of_address.h>
> > >
> > > #include <asm/smp_plat.h>
> > > #include <asm/smp_scu.h>
> > > @@ -70,6 +71,61 @@ void scu_enable(void __iomem *scu_base)
> > > */
> > > flush_cache_all();
> > > }
> > > +
> > > +static const struct of_device_id scu_match[] = {
> > > + { .compatible = "arm,cortex-a9-scu", },
> > > + { .compatible = "arm,cortex-a5-scu", },
> > > + { }
> > > +};
> > > +
> > > +/*
> > > + * Helper API to get SCU base address
> > > + * In case platform DT do not have SCU node, or iomap fails
> > > + * this call will fallback and will try to map via call to
> > > + * scu_a9_get_base.
> > > + * This will return ownership of scu_base to the caller
> > > + */
> > > +void __iomem *of_scu_get_base(void)
> > > +{
> > > + unsigned long base = 0;
> > > + struct device_node *np;
> > > + void __iomem *scu_base;
> > > +
> > > + np = of_find_matching_node(NULL, scu_match);
> >
> > could we check np before calling of_iomap()?
> >
> > > + scu_base = of_iomap(np, 0);
> > > + of_node_put(np);
> > > + if (!scu_base) {
> > > + pr_err("%s failed to map scu_base via DT\n", __func__);
> >
> > For non-ca5, non-ca9 based SoCs, we'll see this error msg. We understand
> > what does it mean, but it may confuse normal users. In current version,
> > berlin doesn't complain like this for non-ca9 SoCs
>
> oops, I just realized that the non-ca9 berlin arm SoC version isn't upstreamed.
> Below is the draft version I planed. Basically speaking, the code tries to
> find "arm,cortex-a9-scu" node from DT, if can't, we think we don't need to
> worry about SCU. Is there any elegant solution for my situation?
I just realized that (another realized :D) we uses PSCI enable method for
non-ca9 base SoCs, so "marvell,berlin-smp" enable method is only for the SoCs
which have CA9 compatible SCU. Sorry for the noise, your patch looks good to me.
[toc] | [prev] | [next] | [standalone]
| From | "pankaj.dubey" <pankaj.dubey@samsung.com> |
|---|---|
| Date | 2016-11-14 09:40 +0100 |
| Subject | Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU |
| Message-ID | <sDkY1-4yt-23@gated-at.bofh.it> |
| In reply to | #1521345 |
Hi Jisheng,
On Monday 14 November 2016 11:42 AM, Jisheng Zhang wrote:
> Hi Pankaj,
>
> On Mon, 14 Nov 2016 10:31:56 +0530 Pankaj Dubey wrote:
<snip>
>> +
>> + np = of_find_matching_node(NULL, scu_match);
>
> could we check np before calling of_iomap()?
>
of_iomap takes care of that, and will return NULL if np is NULL.
So additional check of np is not required here.
>> + scu_base = of_iomap(np, 0);
>> + of_node_put(np);
>> + if (!scu_base) {
>> + pr_err("%s failed to map scu_base via DT\n", __func__);
>
> For non-ca5, non-ca9 based SoCs, we'll see this error msg. We understand
> what does it mean, but it may confuse normal users. In current version,
> berlin doesn't complain like this for non-ca9 SoCs
>
OK, let me see other reviewer's comment on this. Then we will decide if
this error message is required or can be omitted.
Thanks,
Pankaj Dubey
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-11-14 13:10 +0100 |
| Subject | Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU |
| Message-ID | <sDofg-6Oy-49@gated-at.bofh.it> |
| In reply to | #1521418 |
On Monday, November 14, 2016 2:10:16 PM CET pankaj.dubey wrote:
> >> + scu_base = of_iomap(np, 0);
> >> + of_node_put(np);
> >> + if (!scu_base) {
> >> + pr_err("%s failed to map scu_base via DT\n", __func__);
> >
> > For non-ca5, non-ca9 based SoCs, we'll see this error msg. We understand
> > what does it mean, but it may confuse normal users. In current version,
> > berlin doesn't complain like this for non-ca9 SoCs
> >
>
> OK, let me see other reviewer's comment on this. Then we will decide if
> this error message is required or can be omitted.
We need to look at all callers here, to see if the function ever gets
called for a CPU that doesn't have an SCU. I'd say we should warn if
we know there is an SCU but we cannot map it, but never warn on
any of the CPU cores that don't support an SCU.
Arnd
[toc] | [prev] | [next] | [standalone]
| From | Russell King - ARM Linux <linux@armlinux.org.uk> |
|---|---|
| Date | 2016-11-14 15:00 +0100 |
| Subject | Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU |
| Message-ID | <sDpXI-7Ht-23@gated-at.bofh.it> |
| In reply to | #1521585 |
On Mon, Nov 14, 2016 at 01:03:09PM +0100, Arnd Bergmann wrote:
> On Monday, November 14, 2016 2:10:16 PM CET pankaj.dubey wrote:
> > >> + scu_base = of_iomap(np, 0);
> > >> + of_node_put(np);
> > >> + if (!scu_base) {
> > >> + pr_err("%s failed to map scu_base via DT\n", __func__);
> > >
> > > For non-ca5, non-ca9 based SoCs, we'll see this error msg. We understand
> > > what does it mean, but it may confuse normal users. In current version,
> > > berlin doesn't complain like this for non-ca9 SoCs
> > >
> >
> > OK, let me see other reviewer's comment on this. Then we will decide if
> > this error message is required or can be omitted.
>
> We need to look at all callers here, to see if the function ever gets
> called for a CPU that doesn't have an SCU. I'd say we should warn if
> we know there is an SCU but we cannot map it, but never warn on
> any of the CPU cores that don't support an SCU.
Maybe there should be two helpers:
of_scu_enable() which _only_ looks up the SCU address in DT and enables
it if it finds it, otherwise returning failure.
a9_scu_enable() which tries to use the A9 provided SCU address and
enables it if it finds it, otherwise returning failure.
Then callers can decide which of these to call, and what error messages
to print on their failures.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-11-14 15:40 +0100 |
| Subject | Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU |
| Message-ID | <sDqAq-8cD-35@gated-at.bofh.it> |
| In reply to | #1521668 |
On Monday, November 14, 2016 1:50:18 PM CET Russell King - ARM Linux wrote:
> On Mon, Nov 14, 2016 at 01:03:09PM +0100, Arnd Bergmann wrote:
> > On Monday, November 14, 2016 2:10:16 PM CET pankaj.dubey wrote:
> > > >> + scu_base = of_iomap(np, 0);
> > > >> + of_node_put(np);
> > > >> + if (!scu_base) {
> > > >> + pr_err("%s failed to map scu_base via DT\n", __func__);
> > > >
> > > > For non-ca5, non-ca9 based SoCs, we'll see this error msg. We understand
> > > > what does it mean, but it may confuse normal users. In current version,
> > > > berlin doesn't complain like this for non-ca9 SoCs
> > > >
> > >
> > > OK, let me see other reviewer's comment on this. Then we will decide if
> > > this error message is required or can be omitted.
> >
> > We need to look at all callers here, to see if the function ever gets
> > called for a CPU that doesn't have an SCU. I'd say we should warn if
> > we know there is an SCU but we cannot map it, but never warn on
> > any of the CPU cores that don't support an SCU.
>
> Maybe there should be two helpers:
>
> of_scu_enable() which _only_ looks up the SCU address in DT and enables
> it if it finds it, otherwise returning failure.
>
> a9_scu_enable() which tries to use the A9 provided SCU address and
> enables it if it finds it, otherwise returning failure.
>
> Then callers can decide which of these to call, and what error messages
> to print on their failures.
Splitting the function in two is probably simpler overall, but
we may still have to look at all the callers: Any platform that
currently tries to map it on any CPU and doesn't warn about the
absence of the device node (or about scu_a9_has_base() == false)
should really continue not to warn about that.
If all platforms only call these on SMP machines with an
ARM11MPcore, Cortex-A5 or Cortex-A9, everything should be
fine here, otherwise we can leave the warning in the caller
after checking the return code of the new APIs.
Arnd
[toc] | [prev] | [next] | [standalone]
| From | Russell King - ARM Linux <linux@armlinux.org.uk> |
|---|---|
| Date | 2016-11-14 16:00 +0100 |
| Subject | Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU |
| Message-ID | <sDqTL-8jU-5@gated-at.bofh.it> |
| In reply to | #1521706 |
On Mon, Nov 14, 2016 at 03:37:44PM +0100, Arnd Bergmann wrote:
> On Monday, November 14, 2016 1:50:18 PM CET Russell King - ARM Linux wrote:
> > On Mon, Nov 14, 2016 at 01:03:09PM +0100, Arnd Bergmann wrote:
> > > On Monday, November 14, 2016 2:10:16 PM CET pankaj.dubey wrote:
> > > > >> + scu_base = of_iomap(np, 0);
> > > > >> + of_node_put(np);
> > > > >> + if (!scu_base) {
> > > > >> + pr_err("%s failed to map scu_base via DT\n", __func__);
> > > > >
> > > > > For non-ca5, non-ca9 based SoCs, we'll see this error msg. We understand
> > > > > what does it mean, but it may confuse normal users. In current version,
> > > > > berlin doesn't complain like this for non-ca9 SoCs
> > > > >
> > > >
> > > > OK, let me see other reviewer's comment on this. Then we will decide if
> > > > this error message is required or can be omitted.
> > >
> > > We need to look at all callers here, to see if the function ever gets
> > > called for a CPU that doesn't have an SCU. I'd say we should warn if
> > > we know there is an SCU but we cannot map it, but never warn on
> > > any of the CPU cores that don't support an SCU.
> >
> > Maybe there should be two helpers:
> >
> > of_scu_enable() which _only_ looks up the SCU address in DT and enables
> > it if it finds it, otherwise returning failure.
> >
> > a9_scu_enable() which tries to use the A9 provided SCU address and
> > enables it if it finds it, otherwise returning failure.
> >
> > Then callers can decide which of these to call, and what error messages
> > to print on their failures.
>
> Splitting the function in two is probably simpler overall, but
> we may still have to look at all the callers: Any platform that
> currently tries to map it on any CPU and doesn't warn about the
> absence of the device node (or about scu_a9_has_base() == false)
> should really continue not to warn about that.
Did you miss the bit where none of of_scu_enable() or a9_scu_enable()
should produce any warnings or errors to be printed. It's up to the
caller to report the failure, otherwise doing this doesn't make sense:
if (of_scu_enable() < 0 && a9_scu_enable() < 0)
pr_err("Failed to map and enable the SCU\n");
because if of_scu_enable() prints a warning/error, then it's patently
misleading.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
[toc] | [prev] | [next] | [standalone]
| From | "pankaj.dubey" <pankaj.dubey@samsung.com> |
|---|---|
| Date | 2016-11-17 05:30 +0100 |
| Subject | Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU |
| Message-ID | <sEmuJ-49B-3@gated-at.bofh.it> |
| In reply to | #1521719 |
Hi Russell,
On Monday 14 November 2016 08:21 PM, Russell King - ARM Linux wrote:
> On Mon, Nov 14, 2016 at 03:37:44PM +0100, Arnd Bergmann wrote:
>> On Monday, November 14, 2016 1:50:18 PM CET Russell King - ARM Linux wrote:
>>> On Mon, Nov 14, 2016 at 01:03:09PM +0100, Arnd Bergmann wrote:
>>>> On Monday, November 14, 2016 2:10:16 PM CET pankaj.dubey wrote:
>>>>>>> + scu_base = of_iomap(np, 0);
>>>>>>> + of_node_put(np);
>>>>>>> + if (!scu_base) {
>>>>>>> + pr_err("%s failed to map scu_base via DT\n", __func__);
>>>>>>
>>>>>> For non-ca5, non-ca9 based SoCs, we'll see this error msg. We understand
>>>>>> what does it mean, but it may confuse normal users. In current version,
>>>>>> berlin doesn't complain like this for non-ca9 SoCs
>>>>>>
>>>>>
>>>>> OK, let me see other reviewer's comment on this. Then we will decide if
>>>>> this error message is required or can be omitted.
>>>>
>>>> We need to look at all callers here, to see if the function ever gets
>>>> called for a CPU that doesn't have an SCU. I'd say we should warn if
>>>> we know there is an SCU but we cannot map it, but never warn on
>>>> any of the CPU cores that don't support an SCU.
>>>
>>> Maybe there should be two helpers:
>>>
>>> of_scu_enable() which _only_ looks up the SCU address in DT and enables
>>> it if it finds it, otherwise returning failure.
>>>
>>> a9_scu_enable() which tries to use the A9 provided SCU address and
>>> enables it if it finds it, otherwise returning failure.
>>>
OK, In that case I can see need for following four helpers as:
1: of_scu_enable() which will __only__ lookup the SCU address in DT and
enables it if it finds, otherwise return -ENOMEM failure.
This helper APIs is required and sufficient for most of platforms such
as exynos, berlin, realview, socfpga, STi, ux500, vexpress, rockchip and
mvebu
2: a9_scu_enable(), which will __only__ use A9 provided SCU address and
enables it, if address mapped successfully, otherwise returning failure.
This helper APIs is required and sufficient for two ARM platforms as of
now tegra and hisi.
3: of_scu_get_base() which will lookup the SCU address in DT and if node
found maps address and returns ioremapped address to caller.
This helper APIs is required for three ARM plaforms rockchip, mvebu and
ux500, along with scu_enable() API to enable and find number_of_cores.
4: s9_scu_iomap_base() which will internally use s9_scu_get_base() and
do ioremap of scu address and returns ioremapped address to the caller
along with ownership (caller has responsibility to unmap it).
This helper APIs is required to simplify SCU enable and related code in
two ARM plaforms BCM ans ZX.
For remaining two ARM platforms (IMX and ZYNQ), none of these helpers
are useful for the time-being, as they need SCU mapping very early of
boot, where we can't use iomap APIs. So I will drop patches related to
these platforms in v2 version.
Please let me know if any concern in this approach.
>>> Then callers can decide which of these to call, and what error messages
>>> to print on their failures.
>>
>> Splitting the function in two is probably simpler overall, but
>> we may still have to look at all the callers: Any platform that
>> currently tries to map it on any CPU and doesn't warn about the
>> absence of the device node (or about scu_a9_has_base() == false)
>> should really continue not to warn about that.
>
> Did you miss the bit where none of of_scu_enable() or a9_scu_enable()
> should produce any warnings or errors to be printed. It's up to the
> caller to report the failure, otherwise doing this doesn't make sense:
>
> if (of_scu_enable() < 0 && a9_scu_enable() < 0)
> pr_err("Failed to map and enable the SCU\n");
>
> because if of_scu_enable() prints a warning/error, then it's patently
> misleading.
>
I will move out error message out of these helpers and let caller
(platform specific code) handle and print error if required.
Thanks,
Pankaj Dubey
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-11-17 18:10 +0100 |
| Subject | Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU |
| Message-ID | <sEymf-3v8-131@gated-at.bofh.it> |
| In reply to | #1524081 |
On Thursday, November 17, 2016 9:50:27 AM CET pankaj.dubey wrote:
>
> >>> of_scu_enable() which _only_ looks up the SCU address in DT and enables
> >>> it if it finds it, otherwise returning failure.
> >>>
> >>> a9_scu_enable() which tries to use the A9 provided SCU address and
> >>> enables it if it finds it, otherwise returning failure.
> >>>
>
> OK, In that case I can see need for following four helpers as:
>
> 1: of_scu_enable() which will __only__ lookup the SCU address in DT and
> enables it if it finds, otherwise return -ENOMEM failure.
> This helper APIs is required and sufficient for most of platforms such
> as exynos, berlin, realview, socfpga, STi, ux500, vexpress, rockchip and
> mvebu
>
> 2: a9_scu_enable(), which will __only__ use A9 provided SCU address and
> enables it, if address mapped successfully, otherwise returning failure.
> This helper APIs is required and sufficient for two ARM platforms as of
> now tegra and hisi.
>
> 3: of_scu_get_base() which will lookup the SCU address in DT and if node
> found maps address and returns ioremapped address to caller.
> This helper APIs is required for three ARM plaforms rockchip, mvebu and
> ux500, along with scu_enable() API to enable and find number_of_cores.
>
> 4: s9_scu_iomap_base() which will internally use s9_scu_get_base() and
> do ioremap of scu address and returns ioremapped address to the caller
> along with ownership (caller has responsibility to unmap it).
> This helper APIs is required to simplify SCU enable and related code in
> two ARM plaforms BCM ans ZX.
>
> For remaining two ARM platforms (IMX and ZYNQ), none of these helpers
> are useful for the time-being, as they need SCU mapping very early of
> boot, where we can't use iomap APIs. So I will drop patches related to
> these platforms in v2 version.
>
> Please let me know if any concern in this approach.
I think ideally we wouldn't even need to know the virtual address
outside of smp_scu.c. If we can move all users of the address
into that file directly, it could become a local variable and
we change scu_power_mode() and scu_get_core_count() instead to
not require the address argument.
The only user I could find outside of that file is
static int shmobile_smp_scu_psr_core_disabled(int cpu)
{
unsigned long mask = SCU_PM_POWEROFF << (cpu * 8);
if ((__raw_readl(shmobile_scu_base + 8) & mask) == mask)
return 1;
return 0;
}
which can be done in the same file as well.
> >>> Then callers can decide which of these to call, and what error messages
> >>> to print on their failures.
> >>
> >> Splitting the function in two is probably simpler overall, but
> >> we may still have to look at all the callers: Any platform that
> >> currently tries to map it on any CPU and doesn't warn about the
> >> absence of the device node (or about scu_a9_has_base() == false)
> >> should really continue not to warn about that.
> >
> > Did you miss the bit where none of of_scu_enable() or a9_scu_enable()
> > should produce any warnings or errors to be printed. It's up to the
> > caller to report the failure, otherwise doing this doesn't make sense:
> >
> > if (of_scu_enable() < 0 && a9_scu_enable() < 0)
> > pr_err("Failed to map and enable the SCU\n");
> >
> > because if of_scu_enable() prints a warning/error, then it's patently
> > misleading.
> >
That's why I said "otherwise we can leave the warning in the caller
after checking the return code of the new APIs." for the case where
we actually need it.
> I will move out error message out of these helpers and let caller
> (platform specific code) handle and print error if required.
Ok.
Arnd
[toc] | [prev] | [next] | [standalone]
| From | "pankaj.dubey" <pankaj.dubey@samsung.com> |
|---|---|
| Date | 2016-11-18 04:30 +0100 |
| Subject | Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU |
| Message-ID | <sEI2d-1vi-7@gated-at.bofh.it> |
| In reply to | #1524505 |
On Thursday 17 November 2016 10:33 PM, Arnd Bergmann wrote:
> On Thursday, November 17, 2016 9:50:27 AM CET pankaj.dubey wrote:
>>
>>>>> of_scu_enable() which _only_ looks up the SCU address in DT and enables
>>>>> it if it finds it, otherwise returning failure.
>>>>>
>>>>> a9_scu_enable() which tries to use the A9 provided SCU address and
>>>>> enables it if it finds it, otherwise returning failure.
>>>>>
>>
>> OK, In that case I can see need for following four helpers as:
>>
>> 1: of_scu_enable() which will __only__ lookup the SCU address in DT and
>> enables it if it finds, otherwise return -ENOMEM failure.
>> This helper APIs is required and sufficient for most of platforms such
>> as exynos, berlin, realview, socfpga, STi, ux500, vexpress, rockchip and
>> mvebu
>>
>> 2: a9_scu_enable(), which will __only__ use A9 provided SCU address and
>> enables it, if address mapped successfully, otherwise returning failure.
>> This helper APIs is required and sufficient for two ARM platforms as of
>> now tegra and hisi.
>>
>> 3: of_scu_get_base() which will lookup the SCU address in DT and if node
>> found maps address and returns ioremapped address to caller.
>> This helper APIs is required for three ARM plaforms rockchip, mvebu and
>> ux500, along with scu_enable() API to enable and find number_of_cores.
>>
>> 4: s9_scu_iomap_base() which will internally use s9_scu_get_base() and
>> do ioremap of scu address and returns ioremapped address to the caller
>> along with ownership (caller has responsibility to unmap it).
>> This helper APIs is required to simplify SCU enable and related code in
>> two ARM plaforms BCM ans ZX.
>>
>> For remaining two ARM platforms (IMX and ZYNQ), none of these helpers
>> are useful for the time-being, as they need SCU mapping very early of
>> boot, where we can't use iomap APIs. So I will drop patches related to
>> these platforms in v2 version.
>>
>> Please let me know if any concern in this approach.
>
> I think ideally we wouldn't even need to know the virtual address
> outside of smp_scu.c. If we can move all users of the address
> into that file directly, it could become a local variable and
> we change scu_power_mode() and scu_get_core_count() instead to
> not require the address argument.
>
If we change scu_get_core_count() without address argument, what about
those platforms which are calling this API very early boot (mostly in
smp_init_cpus), during this stage we can't use iomap. These platforms
are doing static mapping of SCU base, and passing virtual address.
Currently following are platforms which needs SCU virtual address at
very early stage mostly for calling scu_get_core_count(scu_address):
IMX, ZYNQ, SPEAr, and OMAP2.
I can think of handling of these platform's early need of SCU in the
following way:
Probably we need something like early_a9_scu_get_core_count() which can
be handled in this way in smp_scu.c file itself:
+static struct map_desc scu_map __initdata = {
+ .length = SZ_256,
+ .type = MT_DEVICE,
+};
+
+static void __iomem *early_scu_map_io(void)
+{
+ unsigned long base;
+ void __iomem *scu_base;
+
+ base = scu_a9_get_base();
+ scu_map.pfn = __phys_to_pfn(base);
+ scu_map.virtual = base;
+ iotable_init(&scu_map, 1);
+ scu_base = (void __iomem *)base;
+ return scu_base;
+}
+
+/*
+ * early_a9_scu_get_core_count - Get number of CPU cores at very early boot
+ * Only platforms which needs number_of_cores during early boot should
call this
+ */
+unsigned int __init early_a9_scu_get_core_count(void)
+{
+ void __iomem *scu_base;
+
+ scu_base = early_scu_map_io();
+ return scu_get_core_count(scu_base);
+}
+
Please let me know how about using above approach to simplify platform
specific code of early users of SCU address.
Also for rest platforms, which are not using scu base at very early
stage, but are using virtual address which is mapped either from SCU
device node or using s9_scu_get_base() to map to SCU virtual address. To
separate these two we discussed to separate scu_enable in two helper
APIs as of_scu_enable and s9_scu_enable. So if we change
scu_get_core_count which do not requires address argument, this also
needs to be separated in two as of_scu_get_core_count and
s9_scu_get_core_count.
Thanks,
Pankaj Dubey
> The only user I could find outside of that file is
>
> static int shmobile_smp_scu_psr_core_disabled(int cpu)
> {
> unsigned long mask = SCU_PM_POWEROFF << (cpu * 8);
>
> if ((__raw_readl(shmobile_scu_base + 8) & mask) == mask)
> return 1;
>
> return 0;
> }
>
> which can be done in the same file as well.
>
>>>>> Then callers can decide which of these to call, and what error messages
>>>>> to print on their failures.
>>>>
>>>> Splitting the function in two is probably simpler overall, but
>>>> we may still have to look at all the callers: Any platform that
>>>> currently tries to map it on any CPU and doesn't warn about the
>>>> absence of the device node (or about scu_a9_has_base() == false)
>>>> should really continue not to warn about that.
>>>
>>> Did you miss the bit where none of of_scu_enable() or a9_scu_enable()
>>> should produce any warnings or errors to be printed. It's up to the
>>> caller to report the failure, otherwise doing this doesn't make sense:
>>>
>>> if (of_scu_enable() < 0 && a9_scu_enable() < 0)
>>> pr_err("Failed to map and enable the SCU\n");
>>>
>>> because if of_scu_enable() prints a warning/error, then it's patently
>>> misleading.
>>>
>
> That's why I said "otherwise we can leave the warning in the caller
> after checking the return code of the new APIs." for the case where
> we actually need it.
>
>> I will move out error message out of these helpers and let caller
>> (platform specific code) handle and print error if required.
>
> Ok.
>
> Arnd
>
>
>
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-11-18 13:20 +0100 |
| Subject | Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU |
| Message-ID | <sEQjd-6Z3-23@gated-at.bofh.it> |
| In reply to | #1524994 |
On Friday, November 18, 2016 8:54:30 AM CET pankaj.dubey wrote:
> >> Please let me know if any concern in this approach.
> >
> > I think ideally we wouldn't even need to know the virtual address
> > outside of smp_scu.c. If we can move all users of the address
> > into that file directly, it could become a local variable and
> > we change scu_power_mode() and scu_get_core_count() instead to
> > not require the address argument.
> >
>
> If we change scu_get_core_count() without address argument, what about
> those platforms which are calling this API very early boot (mostly in
> smp_init_cpus), during this stage we can't use iomap. These platforms
> are doing static mapping of SCU base, and passing virtual address.
>
> Currently following are platforms which needs SCU virtual address at
> very early stage mostly for calling scu_get_core_count(scu_address):
> IMX, ZYNQ, SPEAr, and OMAP2.
Ah, you are right, I missed how this is done earlier than the
scu_enable() call.
> I can think of handling of these platform's early need of SCU in the
> following way:
>
> Probably we need something like early_a9_scu_get_core_count() which can
> be handled in this way in smp_scu.c file itself:
>
> +static struct map_desc scu_map __initdata = {
> + .length = SZ_256,
> + .type = MT_DEVICE,
> +};
> +
> +static void __iomem *early_scu_map_io(void)
> +{
> + unsigned long base;
> + void __iomem *scu_base;
> +
> + base = scu_a9_get_base();
> + scu_map.pfn = __phys_to_pfn(base);
> + scu_map.virtual = base;
> + iotable_init(&scu_map, 1);
> + scu_base = (void __iomem *)base;
> + return scu_base;
> +}
Unfortunately, this doesn't work because scu_map.virtual must be
picked by the platform code in a way that doesn't conflict
with anything else. Setting up the iotable is probably not
something we should move into the smp_scu.c file but leave where
it currently is. You copied the trick from zynq that happens
to work there but probably not on the other three.
At some point we decided that at least new platforms should
always get the core count from DT rather than from the SCU,
but I don't know if we have to keep supporting old DTB files
without a full description of the CPUs on any of the
four platforms.
I see that there are four other users of scu_get_core_count()
that all call it from ->prepare_cpus, and we can probably
just use num_possible_cpus() at that point as the count
must have been initialized from DT already.
> +/*
> + * early_a9_scu_get_core_count - Get number of CPU cores at very early boot
> + * Only platforms which needs number_of_cores during early boot should
> call this
> + */
> +unsigned int __init early_a9_scu_get_core_count(void)
> +{
> + void __iomem *scu_base;
> +
> + scu_base = early_scu_map_io();
> + return scu_get_core_count(scu_base);
> +}
> +
>
> Please let me know how about using above approach to simplify platform
> specific code of early users of SCU address.
>
> Also for rest platforms, which are not using scu base at very early
> stage, but are using virtual address which is mapped either from SCU
> device node or using s9_scu_get_base() to map to SCU virtual address. To
> separate these two we discussed to separate scu_enable in two helper
> APIs as of_scu_enable and s9_scu_enable. So if we change
> scu_get_core_count which do not requires address argument, this also
> needs to be separated in two as of_scu_get_core_count and
> s9_scu_get_core_count.
We can probably leave get_core_count code alone for now, or
we change it so that we pass a virtual address into it
from the platform and have the SCU mapped there. One nice
property of the early mapping is that the later ioremap()
will just return the same virtual address, so we don't get
a double mapping when calling the scu_enable variant later.
Adding two functions of each doesn't sound too great though,
it would make the API more complicated when the intention was
to make it simpler by leaving out the address argument.
Maybe something like this?
diff --git a/arch/arm/kernel/smp_scu.c b/arch/arm/kernel/smp_scu.c
index 72f9241ad5db..c248a16e980f 100644
--- a/arch/arm/kernel/smp_scu.c
+++ b/arch/arm/kernel/smp_scu.c
@@ -24,6 +24,8 @@
#define SCU_INVALIDATE 0x0c
#define SCU_FPGA_REVISION 0x10
+static void __iomem *scu_base_addr;
+
#ifdef CONFIG_SMP
/*
* Get the number of CPU cores from the SCU configuration
@@ -41,6 +43,9 @@ void scu_enable(void __iomem *scu_base)
{
u32 scu_ctrl;
+ if (scu_base)
+ scu_base = scu_base_addr;
+
#ifdef CONFIG_ARM_ERRATA_764369
/* Cortex-A9 only */
if ((read_cpuid_id() & 0xff0ffff0) == 0x410fc090) {
@@ -85,6 +90,9 @@ int scu_power_mode(void __iomem *scu_base, unsigned int mode)
unsigned int val;
int cpu = MPIDR_AFFINITY_LEVEL(cpu_logical_map(smp_processor_id()), 0);
+ if (scu_base)
+ scu_base = scu_base_addr;
+
if (mode > 3 || mode == 1 || cpu > 3)
return -EINVAL;
@@ -94,3 +102,31 @@ int scu_power_mode(void __iomem *scu_base, unsigned int mode)
return 0;
}
+
+int __init scu_probe_a9(void)
+{
+ phys_addr_t base;
+
+ if (!scu_a9_has_base)
+ return -ENODEV;
+
+ base = scu_a9_get_base()
+ if (!base)
+ return -ENODEV;
+
+ scu_base_addr = ioremap(base, PAGE_SIZE);
+ if (scu_base_addr)
+ return -ENOMEM;
+
+ return scu_get_core_count(scu_base_addr);
+}
+
+int __init scu_probe_dt(void)
+{
+ ...
+ scu_base_addr = of_iomap(np, 0);
+ if (scu_base_addr)
+ return -ENOMEM;
+
+ return scu_get_core_count(scu_base_addr);
+}
Arnd
[toc] | [prev] | [next] | [standalone]
| From | Russell King - ARM Linux <linux@armlinux.org.uk> |
|---|---|
| Date | 2016-11-18 13:50 +0100 |
| Subject | Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU |
| Message-ID | <sEQM9-7ax-15@gated-at.bofh.it> |
| In reply to | #1525252 |
On Fri, Nov 18, 2016 at 01:14:35PM +0100, Arnd Bergmann wrote:
> @@ -41,6 +43,9 @@ void scu_enable(void __iomem *scu_base)
> {
> u32 scu_ctrl;
>
> + if (scu_base)
> + scu_base = scu_base_addr;
> +
This looks to me like nonsense.
> #ifdef CONFIG_ARM_ERRATA_764369
> /* Cortex-A9 only */
> if ((read_cpuid_id() & 0xff0ffff0) == 0x410fc090) {
> @@ -85,6 +90,9 @@ int scu_power_mode(void __iomem *scu_base, unsigned int mode)
> unsigned int val;
> int cpu = MPIDR_AFFINITY_LEVEL(cpu_logical_map(smp_processor_id()), 0);
>
> + if (scu_base)
> + scu_base = scu_base_addr;
> +
Ditto.
Rather than doing this, I'd much prefer to always store the SCU base in
the SCU code, and remove the "void __iomem *scu_base" argment from all
these functions.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-11-18 14:40 +0100 |
| Subject | Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU |
| Message-ID | <sERyz-7GV-63@gated-at.bofh.it> |
| In reply to | #1525274 |
On Friday, November 18, 2016 12:48:07 PM CET Russell King - ARM Linux wrote:
> On Fri, Nov 18, 2016 at 01:14:35PM +0100, Arnd Bergmann wrote:
> > @@ -41,6 +43,9 @@ void scu_enable(void __iomem *scu_base)
> > {
> > u32 scu_ctrl;
> >
> > + if (scu_base)
> > + scu_base = scu_base_addr;
> > +
>
> This looks to me like nonsense.
>
> > #ifdef CONFIG_ARM_ERRATA_764369
> > /* Cortex-A9 only */
> > if ((read_cpuid_id() & 0xff0ffff0) == 0x410fc090) {
> > @@ -85,6 +90,9 @@ int scu_power_mode(void __iomem *scu_base, unsigned int mode)
> > unsigned int val;
> > int cpu = MPIDR_AFFINITY_LEVEL(cpu_logical_map(smp_processor_id()), 0);
> >
> > + if (scu_base)
> > + scu_base = scu_base_addr;
> > +
>
> Ditto.
>
> Rather than doing this, I'd much prefer to always store the SCU base in
> the SCU code, and remove the "void __iomem *scu_base" argment from all
> these functions.
Ok, then we just need one scu_probe_*() variant for each of the
four methods of initializing it (iotable, of_iomap,
ioremap(scu_a9_get_base) and hardcoded.
The intention of doing the fallback for the NULL argument was
to avoid having to add lots of new API while also allowing
the change to be done one platform at a time.
If we remove the argument from the other functions, they either
need to get a new name, or we change them all to the new prototype
at once. Either way works fine, do you have a preference between
them?
Arnd
[toc] | [prev] | [next] | [standalone]
| From | Russell King - ARM Linux <linux@armlinux.org.uk> |
|---|---|
| Date | 2016-11-14 14:50 +0100 |
| Subject | Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU |
| Message-ID | <sDpO1-7DY-9@gated-at.bofh.it> |
| In reply to | #1521299 |
This should be sent _to_ me because it's touching generic ARM code.
Thanks.
On Mon, Nov 14, 2016 at 10:31:56AM +0530, Pankaj Dubey wrote:
> Many platforms are duplicating code for enabling SCU, lets add
> common code to enable SCU by parsing SCU device node so the duplication
> in each platform can be avoided.
>
> CC: Krzysztof Kozlowski <krzk@kernel.org>
> CC: Jisheng Zhang <jszhang@marvell.com>
> CC: Russell King <linux@armlinux.org.uk>
> CC: Dinh Nguyen <dinguyen@opensource.altera.com>
> CC: Patrice Chotard <patrice.chotard@st.com>
> CC: Linus Walleij <linus.walleij@linaro.org>
> CC: Liviu Dudau <liviu.dudau@arm.com>
> CC: Ray Jui <rjui@broadcom.com>
> CC: Stephen Warren <swarren@wwwdotorg.org>
> CC: Heiko Stuebner <heiko@sntech.de>
> CC: Shawn Guo <shawnguo@kernel.org>
> CC: Michal Simek <michal.simek@xilinx.com>
> CC: Wei Xu <xuwei5@hisilicon.com>
> CC: Andrew Lunn <andrew@lunn.ch>
> CC: Jun Nie <jun.nie@linaro.org>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> arch/arm/include/asm/smp_scu.h | 4 +++
> arch/arm/kernel/smp_scu.c | 56 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 60 insertions(+)
>
> diff --git a/arch/arm/include/asm/smp_scu.h b/arch/arm/include/asm/smp_scu.h
> index bfe163c..fdeec07 100644
> --- a/arch/arm/include/asm/smp_scu.h
> +++ b/arch/arm/include/asm/smp_scu.h
> @@ -39,8 +39,12 @@ static inline int scu_power_mode(void __iomem *scu_base, unsigned int mode)
>
> #if defined(CONFIG_SMP) && defined(CONFIG_HAVE_ARM_SCU)
> void scu_enable(void __iomem *scu_base);
> +void __iomem *of_scu_get_base(void);
> +int of_scu_enable(void);
> #else
> static inline void scu_enable(void __iomem *scu_base) {}
> +static inline void __iomem *of_scu_get_base(void) {return NULL; }
> +static inline int of_scu_enable(void) {return 0; }
> #endif
>
> #endif
> diff --git a/arch/arm/kernel/smp_scu.c b/arch/arm/kernel/smp_scu.c
> index 72f9241..d0ac3ed 100644
> --- a/arch/arm/kernel/smp_scu.c
> +++ b/arch/arm/kernel/smp_scu.c
> @@ -10,6 +10,7 @@
> */
> #include <linux/init.h>
> #include <linux/io.h>
> +#include <linux/of_address.h>
>
> #include <asm/smp_plat.h>
> #include <asm/smp_scu.h>
> @@ -70,6 +71,61 @@ void scu_enable(void __iomem *scu_base)
> */
> flush_cache_all();
> }
> +
> +static const struct of_device_id scu_match[] = {
> + { .compatible = "arm,cortex-a9-scu", },
> + { .compatible = "arm,cortex-a5-scu", },
> + { }
> +};
> +
> +/*
> + * Helper API to get SCU base address
> + * In case platform DT do not have SCU node, or iomap fails
> + * this call will fallback and will try to map via call to
> + * scu_a9_get_base.
> + * This will return ownership of scu_base to the caller
> + */
> +void __iomem *of_scu_get_base(void)
> +{
> + unsigned long base = 0;
> + struct device_node *np;
> + void __iomem *scu_base;
> +
> + np = of_find_matching_node(NULL, scu_match);
> + scu_base = of_iomap(np, 0);
> + of_node_put(np);
> + if (!scu_base) {
> + pr_err("%s failed to map scu_base via DT\n", __func__);
> + if (scu_a9_has_base()) {
> + base = scu_a9_get_base();
> + scu_base = ioremap(base, SZ_4K);
> + }
> + if (!scu_base) {
> + pr_err("%s failed to map scu_base\n", __func__);
> + return IOMEM_ERR_PTR(-ENOMEM);
I can't see the point of using error-pointers here - it's not like we
really know why we're failing, so just return NULL.
> + }
> + }
> + return scu_base;
> +}
> +
> +/*
> + * Enable SCU via mapping scu_base DT
> + * If scu_base mapped successfully scu will be enabled and in case of
> + * failure if will return non-zero error code
> + */
> +int of_scu_enable(void)
> +{
> + void __iomem *scu_base;
> +
> + scu_base = of_scu_get_base();
> + if (!IS_ERR(scu_base)) {
> + scu_enable(scu_base);
> + iounmap(scu_base);
> + return 0;
> + }
> + return PTR_ERR(scu_base);
and just return your -ENOMEM here.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
[toc] | [prev] | [next] | [standalone]
| From | "pankaj.dubey" <pankaj.dubey@samsung.com> |
|---|---|
| Date | 2016-11-17 03:20 +0100 |
| Subject | Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU |
| Message-ID | <sEksV-2Rd-5@gated-at.bofh.it> |
| In reply to | #1521658 |
Hi Russell,
On Monday 14 November 2016 07:18 PM, Russell King - ARM Linux wrote:
> This should be sent _to_ me because it's touching generic ARM code.
> Thanks.
>
Sorry for this.
I had included your email in CC for this patch, but looks like my email
client had some issue and this patch could not reach to your mailbox. I
will take care in future.
> On Mon, Nov 14, 2016 at 10:31:56AM +0530, Pankaj Dubey wrote:
>> Many platforms are duplicating code for enabling SCU, lets add
>> common code to enable SCU by parsing SCU device node so the duplication
>> in each platform can be avoided.
>>
>> CC: Krzysztof Kozlowski <krzk@kernel.org>
>> CC: Jisheng Zhang <jszhang@marvell.com>
>> CC: Russell King <linux@armlinux.org.uk>
>> CC: Dinh Nguyen <dinguyen@opensource.altera.com>
>> CC: Patrice Chotard <patrice.chotard@st.com>
>> CC: Linus Walleij <linus.walleij@linaro.org>
>> CC: Liviu Dudau <liviu.dudau@arm.com>
>> CC: Ray Jui <rjui@broadcom.com>
>> CC: Stephen Warren <swarren@wwwdotorg.org>
>> CC: Heiko Stuebner <heiko@sntech.de>
>> CC: Shawn Guo <shawnguo@kernel.org>
>> CC: Michal Simek <michal.simek@xilinx.com>
>> CC: Wei Xu <xuwei5@hisilicon.com>
>> CC: Andrew Lunn <andrew@lunn.ch>
>> CC: Jun Nie <jun.nie@linaro.org>
>> Suggested-by: Arnd Bergmann <arnd@arndb.de>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> ---
>> arch/arm/include/asm/smp_scu.h | 4 +++
>> arch/arm/kernel/smp_scu.c | 56 ++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 60 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/smp_scu.h b/arch/arm/include/asm/smp_scu.h
>> index bfe163c..fdeec07 100644
>> --- a/arch/arm/include/asm/smp_scu.h
>> +++ b/arch/arm/include/asm/smp_scu.h
>> @@ -39,8 +39,12 @@ static inline int scu_power_mode(void __iomem *scu_base, unsigned int mode)
>>
>> #if defined(CONFIG_SMP) && defined(CONFIG_HAVE_ARM_SCU)
>> void scu_enable(void __iomem *scu_base);
>> +void __iomem *of_scu_get_base(void);
>> +int of_scu_enable(void);
>> #else
>> static inline void scu_enable(void __iomem *scu_base) {}
>> +static inline void __iomem *of_scu_get_base(void) {return NULL; }
>> +static inline int of_scu_enable(void) {return 0; }
>> #endif
>>
>> #endif
>> diff --git a/arch/arm/kernel/smp_scu.c b/arch/arm/kernel/smp_scu.c
>> index 72f9241..d0ac3ed 100644
>> --- a/arch/arm/kernel/smp_scu.c
>> +++ b/arch/arm/kernel/smp_scu.c
>> @@ -10,6 +10,7 @@
>> */
>> #include <linux/init.h>
>> #include <linux/io.h>
>> +#include <linux/of_address.h>
>>
>> #include <asm/smp_plat.h>
>> #include <asm/smp_scu.h>
>> @@ -70,6 +71,61 @@ void scu_enable(void __iomem *scu_base)
>> */
>> flush_cache_all();
>> }
>> +
>> +static const struct of_device_id scu_match[] = {
>> + { .compatible = "arm,cortex-a9-scu", },
>> + { .compatible = "arm,cortex-a5-scu", },
>> + { }
>> +};
>> +
>> +/*
>> + * Helper API to get SCU base address
>> + * In case platform DT do not have SCU node, or iomap fails
>> + * this call will fallback and will try to map via call to
>> + * scu_a9_get_base.
>> + * This will return ownership of scu_base to the caller
>> + */
>> +void __iomem *of_scu_get_base(void)
>> +{
>> + unsigned long base = 0;
>> + struct device_node *np;
>> + void __iomem *scu_base;
>> +
>> + np = of_find_matching_node(NULL, scu_match);
>> + scu_base = of_iomap(np, 0);
>> + of_node_put(np);
>> + if (!scu_base) {
>> + pr_err("%s failed to map scu_base via DT\n", __func__);
>> + if (scu_a9_has_base()) {
>> + base = scu_a9_get_base();
>> + scu_base = ioremap(base, SZ_4K);
>> + }
>> + if (!scu_base) {
>> + pr_err("%s failed to map scu_base\n", __func__);
>> + return IOMEM_ERR_PTR(-ENOMEM);
>
> I can't see the point of using error-pointers here - it's not like we
> really know why we're failing, so just return NULL.
>
>> + }
>> + }
>> + return scu_base;
>> +}
>> +
>> +/*
>> + * Enable SCU via mapping scu_base DT
>> + * If scu_base mapped successfully scu will be enabled and in case of
>> + * failure if will return non-zero error code
>> + */
>> +int of_scu_enable(void)
>> +{
>> + void __iomem *scu_base;
>> +
>> + scu_base = of_scu_get_base();
>> + if (!IS_ERR(scu_base)) {
>> + scu_enable(scu_base);
>> + iounmap(scu_base);
>> + return 0;
>> + }
>> + return PTR_ERR(scu_base);
>
> and just return your -ENOMEM here.
>
[toc] | [prev] | [next] | [standalone]
| From | Pankaj Dubey <pankaj.dubey@samsung.com> |
|---|---|
| Date | 2016-11-14 06:10 +0100 |
| Subject | [PATCH 09/16] ARM: BCM: use generic API for enabling SCU |
| Message-ID | <sDhGN-2pH-3@gated-at.bofh.it> |
| In reply to | #1521298 |
Now as we have of_scu_enable which takes care of mapping
scu base from DT, lets use it.
CC: Florian Fainelli <f.fainelli@gmail.com>
CC: Ray Jui <rjui@broadcom.com>
CC: Scott Branden <sbranden@broadcom.com>
CC: bcm-kernel-feedback-list@broadcom.com
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
arch/arm/mach-bcm/bcm63xx_smp.c | 18 ++--------------
arch/arm/mach-bcm/platsmp.c | 46 +----------------------------------------
2 files changed, 3 insertions(+), 61 deletions(-)
diff --git a/arch/arm/mach-bcm/bcm63xx_smp.c b/arch/arm/mach-bcm/bcm63xx_smp.c
index 9b6727e..a4c6ecd 100644
--- a/arch/arm/mach-bcm/bcm63xx_smp.c
+++ b/arch/arm/mach-bcm/bcm63xx_smp.c
@@ -20,9 +20,6 @@
#include "bcm63xx_smp.h"
-/* Size of mapped Cortex A9 SCU address space */
-#define CORTEX_A9_SCU_SIZE 0x58
-
/*
* Enable the Cortex A9 Snoop Control Unit
*
@@ -35,7 +32,6 @@
*/
static int __init scu_a9_enable(void)
{
- unsigned long config_base;
void __iomem *scu_base;
unsigned int i, ncores;
@@ -44,19 +40,9 @@ static int __init scu_a9_enable(void)
return -ENXIO;
}
- /* Config base address register value is zero for uniprocessor */
- config_base = scu_a9_get_base();
- if (!config_base) {
- pr_err("hardware reports only one core\n");
- return -ENOENT;
- }
-
- scu_base = ioremap((phys_addr_t)config_base, CORTEX_A9_SCU_SIZE);
- if (!scu_base) {
- pr_err("failed to remap config base (%lu/%u) for SCU\n",
- config_base, CORTEX_A9_SCU_SIZE);
+ scu_base = of_scu_get_base();
+ if (IS_ERR(scu_base))
return -ENOMEM;
- }
scu_enable(scu_base);
diff --git a/arch/arm/mach-bcm/platsmp.c b/arch/arm/mach-bcm/platsmp.c
index 3ac3a9b..743599a 100644
--- a/arch/arm/mach-bcm/platsmp.c
+++ b/arch/arm/mach-bcm/platsmp.c
@@ -28,9 +28,6 @@
#include <asm/smp_plat.h>
#include <asm/smp_scu.h>
-/* Size of mapped Cortex A9 SCU address space */
-#define CORTEX_A9_SCU_SIZE 0x58
-
#define SECONDARY_TIMEOUT_NS NSEC_PER_MSEC /* 1 msec (in nanoseconds) */
#define BOOT_ADDR_CPUID_MASK 0x3
@@ -38,47 +35,6 @@
#define OF_SECONDARY_BOOT "secondary-boot-reg"
#define MPIDR_CPUID_BITMASK 0x3
-/*
- * Enable the Cortex A9 Snoop Control Unit
- *
- * By the time this is called we already know there are multiple
- * cores present. We assume we're running on a Cortex A9 processor,
- * so any trouble getting the base address register or getting the
- * SCU base is a problem.
- *
- * Return 0 if successful or an error code otherwise.
- */
-static int __init scu_a9_enable(void)
-{
- unsigned long config_base;
- void __iomem *scu_base;
-
- if (!scu_a9_has_base()) {
- pr_err("no configuration base address register!\n");
- return -ENXIO;
- }
-
- /* Config base address register value is zero for uniprocessor */
- config_base = scu_a9_get_base();
- if (!config_base) {
- pr_err("hardware reports only one core\n");
- return -ENOENT;
- }
-
- scu_base = ioremap((phys_addr_t)config_base, CORTEX_A9_SCU_SIZE);
- if (!scu_base) {
- pr_err("failed to remap config base (%lu/%u) for SCU\n",
- config_base, CORTEX_A9_SCU_SIZE);
- return -ENOMEM;
- }
-
- scu_enable(scu_base);
-
- iounmap(scu_base); /* That's the last we'll need of this */
-
- return 0;
-}
-
static u32 secondary_boot_addr_for(unsigned int cpu)
{
u32 secondary_boot_addr = 0;
@@ -134,7 +90,7 @@ static void __init bcm_smp_prepare_cpus(unsigned int max_cpus)
const cpumask_t only_cpu_0 = { CPU_BITS_CPU0 };
/* Enable the SCU on Cortex A9 based SoCs */
- if (scu_a9_enable()) {
+ if (of_scu_enable()) {
/* Update the CPU present map to reflect uniprocessor mode */
pr_warn("failed to enable A9 SCU - disabling SMP\n");
init_cpu_present(&only_cpu_0);
--
2.7.4
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web