Re: [RFC PATCH 2/9] ACPI: Document ACPI device specific properties

2014-08-19 Thread Mark Rutland
On Tue, Aug 19, 2014 at 06:45:46AM +0100, Darren Hart wrote:
> On Mon, Aug 18, 2014 at 11:54:29AM +0100, Mark Rutland wrote:
> > Hi Mika,
> >
> > While I am very much in favour of having a structured way of describing
> > device specific data in ACPI I am very concerned by the idea of assuming
> > (a false) equivalence with DT. More on that below.
>
> Hi Mark,

Hi Darren,

> The equivalence is intended in the sense that we should be able to represent 
> any
> of the existing schemas with the same properties in ACPI as in DT.
>
> ...
>
> > > +An example device where we might need properties is a GPIO keys device.
> > > +In addition to the GpioIo/GpioInt resources the driver needs to know how
> > > +to map each GpioIo resource to the corresponding Linux input event.
> > > +
> > > +To solve this we add the following ACPI device properties from the
> > > +gpio-keys schema:
> > > +
> > > +   Device (KEYS) {
> > > +   Name (_DSD, Package () {
> > > +   ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > > +   Package () {
> > > +   Package () {"poll-interval", 100},
> > > +   Package () {"autorepeat", 1}
> > > +   }
> > > +   })
> > > +
> > > +   Device (BTN0) {
> > > +   Name (_DSD, Package () {
> > > +   
> > > ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > > +   Package () {
> > > +   Package () {"linux,code", 105},
> > > +   Package () {"linux,input-type", 
> > > 1},
> >
> > The leakage of Linux implementation details into DTBs is bad enough. I
> > would really not like to see this kind of thing in ACPI tables.
> >
> > Leaking this into HW description of any sort completely rids said
> > description of any OS independence, and it's usually short-sighted,
> > nonsensical, or ill-defined (what does 'autorepeat' actually mean?).
> >
> > I am very worried by the prospect of _DSD making it harder rather than
> > easier for an arbitrary OS to use ACPI data due to assumptions being
> > made about (a particular revision of) a particular OS.
>
> This is perhaps not the best example to include in the documentation. A 
> guiding
> principle has been to only represent in the DSD that which is an actual 
> property
> of the hardware.

We follow the same principle in DT, but we still have to fight against
bad bindings which do not follow this principle.

Ideally all examples would align with that principle.

> It is also a goal of the mechanism to enable ACPI systems to reuse the 200+
> existing OF-aware drivers in the kernel today. If those schemas are
> ill-conceived, we should work to improve the schemas.
>
> On the point of OS-independence, while I agree that using something like the 
> USB
> keycodes here would be better, the namespacing employed in these properties
> makes it possible to use the same DSD for muliple OSes as another OS could add
> it's own keycode, for example, without conflict. Again, that is really an 
> issue
> with the schemas, not the _DSD mechanism itself.

It's true that the issues are with the individual bindings, and that
_DSD is simply a mechanism for conveying them. We certainly need to fix
up those bindings, but for legacy reasons some bad bindings will have to
stay supported when provided from a DTB. However is no reason for an
ACPI system to pick up any of the linux,* DT properties.

So we must treat ACPI and DTB as different providers of information, and
we need to be careful not to assume a false equivalence.

I'm not a big fan of providing different OSs with different views of the
world. I thought everyone using ACPI hated _OSI and the mess that
usually results in?

> Perhaps we should use a different example in the documentation as I would 
> agree
> this is not a model schema.

Yes please.

> > > +Of course the device driver then needs to iterate over these devices but
> > > +it can be done easily via the ->children field of the companion ACPI
> > > +device. This will be demonstrated later on in the document.
> > > +
> > > +If there is an existing Device Tree binding for a device, it is expected
> > > +that the same bindings are used with ACPI properties, so that the driver
> > > +dealing with the device needs only minor modifications if any.
> >
> > For simple devices like a UART which just has a "clock-frequency"
> > property in DT, importing both the concept of the property and the key
> > name itself is a sensible approach.
> >
> > However, I do not think this makes sense as a general design rule. ACPI
> > and DT already have different idioms for describing certain things (e.g.
> > GPIOs, interrupts) and care needs to be taken to distinguish simple
> > device-specific properties from class-specific properties or implied
> > linkages between devices.
>
> I believe we have 

Re: [RFC PATCH 2/9] ACPI: Document ACPI device specific properties

2014-08-19 Thread Mark Rutland
On Tue, Aug 19, 2014 at 06:45:46AM +0100, Darren Hart wrote:
 On Mon, Aug 18, 2014 at 11:54:29AM +0100, Mark Rutland wrote:
  Hi Mika,
 
  While I am very much in favour of having a structured way of describing
  device specific data in ACPI I am very concerned by the idea of assuming
  (a false) equivalence with DT. More on that below.

 Hi Mark,

Hi Darren,

 The equivalence is intended in the sense that we should be able to represent 
 any
 of the existing schemas with the same properties in ACPI as in DT.

 ...

   +An example device where we might need properties is a GPIO keys device.
   +In addition to the GpioIo/GpioInt resources the driver needs to know how
   +to map each GpioIo resource to the corresponding Linux input event.
   +
   +To solve this we add the following ACPI device properties from the
   +gpio-keys schema:
   +
   +   Device (KEYS) {
   +   Name (_DSD, Package () {
   +   ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
   +   Package () {
   +   Package () {poll-interval, 100},
   +   Package () {autorepeat, 1}
   +   }
   +   })
   +
   +   Device (BTN0) {
   +   Name (_DSD, Package () {
   +   
   ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
   +   Package () {
   +   Package () {linux,code, 105},
   +   Package () {linux,input-type, 
   1},
 
  The leakage of Linux implementation details into DTBs is bad enough. I
  would really not like to see this kind of thing in ACPI tables.
 
  Leaking this into HW description of any sort completely rids said
  description of any OS independence, and it's usually short-sighted,
  nonsensical, or ill-defined (what does 'autorepeat' actually mean?).
 
  I am very worried by the prospect of _DSD making it harder rather than
  easier for an arbitrary OS to use ACPI data due to assumptions being
  made about (a particular revision of) a particular OS.

 This is perhaps not the best example to include in the documentation. A 
 guiding
 principle has been to only represent in the DSD that which is an actual 
 property
 of the hardware.

We follow the same principle in DT, but we still have to fight against
bad bindings which do not follow this principle.

Ideally all examples would align with that principle.

 It is also a goal of the mechanism to enable ACPI systems to reuse the 200+
 existing OF-aware drivers in the kernel today. If those schemas are
 ill-conceived, we should work to improve the schemas.

 On the point of OS-independence, while I agree that using something like the 
 USB
 keycodes here would be better, the namespacing employed in these properties
 makes it possible to use the same DSD for muliple OSes as another OS could add
 it's own keycode, for example, without conflict. Again, that is really an 
 issue
 with the schemas, not the _DSD mechanism itself.

It's true that the issues are with the individual bindings, and that
_DSD is simply a mechanism for conveying them. We certainly need to fix
up those bindings, but for legacy reasons some bad bindings will have to
stay supported when provided from a DTB. However is no reason for an
ACPI system to pick up any of the linux,* DT properties.

So we must treat ACPI and DTB as different providers of information, and
we need to be careful not to assume a false equivalence.

I'm not a big fan of providing different OSs with different views of the
world. I thought everyone using ACPI hated _OSI and the mess that
usually results in?

 Perhaps we should use a different example in the documentation as I would 
 agree
 this is not a model schema.

Yes please.

   +Of course the device driver then needs to iterate over these devices but
   +it can be done easily via the -children field of the companion ACPI
   +device. This will be demonstrated later on in the document.
   +
   +If there is an existing Device Tree binding for a device, it is expected
   +that the same bindings are used with ACPI properties, so that the driver
   +dealing with the device needs only minor modifications if any.
 
  For simple devices like a UART which just has a clock-frequency
  property in DT, importing both the concept of the property and the key
  name itself is a sensible approach.
 
  However, I do not think this makes sense as a general design rule. ACPI
  and DT already have different idioms for describing certain things (e.g.
  GPIOs, interrupts) and care needs to be taken to distinguish simple
  device-specific properties from class-specific properties or implied
  linkages between devices.

 I believe we have GPIO handled well in this series. In DT terms, ACPI is the
 controller, providing a list of GpioIO|GpioInt resources in the CRS of the
 referenced object. Arguments are Resource Index, Pin 

Re: [RFC PATCH 2/9] ACPI: Document ACPI device specific properties

2014-08-18 Thread Darren Hart
On Mon, Aug 18, 2014 at 11:54:29AM +0100, Mark Rutland wrote:
> Hi Mika,
> 
> While I am very much in favour of having a structured way of describing
> device specific data in ACPI I am very concerned by the idea of assuming
> (a false) equivalence with DT. More on that below.

Hi Mark,

The equivalence is intended in the sense that we should be able to represent any
of the existing schemas with the same properties in ACPI as in DT.

...

> > +An example device where we might need properties is a GPIO keys device.
> > +In addition to the GpioIo/GpioInt resources the driver needs to know how
> > +to map each GpioIo resource to the corresponding Linux input event.
> > +
> > +To solve this we add the following ACPI device properties from the
> > +gpio-keys schema:
> > +
> > +   Device (KEYS) {
> > +   Name (_DSD, Package () {
> > +   ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > +   Package () {
> > +   Package () {"poll-interval", 100},
> > +   Package () {"autorepeat", 1}
> > +   }
> > +   })
> > +
> > +   Device (BTN0) {
> > +   Name (_DSD, Package () {
> > +   
> > ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > +   Package () {
> > +   Package () {"linux,code", 105},
> > +   Package () {"linux,input-type", 1},
> 
> The leakage of Linux implementation details into DTBs is bad enough. I
> would really not like to see this kind of thing in ACPI tables.
> 
> Leaking this into HW description of any sort completely rids said
> description of any OS independence, and it's usually short-sighted,
> nonsensical, or ill-defined (what does 'autorepeat' actually mean?).
> 
> I am very worried by the prospect of _DSD making it harder rather than
> easier for an arbitrary OS to use ACPI data due to assumptions being
> made about (a particular revision of) a particular OS.

This is perhaps not the best example to include in the documentation. A guiding
principle has been to only represent in the DSD that which is an actual property
of the hardware.

It is also a goal of the mechanism to enable ACPI systems to reuse the 200+
existing OF-aware drivers in the kernel today. If those schemas are
ill-conceived, we should work to improve the schemas.

On the point of OS-independence, while I agree that using something like the USB
keycodes here would be better, the namespacing employed in these properties
makes it possible to use the same DSD for muliple OSes as another OS could add
it's own keycode, for example, without conflict. Again, that is really an issue
with the schemas, not the _DSD mechanism itself.

Perhaps we should use a different example in the documentation as I would agree
this is not a model schema.

> > +Of course the device driver then needs to iterate over these devices but
> > +it can be done easily via the ->children field of the companion ACPI
> > +device. This will be demonstrated later on in the document.
> > +
> > +If there is an existing Device Tree binding for a device, it is expected
> > +that the same bindings are used with ACPI properties, so that the driver
> > +dealing with the device needs only minor modifications if any.
> 
> For simple devices like a UART which just has a "clock-frequency"
> property in DT, importing both the concept of the property and the key
> name itself is a sensible approach.
> 
> However, I do not think this makes sense as a general design rule. ACPI
> and DT already have different idioms for describing certain things (e.g.
> GPIOs, interrupts) and care needs to be taken to distinguish simple
> device-specific properties from class-specific properties or implied
> linkages between devices.

I believe we have GPIO handled well in this series. In DT terms, ACPI is the
controller, providing a list of GpioIO|GpioInt resources in the CRS of the
referenced object. Arguments are Resource Index, Pin Index, Polarity. The rest
of the properties are handled via the GpioIO|GpioInt resources in the _CRS.

> 
> The "#clock-cells" and "#pwm-cells" _DSD examples in ACPI 5.1 are
> complete nonsense. They import an idiom from DT into ACPI without

Yeah... coming up with good examples for a generic mechanism which are both
descriptive and universally unobjectionable. Good point. We might be able to
improve this via an errata update. What would you recommend?

> considering the surrounding context (cells, lack of typing, phandles,

How are the phandles not covered via the ACPI namespace references?

The point on cells is a good one, and I believe some of this can be mitigated by
providing subsystem specific accessors as we have for GPIO, for example.

> etc). As an example, I worry that why it may be trivial to map pinctrl
> bindings into ACPI _DSD, there is no clear way that 

Re: [RFC PATCH 2/9] ACPI: Document ACPI device specific properties

2014-08-18 Thread Mika Westerberg
On Mon, Aug 18, 2014 at 11:54:29AM +0100, Mark Rutland wrote:
> Hi Mika,
> 
> While I am very much in favour of having a structured way of describing
> device specific data in ACPI I am very concerned by the idea of assuming
> (a false) equivalence with DT. More on that below.
> 
> On Sun, Aug 17, 2014 at 07:04:12AM +0100, Mika Westerberg wrote:
> > This document describes the data format and interfaces of ACPI device
> > specific properties.
> > 
> > Signed-off-by: Mika Westerberg 
> > Signed-off-by: Darren Hart 
> > ---
> >  Documentation/acpi/properties.txt | 359 
> > ++
> >  1 file changed, 359 insertions(+)
> >  create mode 100644 Documentation/acpi/properties.txt
> > 
> > diff --git a/Documentation/acpi/properties.txt 
> > b/Documentation/acpi/properties.txt
> > new file mode 100644
> > index ..a1e93267c1fa
> > --- /dev/null
> > +++ b/Documentation/acpi/properties.txt
> > @@ -0,0 +1,359 @@
> > +ACPI device properties
> > +==
> > +This document describes the format and interfaces of ACPI device
> > +properties as specified in "Device Properties UUID For _DSD" available
> > +here:
> > +
> > +http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
> > +
> > +1. Introduction
> > +---
> > +In systems that use ACPI and want to take advantage of device specific
> > +properties, there needs to be a standard way to return and extract
> > +name-value pairs for a given ACPI device.
> > +
> > +An ACPI device that wants to export its properties must implement a
> > +static name called _DSD that takes no arguments and returns a package of
> > +packages:
> > +
> > +   Name (_DSD, Package () {
> > +   ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > +   Package () {
> > +   Package () {"name1", },
> > +   Package () {"name2", }
> > +   }
> > +   })
> > +
> > +The UUID identifies contents of the following package. In case of ACPI
> > +device properties it is daffd814-6eba-4d8c-8a91-bc9bbf4aa301.
> > +
> > +In each returned package, the first item is the name and must be a string.
> > +The corresponding value can be a string, integer, reference, or package. If
> > +a package it may only contain strings, integers, and references.
> > +
> > +An example device where we might need properties is a GPIO keys device.
> > +In addition to the GpioIo/GpioInt resources the driver needs to know how
> > +to map each GpioIo resource to the corresponding Linux input event.
> > +
> > +To solve this we add the following ACPI device properties from the
> > +gpio-keys schema:
> > +
> > +   Device (KEYS) {
> > +   Name (_DSD, Package () {
> > +   ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > +   Package () {
> > +   Package () {"poll-interval", 100},
> > +   Package () {"autorepeat", 1}
> > +   }
> > +   })
> > +
> > +   Device (BTN0) {
> > +   Name (_DSD, Package () {
> > +   
> > ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > +   Package () {
> > +   Package () {"linux,code", 105},
> > +   Package () {"linux,input-type", 1},
> 
> The leakage of Linux implementation details into DTBs is bad enough. I
> would really not like to see this kind of thing in ACPI tables.
> 
> Leaking this into HW description of any sort completely rids said
> description of any OS independence, and it's usually short-sighted,
> nonsensical, or ill-defined (what does 'autorepeat' actually mean?).
> 
> I am very worried by the prospect of _DSD making it harder rather than
> easier for an arbitrary OS to use ACPI data due to assumptions being
> made about (a particular revision of) a particular OS.

You are right - the above example is bad in that sense. The reason why
we have the above is that we tested the implementation on Minnowboard
and the two devices we converted used these descriptions.

We should pick a better example that describes the HW independent of the
OS.

> 
> > +   ...
> > +   }
> > +   })
> > +   }
> > +
> > +   ...
> > +   }
> > +
> > +Of course the device driver then needs to iterate over these devices but
> > +it can be done easily via the ->children field of the companion ACPI
> > +device. This will be demonstrated later on in the document.
> > +
> > +If there is an existing Device Tree binding for a device, it is expected
> > +that the same bindings are used with ACPI properties, so that the driver
> > +dealing with the device needs only minor modifications if any.
> 
> For simple devices like a UART which just has a 

Re: [RFC PATCH 2/9] ACPI: Document ACPI device specific properties

2014-08-18 Thread Mark Rutland
Hi Mika,

While I am very much in favour of having a structured way of describing
device specific data in ACPI I am very concerned by the idea of assuming
(a false) equivalence with DT. More on that below.

On Sun, Aug 17, 2014 at 07:04:12AM +0100, Mika Westerberg wrote:
> This document describes the data format and interfaces of ACPI device
> specific properties.
> 
> Signed-off-by: Mika Westerberg 
> Signed-off-by: Darren Hart 
> ---
>  Documentation/acpi/properties.txt | 359 
> ++
>  1 file changed, 359 insertions(+)
>  create mode 100644 Documentation/acpi/properties.txt
> 
> diff --git a/Documentation/acpi/properties.txt 
> b/Documentation/acpi/properties.txt
> new file mode 100644
> index ..a1e93267c1fa
> --- /dev/null
> +++ b/Documentation/acpi/properties.txt
> @@ -0,0 +1,359 @@
> +ACPI device properties
> +==
> +This document describes the format and interfaces of ACPI device
> +properties as specified in "Device Properties UUID For _DSD" available
> +here:
> +
> +http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
> +
> +1. Introduction
> +---
> +In systems that use ACPI and want to take advantage of device specific
> +properties, there needs to be a standard way to return and extract
> +name-value pairs for a given ACPI device.
> +
> +An ACPI device that wants to export its properties must implement a
> +static name called _DSD that takes no arguments and returns a package of
> +packages:
> +
> +   Name (_DSD, Package () {
> +   ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> +   Package () {
> +   Package () {"name1", },
> +   Package () {"name2", }
> +   }
> +   })
> +
> +The UUID identifies contents of the following package. In case of ACPI
> +device properties it is daffd814-6eba-4d8c-8a91-bc9bbf4aa301.
> +
> +In each returned package, the first item is the name and must be a string.
> +The corresponding value can be a string, integer, reference, or package. If
> +a package it may only contain strings, integers, and references.
> +
> +An example device where we might need properties is a GPIO keys device.
> +In addition to the GpioIo/GpioInt resources the driver needs to know how
> +to map each GpioIo resource to the corresponding Linux input event.
> +
> +To solve this we add the following ACPI device properties from the
> +gpio-keys schema:
> +
> +   Device (KEYS) {
> +   Name (_DSD, Package () {
> +   ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> +   Package () {
> +   Package () {"poll-interval", 100},
> +   Package () {"autorepeat", 1}
> +   }
> +   })
> +
> +   Device (BTN0) {
> +   Name (_DSD, Package () {
> +   
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> +   Package () {
> +   Package () {"linux,code", 105},
> +   Package () {"linux,input-type", 1},

The leakage of Linux implementation details into DTBs is bad enough. I
would really not like to see this kind of thing in ACPI tables.

Leaking this into HW description of any sort completely rids said
description of any OS independence, and it's usually short-sighted,
nonsensical, or ill-defined (what does 'autorepeat' actually mean?).

I am very worried by the prospect of _DSD making it harder rather than
easier for an arbitrary OS to use ACPI data due to assumptions being
made about (a particular revision of) a particular OS.

> +   ...
> +   }
> +   })
> +   }
> +
> +   ...
> +   }
> +
> +Of course the device driver then needs to iterate over these devices but
> +it can be done easily via the ->children field of the companion ACPI
> +device. This will be demonstrated later on in the document.
> +
> +If there is an existing Device Tree binding for a device, it is expected
> +that the same bindings are used with ACPI properties, so that the driver
> +dealing with the device needs only minor modifications if any.

For simple devices like a UART which just has a "clock-frequency"
property in DT, importing both the concept of the property and the key
name itself is a sensible approach.

However, I do not think this makes sense as a general design rule. ACPI
and DT already have different idioms for describing certain things (e.g.
GPIOs, interrupts) and care needs to be taken to distinguish simple
device-specific properties from class-specific properties or implied
linkages between devices.

The "#clock-cells" and "#pwm-cells" _DSD examples in ACPI 5.1 are
complete nonsense. They import an idiom from DT into ACPI without

Re: [RFC PATCH 2/9] ACPI: Document ACPI device specific properties

2014-08-18 Thread Mark Rutland
Hi Mika,

While I am very much in favour of having a structured way of describing
device specific data in ACPI I am very concerned by the idea of assuming
(a false) equivalence with DT. More on that below.

On Sun, Aug 17, 2014 at 07:04:12AM +0100, Mika Westerberg wrote:
 This document describes the data format and interfaces of ACPI device
 specific properties.
 
 Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
 Signed-off-by: Darren Hart dvh...@linux.intel.com
 ---
  Documentation/acpi/properties.txt | 359 
 ++
  1 file changed, 359 insertions(+)
  create mode 100644 Documentation/acpi/properties.txt
 
 diff --git a/Documentation/acpi/properties.txt 
 b/Documentation/acpi/properties.txt
 new file mode 100644
 index ..a1e93267c1fa
 --- /dev/null
 +++ b/Documentation/acpi/properties.txt
 @@ -0,0 +1,359 @@
 +ACPI device properties
 +==
 +This document describes the format and interfaces of ACPI device
 +properties as specified in Device Properties UUID For _DSD available
 +here:
 +
 +http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
 +
 +1. Introduction
 +---
 +In systems that use ACPI and want to take advantage of device specific
 +properties, there needs to be a standard way to return and extract
 +name-value pairs for a given ACPI device.
 +
 +An ACPI device that wants to export its properties must implement a
 +static name called _DSD that takes no arguments and returns a package of
 +packages:
 +
 +   Name (_DSD, Package () {
 +   ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
 +   Package () {
 +   Package () {name1, VALUE1},
 +   Package () {name2, VALUE2}
 +   }
 +   })
 +
 +The UUID identifies contents of the following package. In case of ACPI
 +device properties it is daffd814-6eba-4d8c-8a91-bc9bbf4aa301.
 +
 +In each returned package, the first item is the name and must be a string.
 +The corresponding value can be a string, integer, reference, or package. If
 +a package it may only contain strings, integers, and references.
 +
 +An example device where we might need properties is a GPIO keys device.
 +In addition to the GpioIo/GpioInt resources the driver needs to know how
 +to map each GpioIo resource to the corresponding Linux input event.
 +
 +To solve this we add the following ACPI device properties from the
 +gpio-keys schema:
 +
 +   Device (KEYS) {
 +   Name (_DSD, Package () {
 +   ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
 +   Package () {
 +   Package () {poll-interval, 100},
 +   Package () {autorepeat, 1}
 +   }
 +   })
 +
 +   Device (BTN0) {
 +   Name (_DSD, Package () {
 +   
 ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
 +   Package () {
 +   Package () {linux,code, 105},
 +   Package () {linux,input-type, 1},

The leakage of Linux implementation details into DTBs is bad enough. I
would really not like to see this kind of thing in ACPI tables.

Leaking this into HW description of any sort completely rids said
description of any OS independence, and it's usually short-sighted,
nonsensical, or ill-defined (what does 'autorepeat' actually mean?).

I am very worried by the prospect of _DSD making it harder rather than
easier for an arbitrary OS to use ACPI data due to assumptions being
made about (a particular revision of) a particular OS.

 +   ...
 +   }
 +   })
 +   }
 +
 +   ...
 +   }
 +
 +Of course the device driver then needs to iterate over these devices but
 +it can be done easily via the -children field of the companion ACPI
 +device. This will be demonstrated later on in the document.
 +
 +If there is an existing Device Tree binding for a device, it is expected
 +that the same bindings are used with ACPI properties, so that the driver
 +dealing with the device needs only minor modifications if any.

For simple devices like a UART which just has a clock-frequency
property in DT, importing both the concept of the property and the key
name itself is a sensible approach.

However, I do not think this makes sense as a general design rule. ACPI
and DT already have different idioms for describing certain things (e.g.
GPIOs, interrupts) and care needs to be taken to distinguish simple
device-specific properties from class-specific properties or implied
linkages between devices.

The #clock-cells and #pwm-cells _DSD examples in ACPI 5.1 are
complete nonsense. They import an idiom from DT into ACPI without
considering the surrounding context (cells, lack of 

Re: [RFC PATCH 2/9] ACPI: Document ACPI device specific properties

2014-08-18 Thread Mika Westerberg
On Mon, Aug 18, 2014 at 11:54:29AM +0100, Mark Rutland wrote:
 Hi Mika,
 
 While I am very much in favour of having a structured way of describing
 device specific data in ACPI I am very concerned by the idea of assuming
 (a false) equivalence with DT. More on that below.
 
 On Sun, Aug 17, 2014 at 07:04:12AM +0100, Mika Westerberg wrote:
  This document describes the data format and interfaces of ACPI device
  specific properties.
  
  Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
  Signed-off-by: Darren Hart dvh...@linux.intel.com
  ---
   Documentation/acpi/properties.txt | 359 
  ++
   1 file changed, 359 insertions(+)
   create mode 100644 Documentation/acpi/properties.txt
  
  diff --git a/Documentation/acpi/properties.txt 
  b/Documentation/acpi/properties.txt
  new file mode 100644
  index ..a1e93267c1fa
  --- /dev/null
  +++ b/Documentation/acpi/properties.txt
  @@ -0,0 +1,359 @@
  +ACPI device properties
  +==
  +This document describes the format and interfaces of ACPI device
  +properties as specified in Device Properties UUID For _DSD available
  +here:
  +
  +http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
  +
  +1. Introduction
  +---
  +In systems that use ACPI and want to take advantage of device specific
  +properties, there needs to be a standard way to return and extract
  +name-value pairs for a given ACPI device.
  +
  +An ACPI device that wants to export its properties must implement a
  +static name called _DSD that takes no arguments and returns a package of
  +packages:
  +
  +   Name (_DSD, Package () {
  +   ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
  +   Package () {
  +   Package () {name1, VALUE1},
  +   Package () {name2, VALUE2}
  +   }
  +   })
  +
  +The UUID identifies contents of the following package. In case of ACPI
  +device properties it is daffd814-6eba-4d8c-8a91-bc9bbf4aa301.
  +
  +In each returned package, the first item is the name and must be a string.
  +The corresponding value can be a string, integer, reference, or package. If
  +a package it may only contain strings, integers, and references.
  +
  +An example device where we might need properties is a GPIO keys device.
  +In addition to the GpioIo/GpioInt resources the driver needs to know how
  +to map each GpioIo resource to the corresponding Linux input event.
  +
  +To solve this we add the following ACPI device properties from the
  +gpio-keys schema:
  +
  +   Device (KEYS) {
  +   Name (_DSD, Package () {
  +   ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
  +   Package () {
  +   Package () {poll-interval, 100},
  +   Package () {autorepeat, 1}
  +   }
  +   })
  +
  +   Device (BTN0) {
  +   Name (_DSD, Package () {
  +   
  ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
  +   Package () {
  +   Package () {linux,code, 105},
  +   Package () {linux,input-type, 1},
 
 The leakage of Linux implementation details into DTBs is bad enough. I
 would really not like to see this kind of thing in ACPI tables.
 
 Leaking this into HW description of any sort completely rids said
 description of any OS independence, and it's usually short-sighted,
 nonsensical, or ill-defined (what does 'autorepeat' actually mean?).
 
 I am very worried by the prospect of _DSD making it harder rather than
 easier for an arbitrary OS to use ACPI data due to assumptions being
 made about (a particular revision of) a particular OS.

You are right - the above example is bad in that sense. The reason why
we have the above is that we tested the implementation on Minnowboard
and the two devices we converted used these descriptions.

We should pick a better example that describes the HW independent of the
OS.

 
  +   ...
  +   }
  +   })
  +   }
  +
  +   ...
  +   }
  +
  +Of course the device driver then needs to iterate over these devices but
  +it can be done easily via the -children field of the companion ACPI
  +device. This will be demonstrated later on in the document.
  +
  +If there is an existing Device Tree binding for a device, it is expected
  +that the same bindings are used with ACPI properties, so that the driver
  +dealing with the device needs only minor modifications if any.
 
 For simple devices like a UART which just has a clock-frequency
 property in DT, importing both the concept of the property and the key
 name itself is a sensible approach.
 
 However, I do not think this makes 

Re: [RFC PATCH 2/9] ACPI: Document ACPI device specific properties

2014-08-18 Thread Darren Hart
On Mon, Aug 18, 2014 at 11:54:29AM +0100, Mark Rutland wrote:
 Hi Mika,
 
 While I am very much in favour of having a structured way of describing
 device specific data in ACPI I am very concerned by the idea of assuming
 (a false) equivalence with DT. More on that below.

Hi Mark,

The equivalence is intended in the sense that we should be able to represent any
of the existing schemas with the same properties in ACPI as in DT.

...

  +An example device where we might need properties is a GPIO keys device.
  +In addition to the GpioIo/GpioInt resources the driver needs to know how
  +to map each GpioIo resource to the corresponding Linux input event.
  +
  +To solve this we add the following ACPI device properties from the
  +gpio-keys schema:
  +
  +   Device (KEYS) {
  +   Name (_DSD, Package () {
  +   ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
  +   Package () {
  +   Package () {poll-interval, 100},
  +   Package () {autorepeat, 1}
  +   }
  +   })
  +
  +   Device (BTN0) {
  +   Name (_DSD, Package () {
  +   
  ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
  +   Package () {
  +   Package () {linux,code, 105},
  +   Package () {linux,input-type, 1},
 
 The leakage of Linux implementation details into DTBs is bad enough. I
 would really not like to see this kind of thing in ACPI tables.
 
 Leaking this into HW description of any sort completely rids said
 description of any OS independence, and it's usually short-sighted,
 nonsensical, or ill-defined (what does 'autorepeat' actually mean?).
 
 I am very worried by the prospect of _DSD making it harder rather than
 easier for an arbitrary OS to use ACPI data due to assumptions being
 made about (a particular revision of) a particular OS.

This is perhaps not the best example to include in the documentation. A guiding
principle has been to only represent in the DSD that which is an actual property
of the hardware.

It is also a goal of the mechanism to enable ACPI systems to reuse the 200+
existing OF-aware drivers in the kernel today. If those schemas are
ill-conceived, we should work to improve the schemas.

On the point of OS-independence, while I agree that using something like the USB
keycodes here would be better, the namespacing employed in these properties
makes it possible to use the same DSD for muliple OSes as another OS could add
it's own keycode, for example, without conflict. Again, that is really an issue
with the schemas, not the _DSD mechanism itself.

Perhaps we should use a different example in the documentation as I would agree
this is not a model schema.

  +Of course the device driver then needs to iterate over these devices but
  +it can be done easily via the -children field of the companion ACPI
  +device. This will be demonstrated later on in the document.
  +
  +If there is an existing Device Tree binding for a device, it is expected
  +that the same bindings are used with ACPI properties, so that the driver
  +dealing with the device needs only minor modifications if any.
 
 For simple devices like a UART which just has a clock-frequency
 property in DT, importing both the concept of the property and the key
 name itself is a sensible approach.
 
 However, I do not think this makes sense as a general design rule. ACPI
 and DT already have different idioms for describing certain things (e.g.
 GPIOs, interrupts) and care needs to be taken to distinguish simple
 device-specific properties from class-specific properties or implied
 linkages between devices.

I believe we have GPIO handled well in this series. In DT terms, ACPI is the
controller, providing a list of GpioIO|GpioInt resources in the CRS of the
referenced object. Arguments are Resource Index, Pin Index, Polarity. The rest
of the properties are handled via the GpioIO|GpioInt resources in the _CRS.

 
 The #clock-cells and #pwm-cells _DSD examples in ACPI 5.1 are
 complete nonsense. They import an idiom from DT into ACPI without

Yeah... coming up with good examples for a generic mechanism which are both
descriptive and universally unobjectionable. Good point. We might be able to
improve this via an errata update. What would you recommend?

 considering the surrounding context (cells, lack of typing, phandles,

How are the phandles not covered via the ACPI namespace references?

The point on cells is a good one, and I believe some of this can be mitigated by
providing subsystem specific accessors as we have for GPIO, for example.

 etc). As an example, I worry that why it may be trivial to map pinctrl
 bindings into ACPI _DSD, there is no clear way that they would interact
 with ACPI's HW management. This is going to significantly expand our
 problem surface area.


[RFC PATCH 2/9] ACPI: Document ACPI device specific properties

2014-08-17 Thread Mika Westerberg
This document describes the data format and interfaces of ACPI device
specific properties.

Signed-off-by: Mika Westerberg 
Signed-off-by: Darren Hart 
---
 Documentation/acpi/properties.txt | 359 ++
 1 file changed, 359 insertions(+)
 create mode 100644 Documentation/acpi/properties.txt

diff --git a/Documentation/acpi/properties.txt 
b/Documentation/acpi/properties.txt
new file mode 100644
index ..a1e93267c1fa
--- /dev/null
+++ b/Documentation/acpi/properties.txt
@@ -0,0 +1,359 @@
+ACPI device properties
+==
+This document describes the format and interfaces of ACPI device
+properties as specified in "Device Properties UUID For _DSD" available
+here:
+
+http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
+
+1. Introduction
+---
+In systems that use ACPI and want to take advantage of device specific
+properties, there needs to be a standard way to return and extract
+name-value pairs for a given ACPI device.
+
+An ACPI device that wants to export its properties must implement a
+static name called _DSD that takes no arguments and returns a package of
+packages:
+
+   Name (_DSD, Package () {
+   ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+   Package () {
+   Package () {"name1", },
+   Package () {"name2", }
+   }
+   })
+
+The UUID identifies contents of the following package. In case of ACPI
+device properties it is daffd814-6eba-4d8c-8a91-bc9bbf4aa301.
+
+In each returned package, the first item is the name and must be a string.
+The corresponding value can be a string, integer, reference, or package. If
+a package it may only contain strings, integers, and references.
+
+An example device where we might need properties is a GPIO keys device.
+In addition to the GpioIo/GpioInt resources the driver needs to know how
+to map each GpioIo resource to the corresponding Linux input event.
+
+To solve this we add the following ACPI device properties from the
+gpio-keys schema:
+
+   Device (KEYS) {
+   Name (_DSD, Package () {
+   ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+   Package () {
+   Package () {"poll-interval", 100},
+   Package () {"autorepeat", 1}
+   }
+   })
+
+   Device (BTN0) {
+   Name (_DSD, Package () {
+   ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+   Package () {
+   Package () {"linux,code", 105},
+   Package () {"linux,input-type", 1},
+   ...
+   }
+   })
+   }
+
+   ...
+   }
+
+Of course the device driver then needs to iterate over these devices but
+it can be done easily via the ->children field of the companion ACPI
+device. This will be demonstrated later on in the document.
+
+If there is an existing Device Tree binding for a device, it is expected
+that the same bindings are used with ACPI properties, so that the driver
+dealing with the device needs only minor modifications if any.
+
+2. Formal definition of properties
+--
+The following chapters define the currently supported properties. For
+these there exists a helper function that can be used to extract the
+property value.
+
+2.1 Integer types
+-
+ACPI integers are always 64-bit. However, for drivers the full range is
+typically not needed so we provide a set of functions which convert the
+64-bit integer to a smaller Linux integer type.
+
+An integer property looks like this:
+
+   Package () {"poll-interval", 100},
+   Package () {"i2c-sda-hold-time-ns", 300},
+   Package () {"clock-frequency", 40},
+
+To read a property value, use a unified property accessor as shown
+below:
+
+   u32 val;
+   int ret;
+
+   ret = device_property_read(dev, "poll-interval", DEV_PROP_U32, );
+   if (ret)
+   /* Handle error */
+
+The function returns 0 if the property is copied to 'val' or negative
+errno if something went wrong (or the property does not exist).
+
+2.2 Integer arrays
+--
+An integer array is a package holding only integers. Arrays can be used to
+represent different things like Linux input key codes to GPIO mappings, pin
+control settings, dma request lines, etc.
+
+An integer array looks like this:
+
+   Package () {
+   "max8952,dvs-mode-microvolt",
+   Package () {
+   125,
+   120,
+   105,
+   95,
+   }
+   }
+
+The above array property can be accessed like:

[RFC PATCH 2/9] ACPI: Document ACPI device specific properties

2014-08-17 Thread Mika Westerberg
This document describes the data format and interfaces of ACPI device
specific properties.

Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Signed-off-by: Darren Hart dvh...@linux.intel.com
---
 Documentation/acpi/properties.txt | 359 ++
 1 file changed, 359 insertions(+)
 create mode 100644 Documentation/acpi/properties.txt

diff --git a/Documentation/acpi/properties.txt 
b/Documentation/acpi/properties.txt
new file mode 100644
index ..a1e93267c1fa
--- /dev/null
+++ b/Documentation/acpi/properties.txt
@@ -0,0 +1,359 @@
+ACPI device properties
+==
+This document describes the format and interfaces of ACPI device
+properties as specified in Device Properties UUID For _DSD available
+here:
+
+http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
+
+1. Introduction
+---
+In systems that use ACPI and want to take advantage of device specific
+properties, there needs to be a standard way to return and extract
+name-value pairs for a given ACPI device.
+
+An ACPI device that wants to export its properties must implement a
+static name called _DSD that takes no arguments and returns a package of
+packages:
+
+   Name (_DSD, Package () {
+   ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
+   Package () {
+   Package () {name1, VALUE1},
+   Package () {name2, VALUE2}
+   }
+   })
+
+The UUID identifies contents of the following package. In case of ACPI
+device properties it is daffd814-6eba-4d8c-8a91-bc9bbf4aa301.
+
+In each returned package, the first item is the name and must be a string.
+The corresponding value can be a string, integer, reference, or package. If
+a package it may only contain strings, integers, and references.
+
+An example device where we might need properties is a GPIO keys device.
+In addition to the GpioIo/GpioInt resources the driver needs to know how
+to map each GpioIo resource to the corresponding Linux input event.
+
+To solve this we add the following ACPI device properties from the
+gpio-keys schema:
+
+   Device (KEYS) {
+   Name (_DSD, Package () {
+   ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
+   Package () {
+   Package () {poll-interval, 100},
+   Package () {autorepeat, 1}
+   }
+   })
+
+   Device (BTN0) {
+   Name (_DSD, Package () {
+   ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
+   Package () {
+   Package () {linux,code, 105},
+   Package () {linux,input-type, 1},
+   ...
+   }
+   })
+   }
+
+   ...
+   }
+
+Of course the device driver then needs to iterate over these devices but
+it can be done easily via the -children field of the companion ACPI
+device. This will be demonstrated later on in the document.
+
+If there is an existing Device Tree binding for a device, it is expected
+that the same bindings are used with ACPI properties, so that the driver
+dealing with the device needs only minor modifications if any.
+
+2. Formal definition of properties
+--
+The following chapters define the currently supported properties. For
+these there exists a helper function that can be used to extract the
+property value.
+
+2.1 Integer types
+-
+ACPI integers are always 64-bit. However, for drivers the full range is
+typically not needed so we provide a set of functions which convert the
+64-bit integer to a smaller Linux integer type.
+
+An integer property looks like this:
+
+   Package () {poll-interval, 100},
+   Package () {i2c-sda-hold-time-ns, 300},
+   Package () {clock-frequency, 40},
+
+To read a property value, use a unified property accessor as shown
+below:
+
+   u32 val;
+   int ret;
+
+   ret = device_property_read(dev, poll-interval, DEV_PROP_U32, val);
+   if (ret)
+   /* Handle error */
+
+The function returns 0 if the property is copied to 'val' or negative
+errno if something went wrong (or the property does not exist).
+
+2.2 Integer arrays
+--
+An integer array is a package holding only integers. Arrays can be used to
+represent different things like Linux input key codes to GPIO mappings, pin
+control settings, dma request lines, etc.
+
+An integer array looks like this:
+
+   Package () {
+   max8952,dvs-mode-microvolt,
+   Package () {
+   125,
+   120,
+   105,
+   95,
+   }
+   }
+
+The above