* [PATCH 01/14] ARM: milbeaut: Add basic support for Milbeaut m10v SoC
2018-11-19 1:01 [PATCH 00/14] Add basic support for Socionext Milbeaut M10V SoCs Sugaya Taichi
@ 2018-11-19 1:01 ` Sugaya Taichi
2018-11-19 16:24 ` Rob Herring
2018-11-19 1:01 ` [PATCH 02/14] dt-bindings: soc: milbeaut: Add Milbeaut trampoline description Sugaya Taichi
` (7 subsequent siblings)
8 siblings, 1 reply; 43+ messages in thread
From: Sugaya Taichi @ 2018-11-19 1:01 UTC (permalink / raw)
To: linux-clk, devicetree, linux-arm-kernel, linux-kernel, linux-serial
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Greg Kroah-Hartman, Daniel Lezcano, Thomas Gleixner,
Russell King, Jiri Slaby, Masami Hiramatsu, Jassi Brar,
Sugaya Taichi
This adds the basic M10V SoC support under arch/arm.
Since all cores are activated in the custom bootloader before booting
linux, it is necessary to wait for sub-cores using the trampoline area.
Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
---
arch/arm/Kconfig | 2 +
arch/arm/Makefile | 1 +
arch/arm/mach-milbeaut/Kconfig | 28 +++++++
arch/arm/mach-milbeaut/Makefile | 3 +
arch/arm/mach-milbeaut/m10v_evb.c | 31 ++++++++
arch/arm/mach-milbeaut/platsmp.c | 157 ++++++++++++++++++++++++++++++++++++++
6 files changed, 222 insertions(+)
create mode 100644 arch/arm/mach-milbeaut/Kconfig
create mode 100644 arch/arm/mach-milbeaut/Makefile
create mode 100644 arch/arm/mach-milbeaut/m10v_evb.c
create mode 100644 arch/arm/mach-milbeaut/platsmp.c
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 91be74d..0b8a1af 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -767,6 +767,8 @@ source "arch/arm/mach-mediatek/Kconfig"
source "arch/arm/mach-meson/Kconfig"
+source "arch/arm/mach-milbeaut/Kconfig"
+
source "arch/arm/mach-mmp/Kconfig"
source "arch/arm/mach-moxart/Kconfig"
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 05a91d8..627853c 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -190,6 +190,7 @@ machine-$(CONFIG_ARCH_MV78XX0) += mv78xx0
machine-$(CONFIG_ARCH_MVEBU) += mvebu
machine-$(CONFIG_ARCH_MXC) += imx
machine-$(CONFIG_ARCH_MEDIATEK) += mediatek
+machine-$(CONFIG_ARCH_MILBEAUT) += milbeaut
machine-$(CONFIG_ARCH_MXS) += mxs
machine-$(CONFIG_ARCH_NETX) += netx
machine-$(CONFIG_ARCH_NOMADIK) += nomadik
diff --git a/arch/arm/mach-milbeaut/Kconfig b/arch/arm/mach-milbeaut/Kconfig
new file mode 100644
index 0000000..63b6f69
--- /dev/null
+++ b/arch/arm/mach-milbeaut/Kconfig
@@ -0,0 +1,28 @@
+# SPDX-License-Identifier: GPL-2.0
+menuconfig ARCH_MILBEAUT
+ bool "Socionext Milbeaut SoCs"
+ depends on ARCH_MULTI_V7
+ select ARM_GIC
+ select CLKDEV_LOOKUP
+ select GENERIC_CLOCKEVENTS
+ select CLKSRC_MMIO
+ select ZONE_DMA
+ help
+ This enables support for Socionext Milbeaut SoCs
+
+if ARCH_MILBEAUT
+
+config ARCH_MILBEAUT_M10V
+ bool "Milbeaut SC2000/M10V platform"
+ select ARM_ARCH_TIMER
+ select M10V_TIMER
+ select PINCTRL
+ select PINCTRL_M10V
+ help
+ Support for Socionext's MILBEAUT M10V based systems
+
+config MACH_M10V_EVB
+ bool "Support for Milbeaut Evaluation boards"
+ default y
+
+endif
diff --git a/arch/arm/mach-milbeaut/Makefile b/arch/arm/mach-milbeaut/Makefile
new file mode 100644
index 0000000..64f6f52
--- /dev/null
+++ b/arch/arm/mach-milbeaut/Makefile
@@ -0,0 +1,3 @@
+obj-$(CONFIG_SMP) += platsmp.o
+obj-$(CONFIG_MACH_M10V_EVB) += m10v_evb.o
+
diff --git a/arch/arm/mach-milbeaut/m10v_evb.c b/arch/arm/mach-milbeaut/m10v_evb.c
new file mode 100644
index 0000000..a1fa7c3
--- /dev/null
+++ b/arch/arm/mach-milbeaut/m10v_evb.c
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Socionext Inc.
+ * Copyright: (C) 2015 Linaro Ltd.
+ */
+
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include <asm/mach/arch.h>
+#include <asm/mach/map.h>
+
+static struct map_desc m10v_io_desc[] __initdata = {
+};
+
+void __init m10v_map_io(void)
+{
+ debug_ll_io_init();
+ iotable_init(m10v_io_desc, ARRAY_SIZE(m10v_io_desc));
+}
+
+static const char * const m10v_compat[] = {
+ "socionext,milbeaut-m10v-evb",
+ NULL,
+};
+
+DT_MACHINE_START(M10V_REB, "Socionext Milbeaut")
+ .dt_compat = m10v_compat,
+ .l2c_aux_mask = 0xffffffff,
+ .map_io = m10v_map_io,
+MACHINE_END
diff --git a/arch/arm/mach-milbeaut/platsmp.c b/arch/arm/mach-milbeaut/platsmp.c
new file mode 100644
index 0000000..b706851
--- /dev/null
+++ b/arch/arm/mach-milbeaut/platsmp.c
@@ -0,0 +1,157 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Socionext Inc.
+ * Copyright: (C) 2015 Linaro Ltd.
+ */
+
+#include <linux/cpu_pm.h>
+#include <linux/irqchip/arm-gic.h>
+#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/suspend.h>
+
+#include <asm/cacheflush.h>
+#include <asm/cp15.h>
+#include <asm/idmap.h>
+#include <asm/smp_plat.h>
+#include <asm/suspend.h>
+
+#define M10V_MAX_CPU 4
+
+#define KERNEL_UNBOOT_FLAG 0x12345678
+#define CPU_FINISH_SUSPEND_FLAG 0x56784321
+
+static void __iomem *trampoline;
+
+static int m10v_boot_secondary(unsigned int l_cpu, struct task_struct *idle)
+{
+ unsigned int mpidr, cpu, cluster;
+
+ mpidr = cpu_logical_map(l_cpu);
+ cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+ cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+
+ if (cpu >= M10V_MAX_CPU)
+ return -EINVAL;
+
+ pr_debug("%s: cpu %u cluster %u\n", __func__, cpu, cluster);
+
+ writel(virt_to_phys(secondary_startup), trampoline + cpu * 4);
+ arch_send_wakeup_ipi_mask(cpumask_of(l_cpu));
+
+ return 0;
+}
+
+static void m10v_cpu_die(unsigned int l_cpu)
+{
+ gic_cpu_if_down(0);
+
+ v7_exit_coherency_flush(louis);
+
+ /* Now we are prepared for power-down, do it: */
+ wfi();
+}
+
+static int m10v_cpu_kill(unsigned int l_cpu)
+{
+ unsigned int mpidr, cpu;
+
+ mpidr = cpu_logical_map(l_cpu);
+ cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+
+ writel(KERNEL_UNBOOT_FLAG, trampoline + cpu * 4);
+
+ return 1;
+}
+
+static struct smp_operations m10v_smp_ops __initdata = {
+ .smp_boot_secondary = m10v_boot_secondary,
+ .cpu_die = m10v_cpu_die,
+ .cpu_kill = m10v_cpu_kill,
+};
+
+static int __init m10v_smp_init(void)
+{
+ unsigned int mpidr, cpu, cluster;
+ struct device_node *np;
+
+ np = of_find_compatible_node(NULL, NULL, "socionext,milbeaut-m10v-evb");
+ if (!np || !of_device_is_available(np))
+ return -ENODEV;
+ of_node_put(np);
+
+ np = of_find_compatible_node(NULL, NULL, "socionext,smp-trampoline");
+ if (!np)
+ return -ENODEV;
+
+ trampoline = of_iomap(np, 0);
+ if (!trampoline)
+ return -ENODEV;
+ of_node_put(np);
+
+ mpidr = read_cpuid_mpidr();
+ cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+ cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+
+ pr_info("MCPM boot on cpu_%u cluster_%u\n", cpu, cluster);
+
+ for (cpu = 0; cpu < M10V_MAX_CPU; cpu++)
+ writel(KERNEL_UNBOOT_FLAG, trampoline + cpu * 4);
+
+ smp_set_ops(&m10v_smp_ops);
+
+ return 0;
+}
+early_initcall(m10v_smp_init);
+
+static int m10v_pm_valid(suspend_state_t state)
+{
+ return (state == PM_SUSPEND_STANDBY) || (state == PM_SUSPEND_MEM);
+}
+
+typedef void (*phys_reset_t)(unsigned long);
+static phys_reset_t phys_reset;
+
+static int m10v_die(unsigned long arg)
+{
+ setup_mm_for_reboot();
+ asm("wfi");
+ /* Boot just like a secondary */
+ phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
+ phys_reset(virt_to_phys(cpu_resume));
+
+ return 0;
+}
+
+static int m10v_pm_enter(suspend_state_t state)
+{
+ switch (state) {
+ case PM_SUSPEND_STANDBY:
+ pr_err("STANDBY\n");
+ asm("wfi");
+ break;
+ case PM_SUSPEND_MEM:
+ pr_err("SUSPEND\n");
+ cpu_pm_enter();
+ cpu_suspend(0, m10v_die);
+ cpu_pm_exit();
+ break;
+ }
+ return 0;
+}
+
+static const struct platform_suspend_ops m10v_pm_ops = {
+ .valid = m10v_pm_valid,
+ .enter = m10v_pm_enter,
+};
+
+struct clk *m10v_clclk_register(struct device *cpu_dev);
+
+static int __init m10v_pm_init(void)
+{
+ suspend_set_ops(&m10v_pm_ops);
+
+ return 0;
+}
+late_initcall(m10v_pm_init);
--
1.9.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 01/14] ARM: milbeaut: Add basic support for Milbeaut m10v SoC
2018-11-19 1:01 ` [PATCH 01/14] ARM: milbeaut: Add basic support for Milbeaut m10v SoC Sugaya Taichi
@ 2018-11-19 16:24 ` Rob Herring
2018-11-21 9:33 ` sugaya.taichi
0 siblings, 1 reply; 43+ messages in thread
From: Rob Herring @ 2018-11-19 16:24 UTC (permalink / raw)
To: sugaya.taichi
Cc: linux-clk, devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-kernel, open list:SERIAL DRIVERS, Michael Turquette,
Stephen Boyd, Mark Rutland, Greg Kroah-Hartman, Daniel Lezcano,
Thomas Gleixner, Russell King, Jiri Slaby, Masami Hiramatsu,
Jassi Brar
On Sun, Nov 18, 2018 at 7:00 PM Sugaya Taichi
<sugaya.taichi@socionext.com> wrote:
>
> This adds the basic M10V SoC support under arch/arm.
> Since all cores are activated in the custom bootloader before booting
> linux, it is necessary to wait for sub-cores using the trampoline area.
>
> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
> ---
> arch/arm/Kconfig | 2 +
> arch/arm/Makefile | 1 +
> arch/arm/mach-milbeaut/Kconfig | 28 +++++++
> arch/arm/mach-milbeaut/Makefile | 3 +
> arch/arm/mach-milbeaut/m10v_evb.c | 31 ++++++++
> arch/arm/mach-milbeaut/platsmp.c | 157 ++++++++++++++++++++++++++++++++++++++
> 6 files changed, 222 insertions(+)
> create mode 100644 arch/arm/mach-milbeaut/Kconfig
> create mode 100644 arch/arm/mach-milbeaut/Makefile
> create mode 100644 arch/arm/mach-milbeaut/m10v_evb.c
> create mode 100644 arch/arm/mach-milbeaut/platsmp.c
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 91be74d..0b8a1af 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -767,6 +767,8 @@ source "arch/arm/mach-mediatek/Kconfig"
>
> source "arch/arm/mach-meson/Kconfig"
>
> +source "arch/arm/mach-milbeaut/Kconfig"
> +
> source "arch/arm/mach-mmp/Kconfig"
>
> source "arch/arm/mach-moxart/Kconfig"
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index 05a91d8..627853c 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -190,6 +190,7 @@ machine-$(CONFIG_ARCH_MV78XX0) += mv78xx0
> machine-$(CONFIG_ARCH_MVEBU) += mvebu
> machine-$(CONFIG_ARCH_MXC) += imx
> machine-$(CONFIG_ARCH_MEDIATEK) += mediatek
> +machine-$(CONFIG_ARCH_MILBEAUT) += milbeaut
> machine-$(CONFIG_ARCH_MXS) += mxs
> machine-$(CONFIG_ARCH_NETX) += netx
> machine-$(CONFIG_ARCH_NOMADIK) += nomadik
> diff --git a/arch/arm/mach-milbeaut/Kconfig b/arch/arm/mach-milbeaut/Kconfig
> new file mode 100644
> index 0000000..63b6f69
> --- /dev/null
> +++ b/arch/arm/mach-milbeaut/Kconfig
> @@ -0,0 +1,28 @@
> +# SPDX-License-Identifier: GPL-2.0
> +menuconfig ARCH_MILBEAUT
> + bool "Socionext Milbeaut SoCs"
> + depends on ARCH_MULTI_V7
> + select ARM_GIC
> + select CLKDEV_LOOKUP
> + select GENERIC_CLOCKEVENTS
> + select CLKSRC_MMIO
The clock and timer drivers' kconfig entries should select these.
> + select ZONE_DMA
Why is this needed?
> + help
> + This enables support for Socionext Milbeaut SoCs
> +
> +if ARCH_MILBEAUT
> +
> +config ARCH_MILBEAUT_M10V
> + bool "Milbeaut SC2000/M10V platform"
> + select ARM_ARCH_TIMER
> + select M10V_TIMER
> + select PINCTRL
> + select PINCTRL_M10V
> + help
> + Support for Socionext's MILBEAUT M10V based systems
> +
> +config MACH_M10V_EVB
> + bool "Support for Milbeaut Evaluation boards"
You shouldn't need a kconfig entry for each board.
> + default y
> +
> +endif
> diff --git a/arch/arm/mach-milbeaut/Makefile b/arch/arm/mach-milbeaut/Makefile
> new file mode 100644
> index 0000000..64f6f52
> --- /dev/null
> +++ b/arch/arm/mach-milbeaut/Makefile
> @@ -0,0 +1,3 @@
> +obj-$(CONFIG_SMP) += platsmp.o
> +obj-$(CONFIG_MACH_M10V_EVB) += m10v_evb.o
> +
> diff --git a/arch/arm/mach-milbeaut/m10v_evb.c b/arch/arm/mach-milbeaut/m10v_evb.c
> new file mode 100644
> index 0000000..a1fa7c3
> --- /dev/null
> +++ b/arch/arm/mach-milbeaut/m10v_evb.c
This all looks SoC specific, not board specific.
> @@ -0,0 +1,31 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Socionext Inc.
> + * Copyright: (C) 2015 Linaro Ltd.
> + */
> +
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
Not needed.
> +
> +#include <asm/mach/arch.h>
> +#include <asm/mach/map.h>
> +
> +static struct map_desc m10v_io_desc[] __initdata = {
> +};
> +
> +void __init m10v_map_io(void)
> +{
> + debug_ll_io_init();
If you use earlycon instead, then this isn't needed.
> + iotable_init(m10v_io_desc, ARRAY_SIZE(m10v_io_desc));
This isn't needed.
> +}
> +
> +static const char * const m10v_compat[] = {
> + "socionext,milbeaut-m10v-evb",
> + NULL,
> +};
> +
> +DT_MACHINE_START(M10V_REB, "Socionext Milbeaut")
> + .dt_compat = m10v_compat,
> + .l2c_aux_mask = 0xffffffff,
> + .map_io = m10v_map_io,
It looks like you can remove this entire file and use the default machine desc.
> +MACHINE_END
> diff --git a/arch/arm/mach-milbeaut/platsmp.c b/arch/arm/mach-milbeaut/platsmp.c
> new file mode 100644
> index 0000000..b706851
> --- /dev/null
> +++ b/arch/arm/mach-milbeaut/platsmp.c
> @@ -0,0 +1,157 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Socionext Inc.
> + * Copyright: (C) 2015 Linaro Ltd.
> + */
> +
> +#include <linux/cpu_pm.h>
> +#include <linux/irqchip/arm-gic.h>
> +#include <linux/of_device.h>
Not needed.
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
Not needed.
> +#include <linux/suspend.h>
> +
> +#include <asm/cacheflush.h>
> +#include <asm/cp15.h>
> +#include <asm/idmap.h>
> +#include <asm/smp_plat.h>
> +#include <asm/suspend.h>
> +
> +#define M10V_MAX_CPU 4
> +
> +#define KERNEL_UNBOOT_FLAG 0x12345678
> +#define CPU_FINISH_SUSPEND_FLAG 0x56784321
> +
> +static void __iomem *trampoline;
> +
> +static int m10v_boot_secondary(unsigned int l_cpu, struct task_struct *idle)
> +{
> + unsigned int mpidr, cpu, cluster;
> +
> + mpidr = cpu_logical_map(l_cpu);
> + cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> + cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> +
> + if (cpu >= M10V_MAX_CPU)
> + return -EINVAL;
> +
> + pr_debug("%s: cpu %u cluster %u\n", __func__, cpu, cluster);
> +
> + writel(virt_to_phys(secondary_startup), trampoline + cpu * 4);
> + arch_send_wakeup_ipi_mask(cpumask_of(l_cpu));
> +
> + return 0;
> +}
> +
> +static void m10v_cpu_die(unsigned int l_cpu)
> +{
> + gic_cpu_if_down(0);
> +
> + v7_exit_coherency_flush(louis);
> +
> + /* Now we are prepared for power-down, do it: */
> + wfi();
> +}
> +
> +static int m10v_cpu_kill(unsigned int l_cpu)
> +{
> + unsigned int mpidr, cpu;
> +
> + mpidr = cpu_logical_map(l_cpu);
> + cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> +
> + writel(KERNEL_UNBOOT_FLAG, trampoline + cpu * 4);
> +
> + return 1;
> +}
> +
> +static struct smp_operations m10v_smp_ops __initdata = {
> + .smp_boot_secondary = m10v_boot_secondary,
> + .cpu_die = m10v_cpu_die,
> + .cpu_kill = m10v_cpu_kill,
> +};
> +
> +static int __init m10v_smp_init(void)
> +{
> + unsigned int mpidr, cpu, cluster;
> + struct device_node *np;
> +
> + np = of_find_compatible_node(NULL, NULL, "socionext,milbeaut-m10v-evb");
> + if (!np || !of_device_is_available(np))
Just use of_machine_is_compatible() here.
> + return -ENODEV;
> + of_node_put(np);
> +
> + np = of_find_compatible_node(NULL, NULL, "socionext,smp-trampoline");
> + if (!np)
> + return -ENODEV;
> +
> + trampoline = of_iomap(np, 0);
> + if (!trampoline)
> + return -ENODEV;
> + of_node_put(np);
> +
> + mpidr = read_cpuid_mpidr();
> + cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> + cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> +
> + pr_info("MCPM boot on cpu_%u cluster_%u\n", cpu, cluster);
> +
> + for (cpu = 0; cpu < M10V_MAX_CPU; cpu++)
> + writel(KERNEL_UNBOOT_FLAG, trampoline + cpu * 4);
> +
> + smp_set_ops(&m10v_smp_ops);
> +
> + return 0;
> +}
> +early_initcall(m10v_smp_init);
> +
> +static int m10v_pm_valid(suspend_state_t state)
> +{
> + return (state == PM_SUSPEND_STANDBY) || (state == PM_SUSPEND_MEM);
> +}
> +
> +typedef void (*phys_reset_t)(unsigned long);
> +static phys_reset_t phys_reset;
> +
> +static int m10v_die(unsigned long arg)
> +{
> + setup_mm_for_reboot();
> + asm("wfi");
> + /* Boot just like a secondary */
> + phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
> + phys_reset(virt_to_phys(cpu_resume));
> +
> + return 0;
> +}
> +
> +static int m10v_pm_enter(suspend_state_t state)
> +{
> + switch (state) {
> + case PM_SUSPEND_STANDBY:
> + pr_err("STANDBY\n");
> + asm("wfi");
> + break;
> + case PM_SUSPEND_MEM:
> + pr_err("SUSPEND\n");
> + cpu_pm_enter();
> + cpu_suspend(0, m10v_die);
> + cpu_pm_exit();
> + break;
> + }
> + return 0;
> +}
> +
> +static const struct platform_suspend_ops m10v_pm_ops = {
> + .valid = m10v_pm_valid,
> + .enter = m10v_pm_enter,
> +};
> +
> +struct clk *m10v_clclk_register(struct device *cpu_dev);
> +
> +static int __init m10v_pm_init(void)
> +{
> + suspend_set_ops(&m10v_pm_ops);
> +
> + return 0;
> +}
> +late_initcall(m10v_pm_init);
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread* RE: [PATCH 01/14] ARM: milbeaut: Add basic support for Milbeaut m10v SoC
2018-11-19 16:24 ` Rob Herring
@ 2018-11-21 9:33 ` sugaya.taichi
0 siblings, 0 replies; 43+ messages in thread
From: sugaya.taichi @ 2018-11-21 9:33 UTC (permalink / raw)
To: robh+dt
Cc: linux-clk, devicetree, linux-arm-kernel, linux-kernel,
linux-serial, mturquette, sboyd, mark.rutland, gregkh,
daniel.lezcano, tglx, linux, jslaby, masami.hiramatsu,
jaswinder.singh
Hi Rob
Thank you for your comments.
> -----Original Message-----
> From: Rob Herring [mailto:robh+dt@kernel.org]
> Sent: Tuesday, November 20, 2018 1:24 AM
> To: Sugaya, Taichi
> Cc: linux-clk; devicetree@vger.kernel.org; moderated list:ARM/FREESCALE
> IMX / MXC ARM ARCHITECTURE; linux-kernel@vger.kernel.org; open list:SERIAL
> DRIVERS; Michael Turquette; Stephen Boyd; Mark Rutland; Greg Kroah-Hartman;
> Daniel Lezcano; Thomas Gleixner; Russell King; Jiri Slaby; Masami Hiramatsu;
> Jassi Brar
> Subject: Re: [PATCH 01/14] ARM: milbeaut: Add basic support for Milbeaut
> m10v SoC
>
> On Sun, Nov 18, 2018 at 7:00 PM Sugaya Taichi
> <sugaya.taichi@socionext.com> wrote:
> >
> > This adds the basic M10V SoC support under arch/arm.
> > Since all cores are activated in the custom bootloader before booting
> > linux, it is necessary to wait for sub-cores using the trampoline area.
> >
> > Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
> > ---
> > arch/arm/Kconfig | 2 +
> > arch/arm/Makefile | 1 +
> > arch/arm/mach-milbeaut/Kconfig | 28 +++++++
> > arch/arm/mach-milbeaut/Makefile | 3 +
> > arch/arm/mach-milbeaut/m10v_evb.c | 31 ++++++++
> > arch/arm/mach-milbeaut/platsmp.c | 157
> ++++++++++++++++++++++++++++++++++++++
> > 6 files changed, 222 insertions(+)
> > create mode 100644 arch/arm/mach-milbeaut/Kconfig
> > create mode 100644 arch/arm/mach-milbeaut/Makefile
> > create mode 100644 arch/arm/mach-milbeaut/m10v_evb.c
> > create mode 100644 arch/arm/mach-milbeaut/platsmp.c
> >
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index 91be74d..0b8a1af 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -767,6 +767,8 @@ source "arch/arm/mach-mediatek/Kconfig"
> >
> > source "arch/arm/mach-meson/Kconfig"
> >
> > +source "arch/arm/mach-milbeaut/Kconfig"
> > +
> > source "arch/arm/mach-mmp/Kconfig"
> >
> > source "arch/arm/mach-moxart/Kconfig"
> > diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> > index 05a91d8..627853c 100644
> > --- a/arch/arm/Makefile
> > +++ b/arch/arm/Makefile
> > @@ -190,6 +190,7 @@ machine-$(CONFIG_ARCH_MV78XX0) +=
> mv78xx0
> > machine-$(CONFIG_ARCH_MVEBU) += mvebu
> > machine-$(CONFIG_ARCH_MXC) += imx
> > machine-$(CONFIG_ARCH_MEDIATEK) += mediatek
> > +machine-$(CONFIG_ARCH_MILBEAUT) += milbeaut
> > machine-$(CONFIG_ARCH_MXS) += mxs
> > machine-$(CONFIG_ARCH_NETX) += netx
> > machine-$(CONFIG_ARCH_NOMADIK) += nomadik
> > diff --git a/arch/arm/mach-milbeaut/Kconfig
> b/arch/arm/mach-milbeaut/Kconfig
> > new file mode 100644
> > index 0000000..63b6f69
> > --- /dev/null
> > +++ b/arch/arm/mach-milbeaut/Kconfig
> > @@ -0,0 +1,28 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +menuconfig ARCH_MILBEAUT
> > + bool "Socionext Milbeaut SoCs"
> > + depends on ARCH_MULTI_V7
> > + select ARM_GIC
>
> > + select CLKDEV_LOOKUP
> > + select GENERIC_CLOCKEVENTS
> > + select CLKSRC_MMIO
>
> The clock and timer drivers' kconfig entries should select these.
OK. move these to the correct kconfig.
>
> > + select ZONE_DMA
>
> Why is this needed?
Ah, it may not be needed yet. confirm it.
>
> > + help
> > + This enables support for Socionext Milbeaut SoCs
> > +
> > +if ARCH_MILBEAUT
> > +
> > +config ARCH_MILBEAUT_M10V
> > + bool "Milbeaut SC2000/M10V platform"
> > + select ARM_ARCH_TIMER
> > + select M10V_TIMER
> > + select PINCTRL
> > + select PINCTRL_M10V
> > + help
> > + Support for Socionext's MILBEAUT M10V based systems
> > +
> > +config MACH_M10V_EVB
> > + bool "Support for Milbeaut Evaluation boards"
>
> You shouldn't need a kconfig entry for each board.
I see.
>
> > + default y
> > +
> > +endif
> > diff --git a/arch/arm/mach-milbeaut/Makefile
> b/arch/arm/mach-milbeaut/Makefile
> > new file mode 100644
> > index 0000000..64f6f52
> > --- /dev/null
> > +++ b/arch/arm/mach-milbeaut/Makefile
> > @@ -0,0 +1,3 @@
> > +obj-$(CONFIG_SMP) += platsmp.o
> > +obj-$(CONFIG_MACH_M10V_EVB) += m10v_evb.o
> > +
> > diff --git a/arch/arm/mach-milbeaut/m10v_evb.c
> b/arch/arm/mach-milbeaut/m10v_evb.c
> > new file mode 100644
> > index 0000000..a1fa7c3
> > --- /dev/null
> > +++ b/arch/arm/mach-milbeaut/m10v_evb.c
>
> This all looks SoC specific, not board specific.
Um that is right..
>
> > @@ -0,0 +1,31 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2018 Socionext Inc.
> > + * Copyright: (C) 2015 Linaro Ltd.
> > + */
> > +
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
>
> Not needed.
OK.
>
> > +
> > +#include <asm/mach/arch.h>
> > +#include <asm/mach/map.h>
> > +
> > +static struct map_desc m10v_io_desc[] __initdata = {
> > +};
> > +
> > +void __init m10v_map_io(void)
> > +{
> > + debug_ll_io_init();
>
> If you use earlycon instead, then this isn't needed.
>
> > + iotable_init(m10v_io_desc, ARRAY_SIZE(m10v_io_desc));
>
> This isn't needed.
OK.
>
> > +}
> > +
> > +static const char * const m10v_compat[] = {
> > + "socionext,milbeaut-m10v-evb",
> > + NULL,
> > +};
> > +
> > +DT_MACHINE_START(M10V_REB, "Socionext Milbeaut")
> > + .dt_compat = m10v_compat,
> > + .l2c_aux_mask = 0xffffffff,
> > + .map_io = m10v_map_io,
>
> It looks like you can remove this entire file and use the default machine
> desc.
OK, try to use the default machine descriptor instead.
>
> > +MACHINE_END
> > diff --git a/arch/arm/mach-milbeaut/platsmp.c
> b/arch/arm/mach-milbeaut/platsmp.c
> > new file mode 100644
> > index 0000000..b706851
> > --- /dev/null
> > +++ b/arch/arm/mach-milbeaut/platsmp.c
> > @@ -0,0 +1,157 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2018 Socionext Inc.
> > + * Copyright: (C) 2015 Linaro Ltd.
> > + */
> > +
> > +#include <linux/cpu_pm.h>
> > +#include <linux/irqchip/arm-gic.h>
> > +#include <linux/of_device.h>
>
> Not needed.
Okay.
>
> > +#include <linux/of_address.h>
> > +#include <linux/platform_device.h>
>
> Not needed.
Okay.
>
> > +#include <linux/suspend.h>
> > +
> > +#include <asm/cacheflush.h>
> > +#include <asm/cp15.h>
> > +#include <asm/idmap.h>
> > +#include <asm/smp_plat.h>
> > +#include <asm/suspend.h>
> > +
> > +#define M10V_MAX_CPU 4
> > +
> > +#define KERNEL_UNBOOT_FLAG 0x12345678
> > +#define CPU_FINISH_SUSPEND_FLAG 0x56784321
> > +
> > +static void __iomem *trampoline;
> > +
> > +static int m10v_boot_secondary(unsigned int l_cpu, struct task_struct
> *idle)
> > +{
> > + unsigned int mpidr, cpu, cluster;
> > +
> > + mpidr = cpu_logical_map(l_cpu);
> > + cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> > + cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> > +
> > + if (cpu >= M10V_MAX_CPU)
> > + return -EINVAL;
> > +
> > + pr_debug("%s: cpu %u cluster %u\n", __func__, cpu, cluster);
> > +
> > + writel(virt_to_phys(secondary_startup), trampoline + cpu * 4);
> > + arch_send_wakeup_ipi_mask(cpumask_of(l_cpu));
> > +
> > + return 0;
> > +}
> > +
> > +static void m10v_cpu_die(unsigned int l_cpu)
> > +{
> > + gic_cpu_if_down(0);
> > +
> > + v7_exit_coherency_flush(louis);
> > +
> > + /* Now we are prepared for power-down, do it: */
> > + wfi();
> > +}
> > +
> > +static int m10v_cpu_kill(unsigned int l_cpu)
> > +{
> > + unsigned int mpidr, cpu;
> > +
> > + mpidr = cpu_logical_map(l_cpu);
> > + cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> > +
> > + writel(KERNEL_UNBOOT_FLAG, trampoline + cpu * 4);
> > +
> > + return 1;
> > +}
> > +
> > +static struct smp_operations m10v_smp_ops __initdata = {
> > + .smp_boot_secondary = m10v_boot_secondary,
> > + .cpu_die = m10v_cpu_die,
> > + .cpu_kill = m10v_cpu_kill,
> > +};
> > +
> > +static int __init m10v_smp_init(void)
> > +{
> > + unsigned int mpidr, cpu, cluster;
> > + struct device_node *np;
> > +
> > + np = of_find_compatible_node(NULL, NULL,
> "socionext,milbeaut-m10v-evb");
> > + if (!np || !of_device_is_available(np))
>
> Just use of_machine_is_compatible() here.
I got it, use the function instead.
Thanks
Sugaya Taichi
>
> > + return -ENODEV;
> > + of_node_put(np);
> > +
> > + np = of_find_compatible_node(NULL, NULL,
> "socionext,smp-trampoline");
> > + if (!np)
> > + return -ENODEV;
> > +
> > + trampoline = of_iomap(np, 0);
> > + if (!trampoline)
> > + return -ENODEV;
> > + of_node_put(np);
> > +
> > + mpidr = read_cpuid_mpidr();
> > + cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> > + cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> > +
> > + pr_info("MCPM boot on cpu_%u cluster_%u\n", cpu, cluster);
> > +
> > + for (cpu = 0; cpu < M10V_MAX_CPU; cpu++)
> > + writel(KERNEL_UNBOOT_FLAG, trampoline + cpu * 4);
> > +
> > + smp_set_ops(&m10v_smp_ops);
> > +
> > + return 0;
> > +}
> > +early_initcall(m10v_smp_init);
> > +
> > +static int m10v_pm_valid(suspend_state_t state)
> > +{
> > + return (state == PM_SUSPEND_STANDBY) || (state ==
> PM_SUSPEND_MEM);
> > +}
> > +
> > +typedef void (*phys_reset_t)(unsigned long);
> > +static phys_reset_t phys_reset;
> > +
> > +static int m10v_die(unsigned long arg)
> > +{
> > + setup_mm_for_reboot();
> > + asm("wfi");
> > + /* Boot just like a secondary */
> > + phys_reset = (phys_reset_t)(unsigned
> long)virt_to_phys(cpu_reset);
> > + phys_reset(virt_to_phys(cpu_resume));
> > +
> > + return 0;
> > +}
> > +
> > +static int m10v_pm_enter(suspend_state_t state)
> > +{
> > + switch (state) {
> > + case PM_SUSPEND_STANDBY:
> > + pr_err("STANDBY\n");
> > + asm("wfi");
> > + break;
> > + case PM_SUSPEND_MEM:
> > + pr_err("SUSPEND\n");
> > + cpu_pm_enter();
> > + cpu_suspend(0, m10v_die);
> > + cpu_pm_exit();
> > + break;
> > + }
> > + return 0;
> > +}
> > +
> > +static const struct platform_suspend_ops m10v_pm_ops = {
> > + .valid = m10v_pm_valid,
> > + .enter = m10v_pm_enter,
> > +};
> > +
> > +struct clk *m10v_clclk_register(struct device *cpu_dev);
> > +
> > +static int __init m10v_pm_init(void)
> > +{
> > + suspend_set_ops(&m10v_pm_ops);
> > +
> > + return 0;
> > +}
> > +late_initcall(m10v_pm_init);
> > --
> > 1.9.1
> >
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 02/14] dt-bindings: soc: milbeaut: Add Milbeaut trampoline description
2018-11-19 1:01 [PATCH 00/14] Add basic support for Socionext Milbeaut M10V SoCs Sugaya Taichi
2018-11-19 1:01 ` [PATCH 01/14] ARM: milbeaut: Add basic support for Milbeaut m10v SoC Sugaya Taichi
@ 2018-11-19 1:01 ` Sugaya Taichi
2018-11-28 2:01 ` Stephen Boyd
2018-11-19 1:01 ` [PATCH 03/14] ARM: milbeaut: Add Milbeaut M10V early printk Sugaya Taichi
` (6 subsequent siblings)
8 siblings, 1 reply; 43+ messages in thread
From: Sugaya Taichi @ 2018-11-19 1:01 UTC (permalink / raw)
To: linux-clk, devicetree, linux-arm-kernel, linux-kernel, linux-serial
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Greg Kroah-Hartman, Daniel Lezcano, Thomas Gleixner,
Russell King, Jiri Slaby, Masami Hiramatsu, Jassi Brar,
Sugaya Taichi
Add DT bindings document for Milbeaut trampoline.
Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
---
.../devicetree/bindings/soc/socionext/socionext,m10v.txt | 12 ++++++++++++
1 file changed, 12 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
diff --git a/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
new file mode 100644
index 0000000..f5d906c
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
@@ -0,0 +1,12 @@
+Socionext M10V SMP trampoline driver binding
+
+This is a driver to wait for sub-cores while boot process.
+
+- compatible: should be "socionext,smp-trampoline"
+- reg: should be <0x4C000100 0x100>
+
+EXAMPLE
+ trampoline: trampoline@0x4C000100 {
+ compatible = "socionext,smp-trampoline";
+ reg = <0x4C000100 0x100>;
+ };
--
1.9.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 02/14] dt-bindings: soc: milbeaut: Add Milbeaut trampoline description
2018-11-19 1:01 ` [PATCH 02/14] dt-bindings: soc: milbeaut: Add Milbeaut trampoline description Sugaya Taichi
@ 2018-11-28 2:01 ` Stephen Boyd
2018-11-29 12:24 ` Sugaya, Taichi
0 siblings, 1 reply; 43+ messages in thread
From: Stephen Boyd @ 2018-11-28 2:01 UTC (permalink / raw)
To: Sugaya Taichi, devicetree, linux-arm-kernel, linux-clk,
linux-kernel, linux-serial
Cc: Michael Turquette, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar, Sugaya Taichi
Quoting Sugaya Taichi (2018-11-18 17:01:07)
> Add DT bindings document for Milbeaut trampoline.
>
> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
> ---
> .../devicetree/bindings/soc/socionext/socionext,m10v.txt | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
>
> diff --git a/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
> new file mode 100644
> index 0000000..f5d906c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
> @@ -0,0 +1,12 @@
> +Socionext M10V SMP trampoline driver binding
> +
> +This is a driver to wait for sub-cores while boot process.
> +
> +- compatible: should be "socionext,smp-trampoline"
> +- reg: should be <0x4C000100 0x100>
> +
> +EXAMPLE
> + trampoline: trampoline@0x4C000100 {
Drop the 0x part of unit addresses.
> + compatible = "socionext,smp-trampoline";
> + reg = <0x4C000100 0x100>;
Looks like a software construct, which we wouldn't want to put into DT
this way. DT doesn't describe drivers.
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 02/14] dt-bindings: soc: milbeaut: Add Milbeaut trampoline description
2018-11-28 2:01 ` Stephen Boyd
@ 2018-11-29 12:24 ` Sugaya, Taichi
2018-11-30 8:16 ` Stephen Boyd
0 siblings, 1 reply; 43+ messages in thread
From: Sugaya, Taichi @ 2018-11-29 12:24 UTC (permalink / raw)
To: Stephen Boyd, devicetree, linux-arm-kernel, linux-clk,
linux-kernel, linux-serial
Cc: Michael Turquette, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar
Hi,
Thank you for your comments.
On 2018/11/28 11:01, Stephen Boyd wrote:
> Quoting Sugaya Taichi (2018-11-18 17:01:07)
>> Add DT bindings document for Milbeaut trampoline.
>>
>> Signed-off-by: Sugaya Taichi<sugaya.taichi@socionext.com>
>> ---
>> .../devicetree/bindings/soc/socionext/socionext,m10v.txt | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
>>
>> diff --git a/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
>> new file mode 100644
>> index 0000000..f5d906c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
>> @@ -0,0 +1,12 @@
>> +Socionext M10V SMP trampoline driver binding
>> +
>> +This is a driver to wait for sub-cores while boot process.
>> +
>> +- compatible: should be "socionext,smp-trampoline"
>> +- reg: should be <0x4C000100 0x100>
>> +
>> +EXAMPLE
>> + trampoline: trampoline@0x4C000100 {
> Drop the 0x part of unit addresses.
Okay.
>> + compatible = "socionext,smp-trampoline";
>> + reg = <0x4C000100 0x100>;
> Looks like a software construct, which we wouldn't want to put into DT
> this way. DT doesn't describe drivers.
We would like to use this node only getting the address of the
trampoline area
in which sub-cores wait. (They have finished to go to this area in previous
bootloader process.)
So should we embed the constant value in source codes instead of getting
from
DT because the address is constant at the moment? Or is there other
approach?
Thanks
Sugaya Taichi
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 02/14] dt-bindings: soc: milbeaut: Add Milbeaut trampoline description
2018-11-29 12:24 ` Sugaya, Taichi
@ 2018-11-30 8:16 ` Stephen Boyd
2018-12-03 7:42 ` Sugaya, Taichi
0 siblings, 1 reply; 43+ messages in thread
From: Stephen Boyd @ 2018-11-30 8:16 UTC (permalink / raw)
To: Sugaya, Taichi, devicetree, linux-arm-kernel, linux-clk,
linux-kernel, linux-serial
Cc: Michael Turquette, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar
Quoting Sugaya, Taichi (2018-11-29 04:24:51)
> On 2018/11/28 11:01, Stephen Boyd wrote:
> > Quoting Sugaya Taichi (2018-11-18 17:01:07)
> >> create mode 100644 Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
> >> new file mode 100644
> >> index 0000000..f5d906c
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
> >> @@ -0,0 +1,12 @@
> >> +Socionext M10V SMP trampoline driver binding
> >> +
> >> +This is a driver to wait for sub-cores while boot process.
> >> +
> >> +- compatible: should be "socionext,smp-trampoline"
> >> +- reg: should be <0x4C000100 0x100>
> >> +
> >> +EXAMPLE
> >> + trampoline: trampoline@0x4C000100 {
> > Drop the 0x part of unit addresses.
>
> Okay.
>
>
> >> + compatible = "socionext,smp-trampoline";
> >> + reg = <0x4C000100 0x100>;
> > Looks like a software construct, which we wouldn't want to put into DT
> > this way. DT doesn't describe drivers.
> We would like to use this node only getting the address of the
> trampoline area
> in which sub-cores wait. (They have finished to go to this area in previous
> bootloader process.)
Is this area part of memory, or a special SRAM? If it's part of memory,
I would expect this node to be under the reserved-memory node and
pointed to by some other node that uses this region. Could even be the
CPU nodes.
>
> So should we embed the constant value in source codes instead of getting
> from
> DT because the address is constant at the moment? Or is there other
> approach?
>
If it's constant then that also works. Why does it need to come from DT
at all then?
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 02/14] dt-bindings: soc: milbeaut: Add Milbeaut trampoline description
2018-11-30 8:16 ` Stephen Boyd
@ 2018-12-03 7:42 ` Sugaya, Taichi
2018-12-03 15:49 ` Rob Herring
0 siblings, 1 reply; 43+ messages in thread
From: Sugaya, Taichi @ 2018-12-03 7:42 UTC (permalink / raw)
To: Stephen Boyd, devicetree, linux-arm-kernel, linux-clk,
linux-kernel, linux-serial
Cc: Michael Turquette, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar
Hi,
On 2018/11/30 17:16, Stephen Boyd wrote:
> Quoting Sugaya, Taichi (2018-11-29 04:24:51)
>> On 2018/11/28 11:01, Stephen Boyd wrote:
>>> Quoting Sugaya Taichi (2018-11-18 17:01:07)
>>>> create mode 100644 Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
>>>> new file mode 100644
>>>> index 0000000..f5d906c
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
>>>> @@ -0,0 +1,12 @@
>>>> +Socionext M10V SMP trampoline driver binding
>>>> +
>>>> +This is a driver to wait for sub-cores while boot process.
>>>> +
>>>> +- compatible: should be "socionext,smp-trampoline"
>>>> +- reg: should be <0x4C000100 0x100>
>>>> +
>>>> +EXAMPLE
>>>> + trampoline: trampoline@0x4C000100 {
>>> Drop the 0x part of unit addresses.
>>
>> Okay.
>>
>>
>>>> + compatible = "socionext,smp-trampoline";
>>>> + reg = <0x4C000100 0x100>;
>>> Looks like a software construct, which we wouldn't want to put into DT
>>> this way. DT doesn't describe drivers.
>> We would like to use this node only getting the address of the
>> trampoline area
>> in which sub-cores wait. (They have finished to go to this area in previous
>> bootloader process.)
>
> Is this area part of memory, or a special SRAM? If it's part of memory,
> I would expect this node to be under the reserved-memory node and
> pointed to by some other node that uses this region. Could even be the
> CPU nodes.
Yes, 0x4C000100 is a part of memory under the reserved-memory node. So
we would like to use the SRAM ( allocated 0x00000000 ) area instead.
BTW, sorry, the trampoline address of this example is simply wrong. We
were going to use a part of the SRAM from the beginning.
>
>>
>> So should we embed the constant value in source codes instead of getting
>> from
>> DT because the address is constant at the moment? Or is there other
>> approach?
>>
>
> If it's constant then that also works. Why does it need to come from DT
> at all then?
We think it is not good to embed constant value in driver codes and do
not have another way...
Are there better ways?
Thanks
Sugaya Taichi
>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 02/14] dt-bindings: soc: milbeaut: Add Milbeaut trampoline description
2018-12-03 7:42 ` Sugaya, Taichi
@ 2018-12-03 15:49 ` Rob Herring
2018-12-04 11:30 ` Sugaya, Taichi
0 siblings, 1 reply; 43+ messages in thread
From: Rob Herring @ 2018-12-03 15:49 UTC (permalink / raw)
To: sugaya.taichi
Cc: Stephen Boyd, devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-clk, linux-kernel, open list:SERIAL DRIVERS,
Michael Turquette, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar
On Mon, Dec 3, 2018 at 1:42 AM Sugaya, Taichi
<sugaya.taichi@socionext.com> wrote:
>
> Hi,
>
> On 2018/11/30 17:16, Stephen Boyd wrote:
> > Quoting Sugaya, Taichi (2018-11-29 04:24:51)
> >> On 2018/11/28 11:01, Stephen Boyd wrote:
> >>> Quoting Sugaya Taichi (2018-11-18 17:01:07)
> >>>> create mode 100644 Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
> >>>> new file mode 100644
> >>>> index 0000000..f5d906c
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
> >>>> @@ -0,0 +1,12 @@
> >>>> +Socionext M10V SMP trampoline driver binding
> >>>> +
> >>>> +This is a driver to wait for sub-cores while boot process.
> >>>> +
> >>>> +- compatible: should be "socionext,smp-trampoline"
> >>>> +- reg: should be <0x4C000100 0x100>
> >>>> +
> >>>> +EXAMPLE
> >>>> + trampoline: trampoline@0x4C000100 {
> >>> Drop the 0x part of unit addresses.
> >>
> >> Okay.
> >>
> >>
> >>>> + compatible = "socionext,smp-trampoline";
> >>>> + reg = <0x4C000100 0x100>;
> >>> Looks like a software construct, which we wouldn't want to put into DT
> >>> this way. DT doesn't describe drivers.
> >> We would like to use this node only getting the address of the
> >> trampoline area
> >> in which sub-cores wait. (They have finished to go to this area in previous
> >> bootloader process.)
> >
> > Is this area part of memory, or a special SRAM? If it's part of memory,
> > I would expect this node to be under the reserved-memory node and
> > pointed to by some other node that uses this region. Could even be the
> > CPU nodes.
>
> Yes, 0x4C000100 is a part of memory under the reserved-memory node. So
> we would like to use the SRAM ( allocated 0x00000000 ) area instead.
> BTW, sorry, the trampoline address of this example is simply wrong. We
> were going to use a part of the SRAM from the beginning.
>
> >
> >>
> >> So should we embed the constant value in source codes instead of getting
> >> from
> >> DT because the address is constant at the moment? Or is there other
> >> approach?
> >>
> >
> > If it's constant then that also works. Why does it need to come from DT
> > at all then?
>
> We think it is not good to embed constant value in driver codes and do
> not have another way...
> Are there better ways?
If this is just memory, can you use the standard spin-table binding in
the DT spec? There are some requirements like 64-bit values even on
32-bit machines (though this gets violated).
Rob
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 02/14] dt-bindings: soc: milbeaut: Add Milbeaut trampoline description
2018-12-03 15:49 ` Rob Herring
@ 2018-12-04 11:30 ` Sugaya, Taichi
2018-12-04 13:32 ` Rob Herring
0 siblings, 1 reply; 43+ messages in thread
From: Sugaya, Taichi @ 2018-12-04 11:30 UTC (permalink / raw)
To: Rob Herring
Cc: Stephen Boyd, devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-clk, linux-kernel, open list:SERIAL DRIVERS,
Michael Turquette, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar
Hi
On 2018/12/04 0:49, Rob Herring wrote:
> On Mon, Dec 3, 2018 at 1:42 AM Sugaya, Taichi
> <sugaya.taichi@socionext.com> wrote:
>>
>> Hi,
>>
>> On 2018/11/30 17:16, Stephen Boyd wrote:
>>> Quoting Sugaya, Taichi (2018-11-29 04:24:51)
>>>> On 2018/11/28 11:01, Stephen Boyd wrote:
>>>>> Quoting Sugaya Taichi (2018-11-18 17:01:07)
>>>>>> create mode 100644 Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
>>>>>> new file mode 100644
>>>>>> index 0000000..f5d906c
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
>>>>>> @@ -0,0 +1,12 @@
>>>>>> +Socionext M10V SMP trampoline driver binding
>>>>>> +
>>>>>> +This is a driver to wait for sub-cores while boot process.
>>>>>> +
>>>>>> +- compatible: should be "socionext,smp-trampoline"
>>>>>> +- reg: should be <0x4C000100 0x100>
>>>>>> +
>>>>>> +EXAMPLE
>>>>>> + trampoline: trampoline@0x4C000100 {
>>>>> Drop the 0x part of unit addresses.
>>>>
>>>> Okay.
>>>>
>>>>
>>>>>> + compatible = "socionext,smp-trampoline";
>>>>>> + reg = <0x4C000100 0x100>;
>>>>> Looks like a software construct, which we wouldn't want to put into DT
>>>>> this way. DT doesn't describe drivers.
>>>> We would like to use this node only getting the address of the
>>>> trampoline area
>>>> in which sub-cores wait. (They have finished to go to this area in previous
>>>> bootloader process.)
>>>
>>> Is this area part of memory, or a special SRAM? If it's part of memory,
>>> I would expect this node to be under the reserved-memory node and
>>> pointed to by some other node that uses this region. Could even be the
>>> CPU nodes.
>>
>> Yes, 0x4C000100 is a part of memory under the reserved-memory node. So
>> we would like to use the SRAM ( allocated 0x00000000 ) area instead.
>> BTW, sorry, the trampoline address of this example is simply wrong. We
>> were going to use a part of the SRAM from the beginning.
>>
>>>
>>>>
>>>> So should we embed the constant value in source codes instead of getting
>>>> from
>>>> DT because the address is constant at the moment? Or is there other
>>>> approach?
>>>>
>>>
>>> If it's constant then that also works. Why does it need to come from DT
>>> at all then?
>>
>> We think it is not good to embed constant value in driver codes and do
>> not have another way...
>> Are there better ways?
>
> If this is just memory, can you use the standard spin-table binding in
> the DT spec? There are some requirements like 64-bit values even on
> 32-bit machines (though this gets violated).
The spin-table seems to be used on only 64-bit arch. Have it ever worked
on 32-bit machine?
And I would like not to use it because avoid violation.
Thanks
Sugaya Taichi
>
> Rob
>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 02/14] dt-bindings: soc: milbeaut: Add Milbeaut trampoline description
2018-12-04 11:30 ` Sugaya, Taichi
@ 2018-12-04 13:32 ` Rob Herring
2018-12-05 10:30 ` Sugaya, Taichi
2019-01-22 11:36 ` Sugaya, Taichi
0 siblings, 2 replies; 43+ messages in thread
From: Rob Herring @ 2018-12-04 13:32 UTC (permalink / raw)
To: sugaya.taichi
Cc: Stephen Boyd, devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-clk, linux-kernel, open list:SERIAL DRIVERS,
Michael Turquette, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar
On Tue, Dec 4, 2018 at 5:30 AM Sugaya, Taichi
<sugaya.taichi@socionext.com> wrote:
>
> Hi
>
> On 2018/12/04 0:49, Rob Herring wrote:
> > On Mon, Dec 3, 2018 at 1:42 AM Sugaya, Taichi
> > <sugaya.taichi@socionext.com> wrote:
> >>
> >> Hi,
> >>
> >> On 2018/11/30 17:16, Stephen Boyd wrote:
> >>> Quoting Sugaya, Taichi (2018-11-29 04:24:51)
> >>>> On 2018/11/28 11:01, Stephen Boyd wrote:
> >>>>> Quoting Sugaya Taichi (2018-11-18 17:01:07)
> >>>>>> create mode 100644 Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
> >>>>>>
> >>>>>> diff --git a/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
> >>>>>> new file mode 100644
> >>>>>> index 0000000..f5d906c
> >>>>>> --- /dev/null
> >>>>>> +++ b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
> >>>>>> @@ -0,0 +1,12 @@
> >>>>>> +Socionext M10V SMP trampoline driver binding
> >>>>>> +
> >>>>>> +This is a driver to wait for sub-cores while boot process.
> >>>>>> +
> >>>>>> +- compatible: should be "socionext,smp-trampoline"
> >>>>>> +- reg: should be <0x4C000100 0x100>
> >>>>>> +
> >>>>>> +EXAMPLE
> >>>>>> + trampoline: trampoline@0x4C000100 {
> >>>>> Drop the 0x part of unit addresses.
> >>>>
> >>>> Okay.
> >>>>
> >>>>
> >>>>>> + compatible = "socionext,smp-trampoline";
> >>>>>> + reg = <0x4C000100 0x100>;
> >>>>> Looks like a software construct, which we wouldn't want to put into DT
> >>>>> this way. DT doesn't describe drivers.
> >>>> We would like to use this node only getting the address of the
> >>>> trampoline area
> >>>> in which sub-cores wait. (They have finished to go to this area in previous
> >>>> bootloader process.)
> >>>
> >>> Is this area part of memory, or a special SRAM? If it's part of memory,
> >>> I would expect this node to be under the reserved-memory node and
> >>> pointed to by some other node that uses this region. Could even be the
> >>> CPU nodes.
> >>
> >> Yes, 0x4C000100 is a part of memory under the reserved-memory node. So
> >> we would like to use the SRAM ( allocated 0x00000000 ) area instead.
> >> BTW, sorry, the trampoline address of this example is simply wrong. We
> >> were going to use a part of the SRAM from the beginning.
> >>
> >>>
> >>>>
> >>>> So should we embed the constant value in source codes instead of getting
> >>>> from
> >>>> DT because the address is constant at the moment? Or is there other
> >>>> approach?
> >>>>
> >>>
> >>> If it's constant then that also works. Why does it need to come from DT
> >>> at all then?
> >>
> >> We think it is not good to embed constant value in driver codes and do
> >> not have another way...
> >> Are there better ways?
> >
> > If this is just memory, can you use the standard spin-table binding in
> > the DT spec? There are some requirements like 64-bit values even on
> > 32-bit machines (though this gets violated).
>
> The spin-table seems to be used on only 64-bit arch. Have it ever worked
> on 32-bit machine?
Yes.
> And I would like not to use it because avoid violation.
The issue now that I remember is cpu-release-addr is defined to always
be a 64-bit value while some platforms made it a 32-bit value.
'cpu-release-addr' is also used for some other enable-methods.
Rob
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 02/14] dt-bindings: soc: milbeaut: Add Milbeaut trampoline description
2018-12-04 13:32 ` Rob Herring
@ 2018-12-05 10:30 ` Sugaya, Taichi
2019-01-22 11:36 ` Sugaya, Taichi
1 sibling, 0 replies; 43+ messages in thread
From: Sugaya, Taichi @ 2018-12-05 10:30 UTC (permalink / raw)
To: Rob Herring
Cc: Stephen Boyd, devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-clk, linux-kernel, open list:SERIAL DRIVERS,
Michael Turquette, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar
Hi,
On 2018/12/04 22:32, Rob Herring wrote:
> On Tue, Dec 4, 2018 at 5:30 AM Sugaya, Taichi
> <sugaya.taichi@socionext.com> wrote:
>>
>> Hi
>>
>> On 2018/12/04 0:49, Rob Herring wrote:
>>> On Mon, Dec 3, 2018 at 1:42 AM Sugaya, Taichi
>>> <sugaya.taichi@socionext.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 2018/11/30 17:16, Stephen Boyd wrote:
>>>>> Quoting Sugaya, Taichi (2018-11-29 04:24:51)
>>>>>> On 2018/11/28 11:01, Stephen Boyd wrote:
>>>>>>> Quoting Sugaya Taichi (2018-11-18 17:01:07)
>>>>>>>> create mode 100644 Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
>>>>>>>>
>>>>>>>> diff --git a/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
>>>>>>>> new file mode 100644
>>>>>>>> index 0000000..f5d906c
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
>>>>>>>> @@ -0,0 +1,12 @@
>>>>>>>> +Socionext M10V SMP trampoline driver binding
>>>>>>>> +
>>>>>>>> +This is a driver to wait for sub-cores while boot process.
>>>>>>>> +
>>>>>>>> +- compatible: should be "socionext,smp-trampoline"
>>>>>>>> +- reg: should be <0x4C000100 0x100>
>>>>>>>> +
>>>>>>>> +EXAMPLE
>>>>>>>> + trampoline: trampoline@0x4C000100 {
>>>>>>> Drop the 0x part of unit addresses.
>>>>>>
>>>>>> Okay.
>>>>>>
>>>>>>
>>>>>>>> + compatible = "socionext,smp-trampoline";
>>>>>>>> + reg = <0x4C000100 0x100>;
>>>>>>> Looks like a software construct, which we wouldn't want to put into DT
>>>>>>> this way. DT doesn't describe drivers.
>>>>>> We would like to use this node only getting the address of the
>>>>>> trampoline area
>>>>>> in which sub-cores wait. (They have finished to go to this area in previous
>>>>>> bootloader process.)
>>>>>
>>>>> Is this area part of memory, or a special SRAM? If it's part of memory,
>>>>> I would expect this node to be under the reserved-memory node and
>>>>> pointed to by some other node that uses this region. Could even be the
>>>>> CPU nodes.
>>>>
>>>> Yes, 0x4C000100 is a part of memory under the reserved-memory node. So
>>>> we would like to use the SRAM ( allocated 0x00000000 ) area instead.
>>>> BTW, sorry, the trampoline address of this example is simply wrong. We
>>>> were going to use a part of the SRAM from the beginning.
>>>>
>>>>>
>>>>>>
>>>>>> So should we embed the constant value in source codes instead of getting
>>>>>> from
>>>>>> DT because the address is constant at the moment? Or is there other
>>>>>> approach?
>>>>>>
>>>>>
>>>>> If it's constant then that also works. Why does it need to come from DT
>>>>> at all then?
>>>>
>>>> We think it is not good to embed constant value in driver codes and do
>>>> not have another way...
>>>> Are there better ways?
>>>
>>> If this is just memory, can you use the standard spin-table binding in
>>> the DT spec? There are some requirements like 64-bit values even on
>>> 32-bit machines (though this gets violated).
>>
>> The spin-table seems to be used on only 64-bit arch. Have it ever worked
>> on 32-bit machine?
>
> Yes.
>
>> And I would like not to use it because avoid violation.
>
> The issue now that I remember is cpu-release-addr is defined to always
> be a 64-bit value while some platforms made it a 32-bit value.
> 'cpu-release-addr' is also used for some other enable-methods.
Thanks.
OK, try to use the spin-table.
Best Regards,
Sugaya Taichi
>
> Rob
>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 02/14] dt-bindings: soc: milbeaut: Add Milbeaut trampoline description
2018-12-04 13:32 ` Rob Herring
2018-12-05 10:30 ` Sugaya, Taichi
@ 2019-01-22 11:36 ` Sugaya, Taichi
2019-01-22 11:50 ` Russell King - ARM Linux admin
1 sibling, 1 reply; 43+ messages in thread
From: Sugaya, Taichi @ 2019-01-22 11:36 UTC (permalink / raw)
To: Rob Herring
Cc: Stephen Boyd, devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-clk, linux-kernel, open list:SERIAL DRIVERS,
Michael Turquette, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar
Hi
On 2018/12/04 22:32, Rob Herring wrote:
> On Tue, Dec 4, 2018 at 5:30 AM Sugaya, Taichi
> <sugaya.taichi@socionext.com> wrote:
>>
>> Hi
>>
>> On 2018/12/04 0:49, Rob Herring wrote:
>>> On Mon, Dec 3, 2018 at 1:42 AM Sugaya, Taichi
>>> <sugaya.taichi@socionext.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 2018/11/30 17:16, Stephen Boyd wrote:
>>>>> Quoting Sugaya, Taichi (2018-11-29 04:24:51)
>>>>>> On 2018/11/28 11:01, Stephen Boyd wrote:
>>>>>>> Quoting Sugaya Taichi (2018-11-18 17:01:07)
>>>>>>>> create mode 100644 Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
>>>>>>>>
>>>>>>>> diff --git a/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
>>>>>>>> new file mode 100644
>>>>>>>> index 0000000..f5d906c
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
>>>>>>>> @@ -0,0 +1,12 @@
>>>>>>>> +Socionext M10V SMP trampoline driver binding
>>>>>>>> +
>>>>>>>> +This is a driver to wait for sub-cores while boot process.
>>>>>>>> +
>>>>>>>> +- compatible: should be "socionext,smp-trampoline"
>>>>>>>> +- reg: should be <0x4C000100 0x100>
>>>>>>>> +
>>>>>>>> +EXAMPLE
>>>>>>>> + trampoline: trampoline@0x4C000100 {
>>>>>>> Drop the 0x part of unit addresses.
>>>>>>
>>>>>> Okay.
>>>>>>
>>>>>>
>>>>>>>> + compatible = "socionext,smp-trampoline";
>>>>>>>> + reg = <0x4C000100 0x100>;
>>>>>>> Looks like a software construct, which we wouldn't want to put into DT
>>>>>>> this way. DT doesn't describe drivers.
>>>>>> We would like to use this node only getting the address of the
>>>>>> trampoline area
>>>>>> in which sub-cores wait. (They have finished to go to this area in previous
>>>>>> bootloader process.)
>>>>>
>>>>> Is this area part of memory, or a special SRAM? If it's part of memory,
>>>>> I would expect this node to be under the reserved-memory node and
>>>>> pointed to by some other node that uses this region. Could even be the
>>>>> CPU nodes.
>>>>
>>>> Yes, 0x4C000100 is a part of memory under the reserved-memory node. So
>>>> we would like to use the SRAM ( allocated 0x00000000 ) area instead.
>>>> BTW, sorry, the trampoline address of this example is simply wrong. We
>>>> were going to use a part of the SRAM from the beginning.
>>>>
>>>>>
>>>>>>
>>>>>> So should we embed the constant value in source codes instead of getting
>>>>>> from
>>>>>> DT because the address is constant at the moment? Or is there other
>>>>>> approach?
>>>>>>
>>>>>
>>>>> If it's constant then that also works. Why does it need to come from DT
>>>>> at all then?
>>>>
>>>> We think it is not good to embed constant value in driver codes and do
>>>> not have another way...
>>>> Are there better ways?
>>>
>>> If this is just memory, can you use the standard spin-table binding in
>>> the DT spec? There are some requirements like 64-bit values even on
>>> 32-bit machines (though this gets violated).
>>
>> The spin-table seems to be used on only 64-bit arch. Have it ever worked
>> on 32-bit machine?
>
> Yes.
>
>> And I would like not to use it because avoid violation.
>
> The issue now that I remember is cpu-release-addr is defined to always
> be a 64-bit value while some platforms made it a 32-bit value.
> 'cpu-release-addr' is also used for some other enable-methods.
I have a question about the spin-table.
The code "smp_spin_table.c" is only in "arch/arm64/kernel" directory so
can not be compiled in arm-v7 arch. That means the spin-table can not be
used arm-v7 arch..? , or is there the way to compile the code in arm-v7
arch?
Thanks,
Sugaya Taichi
>
> Rob
>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 02/14] dt-bindings: soc: milbeaut: Add Milbeaut trampoline description
2019-01-22 11:36 ` Sugaya, Taichi
@ 2019-01-22 11:50 ` Russell King - ARM Linux admin
2019-01-29 8:28 ` Sugaya, Taichi
0 siblings, 1 reply; 43+ messages in thread
From: Russell King - ARM Linux admin @ 2019-01-22 11:50 UTC (permalink / raw)
To: Sugaya, Taichi
Cc: Rob Herring, Stephen Boyd, devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-clk, linux-kernel, open list:SERIAL DRIVERS,
Michael Turquette, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Jiri Slaby, Masami Hiramatsu,
Jassi Brar
On Tue, Jan 22, 2019 at 08:36:03PM +0900, Sugaya, Taichi wrote:
> Hi
>
> On 2018/12/04 22:32, Rob Herring wrote:
> > On Tue, Dec 4, 2018 at 5:30 AM Sugaya, Taichi
> > <sugaya.taichi@socionext.com> wrote:
> > >
> > > Hi
> > >
> > > On 2018/12/04 0:49, Rob Herring wrote:
> > > > On Mon, Dec 3, 2018 at 1:42 AM Sugaya, Taichi
> > > > <sugaya.taichi@socionext.com> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > On 2018/11/30 17:16, Stephen Boyd wrote:
> > > > > > Quoting Sugaya, Taichi (2018-11-29 04:24:51)
> > > > > > > On 2018/11/28 11:01, Stephen Boyd wrote:
> > > > > > > > Quoting Sugaya Taichi (2018-11-18 17:01:07)
> > > > > > > > > create mode 100644 Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
> > > > > > > > >
> > > > > > > > > diff --git a/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
> > > > > > > > > new file mode 100644
> > > > > > > > > index 0000000..f5d906c
> > > > > > > > > --- /dev/null
> > > > > > > > > +++ b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
> > > > > > > > > @@ -0,0 +1,12 @@
> > > > > > > > > +Socionext M10V SMP trampoline driver binding
> > > > > > > > > +
> > > > > > > > > +This is a driver to wait for sub-cores while boot process.
> > > > > > > > > +
> > > > > > > > > +- compatible: should be "socionext,smp-trampoline"
> > > > > > > > > +- reg: should be <0x4C000100 0x100>
> > > > > > > > > +
> > > > > > > > > +EXAMPLE
> > > > > > > > > + trampoline: trampoline@0x4C000100 {
> > > > > > > > Drop the 0x part of unit addresses.
> > > > > > >
> > > > > > > Okay.
> > > > > > >
> > > > > > >
> > > > > > > > > + compatible = "socionext,smp-trampoline";
> > > > > > > > > + reg = <0x4C000100 0x100>;
> > > > > > > > Looks like a software construct, which we wouldn't want to put into DT
> > > > > > > > this way. DT doesn't describe drivers.
> > > > > > > We would like to use this node only getting the address of the
> > > > > > > trampoline area
> > > > > > > in which sub-cores wait. (They have finished to go to this area in previous
> > > > > > > bootloader process.)
> > > > > >
> > > > > > Is this area part of memory, or a special SRAM? If it's part of memory,
> > > > > > I would expect this node to be under the reserved-memory node and
> > > > > > pointed to by some other node that uses this region. Could even be the
> > > > > > CPU nodes.
> > > > >
> > > > > Yes, 0x4C000100 is a part of memory under the reserved-memory node. So
> > > > > we would like to use the SRAM ( allocated 0x00000000 ) area instead.
> > > > > BTW, sorry, the trampoline address of this example is simply wrong. We
> > > > > were going to use a part of the SRAM from the beginning.
> > > > >
> > > > > >
> > > > > > >
> > > > > > > So should we embed the constant value in source codes instead of getting
> > > > > > > from
> > > > > > > DT because the address is constant at the moment? Or is there other
> > > > > > > approach?
> > > > > > >
> > > > > >
> > > > > > If it's constant then that also works. Why does it need to come from DT
> > > > > > at all then?
> > > > >
> > > > > We think it is not good to embed constant value in driver codes and do
> > > > > not have another way...
> > > > > Are there better ways?
> > > >
> > > > If this is just memory, can you use the standard spin-table binding in
> > > > the DT spec? There are some requirements like 64-bit values even on
> > > > 32-bit machines (though this gets violated).
> > >
> > > The spin-table seems to be used on only 64-bit arch. Have it ever worked
> > > on 32-bit machine?
> >
> > Yes.
> >
> > > And I would like not to use it because avoid violation.
> >
> > The issue now that I remember is cpu-release-addr is defined to always
> > be a 64-bit value while some platforms made it a 32-bit value.
> > 'cpu-release-addr' is also used for some other enable-methods.
>
> I have a question about the spin-table.
> The code "smp_spin_table.c" is only in "arch/arm64/kernel" directory so can
> not be compiled in arm-v7 arch. That means the spin-table can not be used
> arm-v7 arch..? , or is there the way to compile the code in arm-v7 arch?
Why do you think you need it? Do you have no way to control individual
CPUs?
We are going through a process in 32-bit eliminating the "spin table"
stuff from platforms.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 02/14] dt-bindings: soc: milbeaut: Add Milbeaut trampoline description
2019-01-22 11:50 ` Russell King - ARM Linux admin
@ 2019-01-29 8:28 ` Sugaya, Taichi
2019-01-30 8:40 ` Russell King - ARM Linux admin
0 siblings, 1 reply; 43+ messages in thread
From: Sugaya, Taichi @ 2019-01-29 8:28 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Rob Herring, Stephen Boyd, devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-clk, linux-kernel, open list:SERIAL DRIVERS,
Michael Turquette, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Jiri Slaby, Masami Hiramatsu,
Jassi Brar
Hi,
On 2019/01/22 20:50, Russell King - ARM Linux admin wrote:
> On Tue, Jan 22, 2019 at 08:36:03PM +0900, Sugaya, Taichi wrote:
>> Hi
>>
>> On 2018/12/04 22:32, Rob Herring wrote:
>>> On Tue, Dec 4, 2018 at 5:30 AM Sugaya, Taichi
>>> <sugaya.taichi@socionext.com> wrote:
>>>>
>>>> Hi
>>>>
>>>> On 2018/12/04 0:49, Rob Herring wrote:
>>>>> On Mon, Dec 3, 2018 at 1:42 AM Sugaya, Taichi
>>>>> <sugaya.taichi@socionext.com> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On 2018/11/30 17:16, Stephen Boyd wrote:
>>>>>>> Quoting Sugaya, Taichi (2018-11-29 04:24:51)
>>>>>>>> On 2018/11/28 11:01, Stephen Boyd wrote:
>>>>>>>>> Quoting Sugaya Taichi (2018-11-18 17:01:07)
>>>>>>>>>> create mode 100644 Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
>>>>>>>>>>
>>>>>>>>>> diff --git a/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
>>>>>>>>>> new file mode 100644
>>>>>>>>>> index 0000000..f5d906c
>>>>>>>>>> --- /dev/null
>>>>>>>>>> +++ b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
>>>>>>>>>> @@ -0,0 +1,12 @@
>>>>>>>>>> +Socionext M10V SMP trampoline driver binding
>>>>>>>>>> +
>>>>>>>>>> +This is a driver to wait for sub-cores while boot process.
>>>>>>>>>> +
>>>>>>>>>> +- compatible: should be "socionext,smp-trampoline"
>>>>>>>>>> +- reg: should be <0x4C000100 0x100>
>>>>>>>>>> +
>>>>>>>>>> +EXAMPLE
>>>>>>>>>> + trampoline: trampoline@0x4C000100 {
>>>>>>>>> Drop the 0x part of unit addresses.
>>>>>>>>
>>>>>>>> Okay.
>>>>>>>>
>>>>>>>>
>>>>>>>>>> + compatible = "socionext,smp-trampoline";
>>>>>>>>>> + reg = <0x4C000100 0x100>;
>>>>>>>>> Looks like a software construct, which we wouldn't want to put into DT
>>>>>>>>> this way. DT doesn't describe drivers.
>>>>>>>> We would like to use this node only getting the address of the
>>>>>>>> trampoline area
>>>>>>>> in which sub-cores wait. (They have finished to go to this area in previous
>>>>>>>> bootloader process.)
>>>>>>>
>>>>>>> Is this area part of memory, or a special SRAM? If it's part of memory,
>>>>>>> I would expect this node to be under the reserved-memory node and
>>>>>>> pointed to by some other node that uses this region. Could even be the
>>>>>>> CPU nodes.
>>>>>>
>>>>>> Yes, 0x4C000100 is a part of memory under the reserved-memory node. So
>>>>>> we would like to use the SRAM ( allocated 0x00000000 ) area instead.
>>>>>> BTW, sorry, the trampoline address of this example is simply wrong. We
>>>>>> were going to use a part of the SRAM from the beginning.
>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> So should we embed the constant value in source codes instead of getting
>>>>>>>> from
>>>>>>>> DT because the address is constant at the moment? Or is there other
>>>>>>>> approach?
>>>>>>>>
>>>>>>>
>>>>>>> If it's constant then that also works. Why does it need to come from DT
>>>>>>> at all then?
>>>>>>
>>>>>> We think it is not good to embed constant value in driver codes and do
>>>>>> not have another way...
>>>>>> Are there better ways?
>>>>>
>>>>> If this is just memory, can you use the standard spin-table binding in
>>>>> the DT spec? There are some requirements like 64-bit values even on
>>>>> 32-bit machines (though this gets violated).
>>>>
>>>> The spin-table seems to be used on only 64-bit arch. Have it ever worked
>>>> on 32-bit machine?
>>>
>>> Yes.
>>>
>>>> And I would like not to use it because avoid violation.
>>>
>>> The issue now that I remember is cpu-release-addr is defined to always
>>> be a 64-bit value while some platforms made it a 32-bit value.
>>> 'cpu-release-addr' is also used for some other enable-methods.
>>
>> I have a question about the spin-table.
>> The code "smp_spin_table.c" is only in "arch/arm64/kernel" directory so can
>> not be compiled in arm-v7 arch. That means the spin-table can not be used
>> arm-v7 arch..? , or is there the way to compile the code in arm-v7 arch?
>
> Why do you think you need it? Do you have no way to control individual
> CPUs?
>
> We are going through a process in 32-bit eliminating the "spin table"
> stuff from platforms.
>
Not always necessary to us and considering the history I think it is not
suitable to use the spin-table.
I try to use another way.
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 02/14] dt-bindings: soc: milbeaut: Add Milbeaut trampoline description
2019-01-29 8:28 ` Sugaya, Taichi
@ 2019-01-30 8:40 ` Russell King - ARM Linux admin
0 siblings, 0 replies; 43+ messages in thread
From: Russell King - ARM Linux admin @ 2019-01-30 8:40 UTC (permalink / raw)
To: Sugaya, Taichi
Cc: Rob Herring, Stephen Boyd, devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-clk, linux-kernel, open list:SERIAL DRIVERS,
Michael Turquette, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Jiri Slaby, Masami Hiramatsu,
Jassi Brar
On Tue, Jan 29, 2019 at 05:28:48PM +0900, Sugaya, Taichi wrote:
> Hi,
>
> On 2019/01/22 20:50, Russell King - ARM Linux admin wrote:
> > On Tue, Jan 22, 2019 at 08:36:03PM +0900, Sugaya, Taichi wrote:
> > > Hi
> > >
> > > On 2018/12/04 22:32, Rob Herring wrote:
> > > > On Tue, Dec 4, 2018 at 5:30 AM Sugaya, Taichi
> > > > <sugaya.taichi@socionext.com> wrote:
> > > > >
> > > > > Hi
> > > > >
> > > > > On 2018/12/04 0:49, Rob Herring wrote:
> > > > > > On Mon, Dec 3, 2018 at 1:42 AM Sugaya, Taichi
> > > > > > <sugaya.taichi@socionext.com> wrote:
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > On 2018/11/30 17:16, Stephen Boyd wrote:
> > > > > > > > Quoting Sugaya, Taichi (2018-11-29 04:24:51)
> > > > > > > > > On 2018/11/28 11:01, Stephen Boyd wrote:
> > > > > > > > > > Quoting Sugaya Taichi (2018-11-18 17:01:07)
> > > > > > > > > > > create mode 100644 Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
> > > > > > > > > > >
> > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
> > > > > > > > > > > new file mode 100644
> > > > > > > > > > > index 0000000..f5d906c
> > > > > > > > > > > --- /dev/null
> > > > > > > > > > > +++ b/Documentation/devicetree/bindings/soc/socionext/socionext,m10v.txt
> > > > > > > > > > > @@ -0,0 +1,12 @@
> > > > > > > > > > > +Socionext M10V SMP trampoline driver binding
> > > > > > > > > > > +
> > > > > > > > > > > +This is a driver to wait for sub-cores while boot process.
> > > > > > > > > > > +
> > > > > > > > > > > +- compatible: should be "socionext,smp-trampoline"
> > > > > > > > > > > +- reg: should be <0x4C000100 0x100>
> > > > > > > > > > > +
> > > > > > > > > > > +EXAMPLE
> > > > > > > > > > > + trampoline: trampoline@0x4C000100 {
> > > > > > > > > > Drop the 0x part of unit addresses.
> > > > > > > > >
> > > > > > > > > Okay.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > > > + compatible = "socionext,smp-trampoline";
> > > > > > > > > > > + reg = <0x4C000100 0x100>;
> > > > > > > > > > Looks like a software construct, which we wouldn't want to put into DT
> > > > > > > > > > this way. DT doesn't describe drivers.
> > > > > > > > > We would like to use this node only getting the address of the
> > > > > > > > > trampoline area
> > > > > > > > > in which sub-cores wait. (They have finished to go to this area in previous
> > > > > > > > > bootloader process.)
> > > > > > > >
> > > > > > > > Is this area part of memory, or a special SRAM? If it's part of memory,
> > > > > > > > I would expect this node to be under the reserved-memory node and
> > > > > > > > pointed to by some other node that uses this region. Could even be the
> > > > > > > > CPU nodes.
> > > > > > >
> > > > > > > Yes, 0x4C000100 is a part of memory under the reserved-memory node. So
> > > > > > > we would like to use the SRAM ( allocated 0x00000000 ) area instead.
> > > > > > > BTW, sorry, the trampoline address of this example is simply wrong. We
> > > > > > > were going to use a part of the SRAM from the beginning.
> > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > So should we embed the constant value in source codes instead of getting
> > > > > > > > > from
> > > > > > > > > DT because the address is constant at the moment? Or is there other
> > > > > > > > > approach?
> > > > > > > > >
> > > > > > > >
> > > > > > > > If it's constant then that also works. Why does it need to come from DT
> > > > > > > > at all then?
> > > > > > >
> > > > > > > We think it is not good to embed constant value in driver codes and do
> > > > > > > not have another way...
> > > > > > > Are there better ways?
> > > > > >
> > > > > > If this is just memory, can you use the standard spin-table binding in
> > > > > > the DT spec? There are some requirements like 64-bit values even on
> > > > > > 32-bit machines (though this gets violated).
> > > > >
> > > > > The spin-table seems to be used on only 64-bit arch. Have it ever worked
> > > > > on 32-bit machine?
> > > >
> > > > Yes.
> > > >
> > > > > And I would like not to use it because avoid violation.
> > > >
> > > > The issue now that I remember is cpu-release-addr is defined to always
> > > > be a 64-bit value while some platforms made it a 32-bit value.
> > > > 'cpu-release-addr' is also used for some other enable-methods.
> > >
> > > I have a question about the spin-table.
> > > The code "smp_spin_table.c" is only in "arch/arm64/kernel" directory so can
> > > not be compiled in arm-v7 arch. That means the spin-table can not be used
> > > arm-v7 arch..? , or is there the way to compile the code in arm-v7 arch?
> >
> > Why do you think you need it? Do you have no way to control individual
> > CPUs?
> >
> > We are going through a process in 32-bit eliminating the "spin table"
> > stuff from platforms.
> >
>
> Not always necessary to us and considering the history I think it is not
> suitable to use the spin-table.
> I try to use another way.
Please do - the "spin" approach was a hack to allow ARM Ltd's platforms
to work. These platforms have no power control or reset of secondary
CPUs and no low power states (so can't suspend/resume). Early firmware
only had the capabilities to release _all_ secondary CPUs to the kernel
together. The "spin" approach is incompatible with suspend/resume,
hibernate, and kexec features of the kernel.
Real platforms do not have these problems, and thus should not use the
spin-table approach.
Modern platforms use PSCI to make the control of low power modes and
secondary CPUs independent of the kernel.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 03/14] ARM: milbeaut: Add Milbeaut M10V early printk
2018-11-19 1:01 [PATCH 00/14] Add basic support for Socionext Milbeaut M10V SoCs Sugaya Taichi
2018-11-19 1:01 ` [PATCH 01/14] ARM: milbeaut: Add basic support for Milbeaut m10v SoC Sugaya Taichi
2018-11-19 1:01 ` [PATCH 02/14] dt-bindings: soc: milbeaut: Add Milbeaut trampoline description Sugaya Taichi
@ 2018-11-19 1:01 ` Sugaya Taichi
2018-11-19 15:21 ` Rob Herring
2018-11-19 1:01 ` [PATCH 04/14] dt-bindings: timer: Add Milbeaut M10V timer description Sugaya Taichi
` (5 subsequent siblings)
8 siblings, 1 reply; 43+ messages in thread
From: Sugaya Taichi @ 2018-11-19 1:01 UTC (permalink / raw)
To: linux-clk, devicetree, linux-arm-kernel, linux-kernel, linux-serial
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Greg Kroah-Hartman, Daniel Lezcano, Thomas Gleixner,
Russell King, Jiri Slaby, Masami Hiramatsu, Jassi Brar,
Sugaya Taichi
Add Milbeaut M10V earlyprintk.
Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
---
arch/arm/Kconfig.debug | 12 ++++++++++--
arch/arm/include/debug/milbeaut.S | 25 +++++++++++++++++++++++++
2 files changed, 35 insertions(+), 2 deletions(-)
create mode 100644 arch/arm/include/debug/milbeaut.S
diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index d6a49f5..5c44533 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -1413,6 +1413,11 @@ choice
options; the platform specific options are deprecated
and will be soon removed.
+ config DEBUG_LL_MILBEAUT_UART
+ bool "Kernel low-level debug output via USIO"
+ depends on ARCH_MILBEAUT
+ help
+ Say Y here if you want to debug with USIO
endchoice
config DEBUG_AT91_UART
@@ -1534,6 +1539,7 @@ config DEBUG_LL_INCLUDE
default "debug/bcm63xx.S" if DEBUG_BCM63XX_UART
default "debug/digicolor.S" if DEBUG_DIGICOLOR_UA0
default "debug/brcmstb.S" if DEBUG_BRCMSTB_UART
+ default "debug/milbeaut.S" if DEBUG_LL_MILBEAUT_UART
default "mach/debug-macro.S"
# Compatibility options for PL01x
@@ -1580,6 +1586,7 @@ config DEBUG_UART_PHYS
default 0x18020000 if DEBUG_SIRFATLAS7_UART1
default 0x18023000 if DEBUG_BCM_IPROC_UART3
default 0x1c090000 if DEBUG_VEXPRESS_UART0_RS1
+ default 0x1e700010 if ARCH_MILBEAUT_M10V
default 0x20001000 if DEBUG_HIP01_UART
default 0x20060000 if DEBUG_RK29_UART0
default 0x20064000 if DEBUG_RK29_UART1 || DEBUG_RK3X_UART2
@@ -1681,7 +1688,7 @@ config DEBUG_UART_PHYS
DEBUG_S3C64XX_UART || \
DEBUG_BCM63XX_UART || DEBUG_ASM9260_UART || \
DEBUG_SIRFSOC_UART || DEBUG_DIGICOLOR_UA0 || \
- DEBUG_AT91_UART
+ DEBUG_AT91_UART || DEBUG_LL_MILBEAUT_UART
config DEBUG_UART_VIRT
hex "Virtual base address of debug UART"
@@ -1755,6 +1762,7 @@ config DEBUG_UART_VIRT
default 0xfe100000 if DEBUG_IMX23_UART || DEBUG_IMX28_UART
default 0xfe230000 if DEBUG_PICOXCELL_UART
default 0xfe300000 if DEBUG_BCM_KONA_UART
+ default 0xfe700010 if ARCH_MILBEAUT_M10V
default 0xfe800000 if ARCH_IOP32X
default 0xfeb00000 if DEBUG_HI3620_UART || DEBUG_HIX5HD2_UART
default 0xfeb24000 if DEBUG_RK3X_UART0
@@ -1796,7 +1804,7 @@ config DEBUG_UART_VIRT
DEBUG_S3C64XX_UART || \
DEBUG_BCM63XX_UART || DEBUG_ASM9260_UART || \
DEBUG_SIRFSOC_UART || DEBUG_DIGICOLOR_UA0 || \
- DEBUG_AT91_UART
+ DEBUG_AT91_UART || DEBUG_LL_MILBEAUT_UART
config DEBUG_UART_8250_SHIFT
int "Register offset shift for the 8250 debug UART"
diff --git a/arch/arm/include/debug/milbeaut.S b/arch/arm/include/debug/milbeaut.S
new file mode 100644
index 0000000..0660e0f
--- /dev/null
+++ b/arch/arm/include/debug/milbeaut.S
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2018 Socionext Inc.
+ */
+
+ .macro addruart, rp, rv, tmp
+ ldr \rp, =CONFIG_DEBUG_UART_PHYS
+ ldr \rv, =CONFIG_DEBUG_UART_VIRT
+ .endm
+
+ .macro senduart,rd,rx
+ strh \rd, [\rx, #0x04] @ write to TxData register
+ .endm
+
+ .macro waituart,rd,rx
+1: ldrb \rd, [\rx, #0x03] @ SSR
+ tst \rd, #1 << 1 @ check TDRE bit
+ beq 1b
+ .endm
+
+ .macro busyuart,rd,rx
+2: ldrb \rd, [\rx, #0x03] @ SSR
+ tst \rd, #1 << 1 @ check TEMT bit
+ beq 2b
+ .endm
--
1.9.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 03/14] ARM: milbeaut: Add Milbeaut M10V early printk
2018-11-19 1:01 ` [PATCH 03/14] ARM: milbeaut: Add Milbeaut M10V early printk Sugaya Taichi
@ 2018-11-19 15:21 ` Rob Herring
2018-11-21 4:07 ` sugaya.taichi
0 siblings, 1 reply; 43+ messages in thread
From: Rob Herring @ 2018-11-19 15:21 UTC (permalink / raw)
To: sugaya.taichi
Cc: linux-clk, devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-kernel, open list:SERIAL DRIVERS, Michael Turquette,
Stephen Boyd, Mark Rutland, Greg Kroah-Hartman, Daniel Lezcano,
Thomas Gleixner, Russell King, Jiri Slaby, Masami Hiramatsu,
Jassi Brar
On Sun, Nov 18, 2018 at 7:00 PM Sugaya Taichi
<sugaya.taichi@socionext.com> wrote:
>
> Add Milbeaut M10V earlyprintk.
Do you really need earlyprintk? earlycon is preferred instead. It's
not quite as early, but doesn't require rebuilding the kernel and
supports more than 1 fixed device. If you crash before earlycon is
enabled, that pretty much means either a bad dtb memory node or just
flaky memory.
Rob
^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH 03/14] ARM: milbeaut: Add Milbeaut M10V early printk
2018-11-19 15:21 ` Rob Herring
@ 2018-11-21 4:07 ` sugaya.taichi
0 siblings, 0 replies; 43+ messages in thread
From: sugaya.taichi @ 2018-11-21 4:07 UTC (permalink / raw)
To: robh+dt
Cc: linux-clk, devicetree, linux-arm-kernel, linux-kernel,
linux-serial, mturquette, sboyd, mark.rutland, gregkh,
daniel.lezcano, tglx, linux, jslaby, masami.hiramatsu,
jaswinder.singh
Hi Rob
Thank you for your comment.
> -----Original Message-----
> From: Rob Herring [mailto:robh+dt@kernel.org]
> Sent: Tuesday, November 20, 2018 12:21 AM
> To: Sugaya, Taichi/菅谷 太一
> Cc: linux-clk; devicetree@vger.kernel.org; moderated list:ARM/FREESCALE
> IMX / MXC ARM ARCHITECTURE; linux-kernel@vger.kernel.org; open list:SERIAL
> DRIVERS; Michael Turquette; Stephen Boyd; Mark Rutland; Greg Kroah-Hartman;
> Daniel Lezcano; Thomas Gleixner; Russell King; Jiri Slaby; Masami Hiramatsu;
> Jassi Brar
> Subject: Re: [PATCH 03/14] ARM: milbeaut: Add Milbeaut M10V early printk
>
> On Sun, Nov 18, 2018 at 7:00 PM Sugaya Taichi
> <sugaya.taichi@socionext.com> wrote:
> >
> > Add Milbeaut M10V earlyprintk.
>
> Do you really need earlyprintk? earlycon is preferred instead. It's
> not quite as early, but doesn't require rebuilding the kernel and
> supports more than 1 fixed device. If you crash before earlycon is
> enabled, that pretty much means either a bad dtb memory node or just
> flaky memory.
OK. I use earlycon since would like to use flexible functions.
Thanks
Sugaya Taichi
>
> Rob
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 04/14] dt-bindings: timer: Add Milbeaut M10V timer description
2018-11-19 1:01 [PATCH 00/14] Add basic support for Socionext Milbeaut M10V SoCs Sugaya Taichi
` (2 preceding siblings ...)
2018-11-19 1:01 ` [PATCH 03/14] ARM: milbeaut: Add Milbeaut M10V early printk Sugaya Taichi
@ 2018-11-19 1:01 ` Sugaya Taichi
2018-12-04 23:03 ` Rob Herring
2018-11-19 1:01 ` [PATCH 05/14] clocksource/drivers/timer-milbeaut: Add Milbeaut M10V timer Sugaya Taichi
` (4 subsequent siblings)
8 siblings, 1 reply; 43+ messages in thread
From: Sugaya Taichi @ 2018-11-19 1:01 UTC (permalink / raw)
To: linux-clk, devicetree, linux-arm-kernel, linux-kernel, linux-serial
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Greg Kroah-Hartman, Daniel Lezcano, Thomas Gleixner,
Russell King, Jiri Slaby, Masami Hiramatsu, Jassi Brar,
Sugaya Taichi
Add DT bindings document for Milbeaut M10V timer.
Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
---
.../bindings/timer/socionext,milbeaut-timer.txt | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
create mode 100644 Documentation/devicetree/bindings/timer/socionext,milbeaut-timer.txt
diff --git a/Documentation/devicetree/bindings/timer/socionext,milbeaut-timer.txt b/Documentation/devicetree/bindings/timer/socionext,milbeaut-timer.txt
new file mode 100644
index 0000000..ddb1b31
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/socionext,milbeaut-timer.txt
@@ -0,0 +1,17 @@
+Milbeaut SoCs Timer Controller
+
+Required properties:
+
+- compatible : should be "socionext,milbeaut-m10v-timer"
+- reg : Specifies base physical address and size of the registers.
+- interrupts : The interrupt of the first timer
+- clocks: should be "rclk"
+
+Example:
+
+timer {
+ compatible = "socionext,milbeaut-m10v-timer";
+ reg = <0x1e000050 0x10>, <0x1e000060 0x10>;
+ interrupts = <0 91 4>;
+ clocks = <&rclk>;
+};
--
1.9.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 04/14] dt-bindings: timer: Add Milbeaut M10V timer description
2018-11-19 1:01 ` [PATCH 04/14] dt-bindings: timer: Add Milbeaut M10V timer description Sugaya Taichi
@ 2018-12-04 23:03 ` Rob Herring
2018-12-06 7:42 ` Sugaya, Taichi
0 siblings, 1 reply; 43+ messages in thread
From: Rob Herring @ 2018-12-04 23:03 UTC (permalink / raw)
To: Sugaya Taichi
Cc: linux-clk, devicetree, linux-arm-kernel, linux-kernel,
linux-serial, Michael Turquette, Stephen Boyd, Mark Rutland,
Greg Kroah-Hartman, Daniel Lezcano, Thomas Gleixner,
Russell King, Jiri Slaby, Masami Hiramatsu, Jassi Brar
On Mon, Nov 19, 2018 at 10:01:09AM +0900, Sugaya Taichi wrote:
> Add DT bindings document for Milbeaut M10V timer.
>
> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
> ---
> .../bindings/timer/socionext,milbeaut-timer.txt | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/timer/socionext,milbeaut-timer.txt
>
> diff --git a/Documentation/devicetree/bindings/timer/socionext,milbeaut-timer.txt b/Documentation/devicetree/bindings/timer/socionext,milbeaut-timer.txt
> new file mode 100644
> index 0000000..ddb1b31
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/socionext,milbeaut-timer.txt
> @@ -0,0 +1,17 @@
> +Milbeaut SoCs Timer Controller
> +
> +Required properties:
> +
> +- compatible : should be "socionext,milbeaut-m10v-timer"
> +- reg : Specifies base physical address and size of the registers.
How many register ranges? Looks like 2.
> +- interrupts : The interrupt of the first timer
> +- clocks: should be "rclk"
> +
> +Example:
> +
> +timer {
timer@1e000050
> + compatible = "socionext,milbeaut-m10v-timer";
> + reg = <0x1e000050 0x10>, <0x1e000060 0x10>;
> + interrupts = <0 91 4>;
> + clocks = <&rclk>;
> +};
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 04/14] dt-bindings: timer: Add Milbeaut M10V timer description
2018-12-04 23:03 ` Rob Herring
@ 2018-12-06 7:42 ` Sugaya, Taichi
0 siblings, 0 replies; 43+ messages in thread
From: Sugaya, Taichi @ 2018-12-06 7:42 UTC (permalink / raw)
To: Rob Herring
Cc: linux-clk, devicetree, linux-arm-kernel, linux-kernel,
linux-serial, Michael Turquette, Stephen Boyd, Mark Rutland,
Greg Kroah-Hartman, Daniel Lezcano, Thomas Gleixner,
Russell King, Jiri Slaby, Masami Hiramatsu, Jassi Brar
Hi,
Thank you for your comments.
On 2018/12/05 8:03, Rob Herring wrote:
> On Mon, Nov 19, 2018 at 10:01:09AM +0900, Sugaya Taichi wrote:
>> Add DT bindings document for Milbeaut M10V timer.
>>
>> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
>> ---
>> .../bindings/timer/socionext,milbeaut-timer.txt | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/timer/socionext,milbeaut-timer.txt
>>
>> diff --git a/Documentation/devicetree/bindings/timer/socionext,milbeaut-timer.txt b/Documentation/devicetree/bindings/timer/socionext,milbeaut-timer.txt
>> new file mode 100644
>> index 0000000..ddb1b31
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/timer/socionext,milbeaut-timer.txt
>> @@ -0,0 +1,17 @@
>> +Milbeaut SoCs Timer Controller
>> +
>> +Required properties:
>> +
>> +- compatible : should be "socionext,milbeaut-m10v-timer"
>> +- reg : Specifies base physical address and size of the registers.
>
> How many register ranges? Looks like 2.
Yes, has two ranges.
So add the description about it.
>
>> +- interrupts : The interrupt of the first timer
>> +- clocks: should be "rclk"
>> +
>> +Example:
>> +
>> +timer {
>
> timer@1e000050
Okay.
Thanks
Sugaya Taichi
>
>> + compatible = "socionext,milbeaut-m10v-timer";
>> + reg = <0x1e000050 0x10>, <0x1e000060 0x10>;
>> + interrupts = <0 91 4>;
>> + clocks = <&rclk>;
>> +};
>> --
>> 1.9.1
>>
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 05/14] clocksource/drivers/timer-milbeaut: Add Milbeaut M10V timer
2018-11-19 1:01 [PATCH 00/14] Add basic support for Socionext Milbeaut M10V SoCs Sugaya Taichi
` (3 preceding siblings ...)
2018-11-19 1:01 ` [PATCH 04/14] dt-bindings: timer: Add Milbeaut M10V timer description Sugaya Taichi
@ 2018-11-19 1:01 ` Sugaya Taichi
2018-11-21 10:08 ` Daniel Lezcano
2018-11-19 1:01 ` [PATCH 06/14] dt-bindings: clock: milbeaut: add Milbeaut clock description Sugaya Taichi
` (3 subsequent siblings)
8 siblings, 1 reply; 43+ messages in thread
From: Sugaya Taichi @ 2018-11-19 1:01 UTC (permalink / raw)
To: linux-clk, devicetree, linux-arm-kernel, linux-kernel, linux-serial
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Greg Kroah-Hartman, Daniel Lezcano, Thomas Gleixner,
Russell King, Jiri Slaby, Masami Hiramatsu, Jassi Brar,
Sugaya Taichi
Add Milbeaut M10V timer using 32bit timer in peripheral.
Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
---
drivers/clocksource/Kconfig | 8 +++
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-m10v.c | 146 +++++++++++++++++++++++++++++++++++++++
3 files changed, 155 insertions(+)
create mode 100644 drivers/clocksource/timer-m10v.c
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 55c77e4..a278d72 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -638,4 +638,12 @@ config GX6605S_TIMER
help
This option enables support for gx6605s SOC's timer.
+config M10V_TIMER
+ bool "Milbeaut M10V timer driver" if COMPILE_TEST
+ depends on OF
+ depends on ARM
+ select TIMER_OF
+ help
+ Enables the support for Milbeaut M10V timer driver.
+
endmenu
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index dd91381..8e908b4 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_CLKSRC_TI_32K) += timer-ti-32k.o
obj-$(CONFIG_CLKSRC_NPS) += timer-nps.o
obj-$(CONFIG_OXNAS_RPS_TIMER) += timer-oxnas-rps.o
obj-$(CONFIG_OWL_TIMER) += timer-owl.o
+obj-$(CONFIG_M10V_TIMER) += timer-m10v.o
obj-$(CONFIG_SPRD_TIMER) += timer-sprd.o
obj-$(CONFIG_NPCM7XX_TIMER) += timer-npcm7xx.o
diff --git a/drivers/clocksource/timer-m10v.c b/drivers/clocksource/timer-m10v.c
new file mode 100644
index 0000000..ff97c23
--- /dev/null
+++ b/drivers/clocksource/timer-m10v.c
@@ -0,0 +1,146 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Socionext Inc.
+ */
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqreturn.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+#include "timer-of.h"
+
+#define FSL_TMR_TMCSR_OFS 0x0
+#define FSL_TMR_TMR_OFS 0x4
+#define FSL_TMR_TMRLR1_OFS 0x8
+#define FSL_TMR_TMRLR2_OFS 0xc
+#define FSL_RMT_REGSZPCH 0x10
+
+#define FSL_TMR_TMCSR_OUTL BIT(5)
+#define FSL_TMR_TMCSR_RELD BIT(4)
+#define FSL_TMR_TMCSR_INTE BIT(3)
+#define FSL_TMR_TMCSR_UF BIT(2)
+#define FSL_TMR_TMCSR_CNTE BIT(1)
+#define FSL_TMR_TMCSR_TRG BIT(0)
+
+#define FSL_TMR_TMCSR_CSL_DIV2 0
+#define FSL_TMR_TMCSR_CSL BIT(10)
+
+#define M10V_TIMER_RATING 500
+
+static irqreturn_t m10v_timer_interrupt(int irq, void *dev_id)
+{
+ struct clock_event_device *clk = dev_id;
+ struct timer_of *to = to_timer_of(clk);
+ u32 val;
+
+ val = readl_relaxed(timer_of_base(to) + FSL_TMR_TMCSR_OFS);
+ val &= ~FSL_TMR_TMCSR_UF;
+ writel_relaxed(val, timer_of_base(to) + FSL_TMR_TMCSR_OFS);
+
+ clk->event_handler(clk);
+
+ return IRQ_HANDLED;
+}
+
+static int m10v_set_state_periodic(struct clock_event_device *clk)
+{
+ struct timer_of *to = to_timer_of(clk);
+ u32 val = (FSL_TMR_TMCSR_CSL_DIV2 * FSL_TMR_TMCSR_CSL);
+
+ writel_relaxed(val, timer_of_base(to) + FSL_TMR_TMCSR_OFS);
+
+ writel_relaxed(to->of_clk.period, timer_of_base(to) +
+ FSL_TMR_TMRLR1_OFS);
+ val |= FSL_TMR_TMCSR_RELD | FSL_TMR_TMCSR_CNTE |
+ FSL_TMR_TMCSR_TRG | FSL_TMR_TMCSR_INTE;
+ writel_relaxed(val, timer_of_base(to) + FSL_TMR_TMCSR_OFS);
+ return 0;
+}
+
+static int m10v_set_state_oneshot(struct clock_event_device *clk)
+{
+ struct timer_of *to = to_timer_of(clk);
+ u32 val = (FSL_TMR_TMCSR_CSL_DIV2 * FSL_TMR_TMCSR_CSL);
+
+ writel_relaxed(val, timer_of_base(to) + FSL_TMR_TMCSR_OFS);
+ return 0;
+}
+
+static int m10v_clkevt_next_event(unsigned long event,
+ struct clock_event_device *clk)
+{
+ struct timer_of *to = to_timer_of(clk);
+
+ writel_relaxed(event, timer_of_base(to) + FSL_TMR_TMRLR1_OFS);
+ writel_relaxed((FSL_TMR_TMCSR_CSL_DIV2 * FSL_TMR_TMCSR_CSL) |
+ FSL_TMR_TMCSR_CNTE | FSL_TMR_TMCSR_INTE |
+ FSL_TMR_TMCSR_TRG, timer_of_base(to) +
+ FSL_TMR_TMCSR_OFS);
+ return 0;
+}
+
+static int m10v_config_clock_source(struct timer_of *to)
+{
+ writel_relaxed(0, timer_of_base(to) + FSL_TMR_TMCSR_OFS);
+ writel_relaxed(~0, timer_of_base(to) + FSL_TMR_TMR_OFS);
+ writel_relaxed(~0, timer_of_base(to) + FSL_TMR_TMRLR1_OFS);
+ writel_relaxed(~0, timer_of_base(to) + FSL_TMR_TMRLR2_OFS);
+ writel_relaxed(BIT(4) | BIT(1) | BIT(0), timer_of_base(to) +
+ FSL_TMR_TMCSR_OFS);
+ return 0;
+}
+
+static int m10v_config_clock_event(struct timer_of *to)
+{
+ writel_relaxed(0, timer_of_base(to) + FSL_TMR_TMCSR_OFS);
+ return 0;
+}
+
+static struct timer_of to = {
+ .flags = TIMER_OF_IRQ | TIMER_OF_BASE | TIMER_OF_CLOCK,
+
+ .clkevt = {
+ .name = "m10v-clkevt",
+ .rating = M10V_TIMER_RATING,
+ .cpumask = cpu_possible_mask,
+ },
+
+ .of_irq = {
+ .flags = IRQF_TIMER | IRQF_IRQPOLL,
+ },
+};
+
+static int __init m10v_timer_init(struct device_node *node)
+{
+ int ret;
+
+ to.clkevt.features = CLOCK_EVT_FEAT_DYNIRQ | CLOCK_EVT_FEAT_ONESHOT;
+ to.clkevt.set_state_oneshot = m10v_set_state_oneshot;
+ to.clkevt.set_state_periodic = m10v_set_state_periodic;
+ to.clkevt.set_next_event = m10v_clkevt_next_event;
+ to.of_irq.handler = m10v_timer_interrupt;
+
+ ret = timer_of_init(node, &to);
+ if (ret)
+ goto err;
+
+ m10v_config_clock_source(&to);
+ clocksource_mmio_init(timer_of_base(&to) + FSL_TMR_TMR_OFS,
+ node->name, timer_of_rate(&to), M10V_TIMER_RATING, 32,
+ clocksource_mmio_readl_down);
+ m10v_config_clock_event(&to);
+ clockevents_config_and_register(&to.clkevt, timer_of_rate(&to),
+ 15, 0xffffffff);
+
+ return 0;
+err:
+ timer_of_cleanup(&to);
+ return ret;
+}
+TIMER_OF_DECLARE(m10v_peritimer, "socionext,milbeaut-m10v-timer",
+ m10v_timer_init);
--
1.9.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 05/14] clocksource/drivers/timer-milbeaut: Add Milbeaut M10V timer
2018-11-19 1:01 ` [PATCH 05/14] clocksource/drivers/timer-milbeaut: Add Milbeaut M10V timer Sugaya Taichi
@ 2018-11-21 10:08 ` Daniel Lezcano
2018-11-22 2:23 ` Sugaya, Taichi
0 siblings, 1 reply; 43+ messages in thread
From: Daniel Lezcano @ 2018-11-21 10:08 UTC (permalink / raw)
To: Sugaya Taichi, linux-clk, devicetree, linux-arm-kernel,
linux-kernel, linux-serial
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Greg Kroah-Hartman, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar
Hi Sugaya,
On 19/11/2018 02:01, Sugaya Taichi wrote:
> Add Milbeaut M10V timer using 32bit timer in peripheral.
Give a better description of the timer as it is new timer introduced.
> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
> ---
> drivers/clocksource/Kconfig | 8 +++
> drivers/clocksource/Makefile | 1 +
> drivers/clocksource/timer-m10v.c | 146 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 155 insertions(+)
> create mode 100644 drivers/clocksource/timer-m10v.c
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 55c77e4..a278d72 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -638,4 +638,12 @@ config GX6605S_TIMER
> help
> This option enables support for gx6605s SOC's timer.
>
> +config M10V_TIMER
> + bool "Milbeaut M10V timer driver" if COMPILE_TEST
> + depends on OF
> + depends on ARM
> + select TIMER_OF
> + help
> + Enables the support for Milbeaut M10V timer driver.
> +
> endmenu
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index dd91381..8e908b4 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -55,6 +55,7 @@ obj-$(CONFIG_CLKSRC_TI_32K) += timer-ti-32k.o
> obj-$(CONFIG_CLKSRC_NPS) += timer-nps.o
> obj-$(CONFIG_OXNAS_RPS_TIMER) += timer-oxnas-rps.o
> obj-$(CONFIG_OWL_TIMER) += timer-owl.o
> +obj-$(CONFIG_M10V_TIMER) += timer-m10v.o
> obj-$(CONFIG_SPRD_TIMER) += timer-sprd.o
> obj-$(CONFIG_NPCM7XX_TIMER) += timer-npcm7xx.o
>
> diff --git a/drivers/clocksource/timer-m10v.c b/drivers/clocksource/timer-m10v.c
> new file mode 100644
> index 0000000..ff97c23
> --- /dev/null
> +++ b/drivers/clocksource/timer-m10v.c
> @@ -0,0 +1,146 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Socionext Inc.
> + */
> +
> +#include <linux/clk.h>
------->
> +#include <linux/clockchips.h>
It is included from timer-of.h
<-------
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/irqreturn.h>
------->
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/slab.h>
Those are not needed IMO.
<---------
> +#include "timer-of.h"
> +
> +#define FSL_TMR_TMCSR_OFS 0x0
> +#define FSL_TMR_TMR_OFS 0x4
> +#define FSL_TMR_TMRLR1_OFS 0x8
> +#define FSL_TMR_TMRLR2_OFS 0xc
> +#define FSL_RMT_REGSZPCH 0x10
> +
> +#define FSL_TMR_TMCSR_OUTL BIT(5)
> +#define FSL_TMR_TMCSR_RELD BIT(4)
> +#define FSL_TMR_TMCSR_INTE BIT(3)
> +#define FSL_TMR_TMCSR_UF BIT(2)
> +#define FSL_TMR_TMCSR_CNTE BIT(1)
> +#define FSL_TMR_TMCSR_TRG BIT(0)
> +
> +#define FSL_TMR_TMCSR_CSL_DIV2 0
> +#define FSL_TMR_TMCSR_CSL BIT(10)
> +
> +#define M10V_TIMER_RATING 500
> +
> +static irqreturn_t m10v_timer_interrupt(int irq, void *dev_id)
> +{
> + struct clock_event_device *clk = dev_id;
> + struct timer_of *to = to_timer_of(clk);
> + u32 val;
> +
> + val = readl_relaxed(timer_of_base(to) + FSL_TMR_TMCSR_OFS);
> + val &= ~FSL_TMR_TMCSR_UF;
> + writel_relaxed(val, timer_of_base(to) + FSL_TMR_TMCSR_OFS);
> +
> + clk->event_handler(clk);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int m10v_set_state_periodic(struct clock_event_device *clk)
> +{
> + struct timer_of *to = to_timer_of(clk);
> + u32 val = (FSL_TMR_TMCSR_CSL_DIV2 * FSL_TMR_TMCSR_CSL);
FSL_TMR_TMCSR_CSL_DIV2 is zero, so val is always zero.
> + writel_relaxed(val, timer_of_base(to) + FSL_TMR_TMCSR_OFS);
> +
> + writel_relaxed(to->of_clk.period, timer_of_base(to) +
> + FSL_TMR_TMRLR1_OFS);
> + val |= FSL_TMR_TMCSR_RELD | FSL_TMR_TMCSR_CNTE |
> + FSL_TMR_TMCSR_TRG | FSL_TMR_TMCSR_INTE;
> + writel_relaxed(val, timer_of_base(to) + FSL_TMR_TMCSR_OFS);
> + return 0;
> +}
> +
> +static int m10v_set_state_oneshot(struct clock_event_device *clk)
> +{
> + struct timer_of *to = to_timer_of(clk);
> + u32 val = (FSL_TMR_TMCSR_CSL_DIV2 * FSL_TMR_TMCSR_CSL);
> +
> + writel_relaxed(val, timer_of_base(to) + FSL_TMR_TMCSR_OFS);
> + return 0;
> +}
> +
> +static int m10v_clkevt_next_event(unsigned long event,
> + struct clock_event_device *clk)
> +{
> + struct timer_of *to = to_timer_of(clk);
> +
> + writel_relaxed(event, timer_of_base(to) + FSL_TMR_TMRLR1_OFS);
> + writel_relaxed((FSL_TMR_TMCSR_CSL_DIV2 * FSL_TMR_TMCSR_CSL) |
Same comment here.
> + FSL_TMR_TMCSR_CNTE | FSL_TMR_TMCSR_INTE |
> + FSL_TMR_TMCSR_TRG, timer_of_base(to) +
> + FSL_TMR_TMCSR_OFS);
> + return 0;
> +}
> +
> +static int m10v_config_clock_source(struct timer_of *to)
> +{
> + writel_relaxed(0, timer_of_base(to) + FSL_TMR_TMCSR_OFS);
> + writel_relaxed(~0, timer_of_base(to) + FSL_TMR_TMR_OFS);
> + writel_relaxed(~0, timer_of_base(to) + FSL_TMR_TMRLR1_OFS);
> + writel_relaxed(~0, timer_of_base(to) + FSL_TMR_TMRLR2_OFS);
> + writel_relaxed(BIT(4) | BIT(1) | BIT(0), timer_of_base(to) +
> + FSL_TMR_TMCSR_OFS);
> + return 0;
> +}
> +
> +static int m10v_config_clock_event(struct timer_of *to)
> +{
> + writel_relaxed(0, timer_of_base(to) + FSL_TMR_TMCSR_OFS);
> + return 0;
> +}
> +
> +static struct timer_of to = {
> + .flags = TIMER_OF_IRQ | TIMER_OF_BASE | TIMER_OF_CLOCK,
> +
> + .clkevt = {
> + .name = "m10v-clkevt",
> + .rating = M10V_TIMER_RATING,
> + .cpumask = cpu_possible_mask,
> + },
> +
> + .of_irq = {
> + .flags = IRQF_TIMER | IRQF_IRQPOLL,
> + },
> +};
> +
> +static int __init m10v_timer_init(struct device_node *node)
> +{
> + int ret;
> +
> + to.clkevt.features = CLOCK_EVT_FEAT_DYNIRQ | CLOCK_EVT_FEAT_ONESHOT;
> + to.clkevt.set_state_oneshot = m10v_set_state_oneshot;
> + to.clkevt.set_state_periodic = m10v_set_state_periodic;
> + to.clkevt.set_next_event = m10v_clkevt_next_event;
> + to.of_irq.handler = m10v_timer_interrupt;
Move the initialization in the timer_of structure above.
> + ret = timer_of_init(node, &to);
> + if (ret)
> + goto err;
You should return directly, rollback is done in the timer_of_init().
> +
> + m10v_config_clock_source(&to);
> + clocksource_mmio_init(timer_of_base(&to) + FSL_TMR_TMR_OFS,
> + node->name, timer_of_rate(&to), M10V_TIMER_RATING, 32,
> + clocksource_mmio_readl_down);
May be you can add the sched_clock also ?
> + m10v_config_clock_event(&to);
> + clockevents_config_and_register(&to.clkevt, timer_of_rate(&to),
> + 15, 0xffffffff);
> +
> + return 0;
> +err:
> + timer_of_cleanup(&to);
> + return ret;
> +}
> +TIMER_OF_DECLARE(m10v_peritimer, "socionext,milbeaut-m10v-timer",
> + m10v_timer_init);
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 05/14] clocksource/drivers/timer-milbeaut: Add Milbeaut M10V timer
2018-11-21 10:08 ` Daniel Lezcano
@ 2018-11-22 2:23 ` Sugaya, Taichi
0 siblings, 0 replies; 43+ messages in thread
From: Sugaya, Taichi @ 2018-11-22 2:23 UTC (permalink / raw)
To: Daniel Lezcano, linux-clk, devicetree, linux-arm-kernel,
linux-kernel, linux-serial
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Greg Kroah-Hartman, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar
Hi Daniel
Thank you for your comments.
On 2018/11/21 19:08, Daniel Lezcano wrote:
> Hi Sugaya,
>
>
> On 19/11/2018 02:01, Sugaya Taichi wrote:
>> Add Milbeaut M10V timer using 32bit timer in peripheral.
> Give a better description of the timer as it is new timer introduced.
I got it. Add more description.
>> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
>> ---
>> drivers/clocksource/Kconfig | 8 +++
>> drivers/clocksource/Makefile | 1 +
>> drivers/clocksource/timer-m10v.c | 146 +++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 155 insertions(+)
>> create mode 100644 drivers/clocksource/timer-m10v.c
>>
>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>> index 55c77e4..a278d72 100644
>> --- a/drivers/clocksource/Kconfig
>> +++ b/drivers/clocksource/Kconfig
>> @@ -638,4 +638,12 @@ config GX6605S_TIMER
>> help
>> This option enables support for gx6605s SOC's timer.
>>
>> +config M10V_TIMER
>> + bool "Milbeaut M10V timer driver" if COMPILE_TEST
>> + depends on OF
>> + depends on ARM
>> + select TIMER_OF
>> + help
>> + Enables the support for Milbeaut M10V timer driver.
>> +
>> endmenu
>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>> index dd91381..8e908b4 100644
>> --- a/drivers/clocksource/Makefile
>> +++ b/drivers/clocksource/Makefile
>> @@ -55,6 +55,7 @@ obj-$(CONFIG_CLKSRC_TI_32K) += timer-ti-32k.o
>> obj-$(CONFIG_CLKSRC_NPS) += timer-nps.o
>> obj-$(CONFIG_OXNAS_RPS_TIMER) += timer-oxnas-rps.o
>> obj-$(CONFIG_OWL_TIMER) += timer-owl.o
>> +obj-$(CONFIG_M10V_TIMER) += timer-m10v.o
>> obj-$(CONFIG_SPRD_TIMER) += timer-sprd.o
>> obj-$(CONFIG_NPCM7XX_TIMER) += timer-npcm7xx.o
>>
>> diff --git a/drivers/clocksource/timer-m10v.c b/drivers/clocksource/timer-m10v.c
>> new file mode 100644
>> index 0000000..ff97c23
>> --- /dev/null
>> +++ b/drivers/clocksource/timer-m10v.c
>> @@ -0,0 +1,146 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2018 Socionext Inc.
>> + */
>> +
>> +#include <linux/clk.h>
> ------->
>
>> +#include <linux/clockchips.h>
> It is included from timer-of.h
>
> <-------
OK. Remove it.
>
>> +#include <linux/interrupt.h>
>> +#include <linux/irq.h>
>> +#include <linux/irqreturn.h>
> ------->
>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/slab.h>
> Those are not needed IMO.
>
> <---------
OK, remove these.
>
>
>> +#include "timer-of.h"
>> +
>> +#define FSL_TMR_TMCSR_OFS 0x0
>> +#define FSL_TMR_TMR_OFS 0x4
>> +#define FSL_TMR_TMRLR1_OFS 0x8
>> +#define FSL_TMR_TMRLR2_OFS 0xc
>> +#define FSL_RMT_REGSZPCH 0x10
>> +
>> +#define FSL_TMR_TMCSR_OUTL BIT(5)
>> +#define FSL_TMR_TMCSR_RELD BIT(4)
>> +#define FSL_TMR_TMCSR_INTE BIT(3)
>> +#define FSL_TMR_TMCSR_UF BIT(2)
>> +#define FSL_TMR_TMCSR_CNTE BIT(1)
>> +#define FSL_TMR_TMCSR_TRG BIT(0)
>> +
>> +#define FSL_TMR_TMCSR_CSL_DIV2 0
>> +#define FSL_TMR_TMCSR_CSL BIT(10)
>> +
>> +#define M10V_TIMER_RATING 500
>> +
>> +static irqreturn_t m10v_timer_interrupt(int irq, void *dev_id)
>> +{
>> + struct clock_event_device *clk = dev_id;
>> + struct timer_of *to = to_timer_of(clk);
>> + u32 val;
>> +
>> + val = readl_relaxed(timer_of_base(to) + FSL_TMR_TMCSR_OFS);
>> + val &= ~FSL_TMR_TMCSR_UF;
>> + writel_relaxed(val, timer_of_base(to) + FSL_TMR_TMCSR_OFS);
>> +
>> + clk->event_handler(clk);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int m10v_set_state_periodic(struct clock_event_device *clk)
>> +{
>> + struct timer_of *to = to_timer_of(clk);
>> + u32 val = (FSL_TMR_TMCSR_CSL_DIV2 * FSL_TMR_TMCSR_CSL);
> FSL_TMR_TMCSR_CSL_DIV2 is zero, so val is always zero.
Ah, yes.
The FSL_TMR_TMCSR is the 10th bit field of the register, so it would be
better
to bit-shift.
I will use it as simply 0.
>
>> + writel_relaxed(val, timer_of_base(to) + FSL_TMR_TMCSR_OFS);
>> +
>> + writel_relaxed(to->of_clk.period, timer_of_base(to) +
>> + FSL_TMR_TMRLR1_OFS);
>> + val |= FSL_TMR_TMCSR_RELD | FSL_TMR_TMCSR_CNTE |
>> + FSL_TMR_TMCSR_TRG | FSL_TMR_TMCSR_INTE;
>> + writel_relaxed(val, timer_of_base(to) + FSL_TMR_TMCSR_OFS);
>> + return 0;
>> +}
>> +
>> +static int m10v_set_state_oneshot(struct clock_event_device *clk)
>> +{
>> + struct timer_of *to = to_timer_of(clk);
>> + u32 val = (FSL_TMR_TMCSR_CSL_DIV2 * FSL_TMR_TMCSR_CSL);
>> +
>> + writel_relaxed(val, timer_of_base(to) + FSL_TMR_TMCSR_OFS);
>> + return 0;
>> +}
>> +
>> +static int m10v_clkevt_next_event(unsigned long event,
>> + struct clock_event_device *clk)
>> +{
>> + struct timer_of *to = to_timer_of(clk);
>> +
>> + writel_relaxed(event, timer_of_base(to) + FSL_TMR_TMRLR1_OFS);
>> + writel_relaxed((FSL_TMR_TMCSR_CSL_DIV2 * FSL_TMR_TMCSR_CSL) |
> Same comment here.
I got it.
>> + FSL_TMR_TMCSR_CNTE | FSL_TMR_TMCSR_INTE |
>> + FSL_TMR_TMCSR_TRG, timer_of_base(to) +
>> + FSL_TMR_TMCSR_OFS);
>> + return 0;
>> +}
>> +
>> +static int m10v_config_clock_source(struct timer_of *to)
>> +{
>> + writel_relaxed(0, timer_of_base(to) + FSL_TMR_TMCSR_OFS);
>> + writel_relaxed(~0, timer_of_base(to) + FSL_TMR_TMR_OFS);
>> + writel_relaxed(~0, timer_of_base(to) + FSL_TMR_TMRLR1_OFS);
>> + writel_relaxed(~0, timer_of_base(to) + FSL_TMR_TMRLR2_OFS);
>> + writel_relaxed(BIT(4) | BIT(1) | BIT(0), timer_of_base(to) +
>> + FSL_TMR_TMCSR_OFS);
>> + return 0;
>> +}
>> +
>> +static int m10v_config_clock_event(struct timer_of *to)
>> +{
>> + writel_relaxed(0, timer_of_base(to) + FSL_TMR_TMCSR_OFS);
>> + return 0;
>> +}
>> +
>> +static struct timer_of to = {
>> + .flags = TIMER_OF_IRQ | TIMER_OF_BASE | TIMER_OF_CLOCK,
>> +
>> + .clkevt = {
>> + .name = "m10v-clkevt",
>> + .rating = M10V_TIMER_RATING,
>> + .cpumask = cpu_possible_mask,
>> + },
>> +
>> + .of_irq = {
>> + .flags = IRQF_TIMER | IRQF_IRQPOLL,
>> + },
>> +};
>> +
>> +static int __init m10v_timer_init(struct device_node *node)
>> +{
>> + int ret;
>> +
>> + to.clkevt.features = CLOCK_EVT_FEAT_DYNIRQ | CLOCK_EVT_FEAT_ONESHOT;
>> + to.clkevt.set_state_oneshot = m10v_set_state_oneshot;
>> + to.clkevt.set_state_periodic = m10v_set_state_periodic;
>> + to.clkevt.set_next_event = m10v_clkevt_next_event;
>> + to.of_irq.handler = m10v_timer_interrupt;
> Move the initialization in the timer_of structure above.
Okay.
>
>> + ret = timer_of_init(node, &to);
>> + if (ret)
>> + goto err;
> You should return directly, rollback is done in the timer_of_init().
OK.
>
>> +
>> + m10v_config_clock_source(&to);
>> + clocksource_mmio_init(timer_of_base(&to) + FSL_TMR_TMR_OFS,
>> + node->name, timer_of_rate(&to), M10V_TIMER_RATING, 32,
>> + clocksource_mmio_readl_down);
> May be you can add the sched_clock also ?
OK. Add it and confirm.
Thanks
Sugaya Taichi
>
>> + m10v_config_clock_event(&to);
>> + clockevents_config_and_register(&to.clkevt, timer_of_rate(&to),
>> + 15, 0xffffffff);
>> +
>> + return 0;
>> +err:
>> + timer_of_cleanup(&to);
>> + return ret;
>> +}
>> +TIMER_OF_DECLARE(m10v_peritimer, "socionext,milbeaut-m10v-timer",
>> + m10v_timer_init);
>>
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 06/14] dt-bindings: clock: milbeaut: add Milbeaut clock description
2018-11-19 1:01 [PATCH 00/14] Add basic support for Socionext Milbeaut M10V SoCs Sugaya Taichi
` (4 preceding siblings ...)
2018-11-19 1:01 ` [PATCH 05/14] clocksource/drivers/timer-milbeaut: Add Milbeaut M10V timer Sugaya Taichi
@ 2018-11-19 1:01 ` Sugaya Taichi
2018-11-30 8:19 ` Stephen Boyd
2018-11-19 1:01 ` [PATCH 07/14] clock: milbeaut: Add Milbeaut M10V clock control Sugaya Taichi
` (2 subsequent siblings)
8 siblings, 1 reply; 43+ messages in thread
From: Sugaya Taichi @ 2018-11-19 1:01 UTC (permalink / raw)
To: linux-clk, devicetree, linux-arm-kernel, linux-kernel, linux-serial
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Greg Kroah-Hartman, Daniel Lezcano, Thomas Gleixner,
Russell King, Jiri Slaby, Masami Hiramatsu, Jassi Brar,
Sugaya Taichi
Add DT bindings document for Milbeaut clock.
Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
---
.../devicetree/bindings/clock/milbeaut-clock.txt | 93 ++++++++++++++++++++++
1 file changed, 93 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/milbeaut-clock.txt
diff --git a/Documentation/devicetree/bindings/clock/milbeaut-clock.txt b/Documentation/devicetree/bindings/clock/milbeaut-clock.txt
new file mode 100644
index 0000000..5c093c8
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/milbeaut-clock.txt
@@ -0,0 +1,93 @@
+Milbeaut M10V Clock Controller Binding
+----------------------------------------
+Milbeaut clock controller is consists of few oscillators, PLL, multiplexer
+and few divider modules
+
+This binding uses common clock bindings
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible: shall be "socionext,milbeaut-m10v-clk-regs"
+- reg: shall contain base address and length of clock registers
+- #clock-cells: shall be 0
+
+Example:
+ m10v-clk-tree@ {
+ compatible = "socionext,milbeaut-m10v-clk-regs";
+ reg = <0x1d021000 0x4000>;
+
+ clocks {
+ #address-cells = <0>;
+ #size-cells = <0>;
+
+ uclk40xi: uclk40xi {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <40000000>;
+ };
+ };
+ }
+
+The clock consumer shall specify the desired clock-output of the clock
+controller (as defined in [2]) by specifying output-id in its "clock"
+phandle cell
+[2] arch/arm/boot/dts/milbeaut-m10v-clk.h
+
+For example for UART1:
+ usio1: usio_uart@1e700010 {
+ index = <0>;
+ compatible = "socionext,milbeaut-m10v-usio-uart";
+ reg = <0x1e700010 0x10>;
+ interrupts = <0 141 0x4>, <0 149 0x4>;
+ interrupt-names = "rx", "tx";
+ clocks = <&hclk>;
+ };
+
+
+
+
+Required properties:
+- compatible:
+ "socionext,milbeaut-m10v-clk-mux"
+ -clock-cells: should be 0
+ -clocks: should be two factor
+ "socionext,milbeaut-m10v-pll-fixed-factor"
+ -clock-cells: should be 0
+ -clocks: should be one factor
+ -offset: offset
+ -clock-div: div number
+ -clock-mult: multiple number
+ "socionext,milbeaut-m10v-clk-div"
+ -clock-cells: should be 0
+ -clocks: should be one factor
+ -offset: offset
+ -mask: mask bit
+ -ratios: div ratio
+
+Example
+ piclk_mux_0: spiclk_mux_0 {
+ compatible = "socionext,m10v-clk-div";
+ #clock-cells = <0>;
+ clocks = <&pll10_div_1_2>;
+ offset = <bSPICLK>;
+ mask = <0x3>;
+ ratios = <4 0x5 2 0x4>;
+ };
+
+ pll10: pll10 {
+ compatible = "socionext,m10v-pll-fixed-factor";
+ #clock-cells = <0>;
+ clocks = <&uclk40xi>;
+ offset = <10>;
+ clock-div = <5>;
+ clock-mult = <108>;
+ };
+
+ emmcclk: emmcclk {
+ compatible = "socionext,m10v-clk-div";
+ #clock-cells = <0>;
+ clocks = <&pll11>;
+ offset = <bEMMCCLK>;
+ mask = <0x3>;
+ ratios = <15 0x7 10 0x6 9 0x5 8 0x4>;
+ };
--
1.9.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 06/14] dt-bindings: clock: milbeaut: add Milbeaut clock description
2018-11-19 1:01 ` [PATCH 06/14] dt-bindings: clock: milbeaut: add Milbeaut clock description Sugaya Taichi
@ 2018-11-30 8:19 ` Stephen Boyd
2018-12-03 8:08 ` Sugaya, Taichi
0 siblings, 1 reply; 43+ messages in thread
From: Stephen Boyd @ 2018-11-30 8:19 UTC (permalink / raw)
To: Sugaya Taichi, devicetree, linux-arm-kernel, linux-clk,
linux-kernel, linux-serial
Cc: Michael Turquette, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar, Sugaya Taichi
Quoting Sugaya Taichi (2018-11-18 17:01:11)
> Add DT bindings document for Milbeaut clock.
>
> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
> ---
> .../devicetree/bindings/clock/milbeaut-clock.txt | 93 ++++++++++++++++++++++
> 1 file changed, 93 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/milbeaut-clock.txt
>
> diff --git a/Documentation/devicetree/bindings/clock/milbeaut-clock.txt b/Documentation/devicetree/bindings/clock/milbeaut-clock.txt
> new file mode 100644
> index 0000000..5c093c8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/milbeaut-clock.txt
> @@ -0,0 +1,93 @@
> +Milbeaut M10V Clock Controller Binding
> +----------------------------------------
> +Milbeaut clock controller is consists of few oscillators, PLL, multiplexer
> +and few divider modules
> +
> +This binding uses common clock bindings
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +Required properties:
> +- compatible: shall be "socionext,milbeaut-m10v-clk-regs"
> +- reg: shall contain base address and length of clock registers
> +- #clock-cells: shall be 0
> +
> +Example:
> + m10v-clk-tree@ {
> + compatible = "socionext,milbeaut-m10v-clk-regs";
> + reg = <0x1d021000 0x4000>;
> +
> + clocks {
> + #address-cells = <0>;
> + #size-cells = <0>;
> +
> + uclk40xi: uclk40xi {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <40000000>;
> + };
> + };
This style of binding is highly discouraged. We don't describe each and
every clk in DT, we describe clk controllers and their outputs and
inputs in DT. The driver is the place where the clock controller
describes the internal clk topology of that controller. Also, fixed
frequency clks are typically oscillators and those would come from the
board dts file, but otherwise I wouldn't expect to see fixed frequency
clks in DT.
> + }
> +
> +The clock consumer shall specify the desired clock-output of the clock
> +controller (as defined in [2]) by specifying output-id in its "clock"
> +phandle cell
> +[2] arch/arm/boot/dts/milbeaut-m10v-clk.h
> +
[...]
> +
> +Example
> + piclk_mux_0: spiclk_mux_0 {
> + compatible = "socionext,m10v-clk-div";
> + #clock-cells = <0>;
> + clocks = <&pll10_div_1_2>;
> + offset = <bSPICLK>;
> + mask = <0x3>;
> + ratios = <4 0x5 2 0x4>;
> + };
> +
> + pll10: pll10 {
> + compatible = "socionext,m10v-pll-fixed-factor";
> + #clock-cells = <0>;
> + clocks = <&uclk40xi>;
> + offset = <10>;
> + clock-div = <5>;
> + clock-mult = <108>;
> + };
> +
> + emmcclk: emmcclk {
> + compatible = "socionext,m10v-clk-div";
> + #clock-cells = <0>;
> + clocks = <&pll11>;
> + offset = <bEMMCCLK>;
> + mask = <0x3>;
> + ratios = <15 0x7 10 0x6 9 0x5 8 0x4>;
Yeah, please no. This whole binding needs a rewrite to not have one node
per clk.
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 06/14] dt-bindings: clock: milbeaut: add Milbeaut clock description
2018-11-30 8:19 ` Stephen Boyd
@ 2018-12-03 8:08 ` Sugaya, Taichi
0 siblings, 0 replies; 43+ messages in thread
From: Sugaya, Taichi @ 2018-12-03 8:08 UTC (permalink / raw)
To: Stephen Boyd, devicetree, linux-arm-kernel, linux-clk,
linux-kernel, linux-serial
Cc: Michael Turquette, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar
Hi,
Thank you for your comments.
On 2018/11/30 17:19, Stephen Boyd wrote:
> Quoting Sugaya Taichi (2018-11-18 17:01:11)
>> Add DT bindings document for Milbeaut clock.
>>
>> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
>> ---
>> .../devicetree/bindings/clock/milbeaut-clock.txt | 93 ++++++++++++++++++++++
>> 1 file changed, 93 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/clock/milbeaut-clock.txt
>>
>> diff --git a/Documentation/devicetree/bindings/clock/milbeaut-clock.txt b/Documentation/devicetree/bindings/clock/milbeaut-clock.txt
>> new file mode 100644
>> index 0000000..5c093c8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/milbeaut-clock.txt
>> @@ -0,0 +1,93 @@
>> +Milbeaut M10V Clock Controller Binding
>> +----------------------------------------
>> +Milbeaut clock controller is consists of few oscillators, PLL, multiplexer
>> +and few divider modules
>> +
>> +This binding uses common clock bindings
>> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
>> +
>> +Required properties:
>> +- compatible: shall be "socionext,milbeaut-m10v-clk-regs"
>> +- reg: shall contain base address and length of clock registers
>> +- #clock-cells: shall be 0
>> +
>> +Example:
>> + m10v-clk-tree@ {
>> + compatible = "socionext,milbeaut-m10v-clk-regs";
>> + reg = <0x1d021000 0x4000>;
>> +
>> + clocks {
>> + #address-cells = <0>;
>> + #size-cells = <0>;
>> +
>> + uclk40xi: uclk40xi {
>> + compatible = "fixed-clock";
>> + #clock-cells = <0>;
>> + clock-frequency = <40000000>;
>> + };
>> + };
>
> This style of binding is highly discouraged. We don't describe each and
> every clk in DT, we describe clk controllers and their outputs and
> inputs in DT. The driver is the place where the clock controller
> describes the internal clk topology of that controller. Also, fixed
> frequency clks are typically oscillators and those would come from the
> board dts file, but otherwise I wouldn't expect to see fixed frequency
> clks in DT.
I understand. Received the similar comment from Rob in the DT part also.
>
>> + }
>> +
>> +The clock consumer shall specify the desired clock-output of the clock
>> +controller (as defined in [2]) by specifying output-id in its "clock"
>> +phandle cell
>> +[2] arch/arm/boot/dts/milbeaut-m10v-clk.h
>> +
> [...]
>> +
>> +Example
>> + piclk_mux_0: spiclk_mux_0 {
>> + compatible = "socionext,m10v-clk-div";
>> + #clock-cells = <0>;
>> + clocks = <&pll10_div_1_2>;
>> + offset = <bSPICLK>;
>> + mask = <0x3>;
>> + ratios = <4 0x5 2 0x4>;
>> + };
>> +
>> + pll10: pll10 {
>> + compatible = "socionext,m10v-pll-fixed-factor";
>> + #clock-cells = <0>;
>> + clocks = <&uclk40xi>;
>> + offset = <10>;
>> + clock-div = <5>;
>> + clock-mult = <108>;
>> + };
>> +
>> + emmcclk: emmcclk {
>> + compatible = "socionext,m10v-clk-div";
>> + #clock-cells = <0>;
>> + clocks = <&pll11>;
>> + offset = <bEMMCCLK>;
>> + mask = <0x3>;
>> + ratios = <15 0x7 10 0x6 9 0x5 8 0x4>;
>
> Yeah, please no. This whole binding needs a rewrite to not have one node
> per clk.
OK, I will renew the binding.
Thanks.
Sugaya Taichi
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 07/14] clock: milbeaut: Add Milbeaut M10V clock control
2018-11-19 1:01 [PATCH 00/14] Add basic support for Socionext Milbeaut M10V SoCs Sugaya Taichi
` (5 preceding siblings ...)
2018-11-19 1:01 ` [PATCH 06/14] dt-bindings: clock: milbeaut: add Milbeaut clock description Sugaya Taichi
@ 2018-11-19 1:01 ` Sugaya Taichi
2018-11-30 8:31 ` Stephen Boyd
2018-11-19 1:01 ` [PATCH 08/14] dt-bindings: serial: Add Milbeaut M10V serial description Sugaya Taichi
2018-11-19 1:01 ` [PATCH 09/14] serial: Add Milbeaut M10V serial control Sugaya Taichi
8 siblings, 1 reply; 43+ messages in thread
From: Sugaya Taichi @ 2018-11-19 1:01 UTC (permalink / raw)
To: linux-clk, devicetree, linux-arm-kernel, linux-kernel, linux-serial
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Greg Kroah-Hartman, Daniel Lezcano, Thomas Gleixner,
Russell King, Jiri Slaby, Masami Hiramatsu, Jassi Brar,
Sugaya Taichi
Add Milbeaut M10V clock ( including PLL ) control.
Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
---
drivers/clk/Makefile | 1 +
drivers/clk/clk-m10v.c | 671 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 672 insertions(+)
create mode 100644 drivers/clk/clk-m10v.c
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 72be7a3..da5b282 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_COMMON_CLK_GEMINI) += clk-gemini.o
obj-$(CONFIG_COMMON_CLK_ASPEED) += clk-aspeed.o
obj-$(CONFIG_ARCH_HIGHBANK) += clk-highbank.o
obj-$(CONFIG_CLK_HSDK) += clk-hsdk-pll.o
+obj-$(CONFIG_ARCH_MILBEAUT_M10V) += clk-m10v.o
obj-$(CONFIG_COMMON_CLK_MAX77686) += clk-max77686.o
obj-$(CONFIG_COMMON_CLK_MAX9485) += clk-max9485.o
obj-$(CONFIG_ARCH_MOXART) += clk-moxart.o
diff --git a/drivers/clk/clk-m10v.c b/drivers/clk/clk-m10v.c
new file mode 100644
index 0000000..aa92a69
--- /dev/null
+++ b/drivers/clk/clk-m10v.c
@@ -0,0 +1,671 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Socionext Inc.
+ * Copyright (C) 2016 Linaro Ltd.
+ *
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/of.h>
+#include <linux/spinlock.h>
+
+#define CLKSEL1 0x0
+#define CLKSEL(n) (((n) - 1) * 4 + CLKSEL1)
+
+#define PLLCNT1 0x30
+#define PLLCNT(n) (((n) - 1) * 4 + PLLCNT1)
+
+#define CLKSTOP1 0x54
+#define CLKSTOP(n) (((n) - 1) * 4 + CLKSTOP1)
+
+#define CRSWR 0x8c
+#define CRRRS 0x90
+#define CRRSM 0x94
+
+#define to_m10v_mux(_hw) container_of(_hw, struct m10v_mux, hw)
+#define to_m10v_gate(_hw) container_of(_hw, struct m10v_gate, hw)
+#define to_m10v_div(_hw) container_of(_hw, struct m10v_div, hw)
+#define to_m10v_pll(_hw) container_of(_hw, struct m10v_pll, hw)
+
+static void __iomem *clk_base;
+static struct device_node *np_top;
+static DEFINE_SPINLOCK(crglock);
+
+static __init void __iomem *m10v_clk_iomap(void)
+{
+ if (clk_base)
+ return clk_base;
+
+ np_top = of_find_compatible_node(NULL, NULL,
+ "socionext,milbeaut-m10v-clk-regs");
+ if (!np_top) {
+ pr_err("%s: CLK iomap failed!\n", __func__);
+ return NULL;
+ }
+
+ clk_base = of_iomap(np_top, 0);
+ of_node_put(np_top);
+
+ return clk_base;
+}
+
+struct m10v_mux {
+ struct clk_hw hw;
+ const char *cname;
+ u32 parent;
+};
+
+static u8 m10v_mux_get_parent(struct clk_hw *hw)
+{
+ struct m10v_mux *mcm = to_m10v_mux(hw);
+ struct clk_hw *parent;
+ int i;
+
+ i = clk_hw_get_num_parents(hw);
+ while (i--) {
+ parent = clk_hw_get_parent_by_index(hw, i);
+ if (clk_hw_get_rate(parent))
+ break;
+ }
+
+ if (i < 0) {
+ pr_info("%s:%s no parent?!\n",
+ __func__, mcm->cname);
+ i = 0;
+ }
+
+ return i;
+}
+
+static int m10v_mux_set_parent(struct clk_hw *hw, u8 index)
+{
+ struct m10v_mux *mcm = to_m10v_mux(hw);
+
+ mcm->parent = index;
+ return 0;
+}
+
+static const struct clk_ops m10v_mux_ops = {
+ .get_parent = m10v_mux_get_parent,
+ .set_parent = m10v_mux_set_parent,
+ .determine_rate = __clk_mux_determine_rate,
+};
+
+void __init m10v_clk_mux_setup(struct device_node *node)
+{
+ const char *clk_name = node->name;
+ struct clk_init_data init;
+ const char **parent_names;
+ struct m10v_mux *mcm;
+ struct clk *clk;
+ int i, parents;
+
+ if (!m10v_clk_iomap())
+ return;
+
+ of_property_read_string(node, "clock-output-names", &clk_name);
+
+ parents = of_clk_get_parent_count(node);
+ if (parents < 2) {
+ pr_err("%s: not a mux\n", clk_name);
+ return;
+ }
+
+ parent_names = kzalloc((sizeof(char *) * parents), GFP_KERNEL);
+ if (!parent_names)
+ return;
+
+ for (i = 0; i < parents; i++)
+ parent_names[i] = of_clk_get_parent_name(node, i);
+
+ mcm = kzalloc(sizeof(*mcm), GFP_KERNEL);
+ if (!mcm)
+ goto err_mcm;
+
+ init.name = clk_name;
+ init.ops = &m10v_mux_ops;
+ init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
+ init.num_parents = parents;
+ init.parent_names = parent_names;
+
+ mcm->cname = clk_name;
+ mcm->parent = 0;
+ mcm->hw.init = &init;
+
+ clk = clk_register(NULL, &mcm->hw);
+ if (IS_ERR(clk))
+ goto err_clk;
+
+ of_clk_add_provider(node, of_clk_src_simple_get, clk);
+ return;
+
+err_clk:
+ kfree(mcm);
+err_mcm:
+ kfree(parent_names);
+}
+CLK_OF_DECLARE(m10v_clk_mux, "socionext,milbeaut-m10v-clk-mux",
+ m10v_clk_mux_setup);
+
+struct m10v_pll {
+ struct clk_hw hw;
+ const char *cname;
+ const struct clk_ops ops;
+ u32 offset;
+ u32 div, mult;
+ bool ro;
+};
+
+#define ST 1
+#define SEL 2
+
+static void _mpg_enable(struct clk_hw *hw, unsigned int enable)
+{
+ struct m10v_pll *mpg = to_m10v_pll(hw);
+ unsigned long flags;
+ u32 val;
+
+ if (mpg->ro) {
+ pr_debug("%s:%d %s: read-only\n",
+ __func__, __LINE__, mpg->cname);
+ return;
+ }
+
+ spin_lock_irqsave(&crglock, flags);
+
+ val = readl(clk_base + PLLCNT(SEL));
+ if (enable)
+ val |= BIT(mpg->offset);
+ else
+ val &= ~BIT(mpg->offset);
+ writel(val, clk_base + PLLCNT(SEL));
+
+ spin_unlock_irqrestore(&crglock, flags);
+}
+
+static int mpg_enable(struct clk_hw *hw)
+{
+ _mpg_enable(hw, 1);
+ return 0;
+}
+
+static void mpg_disable(struct clk_hw *hw)
+{
+ _mpg_enable(hw, 0);
+}
+
+static int mpg_is_enabled(struct clk_hw *hw)
+{
+ struct m10v_pll *mpg = to_m10v_pll(hw);
+
+ return readl(clk_base + PLLCNT(SEL)) & (1 << mpg->offset);
+}
+
+static void _mpg_prepare(struct clk_hw *hw, unsigned int on)
+{
+ struct m10v_pll *mpg = to_m10v_pll(hw);
+ unsigned long flags;
+ u32 val;
+
+ if (mpg->ro) {
+ pr_debug("%s:%d %s: read-only\n",
+ __func__, __LINE__, mpg->cname);
+ return;
+ }
+
+ val = readl(clk_base + PLLCNT(ST));
+ if (!on == !(val & BIT(mpg->offset)))
+ return;
+
+ /* disable */
+ mpg_disable(hw);
+
+ spin_lock_irqsave(&crglock, flags);
+
+ val = readl(clk_base + PLLCNT(ST));
+ if (on)
+ val |= BIT(mpg->offset);
+ else
+ val &= ~BIT(mpg->offset);
+ writel(val, clk_base + PLLCNT(ST));
+
+ spin_unlock_irqrestore(&crglock, flags);
+
+ udelay(on ? 200 : 10);
+}
+
+static int mpg_prepare(struct clk_hw *hw)
+{
+ _mpg_prepare(hw, 1);
+ return 0;
+}
+
+static void mpg_unprepare(struct clk_hw *hw)
+{
+ _mpg_prepare(hw, 0);
+}
+
+static int mpg_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long prate)
+{
+ return 0;
+}
+
+static unsigned long mpg_recalc_rate(struct clk_hw *hw,
+ unsigned long prate)
+{
+ struct m10v_pll *mpg = to_m10v_pll(hw);
+ unsigned long long rate = prate;
+
+ if (mpg_is_enabled(hw)) {
+ rate = (unsigned long long)prate * mpg->mult;
+ do_div(rate, mpg->div);
+ }
+
+ return (unsigned long)rate;
+}
+
+static long mpg_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *prate)
+{
+ struct m10v_pll *mpg = to_m10v_pll(hw);
+ unsigned long long temp_rate = (unsigned long long)*prate * mpg->mult;
+
+ if (mpg->ro)
+ return mpg_recalc_rate(hw, *prate);
+
+ return do_div(temp_rate, mpg->div);
+}
+
+static const struct clk_ops m10v_pll_ops = {
+ .prepare = mpg_prepare,
+ .enable = mpg_enable,
+ .is_enabled = mpg_is_enabled,
+ .disable = mpg_disable,
+ .unprepare = mpg_unprepare,
+ .round_rate = mpg_round_rate,
+ .set_rate = mpg_set_rate,
+ .recalc_rate = mpg_recalc_rate,
+};
+
+void __init m10v_pll_setup(struct device_node *node)
+{
+ const char *clk_name = node->name;
+ struct clk_init_data init;
+ const char *parent_name;
+ u32 offset, div, mult;
+ struct m10v_pll *mpg;
+ struct clk *clk;
+ int ret;
+
+ if (!m10v_clk_iomap())
+ return;
+
+ of_property_read_string(node, "clock-output-names", &clk_name);
+
+ ret = of_property_read_u32(node, "offset", &offset);
+ if (ret) {
+ pr_err("%s: missing 'offset' property\n", clk_name);
+ return;
+ }
+
+ div = mult = 1;
+ of_property_read_u32(node, "clock-div", &div);
+ of_property_read_u32(node, "clock-mult", &mult);
+
+ parent_name = of_clk_get_parent_name(node, 0);
+
+ mpg = kzalloc(sizeof(*mpg), GFP_KERNEL);
+ if (!mpg)
+ return;
+
+ init.name = clk_name;
+ init.ops = &m10v_pll_ops;
+ init.flags = CLK_IS_BASIC | CLK_SET_RATE_GATE;
+ init.parent_names = &parent_name;
+ init.num_parents = 1;
+
+ mpg->cname = clk_name;
+ mpg->offset = offset;
+ mpg->div = div;
+ mpg->mult = mult;
+ mpg->hw.init = &init;
+ if (of_get_property(node, "read-only", NULL))
+ mpg->ro = true;
+
+ clk = clk_register(NULL, &mpg->hw);
+ if (IS_ERR(clk))
+ kfree(mpg);
+ else
+ of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+CLK_OF_DECLARE(m10v_clk_pll_gate, "socionext,milbeaut-m10v-pll-fixed-factor",
+ m10v_pll_setup);
+
+struct m10v_div {
+ struct clk_hw hw;
+ const char *cname;
+ bool waitdchreq;
+ u32 offset;
+ u32 mask;
+ u32 *table;
+ u32 tlen;
+ bool ro;
+};
+
+static void mdc_set_div(struct m10v_div *mdc, u32 div)
+{
+ u32 off, shift, val;
+
+ off = mdc->offset / 32 * 4;
+ shift = mdc->offset % 32;
+
+ val = readl(clk_base + CLKSEL1 + off);
+ val &= ~(mdc->mask << shift);
+ val |= (div << shift);
+ writel(val, clk_base + CLKSEL1 + off);
+
+ if (mdc->waitdchreq) {
+ unsigned int count = 250;
+
+ writel(1, clk_base + CLKSEL(11));
+
+ do {
+ udelay(1);
+ } while (--count && readl(clk_base + CLKSEL(11)) & 1);
+
+ if (!count)
+ pr_err("%s:%s CLK(%d) couldn't stabilize\n",
+ __func__, mdc->cname, mdc->offset);
+ }
+}
+
+static u32 mdc_get_div(struct m10v_div *mdc)
+{
+ u32 off, shift, div;
+
+ off = mdc->offset / 32 * 4;
+ shift = mdc->offset % 32;
+
+ div = readl(clk_base + CLKSEL1 + off);
+ div >>= shift;
+ div &= mdc->mask;
+
+ return div;
+}
+
+static int mdc_div_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long prate)
+{
+ struct m10v_div *mdc = to_m10v_div(hw);
+ u64 pr;
+ int i;
+
+ if (mdc->ro) {
+ pr_debug("%s:%d %s: read-only\n",
+ __func__, __LINE__, mdc->cname);
+ return 0;
+ }
+
+ /* divisors are already in descending order in DT */
+ for (i = mdc->tlen - 2; i >= 0; i -= 2) {
+ pr = prate;
+ do_div(pr, mdc->table[i]);
+
+ if (rate >= pr)
+ break;
+ }
+ if (i < 0)
+ i = 0;
+
+ mdc_set_div(mdc, mdc->table[i + 1]);
+
+ return 0;
+}
+
+static unsigned long mdc_div_recalc_rate(struct clk_hw *hw,
+ unsigned long prate)
+{
+ struct m10v_div *mdc = to_m10v_div(hw);
+ u64 prate64 = prate;
+ u32 div;
+ int i;
+
+ div = mdc_get_div(mdc);
+
+ for (i = 1; i < mdc->tlen && div != mdc->table[i]; i += 2)
+ if (div == (mdc->table[i] & mdc->mask))
+ break; /* the MSB is already read back as 0 */
+
+ if (i > mdc->tlen) /* some other is enabled in the mux */
+ prate64 = 0;
+ else
+ do_div(prate64, mdc->table[i - 1]);
+
+ return (unsigned long)prate64;
+}
+
+static long mdc_div_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *parent)
+{
+ struct m10v_div *mdc = to_m10v_div(hw);
+ u64 prate;
+ int i;
+
+ if (mdc->ro)
+ return mdc_div_recalc_rate(hw, *parent);
+
+ /* divisors are already in descending order in DT */
+ for (i = mdc->tlen - 2; i >= 0; i -= 2) {
+ prate = *parent;
+ do_div(prate, mdc->table[i]);
+
+ if (rate >= prate)
+ break;
+ }
+
+ return (unsigned long)prate;
+}
+
+static const struct clk_ops m10v_div_ops = {
+ .round_rate = mdc_div_round_rate,
+ .set_rate = mdc_div_set_rate,
+ .recalc_rate = mdc_div_recalc_rate,
+};
+
+void __init m10v_clk_div_setup(struct device_node *node)
+{
+ const char *clk_name = node->name;
+ struct clk_init_data init;
+ const char *parent_name;
+ struct m10v_div *mdc;
+ struct clk *clk;
+ u32 *table, mask;
+ u32 offset;
+ int count, ret;
+
+ if (!m10v_clk_iomap())
+ return;
+
+ of_property_read_string(node, "clock-output-names", &clk_name);
+
+ parent_name = of_clk_get_parent_name(node, 0);
+ if (!parent_name) {
+ pr_err("%s: no parent specified\n", clk_name);
+ return;
+ }
+
+ ret = of_property_read_u32(node, "offset", &offset);
+ if (ret) {
+ pr_err("%s: missing 'offset' property\n", clk_name);
+ return;
+ }
+
+ ret = of_property_read_u32(node, "mask", &mask);
+ if (ret) {
+ pr_err("%s: missing 'mask' property\n", clk_name);
+ return;
+ }
+
+ count = of_property_count_u32_elems(node, "ratios");
+ if (count < 2 || count%2) {
+ pr_err("%s: invalid 'ratios' property\n", clk_name);
+ return;
+ }
+
+ table = kzalloc(sizeof(*table) * count, GFP_KERNEL);
+ if (!table)
+ return;
+
+ /*
+ * The 'ratios' must be in descending order, we park at
+ * first ratio (biggest divider) when disabled.
+ */
+ ret = of_property_read_u32_array(node, "ratios", table, count);
+ if (ret) {
+ pr_err("%s: 'ratios' property read fail\n", clk_name);
+ goto err_mdc;
+ }
+
+ mdc = kzalloc(sizeof(*mdc), GFP_KERNEL);
+ if (!mdc)
+ goto err_mdc;
+
+ if (of_get_property(node, "wait-on-dchreq", NULL))
+ mdc->waitdchreq = true;
+
+ init.name = clk_name;
+ init.ops = &m10v_div_ops;
+ init.flags = CLK_IS_BASIC;
+ init.parent_names = &parent_name;
+ init.num_parents = 1;
+
+ mdc->cname = clk_name;
+ mdc->offset = offset;
+ mdc->mask = mask;
+ mdc->table = table;
+ mdc->tlen = count;
+ mdc->hw.init = &init;
+ if (of_get_property(node, "read-only", NULL))
+ mdc->ro = true;
+
+ clk = clk_register(NULL, &mdc->hw);
+ if (IS_ERR(clk))
+ goto err_clk;
+
+ of_clk_add_provider(node, of_clk_src_simple_get, clk);
+ return;
+
+err_clk:
+ kfree(mdc);
+err_mdc:
+ kfree(table);
+}
+CLK_OF_DECLARE(m10v_clk_div, "socionext,milbeaut-m10v-clk-div",
+ m10v_clk_div_setup);
+
+struct m10v_gate {
+ struct clk_hw hw;
+ const char *cname;
+ u32 offset;
+ bool ro;
+};
+
+static void _mgc_enable(struct clk_hw *hw, bool en)
+{
+ struct m10v_gate *mgc = to_m10v_gate(hw);
+ u32 off, mask;
+
+ if (mgc->ro) {
+ pr_debug("%s:%d %s: read-only\n",
+ __func__, __LINE__, mgc->cname);
+ return;
+ }
+
+ off = CLKSTOP1 + (mgc->offset / 32) * 4;
+
+ mask = (en ? 2 : 3) << (mgc->offset % 32);
+
+ writel(mask, clk_base + off);
+}
+
+static int mgc_enable(struct clk_hw *hw)
+{
+ _mgc_enable(hw, true);
+ return 0;
+}
+
+static void mgc_disable(struct clk_hw *hw)
+{
+ _mgc_enable(hw, false);
+}
+
+static int mgc_is_enabled(struct clk_hw *hw)
+{
+ struct m10v_gate *mgc = to_m10v_gate(hw);
+ u32 off, val, mask = 1 << (mgc->offset % 32);
+
+ off = CLKSTOP1 + (mgc->offset / 32) * 4;
+ val = readl(clk_base + off);
+
+ return !(val & mask);
+}
+
+static const struct clk_ops m10v_gate_ops = {
+ .enable = mgc_enable,
+ .disable = mgc_disable,
+ .is_enabled = mgc_is_enabled,
+};
+
+void __init m10v_clk_gate_setup(struct device_node *node)
+{
+ const char *clk_name = node->name;
+ struct clk_init_data init;
+ const char *parent_name;
+ struct m10v_gate *mgc;
+ struct clk *clk;
+ u32 offset;
+ int ret;
+
+ if (!m10v_clk_iomap())
+ return;
+
+ of_property_read_string(node, "clock-output-names", &clk_name);
+
+ ret = of_property_read_u32(node, "offset", &offset);
+ if (ret) {
+ pr_err("%s: missing 'offset' property\n", clk_name);
+ return;
+ }
+
+ parent_name = of_clk_get_parent_name(node, 0);
+
+ mgc = kzalloc(sizeof(*mgc), GFP_KERNEL);
+ if (!mgc)
+ return;
+
+ init.name = clk_name;
+ init.ops = &m10v_gate_ops;
+ init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
+ init.parent_names = &parent_name;
+ init.num_parents = 1;
+
+ mgc->cname = clk_name;
+ mgc->offset = offset;
+ mgc->hw.init = &init;
+ if (of_get_property(node, "read-only", NULL))
+ mgc->ro = true;
+
+ clk = clk_register(NULL, &mgc->hw);
+ if (IS_ERR(clk))
+ kfree(mgc);
+ else
+ of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+CLK_OF_DECLARE(m10v_clk_gate, "socionext,milbeaut-m10v-clk-gate",
+ m10v_clk_gate_setup);
--
1.9.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 07/14] clock: milbeaut: Add Milbeaut M10V clock control
2018-11-19 1:01 ` [PATCH 07/14] clock: milbeaut: Add Milbeaut M10V clock control Sugaya Taichi
@ 2018-11-30 8:31 ` Stephen Boyd
2018-12-04 8:26 ` Sugaya, Taichi
` (2 more replies)
0 siblings, 3 replies; 43+ messages in thread
From: Stephen Boyd @ 2018-11-30 8:31 UTC (permalink / raw)
To: Sugaya Taichi, devicetree, linux-arm-kernel, linux-clk,
linux-kernel, linux-serial
Cc: Michael Turquette, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar, Sugaya Taichi
Quoting Sugaya Taichi (2018-11-18 17:01:12)
> Add Milbeaut M10V clock ( including PLL ) control.
Please give some more details here.
>
> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
> ---
> drivers/clk/Makefile | 1 +
> drivers/clk/clk-m10v.c | 671 +++++++++++++++++++++++++++++++++++++++++++++++++
And this is different from Uniphier? Maybe we need a socionext
directory under drivers/clk/.
> 2 files changed, 672 insertions(+)
> create mode 100644 drivers/clk/clk-m10v.c
>
> diff --git a/drivers/clk/clk-m10v.c b/drivers/clk/clk-m10v.c
> new file mode 100644
> index 0000000..aa92a69
> --- /dev/null
> +++ b/drivers/clk/clk-m10v.c
> @@ -0,0 +1,671 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Socionext Inc.
> + * Copyright (C) 2016 Linaro Ltd.
> + *
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/clkdev.h>
Is this include used?
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/of_address.h>
> +#include <linux/of.h>
> +#include <linux/spinlock.h>
> +
> +#define CLKSEL1 0x0
> +#define CLKSEL(n) (((n) - 1) * 4 + CLKSEL1)
> +
> +#define PLLCNT1 0x30
> +#define PLLCNT(n) (((n) - 1) * 4 + PLLCNT1)
> +
> +#define CLKSTOP1 0x54
> +#define CLKSTOP(n) (((n) - 1) * 4 + CLKSTOP1)
> +
> +#define CRSWR 0x8c
> +#define CRRRS 0x90
> +#define CRRSM 0x94
> +
> +#define to_m10v_mux(_hw) container_of(_hw, struct m10v_mux, hw)
> +#define to_m10v_gate(_hw) container_of(_hw, struct m10v_gate, hw)
> +#define to_m10v_div(_hw) container_of(_hw, struct m10v_div, hw)
> +#define to_m10v_pll(_hw) container_of(_hw, struct m10v_pll, hw)
> +
> +static void __iomem *clk_base;
> +static struct device_node *np_top;
> +static DEFINE_SPINLOCK(crglock);
Please make more specific names for these global variables by prefixing
with m10v_. Also consider getting rid of the iomem and np_top globals
entirely and associate those with clks differently.
> +
> +static __init void __iomem *m10v_clk_iomap(void)
> +{
> + if (clk_base)
> + return clk_base;
> +
> + np_top = of_find_compatible_node(NULL, NULL,
> + "socionext,milbeaut-m10v-clk-regs");
> + if (!np_top) {
> + pr_err("%s: CLK iomap failed!\n", __func__);
We haven't iomapped yet though.
> + return NULL;
> + }
> +
> + clk_base = of_iomap(np_top, 0);
> + of_node_put(np_top);
Would be nicer to use platform_device APIs instead of OF ones.
> +
> + return clk_base;
> +}
> +
> +struct m10v_mux {
> + struct clk_hw hw;
> + const char *cname;
> + u32 parent;
> +};
> +
> +static u8 m10v_mux_get_parent(struct clk_hw *hw)
> +{
> + struct m10v_mux *mcm = to_m10v_mux(hw);
> + struct clk_hw *parent;
> + int i;
> +
> + i = clk_hw_get_num_parents(hw);
> + while (i--) {
> + parent = clk_hw_get_parent_by_index(hw, i);
> + if (clk_hw_get_rate(parent))
> + break;
> + }
> +
> + if (i < 0) {
> + pr_info("%s:%s no parent?!\n",
> + __func__, mcm->cname);
> + i = 0;
> + }
> +
> + return i;
> +}
> +
> +static int m10v_mux_set_parent(struct clk_hw *hw, u8 index)
> +{
> + struct m10v_mux *mcm = to_m10v_mux(hw);
> +
> + mcm->parent = index;
> + return 0;
> +}
> +
> +static const struct clk_ops m10v_mux_ops = {
> + .get_parent = m10v_mux_get_parent,
> + .set_parent = m10v_mux_set_parent,
> + .determine_rate = __clk_mux_determine_rate,
> +};
> +
> +void __init m10v_clk_mux_setup(struct device_node *node)
> +{
> + const char *clk_name = node->name;
> + struct clk_init_data init;
> + const char **parent_names;
> + struct m10v_mux *mcm;
> + struct clk *clk;
> + int i, parents;
> +
> + if (!m10v_clk_iomap())
> + return;
> +
> + of_property_read_string(node, "clock-output-names", &clk_name);
> +
> + parents = of_clk_get_parent_count(node);
> + if (parents < 2) {
> + pr_err("%s: not a mux\n", clk_name);
How is this possible?
> + return;
> + }
> +
> + parent_names = kzalloc((sizeof(char *) * parents), GFP_KERNEL);
> + if (!parent_names)
> + return;
> +
> + for (i = 0; i < parents; i++)
> + parent_names[i] = of_clk_get_parent_name(node, i);
This is of_clk_parent_fill().
> +
> + mcm = kzalloc(sizeof(*mcm), GFP_KERNEL);
> + if (!mcm)
> + goto err_mcm;
> +
> + init.name = clk_name;
> + init.ops = &m10v_mux_ops;
> + init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
Please don't use CLK_IS_BASIC unless you need it.
> + init.num_parents = parents;
> + init.parent_names = parent_names;
> +
> + mcm->cname = clk_name;
> + mcm->parent = 0;
> + mcm->hw.init = &init;
> +
> + clk = clk_register(NULL, &mcm->hw);
> + if (IS_ERR(clk))
> + goto err_clk;
> +
> + of_clk_add_provider(node, of_clk_src_simple_get, clk);
> + return;
> +
> +err_clk:
> + kfree(mcm);
> +err_mcm:
> + kfree(parent_names);
> +}
> +CLK_OF_DECLARE(m10v_clk_mux, "socionext,milbeaut-m10v-clk-mux",
> + m10v_clk_mux_setup);
Any chance you can use a platform driver?
> +
> +struct m10v_pll {
> + struct clk_hw hw;
> + const char *cname;
> + const struct clk_ops ops;
> + u32 offset;
> + u32 div, mult;
> + bool ro;
> +};
> +
> +#define ST 1
> +#define SEL 2
> +
> +static void _mpg_enable(struct clk_hw *hw, unsigned int enable)
> +{
> + struct m10v_pll *mpg = to_m10v_pll(hw);
> + unsigned long flags;
> + u32 val;
> +
> + if (mpg->ro) {
> + pr_debug("%s:%d %s: read-only\n",
> + __func__, __LINE__, mpg->cname);
> + return;
> + }
> +
> + spin_lock_irqsave(&crglock, flags);
> +
> + val = readl(clk_base + PLLCNT(SEL));
> + if (enable)
> + val |= BIT(mpg->offset);
> + else
> + val &= ~BIT(mpg->offset);
> + writel(val, clk_base + PLLCNT(SEL));
> +
> + spin_unlock_irqrestore(&crglock, flags);
> +}
> +
> +static int mpg_enable(struct clk_hw *hw)
> +{
> + _mpg_enable(hw, 1);
> + return 0;
> +}
> +
> +static void mpg_disable(struct clk_hw *hw)
> +{
> + _mpg_enable(hw, 0);
> +}
> +
> +static int mpg_is_enabled(struct clk_hw *hw)
> +{
> + struct m10v_pll *mpg = to_m10v_pll(hw);
> +
> + return readl(clk_base + PLLCNT(SEL)) & (1 << mpg->offset);
> +}
> +
> +static void _mpg_prepare(struct clk_hw *hw, unsigned int on)
> +{
> + struct m10v_pll *mpg = to_m10v_pll(hw);
> + unsigned long flags;
> + u32 val;
> +
> + if (mpg->ro) {
Should have different RO ops for read-only clks.
> + pr_debug("%s:%d %s: read-only\n",
> + __func__, __LINE__, mpg->cname);
> + return;
> + }
> +
> + val = readl(clk_base + PLLCNT(ST));
> + if (!on == !(val & BIT(mpg->offset)))
> + return;
> +
> + /* disable */
Please remove obvious comments.
> + mpg_disable(hw);
> +
> + spin_lock_irqsave(&crglock, flags);
> +
> + val = readl(clk_base + PLLCNT(ST));
> + if (on)
> + val |= BIT(mpg->offset);
> + else
> + val &= ~BIT(mpg->offset);
> + writel(val, clk_base + PLLCNT(ST));
> +
> + spin_unlock_irqrestore(&crglock, flags);
> +
> + udelay(on ? 200 : 10);
> +}
> +
> +static int mpg_prepare(struct clk_hw *hw)
> +{
> + _mpg_prepare(hw, 1);
> + return 0;
> +}
> +
> +static void mpg_unprepare(struct clk_hw *hw)
> +{
> + _mpg_prepare(hw, 0);
> +}
> +
> +static int mpg_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long prate)
> +{
Why is this implemented then?
> + return 0;
> +}
> +
> +static unsigned long mpg_recalc_rate(struct clk_hw *hw,
> + unsigned long prate)
> +{
> + struct m10v_pll *mpg = to_m10v_pll(hw);
> + unsigned long long rate = prate;
> +
> + if (mpg_is_enabled(hw)) {
> + rate = (unsigned long long)prate * mpg->mult;
> + do_div(rate, mpg->div);
> + }
> +
> + return (unsigned long)rate;
> +}
> +
> +static long mpg_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *prate)
> +{
> + struct m10v_pll *mpg = to_m10v_pll(hw);
> + unsigned long long temp_rate = (unsigned long long)*prate * mpg->mult;
> +
> + if (mpg->ro)
> + return mpg_recalc_rate(hw, *prate);
> +
> + return do_div(temp_rate, mpg->div);
There shouldn't be round_rate implemented at all if the device is
'read-only' or can't change frequency because set_rate op is empty.
> +}
> +
[..]
> +
> +static void mdc_set_div(struct m10v_div *mdc, u32 div)
> +{
> + u32 off, shift, val;
> +
> + off = mdc->offset / 32 * 4;
> + shift = mdc->offset % 32;
> +
> + val = readl(clk_base + CLKSEL1 + off);
> + val &= ~(mdc->mask << shift);
> + val |= (div << shift);
> + writel(val, clk_base + CLKSEL1 + off);
> +
> + if (mdc->waitdchreq) {
> + unsigned int count = 250;
> +
> + writel(1, clk_base + CLKSEL(11));
> +
> + do {
> + udelay(1);
> + } while (--count && readl(clk_base + CLKSEL(11)) & 1);
Use readl_poll_timeout()?
> +
> + if (!count)
> + pr_err("%s:%s CLK(%d) couldn't stabilize\n",
> + __func__, mdc->cname, mdc->offset);
> + }
> +}
> +
[...]
> +
> +void __init m10v_clk_gate_setup(struct device_node *node)
> +{
> + const char *clk_name = node->name;
> + struct clk_init_data init;
> + const char *parent_name;
> + struct m10v_gate *mgc;
> + struct clk *clk;
> + u32 offset;
> + int ret;
> +
> + if (!m10v_clk_iomap())
> + return;
> +
> + of_property_read_string(node, "clock-output-names", &clk_name);
> +
> + ret = of_property_read_u32(node, "offset", &offset);
> + if (ret) {
> + pr_err("%s: missing 'offset' property\n", clk_name);
> + return;
> + }
> +
> + parent_name = of_clk_get_parent_name(node, 0);
> +
> + mgc = kzalloc(sizeof(*mgc), GFP_KERNEL);
> + if (!mgc)
> + return;
> +
> + init.name = clk_name;
> + init.ops = &m10v_gate_ops;
> + init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
> + init.parent_names = &parent_name;
> + init.num_parents = 1;
> +
> + mgc->cname = clk_name;
> + mgc->offset = offset;
> + mgc->hw.init = &init;
> + if (of_get_property(node, "read-only", NULL))
> + mgc->ro = true;
> +
> + clk = clk_register(NULL, &mgc->hw);
Please use clk_hw based registration and provider APIs.
> + if (IS_ERR(clk))
> + kfree(mgc);
> + else
> + of_clk_add_provider(node, of_clk_src_simple_get, clk);
> +}
> +CLK_OF_DECLARE(m10v_clk_gate, "socionext,milbeaut-m10v-clk-gate",
> + m10v_clk_gate_setup);
> --
I suspect this driver will significantly change so I'm not reviewing
any further until it's sent again.
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 07/14] clock: milbeaut: Add Milbeaut M10V clock control
2018-11-30 8:31 ` Stephen Boyd
@ 2018-12-04 8:26 ` Sugaya, Taichi
2018-12-04 18:15 ` Stephen Boyd
2018-12-04 11:03 ` Masahiro Yamada
2018-12-26 1:35 ` Sugaya, Taichi
2 siblings, 1 reply; 43+ messages in thread
From: Sugaya, Taichi @ 2018-12-04 8:26 UTC (permalink / raw)
To: Stephen Boyd, devicetree, linux-arm-kernel, linux-clk,
linux-kernel, linux-serial
Cc: Michael Turquette, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar
Hi,
Thank you for your comments.
On 2018/11/30 17:31, Stephen Boyd wrote:
> Quoting Sugaya Taichi (2018-11-18 17:01:12)
>> Add Milbeaut M10V clock ( including PLL ) control.
>
> Please give some more details here.
OK, add more description.
>
>>
>> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
>> ---
>> drivers/clk/Makefile | 1 +
>> drivers/clk/clk-m10v.c | 671 +++++++++++++++++++++++++++++++++++++++++++++++++
>
> And this is different from Uniphier? Maybe we need a socionext
> directory under drivers/clk/.
Yes, M10V is a one of the Milbeaut series ( not Uniphier ).
Anyway, I will talk to Uniphier team about creating a socionext directory.
>
>> 2 files changed, 672 insertions(+)
>> create mode 100644 drivers/clk/clk-m10v.c
>>
>> diff --git a/drivers/clk/clk-m10v.c b/drivers/clk/clk-m10v.c
>> new file mode 100644
>> index 0000000..aa92a69
>> --- /dev/null
>> +++ b/drivers/clk/clk-m10v.c
>> @@ -0,0 +1,671 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2018 Socionext Inc.
>> + * Copyright (C) 2016 Linaro Ltd.
>> + *
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/clkdev.h>
>
> Is this include used?
I will check and drop if they are not used.
>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of.h>
>> +#include <linux/spinlock.h>
>> +
>> +#define CLKSEL1 0x0
>> +#define CLKSEL(n) (((n) - 1) * 4 + CLKSEL1)
>> +
>> +#define PLLCNT1 0x30
>> +#define PLLCNT(n) (((n) - 1) * 4 + PLLCNT1)
>> +
>> +#define CLKSTOP1 0x54
>> +#define CLKSTOP(n) (((n) - 1) * 4 + CLKSTOP1)
>> +
>> +#define CRSWR 0x8c
>> +#define CRRRS 0x90
>> +#define CRRSM 0x94
>> +
>> +#define to_m10v_mux(_hw) container_of(_hw, struct m10v_mux, hw)
>> +#define to_m10v_gate(_hw) container_of(_hw, struct m10v_gate, hw)
>> +#define to_m10v_div(_hw) container_of(_hw, struct m10v_div, hw)
>> +#define to_m10v_pll(_hw) container_of(_hw, struct m10v_pll, hw)
>> +
>> +static void __iomem *clk_base;
>> +static struct device_node *np_top;
>> +static DEFINE_SPINLOCK(crglock);
>
> Please make more specific names for these global variables by prefixing
> with m10v_. Also consider getting rid of the iomem and np_top globals
> entirely and associate those with clks differently.
I got it.
>
>> +
>> +static __init void __iomem *m10v_clk_iomap(void)
>> +{
>> + if (clk_base)
>> + return clk_base;
>> +
>> + np_top = of_find_compatible_node(NULL, NULL,
>> + "socionext,milbeaut-m10v-clk-regs");
>> + if (!np_top) {
>> + pr_err("%s: CLK iomap failed!\n", __func__);
>
> We haven't iomapped yet though.
Yes.
>
>> + return NULL;
>> + }
>> +
>> + clk_base = of_iomap(np_top, 0);
>> + of_node_put(np_top);
>
> Would be nicer to use platform_device APIs instead of OF ones.
OK, use platform_device APIs.
>
>> +
>> + return clk_base;
>> +}
>> +
>> +struct m10v_mux {
>> + struct clk_hw hw;
>> + const char *cname;
>> + u32 parent;
>> +};
>> +
>> +static u8 m10v_mux_get_parent(struct clk_hw *hw)
>> +{
>> + struct m10v_mux *mcm = to_m10v_mux(hw);
>> + struct clk_hw *parent;
>> + int i;
>> +
>> + i = clk_hw_get_num_parents(hw);
>> + while (i--) {
>> + parent = clk_hw_get_parent_by_index(hw, i);
>> + if (clk_hw_get_rate(parent))
>> + break;
>> + }
>> +
>> + if (i < 0) {
>> + pr_info("%s:%s no parent?!\n",
>> + __func__, mcm->cname);
>> + i = 0;
>> + }
>> +
>> + return i;
>> +}
>> +
>> +static int m10v_mux_set_parent(struct clk_hw *hw, u8 index)
>> +{
>> + struct m10v_mux *mcm = to_m10v_mux(hw);
>> +
>> + mcm->parent = index;
>> + return 0;
>> +}
>> +
>> +static const struct clk_ops m10v_mux_ops = {
>> + .get_parent = m10v_mux_get_parent,
>> + .set_parent = m10v_mux_set_parent,
>> + .determine_rate = __clk_mux_determine_rate,
>> +};
>> +
>> +void __init m10v_clk_mux_setup(struct device_node *node)
>> +{
>> + const char *clk_name = node->name;
>> + struct clk_init_data init;
>> + const char **parent_names;
>> + struct m10v_mux *mcm;
>> + struct clk *clk;
>> + int i, parents;
>> +
>> + if (!m10v_clk_iomap())
>> + return;
>> +
>> + of_property_read_string(node, "clock-output-names", &clk_name);
>> +
>> + parents = of_clk_get_parent_count(node);
>> + if (parents < 2) {
>> + pr_err("%s: not a mux\n", clk_name);
>
> How is this possible?
When the node has more than 1 clks...
Or I am misunderstanding your question?
>
>> + return;
>> + }
>> +
>> + parent_names = kzalloc((sizeof(char *) * parents), GFP_KERNEL);
>> + if (!parent_names)
>> + return;
>> +
>> + for (i = 0; i < parents; i++)
>> + parent_names[i] = of_clk_get_parent_name(node, i);
>
> This is of_clk_parent_fill().
OK, use it instead.
>
>> +
>> + mcm = kzalloc(sizeof(*mcm), GFP_KERNEL);
>> + if (!mcm)
>> + goto err_mcm;
>> +
>> + init.name = clk_name;
>> + init.ops = &m10v_mux_ops;
>> + init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
>
> Please don't use CLK_IS_BASIC unless you need it.
OK, confirm it.
>
>> + init.num_parents = parents;
>> + init.parent_names = parent_names;
>> +
>> + mcm->cname = clk_name;
>> + mcm->parent = 0;
>> + mcm->hw.init = &init;
>> +
>> + clk = clk_register(NULL, &mcm->hw);
>> + if (IS_ERR(clk))
>> + goto err_clk;
>> +
>> + of_clk_add_provider(node, of_clk_src_simple_get, clk);
>> + return;
>> +
>> +err_clk:
>> + kfree(mcm);
>> +err_mcm:
>> + kfree(parent_names);
>> +}
>> +CLK_OF_DECLARE(m10v_clk_mux, "socionext,milbeaut-m10v-clk-mux",
>> + m10v_clk_mux_setup);
>
> Any chance you can use a platform driver?
OK. try to use platform driver.
>
>> +
>> +struct m10v_pll {
>> + struct clk_hw hw;
>> + const char *cname;
>> + const struct clk_ops ops;
>> + u32 offset;
>> + u32 div, mult;
>> + bool ro;
>> +};
>> +
>> +#define ST 1
>> +#define SEL 2
>> +
>> +static void _mpg_enable(struct clk_hw *hw, unsigned int enable)
>> +{
>> + struct m10v_pll *mpg = to_m10v_pll(hw);
>> + unsigned long flags;
>> + u32 val;
>> +
>> + if (mpg->ro) {
>> + pr_debug("%s:%d %s: read-only\n",
>> + __func__, __LINE__, mpg->cname);
>> + return;
>> + }
>> +
>> + spin_lock_irqsave(&crglock, flags);
>> +
>> + val = readl(clk_base + PLLCNT(SEL));
>> + if (enable)
>> + val |= BIT(mpg->offset);
>> + else
>> + val &= ~BIT(mpg->offset);
>> + writel(val, clk_base + PLLCNT(SEL));
>> +
>> + spin_unlock_irqrestore(&crglock, flags);
>> +}
>> +
>> +static int mpg_enable(struct clk_hw *hw)
>> +{
>> + _mpg_enable(hw, 1);
>> + return 0;
>> +}
>> +
>> +static void mpg_disable(struct clk_hw *hw)
>> +{
>> + _mpg_enable(hw, 0);
>> +}
>> +
>> +static int mpg_is_enabled(struct clk_hw *hw)
>> +{
>> + struct m10v_pll *mpg = to_m10v_pll(hw);
>> +
>> + return readl(clk_base + PLLCNT(SEL)) & (1 << mpg->offset);
>> +}
>> +
>> +static void _mpg_prepare(struct clk_hw *hw, unsigned int on)
>> +{
>> + struct m10v_pll *mpg = to_m10v_pll(hw);
>> + unsigned long flags;
>> + u32 val;
>> +
>> + if (mpg->ro) {
>
> Should have different RO ops for read-only clks.
I got it.
>
>> + pr_debug("%s:%d %s: read-only\n",
>> + __func__, __LINE__, mpg->cname);
>> + return;
>> + }
>> +
>> + val = readl(clk_base + PLLCNT(ST));
>> + if (!on == !(val & BIT(mpg->offset)))
>> + return;
>> +
>> + /* disable */
>
> Please remove obvious comments.
Oops, OK remove.
>
>> + mpg_disable(hw);
>> +
>> + spin_lock_irqsave(&crglock, flags);
>> +
>> + val = readl(clk_base + PLLCNT(ST));
>> + if (on)
>> + val |= BIT(mpg->offset);
>> + else
>> + val &= ~BIT(mpg->offset);
>> + writel(val, clk_base + PLLCNT(ST));
>> +
>> + spin_unlock_irqrestore(&crglock, flags);
>> +
>> + udelay(on ? 200 : 10);
>> +}
>> +
>> +static int mpg_prepare(struct clk_hw *hw)
>> +{
>> + _mpg_prepare(hw, 1);
>> + return 0;
>> +}
>> +
>> +static void mpg_unprepare(struct clk_hw *hw)
>> +{
>> + _mpg_prepare(hw, 0);
>> +}
>> +
>> +static int mpg_set_rate(struct clk_hw *hw, unsigned long rate,
>> + unsigned long prate)
>> +{
>
> Why is this implemented then?
This is not necessary maybe. so consider whether getting rid of it.
>
>> + return 0;
>> +}
>> +
>> +static unsigned long mpg_recalc_rate(struct clk_hw *hw,
>> + unsigned long prate)
>> +{
>> + struct m10v_pll *mpg = to_m10v_pll(hw);
>> + unsigned long long rate = prate;
>> +
>> + if (mpg_is_enabled(hw)) {
>> + rate = (unsigned long long)prate * mpg->mult;
>> + do_div(rate, mpg->div);
>> + }
>> +
>> + return (unsigned long)rate;
>> +}
>> +
>> +static long mpg_round_rate(struct clk_hw *hw, unsigned long rate,
>> + unsigned long *prate)
>> +{
>> + struct m10v_pll *mpg = to_m10v_pll(hw);
>> + unsigned long long temp_rate = (unsigned long long)*prate * mpg->mult;
>> +
>> + if (mpg->ro)
>> + return mpg_recalc_rate(hw, *prate);
>> +
>> + return do_div(temp_rate, mpg->div);
>
> There shouldn't be round_rate implemented at all if the device is
> 'read-only' or can't change frequency because set_rate op is empty.
I understand. I will describe correctly.
>
>> +}
>> +
> [..]
>> +
>> +static void mdc_set_div(struct m10v_div *mdc, u32 div)
>> +{
>> + u32 off, shift, val;
>> +
>> + off = mdc->offset / 32 * 4;
>> + shift = mdc->offset % 32;
>> +
>> + val = readl(clk_base + CLKSEL1 + off);
>> + val &= ~(mdc->mask << shift);
>> + val |= (div << shift);
>> + writel(val, clk_base + CLKSEL1 + off);
>> +
>> + if (mdc->waitdchreq) {
>> + unsigned int count = 250;
>> +
>> + writel(1, clk_base + CLKSEL(11));
>> +
>> + do {
>> + udelay(1);
>> + } while (--count && readl(clk_base + CLKSEL(11)) & 1);
>
> Use readl_poll_timeout()?
OK. use it instead.
>
>> +
>> + if (!count)
>> + pr_err("%s:%s CLK(%d) couldn't stabilize\n",
>> + __func__, mdc->cname, mdc->offset);
>> + }
>> +}
>> +
> [...]
>> +
>> +void __init m10v_clk_gate_setup(struct device_node *node)
>> +{
>> + const char *clk_name = node->name;
>> + struct clk_init_data init;
>> + const char *parent_name;
>> + struct m10v_gate *mgc;
>> + struct clk *clk;
>> + u32 offset;
>> + int ret;
>> +
>> + if (!m10v_clk_iomap())
>> + return;
>> +
>> + of_property_read_string(node, "clock-output-names", &clk_name);
>> +
>> + ret = of_property_read_u32(node, "offset", &offset);
>> + if (ret) {
>> + pr_err("%s: missing 'offset' property\n", clk_name);
>> + return;
>> + }
>> +
>> + parent_name = of_clk_get_parent_name(node, 0);
>> +
>> + mgc = kzalloc(sizeof(*mgc), GFP_KERNEL);
>> + if (!mgc)
>> + return;
>> +
>> + init.name = clk_name;
>> + init.ops = &m10v_gate_ops;
>> + init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
>> + init.parent_names = &parent_name;
>> + init.num_parents = 1;
>> +
>> + mgc->cname = clk_name;
>> + mgc->offset = offset;
>> + mgc->hw.init = &init;
>> + if (of_get_property(node, "read-only", NULL))
>> + mgc->ro = true;
>> +
>> + clk = clk_register(NULL, &mgc->hw);
>
> Please use clk_hw based registration and provider APIs.
I got it. I try to describe with referring other drivers.
>
>> + if (IS_ERR(clk))
>> + kfree(mgc);
>> + else
>> + of_clk_add_provider(node, of_clk_src_simple_get, clk);
>> +}
>> +CLK_OF_DECLARE(m10v_clk_gate, "socionext,milbeaut-m10v-clk-gate",
>> + m10v_clk_gate_setup);
>> --
>
> I suspect this driver will significantly change so I'm not reviewing
> any further until it's sent again.
I understand. I study and try to renew the driver.
>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 07/14] clock: milbeaut: Add Milbeaut M10V clock control
2018-12-04 8:26 ` Sugaya, Taichi
@ 2018-12-04 18:15 ` Stephen Boyd
2018-12-05 11:42 ` Sugaya, Taichi
0 siblings, 1 reply; 43+ messages in thread
From: Stephen Boyd @ 2018-12-04 18:15 UTC (permalink / raw)
To: Sugaya, Taichi, devicetree, linux-arm-kernel, linux-clk,
linux-kernel, linux-serial
Cc: Michael Turquette, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar
Quoting Sugaya, Taichi (2018-12-04 00:26:16)
> On 2018/11/30 17:31, Stephen Boyd wrote:
> > Quoting Sugaya Taichi (2018-11-18 17:01:12)
> >> +void __init m10v_clk_mux_setup(struct device_node *node)
> >> +{
> >> + const char *clk_name = node->name;
> >> + struct clk_init_data init;
> >> + const char **parent_names;
> >> + struct m10v_mux *mcm;
> >> + struct clk *clk;
> >> + int i, parents;
> >> +
> >> + if (!m10v_clk_iomap())
> >> + return;
> >> +
> >> + of_property_read_string(node, "clock-output-names", &clk_name);
> >> +
> >> + parents = of_clk_get_parent_count(node);
> >> + if (parents < 2) {
> >> + pr_err("%s: not a mux\n", clk_name);
> >
> > How is this possible?
>
> When the node has more than 1 clks...
> Or I am misunderstanding your question?
This looks like code that's checking DT for correctness. We don't
typically do that in the kernel because the kernel isn't a DT validator.
That's all I'm saying. I think this comment is not useful if the driver
design is done to specify parent linkages in C code instead of DT, so
don't worry about this too much.
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 07/14] clock: milbeaut: Add Milbeaut M10V clock control
2018-12-04 18:15 ` Stephen Boyd
@ 2018-12-05 11:42 ` Sugaya, Taichi
0 siblings, 0 replies; 43+ messages in thread
From: Sugaya, Taichi @ 2018-12-05 11:42 UTC (permalink / raw)
To: Stephen Boyd, devicetree, linux-arm-kernel, linux-clk,
linux-kernel, linux-serial
Cc: Michael Turquette, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar
Hi
On 2018/12/05 3:15, Stephen Boyd wrote:
> Quoting Sugaya, Taichi (2018-12-04 00:26:16)
>> On 2018/11/30 17:31, Stephen Boyd wrote:
>>> Quoting Sugaya Taichi (2018-11-18 17:01:12)
>>>> +void __init m10v_clk_mux_setup(struct device_node *node)
>>>> +{
>>>> + const char *clk_name = node->name;
>>>> + struct clk_init_data init;
>>>> + const char **parent_names;
>>>> + struct m10v_mux *mcm;
>>>> + struct clk *clk;
>>>> + int i, parents;
>>>> +
>>>> + if (!m10v_clk_iomap())
>>>> + return;
>>>> +
>>>> + of_property_read_string(node, "clock-output-names", &clk_name);
>>>> +
>>>> + parents = of_clk_get_parent_count(node);
>>>> + if (parents < 2) {
>>>> + pr_err("%s: not a mux\n", clk_name);
>>>
>>> How is this possible?
>>
>> When the node has more than 1 clks...
>> Or I am misunderstanding your question?
>
> This looks like code that's checking DT for correctness. We don't
> typically do that in the kernel because the kernel isn't a DT validator.
> That's all I'm saying. I think this comment is not useful if the driver
> design is done to specify parent linkages in C code instead of DT, so
> don't worry about this too much.
I understand.
Thank you for additional information.
Sugaya Taichi
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 07/14] clock: milbeaut: Add Milbeaut M10V clock control
2018-11-30 8:31 ` Stephen Boyd
2018-12-04 8:26 ` Sugaya, Taichi
@ 2018-12-04 11:03 ` Masahiro Yamada
2018-12-04 18:14 ` Stephen Boyd
2018-12-26 1:35 ` Sugaya, Taichi
2 siblings, 1 reply; 43+ messages in thread
From: Masahiro Yamada @ 2018-12-04 11:03 UTC (permalink / raw)
To: Stephen Boyd
Cc: sugaya.taichi, DTML, linux-arm-kernel, linux-clk,
Linux Kernel Mailing List, linux-serial, Michael Turquette,
Rob Herring, Mark Rutland, Greg Kroah-Hartman, Daniel Lezcano,
Thomas Gleixner, Russell King, Jiri Slaby, Masami Hiramatsu,
Jassi Brar
Hi Stephen,
On Fri, Nov 30, 2018 at 5:31 PM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Sugaya Taichi (2018-11-18 17:01:12)
> > Add Milbeaut M10V clock ( including PLL ) control.
>
> Please give some more details here.
>
> >
> > Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
> > ---
> > drivers/clk/Makefile | 1 +
> > drivers/clk/clk-m10v.c | 671 +++++++++++++++++++++++++++++++++++++++++++++++++
>
> And this is different from Uniphier? Maybe we need a socionext
> directory under drivers/clk/.
This is always a difficult question,
and I do not have a strong opinion.
I am fine with moving the files to drivers/clk/socionext
although no file would be shared.
FYI
UniPhier and Milbeaut are completely different platforms
developed/maintained by different teams.
They happen to live in the same company now
just because Socionext merged the LSI business from Panasonic and Fujitsu.
UniPhier originates in Panasonic, while Milbeaut in Fujitsu.
Thanks.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 07/14] clock: milbeaut: Add Milbeaut M10V clock control
2018-12-04 11:03 ` Masahiro Yamada
@ 2018-12-04 18:14 ` Stephen Boyd
2018-12-05 4:26 ` Masahiro Yamada
0 siblings, 1 reply; 43+ messages in thread
From: Stephen Boyd @ 2018-12-04 18:14 UTC (permalink / raw)
To: Masahiro Yamada
Cc: sugaya.taichi, DTML, linux-arm-kernel, linux-clk,
Linux Kernel Mailing List, linux-serial, Michael Turquette,
Rob Herring, Mark Rutland, Greg Kroah-Hartman, Daniel Lezcano,
Thomas Gleixner, Russell King, Jiri Slaby, Masami Hiramatsu,
Jassi Brar
Quoting Masahiro Yamada (2018-12-04 03:03:53)
> Hi Stephen,
>
>
> On Fri, Nov 30, 2018 at 5:31 PM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Sugaya Taichi (2018-11-18 17:01:12)
> > > Add Milbeaut M10V clock ( including PLL ) control.
> >
> > Please give some more details here.
> >
> > >
> > > Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
> > > ---
> > > drivers/clk/Makefile | 1 +
> > > drivers/clk/clk-m10v.c | 671 +++++++++++++++++++++++++++++++++++++++++++++++++
> >
> > And this is different from Uniphier? Maybe we need a socionext
> > directory under drivers/clk/.
>
>
>
> This is always a difficult question,
> and I do not have a strong opinion.
>
>
> I am fine with moving the files to drivers/clk/socionext
> although no file would be shared.
>
>
> FYI
>
> UniPhier and Milbeaut are completely different platforms
> developed/maintained by different teams.
>
> They happen to live in the same company now
> just because Socionext merged the LSI business from Panasonic and Fujitsu.
>
> UniPhier originates in Panasonic, while Milbeaut in Fujitsu.
>
Thanks for the background info. I'd prefer to defer to however the dts
files are getting split up into directories. If they're all put under
arch/arm64/boot/dts/socionext/ then I would say combine the two clk
drivers into a socionext directory. Otherwise, keep them split out.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 07/14] clock: milbeaut: Add Milbeaut M10V clock control
2018-12-04 18:14 ` Stephen Boyd
@ 2018-12-05 4:26 ` Masahiro Yamada
2018-12-05 6:57 ` Stephen Boyd
0 siblings, 1 reply; 43+ messages in thread
From: Masahiro Yamada @ 2018-12-05 4:26 UTC (permalink / raw)
To: Stephen Boyd
Cc: sugaya.taichi, DTML, linux-arm-kernel, linux-clk,
Linux Kernel Mailing List, linux-serial, Michael Turquette,
Rob Herring, Mark Rutland, Greg Kroah-Hartman, Daniel Lezcano,
Thomas Gleixner, Russell King, Jiri Slaby, Masami Hiramatsu,
Jassi Brar
On Wed, Dec 5, 2018 at 3:14 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Masahiro Yamada (2018-12-04 03:03:53)
> > Hi Stephen,
> >
> >
> > On Fri, Nov 30, 2018 at 5:31 PM Stephen Boyd <sboyd@kernel.org> wrote:
> > >
> > > Quoting Sugaya Taichi (2018-11-18 17:01:12)
> > > > Add Milbeaut M10V clock ( including PLL ) control.
> > >
> > > Please give some more details here.
> > >
> > > >
> > > > Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
> > > > ---
> > > > drivers/clk/Makefile | 1 +
> > > > drivers/clk/clk-m10v.c | 671 +++++++++++++++++++++++++++++++++++++++++++++++++
> > >
> > > And this is different from Uniphier? Maybe we need a socionext
> > > directory under drivers/clk/.
> >
> >
> >
> > This is always a difficult question,
> > and I do not have a strong opinion.
> >
> >
> > I am fine with moving the files to drivers/clk/socionext
> > although no file would be shared.
> >
> >
> > FYI
> >
> > UniPhier and Milbeaut are completely different platforms
> > developed/maintained by different teams.
> >
> > They happen to live in the same company now
> > just because Socionext merged the LSI business from Panasonic and Fujitsu.
> >
> > UniPhier originates in Panasonic, while Milbeaut in Fujitsu.
> >
>
> Thanks for the background info. I'd prefer to defer to however the dts
> files are getting split up into directories. If they're all put under
> arch/arm64/boot/dts/socionext/ then I would say combine the two clk
> drivers into a socionext directory. Otherwise, keep them split out.
If you want to align with the DT directory structure,
the answer is clear.
Milbeaut DT files will be put together with UniPhier ones
into socionext directory.
For arm64, DT directories are already sorted out by vendors.
Even 32-bit ARM is going to that way.
Rob Herring just posted a python script
to move all DT files in arch/arm/boot/dts/
into vendor subdirectories.
Please let me know if you want me to
move drivers/clk/uniphier/* to drivers/clk/socionext/*.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 07/14] clock: milbeaut: Add Milbeaut M10V clock control
2018-12-05 4:26 ` Masahiro Yamada
@ 2018-12-05 6:57 ` Stephen Boyd
0 siblings, 0 replies; 43+ messages in thread
From: Stephen Boyd @ 2018-12-05 6:57 UTC (permalink / raw)
To: Masahiro Yamada
Cc: sugaya.taichi, DTML, linux-arm-kernel, linux-clk,
Linux Kernel Mailing List, linux-serial, Michael Turquette,
Rob Herring, Mark Rutland, Greg Kroah-Hartman, Daniel Lezcano,
Thomas Gleixner, Russell King, Jiri Slaby, Masami Hiramatsu,
Jassi Brar
Quoting Masahiro Yamada (2018-12-04 20:26:06)
> On Wed, Dec 5, 2018 at 3:14 AM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Masahiro Yamada (2018-12-04 03:03:53)
> > > Hi Stephen,
> > >
> > >
> > > On Fri, Nov 30, 2018 at 5:31 PM Stephen Boyd <sboyd@kernel.org> wrote:
> > > >
> > > > Quoting Sugaya Taichi (2018-11-18 17:01:12)
> > > > > Add Milbeaut M10V clock ( including PLL ) control.
> > > >
> > > > Please give some more details here.
> > > >
> > > > >
> > > > > Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
> > > > > ---
> > > > > drivers/clk/Makefile | 1 +
> > > > > drivers/clk/clk-m10v.c | 671 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > >
> > > > And this is different from Uniphier? Maybe we need a socionext
> > > > directory under drivers/clk/.
> > >
> > >
> > >
> > > This is always a difficult question,
> > > and I do not have a strong opinion.
> > >
> > >
> > > I am fine with moving the files to drivers/clk/socionext
> > > although no file would be shared.
> > >
> > >
> > > FYI
> > >
> > > UniPhier and Milbeaut are completely different platforms
> > > developed/maintained by different teams.
> > >
> > > They happen to live in the same company now
> > > just because Socionext merged the LSI business from Panasonic and Fujitsu.
> > >
> > > UniPhier originates in Panasonic, while Milbeaut in Fujitsu.
> > >
> >
> > Thanks for the background info. I'd prefer to defer to however the dts
> > files are getting split up into directories. If they're all put under
> > arch/arm64/boot/dts/socionext/ then I would say combine the two clk
> > drivers into a socionext directory. Otherwise, keep them split out.
>
>
> If you want to align with the DT directory structure,
> the answer is clear.
>
>
> Milbeaut DT files will be put together with UniPhier ones
> into socionext directory.
>
>
> For arm64, DT directories are already sorted out by vendors.
>
> Even 32-bit ARM is going to that way.
>
> Rob Herring just posted a python script
> to move all DT files in arch/arm/boot/dts/
> into vendor subdirectories.
>
>
> Please let me know if you want me to
> move drivers/clk/uniphier/* to drivers/clk/socionext/*.
>
Maybe the dts needs to be split up instead? Looks like the gpio drivers
are in a uniphier directory and there is some precedence to keep the
"taken over" company name when vendors are merged into other vendors.
Maybe that's how things have happened here? It would be nice to be
consistent, but I leave the decision up to you to figure out if that
really matters to you. I'll be fine either way.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 07/14] clock: milbeaut: Add Milbeaut M10V clock control
2018-11-30 8:31 ` Stephen Boyd
2018-12-04 8:26 ` Sugaya, Taichi
2018-12-04 11:03 ` Masahiro Yamada
@ 2018-12-26 1:35 ` Sugaya, Taichi
2018-12-28 0:39 ` Stephen Boyd
2 siblings, 1 reply; 43+ messages in thread
From: Sugaya, Taichi @ 2018-12-26 1:35 UTC (permalink / raw)
To: Stephen Boyd, devicetree, linux-arm-kernel, linux-clk,
linux-kernel, linux-serial
Cc: Michael Turquette, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar
Hi
On 2018/11/30 17:31, Stephen Boyd wrote:
>> + init.num_parents = parents;
>> + init.parent_names = parent_names;
>> +
>> + mcm->cname = clk_name;
>> + mcm->parent = 0;
>> + mcm->hw.init = &init;
>> +
>> + clk = clk_register(NULL, &mcm->hw);
>> + if (IS_ERR(clk))
>> + goto err_clk;
>> +
>> + of_clk_add_provider(node, of_clk_src_simple_get, clk);
>> + return;
>> +
>> +err_clk:
>> + kfree(mcm);
>> +err_mcm:
>> + kfree(parent_names);
>> +}
>> +CLK_OF_DECLARE(m10v_clk_mux, "socionext,milbeaut-m10v-clk-mux",
>> + m10v_clk_mux_setup);
>
> Any chance you can use a platform driver?
>
Excuse me to re-ask you.
Why do you recommend to use a platform driver? Is that current fad?
Thanks
Sugaya Taichi
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 07/14] clock: milbeaut: Add Milbeaut M10V clock control
2018-12-26 1:35 ` Sugaya, Taichi
@ 2018-12-28 0:39 ` Stephen Boyd
2018-12-28 6:38 ` Sugaya, Taichi
0 siblings, 1 reply; 43+ messages in thread
From: Stephen Boyd @ 2018-12-28 0:39 UTC (permalink / raw)
To: Sugaya, Taichi, devicetree, linux-arm-kernel, linux-clk,
linux-kernel, linux-serial
Cc: Michael Turquette, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar
Quoting Sugaya, Taichi (2018-12-25 17:35:27)
> Hi
>
> On 2018/11/30 17:31, Stephen Boyd wrote:
> >> + init.num_parents = parents;
> >> + init.parent_names = parent_names;
> >> +
> >> + mcm->cname = clk_name;
> >> + mcm->parent = 0;
> >> + mcm->hw.init = &init;
> >> +
> >> + clk = clk_register(NULL, &mcm->hw);
> >> + if (IS_ERR(clk))
> >> + goto err_clk;
> >> +
> >> + of_clk_add_provider(node, of_clk_src_simple_get, clk);
> >> + return;
> >> +
> >> +err_clk:
> >> + kfree(mcm);
> >> +err_mcm:
> >> + kfree(parent_names);
> >> +}
> >> +CLK_OF_DECLARE(m10v_clk_mux, "socionext,milbeaut-m10v-clk-mux",
> >> + m10v_clk_mux_setup);
> >
> > Any chance you can use a platform driver?
> >
>
> Excuse me to re-ask you.
> Why do you recommend to use a platform driver? Is that current fad?
Not exactly a fad. We've been doing it for some time now. From an older
email on the list:
Reasons (in no particular order):
1. We get a dev pointer to use with clk_hw_register()
2. We can handle probe defer if some resource is not available
3. Using device model gets us a hook into power management frameworks
like runtime PM and system PM for things like suspend and hibernate
4. It encourages a single DT node clk controller style binding
instead of a single node per clk style binding
5. We can use non-DT specific functions like devm_ioremap_resource() to map
registers and acquire other resources, leading to more portable and
generic code
6. We may be able to make the device driver a module, which will
make distros happy if we don't have to compile in all
these clk drivers to the resulting vmlinux
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 07/14] clock: milbeaut: Add Milbeaut M10V clock control
2018-12-28 0:39 ` Stephen Boyd
@ 2018-12-28 6:38 ` Sugaya, Taichi
0 siblings, 0 replies; 43+ messages in thread
From: Sugaya, Taichi @ 2018-12-28 6:38 UTC (permalink / raw)
To: Stephen Boyd, devicetree, linux-arm-kernel, linux-clk,
linux-kernel, linux-serial
Cc: Michael Turquette, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
Daniel Lezcano, Thomas Gleixner, Russell King, Jiri Slaby,
Masami Hiramatsu, Jassi Brar
Hi
On 2018/12/28 9:39, Stephen Boyd wrote:
> Quoting Sugaya, Taichi (2018-12-25 17:35:27)
>> Hi
>>
>> On 2018/11/30 17:31, Stephen Boyd wrote:
>>>> + init.num_parents = parents;
>>>> + init.parent_names = parent_names;
>>>> +
>>>> + mcm->cname = clk_name;
>>>> + mcm->parent = 0;
>>>> + mcm->hw.init = &init;
>>>> +
>>>> + clk = clk_register(NULL, &mcm->hw);
>>>> + if (IS_ERR(clk))
>>>> + goto err_clk;
>>>> +
>>>> + of_clk_add_provider(node, of_clk_src_simple_get, clk);
>>>> + return;
>>>> +
>>>> +err_clk:
>>>> + kfree(mcm);
>>>> +err_mcm:
>>>> + kfree(parent_names);
>>>> +}
>>>> +CLK_OF_DECLARE(m10v_clk_mux, "socionext,milbeaut-m10v-clk-mux",
>>>> + m10v_clk_mux_setup);
>>>
>>> Any chance you can use a platform driver?
>>>
>>
>> Excuse me to re-ask you.
>> Why do you recommend to use a platform driver? Is that current fad?
>
> Not exactly a fad. We've been doing it for some time now. From an older
> email on the list:
>
> Reasons (in no particular order):
>
> 1. We get a dev pointer to use with clk_hw_register()
>
> 2. We can handle probe defer if some resource is not available
>
> 3. Using device model gets us a hook into power management frameworks
> like runtime PM and system PM for things like suspend and hibernate
>
> 4. It encourages a single DT node clk controller style binding
> instead of a single node per clk style binding
>
> 5. We can use non-DT specific functions like devm_ioremap_resource() to map
> registers and acquire other resources, leading to more portable and
> generic code
>
> 6. We may be able to make the device driver a module, which will
> make distros happy if we don't have to compile in all
> these clk drivers to the resulting vmlinux
>
Great thanks for answering. I strongly understand.
#It takes a bit of time to send v2.
Best Regards,
Sugaya Taichi
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 08/14] dt-bindings: serial: Add Milbeaut M10V serial description
2018-11-19 1:01 [PATCH 00/14] Add basic support for Socionext Milbeaut M10V SoCs Sugaya Taichi
` (6 preceding siblings ...)
2018-11-19 1:01 ` [PATCH 07/14] clock: milbeaut: Add Milbeaut M10V clock control Sugaya Taichi
@ 2018-11-19 1:01 ` Sugaya Taichi
2018-11-19 1:01 ` [PATCH 09/14] serial: Add Milbeaut M10V serial control Sugaya Taichi
8 siblings, 0 replies; 43+ messages in thread
From: Sugaya Taichi @ 2018-11-19 1:01 UTC (permalink / raw)
To: linux-clk, devicetree, linux-arm-kernel, linux-kernel, linux-serial
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Greg Kroah-Hartman, Daniel Lezcano, Thomas Gleixner,
Russell King, Jiri Slaby, Masami Hiramatsu, Jassi Brar,
Sugaya Taichi
Add DT bindings document for Milbeaut M10V serial.
Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
---
.../devicetree/bindings/serial/milbeaut-uart.txt | 23 ++++++++++++++++++++++
1 file changed, 23 insertions(+)
create mode 100644 Documentation/devicetree/bindings/serial/milbeaut-uart.txt
diff --git a/Documentation/devicetree/bindings/serial/milbeaut-uart.txt b/Documentation/devicetree/bindings/serial/milbeaut-uart.txt
new file mode 100644
index 0000000..19d1480
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/milbeaut-uart.txt
@@ -0,0 +1,23 @@
+Socionext Milbeaut UART controller
+
+Required properties:
+- index: must be 0
+- compatible: should be "socionext,milbeaut-m10v-usio-uart".
+- reg: offset and length of the register set for the device.
+- interrupts: two interrupts specifier.
+- clocks: phandle to the input clock.
+- interrupt-names: should be "rx", "tx".
+
+Optional properties:
+- uart-flow-enable: flow control enable.
+
+Example:
+ usio1: usio_uart@1e700010 {
+ index = <0>;
+ compatible = "socionext,milbeaut-m10v-usio-uart";
+ reg = <0x1e700010 0x10>;
+ interrupts = <0 141 0x4>, <0 149 0x4>;
+ interrupt-names = "rx", "tx";
+ clocks = <&hclk>;
+ uart-flow-enable;
+ };
--
1.9.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 09/14] serial: Add Milbeaut M10V serial control
2018-11-19 1:01 [PATCH 00/14] Add basic support for Socionext Milbeaut M10V SoCs Sugaya Taichi
` (7 preceding siblings ...)
2018-11-19 1:01 ` [PATCH 08/14] dt-bindings: serial: Add Milbeaut M10V serial description Sugaya Taichi
@ 2018-11-19 1:01 ` Sugaya Taichi
8 siblings, 0 replies; 43+ messages in thread
From: Sugaya Taichi @ 2018-11-19 1:01 UTC (permalink / raw)
To: linux-clk, devicetree, linux-arm-kernel, linux-kernel, linux-serial
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Greg Kroah-Hartman, Daniel Lezcano, Thomas Gleixner,
Russell King, Jiri Slaby, Masami Hiramatsu, Jassi Brar,
Sugaya Taichi
Add Milbeaut M10V serial control.
Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
---
drivers/tty/serial/Kconfig | 24 ++
drivers/tty/serial/Makefile | 1 +
drivers/tty/serial/m10v_usio.c | 605 +++++++++++++++++++++++++++++++++++++++
include/uapi/linux/serial_core.h | 3 +
4 files changed, 633 insertions(+)
create mode 100644 drivers/tty/serial/m10v_usio.c
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 32886c3..cd28a7e 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1529,6 +1529,30 @@ config SERIAL_OWL_CONSOLE
Say 'Y' here if you wish to use Actions Semiconductor S500/S900 UART
as the system console.
+config SERIAL_M10V_USIO
+ tristate "M10V USIO/UART serial port support"
+ depends on ARCH_MILBEAUT
+ default y
+ select SERIAL_CORE
+ help
+ This selects the USIO/UART IP found in Socionext Milbeaut M10V.
+
+config SERIAL_M10V_USIO_PORTS
+ int "Maximum number of CSIO/UART ports (1-8)"
+ range 1 8
+ depends on SERIAL_M10V_USIO
+ default "4"
+
+config SERIAL_M10V_USIO_CONSOLE
+ bool "Support for console on M10V USIO/UART serial port"
+ depends on SERIAL_M10V_USIO=y
+ select SERIAL_CORE_CONSOLE
+ help
+ Say 'Y' here if you wish to use a USIO/UART of Socionext Milbeaut
+ M10V as the system console (the system console is the device which
+ receives all kernel messages and warnings and which allows logins in
+ single user mode).
+
endmenu
config SERIAL_MCTRL_GPIO
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index daac675..5ea46bc 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -89,6 +89,7 @@ obj-$(CONFIG_SERIAL_MVEBU_UART) += mvebu-uart.o
obj-$(CONFIG_SERIAL_PIC32) += pic32_uart.o
obj-$(CONFIG_SERIAL_MPS2_UART) += mps2-uart.o
obj-$(CONFIG_SERIAL_OWL) += owl-uart.o
+obj-$(CONFIG_SERIAL_M10V_USIO) += m10v_usio.o
# GPIOLIB helpers for modem control lines
obj-$(CONFIG_SERIAL_MCTRL_GPIO) += serial_mctrl_gpio.o
diff --git a/drivers/tty/serial/m10v_usio.c b/drivers/tty/serial/m10v_usio.c
new file mode 100644
index 0000000..3abb465
--- /dev/null
+++ b/drivers/tty/serial/m10v_usio.c
@@ -0,0 +1,605 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Socionext Inc.
+ */
+
+#if defined(CONFIG_SERIAL_M10V_USIO_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
+#define SUPPORT_SYSRQ
+#endif
+
+#include <linux/clk.h>
+#include <linux/console.h>
+#include <linux/module.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/serial_core.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+
+
+#define USIO_NAME "sn-usio-uart"
+#define USIO_UART_DEV_NAME "ttyUSI"
+
+static struct uart_port usio_ports[CONFIG_SERIAL_M10V_USIO_PORTS];
+
+#define RX 0
+#define TX 1
+static int usio_irq[CONFIG_SERIAL_M10V_USIO_PORTS][2];
+
+#define SN_USIO_REG_SMR 0
+#define SN_USIO_REG_SCR 1
+#define SN_USIO_REG_ESCR 2
+#define SN_USIO_REG_SSR 3
+#define SN_USIO_REG_DR 4
+#define SN_USIO_REG_BGR 6
+#define SN_USIO_REG_FCR 12
+#define SN_USIO_REG_FBYTE 14
+
+#define SN_USIO_SMR_SOE BIT(0)
+#define SN_USIO_SMR_SBL BIT(3)
+#define SN_USIO_SCR_TXE BIT(0)
+#define SN_USIO_SCR_RXE BIT(1)
+#define SN_USIO_SCR_TBIE BIT(2)
+#define SN_USIO_SCR_TIE BIT(3)
+#define SN_USIO_SCR_RIE BIT(4)
+#define SN_USIO_SCR_UPCL BIT(7)
+#define SN_USIO_ESCR_L_8BIT 0
+#define SN_USIO_ESCR_L_5BIT 1
+#define SN_USIO_ESCR_L_6BIT 2
+#define SN_USIO_ESCR_L_7BIT 3
+#define SN_USIO_ESCR_P BIT(3)
+#define SN_USIO_ESCR_PEN BIT(4)
+#define SN_USIO_ESCR_FLWEN BIT(7)
+#define SN_USIO_SSR_TBI BIT(0)
+#define SN_USIO_SSR_TDRE BIT(1)
+#define SN_USIO_SSR_RDRF BIT(2)
+#define SN_USIO_SSR_ORE BIT(3)
+#define SN_USIO_SSR_FRE BIT(4)
+#define SN_USIO_SSR_PE BIT(5)
+#define SN_USIO_SSR_REC BIT(7)
+#define SN_USIO_SSR_BRK BIT(8)
+#define SN_USIO_FCR_FE1 BIT(0)
+#define SN_USIO_FCR_FE2 BIT(1)
+#define SN_USIO_FCR_FCL1 BIT(2)
+#define SN_USIO_FCR_FCL2 BIT(3)
+#define SN_USIO_FCR_FSET BIT(4)
+#define SN_USIO_FCR_FTIE BIT(9)
+#define SN_USIO_FCR_FDRQ BIT(10)
+#define SN_USIO_FCR_FRIIE BIT(11)
+
+static void usio_stop_tx(struct uart_port *port)
+{
+ writew(readw(port->membase + SN_USIO_REG_FCR) & ~SN_USIO_FCR_FTIE,
+ port->membase + SN_USIO_REG_FCR);
+ writeb(readb(port->membase + SN_USIO_REG_SCR) & ~SN_USIO_SCR_TBIE,
+ port->membase + SN_USIO_REG_SCR);
+}
+
+static void usio_tx_chars(struct uart_port *port)
+{
+ struct circ_buf *xmit = &port->state->xmit;
+ int count;
+
+ writew(readw(port->membase + SN_USIO_REG_FCR) & ~SN_USIO_FCR_FTIE,
+ port->membase + SN_USIO_REG_FCR);
+ writeb(readb(port->membase + SN_USIO_REG_SCR) &
+ ~(SN_USIO_SCR_TIE | SN_USIO_SCR_TBIE),
+ port->membase + SN_USIO_REG_SCR);
+
+ if (port->x_char) {
+ writew(port->x_char, port->membase + SN_USIO_REG_DR);
+ port->icount.tx++;
+ port->x_char = 0;
+ return;
+ }
+ if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
+ usio_stop_tx(port);
+ return;
+ }
+
+ count = port->fifosize -
+ (readw(port->membase + SN_USIO_REG_FBYTE) & 0xff);
+
+ do {
+ writew(xmit->buf[xmit->tail], port->membase + SN_USIO_REG_DR);
+
+ xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
+ port->icount.tx++;
+ if (uart_circ_empty(xmit))
+ break;
+
+ } while (--count > 0);
+
+ writew(readw(port->membase + SN_USIO_REG_FCR) & ~SN_USIO_FCR_FDRQ,
+ port->membase + SN_USIO_REG_FCR);
+
+ writeb(readb(port->membase + SN_USIO_REG_SCR) | SN_USIO_SCR_TBIE,
+ port->membase + SN_USIO_REG_SCR);
+
+ if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+ uart_write_wakeup(port);
+
+ if (uart_circ_empty(xmit))
+ usio_stop_tx(port);
+}
+
+static void usio_start_tx(struct uart_port *port)
+{
+ u16 fcr = readw(port->membase + SN_USIO_REG_FCR);
+
+ writew(fcr | SN_USIO_FCR_FTIE, port->membase + SN_USIO_REG_FCR);
+ if (!(fcr & SN_USIO_FCR_FDRQ))
+ return;
+
+ writeb(readb(port->membase + SN_USIO_REG_SCR) | SN_USIO_SCR_TBIE,
+ port->membase + SN_USIO_REG_SCR);
+
+ if (readb(port->membase + SN_USIO_REG_SSR) & SN_USIO_SSR_TBI)
+ usio_tx_chars(port);
+}
+
+static void usio_stop_rx(struct uart_port *port)
+{
+ writeb(readb(port->membase + SN_USIO_REG_SCR) & ~SN_USIO_SCR_RIE,
+ port->membase + SN_USIO_REG_SCR);
+}
+
+static void usio_enable_ms(struct uart_port *port)
+{
+ writeb(readb(port->membase + SN_USIO_REG_SCR) |
+ SN_USIO_SCR_RIE | SN_USIO_SCR_RXE,
+ port->membase + SN_USIO_REG_SCR);
+}
+
+static void usio_rx_chars(struct uart_port *port)
+{
+ struct tty_port *ttyport = &port->state->port;
+ unsigned long flag = 0;
+ char ch = 0;
+ u8 status;
+ int max_count = 2;
+
+ while (max_count--) {
+ status = readb(port->membase + SN_USIO_REG_SSR);
+
+ if (!(status & SN_USIO_SSR_RDRF))
+ break;
+
+ if (!(status & (SN_USIO_SSR_ORE | SN_USIO_SSR_FRE |
+ SN_USIO_SSR_PE))) {
+ ch = readw(port->membase + SN_USIO_REG_DR);
+ flag = TTY_NORMAL;
+ port->icount.rx++;
+ if (uart_handle_sysrq_char(port, ch))
+ continue;
+ uart_insert_char(port, status, SN_USIO_SSR_ORE,
+ ch, flag);
+ continue;
+ }
+ if (status & SN_USIO_SSR_PE)
+ port->icount.parity++;
+ if (status & SN_USIO_SSR_ORE)
+ port->icount.overrun++;
+ status &= port->read_status_mask;
+ if (status & SN_USIO_SSR_BRK) {
+ flag = TTY_BREAK;
+ ch = 0;
+ } else
+ if (status & SN_USIO_SSR_PE) {
+ flag = TTY_PARITY;
+ ch = 0;
+ } else
+ if (status & SN_USIO_SSR_FRE) {
+ flag = TTY_FRAME;
+ ch = 0;
+ }
+ if (flag)
+ uart_insert_char(port, status, SN_USIO_SSR_ORE,
+ ch, flag);
+
+ writeb(readb(port->membase + SN_USIO_REG_SSR) | SN_USIO_SSR_REC,
+ port->membase + SN_USIO_REG_SSR);
+
+ max_count = readw(port->membase + SN_USIO_REG_FBYTE) >> 8;
+ writew(readw(port->membase + SN_USIO_REG_FCR) |
+ SN_USIO_FCR_FE2 | SN_USIO_FCR_FRIIE,
+ port->membase + SN_USIO_REG_FCR);
+ }
+
+ tty_flip_buffer_push(ttyport);
+}
+
+static irqreturn_t usio_rx_irq(int irq, void *dev_id)
+{
+ struct uart_port *port = dev_id;
+
+ spin_lock(&port->lock);
+ usio_rx_chars(port);
+ spin_unlock(&port->lock);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t usio_tx_irq(int irq, void *dev_id)
+{
+ struct uart_port *port = dev_id;
+
+ spin_lock(&port->lock);
+ if (readb(port->membase + SN_USIO_REG_SSR) & SN_USIO_SSR_TBI)
+ usio_tx_chars(port);
+ spin_unlock(&port->lock);
+
+ return IRQ_HANDLED;
+}
+
+static unsigned int usio_tx_empty(struct uart_port *port)
+{
+ return (readb(port->membase + SN_USIO_REG_SSR) & SN_USIO_SSR_TBI) ?
+ TIOCSER_TEMT : 0;
+}
+
+static void usio_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+}
+
+static unsigned int usio_get_mctrl(struct uart_port *port)
+{
+ return TIOCM_CAR | TIOCM_DSR | TIOCM_CTS;
+
+}
+
+static void usio_break_ctl(struct uart_port *port, int break_state)
+{
+}
+
+static int usio_startup(struct uart_port *port)
+{
+ const char *portname = to_platform_device(port->dev)->name;
+ unsigned long flags;
+ int ret, index = port->line;
+ unsigned char escr;
+
+ ret = request_irq(usio_irq[index][RX], usio_rx_irq, 0, portname, port);
+ if (ret)
+ return ret;
+ ret = request_irq(usio_irq[index][TX], usio_tx_irq, 0, portname, port);
+ if (ret) {
+ free_irq(usio_irq[index][RX], port);
+ return ret;
+ }
+
+ escr = readb(port->membase + SN_USIO_REG_ESCR);
+ if (of_property_read_bool(port->dev->of_node, "uart-flow-enable"))
+ escr |= SN_USIO_ESCR_FLWEN;
+ spin_lock_irqsave(&port->lock, flags);
+ writeb(0, port->membase + SN_USIO_REG_SCR);
+ writeb(escr, port->membase + SN_USIO_REG_ESCR);
+ writeb(SN_USIO_SCR_UPCL, port->membase + SN_USIO_REG_SCR);
+ writeb(SN_USIO_SSR_REC, port->membase + SN_USIO_REG_SSR);
+ writew(0, port->membase + SN_USIO_REG_FCR);
+ writew(SN_USIO_FCR_FCL1 | SN_USIO_FCR_FCL2,
+ port->membase + SN_USIO_REG_FCR);
+ writew(SN_USIO_FCR_FE1 | SN_USIO_FCR_FE2 | SN_USIO_FCR_FRIIE,
+ port->membase + SN_USIO_REG_FCR);
+ writew(0, port->membase + SN_USIO_REG_FBYTE);
+ writew(BIT(12), port->membase + SN_USIO_REG_FBYTE);
+
+ writeb(SN_USIO_SCR_TXE | SN_USIO_SCR_RIE | SN_USIO_SCR_TBIE |
+ SN_USIO_SCR_RXE, port->membase + SN_USIO_REG_SCR);
+ spin_unlock_irqrestore(&port->lock, flags);
+
+ return 0;
+}
+
+static void usio_shutdown(struct uart_port *port)
+{
+ int index = port->line;
+
+ free_irq(usio_irq[index][RX], port);
+ free_irq(usio_irq[index][TX], port);
+}
+
+static void usio_set_termios(struct uart_port *port, struct ktermios *termios,
+ struct ktermios *old)
+{
+ unsigned int escr, smr = SN_USIO_SMR_SOE;
+ unsigned long flags, baud, quot;
+
+ switch (termios->c_cflag & CSIZE) {
+ case CS5:
+ escr = SN_USIO_ESCR_L_5BIT;
+ break;
+ case CS6:
+ escr = SN_USIO_ESCR_L_6BIT;
+ break;
+ case CS7:
+ escr = SN_USIO_ESCR_L_7BIT;
+ break;
+ case CS8:
+ default:
+ escr = SN_USIO_ESCR_L_8BIT;
+ break;
+ }
+
+ if (termios->c_cflag & CSTOPB)
+ smr |= SN_USIO_SMR_SBL;
+
+ if (termios->c_cflag & PARENB) {
+ escr |= SN_USIO_ESCR_PEN;
+ if (termios->c_cflag & PARODD)
+ escr |= SN_USIO_ESCR_P;
+ }
+ /* Set hard flow control */
+ if (of_property_read_bool(port->dev->of_node, "uart-flow-enable") ||
+ (termios->c_cflag & CRTSCTS))
+ escr |= SN_USIO_ESCR_FLWEN;
+
+ baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk);
+ if (baud > 1)
+ quot = port->uartclk / baud - 1;
+ else
+ quot = 0;
+
+ spin_lock_irqsave(&port->lock, flags);
+ uart_update_timeout(port, termios->c_cflag, baud);
+ port->read_status_mask = SN_USIO_SSR_ORE | SN_USIO_SSR_RDRF |
+ SN_USIO_SSR_TDRE;
+ if (termios->c_iflag & INPCK)
+ port->read_status_mask |= SN_USIO_SSR_FRE | SN_USIO_SSR_PE;
+
+ port->ignore_status_mask = 0;
+ if (termios->c_iflag & IGNPAR)
+ port->ignore_status_mask |= SN_USIO_SSR_FRE | SN_USIO_SSR_PE;
+ if ((termios->c_iflag & IGNBRK) && (termios->c_iflag & IGNPAR))
+ port->ignore_status_mask |= SN_USIO_SSR_ORE;
+ if ((termios->c_cflag & CREAD) == 0)
+ port->ignore_status_mask |= SN_USIO_SSR_RDRF;
+
+ writeb(0, port->membase + SN_USIO_REG_SCR);
+ writeb(SN_USIO_SCR_UPCL, port->membase + SN_USIO_REG_SCR);
+ writeb(SN_USIO_SSR_REC, port->membase + SN_USIO_REG_SSR);
+ writew(0, port->membase + SN_USIO_REG_FCR);
+ writeb(smr, port->membase + SN_USIO_REG_SMR);
+ writeb(escr, port->membase + SN_USIO_REG_ESCR);
+ writew(quot, port->membase + SN_USIO_REG_BGR);
+ writew(0, port->membase + SN_USIO_REG_FCR);
+ writew(SN_USIO_FCR_FCL1 | SN_USIO_FCR_FCL2 | SN_USIO_FCR_FE1 |
+ SN_USIO_FCR_FE2 | SN_USIO_FCR_FRIIE,
+ port->membase + SN_USIO_REG_FCR);
+ writew(0, port->membase + SN_USIO_REG_FBYTE);
+ writew(BIT(12), port->membase + SN_USIO_REG_FBYTE);
+ writeb(SN_USIO_SCR_RIE | SN_USIO_SCR_RXE | SN_USIO_SCR_TBIE |
+ SN_USIO_SCR_TXE, port->membase + SN_USIO_REG_SCR);
+ spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static const char *usio_type(struct uart_port *port)
+{
+ return ((port->type == PORT_SN_USIO) ? USIO_NAME : NULL);
+}
+
+static void usio_config_port(struct uart_port *port, int flags)
+{
+ if (flags & UART_CONFIG_TYPE)
+ port->type = PORT_SN_USIO;
+}
+
+static const struct uart_ops usio_ops = {
+ .tx_empty = usio_tx_empty,
+ .set_mctrl = usio_set_mctrl,
+ .get_mctrl = usio_get_mctrl,
+ .stop_tx = usio_stop_tx,
+ .start_tx = usio_start_tx,
+ .stop_rx = usio_stop_rx,
+ .enable_ms = usio_enable_ms,
+ .break_ctl = usio_break_ctl,
+ .startup = usio_startup,
+ .shutdown = usio_shutdown,
+ .set_termios = usio_set_termios,
+ .type = usio_type,
+ .config_port = usio_config_port,
+};
+
+#ifdef CONFIG_SERIAL_M10V_USIO_CONSOLE
+
+static void usio_console_putchar(struct uart_port *port, int c)
+{
+ while (!(readb(port->membase + SN_USIO_REG_SSR) & SN_USIO_SSR_TDRE))
+ cpu_relax();
+
+ writew(c, port->membase + SN_USIO_REG_DR);
+}
+
+static void usio_console_write(struct console *co, const char *s,
+ unsigned int count)
+{
+ struct uart_port *port = &usio_ports[co->index];
+
+ uart_console_write(port, s, count, usio_console_putchar);
+}
+
+static int __init usio_console_setup(struct console *co, char *options)
+{
+ struct uart_port *port;
+ int baud = 115200;
+ int parity = 'n';
+ int flow = 'n';
+ int bits = 8;
+
+ if (co->index >= CONFIG_SERIAL_M10V_USIO_PORTS)
+ return -ENODEV;
+
+ port = &usio_ports[co->index];
+ if (!port->membase)
+ return -ENODEV;
+
+
+ if (options)
+ uart_parse_options(options, &baud, &parity, &bits, &flow);
+
+ if (of_property_read_bool(port->dev->of_node, "uart-flow-enable"))
+ flow = 'r';
+
+ return uart_set_options(port, co, baud, parity, bits, flow);
+}
+
+
+static struct uart_driver usio_uart_driver;
+static struct console usio_console = {
+ .name = USIO_UART_DEV_NAME,
+ .write = usio_console_write,
+ .device = uart_console_device,
+ .setup = usio_console_setup,
+ .flags = CON_PRINTBUFFER,
+ .index = -1,
+ .data = &usio_uart_driver,
+};
+
+static int __init usio_console_init(void)
+{
+ register_console(&usio_console);
+ return 0;
+}
+console_initcall(usio_console_init);
+
+#define USIO_CONSOLE (&usio_console)
+#else
+#define USIO_CONSOLE NULL
+#endif
+
+
+static struct uart_driver usio_uart_driver = {
+ .owner = THIS_MODULE,
+ .driver_name = USIO_NAME,
+ .dev_name = USIO_UART_DEV_NAME,
+ .cons = USIO_CONSOLE,
+ .nr = CONFIG_SERIAL_M10V_USIO_PORTS,
+};
+
+static int usio_probe(struct platform_device *pdev)
+{
+ struct clk *clk = devm_clk_get(&pdev->dev, 0);
+ struct uart_port *port;
+ struct resource *res;
+ int index = 0;
+ int ret;
+
+ if (IS_ERR(clk)) {
+ dev_err(&pdev->dev, "Missing clock\n");
+ return PTR_ERR(clk);
+ }
+ ret = clk_prepare_enable(clk);
+ if (ret) {
+ dev_err(&pdev->dev, "Clock enable failed: %d\n", ret);
+ return ret;
+ }
+ of_property_read_u32(pdev->dev.of_node, "index", &index);
+ port = &usio_ports[index];
+
+ port->private_data = (void *)clk;
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (res == NULL) {
+ dev_err(&pdev->dev, "Missing regs\n");
+ ret = -ENODEV;
+ goto failed;
+ }
+ port->mapbase = res->start;
+ port->membase = ioremap(res->start, (res->end - res->start + 1));
+ port->membase = devm_ioremap(&pdev->dev, res->start,
+ resource_size(res));
+
+ ret = platform_get_irq_byname(pdev, "rx");
+ usio_irq[index][RX] = ret;
+
+ ret = platform_get_irq_byname(pdev, "tx");
+ usio_irq[index][TX] = ret;
+
+ port->irq = usio_irq[index][RX];
+ port->uartclk = clk_get_rate(clk);
+ port->fifosize = 128;
+ port->iotype = UPIO_MEM32;
+ port->flags = UPF_BOOT_AUTOCONF | UPF_SPD_VHI;
+ port->line = index;
+ port->ops = &usio_ops;
+ port->dev = &pdev->dev;
+
+ ret = uart_add_one_port(&usio_uart_driver, port);
+ if (ret) {
+ dev_err(&pdev->dev, "Adding port failed: %d\n", ret);
+ goto failed1;
+ }
+ return 0;
+
+failed1:
+ iounmap(port->membase);
+
+failed:
+ clk_disable_unprepare(clk);
+ clk_put(clk);
+
+ return ret;
+}
+
+static int usio_remove(struct platform_device *pdev)
+{
+ struct uart_port *port = &usio_ports[pdev->id];
+ struct clk *clk = port->private_data;
+
+ uart_remove_one_port(&usio_uart_driver, port);
+ clk_disable_unprepare(clk);
+ clk_put(clk);
+
+ return 0;
+}
+
+#define usio_suspend NULL
+#define usio_resume NULL
+
+static const struct of_device_id m10v_usio_dt_ids[] = {
+ { .compatible = "socionext,milbeaut-m10v-usio-uart" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, m10v_usio_dt_ids);
+
+static struct platform_driver usio_driver = {
+ .probe = usio_probe,
+ .remove = usio_remove,
+ .suspend = usio_suspend,
+ .resume = usio_resume,
+ .driver = {
+ .name = USIO_NAME,
+ .of_match_table = m10v_usio_dt_ids,
+ },
+};
+
+static int __init usio_init(void)
+{
+ int ret = uart_register_driver(&usio_uart_driver);
+
+ if (ret) {
+ pr_err("%s: uart registration failed: %d\n", __func__, ret);
+ return ret;
+ }
+ ret = platform_driver_register(&usio_driver);
+ if (ret) {
+ uart_unregister_driver(&usio_uart_driver);
+ pr_err("%s: drv registration failed: %d\n", __func__, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static void __exit usio_exit(void)
+{
+ platform_driver_unregister(&usio_driver);
+ uart_unregister_driver(&usio_uart_driver);
+}
+
+module_init(usio_init);
+module_exit(usio_exit);
+
+MODULE_AUTHOR("SOCIONEXT");
+MODULE_DESCRIPTION("SN_USIO/UART Driver");
+MODULE_LICENSE("GPL");
+
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index dce5f9d..984df0d 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -281,4 +281,7 @@
/* MediaTek BTIF */
#define PORT_MTK_BTIF 117
+/* Socionext UART */
+#define PORT_SN_USIO 118
+
#endif /* _UAPILINUX_SERIAL_CORE_H */
--
1.9.1
^ permalink raw reply related [flat|nested] 43+ messages in thread