_DIS ACPI PCI Interrupt Links

_DIS ACPI PCI Interrupt Links

[Len Brown]

We've had the reference counting on Links disabled
since 2.6.12.  Time to try to enable it and re-learn
why we disabled it back then:-)

This allows links to get back into the _DISabled state
when drivers free IRQs.  They may free IRQs at
unload time, or at suspend time.

For suspend, this allows us to "act more like boot",
in that links start off in _DIS and are programmed
as the drivers request them.

irqrouter_resume() will still restore the
links that were not in the _DIS state at suspend.
(Though I really don't like this routine, and how
 it runs AML with irqs disabled)

One opportunity for a bug on resume we may have is
that we don't actually force all the links to _DIS
before we start programming them as we do at boot time.
We can add that if it turns out to be a problem --
but would do it with interrupts on, like we do at boot...

Anyway, this all works for me...
If Rafael comes through with PCI Core code to
free IRQ at suspend for all devices, then we may
find outselves very close to the state where all
the links are disabled at suspend and irqrouter_resume()
has nothing to do.


cheers,
-- Len Brown - Intel Open Source Technology Center

[PATCH 1/2] ACPI: update pci_link debug messages

[Len Brown]

From: Len Brown <len.brown@intel.com>

debug message format/content, no functional change

Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/pci_link.c |   28 ++++++++++++++++++----------
 1 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
index e52ad91..595b131 100644
--- a/drivers/acpi/pci_link.c
+++ b/drivers/acpi/pci_link.c
@@ -414,7 +414,8 @@ static int acpi_pci_link_set(struct acpi_pci_link *link, int irq)
 		link->irq.active = irq;
 	}
 
-	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Set IRQ %d\n", link->irq.active));
+	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Link %s set to IRQ %d\n",
+		acpi_device_bid(link->device), link->irq.active));
 
       end:
 	kfree(resource);
@@ -649,6 +650,9 @@ acpi_pci_link_allocate_irq(acpi_handle handle,
 		return -1;
 	}
 	link->refcnt++;
+	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+			  "Link %s is referenced %d\n",
+			  acpi_device_bid(link->device), link->refcnt));
 	mutex_unlock(&acpi_link_lock);
 
 	if (triggering)
@@ -657,9 +661,6 @@ acpi_pci_link_allocate_irq(acpi_handle handle,
 		*polarity = link->irq.polarity;
 	if (name)
 		*name = acpi_device_bid(link->device);
-	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-			  "Link %s is referenced\n",
-			  acpi_device_bid(link->device)));
 	return (link->irq.active);
 }
 
@@ -705,10 +706,13 @@ int acpi_pci_link_free_irq(acpi_handle handle)
 	link->refcnt--;
 #endif
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-			  "Link %s is dereferenced\n",
-			  acpi_device_bid(link->device)));
+			  "Link %s is dereferenced %d\n",
+			  acpi_device_bid(link->device), link->refcnt));
 
 	if (link->refcnt == 0) {
+		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+			  "Link %s is disabled\n",
+			  acpi_device_bid(link->device)));
 		acpi_evaluate_object(link->device->handle, "_DIS", NULL, NULL);
 	}
 	mutex_unlock(&acpi_link_lock);
@@ -782,13 +786,17 @@ static int acpi_pci_link_add(struct acpi_device *device)
 	return result;
 }
 
-static int acpi_pci_link_resume(struct acpi_pci_link *link)
+static void acpi_pci_link_resume(struct acpi_pci_link *link)
 {
 
+	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Link %s resume ref %d act %d ini %d\n",
+		acpi_device_bid(link->device), link->refcnt,
+		link->irq.active, link->irq.initialized));
+
 	if (link->refcnt && link->irq.active && link->irq.initialized)
-		return (acpi_pci_link_set(link, link->irq.active));
-	else
-		return 0;
+		acpi_pci_link_set(link, link->irq.active);
+
+	return;
 }
 
 static int irqrouter_resume(struct sys_device *dev)
-- 
1.5.6.5

[PATCH 2/2] ACPI: PCI Interrupt Links -- disable when unused

[Len Brown]

From: Len Brown <len.brown@intel.com>

We start the system off with disabled links.
Here we enable the reference counting
on the links so that when drivers free IRQs
the links can return to the disabled state.

Drivers free their IRQ and reference on a link
when they unload.  They may also do so upon
suspend.  However, if they don't, irqrouter_resume()
will still resume any links that were
not disabled before suspend.

Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/pci_link.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
index 595b131..7d12762 100644
--- a/drivers/acpi/pci_link.c
+++ b/drivers/acpi/pci_link.c
@@ -693,6 +693,7 @@ int acpi_pci_link_free_irq(acpi_handle handle)
 		printk(KERN_ERR PREFIX "Link isn't initialized\n");
 		return -1;
 	}
+<<<<<<< HEAD:drivers/acpi/pci_link.c
 #ifdef	FUTURE_USE
 	/*
 	 * The Link reference count allows us to _DISable an unused link
@@ -703,8 +704,9 @@ int acpi_pci_link_free_irq(acpi_handle handle)
 	 * to prevent duplicate acpi_pci_link_set()
 	 * which would harm some systems
 	 */
+=======
+>>>>>>> 683984a... ACPI: PCI Interrupt Links -- disable when unused:drivers/acpi/pci_link.c
 	link->refcnt--;
-#endif
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 			  "Link %s is dereferenced %d\n",
 			  acpi_device_bid(link->device), link->refcnt));
-- 
1.5.6.5

Re: [PATCH 2/2] ACPI: PCI Interrupt Links -- disable when unused

[Yinghai Lu]

On Fri, Dec 19, 2008 at 12:22 AM, Len Brown <lenb@kernel.org> wrote:
> From: Len Brown <len.brown@intel.com>
>
> We start the system off with disabled links.
> Here we enable the reference counting
> on the links so that when drivers free IRQs
> the links can return to the disabled state.
>
> Drivers free their IRQ and reference on a link
> when they unload.  They may also do so upon
> suspend.  However, if they don't, irqrouter_resume()
> will still resume any links that were
> not disabled before suspend.
>
> Signed-off-by: Len Brown <len.brown@intel.com>
> ---
>  drivers/acpi/pci_link.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
> index 595b131..7d12762 100644
> --- a/drivers/acpi/pci_link.c
> +++ b/drivers/acpi/pci_link.c
> @@ -693,6 +693,7 @@ int acpi_pci_link_free_irq(acpi_handle handle)
>                printk(KERN_ERR PREFIX "Link isn't initialized\n");
>                return -1;
>        }
> +<<<<<<< HEAD:drivers/acpi/pci_link.c
>  #ifdef FUTURE_USE
>        /*
>         * The Link reference count allows us to _DISable an unused link
> @@ -703,8 +704,9 @@ int acpi_pci_link_free_irq(acpi_handle handle)
>         * to prevent duplicate acpi_pci_link_set()
>         * which would harm some systems
>         */
> +=======
> +>>>>>>> 683984a... ACPI: PCI Interrupt Links -- disable when unused:drivers/acpi/pci_link.c
>        link->refcnt--;
> -#endif

looks like merge conflicts fix.

YH

Re: [PATCH 2/2] ACPI: PCI Interrupt Links -- disable when unused

[Len Brown]

From: Len Brown <len.brown@intel.com>
Organization: Intel Open Source Technology Center

We start the system off with disabled links.
Here we enable the reference counting
on the links so that when drivers free IRQs
the links can return to the disabled state.

Drivers free their IRQ and reference on a link
when they unload.  They may also do so upon
suspend.  However, if they don't, irqrouter_resume()
will still resume any links that were
not disabled before suspend.

Signed-off-by: Len Brown <len.brown@intel.com>
---
Hmmm, look like I confused git-rebase on that one,
here it is again...

thanks,
-Len
---
 drivers/acpi/pci_link.c |   11 -----------
 1 files changed, 0 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
index 595b131..0f7722d 100644
--- a/drivers/acpi/pci_link.c
+++ b/drivers/acpi/pci_link.c
@@ -693,18 +693,7 @@ int acpi_pci_link_free_irq(acpi_handle handle)
 		printk(KERN_ERR PREFIX "Link isn't initialized\n");
 		return -1;
 	}
-#ifdef	FUTURE_USE
-	/*
-	 * The Link reference count allows us to _DISable an unused link
-	 * and suspend time, and set it again  on resume.
-	 * However, 2.6.12 still has irq_router.resume
-	 * which blindly restores the link state.
-	 * So we disable the reference count method
-	 * to prevent duplicate acpi_pci_link_set()
-	 * which would harm some systems
-	 */
 	link->refcnt--;
-#endif
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 			  "Link %s is dereferenced %d\n",
 			  acpi_device_bid(link->device), link->refcnt));
-- 
1.6.1.rc3.35.gc0ceb

Re: [PATCH 2/2] ACPI: PCI Interrupt Links -- disable when unused

[Yinghai Lu]

On Fri, Dec 19, 2008 at 1:33 AM, Len Brown <lenb@kernel.org> wrote:
> From: Len Brown <len.brown@intel.com>
> Organization: Intel Open Source Technology Center
>
> We start the system off with disabled links.
> Here we enable the reference counting
> on the links so that when drivers free IRQs
> the links can return to the disabled state.
>
> Drivers free their IRQ and reference on a link
> when they unload.  They may also do so upon
> suspend.  However, if they don't, irqrouter_resume()
> will still resume any links that were
> not disabled before suspend.
>
> Signed-off-by: Len Brown <len.brown@intel.com>
> ---
> Hmmm, look like I confused git-rebase on that one,
> here it is again...
>
> thanks,
> -Len
> ---
>  drivers/acpi/pci_link.c |   11 -----------
>  1 files changed, 0 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
> index 595b131..0f7722d 100644
> --- a/drivers/acpi/pci_link.c
> +++ b/drivers/acpi/pci_link.c
> @@ -693,18 +693,7 @@ int acpi_pci_link_free_irq(acpi_handle handle)
>                printk(KERN_ERR PREFIX "Link isn't initialized\n");
>                return -1;
>        }
> -#ifdef FUTURE_USE
> -       /*
> -        * The Link reference count allows us to _DISable an unused link
> -        * and suspend time, and set it again  on resume.
> -        * However, 2.6.12 still has irq_router.resume
> -        * which blindly restores the link state.
> -        * So we disable the reference count method
> -        * to prevent duplicate acpi_pci_link_set()
> -        * which would harm some systems
> -        */
>        link->refcnt--;
> -#endif
>        ACPI_DEBUG_PRINT((ACPI_DB_INFO,
>                          "Link %s is dereferenced %d\n",
>                          acpi_device_bid(link->device), link->refcnt));
> --

maybe could have one switch in /proc so could not disable that for
some kexec path...

YH

Re: [PATCH 2/2] ACPI: PCI Interrupt Links -- disable when unused

[Len Brown]

> >  drivers/acpi/pci_link.c |   11 -----------
> >  1 files changed, 0 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
> > index 595b131..0f7722d 100644
> > --- a/drivers/acpi/pci_link.c
> > +++ b/drivers/acpi/pci_link.c
> > @@ -693,18 +693,7 @@ int acpi_pci_link_free_irq(acpi_handle handle)
> >                printk(KERN_ERR PREFIX "Link isn't initialized\n");
> >                return -1;
> >        }
> > -#ifdef FUTURE_USE
> > -       /*
> > -        * The Link reference count allows us to _DISable an unused link
> > -        * and suspend time, and set it again  on resume.
> > -        * However, 2.6.12 still has irq_router.resume
> > -        * which blindly restores the link state.
> > -        * So we disable the reference count method
> > -        * to prevent duplicate acpi_pci_link_set()
> > -        * which would harm some systems
> > -        */
> >        link->refcnt--;
> > -#endif
> >        ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> >                          "Link %s is dereferenced %d\n",
> >                          acpi_device_bid(link->device), link->refcnt));
> > --
> 
> maybe could have one switch in /proc so could not disable that for
> some kexec path...

I fail to comprehend the benefits of kexec,
and the requirements that kexec puts on the kernel, other than:

# CONFIG_KEXEC is not set

-- Len Brown, Intel Open Source Technology Center

Re: [PATCH 2/2] ACPI: PCI Interrupt Links -- disable when unused

[Yinghai Lu]

>> maybe could have one switch in /proc so could not disable that for
>> some kexec path...
> 
> I fail to comprehend the benefits of kexec,
> and the requirements that kexec puts on the kernel, other than:
> 
> # CONFIG_KEXEC is not set
> 

for me: it is a tools that i could use to make sure root-cause is in FW, and BIOS guys will not kick back the ball to os team.
after modifying table or hw reg in first kernel, and kexec even stock kernel, everything will work well.

YH

why kexec (was Re: [PATCH 2/2] ACPI: PCI Interrupt Links -- disable when unused)

[Len Brown]

> >> maybe could have one switch in /proc so could not disable that for
> >> some kexec path...
> > 
> > I fail to comprehend the benefits of kexec,
> > and the requirements that kexec puts on the kernel, other than:
> > 
> > # CONFIG_KEXEC is not set
> > 
> 
> for me: it is a tools that i could use to make sure root-cause is in FW, and BIOS guys will not kick back the ball to os team.
> after modifying table or hw reg in first kernel, and kexec even stock kernel, everything will work well.

For ACPI, we already have the ability to override the BIOS tables
upon the 1st boot.

I don't know which BIOS guys you refer to,
but when we find a BIOS bug and have access to
the associated BIOS developer, we've never needed
to do such a demonstration to convince them they have a bug.

kexec seems like a science project that has a chance of working
only under extremely controlled conditions.
I have no problem with that, as long as it is not built into my kernel.

But say kexec is useful to somebody out there -- what are
the requirements that kexec puts on the kernel?

-Len

Re: why kexec (was Re: [PATCH 2/2] ACPI: PCI Interrupt Links -- disable when unused)

[Yinghai Lu]

On Thu, Jan 8, 2009 at 10:55 PM, Len Brown <lenb@kernel.org> wrote:
>> >> maybe could have one switch in /proc so could not disable that for
>> >> some kexec path...
>> >
>> > I fail to comprehend the benefits of kexec,
>> > and the requirements that kexec puts on the kernel, other than:
>> >
>> > # CONFIG_KEXEC is not set
>> >
>>
>> for me: it is a tools that i could use to make sure root-cause is in FW, and BIOS guys will not kick back the ball to os team.
>> after modifying table or hw reg in first kernel, and kexec even stock kernel, everything will work well.
>
> For ACPI, we already have the ability to override the BIOS tables
> upon the 1st boot.
>
> I don't know which BIOS guys you refer to,
> but when we find a BIOS bug and have access to
> the associated BIOS developer, we've never needed
> to do such a demonstration to convince them they have a bug.

Your BIOS developer must have enough time or have bunch of BIOS engineers.

>
> kexec seems like a science project that has a chance of working
> only under extremely controlled conditions.
> I have no problem with that, as long as it is not built into my kernel.

could reduce boot time.
or some system has problem with reset...

or linuxbios + tinykernel to kexec final production kernel.
here linuxbios only init ram and basic pci resource allocation ...

>
> But say kexec is useful to somebody out there -- what are
> the requirements that kexec puts on the kernel?

the support got into the mainline kernel several years ago... from 2.6.16?

YH

Re: why kexec (was Re: [PATCH 2/2] ACPI: PCI Interrupt Links -- disable when unused)

[Len Brown]

> On Thu, Jan 8, 2009 at 10:55 PM, Len Brown <lenb@kernel.org> wrote:
> >> >> maybe could have one switch in /proc so could not disable that for
> >> >> some kexec path...
> >> >
> >> > I fail to comprehend the benefits of kexec,
> >> > and the requirements that kexec puts on the kernel, other than:
> >> >
> >> > # CONFIG_KEXEC is not set
> >> >
> >>
> >> for me: it is a tools that i could use to make sure root-cause is in FW, and BIOS guys will not kick back the ball to os team.
> >> after modifying table or hw reg in first kernel, and kexec even stock kernel, everything will work well.
> >
> > For ACPI, we already have the ability to override the BIOS tables
> > upon the 1st boot.
> >
> > I don't know which BIOS guys you refer to,
> > but when we find a BIOS bug and have access to
> > the associated BIOS developer, we've never needed
> > to do such a demonstration to convince them they have a bug.
> 
> Your BIOS developer must have enough time or have bunch of BIOS engineers.

Reasonable engineers do not need their own bugs
handed to them on a silver platter.

> > kexec seems like a science project that has a chance of working
> > only under extremely controlled conditions.
> > I have no problem with that, as long as it is not built into my kernel.
> 
> could reduce boot time.

Effort would be better spent on reducing boot time w/ kexec.

> or some system has problem with reset...
> 
> or linuxbios + tinykernel to kexec final production kernel.
> here linuxbios only init ram and basic pci resource allocation ...

My understanding is that the linuxbios people realized
that creating a BIOS that boots Linux (tiny or not)
only to then boot Linux again, was sort of subgenius.
So they re-named linuxbios to be coreboot and now
they talk about loading a "payload"....

> > But say kexec is useful to somebody out there -- what are
> > the requirements that kexec puts on the kernel?
> 
> the support got into the mainline kernel several years ago... from 2.6.16?

I'm afraid I'm not being clear.
This conversation started when you suggested that we should
put a hook in to ask ACPI to manipulate Link state
for the benefit of kexec.

My question is, what else does kexec need to properly
manage the state of the sytem?

I don't think that question has an answer.
I don't think it will ever have an answer.
I think that kexec is a viable concept only
under conditions I don't care about.

-Len