PNP: Allow PNP resources to be disabled (interface)

Hello everybody,

this simple patch series continues the work begun in commit 18fd470a48396c8795ba7256c5973e92ffa25cb3 where ACPI PNP resource templates with empty/disabled resources are handled.

The aim of this patch series is to allow to set resources as "disabled" using the "/sys/bus/pnp/devices/*/resources" interface. Such "disabled" resources are needed by some vintage IBM ThinkPads like the 600E where some devices need to have their IRQs disabled in order to support all the devices the 600E has.

To better understand the motivation, let's look at an excerpt from the 600E's DSDT:

    Name (PLPT, ResourceTemplate ()
    {
        StartDependentFnNoPri ()
        {
            IO (Decode16, 0x03BC, 0x03BC, 0x01, 0x04)
            IRQNoFlags () {7}
        }
        /* Some entries deleted */
        StartDependentFnNoPri ()
        {
            IO (Decode16, 0x03BC, 0x03BC, 0x01, 0x04)
            IRQNoFlags () {}
        }
        EndDependentFn ()
    })

As one can see, the IRQ line for the last option is empty/disabled. Also, both options share the same priority, meaning they are equal alternatives. In order to be able to use the IRQ 7 for some other device, it is necessary to select the second option, which can be done with the patch series applied.

Hi,

Do I understand correctly that you want to disable those things through sysfs?

Rafael

the aim is to select a PNP ACPI option where resources can be disabled (or are not needed). E.g., the parallel port of the 600E can be used with and without IRQ lines. The means to allow for this is to use the sysfs interface to select disabled resources (just like any other resource value). In https://lkml.org/lkml/2011/7/3/41, I used the following example:

echo disable > /sys/bus/pnp/devices/$device/resources echo clear > /sys/bus/pnp/devices/$device/resources echo set irq disabled > /sys/bus/pnp/devices/$device/resources echo fill > /sys/bus/pnp/devices/$device/resources echo activate > /sys/bus/pnp/devices/$device/resources

The third line is made possible by the patch series. All other lines are already implemented.

Quoted 7 lines

the aim is to select a PNP ACPI option where resources can be disabled (or are not needed). E.g., the parallel port of the 600E can be used with and without IRQ lines. The means to allow for this is to use the sysfs interface to select disabled resources (just like any other resource value). In https://lkml.org/lkml/2011/7/3/41, I used the following example:

Quoted 8 lines

echo disable > /sys/bus/pnp/devices/$device/resources echo clear > /sys/bus/pnp/devices/$device/resources echo set irq disabled > /sys/bus/pnp/devices/$device/resources echo fill > /sys/bus/pnp/devices/$device/resources echo activate > /sys/bus/pnp/devices/$device/resources

The third line is made possible by the patch series. All other lines are already implemented.

Shouldn't this be rather "disable_irq" or something which is a single word and thus would simplify parsing a lot?

Also, <Documentation/filesystems/sysfs.txt> says

"Attributes should be ASCII text files, preferably with only one value per file. It is noted that it may not be efficient to contain only one value per file, so it is socially acceptable to express an array of values of the same type."

Thanks.

Signature · 2 lines

Regards/Gruss,
    Boris.

Quoted 13 lines · Witold Szczeponik

the aim is to select a PNP ACPI option where resources can be disabled (or are not needed). E.g., the parallel port of the 600E can be used with and without IRQ lines. The means to allow for this is to use the sysfs interface to select disabled resources (just like any other resource value). In https://lkml.org/lkml/2011/7/3/41, I used the following example:

echo disable > /sys/bus/pnp/devices/$device/resources echo clear > /sys/bus/pnp/devices/$device/resources echo set irq disabled > /sys/bus/pnp/devices/$device/resources echo fill > /sys/bus/pnp/devices/$device/resources echo activate > /sys/bus/pnp/devices/$device/resources

Quoted 2 lines

The third line is made possible by the patch series. All other lines are already implemented.

Shouldn't this be rather "disable_irq" or something which is a single word and thus would simplify parsing a lot?

Quoted 17 lines · Borislav Petkov

Quoted 15 lines · Witold Szczeponik

the aim is to select a PNP ACPI option where resources can be disabled (or are not needed). E.g., the parallel port of the 600E can be used with and without IRQ lines. The means to allow for this is to use the sysfs interface to select disabled resources (just like any other resource value). In https://lkml.org/lkml/2011/7/3/41, I used the following example:

echo disable > /sys/bus/pnp/devices/$device/resources echo clear > /sys/bus/pnp/devices/$device/resources echo set irq disabled > /sys/bus/pnp/devices/$device/resources echo fill > /sys/bus/pnp/devices/$device/resources echo activate > /sys/bus/pnp/devices/$device/resources

The third line is made possible by the patch series. All other lines are already implemented.

Quoted 2 lines

Shouldn't this be rather "disable_irq" or something which is a single word and thus would simplify parsing a lot?

Or just "irq", which isn't going to be confused with anything else it seems.

Thanks,
Rafael

Quoted 18 lines · Rafael J. Wysocki

Quoted 17 lines · Borislav Petkov

Quoted 15 lines · Witold Szczeponik

the aim is to select a PNP ACPI option where resources can be disabled (or are not needed). E.g., the parallel port of the 600E can be used with and without IRQ lines. The means to allow for this is to use the sysfs interface to select disabled resources (just like any other resource value). In https://lkml.org/lkml/2011/7/3/41, I used the following example:

echo disable > /sys/bus/pnp/devices/$device/resources echo clear > /sys/bus/pnp/devices/$device/resources echo set irq disabled > /sys/bus/pnp/devices/$device/resources echo fill > /sys/bus/pnp/devices/$device/resources echo activate > /sys/bus/pnp/devices/$device/resources

The third line is made possible by the patch series. All other lines are already implemented.

Quoted 8 lines

Quoted 2 lines

Shouldn't this be rather "disable_irq" or something which is a single word and thus would simplify parsing a lot?

Or just "irq", which isn't going to be confused with anything else it seems.

Thanks,
Rafael

Hi Rafael,

the code in "drivers/pnp/interface.c" implements a (non-trivial) interface which accepts the keywords "disable", "activate", "fill", "auto", "clear", and "get" as simple, one word commands. The remaining "set" command is more complex, for it determines which resource is to be set ("io", "mem", "irq", "dma", and "bus"), followed by the actual value(s) of this resource (e.g., "0x0200-0x021f", or "7").

The patch series allows to use the term "disabled" or "<none>" as a resource value (c.f. my example above) when needed (c.f. my motivation for the patch series).

We could, of course, change the parser in "interface.c", but this would change the ABI, I am afraid. Something that I'd rather not do...

I hope, this makes the scope of the patch series clear(er).

Quoted 3 lines · Witold Szczeponik

Quoted 2 lines · Rafael J. Wysocki

Quoted 1 line · Borislav Petkov

On Sun, Jul 29, 2012 at 09:31:53PM +0200, Witold Szczeponik wrote:

Quoted 19 lines

Quoted 9 lines

Quoted 3 lines

Shouldn't this be rather "disable_irq" or something which is a single word and thus would simplify parsing a lot?

Or just "irq", which isn't going to be confused with anything else it seems.

Thanks,
Rafael

Hi Rafael,

the code in "drivers/pnp/interface.c" implements a (non-trivial) interface which accepts the keywords "disable", "activate", "fill", "auto", "clear", and "get" as simple, one word commands. The remaining "set" command is more complex, for it determines which resource is to be set ("io", "mem", "irq", "dma", and "bus"), followed by the actual value(s) of this resource (e.g., "0x0200-0x021f", or "7").

Quoted 6 lines

The patch series allows to use the term "disabled" or "<none>" as a resource value (c.f. my example above) when needed (c.f. my motivation for the patch series).

We could, of course, change the parser in "interface.c", but this would change the ABI, I am afraid. Something that I'd rather not do...

Still, you _are_ doing that by extending the ABI, aren't you?

Quoted 2 lines

I hope, this makes the scope of the patch series clear(er).

Yes, it does, thanks.

My opinion is that the whole interface is wrong and should be changed. How to do that is a different matter that would require some consideration. Perhaps the least painful way would be to add a new, hopefully better, interface along with the old one and then deprecate the latter at one point.

Now, since I don't like the existing interface, I'd prefer it not to be extended.

Thanks,
Rafael

Hi Rafael,

what about the patches 1 and 3 which do not make any changes to the ABI? The first patch simplifies the code, while the third patch fixes a problem in the PNP resource allocation. Any chances to have them included in 3.7?

Hi,

Quoted 2 lines · Witold Szczeponik

Hi Rafael,

Quoted 3 lines

what about the patches 1 and 3 which do not make any changes to the ABI? The first patch simplifies the code, while the third patch fixes a problem in the PNP resource allocation. Any chances to have them included in 3.7?

They would be fine as far as I'm concerned, but I'm not maintaining that part of the kernel (at least at the moment).

I'm not sure who's the maintainer of it, to be honest.

Thanks,
Rafael

Len and Bjorn,

any chances to include the patches in 3.7? I checked them against 3.6 and they applied without any change. Or should I resend them individually?

Thanks.

--- Witold

Hi,

Quoted 2 lines · Witold Szczeponik

Hi Rafael,

Quoted 3 lines

what about the patches 1 and 3 which do not make any changes to the ABI? The first patch simplifies the code, while the third patch fixes a problem in the PNP resource allocation. Any chances to have them included in 3.7?

They would be fine as far as I'm concerned, but I'm not maintaining that part of the kernel (at least at the moment).

I'm not sure who's the maintainer of it, to be honest.

Thanks,
Rafael

Quoted 12 lines · Rafael J. Wysocki

Hi,

Quoted 5 lines · Witold Szczeponik

Hi Rafael,

what about the patches 1 and 3 which do not make any changes to the ABI? The first patch simplifies the code, while the third patch fixes a problem in the PNP resource allocation. Any chances to have them included in 3.7?

They would be fine as far as I'm concerned, but I'm not maintaining that part of the kernel (at least at the moment).

Quoted 1 line

I'm not sure who's the maintainer of it, to be honest.

Well, according to https://lkml.org/lkml/2012/10/19/375, that would be you (together with Bjorn and Len). :-) Hence I am resending the request for inclusion to you as well. PNP code went, if I am not mistaken, traditionally through Len's tree.