lkml.org 
[lkml]   [2017]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 1/2] acpi/x86: Fix improper handling of SCI INT for platforms supporting only IOAPIC mode
On Thu, 16 Nov 2017, Vikas C Sajjan wrote:

> The platforms which support only IOAPIC mode, pass the SCI information
> above the legacy space (0-15) via the FADT mechanism and not via MADT.
> In such cases the mp_override_legacy_irq() used by acpi_sci_ioapic_setup()
> to register SCI interrupts fails for interrupts >= 16, since it is meant to
> handle only legacy space and throws error "Invalid bus_irq %u for legacy
> override". Hence add a new function to handle SCI interrupts >= 16 and
> invoke it conditionally in acpi_sci_ioapic_setup().The code duplication
> due to this new function will be cleaned up in a separate patch.

This reads way better, but I have a small nit pick. In the example I gave
you there were multiple paragraphs on purpose to separate the different
parts. So if I just split the above lump into separate paragraphs:

[1]
The platforms which support only IOAPIC mode, pass the SCI information
above the legacy space (0-15) via the FADT mechanism and not via MADT.

[2]
In such cases the mp_override_legacy_irq() used by acpi_sci_ioapic_setup()
to register SCI interrupts fails for interrupts >= 16, since it is meant to
handle only legacy space and throws error "Invalid bus_irq %u for legacy
override".

[3]
Hence add a new function to handle SCI interrupts >= 16 and
invoke it conditionally in acpi_sci_ioapic_setup().

[4]
The code duplication due to this new function will be cleaned up in a
separate patch.

then this is clearly structured:

[1] describes the context.

[2] describes the failure

[3] describes the solution

[4] is an extra note to tell the reviewer/reader that you are aware of the
code duplication and this is addressed later.

No need to resend. I can do that when picking it up.

> Co-developed-by: Sunil V L <sunil.vl@hpe.com>

I had a discussion with Greg about this tag which resulted in a patch so
it should be soon part of the official documentation:

https://lkml.kernel.org/r/20171116132309.GA8449@kroah.com

We agreed that both authors should add their Signed-off-by to document that
the work conforms with the Developer Certificate of Origin. I'll add that
if that's ok for you.

Thanks for following up!

tglx

\
 
 \ /
  Last update: 2017-11-17 17:19    [W:0.054 / U:4.976 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site