On Wed, 31 Jul 2013 16:56:08 +0100 Mark Rutland <mark.rutland@xxxxxxx> wrote: > On Tue, Jul 30, 2013 at 06:59:01PM +0100, Stephen Warren wrote: > > On 07/30/2013 10:22 AM, Olof Johansson wrote: > > > On Tue, Jul 30, 2013 at 03:18:35PM +0400, Alexander Shiyan wrote: > > >> This patch adds DT support for generic (MMIO) GPIO driver. > > > > >> diff --git a/Documentation/devicetree/bindings/gpio/gpio-generic.txt b/Documentation/devicetree/bindings/gpio/gpio-generic.txt > > > > >> +Generic memory-mapped GPIO controller > > >> + > > >> +Required properties: > > >> +- compatible: Should be "basic-mmio-gpio" or "basic-mmio-gpio-be". > > > > I think naming this "simple-gpio" would match other simple/basic/generic > > bindings better. > > > > >> +- reg: Physical base GPIO controller registers location and length. > > >> +- reg-names: Should be the names of reg resources. Each register uses > > >> + its own reg name, so there should be as many reg names as referenced > > >> + registers: > > >> + "dat" : Input/output register (Required), > > >> + "set" : Register for set output bits (Optional), > > >> + "clr" : Register for clear output bits (Optional), > > >> + "dirout" : Register for setup direction as output (Optional), > > >> + "dirin" : Register for setup direction as input (Optional). > > >> +- gpio-controller: Marks the device node as a gpio controller. > > >> +- #gpio-cells: Should be two. > > ... > > > > > > I'm trying to figure out what to say about this binding. It's not really > > > describing hardware, instead it's strongly tied to how the basic-mmio-gpio > > > driver expects the platform data to look. > > > > I'm not sure; the binding seems to only contain a direct representation > > of pure HW properties; the addresses/offsets of various registers in the > > HW block. > > > > > It makes more sense to actually describe the hardware in question, > > > and then have the driver handle that as expected. I.e. either have a > > > small conversion layer that binds to the actual hardware compatible > > > value and registers a basic-mmio-gpio device from there, or extend the > > > basic-mmio-gpio driver to do it by itself. > > > > Ah, I guess the question more: Do we want generic bindings that describe > > the low-level details of the HW in a completely generic fashion so that > > new HW can be supported with zero kernel changes, or do we want a simple > > driver with a lookup table that maps from compatible value to the HW > > configuration? One of the potential benefits of DT is to be able to > > support new HW without code changes, although perhaps that's more > > typically considered in the context of new boards rather than new IP > > blocks or SoCs. > > I think that going forward it would be better to have have a compatible > string per different device. As Olof pointed out, we're leaking the way > we currently handle things in Linux into the binding, rather than > precisely describing the hardware (with a unique compatible string). I'm > not sure if this is much better than embedding a bytecode describing how > to poke devices. > > Certainly there should be more-specific bindings for each device, so we > can later improve support for them. If we have that anyway, I think it > would be nicer to have the mapping from that compatible string to some > internal data passed to the simple-gpio driver, rather than explicitly > stating that the current version of the Linux simple-gpio driver is > capable of driving the device. > > I think the issue is that we're describing a hardware block in general, > rather than the instance of the hardware block, and that limits how > flexibly we can use the data in the description. A lot of people - a lot of opinions. So, what the final resolution for this? -- Alexander Shiyan <shc_work@xxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
