linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Aleksey Makarov <aleksey.makarov@linaro.org>
To: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	"linux-serial@vger.kernel.org" <linux-serial@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	Russell King <linux@arm.linux.org.uk>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>,
	Leif Lindholm <leif.lindholm@linaro.org>,
	Graeme Gregory <graeme.gregory@linaro.org>,
	Al Stone <ahs3@redhat.com>,
	Christopher Covington <cov@codeaurora.org>,
	Len Brown <lenb@kernel.org>
Subject: Re: [PATCH v2 3/9] ACPI: introduce acpi_table_parse2()
Date: Mon, 15 Feb 2016 15:51:52 +0300	[thread overview]
Message-ID: <56C1C9E8.2050300@linaro.org> (raw)
In-Reply-To: <CAJZ5v0i2RCEEZwCN+Ran3Qmasn4yEk1rbfWkZ5QbEkUQWvaJ+g@mail.gmail.com>



On 02/13/2016 02:07 AM, Rafael J. Wysocki wrote:
> On Fri, Feb 12, 2016 at 6:43 PM, Aleksey Makarov
> <aleksey.makarov@linaro.org> wrote:
>> The function acpi_table_parse() has some problems:
>> 1 It can be called only from __init code
>> 2 It does not pass any data to the handler
>> 3 It just throws out the value returned from the handler
> 
> So why are those problems?

1.  We need this function to be non-__init because we need access to
some tables at unpredictable time--it may be before or after
the init functions are removed.  For example, SPCR (Serial Port Console
Redirection) table is needed each time a new console is registered.
It can be quite early (console_initcall) or when a module is inserted.

2. Having an additional pointer to void is very useful because it allows
drivers to pass (local) data to the handler.  Without this, we would need 
to have global data and a mutex as it was done in drivers/acpi/scan.c
(vars ape, acpi_probe_count, acpi_probe_lock).

3. Passing the value returned from the callback is less important as it 
could be modelled by having a (return) field in the data passed to 
the handler.  It is very convenient though.

I use these three properties in my next patch of this series.

>> These issues are addressed in this patch
> 
> How are they addressed?

A new function acpi_table_parse2() has been created that is just 
the old acpi_table_parse() without __init, taking the handler with 
additional data pointer and respecting the value returned from 
the handler.  The old acpi_table_parse() was implemented witht this 
new function.  Dropping __init was made possible by the previous 
patch where the attribute of the function early_acpi_os_unmap_memory()
was changed from __init to __ref and by dropping __init from 
acpi_apic_instance.

I understand why this approach is not good and I am going to fix it.

Thank you
Aleksey Makarov

  reply	other threads:[~2016-02-15 12:57 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-12 17:43 [PATCH v2 0/9] ACPI: parse the SPCR table Aleksey Makarov
2016-02-12 17:43 ` [PATCH v2 1/9] printk: fix name and type of some variables Aleksey Makarov
2016-02-12 17:43 ` [PATCH v2 2/9] ACPI: Change __init to __ref for early_acpi_os_unmap_memory() Aleksey Makarov
2016-02-12 17:43 ` [PATCH v2 3/9] ACPI: introduce acpi_table_parse2() Aleksey Makarov
2016-02-12 18:44   ` kbuild test robot
2016-02-12 18:51   ` Greg Kroah-Hartman
2016-02-12 23:08     ` Rafael J. Wysocki
2016-02-15 12:57       ` Aleksey Makarov
2016-02-12 23:07   ` Rafael J. Wysocki
2016-02-15 12:51     ` Aleksey Makarov [this message]
2016-02-12 17:43 ` [PATCH v2 4/9] ACPI: parse SPCR and enable matching console Aleksey Makarov
2016-02-12 18:53   ` Greg Kroah-Hartman
2016-02-12 17:43 ` [PATCH v2 5/9] ACPI: enable ACPI_SPCR_TABLE on ARM64 Aleksey Makarov
2016-02-12 17:43 ` [PATCH v2 6/9] ACPI: add definition of DBG2 subtypes Aleksey Makarov
2016-02-12 22:47   ` Moore, Robert
2016-02-12 17:43 ` [PATCH v2 7/9] serial: pl011: add acpi_match Aleksey Makarov
2016-02-12 17:43 ` [PATCH v2 8/9] serial: 8250: " Aleksey Makarov
2016-02-12 18:56   ` Greg Kroah-Hartman
2016-02-12 17:43 ` [PATCH v2 9/9] serial: pl011: use SPCR to setup 32-bit access Aleksey Makarov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56C1C9E8.2050300@linaro.org \
    --to=aleksey.makarov@linaro.org \
    --cc=ahs3@redhat.com \
    --cc=cov@codeaurora.org \
    --cc=graeme.gregory@linaro.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=leif.lindholm@linaro.org \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=rafael@kernel.org \
    --cc=rjw@rjwysocki.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).