linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V3 0/3] PNP: Allow PNP resources to be disabled (interface)
@ 2012-07-29 18:38 Witold Szczeponik
  2012-07-29 19:22 ` Rafael J. Wysocki
  0 siblings, 1 reply; 17+ messages in thread
From: Witold Szczeponik @ 2012-07-29 18:38 UTC (permalink / raw)
  To: bhelgaas, lenb; +Cc: linux-kernel, linux-acpi

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.

To this end, some preparatory work is done, simplifying the code, and fixing a
potential issue when explicitely assigning resources. 

Here is a brief description of these patches. 

[1/3] - Factor out common some code
[2/3] - Perform the actual setting
[3/3] - Handle IORESOURCE_BITS in resource allocation

The patches are applied against Linux 3.5.x. 

Comments are, as always, welcome.  If the patches should be sent to someone
else, please let me know. 


--- Witold


Changes from previous versions:

V3 -> V2: Added Bjorn Helgaas as "Reviewed by"
          No changes in the code itself
          Based on Linux 3.5.x

V1 -> V2: Split [V1 2/3] into [V2 2/3] and [V2 3/3]
          Removed [V1 3/3], will be submitted separately
          Wrote more comments in response to the previous version
          Sent to a broader audience
          (https://lkml.org/lkml/2012/4/11/442)

V1:       Initial version 
          (https://lkml.org/lkml/2012/3/20/358)

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 0/3] PNP: Allow PNP resources to be disabled (interface)
  2012-07-29 18:38 Witold Szczeponik
@ 2012-07-29 19:22 ` Rafael J. Wysocki
  2012-07-29 19:31   ` Witold Szczeponik
  0 siblings, 1 reply; 17+ messages in thread
From: Rafael J. Wysocki @ 2012-07-29 19:22 UTC (permalink / raw)
  To: Witold Szczeponik; +Cc: bhelgaas, lenb, linux-kernel, linux-acpi

Hi,

On Sunday, July 29, 2012, Witold Szczeponik wrote:
> 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.

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

Rafael

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 0/3] PNP: Allow PNP resources to be disabled (interface)
  2012-07-29 19:22 ` Rafael J. Wysocki
@ 2012-07-29 19:31   ` Witold Szczeponik
  2012-07-30  8:28     ` Borislav Petkov
  0 siblings, 1 reply; 17+ messages in thread
From: Witold Szczeponik @ 2012-07-29 19:31 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: bhelgaas, lenb, linux-kernel, linux-acpi

On 29/07/12 21:22, Rafael J. Wysocki wrote:

> Hi,
> 

[... long snip ...]

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

Hi 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. 

--- Witold

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 0/3] PNP: Allow PNP resources to be disabled (interface)
  2012-07-29 19:31   ` Witold Szczeponik
@ 2012-07-30  8:28     ` Borislav Petkov
  2012-07-30 10:58       ` Witold Szczeponik
  2012-08-02 20:09       ` Rafael J. Wysocki
  0 siblings, 2 replies; 17+ messages in thread
From: Borislav Petkov @ 2012-07-30  8:28 UTC (permalink / raw)
  To: Witold Szczeponik
  Cc: Rafael J. Wysocki, bhelgaas, lenb, linux-kernel, linux-acpi

On Sun, Jul 29, 2012 at 09:31:53PM +0200, Witold Szczeponik wrote:
> 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.

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.

-- 
Regards/Gruss,
    Boris.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 0/3] PNP: Allow PNP resources to be disabled (interface)
  2012-07-30  8:28     ` Borislav Petkov
@ 2012-07-30 10:58       ` Witold Szczeponik
  2012-08-02 20:09       ` Rafael J. Wysocki
  1 sibling, 0 replies; 17+ messages in thread
From: Witold Szczeponik @ 2012-07-30 10:58 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: linux-acpi, linux-kernel, lenb, bhelgaas, rjw

> On Sun, Jul 29, 2012 at 09:31:53PM +0200, Witold Szczeponik wrote:
> > 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.
> 
> 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.
> 
> -- 
> Regards/Gruss,
>     Boris.

Hi Boris, 

the patch series is about adding the term "disabled" (or "<none") to
the list of possible values for resources.  The current state of the
kernel ABI already allows to use the following statement (cf. DSDT
excerpt from https://lkml.org/lkml/2011/7/3/41): 

  echo set irq 7 > /sys/bus/pnp/devices/$device/resources

i.e., the "resources" file can already parse all the values necessary for setting PNP values.  

The patch series is not about adding a new ABI or changing an existing ABI.  It is about extending the existing one to be able to handle the term "disabled" (or "<none>") as a special for a resource value. 

--- Witold

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 0/3] PNP: Allow PNP resources to be disabled (interface)
@ 2012-08-02 16:32 Witold Szczeponik
  2012-08-02 16:38 ` Witold Szczeponik
  0 siblings, 1 reply; 17+ messages in thread
From: Witold Szczeponik @ 2012-08-02 16:32 UTC (permalink / raw)
  To: lenb, bhelgaas; +Cc: linux-acpi, linux-kernel

Hi Len and Bjorn, 

is there anything that needs to be done in order for the patch series
to be included in either 3.6 or 3.7?  Except for the (viable) question
as to whether or not a sysfs interface should accept complex inputs
(as it currently does and which is not introduced by this patch series), 
there have been no comments since March, when I made some changes due 
to Bjorn's well made observations. 

The patches extend Linux's PNP capabilities and are required for some
hardware, like the IBM ThinkPad 600E (and similar machines). 

All other users of the ABI should not see any regressions (the ABI
stays the same).  

--- Witold


> 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.
> 
> To this end, some preparatory work is done, simplifying the code, and
> fixing a
> potential issue when explicitely assigning resources. 
> 
> Here is a brief description of these patches. 
> 
> [1/3] - Factor out common some code
> [2/3] - Perform the actual setting
> [3/3] - Handle IORESOURCE_BITS in resource allocation
> 
> The patches are applied against Linux 3.5.x. 
> 
> Comments are, as always, welcome.  If the patches should be sent to
> someone
> else, please let me know. 
> 
> 
> --- Witold
> 
> 
> Changes from previous versions:
> 
> V3 -> V2: Added Bjorn Helgaas as "Reviewed by"
>           No changes in the code itself
>           Based on Linux 3.5.x
> 
> V1 -> V2: Split [V1 2/3] into [V2 2/3] and [V2 3/3]
>           Removed [V1 3/3], will be submitted separately
>           Wrote more comments in response to the previous version
>           Sent to a broader audience
>           (https://lkml.org/lkml/2012/4/11/442)
> 
> V1:       Initial version 
>           (https://lkml.org/lkml/2012/3/20/358)

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 0/3] PNP: Allow PNP resources to be disabled (interface)
  2012-08-02 16:32 [PATCH V3 0/3] PNP: Allow PNP resources to be disabled (interface) Witold Szczeponik
@ 2012-08-02 16:38 ` Witold Szczeponik
  0 siblings, 0 replies; 17+ messages in thread
From: Witold Szczeponik @ 2012-08-02 16:38 UTC (permalink / raw)
  To: bhelgaas, lenb; +Cc: linux-kernel, linux-acpi

Hi all, 

the original mail should have been sent out as a reply to 
https://lkml.org/lkml/2012/7/29/85, but it seems as if I clicked the
wrong button.  

Apologies for any inconvenience. 

--- Witold

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 0/3] PNP: Allow PNP resources to be disabled (interface)
  2012-07-30  8:28     ` Borislav Petkov
  2012-07-30 10:58       ` Witold Szczeponik
@ 2012-08-02 20:09       ` Rafael J. Wysocki
  2012-08-02 20:20         ` Witold Szczeponik
  1 sibling, 1 reply; 17+ messages in thread
From: Rafael J. Wysocki @ 2012-08-02 20:09 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Witold Szczeponik, bhelgaas, lenb, linux-kernel, linux-acpi

On Monday, July 30, 2012, Borislav Petkov wrote:
> On Sun, Jul 29, 2012 at 09:31:53PM +0200, Witold Szczeponik wrote:
> > 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.
> 
> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 0/3] PNP: Allow PNP resources to be disabled (interface)
  2012-08-02 20:09       ` Rafael J. Wysocki
@ 2012-08-02 20:20         ` Witold Szczeponik
  2012-08-02 21:40           ` Rafael J. Wysocki
  0 siblings, 1 reply; 17+ messages in thread
From: Witold Szczeponik @ 2012-08-02 20:20 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Borislav Petkov, bhelgaas, lenb, linux-kernel, linux-acpi

On 02/08/12 22:09, Rafael J. Wysocki wrote:
> On Monday, July 30, 2012, Borislav Petkov wrote:
>> On Sun, Jul 29, 2012 at 09:31:53PM +0200, Witold Szczeponik wrote:
>>> 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.
>>
>> 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).  

--- Witold

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 0/3] PNP: Allow PNP resources to be disabled (interface)
  2012-08-02 20:20         ` Witold Szczeponik
@ 2012-08-02 21:40           ` Rafael J. Wysocki
  2012-08-02 21:57             ` Witold Szczeponik
  2012-09-16 14:18             ` Witold Szczeponik
  0 siblings, 2 replies; 17+ messages in thread
From: Rafael J. Wysocki @ 2012-08-02 21:40 UTC (permalink / raw)
  To: Witold Szczeponik
  Cc: Borislav Petkov, bhelgaas, lenb, linux-kernel, linux-acpi

On Thursday, August 02, 2012, Witold Szczeponik wrote:
> On 02/08/12 22:09, Rafael J. Wysocki wrote:
> > On Monday, July 30, 2012, Borislav Petkov wrote:
> >> On Sun, Jul 29, 2012 at 09:31:53PM +0200, Witold Szczeponik wrote:
> >>> 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.
> >>
> >> 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... 

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

> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 0/3] PNP: Allow PNP resources to be disabled (interface)
  2012-08-02 21:40           ` Rafael J. Wysocki
@ 2012-08-02 21:57             ` Witold Szczeponik
  2012-09-16 14:18             ` Witold Szczeponik
  1 sibling, 0 replies; 17+ messages in thread
From: Witold Szczeponik @ 2012-08-02 21:57 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Borislav Petkov, bhelgaas, lenb, linux-kernel, linux-acpi

On 02/08/12 23:40, Rafael J. Wysocki wrote:

> On Thursday, August 02, 2012, Witold Szczeponik wrote:
>> On 02/08/12 22:09, Rafael J. Wysocki wrote:
>>> On Monday, July 30, 2012, Borislav Petkov wrote:
>>>> On Sun, Jul 29, 2012 at 09:31:53PM +0200, Witold Szczeponik wrote:

[... snip ...]

>>>>
>>>> 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... 
> 
> Still, you _are_ doing that by extending the ABI, aren't you?

As the special value "disabled" is available as of these patches, one could
consider this an extension.  I agree. 

> 
>> 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.

Personally, I too think that the PNP ABI in sysfs has its rough edges.  However, 
as with the deprecation of any existing ABI, this would require a new ABI first, 
then some time where the old and new ABI live in co-existence, and then to remove 
the currently available ABI. 

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

The current ABI does not allow for the kernel to run on my hardware: this is 
a/the problem.  The proposed extension fixes the problem.  

While I agree with your first statement, for the time being I do not see a 
better solution other than to extend the ABI. 

At this point I am repeating my "call for comments" to the community. :-) 

--- Witold

> 
> Thanks,
> Rafael
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 0/3] PNP: Allow PNP resources to be disabled (interface)
  2012-08-02 21:40           ` Rafael J. Wysocki
  2012-08-02 21:57             ` Witold Szczeponik
@ 2012-09-16 14:18             ` Witold Szczeponik
  2012-09-18 21:42               ` Rafael J. Wysocki
  1 sibling, 1 reply; 17+ messages in thread
From: Witold Szczeponik @ 2012-09-16 14:18 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Borislav Petkov, bhelgaas, lenb, linux-kernel, linux-acpi

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?

--- Witold

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 0/3] PNP: Allow PNP resources to be disabled (interface)
  2012-09-16 14:18             ` Witold Szczeponik
@ 2012-09-18 21:42               ` Rafael J. Wysocki
  2012-10-03 15:57                 ` Witold Szczeponik
  2012-10-19 19:02                 ` Witold Szczeponik
  0 siblings, 2 replies; 17+ messages in thread
From: Rafael J. Wysocki @ 2012-09-18 21:42 UTC (permalink / raw)
  To: Witold Szczeponik
  Cc: Borislav Petkov, bhelgaas, lenb, linux-kernel, linux-acpi

Hi,

On Sunday, September 16, 2012, Witold Szczeponik wrote:
> 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).

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

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 0/3] PNP: Allow PNP resources to be disabled (interface)
  2012-09-18 21:42               ` Rafael J. Wysocki
@ 2012-10-03 15:57                 ` Witold Szczeponik
  2012-10-14 15:57                   ` Witold Szczeponik
  2012-10-19 19:02                 ` Witold Szczeponik
  1 sibling, 1 reply; 17+ messages in thread
From: Witold Szczeponik @ 2012-10-03 15:57 UTC (permalink / raw)
  To: Rafael J. Wysocki, bhelgaas, lenb
  Cc: Borislav Petkov, linux-kernel, linux-acpi

On 18/09/12 23:42, Rafael J. Wysocki wrote:
> Hi,
> 
> On Sunday, September 16, 2012, Witold Szczeponik wrote:
>> 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).
> 
> I'm not sure who's the maintainer of it, to be honest.

According to the maintainer list, this would be Len and Bjorn.  
Hence...


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

> 
> Thanks,
> Rafael
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 0/3] PNP: Allow PNP resources to be disabled (interface)
  2012-10-03 15:57                 ` Witold Szczeponik
@ 2012-10-14 15:57                   ` Witold Szczeponik
  0 siblings, 0 replies; 17+ messages in thread
From: Witold Szczeponik @ 2012-10-14 15:57 UTC (permalink / raw)
  To: bhelgaas, lenb
  Cc: Rafael J. Wysocki, Borislav Petkov, linux-kernel, linux-acpi

On 03/10/12 17:57, Witold Szczeponik wrote:

[...]

> 
> 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 Len, hello Bjorn, 

the patches submitted in https://lkml.org/lkml/2012/7/29/85 have
not been objected by anyone, make the code simpler, and fix a 
current problem when using the "/sys/bus/pnp/devices/*/resources" 
interface.  I have been running them since at least Linux 3.0, 
with adapted changes in previous versions.  I have not seen any
regressions and I do not expect any regressions from these
patches. 

Bjorn had reviewed them and I am wondering what I can do to have 
them included in (one of) the next Linux kernel(s).

If there is any information that you need before an inclusion is
possible, just let me know. 

--- Witold

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 0/3] PNP: Allow PNP resources to be disabled (interface)
  2012-09-18 21:42               ` Rafael J. Wysocki
  2012-10-03 15:57                 ` Witold Szczeponik
@ 2012-10-19 19:02                 ` Witold Szczeponik
  2012-10-19 22:19                   ` Rafael J. Wysocki
  1 sibling, 1 reply; 17+ messages in thread
From: Witold Szczeponik @ 2012-10-19 19:02 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Borislav Petkov, bhelgaas, lenb, linux-kernel, linux-acpi

On 18/09/12 23:42, Rafael J. Wysocki wrote:

> Hi,
> 
> On Sunday, September 16, 2012, Witold Szczeponik wrote:
>> 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).
> 
> 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.  

--- Witold

> 
> Thanks,
> Rafael
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V3 0/3] PNP: Allow PNP resources to be disabled (interface)
  2012-10-19 19:02                 ` Witold Szczeponik
@ 2012-10-19 22:19                   ` Rafael J. Wysocki
  0 siblings, 0 replies; 17+ messages in thread
From: Rafael J. Wysocki @ 2012-10-19 22:19 UTC (permalink / raw)
  To: Witold Szczeponik
  Cc: Borislav Petkov, bhelgaas, lenb, linux-kernel, linux-acpi

On Friday 19 of October 2012 21:02:20 Witold Szczeponik wrote:
> On 18/09/12 23:42, Rafael J. Wysocki wrote:
> 
> > Hi,
> > 
> > On Sunday, September 16, 2012, Witold Szczeponik wrote:
> >> 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).
> > 
> > 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.  

OK, so can you please resend them?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2012-10-19 22:19 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-02 16:32 [PATCH V3 0/3] PNP: Allow PNP resources to be disabled (interface) Witold Szczeponik
2012-08-02 16:38 ` Witold Szczeponik
  -- strict thread matches above, loose matches on Subject: below --
2012-07-29 18:38 Witold Szczeponik
2012-07-29 19:22 ` Rafael J. Wysocki
2012-07-29 19:31   ` Witold Szczeponik
2012-07-30  8:28     ` Borislav Petkov
2012-07-30 10:58       ` Witold Szczeponik
2012-08-02 20:09       ` Rafael J. Wysocki
2012-08-02 20:20         ` Witold Szczeponik
2012-08-02 21:40           ` Rafael J. Wysocki
2012-08-02 21:57             ` Witold Szczeponik
2012-09-16 14:18             ` Witold Szczeponik
2012-09-18 21:42               ` Rafael J. Wysocki
2012-10-03 15:57                 ` Witold Szczeponik
2012-10-14 15:57                   ` Witold Szczeponik
2012-10-19 19:02                 ` Witold Szczeponik
2012-10-19 22:19                   ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).