From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTP id E79054C6 for ; Sat, 17 May 2014 03:41:04 +0000 (UTC) Received: from mail-pb0-f54.google.com (mail-pb0-f54.google.com [209.85.160.54]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 0A4D61F977 for ; Sat, 17 May 2014 03:41:03 +0000 (UTC) Received: by mail-pb0-f54.google.com with SMTP id jt11so3408314pbb.13 for ; Fri, 16 May 2014 20:41:03 -0700 (PDT) Sender: Darren Hart Message-ID: <1400296973.9575.114.camel@wasp-deb.dvhart.com> From: Darren Vincent Hart To: "Rafael J. Wysocki" Date: Fri, 16 May 2014 20:22:53 -0700 In-Reply-To: <6339662.3mIpLKDMAO@vostro.rjw.lan> References: <1490957.7A98jlv5Y2@vostro.rjw.lan> <6339662.3mIpLKDMAO@vostro.rjw.lan> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Cc: Greg Kroah-Hartman , "ksummit-discuss@lists.linuxfoundation.org" Subject: Re: [Ksummit-discuss] [TECH TOPIC] Driver model/resources, ACPI, DT, etc (sigh) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Tue, 2014-05-13 at 23:51 +0200, Rafael J. Wysocki wrote: > On Tuesday, May 13, 2014 02:28:05 PM Olof Johansson wrote: > > On Tue, May 13, 2014 at 2:40 PM, Rafael J. Wysocki wrote: > > > On Tuesday, May 13, 2014 02:14:42 PM Olof Johansson wrote: > > >> On Tue, May 06, 2014 at 03:35:41PM +0200, Arnd Bergmann wrote: > > >> > On Tuesday 06 May 2014 14:18:56 Rafael J. Wysocki wrote: > > >> > > On Monday, May 05, 2014 09:31:04 PM Benjamin Herrenschmidt wrote: > > >> > > > On Mon, 2014-05-05 at 13:32 +0200, Rafael J. Wysocki wrote: > > >> > > > > The interpretation - yes, the translation - not necessarily. By putting > > >> > > > > the translation part into drivers we'll duplicate a lot of code used for > > >> > > > > that. In my opinion there needs to be a layer doing the translation > > >> > > > > automatically. > > >> > > > > > >> > > > I'm afraid I don't understand. How can you do the translation > > >> > > > automatically if you have fundamentally different representation of > > >> > > > device-specific concepts ? > > >> > > > > >> > > I'm not entirely sure how fundamentally different it is to be honest, but in > > >> > > any case I'm talking about the situation in which the hardware is the same > > >> > > and the driver is the same, only the firmware interface is different. In > > >> > > that case either the firmware supplies the information the driver needs, or > > >> > > it doesn't (and as long as the information is supplied, the format doesn't > > >> > > really matter, because it only is a matter of translation). > > >> > > > >> > One example for something that is fundamentally different between an > > >> > embedded system using DT and any ACPI system is how a PCI(e) host bridge > > >> > gets treated, we are currently having long discussions about this elsewhere. > > >> > > > >> > With ACPI (or a server running OF for that matter), you assume that the > > >> > PCI host is operational and all buses are fully probed and then you > > >> > have methods for doing hotplugging, you just look at the device that > > >> > are already initialized by the firmware. > > >> > > > >> > With embedded systems, you (in general, some steps may get skipped > > >> > in practice) go through a lot more steps: > > >> > - detect the presence of the host bridge device > > >> > - enable clocks, voltages, phys, resets, etc. > > >> > - set up inbound and outbound MMIO and PIO address translation windows > > >> > - perform a full bus scan and assign bus and device resources > > >> > > > >> > We probably woulnd't go through that hassle on ACPI systems. I think > > >> > we shouldn't attempt any translation here, and keep the code completely > > >> > separate, with the common parts being handled by the PCI core as we > > >> > do today. > > >> > > > >> > An example about a subsystem where translate complex information > > >> > from both DT and ACPI into a common format is the dmaengine: We can > > >> > have references to dma channels in completely incompatible ways > > >> > but still use the same dma_request_slave_channel() interface for > > >> > both. The ACPI path currently is lacking a bit (e.g. there is no > > >> > way to specify the channel name in ACPI, so the code makes > > >> > assumptions that don't hold true in general), but that's fixable. > > >> > > > >> > I assume we will see more of the second kind in the future, and this > > >> > would be limited to embedded systems on both ACPI and DT, since you > > >> > shouldn't see dma slave devices on general purpose systems with > > >> > either. > > >> > > > >> > > Bottom line: I'd like to avoid modifying drivers that use the of_ interface > > >> > > today if possible. Instead, I'd prefer to modify the firmware layer so that > > >> > > those drivers can get what they ask for regardless of what firmware interface > > >> > > the platform is using. > > >> > > > >> > Looking at the most trivial example: > > >> > > > >> > bool of_property_read_bool(const struct device_node *np, const char *propname); > > >> > > > >> > We should definitely be able to have an interface like this for > > >> > ACPI, but you don't have a 'struct device_node' pointer then. > > >> > We can add a new > > >> > > > >> > bool dev_property_read_bool(const struct device *dev, > > >> > const char *propname); > > >> > > >> > > > >> > that handles both, but then you have to change every driver > > >> > that is already using of_property_read_bool() and that can > > >> > get used with ACPI. > > >> > > >> It's actually not that easy in practice. There's not a one-to-one > > >> correspondence between device tree nodes and struct devices in the > > >> kernel. There are many bindings that use subnodes for sections of their > > >> information, and so on. > > >> > > >> > An advantage of the dev_property_read_bool() > > >> > interface however is that it would also work for platform > > >> > devices that are created in platform code using > > >> > platform_device_register_full() or similar if we add a way > > >> > to define simple properties in C code. That would also simplify > > >> > a lot of drivers that use both (simple) platform_data and DT > > >> > today. > > >> > > >> I'm not a fan of this "grand unified abstraction layer". Driver writes are > > >> already complaining that things are too complicated with the move to device > > >> tree, and we're making things even more abstracted and complicated here. People > > >> will need to peel back even more layers to figure out what's actually going on. > > >> > > >> > > >> For the truly trivial stuff, there are some quite simple ways to handle this: > > >> > > >> 1. Make sure that anything that can be a named IORESOURCE_* is so. Which could > > >> mean moving over GPIO and possibly clocks to that, and use those for lookup. > > >> > > >> 2. If Rafael's arguments are to be believed, that the translation should be > > >> trivial and easy to do, then we might as well reuse the structures and APIs > > >> that are already in the kernel. In other words, allocate a struct > > >> device_node, populate it with the properties from the ACPI structure, and > > >> attach it to the struct device. Then there's no need for any churn in the > > >> drivers (until they need ACPI-specific tweaks, which they will all need). > > > > > > The problem with that is that we have either device_node or acpi_node.companion > > > under struct device and they are mutually exclusive. So this isn't going to fly. > > > [And it doesn't make sense to duplicate information already available from > > > acpi_node.companion this way IMHO.] > > > > > > Also device references are problematic here, because both interfaces use > > > different ways to do that (phandles vs object paths in the ACPI namespace). > > > > > > So actually, what Arnd proposed seems to be the simplest approach. > > > > Again, that suggestion assumes a 1:1 mapping between device nodes and > > struct devices, which does not exist. > > I suppose your concern is that there may be device nodes without struct device, > right? So drivers may need to access those device nodes directly? > > Can you please point me to code where this is the case? gpio-keys.c is a good example, and the one I chose for the MinnowBoard example last year. This is a really good point. My thinking was that we iterate through the devices - but I had my ACPI glasses on at the time and wasn't thinking about how that iteration would work for DT in with the properties abstraction. The ACPI sub-devices do have something in common with the DT sub-nodes, they are not themselves "compatible" with a specific driver, nor are the known by the acpi_platform match table. So in both cases we have effectively a hierarchy of properties. Could we not support such a concept in the properties interface? The basic signature might be something more like: device_get_property(dev, NULL, "foo") // for the top level properties We could have some kind of a foreach iterator for children, and device_get_property(dev, child, "baz") Thoughts? -- Darren Hart Intel Open Source Technology Center