Re: [PATCH v4 00/13] Add ACPI _DSD and unified device properties support

David Woodhouse
2014-10-15T13:28:56Z

[-- Attachment #1: Type: text/plain, Size: 1905 bytes --]

On Wed, 2014-10-15 at 14:15 +0100, Mark Rutland wrote:
> > @@ -155,7 +168,7 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
> > if (!match)
> > return -EINVAL;
> >
> > - if (of_find_property(ofdev->dev.of_node, "used-by-rtas", NULL))
> > + if (!device_get_property(&ofdev->dev, "used-by-rtas", NULL))
> > return -EBUSY;
>
> This property should never be present on an ACPI system. RTAS is a
> completely different firmware interface on PowerPC.

Yes, I sincerely hope we never see used-by-rtas being set on a non-PPC
system. But this isn't a new consideration; we were already checking for
'used-by-rtas' on *all* platforms. Perhaps we shouldn't be. But that's
almost orthogonal to the issue at hand.

> As a general note, I would hope that we're not going to blindly convert
> drivers and subsystems over to a common property interface without
> considering each property w.r.t. the particular FW interface.
>
> Each addition to _DSD, especially if through a common accessor needs
> _more_ scrutiny than is applied to DT bindings, and we hardly manage to
> review DT bindings.

The whole point here is to use existing bindings rather than having to
reinvent the wheel. Sure, where the existing binding really makes no
sense for certain subsystems, we should come up with something
different.

But in the general case for 'leaf-node' peripherals we would hope that
we don't really have to change *anything* other than to make sure the
driver is using generic property accessor functions instead of the old
OF-specific ones. The point here is *consistency*. We really don't want
to make a habit of reinventing different bindings to be exposed through
the different firmware types.

--
David Woodhouse Open Source Technology Centre
David.Woodhouse@intel.com Intel Corporation

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5745 bytes --]