Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On 11/5/14 12:59, Rafael J. Wysocki wrote:
> On Tuesday, November 04, 2014 03:42:38 PM Darren Hart wrote:
>>
>> On 11/4/14 14:54, Rafael J. Wysocki wrote:
>>
>>> From: Rafael J. Wysocki
>>> Subject: ACPI / property: Drop size_prop from
>>> acpi_dev_get_property_reference()
>>>
>>> The size_prop argument of the recently added function
>>> acpi_dev_get_property_reference() is not used by the only current
>>> caller of that function and is very unlikely to be used at any time
>>> going forward.
>>>
>>> Namely, for a property whose value is a list of items each containing
>>> a references to a device object possibly accompanied by some integers,
>>> the number of items in the list can always be computed as the number
>>> of elements of type ACPI_TYPE_LOCAL_REFERENCE in the property package.
>>> Thus it should never be necessary to provide an additional "cells"
>>> property with a value equal to the number of items in that list.
>>
>> In this case, do we never expect a property to contain more than one
>> ACPI_TYPE_LOCAL_REFERENCE?
>>
>> Package () { "foobar",
>> Package () {
>> "PCI0.FOO", "PCI0.BAR", 0, 1, 0,
>> "PCI0.FOO", "PCI0.BAR2", 0, 1, 1
>> }
>> }
>>
>> This seems like it could be useful for connecting various types of
>> devices together, but I confess not to have a specific exmaple in mind.
>> It does concern me to limit the data format in this way.
>
> We don't support this even with size_prop, so it doesn't seem to be relevant
> here.
>
> Now, if we were to support this, I'd rather not use
> acpi_dev_get_property_reference()
> for that, but add a new function specifically for it. Moreover, I would
> extend the
> format definition then so that we could do
>
> Package () {
> "foobar", Package () {
> Package () {"PCI0.FOO", "PCI0.BAR", 0, 1, 0},
> Package () {"PCI0.FOO", "PCI0.BAR2", 0, 1, 1}
> }
> }
>
> in which case adding a special "size" property could be avoided.
>
> That said, I have no idea why it might be necessary. One reference in a
> property
> value means that we're connecting the current node (the owner of the _DSD
> containing that property) with some other node in the namespace. Two
> references
> in there would mean that the current node is to be connected with *two* other
> nodes in the namespace at the same time. That raises some questions that I'd
> rather not consider in detail here, unless you insist. ;-)
>
>> I suppose should such a case become necessary, we can deal with the
>> issue then - and still avoid having a potential abuse point in the API
>> from the start.
>
> What we have today is sufficient for all of the cases we've considered so far.
> If we find a case where it is not sufficient, we'll need to consider extending
> the data format as well as the API.
>
> Rafael
Agreed on all points. Thanks Rafael.
--
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Tuesday, November 04, 2014 03:42:38 PM Darren Hart wrote:
>
> On 11/4/14 14:54, Rafael J. Wysocki wrote:
>
> > From: Rafael J. Wysocki
> > Subject: ACPI / property: Drop size_prop from
> > acpi_dev_get_property_reference()
> >
> > The size_prop argument of the recently added function
> > acpi_dev_get_property_reference() is not used by the only current
> > caller of that function and is very unlikely to be used at any time
> > going forward.
> >
> > Namely, for a property whose value is a list of items each containing
> > a references to a device object possibly accompanied by some integers,
> > the number of items in the list can always be computed as the number
> > of elements of type ACPI_TYPE_LOCAL_REFERENCE in the property package.
> > Thus it should never be necessary to provide an additional "cells"
> > property with a value equal to the number of items in that list.
>
> In this case, do we never expect a property to contain more than one
> ACPI_TYPE_LOCAL_REFERENCE?
>
> Package () { "foobar",
> Package () {
> "PCI0.FOO", "PCI0.BAR", 0, 1, 0,
> "PCI0.FOO", "PCI0.BAR2", 0, 1, 1
> }
> }
>
> This seems like it could be useful for connecting various types of
> devices together, but I confess not to have a specific exmaple in mind.
> It does concern me to limit the data format in this way.
We don't support this even with size_prop, so it doesn't seem to be relevant
here.
Now, if we were to support this, I'd rather not use
acpi_dev_get_property_reference()
for that, but add a new function specifically for it. Moreover, I would extend
the
format definition then so that we could do
Package () {
"foobar", Package () {
Package () {"PCI0.FOO", "PCI0.BAR", 0, 1, 0},
Package () {"PCI0.FOO", "PCI0.BAR2", 0, 1, 1}
}
}
in which case adding a special "size" property could be avoided.
That said, I have no idea why it might be necessary. One reference in a
property
value means that we're connecting the current node (the owner of the _DSD
containing that property) with some other node in the namespace. Two references
in there would mean that the current node is to be connected with *two* other
nodes in the namespace at the same time. That raises some questions that I'd
rather not consider in detail here, unless you insist. ;-)
> I suppose should such a case become necessary, we can deal with the
> issue then - and still avoid having a potential abuse point in the API
> from the start.
What we have today is sufficient for all of the cases we've considered so far.
If we find a case where it is not sufficient, we'll need to consider extending
the data format as well as the API.
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Wednesday, November 05, 2014 11:16:22 AM Mika Westerberg wrote: > On Tue, Nov 04, 2014 at 11:54:41PM +0100, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki > > Subject: ACPI / property: Drop size_prop from > > acpi_dev_get_property_reference() > > > > The size_prop argument of the recently added function > > acpi_dev_get_property_reference() is not used by the only current > > caller of that function and is very unlikely to be used at any time > > going forward. > > > > Namely, for a property whose value is a list of items each containing > > a references to a device object possibly accompanied by some integers, > > the number of items in the list can always be computed as the number > > of elements of type ACPI_TYPE_LOCAL_REFERENCE in the property package. > > Thus it should never be necessary to provide an additional "cells" > > property with a value equal to the number of items in that list. > > > > For this reason, drop the size_prop argument from > > acpi_dev_get_property_reference() and update its caller accordingly. > > > > Link: http://marc.info/?l=linux-kernel=141511255610556=2 > > Suggested-by: Grant Likely > > Signed-off-by: Rafael J. Wysocki > > Just pulled your latest bleeding-edge (including this patch) and it > still works fine with both Minnowboards. Feel free to add my: > > Acked-by: Mika Westerberg > Tested-by: Mika Westerberg Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On 11/4/14 14:54, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki
> Subject: ACPI / property: Drop size_prop from
> acpi_dev_get_property_reference()
>
> The size_prop argument of the recently added function
> acpi_dev_get_property_reference() is not used by the only current
> caller of that function and is very unlikely to be used at any time
> going forward.
>
> Namely, for a property whose value is a list of items each containing
> a references to a device object possibly accompanied by some integers,
> the number of items in the list can always be computed as the number
> of elements of type ACPI_TYPE_LOCAL_REFERENCE in the property package.
> Thus it should never be necessary to provide an additional "cells"
> property with a value equal to the number of items in that list.
In this case, do we never expect a property to contain more than one
ACPI_TYPE_LOCAL_REFERENCE?
Package () { "foobar",
Package () {
"PCI0.FOO", "PCI0.BAR", 0, 1, 0,
"PCI0.FOO", "PCI0.BAR2", 0, 1, 1
}
}
This seems like it could be useful for connecting various types of
devices together, but I confess not to have a specific exmaple in mind.
It does concern me to limit the data format in this way.
I suppose should such a case become necessary, we can deal with the
issue then - and still avoid having a potential abuse point in the API
from the start.
--
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Tue, Nov 04, 2014 at 11:54:41PM +0100, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki > Subject: ACPI / property: Drop size_prop from > acpi_dev_get_property_reference() > > The size_prop argument of the recently added function > acpi_dev_get_property_reference() is not used by the only current > caller of that function and is very unlikely to be used at any time > going forward. > > Namely, for a property whose value is a list of items each containing > a references to a device object possibly accompanied by some integers, > the number of items in the list can always be computed as the number > of elements of type ACPI_TYPE_LOCAL_REFERENCE in the property package. > Thus it should never be necessary to provide an additional "cells" > property with a value equal to the number of items in that list. > > For this reason, drop the size_prop argument from > acpi_dev_get_property_reference() and update its caller accordingly. > > Link: http://marc.info/?l=linux-kernel=141511255610556=2 > Suggested-by: Grant Likely > Signed-off-by: Rafael J. Wysocki Just pulled your latest bleeding-edge (including this patch) and it still works fine with both Minnowboards. Feel free to add my: Acked-by: Mika Westerberg Tested-by: Mika Westerberg -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Tue, Nov 04, 2014 at 11:54:41PM +0100, Rafael J. Wysocki wrote: From: Rafael J. Wysocki rafael.j.wyso...@intel.com Subject: ACPI / property: Drop size_prop from acpi_dev_get_property_reference() The size_prop argument of the recently added function acpi_dev_get_property_reference() is not used by the only current caller of that function and is very unlikely to be used at any time going forward. Namely, for a property whose value is a list of items each containing a references to a device object possibly accompanied by some integers, the number of items in the list can always be computed as the number of elements of type ACPI_TYPE_LOCAL_REFERENCE in the property package. Thus it should never be necessary to provide an additional cells property with a value equal to the number of items in that list. For this reason, drop the size_prop argument from acpi_dev_get_property_reference() and update its caller accordingly. Link: http://marc.info/?l=linux-kernelm=141511255610556w=2 Suggested-by: Grant Likely grant.lik...@linaro.org Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com Just pulled your latest bleeding-edge (including this patch) and it still works fine with both Minnowboards. Feel free to add my: Acked-by: Mika Westerberg mika.westerb...@linux.intel.com Tested-by: Mika Westerberg mika.westerb...@linux.intel.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On 11/4/14 14:54, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
Subject: ACPI / property: Drop size_prop from
acpi_dev_get_property_reference()
The size_prop argument of the recently added function
acpi_dev_get_property_reference() is not used by the only current
caller of that function and is very unlikely to be used at any time
going forward.
Namely, for a property whose value is a list of items each containing
a references to a device object possibly accompanied by some integers,
the number of items in the list can always be computed as the number
of elements of type ACPI_TYPE_LOCAL_REFERENCE in the property package.
Thus it should never be necessary to provide an additional cells
property with a value equal to the number of items in that list.
In this case, do we never expect a property to contain more than one
ACPI_TYPE_LOCAL_REFERENCE?
Package () { foobar,
Package () {
PCI0.FOO, PCI0.BAR, 0, 1, 0,
PCI0.FOO, PCI0.BAR2, 0, 1, 1
}
}
This seems like it could be useful for connecting various types of
devices together, but I confess not to have a specific exmaple in mind.
It does concern me to limit the data format in this way.
I suppose should such a case become necessary, we can deal with the
issue then - and still avoid having a potential abuse point in the API
from the start.
--
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Wednesday, November 05, 2014 11:16:22 AM Mika Westerberg wrote: On Tue, Nov 04, 2014 at 11:54:41PM +0100, Rafael J. Wysocki wrote: From: Rafael J. Wysocki rafael.j.wyso...@intel.com Subject: ACPI / property: Drop size_prop from acpi_dev_get_property_reference() The size_prop argument of the recently added function acpi_dev_get_property_reference() is not used by the only current caller of that function and is very unlikely to be used at any time going forward. Namely, for a property whose value is a list of items each containing a references to a device object possibly accompanied by some integers, the number of items in the list can always be computed as the number of elements of type ACPI_TYPE_LOCAL_REFERENCE in the property package. Thus it should never be necessary to provide an additional cells property with a value equal to the number of items in that list. For this reason, drop the size_prop argument from acpi_dev_get_property_reference() and update its caller accordingly. Link: http://marc.info/?l=linux-kernelm=141511255610556w=2 Suggested-by: Grant Likely grant.lik...@linaro.org Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com Just pulled your latest bleeding-edge (including this patch) and it still works fine with both Minnowboards. Feel free to add my: Acked-by: Mika Westerberg mika.westerb...@linux.intel.com Tested-by: Mika Westerberg mika.westerb...@linux.intel.com Thanks! -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Tuesday, November 04, 2014 03:42:38 PM Darren Hart wrote:
On 11/4/14 14:54, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
Subject: ACPI / property: Drop size_prop from
acpi_dev_get_property_reference()
The size_prop argument of the recently added function
acpi_dev_get_property_reference() is not used by the only current
caller of that function and is very unlikely to be used at any time
going forward.
Namely, for a property whose value is a list of items each containing
a references to a device object possibly accompanied by some integers,
the number of items in the list can always be computed as the number
of elements of type ACPI_TYPE_LOCAL_REFERENCE in the property package.
Thus it should never be necessary to provide an additional cells
property with a value equal to the number of items in that list.
In this case, do we never expect a property to contain more than one
ACPI_TYPE_LOCAL_REFERENCE?
Package () { foobar,
Package () {
PCI0.FOO, PCI0.BAR, 0, 1, 0,
PCI0.FOO, PCI0.BAR2, 0, 1, 1
}
}
This seems like it could be useful for connecting various types of
devices together, but I confess not to have a specific exmaple in mind.
It does concern me to limit the data format in this way.
We don't support this even with size_prop, so it doesn't seem to be relevant
here.
Now, if we were to support this, I'd rather not use
acpi_dev_get_property_reference()
for that, but add a new function specifically for it. Moreover, I would extend
the
format definition then so that we could do
Package () {
foobar, Package () {
Package () {PCI0.FOO, PCI0.BAR, 0, 1, 0},
Package () {PCI0.FOO, PCI0.BAR2, 0, 1, 1}
}
}
in which case adding a special size property could be avoided.
That said, I have no idea why it might be necessary. One reference in a
property
value means that we're connecting the current node (the owner of the _DSD
containing that property) with some other node in the namespace. Two references
in there would mean that the current node is to be connected with *two* other
nodes in the namespace at the same time. That raises some questions that I'd
rather not consider in detail here, unless you insist. ;-)
I suppose should such a case become necessary, we can deal with the
issue then - and still avoid having a potential abuse point in the API
from the start.
What we have today is sufficient for all of the cases we've considered so far.
If we find a case where it is not sufficient, we'll need to consider extending
the data format as well as the API.
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On 11/5/14 12:59, Rafael J. Wysocki wrote:
On Tuesday, November 04, 2014 03:42:38 PM Darren Hart wrote:
On 11/4/14 14:54, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
Subject: ACPI / property: Drop size_prop from
acpi_dev_get_property_reference()
The size_prop argument of the recently added function
acpi_dev_get_property_reference() is not used by the only current
caller of that function and is very unlikely to be used at any time
going forward.
Namely, for a property whose value is a list of items each containing
a references to a device object possibly accompanied by some integers,
the number of items in the list can always be computed as the number
of elements of type ACPI_TYPE_LOCAL_REFERENCE in the property package.
Thus it should never be necessary to provide an additional cells
property with a value equal to the number of items in that list.
In this case, do we never expect a property to contain more than one
ACPI_TYPE_LOCAL_REFERENCE?
Package () { foobar,
Package () {
PCI0.FOO, PCI0.BAR, 0, 1, 0,
PCI0.FOO, PCI0.BAR2, 0, 1, 1
}
}
This seems like it could be useful for connecting various types of
devices together, but I confess not to have a specific exmaple in mind.
It does concern me to limit the data format in this way.
We don't support this even with size_prop, so it doesn't seem to be relevant
here.
Now, if we were to support this, I'd rather not use
acpi_dev_get_property_reference()
for that, but add a new function specifically for it. Moreover, I would
extend the
format definition then so that we could do
Package () {
foobar, Package () {
Package () {PCI0.FOO, PCI0.BAR, 0, 1, 0},
Package () {PCI0.FOO, PCI0.BAR2, 0, 1, 1}
}
}
in which case adding a special size property could be avoided.
That said, I have no idea why it might be necessary. One reference in a
property
value means that we're connecting the current node (the owner of the _DSD
containing that property) with some other node in the namespace. Two
references
in there would mean that the current node is to be connected with *two* other
nodes in the namespace at the same time. That raises some questions that I'd
rather not consider in detail here, unless you insist. ;-)
I suppose should such a case become necessary, we can deal with the
issue then - and still avoid having a potential abuse point in the API
from the start.
What we have today is sufficient for all of the cases we've considered so far.
If we find a case where it is not sufficient, we'll need to consider extending
the data format as well as the API.
Rafael
Agreed on all points. Thanks Rafael.
--
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Tue, Nov 4, 2014 at 10:54 PM, Rafael J. Wysocki wrote:
> On Tuesday, November 04, 2014 05:06:40 PM Rafael J. Wysocki wrote:
>> On Tuesday, November 04, 2014 02:48:40 PM Grant Likely wrote:
>> > On Mon, Nov 3, 2014 at 9:52 PM, Rafael J. Wysocki
>> > wrote:
>> > > On Monday, November 03, 2014 04:25:08 PM Rafael J. Wysocki wrote:
>> > >> On Sunday, November 02, 2014 08:49:37 PM Darren Hart wrote:
>> > >> >
>> > >> > On 11/1/14 4:11, Grant Likely wrote:
>> > >> > > On Tue, 28 Oct 2014 22:59:57 +0100
>> > >> > > , "Rafael J. Wysocki"
>> > >> > > wrote:
>> > >> > >> On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
>> > >> > >>> acpi_dev_add_driver_gpios() makes it possible to set up mapping
>> > >> > >>> between
>> > >> > >>> properties and ACPI GpioIo resources in a driver, so we can take
>> > >> > >>> index
>> > >> > >>> parameter in acpi_find_gpio() into use with _DSD device
>> > >> > >>> properties now.
>> > >> > >>>
>> > >> > >>> This index can be used to select a GPIO from a property with
>> > >> > >>> multiple
>> > >> > >>> GPIOs:
>> > >> > >>>
>> > >> > >>> Package () {
>> > >> > >>> "data-gpios",
>> > >> > >>> Package () {
>> > >> > >>> \_SB.GPIO, 0, 0, 0,
>> > >> > >>> \_SB.GPIO, 1, 0, 0,
>> > >> > >>> \_SB.GPIO, 2, 0, 1,
>> > >> > >>> }
>> > >> > >>> }
>> > >> > >>>
>> > >> > >>> In order to retrieve the last GPIO from a driver we can simply do:
>> > >> > >>>
>> > >> > >>> desc = devm_gpiod_get_index(dev, "data", 2);
>> > >> > >>>
>> > >> > >>> and so on.
>> > >> > >>>
>> > >> > >>> Signed-off-by: Mika Westerberg
>> > >> > >>
>> > >> > >> Cool. :-)
>> > >> > >>
>> > >> > >> Any objections anyone?
>> > >> > >
>> > >> > > Actually, I do. Not in the idea, but in the implementation. The way
>> > >> > > this gets encoded:
>> > >> > >
>> > >> > > Package () {
>> > >> > > \_SB.GPIO, 0, 0, 0,
>> > >> > > \_SB.GPIO, 1, 0, 0,
>> > >> > > \_SB.GPIO, 2, 0, 1,
>> > >> > > }
>> > >> > >
>> > >> > > Means that decoding each GPIO tuple requires the length of a tuple
>> > >> > > to be
>> > >> > > fixed, or to implement a DT-like #gpio-cells. If it is fixed, then
>> > >> > > there
>> > >> > > is no way to expand the binding later. Can this be done in the
>> > >> > > following
>> > >> > > way instead?
>> > >> > >
>> > >> > > Package () {
>> > >> > > Package () { \_SB.GPIO, 0, 0, 0 },
>> > >> > > Package () { \_SB.GPIO, 1, 0, 0 },
>> > >> > > Package () { \_SB.GPIO, 2, 0, 1 },
>> > >> > > }
>> > >> > >
>> > >> > > This is one of the biggest pains in device tree. We don't have any
>> > >> > > way
>> > >> > > to group tuples so it requires looking up stuff across the tree to
>> > >> > > figure out how to parse each multi-item property.
>> > >> > >
>> > >> > > I know that last year we talked about how bios vendors would get
>> > >> > > complicated properties wrong, but I think there is little risk in
>> > >> > > this
>> > >> > > case. If the property is encoded wrong, the driver simply won't
>> > >> > > work and
>> > >> > > it is unlikely to get shipped before being fixed.
>> > >> >
>> > >> > This particular nesting of Packages is expressly prohibited by the
>> > >> > Device Properties UUID for the reasons you mention.
>> > >> >
>> > >> > http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
>> > >>
>> > >> Also we don't use properties where single name is assigned to multiple
>> > >> GPIOs
>> > >> anywhere in the current device-properties patchset, so this is not
>> > >> relevant at
>> > >> the moment.
>> > >>
>> > >> Moreover, even if we were to use them, we would need to ensure that
>> > >> this:
>> > >>
>> > >> Package () {
>> > >> \_SB.GPIO, 0, 0, 0
>> > >> }
>> > >>
>> > >> was equivalent to
>> > >>
>> > >> Package () {
>> > >> Package () { \_SB.GPIO, 0, 0, 0 }
>> > >> }
>> > >>
>> > >> This is not impossible to do and I suppose we could even explain that
>> > >> in the
>> > >> implementation guide document in a sensible way, but that would require
>> > >> the
>> > >> document linked above to be changed first and *then* we can think about
>> > >> writing
>> > >> kernel code to it. Not the other way around, please.
>> > >>
>> > >> So Grant, do you want us to proceed with that?
>> > >
>> > > Before you reply, one more observation that seems to be relevant.
>> > >
>> > > In ACPI, both this:
>> > >
>> > > Package () {
>> > > \_SB.GPIO, 0, 0, 0,
>> > > \_SB.GPIO, 1, 0, 0,
>> > > \_SB.GPIO, 2, 0, 1,
>> > > }
>> > >
>> > > and this:
>> > >
>> > > Package () {
>> > > Package () { \_SB.GPIO, 0, 0, 0 },
>> > > Package () { \_SB.GPIO, 1, 0, 0 },
>> > > Package () { \_SB.GPIO, 2, 0, 1 },
>> > > }
>> > >
>> > > carry the same information,
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Tuesday, November 04, 2014 05:06:40 PM Rafael J. Wysocki wrote:
> On Tuesday, November 04, 2014 02:48:40 PM Grant Likely wrote:
> > On Mon, Nov 3, 2014 at 9:52 PM, Rafael J. Wysocki
> > wrote:
> > > On Monday, November 03, 2014 04:25:08 PM Rafael J. Wysocki wrote:
> > >> On Sunday, November 02, 2014 08:49:37 PM Darren Hart wrote:
> > >> >
> > >> > On 11/1/14 4:11, Grant Likely wrote:
> > >> > > On Tue, 28 Oct 2014 22:59:57 +0100
> > >> > > , "Rafael J. Wysocki"
> > >> > > wrote:
> > >> > >> On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
> > >> > >>> acpi_dev_add_driver_gpios() makes it possible to set up mapping
> > >> > >>> between
> > >> > >>> properties and ACPI GpioIo resources in a driver, so we can take
> > >> > >>> index
> > >> > >>> parameter in acpi_find_gpio() into use with _DSD device properties
> > >> > >>> now.
> > >> > >>>
> > >> > >>> This index can be used to select a GPIO from a property with
> > >> > >>> multiple
> > >> > >>> GPIOs:
> > >> > >>>
> > >> > >>> Package () {
> > >> > >>> "data-gpios",
> > >> > >>> Package () {
> > >> > >>> \_SB.GPIO, 0, 0, 0,
> > >> > >>> \_SB.GPIO, 1, 0, 0,
> > >> > >>> \_SB.GPIO, 2, 0, 1,
> > >> > >>> }
> > >> > >>> }
> > >> > >>>
> > >> > >>> In order to retrieve the last GPIO from a driver we can simply do:
> > >> > >>>
> > >> > >>> desc = devm_gpiod_get_index(dev, "data", 2);
> > >> > >>>
> > >> > >>> and so on.
> > >> > >>>
> > >> > >>> Signed-off-by: Mika Westerberg
> > >> > >>
> > >> > >> Cool. :-)
> > >> > >>
> > >> > >> Any objections anyone?
> > >> > >
> > >> > > Actually, I do. Not in the idea, but in the implementation. The way
> > >> > > this gets encoded:
> > >> > >
> > >> > > Package () {
> > >> > > \_SB.GPIO, 0, 0, 0,
> > >> > > \_SB.GPIO, 1, 0, 0,
> > >> > > \_SB.GPIO, 2, 0, 1,
> > >> > > }
> > >> > >
> > >> > > Means that decoding each GPIO tuple requires the length of a tuple
> > >> > > to be
> > >> > > fixed, or to implement a DT-like #gpio-cells. If it is fixed, then
> > >> > > there
> > >> > > is no way to expand the binding later. Can this be done in the
> > >> > > following
> > >> > > way instead?
> > >> > >
> > >> > > Package () {
> > >> > > Package () { \_SB.GPIO, 0, 0, 0 },
> > >> > > Package () { \_SB.GPIO, 1, 0, 0 },
> > >> > > Package () { \_SB.GPIO, 2, 0, 1 },
> > >> > > }
> > >> > >
> > >> > > This is one of the biggest pains in device tree. We don't have any
> > >> > > way
> > >> > > to group tuples so it requires looking up stuff across the tree to
> > >> > > figure out how to parse each multi-item property.
> > >> > >
> > >> > > I know that last year we talked about how bios vendors would get
> > >> > > complicated properties wrong, but I think there is little risk in
> > >> > > this
> > >> > > case. If the property is encoded wrong, the driver simply won't work
> > >> > > and
> > >> > > it is unlikely to get shipped before being fixed.
> > >> >
> > >> > This particular nesting of Packages is expressly prohibited by the
> > >> > Device Properties UUID for the reasons you mention.
> > >> >
> > >> > http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
> > >>
> > >> Also we don't use properties where single name is assigned to multiple
> > >> GPIOs
> > >> anywhere in the current device-properties patchset, so this is not
> > >> relevant at
> > >> the moment.
> > >>
> > >> Moreover, even if we were to use them, we would need to ensure that this:
> > >>
> > >> Package () {
> > >> \_SB.GPIO, 0, 0, 0
> > >> }
> > >>
> > >> was equivalent to
> > >>
> > >> Package () {
> > >> Package () { \_SB.GPIO, 0, 0, 0 }
> > >> }
> > >>
> > >> This is not impossible to do and I suppose we could even explain that in
> > >> the
> > >> implementation guide document in a sensible way, but that would require
> > >> the
> > >> document linked above to be changed first and *then* we can think about
> > >> writing
> > >> kernel code to it. Not the other way around, please.
> > >>
> > >> So Grant, do you want us to proceed with that?
> > >
> > > Before you reply, one more observation that seems to be relevant.
> > >
> > > In ACPI, both this:
> > >
> > > Package () {
> > > \_SB.GPIO, 0, 0, 0,
> > > \_SB.GPIO, 1, 0, 0,
> > > \_SB.GPIO, 2, 0, 1,
> > > }
> > >
> > > and this:
> > >
> > > Package () {
> > > Package () { \_SB.GPIO, 0, 0, 0 },
> > > Package () { \_SB.GPIO, 1, 0, 0 },
> > > Package () { \_SB.GPIO, 2, 0, 1 },
> > > }
> > >
> > > carry the same information, because every element of a package has a type,
> > > so there is no danger of confusing an ACPI_TYPE_LOCAL_REFERENCE with
> > > ACPI_TYPE_INTEGER. Thus one can easily count the number of GPIOs
> > >
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Tuesday, November 04, 2014 02:48:40 PM Grant Likely wrote:
> On Mon, Nov 3, 2014 at 9:52 PM, Rafael J. Wysocki wrote:
> > On Monday, November 03, 2014 04:25:08 PM Rafael J. Wysocki wrote:
> >> On Sunday, November 02, 2014 08:49:37 PM Darren Hart wrote:
> >> >
> >> > On 11/1/14 4:11, Grant Likely wrote:
> >> > > On Tue, 28 Oct 2014 22:59:57 +0100
> >> > > , "Rafael J. Wysocki"
> >> > > wrote:
> >> > >> On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
> >> > >>> acpi_dev_add_driver_gpios() makes it possible to set up mapping
> >> > >>> between
> >> > >>> properties and ACPI GpioIo resources in a driver, so we can take
> >> > >>> index
> >> > >>> parameter in acpi_find_gpio() into use with _DSD device properties
> >> > >>> now.
> >> > >>>
> >> > >>> This index can be used to select a GPIO from a property with multiple
> >> > >>> GPIOs:
> >> > >>>
> >> > >>> Package () {
> >> > >>> "data-gpios",
> >> > >>> Package () {
> >> > >>> \_SB.GPIO, 0, 0, 0,
> >> > >>> \_SB.GPIO, 1, 0, 0,
> >> > >>> \_SB.GPIO, 2, 0, 1,
> >> > >>> }
> >> > >>> }
> >> > >>>
> >> > >>> In order to retrieve the last GPIO from a driver we can simply do:
> >> > >>>
> >> > >>> desc = devm_gpiod_get_index(dev, "data", 2);
> >> > >>>
> >> > >>> and so on.
> >> > >>>
> >> > >>> Signed-off-by: Mika Westerberg
> >> > >>
> >> > >> Cool. :-)
> >> > >>
> >> > >> Any objections anyone?
> >> > >
> >> > > Actually, I do. Not in the idea, but in the implementation. The way
> >> > > this gets encoded:
> >> > >
> >> > > Package () {
> >> > > \_SB.GPIO, 0, 0, 0,
> >> > > \_SB.GPIO, 1, 0, 0,
> >> > > \_SB.GPIO, 2, 0, 1,
> >> > > }
> >> > >
> >> > > Means that decoding each GPIO tuple requires the length of a tuple to
> >> > > be
> >> > > fixed, or to implement a DT-like #gpio-cells. If it is fixed, then
> >> > > there
> >> > > is no way to expand the binding later. Can this be done in the
> >> > > following
> >> > > way instead?
> >> > >
> >> > > Package () {
> >> > > Package () { \_SB.GPIO, 0, 0, 0 },
> >> > > Package () { \_SB.GPIO, 1, 0, 0 },
> >> > > Package () { \_SB.GPIO, 2, 0, 1 },
> >> > > }
> >> > >
> >> > > This is one of the biggest pains in device tree. We don't have any way
> >> > > to group tuples so it requires looking up stuff across the tree to
> >> > > figure out how to parse each multi-item property.
> >> > >
> >> > > I know that last year we talked about how bios vendors would get
> >> > > complicated properties wrong, but I think there is little risk in this
> >> > > case. If the property is encoded wrong, the driver simply won't work
> >> > > and
> >> > > it is unlikely to get shipped before being fixed.
> >> >
> >> > This particular nesting of Packages is expressly prohibited by the
> >> > Device Properties UUID for the reasons you mention.
> >> >
> >> > http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
> >>
> >> Also we don't use properties where single name is assigned to multiple
> >> GPIOs
> >> anywhere in the current device-properties patchset, so this is not
> >> relevant at
> >> the moment.
> >>
> >> Moreover, even if we were to use them, we would need to ensure that this:
> >>
> >> Package () {
> >> \_SB.GPIO, 0, 0, 0
> >> }
> >>
> >> was equivalent to
> >>
> >> Package () {
> >> Package () { \_SB.GPIO, 0, 0, 0 }
> >> }
> >>
> >> This is not impossible to do and I suppose we could even explain that in
> >> the
> >> implementation guide document in a sensible way, but that would require the
> >> document linked above to be changed first and *then* we can think about
> >> writing
> >> kernel code to it. Not the other way around, please.
> >>
> >> So Grant, do you want us to proceed with that?
> >
> > Before you reply, one more observation that seems to be relevant.
> >
> > In ACPI, both this:
> >
> > Package () {
> > \_SB.GPIO, 0, 0, 0,
> > \_SB.GPIO, 1, 0, 0,
> > \_SB.GPIO, 2, 0, 1,
> > }
> >
> > and this:
> >
> > Package () {
> > Package () { \_SB.GPIO, 0, 0, 0 },
> > Package () { \_SB.GPIO, 1, 0, 0 },
> > Package () { \_SB.GPIO, 2, 0, 1 },
> > }
> >
> > carry the same information, because every element of a package has a type,
> > so there is no danger of confusing an ACPI_TYPE_LOCAL_REFERENCE with
> > ACPI_TYPE_INTEGER. Thus one can easily count the number of GPIOs
> > represented
> > by the first package by counting the number of reference elements in it.
> > The second one has more structure which in this particular case is arguably
> > redundant.
>
> Okay, this make sense. I'm okay with this approach, and I would
> recommend making that the only valid method for parsing in
> acpi_dev_get_property_reference(). Get rid of the
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Mon, Nov 3, 2014 at 9:52 PM, Rafael J. Wysocki wrote:
> On Monday, November 03, 2014 04:25:08 PM Rafael J. Wysocki wrote:
>> On Sunday, November 02, 2014 08:49:37 PM Darren Hart wrote:
>> >
>> > On 11/1/14 4:11, Grant Likely wrote:
>> > > On Tue, 28 Oct 2014 22:59:57 +0100
>> > > , "Rafael J. Wysocki"
>> > > wrote:
>> > >> On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
>> > >>> acpi_dev_add_driver_gpios() makes it possible to set up mapping between
>> > >>> properties and ACPI GpioIo resources in a driver, so we can take index
>> > >>> parameter in acpi_find_gpio() into use with _DSD device properties now.
>> > >>>
>> > >>> This index can be used to select a GPIO from a property with multiple
>> > >>> GPIOs:
>> > >>>
>> > >>> Package () {
>> > >>> "data-gpios",
>> > >>> Package () {
>> > >>> \_SB.GPIO, 0, 0, 0,
>> > >>> \_SB.GPIO, 1, 0, 0,
>> > >>> \_SB.GPIO, 2, 0, 1,
>> > >>> }
>> > >>> }
>> > >>>
>> > >>> In order to retrieve the last GPIO from a driver we can simply do:
>> > >>>
>> > >>> desc = devm_gpiod_get_index(dev, "data", 2);
>> > >>>
>> > >>> and so on.
>> > >>>
>> > >>> Signed-off-by: Mika Westerberg
>> > >>
>> > >> Cool. :-)
>> > >>
>> > >> Any objections anyone?
>> > >
>> > > Actually, I do. Not in the idea, but in the implementation. The way this
>> > > gets encoded:
>> > >
>> > > Package () {
>> > > \_SB.GPIO, 0, 0, 0,
>> > > \_SB.GPIO, 1, 0, 0,
>> > > \_SB.GPIO, 2, 0, 1,
>> > > }
>> > >
>> > > Means that decoding each GPIO tuple requires the length of a tuple to be
>> > > fixed, or to implement a DT-like #gpio-cells. If it is fixed, then there
>> > > is no way to expand the binding later. Can this be done in the following
>> > > way instead?
>> > >
>> > > Package () {
>> > > Package () { \_SB.GPIO, 0, 0, 0 },
>> > > Package () { \_SB.GPIO, 1, 0, 0 },
>> > > Package () { \_SB.GPIO, 2, 0, 1 },
>> > > }
>> > >
>> > > This is one of the biggest pains in device tree. We don't have any way
>> > > to group tuples so it requires looking up stuff across the tree to
>> > > figure out how to parse each multi-item property.
>> > >
>> > > I know that last year we talked about how bios vendors would get
>> > > complicated properties wrong, but I think there is little risk in this
>> > > case. If the property is encoded wrong, the driver simply won't work and
>> > > it is unlikely to get shipped before being fixed.
>> >
>> > This particular nesting of Packages is expressly prohibited by the
>> > Device Properties UUID for the reasons you mention.
>> >
>> > http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
>>
>> Also we don't use properties where single name is assigned to multiple GPIOs
>> anywhere in the current device-properties patchset, so this is not relevant
>> at
>> the moment.
>>
>> Moreover, even if we were to use them, we would need to ensure that this:
>>
>> Package () {
>> \_SB.GPIO, 0, 0, 0
>> }
>>
>> was equivalent to
>>
>> Package () {
>> Package () { \_SB.GPIO, 0, 0, 0 }
>> }
>>
>> This is not impossible to do and I suppose we could even explain that in the
>> implementation guide document in a sensible way, but that would require the
>> document linked above to be changed first and *then* we can think about
>> writing
>> kernel code to it. Not the other way around, please.
>>
>> So Grant, do you want us to proceed with that?
>
> Before you reply, one more observation that seems to be relevant.
>
> In ACPI, both this:
>
> Package () {
> \_SB.GPIO, 0, 0, 0,
> \_SB.GPIO, 1, 0, 0,
> \_SB.GPIO, 2, 0, 1,
> }
>
> and this:
>
> Package () {
> Package () { \_SB.GPIO, 0, 0, 0 },
> Package () { \_SB.GPIO, 1, 0, 0 },
> Package () { \_SB.GPIO, 2, 0, 1 },
> }
>
> carry the same information, because every element of a package has a type,
> so there is no danger of confusing an ACPI_TYPE_LOCAL_REFERENCE with
> ACPI_TYPE_INTEGER. Thus one can easily count the number of GPIOs represented
> by the first package by counting the number of reference elements in it.
> The second one has more structure which in this particular case is arguably
> redundant.
Okay, this make sense. I'm okay with this approach, and I would
recommend making that the only valid method for parsing in
acpi_dev_get_property_reference(). Get rid of the *size_prop argument
so that it always behaves the same way and users aren't tempted to do
something clever.
>
> Of course, that's not the case for list properties where each item consists
> of a bunch of integers, like
>
> Package () {
> Package () { 0, 0, 0 },
> Package () { 1, 0, 0 },
> Package () { 2, 0, 1 },
> }
>
> but I'm not sure if this
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Mon, Nov 3, 2014 at 9:52 PM, Rafael J. Wysocki r...@rjwysocki.net wrote:
On Monday, November 03, 2014 04:25:08 PM Rafael J. Wysocki wrote:
On Sunday, November 02, 2014 08:49:37 PM Darren Hart wrote:
On 11/1/14 4:11, Grant Likely wrote:
On Tue, 28 Oct 2014 22:59:57 +0100
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
acpi_dev_add_driver_gpios() makes it possible to set up mapping between
properties and ACPI GpioIo resources in a driver, so we can take index
parameter in acpi_find_gpio() into use with _DSD device properties now.
This index can be used to select a GPIO from a property with multiple
GPIOs:
Package () {
data-gpios,
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, data, 2);
and so on.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Cool. :-)
Any objections anyone?
Actually, I do. Not in the idea, but in the implementation. The way this
gets encoded:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
Means that decoding each GPIO tuple requires the length of a tuple to be
fixed, or to implement a DT-like #gpio-cells. If it is fixed, then there
is no way to expand the binding later. Can this be done in the following
way instead?
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
This is one of the biggest pains in device tree. We don't have any way
to group tuples so it requires looking up stuff across the tree to
figure out how to parse each multi-item property.
I know that last year we talked about how bios vendors would get
complicated properties wrong, but I think there is little risk in this
case. If the property is encoded wrong, the driver simply won't work and
it is unlikely to get shipped before being fixed.
This particular nesting of Packages is expressly prohibited by the
Device Properties UUID for the reasons you mention.
http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
Also we don't use properties where single name is assigned to multiple GPIOs
anywhere in the current device-properties patchset, so this is not relevant
at
the moment.
Moreover, even if we were to use them, we would need to ensure that this:
Package () {
\_SB.GPIO, 0, 0, 0
}
was equivalent to
Package () {
Package () { \_SB.GPIO, 0, 0, 0 }
}
This is not impossible to do and I suppose we could even explain that in the
implementation guide document in a sensible way, but that would require the
document linked above to be changed first and *then* we can think about
writing
kernel code to it. Not the other way around, please.
So Grant, do you want us to proceed with that?
Before you reply, one more observation that seems to be relevant.
In ACPI, both this:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
and this:
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
carry the same information, because every element of a package has a type,
so there is no danger of confusing an ACPI_TYPE_LOCAL_REFERENCE with
ACPI_TYPE_INTEGER. Thus one can easily count the number of GPIOs represented
by the first package by counting the number of reference elements in it.
The second one has more structure which in this particular case is arguably
redundant.
Okay, this make sense. I'm okay with this approach, and I would
recommend making that the only valid method for parsing in
acpi_dev_get_property_reference(). Get rid of the *size_prop argument
so that it always behaves the same way and users aren't tempted to do
something clever.
Of course, that's not the case for list properties where each item consists
of a bunch of integers, like
Package () {
Package () { 0, 0, 0 },
Package () { 1, 0, 0 },
Package () { 2, 0, 1 },
}
but I'm not sure if this is relevant at all.
Probably not. With a pure list it isn't implicitly referencing data in
another node. In the ref+args pattern the length of each tuple can
vary based on which node it references, but on a simple list the
parsing is going to be a lot simpler.
g.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Tuesday, November 04, 2014 02:48:40 PM Grant Likely wrote:
On Mon, Nov 3, 2014 at 9:52 PM, Rafael J. Wysocki r...@rjwysocki.net wrote:
On Monday, November 03, 2014 04:25:08 PM Rafael J. Wysocki wrote:
On Sunday, November 02, 2014 08:49:37 PM Darren Hart wrote:
On 11/1/14 4:11, Grant Likely wrote:
On Tue, 28 Oct 2014 22:59:57 +0100
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
acpi_dev_add_driver_gpios() makes it possible to set up mapping
between
properties and ACPI GpioIo resources in a driver, so we can take
index
parameter in acpi_find_gpio() into use with _DSD device properties
now.
This index can be used to select a GPIO from a property with multiple
GPIOs:
Package () {
data-gpios,
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, data, 2);
and so on.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Cool. :-)
Any objections anyone?
Actually, I do. Not in the idea, but in the implementation. The way
this gets encoded:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
Means that decoding each GPIO tuple requires the length of a tuple to
be
fixed, or to implement a DT-like #gpio-cells. If it is fixed, then
there
is no way to expand the binding later. Can this be done in the
following
way instead?
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
This is one of the biggest pains in device tree. We don't have any way
to group tuples so it requires looking up stuff across the tree to
figure out how to parse each multi-item property.
I know that last year we talked about how bios vendors would get
complicated properties wrong, but I think there is little risk in this
case. If the property is encoded wrong, the driver simply won't work
and
it is unlikely to get shipped before being fixed.
This particular nesting of Packages is expressly prohibited by the
Device Properties UUID for the reasons you mention.
http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
Also we don't use properties where single name is assigned to multiple
GPIOs
anywhere in the current device-properties patchset, so this is not
relevant at
the moment.
Moreover, even if we were to use them, we would need to ensure that this:
Package () {
\_SB.GPIO, 0, 0, 0
}
was equivalent to
Package () {
Package () { \_SB.GPIO, 0, 0, 0 }
}
This is not impossible to do and I suppose we could even explain that in
the
implementation guide document in a sensible way, but that would require the
document linked above to be changed first and *then* we can think about
writing
kernel code to it. Not the other way around, please.
So Grant, do you want us to proceed with that?
Before you reply, one more observation that seems to be relevant.
In ACPI, both this:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
and this:
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
carry the same information, because every element of a package has a type,
so there is no danger of confusing an ACPI_TYPE_LOCAL_REFERENCE with
ACPI_TYPE_INTEGER. Thus one can easily count the number of GPIOs
represented
by the first package by counting the number of reference elements in it.
The second one has more structure which in this particular case is arguably
redundant.
Okay, this make sense. I'm okay with this approach, and I would
recommend making that the only valid method for parsing in
acpi_dev_get_property_reference(). Get rid of the *size_prop argument
so that it always behaves the same way and users aren't tempted to do
something clever.
OK, I'll send a followup patch to remove the size_prop arg from
acpi_dev_get_property_reference().
Of course, that's not the case for list properties where each item consists
of a bunch of integers, like
Package () {
Package () { 0, 0, 0 },
Package () { 1, 0, 0 },
Package () { 2, 0, 1 },
}
but I'm not sure if this
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Tuesday, November 04, 2014 05:06:40 PM Rafael J. Wysocki wrote:
On Tuesday, November 04, 2014 02:48:40 PM Grant Likely wrote:
On Mon, Nov 3, 2014 at 9:52 PM, Rafael J. Wysocki r...@rjwysocki.net
wrote:
On Monday, November 03, 2014 04:25:08 PM Rafael J. Wysocki wrote:
On Sunday, November 02, 2014 08:49:37 PM Darren Hart wrote:
On 11/1/14 4:11, Grant Likely wrote:
On Tue, 28 Oct 2014 22:59:57 +0100
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
acpi_dev_add_driver_gpios() makes it possible to set up mapping
between
properties and ACPI GpioIo resources in a driver, so we can take
index
parameter in acpi_find_gpio() into use with _DSD device properties
now.
This index can be used to select a GPIO from a property with
multiple
GPIOs:
Package () {
data-gpios,
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, data, 2);
and so on.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Cool. :-)
Any objections anyone?
Actually, I do. Not in the idea, but in the implementation. The way
this gets encoded:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
Means that decoding each GPIO tuple requires the length of a tuple
to be
fixed, or to implement a DT-like #gpio-cells. If it is fixed, then
there
is no way to expand the binding later. Can this be done in the
following
way instead?
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
This is one of the biggest pains in device tree. We don't have any
way
to group tuples so it requires looking up stuff across the tree to
figure out how to parse each multi-item property.
I know that last year we talked about how bios vendors would get
complicated properties wrong, but I think there is little risk in
this
case. If the property is encoded wrong, the driver simply won't work
and
it is unlikely to get shipped before being fixed.
This particular nesting of Packages is expressly prohibited by the
Device Properties UUID for the reasons you mention.
http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
Also we don't use properties where single name is assigned to multiple
GPIOs
anywhere in the current device-properties patchset, so this is not
relevant at
the moment.
Moreover, even if we were to use them, we would need to ensure that this:
Package () {
\_SB.GPIO, 0, 0, 0
}
was equivalent to
Package () {
Package () { \_SB.GPIO, 0, 0, 0 }
}
This is not impossible to do and I suppose we could even explain that in
the
implementation guide document in a sensible way, but that would require
the
document linked above to be changed first and *then* we can think about
writing
kernel code to it. Not the other way around, please.
So Grant, do you want us to proceed with that?
Before you reply, one more observation that seems to be relevant.
In ACPI, both this:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
and this:
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
carry the same information, because every element of a package has a type,
so there is no danger of confusing an ACPI_TYPE_LOCAL_REFERENCE with
ACPI_TYPE_INTEGER. Thus one can easily count the number of GPIOs
represented
by the first package by counting the number of reference elements in it.
The second one has more structure which in this particular case is
arguably
redundant.
Okay, this make sense. I'm okay with this approach, and I would
recommend making that the only valid method for parsing in
acpi_dev_get_property_reference(). Get rid of the *size_prop argument
so that it always behaves the same way and users aren't tempted to do
something clever.
OK, I'll send a followup patch to remove the size_prop arg from
acpi_dev_get_property_reference().
This:
---
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Tue, Nov 4, 2014 at 10:54 PM, Rafael J. Wysocki r...@rjwysocki.net wrote:
On Tuesday, November 04, 2014 05:06:40 PM Rafael J. Wysocki wrote:
On Tuesday, November 04, 2014 02:48:40 PM Grant Likely wrote:
On Mon, Nov 3, 2014 at 9:52 PM, Rafael J. Wysocki r...@rjwysocki.net
wrote:
On Monday, November 03, 2014 04:25:08 PM Rafael J. Wysocki wrote:
On Sunday, November 02, 2014 08:49:37 PM Darren Hart wrote:
On 11/1/14 4:11, Grant Likely wrote:
On Tue, 28 Oct 2014 22:59:57 +0100
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
acpi_dev_add_driver_gpios() makes it possible to set up mapping
between
properties and ACPI GpioIo resources in a driver, so we can take
index
parameter in acpi_find_gpio() into use with _DSD device
properties now.
This index can be used to select a GPIO from a property with
multiple
GPIOs:
Package () {
data-gpios,
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, data, 2);
and so on.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Cool. :-)
Any objections anyone?
Actually, I do. Not in the idea, but in the implementation. The way
this gets encoded:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
Means that decoding each GPIO tuple requires the length of a tuple
to be
fixed, or to implement a DT-like #gpio-cells. If it is fixed, then
there
is no way to expand the binding later. Can this be done in the
following
way instead?
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
This is one of the biggest pains in device tree. We don't have any
way
to group tuples so it requires looking up stuff across the tree to
figure out how to parse each multi-item property.
I know that last year we talked about how bios vendors would get
complicated properties wrong, but I think there is little risk in
this
case. If the property is encoded wrong, the driver simply won't
work and
it is unlikely to get shipped before being fixed.
This particular nesting of Packages is expressly prohibited by the
Device Properties UUID for the reasons you mention.
http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
Also we don't use properties where single name is assigned to multiple
GPIOs
anywhere in the current device-properties patchset, so this is not
relevant at
the moment.
Moreover, even if we were to use them, we would need to ensure that
this:
Package () {
\_SB.GPIO, 0, 0, 0
}
was equivalent to
Package () {
Package () { \_SB.GPIO, 0, 0, 0 }
}
This is not impossible to do and I suppose we could even explain that
in the
implementation guide document in a sensible way, but that would require
the
document linked above to be changed first and *then* we can think about
writing
kernel code to it. Not the other way around, please.
So Grant, do you want us to proceed with that?
Before you reply, one more observation that seems to be relevant.
In ACPI, both this:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
and this:
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
carry the same information, because every element of a package has a
type,
so there is no danger of confusing an ACPI_TYPE_LOCAL_REFERENCE with
ACPI_TYPE_INTEGER. Thus one can easily count the number of GPIOs
represented
by the first package by counting the number of reference elements in it.
The second one has more structure which in this particular case is
arguably
redundant.
Okay, this make sense. I'm okay with this approach, and I would
recommend making that the only valid method for parsing in
acpi_dev_get_property_reference(). Get rid of the *size_prop argument
so that it always behaves the same way and users aren't tempted to do
something clever.
OK, I'll send a followup patch to remove the size_prop arg from
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Monday, November 03, 2014 04:25:08 PM Rafael J. Wysocki wrote:
> On Sunday, November 02, 2014 08:49:37 PM Darren Hart wrote:
> >
> > On 11/1/14 4:11, Grant Likely wrote:
> > > On Tue, 28 Oct 2014 22:59:57 +0100
> > > , "Rafael J. Wysocki"
> > > wrote:
> > >> On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
> > >>> acpi_dev_add_driver_gpios() makes it possible to set up mapping between
> > >>> properties and ACPI GpioIo resources in a driver, so we can take index
> > >>> parameter in acpi_find_gpio() into use with _DSD device properties now.
> > >>>
> > >>> This index can be used to select a GPIO from a property with multiple
> > >>> GPIOs:
> > >>>
> > >>> Package () {
> > >>> "data-gpios",
> > >>> Package () {
> > >>> \_SB.GPIO, 0, 0, 0,
> > >>> \_SB.GPIO, 1, 0, 0,
> > >>> \_SB.GPIO, 2, 0, 1,
> > >>> }
> > >>> }
> > >>>
> > >>> In order to retrieve the last GPIO from a driver we can simply do:
> > >>>
> > >>> desc = devm_gpiod_get_index(dev, "data", 2);
> > >>>
> > >>> and so on.
> > >>>
> > >>> Signed-off-by: Mika Westerberg
> > >>
> > >> Cool. :-)
> > >>
> > >> Any objections anyone?
> > >
> > > Actually, I do. Not in the idea, but in the implementation. The way this
> > > gets encoded:
> > >
> > > Package () {
> > > \_SB.GPIO, 0, 0, 0,
> > > \_SB.GPIO, 1, 0, 0,
> > > \_SB.GPIO, 2, 0, 1,
> > > }
> > >
> > > Means that decoding each GPIO tuple requires the length of a tuple to be
> > > fixed, or to implement a DT-like #gpio-cells. If it is fixed, then there
> > > is no way to expand the binding later. Can this be done in the following
> > > way instead?
> > >
> > > Package () {
> > > Package () { \_SB.GPIO, 0, 0, 0 },
> > > Package () { \_SB.GPIO, 1, 0, 0 },
> > > Package () { \_SB.GPIO, 2, 0, 1 },
> > > }
> > >
> > > This is one of the biggest pains in device tree. We don't have any way
> > > to group tuples so it requires looking up stuff across the tree to
> > > figure out how to parse each multi-item property.
> > >
> > > I know that last year we talked about how bios vendors would get
> > > complicated properties wrong, but I think there is little risk in this
> > > case. If the property is encoded wrong, the driver simply won't work and
> > > it is unlikely to get shipped before being fixed.
> >
> > This particular nesting of Packages is expressly prohibited by the
> > Device Properties UUID for the reasons you mention.
> >
> > http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
>
> Also we don't use properties where single name is assigned to multiple GPIOs
> anywhere in the current device-properties patchset, so this is not relevant at
> the moment.
>
> Moreover, even if we were to use them, we would need to ensure that this:
>
> Package () {
> \_SB.GPIO, 0, 0, 0
> }
>
> was equivalent to
>
> Package () {
> Package () { \_SB.GPIO, 0, 0, 0 }
> }
>
> This is not impossible to do and I suppose we could even explain that in the
> implementation guide document in a sensible way, but that would require the
> document linked above to be changed first and *then* we can think about
> writing
> kernel code to it. Not the other way around, please.
>
> So Grant, do you want us to proceed with that?
Before you reply, one more observation that seems to be relevant.
In ACPI, both this:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
and this:
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
carry the same information, because every element of a package has a type,
so there is no danger of confusing an ACPI_TYPE_LOCAL_REFERENCE with
ACPI_TYPE_INTEGER. Thus one can easily count the number of GPIOs represented
by the first package by counting the number of reference elements in it.
The second one has more structure which in this particular case is arguably
redundant.
Of course, that's not the case for list properties where each item consists
of a bunch of integers, like
Package () {
Package () { 0, 0, 0 },
Package () { 1, 0, 0 },
Package () { 2, 0, 1 },
}
but I'm not sure if this is relevant at all.
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Sunday, November 02, 2014 08:49:37 PM Darren Hart wrote:
>
> On 11/1/14 4:11, Grant Likely wrote:
> > On Tue, 28 Oct 2014 22:59:57 +0100
> > , "Rafael J. Wysocki"
> > wrote:
> >> On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
> >>> acpi_dev_add_driver_gpios() makes it possible to set up mapping between
> >>> properties and ACPI GpioIo resources in a driver, so we can take index
> >>> parameter in acpi_find_gpio() into use with _DSD device properties now.
> >>>
> >>> This index can be used to select a GPIO from a property with multiple
> >>> GPIOs:
> >>>
> >>> Package () {
> >>> "data-gpios",
> >>> Package () {
> >>> \_SB.GPIO, 0, 0, 0,
> >>> \_SB.GPIO, 1, 0, 0,
> >>> \_SB.GPIO, 2, 0, 1,
> >>> }
> >>> }
> >>>
> >>> In order to retrieve the last GPIO from a driver we can simply do:
> >>>
> >>> desc = devm_gpiod_get_index(dev, "data", 2);
> >>>
> >>> and so on.
> >>>
> >>> Signed-off-by: Mika Westerberg
> >>
> >> Cool. :-)
> >>
> >> Any objections anyone?
> >
> > Actually, I do. Not in the idea, but in the implementation. The way this
> > gets encoded:
> >
> > Package () {
> > \_SB.GPIO, 0, 0, 0,
> > \_SB.GPIO, 1, 0, 0,
> > \_SB.GPIO, 2, 0, 1,
> > }
> >
> > Means that decoding each GPIO tuple requires the length of a tuple to be
> > fixed, or to implement a DT-like #gpio-cells. If it is fixed, then there
> > is no way to expand the binding later. Can this be done in the following
> > way instead?
> >
> > Package () {
> > Package () { \_SB.GPIO, 0, 0, 0 },
> > Package () { \_SB.GPIO, 1, 0, 0 },
> > Package () { \_SB.GPIO, 2, 0, 1 },
> > }
> >
> > This is one of the biggest pains in device tree. We don't have any way
> > to group tuples so it requires looking up stuff across the tree to
> > figure out how to parse each multi-item property.
> >
> > I know that last year we talked about how bios vendors would get
> > complicated properties wrong, but I think there is little risk in this
> > case. If the property is encoded wrong, the driver simply won't work and
> > it is unlikely to get shipped before being fixed.
>
> This particular nesting of Packages is expressly prohibited by the
> Device Properties UUID for the reasons you mention.
>
> http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
Also we don't use properties where single name is assigned to multiple GPIOs
anywhere in the current device-properties patchset, so this is not relevant at
the moment.
Moreover, even if we were to use them, we would need to ensure that this:
Package () {
\_SB.GPIO, 0, 0, 0
}
was equivalent to
Package () {
Package () { \_SB.GPIO, 0, 0, 0 }
}
This is not impossible to do and I suppose we could even explain that in the
implementation guide document in a sensible way, but that would require the
document linked above to be changed first and *then* we can think about writing
kernel code to it. Not the other way around, please.
So Grant, do you want us to proceed with that?
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Sunday, November 02, 2014 08:49:37 PM Darren Hart wrote:
On 11/1/14 4:11, Grant Likely wrote:
On Tue, 28 Oct 2014 22:59:57 +0100
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
acpi_dev_add_driver_gpios() makes it possible to set up mapping between
properties and ACPI GpioIo resources in a driver, so we can take index
parameter in acpi_find_gpio() into use with _DSD device properties now.
This index can be used to select a GPIO from a property with multiple
GPIOs:
Package () {
data-gpios,
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, data, 2);
and so on.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Cool. :-)
Any objections anyone?
Actually, I do. Not in the idea, but in the implementation. The way this
gets encoded:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
Means that decoding each GPIO tuple requires the length of a tuple to be
fixed, or to implement a DT-like #gpio-cells. If it is fixed, then there
is no way to expand the binding later. Can this be done in the following
way instead?
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
This is one of the biggest pains in device tree. We don't have any way
to group tuples so it requires looking up stuff across the tree to
figure out how to parse each multi-item property.
I know that last year we talked about how bios vendors would get
complicated properties wrong, but I think there is little risk in this
case. If the property is encoded wrong, the driver simply won't work and
it is unlikely to get shipped before being fixed.
This particular nesting of Packages is expressly prohibited by the
Device Properties UUID for the reasons you mention.
http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
Also we don't use properties where single name is assigned to multiple GPIOs
anywhere in the current device-properties patchset, so this is not relevant at
the moment.
Moreover, even if we were to use them, we would need to ensure that this:
Package () {
\_SB.GPIO, 0, 0, 0
}
was equivalent to
Package () {
Package () { \_SB.GPIO, 0, 0, 0 }
}
This is not impossible to do and I suppose we could even explain that in the
implementation guide document in a sensible way, but that would require the
document linked above to be changed first and *then* we can think about writing
kernel code to it. Not the other way around, please.
So Grant, do you want us to proceed with that?
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Monday, November 03, 2014 04:25:08 PM Rafael J. Wysocki wrote:
On Sunday, November 02, 2014 08:49:37 PM Darren Hart wrote:
On 11/1/14 4:11, Grant Likely wrote:
On Tue, 28 Oct 2014 22:59:57 +0100
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
acpi_dev_add_driver_gpios() makes it possible to set up mapping between
properties and ACPI GpioIo resources in a driver, so we can take index
parameter in acpi_find_gpio() into use with _DSD device properties now.
This index can be used to select a GPIO from a property with multiple
GPIOs:
Package () {
data-gpios,
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, data, 2);
and so on.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Cool. :-)
Any objections anyone?
Actually, I do. Not in the idea, but in the implementation. The way this
gets encoded:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
Means that decoding each GPIO tuple requires the length of a tuple to be
fixed, or to implement a DT-like #gpio-cells. If it is fixed, then there
is no way to expand the binding later. Can this be done in the following
way instead?
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
This is one of the biggest pains in device tree. We don't have any way
to group tuples so it requires looking up stuff across the tree to
figure out how to parse each multi-item property.
I know that last year we talked about how bios vendors would get
complicated properties wrong, but I think there is little risk in this
case. If the property is encoded wrong, the driver simply won't work and
it is unlikely to get shipped before being fixed.
This particular nesting of Packages is expressly prohibited by the
Device Properties UUID for the reasons you mention.
http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
Also we don't use properties where single name is assigned to multiple GPIOs
anywhere in the current device-properties patchset, so this is not relevant at
the moment.
Moreover, even if we were to use them, we would need to ensure that this:
Package () {
\_SB.GPIO, 0, 0, 0
}
was equivalent to
Package () {
Package () { \_SB.GPIO, 0, 0, 0 }
}
This is not impossible to do and I suppose we could even explain that in the
implementation guide document in a sensible way, but that would require the
document linked above to be changed first and *then* we can think about
writing
kernel code to it. Not the other way around, please.
So Grant, do you want us to proceed with that?
Before you reply, one more observation that seems to be relevant.
In ACPI, both this:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
and this:
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
carry the same information, because every element of a package has a type,
so there is no danger of confusing an ACPI_TYPE_LOCAL_REFERENCE with
ACPI_TYPE_INTEGER. Thus one can easily count the number of GPIOs represented
by the first package by counting the number of reference elements in it.
The second one has more structure which in this particular case is arguably
redundant.
Of course, that's not the case for list properties where each item consists
of a bunch of integers, like
Package () {
Package () { 0, 0, 0 },
Package () { 1, 0, 0 },
Package () { 2, 0, 1 },
}
but I'm not sure if this is relevant at all.
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On 11/1/14 4:11, Grant Likely wrote:
> On Tue, 28 Oct 2014 22:59:57 +0100
> , "Rafael J. Wysocki"
> wrote:
>> On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
>>> acpi_dev_add_driver_gpios() makes it possible to set up mapping between
>>> properties and ACPI GpioIo resources in a driver, so we can take index
>>> parameter in acpi_find_gpio() into use with _DSD device properties now.
>>>
>>> This index can be used to select a GPIO from a property with multiple
>>> GPIOs:
>>>
>>> Package () {
>>> "data-gpios",
>>> Package () {
>>> \_SB.GPIO, 0, 0, 0,
>>> \_SB.GPIO, 1, 0, 0,
>>> \_SB.GPIO, 2, 0, 1,
>>> }
>>> }
>>>
>>> In order to retrieve the last GPIO from a driver we can simply do:
>>>
>>> desc = devm_gpiod_get_index(dev, "data", 2);
>>>
>>> and so on.
>>>
>>> Signed-off-by: Mika Westerberg
>>
>> Cool. :-)
>>
>> Any objections anyone?
>
> Actually, I do. Not in the idea, but in the implementation. The way this gets
> encoded:
>
> Package () {
> \_SB.GPIO, 0, 0, 0,
> \_SB.GPIO, 1, 0, 0,
> \_SB.GPIO, 2, 0, 1,
> }
>
> Means that decoding each GPIO tuple requires the length of a tuple to be
> fixed, or to implement a DT-like #gpio-cells. If it is fixed, then there
> is no way to expand the binding later. Can this be done in the following
> way instead?
>
> Package () {
> Package () { \_SB.GPIO, 0, 0, 0 },
> Package () { \_SB.GPIO, 1, 0, 0 },
> Package () { \_SB.GPIO, 2, 0, 1 },
> }
>
> This is one of the biggest pains in device tree. We don't have any way
> to group tuples so it requires looking up stuff across the tree to
> figure out how to parse each multi-item property.
>
> I know that last year we talked about how bios vendors would get
> complicated properties wrong, but I think there is little risk in this
> case. If the property is encoded wrong, the driver simply won't work and
> it is unlikely to get shipped before being fixed.
This particular nesting of Packages is expressly prohibited by the
Device Properties UUID for the reasons you mention.
http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
2.1 Data Format Definition
...
" a Package consisting entirely of Integer, String, or Reference objects
(and
specifically not containing a nested Package)."
That said, I am not fond of the many properties mixed in as a single
Package. We discussed this at some length while this was being proposed,
and this was deemed the lesser evil.
--
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On 11/1/14 4:11, Grant Likely wrote:
On Tue, 28 Oct 2014 22:59:57 +0100
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
acpi_dev_add_driver_gpios() makes it possible to set up mapping between
properties and ACPI GpioIo resources in a driver, so we can take index
parameter in acpi_find_gpio() into use with _DSD device properties now.
This index can be used to select a GPIO from a property with multiple
GPIOs:
Package () {
data-gpios,
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, data, 2);
and so on.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Cool. :-)
Any objections anyone?
Actually, I do. Not in the idea, but in the implementation. The way this gets
encoded:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
Means that decoding each GPIO tuple requires the length of a tuple to be
fixed, or to implement a DT-like #gpio-cells. If it is fixed, then there
is no way to expand the binding later. Can this be done in the following
way instead?
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
This is one of the biggest pains in device tree. We don't have any way
to group tuples so it requires looking up stuff across the tree to
figure out how to parse each multi-item property.
I know that last year we talked about how bios vendors would get
complicated properties wrong, but I think there is little risk in this
case. If the property is encoded wrong, the driver simply won't work and
it is unlikely to get shipped before being fixed.
This particular nesting of Packages is expressly prohibited by the
Device Properties UUID for the reasons you mention.
http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
2.1 Data Format Definition
...
a Package consisting entirely of Integer, String, or Reference objects
(and
specifically not containing a nested Package).
That said, I am not fond of the many properties mixed in as a single
Package. We discussed this at some length while this was being proposed,
and this was deemed the lesser evil.
--
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Tue, 28 Oct 2014 22:59:57 +0100
, "Rafael J. Wysocki"
wrote:
> On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
> > acpi_dev_add_driver_gpios() makes it possible to set up mapping between
> > properties and ACPI GpioIo resources in a driver, so we can take index
> > parameter in acpi_find_gpio() into use with _DSD device properties now.
> >
> > This index can be used to select a GPIO from a property with multiple
> > GPIOs:
> >
> > Package () {
> > "data-gpios",
> > Package () {
> > \_SB.GPIO, 0, 0, 0,
> > \_SB.GPIO, 1, 0, 0,
> > \_SB.GPIO, 2, 0, 1,
> > }
> > }
> >
> > In order to retrieve the last GPIO from a driver we can simply do:
> >
> > desc = devm_gpiod_get_index(dev, "data", 2);
> >
> > and so on.
> >
> > Signed-off-by: Mika Westerberg
>
> Cool. :-)
>
> Any objections anyone?
Actually, I do. Not in the idea, but in the implementation. The way this gets
encoded:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
Means that decoding each GPIO tuple requires the length of a tuple to be
fixed, or to implement a DT-like #gpio-cells. If it is fixed, then there
is no way to expand the binding later. Can this be done in the following
way instead?
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
This is one of the biggest pains in device tree. We don't have any way
to group tuples so it requires looking up stuff across the tree to
figure out how to parse each multi-item property.
I know that last year we talked about how bios vendors would get
complicated properties wrong, but I think there is little risk in this
case. If the property is encoded wrong, the driver simply won't work and
it is unlikely to get shipped before being fixed.
g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Tue, 28 Oct 2014 22:59:57 +0100
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
acpi_dev_add_driver_gpios() makes it possible to set up mapping between
properties and ACPI GpioIo resources in a driver, so we can take index
parameter in acpi_find_gpio() into use with _DSD device properties now.
This index can be used to select a GPIO from a property with multiple
GPIOs:
Package () {
data-gpios,
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, data, 2);
and so on.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Cool. :-)
Any objections anyone?
Actually, I do. Not in the idea, but in the implementation. The way this gets
encoded:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
Means that decoding each GPIO tuple requires the length of a tuple to be
fixed, or to implement a DT-like #gpio-cells. If it is fixed, then there
is no way to expand the binding later. Can this be done in the following
way instead?
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
This is one of the biggest pains in device tree. We don't have any way
to group tuples so it requires looking up stuff across the tree to
figure out how to parse each multi-item property.
I know that last year we talked about how bios vendors would get
complicated properties wrong, but I think there is little risk in this
case. If the property is encoded wrong, the driver simply won't work and
it is unlikely to get shipped before being fixed.
g.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Wed, Oct 29, 2014 at 03:51:59PM +0100, Rafael J. Wysocki wrote: > Never mind. I need to rebase the series anyway because of a bug in the > second patch, so I'll fold the $subject one into patch [5/12]. Thank you. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Wednesday, October 29, 2014 03:42:23 PM Rafael J. Wysocki wrote:
> On Wednesday, October 29, 2014 10:54:29 AM Mika Westerberg wrote:
> > On Wed, Oct 29, 2014 at 04:41:47PM +0900, Alexandre Courbot wrote:
> > > On 10/29/2014 06:59 AM, Rafael J. Wysocki wrote:
> > > >On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
> > > >>acpi_dev_add_driver_gpios() makes it possible to set up mapping between
> > > >>properties and ACPI GpioIo resources in a driver, so we can take index
> > > >>parameter in acpi_find_gpio() into use with _DSD device properties now.
> > > >>
> > > >>This index can be used to select a GPIO from a property with multiple
> > > >>GPIOs:
> > > >>
> > > >> Package () {
> > > >>"data-gpios",
> > > >>Package () {
> > > >>\_SB.GPIO, 0, 0, 0,
> > > >>\_SB.GPIO, 1, 0, 0,
> > > >>\_SB.GPIO, 2, 0, 1,
> > > >>}
> > > >> }
> > > >>
> > > >>In order to retrieve the last GPIO from a driver we can simply do:
> > > >>
> > > >> desc = devm_gpiod_get_index(dev, "data", 2);
> > > >>
> > > >>and so on.
> > > >>
> > > >>Signed-off-by: Mika Westerberg
> > > >
> > > >Cool. :-)
> > > >
> > > >Any objections anyone?
> > >
> > > Looks good to me!
> > >
> > > Acked-by: Alexandre Courbot
> >
> > Thanks!
> >
> > > Since this looks like a bug fix, shouldn't this be squashed into the
> > > relevant patch of the device-properties set?
> >
> > I agree.
>
> Well, except that the set is in my linux-next branch now and I very much
> prefer to do fixes on top of it.
>
> Besides, I'm not sure if that even matters for the current series.
Never mind. I need to rebase the series anyway because of a bug in the
second patch, so I'll fold the $subject one into patch [5/12].
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Wednesday, October 29, 2014 10:54:29 AM Mika Westerberg wrote:
> On Wed, Oct 29, 2014 at 04:41:47PM +0900, Alexandre Courbot wrote:
> > On 10/29/2014 06:59 AM, Rafael J. Wysocki wrote:
> > >On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
> > >>acpi_dev_add_driver_gpios() makes it possible to set up mapping between
> > >>properties and ACPI GpioIo resources in a driver, so we can take index
> > >>parameter in acpi_find_gpio() into use with _DSD device properties now.
> > >>
> > >>This index can be used to select a GPIO from a property with multiple
> > >>GPIOs:
> > >>
> > >> Package () {
> > >> "data-gpios",
> > >> Package () {
> > >> \_SB.GPIO, 0, 0, 0,
> > >> \_SB.GPIO, 1, 0, 0,
> > >> \_SB.GPIO, 2, 0, 1,
> > >> }
> > >> }
> > >>
> > >>In order to retrieve the last GPIO from a driver we can simply do:
> > >>
> > >> desc = devm_gpiod_get_index(dev, "data", 2);
> > >>
> > >>and so on.
> > >>
> > >>Signed-off-by: Mika Westerberg
> > >
> > >Cool. :-)
> > >
> > >Any objections anyone?
> >
> > Looks good to me!
> >
> > Acked-by: Alexandre Courbot
>
> Thanks!
>
> > Since this looks like a bug fix, shouldn't this be squashed into the
> > relevant patch of the device-properties set?
>
> I agree.
Well, except that the set is in my linux-next branch now and I very much
prefer to do fixes on top of it.
Besides, I'm not sure if that even matters for the current series.
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Wed, Oct 29, 2014 at 04:41:47PM +0900, Alexandre Courbot wrote:
> On 10/29/2014 06:59 AM, Rafael J. Wysocki wrote:
> >On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
> >>acpi_dev_add_driver_gpios() makes it possible to set up mapping between
> >>properties and ACPI GpioIo resources in a driver, so we can take index
> >>parameter in acpi_find_gpio() into use with _DSD device properties now.
> >>
> >>This index can be used to select a GPIO from a property with multiple
> >>GPIOs:
> >>
> >> Package () {
> >>"data-gpios",
> >>Package () {
> >>\_SB.GPIO, 0, 0, 0,
> >>\_SB.GPIO, 1, 0, 0,
> >>\_SB.GPIO, 2, 0, 1,
> >>}
> >> }
> >>
> >>In order to retrieve the last GPIO from a driver we can simply do:
> >>
> >> desc = devm_gpiod_get_index(dev, "data", 2);
> >>
> >>and so on.
> >>
> >>Signed-off-by: Mika Westerberg
> >
> >Cool. :-)
> >
> >Any objections anyone?
>
> Looks good to me!
>
> Acked-by: Alexandre Courbot
Thanks!
> Since this looks like a bug fix, shouldn't this be squashed into the
> relevant patch of the device-properties set?
I agree.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On 10/29/2014 06:59 AM, Rafael J. Wysocki wrote:
On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
acpi_dev_add_driver_gpios() makes it possible to set up mapping between
properties and ACPI GpioIo resources in a driver, so we can take index
parameter in acpi_find_gpio() into use with _DSD device properties now.
This index can be used to select a GPIO from a property with multiple
GPIOs:
Package () {
"data-gpios",
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, "data", 2);
and so on.
Signed-off-by: Mika Westerberg
Cool. :-)
Any objections anyone?
Looks good to me!
Acked-by: Alexandre Courbot
Since this looks like a bug fix, shouldn't this be squashed into the
relevant patch of the device-properties set?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On 10/29/2014 06:59 AM, Rafael J. Wysocki wrote:
On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
acpi_dev_add_driver_gpios() makes it possible to set up mapping between
properties and ACPI GpioIo resources in a driver, so we can take index
parameter in acpi_find_gpio() into use with _DSD device properties now.
This index can be used to select a GPIO from a property with multiple
GPIOs:
Package () {
data-gpios,
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, data, 2);
and so on.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Cool. :-)
Any objections anyone?
Looks good to me!
Acked-by: Alexandre Courbot acour...@nvidia.com
Since this looks like a bug fix, shouldn't this be squashed into the
relevant patch of the device-properties set?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Wed, Oct 29, 2014 at 04:41:47PM +0900, Alexandre Courbot wrote:
On 10/29/2014 06:59 AM, Rafael J. Wysocki wrote:
On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
acpi_dev_add_driver_gpios() makes it possible to set up mapping between
properties and ACPI GpioIo resources in a driver, so we can take index
parameter in acpi_find_gpio() into use with _DSD device properties now.
This index can be used to select a GPIO from a property with multiple
GPIOs:
Package () {
data-gpios,
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, data, 2);
and so on.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Cool. :-)
Any objections anyone?
Looks good to me!
Acked-by: Alexandre Courbot acour...@nvidia.com
Thanks!
Since this looks like a bug fix, shouldn't this be squashed into the
relevant patch of the device-properties set?
I agree.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Wednesday, October 29, 2014 10:54:29 AM Mika Westerberg wrote:
On Wed, Oct 29, 2014 at 04:41:47PM +0900, Alexandre Courbot wrote:
On 10/29/2014 06:59 AM, Rafael J. Wysocki wrote:
On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
acpi_dev_add_driver_gpios() makes it possible to set up mapping between
properties and ACPI GpioIo resources in a driver, so we can take index
parameter in acpi_find_gpio() into use with _DSD device properties now.
This index can be used to select a GPIO from a property with multiple
GPIOs:
Package () {
data-gpios,
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, data, 2);
and so on.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Cool. :-)
Any objections anyone?
Looks good to me!
Acked-by: Alexandre Courbot acour...@nvidia.com
Thanks!
Since this looks like a bug fix, shouldn't this be squashed into the
relevant patch of the device-properties set?
I agree.
Well, except that the set is in my linux-next branch now and I very much
prefer to do fixes on top of it.
Besides, I'm not sure if that even matters for the current series.
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Wednesday, October 29, 2014 03:42:23 PM Rafael J. Wysocki wrote:
On Wednesday, October 29, 2014 10:54:29 AM Mika Westerberg wrote:
On Wed, Oct 29, 2014 at 04:41:47PM +0900, Alexandre Courbot wrote:
On 10/29/2014 06:59 AM, Rafael J. Wysocki wrote:
On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
acpi_dev_add_driver_gpios() makes it possible to set up mapping between
properties and ACPI GpioIo resources in a driver, so we can take index
parameter in acpi_find_gpio() into use with _DSD device properties now.
This index can be used to select a GPIO from a property with multiple
GPIOs:
Package () {
data-gpios,
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, data, 2);
and so on.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Cool. :-)
Any objections anyone?
Looks good to me!
Acked-by: Alexandre Courbot acour...@nvidia.com
Thanks!
Since this looks like a bug fix, shouldn't this be squashed into the
relevant patch of the device-properties set?
I agree.
Well, except that the set is in my linux-next branch now and I very much
prefer to do fixes on top of it.
Besides, I'm not sure if that even matters for the current series.
Never mind. I need to rebase the series anyway because of a bug in the
second patch, so I'll fold the $subject one into patch [5/12].
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Wed, Oct 29, 2014 at 03:51:59PM +0100, Rafael J. Wysocki wrote: Never mind. I need to rebase the series anyway because of a bug in the second patch, so I'll fold the $subject one into patch [5/12]. Thank you. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
> acpi_dev_add_driver_gpios() makes it possible to set up mapping between
> properties and ACPI GpioIo resources in a driver, so we can take index
> parameter in acpi_find_gpio() into use with _DSD device properties now.
>
> This index can be used to select a GPIO from a property with multiple
> GPIOs:
>
> Package () {
> "data-gpios",
> Package () {
> \_SB.GPIO, 0, 0, 0,
> \_SB.GPIO, 1, 0, 0,
> \_SB.GPIO, 2, 0, 1,
> }
> }
>
> In order to retrieve the last GPIO from a driver we can simply do:
>
> desc = devm_gpiod_get_index(dev, "data", 2);
>
> and so on.
>
> Signed-off-by: Mika Westerberg
Cool. :-)
Any objections anyone?
> ---
> This is on top of latest linux-pm/device-properties.
>
> drivers/gpio/gpiolib.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index fbf717a56b0a..58659dbe702a 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1522,7 +1522,7 @@ static struct gpio_desc *acpi_find_gpio(struct device
> *dev, const char *con_id,
>suffixes[i]);
> }
>
> - desc = acpi_get_gpiod_by_index(adev, propname, 0, );
> + desc = acpi_get_gpiod_by_index(adev, propname, idx, );
> if (!IS_ERR(desc) || (PTR_ERR(desc) == -EPROBE_DEFER))
> break;
> }
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On 10/28/14 4:15, Mika Westerberg wrote:
> acpi_dev_add_driver_gpios() makes it possible to set up mapping between
> properties and ACPI GpioIo resources in a driver, so we can take index
> parameter in acpi_find_gpio() into use with _DSD device properties now.
>
> This index can be used to select a GPIO from a property with multiple
> GPIOs:
>
> Package () {
> "data-gpios",
> Package () {
> \_SB.GPIO, 0, 0, 0,
> \_SB.GPIO, 1, 0, 0,
> \_SB.GPIO, 2, 0, 1,
> }
> }
>
> In order to retrieve the last GPIO from a driver we can simply do:
>
> desc = devm_gpiod_get_index(dev, "data", 2);
>
> and so on.
>
> Signed-off-by: Mika Westerberg
Nice, that was a gap that had been gnawing at me. Thanks Mika :-)
Acked-by: Darren Hart
--
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
acpi_dev_add_driver_gpios() makes it possible to set up mapping between
properties and ACPI GpioIo resources in a driver, so we can take index
parameter in acpi_find_gpio() into use with _DSD device properties now.
This index can be used to select a GPIO from a property with multiple
GPIOs:
Package () {
"data-gpios",
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, "data", 2);
and so on.
Signed-off-by: Mika Westerberg
---
This is on top of latest linux-pm/device-properties.
drivers/gpio/gpiolib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index fbf717a56b0a..58659dbe702a 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1522,7 +1522,7 @@ static struct gpio_desc *acpi_find_gpio(struct device
*dev, const char *con_id,
suffixes[i]);
}
- desc = acpi_get_gpiod_by_index(adev, propname, 0, );
+ desc = acpi_get_gpiod_by_index(adev, propname, idx, );
if (!IS_ERR(desc) || (PTR_ERR(desc) == -EPROBE_DEFER))
break;
}
--
2.1.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
acpi_dev_add_driver_gpios() makes it possible to set up mapping between
properties and ACPI GpioIo resources in a driver, so we can take index
parameter in acpi_find_gpio() into use with _DSD device properties now.
This index can be used to select a GPIO from a property with multiple
GPIOs:
Package () {
data-gpios,
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, data, 2);
and so on.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
---
This is on top of latest linux-pm/device-properties.
drivers/gpio/gpiolib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index fbf717a56b0a..58659dbe702a 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1522,7 +1522,7 @@ static struct gpio_desc *acpi_find_gpio(struct device
*dev, const char *con_id,
suffixes[i]);
}
- desc = acpi_get_gpiod_by_index(adev, propname, 0, info);
+ desc = acpi_get_gpiod_by_index(adev, propname, idx, info);
if (!IS_ERR(desc) || (PTR_ERR(desc) == -EPROBE_DEFER))
break;
}
--
2.1.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On 10/28/14 4:15, Mika Westerberg wrote:
acpi_dev_add_driver_gpios() makes it possible to set up mapping between
properties and ACPI GpioIo resources in a driver, so we can take index
parameter in acpi_find_gpio() into use with _DSD device properties now.
This index can be used to select a GPIO from a property with multiple
GPIOs:
Package () {
data-gpios,
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, data, 2);
and so on.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Nice, that was a gap that had been gnawing at me. Thanks Mika :-)
Acked-by: Darren Hart dvh...@linux.intel.com
--
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
acpi_dev_add_driver_gpios() makes it possible to set up mapping between
properties and ACPI GpioIo resources in a driver, so we can take index
parameter in acpi_find_gpio() into use with _DSD device properties now.
This index can be used to select a GPIO from a property with multiple
GPIOs:
Package () {
data-gpios,
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, data, 2);
and so on.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Cool. :-)
Any objections anyone?
---
This is on top of latest linux-pm/device-properties.
drivers/gpio/gpiolib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index fbf717a56b0a..58659dbe702a 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1522,7 +1522,7 @@ static struct gpio_desc *acpi_find_gpio(struct device
*dev, const char *con_id,
suffixes[i]);
}
- desc = acpi_get_gpiod_by_index(adev, propname, 0, info);
+ desc = acpi_get_gpiod_by_index(adev, propname, idx, info);
if (!IS_ERR(desc) || (PTR_ERR(desc) == -EPROBE_DEFER))
break;
}
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
