* [PATCH V9 0/2] ACPI, PCI, irq: remove interrupt limitations
@ 2015-12-09 16:18 Sinan Kaya
2015-12-09 16:18 ` [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction Sinan Kaya
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Sinan Kaya @ 2015-12-09 16:18 UTC (permalink / raw)
To: linux-acpi, timur, cov, jcm, helgaas; +Cc: Sinan Kaya
Code currently supports 256 maximum interrupts at this moment. The patch is
reconfiguring the penalty array as a dynamic list to remove this
limitation.
The ACPI compiler uses the extended format when used interrupt numbers
are greater than 15. The extended IRQ is 32 bits according to the ACPI
spec. The code supports parsing the extended interrupt numbers. However,
due to used data structure type; the code silently truncates interrupt
numbers greater than 256.
Changes from V8: (https://lkml.org/lkml/2015/12/3/579)
* split the patch into two as
** ACPI, PCI, irq: remove interrupt count restriction
** ACPI, PCI, irq: remove interrupt number restriction
* add acpi_irq_add_penalty API to simplify code
Sinan Kaya (2):
ACPI, PCI, irq: remove interrupt count restriction
ACPI, PCI, irq: remove interrupt number restriction
drivers/acpi/pci_link.c | 140 +++++++++++++++++++++++++++++++++++-------------
1 file changed, 104 insertions(+), 36 deletions(-)
--
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction 2015-12-09 16:18 [PATCH V9 0/2] ACPI, PCI, irq: remove interrupt limitations Sinan Kaya @ 2015-12-09 16:18 ` Sinan Kaya 2015-12-09 16:59 ` Andy Shevchenko 2015-12-09 16:18 ` [PATCH V9 2/2] ACPI, PCI, irq: remove interrupt number restriction Sinan Kaya 2016-01-03 0:35 ` [PATCH V9 0/2] ACPI, PCI, irq: remove interrupt limitations Rafael J. Wysocki 2 siblings, 1 reply; 19+ messages in thread From: Sinan Kaya @ 2015-12-09 16:18 UTC (permalink / raw) To: linux-acpi, timur, cov, jcm, helgaas Cc: Sinan Kaya, Rafael J. Wysocki, Len Brown, linux-kernel Code currently supports 256 maximum interrupts at this moment. The patch is reconfiguring the penalty array as a dynamic list to remove this limitation. A new penalty linklist has been added for all other interrupts greater than 16. If an IRQ is not found in the link list, an IRQ info structure will be dynamically allocated on the first access and will be placed on the list for further reuse. The list will grow by the number of supported interrupts in the ACPI table rather than having a 256 hard limitation. Acked-by: Bjorn Helgaas <bhelgaas@google.com> Signed-off-by: Sinan Kaya <okaya@codeaurora.org> --- drivers/acpi/pci_link.c | 136 ++++++++++++++++++++++++++++++++++++------------ 1 file changed, 102 insertions(+), 34 deletions(-) diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c index 7c8408b..0286f17 100644 --- a/drivers/acpi/pci_link.c +++ b/drivers/acpi/pci_link.c @@ -4,6 +4,7 @@ * Copyright (C) 2001, 2002 Andy Grover <andrew.grover@intel.com> * Copyright (C) 2001, 2002 Paul Diefenbaugh <paul.s.diefenbaugh@intel.com> * Copyright (C) 2002 Dominik Brodowski <devel@brodo.de> + * Copyright (c) 2015, The Linux Foundation. All rights reserved. * * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ * @@ -437,7 +438,6 @@ static int acpi_pci_link_set(struct acpi_pci_link *link, int irq) * enabled system. */ -#define ACPI_MAX_IRQS 256 #define ACPI_MAX_ISA_IRQ 16 #define PIRQ_PENALTY_PCI_AVAILABLE (0) @@ -447,7 +447,7 @@ static int acpi_pci_link_set(struct acpi_pci_link *link, int irq) #define PIRQ_PENALTY_ISA_USED (16*16*16*16*16) #define PIRQ_PENALTY_ISA_ALWAYS (16*16*16*16*16*16) -static int acpi_irq_penalty[ACPI_MAX_IRQS] = { +static int acpi_irq_isa_penalty[ACPI_MAX_ISA_IRQ] = { PIRQ_PENALTY_ISA_ALWAYS, /* IRQ0 timer */ PIRQ_PENALTY_ISA_ALWAYS, /* IRQ1 keyboard */ PIRQ_PENALTY_ISA_ALWAYS, /* IRQ2 cascade */ @@ -464,9 +464,68 @@ static int acpi_irq_penalty[ACPI_MAX_IRQS] = { PIRQ_PENALTY_ISA_USED, /* IRQ13 fpe, sometimes */ PIRQ_PENALTY_ISA_USED, /* IRQ14 ide0 */ PIRQ_PENALTY_ISA_USED, /* IRQ15 ide1 */ - /* >IRQ15 */ }; +struct irq_penalty_info { + int irq; + int penalty; + struct list_head node; +}; + +static LIST_HEAD(acpi_irq_penalty_list); + +static int acpi_irq_get_penalty(int irq) +{ + struct irq_penalty_info *irq_info; + + if (irq < ACPI_MAX_ISA_IRQ) + return acpi_irq_isa_penalty[irq]; + + list_for_each_entry(irq_info, &acpi_irq_penalty_list, node) { + if (irq_info->irq == irq) + return irq_info->penalty; + } + + return 0; +} + +static int acpi_irq_set_penalty(int irq, int new_penalty) +{ + struct irq_penalty_info *irq_info; + + /* see if this is a ISA IRQ */ + if (irq < ACPI_MAX_ISA_IRQ) { + acpi_irq_isa_penalty[irq] = new_penalty; + return 0; + } + + /* next, try to locate from the dynamic list */ + list_for_each_entry(irq_info, &acpi_irq_penalty_list, node) { + if (irq_info->irq == irq) { + irq_info->penalty = new_penalty; + return 0; + } + } + + /* nope, let's allocate a slot for this IRQ */ + irq_info = kzalloc(sizeof(*irq_info), GFP_KERNEL); + if (!irq_info) + return -ENOMEM; + + irq_info->irq = irq; + irq_info->penalty = new_penalty; + list_add_tail(&irq_info->node, &acpi_irq_penalty_list); + + return 0; +} + +static void acpi_irq_add_penalty(int irq, int penalty) +{ + int curpen = acpi_irq_get_penalty(irq); + + acpi_irq_set_penalty(irq, curpen + penalty); +} + int __init acpi_irq_penalty_init(void) { struct acpi_pci_link *link; @@ -487,15 +546,16 @@ int __init acpi_irq_penalty_init(void) link->irq.possible_count; for (i = 0; i < link->irq.possible_count; i++) { - if (link->irq.possible[i] < ACPI_MAX_ISA_IRQ) - acpi_irq_penalty[link->irq. - possible[i]] += - penalty; + if (link->irq.possible[i] < ACPI_MAX_ISA_IRQ) { + int irqpos = link->irq.possible[i]; + + acpi_irq_add_penalty(irqpos, penalty); + } } } else if (link->irq.active) { - acpi_irq_penalty[link->irq.active] += - PIRQ_PENALTY_PCI_POSSIBLE; + acpi_irq_add_penalty(link->irq.active, + PIRQ_PENALTY_PCI_POSSIBLE); } } @@ -547,12 +607,12 @@ static int acpi_pci_link_allocate(struct acpi_pci_link *link) * the use of IRQs 9, 10, 11, and >15. */ for (i = (link->irq.possible_count - 1); i >= 0; i--) { - if (acpi_irq_penalty[irq] > - acpi_irq_penalty[link->irq.possible[i]]) + if (acpi_irq_get_penalty(irq) > + acpi_irq_get_penalty(link->irq.possible[i])) irq = link->irq.possible[i]; } } - if (acpi_irq_penalty[irq] >= PIRQ_PENALTY_ISA_ALWAYS) { + if (acpi_irq_get_penalty(irq) >= PIRQ_PENALTY_ISA_ALWAYS) { printk(KERN_ERR PREFIX "No IRQ available for %s [%s]. " "Try pci=noacpi or acpi=off\n", acpi_device_name(link->device), @@ -568,7 +628,8 @@ static int acpi_pci_link_allocate(struct acpi_pci_link *link) acpi_device_bid(link->device)); return -ENODEV; } else { - acpi_irq_penalty[link->irq.active] += PIRQ_PENALTY_PCI_USING; + acpi_irq_add_penalty(link->irq.active, PIRQ_PENALTY_PCI_USING); + printk(KERN_WARNING PREFIX "%s [%s] enabled at IRQ %d\n", acpi_device_name(link->device), acpi_device_bid(link->device), link->irq.active); @@ -778,7 +839,7 @@ static void acpi_pci_link_remove(struct acpi_device *device) } /* - * modify acpi_irq_penalty[] from cmdline + * modify penalty from cmdline */ static int __init acpi_irq_penalty_update(char *str, int used) { @@ -796,13 +857,10 @@ static int __init acpi_irq_penalty_update(char *str, int used) if (irq < 0) continue; - if (irq >= ARRAY_SIZE(acpi_irq_penalty)) - continue; - if (used) - acpi_irq_penalty[irq] += PIRQ_PENALTY_ISA_USED; + acpi_irq_add_penalty(irq, PIRQ_PENALTY_ISA_USED); else - acpi_irq_penalty[irq] = PIRQ_PENALTY_PCI_AVAILABLE; + acpi_irq_set_penalty(irq, PIRQ_PENALTY_PCI_AVAILABLE); if (retval != 2) /* no next number */ break; @@ -819,18 +877,23 @@ static int __init acpi_irq_penalty_update(char *str, int used) */ void acpi_penalize_isa_irq(int irq, int active) { - if (irq >= 0 && irq < ARRAY_SIZE(acpi_irq_penalty)) { - if (active) - acpi_irq_penalty[irq] += PIRQ_PENALTY_ISA_USED; - else - acpi_irq_penalty[irq] += PIRQ_PENALTY_PCI_USING; - } + int penalty; + + if (irq < 0) + return; + + if (active) + penalty = PIRQ_PENALTY_ISA_USED; + else + penalty = PIRQ_PENALTY_PCI_USING; + + acpi_irq_add_penalty(irq, penalty); } bool acpi_isa_irq_available(int irq) { - return irq >= 0 && (irq >= ARRAY_SIZE(acpi_irq_penalty) || - acpi_irq_penalty[irq] < PIRQ_PENALTY_ISA_ALWAYS); + return irq >= 0 && + (acpi_irq_get_penalty(irq) < PIRQ_PENALTY_ISA_ALWAYS); } /* @@ -840,13 +903,18 @@ bool acpi_isa_irq_available(int irq) */ void acpi_penalize_sci_irq(int irq, int trigger, int polarity) { - if (irq >= 0 && irq < ARRAY_SIZE(acpi_irq_penalty)) { - if (trigger != ACPI_MADT_TRIGGER_LEVEL || - polarity != ACPI_MADT_POLARITY_ACTIVE_LOW) - acpi_irq_penalty[irq] += PIRQ_PENALTY_ISA_ALWAYS; - else - acpi_irq_penalty[irq] += PIRQ_PENALTY_PCI_USING; - } + int penalty; + + if (irq < 0) + return; + + if (trigger != ACPI_MADT_TRIGGER_LEVEL || + polarity != ACPI_MADT_POLARITY_ACTIVE_LOW) + penalty = PIRQ_PENALTY_ISA_ALWAYS; + else + penalty = PIRQ_PENALTY_PCI_USING; + + acpi_irq_add_penalty(irq, penalty); } /* -- Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction 2015-12-09 16:18 ` [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction Sinan Kaya @ 2015-12-09 16:59 ` Andy Shevchenko 2015-12-09 17:09 ` Sinan Kaya 0 siblings, 1 reply; 19+ messages in thread From: Andy Shevchenko @ 2015-12-09 16:59 UTC (permalink / raw) To: Sinan Kaya Cc: linux-acpi, Timur Tabi, cov, jcm, helgaas, Rafael J. Wysocki, Len Brown, linux-kernel On Wed, Dec 9, 2015 at 6:18 PM, Sinan Kaya <okaya@codeaurora.org> wrote: > Code currently supports 256 maximum interrupts at this moment. The patch is > reconfiguring the penalty array as a dynamic list to remove this > limitation. > > A new penalty linklist has been added for all other interrupts greater than > 16. If an IRQ is not found in the link list, an IRQ info structure will be > dynamically allocated on the first access and will be placed on the list > for further reuse. The list will grow by the number of supported interrupts > in the ACPI table rather than having a 256 hard limitation. > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> Few nitpicks, though if Bjorn is okay with this one, you may ignore below. > --- > drivers/acpi/pci_link.c | 136 ++++++++++++++++++++++++++++++++++++------------ > 1 file changed, 102 insertions(+), 34 deletions(-) > > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > index 7c8408b..0286f17 100644 > --- a/drivers/acpi/pci_link.c > +++ b/drivers/acpi/pci_link.c > @@ -4,6 +4,7 @@ > * Copyright (C) 2001, 2002 Andy Grover <andrew.grover@intel.com> > * Copyright (C) 2001, 2002 Paul Diefenbaugh <paul.s.diefenbaugh@intel.com> > * Copyright (C) 2002 Dominik Brodowski <devel@brodo.de> > + * Copyright (c) 2015, The Linux Foundation. All rights reserved. > * > * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > * > @@ -437,7 +438,6 @@ static int acpi_pci_link_set(struct acpi_pci_link *link, int irq) > * enabled system. > */ > > -#define ACPI_MAX_IRQS 256 > #define ACPI_MAX_ISA_IRQ 16 > > #define PIRQ_PENALTY_PCI_AVAILABLE (0) > @@ -447,7 +447,7 @@ static int acpi_pci_link_set(struct acpi_pci_link *link, int irq) > #define PIRQ_PENALTY_ISA_USED (16*16*16*16*16) > #define PIRQ_PENALTY_ISA_ALWAYS (16*16*16*16*16*16) > > -static int acpi_irq_penalty[ACPI_MAX_IRQS] = { > +static int acpi_irq_isa_penalty[ACPI_MAX_ISA_IRQ] = { > PIRQ_PENALTY_ISA_ALWAYS, /* IRQ0 timer */ > PIRQ_PENALTY_ISA_ALWAYS, /* IRQ1 keyboard */ > PIRQ_PENALTY_ISA_ALWAYS, /* IRQ2 cascade */ > @@ -464,9 +464,68 @@ static int acpi_irq_penalty[ACPI_MAX_IRQS] = { > PIRQ_PENALTY_ISA_USED, /* IRQ13 fpe, sometimes */ > PIRQ_PENALTY_ISA_USED, /* IRQ14 ide0 */ > PIRQ_PENALTY_ISA_USED, /* IRQ15 ide1 */ > - /* >IRQ15 */ > }; > > +struct irq_penalty_info { > + int irq; > + int penalty; > + struct list_head node; > +}; > + > +static LIST_HEAD(acpi_irq_penalty_list); > + > +static int acpi_irq_get_penalty(int irq) > +{ > + struct irq_penalty_info *irq_info; > + > + if (irq < ACPI_MAX_ISA_IRQ) > + return acpi_irq_isa_penalty[irq]; > + > + list_for_each_entry(irq_info, &acpi_irq_penalty_list, node) { > + if (irq_info->irq == irq) > + return irq_info->penalty; > + } > + > + return 0; > +} > + > +static int acpi_irq_set_penalty(int irq, int new_penalty) > +{ > + struct irq_penalty_info *irq_info; > + > + /* see if this is a ISA IRQ */ > + if (irq < ACPI_MAX_ISA_IRQ) { > + acpi_irq_isa_penalty[irq] = new_penalty; > + return 0; > + } > + > + /* next, try to locate from the dynamic list */ > + list_for_each_entry(irq_info, &acpi_irq_penalty_list, node) { > + if (irq_info->irq == irq) { > + irq_info->penalty = new_penalty; > + return 0; > + } > + } > + > + /* nope, let's allocate a slot for this IRQ */ > + irq_info = kzalloc(sizeof(*irq_info), GFP_KERNEL); Maybe a comment to explain why we don't have a symmetric free() option. > + if (!irq_info) > + return -ENOMEM; > + > + irq_info->irq = irq; > + irq_info->penalty = new_penalty; > + list_add_tail(&irq_info->node, &acpi_irq_penalty_list); > + > + return 0; > +} > + > +static void acpi_irq_add_penalty(int irq, int penalty) > +{ > + int curpen = acpi_irq_get_penalty(irq); > + > + acpi_irq_set_penalty(irq, curpen + penalty); Can it be one line? > +} > + > int __init acpi_irq_penalty_init(void) > { > struct acpi_pci_link *link; > @@ -487,15 +546,16 @@ int __init acpi_irq_penalty_init(void) > link->irq.possible_count; > > for (i = 0; i < link->irq.possible_count; i++) { > - if (link->irq.possible[i] < ACPI_MAX_ISA_IRQ) > - acpi_irq_penalty[link->irq. > - possible[i]] += > - penalty; > + if (link->irq.possible[i] < ACPI_MAX_ISA_IRQ) { > + int irqpos = link->irq.possible[i]; > + > + acpi_irq_add_penalty(irqpos, penalty); > + } > } > > } else if (link->irq.active) { > - acpi_irq_penalty[link->irq.active] += > - PIRQ_PENALTY_PCI_POSSIBLE; > + acpi_irq_add_penalty(link->irq.active, > + PIRQ_PENALTY_PCI_POSSIBLE); > } > } > > @@ -547,12 +607,12 @@ static int acpi_pci_link_allocate(struct acpi_pci_link *link) > * the use of IRQs 9, 10, 11, and >15. > */ > for (i = (link->irq.possible_count - 1); i >= 0; i--) { > - if (acpi_irq_penalty[irq] > > - acpi_irq_penalty[link->irq.possible[i]]) > + if (acpi_irq_get_penalty(irq) > > + acpi_irq_get_penalty(link->irq.possible[i])) > irq = link->irq.possible[i]; > } > } > - if (acpi_irq_penalty[irq] >= PIRQ_PENALTY_ISA_ALWAYS) { > + if (acpi_irq_get_penalty(irq) >= PIRQ_PENALTY_ISA_ALWAYS) { > printk(KERN_ERR PREFIX "No IRQ available for %s [%s]. " > "Try pci=noacpi or acpi=off\n", > acpi_device_name(link->device), > @@ -568,7 +628,8 @@ static int acpi_pci_link_allocate(struct acpi_pci_link *link) > acpi_device_bid(link->device)); > return -ENODEV; > } else { > - acpi_irq_penalty[link->irq.active] += PIRQ_PENALTY_PCI_USING; > + acpi_irq_add_penalty(link->irq.active, PIRQ_PENALTY_PCI_USING); > + > printk(KERN_WARNING PREFIX "%s [%s] enabled at IRQ %d\n", > acpi_device_name(link->device), > acpi_device_bid(link->device), link->irq.active); > @@ -778,7 +839,7 @@ static void acpi_pci_link_remove(struct acpi_device *device) > } > > /* > - * modify acpi_irq_penalty[] from cmdline > + * modify penalty from cmdline > */ > static int __init acpi_irq_penalty_update(char *str, int used) > { > @@ -796,13 +857,10 @@ static int __init acpi_irq_penalty_update(char *str, int used) > if (irq < 0) > continue; > > - if (irq >= ARRAY_SIZE(acpi_irq_penalty)) > - continue; > - > if (used) > - acpi_irq_penalty[irq] += PIRQ_PENALTY_ISA_USED; > + acpi_irq_add_penalty(irq, PIRQ_PENALTY_ISA_USED); > else > - acpi_irq_penalty[irq] = PIRQ_PENALTY_PCI_AVAILABLE; > + acpi_irq_set_penalty(irq, PIRQ_PENALTY_PCI_AVAILABLE); > > if (retval != 2) /* no next number */ > break; > @@ -819,18 +877,23 @@ static int __init acpi_irq_penalty_update(char *str, int used) > */ > void acpi_penalize_isa_irq(int irq, int active) > { > - if (irq >= 0 && irq < ARRAY_SIZE(acpi_irq_penalty)) { > - if (active) > - acpi_irq_penalty[irq] += PIRQ_PENALTY_ISA_USED; > - else > - acpi_irq_penalty[irq] += PIRQ_PENALTY_PCI_USING; > - } > + int penalty; > + > + if (irq < 0) > + return; > + > + if (active) > + penalty = PIRQ_PENALTY_ISA_USED; > + else > + penalty = PIRQ_PENALTY_PCI_USING; > + > + acpi_irq_add_penalty(irq, penalty); Same as below > } > > bool acpi_isa_irq_available(int irq) > { > - return irq >= 0 && (irq >= ARRAY_SIZE(acpi_irq_penalty) || > - acpi_irq_penalty[irq] < PIRQ_PENALTY_ISA_ALWAYS); > + return irq >= 0 && > + (acpi_irq_get_penalty(irq) < PIRQ_PENALTY_ISA_ALWAYS); > } > > /* > @@ -840,13 +903,18 @@ bool acpi_isa_irq_available(int irq) > */ > void acpi_penalize_sci_irq(int irq, int trigger, int polarity) > { > - if (irq >= 0 && irq < ARRAY_SIZE(acpi_irq_penalty)) { > - if (trigger != ACPI_MADT_TRIGGER_LEVEL || > - polarity != ACPI_MADT_POLARITY_ACTIVE_LOW) > - acpi_irq_penalty[irq] += PIRQ_PENALTY_ISA_ALWAYS; > - else > - acpi_irq_penalty[irq] += PIRQ_PENALTY_PCI_USING; > - } > + int penalty; > + > + if (irq < 0) > + return; > + > + if (trigger != ACPI_MADT_TRIGGER_LEVEL || > + polarity != ACPI_MADT_POLARITY_ACTIVE_LOW) > + penalty = PIRQ_PENALTY_ISA_ALWAYS; > + else > + penalty = PIRQ_PENALTY_PCI_USING; > + > + acpi_irq_add_penalty(irq, penalty); Why not to change in place? I think a common sense rule is not to change something existing if it doesn't add any significant value. - acpi_irq_penalty[irq] += PIRQ_PENALTY_PCI_USING; + acpi_irq_add_penalty(irq, PIRQ_PENALTY_PCI_USING); > } -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction 2015-12-09 16:59 ` Andy Shevchenko @ 2015-12-09 17:09 ` Sinan Kaya 2015-12-09 17:14 ` Christopher Covington 0 siblings, 1 reply; 19+ messages in thread From: Sinan Kaya @ 2015-12-09 17:09 UTC (permalink / raw) To: Andy Shevchenko Cc: linux-acpi, Timur Tabi, cov, jcm, helgaas, Rafael J. Wysocki, Len Brown, linux-kernel On 12/9/2015 11:59 AM, Andy Shevchenko wrote: >> + if (trigger != ACPI_MADT_TRIGGER_LEVEL || >> > + polarity != ACPI_MADT_POLARITY_ACTIVE_LOW) >> > + penalty = PIRQ_PENALTY_ISA_ALWAYS; >> > + else >> > + penalty = PIRQ_PENALTY_PCI_USING; >> > + >> > + acpi_irq_add_penalty(irq, penalty); > Why not to change in place? I think a common sense rule is not to > change something existing if it doesn't add any significant value. > Sorry, I didn't understand what you mean. Are you asking why we are changing lines like above? If yes, acpi_irq_penalty used to be an array of 256 entries. Now, acpi_irq_penalty doesn't exist anymore as it was replaced with a linklist. > - acpi_irq_penalty[irq] += PIRQ_PENALTY_PCI_USING; > + acpi_irq_add_penalty(irq, PIRQ_PENALTY_PCI_USING); > -- Sinan Kaya Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction 2015-12-09 17:09 ` Sinan Kaya @ 2015-12-09 17:14 ` Christopher Covington 2015-12-30 13:23 ` Sinan Kaya 0 siblings, 1 reply; 19+ messages in thread From: Christopher Covington @ 2015-12-09 17:14 UTC (permalink / raw) To: Sinan Kaya, Andy Shevchenko Cc: linux-acpi, Timur Tabi, jcm, helgaas, Rafael J. Wysocki, Len Brown, linux-kernel Hi Sinan, On 12/09/2015 12:09 PM, Sinan Kaya wrote: > On 12/9/2015 11:59 AM, Andy Shevchenko wrote: >>> + if (trigger != ACPI_MADT_TRIGGER_LEVEL || >>>> + polarity != ACPI_MADT_POLARITY_ACTIVE_LOW) >>>> + penalty = PIRQ_PENALTY_ISA_ALWAYS; >>>> + else >>>> + penalty = PIRQ_PENALTY_PCI_USING; >>>> + >>>> + acpi_irq_add_penalty(irq, penalty); >> Why not to change in place? I think a common sense rule is not to >> change something existing if it doesn't add any significant value. >> > Sorry, I didn't understand what you mean. Are you asking why we are > changing lines like above? > > If yes, acpi_irq_penalty used to be an array of 256 entries. Now, > acpi_irq_penalty doesn't exist anymore as it was replaced with a linklist. > >> - acpi_irq_penalty[irq] += PIRQ_PENALTY_PCI_USING; >> + acpi_irq_add_penalty(irq, PIRQ_PENALTY_PCI_USING); I think Andy was suggesting that you make the change without introducing the penalty variable. Christopher Covington -- Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction 2015-12-09 17:14 ` Christopher Covington @ 2015-12-30 13:23 ` Sinan Kaya 2015-12-30 13:28 ` Andy Shevchenko 0 siblings, 1 reply; 19+ messages in thread From: Sinan Kaya @ 2015-12-30 13:23 UTC (permalink / raw) To: Christopher Covington, Andy Shevchenko Cc: linux-acpi, Timur Tabi, jcm, helgaas, Rafael J. Wysocki, Len Brown, linux-kernel Hi Bjorn, Andy; On 12/9/2015 12:14 PM, Christopher Covington wrote: > Hi Sinan, > > On 12/09/2015 12:09 PM, Sinan Kaya wrote: >> On 12/9/2015 11:59 AM, Andy Shevchenko wrote: >>>> + if (trigger != ACPI_MADT_TRIGGER_LEVEL || >>>>> + polarity != ACPI_MADT_POLARITY_ACTIVE_LOW) >>>>> + penalty = PIRQ_PENALTY_ISA_ALWAYS; >>>>> + else >>>>> + penalty = PIRQ_PENALTY_PCI_USING; >>>>> + >>>>> + acpi_irq_add_penalty(irq, penalty); >>> Why not to change in place? I think a common sense rule is not to >>> change something existing if it doesn't add any significant value. >>> >> Sorry, I didn't understand what you mean. Are you asking why we are >> changing lines like above? >> >> If yes, acpi_irq_penalty used to be an array of 256 entries. Now, >> acpi_irq_penalty doesn't exist anymore as it was replaced with a linklist. >> >>> - acpi_irq_penalty[irq] += PIRQ_PENALTY_PCI_USING; >>> + acpi_irq_add_penalty(irq, PIRQ_PENALTY_PCI_USING); > > I think Andy was suggesting that you make the change without introducing > the penalty variable. > > Christopher Covington > Andy, Is Chris' interpretation correct? BTW, I suggest you spend some time around checkpatch for contributions. I could have caught most of the issues you are generally concerned before submitting a patch. Bjorn, Is there any other question you need me to address on this patch? -- Sinan Kaya Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction 2015-12-30 13:23 ` Sinan Kaya @ 2015-12-30 13:28 ` Andy Shevchenko 2015-12-30 19:17 ` Sinan Kaya 0 siblings, 1 reply; 19+ messages in thread From: Andy Shevchenko @ 2015-12-30 13:28 UTC (permalink / raw) To: Sinan Kaya Cc: Christopher Covington, linux-acpi, Timur Tabi, jcm, helgaas, Rafael J. Wysocki, Len Brown, linux-kernel On Wed, Dec 30, 2015 at 3:23 PM, Sinan Kaya <okaya@codeaurora.org> wrote: > On 12/9/2015 12:14 PM, Christopher Covington wrote: >>> On 12/9/2015 11:59 AM, Andy Shevchenko wrote: >>>>> + if (trigger != ACPI_MADT_TRIGGER_LEVEL || >>>>>> + polarity != ACPI_MADT_POLARITY_ACTIVE_LOW) >>>>>> + penalty = PIRQ_PENALTY_ISA_ALWAYS; >>>>>> + else >>>>>> + penalty = PIRQ_PENALTY_PCI_USING; >>>>>> + >>>>>> + acpi_irq_add_penalty(irq, penalty); >>>> Why not to change in place? I think a common sense rule is not to >>>> change something existing if it doesn't add any significant value. >>>> - acpi_irq_penalty[irq] += PIRQ_PENALTY_PCI_USING; >>>> + acpi_irq_add_penalty(irq, PIRQ_PENALTY_PCI_USING); >> >> I think Andy was suggesting that you make the change without introducing >> the penalty variable. > Is Chris' interpretation correct? Yep, I meant not to use an additional variable. > BTW, I suggest you spend some time around checkpatch for contributions. I could > have caught most of the issues you are generally concerned before submitting a patch. Is it a question? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction 2015-12-30 13:28 ` Andy Shevchenko @ 2015-12-30 19:17 ` Sinan Kaya 2015-12-30 19:55 ` Andy Shevchenko 0 siblings, 1 reply; 19+ messages in thread From: Sinan Kaya @ 2015-12-30 19:17 UTC (permalink / raw) To: Andy Shevchenko Cc: Christopher Covington, linux-acpi, Timur Tabi, jcm, helgaas, Rafael J. Wysocki, Len Brown, linux-kernel On 12/30/2015 8:28 AM, Andy Shevchenko wrote: > Yep, I meant not to use an additional variable. > >> > BTW, I suggest you spend some time around checkpatch for contributions. I could >> > have caught most of the issues you are generally concerned before submitting a patch. > Is it a question? It is a request not a question. I hate wasting your time and my time with things that I could have fixed before submitting a patch. I ran the checkpatch and it said I'm good to go. But, obviously I'm not. -- Sinan Kaya Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction 2015-12-30 19:17 ` Sinan Kaya @ 2015-12-30 19:55 ` Andy Shevchenko 0 siblings, 0 replies; 19+ messages in thread From: Andy Shevchenko @ 2015-12-30 19:55 UTC (permalink / raw) To: Sinan Kaya Cc: Christopher Covington, linux-acpi, Timur Tabi, jcm, helgaas, Rafael J. Wysocki, Len Brown, linux-kernel On Wed, Dec 30, 2015 at 9:17 PM, Sinan Kaya <okaya@codeaurora.org> wrote: > On 12/30/2015 8:28 AM, Andy Shevchenko wrote: >> Yep, I meant not to use an additional variable. >> >>> > BTW, I suggest you spend some time around checkpatch for contributions. I could >>> > have caught most of the issues you are generally concerned before submitting a patch. >> Is it a question? > > It is a request not a question. I hate wasting your time and my time with things that I could > have fixed before submitting a patch. > > I ran the checkpatch and it said I'm good to go. But, obviously I'm not. Hmm… checkpatch.pl is just a small helper to fix style issues. Here is just a common sense rule, or kind of Occam's razor: no need to have more variables then needed if it doesn't improve something really significantly. -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction @ 2015-12-30 19:55 ` Andy Shevchenko 0 siblings, 0 replies; 19+ messages in thread From: Andy Shevchenko @ 2015-12-30 19:55 UTC (permalink / raw) To: Sinan Kaya Cc: Christopher Covington, linux-acpi, Timur Tabi, jcm, helgaas, Rafael J. Wysocki, Len Brown, linux-kernel On Wed, Dec 30, 2015 at 9:17 PM, Sinan Kaya <okaya@codeaurora.org> wrote: > On 12/30/2015 8:28 AM, Andy Shevchenko wrote: >> Yep, I meant not to use an additional variable. >> >>> > BTW, I suggest you spend some time around checkpatch for contributions. I could >>> > have caught most of the issues you are generally concerned before submitting a patch. >> Is it a question? > > It is a request not a question. I hate wasting your time and my time with things that I could > have fixed before submitting a patch. > > I ran the checkpatch and it said I'm good to go. But, obviously I'm not. Hmm… checkpatch.pl is just a small helper to fix style issues. Here is just a common sense rule, or kind of Occam's razor: no need to have more variables then needed if it doesn't improve something really significantly. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction 2015-12-30 19:55 ` Andy Shevchenko @ 2016-01-01 0:48 ` Rafael J. Wysocki -1 siblings, 0 replies; 19+ messages in thread From: Rafael J. Wysocki @ 2016-01-01 0:48 UTC (permalink / raw) To: Andy Shevchenko Cc: Sinan Kaya, Christopher Covington, linux-acpi, Timur Tabi, jcm, helgaas, Len Brown, linux-kernel On Wednesday, December 30, 2015 09:55:35 PM Andy Shevchenko wrote: > On Wed, Dec 30, 2015 at 9:17 PM, Sinan Kaya <okaya@codeaurora.org> wrote: > > On 12/30/2015 8:28 AM, Andy Shevchenko wrote: > >> Yep, I meant not to use an additional variable. > >> > >>> > BTW, I suggest you spend some time around checkpatch for contributions. I could > >>> > have caught most of the issues you are generally concerned before submitting a patch. > >> Is it a question? > > > > It is a request not a question. I hate wasting your time and my time with things that I could > > have fixed before submitting a patch. > > > > I ran the checkpatch and it said I'm good to go. But, obviously I'm not. > > Hmm… checkpatch.pl is just a small helper to fix style issues. Here is > just a common sense rule, or kind of Occam's razor: no need to have > more variables then needed if it doesn't improve something really > significantly. That said, compilers optimize things anyway, so using an extra local variable shouldn't matter for the resulting machine code. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction @ 2016-01-01 0:48 ` Rafael J. Wysocki 0 siblings, 0 replies; 19+ messages in thread From: Rafael J. Wysocki @ 2016-01-01 0:48 UTC (permalink / raw) To: Andy Shevchenko Cc: Sinan Kaya, Christopher Covington, linux-acpi, Timur Tabi, jcm, helgaas, Len Brown, linux-kernel On Wednesday, December 30, 2015 09:55:35 PM Andy Shevchenko wrote: > On Wed, Dec 30, 2015 at 9:17 PM, Sinan Kaya <okaya@codeaurora.org> wrote: > > On 12/30/2015 8:28 AM, Andy Shevchenko wrote: > >> Yep, I meant not to use an additional variable. > >> > >>> > BTW, I suggest you spend some time around checkpatch for contributions. I could > >>> > have caught most of the issues you are generally concerned before submitting a patch. > >> Is it a question? > > > > It is a request not a question. I hate wasting your time and my time with things that I could > > have fixed before submitting a patch. > > > > I ran the checkpatch and it said I'm good to go. But, obviously I'm not. > > Hmm… checkpatch.pl is just a small helper to fix style issues. Here is > just a common sense rule, or kind of Occam's razor: no need to have > more variables then needed if it doesn't improve something really > significantly. That said, compilers optimize things anyway, so using an extra local variable shouldn't matter for the resulting machine code. Thanks, Rafael ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction 2016-01-01 0:48 ` Rafael J. Wysocki @ 2016-01-04 9:01 ` Andy Shevchenko -1 siblings, 0 replies; 19+ messages in thread From: Andy Shevchenko @ 2016-01-04 9:01 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Sinan Kaya, Christopher Covington, linux-acpi, Timur Tabi, jcm, helgaas, Len Brown, linux-kernel On Fri, Jan 1, 2016 at 2:48 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Wednesday, December 30, 2015 09:55:35 PM Andy Shevchenko wrote: >> On Wed, Dec 30, 2015 at 9:17 PM, Sinan Kaya <okaya@codeaurora.org> wrote: >> > On 12/30/2015 8:28 AM, Andy Shevchenko wrote: >> >> Yep, I meant not to use an additional variable. >> >> >> >>> > BTW, I suggest you spend some time around checkpatch for contributions. I could >> >>> > have caught most of the issues you are generally concerned before submitting a patch. >> >> Is it a question? >> > >> > It is a request not a question. I hate wasting your time and my time with things that I could >> > have fixed before submitting a patch. >> > >> > I ran the checkpatch and it said I'm good to go. But, obviously I'm not. >> >> Hmm… checkpatch.pl is just a small helper to fix style issues. Here is >> just a common sense rule, or kind of Occam's razor: no need to have >> more variables then needed if it doesn't improve something really >> significantly. > > That said, compilers optimize things anyway, so using an extra local variable > shouldn't matter for the resulting machine code. I'm not totally against that, but is the additional variable helpful here? -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction @ 2016-01-04 9:01 ` Andy Shevchenko 0 siblings, 0 replies; 19+ messages in thread From: Andy Shevchenko @ 2016-01-04 9:01 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Sinan Kaya, Christopher Covington, linux-acpi, Timur Tabi, jcm, helgaas, Len Brown, linux-kernel On Fri, Jan 1, 2016 at 2:48 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Wednesday, December 30, 2015 09:55:35 PM Andy Shevchenko wrote: >> On Wed, Dec 30, 2015 at 9:17 PM, Sinan Kaya <okaya@codeaurora.org> wrote: >> > On 12/30/2015 8:28 AM, Andy Shevchenko wrote: >> >> Yep, I meant not to use an additional variable. >> >> >> >>> > BTW, I suggest you spend some time around checkpatch for contributions. I could >> >>> > have caught most of the issues you are generally concerned before submitting a patch. >> >> Is it a question? >> > >> > It is a request not a question. I hate wasting your time and my time with things that I could >> > have fixed before submitting a patch. >> > >> > I ran the checkpatch and it said I'm good to go. But, obviously I'm not. >> >> Hmm… checkpatch.pl is just a small helper to fix style issues. Here is >> just a common sense rule, or kind of Occam's razor: no need to have >> more variables then needed if it doesn't improve something really >> significantly. > > That said, compilers optimize things anyway, so using an extra local variable > shouldn't matter for the resulting machine code. I'm not totally against that, but is the additional variable helpful here? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction 2016-01-04 9:01 ` Andy Shevchenko (?) @ 2016-01-04 13:50 ` Rafael J. Wysocki 2016-01-04 13:34 ` Andy Shevchenko -1 siblings, 1 reply; 19+ messages in thread From: Rafael J. Wysocki @ 2016-01-04 13:50 UTC (permalink / raw) To: Andy Shevchenko Cc: Sinan Kaya, Christopher Covington, linux-acpi, Timur Tabi, jcm, helgaas, Len Brown, linux-kernel On Monday, January 04, 2016 11:01:05 AM Andy Shevchenko wrote: > On Fri, Jan 1, 2016 at 2:48 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > On Wednesday, December 30, 2015 09:55:35 PM Andy Shevchenko wrote: > >> On Wed, Dec 30, 2015 at 9:17 PM, Sinan Kaya <okaya@codeaurora.org> wrote: > >> > On 12/30/2015 8:28 AM, Andy Shevchenko wrote: > >> >> Yep, I meant not to use an additional variable. > >> >> > >> >>> > BTW, I suggest you spend some time around checkpatch for contributions. I could > >> >>> > have caught most of the issues you are generally concerned before submitting a patch. > >> >> Is it a question? > >> > > >> > It is a request not a question. I hate wasting your time and my time with things that I could > >> > have fixed before submitting a patch. > >> > > >> > I ran the checkpatch and it said I'm good to go. But, obviously I'm not. > >> > >> Hmm… checkpatch.pl is just a small helper to fix style issues. Here is > >> just a common sense rule, or kind of Occam's razor: no need to have > >> more variables then needed if it doesn't improve something really > >> significantly. > > > > That said, compilers optimize things anyway, so using an extra local variable > > shouldn't matter for the resulting machine code. > > I'm not totally against that, but is the additional variable helpful here? Well, I guess you can argue both ways. Surely, the same result can be achieved with fewer lines of code if that's what you mean, so what about the following change on top of the $subject patch? --- From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Subject: [PATCH] ACPI / PCI: Simplify acpi_penalize_isa_irq() acpi_penalize_isa_irq() can be written in fewer lines of code, so do that. No functional change. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/acpi/pci_link.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) Index: linux-pm/drivers/acpi/pci_link.c =================================================================== --- linux-pm.orig/drivers/acpi/pci_link.c +++ linux-pm/drivers/acpi/pci_link.c @@ -877,17 +877,9 @@ static int __init acpi_irq_penalty_updat */ void acpi_penalize_isa_irq(int irq, int active) { - int penalty; - - if (irq < 0) - return; - - if (active) - penalty = PIRQ_PENALTY_ISA_USED; - else - penalty = PIRQ_PENALTY_PCI_USING; - - acpi_irq_add_penalty(irq, penalty); + if (irq >= 0) + acpi_irq_add_penalty(irq, active ? + PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING); } bool acpi_isa_irq_available(int irq) ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction 2016-01-04 13:50 ` Rafael J. Wysocki @ 2016-01-04 13:34 ` Andy Shevchenko 0 siblings, 0 replies; 19+ messages in thread From: Andy Shevchenko @ 2016-01-04 13:34 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Sinan Kaya, Christopher Covington, linux-acpi, Timur Tabi, jcm, helgaas, Len Brown, linux-kernel On Mon, Jan 4, 2016 at 3:50 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Monday, January 04, 2016 11:01:05 AM Andy Shevchenko wrote: >> On Fri, Jan 1, 2016 at 2:48 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >> > On Wednesday, December 30, 2015 09:55:35 PM Andy Shevchenko wrote: >> >> On Wed, Dec 30, 2015 at 9:17 PM, Sinan Kaya <okaya@codeaurora.org> wrote: >> >> > On 12/30/2015 8:28 AM, Andy Shevchenko wrote: >> >> >> Yep, I meant not to use an additional variable. >> >> >> >> >> >>> > BTW, I suggest you spend some time around checkpatch for contributions. I could >> >> >>> > have caught most of the issues you are generally concerned before submitting a patch. >> >> >> Is it a question? >> >> > >> >> > It is a request not a question. I hate wasting your time and my time with things that I could >> >> > have fixed before submitting a patch. >> >> > >> >> > I ran the checkpatch and it said I'm good to go. But, obviously I'm not. >> >> >> >> Hmm… checkpatch.pl is just a small helper to fix style issues. Here is >> >> just a common sense rule, or kind of Occam's razor: no need to have >> >> more variables then needed if it doesn't improve something really >> >> significantly. >> > >> > That said, compilers optimize things anyway, so using an extra local variable >> > shouldn't matter for the resulting machine code. >> >> I'm not totally against that, but is the additional variable helpful here? > > Well, I guess you can argue both ways. > > Surely, the same result can be achieved with fewer lines of code if that's > what you mean, so what about the following change on top of the $subject patch? > > --- > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Subject: [PATCH] ACPI / PCI: Simplify acpi_penalize_isa_irq() > > acpi_penalize_isa_irq() can be written in fewer lines of code, > so do that. No functional change. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/acpi/pci_link.c | 14 +++----------- > 1 file changed, 3 insertions(+), 11 deletions(-) > > Index: linux-pm/drivers/acpi/pci_link.c > =================================================================== > --- linux-pm.orig/drivers/acpi/pci_link.c > +++ linux-pm/drivers/acpi/pci_link.c > @@ -877,17 +877,9 @@ static int __init acpi_irq_penalty_updat > */ > void acpi_penalize_isa_irq(int irq, int active) > { > - int penalty; > - > - if (irq < 0) > - return; > - > - if (active) > - penalty = PIRQ_PENALTY_ISA_USED; > - else > - penalty = PIRQ_PENALTY_PCI_USING; > - > - acpi_irq_add_penalty(irq, penalty); > + if (irq >= 0) > + acpi_irq_add_penalty(irq, active ? > + PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING); Works for me as well! > } > > bool acpi_isa_irq_available(int irq) > -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction @ 2016-01-04 13:34 ` Andy Shevchenko 0 siblings, 0 replies; 19+ messages in thread From: Andy Shevchenko @ 2016-01-04 13:34 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Sinan Kaya, Christopher Covington, linux-acpi, Timur Tabi, jcm, helgaas, Len Brown, linux-kernel On Mon, Jan 4, 2016 at 3:50 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Monday, January 04, 2016 11:01:05 AM Andy Shevchenko wrote: >> On Fri, Jan 1, 2016 at 2:48 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >> > On Wednesday, December 30, 2015 09:55:35 PM Andy Shevchenko wrote: >> >> On Wed, Dec 30, 2015 at 9:17 PM, Sinan Kaya <okaya@codeaurora.org> wrote: >> >> > On 12/30/2015 8:28 AM, Andy Shevchenko wrote: >> >> >> Yep, I meant not to use an additional variable. >> >> >> >> >> >>> > BTW, I suggest you spend some time around checkpatch for contributions. I could >> >> >>> > have caught most of the issues you are generally concerned before submitting a patch. >> >> >> Is it a question? >> >> > >> >> > It is a request not a question. I hate wasting your time and my time with things that I could >> >> > have fixed before submitting a patch. >> >> > >> >> > I ran the checkpatch and it said I'm good to go. But, obviously I'm not. >> >> >> >> Hmm… checkpatch.pl is just a small helper to fix style issues. Here is >> >> just a common sense rule, or kind of Occam's razor: no need to have >> >> more variables then needed if it doesn't improve something really >> >> significantly. >> > >> > That said, compilers optimize things anyway, so using an extra local variable >> > shouldn't matter for the resulting machine code. >> >> I'm not totally against that, but is the additional variable helpful here? > > Well, I guess you can argue both ways. > > Surely, the same result can be achieved with fewer lines of code if that's > what you mean, so what about the following change on top of the $subject patch? > > --- > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Subject: [PATCH] ACPI / PCI: Simplify acpi_penalize_isa_irq() > > acpi_penalize_isa_irq() can be written in fewer lines of code, > so do that. No functional change. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/acpi/pci_link.c | 14 +++----------- > 1 file changed, 3 insertions(+), 11 deletions(-) > > Index: linux-pm/drivers/acpi/pci_link.c > =================================================================== > --- linux-pm.orig/drivers/acpi/pci_link.c > +++ linux-pm/drivers/acpi/pci_link.c > @@ -877,17 +877,9 @@ static int __init acpi_irq_penalty_updat > */ > void acpi_penalize_isa_irq(int irq, int active) > { > - int penalty; > - > - if (irq < 0) > - return; > - > - if (active) > - penalty = PIRQ_PENALTY_ISA_USED; > - else > - penalty = PIRQ_PENALTY_PCI_USING; > - > - acpi_irq_add_penalty(irq, penalty); > + if (irq >= 0) > + acpi_irq_add_penalty(irq, active ? > + PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING); Works for me as well! > } > > bool acpi_isa_irq_available(int irq) > -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH V9 2/2] ACPI, PCI, irq: remove interrupt number restriction 2015-12-09 16:18 [PATCH V9 0/2] ACPI, PCI, irq: remove interrupt limitations Sinan Kaya 2015-12-09 16:18 ` [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction Sinan Kaya @ 2015-12-09 16:18 ` Sinan Kaya 2016-01-03 0:35 ` [PATCH V9 0/2] ACPI, PCI, irq: remove interrupt limitations Rafael J. Wysocki 2 siblings, 0 replies; 19+ messages in thread From: Sinan Kaya @ 2015-12-09 16:18 UTC (permalink / raw) To: linux-acpi, timur, cov, jcm, helgaas Cc: Sinan Kaya, Rafael J. Wysocki, Len Brown, linux-kernel The ACPI compiler uses the extended format when used interrupt numbers are greater than 15. The extended IRQ syntax is 32 bits according to the ACPI spec. The code supports parsing the extended interrupt numbers. However, due to used data structure type; the code silently truncates interrupt numbers greater than 256. Acked-by: Bjorn Helgaas <bhelgaas@google.com> Signed-off-by: Sinan Kaya <okaya@codeaurora.org> --- drivers/acpi/pci_link.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c index 0286f17..5114d80 100644 --- a/drivers/acpi/pci_link.c +++ b/drivers/acpi/pci_link.c @@ -68,12 +68,12 @@ static struct acpi_scan_handler pci_link_handler = { * later even the link is disable. Instead, we just repick the active irq */ struct acpi_pci_link_irq { - u8 active; /* Current IRQ */ + u32 active; /* Current IRQ */ u8 triggering; /* All IRQs */ u8 polarity; /* All IRQs */ u8 resource_type; u8 possible_count; - u8 possible[ACPI_PCI_LINK_MAX_POSSIBLE]; + u32 possible[ACPI_PCI_LINK_MAX_POSSIBLE]; u8 initialized:1; u8 reserved:7; }; -- Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH V9 0/2] ACPI, PCI, irq: remove interrupt limitations 2015-12-09 16:18 [PATCH V9 0/2] ACPI, PCI, irq: remove interrupt limitations Sinan Kaya 2015-12-09 16:18 ` [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction Sinan Kaya 2015-12-09 16:18 ` [PATCH V9 2/2] ACPI, PCI, irq: remove interrupt number restriction Sinan Kaya @ 2016-01-03 0:35 ` Rafael J. Wysocki 2 siblings, 0 replies; 19+ messages in thread From: Rafael J. Wysocki @ 2016-01-03 0:35 UTC (permalink / raw) To: Sinan Kaya; +Cc: linux-acpi, timur, cov, jcm, helgaas On Wednesday, December 09, 2015 11:18:26 AM Sinan Kaya wrote: > Code currently supports 256 maximum interrupts at this moment. The patch is > reconfiguring the penalty array as a dynamic list to remove this > limitation. > > The ACPI compiler uses the extended format when used interrupt numbers > are greater than 15. The extended IRQ is 32 bits according to the ACPI > spec. The code supports parsing the extended interrupt numbers. However, > due to used data structure type; the code silently truncates interrupt > numbers greater than 256. > > Changes from V8: (https://lkml.org/lkml/2015/12/3/579) > * split the patch into two as > ** ACPI, PCI, irq: remove interrupt count restriction > ** ACPI, PCI, irq: remove interrupt number restriction > * add acpi_irq_add_penalty API to simplify code > > Sinan Kaya (2): > ACPI, PCI, irq: remove interrupt count restriction > ACPI, PCI, irq: remove interrupt number restriction > > drivers/acpi/pci_link.c | 140 +++++++++++++++++++++++++++++++++++------------- > 1 file changed, 104 insertions(+), 36 deletions(-) Both patches applied, thanks! ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2016-01-04 13:50 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-09 16:18 [PATCH V9 0/2] ACPI, PCI, irq: remove interrupt limitations Sinan Kaya 2015-12-09 16:18 ` [PATCH V9 1/2] ACPI, PCI, irq: remove interrupt count restriction Sinan Kaya 2015-12-09 16:59 ` Andy Shevchenko 2015-12-09 17:09 ` Sinan Kaya 2015-12-09 17:14 ` Christopher Covington 2015-12-30 13:23 ` Sinan Kaya 2015-12-30 13:28 ` Andy Shevchenko 2015-12-30 19:17 ` Sinan Kaya 2015-12-30 19:55 ` Andy Shevchenko 2015-12-30 19:55 ` Andy Shevchenko 2016-01-01 0:48 ` Rafael J. Wysocki 2016-01-01 0:48 ` Rafael J. Wysocki 2016-01-04 9:01 ` Andy Shevchenko 2016-01-04 9:01 ` Andy Shevchenko 2016-01-04 13:50 ` Rafael J. Wysocki 2016-01-04 13:34 ` Andy Shevchenko 2016-01-04 13:34 ` Andy Shevchenko 2015-12-09 16:18 ` [PATCH V9 2/2] ACPI, PCI, irq: remove interrupt number restriction Sinan Kaya 2016-01-03 0:35 ` [PATCH V9 0/2] ACPI, PCI, irq: remove interrupt limitations Rafael J. Wysocki
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.