[PATCH v3] ARM: Make a compile firmware conditionally

[PATCH v3] ARM: Make a compile firmware conditionally

[Kyungmin Park]

From: Kyungmin Park <kyungmin.park@samsung.com>

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 <kyungmin.park@samsung.com>
---
diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
index 283fa1d..5b097ca 100644
--- a/arch/arm/common/Kconfig
+++ b/arch/arm/common/Kconfig
@@ -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
 
diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
index e8a4e58..4af6568 100644
--- a/arch/arm/common/Makefile
+++ b/arch/arm/common/Makefile
@@ -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
diff --git a/arch/arm/common/firmware.c b/arch/arm/common/firmware.c
new file mode 100644
index 0000000..ffb6822
--- /dev/null
+++ b/arch/arm/common/firmware.c
@@ -0,0 +1,23 @@
+/*
+ *  Copyright (C) 2012 Samsung Electronics.
+ *  Kyungmin Park <kyungmin.park@samsung.com>
+ *
+ * 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,
+};
diff --git a/arch/arm/include/asm/firmware.h b/arch/arm/include/asm/firmware.h
new file mode 100644
index 0000000..c8c28bf
--- /dev/null
+++ b/arch/arm/include/asm/firmware.h
@@ -0,0 +1,20 @@
+/*
+ *  Copyright (C) 2012 Samsung Electronics.
+ *  Kyungmin Park <kyungmin.park@samsung.com>
+ *
+ * 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

[PATCH v3] ARM: Make a compile firmware conditionally

[Dave Martin]

On Fri, Jul 20, 2012 at 05:24:19PM +0900, Kyungmin Park wrote:
> From: Kyungmin Park <kyungmin.park@samsung.com>
> 
> 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.

> 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 <kyungmin.park@samsung.com>
> ---
> diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
> index 283fa1d..5b097ca 100644
> --- a/arch/arm/common/Kconfig
> +++ b/arch/arm/common/Kconfig
> @@ -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
>  
> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
> index e8a4e58..4af6568 100644
> --- a/arch/arm/common/Makefile
> +++ b/arch/arm/common/Makefile
> @@ -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
> diff --git a/arch/arm/common/firmware.c b/arch/arm/common/firmware.c
> new file mode 100644
> index 0000000..ffb6822
> --- /dev/null
> +++ b/arch/arm/common/firmware.c
> @@ -0,0 +1,23 @@
> +/*
> + *  Copyright (C) 2012 Samsung Electronics.
> + *  Kyungmin Park <kyungmin.park@samsung.com>
> + *
> + * 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,
> +};
> diff --git a/arch/arm/include/asm/firmware.h b/arch/arm/include/asm/firmware.h
> new file mode 100644
> index 0000000..c8c28bf
> --- /dev/null
> +++ b/arch/arm/include/asm/firmware.h
> @@ -0,0 +1,20 @@
> +/*
> + *  Copyright (C) 2012 Samsung Electronics.
> + *  Kyungmin Park <kyungmin.park@samsung.com>
> + *
> + * 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

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

[PATCH v3] ARM: Make a compile firmware conditionally

[Kyungmin Park]

On Fri, Jul 20, 2012 at 6:42 PM, Dave Martin <dave.martin@linaro.org> wrote:
> On Fri, Jul 20, 2012 at 05:24:19PM +0900, Kyungmin Park wrote:
>> From: Kyungmin Park <kyungmin.park@samsung.com>
>>
>> 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.
>
>> 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 <kyungmin.park@samsung.com>
>> ---
>> diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
>> index 283fa1d..5b097ca 100644
>> --- a/arch/arm/common/Kconfig
>> +++ b/arch/arm/common/Kconfig
>> @@ -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
>>
>> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
>> index e8a4e58..4af6568 100644
>> --- a/arch/arm/common/Makefile
>> +++ b/arch/arm/common/Makefile
>> @@ -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
>> diff --git a/arch/arm/common/firmware.c b/arch/arm/common/firmware.c
>> new file mode 100644
>> index 0000000..ffb6822
>> --- /dev/null
>> +++ b/arch/arm/common/firmware.c
>> @@ -0,0 +1,23 @@
>> +/*
>> + *  Copyright (C) 2012 Samsung Electronics.
>> + *  Kyungmin Park <kyungmin.park@samsung.com>
>> + *
>> + * 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,
>> +};
>> diff --git a/arch/arm/include/asm/firmware.h b/arch/arm/include/asm/firmware.h
>> new file mode 100644
>> index 0000000..c8c28bf
>> --- /dev/null
>> +++ b/arch/arm/include/asm/firmware.h
>> @@ -0,0 +1,20 @@
>> +/*
>> + *  Copyright (C) 2012 Samsung Electronics.
>> + *  Kyungmin Park <kyungmin.park@samsung.com>
>> + *
>> + * 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
>
> 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).
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.
>
> 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

[PATCH v3] ARM: Make a compile firmware conditionally

[Dave Martin]

On Fri, Jul 20, 2012 at 10:28:39PM +0900, Kyungmin Park wrote:
> On Fri, Jul 20, 2012 at 6:42 PM, Dave Martin <dave.martin@linaro.org> wrote:
> > On Fri, Jul 20, 2012 at 05:24:19PM +0900, Kyungmin Park wrote:
> >> From: Kyungmin Park <kyungmin.park@samsung.com>
> >>
> >> 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.
> >
> >> 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 <kyungmin.park@samsung.com>
> >> ---
> >> diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
> >> index 283fa1d..5b097ca 100644
> >> --- a/arch/arm/common/Kconfig
> >> +++ b/arch/arm/common/Kconfig
> >> @@ -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
> >>
> >> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
> >> index e8a4e58..4af6568 100644
> >> --- a/arch/arm/common/Makefile
> >> +++ b/arch/arm/common/Makefile
> >> @@ -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
> >> diff --git a/arch/arm/common/firmware.c b/arch/arm/common/firmware.c
> >> new file mode 100644
> >> index 0000000..ffb6822
> >> --- /dev/null
> >> +++ b/arch/arm/common/firmware.c
> >> @@ -0,0 +1,23 @@
> >> +/*
> >> + *  Copyright (C) 2012 Samsung Electronics.
> >> + *  Kyungmin Park <kyungmin.park@samsung.com>
> >> + *
> >> + * 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,
> >> +};
> >> diff --git a/arch/arm/include/asm/firmware.h b/arch/arm/include/asm/firmware.h
> >> new file mode 100644
> >> index 0000000..c8c28bf
> >> --- /dev/null
> >> +++ b/arch/arm/include/asm/firmware.h
> >> @@ -0,0 +1,20 @@
> >> +/*
> >> + *  Copyright (C) 2012 Samsung Electronics.
> >> + *  Kyungmin Park <kyungmin.park@samsung.com>
> >> + *
> >> + * 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
> >
> > 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).
> 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

> > 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.

[PATCH v3] ARM: Make a compile firmware conditionally

[Kyungmin Park]

On 7/20/12, Dave Martin <dave.martin@linaro.org> wrote:
> On Fri, Jul 20, 2012 at 10:28:39PM +0900, Kyungmin Park wrote:
>> On Fri, Jul 20, 2012 at 6:42 PM, Dave Martin <dave.martin@linaro.org>
>> wrote:
>> > On Fri, Jul 20, 2012 at 05:24:19PM +0900, Kyungmin Park wrote:
>> >> From: Kyungmin Park <kyungmin.park@samsung.com>
>> >>
>> >> 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.
>> >
>> >> 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 <kyungmin.park@samsung.com>
>> >> ---
>> >> diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
>> >> index 283fa1d..5b097ca 100644
>> >> --- a/arch/arm/common/Kconfig
>> >> +++ b/arch/arm/common/Kconfig
>> >> @@ -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
>> >>
>> >> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
>> >> index e8a4e58..4af6568 100644
>> >> --- a/arch/arm/common/Makefile
>> >> +++ b/arch/arm/common/Makefile
>> >> @@ -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
>> >> diff --git a/arch/arm/common/firmware.c b/arch/arm/common/firmware.c
>> >> new file mode 100644
>> >> index 0000000..ffb6822
>> >> --- /dev/null
>> >> +++ b/arch/arm/common/firmware.c
>> >> @@ -0,0 +1,23 @@
>> >> +/*
>> >> + *  Copyright (C) 2012 Samsung Electronics.
>> >> + *  Kyungmin Park <kyungmin.park@samsung.com>
>> >> + *
>> >> + * 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,
>> >> +};
>> >> diff --git a/arch/arm/include/asm/firmware.h
>> >> b/arch/arm/include/asm/firmware.h
>> >> new file mode 100644
>> >> index 0000000..c8c28bf
>> >> --- /dev/null
>> >> +++ b/arch/arm/include/asm/firmware.h
>> >> @@ -0,0 +1,20 @@
>> >> +/*
>> >> + *  Copyright (C) 2012 Samsung Electronics.
>> >> + *  Kyungmin Park <kyungmin.park@samsung.com>
>> >> + *
>> >> + * 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
>> >
>> > 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).
>> 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.
Can you tell me it in detail? I can't find out the proper way.
>
> 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
>
>> > 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
>
>