Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1362871

Re: [PATCH v5 3/6] ACPI: parse SPCR and enable matching console

Show all headers | View raw

On 03/22/2016 09:51 AM, Yury Norov wrote:
> On Tue, Mar 22, 2016 at 07:57:04AM -0700, Peter Hurley wrote:
> 
> [...]
> 
>>>> +static bool init_earlycon;
>>>> +
>>>> +void __init init_spcr_earlycon(void)
>>>> +{
>>>> +	init_earlycon = true;
>>>> +}
>>>> +
>>>
>>> 1. I see you keep in mind multiple access.
>>
>> Concurrent access is not a concern here: only the boot cpu is running
>> and intrs are off.
>>
>> The "init_earlycon" flag is used because parsing the "earlycon" early param
>> is earlier than parsing ACPI tables.
>>
> 
> OK got it. My concern is that it's generic code, and parse_spcr() is public
> function. I think corresponding comment is needed at least. The other option is
> to make it race-safe and forget. I prefer second one, moreover it's 2 simple
> changes.

Earlycon is generic only in the sense of platform independence, not in the
sense of temporal independence. There is no serialization in earlycon, anywhere.
Adding serialization will unnecessarily confuse casual observers into
believing it is necessary.

An argument could be made that earlycon needs some standalone documentation,
but I don't think this patch needs to be that.


>>  Then you'd worry about race
>>> conditions as well. In this case, I'd consider atomic access to
>>> variable.
>>> 2. It seems you need is_init() helper too.
>>>
>>>> +int __init parse_spcr(void)
>>>> +{
>>>> +	static char opts[64];
>>>> +	struct acpi_table_spcr *table;
>>>> +	acpi_size table_size;
>>>> +	acpi_status status;
>>>> +	char *uart;
>>>> +	char *iotype;
>>>> +	int baud_rate;
>>>> +	int err = 0;
>>>
>>> You can do not initialize 'err'.
>>
>> Why?
>>
> 
> Because there's no path here that doesn't init err with some value.
> So this initialization is useless waste of cycles.

Ok.

Regards,
Peter Hurley

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v5 0/6] ACPI: parse the SPCR table Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-03-22 11:50 +0100
  [PATCH v5 5/6] serial: pl011: add console matching function Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-03-22 11:50 +0100
  [PATCH v5 6/6] serial: pl011: add EARLYCON_DECLARE Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-03-22 11:50 +0100
    Re: [PATCH v5 6/6] serial: pl011: add EARLYCON_DECLARE Peter Hurley <peter@hurleysoftware.com> - 2016-03-22 17:20 +0100
      Re: [PATCH v5 6/6] serial: pl011: add EARLYCON_DECLARE Aleksey Makarov <amakarov.linux@gmail.com> - 2016-03-22 18:20 +0100
        Re: [PATCH v5 6/6] serial: pl011: add EARLYCON_DECLARE Peter Hurley <peter@hurleysoftware.com> - 2016-03-22 18:50 +0100
  [PATCH v5 3/6] ACPI: parse SPCR and enable matching console Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-03-22 12:00 +0100
    Re: [PATCH v5 3/6] ACPI: parse SPCR and enable matching console Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-03-22 12:10 +0100
      Re: [PATCH v5 3/6] ACPI: parse SPCR and enable matching console Peter Hurley <peter@hurleysoftware.com> - 2016-03-22 17:10 +0100
        Re: [PATCH v5 3/6] ACPI: parse SPCR and enable matching console Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-03-22 18:10 +0100
          Re: [PATCH v5 3/6] ACPI: parse SPCR and enable matching console Peter Hurley <peter@hurleysoftware.com> - 2016-03-22 18:30 +0100
    Re: [PATCH v5 3/6] ACPI: parse SPCR and enable matching console Peter Hurley <peter@hurleysoftware.com> - 2016-03-22 16:00 +0100
      Re: [PATCH v5 3/6] ACPI: parse SPCR and enable matching console Yury Norov <ynorov@caviumnetworks.com> - 2016-03-22 18:00 +0100
        Re: [PATCH v5 3/6] ACPI: parse SPCR and enable matching console Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-03-22 18:20 +0100
        Re: [PATCH v5 3/6] ACPI: parse SPCR and enable matching console Peter Hurley <peter@hurleysoftware.com> - 2016-03-22 18:40 +0100
  [PATCH v5 2/6] ACPI: add definitions of DBG2 subtypes Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-03-22 12:00 +0100
  [PATCH v5 4/6] ACPI: enable ACPI_SPCR_TABLE on ARM64 Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-03-22 12:00 +0100

csiph-web