linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lukasz Anaczkowski <lukasz.anaczkowski@intel.com>
To: marc.zyngier@arm.com, lorenzo.pieralisi@arm.com,
	tomasz.nowicki@linaro.org, tglx@linutronix.de, mingo@redhat.com,
	hpa@zytor.com, x86@kernel.org, jason@lakedaemon.net
Cc: rjw@rjwysocki.net, len.brown@intel.com, pavel@ucw.cz,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-acpi@vger.kernel.org
Subject: Re: [PATCH] x86, acpi: Handle lapic/x2apic entries in MADT
Date: Wed, 26 Aug 2015 19:49:28 +0200	[thread overview]
Message-ID: <1440611369-26314-1-git-send-email-lukasz.anaczkowski@intel.com> (raw)
In-Reply-To: <55DDB45D.2030901@arm.com>

Marc nad Lorenzo,

First of all appologies for breaking arm64 (again) and thank you for
debugging effort. I own you.

> - count is only incremented when max_entries != 0, as you noticed

You are right, sorry for that, it's fixed in v3.

> - With max_entries != 0, count now represent the sum of all matches
>  Is that expected?

I have no strong opinion on that one. All of the x86 ACPI entries
handling only checks for count < 0, or uses count from the
acpi_subtable_proc structure (and that's why I didn't noticed the
mainline breakage).
If you think it's not correct or less usable than other approach,
let me know.

> - The proc iteration stops after the first match. Why?

So, the initial implementation of the acpi_parse_entries accepted
single handler for the ACPI table. Now, with this change, assumption
is that different handlers for different tables/subtables are passed,
meaning only one can meet entry->type == proc[i].id condition.
mainline breakage). This approach saves one local varaible, but
I don't think this is ultimate argument :)

> - The test for max_entries is done inside the proc loop. Why?

That's obviously wrong in context of the overall wrong counting.

> [...] this should be documented and agreed upon.

I've added description with assumptions. Again, if you think it's
not correct, let me know.

Tomasz Nowicki wrote:
> should acpi_table_parse_entries suppose to be removed above?

Thanks for pointing this out. I've missed implementation of
acpi_table_parse_entries when was backporting initial patch.
I've added it back.

Cheers,
Lukasz


  reply	other threads:[~2015-08-26 17:49 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <alpine.DEB.2.11.1507211017590.18576 () nanos>
2015-07-30 17:43 ` [PATCH] x86, acpi: Handle xapic/x2apic entries in MADT Lukasz Anaczkowski
2015-07-30 17:43   ` [PATCH] x86, acpi: Handle lapic/x2apic " Lukasz Anaczkowski
2015-08-02  9:57     ` Thomas Gleixner
2015-08-02 12:40     ` Marc Zyngier
2015-08-03 18:26       ` Lukasz Anaczkowski
2015-08-03 18:26         ` Lukasz Anaczkowski
2015-08-26  7:04           ` Anaczkowski, Lukasz
2015-08-26 10:43             ` Marc Zyngier
2015-08-26 11:42               ` Lorenzo Pieralisi
2015-08-26 12:43                 ` Marc Zyngier
2015-08-26 17:49                   ` Lukasz Anaczkowski [this message]
2015-08-26 17:49                     ` [PATCH] x86, arm64, " Lukasz Anaczkowski
2015-08-27  9:37                       ` Lorenzo Pieralisi
2015-09-08 11:07                         ` Lukasz Anaczkowski
2015-09-08 11:07                           ` [PATCH 0/4] Fix how CPUs are enumerated when there's more than 255 CPUs Lukasz Anaczkowski
2015-09-08 11:07                             ` [PATCH 1/4] acpi: rename acpi_table_parse_entries Lukasz Anaczkowski
2015-09-08 11:07                               ` [PATCH 2/4] x86, arm64, acpi: Added acpi_subtable_proc Lukasz Anaczkowski
2015-09-08 11:07                                 ` [PATCH 3/4] acpi: multi proc support Lukasz Anaczkowski
2015-09-08 11:08                                   ` [PATCH 4/4] x86, acpi: Handle apic/x2apic entries in MADT in correct order Lukasz Anaczkowski
2015-09-08 15:22                                     ` Marc Zyngier
2015-09-08 16:27                             ` [PATCH 0/4] Fix how CPUs are enumerated when there's more than 255 CPUs Marc Zyngier
2015-09-08 22:45                               ` Al Stone
2015-09-09  7:01                               ` Anaczkowski, Lukasz
2015-09-09  9:30                               ` [PATCH 0/2] " Lukasz Anaczkowski
2015-09-09  9:30                                 ` [PATCH 1/2] acpi: Added acpi_subtable_proc to ACPI table parsers Lukasz Anaczkowski
2015-09-09  9:30                                   ` [PATCH 2/2] x86, acpi: Handle apic/x2apic entries in MADT in correct order Lukasz Anaczkowski
2015-09-09 13:56                                     ` Lorenzo Pieralisi
2015-09-09 14:27                                       ` Anaczkowski, Lukasz
2015-09-09 15:43                                         ` Lorenzo Pieralisi
2015-09-09 10:47                                   ` [PATCH 1/2] acpi: Added acpi_subtable_proc to ACPI table parsers Marc Zyngier
2015-09-09 13:47                                     ` Lukasz Anaczkowski
2015-09-09 13:47                                       ` [PATCH v4 0/2] Fix how CPUs are enumerated when there's more than 255 CPUs Lukasz Anaczkowski
2015-09-09 13:47                                         ` [PATCH v4 1/2] acpi: Added acpi_subtable_proc to ACPI table parsers Lukasz Anaczkowski
2015-09-09 13:47                                           ` [PATCH v4 2/2] x86, acpi: Handle apic/x2apic entries in MADT in correct order Lukasz Anaczkowski
2015-09-09 20:45                                         ` [PATCH v4 0/2] Fix how CPUs are enumerated when there's more than 255 CPUs Rafael J. Wysocki
2015-09-18 22:38                                           ` Rafael J. Wysocki
2015-08-28  8:30                       ` [PATCH] x86, arm64, acpi: Handle lapic/x2apic entries in MADT Ingo Molnar
2015-09-01  8:02                       ` Tomasz Nowicki
2015-09-01 12:07                         ` Anaczkowski, Lukasz
2015-09-01 13:36                           ` Tomasz Nowicki
2015-09-07 14:04                             ` Anaczkowski, Lukasz
2015-09-08 14:44                               ` Tomasz Nowicki
2015-08-26 11:03           ` [PATCH] x86, " Marc Zyngier
2015-08-26 12:56           ` Tomasz Nowicki

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=1440611369-26314-1-git-send-email-lukasz.anaczkowski@intel.com \
    --to=lukasz.anaczkowski@intel.com \
    --cc=hpa@zytor.com \
    --cc=jason@lakedaemon.net \
    --cc=len.brown@intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=marc.zyngier@arm.com \
    --cc=mingo@redhat.com \
    --cc=pavel@ucw.cz \
    --cc=rjw@rjwysocki.net \
    --cc=tglx@linutronix.de \
    --cc=tomasz.nowicki@linaro.org \
    --cc=x86@kernel.org \
    /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).