Skip to contentSkip to conversation

ARM: Make a compile firmware conditionally

PATCH Superseded Kyungmin Park 2 participants 5 messages Jul 20, 2012 – Jul 21, 2012 Arm (32-bit)

v3 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Kyungmin Park

author
#0 lore raw

PATCH v3 1/1 ARM: Make a compile firmware conditionally

From: Kyungmin Park <[email protected]>

Some boards can use the firmware at trustzone but others can't use this. However we need to build it simultaneously. To address this issue, introduce arm firmware support and use it at each board files.

e.g., In boot_secondary at mach-exynos/platsmp.c

        __raw_writel(virt_to_phys(exynos4_secondary_startup),
                CPU1_BOOT_REG);

        if (IS_ENABLED(CONFIG_ARM_FIRMWARE)) {
                /* Call Exynos specific smc call */
                firmware_ops.cpu_boot(cpu);
        }

        gic_raise_softirq(cpumask_of(cpu), 1);

        if (pen_release == -1)

Now smp_ops is not yet merged, now just call the firmware_init at init_early
at board file

e.g., exynos4412 based board

static void __init board_init_early(void)

{
        exynos_firmware_init();
}

TODO
1. DT support.
2. call firmware init by smp_ops.

Signed-off-by Kyungmin Park <[email protected]>

---

arch/arm/common/Kconfig +3 -0
index 283fa1d..5b097ca 100644
@@ -21,6 +21,9 @@ config ARM_VIC_NR
 	  The maximum number of VICs available in the system, for
 	  power management.
 
+config ARM_FIRMWARE
+	bool
+
 config ICST
 	bool
arch/arm/common/Makefile +1 -0
index e8a4e58..4af6568 100644
@@ -4,6 +4,7 @@
 
 obj-$(CONFIG_ARM_GIC)		+= gic.o
 obj-$(CONFIG_ARM_VIC)		+= vic.o
+obj-$(CONFIG_ARM_FIRMWARE)	+= firmware.o
 obj-$(CONFIG_ICST)		+= icst.o
 obj-$(CONFIG_SA1111)		+= sa1111.o
 obj-$(CONFIG_PCI_HOST_VIA82C505) += via82c505.o
arch/arm/common/firmware.c new file +23 -0
new file mode 100644
index 0000000..ffb6822
@@ -0,0 +1,23 @@
+/*
+ *  Copyright (C) 2012 Samsung Electronics.
+ *  Kyungmin Park <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/suspend.h>
+
+#include <asm/firmware.h>
+
+static void cpu_boot(int cpu)
+{
+	/* Do nothing */
+}
+
+struct firmware_ops firmware_ops = {
+	.do_idle		= cpu_do_idle,
+	.cpu_boot		= cpu_boot,
+};
arch/arm/include/asm/firmware.h new file +20 -0
new file mode 100644
index 0000000..c8c28bf
@@ -0,0 +1,20 @@
+/*
+ *  Copyright (C) 2012 Samsung Electronics.
+ *  Kyungmin Park <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __ASM_ARM_FIRMWARE_H
+#define __ASM_ARM_FIRMWARE_H
+
+struct firmware_ops {
+	int	(*do_idle)(void);
+	void	(*cpu_boot)(int cpu);
+};
+
+extern struct firmware_ops firmware_ops;
+
+#endif
#1 lore raw

PATCH v3 1/1 ARM: Make a compile firmware conditionally

Quoted 3 lines · Kyungmin Park

From: Kyungmin Park <[email protected]>

Some boards can use the firmware at trustzone but others can't use this.

Note that this isn't just about TrustZone. For example, firmware could be implemented in a hypervisor.

Quoted 104 lines

However we need to build it simultaneously. To address this issue, introduce arm firmware support and use it at each board files.

e.g., In boot_secondary at mach-exynos/platsmp.c

        __raw_writel(virt_to_phys(exynos4_secondary_startup),
                CPU1_BOOT_REG);

        if (IS_ENABLED(CONFIG_ARM_FIRMWARE)) {
                /* Call Exynos specific smc call */
                firmware_ops.cpu_boot(cpu);
        }

        gic_raise_softirq(cpumask_of(cpu), 1);

        if (pen_release == -1)

Now smp_ops is not yet merged, now just call the firmware_init at init_early
at board file

e.g., exynos4412 based board

static void __init board_init_early(void)

{
        exynos_firmware_init();
}

TODO
1. DT support.
2. call firmware init by smp_ops.

Signed-off-by Kyungmin Park <[email protected]>

---

arch/arm/common/Kconfig +3 -0
index 283fa1d..5b097ca 100644
@@ -21,6 +21,9 @@ config ARM_VIC_NR
 	  The maximum number of VICs available in the system, for
 	  power management.
 
+config ARM_FIRMWARE
+	bool
+
 config ICST
 	bool
arch/arm/common/Makefile +1 -0
index e8a4e58..4af6568 100644
@@ -4,6 +4,7 @@
 
 obj-$(CONFIG_ARM_GIC)		+= gic.o
 obj-$(CONFIG_ARM_VIC)		+= vic.o
+obj-$(CONFIG_ARM_FIRMWARE)	+= firmware.o
 obj-$(CONFIG_ICST)		+= icst.o
 obj-$(CONFIG_SA1111)		+= sa1111.o
 obj-$(CONFIG_PCI_HOST_VIA82C505) += via82c505.o
arch/arm/common/firmware.c new file +23 -0
new file mode 100644
index 0000000..ffb6822
@@ -0,0 +1,23 @@
+/*
+ *  Copyright (C) 2012 Samsung Electronics.
+ *  Kyungmin Park <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/suspend.h>
+
+#include <asm/firmware.h>
+
+static void cpu_boot(int cpu)
+{
+	/* Do nothing */
+}
+
+struct firmware_ops firmware_ops = {
+	.do_idle		= cpu_do_idle,
+	.cpu_boot		= cpu_boot,
+};
arch/arm/include/asm/firmware.h new file +10 -0
new file mode 100644
index 0000000..c8c28bf
@@ -0,0 +1,20 @@
+/*
+ *  Copyright (C) 2012 Samsung Electronics.
+ *  Kyungmin Park <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __ASM_ARM_FIRMWARE_H

arch/arm/include/asm/firmware.h line 20

Quoted 10 lines
arch/arm/include/asm/firmware.h new file +10 -0
+#define __ASM_ARM_FIRMWARE_H
+
+struct firmware_ops {
+	int	(*do_idle)(void);
+	void	(*cpu_boot)(int cpu);
+};
+
+extern struct firmware_ops firmware_ops;
+
+#endif

Do you plan to have a registration interface for this?

I imagine that struct firmware_ops will get extended over time, so it might contain many different function pointers. Each board will probably only provide a subset of these, by sparsely populating its own struct firmware_ops (similarly to other existing registration interfaces).

It may make sense to have a way to define default fallback actions for ops not defined by a particular board. If CONFIG_ARM_FIRMWARE=n, we could use the fallbacks for everything.

Cheers
---Dave

author
#2 lore raw

PATCH v3 1/1 ARM: Make a compile firmware conditionally

Quoted 7 lines · Dave Martin
Quoted 3 lines · Kyungmin Park

From: Kyungmin Park <[email protected]>

Some boards can use the firmware at trustzone but others can't use this.

Note that this isn't just about TrustZone. For example, firmware could be implemented in a hypervisor.

Okay, I will update the description.

Quoted 118 lines
Quoted 105 lines

However we need to build it simultaneously. To address this issue, introduce arm firmware support and use it at each board files.

e.g., In boot_secondary at mach-exynos/platsmp.c

        __raw_writel(virt_to_phys(exynos4_secondary_startup),
                CPU1_BOOT_REG);

        if (IS_ENABLED(CONFIG_ARM_FIRMWARE)) {
                /* Call Exynos specific smc call */
                firmware_ops.cpu_boot(cpu);
        }

        gic_raise_softirq(cpumask_of(cpu), 1);

        if (pen_release == -1)

Now smp_ops is not yet merged, now just call the firmware_init at init_early
at board file

e.g., exynos4412 based board

static void __init board_init_early(void)

{
        exynos_firmware_init();
}

TODO
1. DT support.
2. call firmware init by smp_ops.

Signed-off-by Kyungmin Park <[email protected]>

---

arch/arm/common/Kconfig +3 -0
index 283fa1d..5b097ca 100644
@@ -21,6 +21,9 @@ config ARM_VIC_NR
        The maximum number of VICs available in the system, for
        power management.
 
+config ARM_FIRMWARE
+     bool
+
 config ICST
      bool
arch/arm/common/Makefile +1 -0
index e8a4e58..4af6568 100644
@@ -4,6 +4,7 @@
 
 obj-$(CONFIG_ARM_GIC)                += gic.o
 obj-$(CONFIG_ARM_VIC)                += vic.o
+obj-$(CONFIG_ARM_FIRMWARE)   += firmware.o
 obj-$(CONFIG_ICST)           += icst.o
 obj-$(CONFIG_SA1111)         += sa1111.o
 obj-$(CONFIG_PCI_HOST_VIA82C505) += via82c505.o
arch/arm/common/firmware.c new file +23 -0
new file mode 100644
index 0000000..ffb6822
@@ -0,0 +1,23 @@
+/*
+ *  Copyright (C) 2012 Samsung Electronics.
+ *  Kyungmin Park <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/suspend.h>
+
+#include <asm/firmware.h>
+
+static void cpu_boot(int cpu)
+{
+     /* Do nothing */
+}
+
+struct firmware_ops firmware_ops = {
+     .do_idle                = cpu_do_idle,
+     .cpu_boot               = cpu_boot,
+};
arch/arm/include/asm/firmware.h new file +10 -0
new file mode 100644
index 0000000..c8c28bf
@@ -0,0 +1,20 @@
+/*
+ *  Copyright (C) 2012 Samsung Electronics.
+ *  Kyungmin Park <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __ASM_ARM_FIRMWARE_H

arch/arm/include/asm/firmware.h line 20

Quoted 10 lines
arch/arm/include/asm/firmware.h new file +10 -0
+#define __ASM_ARM_FIRMWARE_H
+
+struct firmware_ops {
+     int     (*do_idle)(void);
+     void    (*cpu_boot)(int cpu);
+};
+
+extern struct firmware_ops firmware_ops;
+
+#endif

Do you plan to have a registration interface for this?

Quoted 5 lines

I imagine that struct firmware_ops will get extended over time, so it might contain many different function pointers. Each board will probably only provide a subset of these, by sparsely populating its own struct firmware_ops (similarly to other existing registration interfaces).

Not yet, since I only used the real use case at exynos SoC. If you use other SoC or use case, please add or extend it more.

Quoted 4 lines

It may make sense to have a way to define default fallback actions for ops not defined by a particular board. If CONFIG_ARM_FIRMWARE=n, we could use the fallbacks for everything.

Yes, I also tested it but interesting things. If it uses the IS_ENABLED(CONFIG_ARM_FIRMWARE) with firmware_ops, I can't see error nor warning. Anyway I'll add fackback operation.

Thank you,
Kyungmin Park

#3 lore raw

PATCH v3 1/1 ARM: Make a compile firmware conditionally

Quoted 127 lines · Kyungmin Park
Quoted 7 lines · Dave Martin
Quoted 3 lines · Kyungmin Park

From: Kyungmin Park <[email protected]>

Some boards can use the firmware at trustzone but others can't use this.

Note that this isn't just about TrustZone. For example, firmware could be implemented in a hypervisor.

Okay, I will update the description.

Quoted 118 lines
Quoted 105 lines

However we need to build it simultaneously. To address this issue, introduce arm firmware support and use it at each board files.

e.g., In boot_secondary at mach-exynos/platsmp.c

        __raw_writel(virt_to_phys(exynos4_secondary_startup),
                CPU1_BOOT_REG);

        if (IS_ENABLED(CONFIG_ARM_FIRMWARE)) {
                /* Call Exynos specific smc call */
                firmware_ops.cpu_boot(cpu);
        }

        gic_raise_softirq(cpumask_of(cpu), 1);

        if (pen_release == -1)

Now smp_ops is not yet merged, now just call the firmware_init at init_early
at board file

e.g., exynos4412 based board

static void __init board_init_early(void)

{
        exynos_firmware_init();
}

TODO
1. DT support.
2. call firmware init by smp_ops.

Signed-off-by Kyungmin Park <[email protected]>

---

arch/arm/common/Kconfig +3 -0
index 283fa1d..5b097ca 100644
@@ -21,6 +21,9 @@ config ARM_VIC_NR
        The maximum number of VICs available in the system, for
        power management.
 
+config ARM_FIRMWARE
+     bool
+
 config ICST
      bool
arch/arm/common/Makefile +1 -0
index e8a4e58..4af6568 100644
@@ -4,6 +4,7 @@
 
 obj-$(CONFIG_ARM_GIC)                += gic.o
 obj-$(CONFIG_ARM_VIC)                += vic.o
+obj-$(CONFIG_ARM_FIRMWARE)   += firmware.o
 obj-$(CONFIG_ICST)           += icst.o
 obj-$(CONFIG_SA1111)         += sa1111.o
 obj-$(CONFIG_PCI_HOST_VIA82C505) += via82c505.o
arch/arm/common/firmware.c new file +23 -0
new file mode 100644
index 0000000..ffb6822
@@ -0,0 +1,23 @@
+/*
+ *  Copyright (C) 2012 Samsung Electronics.
+ *  Kyungmin Park <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/suspend.h>
+
+#include <asm/firmware.h>
+
+static void cpu_boot(int cpu)
+{
+     /* Do nothing */
+}
+
+struct firmware_ops firmware_ops = {
+     .do_idle                = cpu_do_idle,
+     .cpu_boot               = cpu_boot,
+};
arch/arm/include/asm/firmware.h new file +10 -0
new file mode 100644
index 0000000..c8c28bf
@@ -0,0 +1,20 @@
+/*
+ *  Copyright (C) 2012 Samsung Electronics.
+ *  Kyungmin Park <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __ASM_ARM_FIRMWARE_H

arch/arm/include/asm/firmware.h line 20

Quoted 10 lines
arch/arm/include/asm/firmware.h new file +10 -0
+#define __ASM_ARM_FIRMWARE_H
+
+struct firmware_ops {
+     int     (*do_idle)(void);
+     void    (*cpu_boot)(int cpu);
+};
+
+extern struct firmware_ops firmware_ops;
+
+#endif

Do you plan to have a registration interface for this?

Quoted 7 lines
Quoted 5 lines

I imagine that struct firmware_ops will get extended over time, so it might contain many different function pointers. Each board will probably only provide a subset of these, by sparsely populating its own struct firmware_ops (similarly to other existing registration interfaces).

Not yet, since I only used the real use case at exynos SoC. If you use other SoC or use case, please add or extend it more.

I'm just thinking about how we can make this easily reusable in a single zImage kernel for example.

Setting all the individual function pointers from the board code will get tedious if more functions are added over time, and it is different from the way other registrations for platform-specific functionality generally work in the kernel.

If we had a simple function in common/firmware.c like this:

void register_firmware_ops(struct firmware_ops *ops);

Then that function can sort it out, either by setting the appropriate function pointers, or by simply keeping a pointer to the currently registered struct.

We can also trigger a BUG() if a programmer makes a mistake such as trying to register more than one set of firmware ops.

Cheers
---Dave

Quoted 6 lines
Quoted 3 lines

It may make sense to have a way to define default fallback actions for ops not defined by a particular board. If CONFIG_ARM_FIRMWARE=n, we could use the fallbacks for everything.

Yes, I also tested it but interesting things. If it uses the IS_ENABLED(CONFIG_ARM_FIRMWARE) with firmware_ops, I can't see error nor warning. Anyway I'll add fackback operation.

author
#4 lore raw

PATCH v3 1/1 ARM: Make a compile firmware conditionally

Quoted 150 lines · Dave Martin
Quoted 133 lines · Kyungmin Park
Quoted 8 lines · Dave Martin
Quoted 4 lines · Kyungmin Park

From: Kyungmin Park <[email protected]>

Some boards can use the firmware at trustzone but others can't use this.

Note that this isn't just about TrustZone. For example, firmware could be implemented in a hypervisor.

Okay, I will update the description.

Quoted 122 lines
Quoted 109 lines

However we need to build it simultaneously. To address this issue, introduce arm firmware support and use it at each board files.

e.g., In boot_secondary at mach-exynos/platsmp.c

        __raw_writel(virt_to_phys(exynos4_secondary_startup),
                CPU1_BOOT_REG);

        if (IS_ENABLED(CONFIG_ARM_FIRMWARE)) {
                /* Call Exynos specific smc call */
                firmware_ops.cpu_boot(cpu);
        }

        gic_raise_softirq(cpumask_of(cpu), 1);

        if (pen_release == -1)

Now smp_ops is not yet merged, now just call the firmware_init at init_early
at board file

e.g., exynos4412 based board

static void __init board_init_early(void)

{
        exynos_firmware_init();
}

TODO
1. DT support.
2. call firmware init by smp_ops.

Signed-off-by Kyungmin Park <[email protected]>

---

arch/arm/common/Kconfig +3 -0
index 283fa1d..5b097ca 100644
@@ -21,6 +21,9 @@ config ARM_VIC_NR
        The maximum number of VICs available in the system, for
        power management.
 
+config ARM_FIRMWARE
+     bool
+
 config ICST
      bool
arch/arm/common/Makefile +1 -0
index e8a4e58..4af6568 100644
@@ -4,6 +4,7 @@
 
 obj-$(CONFIG_ARM_GIC)                += gic.o
 obj-$(CONFIG_ARM_VIC)                += vic.o
+obj-$(CONFIG_ARM_FIRMWARE)   += firmware.o
 obj-$(CONFIG_ICST)           += icst.o
 obj-$(CONFIG_SA1111)         += sa1111.o
 obj-$(CONFIG_PCI_HOST_VIA82C505) += via82c505.o
arch/arm/common/firmware.c new file +5 -0
new file mode 100644
index 0000000..ffb6822
@@ -0,0 +1,23 @@
+/*
+ *  Copyright (C) 2012 Samsung Electronics.
+ *  Kyungmin Park <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or

modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */

arch/arm/common/firmware.c new file +15 -0
+
+#include <linux/kernel.h>
+#include <linux/suspend.h>
+
+#include <asm/firmware.h>
+
+static void cpu_boot(int cpu)
+{
+     /* Do nothing */
+}
+
+struct firmware_ops firmware_ops = {
+     .do_idle                = cpu_do_idle,
+     .cpu_boot               = cpu_boot,
+};
arch/arm/include/asm/firmware.h new file +5 -0
new file mode 100644
index 0000000..c8c28bf
@@ -0,0 +1,20 @@
+/*
+ *  Copyright (C) 2012 Samsung Electronics.
+ *  Kyungmin Park <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or

modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */

arch/arm/include/asm/firmware.h new file +2 -0
+
+#ifndef __ASM_ARM_FIRMWARE_H

arch/arm/include/asm/firmware.h line 20

Quoted 10 lines
arch/arm/include/asm/firmware.h new file +10 -0
+#define __ASM_ARM_FIRMWARE_H
+
+struct firmware_ops {
+     int     (*do_idle)(void);
+     void    (*cpu_boot)(int cpu);
+};
+
+extern struct firmware_ops firmware_ops;
+
+#endif

Do you plan to have a registration interface for this?

Quoted 7 lines
Quoted 5 lines

I imagine that struct firmware_ops will get extended over time, so it might contain many different function pointers. Each board will probably only provide a subset of these, by sparsely populating its own struct firmware_ops (similarly to other existing registration interfaces).

Not yet, since I only used the real use case at exynos SoC. If you use other SoC or use case, please add or extend it more.

I'm just thinking about how we can make this easily reusable in a single zImage kernel for example.

Setting all the individual function pointers from the board code will get tedious if more functions are added over time, and it is different from the way other registrations for platform-specific functionality generally work in the kernel.

Quoted 7 lines

If we had a simple function in common/firmware.c like this:

void register_firmware_ops(struct firmware_ops *ops);

Then that function can sort it out, either by setting the appropriate function pointers, or by simply keeping a pointer to the currently registered struct.

Can you tell me it in detail? I can't find out the proper way.

Quoted 13 lines

We can also trigger a BUG() if a programmer makes a mistake such as trying to register more than one set of firmware ops.

Cheers
---Dave

Quoted 6 lines
Quoted 3 lines

It may make sense to have a way to define default fallback actions for ops not defined by a particular board. If CONFIG_ARM_FIRMWARE=n, we could use the fallbacks for everything.

Yes, I also tested it but interesting things. If it uses the IS_ENABLED(CONFIG_ARM_FIRMWARE) with firmware_ops, I can't see error nor warning. Anyway I'll add fackback operation.

Now I used the IS_ENABLED(...) and declare the firmware_ops as extern, it's okay when build CONFIG_ARM_FIRMWARE=n case.

Does it still requires the fallback case? doesn't better to use the IS_ENABLED(...)? Otherwise, it happends compiler error.

Thank you,
Kyungmin Park

Quoted 2 lines