* [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial
2016-02-29 12:41 [PATCH v3 0/7] ACPI: parse the DBG2 table Aleksey Makarov
@ 2016-02-29 12:41 ` Aleksey Makarov
2016-03-01 14:50 ` Peter Hurley
2016-02-29 12:41 ` [PATCH v3 2/7] ACPI: add definitions of DBG2 subtypes Aleksey Makarov
` (5 subsequent siblings)
6 siblings, 1 reply; 32+ messages in thread
From: Aleksey Makarov @ 2016-02-29 12:41 UTC (permalink / raw)
To: linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Aleksey Makarov,
Russell King, Greg Kroah-Hartman, Rafael J . Wysocki,
Leif Lindholm, Graeme Gregory, Al Stone, Christopher Covington,
Matthias Brugger, Rob Herring, Frank Rowand, Grant Likely,
Jiri Slaby, devicetree
From: Leif Lindholm <leif.lindholm@linaro.org>
We have multiple "earlycon" early_param handlers - merge the DT one into
the main earlycon one. This means the earlycon early_param handler does
not just return success if no options are specified.
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
drivers/of/fdt.c | 11 +----------
drivers/tty/serial/earlycon.c | 3 ++-
include/linux/of_fdt.h | 2 ++
3 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 3349d2a..0547256 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -805,7 +805,7 @@ static inline void early_init_dt_check_for_initrd(unsigned long node)
#ifdef CONFIG_SERIAL_EARLYCON
-static int __init early_init_dt_scan_chosen_serial(void)
+int __init early_init_dt_scan_chosen_serial(void)
{
int offset;
const char *p, *q, *options = NULL;
@@ -849,15 +849,6 @@ static int __init early_init_dt_scan_chosen_serial(void)
}
return -ENODEV;
}
-
-static int __init setup_of_earlycon(char *buf)
-{
- if (buf)
- return 0;
-
- return early_init_dt_scan_chosen_serial();
-}
-early_param("earlycon", setup_of_earlycon);
#endif
/**
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 067783f..d217366 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -17,6 +17,7 @@
#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/io.h>
+#include <linux/of_fdt.h>
#include <linux/serial_core.h>
#include <linux/sizes.h>
#include <linux/of.h>
@@ -209,7 +210,7 @@ static int __init param_setup_earlycon(char *buf)
* don't generate a warning from parse_early_params() in that case
*/
if (!buf || !buf[0])
- return 0;
+ return early_init_dt_scan_chosen_serial();
err = setup_earlycon(buf);
if (err == -ENOENT || err == -EALREADY)
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index 2fbe868..56b2a43 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -63,6 +63,7 @@ extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
int depth, void *data);
extern int early_init_dt_scan_memory(unsigned long node, const char *uname,
int depth, void *data);
+extern int early_init_dt_scan_chosen_serial(void);
extern void early_init_fdt_scan_reserved_mem(void);
extern void early_init_fdt_reserve_self(void);
extern void early_init_dt_add_memory_arch(u64 base, u64 size);
@@ -91,6 +92,7 @@ extern void early_get_first_memblock_info(void *, phys_addr_t *);
extern u64 of_flat_dt_translate_address(unsigned long node);
extern void of_fdt_limit_memory(int limit);
#else /* CONFIG_OF_FLATTREE */
+static inline int early_init_dt_scan_chosen_serial(void) { return -ENODEV; }
static inline void early_init_fdt_scan_reserved_mem(void) {}
static inline void early_init_fdt_reserve_self(void) {}
static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
--
2.7.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial
2016-02-29 12:41 ` [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial Aleksey Makarov
@ 2016-03-01 14:50 ` Peter Hurley
2016-03-01 16:31 ` Aleksey Makarov
0 siblings, 1 reply; 32+ messages in thread
From: Peter Hurley @ 2016-03-01 14:50 UTC (permalink / raw)
To: Aleksey Makarov, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Rob Herring, Frank Rowand, Grant Likely, Jiri Slaby, devicetree
On 02/29/2016 04:41 AM, Aleksey Makarov wrote:
> From: Leif Lindholm <leif.lindholm@linaro.org>
>
> We have multiple "earlycon" early_param handlers - merge the DT one into
> the main earlycon one. This means the earlycon early_param handler does
> not just return success if no options are specified.
Why is this patch necessary?
Is this cleanup or required for some reason in other parts of this
series?
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> ---
> drivers/of/fdt.c | 11 +----------
> drivers/tty/serial/earlycon.c | 3 ++-
> include/linux/of_fdt.h | 2 ++
> 3 files changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 3349d2a..0547256 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -805,7 +805,7 @@ static inline void early_init_dt_check_for_initrd(unsigned long node)
>
> #ifdef CONFIG_SERIAL_EARLYCON
>
> -static int __init early_init_dt_scan_chosen_serial(void)
> +int __init early_init_dt_scan_chosen_serial(void)
> {
> int offset;
> const char *p, *q, *options = NULL;
> @@ -849,15 +849,6 @@ static int __init early_init_dt_scan_chosen_serial(void)
> }
> return -ENODEV;
> }
> -
> -static int __init setup_of_earlycon(char *buf)
> -{
> - if (buf)
> - return 0;
> -
> - return early_init_dt_scan_chosen_serial();
> -}
> -early_param("earlycon", setup_of_earlycon);
> #endif
>
> /**
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index 067783f..d217366 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -17,6 +17,7 @@
> #include <linux/kernel.h>
> #include <linux/init.h>
> #include <linux/io.h>
> +#include <linux/of_fdt.h>
> #include <linux/serial_core.h>
> #include <linux/sizes.h>
> #include <linux/of.h>
> @@ -209,7 +210,7 @@ static int __init param_setup_earlycon(char *buf)
> * don't generate a warning from parse_early_params() in that case
> */
> if (!buf || !buf[0])
> - return 0;
> + return early_init_dt_scan_chosen_serial();
>
> err = setup_earlycon(buf);
> if (err == -ENOENT || err == -EALREADY)
> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> index 2fbe868..56b2a43 100644
> --- a/include/linux/of_fdt.h
> +++ b/include/linux/of_fdt.h
> @@ -63,6 +63,7 @@ extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
> int depth, void *data);
> extern int early_init_dt_scan_memory(unsigned long node, const char *uname,
> int depth, void *data);
> +extern int early_init_dt_scan_chosen_serial(void);
> extern void early_init_fdt_scan_reserved_mem(void);
> extern void early_init_fdt_reserve_self(void);
> extern void early_init_dt_add_memory_arch(u64 base, u64 size);
> @@ -91,6 +92,7 @@ extern void early_get_first_memblock_info(void *, phys_addr_t *);
> extern u64 of_flat_dt_translate_address(unsigned long node);
> extern void of_fdt_limit_memory(int limit);
> #else /* CONFIG_OF_FLATTREE */
> +static inline int early_init_dt_scan_chosen_serial(void) { return -ENODEV; }
> static inline void early_init_fdt_scan_reserved_mem(void) {}
> static inline void early_init_fdt_reserve_self(void) {}
> static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial
2016-03-01 14:50 ` Peter Hurley
@ 2016-03-01 16:31 ` Aleksey Makarov
2016-03-01 17:24 ` Peter Hurley
0 siblings, 1 reply; 32+ messages in thread
From: Aleksey Makarov @ 2016-03-01 16:31 UTC (permalink / raw)
To: Peter Hurley, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Rob Herring, Frank Rowand, Grant Likely, Jiri Slaby, devicetree
On 03/01/2016 05:50 PM, Peter Hurley wrote:
> On 02/29/2016 04:41 AM, Aleksey Makarov wrote:
>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>
>> We have multiple "earlycon" early_param handlers - merge the DT one into
>> the main earlycon one. This means the earlycon early_param handler does
>> not just return success if no options are specified.
>
> Why is this patch necessary?
>
> Is this cleanup or required for some reason in other parts of this
> series?
It's a cleanup.
Also, without this, the series has to introduce a new (third) handler for "earlycon=".
>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>> ---
>> drivers/of/fdt.c | 11 +----------
>> drivers/tty/serial/earlycon.c | 3 ++-
>> include/linux/of_fdt.h | 2 ++
>> 3 files changed, 5 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index 3349d2a..0547256 100644
>> --- a/drivers/of/fdt.c
>> +++ b/drivers/of/fdt.c
>> @@ -805,7 +805,7 @@ static inline void early_init_dt_check_for_initrd(unsigned long node)
>>
>> #ifdef CONFIG_SERIAL_EARLYCON
>>
>> -static int __init early_init_dt_scan_chosen_serial(void)
>> +int __init early_init_dt_scan_chosen_serial(void)
>> {
>> int offset;
>> const char *p, *q, *options = NULL;
>> @@ -849,15 +849,6 @@ static int __init early_init_dt_scan_chosen_serial(void)
>> }
>> return -ENODEV;
>> }
>> -
>> -static int __init setup_of_earlycon(char *buf)
>> -{
>> - if (buf)
>> - return 0;
>> -
>> - return early_init_dt_scan_chosen_serial();
>> -}
>> -early_param("earlycon", setup_of_earlycon);
>> #endif
>>
>> /**
>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>> index 067783f..d217366 100644
>> --- a/drivers/tty/serial/earlycon.c
>> +++ b/drivers/tty/serial/earlycon.c
>> @@ -17,6 +17,7 @@
>> #include <linux/kernel.h>
>> #include <linux/init.h>
>> #include <linux/io.h>
>> +#include <linux/of_fdt.h>
>> #include <linux/serial_core.h>
>> #include <linux/sizes.h>
>> #include <linux/of.h>
>> @@ -209,7 +210,7 @@ static int __init param_setup_earlycon(char *buf)
>> * don't generate a warning from parse_early_params() in that case
>> */
>> if (!buf || !buf[0])
>> - return 0;
>> + return early_init_dt_scan_chosen_serial();
>>
>> err = setup_earlycon(buf);
>> if (err == -ENOENT || err == -EALREADY)
>> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
>> index 2fbe868..56b2a43 100644
>> --- a/include/linux/of_fdt.h
>> +++ b/include/linux/of_fdt.h
>> @@ -63,6 +63,7 @@ extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
>> int depth, void *data);
>> extern int early_init_dt_scan_memory(unsigned long node, const char *uname,
>> int depth, void *data);
>> +extern int early_init_dt_scan_chosen_serial(void);
>> extern void early_init_fdt_scan_reserved_mem(void);
>> extern void early_init_fdt_reserve_self(void);
>> extern void early_init_dt_add_memory_arch(u64 base, u64 size);
>> @@ -91,6 +92,7 @@ extern void early_get_first_memblock_info(void *, phys_addr_t *);
>> extern u64 of_flat_dt_translate_address(unsigned long node);
>> extern void of_fdt_limit_memory(int limit);
>> #else /* CONFIG_OF_FLATTREE */
>> +static inline int early_init_dt_scan_chosen_serial(void) { return -ENODEV; }
>> static inline void early_init_fdt_scan_reserved_mem(void) {}
>> static inline void early_init_fdt_reserve_self(void) {}
>> static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
>>
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial
2016-03-01 16:31 ` Aleksey Makarov
@ 2016-03-01 17:24 ` Peter Hurley
2016-03-01 17:52 ` Aleksey Makarov
0 siblings, 1 reply; 32+ messages in thread
From: Peter Hurley @ 2016-03-01 17:24 UTC (permalink / raw)
To: Aleksey Makarov, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Rob Herring, Frank Rowand, Grant Likely, Jiri Slaby, devicetree
On 03/01/2016 08:31 AM, Aleksey Makarov wrote:
>
>
> On 03/01/2016 05:50 PM, Peter Hurley wrote:
>> On 02/29/2016 04:41 AM, Aleksey Makarov wrote:
>>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>>
>>> We have multiple "earlycon" early_param handlers - merge the DT one into
>>> the main earlycon one. This means the earlycon early_param handler does
>>> not just return success if no options are specified.
>>
>> Why is this patch necessary?
>>
>> Is this cleanup or required for some reason in other parts of this
>> series?
>
> It's a cleanup.
>
> Also, without this, the series has to introduce a new (third) handler for "earlycon=".
Not if it were opt-in like OF is via the same blank
"earlycon" parameter.
>>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>> ---
>>> drivers/of/fdt.c | 11 +----------
>>> drivers/tty/serial/earlycon.c | 3 ++-
>>> include/linux/of_fdt.h | 2 ++
>>> 3 files changed, 5 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>>> index 3349d2a..0547256 100644
>>> --- a/drivers/of/fdt.c
>>> +++ b/drivers/of/fdt.c
>>> @@ -805,7 +805,7 @@ static inline void early_init_dt_check_for_initrd(unsigned long node)
>>>
>>> #ifdef CONFIG_SERIAL_EARLYCON
>>>
>>> -static int __init early_init_dt_scan_chosen_serial(void)
>>> +int __init early_init_dt_scan_chosen_serial(void)
>>> {
>>> int offset;
>>> const char *p, *q, *options = NULL;
>>> @@ -849,15 +849,6 @@ static int __init early_init_dt_scan_chosen_serial(void)
>>> }
>>> return -ENODEV;
>>> }
>>> -
>>> -static int __init setup_of_earlycon(char *buf)
>>> -{
>>> - if (buf)
>>> - return 0;
>>> -
>>> - return early_init_dt_scan_chosen_serial();
>>> -}
>>> -early_param("earlycon", setup_of_earlycon);
>>> #endif
>>>
>>> /**
>>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>>> index 067783f..d217366 100644
>>> --- a/drivers/tty/serial/earlycon.c
>>> +++ b/drivers/tty/serial/earlycon.c
>>> @@ -17,6 +17,7 @@
>>> #include <linux/kernel.h>
>>> #include <linux/init.h>
>>> #include <linux/io.h>
>>> +#include <linux/of_fdt.h>
>>> #include <linux/serial_core.h>
>>> #include <linux/sizes.h>
>>> #include <linux/of.h>
>>> @@ -209,7 +210,7 @@ static int __init param_setup_earlycon(char *buf)
>>> * don't generate a warning from parse_early_params() in that case
>>> */
>>> if (!buf || !buf[0])
>>> - return 0;
>>> + return early_init_dt_scan_chosen_serial();
>>>
>>> err = setup_earlycon(buf);
>>> if (err == -ENOENT || err == -EALREADY)
>>> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
>>> index 2fbe868..56b2a43 100644
>>> --- a/include/linux/of_fdt.h
>>> +++ b/include/linux/of_fdt.h
>>> @@ -63,6 +63,7 @@ extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
>>> int depth, void *data);
>>> extern int early_init_dt_scan_memory(unsigned long node, const char *uname,
>>> int depth, void *data);
>>> +extern int early_init_dt_scan_chosen_serial(void);
>>> extern void early_init_fdt_scan_reserved_mem(void);
>>> extern void early_init_fdt_reserve_self(void);
>>> extern void early_init_dt_add_memory_arch(u64 base, u64 size);
>>> @@ -91,6 +92,7 @@ extern void early_get_first_memblock_info(void *, phys_addr_t *);
>>> extern u64 of_flat_dt_translate_address(unsigned long node);
>>> extern void of_fdt_limit_memory(int limit);
>>> #else /* CONFIG_OF_FLATTREE */
>>> +static inline int early_init_dt_scan_chosen_serial(void) { return -ENODEV; }
>>> static inline void early_init_fdt_scan_reserved_mem(void) {}
>>> static inline void early_init_fdt_reserve_self(void) {}
>>> static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
>>>
>>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial
2016-03-01 17:24 ` Peter Hurley
@ 2016-03-01 17:52 ` Aleksey Makarov
2016-03-01 18:26 ` Peter Hurley
0 siblings, 1 reply; 32+ messages in thread
From: Aleksey Makarov @ 2016-03-01 17:52 UTC (permalink / raw)
To: Peter Hurley, Aleksey Makarov, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Rob Herring, Frank Rowand, Grant Likely, Jiri Slaby, devicetree
On 03/01/2016 08:24 PM, Peter Hurley wrote:
> On 03/01/2016 08:31 AM, Aleksey Makarov wrote:
>>
>>
>> On 03/01/2016 05:50 PM, Peter Hurley wrote:
>>> On 02/29/2016 04:41 AM, Aleksey Makarov wrote:
>>>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>>>
>>>> We have multiple "earlycon" early_param handlers - merge the DT one into
>>>> the main earlycon one. This means the earlycon early_param handler does
>>>> not just return success if no options are specified.
>>>
>>> Why is this patch necessary?
>>>
>>> Is this cleanup or required for some reason in other parts of this
>>> series?
>>
>> It's a cleanup.
>>
>> Also, without this, the series has to introduce a new (third) handler for "earlycon=".
>
> Not if it were opt-in like OF is via the same blank
> "earlycon" parameter.
So you are suggesting to enable ACPI earlycon with just "earlyon" without
any arguments to this paramener, right? That's good idea as OF and ACPI
can not be used simultaneously to specify earlycon. Actually that is
how it worked in the original submussion. I will change this in the next version.
Still I think that having this parameter parsed in one place is a good idea.
Or would you prefer to drop this patch?
>>>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>>> ---
>>>> drivers/of/fdt.c | 11 +----------
>>>> drivers/tty/serial/earlycon.c | 3 ++-
>>>> include/linux/of_fdt.h | 2 ++
>>>> 3 files changed, 5 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>>>> index 3349d2a..0547256 100644
>>>> --- a/drivers/of/fdt.c
>>>> +++ b/drivers/of/fdt.c
>>>> @@ -805,7 +805,7 @@ static inline void early_init_dt_check_for_initrd(unsigned long node)
>>>>
>>>> #ifdef CONFIG_SERIAL_EARLYCON
>>>>
>>>> -static int __init early_init_dt_scan_chosen_serial(void)
>>>> +int __init early_init_dt_scan_chosen_serial(void)
>>>> {
>>>> int offset;
>>>> const char *p, *q, *options = NULL;
>>>> @@ -849,15 +849,6 @@ static int __init early_init_dt_scan_chosen_serial(void)
>>>> }
>>>> return -ENODEV;
>>>> }
>>>> -
>>>> -static int __init setup_of_earlycon(char *buf)
>>>> -{
>>>> - if (buf)
>>>> - return 0;
>>>> -
>>>> - return early_init_dt_scan_chosen_serial();
>>>> -}
>>>> -early_param("earlycon", setup_of_earlycon);
>>>> #endif
>>>>
>>>> /**
>>>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>>>> index 067783f..d217366 100644
>>>> --- a/drivers/tty/serial/earlycon.c
>>>> +++ b/drivers/tty/serial/earlycon.c
>>>> @@ -17,6 +17,7 @@
>>>> #include <linux/kernel.h>
>>>> #include <linux/init.h>
>>>> #include <linux/io.h>
>>>> +#include <linux/of_fdt.h>
>>>> #include <linux/serial_core.h>
>>>> #include <linux/sizes.h>
>>>> #include <linux/of.h>
>>>> @@ -209,7 +210,7 @@ static int __init param_setup_earlycon(char *buf)
>>>> * don't generate a warning from parse_early_params() in that case
>>>> */
>>>> if (!buf || !buf[0])
>>>> - return 0;
>>>> + return early_init_dt_scan_chosen_serial();
>>>>
>>>> err = setup_earlycon(buf);
>>>> if (err == -ENOENT || err == -EALREADY)
>>>> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
>>>> index 2fbe868..56b2a43 100644
>>>> --- a/include/linux/of_fdt.h
>>>> +++ b/include/linux/of_fdt.h
>>>> @@ -63,6 +63,7 @@ extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
>>>> int depth, void *data);
>>>> extern int early_init_dt_scan_memory(unsigned long node, const char *uname,
>>>> int depth, void *data);
>>>> +extern int early_init_dt_scan_chosen_serial(void);
>>>> extern void early_init_fdt_scan_reserved_mem(void);
>>>> extern void early_init_fdt_reserve_self(void);
>>>> extern void early_init_dt_add_memory_arch(u64 base, u64 size);
>>>> @@ -91,6 +92,7 @@ extern void early_get_first_memblock_info(void *, phys_addr_t *);
>>>> extern u64 of_flat_dt_translate_address(unsigned long node);
>>>> extern void of_fdt_limit_memory(int limit);
>>>> #else /* CONFIG_OF_FLATTREE */
>>>> +static inline int early_init_dt_scan_chosen_serial(void) { return -ENODEV; }
>>>> static inline void early_init_fdt_scan_reserved_mem(void) {}
>>>> static inline void early_init_fdt_reserve_self(void) {}
>>>> static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
>>>>
>>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial
2016-03-01 17:52 ` Aleksey Makarov
@ 2016-03-01 18:26 ` Peter Hurley
0 siblings, 0 replies; 32+ messages in thread
From: Peter Hurley @ 2016-03-01 18:26 UTC (permalink / raw)
To: Aleksey Makarov, Aleksey Makarov, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Rob Herring, Frank Rowand, Grant Likely, Jiri Slaby, devicetree
On 03/01/2016 09:52 AM, Aleksey Makarov wrote:
>
>
> On 03/01/2016 08:24 PM, Peter Hurley wrote:
>> On 03/01/2016 08:31 AM, Aleksey Makarov wrote:
>>>
>>>
>>> On 03/01/2016 05:50 PM, Peter Hurley wrote:
>>>> On 02/29/2016 04:41 AM, Aleksey Makarov wrote:
>>>>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>>>>
>>>>> We have multiple "earlycon" early_param handlers - merge the DT one into
>>>>> the main earlycon one. This means the earlycon early_param handler does
>>>>> not just return success if no options are specified.
>>>>
>>>> Why is this patch necessary?
>>>>
>>>> Is this cleanup or required for some reason in other parts of this
>>>> series?
>>>
>>> It's a cleanup.
>>>
>>> Also, without this, the series has to introduce a new (third) handler for "earlycon=".
>>
>> Not if it were opt-in like OF is via the same blank
>> "earlycon" parameter.
>
> So you are suggesting to enable ACPI earlycon with just "earlyon" without
> any arguments to this paramener, right? That's good idea as OF and ACPI
> can not be used simultaneously to specify earlycon. Actually that is
> how it worked in the original submussion. I will change this in the next version.
>
> Still I think that having this parameter parsed in one place is a good idea.
> Or would you prefer to drop this patch?
No need to drop it, imo; the cleanup is fine with me.
But it really helps if either the commit message or the cover letter
describes the purpose and not just the effect (ie., the why and not just
the what).
Regards,
Peter Hurley
>>>>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>>>> ---
>>>>> drivers/of/fdt.c | 11 +----------
>>>>> drivers/tty/serial/earlycon.c | 3 ++-
>>>>> include/linux/of_fdt.h | 2 ++
>>>>> 3 files changed, 5 insertions(+), 11 deletions(-)
>>>>>
>>>>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>>>>> index 3349d2a..0547256 100644
>>>>> --- a/drivers/of/fdt.c
>>>>> +++ b/drivers/of/fdt.c
>>>>> @@ -805,7 +805,7 @@ static inline void early_init_dt_check_for_initrd(unsigned long node)
>>>>>
>>>>> #ifdef CONFIG_SERIAL_EARLYCON
>>>>>
>>>>> -static int __init early_init_dt_scan_chosen_serial(void)
>>>>> +int __init early_init_dt_scan_chosen_serial(void)
>>>>> {
>>>>> int offset;
>>>>> const char *p, *q, *options = NULL;
>>>>> @@ -849,15 +849,6 @@ static int __init early_init_dt_scan_chosen_serial(void)
>>>>> }
>>>>> return -ENODEV;
>>>>> }
>>>>> -
>>>>> -static int __init setup_of_earlycon(char *buf)
>>>>> -{
>>>>> - if (buf)
>>>>> - return 0;
>>>>> -
>>>>> - return early_init_dt_scan_chosen_serial();
>>>>> -}
>>>>> -early_param("earlycon", setup_of_earlycon);
>>>>> #endif
>>>>>
>>>>> /**
>>>>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>>>>> index 067783f..d217366 100644
>>>>> --- a/drivers/tty/serial/earlycon.c
>>>>> +++ b/drivers/tty/serial/earlycon.c
>>>>> @@ -17,6 +17,7 @@
>>>>> #include <linux/kernel.h>
>>>>> #include <linux/init.h>
>>>>> #include <linux/io.h>
>>>>> +#include <linux/of_fdt.h>
>>>>> #include <linux/serial_core.h>
>>>>> #include <linux/sizes.h>
>>>>> #include <linux/of.h>
>>>>> @@ -209,7 +210,7 @@ static int __init param_setup_earlycon(char *buf)
>>>>> * don't generate a warning from parse_early_params() in that case
>>>>> */
>>>>> if (!buf || !buf[0])
>>>>> - return 0;
>>>>> + return early_init_dt_scan_chosen_serial();
>>>>>
>>>>> err = setup_earlycon(buf);
>>>>> if (err == -ENOENT || err == -EALREADY)
>>>>> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
>>>>> index 2fbe868..56b2a43 100644
>>>>> --- a/include/linux/of_fdt.h
>>>>> +++ b/include/linux/of_fdt.h
>>>>> @@ -63,6 +63,7 @@ extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
>>>>> int depth, void *data);
>>>>> extern int early_init_dt_scan_memory(unsigned long node, const char *uname,
>>>>> int depth, void *data);
>>>>> +extern int early_init_dt_scan_chosen_serial(void);
>>>>> extern void early_init_fdt_scan_reserved_mem(void);
>>>>> extern void early_init_fdt_reserve_self(void);
>>>>> extern void early_init_dt_add_memory_arch(u64 base, u64 size);
>>>>> @@ -91,6 +92,7 @@ extern void early_get_first_memblock_info(void *, phys_addr_t *);
>>>>> extern u64 of_flat_dt_translate_address(unsigned long node);
>>>>> extern void of_fdt_limit_memory(int limit);
>>>>> #else /* CONFIG_OF_FLATTREE */
>>>>> +static inline int early_init_dt_scan_chosen_serial(void) { return -ENODEV; }
>>>>> static inline void early_init_fdt_scan_reserved_mem(void) {}
>>>>> static inline void early_init_fdt_reserve_self(void) {}
>>>>> static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
>>>>>
>>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 2/7] ACPI: add definitions of DBG2 subtypes
2016-02-29 12:41 [PATCH v3 0/7] ACPI: parse the DBG2 table Aleksey Makarov
2016-02-29 12:41 ` [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial Aleksey Makarov
@ 2016-02-29 12:41 ` Aleksey Makarov
2016-02-29 12:42 ` [PATCH v3 3/7] ACPI: genaralize iterating over subtables in ACPI_PROBE_TABLE() Aleksey Makarov
` (4 subsequent siblings)
6 siblings, 0 replies; 32+ messages in thread
From: Aleksey Makarov @ 2016-02-29 12:41 UTC (permalink / raw)
To: linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Aleksey Makarov,
Russell King, Greg Kroah-Hartman, Rafael J . Wysocki,
Leif Lindholm, Graeme Gregory, Al Stone, Christopher Covington,
Matthias Brugger, Len Brown, Robert Moore, Lv Zheng, devel
The recent version of Microsoft Debug Port Table 2 (DBG2) [1]
specifies additional serial debug port subtypes. These constants
are also referred by Serial Port Console Redirection Table (SPCR) [2]
Add these constants.
[1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).aspx
[2] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85).aspx
Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
include/acpi/actbl2.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index a4ef625..652f747 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -371,6 +371,11 @@ struct acpi_dbg2_device {
#define ACPI_DBG2_16550_COMPATIBLE 0x0000
#define ACPI_DBG2_16550_SUBSET 0x0001
+#define ACPI_DBG2_ARM_PL011 0x0003
+#define ACPI_DBG2_ARM_SBSA_32BIT 0x000D
+#define ACPI_DBG2_ARM_SBSA_GENERIC 0x000E
+#define ACPI_DBG2_ARM_DCC 0x000F
+#define ACPI_DBG2_BCM2835 0x0010
#define ACPI_DBG2_1394_STANDARD 0x0000
--
2.7.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v3 3/7] ACPI: genaralize iterating over subtables in ACPI_PROBE_TABLE()
2016-02-29 12:41 [PATCH v3 0/7] ACPI: parse the DBG2 table Aleksey Makarov
2016-02-29 12:41 ` [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial Aleksey Makarov
2016-02-29 12:41 ` [PATCH v3 2/7] ACPI: add definitions of DBG2 subtypes Aleksey Makarov
@ 2016-02-29 12:42 ` Aleksey Makarov
2016-02-29 12:42 ` [PATCH v3 4/7] ACPI: parse DBG2 table Aleksey Makarov
` (3 subsequent siblings)
6 siblings, 0 replies; 32+ messages in thread
From: Aleksey Makarov @ 2016-02-29 12:42 UTC (permalink / raw)
To: linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Aleksey Makarov,
Russell King, Greg Kroah-Hartman, Rafael J . Wysocki,
Leif Lindholm, Graeme Gregory, Al Stone, Christopher Covington,
Matthias Brugger, Len Brown, Daniel Lezcano, Thomas Gleixner,
Jason Cooper, Marc Zyngier, John Stultz
Refactor ACPI_PROBE_TABLE() so that it is possible to iterate
over subtables that do not have acpi_subtype header (for example
over subtables of DBG2 table)
To do so:
- Add void * data pointer argument for callback. It will allow to
parse the table and iterate over subtables calling the subtable
callbacks that are provided via this data pointer.
- Save only ACPI table id, callback and callback data in the linker
tables. This makes tables more terse and simplifies the semantics
of the fields of it's elements.
The additional data that required for subtable parsing are saved
in static namespace without increasing the size of linker tables.
- Introduce two macros with clear meaning of arguments, instead one.
The old ACPI_DECLARE_PROBE_ENTRY() macros has arguments that have
different meaning if the entry is for table or for subtable. The
arguments of the new macroses ACPI_DECLARE_PROBE_ENTRY() and
ACPI_DECLARE_PROBE_SUBTYPE_ENTRY() have unambiguous meaning.
- Document arguments of those macroses instead of the fields of linker
table data.
- Fix the issue that prevents using this macros for parsing subtables
other than MADT. In the original code, MADT-specific function
was used to iterate over subtables, while the name was common.
Introduce a new parameter to macors that specifies the size of
the table to do it in common way.
- Don't expose the internals of the tables to the drivers passing
a pointer to struct acpi_probe_entry to the subtable callback validator.
Instead, pass the sideband data specified by the driver.
- Fix the driver's callback signatures: a. Add an unused pointer to void
for table matchers (currently only clocksource callback) b. Change
the type of sideband data to kernel_ulong_t.
Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
drivers/acpi/scan.c | 44 ++++++++++-----
drivers/clocksource/arm_arch_timer.c | 3 +-
drivers/irqchip/irq-gic.c | 4 +-
include/linux/acpi.h | 104 ++++++++++++++++++++++++-----------
include/linux/clocksource.h | 2 +-
include/linux/irqchip.h | 5 +-
6 files changed, 109 insertions(+), 53 deletions(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 5f28cf7..09b5f63 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1969,16 +1969,38 @@ 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)
+static int __init acpi_match_and_setup(struct acpi_subtable_header *header,
+ const unsigned long end)
{
- if (!ape->subtable_valid || ape->subtable_valid(header, ape))
- if (!ape->probe_subtbl(header, end))
+ const struct acpi_probe_subtype_data *data = ape->data;
+
+ if (data->valid(header, data->data))
+ if (!data->setup(header, end))
acpi_probe_count++;
return 0;
}
+int __init acpi_probe_subtype_setup(struct acpi_table_header *table,
+ const void *data)
+{
+ const struct acpi_probe_subtype_data *data_subtype = data;
+
+ return acpi_parse_entries(ape->id, data_subtype->size,
+ acpi_match_and_setup, table,
+ data_subtype->subtype, 0);
+}
+
+static int __init acpi_probe_table_handler(struct acpi_table_header *table)
+{
+ int err = ape->setup(table, ape->data);
+
+ if (ape->setup != acpi_probe_subtype_setup && err >= 0)
+ acpi_probe_count++;
+
+ return err;
+}
+
int __init __acpi_probe_device_table(struct acpi_probe_entry *ap_head, int nr)
{
int count = 0;
@@ -1988,16 +2010,10 @@ int __init __acpi_probe_device_table(struct acpi_probe_entry *ap_head, int nr)
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++;
- }
+ acpi_probe_count = 0;
+ if (acpi_table_parse(ape->id, acpi_probe_table_handler) < 0)
+ continue;
+ count += acpi_probe_count;
}
spin_unlock(&acpi_probe_lock);
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index f492ced..c0df7cb 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -868,7 +868,8 @@ static int __init map_generic_timer_interrupt(u32 interrupt, u32 flags)
}
/* Initialize per-processor generic timer */
-static int __init arch_timer_acpi_init(struct acpi_table_header *table)
+static int __init arch_timer_acpi_init(struct acpi_table_header *table,
+ const void *data)
{
struct acpi_table_gtdt *gtdt;
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 282344b..13d8323 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1287,12 +1287,12 @@ static bool __init acpi_gic_redist_is_present(void)
}
static bool __init gic_validate_dist(struct acpi_subtable_header *header,
- struct acpi_probe_entry *ape)
+ kernel_ulong_t driver_data)
{
struct acpi_madt_generic_distributor *dist;
dist = (struct acpi_madt_generic_distributor *)header;
- return (dist->version == ape->driver_data &&
+ return (dist->version == driver_data &&
(dist->version != ACPI_MADT_GIC_VERSION_NONE ||
!acpi_gic_redist_is_present()));
}
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 06ed7e5..9636c32 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -879,47 +879,76 @@ 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 *);
+ kernel_ulong_t);
+
+typedef int (*acpi_probe_entry_setup)(struct acpi_table_header *, const void *);
#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;
+ acpi_probe_entry_setup setup;
+ const void *data;
};
-#define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, subtable, valid, data, fn) \
+/**
+ * ACPI_DECLARE_PROBE_ENTRY() - declare boot-time probing entry
+ * @table: Name of subsystem to match
+ * @name: Identifier to compose name of table data
+ * @table_id: ACPI table name
+ * @setup_fn: Callback to the driver being probed when table
+ * matches (of type acpi_probe_entry_setup)
+ * @data_ptr: Sideband data provided back to the driver
+ */
+#define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, setup_fn, data_ptr) \
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, \
+ .setup = setup_fn, \
+ .data = data_ptr, \
}
+struct acpi_probe_subtype_data {
+ u8 subtype;
+ unsigned long size;
+ acpi_probe_entry_validate_subtbl valid;
+ acpi_tbl_entry_handler setup;
+ kernel_ulong_t data;
+};
+
+int acpi_probe_subtype_setup(struct acpi_table_header *table, const void *data);
+
+/**
+ * ACPI_DECLARE_PROBE_SUBTYPE_ENTRY() - declare boot-time probing entry that
+ * matches subtables
+ * @table: Name of subsystem to match
+ * @name: Identifier to compose name of table data
+ * @table_id: ACPI table name
+ * @table_size: Size of the table (without subtables) of type table_id
+ * @subtable: Subtable type to match
+ * @valid_fn: Callback to check the validity of the subtable
+ * (of type acpi_probe_entry_validate_subtbl)
+ * @data_int: Sideband data provided back to the driver
+ * (of type kernel_ulong_t)
+ * @setup_fn: Callback to the driver being probed when table and
+ * subtable match and valid_fn callback is successful
+ * (of type acpi_tbl_entry_handler)
+ */
+#define ACPI_DECLARE_PROBE_SUBTYPE_ENTRY(table, name, table_id, table_size, \
+ subtable, valid_fn, data_int, setup_fn) \
+ static const struct acpi_probe_subtype_data \
+ __acpi_probe_subtype_##name __used = { \
+ .subtype = subtable, \
+ .size = table_size, \
+ .valid = valid_fn, \
+ .setup = setup_fn, \
+ .data = data_int, \
+ }; \
+ ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, \
+ acpi_probe_subtype_setup, &__acpi_probe_subtype_##name)
+
#define ACPI_PROBE_TABLE(name) __##name##_acpi_probe_table
#define ACPI_PROBE_TABLE_END(name) __##name##_acpi_probe_table_end
@@ -992,14 +1021,23 @@ 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) \
+#define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, match, data) \
static const void * __acpi_table_##name[] \
__attribute__((unused)) \
- = { (void *) table_id, \
- (void *) subtable, \
- (void *) valid, \
- (void *) fn, \
- (void *) data }
+ = { (void *)table_id, \
+ (void *)match, \
+ (void *)data }
+
+#define ACPI_DECLARE_PROBE_SUBTYPE_ENTRY(table, name, table_id, table_size, \
+ subtable, valid_fn, data_int, setup_fn) \
+ static const void *__acpi_table_##name[] \
+ __attribute__((unused)) \
+ = { (void *)table_id, \
+ (void *)table_size, \
+ (void *)subtable, \
+ (void *)valid_fn, \
+ (void *)data_int, \
+ (void *)setup_fn }
#define acpi_probe_device_table(t) ({ int __r = 0; __r;})
#endif
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 6013021..2bb5d51 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -259,6 +259,6 @@ static inline void clocksource_probe(void) {}
#endif
#define CLOCKSOURCE_ACPI_DECLARE(name, table_id, fn) \
- ACPI_DECLARE_PROBE_ENTRY(clksrc, name, table_id, 0, NULL, 0, fn)
+ ACPI_DECLARE_PROBE_ENTRY(clksrc, name, table_id, fn, NULL)
#endif /* _LINUX_CLOCKSOURCE_H */
diff --git a/include/linux/irqchip.h b/include/linux/irqchip.h
index 89c34b2..cc4c455 100644
--- a/include/linux/irqchip.h
+++ b/include/linux/irqchip.h
@@ -39,8 +39,9 @@
* @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)
+ ACPI_DECLARE_PROBE_SUBTYPE_ENTRY(irqchip, name, ACPI_SIG_MADT, \
+ sizeof(struct acpi_table_madt),\
+ subtable, validate, data, fn)
#ifdef CONFIG_IRQCHIP
void irqchip_init(void);
--
2.7.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v3 4/7] ACPI: parse DBG2 table
2016-02-29 12:41 [PATCH v3 0/7] ACPI: parse the DBG2 table Aleksey Makarov
` (2 preceding siblings ...)
2016-02-29 12:42 ` [PATCH v3 3/7] ACPI: genaralize iterating over subtables in ACPI_PROBE_TABLE() Aleksey Makarov
@ 2016-02-29 12:42 ` Aleksey Makarov
2016-03-01 14:49 ` Peter Hurley
2016-02-29 12:42 ` [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port Aleksey Makarov
` (2 subsequent siblings)
6 siblings, 1 reply; 32+ messages in thread
From: Aleksey Makarov @ 2016-02-29 12:42 UTC (permalink / raw)
To: linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Aleksey Makarov,
Russell King, Greg Kroah-Hartman, Rafael J . Wysocki,
Leif Lindholm, Graeme Gregory, Al Stone, Christopher Covington,
Matthias Brugger, Len Brown, Arnd Bergmann, linux-arch
'ARM Server Base Boot Requirements' [1] mentions DBG2 (Microsoft Debug
Port Table 2) [2] as a mandatory ACPI table that specifies debug ports.
- Implement macros
ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr)
that defines a handler for the port of given type and subtype.
- For each declared port that is also described in the ACPI DBG2 table
call the provided callback.
[1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
[2] http://go.microsoft.com/fwlink/p/?LinkId=234837
Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
drivers/acpi/Kconfig | 3 ++
drivers/acpi/Makefile | 1 +
drivers/acpi/dbg2.c | 88 +++++++++++++++++++++++++++++++++++++++
include/asm-generic/vmlinux.lds.h | 1 +
include/linux/acpi_dbg2.h | 48 +++++++++++++++++++++
5 files changed, 141 insertions(+)
create mode 100644 drivers/acpi/dbg2.c
create mode 100644 include/linux/acpi_dbg2.h
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 65fb483..660666e 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
config ACPI_CCA_REQUIRED
bool
+config ACPI_DBG2_TABLE
+ bool
+
config ACPI_DEBUGGER
bool "AML debugger interface"
select ACPI_DEBUG
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 7395928..3b5f1ea 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -83,6 +83,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
obj-$(CONFIG_ACPI_BGRT) += bgrt.o
obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o
obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
+obj-$(CONFIG_ACPI_DBG2_TABLE) += dbg2.o
# processor has its own "processor." module_param namespace
processor-y := processor_driver.o
diff --git a/drivers/acpi/dbg2.c b/drivers/acpi/dbg2.c
new file mode 100644
index 0000000..0f0f6ca
--- /dev/null
+++ b/drivers/acpi/dbg2.c
@@ -0,0 +1,88 @@
+/*
+ * Copyright (c) 2012, Intel Corporation
+ * Copyright (c) 2015, 2016 Linaro Ltd.
+ *
+ * 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.
+ *
+ */
+
+#define pr_fmt(fmt) "ACPI: DBG2: " fmt
+
+#include <linux/acpi_dbg2.h>
+#include <linux/acpi.h>
+#include <linux/kernel.h>
+
+static const char * __init type2string(u16 type)
+{
+ switch (type) {
+ case ACPI_DBG2_SERIAL_PORT:
+ return "SERIAL";
+ case ACPI_DBG2_1394_PORT:
+ return "1394";
+ case ACPI_DBG2_USB_PORT:
+ return "USB";
+ case ACPI_DBG2_NET_PORT:
+ return "NET";
+ default:
+ return "?";
+ }
+}
+
+static const char * __init subtype2string(u16 subtype)
+{
+ switch (subtype) {
+ case ACPI_DBG2_16550_COMPATIBLE:
+ return "16550_COMPATIBLE";
+ case ACPI_DBG2_16550_SUBSET:
+ return "16550_SUBSET";
+ case ACPI_DBG2_ARM_PL011:
+ return "ARM_PL011";
+ case ACPI_DBG2_ARM_SBSA_32BIT:
+ return "ARM_SBSA_32BIT";
+ case ACPI_DBG2_ARM_SBSA_GENERIC:
+ return "ARM_SBSA_GENERIC";
+ case ACPI_DBG2_ARM_DCC:
+ return "ARM_DCC";
+ case ACPI_DBG2_BCM2835:
+ return "BCM2835";
+ default:
+ return "?";
+ }
+}
+
+int __init acpi_dbg2_setup(struct acpi_table_header *table, const void *data)
+{
+ struct acpi_table_dbg2 *dbg2 = (struct acpi_table_dbg2 *)table;
+ struct acpi_dbg2_data *dbg2_data = (struct acpi_dbg2_data *)data;
+ struct acpi_dbg2_device *dbg2_device, *dbg2_end;
+ int i;
+
+ dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2,
+ dbg2->info_offset);
+ dbg2_end = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2, table->length);
+
+ for (i = 0; i < dbg2->info_count; i++) {
+ if (dbg2_device + 1 > dbg2_end) {
+ pr_err("device pointer overflows, bad table\n");
+ return 0;
+ }
+
+ if (dbg2_device->port_type == dbg2_data->port_type &&
+ dbg2_device->port_subtype == dbg2_data->port_subtype) {
+ if (dbg2_device->port_type == ACPI_DBG2_SERIAL_PORT)
+ pr_info("debug port: SERIAL; subtype: %s\n",
+ subtype2string(dbg2_device->port_subtype));
+ else
+ pr_info("debug port: %s\n",
+ type2string(dbg2_device->port_type));
+ dbg2_data->setup(dbg2_device, dbg2_data->data);
+ }
+
+ dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2_device,
+ dbg2_device->length);
+ }
+
+ return 0;
+}
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 8f5a12a..8cc49ba 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -526,6 +526,7 @@
IRQCHIP_OF_MATCH_TABLE() \
ACPI_PROBE_TABLE(irqchip) \
ACPI_PROBE_TABLE(clksrc) \
+ ACPI_PROBE_TABLE(dbg2) \
EARLYCON_TABLE()
#define INIT_TEXT \
diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
new file mode 100644
index 0000000..125ae7e
--- /dev/null
+++ b/include/linux/acpi_dbg2.h
@@ -0,0 +1,48 @@
+#ifndef _ACPI_DBG2_H_
+#define _ACPI_DBG2_H_
+
+#ifdef CONFIG_ACPI_DBG2_TABLE
+
+#include <linux/kernel.h>
+
+struct acpi_dbg2_device;
+struct acpi_table_header;
+
+struct acpi_dbg2_data {
+ u16 port_type;
+ u16 port_subtype;
+ int (*setup)(struct acpi_dbg2_device *, void *);
+ void *data;
+};
+
+int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
+
+/**
+ * ACPI_DBG2_DECLARE() - Define handler for ACPI DBG2 port
+ * @name: Identifier to compose name of table data
+ * @type: Type of the port
+ * @subtype: Subtype of the port
+ * @setup_fn: Function to be called to setup the port
+ * (of type int (*)(struct acpi_dbg2_device *, void *);)
+ * @data_ptr: Sideband data provided back to the driver
+ */
+#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
+ static const struct acpi_dbg2_data \
+ __acpi_dbg2_data_##name __used = { \
+ .port_type = type, \
+ .port_subtype = subtype, \
+ .setup = setup_fn, \
+ .data = data_ptr, \
+ }; \
+ ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
+ acpi_dbg2_setup, &__acpi_dbg2_data_##name)
+
+#else
+
+#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
+ static const void *__acpi_dbg_data_##name[] \
+ __used __initdata = { (void *)setup_fn, (void *)data_ptr }
+
+#endif
+
+#endif
--
2.7.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v3 4/7] ACPI: parse DBG2 table
2016-02-29 12:42 ` [PATCH v3 4/7] ACPI: parse DBG2 table Aleksey Makarov
@ 2016-03-01 14:49 ` Peter Hurley
2016-03-01 16:24 ` Aleksey Makarov
0 siblings, 1 reply; 32+ messages in thread
From: Peter Hurley @ 2016-03-01 14:49 UTC (permalink / raw)
To: Aleksey Makarov, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Len Brown, Arnd Bergmann, linux-arch
On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
> 'ARM Server Base Boot Requirements' [1] mentions DBG2 (Microsoft Debug
> Port Table 2) [2] as a mandatory ACPI table that specifies debug ports.
>
> - Implement macros
>
> ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr)
>
> that defines a handler for the port of given type and subtype.
>
> - For each declared port that is also described in the ACPI DBG2 table
> call the provided callback.
On 02/22/2016 06:43 AM, Aleksey Makarov wrote:
> On 02/19/2016 08:20 PM, Christopher Covington wrote:
>> Can the device specified in DBG2 be used for both earlycon and KGDB? If it can only be used for one, let's make sure the choice of earlycon vs KGDB is intentional rather than accidental.
>
> I just sent the DBG2 series. It enables an earlycon on DBG2 port with
> an "earlycon=acpi_dbg2" option (we can discuss particular name).
> If you need KGDB on that port just support it for that port in the kernel
> (i. e. add a new instance of ACPI_DBG2_DECLARE() macros for that port, see the patches)
> and change the command line options.
> I hope that is OK. We could continue this discussion in the DBG2 thread.
This method will not work for kgdb, since kgdb doesn't actually
implement the i/o but rather runs on top of a console.
> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
> [2] http://go.microsoft.com/fwlink/p/?LinkId=234837
>
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> ---
> drivers/acpi/Kconfig | 3 ++
> drivers/acpi/Makefile | 1 +
> drivers/acpi/dbg2.c | 88 +++++++++++++++++++++++++++++++++++++++
> include/asm-generic/vmlinux.lds.h | 1 +
> include/linux/acpi_dbg2.h | 48 +++++++++++++++++++++
> 5 files changed, 141 insertions(+)
> create mode 100644 drivers/acpi/dbg2.c
> create mode 100644 include/linux/acpi_dbg2.h
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 65fb483..660666e 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
> config ACPI_CCA_REQUIRED
> bool
>
> +config ACPI_DBG2_TABLE
> + bool
> +
> config ACPI_DEBUGGER
> bool "AML debugger interface"
> select ACPI_DEBUG
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 7395928..3b5f1ea 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -83,6 +83,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
> obj-$(CONFIG_ACPI_BGRT) += bgrt.o
> obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o
> obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
> +obj-$(CONFIG_ACPI_DBG2_TABLE) += dbg2.o
>
> # processor has its own "processor." module_param namespace
> processor-y := processor_driver.o
> diff --git a/drivers/acpi/dbg2.c b/drivers/acpi/dbg2.c
> new file mode 100644
> index 0000000..0f0f6ca
> --- /dev/null
> +++ b/drivers/acpi/dbg2.c
> @@ -0,0 +1,88 @@
> +/*
> + * Copyright (c) 2012, Intel Corporation
> + * Copyright (c) 2015, 2016 Linaro Ltd.
> + *
> + * 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.
> + *
> + */
> +
> +#define pr_fmt(fmt) "ACPI: DBG2: " fmt
> +
> +#include <linux/acpi_dbg2.h>
> +#include <linux/acpi.h>
> +#include <linux/kernel.h>
> +
> +static const char * __init type2string(u16 type)
> +{
> + switch (type) {
> + case ACPI_DBG2_SERIAL_PORT:
> + return "SERIAL";
> + case ACPI_DBG2_1394_PORT:
> + return "1394";
> + case ACPI_DBG2_USB_PORT:
> + return "USB";
> + case ACPI_DBG2_NET_PORT:
> + return "NET";
> + default:
> + return "?";
> + }
> +}
> +
> +static const char * __init subtype2string(u16 subtype)
> +{
> + switch (subtype) {
> + case ACPI_DBG2_16550_COMPATIBLE:
> + return "16550_COMPATIBLE";
> + case ACPI_DBG2_16550_SUBSET:
> + return "16550_SUBSET";
> + case ACPI_DBG2_ARM_PL011:
> + return "ARM_PL011";
> + case ACPI_DBG2_ARM_SBSA_32BIT:
> + return "ARM_SBSA_32BIT";
> + case ACPI_DBG2_ARM_SBSA_GENERIC:
> + return "ARM_SBSA_GENERIC";
> + case ACPI_DBG2_ARM_DCC:
> + return "ARM_DCC";
> + case ACPI_DBG2_BCM2835:
> + return "BCM2835";
> + default:
> + return "?";
> + }
> +}
> +
> +int __init acpi_dbg2_setup(struct acpi_table_header *table, const void *data)
> +{
> + struct acpi_table_dbg2 *dbg2 = (struct acpi_table_dbg2 *)table;
> + struct acpi_dbg2_data *dbg2_data = (struct acpi_dbg2_data *)data;
> + struct acpi_dbg2_device *dbg2_device, *dbg2_end;
> + int i;
> +
> + dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2,
> + dbg2->info_offset);
> + dbg2_end = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2, table->length);
> +
> + for (i = 0; i < dbg2->info_count; i++) {
> + if (dbg2_device + 1 > dbg2_end) {
> + pr_err("device pointer overflows, bad table\n");
> + return 0;
> + }
> +
> + if (dbg2_device->port_type == dbg2_data->port_type &&
> + dbg2_device->port_subtype == dbg2_data->port_subtype) {
> + if (dbg2_device->port_type == ACPI_DBG2_SERIAL_PORT)
> + pr_info("debug port: SERIAL; subtype: %s\n",
> + subtype2string(dbg2_device->port_subtype));
> + else
> + pr_info("debug port: %s\n",
> + type2string(dbg2_device->port_type));
> + dbg2_data->setup(dbg2_device, dbg2_data->data);
> + }
> +
> + dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2_device,
> + dbg2_device->length);
> + }
> +
> + return 0;
> +}
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 8f5a12a..8cc49ba 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -526,6 +526,7 @@
> IRQCHIP_OF_MATCH_TABLE() \
> ACPI_PROBE_TABLE(irqchip) \
> ACPI_PROBE_TABLE(clksrc) \
> + ACPI_PROBE_TABLE(dbg2) \
> EARLYCON_TABLE()
>
> #define INIT_TEXT \
> diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
> new file mode 100644
> index 0000000..125ae7e
> --- /dev/null
> +++ b/include/linux/acpi_dbg2.h
> @@ -0,0 +1,48 @@
> +#ifndef _ACPI_DBG2_H_
> +#define _ACPI_DBG2_H_
> +
> +#ifdef CONFIG_ACPI_DBG2_TABLE
> +
> +#include <linux/kernel.h>
> +
> +struct acpi_dbg2_device;
> +struct acpi_table_header;
> +
> +struct acpi_dbg2_data {
> + u16 port_type;
> + u16 port_subtype;
> + int (*setup)(struct acpi_dbg2_device *, void *);
> + void *data;
> +};
> +
> +int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
> +
> +/**
> + * ACPI_DBG2_DECLARE() - Define handler for ACPI DBG2 port
> + * @name: Identifier to compose name of table data
> + * @type: Type of the port
> + * @subtype: Subtype of the port
> + * @setup_fn: Function to be called to setup the port
> + * (of type int (*)(struct acpi_dbg2_device *, void *);)
> + * @data_ptr: Sideband data provided back to the driver
> + */
> +#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
> + static const struct acpi_dbg2_data \
> + __acpi_dbg2_data_##name __used = { \
> + .port_type = type, \
> + .port_subtype = subtype, \
> + .setup = setup_fn, \
> + .data = data_ptr, \
> + }; \
> + ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
> + acpi_dbg2_setup, &__acpi_dbg2_data_##name)
> +
> +#else
> +
> +#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
> + static const void *__acpi_dbg_data_##name[] \
> + __used __initdata = { (void *)setup_fn, (void *)data_ptr }
> +
> +#endif
> +
> +#endif
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 4/7] ACPI: parse DBG2 table
2016-03-01 14:49 ` Peter Hurley
@ 2016-03-01 16:24 ` Aleksey Makarov
2016-03-01 17:22 ` Peter Hurley
0 siblings, 1 reply; 32+ messages in thread
From: Aleksey Makarov @ 2016-03-01 16:24 UTC (permalink / raw)
To: Peter Hurley, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Len Brown, Arnd Bergmann, linux-arch
On 03/01/2016 05:49 PM, Peter Hurley wrote:
> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>> 'ARM Server Base Boot Requirements' [1] mentions DBG2 (Microsoft Debug
>> Port Table 2) [2] as a mandatory ACPI table that specifies debug ports.
>>
>> - Implement macros
>>
>> ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr)
>>
>> that defines a handler for the port of given type and subtype.
>>
>> - For each declared port that is also described in the ACPI DBG2 table
>> call the provided callback.
>
> On 02/22/2016 06:43 AM, Aleksey Makarov wrote:
>> On 02/19/2016 08:20 PM, Christopher Covington wrote:
>>> Can the device specified in DBG2 be used for both earlycon and KGDB? If it can only be used for one, let's make sure the choice of earlycon vs KGDB is intentional rather than accidental.
>>
>> I just sent the DBG2 series. It enables an earlycon on DBG2 port with
>> an "earlycon=acpi_dbg2" option (we can discuss particular name).
>> If you need KGDB on that port just support it for that port in the kernel
>> (i. e. add a new instance of ACPI_DBG2_DECLARE() macros for that port, see the patches)
>> and change the command line options.
>> I hope that is OK. We could continue this discussion in the DBG2 thread.
>
> This method will not work for kgdb, since kgdb doesn't actually
> implement the i/o but rather runs on top of a console.
I see. Thank you for pointing this out.
I don't have requirements to implement running kgdb over the serial port
specified with DBG2. This feature should be supported separately.
Thank you
Aleksey Makarov
>> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
>> [2] http://go.microsoft.com/fwlink/p/?LinkId=234837
>>
>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>> ---
>> drivers/acpi/Kconfig | 3 ++
>> drivers/acpi/Makefile | 1 +
>> drivers/acpi/dbg2.c | 88 +++++++++++++++++++++++++++++++++++++++
>> include/asm-generic/vmlinux.lds.h | 1 +
>> include/linux/acpi_dbg2.h | 48 +++++++++++++++++++++
>> 5 files changed, 141 insertions(+)
>> create mode 100644 drivers/acpi/dbg2.c
>> create mode 100644 include/linux/acpi_dbg2.h
>>
>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> index 65fb483..660666e 100644
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
>> config ACPI_CCA_REQUIRED
>> bool
>>
>> +config ACPI_DBG2_TABLE
>> + bool
>> +
>> config ACPI_DEBUGGER
>> bool "AML debugger interface"
>> select ACPI_DEBUG
>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>> index 7395928..3b5f1ea 100644
>> --- a/drivers/acpi/Makefile
>> +++ b/drivers/acpi/Makefile
>> @@ -83,6 +83,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
>> obj-$(CONFIG_ACPI_BGRT) += bgrt.o
>> obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o
>> obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
>> +obj-$(CONFIG_ACPI_DBG2_TABLE) += dbg2.o
>>
>> # processor has its own "processor." module_param namespace
>> processor-y := processor_driver.o
>> diff --git a/drivers/acpi/dbg2.c b/drivers/acpi/dbg2.c
>> new file mode 100644
>> index 0000000..0f0f6ca
>> --- /dev/null
>> +++ b/drivers/acpi/dbg2.c
>> @@ -0,0 +1,88 @@
>> +/*
>> + * Copyright (c) 2012, Intel Corporation
>> + * Copyright (c) 2015, 2016 Linaro Ltd.
>> + *
>> + * 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.
>> + *
>> + */
>> +
>> +#define pr_fmt(fmt) "ACPI: DBG2: " fmt
>> +
>> +#include <linux/acpi_dbg2.h>
>> +#include <linux/acpi.h>
>> +#include <linux/kernel.h>
>> +
>> +static const char * __init type2string(u16 type)
>> +{
>> + switch (type) {
>> + case ACPI_DBG2_SERIAL_PORT:
>> + return "SERIAL";
>> + case ACPI_DBG2_1394_PORT:
>> + return "1394";
>> + case ACPI_DBG2_USB_PORT:
>> + return "USB";
>> + case ACPI_DBG2_NET_PORT:
>> + return "NET";
>> + default:
>> + return "?";
>> + }
>> +}
>> +
>> +static const char * __init subtype2string(u16 subtype)
>> +{
>> + switch (subtype) {
>> + case ACPI_DBG2_16550_COMPATIBLE:
>> + return "16550_COMPATIBLE";
>> + case ACPI_DBG2_16550_SUBSET:
>> + return "16550_SUBSET";
>> + case ACPI_DBG2_ARM_PL011:
>> + return "ARM_PL011";
>> + case ACPI_DBG2_ARM_SBSA_32BIT:
>> + return "ARM_SBSA_32BIT";
>> + case ACPI_DBG2_ARM_SBSA_GENERIC:
>> + return "ARM_SBSA_GENERIC";
>> + case ACPI_DBG2_ARM_DCC:
>> + return "ARM_DCC";
>> + case ACPI_DBG2_BCM2835:
>> + return "BCM2835";
>> + default:
>> + return "?";
>> + }
>> +}
>> +
>> +int __init acpi_dbg2_setup(struct acpi_table_header *table, const void *data)
>> +{
>> + struct acpi_table_dbg2 *dbg2 = (struct acpi_table_dbg2 *)table;
>> + struct acpi_dbg2_data *dbg2_data = (struct acpi_dbg2_data *)data;
>> + struct acpi_dbg2_device *dbg2_device, *dbg2_end;
>> + int i;
>> +
>> + dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2,
>> + dbg2->info_offset);
>> + dbg2_end = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2, table->length);
>> +
>> + for (i = 0; i < dbg2->info_count; i++) {
>> + if (dbg2_device + 1 > dbg2_end) {
>> + pr_err("device pointer overflows, bad table\n");
>> + return 0;
>> + }
>> +
>> + if (dbg2_device->port_type == dbg2_data->port_type &&
>> + dbg2_device->port_subtype == dbg2_data->port_subtype) {
>> + if (dbg2_device->port_type == ACPI_DBG2_SERIAL_PORT)
>> + pr_info("debug port: SERIAL; subtype: %s\n",
>> + subtype2string(dbg2_device->port_subtype));
>> + else
>> + pr_info("debug port: %s\n",
>> + type2string(dbg2_device->port_type));
>> + dbg2_data->setup(dbg2_device, dbg2_data->data);
>> + }
>> +
>> + dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2_device,
>> + dbg2_device->length);
>> + }
>> +
>> + return 0;
>> +}
>> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
>> index 8f5a12a..8cc49ba 100644
>> --- a/include/asm-generic/vmlinux.lds.h
>> +++ b/include/asm-generic/vmlinux.lds.h
>> @@ -526,6 +526,7 @@
>> IRQCHIP_OF_MATCH_TABLE() \
>> ACPI_PROBE_TABLE(irqchip) \
>> ACPI_PROBE_TABLE(clksrc) \
>> + ACPI_PROBE_TABLE(dbg2) \
>> EARLYCON_TABLE()
>>
>> #define INIT_TEXT \
>> diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
>> new file mode 100644
>> index 0000000..125ae7e
>> --- /dev/null
>> +++ b/include/linux/acpi_dbg2.h
>> @@ -0,0 +1,48 @@
>> +#ifndef _ACPI_DBG2_H_
>> +#define _ACPI_DBG2_H_
>> +
>> +#ifdef CONFIG_ACPI_DBG2_TABLE
>> +
>> +#include <linux/kernel.h>
>> +
>> +struct acpi_dbg2_device;
>> +struct acpi_table_header;
>> +
>> +struct acpi_dbg2_data {
>> + u16 port_type;
>> + u16 port_subtype;
>> + int (*setup)(struct acpi_dbg2_device *, void *);
>> + void *data;
>> +};
>> +
>> +int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
>> +
>> +/**
>> + * ACPI_DBG2_DECLARE() - Define handler for ACPI DBG2 port
>> + * @name: Identifier to compose name of table data
>> + * @type: Type of the port
>> + * @subtype: Subtype of the port
>> + * @setup_fn: Function to be called to setup the port
>> + * (of type int (*)(struct acpi_dbg2_device *, void *);)
>> + * @data_ptr: Sideband data provided back to the driver
>> + */
>> +#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>> + static const struct acpi_dbg2_data \
>> + __acpi_dbg2_data_##name __used = { \
>> + .port_type = type, \
>> + .port_subtype = subtype, \
>> + .setup = setup_fn, \
>> + .data = data_ptr, \
>> + }; \
>> + ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
>> + acpi_dbg2_setup, &__acpi_dbg2_data_##name)
>> +
>> +#else
>> +
>> +#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>> + static const void *__acpi_dbg_data_##name[] \
>> + __used __initdata = { (void *)setup_fn, (void *)data_ptr }
>> +
>> +#endif
>> +
>> +#endif
>>
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 4/7] ACPI: parse DBG2 table
2016-03-01 16:24 ` Aleksey Makarov
@ 2016-03-01 17:22 ` Peter Hurley
2016-03-01 18:19 ` Aleksey Makarov
0 siblings, 1 reply; 32+ messages in thread
From: Peter Hurley @ 2016-03-01 17:22 UTC (permalink / raw)
To: Aleksey Makarov, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Len Brown, Arnd Bergmann, linux-arch
On 03/01/2016 08:24 AM, Aleksey Makarov wrote:
>
>
> On 03/01/2016 05:49 PM, Peter Hurley wrote:
>> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>>> 'ARM Server Base Boot Requirements' [1] mentions DBG2 (Microsoft Debug
>>> Port Table 2) [2] as a mandatory ACPI table that specifies debug ports.
>>>
>>> - Implement macros
>>>
>>> ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr)
>>>
>>> that defines a handler for the port of given type and subtype.
>>>
>>> - For each declared port that is also described in the ACPI DBG2 table
>>> call the provided callback.
>>
>> On 02/22/2016 06:43 AM, Aleksey Makarov wrote:
>>> On 02/19/2016 08:20 PM, Christopher Covington wrote:
>>>> Can the device specified in DBG2 be used for both earlycon and KGDB? If it can only be used for one, let's make sure the choice of earlycon vs KGDB is intentional rather than accidental.
>>>
>>> I just sent the DBG2 series. It enables an earlycon on DBG2 port with
>>> an "earlycon=acpi_dbg2" option (we can discuss particular name).
>>> If you need KGDB on that port just support it for that port in the kernel
>>> (i. e. add a new instance of ACPI_DBG2_DECLARE() macros for that port, see the patches)
>>> and change the command line options.
>>> I hope that is OK. We could continue this discussion in the DBG2 thread.
>>
>> This method will not work for kgdb, since kgdb doesn't actually
>> implement the i/o but rather runs on top of a console.
>
> I see. Thank you for pointing this out.
>
> I don't have requirements to implement running kgdb over the serial port
> specified with DBG2. This feature should be supported separately.
And this takes us back full-circle to my initial point regarding
supporting earlycon via ACPI: which is that my view is earlycon should
be opt-in for any ACPI-specified console, rather than console via
SPCR and earlycon via DBG2.
> Thank you
> Aleksey Makarov
>
>>> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
>>> [2] http://go.microsoft.com/fwlink/p/?LinkId=234837
>>>
>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>> ---
>>> drivers/acpi/Kconfig | 3 ++
>>> drivers/acpi/Makefile | 1 +
>>> drivers/acpi/dbg2.c | 88 +++++++++++++++++++++++++++++++++++++++
>>> include/asm-generic/vmlinux.lds.h | 1 +
>>> include/linux/acpi_dbg2.h | 48 +++++++++++++++++++++
>>> 5 files changed, 141 insertions(+)
>>> create mode 100644 drivers/acpi/dbg2.c
>>> create mode 100644 include/linux/acpi_dbg2.h
>>>
>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>>> index 65fb483..660666e 100644
>>> --- a/drivers/acpi/Kconfig
>>> +++ b/drivers/acpi/Kconfig
>>> @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
>>> config ACPI_CCA_REQUIRED
>>> bool
>>>
>>> +config ACPI_DBG2_TABLE
>>> + bool
>>> +
>>> config ACPI_DEBUGGER
>>> bool "AML debugger interface"
>>> select ACPI_DEBUG
>>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>>> index 7395928..3b5f1ea 100644
>>> --- a/drivers/acpi/Makefile
>>> +++ b/drivers/acpi/Makefile
>>> @@ -83,6 +83,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
>>> obj-$(CONFIG_ACPI_BGRT) += bgrt.o
>>> obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o
>>> obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
>>> +obj-$(CONFIG_ACPI_DBG2_TABLE) += dbg2.o
>>>
>>> # processor has its own "processor." module_param namespace
>>> processor-y := processor_driver.o
>>> diff --git a/drivers/acpi/dbg2.c b/drivers/acpi/dbg2.c
>>> new file mode 100644
>>> index 0000000..0f0f6ca
>>> --- /dev/null
>>> +++ b/drivers/acpi/dbg2.c
>>> @@ -0,0 +1,88 @@
>>> +/*
>>> + * Copyright (c) 2012, Intel Corporation
>>> + * Copyright (c) 2015, 2016 Linaro Ltd.
>>> + *
>>> + * 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.
>>> + *
>>> + */
>>> +
>>> +#define pr_fmt(fmt) "ACPI: DBG2: " fmt
>>> +
>>> +#include <linux/acpi_dbg2.h>
>>> +#include <linux/acpi.h>
>>> +#include <linux/kernel.h>
>>> +
>>> +static const char * __init type2string(u16 type)
>>> +{
>>> + switch (type) {
>>> + case ACPI_DBG2_SERIAL_PORT:
>>> + return "SERIAL";
>>> + case ACPI_DBG2_1394_PORT:
>>> + return "1394";
>>> + case ACPI_DBG2_USB_PORT:
>>> + return "USB";
>>> + case ACPI_DBG2_NET_PORT:
>>> + return "NET";
>>> + default:
>>> + return "?";
>>> + }
>>> +}
>>> +
>>> +static const char * __init subtype2string(u16 subtype)
>>> +{
>>> + switch (subtype) {
>>> + case ACPI_DBG2_16550_COMPATIBLE:
>>> + return "16550_COMPATIBLE";
>>> + case ACPI_DBG2_16550_SUBSET:
>>> + return "16550_SUBSET";
>>> + case ACPI_DBG2_ARM_PL011:
>>> + return "ARM_PL011";
>>> + case ACPI_DBG2_ARM_SBSA_32BIT:
>>> + return "ARM_SBSA_32BIT";
>>> + case ACPI_DBG2_ARM_SBSA_GENERIC:
>>> + return "ARM_SBSA_GENERIC";
>>> + case ACPI_DBG2_ARM_DCC:
>>> + return "ARM_DCC";
>>> + case ACPI_DBG2_BCM2835:
>>> + return "BCM2835";
>>> + default:
>>> + return "?";
>>> + }
>>> +}
>>> +
>>> +int __init acpi_dbg2_setup(struct acpi_table_header *table, const void *data)
>>> +{
>>> + struct acpi_table_dbg2 *dbg2 = (struct acpi_table_dbg2 *)table;
>>> + struct acpi_dbg2_data *dbg2_data = (struct acpi_dbg2_data *)data;
>>> + struct acpi_dbg2_device *dbg2_device, *dbg2_end;
>>> + int i;
>>> +
>>> + dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2,
>>> + dbg2->info_offset);
>>> + dbg2_end = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2, table->length);
>>> +
>>> + for (i = 0; i < dbg2->info_count; i++) {
>>> + if (dbg2_device + 1 > dbg2_end) {
>>> + pr_err("device pointer overflows, bad table\n");
>>> + return 0;
>>> + }
>>> +
>>> + if (dbg2_device->port_type == dbg2_data->port_type &&
>>> + dbg2_device->port_subtype == dbg2_data->port_subtype) {
>>> + if (dbg2_device->port_type == ACPI_DBG2_SERIAL_PORT)
>>> + pr_info("debug port: SERIAL; subtype: %s\n",
>>> + subtype2string(dbg2_device->port_subtype));
>>> + else
>>> + pr_info("debug port: %s\n",
>>> + type2string(dbg2_device->port_type));
>>> + dbg2_data->setup(dbg2_device, dbg2_data->data);
>>> + }
>>> +
>>> + dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2_device,
>>> + dbg2_device->length);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
>>> index 8f5a12a..8cc49ba 100644
>>> --- a/include/asm-generic/vmlinux.lds.h
>>> +++ b/include/asm-generic/vmlinux.lds.h
>>> @@ -526,6 +526,7 @@
>>> IRQCHIP_OF_MATCH_TABLE() \
>>> ACPI_PROBE_TABLE(irqchip) \
>>> ACPI_PROBE_TABLE(clksrc) \
>>> + ACPI_PROBE_TABLE(dbg2) \
>>> EARLYCON_TABLE()
>>>
>>> #define INIT_TEXT \
>>> diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
>>> new file mode 100644
>>> index 0000000..125ae7e
>>> --- /dev/null
>>> +++ b/include/linux/acpi_dbg2.h
>>> @@ -0,0 +1,48 @@
>>> +#ifndef _ACPI_DBG2_H_
>>> +#define _ACPI_DBG2_H_
>>> +
>>> +#ifdef CONFIG_ACPI_DBG2_TABLE
>>> +
>>> +#include <linux/kernel.h>
>>> +
>>> +struct acpi_dbg2_device;
>>> +struct acpi_table_header;
>>> +
>>> +struct acpi_dbg2_data {
>>> + u16 port_type;
>>> + u16 port_subtype;
>>> + int (*setup)(struct acpi_dbg2_device *, void *);
>>> + void *data;
>>> +};
>>> +
>>> +int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
>>> +
>>> +/**
>>> + * ACPI_DBG2_DECLARE() - Define handler for ACPI DBG2 port
>>> + * @name: Identifier to compose name of table data
>>> + * @type: Type of the port
>>> + * @subtype: Subtype of the port
>>> + * @setup_fn: Function to be called to setup the port
>>> + * (of type int (*)(struct acpi_dbg2_device *, void *);)
>>> + * @data_ptr: Sideband data provided back to the driver
>>> + */
>>> +#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>>> + static const struct acpi_dbg2_data \
>>> + __acpi_dbg2_data_##name __used = { \
>>> + .port_type = type, \
>>> + .port_subtype = subtype, \
>>> + .setup = setup_fn, \
>>> + .data = data_ptr, \
>>> + }; \
>>> + ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
>>> + acpi_dbg2_setup, &__acpi_dbg2_data_##name)
>>> +
>>> +#else
>>> +
>>> +#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>>> + static const void *__acpi_dbg_data_##name[] \
>>> + __used __initdata = { (void *)setup_fn, (void *)data_ptr }
>>> +
>>> +#endif
>>> +
>>> +#endif
>>>
>>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 4/7] ACPI: parse DBG2 table
2016-03-01 17:22 ` Peter Hurley
@ 2016-03-01 18:19 ` Aleksey Makarov
2016-03-03 16:40 ` Peter Hurley
0 siblings, 1 reply; 32+ messages in thread
From: Aleksey Makarov @ 2016-03-01 18:19 UTC (permalink / raw)
To: Peter Hurley, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Len Brown, Arnd Bergmann, linux-arch
On 03/01/2016 08:22 PM, Peter Hurley wrote:
> On 03/01/2016 08:24 AM, Aleksey Makarov wrote:
>>
>>
>> On 03/01/2016 05:49 PM, Peter Hurley wrote:
>>> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>>>> 'ARM Server Base Boot Requirements' [1] mentions DBG2 (Microsoft Debug
>>>> Port Table 2) [2] as a mandatory ACPI table that specifies debug ports.
>>>>
>>>> - Implement macros
>>>>
>>>> ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr)
>>>>
>>>> that defines a handler for the port of given type and subtype.
>>>>
>>>> - For each declared port that is also described in the ACPI DBG2 table
>>>> call the provided callback.
>>>
>>> On 02/22/2016 06:43 AM, Aleksey Makarov wrote:
>>>> On 02/19/2016 08:20 PM, Christopher Covington wrote:
>>>>> Can the device specified in DBG2 be used for both earlycon and KGDB? If it can only be used for one, let's make sure the choice of earlycon vs KGDB is intentional rather than accidental.
>>>>
>>>> I just sent the DBG2 series. It enables an earlycon on DBG2 port with
>>>> an "earlycon=acpi_dbg2" option (we can discuss particular name).
>>>> If you need KGDB on that port just support it for that port in the kernel
>>>> (i. e. add a new instance of ACPI_DBG2_DECLARE() macros for that port, see the patches)
>>>> and change the command line options.
>>>> I hope that is OK. We could continue this discussion in the DBG2 thread.
>>>
>>> This method will not work for kgdb, since kgdb doesn't actually
>>> implement the i/o but rather runs on top of a console.
>>
>> I see. Thank you for pointing this out.
>>
>> I don't have requirements to implement running kgdb over the serial port
>> specified with DBG2. This feature should be supported separately.
>
> And this takes us back full-circle to my initial point regarding
> supporting earlycon via ACPI: which is that my view is earlycon should
> be opt-in for any ACPI-specified console, rather than console via
> SPCR and earlycon via DBG2.
This is the main point on which we do not agree:
should SPCR start a new earlycon or match existing full-featured console.
But I do not see how this kgdb/DBG2 feature makes your point of view
more founded. Can you please elaborate?
On the contrary, if SPCR console is earlycon, we will not be able to
run kgdb on it.
>> Thank you
>> Aleksey Makarov
>>
>>>> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
>>>> [2] http://go.microsoft.com/fwlink/p/?LinkId=234837
>>>>
>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>>> ---
>>>> drivers/acpi/Kconfig | 3 ++
>>>> drivers/acpi/Makefile | 1 +
>>>> drivers/acpi/dbg2.c | 88 +++++++++++++++++++++++++++++++++++++++
>>>> include/asm-generic/vmlinux.lds.h | 1 +
>>>> include/linux/acpi_dbg2.h | 48 +++++++++++++++++++++
>>>> 5 files changed, 141 insertions(+)
>>>> create mode 100644 drivers/acpi/dbg2.c
>>>> create mode 100644 include/linux/acpi_dbg2.h
>>>>
>>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>>>> index 65fb483..660666e 100644
>>>> --- a/drivers/acpi/Kconfig
>>>> +++ b/drivers/acpi/Kconfig
>>>> @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
>>>> config ACPI_CCA_REQUIRED
>>>> bool
>>>>
>>>> +config ACPI_DBG2_TABLE
>>>> + bool
>>>> +
>>>> config ACPI_DEBUGGER
>>>> bool "AML debugger interface"
>>>> select ACPI_DEBUG
>>>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>>>> index 7395928..3b5f1ea 100644
>>>> --- a/drivers/acpi/Makefile
>>>> +++ b/drivers/acpi/Makefile
>>>> @@ -83,6 +83,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
>>>> obj-$(CONFIG_ACPI_BGRT) += bgrt.o
>>>> obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o
>>>> obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
>>>> +obj-$(CONFIG_ACPI_DBG2_TABLE) += dbg2.o
>>>>
>>>> # processor has its own "processor." module_param namespace
>>>> processor-y := processor_driver.o
>>>> diff --git a/drivers/acpi/dbg2.c b/drivers/acpi/dbg2.c
>>>> new file mode 100644
>>>> index 0000000..0f0f6ca
>>>> --- /dev/null
>>>> +++ b/drivers/acpi/dbg2.c
>>>> @@ -0,0 +1,88 @@
>>>> +/*
>>>> + * Copyright (c) 2012, Intel Corporation
>>>> + * Copyright (c) 2015, 2016 Linaro Ltd.
>>>> + *
>>>> + * 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.
>>>> + *
>>>> + */
>>>> +
>>>> +#define pr_fmt(fmt) "ACPI: DBG2: " fmt
>>>> +
>>>> +#include <linux/acpi_dbg2.h>
>>>> +#include <linux/acpi.h>
>>>> +#include <linux/kernel.h>
>>>> +
>>>> +static const char * __init type2string(u16 type)
>>>> +{
>>>> + switch (type) {
>>>> + case ACPI_DBG2_SERIAL_PORT:
>>>> + return "SERIAL";
>>>> + case ACPI_DBG2_1394_PORT:
>>>> + return "1394";
>>>> + case ACPI_DBG2_USB_PORT:
>>>> + return "USB";
>>>> + case ACPI_DBG2_NET_PORT:
>>>> + return "NET";
>>>> + default:
>>>> + return "?";
>>>> + }
>>>> +}
>>>> +
>>>> +static const char * __init subtype2string(u16 subtype)
>>>> +{
>>>> + switch (subtype) {
>>>> + case ACPI_DBG2_16550_COMPATIBLE:
>>>> + return "16550_COMPATIBLE";
>>>> + case ACPI_DBG2_16550_SUBSET:
>>>> + return "16550_SUBSET";
>>>> + case ACPI_DBG2_ARM_PL011:
>>>> + return "ARM_PL011";
>>>> + case ACPI_DBG2_ARM_SBSA_32BIT:
>>>> + return "ARM_SBSA_32BIT";
>>>> + case ACPI_DBG2_ARM_SBSA_GENERIC:
>>>> + return "ARM_SBSA_GENERIC";
>>>> + case ACPI_DBG2_ARM_DCC:
>>>> + return "ARM_DCC";
>>>> + case ACPI_DBG2_BCM2835:
>>>> + return "BCM2835";
>>>> + default:
>>>> + return "?";
>>>> + }
>>>> +}
>>>> +
>>>> +int __init acpi_dbg2_setup(struct acpi_table_header *table, const void *data)
>>>> +{
>>>> + struct acpi_table_dbg2 *dbg2 = (struct acpi_table_dbg2 *)table;
>>>> + struct acpi_dbg2_data *dbg2_data = (struct acpi_dbg2_data *)data;
>>>> + struct acpi_dbg2_device *dbg2_device, *dbg2_end;
>>>> + int i;
>>>> +
>>>> + dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2,
>>>> + dbg2->info_offset);
>>>> + dbg2_end = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2, table->length);
>>>> +
>>>> + for (i = 0; i < dbg2->info_count; i++) {
>>>> + if (dbg2_device + 1 > dbg2_end) {
>>>> + pr_err("device pointer overflows, bad table\n");
>>>> + return 0;
>>>> + }
>>>> +
>>>> + if (dbg2_device->port_type == dbg2_data->port_type &&
>>>> + dbg2_device->port_subtype == dbg2_data->port_subtype) {
>>>> + if (dbg2_device->port_type == ACPI_DBG2_SERIAL_PORT)
>>>> + pr_info("debug port: SERIAL; subtype: %s\n",
>>>> + subtype2string(dbg2_device->port_subtype));
>>>> + else
>>>> + pr_info("debug port: %s\n",
>>>> + type2string(dbg2_device->port_type));
>>>> + dbg2_data->setup(dbg2_device, dbg2_data->data);
>>>> + }
>>>> +
>>>> + dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2_device,
>>>> + dbg2_device->length);
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
>>>> index 8f5a12a..8cc49ba 100644
>>>> --- a/include/asm-generic/vmlinux.lds.h
>>>> +++ b/include/asm-generic/vmlinux.lds.h
>>>> @@ -526,6 +526,7 @@
>>>> IRQCHIP_OF_MATCH_TABLE() \
>>>> ACPI_PROBE_TABLE(irqchip) \
>>>> ACPI_PROBE_TABLE(clksrc) \
>>>> + ACPI_PROBE_TABLE(dbg2) \
>>>> EARLYCON_TABLE()
>>>>
>>>> #define INIT_TEXT \
>>>> diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
>>>> new file mode 100644
>>>> index 0000000..125ae7e
>>>> --- /dev/null
>>>> +++ b/include/linux/acpi_dbg2.h
>>>> @@ -0,0 +1,48 @@
>>>> +#ifndef _ACPI_DBG2_H_
>>>> +#define _ACPI_DBG2_H_
>>>> +
>>>> +#ifdef CONFIG_ACPI_DBG2_TABLE
>>>> +
>>>> +#include <linux/kernel.h>
>>>> +
>>>> +struct acpi_dbg2_device;
>>>> +struct acpi_table_header;
>>>> +
>>>> +struct acpi_dbg2_data {
>>>> + u16 port_type;
>>>> + u16 port_subtype;
>>>> + int (*setup)(struct acpi_dbg2_device *, void *);
>>>> + void *data;
>>>> +};
>>>> +
>>>> +int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
>>>> +
>>>> +/**
>>>> + * ACPI_DBG2_DECLARE() - Define handler for ACPI DBG2 port
>>>> + * @name: Identifier to compose name of table data
>>>> + * @type: Type of the port
>>>> + * @subtype: Subtype of the port
>>>> + * @setup_fn: Function to be called to setup the port
>>>> + * (of type int (*)(struct acpi_dbg2_device *, void *);)
>>>> + * @data_ptr: Sideband data provided back to the driver
>>>> + */
>>>> +#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>>>> + static const struct acpi_dbg2_data \
>>>> + __acpi_dbg2_data_##name __used = { \
>>>> + .port_type = type, \
>>>> + .port_subtype = subtype, \
>>>> + .setup = setup_fn, \
>>>> + .data = data_ptr, \
>>>> + }; \
>>>> + ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
>>>> + acpi_dbg2_setup, &__acpi_dbg2_data_##name)
>>>> +
>>>> +#else
>>>> +
>>>> +#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>>>> + static const void *__acpi_dbg_data_##name[] \
>>>> + __used __initdata = { (void *)setup_fn, (void *)data_ptr }
>>>> +
>>>> +#endif
>>>> +
>>>> +#endif
>>>>
>>>
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 4/7] ACPI: parse DBG2 table
2016-03-01 18:19 ` Aleksey Makarov
@ 2016-03-03 16:40 ` Peter Hurley
2016-03-04 12:19 ` Aleksey Makarov
0 siblings, 1 reply; 32+ messages in thread
From: Peter Hurley @ 2016-03-03 16:40 UTC (permalink / raw)
To: Aleksey Makarov, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Len Brown, Arnd Bergmann, linux-arch
On 03/01/2016 10:19 AM, Aleksey Makarov wrote:
>
>
> On 03/01/2016 08:22 PM, Peter Hurley wrote:
>> On 03/01/2016 08:24 AM, Aleksey Makarov wrote:
>>>
>>>
>>> On 03/01/2016 05:49 PM, Peter Hurley wrote:
>>>> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>>>>> 'ARM Server Base Boot Requirements' [1] mentions DBG2 (Microsoft Debug
>>>>> Port Table 2) [2] as a mandatory ACPI table that specifies debug ports.
>>>>>
>>>>> - Implement macros
>>>>>
>>>>> ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr)
>>>>>
>>>>> that defines a handler for the port of given type and subtype.
>>>>>
>>>>> - For each declared port that is also described in the ACPI DBG2 table
>>>>> call the provided callback.
>>>>
>>>> On 02/22/2016 06:43 AM, Aleksey Makarov wrote:
>>>>> On 02/19/2016 08:20 PM, Christopher Covington wrote:
>>>>>> Can the device specified in DBG2 be used for both earlycon and KGDB? If it can only be used for one, let's make sure the choice of earlycon vs KGDB is intentional rather than accidental.
>>>>>
>>>>> I just sent the DBG2 series. It enables an earlycon on DBG2 port with
>>>>> an "earlycon=acpi_dbg2" option (we can discuss particular name).
>>>>> If you need KGDB on that port just support it for that port in the kernel
>>>>> (i. e. add a new instance of ACPI_DBG2_DECLARE() macros for that port, see the patches)
>>>>> and change the command line options.
>>>>> I hope that is OK. We could continue this discussion in the DBG2 thread.
>>>>
>>>> This method will not work for kgdb, since kgdb doesn't actually
>>>> implement the i/o but rather runs on top of a console.
>>>
>>> I see. Thank you for pointing this out.
>>>
>>> I don't have requirements to implement running kgdb over the serial port
>>> specified with DBG2. This feature should be supported separately.
>>
>> And this takes us back full-circle to my initial point regarding
>> supporting earlycon via ACPI: which is that my view is earlycon should
>> be opt-in for any ACPI-specified console, rather than console via
>> SPCR and earlycon via DBG2.
>
> This is the main point on which we do not agree:
> should SPCR start a new earlycon or match existing full-featured console.
My point of view is not that SPCR should *only* start an earlycon
but rather *optionally also* start an earlycon.
This is the existing behavior of every other firmware-specified console.
> But I do not see how this kgdb/DBG2 feature makes your point of view
> more founded. Can you please elaborate?
Well, my main concern is that other configurations that you have not
provided for will not be supportable at all because of the design choices
you're making here.
For example, your current design does not allow for earlycon+console
on the SPCR port and debugger on DBG2 port. I think that's a problem.
Another concern is that, since you haven't accounted for options which
we'll want to implement in the near future, that a rewrite will be
necessary to implement those.
This framework is fairly heavyweight to already not have properly
considered how to start kgdb via DBG2.
> On the contrary, if SPCR console is earlycon, we will not be able to
> run kgdb on it.
I'm not sure what you mean by "run kgdb on it". What is "it" and why
would that be a problem?
>>> Thank you
>>> Aleksey Makarov
>>>
>>>>> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
>>>>> [2] http://go.microsoft.com/fwlink/p/?LinkId=234837
>>>>>
>>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>>>> ---
>>>>> drivers/acpi/Kconfig | 3 ++
>>>>> drivers/acpi/Makefile | 1 +
>>>>> drivers/acpi/dbg2.c | 88 +++++++++++++++++++++++++++++++++++++++
>>>>> include/asm-generic/vmlinux.lds.h | 1 +
>>>>> include/linux/acpi_dbg2.h | 48 +++++++++++++++++++++
>>>>> 5 files changed, 141 insertions(+)
>>>>> create mode 100644 drivers/acpi/dbg2.c
>>>>> create mode 100644 include/linux/acpi_dbg2.h
>>>>>
>>>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>>>>> index 65fb483..660666e 100644
>>>>> --- a/drivers/acpi/Kconfig
>>>>> +++ b/drivers/acpi/Kconfig
>>>>> @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
>>>>> config ACPI_CCA_REQUIRED
>>>>> bool
>>>>>
>>>>> +config ACPI_DBG2_TABLE
>>>>> + bool
>>>>> +
>>>>> config ACPI_DEBUGGER
>>>>> bool "AML debugger interface"
>>>>> select ACPI_DEBUG
>>>>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>>>>> index 7395928..3b5f1ea 100644
>>>>> --- a/drivers/acpi/Makefile
>>>>> +++ b/drivers/acpi/Makefile
>>>>> @@ -83,6 +83,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
>>>>> obj-$(CONFIG_ACPI_BGRT) += bgrt.o
>>>>> obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o
>>>>> obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
>>>>> +obj-$(CONFIG_ACPI_DBG2_TABLE) += dbg2.o
>>>>>
>>>>> # processor has its own "processor." module_param namespace
>>>>> processor-y := processor_driver.o
>>>>> diff --git a/drivers/acpi/dbg2.c b/drivers/acpi/dbg2.c
>>>>> new file mode 100644
>>>>> index 0000000..0f0f6ca
>>>>> --- /dev/null
>>>>> +++ b/drivers/acpi/dbg2.c
>>>>> @@ -0,0 +1,88 @@
>>>>> +/*
>>>>> + * Copyright (c) 2012, Intel Corporation
>>>>> + * Copyright (c) 2015, 2016 Linaro Ltd.
>>>>> + *
>>>>> + * 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.
>>>>> + *
>>>>> + */
>>>>> +
>>>>> +#define pr_fmt(fmt) "ACPI: DBG2: " fmt
>>>>> +
>>>>> +#include <linux/acpi_dbg2.h>
>>>>> +#include <linux/acpi.h>
>>>>> +#include <linux/kernel.h>
>>>>> +
>>>>> +static const char * __init type2string(u16 type)
>>>>> +{
>>>>> + switch (type) {
>>>>> + case ACPI_DBG2_SERIAL_PORT:
>>>>> + return "SERIAL";
>>>>> + case ACPI_DBG2_1394_PORT:
>>>>> + return "1394";
>>>>> + case ACPI_DBG2_USB_PORT:
>>>>> + return "USB";
>>>>> + case ACPI_DBG2_NET_PORT:
>>>>> + return "NET";
>>>>> + default:
>>>>> + return "?";
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +static const char * __init subtype2string(u16 subtype)
>>>>> +{
>>>>> + switch (subtype) {
>>>>> + case ACPI_DBG2_16550_COMPATIBLE:
>>>>> + return "16550_COMPATIBLE";
>>>>> + case ACPI_DBG2_16550_SUBSET:
>>>>> + return "16550_SUBSET";
>>>>> + case ACPI_DBG2_ARM_PL011:
>>>>> + return "ARM_PL011";
>>>>> + case ACPI_DBG2_ARM_SBSA_32BIT:
>>>>> + return "ARM_SBSA_32BIT";
>>>>> + case ACPI_DBG2_ARM_SBSA_GENERIC:
>>>>> + return "ARM_SBSA_GENERIC";
>>>>> + case ACPI_DBG2_ARM_DCC:
>>>>> + return "ARM_DCC";
>>>>> + case ACPI_DBG2_BCM2835:
>>>>> + return "BCM2835";
>>>>> + default:
>>>>> + return "?";
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +int __init acpi_dbg2_setup(struct acpi_table_header *table, const void *data)
>>>>> +{
>>>>> + struct acpi_table_dbg2 *dbg2 = (struct acpi_table_dbg2 *)table;
>>>>> + struct acpi_dbg2_data *dbg2_data = (struct acpi_dbg2_data *)data;
>>>>> + struct acpi_dbg2_device *dbg2_device, *dbg2_end;
>>>>> + int i;
>>>>> +
>>>>> + dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2,
>>>>> + dbg2->info_offset);
>>>>> + dbg2_end = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2, table->length);
>>>>> +
>>>>> + for (i = 0; i < dbg2->info_count; i++) {
>>>>> + if (dbg2_device + 1 > dbg2_end) {
>>>>> + pr_err("device pointer overflows, bad table\n");
>>>>> + return 0;
>>>>> + }
>>>>> +
>>>>> + if (dbg2_device->port_type == dbg2_data->port_type &&
>>>>> + dbg2_device->port_subtype == dbg2_data->port_subtype) {
>>>>> + if (dbg2_device->port_type == ACPI_DBG2_SERIAL_PORT)
>>>>> + pr_info("debug port: SERIAL; subtype: %s\n",
>>>>> + subtype2string(dbg2_device->port_subtype));
>>>>> + else
>>>>> + pr_info("debug port: %s\n",
>>>>> + type2string(dbg2_device->port_type));
>>>>> + dbg2_data->setup(dbg2_device, dbg2_data->data);
>>>>> + }
>>>>> +
>>>>> + dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2_device,
>>>>> + dbg2_device->length);
>>>>> + }
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
>>>>> index 8f5a12a..8cc49ba 100644
>>>>> --- a/include/asm-generic/vmlinux.lds.h
>>>>> +++ b/include/asm-generic/vmlinux.lds.h
>>>>> @@ -526,6 +526,7 @@
>>>>> IRQCHIP_OF_MATCH_TABLE() \
>>>>> ACPI_PROBE_TABLE(irqchip) \
>>>>> ACPI_PROBE_TABLE(clksrc) \
>>>>> + ACPI_PROBE_TABLE(dbg2) \
>>>>> EARLYCON_TABLE()
>>>>>
>>>>> #define INIT_TEXT \
>>>>> diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
>>>>> new file mode 100644
>>>>> index 0000000..125ae7e
>>>>> --- /dev/null
>>>>> +++ b/include/linux/acpi_dbg2.h
>>>>> @@ -0,0 +1,48 @@
>>>>> +#ifndef _ACPI_DBG2_H_
>>>>> +#define _ACPI_DBG2_H_
>>>>> +
>>>>> +#ifdef CONFIG_ACPI_DBG2_TABLE
>>>>> +
>>>>> +#include <linux/kernel.h>
>>>>> +
>>>>> +struct acpi_dbg2_device;
>>>>> +struct acpi_table_header;
>>>>> +
>>>>> +struct acpi_dbg2_data {
>>>>> + u16 port_type;
>>>>> + u16 port_subtype;
>>>>> + int (*setup)(struct acpi_dbg2_device *, void *);
>>>>> + void *data;
>>>>> +};
>>>>> +
>>>>> +int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
>>>>> +
>>>>> +/**
>>>>> + * ACPI_DBG2_DECLARE() - Define handler for ACPI DBG2 port
>>>>> + * @name: Identifier to compose name of table data
>>>>> + * @type: Type of the port
>>>>> + * @subtype: Subtype of the port
>>>>> + * @setup_fn: Function to be called to setup the port
>>>>> + * (of type int (*)(struct acpi_dbg2_device *, void *);)
>>>>> + * @data_ptr: Sideband data provided back to the driver
>>>>> + */
>>>>> +#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>>>>> + static const struct acpi_dbg2_data \
>>>>> + __acpi_dbg2_data_##name __used = { \
>>>>> + .port_type = type, \
>>>>> + .port_subtype = subtype, \
>>>>> + .setup = setup_fn, \
>>>>> + .data = data_ptr, \
>>>>> + }; \
>>>>> + ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
>>>>> + acpi_dbg2_setup, &__acpi_dbg2_data_##name)
>>>>> +
>>>>> +#else
>>>>> +
>>>>> +#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>>>>> + static const void *__acpi_dbg_data_##name[] \
>>>>> + __used __initdata = { (void *)setup_fn, (void *)data_ptr }
>>>>> +
>>>>> +#endif
>>>>> +
>>>>> +#endif
>>>>>
>>>>
>>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 4/7] ACPI: parse DBG2 table
2016-03-03 16:40 ` Peter Hurley
@ 2016-03-04 12:19 ` Aleksey Makarov
2016-03-04 17:39 ` Peter Hurley
0 siblings, 1 reply; 32+ messages in thread
From: Aleksey Makarov @ 2016-03-04 12:19 UTC (permalink / raw)
To: Peter Hurley, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Len Brown, Arnd Bergmann, linux-arch
On 03/03/2016 07:40 PM, Peter Hurley wrote:
> On 03/01/2016 10:19 AM, Aleksey Makarov wrote:
>>
>>
>> On 03/01/2016 08:22 PM, Peter Hurley wrote:
>>> On 03/01/2016 08:24 AM, Aleksey Makarov wrote:
>>>>
>>>>
>>>> On 03/01/2016 05:49 PM, Peter Hurley wrote:
>>>>> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>>>>>> 'ARM Server Base Boot Requirements' [1] mentions DBG2 (Microsoft Debug
>>>>>> Port Table 2) [2] as a mandatory ACPI table that specifies debug ports.
>>>>>>
>>>>>> - Implement macros
>>>>>>
>>>>>> ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr)
>>>>>>
>>>>>> that defines a handler for the port of given type and subtype.
>>>>>>
>>>>>> - For each declared port that is also described in the ACPI DBG2 table
>>>>>> call the provided callback.
>>>>>
>>>>> On 02/22/2016 06:43 AM, Aleksey Makarov wrote:
>>>>>> On 02/19/2016 08:20 PM, Christopher Covington wrote:
>>>>>>> Can the device specified in DBG2 be used for both earlycon and KGDB? If it can only be used for one, let's make sure the choice of earlycon vs KGDB is intentional rather than accidental.
>>>>>>
>>>>>> I just sent the DBG2 series. It enables an earlycon on DBG2 port with
>>>>>> an "earlycon=acpi_dbg2" option (we can discuss particular name).
>>>>>> If you need KGDB on that port just support it for that port in the kernel
>>>>>> (i. e. add a new instance of ACPI_DBG2_DECLARE() macros for that port, see the patches)
>>>>>> and change the command line options.
>>>>>> I hope that is OK. We could continue this discussion in the DBG2 thread.
>>>>>
>>>>> This method will not work for kgdb, since kgdb doesn't actually
>>>>> implement the i/o but rather runs on top of a console.
>>>>
>>>> I see. Thank you for pointing this out.
>>>>
>>>> I don't have requirements to implement running kgdb over the serial port
>>>> specified with DBG2. This feature should be supported separately.
>>>
>>> And this takes us back full-circle to my initial point regarding
>>> supporting earlycon via ACPI: which is that my view is earlycon should
>>> be opt-in for any ACPI-specified console, rather than console via
>>> SPCR and earlycon via DBG2.
>>
>> This is the main point on which we do not agree:
>> should SPCR start a new earlycon or match existing full-featured console.
>
> My point of view is not that SPCR should *only* start an earlycon
> but rather *optionally also* start an earlycon.
Again, we have DBG2 to specify earlycon.
SPCR specifies full-featured console. DBG2 specifies earlycon.
> This is the existing behavior of every other firmware-specified console.
It SPCR + DBG2 works exactly as every other firmware-specified console
(well, after I change "earlycon=acpi_dbg2" to just "earlycon" back)
plus it allows to specify console and earlycon separately.
>> But I do not see how this kgdb/DBG2 feature makes your point of view
>> more founded. Can you please elaborate?
>
> Well, my main concern is that other configurations that you have not
> provided for will not be supportable at all because of the design choices
> you're making here.
>
> For example, your current design does not allow for earlycon+console
> on the SPCR port and debugger on DBG2 port. I think that's a problem.
It can not run earlycon+console on the SPCR port because earlycon
is specified separately by DBG2. And that is not a problem, just specify it.
As for debugger, it's not the object of these patchets.
But I am sure it will not be hard to run it on DBG2, it's just not
what these patches do.
> Another concern is that, since you haven't accounted for options which
> we'll want to implement in the near future, that a rewrite will be
> necessary to implement those.
Correct. I can not forecast any future design decision. Nobody can.
> This framework is fairly heavyweight to already not have properly
> considered how to start kgdb via DBG2.
There is no framework. It's just a fix for ACPI tables and one simple
callback function. No design decisions were made to restrict kdbg via DBG2.
>> On the contrary, if SPCR console is earlycon, we will not be able to
>> run kgdb on it.
>
> I'm not sure what you mean by "run kgdb on it". What is "it" and why
> would that be a problem?
kgdb can not run over earlycon. If SPCR runs earlycon we can not run kgdb over it.
>>>> Thank you
>>>> Aleksey Makarov
>>>>
>>>>>> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
>>>>>> [2] http://go.microsoft.com/fwlink/p/?LinkId=234837
>>>>>>
>>>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>>>>> ---
>>>>>> drivers/acpi/Kconfig | 3 ++
>>>>>> drivers/acpi/Makefile | 1 +
>>>>>> drivers/acpi/dbg2.c | 88 +++++++++++++++++++++++++++++++++++++++
>>>>>> include/asm-generic/vmlinux.lds.h | 1 +
>>>>>> include/linux/acpi_dbg2.h | 48 +++++++++++++++++++++
>>>>>> 5 files changed, 141 insertions(+)
>>>>>> create mode 100644 drivers/acpi/dbg2.c
>>>>>> create mode 100644 include/linux/acpi_dbg2.h
>>>>>>
>>>>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>>>>>> index 65fb483..660666e 100644
>>>>>> --- a/drivers/acpi/Kconfig
>>>>>> +++ b/drivers/acpi/Kconfig
>>>>>> @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
>>>>>> config ACPI_CCA_REQUIRED
>>>>>> bool
>>>>>>
>>>>>> +config ACPI_DBG2_TABLE
>>>>>> + bool
>>>>>> +
>>>>>> config ACPI_DEBUGGER
>>>>>> bool "AML debugger interface"
>>>>>> select ACPI_DEBUG
>>>>>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>>>>>> index 7395928..3b5f1ea 100644
>>>>>> --- a/drivers/acpi/Makefile
>>>>>> +++ b/drivers/acpi/Makefile
>>>>>> @@ -83,6 +83,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
>>>>>> obj-$(CONFIG_ACPI_BGRT) += bgrt.o
>>>>>> obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o
>>>>>> obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
>>>>>> +obj-$(CONFIG_ACPI_DBG2_TABLE) += dbg2.o
>>>>>>
>>>>>> # processor has its own "processor." module_param namespace
>>>>>> processor-y := processor_driver.o
>>>>>> diff --git a/drivers/acpi/dbg2.c b/drivers/acpi/dbg2.c
>>>>>> new file mode 100644
>>>>>> index 0000000..0f0f6ca
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/acpi/dbg2.c
>>>>>> @@ -0,0 +1,88 @@
>>>>>> +/*
>>>>>> + * Copyright (c) 2012, Intel Corporation
>>>>>> + * Copyright (c) 2015, 2016 Linaro Ltd.
>>>>>> + *
>>>>>> + * 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.
>>>>>> + *
>>>>>> + */
>>>>>> +
>>>>>> +#define pr_fmt(fmt) "ACPI: DBG2: " fmt
>>>>>> +
>>>>>> +#include <linux/acpi_dbg2.h>
>>>>>> +#include <linux/acpi.h>
>>>>>> +#include <linux/kernel.h>
>>>>>> +
>>>>>> +static const char * __init type2string(u16 type)
>>>>>> +{
>>>>>> + switch (type) {
>>>>>> + case ACPI_DBG2_SERIAL_PORT:
>>>>>> + return "SERIAL";
>>>>>> + case ACPI_DBG2_1394_PORT:
>>>>>> + return "1394";
>>>>>> + case ACPI_DBG2_USB_PORT:
>>>>>> + return "USB";
>>>>>> + case ACPI_DBG2_NET_PORT:
>>>>>> + return "NET";
>>>>>> + default:
>>>>>> + return "?";
>>>>>> + }
>>>>>> +}
>>>>>> +
>>>>>> +static const char * __init subtype2string(u16 subtype)
>>>>>> +{
>>>>>> + switch (subtype) {
>>>>>> + case ACPI_DBG2_16550_COMPATIBLE:
>>>>>> + return "16550_COMPATIBLE";
>>>>>> + case ACPI_DBG2_16550_SUBSET:
>>>>>> + return "16550_SUBSET";
>>>>>> + case ACPI_DBG2_ARM_PL011:
>>>>>> + return "ARM_PL011";
>>>>>> + case ACPI_DBG2_ARM_SBSA_32BIT:
>>>>>> + return "ARM_SBSA_32BIT";
>>>>>> + case ACPI_DBG2_ARM_SBSA_GENERIC:
>>>>>> + return "ARM_SBSA_GENERIC";
>>>>>> + case ACPI_DBG2_ARM_DCC:
>>>>>> + return "ARM_DCC";
>>>>>> + case ACPI_DBG2_BCM2835:
>>>>>> + return "BCM2835";
>>>>>> + default:
>>>>>> + return "?";
>>>>>> + }
>>>>>> +}
>>>>>> +
>>>>>> +int __init acpi_dbg2_setup(struct acpi_table_header *table, const void *data)
>>>>>> +{
>>>>>> + struct acpi_table_dbg2 *dbg2 = (struct acpi_table_dbg2 *)table;
>>>>>> + struct acpi_dbg2_data *dbg2_data = (struct acpi_dbg2_data *)data;
>>>>>> + struct acpi_dbg2_device *dbg2_device, *dbg2_end;
>>>>>> + int i;
>>>>>> +
>>>>>> + dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2,
>>>>>> + dbg2->info_offset);
>>>>>> + dbg2_end = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2, table->length);
>>>>>> +
>>>>>> + for (i = 0; i < dbg2->info_count; i++) {
>>>>>> + if (dbg2_device + 1 > dbg2_end) {
>>>>>> + pr_err("device pointer overflows, bad table\n");
>>>>>> + return 0;
>>>>>> + }
>>>>>> +
>>>>>> + if (dbg2_device->port_type == dbg2_data->port_type &&
>>>>>> + dbg2_device->port_subtype == dbg2_data->port_subtype) {
>>>>>> + if (dbg2_device->port_type == ACPI_DBG2_SERIAL_PORT)
>>>>>> + pr_info("debug port: SERIAL; subtype: %s\n",
>>>>>> + subtype2string(dbg2_device->port_subtype));
>>>>>> + else
>>>>>> + pr_info("debug port: %s\n",
>>>>>> + type2string(dbg2_device->port_type));
>>>>>> + dbg2_data->setup(dbg2_device, dbg2_data->data);
>>>>>> + }
>>>>>> +
>>>>>> + dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2_device,
>>>>>> + dbg2_device->length);
>>>>>> + }
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
>>>>>> index 8f5a12a..8cc49ba 100644
>>>>>> --- a/include/asm-generic/vmlinux.lds.h
>>>>>> +++ b/include/asm-generic/vmlinux.lds.h
>>>>>> @@ -526,6 +526,7 @@
>>>>>> IRQCHIP_OF_MATCH_TABLE() \
>>>>>> ACPI_PROBE_TABLE(irqchip) \
>>>>>> ACPI_PROBE_TABLE(clksrc) \
>>>>>> + ACPI_PROBE_TABLE(dbg2) \
>>>>>> EARLYCON_TABLE()
>>>>>>
>>>>>> #define INIT_TEXT \
>>>>>> diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
>>>>>> new file mode 100644
>>>>>> index 0000000..125ae7e
>>>>>> --- /dev/null
>>>>>> +++ b/include/linux/acpi_dbg2.h
>>>>>> @@ -0,0 +1,48 @@
>>>>>> +#ifndef _ACPI_DBG2_H_
>>>>>> +#define _ACPI_DBG2_H_
>>>>>> +
>>>>>> +#ifdef CONFIG_ACPI_DBG2_TABLE
>>>>>> +
>>>>>> +#include <linux/kernel.h>
>>>>>> +
>>>>>> +struct acpi_dbg2_device;
>>>>>> +struct acpi_table_header;
>>>>>> +
>>>>>> +struct acpi_dbg2_data {
>>>>>> + u16 port_type;
>>>>>> + u16 port_subtype;
>>>>>> + int (*setup)(struct acpi_dbg2_device *, void *);
>>>>>> + void *data;
>>>>>> +};
>>>>>> +
>>>>>> +int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
>>>>>> +
>>>>>> +/**
>>>>>> + * ACPI_DBG2_DECLARE() - Define handler for ACPI DBG2 port
>>>>>> + * @name: Identifier to compose name of table data
>>>>>> + * @type: Type of the port
>>>>>> + * @subtype: Subtype of the port
>>>>>> + * @setup_fn: Function to be called to setup the port
>>>>>> + * (of type int (*)(struct acpi_dbg2_device *, void *);)
>>>>>> + * @data_ptr: Sideband data provided back to the driver
>>>>>> + */
>>>>>> +#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>>>>>> + static const struct acpi_dbg2_data \
>>>>>> + __acpi_dbg2_data_##name __used = { \
>>>>>> + .port_type = type, \
>>>>>> + .port_subtype = subtype, \
>>>>>> + .setup = setup_fn, \
>>>>>> + .data = data_ptr, \
>>>>>> + }; \
>>>>>> + ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
>>>>>> + acpi_dbg2_setup, &__acpi_dbg2_data_##name)
>>>>>> +
>>>>>> +#else
>>>>>> +
>>>>>> +#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>>>>>> + static const void *__acpi_dbg_data_##name[] \
>>>>>> + __used __initdata = { (void *)setup_fn, (void *)data_ptr }
>>>>>> +
>>>>>> +#endif
>>>>>> +
>>>>>> +#endif
>>>>>>
>>>>>
>>>
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 4/7] ACPI: parse DBG2 table
2016-03-04 12:19 ` Aleksey Makarov
@ 2016-03-04 17:39 ` Peter Hurley
0 siblings, 0 replies; 32+ messages in thread
From: Peter Hurley @ 2016-03-04 17:39 UTC (permalink / raw)
To: Aleksey Makarov, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Len Brown, Arnd Bergmann, linux-arch
On 03/04/2016 04:19 AM, Aleksey Makarov wrote:
> On 03/03/2016 07:40 PM, Peter Hurley wrote:
>> On 03/01/2016 10:19 AM, Aleksey Makarov wrote:
>>> On 03/01/2016 08:22 PM, Peter Hurley wrote:
>>>>> On 03/01/2016 05:49 PM, Peter Hurley wrote:
>>>>>> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>>>>>>> 'ARM Server Base Boot Requirements' [1] mentions DBG2 (Microsoft Debug
>>>>>>> Port Table 2) [2] as a mandatory ACPI table that specifies debug ports.
>>>>>>>
>>>>>>> - Implement macros
>>>>>>>
>>>>>>> ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr)
>>>>>>>
>>>>>>> that defines a handler for the port of given type and subtype.
>>>>>>>
>>>>>>> - For each declared port that is also described in the ACPI DBG2 table
>>>>>>> call the provided callback.
>>>>>>
>>>>>> On 02/22/2016 06:43 AM, Aleksey Makarov wrote:
>>>>>>> On 02/19/2016 08:20 PM, Christopher Covington wrote:
>>>>>>>> Can the device specified in DBG2 be used for both earlycon and KGDB? If it can only be used for one, let's make sure the choice of earlycon vs KGDB is intentional rather than accidental.
>>>>>>>
>>>>>>> I just sent the DBG2 series. It enables an earlycon on DBG2 port with
>>>>>>> an "earlycon=acpi_dbg2" option (we can discuss particular name).
>>>>>>> If you need KGDB on that port just support it for that port in the kernel
>>>>>>> (i. e. add a new instance of ACPI_DBG2_DECLARE() macros for that port, see the patches)
>>>>>>> and change the command line options.
>>>>>>> I hope that is OK. We could continue this discussion in the DBG2 thread.
>>>>>>
>>>>>> This method will not work for kgdb, since kgdb doesn't actually
>>>>>> implement the i/o but rather runs on top of a console.
>>>>>
>>>>> I see. Thank you for pointing this out.
>>>>>
>>>>> I don't have requirements to implement running kgdb over the serial port
>>>>> specified with DBG2. This feature should be supported separately.
>>>>
>>>> And this takes us back full-circle to my initial point regarding
>>>> supporting earlycon via ACPI: which is that my view is earlycon should
>>>> be opt-in for any ACPI-specified console, rather than console via
>>>> SPCR and earlycon via DBG2.
>>>
>>> This is the main point on which we do not agree:
>>> should SPCR start a new earlycon or match existing full-featured console.
>>
>> My point of view is not that SPCR should *only* start an earlycon
>> but rather *optionally also* start an earlycon.
>
> Again, we have DBG2 to specify earlycon.
> SPCR specifies full-featured console. DBG2 specifies earlycon.
>
>> This is the existing behavior of every other firmware-specified console.
>
> It SPCR + DBG2 works exactly as every other firmware-specified console
No it doesn't.
For OF, /chosen/stdout-path specifies the "full-featured console".
Adding "earlycon" to the command line starts an earlycon on the same
console, without firmware changes.
For PCDP, the firmware-specified console starts an earlycon and
registers a full-featured console for the same device.
Same for MIPS Malta.
In fact, there's not a single in-tree example of firmware that
uses one table/setting for earlycon and a different table/setting for
full-featured console.
> (well, after I change "earlycon=acpi_dbg2" to just "earlycon" back)
> plus it allows to specify console and earlycon separately.
>
>>> But I do not see how this kgdb/DBG2 feature makes your point of view
>>> more founded. Can you please elaborate?
>>
>> Well, my main concern is that other configurations that you have not
>> provided for will not be supportable at all because of the design choices
>> you're making here.
>>
>> For example, your current design does not allow for earlycon+console
>> on the SPCR port and debugger on DBG2 port. I think that's a problem.
>
> It can not run earlycon+console on the SPCR port because earlycon
> is specified separately by DBG2. And that is not a problem, just specify it.
>
> As for debugger, it's not the object of these patchets.
> But I am sure it will not be hard to run it on DBG2, it's just not
> what these patches do.
No offense, but 2 emails ago you were sure it was as trivial
as a macro invocation, until I pointed out that wouldn't work.
>> Another concern is that, since you haven't accounted for options which
>> we'll want to implement in the near future, that a rewrite will be
>> necessary to implement those.
>
> Correct. I can not forecast any future design decision. Nobody can.
Actually Chris already pointed out kgdb needs to run from DBG2.
I understand you don't want to implement that. However, it is a design
decision that has already been forecasted.
So no one's asking you to either forecast or implement it, only to
account for it in the design.
I think console + optional earlycon over the SPCR-specified device
and kgdb over the DBG2-specified device makes more sense,
but I'm also willing to accept a more flexible configuration of
options.
For example,
1: SPCR: console
2: SPCR + earlycon command line: console + earlycon
3: SPCR: console, DBG2 + earlycon command line: earlycon
4: SPCR: console, DBG2 + kgdb command line: console + kgdboc
5: DBG2 + kgdb command line: console + kgdb
6: DBG2 + earlycon + kgdb command line: earlycon + console + kgdboc
etc..
>> This framework is fairly heavyweight to already not have properly
>> considered how to start kgdb via DBG2.
>
> There is no framework. It's just a fix for ACPI tables and one simple
> callback function.
Documentation/kernel-parameters.txt | 3 +
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/acpi.c | 2 +
drivers/acpi/Kconfig | 3 +
drivers/acpi/Makefile | 1 +
drivers/acpi/dbg2.c | 88 +++++++++++++++++++++++++++++
drivers/acpi/scan.c | 44 ++++++++++-----
drivers/clocksource/arm_arch_timer.c | 3 +-
drivers/irqchip/irq-gic.c | 4 +-
drivers/of/fdt.c | 11 +---
drivers/tty/serial/amba-pl011.c | 3 +
drivers/tty/serial/earlycon.c | 61 ++++++++++++++++++++
include/acpi/actbl2.h | 5 ++
include/asm-generic/vmlinux.lds.h | 1 +
include/linux/acpi.h | 104 ++++++++++++++++++++++++-----------
include/linux/acpi_dbg2.h | 68 +++++++++++++++++++++++
include/linux/clocksource.h | 2 +-
include/linux/irqchip.h | 5 +-
include/linux/of_fdt.h | 2 +
19 files changed, 348 insertions(+), 63 deletions(-)
create mode 100644 drivers/acpi/dbg2.c
create mode 100644 include/linux/acpi_dbg2.h
> No design decisions were made to restrict kdbg via DBG2.
A plausible outline for what is required for implementation on top
of what you have submitted would be sufficient; an assertion that
"it will not be hard" is not.
>>> On the contrary, if SPCR console is earlycon, we will not be able to
>>> run kgdb on it.
>>
>> I'm not sure what you mean by "run kgdb on it". What is "it" and why
>> would that be a problem?
>
> kgdb can not run over earlycon.
Right. I'm the one that told you that.
> If SPCR runs earlycon we can not run kgdb over it.
??
Right now I can run an earlycon, a console and kgdb on that console
all on the same device with OF.
Why is this a problem for ACPI?
If SPCR defines the console to start and the user also opts for earlycon
on the same device, kgdb will still be able to later start on the console.
As I explained, when a real console starts, the earlycon is disabled.
>>>>> Thank you
>>>>> Aleksey Makarov
>>>>>
>>>>>>> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
>>>>>>> [2] http://go.microsoft.com/fwlink/p/?LinkId=234837
>>>>>>>
>>>>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>>>>>> ---
>>>>>>> drivers/acpi/Kconfig | 3 ++
>>>>>>> drivers/acpi/Makefile | 1 +
>>>>>>> drivers/acpi/dbg2.c | 88 +++++++++++++++++++++++++++++++++++++++
>>>>>>> include/asm-generic/vmlinux.lds.h | 1 +
>>>>>>> include/linux/acpi_dbg2.h | 48 +++++++++++++++++++++
>>>>>>> 5 files changed, 141 insertions(+)
>>>>>>> create mode 100644 drivers/acpi/dbg2.c
>>>>>>> create mode 100644 include/linux/acpi_dbg2.h
>>>>>>>
>>>>>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>>>>>>> index 65fb483..660666e 100644
>>>>>>> --- a/drivers/acpi/Kconfig
>>>>>>> +++ b/drivers/acpi/Kconfig
>>>>>>> @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
>>>>>>> config ACPI_CCA_REQUIRED
>>>>>>> bool
>>>>>>>
>>>>>>> +config ACPI_DBG2_TABLE
>>>>>>> + bool
>>>>>>> +
>>>>>>> config ACPI_DEBUGGER
>>>>>>> bool "AML debugger interface"
>>>>>>> select ACPI_DEBUG
>>>>>>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>>>>>>> index 7395928..3b5f1ea 100644
>>>>>>> --- a/drivers/acpi/Makefile
>>>>>>> +++ b/drivers/acpi/Makefile
>>>>>>> @@ -83,6 +83,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
>>>>>>> obj-$(CONFIG_ACPI_BGRT) += bgrt.o
>>>>>>> obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o
>>>>>>> obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
>>>>>>> +obj-$(CONFIG_ACPI_DBG2_TABLE) += dbg2.o
>>>>>>>
>>>>>>> # processor has its own "processor." module_param namespace
>>>>>>> processor-y := processor_driver.o
>>>>>>> diff --git a/drivers/acpi/dbg2.c b/drivers/acpi/dbg2.c
>>>>>>> new file mode 100644
>>>>>>> index 0000000..0f0f6ca
>>>>>>> --- /dev/null
>>>>>>> +++ b/drivers/acpi/dbg2.c
>>>>>>> @@ -0,0 +1,88 @@
>>>>>>> +/*
>>>>>>> + * Copyright (c) 2012, Intel Corporation
>>>>>>> + * Copyright (c) 2015, 2016 Linaro Ltd.
>>>>>>> + *
>>>>>>> + * 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.
>>>>>>> + *
>>>>>>> + */
>>>>>>> +
>>>>>>> +#define pr_fmt(fmt) "ACPI: DBG2: " fmt
>>>>>>> +
>>>>>>> +#include <linux/acpi_dbg2.h>
>>>>>>> +#include <linux/acpi.h>
>>>>>>> +#include <linux/kernel.h>
>>>>>>> +
>>>>>>> +static const char * __init type2string(u16 type)
>>>>>>> +{
>>>>>>> + switch (type) {
>>>>>>> + case ACPI_DBG2_SERIAL_PORT:
>>>>>>> + return "SERIAL";
>>>>>>> + case ACPI_DBG2_1394_PORT:
>>>>>>> + return "1394";
>>>>>>> + case ACPI_DBG2_USB_PORT:
>>>>>>> + return "USB";
>>>>>>> + case ACPI_DBG2_NET_PORT:
>>>>>>> + return "NET";
>>>>>>> + default:
>>>>>>> + return "?";
>>>>>>> + }
>>>>>>> +}
>>>>>>> +
>>>>>>> +static const char * __init subtype2string(u16 subtype)
>>>>>>> +{
>>>>>>> + switch (subtype) {
>>>>>>> + case ACPI_DBG2_16550_COMPATIBLE:
>>>>>>> + return "16550_COMPATIBLE";
>>>>>>> + case ACPI_DBG2_16550_SUBSET:
>>>>>>> + return "16550_SUBSET";
>>>>>>> + case ACPI_DBG2_ARM_PL011:
>>>>>>> + return "ARM_PL011";
>>>>>>> + case ACPI_DBG2_ARM_SBSA_32BIT:
>>>>>>> + return "ARM_SBSA_32BIT";
>>>>>>> + case ACPI_DBG2_ARM_SBSA_GENERIC:
>>>>>>> + return "ARM_SBSA_GENERIC";
>>>>>>> + case ACPI_DBG2_ARM_DCC:
>>>>>>> + return "ARM_DCC";
>>>>>>> + case ACPI_DBG2_BCM2835:
>>>>>>> + return "BCM2835";
>>>>>>> + default:
>>>>>>> + return "?";
>>>>>>> + }
>>>>>>> +}
>>>>>>> +
>>>>>>> +int __init acpi_dbg2_setup(struct acpi_table_header *table, const void *data)
>>>>>>> +{
>>>>>>> + struct acpi_table_dbg2 *dbg2 = (struct acpi_table_dbg2 *)table;
>>>>>>> + struct acpi_dbg2_data *dbg2_data = (struct acpi_dbg2_data *)data;
>>>>>>> + struct acpi_dbg2_device *dbg2_device, *dbg2_end;
>>>>>>> + int i;
>>>>>>> +
>>>>>>> + dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2,
>>>>>>> + dbg2->info_offset);
>>>>>>> + dbg2_end = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2, table->length);
>>>>>>> +
>>>>>>> + for (i = 0; i < dbg2->info_count; i++) {
>>>>>>> + if (dbg2_device + 1 > dbg2_end) {
>>>>>>> + pr_err("device pointer overflows, bad table\n");
>>>>>>> + return 0;
>>>>>>> + }
>>>>>>> +
>>>>>>> + if (dbg2_device->port_type == dbg2_data->port_type &&
>>>>>>> + dbg2_device->port_subtype == dbg2_data->port_subtype) {
>>>>>>> + if (dbg2_device->port_type == ACPI_DBG2_SERIAL_PORT)
>>>>>>> + pr_info("debug port: SERIAL; subtype: %s\n",
>>>>>>> + subtype2string(dbg2_device->port_subtype));
>>>>>>> + else
>>>>>>> + pr_info("debug port: %s\n",
>>>>>>> + type2string(dbg2_device->port_type));
>>>>>>> + dbg2_data->setup(dbg2_device, dbg2_data->data);
>>>>>>> + }
>>>>>>> +
>>>>>>> + dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2_device,
>>>>>>> + dbg2_device->length);
>>>>>>> + }
>>>>>>> +
>>>>>>> + return 0;
>>>>>>> +}
>>>>>>> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
>>>>>>> index 8f5a12a..8cc49ba 100644
>>>>>>> --- a/include/asm-generic/vmlinux.lds.h
>>>>>>> +++ b/include/asm-generic/vmlinux.lds.h
>>>>>>> @@ -526,6 +526,7 @@
>>>>>>> IRQCHIP_OF_MATCH_TABLE() \
>>>>>>> ACPI_PROBE_TABLE(irqchip) \
>>>>>>> ACPI_PROBE_TABLE(clksrc) \
>>>>>>> + ACPI_PROBE_TABLE(dbg2) \
>>>>>>> EARLYCON_TABLE()
>>>>>>>
>>>>>>> #define INIT_TEXT \
>>>>>>> diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
>>>>>>> new file mode 100644
>>>>>>> index 0000000..125ae7e
>>>>>>> --- /dev/null
>>>>>>> +++ b/include/linux/acpi_dbg2.h
>>>>>>> @@ -0,0 +1,48 @@
>>>>>>> +#ifndef _ACPI_DBG2_H_
>>>>>>> +#define _ACPI_DBG2_H_
>>>>>>> +
>>>>>>> +#ifdef CONFIG_ACPI_DBG2_TABLE
>>>>>>> +
>>>>>>> +#include <linux/kernel.h>
>>>>>>> +
>>>>>>> +struct acpi_dbg2_device;
>>>>>>> +struct acpi_table_header;
>>>>>>> +
>>>>>>> +struct acpi_dbg2_data {
>>>>>>> + u16 port_type;
>>>>>>> + u16 port_subtype;
>>>>>>> + int (*setup)(struct acpi_dbg2_device *, void *);
>>>>>>> + void *data;
>>>>>>> +};
>>>>>>> +
>>>>>>> +int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
>>>>>>> +
>>>>>>> +/**
>>>>>>> + * ACPI_DBG2_DECLARE() - Define handler for ACPI DBG2 port
>>>>>>> + * @name: Identifier to compose name of table data
>>>>>>> + * @type: Type of the port
>>>>>>> + * @subtype: Subtype of the port
>>>>>>> + * @setup_fn: Function to be called to setup the port
>>>>>>> + * (of type int (*)(struct acpi_dbg2_device *, void *);)
>>>>>>> + * @data_ptr: Sideband data provided back to the driver
>>>>>>> + */
>>>>>>> +#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>>>>>>> + static const struct acpi_dbg2_data \
>>>>>>> + __acpi_dbg2_data_##name __used = { \
>>>>>>> + .port_type = type, \
>>>>>>> + .port_subtype = subtype, \
>>>>>>> + .setup = setup_fn, \
>>>>>>> + .data = data_ptr, \
>>>>>>> + }; \
>>>>>>> + ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
>>>>>>> + acpi_dbg2_setup, &__acpi_dbg2_data_##name)
>>>>>>> +
>>>>>>> +#else
>>>>>>> +
>>>>>>> +#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>>>>>>> + static const void *__acpi_dbg_data_##name[] \
>>>>>>> + __used __initdata = { (void *)setup_fn, (void *)data_ptr }
>>>>>>> +
>>>>>>> +#endif
>>>>>>> +
>>>>>>> +#endif
>>>>>>>
>>>>>>
>>>>
>>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port
2016-02-29 12:41 [PATCH v3 0/7] ACPI: parse the DBG2 table Aleksey Makarov
` (3 preceding siblings ...)
2016-02-29 12:42 ` [PATCH v3 4/7] ACPI: parse DBG2 table Aleksey Makarov
@ 2016-02-29 12:42 ` Aleksey Makarov
2016-03-01 15:53 ` Peter Hurley
2016-02-29 12:42 ` [PATCH v3 6/7] ACPI: enable ACPI_DBG2_TABLE on ARM64 Aleksey Makarov
2016-02-29 12:42 ` [PATCH v3 7/7] serial: pl011: add ACPI DBG2 serial port Aleksey Makarov
6 siblings, 1 reply; 32+ messages in thread
From: Aleksey Makarov @ 2016-02-29 12:42 UTC (permalink / raw)
To: linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Aleksey Makarov,
Russell King, Greg Kroah-Hartman, Rafael J . Wysocki,
Leif Lindholm, Graeme Gregory, Al Stone, Christopher Covington,
Matthias Brugger, Jonathan Corbet, Jiri Slaby, linux-doc
Add ACPI_DBG2_EARLYCON_DECLARE() macros that declares
an earlycon on the serial port specified in the DBG2 ACPI table.
Pass the string "earlycon=acpi_dbg2" to the kernel to activate it.
Callbacks for EARLYCON_DECLARE() and OF_EARLYCON_DECLARE()
can also be used for this macros.
Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
Documentation/kernel-parameters.txt | 3 ++
drivers/tty/serial/earlycon.c | 60 +++++++++++++++++++++++++++++++++++++
include/linux/acpi_dbg2.h | 20 +++++++++++++
3 files changed, 83 insertions(+)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index e0a21e4..19b947b 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1072,6 +1072,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
A valid base address must be provided, and the serial
port must already be setup and configured.
+ acpi_dbg2
+ Use serial port specified by the DBG2 ACPI table.
+
earlyprintk= [X86,SH,BLACKFIN,ARM,M68k]
earlyprintk=vga
earlyprintk=efi
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index d217366..9ba3a04 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -22,6 +22,7 @@
#include <linux/sizes.h>
#include <linux/of.h>
#include <linux/of_fdt.h>
+#include <linux/acpi.h>
#ifdef CONFIG_FIX_EARLYCON_MEM
#include <asm/fixmap.h>
@@ -200,6 +201,8 @@ int __init setup_earlycon(char *buf)
return -ENOENT;
}
+static bool setup_dbg2_earlycon;
+
/* early_param wrapper for setup_earlycon() */
static int __init param_setup_earlycon(char *buf)
{
@@ -212,6 +215,11 @@ static int __init param_setup_earlycon(char *buf)
if (!buf || !buf[0])
return early_init_dt_scan_chosen_serial();
+ if (!strcmp(buf, "acpi_dbg2")) {
+ setup_dbg2_earlycon = true;
+ return 0;
+ }
+
err = setup_earlycon(buf);
if (err == -ENOENT || err == -EALREADY)
return 0;
@@ -286,3 +294,55 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
}
#endif /* CONFIG_OF_EARLY_FLATTREE */
+
+#ifdef CONFIG_ACPI_DBG2_TABLE
+
+int __init acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d)
+{
+ int err;
+ struct uart_port *port = &early_console_dev.port;
+ int (*setup)(struct earlycon_device *, const char *) = d;
+ struct acpi_generic_address *reg;
+
+ if (!setup_dbg2_earlycon)
+ return -ENODEV;
+
+ if (device->register_count < 1)
+ return -ENODEV;
+
+ if (device->base_address_offset >= device->length)
+ return -EINVAL;
+
+ reg = (void *)device + device->base_address_offset;
+
+ if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
+ reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO)
+ return -EINVAL;
+
+ spin_lock_init(&port->lock);
+ port->uartclk = BASE_BAUD * 16;
+
+ if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
+ if (device->port_type == ACPI_DBG2_ARM_SBSA_32BIT)
+ port->iotype = UPIO_MEM32;
+ else
+ port->iotype = UPIO_MEM;
+ port->mapbase = reg->address;
+ port->membase = earlycon_map(reg->address, SZ_4K);
+ } else {
+ port->iotype = UPIO_PORT;
+ port->iobase = reg->address;
+ }
+
+ early_console_dev.con->data = &early_console_dev;
+ err = setup(&early_console_dev, NULL);
+ if (err < 0)
+ return err;
+ if (!early_console_dev.con->write)
+ return -ENODEV;
+
+ register_console(early_console_dev.con);
+ return 0;
+}
+
+#endif /* CONFIG_ACPI_DBG2_TABLE */
diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
index 125ae7e..b653752 100644
--- a/include/linux/acpi_dbg2.h
+++ b/include/linux/acpi_dbg2.h
@@ -37,12 +37,32 @@ int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
acpi_dbg2_setup, &__acpi_dbg2_data_##name)
+int acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d);
+
+/**
+ * ACPI_DBG2_EARLYCON_DECLARE() - Define handler for ACPI GDB2 serial port
+ * @name: Identifier to compose name of table data
+ * @subtype: Subtype of the port
+ * @console_setup: Function to be called to setup the port
+ *
+ * Type of the console_setup() callback is
+ * int (*setup)(struct earlycon_device *, const char *)
+ * It's the type of callback of of_setup_earlycon().
+ */
+#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
+ ACPI_DBG2_DECLARE(name, ACPI_DBG2_SERIAL_PORT, subtype, \
+ acpi_setup_earlycon, console_setup)
+
#else
#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
static const void *__acpi_dbg_data_##name[] \
__used __initdata = { (void *)setup_fn, (void *)data_ptr }
+#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
+ static const void *__acpi_dbg_data_serial_##name[] \
+ __used __initdata = { (void *)console_setup }
+
#endif
#endif
--
2.7.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port
2016-02-29 12:42 ` [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port Aleksey Makarov
@ 2016-03-01 15:53 ` Peter Hurley
2016-03-01 16:57 ` Aleksey Makarov
0 siblings, 1 reply; 32+ messages in thread
From: Peter Hurley @ 2016-03-01 15:53 UTC (permalink / raw)
To: Aleksey Makarov, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Jonathan Corbet, Jiri Slaby, linux-doc
On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
> Add ACPI_DBG2_EARLYCON_DECLARE() macros that declares
> an earlycon on the serial port specified in the DBG2 ACPI table.
>
> Pass the string "earlycon=acpi_dbg2" to the kernel to activate it.
>
> Callbacks for EARLYCON_DECLARE() and OF_EARLYCON_DECLARE()
> can also be used for this macros.
>
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> ---
> Documentation/kernel-parameters.txt | 3 ++
> drivers/tty/serial/earlycon.c | 60 +++++++++++++++++++++++++++++++++++++
> include/linux/acpi_dbg2.h | 20 +++++++++++++
> 3 files changed, 83 insertions(+)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index e0a21e4..19b947b 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1072,6 +1072,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> A valid base address must be provided, and the serial
> port must already be setup and configured.
>
> + acpi_dbg2
> + Use serial port specified by the DBG2 ACPI table.
> +
> earlyprintk= [X86,SH,BLACKFIN,ARM,M68k]
> earlyprintk=vga
> earlyprintk=efi
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index d217366..9ba3a04 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -22,6 +22,7 @@
> #include <linux/sizes.h>
> #include <linux/of.h>
> #include <linux/of_fdt.h>
> +#include <linux/acpi.h>
>
> #ifdef CONFIG_FIX_EARLYCON_MEM
> #include <asm/fixmap.h>
> @@ -200,6 +201,8 @@ int __init setup_earlycon(char *buf)
> return -ENOENT;
> }
>
> +static bool setup_dbg2_earlycon;
> +
> /* early_param wrapper for setup_earlycon() */
> static int __init param_setup_earlycon(char *buf)
> {
> @@ -212,6 +215,11 @@ static int __init param_setup_earlycon(char *buf)
> if (!buf || !buf[0])
> return early_init_dt_scan_chosen_serial();
>
> + if (!strcmp(buf, "acpi_dbg2")) {
> + setup_dbg2_earlycon = true;
> + return 0;
> + }
So this series doesn't start an ACPI earlycon at early_param time?
That doesn't seem very useful.
When does the ACPI earlycon actually start?
And don't say "when the DBG2 table is probed"; that much is obvious.
> +
> err = setup_earlycon(buf);
> if (err == -ENOENT || err == -EALREADY)
> return 0;
> @@ -286,3 +294,55 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
> }
>
> #endif /* CONFIG_OF_EARLY_FLATTREE */
> +
> +#ifdef CONFIG_ACPI_DBG2_TABLE
> +
> +int __init acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d)
> +{
> + int err;
> + struct uart_port *port = &early_console_dev.port;
> + int (*setup)(struct earlycon_device *, const char *) = d;
> + struct acpi_generic_address *reg;
> +
> + if (!setup_dbg2_earlycon)
> + return -ENODEV;
> +
> + if (device->register_count < 1)
> + return -ENODEV;
> +
> + if (device->base_address_offset >= device->length)
> + return -EINVAL;
> +
> + reg = (void *)device + device->base_address_offset;
> +
> + if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
> + reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO)
> + return -EINVAL;
> +
> + spin_lock_init(&port->lock);
> + port->uartclk = BASE_BAUD * 16;
> +
> + if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> + if (device->port_type == ACPI_DBG2_ARM_SBSA_32BIT)
> + port->iotype = UPIO_MEM32;
> + else
> + port->iotype = UPIO_MEM;
> + port->mapbase = reg->address;
> + port->membase = earlycon_map(reg->address, SZ_4K);
> + } else {
> + port->iotype = UPIO_PORT;
> + port->iobase = reg->address;
> + }
> +
> + early_console_dev.con->data = &early_console_dev;
> + err = setup(&early_console_dev, NULL);
> + if (err < 0)
> + return err;
> + if (!early_console_dev.con->write)
> + return -ENODEV;
> +
> + register_console(early_console_dev.con);
> + return 0;
> +}
> +
> +#endif /* CONFIG_ACPI_DBG2_TABLE */
> diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
> index 125ae7e..b653752 100644
> --- a/include/linux/acpi_dbg2.h
> +++ b/include/linux/acpi_dbg2.h
> @@ -37,12 +37,32 @@ int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
> ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
> acpi_dbg2_setup, &__acpi_dbg2_data_##name)
>
> +int acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d);
> +
> +/**
> + * ACPI_DBG2_EARLYCON_DECLARE() - Define handler for ACPI GDB2 serial port
> + * @name: Identifier to compose name of table data
> + * @subtype: Subtype of the port
> + * @console_setup: Function to be called to setup the port
> + *
> + * Type of the console_setup() callback is
> + * int (*setup)(struct earlycon_device *, const char *)
> + * It's the type of callback of of_setup_earlycon().
> + */
> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
> + ACPI_DBG2_DECLARE(name, ACPI_DBG2_SERIAL_PORT, subtype, \
> + acpi_setup_earlycon, console_setup)
> +
> #else
>
> #define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
> static const void *__acpi_dbg_data_##name[] \
> __used __initdata = { (void *)setup_fn, (void *)data_ptr }
>
> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
> + static const void *__acpi_dbg_data_serial_##name[] \
> + __used __initdata = { (void *)console_setup }
> +
> #endif
>
> #endif
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port
2016-03-01 15:53 ` Peter Hurley
@ 2016-03-01 16:57 ` Aleksey Makarov
2016-03-03 17:48 ` Peter Hurley
0 siblings, 1 reply; 32+ messages in thread
From: Aleksey Makarov @ 2016-03-01 16:57 UTC (permalink / raw)
To: Peter Hurley, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Jonathan Corbet, Jiri Slaby, linux-doc
On 03/01/2016 06:53 PM, Peter Hurley wrote:
> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>> Add ACPI_DBG2_EARLYCON_DECLARE() macros that declares
>> an earlycon on the serial port specified in the DBG2 ACPI table.
>>
>> Pass the string "earlycon=acpi_dbg2" to the kernel to activate it.
>>
>> Callbacks for EARLYCON_DECLARE() and OF_EARLYCON_DECLARE()
>> can also be used for this macros.
>>
>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>> ---
>> Documentation/kernel-parameters.txt | 3 ++
>> drivers/tty/serial/earlycon.c | 60 +++++++++++++++++++++++++++++++++++++
>> include/linux/acpi_dbg2.h | 20 +++++++++++++
>> 3 files changed, 83 insertions(+)
>>
>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> index e0a21e4..19b947b 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -1072,6 +1072,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>> A valid base address must be provided, and the serial
>> port must already be setup and configured.
>>
>> + acpi_dbg2
>> + Use serial port specified by the DBG2 ACPI table.
>> +
>> earlyprintk= [X86,SH,BLACKFIN,ARM,M68k]
>> earlyprintk=vga
>> earlyprintk=efi
>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>> index d217366..9ba3a04 100644
>> --- a/drivers/tty/serial/earlycon.c
>> +++ b/drivers/tty/serial/earlycon.c
>> @@ -22,6 +22,7 @@
>> #include <linux/sizes.h>
>> #include <linux/of.h>
>> #include <linux/of_fdt.h>
>> +#include <linux/acpi.h>
>>
>> #ifdef CONFIG_FIX_EARLYCON_MEM
>> #include <asm/fixmap.h>
>> @@ -200,6 +201,8 @@ int __init setup_earlycon(char *buf)
>> return -ENOENT;
>> }
>>
>> +static bool setup_dbg2_earlycon;
>> +
>> /* early_param wrapper for setup_earlycon() */
>> static int __init param_setup_earlycon(char *buf)
>> {
>> @@ -212,6 +215,11 @@ static int __init param_setup_earlycon(char *buf)
>> if (!buf || !buf[0])
>> return early_init_dt_scan_chosen_serial();
>>
>> + if (!strcmp(buf, "acpi_dbg2")) {
>> + setup_dbg2_earlycon = true;
>> + return 0;
>> + }
>
> So this series doesn't start an ACPI earlycon at early_param time?
> That doesn't seem very useful.
>
> When does the ACPI earlycon actually start?
> And don't say "when the DBG2 table is probed"; that much is obvious.
ACPI earlycon starts as soon as ACPI tables become accessible (setup_arch()).
I think that is still quite early.
>> +
>> err = setup_earlycon(buf);
>> if (err == -ENOENT || err == -EALREADY)
>> return 0;
>> @@ -286,3 +294,55 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
>> }
>>
>> #endif /* CONFIG_OF_EARLY_FLATTREE */
>> +
>> +#ifdef CONFIG_ACPI_DBG2_TABLE
>> +
>> +int __init acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d)
>> +{
>> + int err;
>> + struct uart_port *port = &early_console_dev.port;
>> + int (*setup)(struct earlycon_device *, const char *) = d;
>> + struct acpi_generic_address *reg;
>> +
>> + if (!setup_dbg2_earlycon)
>> + return -ENODEV;
>> +
>> + if (device->register_count < 1)
>> + return -ENODEV;
>> +
>> + if (device->base_address_offset >= device->length)
>> + return -EINVAL;
>> +
>> + reg = (void *)device + device->base_address_offset;
>> +
>> + if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
>> + reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO)
>> + return -EINVAL;
>> +
>> + spin_lock_init(&port->lock);
>> + port->uartclk = BASE_BAUD * 16;
>> +
>> + if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
>> + if (device->port_type == ACPI_DBG2_ARM_SBSA_32BIT)
>> + port->iotype = UPIO_MEM32;
>> + else
>> + port->iotype = UPIO_MEM;
>> + port->mapbase = reg->address;
>> + port->membase = earlycon_map(reg->address, SZ_4K);
>> + } else {
>> + port->iotype = UPIO_PORT;
>> + port->iobase = reg->address;
>> + }
>> +
>> + early_console_dev.con->data = &early_console_dev;
>> + err = setup(&early_console_dev, NULL);
>> + if (err < 0)
>> + return err;
>> + if (!early_console_dev.con->write)
>> + return -ENODEV;
>> +
>> + register_console(early_console_dev.con);
>> + return 0;
>> +}
>> +
>> +#endif /* CONFIG_ACPI_DBG2_TABLE */
>> diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
>> index 125ae7e..b653752 100644
>> --- a/include/linux/acpi_dbg2.h
>> +++ b/include/linux/acpi_dbg2.h
>> @@ -37,12 +37,32 @@ int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
>> ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
>> acpi_dbg2_setup, &__acpi_dbg2_data_##name)
>>
>> +int acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d);
>> +
>> +/**
>> + * ACPI_DBG2_EARLYCON_DECLARE() - Define handler for ACPI GDB2 serial port
>> + * @name: Identifier to compose name of table data
>> + * @subtype: Subtype of the port
>> + * @console_setup: Function to be called to setup the port
>> + *
>> + * Type of the console_setup() callback is
>> + * int (*setup)(struct earlycon_device *, const char *)
>> + * It's the type of callback of of_setup_earlycon().
>> + */
>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>> + ACPI_DBG2_DECLARE(name, ACPI_DBG2_SERIAL_PORT, subtype, \
>> + acpi_setup_earlycon, console_setup)
>> +
>> #else
>>
>> #define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>> static const void *__acpi_dbg_data_##name[] \
>> __used __initdata = { (void *)setup_fn, (void *)data_ptr }
>>
>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>> + static const void *__acpi_dbg_data_serial_##name[] \
>> + __used __initdata = { (void *)console_setup }
>> +
>> #endif
>>
>> #endif
>>
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port
2016-03-01 16:57 ` Aleksey Makarov
@ 2016-03-03 17:48 ` Peter Hurley
2016-03-03 19:33 ` Peter Hurley
2016-03-04 13:03 ` Aleksey Makarov
0 siblings, 2 replies; 32+ messages in thread
From: Peter Hurley @ 2016-03-03 17:48 UTC (permalink / raw)
To: Aleksey Makarov, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Jonathan Corbet, Jiri Slaby, linux-doc
On 03/01/2016 08:57 AM, Aleksey Makarov wrote:
>
>
> On 03/01/2016 06:53 PM, Peter Hurley wrote:
>> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>>> Add ACPI_DBG2_EARLYCON_DECLARE() macros that declares
>>> an earlycon on the serial port specified in the DBG2 ACPI table.
>>>
>>> Pass the string "earlycon=acpi_dbg2" to the kernel to activate it.
>>>
>>> Callbacks for EARLYCON_DECLARE() and OF_EARLYCON_DECLARE()
>>> can also be used for this macros.
>>>
>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>> ---
>>> Documentation/kernel-parameters.txt | 3 ++
>>> drivers/tty/serial/earlycon.c | 60 +++++++++++++++++++++++++++++++++++++
>>> include/linux/acpi_dbg2.h | 20 +++++++++++++
>>> 3 files changed, 83 insertions(+)
>>>
>>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>>> index e0a21e4..19b947b 100644
>>> --- a/Documentation/kernel-parameters.txt
>>> +++ b/Documentation/kernel-parameters.txt
>>> @@ -1072,6 +1072,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>> A valid base address must be provided, and the serial
>>> port must already be setup and configured.
>>>
>>> + acpi_dbg2
>>> + Use serial port specified by the DBG2 ACPI table.
>>> +
>>> earlyprintk= [X86,SH,BLACKFIN,ARM,M68k]
>>> earlyprintk=vga
>>> earlyprintk=efi
>>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>>> index d217366..9ba3a04 100644
>>> --- a/drivers/tty/serial/earlycon.c
>>> +++ b/drivers/tty/serial/earlycon.c
>>> @@ -22,6 +22,7 @@
>>> #include <linux/sizes.h>
>>> #include <linux/of.h>
>>> #include <linux/of_fdt.h>
>>> +#include <linux/acpi.h>
>>>
>>> #ifdef CONFIG_FIX_EARLYCON_MEM
>>> #include <asm/fixmap.h>
>>> @@ -200,6 +201,8 @@ int __init setup_earlycon(char *buf)
>>> return -ENOENT;
>>> }
>>>
>>> +static bool setup_dbg2_earlycon;
>>> +
>>> /* early_param wrapper for setup_earlycon() */
>>> static int __init param_setup_earlycon(char *buf)
>>> {
>>> @@ -212,6 +215,11 @@ static int __init param_setup_earlycon(char *buf)
>>> if (!buf || !buf[0])
>>> return early_init_dt_scan_chosen_serial();
>>>
>>> + if (!strcmp(buf, "acpi_dbg2")) {
>>> + setup_dbg2_earlycon = true;
>>> + return 0;
>>> + }
>>
>> So this series doesn't start an ACPI earlycon at early_param time?
>> That doesn't seem very useful.
>>
>> When does the ACPI earlycon actually start?
>> And don't say "when the DBG2 table is probed"; that much is obvious.
>
> ACPI earlycon starts as soon as ACPI tables become accessible (setup_arch()).
> I think that is still quite early.
I see now; the probe is in patch 6/7.
setup_arch()
acpi_boot_table_init()
acpi_probe_device_table()
...
acpi_dbg2_setup()
->setup()
acpi_setup_earlycon()
>>> +
>>> err = setup_earlycon(buf);
>>> if (err == -ENOENT || err == -EALREADY)
>>> return 0;
>>> @@ -286,3 +294,55 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
>>> }
>>>
>>> #endif /* CONFIG_OF_EARLY_FLATTREE */
>>> +
>>> +#ifdef CONFIG_ACPI_DBG2_TABLE
>>> +
>>> +int __init acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d)
>>> +{
>>> + int err;
>>> + struct uart_port *port = &early_console_dev.port;
>>> + int (*setup)(struct earlycon_device *, const char *) = d;
>>> + struct acpi_generic_address *reg;
>>> +
>>> + if (!setup_dbg2_earlycon)
>>> + return -ENODEV;
>>> +
>>> + if (device->register_count < 1)
>>> + return -ENODEV;
>>> +
>>> + if (device->base_address_offset >= device->length)
>>> + return -EINVAL;
>>> +
>>> + reg = (void *)device + device->base_address_offset;
>>> +
>>> + if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
>>> + reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO)
>>> + return -EINVAL;
>>> +
>>> + spin_lock_init(&port->lock);
>>> + port->uartclk = BASE_BAUD * 16;
>>> +
>>> + if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
>>> + if (device->port_type == ACPI_DBG2_ARM_SBSA_32BIT)
>>> + port->iotype = UPIO_MEM32;
>>> + else
>>> + port->iotype = UPIO_MEM;
>>> + port->mapbase = reg->address;
>>> + port->membase = earlycon_map(reg->address, SZ_4K);
>>> + } else {
>>> + port->iotype = UPIO_PORT;
>>> + port->iobase = reg->address;
>>> + }
>>> +
>>> + early_console_dev.con->data = &early_console_dev;
>>> + err = setup(&early_console_dev, NULL);
>>> + if (err < 0)
>>> + return err;
>>> + if (!early_console_dev.con->write)
>>> + return -ENODEV;
>>> +
>>> + register_console(early_console_dev.con);
>>> + return 0;
>>> +}
>>> +
>>> +#endif /* CONFIG_ACPI_DBG2_TABLE */
>>> diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
>>> index 125ae7e..b653752 100644
>>> --- a/include/linux/acpi_dbg2.h
>>> +++ b/include/linux/acpi_dbg2.h
>>> @@ -37,12 +37,32 @@ int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
>>> ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
>>> acpi_dbg2_setup, &__acpi_dbg2_data_##name)
>>>
>>> +int acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d);
>>> +
>>> +/**
>>> + * ACPI_DBG2_EARLYCON_DECLARE() - Define handler for ACPI GDB2 serial port
>>> + * @name: Identifier to compose name of table data
>>> + * @subtype: Subtype of the port
>>> + * @console_setup: Function to be called to setup the port
>>> + *
>>> + * Type of the console_setup() callback is
>>> + * int (*setup)(struct earlycon_device *, const char *)
>>> + * It's the type of callback of of_setup_earlycon().
>>> + */
>>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>>> + ACPI_DBG2_DECLARE(name, ACPI_DBG2_SERIAL_PORT, subtype, \
>>> + acpi_setup_earlycon, console_setup)
>>> +
>>> #else
>>>
>>> #define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>>> static const void *__acpi_dbg_data_##name[] \
>>> __used __initdata = { (void *)setup_fn, (void *)data_ptr }
>>>
>>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>>> + static const void *__acpi_dbg_data_serial_##name[] \
>>> + __used __initdata = { (void *)console_setup }
console_setup is a terrible macro argument name; console_setup() is an
actual kernel function (although file-scope).
Please change it to something short and generic.
Honestly, I'd just prefer you skip all this apparatus that makes
ACPI earlycon appear to be like OF earlycon code-wise, but without any of
the real underpinning or flexibility.
This would be trivial to parse the ACPI table and invoke
setup_earlycon() with a string specifier instead.
For example,
int __init acpi_earlycon_setup(struct acpi_dbg2_device *dbg2)
{
char opts[64];
struct acpi_generic_addr *addr = (void*)dbg2 + dbg2->base_address_offset;
int mmio = addr->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY;
if (dbg2->port_type != ACPI_DBG2_SERIAL_PORT)
return 0;
switch (dbg2->port_subtype) {
case ACPI_DBG2_ARM_PL011:
case ACPI_DBG2_ARM_SBSA_GENERIC:
case ACPI_DBG2_BCM2835:
sprintf(opts, "pl011,%s,0x%llx", mmio, addr->address);
break;
case ACPI_DBG2_ARM_SBSA_32BIT:
sprintf(opts, "pl011,mmio32,0x%llx", addr->address);
break;
case ACPI_DBG2_16550_COMPATIBLE:
case ACPI_DBG2_16550_SUBSET:
sprintf(opts, "uart,%s,0x%llx", mmio, addr->address);
break;
default:
return 0;
}
return setup_earlycon(opts);
}
This supports every earlycon ACPI DBG2 declares, not just the ARM_PL011
subtype of your series.
>>> +
>>> #endif
>>>
>>> #endif
>>>
>>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port
2016-03-03 17:48 ` Peter Hurley
@ 2016-03-03 19:33 ` Peter Hurley
2016-03-04 13:03 ` Aleksey Makarov
1 sibling, 0 replies; 32+ messages in thread
From: Peter Hurley @ 2016-03-03 19:33 UTC (permalink / raw)
To: Aleksey Makarov, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Jonathan Corbet, Jiri Slaby, linux-doc
On 03/03/2016 09:48 AM, Peter Hurley wrote:
> On 03/01/2016 08:57 AM, Aleksey Makarov wrote:
>>
>>
>> On 03/01/2016 06:53 PM, Peter Hurley wrote:
>>> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>>>> Add ACPI_DBG2_EARLYCON_DECLARE() macros that declares
>>>> an earlycon on the serial port specified in the DBG2 ACPI table.
>>>>
>>>> Pass the string "earlycon=acpi_dbg2" to the kernel to activate it.
>>>>
>>>> Callbacks for EARLYCON_DECLARE() and OF_EARLYCON_DECLARE()
>>>> can also be used for this macros.
>>>>
>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>>> ---
>>>> Documentation/kernel-parameters.txt | 3 ++
>>>> drivers/tty/serial/earlycon.c | 60 +++++++++++++++++++++++++++++++++++++
>>>> include/linux/acpi_dbg2.h | 20 +++++++++++++
>>>> 3 files changed, 83 insertions(+)
>>>>
>>>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>>>> index e0a21e4..19b947b 100644
>>>> --- a/Documentation/kernel-parameters.txt
>>>> +++ b/Documentation/kernel-parameters.txt
>>>> @@ -1072,6 +1072,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>>> A valid base address must be provided, and the serial
>>>> port must already be setup and configured.
>>>>
>>>> + acpi_dbg2
>>>> + Use serial port specified by the DBG2 ACPI table.
>>>> +
>>>> earlyprintk= [X86,SH,BLACKFIN,ARM,M68k]
>>>> earlyprintk=vga
>>>> earlyprintk=efi
>>>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>>>> index d217366..9ba3a04 100644
>>>> --- a/drivers/tty/serial/earlycon.c
>>>> +++ b/drivers/tty/serial/earlycon.c
>>>> @@ -22,6 +22,7 @@
>>>> #include <linux/sizes.h>
>>>> #include <linux/of.h>
>>>> #include <linux/of_fdt.h>
>>>> +#include <linux/acpi.h>
>>>>
>>>> #ifdef CONFIG_FIX_EARLYCON_MEM
>>>> #include <asm/fixmap.h>
>>>> @@ -200,6 +201,8 @@ int __init setup_earlycon(char *buf)
>>>> return -ENOENT;
>>>> }
>>>>
>>>> +static bool setup_dbg2_earlycon;
>>>> +
>>>> /* early_param wrapper for setup_earlycon() */
>>>> static int __init param_setup_earlycon(char *buf)
>>>> {
>>>> @@ -212,6 +215,11 @@ static int __init param_setup_earlycon(char *buf)
>>>> if (!buf || !buf[0])
>>>> return early_init_dt_scan_chosen_serial();
>>>>
>>>> + if (!strcmp(buf, "acpi_dbg2")) {
>>>> + setup_dbg2_earlycon = true;
>>>> + return 0;
>>>> + }
>>>
>>> So this series doesn't start an ACPI earlycon at early_param time?
>>> That doesn't seem very useful.
>>>
>>> When does the ACPI earlycon actually start?
>>> And don't say "when the DBG2 table is probed"; that much is obvious.
>>
>> ACPI earlycon starts as soon as ACPI tables become accessible (setup_arch()).
>> I think that is still quite early.
>
> I see now; the probe is in patch 6/7.
>
> setup_arch()
> acpi_boot_table_init()
> acpi_probe_device_table()
> ...
> acpi_dbg2_setup()
> ->setup()
> acpi_setup_earlycon()
>
>
>>>> +
>>>> err = setup_earlycon(buf);
>>>> if (err == -ENOENT || err == -EALREADY)
>>>> return 0;
>>>> @@ -286,3 +294,55 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
>>>> }
>>>>
>>>> #endif /* CONFIG_OF_EARLY_FLATTREE */
>>>> +
>>>> +#ifdef CONFIG_ACPI_DBG2_TABLE
>>>> +
>>>> +int __init acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d)
>>>> +{
>>>> + int err;
>>>> + struct uart_port *port = &early_console_dev.port;
>>>> + int (*setup)(struct earlycon_device *, const char *) = d;
>>>> + struct acpi_generic_address *reg;
>>>> +
>>>> + if (!setup_dbg2_earlycon)
>>>> + return -ENODEV;
>>>> +
>>>> + if (device->register_count < 1)
>>>> + return -ENODEV;
>>>> +
>>>> + if (device->base_address_offset >= device->length)
>>>> + return -EINVAL;
>>>> +
>>>> + reg = (void *)device + device->base_address_offset;
>>>> +
>>>> + if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
>>>> + reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO)
>>>> + return -EINVAL;
>>>> +
>>>> + spin_lock_init(&port->lock);
>>>> + port->uartclk = BASE_BAUD * 16;
>>>> +
>>>> + if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
>>>> + if (device->port_type == ACPI_DBG2_ARM_SBSA_32BIT)
>>>> + port->iotype = UPIO_MEM32;
>>>> + else
>>>> + port->iotype = UPIO_MEM;
>>>> + port->mapbase = reg->address;
>>>> + port->membase = earlycon_map(reg->address, SZ_4K);
>>>> + } else {
>>>> + port->iotype = UPIO_PORT;
>>>> + port->iobase = reg->address;
>>>> + }
>>>> +
>>>> + early_console_dev.con->data = &early_console_dev;
>>>> + err = setup(&early_console_dev, NULL);
>>>> + if (err < 0)
>>>> + return err;
>>>> + if (!early_console_dev.con->write)
>>>> + return -ENODEV;
>>>> +
>>>> + register_console(early_console_dev.con);
>>>> + return 0;
>>>> +}
>>>> +
>>>> +#endif /* CONFIG_ACPI_DBG2_TABLE */
>>>> diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
>>>> index 125ae7e..b653752 100644
>>>> --- a/include/linux/acpi_dbg2.h
>>>> +++ b/include/linux/acpi_dbg2.h
>>>> @@ -37,12 +37,32 @@ int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
>>>> ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
>>>> acpi_dbg2_setup, &__acpi_dbg2_data_##name)
>>>>
>>>> +int acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d);
>>>> +
>>>> +/**
>>>> + * ACPI_DBG2_EARLYCON_DECLARE() - Define handler for ACPI GDB2 serial port
>>>> + * @name: Identifier to compose name of table data
>>>> + * @subtype: Subtype of the port
>>>> + * @console_setup: Function to be called to setup the port
>>>> + *
>>>> + * Type of the console_setup() callback is
>>>> + * int (*setup)(struct earlycon_device *, const char *)
>>>> + * It's the type of callback of of_setup_earlycon().
>>>> + */
>>>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>>>> + ACPI_DBG2_DECLARE(name, ACPI_DBG2_SERIAL_PORT, subtype, \
>>>> + acpi_setup_earlycon, console_setup)
>>>> +
>>>> #else
>>>>
>>>> #define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>>>> static const void *__acpi_dbg_data_##name[] \
>>>> __used __initdata = { (void *)setup_fn, (void *)data_ptr }
>>>>
>>>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>>>> + static const void *__acpi_dbg_data_serial_##name[] \
>>>> + __used __initdata = { (void *)console_setup }
>
> console_setup is a terrible macro argument name; console_setup() is an
> actual kernel function (although file-scope).
> Please change it to something short and generic.
>
> Honestly, I'd just prefer you skip all this apparatus that makes
> ACPI earlycon appear to be like OF earlycon code-wise, but without any of
> the real underpinning or flexibility.
>
> This would be trivial to parse the ACPI table and invoke
> setup_earlycon() with a string specifier instead.
>
> For example,
>
> int __init acpi_earlycon_setup(struct acpi_dbg2_device *dbg2)
> {
> char opts[64];
> struct acpi_generic_addr *addr = (void*)dbg2 + dbg2->base_address_offset;
> int mmio = addr->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY;
This should be
char *mmio = (addr->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) ? "mmio" : "io";
>
> if (dbg2->port_type != ACPI_DBG2_SERIAL_PORT)
> return 0;
>
> switch (dbg2->port_subtype) {
> case ACPI_DBG2_ARM_PL011:
> case ACPI_DBG2_ARM_SBSA_GENERIC:
> case ACPI_DBG2_BCM2835:
> sprintf(opts, "pl011,%s,0x%llx", mmio, addr->address);
> break;
> case ACPI_DBG2_ARM_SBSA_32BIT:
> sprintf(opts, "pl011,mmio32,0x%llx", addr->address);
> break;
> case ACPI_DBG2_16550_COMPATIBLE:
> case ACPI_DBG2_16550_SUBSET:
> sprintf(opts, "uart,%s,0x%llx", mmio, addr->address);
> break;
> default:
> return 0;
> }
>
> return setup_earlycon(opts);
> }
>
> This supports every earlycon ACPI DBG2 declares, not just the ARM_PL011
> subtype of your series.
>
>
>
>>>> +
>>>> #endif
>>>>
>>>> #endif
>>>>
>>>
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port
2016-03-03 17:48 ` Peter Hurley
2016-03-03 19:33 ` Peter Hurley
@ 2016-03-04 13:03 ` Aleksey Makarov
2016-03-04 15:40 ` Peter Hurley
2016-03-04 15:52 ` Peter Hurley
1 sibling, 2 replies; 32+ messages in thread
From: Aleksey Makarov @ 2016-03-04 13:03 UTC (permalink / raw)
To: Peter Hurley, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Jonathan Corbet, Jiri Slaby, linux-doc
On 03/03/2016 08:48 PM, Peter Hurley wrote:
> On 03/01/2016 08:57 AM, Aleksey Makarov wrote:
>>
>>
>> On 03/01/2016 06:53 PM, Peter Hurley wrote:
>>> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>>>> Add ACPI_DBG2_EARLYCON_DECLARE() macros that declares
>>>> an earlycon on the serial port specified in the DBG2 ACPI table.
>>>>
>>>> Pass the string "earlycon=acpi_dbg2" to the kernel to activate it.
>>>>
>>>> Callbacks for EARLYCON_DECLARE() and OF_EARLYCON_DECLARE()
>>>> can also be used for this macros.
>>>>
>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>>> ---
>>>> Documentation/kernel-parameters.txt | 3 ++
>>>> drivers/tty/serial/earlycon.c | 60 +++++++++++++++++++++++++++++++++++++
>>>> include/linux/acpi_dbg2.h | 20 +++++++++++++
>>>> 3 files changed, 83 insertions(+)
>>>>
>>>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>>>> index e0a21e4..19b947b 100644
>>>> --- a/Documentation/kernel-parameters.txt
>>>> +++ b/Documentation/kernel-parameters.txt
>>>> @@ -1072,6 +1072,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>>> A valid base address must be provided, and the serial
>>>> port must already be setup and configured.
>>>>
>>>> + acpi_dbg2
>>>> + Use serial port specified by the DBG2 ACPI table.
>>>> +
>>>> earlyprintk= [X86,SH,BLACKFIN,ARM,M68k]
>>>> earlyprintk=vga
>>>> earlyprintk=efi
>>>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>>>> index d217366..9ba3a04 100644
>>>> --- a/drivers/tty/serial/earlycon.c
>>>> +++ b/drivers/tty/serial/earlycon.c
>>>> @@ -22,6 +22,7 @@
>>>> #include <linux/sizes.h>
>>>> #include <linux/of.h>
>>>> #include <linux/of_fdt.h>
>>>> +#include <linux/acpi.h>
>>>>
>>>> #ifdef CONFIG_FIX_EARLYCON_MEM
>>>> #include <asm/fixmap.h>
>>>> @@ -200,6 +201,8 @@ int __init setup_earlycon(char *buf)
>>>> return -ENOENT;
>>>> }
>>>>
>>>> +static bool setup_dbg2_earlycon;
>>>> +
>>>> /* early_param wrapper for setup_earlycon() */
>>>> static int __init param_setup_earlycon(char *buf)
>>>> {
>>>> @@ -212,6 +215,11 @@ static int __init param_setup_earlycon(char *buf)
>>>> if (!buf || !buf[0])
>>>> return early_init_dt_scan_chosen_serial();
>>>>
>>>> + if (!strcmp(buf, "acpi_dbg2")) {
>>>> + setup_dbg2_earlycon = true;
>>>> + return 0;
>>>> + }
>>>
>>> So this series doesn't start an ACPI earlycon at early_param time?
>>> That doesn't seem very useful.
>>>
>>> When does the ACPI earlycon actually start?
>>> And don't say "when the DBG2 table is probed"; that much is obvious.
>>
>> ACPI earlycon starts as soon as ACPI tables become accessible (setup_arch()).
>> I think that is still quite early.
>
> I see now; the probe is in patch 6/7.
>
> setup_arch()
> acpi_boot_table_init()
> acpi_probe_device_table()
> ...
> acpi_dbg2_setup()
> ->setup()
> acpi_setup_earlycon()
>
>
>>>> +
>>>> err = setup_earlycon(buf);
>>>> if (err == -ENOENT || err == -EALREADY)
>>>> return 0;
>>>> @@ -286,3 +294,55 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
>>>> }
>>>>
>>>> #endif /* CONFIG_OF_EARLY_FLATTREE */
>>>> +
>>>> +#ifdef CONFIG_ACPI_DBG2_TABLE
>>>> +
>>>> +int __init acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d)
>>>> +{
>>>> + int err;
>>>> + struct uart_port *port = &early_console_dev.port;
>>>> + int (*setup)(struct earlycon_device *, const char *) = d;
>>>> + struct acpi_generic_address *reg;
>>>> +
>>>> + if (!setup_dbg2_earlycon)
>>>> + return -ENODEV;
>>>> +
>>>> + if (device->register_count < 1)
>>>> + return -ENODEV;
>>>> +
>>>> + if (device->base_address_offset >= device->length)
>>>> + return -EINVAL;
>>>> +
>>>> + reg = (void *)device + device->base_address_offset;
>>>> +
>>>> + if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
>>>> + reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO)
>>>> + return -EINVAL;
>>>> +
>>>> + spin_lock_init(&port->lock);
>>>> + port->uartclk = BASE_BAUD * 16;
>>>> +
>>>> + if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
>>>> + if (device->port_type == ACPI_DBG2_ARM_SBSA_32BIT)
>>>> + port->iotype = UPIO_MEM32;
>>>> + else
>>>> + port->iotype = UPIO_MEM;
>>>> + port->mapbase = reg->address;
>>>> + port->membase = earlycon_map(reg->address, SZ_4K);
>>>> + } else {
>>>> + port->iotype = UPIO_PORT;
>>>> + port->iobase = reg->address;
>>>> + }
>>>> +
>>>> + early_console_dev.con->data = &early_console_dev;
>>>> + err = setup(&early_console_dev, NULL);
>>>> + if (err < 0)
>>>> + return err;
>>>> + if (!early_console_dev.con->write)
>>>> + return -ENODEV;
>>>> +
>>>> + register_console(early_console_dev.con);
>>>> + return 0;
>>>> +}
>>>> +
>>>> +#endif /* CONFIG_ACPI_DBG2_TABLE */
>>>> diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
>>>> index 125ae7e..b653752 100644
>>>> --- a/include/linux/acpi_dbg2.h
>>>> +++ b/include/linux/acpi_dbg2.h
>>>> @@ -37,12 +37,32 @@ int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
>>>> ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
>>>> acpi_dbg2_setup, &__acpi_dbg2_data_##name)
>>>>
>>>> +int acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d);
>>>> +
>>>> +/**
>>>> + * ACPI_DBG2_EARLYCON_DECLARE() - Define handler for ACPI GDB2 serial port
>>>> + * @name: Identifier to compose name of table data
>>>> + * @subtype: Subtype of the port
>>>> + * @console_setup: Function to be called to setup the port
>>>> + *
>>>> + * Type of the console_setup() callback is
>>>> + * int (*setup)(struct earlycon_device *, const char *)
>>>> + * It's the type of callback of of_setup_earlycon().
>>>> + */
>>>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>>>> + ACPI_DBG2_DECLARE(name, ACPI_DBG2_SERIAL_PORT, subtype, \
>>>> + acpi_setup_earlycon, console_setup)
>>>> +
>>>> #else
>>>>
>>>> #define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>>>> static const void *__acpi_dbg_data_##name[] \
>>>> __used __initdata = { (void *)setup_fn, (void *)data_ptr }
>>>>
>>>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>>>> + static const void *__acpi_dbg_data_serial_##name[] \
>>>> + __used __initdata = { (void *)console_setup }
>
> console_setup is a terrible macro argument name; console_setup() is an
> actual kernel function (although file-scope).
> Please change it to something short and generic.
Is 'setup_fn' ok?
> Honestly, I'd just prefer you skip all this apparatus that makes
> ACPI earlycon appear to be like OF earlycon code-wise, but without any of
> the real underpinning or flexibility.
Actually it was Mark Salter who asked to introduce such macros.
https://lkml.kernel.org/g/1441730339.5459.8.camel@redhat.com
I think reusing the OF functions is a good decision.
Your "but without any of the real underpinning or flexibility" is unfounded.
> This would be trivial to parse the ACPI table and invoke
> setup_earlycon() with a string specifier instead.
>
> For example,
>
> int __init acpi_earlycon_setup(struct acpi_dbg2_device *dbg2)
> {
> char opts[64];
> struct acpi_generic_addr *addr = (void*)dbg2 + dbg2->base_address_offset;
> int mmio = addr->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY;
>
> if (dbg2->port_type != ACPI_DBG2_SERIAL_PORT)
> return 0;
>
> switch (dbg2->port_subtype) {
> case ACPI_DBG2_ARM_PL011:
> case ACPI_DBG2_ARM_SBSA_GENERIC:
> case ACPI_DBG2_BCM2835:
> sprintf(opts, "pl011,%s,0x%llx", mmio, addr->address);
> break;
> case ACPI_DBG2_ARM_SBSA_32BIT:
> sprintf(opts, "pl011,mmio32,0x%llx", addr->address);
> break;
> case ACPI_DBG2_16550_COMPATIBLE:
> case ACPI_DBG2_16550_SUBSET:
> sprintf(opts, "uart,%s,0x%llx", mmio, addr->address);
> break;
> default:
> return 0;
> }
>
> return setup_earlycon(opts);
> }
- Note that this decision forces setting earlycon on GDB2 debug port.
DBG2 does not specify that it should be exactly earlycon.
- You missed ACPI_DBG2_ARM_DCC. And actually I think the list of
debug ports is open. You will have to make up names like "uart" "pl011"
each time a new port is introduced into the specs.
- Most important thing, this way you disclose the internals of serial ports
to the generic earlycon.c Such info as access mode should stay
in the respective drivers.
- I would not like printing address and then parsing it back.
> This supports every earlycon ACPI DBG2 declares, not just the ARM_PL011
> subtype of your series.
To support earlycon on other types of debug port just add literally one
string of code (as in pl011).
>
>
>
>>>> +
>>>> #endif
>>>>
>>>> #endif
>>>>
>>>
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port
2016-03-04 13:03 ` Aleksey Makarov
@ 2016-03-04 15:40 ` Peter Hurley
2016-03-04 15:52 ` Peter Hurley
1 sibling, 0 replies; 32+ messages in thread
From: Peter Hurley @ 2016-03-04 15:40 UTC (permalink / raw)
To: Aleksey Makarov, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Jonathan Corbet, Jiri Slaby, linux-doc
On 03/04/2016 05:03 AM, Aleksey Makarov wrote:
>
>
> On 03/03/2016 08:48 PM, Peter Hurley wrote:
>> On 03/01/2016 08:57 AM, Aleksey Makarov wrote:
>>>
>>>
>>> On 03/01/2016 06:53 PM, Peter Hurley wrote:
>>>> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>>>>> Add ACPI_DBG2_EARLYCON_DECLARE() macros that declares
>>>>> an earlycon on the serial port specified in the DBG2 ACPI table.
>>>>>
>>>>> Pass the string "earlycon=acpi_dbg2" to the kernel to activate it.
>>>>>
>>>>> Callbacks for EARLYCON_DECLARE() and OF_EARLYCON_DECLARE()
>>>>> can also be used for this macros.
>>>>>
>>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>>>> ---
>>>>> Documentation/kernel-parameters.txt | 3 ++
>>>>> drivers/tty/serial/earlycon.c | 60 +++++++++++++++++++++++++++++++++++++
>>>>> include/linux/acpi_dbg2.h | 20 +++++++++++++
>>>>> 3 files changed, 83 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>>>>> index e0a21e4..19b947b 100644
>>>>> --- a/Documentation/kernel-parameters.txt
>>>>> +++ b/Documentation/kernel-parameters.txt
>>>>> @@ -1072,6 +1072,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>>>> A valid base address must be provided, and the serial
>>>>> port must already be setup and configured.
>>>>>
>>>>> + acpi_dbg2
>>>>> + Use serial port specified by the DBG2 ACPI table.
>>>>> +
>>>>> earlyprintk= [X86,SH,BLACKFIN,ARM,M68k]
>>>>> earlyprintk=vga
>>>>> earlyprintk=efi
>>>>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>>>>> index d217366..9ba3a04 100644
>>>>> --- a/drivers/tty/serial/earlycon.c
>>>>> +++ b/drivers/tty/serial/earlycon.c
>>>>> @@ -22,6 +22,7 @@
>>>>> #include <linux/sizes.h>
>>>>> #include <linux/of.h>
>>>>> #include <linux/of_fdt.h>
>>>>> +#include <linux/acpi.h>
>>>>>
>>>>> #ifdef CONFIG_FIX_EARLYCON_MEM
>>>>> #include <asm/fixmap.h>
>>>>> @@ -200,6 +201,8 @@ int __init setup_earlycon(char *buf)
>>>>> return -ENOENT;
>>>>> }
>>>>>
>>>>> +static bool setup_dbg2_earlycon;
>>>>> +
>>>>> /* early_param wrapper for setup_earlycon() */
>>>>> static int __init param_setup_earlycon(char *buf)
>>>>> {
>>>>> @@ -212,6 +215,11 @@ static int __init param_setup_earlycon(char *buf)
>>>>> if (!buf || !buf[0])
>>>>> return early_init_dt_scan_chosen_serial();
>>>>>
>>>>> + if (!strcmp(buf, "acpi_dbg2")) {
>>>>> + setup_dbg2_earlycon = true;
>>>>> + return 0;
>>>>> + }
>>>>
>>>> So this series doesn't start an ACPI earlycon at early_param time?
>>>> That doesn't seem very useful.
>>>>
>>>> When does the ACPI earlycon actually start?
>>>> And don't say "when the DBG2 table is probed"; that much is obvious.
>>>
>>> ACPI earlycon starts as soon as ACPI tables become accessible (setup_arch()).
>>> I think that is still quite early.
>>
>> I see now; the probe is in patch 6/7.
>>
>> setup_arch()
>> acpi_boot_table_init()
>> acpi_probe_device_table()
>> ...
>> acpi_dbg2_setup()
>> ->setup()
>> acpi_setup_earlycon()
>>
>>
>>>>> +
>>>>> err = setup_earlycon(buf);
>>>>> if (err == -ENOENT || err == -EALREADY)
>>>>> return 0;
>>>>> @@ -286,3 +294,55 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
>>>>> }
>>>>>
>>>>> #endif /* CONFIG_OF_EARLY_FLATTREE */
>>>>> +
>>>>> +#ifdef CONFIG_ACPI_DBG2_TABLE
>>>>> +
>>>>> +int __init acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d)
>>>>> +{
>>>>> + int err;
>>>>> + struct uart_port *port = &early_console_dev.port;
>>>>> + int (*setup)(struct earlycon_device *, const char *) = d;
>>>>> + struct acpi_generic_address *reg;
>>>>> +
>>>>> + if (!setup_dbg2_earlycon)
>>>>> + return -ENODEV;
>>>>> +
>>>>> + if (device->register_count < 1)
>>>>> + return -ENODEV;
>>>>> +
>>>>> + if (device->base_address_offset >= device->length)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + reg = (void *)device + device->base_address_offset;
>>>>> +
>>>>> + if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
>>>>> + reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + spin_lock_init(&port->lock);
>>>>> + port->uartclk = BASE_BAUD * 16;
>>>>> +
>>>>> + if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
>>>>> + if (device->port_type == ACPI_DBG2_ARM_SBSA_32BIT)
>>>>> + port->iotype = UPIO_MEM32;
>>>>> + else
>>>>> + port->iotype = UPIO_MEM;
>>>>> + port->mapbase = reg->address;
>>>>> + port->membase = earlycon_map(reg->address, SZ_4K);
>>>>> + } else {
>>>>> + port->iotype = UPIO_PORT;
>>>>> + port->iobase = reg->address;
>>>>> + }
>>>>> +
>>>>> + early_console_dev.con->data = &early_console_dev;
>>>>> + err = setup(&early_console_dev, NULL);
>>>>> + if (err < 0)
>>>>> + return err;
>>>>> + if (!early_console_dev.con->write)
>>>>> + return -ENODEV;
>>>>> +
>>>>> + register_console(early_console_dev.con);
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +#endif /* CONFIG_ACPI_DBG2_TABLE */
>>>>> diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
>>>>> index 125ae7e..b653752 100644
>>>>> --- a/include/linux/acpi_dbg2.h
>>>>> +++ b/include/linux/acpi_dbg2.h
>>>>> @@ -37,12 +37,32 @@ int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
>>>>> ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
>>>>> acpi_dbg2_setup, &__acpi_dbg2_data_##name)
>>>>>
>>>>> +int acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d);
>>>>> +
>>>>> +/**
>>>>> + * ACPI_DBG2_EARLYCON_DECLARE() - Define handler for ACPI GDB2 serial port
>>>>> + * @name: Identifier to compose name of table data
>>>>> + * @subtype: Subtype of the port
>>>>> + * @console_setup: Function to be called to setup the port
>>>>> + *
>>>>> + * Type of the console_setup() callback is
>>>>> + * int (*setup)(struct earlycon_device *, const char *)
>>>>> + * It's the type of callback of of_setup_earlycon().
>>>>> + */
>>>>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>>>>> + ACPI_DBG2_DECLARE(name, ACPI_DBG2_SERIAL_PORT, subtype, \
>>>>> + acpi_setup_earlycon, console_setup)
>>>>> +
>>>>> #else
>>>>>
>>>>> #define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
>>>>> static const void *__acpi_dbg_data_##name[] \
>>>>> __used __initdata = { (void *)setup_fn, (void *)data_ptr }
>>>>>
>>>>> +#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
>>>>> + static const void *__acpi_dbg_data_serial_##name[] \
>>>>> + __used __initdata = { (void *)console_setup }
>>
>> console_setup is a terrible macro argument name; console_setup() is an
>> actual kernel function (although file-scope).
>> Please change it to something short and generic.
>
> Is 'setup_fn' ok?
>
>> Honestly, I'd just prefer you skip all this apparatus that makes
>> ACPI earlycon appear to be like OF earlycon code-wise, but without any of
>> the real underpinning or flexibility.
>
> Actually it was Mark Salter who asked to introduce such macros.
>
> https://lkml.kernel.org/g/1441730339.5459.8.camel@redhat.com
>
> I think reusing the OF functions is a good decision.
>
> Your "but without any of the real underpinning or flexibility" is unfounded.
1. Lack of real underpinning.
Can't start an earlycon at early_param time to debug arch issues.
As a result, everyone will continue using command line for earlycon which
makes this series useless.
2. Lack of flexibility.
OF earlycon supports any new hardware simply by string matching w/o
requiring any approvals. I can add earlycon support for anything in
10 minutes.
ACPI earlycon for any new hardware requires approvals and spec changes.
2-3 months.
Founded.
>> This would be trivial to parse the ACPI table and invoke
>> setup_earlycon() with a string specifier instead.
>>
>> For example,
>>
>> int __init acpi_earlycon_setup(struct acpi_dbg2_device *dbg2)
>> {
>> char opts[64];
>> struct acpi_generic_addr *addr = (void*)dbg2 + dbg2->base_address_offset;
>> int mmio = addr->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY;
>>
>> if (dbg2->port_type != ACPI_DBG2_SERIAL_PORT)
>> return 0;
>>
>> switch (dbg2->port_subtype) {
>> case ACPI_DBG2_ARM_PL011:
>> case ACPI_DBG2_ARM_SBSA_GENERIC:
>> case ACPI_DBG2_BCM2835:
>> sprintf(opts, "pl011,%s,0x%llx", mmio, addr->address);
>> break;
>> case ACPI_DBG2_ARM_SBSA_32BIT:
>> sprintf(opts, "pl011,mmio32,0x%llx", addr->address);
>> break;
>> case ACPI_DBG2_16550_COMPATIBLE:
>> case ACPI_DBG2_16550_SUBSET:
>> sprintf(opts, "uart,%s,0x%llx", mmio, addr->address);
>> break;
>> default:
>> return 0;
>> }
>>
>> return setup_earlycon(opts);
>> }
>
> - Note that this decision forces setting earlycon on GDB2 debug port.
> DBG2 does not specify that it should be exactly earlycon.
Obviously my example was simplified to point out how easy it is
to start an earlycon with a string specifier.
I assumed you would understand that
if (!setup_dbg2_earlycon)
return 0;
was omitted for clarity (or handled at a higher level).
> - You missed ACPI_DBG2_ARM_DCC.
What is this?
Your series _only_ adds ACPI_DBG2_ARM_PL011 and none of the others.
If you add ACPI_DBG2_ARM_DCC support to your series, I'll add it
to my example.
> And actually I think the list of
> debug ports is open. You will have to make up names like "uart" "pl011"
> each time a new port is introduced into the specs.
5 new ports have been added in 1 decade. I think we can keep up with
that rate of change.
And you keep going on about "make up names". _For the last time_,
these "make up names" are the documented and defined console names for
earlycons. They will *never* change, because these same string
specifiers are used on the command line.
> - Most important thing, this way you disclose the internals of serial ports
> to the generic earlycon.c Such info as access mode should stay
> in the respective drivers.
??
My example function above doesn't go in "generic earlycon.c"
You leave it in ACPI where it belongs. setup_earlycon() is already global
scope, because like I've said repeatedly, it's already used by firmware to
start earlycons.
And I don't know what you mean by "access mode".
If you're referring to ["io","mmio","mmio32"], this is how earlycon is
specified and has been since the first patch. Besides your own patch decodes
and sets the iotype (UPIO_IO, UPIO_MEM, UPIO_MEM32) so I don't get what
you're objecting to here.
And if anything, the DBG2 table is under-specified.
Such things as endianness, register stride and i/o width are missing.
Not to mention line settings like baud rate, parity, stop bits, and flow
control.
> - I would not like printing address and then parsing it back.
The "parsing it back" is already implemented: that's how command line
earlycon works. That will never change.
And this method is already used by firmware other than OF.
>> This supports every earlycon ACPI DBG2 declares, not just the ARM_PL011
>> subtype of your series.
>
> To support earlycon on other types of debug port just add literally one
> string of code (as in pl011).
And as I've already shown, so does my way.
In 1/2 as much code, without macros or all the ACPI linker table changes.
>>>>> +
>>>>> #endif
>>>>>
>>>>> #endif
>>>>>
>>>>
>>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port
2016-03-04 13:03 ` Aleksey Makarov
2016-03-04 15:40 ` Peter Hurley
@ 2016-03-04 15:52 ` Peter Hurley
1 sibling, 0 replies; 32+ messages in thread
From: Peter Hurley @ 2016-03-04 15:52 UTC (permalink / raw)
To: Aleksey Makarov, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Jonathan Corbet, Jiri Slaby, linux-doc
On 03/04/2016 05:03 AM, Aleksey Makarov wrote:
> Actually it was Mark Salter who asked to introduce such macros.
>
> https://lkml.kernel.org/g/1441730339.5459.8.camel@redhat.com
I wasn't copied on that series, sorry.
> I think reusing the OF functions is a good decision.
But you're not reusing the OF functions; you're duplicating them.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 6/7] ACPI: enable ACPI_DBG2_TABLE on ARM64
2016-02-29 12:41 [PATCH v3 0/7] ACPI: parse the DBG2 table Aleksey Makarov
` (4 preceding siblings ...)
2016-02-29 12:42 ` [PATCH v3 5/7] ACPI: serial: implement earlycon on ACPI DBG2 port Aleksey Makarov
@ 2016-02-29 12:42 ` Aleksey Makarov
2016-03-01 14:52 ` Peter Hurley
2016-02-29 12:42 ` [PATCH v3 7/7] serial: pl011: add ACPI DBG2 serial port Aleksey Makarov
6 siblings, 1 reply; 32+ messages in thread
From: Aleksey Makarov @ 2016-02-29 12:42 UTC (permalink / raw)
To: linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Aleksey Makarov,
Russell King, Greg Kroah-Hartman, Rafael J . Wysocki,
Leif Lindholm, Graeme Gregory, Al Stone, Christopher Covington,
Matthias Brugger, Catalin Marinas, Will Deacon
SBBR mentions DBG2 as a mandatory ACPI table.
So enable it for ARM64
Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/acpi.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 9dc5209..0b614d4 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -3,6 +3,7 @@ config ARM64
select ACPI_CCA_REQUIRED if ACPI
select ACPI_GENERIC_GSI if ACPI
select ACPI_REDUCED_HARDWARE_ONLY if ACPI
+ select ACPI_DBG2_TABLE if ACPI
select ARCH_HAS_DEVMEM_IS_ALLOWED
select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
select ARCH_HAS_ELF_RANDOMIZE
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index d1ce8e2..3370046 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -208,6 +208,8 @@ void __init acpi_boot_table_init(void)
pr_err("Failed to init ACPI tables\n");
if (!param_acpi_force)
disable_acpi();
+ } else {
+ acpi_probe_device_table(dbg2);
}
}
--
2.7.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v3 6/7] ACPI: enable ACPI_DBG2_TABLE on ARM64
2016-02-29 12:42 ` [PATCH v3 6/7] ACPI: enable ACPI_DBG2_TABLE on ARM64 Aleksey Makarov
@ 2016-03-01 14:52 ` Peter Hurley
2016-03-01 17:02 ` Aleksey Makarov
0 siblings, 1 reply; 32+ messages in thread
From: Peter Hurley @ 2016-03-01 14:52 UTC (permalink / raw)
To: Aleksey Makarov, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Catalin Marinas, Will Deacon
On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
> SBBR mentions DBG2 as a mandatory ACPI table.
> So enable it for ARM64
Why does this have to be opted-in per-arch?
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/kernel/acpi.c | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 9dc5209..0b614d4 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -3,6 +3,7 @@ config ARM64
> select ACPI_CCA_REQUIRED if ACPI
> select ACPI_GENERIC_GSI if ACPI
> select ACPI_REDUCED_HARDWARE_ONLY if ACPI
> + select ACPI_DBG2_TABLE if ACPI
> select ARCH_HAS_DEVMEM_IS_ALLOWED
> select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
> select ARCH_HAS_ELF_RANDOMIZE
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index d1ce8e2..3370046 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -208,6 +208,8 @@ void __init acpi_boot_table_init(void)
> pr_err("Failed to init ACPI tables\n");
> if (!param_acpi_force)
> disable_acpi();
> + } else {
> + acpi_probe_device_table(dbg2);
> }
> }
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 6/7] ACPI: enable ACPI_DBG2_TABLE on ARM64
2016-03-01 14:52 ` Peter Hurley
@ 2016-03-01 17:02 ` Aleksey Makarov
2016-03-01 17:25 ` Peter Hurley
0 siblings, 1 reply; 32+ messages in thread
From: Aleksey Makarov @ 2016-03-01 17:02 UTC (permalink / raw)
To: Peter Hurley, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Catalin Marinas, Will Deacon
On 03/01/2016 05:52 PM, Peter Hurley wrote:
> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>> SBBR mentions DBG2 as a mandatory ACPI table.
>> So enable it for ARM64
>
> Why does this have to be opted-in per-arch?
DBG2 can specify earlycons. They should start as soon as possible.
On ARM64 it is just after boot tables become available (in arch_init()).
So it has to be per-arch unless we decide not to start earlycons on
DBG2 ports.
>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>> ---
>> arch/arm64/Kconfig | 1 +
>> arch/arm64/kernel/acpi.c | 2 ++
>> 2 files changed, 3 insertions(+)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 9dc5209..0b614d4 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -3,6 +3,7 @@ config ARM64
>> select ACPI_CCA_REQUIRED if ACPI
>> select ACPI_GENERIC_GSI if ACPI
>> select ACPI_REDUCED_HARDWARE_ONLY if ACPI
>> + select ACPI_DBG2_TABLE if ACPI
>> select ARCH_HAS_DEVMEM_IS_ALLOWED
>> select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>> select ARCH_HAS_ELF_RANDOMIZE
>> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
>> index d1ce8e2..3370046 100644
>> --- a/arch/arm64/kernel/acpi.c
>> +++ b/arch/arm64/kernel/acpi.c
>> @@ -208,6 +208,8 @@ void __init acpi_boot_table_init(void)
>> pr_err("Failed to init ACPI tables\n");
>> if (!param_acpi_force)
>> disable_acpi();
>> + } else {
>> + acpi_probe_device_table(dbg2);
>> }
>> }
>>
>>
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 6/7] ACPI: enable ACPI_DBG2_TABLE on ARM64
2016-03-01 17:02 ` Aleksey Makarov
@ 2016-03-01 17:25 ` Peter Hurley
2016-03-03 11:41 ` Aleksey Makarov
0 siblings, 1 reply; 32+ messages in thread
From: Peter Hurley @ 2016-03-01 17:25 UTC (permalink / raw)
To: Aleksey Makarov, linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Russell King,
Greg Kroah-Hartman, Rafael J . Wysocki, Leif Lindholm,
Graeme Gregory, Al Stone, Christopher Covington, Matthias Brugger,
Catalin Marinas, Will Deacon
On 03/01/2016 09:02 AM, Aleksey Makarov wrote:
>
>
> On 03/01/2016 05:52 PM, Peter Hurley wrote:
>> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>>> SBBR mentions DBG2 as a mandatory ACPI table.
>>> So enable it for ARM64
>>
>> Why does this have to be opted-in per-arch?
>
> DBG2 can specify earlycons. They should start as soon as possible.
> On ARM64 it is just after boot tables become available (in arch_init()).
> So it has to be per-arch unless we decide not to start earlycons on
> DBG2 ports.
OF does all that already but the earlycon opt-in is not per-arch.
>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>> ---
>>> arch/arm64/Kconfig | 1 +
>>> arch/arm64/kernel/acpi.c | 2 ++
>>> 2 files changed, 3 insertions(+)
>>>
>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>> index 9dc5209..0b614d4 100644
>>> --- a/arch/arm64/Kconfig
>>> +++ b/arch/arm64/Kconfig
>>> @@ -3,6 +3,7 @@ config ARM64
>>> select ACPI_CCA_REQUIRED if ACPI
>>> select ACPI_GENERIC_GSI if ACPI
>>> select ACPI_REDUCED_HARDWARE_ONLY if ACPI
>>> + select ACPI_DBG2_TABLE if ACPI
>>> select ARCH_HAS_DEVMEM_IS_ALLOWED
>>> select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>>> select ARCH_HAS_ELF_RANDOMIZE
>>> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
>>> index d1ce8e2..3370046 100644
>>> --- a/arch/arm64/kernel/acpi.c
>>> +++ b/arch/arm64/kernel/acpi.c
>>> @@ -208,6 +208,8 @@ void __init acpi_boot_table_init(void)
>>> pr_err("Failed to init ACPI tables\n");
>>> if (!param_acpi_force)
>>> disable_acpi();
>>> + } else {
>>> + acpi_probe_device_table(dbg2);
>>> }
>>> }
>>>
>>>
>>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 6/7] ACPI: enable ACPI_DBG2_TABLE on ARM64
2016-03-01 17:25 ` Peter Hurley
@ 2016-03-03 11:41 ` Aleksey Makarov
2016-03-03 15:51 ` Peter Hurley
0 siblings, 1 reply; 32+ messages in thread
From: Aleksey Makarov @ 2016-03-03 11:41 UTC (permalink / raw)
To: Peter Hurley, Aleksey Makarov, linux-acpi
Cc: Matthias Brugger, Russell King, Graeme Gregory,
Greg Kroah-Hartman, Rafael J . Wysocki, linux-kernel,
Leif Lindholm, Christopher Covington, linux-serial,
Catalin Marinas, Will Deacon, Al Stone, linux-arm-kernel
On 03/01/2016 08:25 PM, Peter Hurley wrote:
> On 03/01/2016 09:02 AM, Aleksey Makarov wrote:
>>
>>
>> On 03/01/2016 05:52 PM, Peter Hurley wrote:
>>> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>>>> SBBR mentions DBG2 as a mandatory ACPI table.
>>>> So enable it for ARM64
>>>
>>> Why does this have to be opted-in per-arch?
>>
>> DBG2 can specify earlycons. They should start as soon as possible.
>> On ARM64 it is just after boot tables become available (in arch_init()).
>> So it has to be per-arch unless we decide not to start earlycons on
>> DBG2 ports.
>
> OF does all that already but the earlycon opt-in is not per-arch.
In the last next-XXXX kernel support for OF earlycon depends on
CONFIG_OF_EARLY_FLATTREE which *is* per-arch.
Probably we need the same kind of configuration option here,
something like CONFIG_ACPI_BOOT_TABLES instead of ACPI_DBG2_TABLE.
>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>>> ---
>>>> arch/arm64/Kconfig | 1 +
>>>> arch/arm64/kernel/acpi.c | 2 ++
>>>> 2 files changed, 3 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>> index 9dc5209..0b614d4 100644
>>>> --- a/arch/arm64/Kconfig
>>>> +++ b/arch/arm64/Kconfig
>>>> @@ -3,6 +3,7 @@ config ARM64
>>>> select ACPI_CCA_REQUIRED if ACPI
>>>> select ACPI_GENERIC_GSI if ACPI
>>>> select ACPI_REDUCED_HARDWARE_ONLY if ACPI
>>>> + select ACPI_DBG2_TABLE if ACPI
>>>> select ARCH_HAS_DEVMEM_IS_ALLOWED
>>>> select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>>>> select ARCH_HAS_ELF_RANDOMIZE
>>>> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
>>>> index d1ce8e2..3370046 100644
>>>> --- a/arch/arm64/kernel/acpi.c
>>>> +++ b/arch/arm64/kernel/acpi.c
>>>> @@ -208,6 +208,8 @@ void __init acpi_boot_table_init(void)
>>>> pr_err("Failed to init ACPI tables\n");
>>>> if (!param_acpi_force)
>>>> disable_acpi();
>>>> + } else {
>>>> + acpi_probe_device_table(dbg2);
>>>> }
>>>> }
>>>>
>>>>
>>>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 6/7] ACPI: enable ACPI_DBG2_TABLE on ARM64
2016-03-03 11:41 ` Aleksey Makarov
@ 2016-03-03 15:51 ` Peter Hurley
0 siblings, 0 replies; 32+ messages in thread
From: Peter Hurley @ 2016-03-03 15:51 UTC (permalink / raw)
To: Aleksey Makarov, Aleksey Makarov, linux-acpi
Cc: Matthias Brugger, Russell King, Graeme Gregory,
Greg Kroah-Hartman, Rafael J . Wysocki, linux-kernel,
Leif Lindholm, Christopher Covington, linux-serial,
Catalin Marinas, Will Deacon, Al Stone, linux-arm-kernel
On 03/03/2016 03:41 AM, Aleksey Makarov wrote:
>
>
> On 03/01/2016 08:25 PM, Peter Hurley wrote:
>> On 03/01/2016 09:02 AM, Aleksey Makarov wrote:
>>>
>>>
>>> On 03/01/2016 05:52 PM, Peter Hurley wrote:
>>>> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>>>>> SBBR mentions DBG2 as a mandatory ACPI table.
>>>>> So enable it for ARM64
>>>>
>>>> Why does this have to be opted-in per-arch?
>>>
>>> DBG2 can specify earlycons. They should start as soon as possible.
>>> On ARM64 it is just after boot tables become available (in arch_init()).
>>> So it has to be per-arch unless we decide not to start earlycons on
>>> DBG2 ports.
>>
>> OF does all that already but the earlycon opt-in is not per-arch.
>
> In the last next-XXXX kernel support for OF earlycon depends on
> CONFIG_OF_EARLY_FLATTREE which *is* per-arch.
That's true.
(of_setup_earlycon() was really always CONFIG_OF_EARLY_FLATTREE because
its lone caller was CONFIG_OF_EARLY_FLATTREE).
> Probably we need the same kind of configuration option here,
> something like CONFIG_ACPI_BOOT_TABLES instead of ACPI_DBG2_TABLE.
I'll let acpi maintainers sort that.
>>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>>>> ---
>>>>> arch/arm64/Kconfig | 1 +
>>>>> arch/arm64/kernel/acpi.c | 2 ++
>>>>> 2 files changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>>> index 9dc5209..0b614d4 100644
>>>>> --- a/arch/arm64/Kconfig
>>>>> +++ b/arch/arm64/Kconfig
>>>>> @@ -3,6 +3,7 @@ config ARM64
>>>>> select ACPI_CCA_REQUIRED if ACPI
>>>>> select ACPI_GENERIC_GSI if ACPI
>>>>> select ACPI_REDUCED_HARDWARE_ONLY if ACPI
>>>>> + select ACPI_DBG2_TABLE if ACPI
>>>>> select ARCH_HAS_DEVMEM_IS_ALLOWED
>>>>> select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>>>>> select ARCH_HAS_ELF_RANDOMIZE
>>>>> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
>>>>> index d1ce8e2..3370046 100644
>>>>> --- a/arch/arm64/kernel/acpi.c
>>>>> +++ b/arch/arm64/kernel/acpi.c
>>>>> @@ -208,6 +208,8 @@ void __init acpi_boot_table_init(void)
>>>>> pr_err("Failed to init ACPI tables\n");
>>>>> if (!param_acpi_force)
>>>>> disable_acpi();
>>>>> + } else {
>>>>> + acpi_probe_device_table(dbg2);
>>>>> }
>>>>> }
>>>>>
>>>>>
>>>>
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 7/7] serial: pl011: add ACPI DBG2 serial port
2016-02-29 12:41 [PATCH v3 0/7] ACPI: parse the DBG2 table Aleksey Makarov
` (5 preceding siblings ...)
2016-02-29 12:42 ` [PATCH v3 6/7] ACPI: enable ACPI_DBG2_TABLE on ARM64 Aleksey Makarov
@ 2016-02-29 12:42 ` Aleksey Makarov
6 siblings, 0 replies; 32+ messages in thread
From: Aleksey Makarov @ 2016-02-29 12:42 UTC (permalink / raw)
To: linux-acpi
Cc: linux-serial, linux-kernel, linux-arm-kernel, Aleksey Makarov,
Russell King, Greg Kroah-Hartman, Rafael J . Wysocki,
Leif Lindholm, Graeme Gregory, Al Stone, Christopher Covington,
Matthias Brugger, Jiri Slaby
Add a handler for ACPI DBG2 serial port of type ACPI_DBG2_ARM_PL011
that sets up an earlycon on it.
Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
drivers/tty/serial/amba-pl011.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 500232a..d38da86 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -59,6 +59,7 @@
#include <linux/sizes.h>
#include <linux/io.h>
#include <linux/acpi.h>
+#include <linux/acpi_dbg2.h>
#include "amba-pl011.h"
@@ -2327,6 +2328,8 @@ static int __init pl011_early_console_setup(struct earlycon_device *device,
return 0;
}
OF_EARLYCON_DECLARE(pl011, "arm,pl011", pl011_early_console_setup);
+ACPI_DBG2_EARLYCON_DECLARE(pl011, ACPI_DBG2_ARM_PL011,
+ pl011_early_console_setup);
#else
#define AMBA_CONSOLE NULL
--
2.7.1
^ permalink raw reply related [flat|nested] 32+ messages in thread