Antwort: Re: Re: Re: [PATCH v3 13/29] dts: Add a binding for hid-over-i2c

Wed Apr 15 17:17:31 CEST 2020

On Wed, Apr 15, 2020 at 06:15:39PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 15, 2020 at 04:57:33PM +0200, Wolfgang Wallner wrote:
> > > On Wed, Apr 15, 2020 at 5:00 PM Wolfgang Wallner
> > > <wolfgang.wallner at br-automation.com> wrote:
> > > > -----"Andy Shevchenko" <andy.shevchenko at gmail.com> schrieb: -----
> > > > > Betreff: Re: Re: [PATCH v3 13/29] dts: Add a binding for hid-over-i2c
> > > > >
> > > > > On Wed, Apr 8, 2020 at 11:40 PM Andy Shevchenko
> > > > > <andy.shevchenko at gmail.com> wrote:
> > > > > > On Wed, Apr 8, 2020 at 10:39 PM Wolfgang Wallner
> > > > > > <wolfgang.wallner at br-automation.com> wrote:
> > > > > > > > On Tue, Apr 07, 2020 at 08:58:13PM -0600, Simon Glass wrote:
> > > > > > > > > On Tue, 31 Mar 2020 at 13:25, Wolfgang Wallner
> > > > > > > > > <wolfgang.wallner at br-automation.com> wrote:
> > > > > > > > > > >An: u-boot at lists.denx.de
> > > > > > > > > > >Von: "Simon Glass" <sjg at chromium.org>
> > > > > > > > > > >Datum: 31.03.2020 01:14
> > > > > > > > > > >Kopie: "Andy Shevchenko" <andriy.shevchenko at linux.intel.com>,
> > > > > > > > > > >"Wolfgang Wallner" <wolfgang.wallner at br-automation.com>, "Leif
> > > > > > > > > > >Lindholm" <leif at nuviainc.com>, "Simon Glass" <sjg at chromium.org>
> > > > > > > > > > >Betreff: [PATCH v3 14/29] acpi: Add a binding for ACPI settings in
> > > > > > > > > > >the device tree
> > > > > > > >
> > > > > > > > > > The _DSD-method for "PRP0001"-devices in ACPI allows to use Devicetree
> > > > > > > > > > properties inside ACPI, especially it allows to re-use Devicetree's
> > > > > > > > > > "compatible"-property. But this is for a different use case (using Devicetree
> > > > > > > > > > properties inside ACPI, not add ACPI properties in Devicetree).
> > > > > > > >
> > > > > > > > Before we are going further with this here is a BIG CAVEAT.
> > > > > > > >
> > > > > > > > PRP0001   MUST NOT be used in production devices.
> > > > > > > >
> > > > > > > > This has been derived solely for debugging / pre-production testing / etc
> > > > > > > > purposes. The real devices must have an official ACPI _HID.
> > > > > > >
> > > > > > > Thanks for pointing this out! I was not aware of this.
> > > > > > > I have tried to understand how the PRP0001 is meant to be used, but could not
> > > > > > > find sufficient documentation. The best document I could find is
> > > > > > > Documentation/firmware-guide/acpi/enumeration.rst in the Linux kernel, and
> > > > > > > as far as I can tell the constraint that you mention is also not described
> > > > > > > there.
> > > > > > >
> > > > > > > Do you know any other resources regarding PRP0001, e.g. some kind of
> > > > > > > speficiation?
> > > > > >
> > > > > > I guess the best one is to ask somebody from UEFI Forum / ASWG. PRP is
> > > > > > a PNP ID for UEFI Forum.
> > > > >
> > > > > Basically last message in [3] from Rafael mentions his view on
> > > > > PRP0001. I guess there is still no document, although as I noticed the
> > > > > PRP prefix become official in a mean time.
> > > >
> > > > Thanks for the references. I have read them carefully, especially the thread
> > > > in [3].
> > > >
> > > > My understanding up to now was based on presentations by David Woodhouse,
> > > > so it matches with his viewpoint in the thread at [3]. And as far as I can
> > > > tell Rafael Wysocki agrees with him in this thread.
> > > >
> > > > What I could not find in either of the references is that PRP0001 is only
> > > > for debugging, I only know about this constraint from your mail. Could you
> > > > point me to any source for this?
> > > 
> > > From [3], Rafael said:
> > > "Let alone the fact that PRP0001 is actually quite useful at the
> > > prototyping stage when it is premature to allocate a new device ID just
> > > yet.  Taking that to the extreme, if someone whittles a board in his or
> > > her garage and wants to use it to drive their homemade grass watering
> > > system, having to invent a new device ID and put it into the existing
> > > driver that otherwise doesn't require any modifications is ... you know
> > > what I mean."
> > > 
> > > It implies that the process should have included the allocation of an
> > > official ACPI ID.
> > 
> > I understand the quoted paragraph differently. In the paragraphs above Rafael
> > argues that PRP0001 is useful, as with PRP0001 we can avoid registering
> > redundant ACPI IDs. In the quoted paragraph he then gives another example
> > where PRP0001 is also useful: for debugging.
> > But I don't understand it in a way that PRP0001 would be limited to only
> > debugging.
> > 
> > A few posts before in [4] he explains in more detail, and I think it matches
> > my understanding of the last post, especially the following sentence:
> > 
> > "It would be a failure if people had to write separate drivers for 
> > ACPI-based and DT-based platforms to handle the same hardware, at least 
> > at the leaf driver level."
> > 
> 
> Maybe better to ask him directly?

I guess here is a root of misunderstanding, i.e. PRP0001 as _HID() vs. Device
Properties as _DSD() method.

I think you quoted the thoughts WRT latter, and not related to the _HID() part!

> > [4] https://patchwork.kernel.org/comment/15339311/
> > 
> > > You always can ask ASWG for the clarification. Maybe I learn something
> > > new about PRP0001 :-)
> > 
> > I had no interaction with ASWG before so I'm not sure what the process is,
> > but I will look it up.
> > 
> > > > > > > If PRP0001 is only for debugging, then I must also have misunderstood the
> > > > > > > Linux "device-property" API (define in include/linux/property.h).
> > > > > >
> > > > > > Not exactly.
> > > > > >
> > > > > > > There are some presentations available on the internet, e.g. [1], that I
> > > > > > > understand like PRP0001 + "device-property" API provide a way do access data
> > > > > > > from either Devicetree or ACPI, depending on what kind of platform you are on.
> > > > > >
> > > > > > No, these are not hard linked to each other (the relation is that
> > > > > > PRP0001 is a way to enumerate devices, which don't have dedicated ACPI
> > > > > > _HID, by using compatible property [1]). The _DSD per se (i.o.w.
> > > > > > device properties implementation in ACPI) is a different story [2].
> > > > > >
> > > > > > And I put [3] here, interesting to read. However, at that time I was
> > > > > > quite far from this topic.
> > > > > >
> > > > > > [1]: https://www.kernel.org/doc/html/latest/firmware-guide/acpi/enumeration.html#device-tree-namespace-link-device-id
> > > > > > [2]: https://uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel-1_2-3.htm.
> > > > > > [3]: https://patchwork.kernel.org/patch/7004241/
> > > > > >
> > > > > > > [1] https://elinux.org/images/2/2d/Device_tree_acpi_compatibility-david_woodhouse-kernel_recipes_2015.pdf
> > 
> > regards, Wolfgang
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

-- 
With Best Regards,
Andy Shevchenko