[PATCH v3 0/7] Early ACPI probing infrastructure

[PATCH v3 0/7] Early ACPI probing infrastructure

[Marc Zyngier]

IRQ controllers and timers are the two types of device the kernel
requires before being able to use the device driver model.

The Device Tree infrastructure makes it very easy to make these
discoverable by the rest of the kernel. For example, each interrupt
controller driver has at least one entry like this:

IRQCHIP_DECLARE(gic_400, "arm,gic-400", gic_of_init);

which says: if you find a node having "arm,gic-400" as a compatible
string in the device tree, then call gic_of_init with this node as a
parameter. The probing itself is done by the OF layer when the
architecture code calls of_irq_init() (usually via irqchip_init).

This has a number of benefits:

- The irqchip code is self-contained. No architecture specific entry
point, no exposed symbols. Just a standard interface.

- The low-level architecture code doesn't have to know about which
interrupt controller is present. It just calls into the firmware
interface (of_irq_init) which is going to sort things out.

Similar infrastructure is provided for the timers/clock sources. Note
that this is not a replacement for the device model, but acts as a
probing infrastructure for things that are required too early for the
device infrastructure to be available.

What I'm aiming for is to introduce the same level of abstraction for
ACPI, or at least for the few bits that are required before a full blown
ACPI/device model can be used. For this, I introduce something vaguely
similar:

IRQCHIP_ACPI_DECLARE(gic_v2, ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
		     gic_validate_dist, ACPI_MADT_GIC_VERSION_V2,
		     gic_v2_acpi_init);

which says: if you find a ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR entry in
MADT (implied by the macro), and that entry is of type
ACPI_MADT_GIC_VERSION_V2 (as checked by gic_validate_dist), then call
gic_v2_acpi_init with the entry as a parameter. A bit more convoluted,
but still without any special entry point.

The various interrupt controller drivers can then implement the above,
and the arch code can use a firmware-specific call to get the probing
done, still oblivious to what interrupt controller is being used. It
also makes the adaptation of a DT driver to ACPI easier.

It turns out that providing such a probing infrastructure is rather
easy, and provides a much deserved cleanup in both the arch code, the
GIC driver, and the architected timer driver. An additional (and very
much optional) patch renames files and symbols to make it obvious that
the infrastructure is not just for DT anymore.

I'm sure there is some more code to be deleted, and one can only
wonder why this wasn't done before the arm64 code was initially merged
(the diffstat says it all...).

Patches are against Rafael's bleeding-edge branch, and a branch is
available at

git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git acpi/device-probing-v3

* From v2:
  - Addressed Rafael's comments (title for patch #1, macro/field names)
  - New config symbols for clocksources
  - Additional and optional last patch to get rid of the apparent
    dependency between the probing infrastructure and DT.

* From the initial version:
  - Make the infrastructure more DT like by providing an
    acpi_probe_entry array per "device type" (one for irqchips, one
    for clocksources). This means that entries can depend on any ACPI
    static table.
  - Use some cpp magic to reduce the amount of code added to an
    absolute minimum.
  - Rebased on v4.3-rc1

Marc Zyngier (7):
  acpi: Add early device probing infrastructure
  irqchip/acpi: Add probing infrastructure for ACPI-based irqchips
  irqchip/gic: Convert the GIC driver to ACPI probing
  clocksource/acpi: Add probing infrastructure for ACPI-based
    clocksources
  clocksource: Add new CLKSRC_{PROBE,ACPI} config symbols
  clocksource/arm_arch_timer: Convert to ACPI probing
  clocksource: cosmetic: Drop OF 'dependency' from symbols

 arch/arm/kernel/time.c                             |  2 +-
 arch/arm/mach-omap2/timer.c                        |  4 +-
 arch/arm/mach-rockchip/rockchip.c                  |  2 +-
 arch/arm/mach-shmobile/setup-r8a7779.c             |  2 +-
 arch/arm/mach-shmobile/setup-rcar-gen2.c           |  2 +-
 arch/arm/mach-spear/spear13xx.c                    |  2 +-
 arch/arm/mach-sunxi/sunxi.c                        |  2 +-
 arch/arm/mach-u300/core.c                          |  2 +-
 arch/arm/mach-ux500/timer.c                        |  2 +-
 arch/arm/mach-zynq/common.c                        |  2 +-
 arch/arm64/include/asm/acpi.h                      |  1 -
 arch/arm64/include/asm/irq.h                       | 13 ----
 arch/arm64/kernel/acpi.c                           | 25 --------
 arch/arm64/kernel/time.c                           |  8 +--
 arch/microblaze/kernel/setup.c                     |  2 +-
 arch/mips/pistachio/time.c                         |  2 +-
 arch/mips/ralink/clk.c                             |  2 +-
 arch/nios2/kernel/time.c                           |  2 +-
 arch/xtensa/kernel/time.c                          |  2 +-
 drivers/acpi/scan.c                                | 39 ++++++++++++
 drivers/clocksource/Kconfig                        |  9 +++
 drivers/clocksource/Makefile                       |  2 +-
 drivers/clocksource/arm_arch_timer.c               | 10 +---
 .../clocksource/{clksrc-of.c => clksrc-probe.c}    |  6 +-
 drivers/irqchip/irq-gic.c                          | 69 +++++++++++-----------
 drivers/irqchip/irqchip.c                          |  5 +-
 include/asm-generic/vmlinux.lds.h                  | 12 ++++
 include/linux/acpi.h                               | 66 +++++++++++++++++++++
 include/linux/acpi_irq.h                           | 10 ----
 include/linux/clocksource.h                        | 13 ++--
 include/linux/irqchip.h                            | 17 ++++++
 include/linux/irqchip/arm-gic-acpi.h               | 31 ----------
 32 files changed, 209 insertions(+), 159 deletions(-)
 rename drivers/clocksource/{clksrc-of.c => clksrc-probe.c} (91%)
 delete mode 100644 include/linux/acpi_irq.h
 delete mode 100644 include/linux/irqchip/arm-gic-acpi.h

-- 
2.1.4

[PATCH v3 1/7] acpi: Add early device probing infrastructure

[Marc Zyngier]

IRQ controllers and timers are the two types of device the kernel
requires before being able to use the device driver model.

ACPI so far lacks a proper probing infrastructure similar to the one
we have with DT, where we're able to declare IRQ chips and
clocksources inside the driver code, and let the core code pick it up
and call us back on a match. This leads to all kind of really ugly
hacks all over the arm64 code and even in the ACPI layer.

In order to allow some basic probing based on the ACPI tables,
introduce "struct acpi_probe_entry" which contains just enough
data and callbacks to match a table, an optional subtable, and
call a probe function. A driver can, at build time, register itself
and expect being called if the right entry exists in the ACPI
table.

A acpi_probe_device_table() is provided, taking an identifier for
a set of acpi_prove_entries, and iterating over the registered
entries.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/acpi/scan.c               | 39 +++++++++++++++++++++++
 include/asm-generic/vmlinux.lds.h | 10 ++++++
 include/linux/acpi.h              | 66 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 115 insertions(+)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index f834b8c..daf9fc8 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1913,3 +1913,42 @@ int __init acpi_scan_init(void)
 	mutex_unlock(&acpi_scan_lock);
 	return result;
 }
+
+static struct acpi_probe_entry *ape;
+static int acpi_probe_count;
+static DEFINE_SPINLOCK(acpi_probe_lock);
+
+static int __init acpi_match_madt(struct acpi_subtable_header *header,
+				  const unsigned long end)
+{
+	if (!ape->subtable_valid || ape->subtable_valid(header, ape))
+		if (!ape->probe_subtbl(header, end))
+			acpi_probe_count++;
+
+	return 0;
+}
+
+int __init __acpi_probe_device_table(struct acpi_probe_entry *ap_head, int nr)
+{
+	int count = 0;
+
+	if (acpi_disabled)
+		return 0;
+
+	spin_lock(&acpi_probe_lock);
+	for (ape = ap_head; nr; ape++, nr--) {
+		if (ACPI_COMPARE_NAME(ACPI_SIG_MADT, ape->id)) {
+			acpi_probe_count = 0;
+			acpi_table_parse_madt(ape->type, acpi_match_madt, 0);
+			count += acpi_probe_count;
+		} else {
+			int res;
+			res = acpi_table_parse(ape->id, ape->probe_table);
+			if (!res)
+				count++;
+		}
+	}
+	spin_unlock(&acpi_probe_lock);
+
+	return count;
+}
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 1781e54..efd7ed1 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -181,6 +181,16 @@
 #define CPUIDLE_METHOD_OF_TABLES() OF_TABLE(CONFIG_CPU_IDLE, cpuidle_method)
 #define EARLYCON_OF_TABLES()	OF_TABLE(CONFIG_SERIAL_EARLYCON, earlycon)
 
+#ifdef CONFIG_ACPI
+#define ACPI_PROBE_TABLE(name)						\
+	. = ALIGN(8);							\
+	VMLINUX_SYMBOL(__##name##_acpi_probe_table) = .;		\
+	*(__##name##_acpi_probe_table)					\
+	VMLINUX_SYMBOL(__##name##_acpi_probe_table_end) = .;
+#else
+#define ACPI_PROBE_TABLE(name)
+#endif
+
 #define KERNEL_DTB()							\
 	STRUCT_ALIGN();							\
 	VMLINUX_SYMBOL(__dtb_start) = .;				\
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 84e7055..51a96a8 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -787,6 +787,61 @@ int acpi_dev_prop_read(struct acpi_device *adev, const char *propname,
 
 struct fwnode_handle *acpi_get_next_subnode(struct device *dev,
 					    struct fwnode_handle *subnode);
+
+struct acpi_probe_entry;
+typedef bool (*acpi_probe_entry_validate_subtbl)(struct acpi_subtable_header *,
+						 struct acpi_probe_entry *);
+
+#define ACPI_TABLE_ID_LEN	5
+
+/**
+ * struct acpi_probe_entry - boot-time probing entry
+ * @id:			ACPI table name
+ * @type:		Optional subtable type to match
+ *			(if @id contains subtables)
+ * @subtable_valid:	Optional callback to check the validity of
+ *			the subtable
+ * @probe_table:	Callback to the driver being probed when table
+ *			match is successful
+ * @probe_subtbl:	Callback to the driver being probed when table and
+ *			subtable match (and optional callback is successful)
+ * @driver_data:	Sideband data provided back to the driver
+ */
+struct acpi_probe_entry {
+	__u8 id[ACPI_TABLE_ID_LEN];
+	__u8 type;
+	acpi_probe_entry_validate_subtbl subtable_valid;
+	union {
+		acpi_tbl_table_handler probe_table;
+		acpi_tbl_entry_handler probe_subtbl;
+	};
+	kernel_ulong_t driver_data;
+};
+
+#define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, subtable, valid, data, fn)	\
+	static const struct acpi_probe_entry __acpi_probe_##name	\
+		__used __section(__##table##_acpi_probe_table)		\
+		 = {							\
+			.id = table_id,					\
+			.type = subtable,				\
+			.subtable_valid = valid,			\
+			.probe_table = (acpi_tbl_table_handler)fn,	\
+			.driver_data = data, 				\
+		   }
+
+#define ACPI_PROBE_TABLE(name)		__##name##_acpi_probe_table
+#define ACPI_PROBE_TABLE_END(name)	__##name##_acpi_probe_table_end
+
+int __acpi_probe_device_table(struct acpi_probe_entry *start, int nr);
+
+#define acpi_probe_device_table(t)					\
+	({ 								\
+		extern struct acpi_probe_entry ACPI_PROBE_TABLE(t),	\
+			                       ACPI_PROBE_TABLE_END(t);	\
+		__acpi_probe_device_table(&ACPI_PROBE_TABLE(t),		\
+					  (&ACPI_PROBE_TABLE_END(t) -	\
+					   &ACPI_PROBE_TABLE(t)));	\
+	})
 #else
 static inline int acpi_dev_get_property(struct acpi_device *adev,
 					const char *name, acpi_object_type type,
@@ -845,6 +900,17 @@ static inline struct fwnode_handle *acpi_get_next_subnode(struct device *dev,
 {
 	return NULL;
 }
+
+#define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, subtable, validate, data, fn) \
+	static const void * __acpi_table_##name[]			\
+		__attribute__((unused))					\
+		 = { (void *) table_id,					\
+		     (void *) subtable,					\
+		     (void *) valid,					\
+		     (void *) fn,					\
+		     (void *) data }
+
+#define acpi_probe_device_table(t)	({ int __r = 0; __r;})
 #endif
 
 #endif	/*_LINUX_ACPI_H*/
-- 
2.1.4

Re: [PATCH v3 1/7] acpi: Add early device probing infrastructure

[Daniel Lezcano]

Hi Marc,

On 09/28/2015 04:49 PM, Marc Zyngier wrote:
> IRQ controllers and timers are the two types of device the kernel
> requires before being able to use the device driver model.
>
> ACPI so far lacks a proper probing infrastructure similar to the one
> we have with DT, where we're able to declare IRQ chips and
> clocksources inside the driver code, and let the core code pick it up
> and call us back on a match. This leads to all kind of really ugly
> hacks all over the arm64 code and even in the ACPI layer.
>
> In order to allow some basic probing based on the ACPI tables,
> introduce "struct acpi_probe_entry" which contains just enough
> data and callbacks to match a table, an optional subtable, and
> call a probe function. A driver can, at build time, register itself
> and expect being called if the right entry exists in the ACPI
> table.
>
> A acpi_probe_device_table() is provided, taking an identifier for
> a set of acpi_prove_entries, and iterating over the registered
> entries.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>   drivers/acpi/scan.c               | 39 +++++++++++++++++++++++
>   include/asm-generic/vmlinux.lds.h | 10 ++++++
>   include/linux/acpi.h              | 66 +++++++++++++++++++++++++++++++++++++++
>   3 files changed, 115 insertions(+)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index f834b8c..daf9fc8 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1913,3 +1913,42 @@ int __init acpi_scan_init(void)
>   	mutex_unlock(&acpi_scan_lock);
>   	return result;
>   }
> +
> +static struct acpi_probe_entry *ape;
> +static int acpi_probe_count;
> +static DEFINE_SPINLOCK(acpi_probe_lock);
> +
> +static int __init acpi_match_madt(struct acpi_subtable_header *header,
> +				  const unsigned long end)
> +{
> +	if (!ape->subtable_valid || ape->subtable_valid(header, ape))
> +		if (!ape->probe_subtbl(header, end))
> +			acpi_probe_count++;
> +
> +	return 0;
> +}
> +
> +int __init __acpi_probe_device_table(struct acpi_probe_entry *ap_head, int nr)
> +{
> +	int count = 0;
> +
> +	if (acpi_disabled)
> +		return 0;
> +
> +	spin_lock(&acpi_probe_lock);
> +	for (ape = ap_head; nr; ape++, nr--) {
> +		if (ACPI_COMPARE_NAME(ACPI_SIG_MADT, ape->id)) {
> +			acpi_probe_count = 0;
> +			acpi_table_parse_madt(ape->type, acpi_match_madt, 0);

Isn't supposed 'acpi_table_parse_madt' to return the count ? and 
shouldn't the return code be checked ?

> +			count += acpi_probe_count;
> +		} else {
> +			int res;
> +			res = acpi_table_parse(ape->id, ape->probe_table);
> +			if (!res)
> +				count++;
> +		}
> +	}
> +	spin_unlock(&acpi_probe_lock);
> +
> +	return count;
> +}




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

Re: [PATCH v3 1/7] acpi: Add early device probing infrastructure

[Marc Zyngier]

On Tue, 29 Sep 2015 06:30:52 +0200
Daniel Lezcano <daniel.lezcano@linaro.org> wrote:

> 
> Hi Marc,
> 
> On 09/28/2015 04:49 PM, Marc Zyngier wrote:
> > IRQ controllers and timers are the two types of device the kernel
> > requires before being able to use the device driver model.
> >
> > ACPI so far lacks a proper probing infrastructure similar to the one
> > we have with DT, where we're able to declare IRQ chips and
> > clocksources inside the driver code, and let the core code pick it up
> > and call us back on a match. This leads to all kind of really ugly
> > hacks all over the arm64 code and even in the ACPI layer.
> >
> > In order to allow some basic probing based on the ACPI tables,
> > introduce "struct acpi_probe_entry" which contains just enough
> > data and callbacks to match a table, an optional subtable, and
> > call a probe function. A driver can, at build time, register itself
> > and expect being called if the right entry exists in the ACPI
> > table.
> >
> > A acpi_probe_device_table() is provided, taking an identifier for
> > a set of acpi_prove_entries, and iterating over the registered
> > entries.
> >
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >   drivers/acpi/scan.c               | 39 +++++++++++++++++++++++
> >   include/asm-generic/vmlinux.lds.h | 10 ++++++
> >   include/linux/acpi.h              | 66 +++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 115 insertions(+)
> >
> > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > index f834b8c..daf9fc8 100644
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -1913,3 +1913,42 @@ int __init acpi_scan_init(void)
> >   	mutex_unlock(&acpi_scan_lock);
> >   	return result;
> >   }
> > +
> > +static struct acpi_probe_entry *ape;
> > +static int acpi_probe_count;
> > +static DEFINE_SPINLOCK(acpi_probe_lock);
> > +
> > +static int __init acpi_match_madt(struct acpi_subtable_header *header,
> > +				  const unsigned long end)
> > +{
> > +	if (!ape->subtable_valid || ape->subtable_valid(header, ape))
> > +		if (!ape->probe_subtbl(header, end))
> > +			acpi_probe_count++;
> > +
> > +	return 0;
> > +}
> > +
> > +int __init __acpi_probe_device_table(struct acpi_probe_entry *ap_head, int nr)
> > +{
> > +	int count = 0;
> > +
> > +	if (acpi_disabled)
> > +		return 0;
> > +
> > +	spin_lock(&acpi_probe_lock);
> > +	for (ape = ap_head; nr; ape++, nr--) {
> > +		if (ACPI_COMPARE_NAME(ACPI_SIG_MADT, ape->id)) {
> > +			acpi_probe_count = 0;
> > +			acpi_table_parse_madt(ape->type, acpi_match_madt, 0);
> 
> Isn't supposed 'acpi_table_parse_madt' to return the count ? and 
> shouldn't the return code be checked ?

acpi_table_madt_parse() returns the count of the entries it has parsed.
We're interested in the count of entries that have been successfully
probed. Not quite the same thing.

As for the return code, checking it is highly symbolic, because there
is no way we can recover from  an error in the ACPI parsing - we're
dead anyway, as we end up without interrupt controller. I can add a
WARN_ON(), but I'm not sure more noise will help understanding the
problem.

There is also the perfectly valid case where ACPI has been forcefully
disabled (or on arm64, not forcefully enabled). In which case, the
parsing code will abort early, and there is no reason to scream about
it.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.

Re: [PATCH v3 1/7] acpi: Add early device probing infrastructure

[Daniel Lezcano]

On 09/29/2015 09:29 AM, Marc Zyngier wrote:
> On Tue, 29 Sep 2015 06:30:52 +0200
> Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
>>
>> Hi Marc,
>>
>> On 09/28/2015 04:49 PM, Marc Zyngier wrote:
>>> IRQ controllers and timers are the two types of device the kernel
>>> requires before being able to use the device driver model.
>>>
>>> ACPI so far lacks a proper probing infrastructure similar to the one
>>> we have with DT, where we're able to declare IRQ chips and
>>> clocksources inside the driver code, and let the core code pick it up
>>> and call us back on a match. This leads to all kind of really ugly
>>> hacks all over the arm64 code and even in the ACPI layer.
>>>
>>> In order to allow some basic probing based on the ACPI tables,
>>> introduce "struct acpi_probe_entry" which contains just enough
>>> data and callbacks to match a table, an optional subtable, and
>>> call a probe function. A driver can, at build time, register itself
>>> and expect being called if the right entry exists in the ACPI
>>> table.
>>>
>>> A acpi_probe_device_table() is provided, taking an identifier for
>>> a set of acpi_prove_entries, and iterating over the registered
>>> entries.
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>> ---
>>>    drivers/acpi/scan.c               | 39 +++++++++++++++++++++++
>>>    include/asm-generic/vmlinux.lds.h | 10 ++++++
>>>    include/linux/acpi.h              | 66 +++++++++++++++++++++++++++++++++++++++
>>>    3 files changed, 115 insertions(+)
>>>
>>> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
>>> index f834b8c..daf9fc8 100644
>>> --- a/drivers/acpi/scan.c
>>> +++ b/drivers/acpi/scan.c
>>> @@ -1913,3 +1913,42 @@ int __init acpi_scan_init(void)
>>>    	mutex_unlock(&acpi_scan_lock);
>>>    	return result;
>>>    }
>>> +
>>> +static struct acpi_probe_entry *ape;
>>> +static int acpi_probe_count;
>>> +static DEFINE_SPINLOCK(acpi_probe_lock);
>>> +
>>> +static int __init acpi_match_madt(struct acpi_subtable_header *header,
>>> +				  const unsigned long end)
>>> +{
>>> +	if (!ape->subtable_valid || ape->subtable_valid(header, ape))
>>> +		if (!ape->probe_subtbl(header, end))
>>> +			acpi_probe_count++;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +int __init __acpi_probe_device_table(struct acpi_probe_entry *ap_head, int nr)
>>> +{
>>> +	int count = 0;
>>> +
>>> +	if (acpi_disabled)
>>> +		return 0;
>>> +
>>> +	spin_lock(&acpi_probe_lock);
>>> +	for (ape = ap_head; nr; ape++, nr--) {
>>> +		if (ACPI_COMPARE_NAME(ACPI_SIG_MADT, ape->id)) {
>>> +			acpi_probe_count = 0;
>>> +			acpi_table_parse_madt(ape->type, acpi_match_madt, 0);
>>
>> Isn't supposed 'acpi_table_parse_madt' to return the count ? and
>> shouldn't the return code be checked ?
>
> acpi_table_madt_parse() returns the count of the entries it has parsed.
> We're interested in the count of entries that have been successfully
> probed. Not quite the same thing.
>
> As for the return code, checking it is highly symbolic, because there
> is no way we can recover from  an error in the ACPI parsing - we're
> dead anyway, as we end up without interrupt controller. I can add a
> WARN_ON(), but I'm not sure more noise will help understanding the
> problem.
>
> There is also the perfectly valid case where ACPI has been forcefully
> disabled (or on arm64, not forcefully enabled). In which case, the
> parsing code will abort early, and there is no reason to scream about
> it.

I see. Thanks for the details.

   - Daniel

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

Re: [PATCH v3 1/7] acpi: Add early device probing infrastructure

[Lorenzo Pieralisi]

On Mon, Sep 28, 2015 at 03:49:12PM +0100, Marc Zyngier wrote:
> IRQ controllers and timers are the two types of device the kernel
> requires before being able to use the device driver model.
> 
> ACPI so far lacks a proper probing infrastructure similar to the one
> we have with DT, where we're able to declare IRQ chips and
> clocksources inside the driver code, and let the core code pick it up
> and call us back on a match. This leads to all kind of really ugly
> hacks all over the arm64 code and even in the ACPI layer.
> 
> In order to allow some basic probing based on the ACPI tables,
> introduce "struct acpi_probe_entry" which contains just enough
> data and callbacks to match a table, an optional subtable, and
> call a probe function. A driver can, at build time, register itself
> and expect being called if the right entry exists in the ACPI
> table.
> 
> A acpi_probe_device_table() is provided, taking an identifier for
> a set of acpi_prove_entries, and iterating over the registered
> entries.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/acpi/scan.c               | 39 +++++++++++++++++++++++
>  include/asm-generic/vmlinux.lds.h | 10 ++++++
>  include/linux/acpi.h              | 66 +++++++++++++++++++++++++++++++++++++++

Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

Re: [PATCH v3 1/7] acpi: Add early device probing infrastructure

[Hanjun Guo]

On 09/28/2015 10:49 PM, Marc Zyngier wrote:
> IRQ controllers and timers are the two types of device the kernel
> requires before being able to use the device driver model.
>
> ACPI so far lacks a proper probing infrastructure similar to the one
> we have with DT, where we're able to declare IRQ chips and
> clocksources inside the driver code, and let the core code pick it up
> and call us back on a match. This leads to all kind of really ugly
> hacks all over the arm64 code and even in the ACPI layer.
>
> In order to allow some basic probing based on the ACPI tables,
> introduce "struct acpi_probe_entry" which contains just enough
> data and callbacks to match a table, an optional subtable, and
> call a probe function. A driver can, at build time, register itself
> and expect being called if the right entry exists in the ACPI
> table.
>
> A acpi_probe_device_table() is provided, taking an identifier for
> a set of acpi_prove_entries, and iterating over the registered
> entries.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

Re: [PATCH v3 1/7] acpi: Add early device probing infrastructure

[Wei Huang]

Hi Marc,

On 09/28/2015 09:49 AM, Marc Zyngier wrote:
> IRQ controllers and timers are the two types of device the kernel
> requires before being able to use the device driver model.
> 
> ACPI so far lacks a proper probing infrastructure similar to the one
> we have with DT, where we're able to declare IRQ chips and
> clocksources inside the driver code, and let the core code pick it up
> and call us back on a match. This leads to all kind of really ugly
> hacks all over the arm64 code and even in the ACPI layer.
> 
> In order to allow some basic probing based on the ACPI tables,
> introduce "struct acpi_probe_entry" which contains just enough
> data and callbacks to match a table, an optional subtable, and
> call a probe function. A driver can, at build time, register itself
> and expect being called if the right entry exists in the ACPI
> table.
> 
> A acpi_probe_device_table() is provided, taking an identifier for
> a set of acpi_prove_entries, and iterating over the registered
> entries.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/acpi/scan.c               | 39 +++++++++++++++++++++++
>  include/asm-generic/vmlinux.lds.h | 10 ++++++
>  include/linux/acpi.h              | 66 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 115 insertions(+)
> 
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index f834b8c..daf9fc8 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1913,3 +1913,42 @@ int __init acpi_scan_init(void)
>  	mutex_unlock(&acpi_scan_lock);
>  	return result;
>  }
> +
> +static struct acpi_probe_entry *ape;
> +static int acpi_probe_count;
> +static DEFINE_SPINLOCK(acpi_probe_lock);
> +
> +static int __init acpi_match_madt(struct acpi_subtable_header *header,
> +				  const unsigned long end)
> +{
> +	if (!ape->subtable_valid || ape->subtable_valid(header, ape))
> +		if (!ape->probe_subtbl(header, end))
> +			acpi_probe_count++;
> +
> +	return 0;
> +}
> +
> +int __init __acpi_probe_device_table(struct acpi_probe_entry *ap_head, int nr)
> +{
> +	int count = 0;
> +
> +	if (acpi_disabled)
> +		return 0;
> +
> +	spin_lock(&acpi_probe_lock);
> +	for (ape = ap_head; nr; ape++, nr--) {
> +		if (ACPI_COMPARE_NAME(ACPI_SIG_MADT, ape->id)) {
> +			acpi_probe_count = 0;
> +			acpi_table_parse_madt(ape->type, acpi_match_madt, 0);
> +			count += acpi_probe_count;
> +		} else {
> +			int res;
> +			res = acpi_table_parse(ape->id, ape->probe_table);
> +			if (!res)
> +				count++;
> +		}
> +	}
> +	spin_unlock(&acpi_probe_lock);
> +
> +	return count;
> +}
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 1781e54..efd7ed1 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -181,6 +181,16 @@
>  #define CPUIDLE_METHOD_OF_TABLES() OF_TABLE(CONFIG_CPU_IDLE, cpuidle_method)
>  #define EARLYCON_OF_TABLES()	OF_TABLE(CONFIG_SERIAL_EARLYCON, earlycon)
>  
> +#ifdef CONFIG_ACPI
> +#define ACPI_PROBE_TABLE(name)						\
> +	. = ALIGN(8);							\
> +	VMLINUX_SYMBOL(__##name##_acpi_probe_table) = .;		\
> +	*(__##name##_acpi_probe_table)					\
> +	VMLINUX_SYMBOL(__##name##_acpi_probe_table_end) = .;
> +#else
> +#define ACPI_PROBE_TABLE(name)
> +#endif
> +
>  #define KERNEL_DTB()							\
>  	STRUCT_ALIGN();							\
>  	VMLINUX_SYMBOL(__dtb_start) = .;				\
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 84e7055..51a96a8 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -787,6 +787,61 @@ int acpi_dev_prop_read(struct acpi_device *adev, const char *propname,
>  
>  struct fwnode_handle *acpi_get_next_subnode(struct device *dev,
>  					    struct fwnode_handle *subnode);
> +
> +struct acpi_probe_entry;
> +typedef bool (*acpi_probe_entry_validate_subtbl)(struct acpi_subtable_header *,
> +						 struct acpi_probe_entry *);
> +
> +#define ACPI_TABLE_ID_LEN	5
> +
> +/**
> + * struct acpi_probe_entry - boot-time probing entry
> + * @id:			ACPI table name
> + * @type:		Optional subtable type to match
> + *			(if @id contains subtables)
> + * @subtable_valid:	Optional callback to check the validity of
> + *			the subtable
> + * @probe_table:	Callback to the driver being probed when table
> + *			match is successful
> + * @probe_subtbl:	Callback to the driver being probed when table and
> + *			subtable match (and optional callback is successful)
> + * @driver_data:	Sideband data provided back to the driver
> + */

A suggestion: could you add more details to the description above? I couldn't figure how to use it without reading __acpi_probe_device_table(). Something like "if @id matches the one defined in __acpi_probe_device_table(), then probe_subtbl() will be invoked; otherwise probe_table() will be invoked" would be helpful for programmers use this struct. 

> +struct acpi_probe_entry {
> +	__u8 id[ACPI_TABLE_ID_LEN];
> +	__u8 type;
> +	acpi_probe_entry_validate_subtbl subtable_valid;
> +	union {
> +		acpi_tbl_table_handler probe_table;
> +		acpi_tbl_entry_handler probe_subtbl;
> +	};

Could we avoid using union for probe_table & probe_subtbl? The benefit is that we don't need to do function casting below and compiler can automatically check the correctness.

> +	kernel_ulong_t driver_data;
> +};
> +
> +#define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, subtable, valid, data, fn)	\
> +	static const struct acpi_probe_entry __acpi_probe_##name	\
> +		__used __section(__##table##_acpi_probe_table)		\
> +		 = {							\
> +			.id = table_id,					\
> +			.type = subtable,				\
> +			.subtable_valid = valid,			\
> +			.probe_table = (acpi_tbl_table_handler)fn,	\
> +			.driver_data = data, 				\
> +		   }
> +

Something like: 

#define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, subtable, valid, data, fn, subfn)	\
	static const struct acpi_probe_entry __acpi_probe_##name	\
		__used __section(__##table##_acpi_probe_table)		\
		 = {							\
			.id = table_id,					\
			.type = subtable,				\
			.subtable_valid = valid,			\
			.probe_table = fn,				\
			.probe_subtbl = subfn,				\
			.driver_data = data, 				\
		   }

Then in patch 3, you can define new entries as:

IRQCHIP_ACPI_DECLARE(gic_v2, ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
		     gic_validate_dist, ACPI_MADT_GIC_VERSION_V2,
		     NULL, gic_v2_acpi_init);
IRQCHIP_ACPI_DECLARE(gic_v2_maybe, ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
		     gic_validate_dist, ACPI_MADT_GIC_VERSION_NONE,
		     NULL, gic_v2_acpi_init);


> +#define ACPI_PROBE_TABLE(name)		__##name##_acpi_probe_table
> +#define ACPI_PROBE_TABLE_END(name)	__##name##_acpi_probe_table_end
> +
> +int __acpi_probe_device_table(struct acpi_probe_entry *start, int nr);
> +
> +#define acpi_probe_device_table(t)					\
> +	({ 								\
> +		extern struct acpi_probe_entry ACPI_PROBE_TABLE(t),	\
> +			                       ACPI_PROBE_TABLE_END(t);	\
> +		__acpi_probe_device_table(&ACPI_PROBE_TABLE(t),		\
> +					  (&ACPI_PROBE_TABLE_END(t) -	\
> +					   &ACPI_PROBE_TABLE(t)));	\
> +	})
>  #else
>  static inline int acpi_dev_get_property(struct acpi_device *adev,
>  					const char *name, acpi_object_type type,
> @@ -845,6 +900,17 @@ static inline struct fwnode_handle *acpi_get_next_subnode(struct device *dev,
>  {
>  	return NULL;
>  }
> +
> +#define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, subtable, validate, data, fn) \
> +	static const void * __acpi_table_##name[]			\
> +		__attribute__((unused))					\
> +		 = { (void *) table_id,					\
> +		     (void *) subtable,					\
> +		     (void *) valid,					\
> +		     (void *) fn,					\
> +		     (void *) data }
> +
> +#define acpi_probe_device_table(t)	({ int __r = 0; __r;})
>  #endif
>  
>  #endif	/*_LINUX_ACPI_H*/
> 

Re: [PATCH v3 1/7] acpi: Add early device probing infrastructure

[Marc Zyngier]

On Fri, 2 Oct 2015 16:06:05 -0500
Wei Huang <wei@redhat.com> wrote:

Hi Wei,

> Hi Marc,

[...]

> > +struct acpi_probe_entry {
> > +	__u8 id[ACPI_TABLE_ID_LEN];
> > +	__u8 type;
> > +	acpi_probe_entry_validate_subtbl subtable_valid;
> > +	union {
> > +		acpi_tbl_table_handler probe_table;
> > +		acpi_tbl_entry_handler probe_subtbl;
> > +	};
> 
> Could we avoid using union for probe_table & probe_subtbl? The benefit is that we don't need to do function casting below and compiler can automatically check the correctness.
> 
> > +	kernel_ulong_t driver_data;
> > +};
> > +
> > +#define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, subtable, valid, data, fn)	\
> > +	static const struct acpi_probe_entry __acpi_probe_##name	\
> > +		__used __section(__##table##_acpi_probe_table)		\
> > +		 = {							\
> > +			.id = table_id,					\
> > +			.type = subtable,				\
> > +			.subtable_valid = valid,			\
> > +			.probe_table = (acpi_tbl_table_handler)fn,	\
> > +			.driver_data = data, 				\
> > +		   }
> > +
> 
> Something like: 
> 
> #define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, subtable, valid, data, fn, subfn)	\
> 	static const struct acpi_probe_entry __acpi_probe_##name	\
> 		__used __section(__##table##_acpi_probe_table)		\
> 		 = {							\
> 			.id = table_id,					\
> 			.type = subtable,				\
> 			.subtable_valid = valid,			\
> 			.probe_table = fn,				\
> 			.probe_subtbl = subfn,				\
> 			.driver_data = data, 				\
> 		   }
> 
> Then in patch 3, you can define new entries as:
> 
> IRQCHIP_ACPI_DECLARE(gic_v2, ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
> 		     gic_validate_dist, ACPI_MADT_GIC_VERSION_V2,
> 		     NULL, gic_v2_acpi_init);
> IRQCHIP_ACPI_DECLARE(gic_v2_maybe, ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
> 		     gic_validate_dist, ACPI_MADT_GIC_VERSION_NONE,
> 		     NULL, gic_v2_acpi_init);
> 

That's exactly what I was trying to avoid. If you want to do that, do
it in the IRQCHIP_ACPI_DECLARE macro, as there is strictly no need for
this this NULL to appear here (MADT always matches by subtable).

Or even better, have two ACPI_DECLARE* that populate the probe entry in
a mutually exclusive way (either probe_table is set and both
valid/subtbl are NULL, or probe_table is NULL and the two other fields
are set).

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.

Re: [PATCH v3 1/7] acpi: Add early device probing infrastructure

[Wei Huang]

On 10/03/2015 05:04 AM, Marc Zyngier wrote:
> On Fri, 2 Oct 2015 16:06:05 -0500
> Wei Huang <wei@redhat.com> wrote:
> 
> Hi Wei,
> 
>> Hi Marc,
> 
> [...]
> 
>>> +struct acpi_probe_entry {
>>> +	__u8 id[ACPI_TABLE_ID_LEN];
>>> +	__u8 type;
>>> +	acpi_probe_entry_validate_subtbl subtable_valid;
>>> +	union {
>>> +		acpi_tbl_table_handler probe_table;
>>> +		acpi_tbl_entry_handler probe_subtbl;
>>> +	};
>>
>> Could we avoid using union for probe_table & probe_subtbl? The benefit is that we don't need to do function casting below and compiler can automatically check the correctness.
>>
>>> +	kernel_ulong_t driver_data;
>>> +};
>>> +
>>> +#define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, subtable, valid, data, fn)	\
>>> +	static const struct acpi_probe_entry __acpi_probe_##name	\
>>> +		__used __section(__##table##_acpi_probe_table)		\
>>> +		 = {							\
>>> +			.id = table_id,					\
>>> +			.type = subtable,				\
>>> +			.subtable_valid = valid,			\
>>> +			.probe_table = (acpi_tbl_table_handler)fn,	\
>>> +			.driver_data = data, 				\
>>> +		   }
>>> +
>>
>> Something like: 
>>
>> #define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, subtable, valid, data, fn, subfn)	\
>> 	static const struct acpi_probe_entry __acpi_probe_##name	\
>> 		__used __section(__##table##_acpi_probe_table)		\
>> 		 = {							\
>> 			.id = table_id,					\
>> 			.type = subtable,				\
>> 			.subtable_valid = valid,			\
>> 			.probe_table = fn,				\
>> 			.probe_subtbl = subfn,				\
>> 			.driver_data = data, 				\
>> 		   }
>>
>> Then in patch 3, you can define new entries as:
>>
>> IRQCHIP_ACPI_DECLARE(gic_v2, ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
>> 		     gic_validate_dist, ACPI_MADT_GIC_VERSION_V2,
>> 		     NULL, gic_v2_acpi_init);
>> IRQCHIP_ACPI_DECLARE(gic_v2_maybe, ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
>> 		     gic_validate_dist, ACPI_MADT_GIC_VERSION_NONE,
>> 		     NULL, gic_v2_acpi_init);
>>
> 
> That's exactly what I was trying to avoid. If you want to do that, do
> it in the IRQCHIP_ACPI_DECLARE macro, as there is strictly no need for
> this this NULL to appear here (MADT always matches by subtable).
> 
> Or even better, have two ACPI_DECLARE* that populate the probe entry in
> a mutually exclusive way (either probe_table is set and both
> valid/subtbl are NULL, or probe_table is NULL and the two other fields
> are set).

Yes, this approach would be sufficient. So users can clearly tell them
apart in terms of usage cases.

Thanks,
-Wei

> 
> Thanks,
> 
> 	M.
> 

[PATCH v3 2/7] irqchip/acpi: Add probing infrastructure for ACPI-based irqchips

[Marc Zyngier]

DT enjoys a rather nice probing infrastructure for irqchips, while
ACPI is so far stuck into a very distant past.

This patch introduces a declarative API, allowing irqchips to be
self-contained and be called when a particular entry is matched
in the MADT table.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/irq.h      | 11 -----------
 drivers/irqchip/irqchip.c         |  8 +++++---
 include/asm-generic/vmlinux.lds.h |  1 +
 include/linux/acpi_irq.h          | 10 ----------
 include/linux/irqchip.h           | 17 +++++++++++++++++
 5 files changed, 23 insertions(+), 24 deletions(-)
 delete mode 100644 include/linux/acpi_irq.h

diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
index bbb251b..1a1037a 100644
--- a/arch/arm64/include/asm/irq.h
+++ b/arch/arm64/include/asm/irq.h
@@ -10,15 +10,4 @@ struct pt_regs;
 extern void migrate_irqs(void);
 extern void set_handle_irq(void (*handle_irq)(struct pt_regs *));
 
-static inline void acpi_irq_init(void)
-{
-	/*
-	 * Hardcode ACPI IRQ chip initialization to GICv2 for now.
-	 * Proper irqchip infrastructure will be implemented along with
-	 * incoming  GICv2m|GICv3|ITS bits.
-	 */
-	acpi_gic_init();
-}
-#define acpi_irq_init acpi_irq_init
-
 #endif
diff --git a/drivers/irqchip/irqchip.c b/drivers/irqchip/irqchip.c
index afd1af3..1ee773e 100644
--- a/drivers/irqchip/irqchip.c
+++ b/drivers/irqchip/irqchip.c
@@ -8,7 +8,7 @@
  * warranty of any kind, whether express or implied.
  */
 
-#include <linux/acpi_irq.h>
+#include <linux/acpi.h>
 #include <linux/init.h>
 #include <linux/of_irq.h>
 #include <linux/irqchip.h>
@@ -27,6 +27,8 @@ extern struct of_device_id __irqchip_of_table[];
 void __init irqchip_init(void)
 {
 	of_irq_init(__irqchip_of_table);
-
-	acpi_irq_init();
+#if defined(CONFIG_ARM64) && defined(CONFIG_ACPI)
+	acpi_gic_init();	/* Temporary hack */
+#endif
+	acpi_probe_device_table(irqchip);
 }
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index efd7ed1..cd836cd 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -524,6 +524,7 @@
 	CPUIDLE_METHOD_OF_TABLES()					\
 	KERNEL_DTB()							\
 	IRQCHIP_OF_MATCH_TABLE()					\
+	ACPI_PROBE_TABLE(irqchip)					\
 	EARLYCON_TABLE()						\
 	EARLYCON_OF_TABLES()
 
diff --git a/include/linux/acpi_irq.h b/include/linux/acpi_irq.h
deleted file mode 100644
index f10c872..0000000
--- a/include/linux/acpi_irq.h
+++ /dev/null
@@ -1,10 +0,0 @@
-#ifndef _LINUX_ACPI_IRQ_H
-#define _LINUX_ACPI_IRQ_H
-
-#include <linux/irq.h>
-
-#ifndef acpi_irq_init
-static inline void acpi_irq_init(void) { }
-#endif
-
-#endif /* _LINUX_ACPI_IRQ_H */
diff --git a/include/linux/irqchip.h b/include/linux/irqchip.h
index 6388873..89c34b2 100644
--- a/include/linux/irqchip.h
+++ b/include/linux/irqchip.h
@@ -11,6 +11,7 @@
 #ifndef _LINUX_IRQCHIP_H
 #define _LINUX_IRQCHIP_H
 
+#include <linux/acpi.h>
 #include <linux/of.h>
 
 /*
@@ -25,6 +26,22 @@
  */
 #define IRQCHIP_DECLARE(name, compat, fn) OF_DECLARE_2(irqchip, name, compat, fn)
 
+/*
+ * This macro must be used by the different irqchip drivers to declare
+ * the association between their version and their initialization function.
+ *
+ * @name: name that must be unique accross all IRQCHIP_ACPI_DECLARE of the
+ * same file.
+ * @subtable: Subtable to be identified in MADT
+ * @validate: Function to be called on that subtable to check its validity.
+ *            Can be NULL.
+ * @data: data to be checked by the validate function.
+ * @fn: initialization function
+ */
+#define IRQCHIP_ACPI_DECLARE(name, subtable, validate, data, fn)	\
+	ACPI_DECLARE_PROBE_ENTRY(irqchip, name, ACPI_SIG_MADT, 		\
+				 subtable, validate, data, fn)
+
 #ifdef CONFIG_IRQCHIP
 void irqchip_init(void);
 #else
-- 
2.1.4

Re: [PATCH v3 2/7] irqchip/acpi: Add probing infrastructure for ACPI-based irqchips

[Catalin Marinas]

On Mon, Sep 28, 2015 at 03:49:13PM +0100, Marc Zyngier wrote:
> DT enjoys a rather nice probing infrastructure for irqchips, while
> ACPI is so far stuck into a very distant past.
> 
> This patch introduces a declarative API, allowing irqchips to be
> self-contained and be called when a particular entry is matched
> in the MADT table.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm64/include/asm/irq.h      | 11 -----------
>  drivers/irqchip/irqchip.c         |  8 +++++---
>  include/asm-generic/vmlinux.lds.h |  1 +
>  include/linux/acpi_irq.h          | 10 ----------
>  include/linux/irqchip.h           | 17 +++++++++++++++++
>  5 files changed, 23 insertions(+), 24 deletions(-)
>  delete mode 100644 include/linux/acpi_irq.h

For the arm64 part:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Re: [PATCH v3 2/7] irqchip/acpi: Add probing infrastructure for ACPI-based irqchips

[Hanjun Guo]

On 09/28/2015 10:49 PM, Marc Zyngier wrote:
> DT enjoys a rather nice probing infrastructure for irqchips, while
> ACPI is so far stuck into a very distant past.
>
> This patch introduces a declarative API, allowing irqchips to be
> self-contained and be called when a particular entry is matched
> in the MADT table.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

[PATCH v3 3/7] irqchip/gic: Convert the GIC driver to ACPI probing

[Marc Zyngier]

Now that we have a basic infrastructure to register irqchips and
call them on discovery of a matching entry in MADT, convert the
GIC driver to this new probing method.

It ends up being a code deletion party, which is a rather good thing.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/acpi.h        |  1 -
 arch/arm64/include/asm/irq.h         |  2 --
 arch/arm64/kernel/acpi.c             | 25 -------------
 drivers/irqchip/irq-gic.c            | 69 ++++++++++++++++++------------------
 drivers/irqchip/irqchip.c            |  3 --
 include/linux/irqchip/arm-gic-acpi.h | 31 ----------------
 6 files changed, 35 insertions(+), 96 deletions(-)
 delete mode 100644 include/linux/irqchip/arm-gic-acpi.h

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index 208cec0..6894205 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -12,7 +12,6 @@
 #ifndef _ASM_ACPI_H
 #define _ASM_ACPI_H
 
-#include <linux/irqchip/arm-gic-acpi.h>
 #include <linux/mm.h>
 #include <linux/psci.h>
 
diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
index 1a1037a..94c5367 100644
--- a/arch/arm64/include/asm/irq.h
+++ b/arch/arm64/include/asm/irq.h
@@ -1,8 +1,6 @@
 #ifndef __ASM_IRQ_H
 #define __ASM_IRQ_H
 
-#include <linux/irqchip/arm-gic-acpi.h>
-
 #include <asm-generic/irq.h>
 
 struct pt_regs;
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 19de753..d6463bb 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -205,28 +205,3 @@ void __init acpi_boot_table_init(void)
 			disable_acpi();
 	}
 }
-
-void __init acpi_gic_init(void)
-{
-	struct acpi_table_header *table;
-	acpi_status status;
-	acpi_size tbl_size;
-	int err;
-
-	if (acpi_disabled)
-		return;
-
-	status = acpi_get_table_with_size(ACPI_SIG_MADT, 0, &table, &tbl_size);
-	if (ACPI_FAILURE(status)) {
-		const char *msg = acpi_format_exception(status);
-
-		pr_err("Failed to get MADT table, %s\n", msg);
-		return;
-	}
-
-	err = gic_v2_acpi_init(table);
-	if (err)
-		pr_err("Failed to initialize GIC IRQ controller");
-
-	early_acpi_os_unmap_memory((char *)table, tbl_size);
-}
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 982c09c..d4add30 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -41,7 +41,6 @@
 #include <linux/irqchip.h>
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqchip/arm-gic.h>
-#include <linux/irqchip/arm-gic-acpi.h>
 
 #include <asm/cputype.h>
 #include <asm/irq.h>
@@ -1195,7 +1194,7 @@ IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
 #endif
 
 #ifdef CONFIG_ACPI
-static phys_addr_t dist_phy_base, cpu_phy_base __initdata;
+static phys_addr_t cpu_phy_base __initdata;
 
 static int __init
 gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
@@ -1223,60 +1222,56 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
 	return 0;
 }
 
-static int __init
-gic_acpi_parse_madt_distributor(struct acpi_subtable_header *header,
-				const unsigned long end)
+/* The things you have to do to just *count* something... */
+static int __init acpi_dummy_func(struct acpi_subtable_header *header,
+				  const unsigned long end)
 {
-	struct acpi_madt_generic_distributor *dist;
+	return 0;
+}
 
-	dist = (struct acpi_madt_generic_distributor *)header;
+static bool __init acpi_gic_redist_is_present(void)
+{
+	return acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR,
+				     acpi_dummy_func, 0) > 0;
+}
 
-	if (BAD_MADT_ENTRY(dist, end))
-		return -EINVAL;
+static bool __init gic_validate_dist(struct acpi_subtable_header *header,
+				     struct acpi_probe_entry *ape)
+{
+	struct acpi_madt_generic_distributor *dist;
+	dist = (struct acpi_madt_generic_distributor *)header;
 
-	dist_phy_base = dist->base_address;
-	return 0;
+	return (dist->version == ape->driver_data &&
+		(dist->version != ACPI_MADT_GIC_VERSION_NONE ||
+		 !acpi_gic_redist_is_present()));
 }
 
-int __init
-gic_v2_acpi_init(struct acpi_table_header *table)
+#define ACPI_GICV2_DIST_MEM_SIZE	(SZ_4K)
+#define ACPI_GIC_CPU_IF_MEM_SIZE	(SZ_8K)
+
+static int __init gic_v2_acpi_init(struct acpi_subtable_header *header,
+				   const unsigned long end)
 {
+	struct acpi_madt_generic_distributor *dist;
 	void __iomem *cpu_base, *dist_base;
 	int count;
 
 	/* Collect CPU base addresses */
-	count = acpi_parse_entries(ACPI_SIG_MADT,
-				   sizeof(struct acpi_table_madt),
-				   gic_acpi_parse_madt_cpu, table,
-				   ACPI_MADT_TYPE_GENERIC_INTERRUPT, 0);
+	count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
+				      gic_acpi_parse_madt_cpu, 0);
 	if (count <= 0) {
 		pr_err("No valid GICC entries exist\n");
 		return -EINVAL;
 	}
 
-	/*
-	 * Find distributor base address. We expect one distributor entry since
-	 * ACPI 5.1 spec neither support multi-GIC instances nor GIC cascade.
-	 */
-	count = acpi_parse_entries(ACPI_SIG_MADT,
-				   sizeof(struct acpi_table_madt),
-				   gic_acpi_parse_madt_distributor, table,
-				   ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR, 0);
-	if (count <= 0) {
-		pr_err("No valid GICD entries exist\n");
-		return -EINVAL;
-	} else if (count > 1) {
-		pr_err("More than one GICD entry detected\n");
-		return -EINVAL;
-	}
-
 	cpu_base = ioremap(cpu_phy_base, ACPI_GIC_CPU_IF_MEM_SIZE);
 	if (!cpu_base) {
 		pr_err("Unable to map GICC registers\n");
 		return -ENOMEM;
 	}
 
-	dist_base = ioremap(dist_phy_base, ACPI_GICV2_DIST_MEM_SIZE);
+	dist = (struct acpi_madt_generic_distributor *)header;
+	dist_base = ioremap(dist->base_address, ACPI_GICV2_DIST_MEM_SIZE);
 	if (!dist_base) {
 		pr_err("Unable to map GICD registers\n");
 		iounmap(cpu_base);
@@ -1302,4 +1297,10 @@ gic_v2_acpi_init(struct acpi_table_header *table)
 	acpi_irq_model = ACPI_IRQ_MODEL_GIC;
 	return 0;
 }
+IRQCHIP_ACPI_DECLARE(gic_v2, ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
+		     gic_validate_dist, ACPI_MADT_GIC_VERSION_V2,
+		     gic_v2_acpi_init);
+IRQCHIP_ACPI_DECLARE(gic_v2_maybe, ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
+		     gic_validate_dist, ACPI_MADT_GIC_VERSION_NONE,
+		     gic_v2_acpi_init);
 #endif
diff --git a/drivers/irqchip/irqchip.c b/drivers/irqchip/irqchip.c
index 1ee773e..2b35e68 100644
--- a/drivers/irqchip/irqchip.c
+++ b/drivers/irqchip/irqchip.c
@@ -27,8 +27,5 @@ extern struct of_device_id __irqchip_of_table[];
 void __init irqchip_init(void)
 {
 	of_irq_init(__irqchip_of_table);
-#if defined(CONFIG_ARM64) && defined(CONFIG_ACPI)
-	acpi_gic_init();	/* Temporary hack */
-#endif
 	acpi_probe_device_table(irqchip);
 }
diff --git a/include/linux/irqchip/arm-gic-acpi.h b/include/linux/irqchip/arm-gic-acpi.h
deleted file mode 100644
index de3419e..0000000
--- a/include/linux/irqchip/arm-gic-acpi.h
+++ /dev/null
@@ -1,31 +0,0 @@
-/*
- * Copyright (C) 2014, Linaro Ltd.
- *	Author: Tomasz Nowicki <tomasz.nowicki@linaro.org>
- *
- * 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 ARM_GIC_ACPI_H_
-#define ARM_GIC_ACPI_H_
-
-#ifdef CONFIG_ACPI
-
-/*
- * Hard code here, we can not get memory size from MADT (but FDT does),
- * Actually no need to do that, because this size can be inferred
- * from GIC spec.
- */
-#define ACPI_GICV2_DIST_MEM_SIZE	(SZ_4K)
-#define ACPI_GIC_CPU_IF_MEM_SIZE	(SZ_8K)
-
-struct acpi_table_header;
-
-int gic_v2_acpi_init(struct acpi_table_header *table);
-void acpi_gic_init(void);
-#else
-static inline void acpi_gic_init(void) { }
-#endif
-
-#endif /* ARM_GIC_ACPI_H_ */
-- 
2.1.4

Re: [PATCH v3 3/7] irqchip/gic: Convert the GIC driver to ACPI probing

[Catalin Marinas]

On Mon, Sep 28, 2015 at 03:49:14PM +0100, Marc Zyngier wrote:
> Now that we have a basic infrastructure to register irqchips and
> call them on discovery of a matching entry in MADT, convert the
> GIC driver to this new probing method.
> 
> It ends up being a code deletion party, which is a rather good thing.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm64/include/asm/acpi.h        |  1 -
>  arch/arm64/include/asm/irq.h         |  2 --
>  arch/arm64/kernel/acpi.c             | 25 -------------
>  drivers/irqchip/irq-gic.c            | 69 ++++++++++++++++++------------------
>  drivers/irqchip/irqchip.c            |  3 --
>  include/linux/irqchip/arm-gic-acpi.h | 31 ----------------
>  6 files changed, 35 insertions(+), 96 deletions(-)
>  delete mode 100644 include/linux/irqchip/arm-gic-acpi.h

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Re: [PATCH v3 3/7] irqchip/gic: Convert the GIC driver to ACPI probing

[Hanjun Guo]

On 09/28/2015 10:49 PM, Marc Zyngier wrote:
> Now that we have a basic infrastructure to register irqchips and
> call them on discovery of a matching entry in MADT, convert the
> GIC driver to this new probing method.
>
> It ends up being a code deletion party, which is a rather good thing.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>   arch/arm64/include/asm/acpi.h        |  1 -
>   arch/arm64/include/asm/irq.h         |  2 --
>   arch/arm64/kernel/acpi.c             | 25 -------------
>   drivers/irqchip/irq-gic.c            | 69 ++++++++++++++++++------------------
>   drivers/irqchip/irqchip.c            |  3 --
>   include/linux/irqchip/arm-gic-acpi.h | 31 ----------------
>   6 files changed, 35 insertions(+), 96 deletions(-)
>   delete mode 100644 include/linux/irqchip/arm-gic-acpi.h

I love the code deletion party too :)

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

[PATCH v3 4/7] clocksource/acpi: Add probing infrastructure for ACPI-based clocksources

[Marc Zyngier]

DT enjoys a rather nice probing infrastructure for clocksources,
while ACPI is so far stuck into a very distant past.

This patch introduces a declarative API, allowing clocksources
to be self-contained and be called when parsing the GTDT table.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/clocksource/clksrc-of.c   | 4 ++++
 include/asm-generic/vmlinux.lds.h | 1 +
 include/linux/clocksource.h       | 3 +++
 3 files changed, 8 insertions(+)

diff --git a/drivers/clocksource/clksrc-of.c b/drivers/clocksource/clksrc-of.c
index 0093a8e..a2105bd 100644
--- a/drivers/clocksource/clksrc-of.c
+++ b/drivers/clocksource/clksrc-of.c
@@ -14,6 +14,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/acpi.h>
 #include <linux/init.h>
 #include <linux/of.h>
 #include <linux/clocksource.h>
@@ -38,6 +39,9 @@ void __init clocksource_of_init(void)
 		init_func(np);
 		clocksources++;
 	}
+
+	clocksources += acpi_probe_device_table(clksrc);
+
 	if (!clocksources)
 		pr_crit("%s: no matching clocksources found\n", __func__);
 }
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index cd836cd..c4bd0e2 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -525,6 +525,7 @@
 	KERNEL_DTB()							\
 	IRQCHIP_OF_MATCH_TABLE()					\
 	ACPI_PROBE_TABLE(irqchip)					\
+	ACPI_PROBE_TABLE(clksrc)					\
 	EARLYCON_TABLE()						\
 	EARLYCON_OF_TABLES()
 
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 278dd27..8cde048 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -258,4 +258,7 @@ void acpi_generic_timer_init(void);
 static inline void acpi_generic_timer_init(void) { }
 #endif
 
+#define CLOCKSOURCE_ACPI_DECLARE(name, table_id, fn)		\
+	ACPI_DECLARE_PROBE_ENTRY(clksrc, name, table_id, 0, NULL, 0, fn)
+
 #endif /* _LINUX_CLOCKSOURCE_H */
-- 
2.1.4

[PATCH v3 5/7] clocksource: Add new CLKSRC_{PROBE,ACPI} config symbols

[Marc Zyngier]

The clocksource probing infrastructure currently depends on
CONFIG_CLKSRC_OF, which depends on CONFIG_OF. In order to make
this infrastructure selectable even if CONFIG_OF is not selected,
introduce a new CONFIG_CLKSRC_PROBE (which allow the infrastructure
to be compiled in), and CONFIG_CLKSRC_ACPI (which is the pendent
of CONFIG_CLKSRC_OF for ACPI).

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/clocksource/Kconfig  | 8 ++++++++
 drivers/clocksource/Makefile | 2 +-
 include/linux/clocksource.h  | 2 +-
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index a7726db..fefedf1 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -2,6 +2,14 @@ menu "Clock Source drivers"
 
 config CLKSRC_OF
 	bool
+	select CLKSRC_PROBE
+
+config CLKSRC_ACPI
+	bool
+	select CLKSRC_PROBE
+
+config CLKSRC_PROBE
+	bool
 
 config CLKSRC_I8253
 	bool
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 5c00863..b47be92 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -1,4 +1,4 @@
-obj-$(CONFIG_CLKSRC_OF)	+= clksrc-of.o
+obj-$(CONFIG_CLKSRC_PROBE)	+= clksrc-of.o
 obj-$(CONFIG_ATMEL_PIT)		+= timer-atmel-pit.o
 obj-$(CONFIG_ATMEL_ST)		+= timer-atmel-st.o
 obj-$(CONFIG_ATMEL_TCB_CLKSRC)	+= tcb_clksrc.o
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 8cde048..6eab6ab 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -246,7 +246,7 @@ extern int clocksource_i8253_init(void);
 #define CLOCKSOURCE_OF_DECLARE(name, compat, fn) \
 	OF_DECLARE_1(clksrc, name, compat, fn)
 
-#ifdef CONFIG_CLKSRC_OF
+#ifdef CONFIG_CLKSRC_PROBE
 extern void clocksource_of_init(void);
 #else
 static inline void clocksource_of_init(void) {}
-- 
2.1.4

[PATCH v3 6/7] clocksource/arm_arch_timer: Convert to ACPI probing

[Marc Zyngier]

It is now absolutely trivial to convert the arch timer driver to
use ACPI probing, just like its DT counterpart.

Let's enjoy another crapectomy.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kernel/time.c             |  6 ------
 drivers/clocksource/Kconfig          |  1 +
 drivers/clocksource/arm_arch_timer.c | 10 +---------
 include/linux/clocksource.h          |  6 ------
 4 files changed, 2 insertions(+), 21 deletions(-)

diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c
index 149151f..819a105 100644
--- a/arch/arm64/kernel/time.c
+++ b/arch/arm64/kernel/time.c
@@ -71,12 +71,6 @@ void __init time_init(void)
 
 	tick_setup_hrtimer_broadcast();
 
-	/*
-	 * Since ACPI or FDT will only one be available in the system,
-	 * we can use acpi_generic_timer_init() here safely
-	 */
-	acpi_generic_timer_init();
-
 	arch_timer_rate = arch_timer_get_rate();
 	if (!arch_timer_rate)
 		panic("Unable to initialise architected timer.\n");
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index fefedf1..1a9c5dc 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -131,6 +131,7 @@ config CLKSRC_STM32
 config ARM_ARCH_TIMER
 	bool
 	select CLKSRC_OF if OF
+	select CLKSRC_ACPI if ACPI
 
 config ARM_ARCH_TIMER_EVTSTREAM
 	bool "Support for ARM architected timer event stream generation"
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index d6e3e49..c64d543 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -864,13 +864,5 @@ static int __init arch_timer_acpi_init(struct acpi_table_header *table)
 	arch_timer_init();
 	return 0;
 }
-
-/* Initialize all the generic timers presented in GTDT */
-void __init acpi_generic_timer_init(void)
-{
-	if (acpi_disabled)
-		return;
-
-	acpi_table_parse(ACPI_SIG_GTDT, arch_timer_acpi_init);
-}
+CLOCKSOURCE_ACPI_DECLARE(arch_timer, ACPI_SIG_GTDT, arch_timer_acpi_init);
 #endif
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 6eab6ab..116645f 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -252,12 +252,6 @@ extern void clocksource_of_init(void);
 static inline void clocksource_of_init(void) {}
 #endif
 
-#ifdef CONFIG_ACPI
-void acpi_generic_timer_init(void);
-#else
-static inline void acpi_generic_timer_init(void) { }
-#endif
-
 #define CLOCKSOURCE_ACPI_DECLARE(name, table_id, fn)		\
 	ACPI_DECLARE_PROBE_ENTRY(clksrc, name, table_id, 0, NULL, 0, fn)
 
-- 
2.1.4

Re: [PATCH v3 6/7] clocksource/arm_arch_timer: Convert to ACPI probing

[Catalin Marinas]

On Mon, Sep 28, 2015 at 03:49:17PM +0100, Marc Zyngier wrote:
> It is now absolutely trivial to convert the arch timer driver to
> use ACPI probing, just like its DT counterpart.
> 
> Let's enjoy another crapectomy.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

[PATCH v3 7/7] clocksource: cosmetic: Drop OF 'dependency' from symbols

[Marc Zyngier]

Seeing the 'of' characters in a symbol that is being called from
ACPI seems to freak out people. So let's do a bit of pointless
renaming so that these folks do feel at home.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kernel/time.c                              | 2 +-
 arch/arm/mach-omap2/timer.c                         | 4 ++--
 arch/arm/mach-rockchip/rockchip.c                   | 2 +-
 arch/arm/mach-shmobile/setup-r8a7779.c              | 2 +-
 arch/arm/mach-shmobile/setup-rcar-gen2.c            | 2 +-
 arch/arm/mach-spear/spear13xx.c                     | 2 +-
 arch/arm/mach-sunxi/sunxi.c                         | 2 +-
 arch/arm/mach-u300/core.c                           | 2 +-
 arch/arm/mach-ux500/timer.c                         | 2 +-
 arch/arm/mach-zynq/common.c                         | 2 +-
 arch/arm64/kernel/time.c                            | 2 +-
 arch/microblaze/kernel/setup.c                      | 2 +-
 arch/mips/pistachio/time.c                          | 2 +-
 arch/mips/ralink/clk.c                              | 2 +-
 arch/nios2/kernel/time.c                            | 2 +-
 arch/xtensa/kernel/time.c                           | 2 +-
 drivers/clocksource/Makefile                        | 2 +-
 drivers/clocksource/{clksrc-of.c => clksrc-probe.c} | 2 +-
 include/linux/clocksource.h                         | 4 ++--
 19 files changed, 21 insertions(+), 21 deletions(-)
 rename drivers/clocksource/{clksrc-of.c => clksrc-probe.c} (97%)

diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
index a66e37e..97b22fa 100644
--- a/arch/arm/kernel/time.c
+++ b/arch/arm/kernel/time.c
@@ -120,6 +120,6 @@ void __init time_init(void)
 #ifdef CONFIG_COMMON_CLK
 		of_clk_init(NULL);
 #endif
-		clocksource_of_init();
+		clocksource_probe();
 	}
 }
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index a556551..bef4183 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -647,7 +647,7 @@ static OMAP_SYS_32K_TIMER_INIT(4, 1, "timer_32k_ck", "ti,timer-alwon",
 void __init omap4_local_timer_init(void)
 {
 	omap4_sync32k_timer_init();
-	clocksource_of_init();
+	clocksource_probe();
 }
 #else
 void __init omap4_local_timer_init(void)
@@ -663,7 +663,7 @@ void __init omap5_realtime_timer_init(void)
 	omap4_sync32k_timer_init();
 	realtime_counter_init();
 
-	clocksource_of_init();
+	clocksource_probe();
 }
 #endif /* CONFIG_SOC_OMAP5 || CONFIG_SOC_DRA7XX */
 
diff --git a/arch/arm/mach-rockchip/rockchip.c b/arch/arm/mach-rockchip/rockchip.c
index b6cf3b4..251c7b9 100644
--- a/arch/arm/mach-rockchip/rockchip.c
+++ b/arch/arm/mach-rockchip/rockchip.c
@@ -67,7 +67,7 @@ static void __init rockchip_timer_init(void)
 	}
 
 	of_clk_init(NULL);
-	clocksource_of_init();
+	clocksource_probe();
 }
 
 static void __init rockchip_dt_init(void)
diff --git a/arch/arm/mach-shmobile/setup-r8a7779.c b/arch/arm/mach-shmobile/setup-r8a7779.c
index 6bfa640..1e572a9 100644
--- a/arch/arm/mach-shmobile/setup-r8a7779.c
+++ b/arch/arm/mach-shmobile/setup-r8a7779.c
@@ -97,7 +97,7 @@ static u32 __init r8a7779_read_mode_pins(void)
 static void __init r8a7779_init_time(void)
 {
 	r8a7779_clocks_init(r8a7779_read_mode_pins());
-	clocksource_of_init();
+	clocksource_probe();
 }
 
 static const char *const r8a7779_compat_dt[] __initconst = {
diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c b/arch/arm/mach-shmobile/setup-rcar-gen2.c
index aa33392..9eccde3 100644
--- a/arch/arm/mach-shmobile/setup-rcar-gen2.c
+++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c
@@ -128,7 +128,7 @@ void __init rcar_gen2_timer_init(void)
 #endif /* CONFIG_ARM_ARCH_TIMER */
 
 	rcar_gen2_clocks_init(mode);
-	clocksource_of_init();
+	clocksource_probe();
 }
 
 struct memory_reserve_config {
diff --git a/arch/arm/mach-spear/spear13xx.c b/arch/arm/mach-spear/spear13xx.c
index b7afce6..ca2f6a8 100644
--- a/arch/arm/mach-spear/spear13xx.c
+++ b/arch/arm/mach-spear/spear13xx.c
@@ -124,5 +124,5 @@ void __init spear13xx_timer_init(void)
 	clk_put(pclk);
 
 	spear_setup_of_timer();
-	clocksource_of_init();
+	clocksource_probe();
 }
diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c
index 65bab28..223c9e9 100644
--- a/arch/arm/mach-sunxi/sunxi.c
+++ b/arch/arm/mach-sunxi/sunxi.c
@@ -46,7 +46,7 @@ static void __init sun6i_timer_init(void)
 	of_clk_init(NULL);
 	if (IS_ENABLED(CONFIG_RESET_CONTROLLER))
 		sun6i_reset_init();
-	clocksource_of_init();
+	clocksource_probe();
 }
 
 DT_MACHINE_START(SUN6I_DT, "Allwinner sun6i (A31) Family")
diff --git a/arch/arm/mach-u300/core.c b/arch/arm/mach-u300/core.c
index 35670b1..546338b 100644
--- a/arch/arm/mach-u300/core.c
+++ b/arch/arm/mach-u300/core.c
@@ -408,7 +408,7 @@ static const char * u300_board_compat[] = {
 DT_MACHINE_START(U300_DT, "U300 S335/B335 (Device Tree)")
 	.map_io		= u300_map_io,
 	.init_irq	= u300_init_irq_dt,
-	.init_time	= clocksource_of_init,
+	.init_time	= clocksource_probe,
 	.init_machine	= u300_init_machine_dt,
 	.restart	= u300_restart,
 	.dt_compat      = u300_board_compat,
diff --git a/arch/arm/mach-ux500/timer.c b/arch/arm/mach-ux500/timer.c
index ff28d8a..8d2d233 100644
--- a/arch/arm/mach-ux500/timer.c
+++ b/arch/arm/mach-ux500/timer.c
@@ -44,5 +44,5 @@ void __init ux500_timer_init(void)
 
 dt_fail:
 	clksrc_dbx500_prcmu_init(prcmu_timer_base);
-	clocksource_of_init();
+	clocksource_probe();
 }
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index 5a6e4e2..6f39d03 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -154,7 +154,7 @@ static void __init zynq_timer_init(void)
 
 	zynq_clock_init();
 	of_clk_init(NULL);
-	clocksource_of_init();
+	clocksource_probe();
 }
 
 static struct map_desc zynq_cortex_a9_scu_map __initdata = {
diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c
index 819a105..13339b6 100644
--- a/arch/arm64/kernel/time.c
+++ b/arch/arm64/kernel/time.c
@@ -67,7 +67,7 @@ void __init time_init(void)
 	u32 arch_timer_rate;
 
 	of_clk_init(NULL);
-	clocksource_of_init();
+	clocksource_probe();
 
 	tick_setup_hrtimer_broadcast();
 
diff --git a/arch/microblaze/kernel/setup.c b/arch/microblaze/kernel/setup.c
index ab5b488..89a2a93 100644
--- a/arch/microblaze/kernel/setup.c
+++ b/arch/microblaze/kernel/setup.c
@@ -194,7 +194,7 @@ void __init time_init(void)
 {
 	of_clk_init(NULL);
 	setup_cpuinfo_clk();
-	clocksource_of_init();
+	clocksource_probe();
 }
 
 #ifdef CONFIG_DEBUG_FS
diff --git a/arch/mips/pistachio/time.c b/arch/mips/pistachio/time.c
index 8a37734..1022201 100644
--- a/arch/mips/pistachio/time.c
+++ b/arch/mips/pistachio/time.c
@@ -39,7 +39,7 @@ void __init plat_time_init(void)
 	struct clk *clk;
 
 	of_clk_init(NULL);
-	clocksource_of_init();
+	clocksource_probe();
 
 	np = of_get_cpu_node(0, NULL);
 	if (!np) {
diff --git a/arch/mips/ralink/clk.c b/arch/mips/ralink/clk.c
index feb5a9b..25c4a61 100644
--- a/arch/mips/ralink/clk.c
+++ b/arch/mips/ralink/clk.c
@@ -75,5 +75,5 @@ void __init plat_time_init(void)
 	pr_info("CPU Clock: %ldMHz\n", clk_get_rate(clk) / 1000000);
 	mips_hpt_frequency = clk_get_rate(clk) / 2;
 	clk_put(clk);
-	clocksource_of_init();
+	clocksource_probe();
 }
diff --git a/arch/nios2/kernel/time.c b/arch/nios2/kernel/time.c
index bbc3f91..e835dda 100644
--- a/arch/nios2/kernel/time.c
+++ b/arch/nios2/kernel/time.c
@@ -324,7 +324,7 @@ void __init time_init(void)
 	if (count < 2)
 		panic("%d timer is found, it needs 2 timers in system\n", count);
 
-	clocksource_of_init();
+	clocksource_probe();
 }
 
 CLOCKSOURCE_OF_DECLARE(nios2_timer, ALTR_TIMER_COMPATIBLE, nios2_time_init);
diff --git a/arch/xtensa/kernel/time.c b/arch/xtensa/kernel/time.c
index b97767d..b9ad9fe 100644
--- a/arch/xtensa/kernel/time.c
+++ b/arch/xtensa/kernel/time.c
@@ -148,7 +148,7 @@ void __init time_init(void)
 	local_timer_setup(0);
 	setup_irq(this_cpu_ptr(&ccount_timer)->evt.irq, &timer_irqaction);
 	sched_clock_register(ccount_sched_clock_read, 32, ccount_freq);
-	clocksource_of_init();
+	clocksource_probe();
 }
 
 /*
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index b47be92..51856d5 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -1,4 +1,4 @@
-obj-$(CONFIG_CLKSRC_PROBE)	+= clksrc-of.o
+obj-$(CONFIG_CLKSRC_PROBE)	+= clksrc-probe.o
 obj-$(CONFIG_ATMEL_PIT)		+= timer-atmel-pit.o
 obj-$(CONFIG_ATMEL_ST)		+= timer-atmel-st.o
 obj-$(CONFIG_ATMEL_TCB_CLKSRC)	+= tcb_clksrc.o
diff --git a/drivers/clocksource/clksrc-of.c b/drivers/clocksource/clksrc-probe.c
similarity index 97%
rename from drivers/clocksource/clksrc-of.c
rename to drivers/clocksource/clksrc-probe.c
index a2105bd..7cb6c92 100644
--- a/drivers/clocksource/clksrc-of.c
+++ b/drivers/clocksource/clksrc-probe.c
@@ -24,7 +24,7 @@ extern struct of_device_id __clksrc_of_table[];
 static const struct of_device_id __clksrc_of_table_sentinel
 	__used __section(__clksrc_of_table_end);
 
-void __init clocksource_of_init(void)
+void __init clocksource_probe(void)
 {
 	struct device_node *np;
 	const struct of_device_id *match;
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 116645f..7784b59 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -247,9 +247,9 @@ extern int clocksource_i8253_init(void);
 	OF_DECLARE_1(clksrc, name, compat, fn)
 
 #ifdef CONFIG_CLKSRC_PROBE
-extern void clocksource_of_init(void);
+extern void clocksource_probe(void);
 #else
-static inline void clocksource_of_init(void) {}
+static inline void clocksource_probe(void) {}
 #endif
 
 #define CLOCKSOURCE_ACPI_DECLARE(name, table_id, fn)		\
-- 
2.1.4

Re: [PATCH v3 7/7] clocksource: cosmetic: Drop OF 'dependency' from symbols

[Catalin Marinas]

On Mon, Sep 28, 2015 at 03:49:18PM +0100, Marc Zyngier wrote:
> Seeing the 'of' characters in a symbol that is being called from
> ACPI seems to freak out people. So let's do a bit of pointless
> renaming so that these folks do feel at home.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/kernel/time.c                              | 2 +-
>  arch/arm/mach-omap2/timer.c                         | 4 ++--
>  arch/arm/mach-rockchip/rockchip.c                   | 2 +-
>  arch/arm/mach-shmobile/setup-r8a7779.c              | 2 +-
>  arch/arm/mach-shmobile/setup-rcar-gen2.c            | 2 +-
>  arch/arm/mach-spear/spear13xx.c                     | 2 +-
>  arch/arm/mach-sunxi/sunxi.c                         | 2 +-
>  arch/arm/mach-u300/core.c                           | 2 +-
>  arch/arm/mach-ux500/timer.c                         | 2 +-
>  arch/arm/mach-zynq/common.c                         | 2 +-
>  arch/arm64/kernel/time.c                            | 2 +-
>  arch/microblaze/kernel/setup.c                      | 2 +-
>  arch/mips/pistachio/time.c                          | 2 +-
>  arch/mips/ralink/clk.c                              | 2 +-
>  arch/nios2/kernel/time.c                            | 2 +-
>  arch/xtensa/kernel/time.c                           | 2 +-
>  drivers/clocksource/Makefile                        | 2 +-
>  drivers/clocksource/{clksrc-of.c => clksrc-probe.c} | 2 +-
>  include/linux/clocksource.h                         | 4 ++--
>  19 files changed, 21 insertions(+), 21 deletions(-)
>  rename drivers/clocksource/{clksrc-of.c => clksrc-probe.c} (97%)

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Re: [PATCH v3 7/7] clocksource: cosmetic: Drop OF 'dependency' from symbols

[Hanjun Guo]

On 09/28/2015 10:49 PM, Marc Zyngier wrote:
> Seeing the 'of' characters in a symbol that is being called from
> ACPI seems to freak out people. So let's do a bit of pointless
> renaming so that these folks do feel at home.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>   arch/arm/kernel/time.c                              | 2 +-
>   arch/arm/mach-omap2/timer.c                         | 4 ++--
>   arch/arm/mach-rockchip/rockchip.c                   | 2 +-
>   arch/arm/mach-shmobile/setup-r8a7779.c              | 2 +-
>   arch/arm/mach-shmobile/setup-rcar-gen2.c            | 2 +-
>   arch/arm/mach-spear/spear13xx.c                     | 2 +-
>   arch/arm/mach-sunxi/sunxi.c                         | 2 +-
>   arch/arm/mach-u300/core.c                           | 2 +-
>   arch/arm/mach-ux500/timer.c                         | 2 +-
>   arch/arm/mach-zynq/common.c                         | 2 +-
>   arch/arm64/kernel/time.c                            | 2 +-
>   arch/microblaze/kernel/setup.c                      | 2 +-
>   arch/mips/pistachio/time.c                          | 2 +-
>   arch/mips/ralink/clk.c                              | 2 +-
>   arch/nios2/kernel/time.c                            | 2 +-
>   arch/xtensa/kernel/time.c                           | 2 +-
>   drivers/clocksource/Makefile                        | 2 +-
>   drivers/clocksource/{clksrc-of.c => clksrc-probe.c} | 2 +-
>   include/linux/clocksource.h                         | 4 ++--
>   19 files changed, 21 insertions(+), 21 deletions(-)
>   rename drivers/clocksource/{clksrc-of.c => clksrc-probe.c} (97%)

Thanks for fixing my worrying of making ACPI depends on OF
in previous version, for patch 4/7 to patch 7/7,

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

Re: [PATCH v3 0/7] Early ACPI probing infrastructure

[Rafael J. Wysocki]

On Monday, September 28, 2015 03:49:11 PM Marc Zyngier wrote:
> IRQ controllers and timers are the two types of device the kernel
> requires before being able to use the device driver model.
> 
> The Device Tree infrastructure makes it very easy to make these
> discoverable by the rest of the kernel. For example, each interrupt
> controller driver has at least one entry like this:
> 
> IRQCHIP_DECLARE(gic_400, "arm,gic-400", gic_of_init);
> 
> which says: if you find a node having "arm,gic-400" as a compatible
> string in the device tree, then call gic_of_init with this node as a
> parameter. The probing itself is done by the OF layer when the
> architecture code calls of_irq_init() (usually via irqchip_init).
> 
> This has a number of benefits:
> 
> - The irqchip code is self-contained. No architecture specific entry
> point, no exposed symbols. Just a standard interface.
> 
> - The low-level architecture code doesn't have to know about which
> interrupt controller is present. It just calls into the firmware
> interface (of_irq_init) which is going to sort things out.
> 
> Similar infrastructure is provided for the timers/clock sources. Note
> that this is not a replacement for the device model, but acts as a
> probing infrastructure for things that are required too early for the
> device infrastructure to be available.
> 
> What I'm aiming for is to introduce the same level of abstraction for
> ACPI, or at least for the few bits that are required before a full blown
> ACPI/device model can be used. For this, I introduce something vaguely
> similar:
> 
> IRQCHIP_ACPI_DECLARE(gic_v2, ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
> 		     gic_validate_dist, ACPI_MADT_GIC_VERSION_V2,
> 		     gic_v2_acpi_init);
> 
> which says: if you find a ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR entry in
> MADT (implied by the macro), and that entry is of type
> ACPI_MADT_GIC_VERSION_V2 (as checked by gic_validate_dist), then call
> gic_v2_acpi_init with the entry as a parameter. A bit more convoluted,
> but still without any special entry point.
> 
> The various interrupt controller drivers can then implement the above,
> and the arch code can use a firmware-specific call to get the probing
> done, still oblivious to what interrupt controller is being used. It
> also makes the adaptation of a DT driver to ACPI easier.
> 
> It turns out that providing such a probing infrastructure is rather
> easy, and provides a much deserved cleanup in both the arch code, the
> GIC driver, and the architected timer driver. An additional (and very
> much optional) patch renames files and symbols to make it obvious that
> the infrastructure is not just for DT anymore.
> 
> I'm sure there is some more code to be deleted, and one can only
> wonder why this wasn't done before the arm64 code was initially merged
> (the diffstat says it all...).
> 
> Patches are against Rafael's bleeding-edge branch, and a branch is
> available at
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git acpi/device-probing-v3
> 
> * From v2:
>   - Addressed Rafael's comments (title for patch #1, macro/field names)
>   - New config symbols for clocksources
>   - Additional and optional last patch to get rid of the apparent
>     dependency between the probing infrastructure and DT.
> 
> * From the initial version:
>   - Make the infrastructure more DT like by providing an
>     acpi_probe_entry array per "device type" (one for irqchips, one
>     for clocksources). This means that entries can depend on any ACPI
>     static table.
>   - Use some cpp magic to reduce the amount of code added to an
>     absolute minimum.
>   - Rebased on v4.3-rc1
> 
> Marc Zyngier (7):
>   acpi: Add early device probing infrastructure
>   irqchip/acpi: Add probing infrastructure for ACPI-based irqchips
>   irqchip/gic: Convert the GIC driver to ACPI probing
>   clocksource/acpi: Add probing infrastructure for ACPI-based
>     clocksources
>   clocksource: Add new CLKSRC_{PROBE,ACPI} config symbols
>   clocksource/arm_arch_timer: Convert to ACPI probing
>   clocksource: cosmetic: Drop OF 'dependency' from symbols

If you want me to apply this series, ACKs are needed for patches [3-7/7].

They are fine by me, but I'd also like to hear from the other maintainers.

Thanks,
Rafael

Re: [PATCH v3 0/7] Early ACPI probing infrastructure

[Thomas Gleixner]

On Tue, 29 Sep 2015, Rafael J. Wysocki wrote:
> If you want me to apply this series, ACKs are needed for patches [3-7/7].
> 
> They are fine by me, but I'd also like to hear from the other maintainers.

Acked-by: Thomas Gleixner <tglx@linutronix.de>

Re: [PATCH v3 0/7] Early ACPI probing infrastructure

[Rafael J. Wysocki]

On Wednesday, September 30, 2015 12:44:29 PM Thomas Gleixner wrote:
> On Tue, 29 Sep 2015, Rafael J. Wysocki wrote:
> > If you want me to apply this series, ACKs are needed for patches [3-7/7].
> > 
> > They are fine by me, but I'd also like to hear from the other maintainers.
> 
> Acked-by: Thomas Gleixner <tglx@linutronix.de>

Thanks Thomas!

Series applied.

Rafael

Re: [PATCH v3 0/7] Early ACPI probing infrastructure

[Hanjun Guo]

On 09/28/2015 10:49 PM, Marc Zyngier wrote:
> IRQ controllers and timers are the two types of device the kernel
> requires before being able to use the device driver model.
>
> The Device Tree infrastructure makes it very easy to make these
> discoverable by the rest of the kernel. For example, each interrupt
> controller driver has at least one entry like this:
>
> IRQCHIP_DECLARE(gic_400, "arm,gic-400", gic_of_init);
>
> which says: if you find a node having "arm,gic-400" as a compatible
> string in the device tree, then call gic_of_init with this node as a
> parameter. The probing itself is done by the OF layer when the
> architecture code calls of_irq_init() (usually via irqchip_init).
>
> This has a number of benefits:
>
> - The irqchip code is self-contained. No architecture specific entry
> point, no exposed symbols. Just a standard interface.
>
> - The low-level architecture code doesn't have to know about which
> interrupt controller is present. It just calls into the firmware
> interface (of_irq_init) which is going to sort things out.
>
> Similar infrastructure is provided for the timers/clock sources. Note
> that this is not a replacement for the device model, but acts as a
> probing infrastructure for things that are required too early for the
> device infrastructure to be available.
>
> What I'm aiming for is to introduce the same level of abstraction for
> ACPI, or at least for the few bits that are required before a full blown
> ACPI/device model can be used. For this, I introduce something vaguely
> similar:
>
> IRQCHIP_ACPI_DECLARE(gic_v2, ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
> 		     gic_validate_dist, ACPI_MADT_GIC_VERSION_V2,
> 		     gic_v2_acpi_init);
>
> which says: if you find a ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR entry in
> MADT (implied by the macro), and that entry is of type
> ACPI_MADT_GIC_VERSION_V2 (as checked by gic_validate_dist), then call
> gic_v2_acpi_init with the entry as a parameter. A bit more convoluted,
> but still without any special entry point.
>
> The various interrupt controller drivers can then implement the above,
> and the arch code can use a firmware-specific call to get the probing
> done, still oblivious to what interrupt controller is being used. It
> also makes the adaptation of a DT driver to ACPI easier.
>
> It turns out that providing such a probing infrastructure is rather
> easy, and provides a much deserved cleanup in both the arch code, the
> GIC driver, and the architected timer driver. An additional (and very
> much optional) patch renames files and symbols to make it obvious that
> the infrastructure is not just for DT anymore.
>
> I'm sure there is some more code to be deleted, and one can only
> wonder why this wasn't done before the arm64 code was initially merged
> (the diffstat says it all...).
>
> Patches are against Rafael's bleeding-edge branch, and a branch is
> available at
>
> git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git acpi/device-probing-v3
>
> * From v2:
>    - Addressed Rafael's comments (title for patch #1, macro/field names)
>    - New config symbols for clocksources

I like this one most :)

>    - Additional and optional last patch to get rid of the apparent
>      dependency between the probing infrastructure and DT.
>
> * From the initial version:
>    - Make the infrastructure more DT like by providing an
>      acpi_probe_entry array per "device type" (one for irqchips, one
>      for clocksources). This means that entries can depend on any ACPI
>      static table.
>    - Use some cpp magic to reduce the amount of code added to an
>      absolute minimum.
>    - Rebased on v4.3-rc1
>
> Marc Zyngier (7):
>    acpi: Add early device probing infrastructure
>    irqchip/acpi: Add probing infrastructure for ACPI-based irqchips
>    irqchip/gic: Convert the GIC driver to ACPI probing
>    clocksource/acpi: Add probing infrastructure for ACPI-based
>      clocksources
>    clocksource: Add new CLKSRC_{PROBE,ACPI} config symbols
>    clocksource/arm_arch_timer: Convert to ACPI probing
>    clocksource: cosmetic: Drop OF 'dependency' from symbols

Tested locally on QEMU (as I'm in home so no hardware is available) and
it boots as before with GICv2 and arch timer work ok (sure with ACPI),
for this patch set:

Tested-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun