Re: [PATCH v3 1/7] of/serial: move earlycon early_param handling to serial

[Peter Hurley]

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@xxxxxxxxxx>
>>>>>
>>>>> 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@xxxxxxxxxx>
>>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@xxxxxxxxxx>
>>>>> ---
>>>>>  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@xxxxxxxxxxxxxxx
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>