why do drivers evaluate _INI?

why do drivers evaluate _INI?

[Bjorn Helgaas]

[Sorry for the duplicate; I meant to CC: linux-acpi, so I added it here.]

Why do we have ACPI device drivers evaluating _INI?  That seems
like something that should be done by Linux/ACPI, not by the driver.

I see the following drivers using _INI:
  drivers/hwmon/hp_accel.c
  drivers/platform/x86/fujitsu-laptop.c
  drivers/platform/x86/sony-laptop.c

I looked at the git logs where the _INI usage was introduced in
these drivers, but none gives enough information for me to understand
why.

If running _INI in the driver makes a difference, I think it's
really telling us about a problem in Linux/ACPI, and we should
fix that problem rather than sprinkling _INI evaluation around
in drivers.

I do see _INI evaluation in this path:

    acpi_init
        acpi_bus_init
            acpi_initialize_objects(ACPI_FULL_INITIALIZATION)
                acpi_ns_initialize_devices
                    acpi_ns_walk_namespace .. acpi_ns_init_one_device

The spec (section 6.5.1) says OSPM should run _INI when a
description table is loaded.  I assume the above path does
this for the DSDT, at least, but I'm not smart enough about
the ACPI CA to know whether we also handle SSDTs and dynamic
LoadTables correctly.

Bjorn

P.S.  I'm about to go on vacation for a couple weeks, so I'll
be slow in responding to any discussion here.

Re: why do drivers evaluate _INI?

[Jonathan Woithe]

Hi

> Why do we have ACPI device drivers evaluating _INI?  That seems
> like something that should be done by Linux/ACPI, not by the driver.
> 
> I see the following drivers using _INI:
>   drivers/hwmon/hp_accel.c
>   drivers/platform/x86/fujitsu-laptop.c
>   drivers/platform/x86/sony-laptop.c
> 
> I looked at the git logs where the _INI usage was introduced in
> these drivers, but none gives enough information for me to understand
> why.

I can't shed a lot of light on this from the point of view of
fujitsu-laptop.

It appears that the evaluation of _INI was added to fujitsu-laptop as part
of the patch which added support for hotkeys present on some Fujitsu
laptops.  This patch was contributed by Peter Gruber - I've added him to the
CC.  Perhaps he might explain why he added _INI evaluation as part of this
patch (that is, what problem was encountered which required evaluation of
_INI).

Regards
  jonathan

Re: why do drivers evaluate _INI?

[Tony Vroon]

[-- Attachment #1: Type: text/plain, Size: 420 bytes --]

On Mon, 2009-06-22 at 17:32 -0600, Bjorn Helgaas wrote:
> I see the following drivers using _INI:
>   drivers/platform/x86/fujitsu-laptop.c

Bjorn, can you send me an experimental patch where you remove it? I can
regression test that for you on the Lifebook S6420.

I'm happy to sign off on it if I see no functional regressions, and if
not at least we'll have an idea of what to look for.

Regards,
Tony V.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: why do drivers evaluate _INI?

[Jonathan Woithe]

Hi Tony

> On Mon, 2009-06-22 at 17:32 -0600, Bjorn Helgaas wrote:
> > I see the following drivers using _INI:
> >   drivers/platform/x86/fujitsu-laptop.c
> 
> Bjorn, can you send me an experimental patch where you remove it? I can
> regression test that for you on the Lifebook S6420.

As far as I can tell it's just a trivial removal of the relevent section of
the acpi_*_add() functions.  A patch doing this is at the end of this email.
This is completely untested so there may be something subtle I've missed
(for instance, the local "handle" variable is unused now I suspect but this
won't stop compilation or testing).

The patch is against fujitsu-laptop.c from 2.6.27.9 (the kernel source tree
I happen to have quick access to at present) but I don't *think* a lot has
changed since then so it should apply to other recent versions.

Regards
  jonathan

--- fujitsu-laptop.c-orig	2009-06-23 09:51:08.540087000 +0930
+++ fujitsu-laptop.c	2009-06-23 09:53:54.491830386 +0930
@@ -537,15 +537,6 @@ static int acpi_fujitsu_add(struct acpi_
 
 	fujitsu->dev = device;
 
-	if (ACPI_SUCCESS
-	    (acpi_get_handle(device->handle, METHOD_NAME__INI, &handle))) {
-		vdbg_printk(FUJLAPTOP_DBG_INFO, "Invoking _INI\n");
-		if (ACPI_FAILURE
-		    (acpi_evaluate_object
-		     (device->handle, METHOD_NAME__INI, NULL, NULL)))
-			printk(KERN_ERR "_INI Method failed\n");
-	}
-
 	/* do config (detect defaults) */
 	dmi_check_system(fujitsu_dmi_table);
 	use_alt_lcd_levels = use_alt_lcd_levels == 1 ? 1 : 0;
@@ -764,15 +755,6 @@ static int acpi_fujitsu_hotkey_add(struc
 
 	fujitsu_hotkey->dev = device;
 
-	if (ACPI_SUCCESS
-	    (acpi_get_handle(device->handle, METHOD_NAME__INI, &handle))) {
-		vdbg_printk(FUJLAPTOP_DBG_INFO, "Invoking _INI\n");
-		if (ACPI_FAILURE
-		    (acpi_evaluate_object
-		     (device->handle, METHOD_NAME__INI, NULL, NULL)))
-			printk(KERN_ERR "_INI Method failed\n");
-	}
-
 	i = 0;			/* Discard hotkey ringbuffer */
 	while (get_irb() != 0 && (i++) < MAX_HOTKEY_RINGBUFFER_SIZE) ;
 	vdbg_printk(FUJLAPTOP_DBG_INFO, "Discarded %i ringbuffer entries\n", i);

Re: why do drivers evaluate _INI?

[Mattia Dongili]

On Mon, Jun 22, 2009 at 05:32:35PM -0600, Bjorn Helgaas wrote:
> [Sorry for the duplicate; I meant to CC: linux-acpi, so I added it here.]
> 
> Why do we have ACPI device drivers evaluating _INI?  That seems
> like something that should be done by Linux/ACPI, not by the driver.
> 
> I see the following drivers using _INI:
>   drivers/hwmon/hp_accel.c
>   drivers/platform/x86/fujitsu-laptop.c
>   drivers/platform/x86/sony-laptop.c
> 
> I looked at the git logs where the _INI usage was introduced in
> these drivers, but none gives enough information for me to understand
> why.
> 
> If running _INI in the driver makes a difference, I think it's
> really telling us about a problem in Linux/ACPI, and we should
> fix that problem rather than sprinkling _INI evaluation around
> in drivers.
> 
> I do see _INI evaluation in this path:
> 
>     acpi_init
>         acpi_bus_init
>             acpi_initialize_objects(ACPI_FULL_INITIALIZATION)
>                 acpi_ns_initialize_devices
>                     acpi_ns_walk_namespace .. acpi_ns_init_one_device
> 
> The spec (section 6.5.1) says OSPM should run _INI when a
> description table is loaded.  I assume the above path does
> this for the DSDT, at least, but I'm not smart enough about
> the ACPI CA to know whether we also handle SSDTs and dynamic
> LoadTables correctly.

well, it was long ago... I was going to say that when I added it it was because
ospm wasn't calling _INI on the SNY6001 device but it doesn't seem to be the
case... it was just my imagination I guess. I'll double check.

Anyway, from a quick look, _STA on the SNY6001 is always returning 0x0F
or 0x0D depending on the device being assigned an irq or not so ospm
should be good.  I think the _INI call can be removed for sony-laptop.

cheers
-- 
mattia
:wq!

Re: why do drivers evaluate _INI?

[Corentin Chary]

On Tue, Jun 23, 2009 at 1:32 AM, Bjorn Helgaas<bjorn.helgaas@hp.com> wrote:
> [Sorry for the duplicate; I meant to CC: linux-acpi, so I added it here.]
>
> Why do we have ACPI device drivers evaluating _INI?  That seems
> like something that should be done by Linux/ACPI, not by the driver.
>
> I see the following drivers using _INI:
>  drivers/hwmon/hp_accel.c
>  drivers/platform/x86/fujitsu-laptop.c
>  drivers/platform/x86/sony-laptop.c
>
> I looked at the git logs where the _INI usage was introduced in
> these drivers, but none gives enough information for me to understand
> why.
>
> If running _INI in the driver makes a difference, I think it's
> really telling us about a problem in Linux/ACPI, and we should
> fix that problem rather than sprinkling _INI evaluation around
> in drivers.
>
> I do see _INI evaluation in this path:
>
>    acpi_init
>        acpi_bus_init
>            acpi_initialize_objects(ACPI_FULL_INITIALIZATION)
>                acpi_ns_initialize_devices
>                    acpi_ns_walk_namespace .. acpi_ns_init_one_device
>
> The spec (section 6.5.1) says OSPM should run _INI when a
> description table is loaded.  I assume the above path does
> this for the DSDT, at least, but I'm not smart enough about
> the ACPI CA to know whether we also handle SSDTs and dynamic
> LoadTables correctly.
>
> Bjorn
>
> P.S.  I'm about to go on vacation for a couple weeks, so I'll
> be slow in responding to any discussion here.
> --
> 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
>

Hi,
I don't know if it's related, but:
eeepc-laptop is calling "INIT" for device ASUS010
asus-laptop is calling "INIT" for device ATK0100

But for eeepc-laptop we provide some flags special to INIT, and in
asus-laptop it returns the model name.
So I don't think we can move INIT for these.

-- 
Corentin Chary
http://xf.iksaif.net - http://uffs.org
--
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

Re: why do drivers evaluate _INI?

[Bjorn Helgaas]

On Tuesday 23 June 2009 2:53:07 am Corentin Chary wrote:
> I don't know if it's related, but:
> eeepc-laptop is calling "INIT" for device ASUS010
> asus-laptop is calling "INIT" for device ATK0100
> 
> But for eeepc-laptop we provide some flags special to INIT, and in
> asus-laptop it returns the model name.
> So I don't think we can move INIT for these.

Thanks for looking into this.  The "INIT" methods are something
completely different -- they are device-specific things, so it's
fine for them to be in the driver.  The "_INI" method, like others
named with a leading underscore, is defined by the ACPI spec.
Drivers do even use some of these spec-defined methods, but in
the case of "_INI", I think it makes more sense to take care of
it in the ACPI CA or the Linux/ACPI core.

Bjorn

Re: why do drivers evaluate _INI?

[nokos]

Am Dienstag, den 23.06.2009, 09:29 +0930 schrieb Jonathan Woithe:
> Hi
> 
> > Why do we have ACPI device drivers evaluating _INI?  That seems
> > like something that should be done by Linux/ACPI, not by the driver.
> > 
> > I see the following drivers using _INI:
> >   drivers/hwmon/hp_accel.c
> >   drivers/platform/x86/fujitsu-laptop.c
> >   drivers/platform/x86/sony-laptop.c
> > 
> > I looked at the git logs where the _INI usage was introduced in
> > these drivers, but none gives enough information for me to understand
> > why.
> 
> I can't shed a lot of light on this from the point of view of
> fujitsu-laptop.
> 
> It appears that the evaluation of _INI was added to fujitsu-laptop as part
> of the patch which added support for hotkeys present on some Fujitsu
> laptops.  This patch was contributed by Peter Gruber - I've added him to the
> CC.  Perhaps he might explain why he added _INI evaluation as part of this
> patch (that is, what problem was encountered which required evaluation of
> _INI).

The hotkey communication between hardware and the drivers is done using
acpi notifies and a (small) ringbuffer in the acpi dsdt code. The
easiest way to assure a consistent. The _INI method simply resets the
internal state of the ringbuffer which seems to be unavailable for
direct manipulation. Another possible path would be to read and discard
the ringbuffer on module load to ensure a consistent state.  

Peter

Re: why do drivers evaluate _INI?

[Jonathan Woithe]

Hi Peter

> > > Why do we have ACPI device drivers evaluating _INI?  That seems
> > > like something that should be done by Linux/ACPI, not by the driver.
> > > 
> > > I see the following drivers using _INI:
> > >   drivers/hwmon/hp_accel.c
> > >   drivers/platform/x86/fujitsu-laptop.c
> > >   drivers/platform/x86/sony-laptop.c
> > > 
> > > I looked at the git logs where the _INI usage was introduced in
> > > these drivers, but none gives enough information for me to understand
> > > why.
> > 
> > I can't shed a lot of light on this from the point of view of
> > fujitsu-laptop.
> > 
> > It appears that the evaluation of _INI was added to fujitsu-laptop as part
> > of the patch which added support for hotkeys present on some Fujitsu
> > laptops.  This patch was contributed by Peter Gruber - I've added him to the
> > CC.  Perhaps he might explain why he added _INI evaluation as part of this
> > patch (that is, what problem was encountered which required evaluation of
> > _INI).
> 
> The hotkey communication between hardware and the drivers is done using
> acpi notifies and a (small) ringbuffer in the acpi dsdt code. The
> easiest way to assure a consistent. The _INI method simply resets the
> internal state of the ringbuffer which seems to be unavailable for
> direct manipulation.

Thanks for this info Peter.  So evaluating _INI is required to ensure we
start with an empty ringbuffer and don't end up with partial sequences
and/or old keypresses being processed when the module is loaded - right?

> Another possible path would be to read and discard the ringbuffer on
> module load to ensure a consistent state.

True - but is this really any neater than calling _INI in this context?  In
addition, is there a definitive way to know that we have a consistant
ringbuffer state?  If not we won't know when to stop reading.

Having said that, the _INI method is called by acpi_ns_initialize_devices()
which I assume forms part of the APCI core initialisation.  Presumedly there
won't be a great deal of time elapsed between this and the initialisation of
fujitsu-laptop in most cases.  If fujitsu-laptop didn't call _INI (as per
the quick and dirty patch yesterday) there seems little scope for a real
problem.  The only time when it *could* be an issue is when considerable
time elapsed between ACPI initialisation and the loading of fujitsu-laptop -
something which in practice is only going to apply to the machines of
developers.

So, given this background, is there likely to be any real-world effect
caused by the removal of the _INI call in fujitsu-laptop?

Regards
  jonathan

Re: why do drivers evaluate _INI?

[nokos]

Am Mittwoch, den 24.06.2009, 09:45 +0930 schrieb Jonathan Woithe:
> Hi Peter
> 
> > > > Why do we have ACPI device drivers evaluating _INI?  That seems
> > > > like something that should be done by Linux/ACPI, not by the driver.
> > > > 
> > > > I see the following drivers using _INI:
> > > >   drivers/hwmon/hp_accel.c
> > > >   drivers/platform/x86/fujitsu-laptop.c
> > > >   drivers/platform/x86/sony-laptop.c
> > > > 
> > > > I looked at the git logs where the _INI usage was introduced in
> > > > these drivers, but none gives enough information for me to understand
> > > > why.
> > > 
> > > I can't shed a lot of light on this from the point of view of
> > > fujitsu-laptop.
> > > 
> > > It appears that the evaluation of _INI was added to fujitsu-laptop as part
> > > of the patch which added support for hotkeys present on some Fujitsu
> > > laptops.  This patch was contributed by Peter Gruber - I've added him to the
> > > CC.  Perhaps he might explain why he added _INI evaluation as part of this
> > > patch (that is, what problem was encountered which required evaluation of
> > > _INI).
> > 
> > The hotkey communication between hardware and the drivers is done using
> > acpi notifies and a (small) ringbuffer in the acpi dsdt code. The
> > easiest way to assure a consistent. The _INI method simply resets the
> > internal state of the ringbuffer which seems to be unavailable for
> > direct manipulation.
> 
> Thanks for this info Peter.  So evaluating _INI is required to ensure we
> start with an empty ringbuffer and don't end up with partial sequences
> and/or old keypresses being processed when the module is loaded - right?
> 
> > Another possible path would be to read and discard the ringbuffer on
> > module load to ensure a consistent state.
> 
> True - but is this really any neater than calling _INI in this context?  In
> addition, is there a definitive way to know that we have a consistant
> ringbuffer state?  If not we won't know when to stop reading.

usually you stop reading if the out pointer is at the in pointer

> Having said that, the _INI method is called by acpi_ns_initialize_devices()
> which I assume forms part of the APCI core initialisation.  Presumedly there
> won't be a great deal of time elapsed between this and the initialisation of
> fujitsu-laptop in most cases.  If fujitsu-laptop didn't call _INI (as per
> the quick and dirty patch yesterday) there seems little scope for a real
> problem.  The only time when it *could* be an issue is when considerable
> time elapsed between ACPI initialisation and the loading of fujitsu-laptop -
> something which in practice is only going to apply to the machines of
> developers.
> 
> So, given this background, is there likely to be any real-world effect
> caused by the removal of the _INI call in fujitsu-laptop?

probably not ... though there seem to be some (quasi-)periodic effect
which write non-button(release) events into the ringbuffer and the the
ringbuffer is quite small. So you might loose some key-up events if a
key is pressed while the the modules is not loaded yet (and it is loaded
some minutes after the ACPI init) ... key-down is never a problem since
its specific.
It only might be a bit surprising if all keypresses which happened
between ACPI init and module loading happen at the first keypress after
module load.


Peter

> Regards
>   jonathan